don't panic when /sys/fs/cgroup is missing for rootless

[coryb]

Fixes issue #2573

Downgrade the panic to a debug message when /sys/fs/cgroup is missing. This issue is preventing runc exec from working within buildkit when running rootless (where /sys is not mounted at all).

Steps to reproduce:

$ mkdir ~/mycontainer
$ cd ~/mycontainer
$ mkdir rootfs
$ docker export $(docker create busybox) | tar -C rootfs -xvf -
$ runc spec --rootless
<edit config.json to remove /sys mount>
$ runc --root /tmp/runc run mycontainerid
# 

now in separate shell:

cd ~/mycontainer
runc --root /tmp/runc exec --tty mycontainerid /bin/sh
# 

A potentially better solution is to change the signature from:

func IsCgroup2UnifiedMode() bool

to:

func IsCgroup2UnifiedMode() (bool, error)

so that we can manage the error more directly in rootless mode, but the diff for this change would be much larger, since the function is used quite a lot. I can update is this direction is preferred.

[kolyshkin]

1. It should be Info not Debug I think as it's pretty major.
2. Amend the message to say "assuming cgroup v1".

[kolyshkin]

3. Maybe add a TODO item to change the default to cgroupv2 somewhere in 2024 :)

[coryb]

yeah, this is partially why I am leaning towards changing the signature to return an error, otherwise users will get this message 100% of the time exec'ing to a container via buildkit with rootless.

[coryb]

Does something like this seem more reasonable to you?

		err := unix.Statfs(unifiedMountpoint, &st)
		if err != nil && os.IsNotExist(err) {
			// /sys/fs/cgroup not found, likely rootless
			logrus.WithError(err).Debugf("%s missing, assuming cgroup v1", unifiedMountpoint)
			isUnified = false
			return
		} else if err != nil {
			panic(fmt.Sprintf("cannot statfs cgroup root: %s", err))
		}

This would preserve the panic behavior for all error scenarios except where the /sys/fs/cgroup path simply does not exist (which is a normal situation under rootless)

[AkihiroSuda]

Also better to panic when os.IsNotExist && !system.RunningInUserNS()

[coryb]

updated the check to only ignore (w/ debug msg) errors for NotExists while RunningInUserNS

[kolyshkin]

@AkihiroSuda PTAL

[AkihiroSuda]

Thanks

[kolyshkin]

couple of nits

[coryb]

The test failure (timeout) seems unrelated to my change, and Travis is not allowing me to retry the build.

[coryb]

I squashed and forced push, tests passed after the push this time. I think I have addressed all the feedback, so hopefully we can get this merged in.

[AkihiroSuda]

ping @kolyshkin @cyphar @mrunalp

[AkihiroSuda]

@kolyshkin @cyphar @mrunalp PTAL

[cyphar]

LGTM.

outdated