OSDOCS-2427: Update best practices guidance #68477
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@skopacz1: This pull request references OSDOCS-2427 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. 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. |
openshift-ci
bot
added
the
size/M
|
🤖 Fri Jan 12 20:33:58 - Prow CI generated the docs preview: https://68477--ocpdocs-pr.netlify.app |
|
@skopacz1: This pull request references OSDOCS-2427 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. 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. |
modules/update-best-practices.adoc
Outdated
| === Ensure that the cluster is in an Upgradable state | ||
|
|
||
| When one or more Operators have not reported their `Upgradeable` condition as `true` for more than an hour, the `ClusterNotUpgradeable` warning alert is triggered in the cluster. | ||
| In most cases patch updates are not blocked by this alert, but you cannot perform a minor version update until this alert is resolved and all Operators report `Upgradeable` as `true`. |
|
/cc |
modules/update-best-practices.adoc
Outdated
| === Choose versions recommended by the OpenShift Update Service | ||
|
|
||
| The OpenShift Update Service (OSUS) provides update recommendations based on cluster characteristics such as the cluster's subscribed channel, which are then saved by the Cluster Version Operator as either recommended or conditional updates. | ||
| While it is possible to attempt an update to a version that is not recommended by OSUS, doing so significantly increases the risk of update failure or unintended consequences to the cluster after the update has finished. |
There was a problem hiding this comment.
Can we spin this into a positive-sounding (or at least less negative) statement? I think "significantly increases risk" is not even that much true. "update failure" is very strong term, but actually very rare, most upgrade issues are annoyances. I'd say recommended paths are near-zero risk and non-recommended is low risk, we just don't have enough data to actually claim near-zero risk (even if it likely is safe - we do a lot of testing, and we do not have enough resources to test every possible path).
So I would maybe just say that following recommended paths protects the users from encountering known issues or unintended consequences, or something like that.
There was a problem hiding this comment.
I'll replace my wording with yours, sure thing.
modules/update-best-practices.adoc
Outdated
|
|
||
| [discrete] | ||
| [id="nodes-ready_{context}"] | ||
| === Ensure that all nodes are available |
There was a problem hiding this comment.
I think "all nodes are available" is not exactly the condition that we want. The point is that the cluster should not be running with little to no spare capacity. If the cluster is "full", then taking down a node to upgrade it can be either slow or even impossible. So I would describe this section as "make sure the cluster has enough spare capacity so that the upgrade process can actually take some nodes offline to update them."
This has two flavors, (you already mention one of them):
- There is spare capacity on compute nodes, so that when taking a node offline temporarily, its workloads can be placed on other nodes comfortably
- Worker pools do not have enough unavailable notes to prevent taking more nodes offline temporarily.
There was a problem hiding this comment.
We have noticed that many times when a node is down MCO gets stuck. So I am supportive of the idea. I also agree that we should have spare nodes which would give some breathing room to pods.
modules/update-best-practices.adoc
Outdated
| === Ensure that the cluster's PodDisruptionBudget is properly configured | ||
|
|
||
| The `PodDisruptionBudget` object allows you to define the minimum number or percentage of pod replicas that must be available at any given time. | ||
| This configuration allows workloads to be protected from disruptions during maintenance tasks such as cluster updates. |
There was a problem hiding this comment.
I think we can afford to be a bit more specific:
- Make sure that for HA workloads, there are replicas that can be temporarily taken offline without PDB configuration prohibiting that
- Make sure that non-HA workloads are either not protected by PDBs, or that there is an alternative reliable mechanism that allows draining these workloads eventually (such as periodic restart, guaranteed eventual termination or something else)
modules/update-best-practices.adoc
Outdated
| Critical alerts must always be addressed as soon as possible, but it is especially important to address these alerts and resolve any problems before initiating a cluster update. | ||
| Failing to address critical alerts before beginning an update can cause a loss of data or a major failure of cluster services. | ||
|
|
||
| You should also periodically review Warning and Info alerts on the cluster to address any potentially problematic conditions before initiating an update. |
There was a problem hiding this comment.
I do not think we should ask customers to review info alerts , thats a lot of work without much value for update success. Just asking to review critical alerts is good enough.
There was a problem hiding this comment.
Sounds good, I'll remove this sentence.
modules/update-best-practices.adoc
Outdated
|
|
||
| [discrete] | ||
| [id="nodes-ready_{context}"] | ||
| === Ensure that all nodes are available |
There was a problem hiding this comment.
We have noticed that many times when a node is down MCO gets stuck. So I am supportive of the idea. I also agree that we should have spare nodes which would give some breathing room to pods.
|
LGTM 👍 |
|
@shellyyang1989 could you PTAL when you have a chance? Thanks! |
|
@JianLi-RH Could you please help take a look? Thank you! |
modules/update-best-practices.adoc
Outdated
|
|
||
| [discrete] | ||
| [id="recommended-versions_{context}"] | ||
| === Choose versions recommended by the OpenShift Update Service |
There was a problem hiding this comment.
If this is the second level section of Best practices for cluster updates, can we set it like the second levels of Kubernetes API deprecations and removals, so that we can navigate them in the right side bar.
There was a problem hiding this comment.
I can change it to be a second level heading, sure. I made it a third level for stylistic reasons but it's not super important to keep it that way. The reason they don't show up in the navigation on the right, however is because of the [discrete] elements, such as the one on line 14.
I put those there because I thought it would overwhelm the sidebar navigation, but I guess if there are only 5 of these sections then that's okay for now!
There was a problem hiding this comment.
@skopacz1 I am sure you have updated the navigation level, but I can't see the change on preview page, could you help to confirm it?
There was a problem hiding this comment.
The automated preview page is not working correctly, sorry about that. I've manually uploaded the most recent preview with your feedback here: https://file.rdu.redhat.com/skopacz/OSDOCS-2427/updating/preparing_for_updates/updating-cluster-prepare.html#update-best-practices_updating-cluster-prepare
(you'll need to be on VPN to access this site)
modules/update-best-practices.adoc
Outdated
|
|
||
| [discrete] | ||
| [id="critical-alerts_{context}"] | ||
| === Address all critical alerts on the cluster |
There was a problem hiding this comment.
Please make it as the second level section of Best practices for cluster updates
modules/update-best-practices.adoc
Outdated
|
|
||
| [discrete] | ||
| [id="cluster-upgradeable_{context}"] | ||
| === Ensure that the cluster is in an Upgradable state |
There was a problem hiding this comment.
Please make it as the second level section of Best practices for cluster updates
modules/update-best-practices.adoc
Outdated
|
|
||
| [discrete] | ||
| [id="nodes-ready_{context}"] | ||
| === Ensure that enough spare nodes are available |
There was a problem hiding this comment.
Please make it as the second level section of Best practices for cluster updates
modules/update-best-practices.adoc
Outdated
|
|
||
| [discrete] | ||
| [id="pod-disruption-budget_{context}"] | ||
| === Ensure that the cluster's PodDisruptionBudget is properly configured |
There was a problem hiding this comment.
Please make it as the second level section of Best practices for cluster updates
modules/update-best-practices.adoc
Outdated
|
|
||
| [discrete] | ||
| [id="recommended-versions_{context}"] | ||
| === Choose versions recommended by the OpenShift Update Service |
There was a problem hiding this comment.
I think it is better to let customers know how to get the OSUS recommended versions, like on web console or command line.
There was a problem hiding this comment.
Unless I'm mistaken - I think as long as users follow our update procedures then they'll always use recommended versions, unless users look up how to find conditional updates or use --to-image. But those two options take more work to find in the docs.
So I think we already show users how to get recommended versions because that's the default method that we document in almost all of our procedures.
modules/update-best-practices.adoc
Outdated
|
|
||
| [discrete] | ||
| [id="critical-alerts_{context}"] | ||
| === Address all critical alerts on the cluster |
There was a problem hiding this comment.
Where can customer find the critical alerts? and which alerts are critical? maybe a summarize here is helpful.
There was a problem hiding this comment.
I can add a sentence describing where to find alerts in the UI. I don't think there's currently a command to see alerts.
There was a problem hiding this comment.
I think run oc adm upgrade can also find the critical alerts. /cc @shellyyang1989 for confirm.
There was a problem hiding this comment.
Currently alerts are not surfaced in any oc command. OTA-1080 and OTA-1157 should eventually improve this state.
modules/update-best-practices.adoc
Outdated
|
|
||
| [discrete] | ||
| [id="cluster-upgradeable_{context}"] | ||
| === Ensure that the cluster is in an Upgradable state |
There was a problem hiding this comment.
How do we know if a cluster is in an Upgradable state?
There was a problem hiding this comment.
We document the Upgradeable condition of cluster Operators here: Understanding cluster Operator condition types.
I can add a link to that page underneath this overall best practices section
|
@JianLi-RH Sorry that I've taken so long to get back to your feedback! Would you mind taking another look and seeing if I addressed all of your points? Thanks! |
|
/retest |
|
Closing and reopening to restart automated tests and hopefully causing the preview to be updated |
|
@skopacz1: This pull request references OSDOCS-2427 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
|
@skopacz1: This pull request references OSDOCS-2427 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
Thank you @skopacz1. /lgtm |
|
/label peer-review-needed |
openshift-ci
bot
added
the
peer-review-needed
modules/update-best-practices.adoc
Outdated
| [id="update-best-practices_{context}"] | ||
| = Best practices for cluster updates | ||
|
|
||
| {product-title} is designed to provide a robust update experience that allows clusters to update with minimal disruptions to workloads. |
There was a problem hiding this comment.
| {product-title} is designed to provide a robust update experience that allows clusters to update with minimal disruptions to workloads. | |
| {product-title} provides a robust update experience that minimizes workload disruptions. |
Reworded to avoid passive voice and the use of "allow" per ISG:
"Avoid stating that inanimate objects grant abilities to people, as in “the product allows you to…”. Whenever possible, use a direct, user-focused alternative such as “you can use the product to…” or “with this product, users can…”.
modules/update-best-practices.adoc
Outdated
| = Best practices for cluster updates | ||
|
|
||
| {product-title} is designed to provide a robust update experience that allows clusters to update with minimal disruptions to workloads. | ||
| Updates will not begin unless the cluster is determined to be in an upgradeable state at the time of the update request. |
There was a problem hiding this comment.
| Updates will not begin unless the cluster is determined to be in an upgradeable state at the time of the update request. | |
| Updates will not begin unless the cluster is in an upgradeable state at the time of the update request. |
Passive voice
modules/update-best-practices.adoc
Outdated
| {product-title} is designed to provide a robust update experience that allows clusters to update with minimal disruptions to workloads. | ||
| Updates will not begin unless the cluster is determined to be in an upgradeable state at the time of the update request. | ||
|
|
||
| This design ensures that some key conditions are met before initiating an update, but there are a number of actions you can take to increase your chances of a successful cluster update. |
There was a problem hiding this comment.
| This design ensures that some key conditions are met before initiating an update, but there are a number of actions you can take to increase your chances of a successful cluster update. | |
| This design enforces some key conditions before initiating an update, but there are a number of actions you can take to ensure a successful cluster update. |
Reworded passive voice and added stronger tone. "increase your chances" sounds like the user is gambling
There was a problem hiding this comment.
The first part of your suggestion sounds good, but I'm actively avoiding using terms like "ensuring" to avoid making promises to the user about the success of an update. The spirit of this doc is to acknowledge factors that can affect the success of an update and inform users how they can mitigate risk factors, it's not to assuage users that their updates will be universally successful.
IMO while we wouldn't want to explicitly state that updates are a gamble, that's sort of the nature of major tasks that introduce change and instability to the cluster, such as an update, and we don't want to misrepresent that nature in a doc meant to give users best practices and advice.
modules/update-best-practices.adoc
Outdated
| // | ||
| // * updating/preparing_for_updates/updating-cluster-prepare.adoc | ||
|
|
||
| :_mod-docs-content-type: PROCEDURE |
There was a problem hiding this comment.
| :_mod-docs-content-type: PROCEDURE | |
| :_mod-docs-content-type: REFERENCE |
There was a problem hiding this comment.
Good catch, I copied and pasted this metadata stuff from another module
modules/update-best-practices.adoc
Outdated
| [id="recommended-versions_{context}"] | ||
| == Choose versions recommended by the OpenShift Update Service | ||
|
|
||
| The OpenShift Update Service (OSUS) provides update recommendations based on cluster characteristics such as the cluster's subscribed channel, which are then saved by the Cluster Version Operator as either recommended or conditional updates. |
There was a problem hiding this comment.
| The OpenShift Update Service (OSUS) provides update recommendations based on cluster characteristics such as the cluster's subscribed channel, which are then saved by the Cluster Version Operator as either recommended or conditional updates. | |
| The OpenShift Update Service (OSUS) provides update recommendations based on cluster characteristics such as the cluster's subscribed channel. The Cluster Version Operator saves these recommendations as either recommended or conditional updates. |
Rewording suggestion
modules/update-best-practices.adoc
Outdated
| == Ensure that the cluster is in an Upgradable state | ||
|
|
||
| When one or more Operators have not reported their `Upgradeable` condition as `True` for more than an hour, the `ClusterNotUpgradeable` warning alert is triggered in the cluster. | ||
| In most cases patch updates are not blocked by this alert, but you cannot perform a minor version update until this alert is resolved and all Operators report `Upgradeable` as `True`. |
There was a problem hiding this comment.
| In most cases patch updates are not blocked by this alert, but you cannot perform a minor version update until this alert is resolved and all Operators report `Upgradeable` as `True`. | |
| In most cases this alert does not block patch updates, but you cannot perform a minor version update until you resolve this alert and all Operators report `Upgradeable` as `True`. |
passive voice
There was a problem hiding this comment.
Sounds good for the first part, but as to changing the second part to "until you resolve this alert": I'm honestly not familiar enough with OCP to suggest that this alert always requires user intervention to resolve. Maybe in some cases something weird happens to an Operator for an hour to trigger the Upgradeable=false condition, then it fixes itself, reports Upgradeable as true, and the alert goes away solely due to system actions?
I think I'll keep it as "this alert is resolved" to keep that user vs system ambiguity, unless an SME can confirm that basically every time an Operator reports Upgradeable as false, it's an issue that requires user action to resolve.
CC @petr-muller and @wking
There was a problem hiding this comment.
It can be both, but I think most cases of Upgradeable=False signal that the user needs to do something.
There was a problem hiding this comment.
Sounds good, I suppose the distinction is probably not relevant enough to leave that ambiguity in.
| A cluster should not be running with little to no spare node capacity, especially when initiating a cluster update. | ||
| Nodes that are not running and available may limit a cluster's ability to perform an update with minimal disruption to cluster workloads. |
There was a problem hiding this comment.
I think it would be better to reword this in a positive voice, such as "You should have spare node capacity to ensure.." and "Having available nodes is important for cluster updates to complete with minimal disruption" or something to that effect. I'm not entirely sure what is meant by "running and available" though.
There was a problem hiding this comment.
I'll admit this sounds negative but I think it's useful to talk about the consequences this can have on update success, especially since there's already a more positive sounding, actionable piece of advice at the end of the section here on line 49. I feel like it would be a bit redundant to also have similar phrasing here. That's just my opinion though, let me know if you disagree.
modules/update-best-practices.adoc
Outdated
| A cluster should not be running with little to no spare node capacity, especially when initiating a cluster update. | ||
| Nodes that are not running and available may limit a cluster's ability to perform an update with minimal disruption to cluster workloads. | ||
|
|
||
| Depending on the configured value of the cluster's `maxUnavailable` spec, an unavailable node can also prevent itself and other nodes from having machine configuration changes applied during a cluster update. |
There was a problem hiding this comment.
| Depending on the configured value of the cluster's `maxUnavailable` spec, an unavailable node can also prevent itself and other nodes from having machine configuration changes applied during a cluster update. | |
| Depending on the configured value of the cluster's `maxUnavailable` spec, the cluster may not be able to apply machine configuration changes to nodes if there is an unavailable node. |
passive voice
There was a problem hiding this comment.
I can try and reword it in active voice, sure.
modules/update-best-practices.adoc
Outdated
| [id="pod-disruption-budget_{context}"] | ||
| == Ensure that the cluster's PodDisruptionBudget is properly configured | ||
|
|
||
| The `PodDisruptionBudget` object allows you to define the minimum number or percentage of pod replicas that must be available at any given time. |
There was a problem hiding this comment.
| The `PodDisruptionBudget` object allows you to define the minimum number or percentage of pod replicas that must be available at any given time. | |
| You can use the `PodDisruptionBudget` object to define the minimum number or percentage of pod replicas that must be available at any given time. |
Removed permissive language
modules/update-best-practices.adoc
Outdated
| == Ensure that the cluster's PodDisruptionBudget is properly configured | ||
|
|
||
| The `PodDisruptionBudget` object allows you to define the minimum number or percentage of pod replicas that must be available at any given time. | ||
| This configuration allows workloads to be protected from disruptions during maintenance tasks such as cluster updates. |
There was a problem hiding this comment.
| This configuration allows workloads to be protected from disruptions during maintenance tasks such as cluster updates. | |
| This configuration protects workloads from disruptions during maintenance tasks such as cluster updates. |
openshift-ci
bot
added
peer-review-done
|
New changes are detected. LGTM label has been removed. |
|
@skopacz1: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
/label merge-review-needed |
openshift-ci
bot
added
the
merge-review-needed
|
/label merge-review-in-progress |
openshift-ci
bot
added
the
merge-review-in-progress
|
/remove-label merge-review-needed |
openshift-ci
bot
removed
the
merge-review-needed
|
/cherrypick enterprise-4.15 |
|
/cherrypick enterprise-4.14 |
|
/cherrypick enterprise-4.13 |
|
/remove-label merge-review-in-progress |
openshift-ci
bot
removed
the
merge-review-in-progress
|
@ShaunaDiaz: new pull request created: #70234 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. |
|
@ShaunaDiaz: #68477 failed to apply on top of branch "enterprise-4.14": 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. |
|
@ShaunaDiaz: #68477 failed to apply on top of branch "enterprise-4.13": 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. |
OSDOCS-2427
Version(s): 4.13+ (note to merge reviewer - the 4.13 auto CP will likely fail)
This PR adds guidance for users about best practices they can employ to ensure that their cluster update is successful. This is not meant to be a list of additional prerequisites that users must follow before initiating an update.
QE review:
Preview: Best practices for cluster updates (VPN required)