Delegate cgroup exists to systemd and libcontainer

[odinuge]

What type of PR is this?

/kind bug
/kind failing-test

What this PR does / why we need it:

When systemd cgroup driver is used, systemd is responsible for ensuring
the correct controllers are propagated. If we try to propagate a
controller controlled by systemd, it can remove it for us, creating a
mess.

This fixes issues when burstable.slice exists, but systemd removes the cpuset controller since it isn't in use. In this case, trying to recreate the slice will cause a fatal error in kubelet.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[odinuge]

/test pull-kubernetes-node-kubelet-serial-crio-cgroupv2

[odinuge]

/test pull-kubernetes-node-kubelet-serial-crio-cgroupv1

[odinuge]

Still WIP and testing, but;

/cc @harche

[odinuge]

/test pull-kubernetes-node-kubelet-serial-crio-cgroupv1
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv2

[odinuge]

/test pull-kubernetes-node-kubelet-serial-crio-cgroupv1
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv2

[kolyshkin]

By the way, since opencontainers/runc@9087f2e all runc/libcontainer's cgroup controllers have Exists() method which might be used here. The difference though is the libcontainer Exists do not check the presence of all the controllers, assuming that if Apply() was called, they all either do or do not exist.

If it is possible to relax the "exists" check in here, this might result in removing a lot of code :)

[kolyshkin]

I would also audit all the uses of Exists (there are 6, excluding tests) and maybe drop some of them.

Say, calling Exists before Create does not make much sense as Create is relatively fast and (AFAICS) repeated Create is won't break anything, while Exists (before this PR at least) is somewhat heavy.

Calling Exists before Update does not make much sense either -- if cgroup does not exists, Update will return an error anyway.

[odinuge]

By the way, since opencontainers/runc@9087f2e all runc/libcontainer's cgroup controllers have Exists() method which might be used here. The difference though is the libcontainer Exists do not check the presence of all the controllers, assuming that if Apply() was called, they all either do or do not exist.

Ahh, I see. I just saw this issues opencontainers/runc#1440 and thought it wasn't fixed.

Say, calling Exists before Create does not make much sense as Create is relatively fast and (AFAICS) repeated Create is won't break anything, while Exists (before this PR at least) is somewhat heavy.

Well, it kinda depends. Calling create in case the systemd slice/scope already exists results in an error that is fatal to kubelet. This is essentially whan happen in case of a kubelet restart. In case of cgroupfs we could probably just use Create anyways, since that will ensure all controllers are enabled.

Calling Exists before Update does not make much sense either -- if cgroup does not exists, Update will return an error anyway.

Well, yeah. If we want to create it, we would of course need to verify if we should run create or update.

[odinuge]

/test pull-kubernetes-node-kubelet-serial-crio-cgroupv1
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv2

[dims]

/milestone v1.22

/assign @mrunalp @Random-Liu

[odinuge]

Test failures are btw. because those test suites have a huuuge amount of failing tests (that we are still working on). This one helps a bit on the overall health.

/skip

[ehashman]

/test pull-kubernetes-node-kubelet-serial-crio-cgroupv2
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv1

[k8s-ci-robot]

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

Test name	Commit	Details	Rerun command
pull-kubernetes-node-kubelet-serial-crio-cgroupv2	d4c3d55	link	/test pull-kubernetes-node-kubelet-serial-crio-cgroupv2
pull-kubernetes-node-kubelet-serial-crio-cgroupv1	d4c3d55	link	/test pull-kubernetes-node-kubelet-serial-crio-cgroupv1

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

[voigt]

Hi (@bobbypage, @yujuhong, @harche, @dims)
Bug-Triage here:
we are already past code freeze, so I suggest moving this PR to milestone v1.23. WDYT?

[georgyo]

Hi (@bobbypage, @yujuhong, @harche, @dims)
Bug-Triage here:
we are already past code freeze, so I suggest moving this PR to milestone v1.23. WDYT?

This regression affects me. Since it is a regression I don't think it should be affected by the code freeze. During a code freeze, bug fixes can still go in.

[liggitt]

if this is fixing a release-blocking regression, that should be linked prominently in the PR description so it doesn't get bumped from the release incorrectly

[ehashman]

Hi (@bobbypage, @yujuhong, @harche, @dims)
Bug-Triage here:
we are already past code freeze, so I suggest moving this PR to milestone v1.23. WDYT?

I believe we may end up closing this PR in favour of #103743. As @liggitt pointed out the description is a bit coy, this addresses a serious issue.

[ehashman]

/milestone clear

from SIG Node CI subgroup today, this isn't release-critical -- we will handle this via the runc bump

[odinuge]

while we wait for runc 1.0.0

We are now on runc v1.0.2, and that should hopefully have fixed all these major issues.

/hold cancel

[ehashman]

/remove-kind failing-test

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[dims]

@ehashman @odinuge do we still need this?

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.