Skip to content

OCPNODE-1716: Update CgroupModeDefault to CgroupModeV2 #1527

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sairameshv
Copy link
Member

@sairameshv sairameshv commented Jul 14, 2023

cgroupsv2 is the default cgroupMode since OCP-4.14 , hence, the CgroupModeDefault needs to be updated to CgroupModeV2 in the nodes.config.openshift.io object.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 14, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 14, 2023

@sairameshv: This pull request references OCPNODE-1716 which is a valid jira issue.

In response to this:

In order to make cgroupsv2 by default feature in OCP-4.14, the CgroupModeDefault needs to be updated to CgroupModeV2 in the nodes.config.openshift.io object.

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 openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 14, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 14, 2023

Hello @sairameshv! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 14, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 14, 2023
@sairameshv sairameshv marked this pull request as ready for review July 15, 2023 01:12
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 15, 2023
@JoelSpeed
Copy link
Contributor

Is there a related PR from a payload component vendoring this change to prove the stability of the updated configuration here?

@sairameshv
Copy link
Member Author

Is there a related PR from a payload component vendoring this change to prove the stability of the updated configuration here?

There is no corresponding PR as of now but this should be part of the MCO code base.

@JoelSpeed
Copy link
Contributor

Can we create the PR and run CI against it to prove that it's not breaking anything by making this change?

@sairameshv
Copy link
Member Author

Can we create the PR and run CI against it to prove that it's not breaking anything by making this change?

This change may not break the existing code. Anyway, just to be sure, I've raised a dummy PR to actually monitor the CI jobs

@JoelSpeed
Copy link
Contributor

I've just triggered the tests on the MachineConfigOperator PR, if those are green, then I think this can be merged safely

@sairameshv
Copy link
Member Author

I've just triggered the tests on the MachineConfigOperator PR, if those are green, then I think this can be merged safely

Thanks @JoelSpeed !!
Sorry that I missed triggering the tests

@sairameshv
Copy link
Member Author

Hey @JoelSpeed , openshift/machine-config-operator#3789 has been merged and CgroupModeV2 is the default from the 4.14 release.

Here, I tried to bump api with this change in the MCO repository but looks like a couple of ci tests are failing (not because of this change). I would either try to work on the failures to resolve the same or request MCO team to fix them based on the availability.

Apart from that, I think this PR is ready for the merge.
cc: @haircommander @harche @rphillips

@MarSik
Copy link

MarSik commented Jul 26, 2023

/hold

This can't be merged into 4.14 folks. And especially not after the original branching day has passed already. The openshift/machine-config-operator#3789 MUST be reverted too.

NTO and other components are not ready as this was never planned for 4.14.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 26, 2023
@MarSik
Copy link

MarSik commented Jul 26, 2023

Ok, we have a plan now. @haircommander is preparing a patch that allows v2 as default, but moves all PerformanceProfile users to v1. That will work, if he manages to post it in time.

But please make sure we (NTO) know about disruptive changes like this before they happen please.

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 26, 2023
@harche
Copy link
Contributor

harche commented Aug 29, 2023

/hold till 4.15

We have merged MCO changes to set v2 as default. The MCO change is sufficient for 4.14 to set v2 as default for day-0. We would like to retain an ability to quickly revert to cgroups v1, just in case. Having a single MCO PR would be far easier than cycling through API and it's dependent repos.

Hence once the cgroup mode change sails through 4.14, we would merge this PR in 4.15 to mark the API default to v2. Until then we would continue to have a hybrid approach using that MCO PR where day-0 is defaulted to cgroups v2 and upgrade is maintained at the cgroup mode of 4.13 (whichever mode user had selected in 4.13 we carry it forward to 4.14 instead of forcing them to use cgroups v2)

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 29, 2023
…roupsv2 default in 4.14

Signed-off-by: Sai Ramesh Vanka <[email protected]>
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jan 24, 2024

@sairameshv: This pull request references OCPNODE-1716 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:

cgroupsv2 is the default cgroupMode since OCP-4.14 , hence, the CgroupModeDefault needs to be updated to CgroupModeV2 in the nodes.config.openshift.io object.

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
Copy link
Member Author

/assign @rphillips @harche @haircommander

@harche
Copy link
Contributor

harche commented Jan 24, 2024

/hold till 4.15

We have merged MCO changes to set v2 as default. The MCO change is sufficient for 4.14 to set v2 as default for day-0. We would like to retain an ability to quickly revert to cgroups v1, just in case. Having a single MCO PR would be far easier than cycling through API and it's dependent repos.

Hence once the cgroup mode change sails through 4.14, we would merge this PR in 4.15 to mark the API default to v2. Until then we would continue to have a hybrid approach using that MCO PR where day-0 is defaulted to cgroups v2 and upgrade is maintained at the cgroup mode of 4.13 (whichever mode user had selected in 4.13 we carry it forward to 4.14 instead of forcing them to use cgroups v2)

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 24, 2024
@haircommander
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 24, 2024
Copy link
Contributor

openshift-ci bot commented Jan 24, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: haircommander, sairameshv
Once this PR has been reviewed and has the lgtm label, please assign deads2k for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 18, 2024
@rphillips
Copy link
Contributor

/remove-lifecycle stale

@openshift-ci openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 15, 2024
@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 14, 2024
@harche
Copy link
Contributor

harche commented Nov 14, 2024

/remove-lifecycle stale

cc @sairameshv

@openshift-ci openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 14, 2024
Copy link
Contributor

openshift-ci bot commented Apr 11, 2025

@sairameshv: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-ovn ccc443d link true /test e2e-aws-ovn
ci/prow/e2e-aws-ovn-techpreview ccc443d link true /test e2e-aws-ovn-techpreview
ci/prow/e2e-aws-serial ccc443d link true /test e2e-aws-serial
ci/prow/e2e-upgrade-minor ccc443d link true /test e2e-upgrade-minor
ci/prow/e2e-aws-serial-techpreview ccc443d link true /test e2e-aws-serial-techpreview
ci/prow/e2e-upgrade ccc443d link true /test e2e-upgrade
ci/prow/e2e-aws-ovn-hypershift ccc443d link true /test e2e-aws-ovn-hypershift
ci/prow/minor-e2e-upgrade-minor ccc443d link true /test minor-e2e-upgrade-minor
ci/prow/verify-client-go ccc443d link true /test verify-client-go

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants