Skip to content

OCPNODE-3874: Remove CgroupModeV1 reference from the code #1428

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 11, 2025
Merged

OCPNODE-3874: Remove CgroupModeV1 reference from the code #1428

merged 1 commit into from
Nov 11, 2025

Conversation

@sairameshv
Copy link
Member

@sairameshv sairameshv commented Nov 11, 2025 •
edited
Loading

  • cgroupv1 has been removed from ocp > 4.19
  • Hence, the CgroupModeV1 constant can be safely removed from the api code

Reference:
openshift/api#2579

/cc @wking

@sairameshv
Remove CgroupModeV1 reference from the code
21a3c04 
- cgroupv1 has been removed from ocp > 4.19
- Hence, the CgroupModeV1 constant can be safely removed from the api
  code

Signed-off-by: Sai Ramesh Vanka <[email protected]>
@openshift-ci openshift-ci bot requested a review from wking November 11, 2025 01:49
@sairameshv sairameshv mentioned this pull request Nov 11, 2025
Merged
@sairameshv sairameshv changed the title Remove CgroupModeV1 reference from the code OCPNODE-3874: Remove CgroupModeV1 reference from the code Nov 11, 2025
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 11, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Nov 11, 2025 •
edited by openshift-ci bot
Loading

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

In response to this:

  • cgroupv1 has been removed from ocp > 4.19
  • Hence, the CgroupModeV1 constant can be safely removed from the api code

Reference:
openshift/api#2579

/cc @wking

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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 11, 2025

@sairameshv: 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-sigs/prow repository. I understand the commands that are listed here.

yanirq
yanirq reviewed Nov 11, 2025
View reviewed changes
test/e2e/performanceprofile/functests/utils/cgroup/cgroup.go
// in case `apiconfigv1.CgroupMode` not set, it's returned true since
// the platform's default is cgroupV2
return true, nil
if nodecfg.Spec.CgroupMode == apiconfigv1.CgroupModeV2 || nodecfg.Spec.CgroupMode == apiconfigv1.CgroupModeEmpty {
Copy link
Contributor

@yanirq yanirq Nov 11, 2025

Choose a reason for hiding this comment

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

this is a helper function that detects if cgroupsV1 or V2 is in use . probably should be removed completely from tests unless there are real scenarios where CgroupMode is empty (and then the test should be renamed a bit) - can be done in a different PR

Copy link
Member Author

@sairameshv sairameshv Nov 11, 2025

Choose a reason for hiding this comment

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

Yes, it would be better to cover all the refactoring in a separate PR. This PR is to make sure the CgroupModeV1 constant references are removed/replaced.

@yanirq
Copy link
Contributor

yanirq commented Nov 11, 2025

/cc @mrniranjan @MarSik

@openshift-ci openshift-ci bot requested review from MarSik and mrniranjan November 11, 2025 08:34
Tal-or
Tal-or approved these changes Nov 11, 2025
View reviewed changes
Copy link
Contributor

@Tal-or Tal-or left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2025
@yanirq
Copy link
Contributor

yanirq commented Nov 11, 2025

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 11, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sairameshv, Tal-or, yanirq

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 11, 2025
@yanirq
Copy link
Contributor

yanirq commented Nov 11, 2025

/label verified

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 11, 2025

@yanirq: The label(s) /label verified cannot be applied. These labels are supported: acknowledge-critical-fixes-only, platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, ux-approved, no-qe, downstream-change-needed, rebase/manual, cluster-config-api-changed, run-integration-tests, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, jira/valid-bug, ok-to-test, stability-fix-approved, staff-eng-approved. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/label verified

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.

@yanirq
Copy link
Contributor

yanirq commented Nov 11, 2025

/verified

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Nov 11, 2025

@yanirq: The /verified command must be used with one of the following actions: by, later, remove, or bypass. See https://docs.ci.openshift.org/docs/architecture/jira/#premerge-verification for more information.

In response to this:

/verified

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.

@rshemtov13
Copy link
Contributor

rshemtov13 commented Nov 11, 2025

/verified

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Nov 11, 2025

@rshemtov13: The /verified command must be used with one of the following actions: by, later, remove, or bypass. See https://docs.ci.openshift.org/docs/architecture/jira/#premerge-verification for more information.

In response to this:

/verified

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.

@rshemtov13
Copy link
Contributor

rshemtov13 commented Nov 11, 2025

/verified by @rshemtov13

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Nov 11, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Nov 11, 2025

@rshemtov13: This PR has been marked as verified by @rshemtov13.

In response to this:

/verified by @rshemtov13

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.

@openshift-merge-bot openshift-merge-bot bot merged commit 31b94c0 into openshift:main Nov 11, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wking wking Awaiting requested review from wking

@MarSik MarSik Awaiting requested review from MarSik

@mrniranjan mrniranjan Awaiting requested review from mrniranjan

2 more reviewers

@yanirq yanirq yanirq left review comments

@Tal-or Tal-or Tal-or approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Tal-or Tal-or

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sairameshv @openshift-ci-robot @yanirq @rshemtov13 @Tal-or