Fix: If cgroup manager is cgroupfs then allow conmon_cgroup to be empty #8888

xw19 · 2025-01-05T19:45:08Z

conmon_cgroup is deprecated and is replaced with RuntimeHandler monitor_cgroup and we prepopulate with default 'system.slice'

What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #6047

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

Fixed bug to allow `conmon_cgroup` to be empty when cgroup manager is `cgroupfs` .

openshift-ci · 2025-01-05T19:45:22Z

Hi @xw19. Thanks for your PR.

I'm waiting for a cri-o member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

codecov · 2025-01-05T19:52:44Z

Codecov Report

Attention: Patch coverage is 93.10345% with 4 lines in your changes missing coverage. Please review.

Project coverage is 47.49%. Comparing base (b49a38f) to head (a4c67cc).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8888      +/-   ##
==========================================
+ Coverage   47.47%   47.49%   +0.02%     
==========================================
  Files         154      154              
  Lines       23111    23124      +13     
==========================================
+ Hits        10971    10983      +12     
  Misses      11071    11071              
- Partials     1069     1070       +1

kwilczynski · 2025-01-05T20:06:35Z

/ok-to-test

pkg/config/config.go

xw19 · 2025-01-06T02:37:25Z

/retest

pkg/config/config.go

xw19 · 2025-01-06T03:01:37Z

/test ci-crun-e2e

pkg/config/config.go

xw19 · 2025-01-10T14:55:32Z

/retest

xw19 · 2025-01-20T13:15:25Z

/retest

xw19 · 2025-01-20T17:14:13Z

/retest

xw19 · 2025-01-27T16:32:59Z

/test ci-fedora-kata

sohankunkerkar

/approve
Thanks for your patience!
@cri-o/cri-o-maintainers PTAL

saschagrunert · 2025-01-29T08:18:44Z

@xw19 please rebase this one

xw19 · 2025-01-29T13:42:41Z

/retest

xw19 · 2025-01-29T14:29:56Z

/retest

haircommander · 2025-02-06T17:25:37Z

pkg/config/config.go

 }

+// DefaultRuntimeConfig returns the default Runtime configs.
+func DefaultRuntimeConfig(cgroupManager cgmgr.CgroupManager) *RuntimeConfig {


nit: can this be defined below where it's callled?

conmon_cgroup is deprecated and is replaced with RuntimeHandler monitor_cgroup and we prepopulate with default 'system.slice' Signed-off-by: Sourav Moitra <[email protected]>

haircommander · 2025-02-07T18:38:16Z

/approve

LGTM, @cri-o/cri-o-maintainers PTAL

xw19 · 2025-02-09T16:00:28Z

/retest

sohankunkerkar

/lgtm

openshift-ci · 2025-02-10T14:13:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, roman-kiselenko, sohankunkerkar, xw19

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:

~~OWNERS~~ [haircommander,sohankunkerkar]

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

xw19 requested a review from mrunalp as a code owner January 5, 2025 19:45

openshift-ci bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/bug Categorizes issue or PR as related to a bug. labels Jan 5, 2025

openshift-ci bot requested review from QiWang19 and hasan4791 January 5, 2025 19:45

openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 5, 2025

openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 5, 2025

roman-kiselenko approved these changes Jan 5, 2025

View reviewed changes

pkg/config/config.go Outdated Show resolved Hide resolved

openshift-ci bot assigned roman-kiselenko Jan 5, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 5, 2025

kwilczynski reviewed Jan 6, 2025

View reviewed changes

pkg/config/config.go Outdated Show resolved Hide resolved

xw19 force-pushed the main branch from dcd52e8 to 24b1842 Compare January 6, 2025 14:53

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 6, 2025

xw19 requested review from kwilczynski and roman-kiselenko January 6, 2025 15:51

roman-kiselenko approved these changes Jan 7, 2025

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 7, 2025

haircommander reviewed Jan 9, 2025

View reviewed changes

pkg/config/config.go Outdated Show resolved Hide resolved

xw19 force-pushed the main branch from 24b1842 to 49c93a0 Compare January 10, 2025 13:50

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 10, 2025

xw19 force-pushed the main branch from 49c93a0 to e2967d8 Compare January 10, 2025 13:54

openshift-ci bot added the dco-signoff: no Indicates the PR's author has not DCO signed all their commits. label Jan 10, 2025

openshift-ci bot removed the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Jan 10, 2025

xw19 force-pushed the main branch from 792451d to e2967d8 Compare January 10, 2025 15:54

openshift-ci bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Jan 10, 2025

xw19 requested a review from haircommander January 20, 2025 13:15

sohankunkerkar reviewed Jan 28, 2025

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 28, 2025

xw19 force-pushed the main branch 2 times, most recently from 39673c4 to 3a8ab78 Compare January 29, 2025 08:58

haircommander reviewed Feb 6, 2025

View reviewed changes

Fix: If cgroup manager is cgroupfs then allow conmon_cgroup to be empty

a4c67cc

conmon_cgroup is deprecated and is replaced with RuntimeHandler monitor_cgroup and we prepopulate with default 'system.slice' Signed-off-by: Sourav Moitra <[email protected]>

xw19 force-pushed the main branch from 3a8ab78 to a4c67cc Compare February 7, 2025 18:27

sohankunkerkar approved these changes Feb 10, 2025

View reviewed changes

openshift-ci bot assigned sohankunkerkar Feb 10, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 10, 2025

openshift-merge-bot bot merged commit 6168738 into cri-o:main Feb 10, 2025
77 of 78 checks passed

Fix: If cgroup manager is cgroupfs then allow conmon_cgroup to be empty #8888

Fix: If cgroup manager is cgroupfs then allow conmon_cgroup to be empty #8888

Uh oh!

Conversation

xw19 commented Jan 5, 2025 • edited by saschagrunert Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

openshift-ci bot commented Jan 5, 2025

Uh oh!

codecov bot commented Jan 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kwilczynski commented Jan 5, 2025

Uh oh!

Uh oh!

xw19 commented Jan 6, 2025

Uh oh!

Uh oh!

xw19 commented Jan 6, 2025

Uh oh!

Uh oh!

xw19 commented Jan 10, 2025

Uh oh!

xw19 commented Jan 20, 2025

Uh oh!

xw19 commented Jan 20, 2025

Uh oh!

xw19 commented Jan 27, 2025

Uh oh!

sohankunkerkar left a comment

Choose a reason for hiding this comment

Uh oh!

saschagrunert commented Jan 29, 2025

Uh oh!

xw19 commented Jan 29, 2025

Uh oh!

xw19 commented Jan 29, 2025

Uh oh!

haircommander Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

haircommander commented Feb 7, 2025

Uh oh!

xw19 commented Feb 9, 2025

Uh oh!

sohankunkerkar left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Feb 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

xw19 commented Jan 5, 2025 •

edited by saschagrunert

Loading

codecov bot commented Jan 5, 2025 •

edited

Loading