Skip to content

Conversation

@kalexand-rh
Copy link
Contributor

@wking, does this sound right to you?

@kalexand-rh kalexand-rh added this to the Future Release milestone Sep 26, 2019
@kalexand-rh kalexand-rh self-assigned this Sep 26, 2019
@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 26, 2019
@openshift-docs-preview-bot

The preview will be available shortly at:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major (e.g. 4.9.10->5.0.0), minor (e.g. 4.1.9->4.2.3), and patch (e.g. 4.1.9->4.1.11)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And in case we are disagreeing on terms, my major.minor.patch terms are from https://semver.org/spec/v2.0.0.html#summary

Copy link
Contributor Author

@kalexand-rh kalexand-rh Sep 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your terminology is better than mine. I'll update it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is {major.minor_version} a thing? Would be nice if you didn't have to bump this manually, but could get it automatically expanded to match whatever flavor the docs were being built for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not. I think that it's a good idea though, so I'm going to bring it through our internal change process.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can tell your cluster to upgrade to whatever version using oc adm upgrade .... But telling it to take upgrades that aren't in our official Cincinnati graph (e.g. https://api.openshift.com/api/upgrades_info/v1/graph?channel=stable-4.1 ) may be unsupported. I'm not clear on the official support policy in this space; maybe @crawford knows?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unsupported. We really need to teach the CVO to do local validation as well so it's less easy to make that mistake.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really need to teach the CVO to do local validation as well so it's less easy to make that mistake.

The CVO may already refuse non graph upgrades without --force. But I was previously not clear on whether that override voided support for the cluster. Even more reason to track edge removal in Git ;).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll clarify that you can only update within the current minor version, but I'm not going to mention that they can make an unsupported update by using --force.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll clarify that you can only update within the current minor version...

Until we start recommending 4.1->4.2 upgrades.

... but I'm not going to mention that they can make an unsupported update by using --force.

But disconnected folks will have to do this, because the CVO cannot pull their signatures to validate them.

On the other hand, without --force it should be impossible to make an unsupported upgrade.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll clarify that you can only update within the current minor version...

Until we start recommending 4.1->4.2 upgrades.

Clayton said that the best user experience was to upgrade 4.1 > 4.2 updates from the web console, so that's the only path I'm going to document.

... but I'm not going to mention that they can make an unsupported update by using --force.

But disconnected folks will have to do this, because the CVO cannot pull their signatures to validate them.

On the other hand, without --force it should be impossible to make an unsupported upgrade.

The disconnected update path will have its own separate topic after all my in-flight PRs merge.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without --force, you have the same choices as in the web UI, and you can see those choices with oc adm upgrade. From oc adm upgrade --help:

If no arguments are passed the command will retrieve the current version info and display whether an upgrade is in progress or whether any errors might prevent an upgrade, as well as show the suggested updates available to the cluster.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did Clayton change his mind about the web UI being the best user experience for updates between versions? I thought that you couldn't change the channel.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did Clayton change his mind about the web UI being the best user experience for updates between versions?

I doubt it ;).

I thought that you couldn't change the channel.

You can oc patch ... like the examples here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UI is the best user experience because the cli doesn't have the tools to list/show you acceptable versions (maybe we should fix that)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that might be a good RFE. Along with the ability to easily retrieve and update the channels in the graph.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UI is the best user experience because the cli doesn't have the tools to list/show you acceptable versions...

oc adm upgrade should list recommend targets from the graph (via ClusterVersion.status`).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[kalexand@localhost ~]$ oc adm upgrade
Cluster version is 4.1.20

No updates available. You may force an upgrade to a specific release image, but doing so may not be supported and result in downtime or data loss.

When is update going to be working as intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am happy to add the appropriate update docs as soon as we can identify the best user path and attempt to validate it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is update going to be working as intended?

No recommended upgrades from 4.1.20 is appropriate now. What targets where you expecting to see? You can test the oc functionality by running against 4.1.18, which should show 4.1.20 if you are in stable-4.1 or prerelease-4.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, then we're still stuck because the question's about updating between minor versions not within a minor version.

@kalexand-rh
Copy link
Contributor Author

@jiajliu, will you PTAL?

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 9, 2019
@kalexand-rh
Copy link
Contributor Author

@wking, will you PTAL at the latest commit? I addressed some of your feedback in Slack and added a topic about updating between minor versions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dunno about rhel---7; when I visit https://access.redhat.com/downloads/content/290/ver=4.1/, I'm redirected to https://access.redhat.com/downloads/content/290/ver=4.1/rhel---8/4.1.18/x86_64/product-software .

Also, I'm not wild about pinning to 4.2.0, will that only show errata fixed by 4.2.0->4.2.1? Or will it show 4.2.0->4.2.{latest}?

And if we do pin to something in 4.2.z, you'll want ver=4.2, not ver=4.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The errata page shows all of the changes for the minor release.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One hop does not guarantee you get to a good jumping-off point. For example, the current stable-4.1 graph is:

$ curl -sH 'Accept:application/json' 'https://api.openshift.com/api/upgrades_info/v1/graph?channel=stable-4.1' | ~/src/openshift/cincinnati/hack/graph.sh | dot -Tpng >graph.png

using graph.sh to generate:

graph

With 4.1.18 as a jumping-off point (as we're currently planning), folks on 4.2.0 would have to go 4.2.0->4.2.14->4.1.18 or similar. Anyone still running the guarded 4.1.10 would have to go 4.1.10->4.1.11->4.1.18 or similar. We probably want to say "and keep upgrading until there are no recommended upgrades left" somewhere for this use-case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better way to get that graph and see what updates are available? "and keep upgrading until there are no recommended upgrades left" isn't the best user experience.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not at the moment, but maybe in the mid-term. Eventual UX will be "turn on auto-upgrades, set your target channel, and we'll route you through the graph".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When that's about to happen, please let me know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"...and wait until the cluster-version operator refreshes the upgrade graph."? It would be nice if we were watching the ClusterVersion object and triggering a fresh fetch when the channel changed (and maybe we are), but even then, it might take some time to pull the graph, or the network could be down, or whatever. I think it's worth saying something to hint that into the reader's conciousness even if we don't take a big dive into all possible complications ;).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence is not between-specific. Maybe it's best to leave the between-minor docs out of this module so it can stay focused on executing a single upgrade, and then have a different module or leading section on between-minors that says "to upgrade between minors, upgrade to the most recent release in your current channel by repeating {the upgrade proceedure} until you no longer have any available updates. Then change your channel to the new stable-* and perform the available update." Or something...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How's this?

@kalexand-rh
Copy link
Contributor Author

@jiajliu, will you PTAL?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can i understand this "user can only upgrade to a minor version through web console"? why not cli supported in minor version upgrade?
and for a 4.1.z version, how user can upgrade the cluster to 4.2? i ask this because i did not see any available update for both 4.1.18 and 4.1.20 when set channel to stable-4.2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've gotten conflicting messages about whether or not it's required to use --force from the CLI. After update between 4.1 and 4.2 is working as intended, I'm happy to revisit.

There's an ongoing discussion about how the update from 4.1.z to 4.2 is supposed to work on the sbr-shift-list, and I've asked Eric to let me know when there's a plan to move forward.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want customers forcing upgrades, we want them using the graphs we provide.
That said we need to document to customers how to use the force option, and clearly explain that its not a recommended decision to do this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Customers can update from cli or web console, the web console (or our Cincinnati graph are the ways you know what acceptable upgrades are).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to add some conceptual material and warnings about the existence of the --force option and emphasize the need to use one of the supported options in the graph. We're only adding steps that we actually want users to take. (If I add steps about how to use the --force option, you know people are going to follow them.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can i understand this "user can only upgrade to a minor version through web console"? why not cli supported in minor version upgrade?
and for a 4.1.z version, how user can upgrade the cluster to 4.2? i ask this because i did not see any available update for both 4.1.18 and 4.1.20 when set channel to stable-4.2?

@jiajliu, Trevor and I talked about this for awhile. It's because you need to change channel from the stable-4.1 to a 4.2 channel to upgrade, and there's not a good way to do that in the CLI. Instead of changing channel in the UI and then swapping to the CLI to finish, we're just staying in the UI.

_topic_map.yml Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this definition about minor version and within a minor version need to be unified.
in v4.1(https://docs.openshift.com/container-platform/4.1/updating/updating-cluster.html), minor version upgrade means 4.1.0-4.1.2,
but in this pr, seems minor version upgrade refer to 4.1-4.2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll open a separate PR to fix that language in 4.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the 4.1 only PR to change these titles: #17631

@jiajliu
Copy link

jiajliu commented Oct 16, 2019

@jiajliu, will you PTAL?

@kalexand-rh sorry for missing your msg, just add two comments in the pr.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 17, 2019
@openshift-ci-robot openshift-ci-robot removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 25, 2019
@kalexand-rh
Copy link
Contributor Author

@jiajliu, will you PTAL? Engineering wants this change to go live on Tuesday, and I'd like to address any of your concerns on Monday, if possible.

@kalexand-rh
Copy link
Contributor Author

@jiajliu, will you PTAL? I think that this content will be valid with tomorrow's errata release, and I want to get it live as soon as possible.

@kalexand-rh kalexand-rh force-pushed the update-4.2 branch 2 times, most recently from d7679aa to e621425 Compare October 30, 2019 19:04
@kalexand-rh kalexand-rh added the peer-review-needed Signifies that the peer review team needs to review this PR label Oct 30, 2019
@kalexand-rh
Copy link
Contributor Author

@openshift/team-documentation, PTAL?

@bergerhoffer bergerhoffer self-requested a review October 30, 2019 19:18
Copy link
Contributor

@bergerhoffer bergerhoffer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few minor things

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not critical, just wondering whether we try to avoid changing anchor links in a version after it's been released.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an error that I regret not catching during peer review. There was a lot of drama in the mod docs repo over how much using malformed anchor IDS can break automation. There are also no other links to this content in the collection.

@bergerhoffer bergerhoffer added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Oct 30, 2019
@mffiedler
Copy link

/lgtm
@jiajliu I reviewed this and tested the steps and approved it. If you have any further comments you can put them here or open a new bz.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2019
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2019
@kalexand-rh
Copy link
Contributor Author

Thanks! Merging.

@kalexand-rh kalexand-rh merged commit 0389b2a into openshift:master Oct 30, 2019
@kalexand-rh
Copy link
Contributor Author

/cherrypick enterprise-4.2

@kalexand-rh
Copy link
Contributor Author

/cherrypick enterprise-4.3

@openshift-cherrypick-robot

@kalexand-rh: new pull request created: #17754

Details

In response to this:

/cherrypick enterprise-4.2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot

@kalexand-rh: new pull request created: #17755

Details

In response to this:

/cherrypick enterprise-4.3

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.2 branch/enterprise-4.3 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.