umount all mount points in runc root dir

[lifubang]

Sometimes, umount with MNT_DETACH flag can't work in a small probability.
So, maybe we should unmount all mount points in runc root dir when we remove this dir.

root@f7c63618dde3:/opt/busybox# runc delete test2
remove /run/runc/test2: unlinkat /run/runc/test2/runc.W24K2t: device or resource busy
ERRO[0000] container "test2" does not exist             

root@f7c63618dde3:/opt/busybox# ls /run/runc/test2 -alh
total 14M
drwx--x--x 2 root root  60 Mar  9 14:24 .
drwx------ 5 root root 100 Mar  9 15:15 ..
-rwxr-xr-x 1 root root 14M Mar  9 15:15 runc.W24K2t

Signed-off-by: lifubang [email protected]

[kolyshkin]

Sometimes, umount with MNT_DETACH flag can't work in a small probability.

You are talking about the try_bindfd function from the cloned_binary.c, right?

What you propose is a workaround. I'd rather understand and fix the actual bug instead.

[kolyshkin]

@lifubang do you have a repro? Have you checked that there's no other runc processes running for this container?

I stared at the code and can't think of any scenarios (except, obviously, for kernel bugs) in which this mount would be left mounted.

[kolyshkin]

Interesting, I just caught the same bug in CI (CentOS 7, from https://github.com/opencontainers/runc/pull/2848/checks?check_run_id=2089879609):

ok 8 runc exec (cgroup v2 + init process in non-root cgroup) succeeds # skip test requires cgroups_v2
not ok 9 runc run (cgroup v1 + unified resources should fail)
# (from function `teardown_bundle' in file tests/integration/helpers.bash, line 481,
#  from function `teardown' in test file tests/integration/cgroups.bats, line 6)
#   `teardown_bundle' failed
# runc spec (status=0):
# 
# runc run -d --console-socket /tmp/bats-run-20054/runc.BNKN97/tty/sock test_cgroups_unified (status=1):
# time="2021-03-11T21:14:39Z" level=warning msg="unable to get oom kill count" error="no directory specified for memory.oom_control"
# time="2021-03-11T21:14:39Z" level=error msg="unlinkat /tmp/bats-run-20054/runc.BNKN97/state/test_cgroups_unified/runc.yuvKqg: device or resource busy"
# time="2021-03-11T21:14:39Z" level=error msg="container_linux.go:367: starting container process caused: process_linux.go:365: applying cgroup configuration for process caused: invalid configuration: cannot use unified on cgroup v1"
# load container test_cgroups_unified: container "test_cgroups_unified" does not exist
# rm: cannot remove ‘/tmp/bats-run-20054/runc.BNKN97/state/test_cgroups_unified/runc.yuvKqg’: Device or resource busy
ok 10 runc run (blkio weight) # skip test requires cgroups_v2

(the "cannot use unified on cgroup v1" error is expected)

[kolyshkin]

[root@centos70-ne runc]# RUNC_USE_SYSTEMD=yes bats tests/integration/cgroups.bats 
 - runc update --kernel-memory{,-tcp} (initialized) (skipped)
 - runc update --kernel-memory (uninitialized) (skipped)
 ✓ runc create (no limits + no cgrouppath + no permission) succeeds
 - runc create (rootless + no limits + cgrouppath + no permission) fails with permission error (skipped: test requires rootless)
 - runc create (rootless + limits + no cgrouppath + no permission) fails with informative error (skipped: test requires rootless)
 ✓ runc create (limits + cgrouppath + permission on the cgroup dir) succeeds
 ✓ runc exec (limits + cgrouppath + permission on the cgroup dir) succeeds
 - runc exec (cgroup v2 + init process in non-root cgroup) succeeds (skipped: test requires cgroups_v2)
 ✗ runc run (cgroup v1 + unified resources should fail)
   (in test file tests/integration/cgroups.bats, line 187)
     `false' failed
   runc spec (status=0):
   
   runc run -d --console-socket /tmp/bats-run-19767/runc.Cc1tr5/tty/sock test_cgroups_unified (status=1):
   time="2021-03-11T15:02:16-08:00" level=warning msg="unable to get oom kill count" error="no directory specified for memory.oom_control"
   time="2021-03-11T15:02:16-08:00" level=error msg="unlinkat /tmp/bats-run-19767/runc.Cc1tr5/state/test_cgroups_unified/runc.dX6t6k: device or resource busy"
   time="2021-03-11T15:02:16-08:00" level=error msg="container_linux.go:367: starting container process caused: process_linux.go:365: applying cgroup configuration for process caused: invalid configuration: cannot use unified on cgroup v1"
   load container test_cgroups_unified: container "test_cgroups_unified" does not exist
   rm: cannot remove ‘/tmp/bats-run-19767/runc.Cc1tr5/state/test_cgroups_unified/runc.dX6t6k’: Device or resource busy
 - runc run (blkio weight) (skipped: test requires cgroups_v2)
 - runc run (cgroup v2 resources.unified only) (skipped: test requires cgroups_v2)
 - runc run (cgroup v2 resources.unified override) (skipped: test requires cgroups_v2)

12 tests, 1 failure, 8 skipped

rm: cannot remove ‘/tmp/bats-run-19767/runc.Cc1tr5/state/test_cgroups_unified/runc.dX6t6k’: Device or resource busy
rm: cannot remove ‘/tmp/bats-run-19767/runc.Cc1tr5/state/test_cgroups_unified/runc.dX6t6k’: Device or resource busy

My guess is this is some kind of kernel race in RHEL7 kernel (between mount/umount and mount propagation). Not sure why we haven't seen it before though.

[kolyshkin]

OK this happens because the child (runc init) is killed in the middle of setting up /proc/self/exe bindmount.

[kolyshkin]

...and we haven't seen it before because before #2812 the process was killed a tad earlier.

I love those bugs that are exposed by adding a line of code here or there, disturbing the precise timing of events that resulted in things happened to work 😆

[kolyshkin]

I suggest we close this one in favor of #2855. WDYT @lifubang ?