-
Notifications
You must be signed in to change notification settings - Fork 416
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
OCPNODE-2842: Set Upgradeable=False when cluster is on cgroup v1 #4822
base: main
Are you sure you want to change the base?
Conversation
@sairameshv: This pull request references OCPNODE-2843 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.19.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. |
Skipping CI for Draft Pull Request. |
/jira refresh |
@sairameshv: This pull request references OCPNODE-2843 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.19.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. |
/jira refresh |
@sairameshv: This pull request references OCPNODE-2843 which is a valid jira issue. 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. |
Related openshift/api PR: openshift/api#2181 |
3e7f5b7
to
d34b287
Compare
25db47f
to
a2b8f7c
Compare
@sairameshv: This pull request references OCPNODE-2276 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.19.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. |
@sairameshv: This pull request references OCPNODE-2276 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.19.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. |
/jira refresh |
@sairameshv: This pull request references OCPNODE-2276 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.19.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. |
/jira refresh |
@sairameshv: This pull request references OCPNODE-2276 which is a valid jira issue. 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. |
Enhancement Proposal Ref: https://github.com/openshift/enhancements/blob/master/enhancements/machine-config/mco-cgroupsv2-support.md |
@sairameshv: This pull request references OCPNODE-2276 which is a valid jira issue. 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. |
aea470d
to
0fee7e0
Compare
@sairameshv: This pull request references OCPNODE-2842 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.19.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. |
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.
some minor comments inline
pkg/operator/status.go
Outdated
if configNode.Spec.CgroupMode == configv1.CgroupModeV1 { | ||
coStatusCondition.Status = configv1.ConditionFalse | ||
coStatusCondition.Reason = "ClusterOnCgroupV1" | ||
coStatusCondition.Message = "Cluster is using cgroup v1 and is not upgradable. Please update the `CgroupMode` in the `nodes.config.openshift.io` object to 'v2'. Once upgraded, the cluster cannot be changed back to cgroup v1" |
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.
For the message, perhaps it would be good to mention the deprecation of v1 as well, just to clarify why it's not upgradeable (does it make sense to link to anything here?)
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 have added deprecated cgroup v1
in the message.
We already have the deprecation of cgroup v1 message in the config node object's status condition for the last few releases.
We have a reference of OCPSTRAT for this feature as of now. We don't have a concrete link to when exactly the RHCOS would be removing the support for cgroup v1 i.e. it's going to be done in the future releases (may be by 4.20?). So, I don't think we have a link at the moment to convey in the message.
CgroupMode `v1` is not supported in OCP-4.19 This change helps in preventing clusters to upgrade to 4.19 before updating the cluster to cgroupMode `v2` Signed-off-by: Sai Ramesh Vanka <[email protected]>
0fee7e0
to
3f3df1b
Compare
/retest |
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 the patch looks from the MCO side. There are quite a bit of test failures, although they seem unrelated?
/retest-required
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sairameshv, yuqi-zhang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
Yes, definitely the errors are not related to these changes. |
if configNode.Spec.CgroupMode == configv1.CgroupModeV1 { | ||
coStatusCondition.Status = configv1.ConditionFalse | ||
coStatusCondition.Reason = "ClusterOnCgroupV1" | ||
coStatusCondition.Message = "Cluster is using deprecated cgroup v1 and is not upgradable. Please update the `CgroupMode` in the `nodes.config.openshift.io` object to 'v2'. Once upgraded, the cluster cannot be changed back to cgroup v1" |
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.
This condition can be overridden by the pools. We may want to update the status and return. @yuqi-zhang ?
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.
Thinking we should append coStatusCondition for cgroupv1 to any pools that are degraded and are preventing the upgrade as well.
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.
Hmm, so the status here should only be overridden by the pools if a pool is degraded, which I thought was the original intent. I think fine if degraded pools took priority, and once that is fixed, the user then sees the cgroupsv1 failure. I guess if we wanted to make it that way we should do it explicitly by ordering this after the pool degrade reporting and have both conditions exit directly.
Based on your second comment, you're looking for something like: if pool degraded && cgroups on v1, then error with Reason = "ClusterOnCgroupV1AndPoolDegraded"? As far as I know the upgradeable condition is not a list of failed reason but rather a singular, but I'm happy to see that change if there's a more elegant way to do this.
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.
There can be multiple blocking CFE conditions separated with a :: delimiter. Similar to what can be found here. IMO is probably worth the modification to return a list of degraded conditions.
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.
Hmm. This PR is modifying the status conditions... Shouldn't it be modifying the CFE conditions?
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.
/retest-required |
/retest |
@sairameshv: The following tests failed, say
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-sigs/prow repository. I understand the commands that are listed here. |
Comment aside, prow changing |
- What I did
Added code to set the cluster operator's status to
Upgradeable=False
when a cluster is found to be on cgroup v1- How to verify it
Update the
CgroupMode
field of ndes.config object tov1
and verify that the MCO cluster operator's status has theUpgradeable=False
condition- Description for the changelog
Cgroupsv1 support deprecation condition/message has been added in the last release and this PR helps in updating all the clusters to cgroup v2 before upgrading to ocp 4.19
References: