-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OCPBUGS#6756: Adds conditional updates in web console and some reorg #63315
Conversation
🤖 Updated build preview is available at: Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/34284 |
02f00b4
to
dad6c71
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the long list of comments, but here are my placement and wording suggestions to clean up this assembly-level prereq list. Feel free to disregard if you have other ideas in mind.
Before updating using the web console, you should: | ||
|
||
* Have access to the cluster as a user with `cluster-admin` privileges. | ||
* Have a recent etcd backup in case your update fails and you must restore your cluster to a previous state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would double check with an SME, but I think this prereq might be moot if there's nothing the user could do to trigger a new backup. If that's true, this is more like "hope that your most recent backup is recent enough to revert to if disaster recovery is necessary."
One idea might be to keep this here but reword it to something like:
"Check to see that your etcd backup is recent enough in case your update fails and you must restore your cluster to a previous state." Plus maybe a sentence about how etcd periodically creates a new backup on its own?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to your suggestion, but wondering if we should specify what "recent enough" means. Leaving this comment thread open to discuss with SMEs. I read through etcd docs and it wasnt clear to me.
* Ensure that all machine config pools (MCPs) are running and not paused. Nodes associated with a paused MCP are skipped during the update process. You can pause the MCPs if you are performing a canary rollout update strategy. | ||
//remove this???^ or maybe just add another bullet that you can break up the update? | ||
* To accommodate the time it takes to update, you are able to do a partial update by updating the worker or custom pool nodes. You can pause and resume within the progress bar of each pool. | ||
* If your cluster uses manually maintained credentials, update the cloud provider resources for the new release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd move this to prereqs for update modules (and ensure parallel wording with other prereqs)
//remove this???^ or maybe just add another bullet that you can break up the update? | ||
* To accommodate the time it takes to update, you are able to do a partial update by updating the worker or custom pool nodes. You can pause and resume within the progress bar of each pool. | ||
* If your cluster uses manually maintained credentials, update the cloud provider resources for the new release. | ||
* Review the list of APIs that were removed in Kubernetes 1.26, migrate any affected components to use the new API version, and provide the administrator acknowledgment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd move this to prereqs for update modules (and ensure parallel wording with other prereqs)
I think this one is tricky because it might not be in each version's prereqs, and the K8s version is probably different in each OCP version this appears in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is always a list of APIs that are removed each release?? I think we could generalize it and keep it under the considerations module as "check the APIs that were removed" maybe
Hi @wking! Could you take a look at this PR when you have a chance? I am doing this one before I start the PRs to backport the change. Thank you! |
The This is because your PR targets the If the update in your PR does NOT apply to version 4.15 onward, please re-target this PR to go directly into the appropriate version branch or branches (enterprise-4.x) instead of main |
192e816
to
20942c3
Compare
Apart from #63315 (comment) , everything looks good to me. |
@shellyyang1989 Could you help with a QE review of this PR? Thank you! |
/label peer-review-in-progress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, @opayne1
I added a couple of comments for your consideration.
/remove-label peer-review-in-progress
/remove-label peer-review-needed
/label peer-review-done
@yanpzhan Could you please help review from UI side? Thank you. |
The changes look good for me |
Thanks @XiyunZhao for reviewing the pr, the update looks good from UI side. |
251713d
to
c3a70e7
Compare
LGTM! Thanks again for the help with update docs! |
/cherrypick enterprise-4.14 |
/cherrypick enterprise-4.15 |
@opayne1: new pull request created: #68298 In response to this:
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. |
@opayne1: new pull request created: #68299 In response to this:
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. |
Adds conditional updates in web console and some reorg
Version(s):
main and 4.14
Issue:
https://issues.redhat.com/browse/OCPBUGS-5756
Link to docs preview:
https://63315--docspreview.netlify.app/openshift-enterprise/latest/updating/updating_a_cluster/updating-cluster-web-console
QE review:
Additional information:
This PR also includes some reorg to the assembly. It moves prerequisites around as well.