Skip to content

deps: bump opencontainers/cgroups to v0.0.2, fix tests #4751

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 2 commits into
base: main
Choose a base branch
from

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Apr 28, 2025

For opencontainers/cgroups changes, see
https://github.com/opencontainers/cgroups/releases/tag/v0.0.2

Fix integration tests according to changes in opencontainers/cgroups#4
(now the CPU quota value set is rounded the same way systemd does it).

Related to: #4639
Fixes: #4622

@kolyshkin kolyshkin changed the title deps: bump opencontainers/cgroups to v0.0.2 deps: bump opencontainers/cgroups to v0.0.2, fix tests Apr 29, 2025
@kolyshkin kolyshkin force-pushed the cgroups-002 branch 2 times, most recently from 58b00e9 to d756289 Compare April 29, 2025 20:27
@kolyshkin kolyshkin marked this pull request as ready for review April 29, 2025 20:27
Copy link
Member

@rata rata left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me, but based on past experience, I think it would be great to run this in moby, containerd and kubernetes CI to make sure no one expects the old thing in tests.

I guess the Kubernetes CI can be the trickiest to run. I hope what is run on an open PR is enough to cover any regressions. But I can reach out to k8s people to ask, if you run into issues :)

@kolyshkin
Copy link
Contributor Author

I think it would be great to run this in moby, containerd and kubernetes CI to make sure no one expects the old thing in tests.

Isn't it why we make pre-releases for?

Apparently k8s did something about it: kubernetes/kubernetes#131059.

I would also add cri-o and podman to the list of software whose tests are potentially broken by this update.

In any case, what's done is the right thing to do, because

  • practically, the rounding doesn't matter;
  • over time systemd may overwrite the value we wrote using fs driver;
  • there is no way to work around the systemd overwriting.

So,

  • if some tests will be broken, they need to be fixed in one way or another;
  • I doubt any real applications will fail, it's mostly about the tests.

kolyshkin added 2 commits May 2, 2025 17:08
Instead of providing systemd CPU quota value (CPUQuotaPerSec),
calculate it based on how opencontainers/cgroups/systemd handles
it (see addCPUQuota).

Signed-off-by: Kir Kolyshkin <[email protected]>
For changes, see https://github.com/opencontainers/cgroups/releases/tag/v0.0.2

Fix integration tests according to changes in [1] (now the CPU quota value set
is rounded the same way systemd does it).

[1]: opencontainers/cgroups#4
Signed-off-by: Kir Kolyshkin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

systemd driver updates CPU quota inconsitently
2 participants