Fix failing unit tests on systems with cgroup v2 unified mode. #119292
Conversation
Update unit tests to mock `libcontainercgroups.IsCgroup2UnifiedMode` so they can pass regardless of whether cgroup v2 unified mode is actually enabled on the system where the unit tests are running.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: brianpursley 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 |
|
part of me feels like we should skip the tests if they're not applicable to the host environment. +1 for the new test cases and the robustness of the tests though. |
Sure, that makes sense too. Are you thinking something like #119329? |
| { | ||
| name: "Request128MBLimit256MBCgroup2", | ||
| cpuReq: generateResourceQuantity("1"), | ||
| cpuLim: generateResourceQuantity("2"), | ||
| memLim: generateResourceQuantity("128Mi"), | ||
| cgroup2UnifiedMode: true, | ||
| expected: &runtimeapi.LinuxContainerResources{ | ||
| CpuPeriod: 100000, | ||
| CpuQuota: 200000, | ||
| CpuShares: 1024, | ||
| MemoryLimitInBytes: 134217728, | ||
| Unified: map[string]string{"memory.oom.group": "1"}, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
yeah I think #119329 is good, but I think it's missing this section. I think we can also have tests that we skip for v1, and I think this case is a good addition @brianpursley
There was a problem hiding this comment.
This is the same test case as Request128MBLimit256MB but with the cgroup2-specific map entry (memory.oom.group).
Are you thinking I should duplicate the test cases for CGroup V2 and conditionally skip either the CGroup 1 or CGroup 2 tests depending on whether the host system has CGroup 1/2?
|
PR needs rebase. 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. |
|
/assign @ffromani |
|
/triage accepted |
|
@brianpursley please rebase, thanks. |
What type of PR is this?
/kind failing-test
What this PR does / why we need it:
Since 4e20a8f, some of the kuberuntime unit tests fail when run on systems that have cgroup v2 unified mode (Ubuntu 22.04 in my case).
The test failures are not happening in the build pipeline, only when you are running them locally on a system with cgroup v2 unified mode (
stat -fc %T /sys/fs/cgroupreturnscgroup2fs)This PR updates the failing unit tests to mock
libcontainercgroups.IsCgroup2UnifiedModeso they can pass regardless of whether cgroup v2 unified mode is actually enabled on the system where the unit tests are running. It also enables the ability to write unit test cases which check the output in both scenarios (v1 and v2) of which I added a couple of new test cases.Which issue(s) this PR fixes:
Special notes for your reviewer:
While I did add a couple of new unit test cases for cgroup v2, this PR is not intended to be comprehensive testing for cgroup v2 functionality. My goal is to enable the existing unit tests to pass.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: