fix(atelet): set OCI CgroupsPath so each actor gets its own cgroup

[ArnaudBger (ArnaudBger)]

Fixes #50

Issue

When the OCI spec leaves Linux.CgroupsPath empty, runsc uses whatever cgroup it inherited from its parent process and does not record it as one it owns. At teardown, runsc only attempts Rmdir on paths it owns:

// runsc/cgroup/cgroup_v2.go — Uninstall
for i := len(c.Own) - 1; i >= 0; i-- {
    current := c.Own[i]
    // unix.Rmdir(current) with backoff on EBUSY
}

That makes the failure mode asymmetric and that is why it is hard to spot:

• Non-owner actors hit Uninstall with an empty c.Own, the loop body never runs, the function returns nil — no log, no error.
• The owner actor actually calls Rmdir on the shared cgroup, and gets EBUSY because the non-owners' processes are still in cgroup.procs. This is the only log line you ever see.

From logs alone it looks like one specific actor is broken — when in reality the bug is shared-cgroup ownership across actors.

How to reproduce

1. Start actor A with no Linux.CgroupsPath. runsc creates the pause cgroup; A owns it.
2. Start actor B in the same sandbox; B attaches processes to A's pause cgroup.
3. Suspend or delete A. runsc calls Rmdir on the pause path.
4. Rmdir returns EBUSY (B is still in cgroup.procs), surfacing removing cgroup path "/sys/fs/cgroup/pause": device or resource busy and leaving stale state — matching the failure in Actor stuck in STATUS_SUSPENDING #50.

Solution

Set a relative, per-actor path:

CgroupsPath: path.Join("actors", actorTemplateNamespace, actorTemplateName, actorID, containerName),

• Relative → runsc resolves it under its own current cgroup (the sandbox pod's cgroup), so per-actor usage rolls up into the pod's kubelet/cAdvisor accounting.
• Unique per actor → each container lives in its own directory, ends up in c.Own, and is created/cleaned up cleanly with no cross-actor EBUSY.
• Layout follows the kubectl <namespace>/<name> convention (actors/<ns>/<template>/<actor>/<container>), so per-namespace and per-template totals are queryable at each level.

Spec construction is extracted into buildSpec so the new behavior is unit-testable.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[Tim Hockin (thockin)]

This seems plausible to me, but I am not deep in this area of the code yet. What I don't follow is:

1. Start actor A with no Linux.CgroupsPath. runsc creates the pause cgroup; A owns it.
2. Start actor B in the same sandbox; B attaches processes to A's pause cgroup.

How would that happen?

[Zoe Zhao (zoez7)]

I'm not sure about the root cause, but also observed the same behaviour in #288

[Benjamin Elder (BenTheElder)]

How would that happen?

Privileged containers have the host cgroups mounted. I think we should pare down the privileges, even if it remains highly permissioned, it would be better to avoid privileged: true (and use pod cgroups instead).

[Zoe Zhao (zoez7)]

The behaviour before this code change was #288, which had multiple runsc-sandboxes under the "pause" cgroup.

I pulled the PR locally and checked the actor's cgroup, the actor's cgroup is under the ateom pod's directory, but not the container's directory:

$ ps --forest -ef -ww

UID          PID    PPID  C STIME TTY          TIME CMD
root      406732       1  0 17:33 ?        00:00:00  \_ /usr/bin/containerd-shim-runc-v2 -namespace k8s.io -id ea788868aa4a85c9af824189897e2015bbba0e5a965389958b3794ecbaac32e7 -address /run/containerd/containerd.sock
root      406763  406732  0 17:33 ?        00:00:00  |   \_ /pause
root      406870  406732  0 17:33 ?        00:00:00  |   \_ /ko-app/ateom-gvisor --pod-uid=17b41a9e-2b0f-4ce6-affb-10f3ecb6c20a
root      407037  406870  0 17:35 ?        00:00:00  |       \_ runsc-gofer --root=/var/lib/ateom-gvisor/actors/ate-demo-counter:counter:c1/runsc-state --log-format=json --alsologtostderr=true gofer --bundle=/var/lib/ateom-gvisor/actors/ate-demo-counter:counter:c1/bundles/pause --gofer-mount-confs=lisafs:self,lisafs:none --io-fds=6,7 --mounts-fd=4 --rpc-fd=5 --spec-fd=3 --sync-chroot-fd=-1 --setup-root=false --sync-userns-fd=-1 --proc-mount-sync-fd=16 --apply-caps=false pause
root      407042  406870  0 17:35 ?        00:00:00  |       \_ runsc-sandbox --root=/var/lib/ateom-gvisor/actors/ate-demo-counter:counter:c1/runsc-state --log-format=json --alsologtostderr=true boot --apply-caps=false --bundle=/var/lib/ateom-gvisor/actors/ate-demo-counter:counter:c1/bundles/pause --controller-fd=8 --cpu-num=4 --cpu-period=100000 --dev-io-fd=-1 --fs-restore-fds=10,11,12,13 --gofer-filestore-fds=5 --gofer-mount-confs=lisafs:self,lisafs:none --io-fds=3,4 --mounts-fd=6 --setup-root=false --spec-fd=9 --start-sync-fd=7 --stdio-fds=14,15,16 --total-host-memory=16773165056 --total-memory=16773165056 --host-thp-defrag=madvise --proc-mount-sync-fd=24 --product-name=Google Compute Engine --host-thp-shmem-enabled=never pause
root      407108  407042  0 17:35 ?        00:00:00  |       |   \_ [exe]
root      407119  407042  0 17:35 ?        00:00:00  |       |   \_ [exe]
root      407120  407119  0 17:35 ?        00:00:00  |       |   |   \_ [exe]
root      407150  407042  0 17:35 ?        00:00:00  |       |   \_ [exe]
root      407151  407150  0 17:35 ?        00:00:00  |       |       \_ [exe]
root      407152  407150  0 17:35 ?        00:00:00  |       |       \_ [exe]
root      407153  407150  0 17:35 ?        00:00:00  |       |       \_ [exe]
root      407154  407150  0 17:35 ?        00:00:00  |       |       \_ [exe]
root      407155  407150  0 17:35 ?        00:00:00  |       |       \_ [exe]
root      407210  407150  0 17:37 ?        00:00:00  |       |       \_ [exe]
root      407133  406870  0 17:35 ?        00:00:00  |       \_ runsc-gofer --alsologtostderr=true --allow-connected-on-save=true --root=/var/lib/ateom-gvisor/actors/ate-demo-counter:counter:c1/runsc-state --log-format=json gofer --bundle=/var/lib/ateom-gvisor/actors/ate-demo-counter:counter:c1/bundles/counter --gofer-mount-confs=lisafs:self,lisafs:none,lisafs:none,lisafs:self --io-fds=6,7,8,9 --mounts-fd=4 --rpc-fd=5 --spec-fd=3 --sync-chroot-fd=-1 --sync-userns-fd=-1 --proc-mount-sync-fd=18 --apply-caps=false --setup-root=false counter

# Check the cgroup of ateom pod:
$ cat /proc/406870/cgroup
0::/kubepods.slice/kubepods-besteffort.slice/kubepods-besteffort-pod17b41a9e_2b0f_4ce6_affb_10f3ecb6c20a.slice/cri-containerd-6ee0643b3b51f5397b23b9490bc3bcddb8ef4c32cf7ac17793ed550be7517648.scope

# Check the cgroup of runsc-sandbox:
$ cat /proc/407042/cgroup
0::/kubepods.slice/kubepods-besteffort.slice/kubepods-besteffort-pod17b41a9e_2b0f_4ce6_affb_10f3ecb6c20a.slice/actors/ate-demo-counter/counter/c1/pause

which means the output of container_cpu_usage_seconds_total metric of the Ateom container will not include the actor's resource usage.

kubectl get --raw "/api/v1/nodes/${NODE}/proxy/metrics/cadvisor" | grep container_cpu_usage_seconds_total | grep "/kubepods.slice/kubepods-besteffort.slice/kubepods-besteffort-pod17b41a9e_2b0f_4ce6_affb_10f3ecb6c20a.slice/cri-containerd-6ee0643b3b51f5397b23b9490bc3bcddb8ef4c32cf7ac17793ed550be7517648.scope"

1. Let's move the actors/ directory inside the ateom container's cgroup, so kubernetes's accounting of the Ateom container's resource usage stays accurate?
2. I think atespace should be in the cgroup path too, for observing resource usage per atespace.

[Tim Hockin (thockin)]

Privileged containers have the host cgroups mounted.

I don't see that - I see what Zoe Zhao (@zoez7) said -- ateom is in the per-pod cgroup, but runsc-gofer and runsc-sandbox are in /pause.

$ k -n ate-demo-counter debug -ti counter-deployment-5c9df6f757-7jpgh --target=ateom --image=busybox
Targeting container "ateom". If you don't see processes from this container it may be because the container runtime doesn't support this feature.
Defaulting debug container name to debugger-h25bb.
All commands and output from this session will be recorded in container logs, including credentials and sensitive information passed through the command prompt.
If you don't see a command prompt, try pressing enter.

/ # ps auxw
PID   USER     TIME  COMMAND
    1 root      0:01 /ko-app/ateom-gvisor --pod-uid=b3247d04-f3b2-4a99-a07b-ad4cdb371a36
  171 root      0:00 {exe} runsc-gofer --root=/var/lib/ateom-gvisor/actors/thockin:actor1/runsc-state --log-format=json --alsologtostderr=true gofer --bundle=/var/lib
  176 root      0:00 {exe} runsc-sandbox --log-format=json --alsologtostderr=true --root=/var/lib/ateom-gvisor/actors/thockin:actor1/runsc-state boot --apply-caps=fal
  243 root      0:00 [exe]
  260 root      0:00 [exe]
  261 root      0:00 [exe]
  274 root      0:00 {exe} runsc-gofer --root=/var/lib/ateom-gvisor/actors/thockin:actor1/runsc-state --log-format=json --alsologtostderr=true --allow-connected-on-sa
  294 root      0:00 [exe]
  295 root      0:00 [exe]
  296 root      0:00 [exe]
  297 root      0:00 [exe]
  298 root      0:00 [exe]
  299 root      0:00 [exe]
  300 root      0:00 [exe]
  307 root      0:00 [exe]
  321 root      0:00 [exe]
  322 root      0:00 sh
  336 root      0:00 ps auxw

/ # cat /proc/1/cgroup 
0::/../cri-containerd-9286bdd362b6a0ec2d37ea98c410846ef9cfc59f03e785e6989c451caa3f54f3.scope

/ # cat /proc/274/cgroup 
0::/../../../../../pause

Fabricio Voznika (@fvoznika) Is this something gvisor is doing internally? Is this "correct" in some way I don't understand?

[Tim Hockin (thockin)]

Oh, I see, it's mounted AS the cgroupfs not some extra mount

[Tim Hockin (thockin)]

Tracking thru gvisor by code, since I don't know how to side-load a custom gvisor binary.

createRoot() calls c.setupCgroupForRoot() which says:

          if spec.Linux == nil || spec.Linux.CgroupsPath == "" {   
              return nil, nil, nil   
          }   

It then sees the nil and does: containerCgroup = parentCgroup - that is how we end up with "/pause". OCI allows .CgroupsPath to be empty and says that the runtime can Define a default, so "/pause" is allowed but, IMO, is wrong. It should be <current_cgroup>/<name> or something.

When I add the following:

diff --git cmd/atelet/oci.go cmd/atelet/oci.go
index 68b6f34a..9b60f22a 100644
--- cmd/atelet/oci.go
+++ cmd/atelet/oci.go
@@ -198,6 +198,7 @@ func buildActorOCISpec(atespace string, actorID string, args []string, env []str
 		Hostname: "runsc",
 		Mounts:   mounts,
 		Linux: &specs.Linux{
+			CgroupsPath: "foo",
 			Namespaces: []specs.LinuxNamespace{
 				{
 					Type: "pid",

I get different behavior. Now the current cgroup is:
/kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-besteffort.slice/kubelet-kubepods-besteffort-pod31f8531a_0fa0_4ceb_9878_ff5fd362f935.slice/cri-containerd-13b17aac315d883af5100b541b2d2d7390ef8fd29cd2c08d8b8c4bc6b4d4541b.scope
and the child is:
/kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-besteffort.slice/kubelet-kubepods-besteffort-pod31f8531a_0fa0_4ceb_9878_ff5fd362f935.slice/foo

So it took the current cgroup value, went up one level and then added the value of CgroupsPath ?

createRoot() calls c.setupCgroupForRoot() calls c.createParentCgroup() calls cgroup.NewFromPath() calls new() calls loadPaths() calls loadPathsHelper() which looks at current cgroup via /proc.

new() says this:

        if p, ok := parents[cgroup2Key]; ok && !useSystemd {                                                                           
            // The cgroup of current pid will have tasks in it and we can't use
            // that, instead, use the its parent which should not have tasks in it.
            cgroupsPath = filepath.Join(filepath.Dir(p), cgroupsPath)
        }

So there it is, but it doesn't make a lot of sense to me. We ARE allowed to have nested groups with processes, I think? Testing shows yes. Why is it going up one level to the pod's cgroup?? This was added in the first cgroups v2 PR: google/gvisor@881a271

Fabricio Voznika (@fvoznika) or someone from gVisor -- can you explain this logic? Both paths seem wrong to me.

[Fabricio Voznika (fvoznika)]

There are 2 modes of operation with cgroupsv2: regular and systemd. Because cgroupsv2 cannot have concurrent access, most people use it through systemd, which is what K8s nodes use. When nesting cgroups inside containers, you cannot map the host cgroup (with privileged containers) to manipulate it, because changes to cgroups do not synchronize with the host's systemd.

The correct way is to use pod cgroups (which Ben mentioned above). It creates a cgroup directory tree dedicated to the pod, which can be modified within the pod without racing with the host. This would also prevent the code from falling outside the pod's cgroups.

The code looking for the parent directory is actually correct. Cgroupsv2 doesn't allow nesting. Once a cgroup has procs, sub-directories cannot be created inside it anymore. This forces the code to look for the parent directory, which is the closest one that can be used to create a sub-directory in.

The empty cgroup case is not being handled correctly though. Instead of staying in the parent's cgroups, it should create a subgroup with a generic name.

[Tim Hockin (thockin)]

Thanks!

The correct way is to use pod cgroups (which Ben mentioned above). It creates a cgroup directory tree dedicated to the pod, which can be modified within the pod without racing with the host

I don't know what that means - can you explain? Is it creating a new cgroup inside the cgroup which is managed by kubelet? Or is it somehow outside of that?

The code looking for the parent directory is actually correct. Cgroupsv2 doesn't allow nesting. Once a cgroup has procs, sub-directories cannot be created inside it anymore. This forces the code to look for the parent directory, which is the closest one that can be used to create a sub-directory in.

Hmm, I thought I was testing against v2, but I guess I wasn't. This behavior is very awkward. It means that the accounting for the sandbox goes to the Pod rather than the particular container.

[Tim Hockin (thockin)]

So I was test v2, but it seems to work. But I was not setting controllers. What if:

# mount | grep cgroup
cgroup2 on /sys/fs/cgroup type cgroup2 (rw,nosuid,nodev,noexec,relatime)

# cd /sys/fs/cgroup/kubepods.slice/kubepods-besteffort.slice/kubepods-besteffort-pod4e6ff34a_f8d8_4bdf_8b0b_1f945827ce99.slice/cri-containerd-0f552b87ec46483d405433894b4abaaef3ed64d8bc819f2a2c52500a5fe720eb.scope

# cat cgroup.procs 
3202

# cat ./cgroup.controllers
cpuset cpu io memory hugetlb pids rdma

# mkdir ateom && echo OK
OK

# cat cgroup.procs > ateom/cgroup.procs && echo OK
OK

# echo "+cpuset +cpu +io +memory +hugetlb +pids +rdma" > cgroup.subtree_control && echo OK
OK

# cat ./cgroup.procs && echo DONE
DONE

# cat ./ateom/cgroup.procs 
3202

# mkdir foo && echo OK
OK

# echo $$ > ./foo/cgroup.procs && echo OK
OK

# cat ./foo/cgroup.procs 
2445898
2446182

Now we have parallel trees under the container instead of under the pod. I don't know what that will do to Kubernetes...

[Fabricio Voznika (fvoznika)]

I don't know what that means - can you explain? Is it creating a new cgroup inside the cgroup which is managed by kubelet? Or is it somehow outside of that?

cgroupsv2 allows a subtree to be delegated to another process (see CGROUPS DELEGATION section in https://man7.org/linux/man-pages/man7/cgroups.7.html). K8s will do this automatically when running a privileged container (no need to map the host's cgroup). What is does is to create a delegated cgroups directory for the container and then mount this directory as the cgroups root mount inside the container. This way, it's safe for the container to take control over its cgroups subtree without racing with systemd, and without risking falling out of the container's cgroups.

So I was test v2, but it seems to work. But I was not setting controllers. ...

That's interesting. In theory you shouldn't be allowed to have .procs and .controllers set in the same directory. The only exception is the cgroups root which is treated special. Hmmm, maybe this could be because the container cgroups is a delegate and considered the root for the container. That would allow procs and controllers in the same directory?!

[Tim Hockin (thockin)]

AIUI writing to cgroup.subtree_control changes the "responsible" cgroup to the next level of children, so this sequence is correct. I will retry it to confirm:

# mount | grep cgroup
cgroup2 on /sys/fs/cgroup type cgroup2 (rw,nosuid,nodev,noexec,relatime)

# pwd
/sys/fs/cgroup/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-pod6229766a_04b4_4600_b900_326c2a7ce808.slice/cri-containerd-db37fa2cd3796aab00a9a72509f1e286c77a51785b90e34c5c9716c48c01b5d1.scope

# cat cgroup.procs
3121

# cat ./cgroup.controllers
cpuset cpu io memory hugetlb pids rdma

# cat ../cgroup.procs  # should be empty

# cat ../cgroup.controllers 
cpuset cpu io memory hugetlb pids rdma

# cat cgroup.subtree_control  # should be empty

# cat ../cgroup.subtree_control  # should be non-empty
cpuset cpu io memory hugetlb pids rdma

# mkdir _orig  # move the current cgroup into a child

# cat cgroup.procs > _orig/cgroup.procs  # first move PIDs

# cat cgroup.procs  # should be empty

# cat _orig/cgroup.procs  # should be non-empty
3121

# cat _orig/cgroup.controllers  # should be empty

# Delegate control
# echo "+cpuset +cpu +io +memory +hugetlb +pids +rdma" > cgroup.subtree_control

# cat _orig/cgroup.controllers 
cpuset cpu io memory hugetlb pids rdma

# cat cgroup.controllers 
cpuset cpu io memory hugetlb pids rdma

# echo $$
2424042

# echo $$ > cgroup.procs  # should fail (it's "internal")
bash: echo: write error: Device or resource busy

# mkdir subcontainer

# echo $$ > subcontainer/cgroup.procs  # should be OK (it's a leaf)

# cat subcontainer/cgroup.procs 
2424042
2424592

# cat subcontainer/cgroup.controllers 
cpuset cpu io memory hugetlb pids rdma

So the question -- is this "too clever" ? What should gvisor do by default?

Should we just run the actor in the current cgroup? There's something nice about having it totally wrapped in its own cgroup, but IMO going up to the parent is totally suspect.

[Tim Hockin (thockin)]

What are we going to do with this? Doing the little dance I described above seems like it requires some deeper factoring of gvisor (mostly just passes the path around now, need to do more than that). I tried using -ignore-cgroups from Config but it seems unsupported ("flag provided but not defined: -ignore-cgroups"). We could keep it as a peer of the container but that seems wrong, too.

[Benjamin Elder (BenTheElder)]

What are we going to do with this? Doing the little dance I described above seems like it requires some deeper factoring of gvisor (mostly just passes the path around now, need to do more than that). I tried using -ignore-cgroups from Config but it seems unsupported ("flag provided but not defined: -ignore-cgroups"). We could keep it as a peer of the container but that seems wrong, too.

So for other reasons I am looking at running without privileged: true (discussed at the summit with NVIDIA folks ... we would like to pass through specific devices instead of all ...), which incidentally requires fixing this.

I have a POC locally, will follow up tomorrow.

[Benjamin Elder (BenTheElder)]

#496 is my suggested solution