podman run --cgroups=disabled: missing "supervisor"

[edsantiago]

One of the integration tests is flaking when reading /proc/NNN/cgroup:

$ podman run --name testctr -d --cgroups=disabled quay.io/libpod/alpine:latest top
$ podman inspect testctr
...
$ (read /proc/X/cgroup)
       Expected
               <string>: 0::/system.slice/google-startup-scripts.service
           to equal
               <string>: 0::/system.slice/google-startup-scripts.service/supervisor

Podman run [It] podman run with cgroups=disabled runs without cgroups

• fedora-33 : int podman fedora-33 root host
  • PR skip flaking auto-update test #11176
    • 08-10 05:55
  • PR Alias build to buildx, so it won't fail #11134
    • 08-09 13:54
  • PR Copy the content from the underlying image into the newly created volume #10970
    • 07-20 12:27
  • PR Support log_tag defaults from containers.conf #10583
    • 06-23 13:43
• fedora-34 : int podman fedora-34 root host
  • PR skip flaking auto-update test #11176
    • 08-10 05:58
  • PR Fix handling of user specified container labels #11101
    • 08-02 18:00
  • PR Bump github.com/cyphar/filepath-securejoin from 0.2.2 to 0.2.3 #10860
    • 07-05 09:08
  • PR Scrub podman commands to use report package #10688
    • 06-16 20:43
  • PR Fix building static podman-remote #10679
    • 06-14 15:48
• ubuntu-2104 : int podman ubuntu-2104 root host
  • PR Fix handling of selinux labels in podman play kube #10992
    • 07-20 09:29
  • PR Handle image user and exposed ports in podman play kube #10383
    • 05-26 17:23

[rhatdan]

Can we change the test to be less specific? I /supervisor critical to the test?

[edsantiago]

The test itself doesn't have any supervisor in it; it's just comparing /proc/self/cgroup to /proc/CONTAINERPID/cgroup. (I think). git blames @mheon in #3581. Matt, how's your memory?

[mheon]

This test verifies that the container is running as the same cgroup that launched it if we run with --cgroups=disabled. An exact match is important as such. The test itself is sound. I can think of no obvious reason why it would not be failing.

[rhatdan]

@giuseppe thoughts?

[giuseppe]

we create the /supervisor sub-cgroup only when the cgroups mode is set to --cgroups=split:

	if ctr.config.CgroupsMode == cgroupSplit {
		if err := utils.MoveUnderCgroupSubtree("supervisor"); err != nil {
			return err
		}
	}

I don't see any other way of ending up under /supervisor.

One thing we could try to make the test more robust is to avoid going through its PID to read the cgroup with --cgroupns=host as:

podman run --name testctr --rm  --cgroupns=host --cgroups=disabled quay.io/libpod/alpine:latest cat /proc/self/cgroup

[github-actions]

A friendly reminder that this issue had no activity for 30 days.

[rhatdan]

@giuseppe @edsantiago @mheon any movement on this issue?

[giuseppe]

opened a PR to simplify the test: #11561

[edsantiago]

#11561 has merged. Closing, with fingers crossed.

[edsantiago]

This is happening again, sort of, except now it's the opposite:

Expected
      <string>: 0::/system.slice/google-startup-scripts.service/supervisor
  to equal
      <string>: 0::/system.slice/google-startup-scripts.service

(it used to be the other way around).

Podman run [It] podman run with cgroups=disabled runs without cgroups

• fedora-34 : int podman fedora-34 root host
  • PR Cirrus: Fix defunct package metadata breaking cache #11834
    • 10-01 16:37
    • 10-01 16:37
    • 10-01 16:37
  • PR added healthcheck to ps command #11609
    • 09-24 13:53
    • 09-24 12:53
    • 09-24 12:53
    • 09-24 11:48
    • 09-24 11:48
• ubuntu-2104 : int podman ubuntu-2104 root host
  • PR [3.4] vendor c/[email protected] #11655
    • 09-20 09:14

[giuseppe]

I think these old PRs were failing because they were not rebased yet to include the fix.

Have you seen again this flake in the last week?

[edsantiago]

No, not since the 10-01 instances reported above. Okay, I'll close again - thank you!

[edsantiago]

Podman run [It] podman run with cgroups=disabled runs without cgroups

• fedora-34 : int podman fedora-34 root host
  • PR Generate Kube should not print default structs #12021
    • 10-19 09:20
    • 10-19 09:20

[cevich]

Note: google-startup-scripts.service is "special". It's run early on by systemd during boot with "unconfined" SELinux labels. This is responsible for executing the Cirrus-CI agent process, which in turn executes the commands from the YAML. i.e. this is a case where running the test in CI is actually different from running it manually (esp. as far as SELinux is concerned). The process tree should look something like:

systemd -> google-startup-scripts.service -> cirrus-agent (name unknown) -> bash -> make

@giuseppe what does this supervisor node presence signify?. Do we even care about this node? I could easily modify the test to pass with or without /supervisor. That would be an easy "fix".

[cevich]

(Re-opening, since clearly the issue has popped up again after rebasing.)

[cevich]

@giuseppe as discussed, there's a cgroup name-clash (supervisor) that's used by both podman and systemd(?). In order to debug this, you'd like to rename the podman one and then see which name shows up in a subsequent test-failure. For now, I will disable the test in #11795 with a Skip() mentioning this issue.

[giuseppe]

@cevich @edsantiago opened a PR to help debugging this issue: #12120

[cevich]

@giuseppe It reproduced again with your new name (log)

         • Failure [1.763 seconds]
         Podman run
         /var/tmp/go/src/github.com/containers/podman/test/e2e/run_test.go:25
           podman run with cgroups=disabled runs without cgroups [It]
           /var/tmp/go/src/github.com/containers/podman/test/e2e/run_test.go:1396
         
           Expected
               <string>: 0::/system.slice/google-startup-scripts.service/runtime
           to equal
               <string>: 0::/system.slice/google-startup-scripts.service

So that means it cannot be systemd, it must be a podman or crun bug, eh?

[giuseppe]

yeah, it seems it is all our fault :/

[giuseppe]

another attempt:#12162

[cevich]

yeah, it seems it is all our fault :/

Proof positive that no matter how hard we try, we're still human. But a positive way of looking at it is: At least we don't need to patch systemd + wait for a new version to be released + incorporate that into our VMs 😁