diff --git a/Makefile b/Makefile index ea71473a2..c4fbb94c3 100644 --- a/Makefile +++ b/Makefile @@ -116,7 +116,8 @@ RUNTIME_LOGS := $(RUNTIME_LOG_DIR)/runsc.log.%TEST%.%TIMESTAMP%.%COMMAND% RUNTIME_ARGS ?= DOCKER_RELOAD_COMMAND ?= sudo systemctl reload docker -ifeq ($(shell stat -f -c "%T" /sys/fs/cgroup 2>/dev/null),cgroup2fs) +SYSFS_GROUP_PATH := /sys/fs/cgroup +ifeq ($(shell stat -f -c "%T" "$(SYSFS_GROUP_PATH)" 2>/dev/null),cgroup2fs) CGROUPV2 := true else CGROUPV2 := false diff --git a/runsc/cgroup/cgroup.go b/runsc/cgroup/cgroup.go index 912ba382a..728f79009 100644 --- a/runsc/cgroup/cgroup.go +++ b/runsc/cgroup/cgroup.go @@ -39,11 +39,18 @@ import ( ) const ( - cgroupRoot = "/sys/fs/cgroup" cgroupv1FsName = "cgroup" cgroupv2FsName = "cgroup2" ) +const ( + // procRoot is the procfs root this module uses. + procRoot = "/proc" + + // cgroupRoot is the cgroupfs root this module uses. + cgroupRoot = "/sys/fs/cgroup" +) + var controllers = map[string]controller{ "blkio": &blockIO{}, "cpu": &cpu{}, @@ -206,7 +213,7 @@ func countCpuset(cpuset string) (int, error) { // loadPaths loads cgroup paths for given 'pid', may be set to 'self'. func loadPaths(pid string) (map[string]string, error) { - procCgroup, err := os.Open(filepath.Join("/proc", pid, "cgroup")) + procCgroup, err := os.Open(filepath.Join(procRoot, pid, "cgroup")) if err != nil { return nil, err } @@ -214,7 +221,7 @@ func loadPaths(pid string) (map[string]string, error) { // Load mountinfo for the current process, because it's where cgroups is // being accessed from. - mountinfo, err := os.Open(filepath.Join("/proc/self/mountinfo")) + mountinfo, err := os.Open(filepath.Join(procRoot, "self/mountinfo")) if err != nil { return nil, err } diff --git a/test/root/cgroup_test.go b/test/root/cgroup_test.go index 6ba0e270e..f1ad62a1f 100644 --- a/test/root/cgroup_test.go +++ b/test/root/cgroup_test.go @@ -20,7 +20,6 @@ import ( "fmt" "io/ioutil" "os" - "os/exec" "path/filepath" "strconv" "strings" @@ -32,6 +31,18 @@ import ( "gvisor.dev/gvisor/runsc/cgroup" ) +// procPath returns a path in procfs. This is usually just `/proc + path components`, but may be +// different if HOST_PROCFS_MOUNTPOINT is set. +func procPath(components ...string) string { + return filepath.Join(append([]string{"/proc"}, components...)...) +} + +// cgroupPath returns a path in cgroupfs. This is usually just `/sys/fs/cgroup + path components`, +// but may be different if HOST_CGROUPFS_MOUNTPOINT is set. +func cgroupPath(components ...string) string { + return filepath.Join(append([]string{"/sys/fs/cgroup"}, components...)...) +} + func verifyPid(pid int, path string) error { f, err := os.Open(path) if err != nil { @@ -54,7 +65,11 @@ func verifyPid(pid int, path string) error { if scanner.Err() != nil { return scanner.Err() } - return fmt.Errorf("got: %v, want: %d", gots, pid) + wholeContents, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("got: %v, want: %d (and cannot read %s: %v)", gots, pid, path, err) + } + return fmt.Errorf("got: %v, want: %d; contents of %s:\n\n%s", gots, pid, path, string(wholeContents)) } func TestMemCgroup(t *testing.T) { @@ -91,11 +106,11 @@ func TestMemCgroup(t *testing.T) { time.Sleep(100 * time.Millisecond) // Read the cgroup memory limit. - path := filepath.Join("/sys/fs/cgroup/memory/docker", gid, "memory.limit_in_bytes") + path := cgroupPath("memory/docker", gid, "memory.limit_in_bytes") if cgroup.IsOnlyV2() { - path = filepath.Join("/sys/fs/cgroup/docker", gid, "memory.max") + path = cgroupPath("docker", gid, "memory.max") if useSystemd { - path = filepath.Join("/sys/fs/cgroup/system.slice/docker-"+gid+".scope", "memory.max") + path = cgroupPath("system.slice/docker-"+gid+".scope", "memory.max") } } // Read the cgroup memory limit. @@ -114,13 +129,13 @@ func TestMemCgroup(t *testing.T) { continue } - path = filepath.Join("/sys/fs/cgroup/memory/docker", gid, "memory.max_usage_in_bytes") + path = cgroupPath("memory/docker", gid, "memory.max_usage_in_bytes") if cgroup.IsOnlyV2() { // v2 does not have max_usage_in_bytes equivalent, so memory.current is the // next best thing that we can use - path = filepath.Join("/sys/fs/cgroup/docker", gid, "memory.current") + path = cgroupPath("docker", gid, "memory.current") if useSystemd { - path = filepath.Join("/sys/fs/cgroup/system.slice/docker-"+gid+".scope", "memory.current") + path = cgroupPath("system.slice/docker-"+gid+".scope", "memory.current") } } // Read the cgroup memory usage. @@ -290,7 +305,7 @@ func TestCgroupV1(t *testing.T) { // Check list of attributes defined above. for _, attr := range attrs { - path := filepath.Join("/sys/fs/cgroup", attr.ctrl, "docker", gid, attr.file) + path := cgroupPath(attr.ctrl, "docker", gid, attr.file) out, err := ioutil.ReadFile(path) if err != nil { if os.IsNotExist(err) && attr.skipIfNotFound { @@ -323,9 +338,9 @@ func TestCgroupV1(t *testing.T) { t.Fatalf("SandboxPid: %v", err) } for _, ctrl := range controllers { - path := filepath.Join("/sys/fs/cgroup", ctrl, "docker", gid, "cgroup.procs") + path := cgroupPath(ctrl, "docker", gid, "cgroup.procs") if err := verifyPid(pid, path); err != nil { - t.Errorf("cgroup control %q processes: %v", ctrl, err) + t.Errorf("cgroup control %q processes (%s): %v", ctrl, path, err) } } } @@ -405,9 +420,9 @@ func TestCgroupV2(t *testing.T) { if err != nil { t.Fatalf("docker run failed: %v", err) } - baseCgroupPath := "/sys/fs/cgroup/docker" + baseCgroupPath := cgroupPath("docker") if useSystemd { - baseCgroupPath = fmt.Sprintf("/sys/fs/cgroup/system.slice") + baseCgroupPath = cgroupPath("system.slice") } // Make configs. conf, hostconf, _ := d.ConfigsFrom(dockerutil.RunOpts{ @@ -464,7 +479,7 @@ func TestCgroupV2(t *testing.T) { // Check list of attributes defined above. for _, attr := range attrs { - path := filepath.Join("/sys/fs/cgroup/docker", gid, attr.file) + path := cgroupPath("docker", gid, attr.file) if useSystemd { path = filepath.Join(baseCgroupPath, "docker-"+gid+".scope", attr.file) } @@ -486,12 +501,12 @@ func TestCgroupV2(t *testing.T) { if err != nil { t.Fatalf("SandboxPid: %v", err) } - path := filepath.Join("/sys/fs/cgroup/docker", gid, "cgroup.procs") + path := cgroupPath("docker", gid, "cgroup.procs") if useSystemd { path = filepath.Join(baseCgroupPath, "docker-"+gid+".scope", "cgroup.procs") } if err := verifyPid(pid, path); err != nil { - t.Errorf("cgroup control processes: %v", err) + t.Errorf("cgroup control processes (%s): %v", path, err) } } @@ -536,14 +551,9 @@ func TestCgroupParent(t *testing.T) { // Finds cgroup for the sandbox's parent process to check that cgroup is // created in the right location relative to the parent. - cmd := fmt.Sprintf("grep PPid: /proc/%d/status | sed 's/PPid:\\s//'", pid) - ppidStr, err := exec.Command("bash", "-c", cmd).CombinedOutput() + ppid, err := getParentPID(pid) if err != nil { - t.Fatalf("Executing %q: %v", cmd, err) - } - ppid, err := strconv.Atoi(strings.TrimSpace(string(ppidStr))) - if err != nil { - t.Fatalf("invalid PID (%s): %v", ppidStr, err) + t.Fatalf("cannot get parent of %d: %v", pid, err) } cgroups, err := cgroup.NewFromPid(ppid, false /* useSystemd */) if err != nil { @@ -554,6 +564,6 @@ func TestCgroupParent(t *testing.T) { path = filepath.Join(cgroups.MakePath("cpuacct"), parent, "docker-"+gid+".scope", "cgroup.procs") } if err := verifyPid(pid, path); err != nil { - t.Errorf("cgroup control %q processes: %v", "memory", err) + t.Errorf("cgroup control %q processes (%s): %v", "cpuacct", path, err) } } diff --git a/test/root/chroot_test.go b/test/root/chroot_test.go index 5114a9602..4058582c8 100644 --- a/test/root/chroot_test.go +++ b/test/root/chroot_test.go @@ -19,6 +19,7 @@ import ( "context" "fmt" "io/ioutil" + "os" "os/exec" "path/filepath" "strconv" @@ -28,6 +29,19 @@ import ( "gvisor.dev/gvisor/pkg/test/dockerutil" ) +func getParentPID(childPID int) (int, error) { + cmd := fmt.Sprintf("grep PPid: %s | sed 's/PPid:\\s//'", procPath(strconv.Itoa(childPID), "status")) + parent, err := exec.Command("bash", "-c", cmd).CombinedOutput() + if err != nil { + return -1, fmt.Errorf("failed to fetch parent PID of %d: %v, out:\n%s", childPID, err, string(parent)) + } + parentPID, err := strconv.Atoi(strings.TrimSpace(string(parent))) + if err != nil { + return -1, fmt.Errorf("failed to parse PPID %q: %v", string(parent), err) + } + return parentPID, nil +} + // TestChroot verifies that the sandbox is chroot'd and that mounts are cleaned // up after the sandbox is destroyed. func TestChroot(t *testing.T) { @@ -47,7 +61,7 @@ func TestChroot(t *testing.T) { } // Check that sandbox is chroot'ed. - procRoot := filepath.Join("/proc", strconv.Itoa(pid), "root") + procRoot := procPath(strconv.Itoa(pid), "root") chroot, err := filepath.EvalSymlinks(procRoot) if err != nil { t.Fatalf("error resolving /proc//root symlink: %v", err) @@ -56,7 +70,7 @@ func TestChroot(t *testing.T) { t.Errorf("sandbox is not chroot'd, it should be inside: /, got: %q", chroot) } - path, err := filepath.EvalSymlinks(filepath.Join("/proc", strconv.Itoa(pid), "cwd")) + path, err := filepath.EvalSymlinks(procPath(strconv.Itoa(pid), "cwd")) if err != nil { t.Fatalf("error resolving /proc//cwd symlink: %v", err) } @@ -102,39 +116,56 @@ func TestChrootGofer(t *testing.T) { } // Find sandbox's parent PID. - cmd := fmt.Sprintf("grep PPid /proc/%d/status | awk '{print $2}'", sandPID) - parent, err := exec.Command("sh", "-c", cmd).CombinedOutput() + parentPID, err := getParentPID(sandPID) if err != nil { - t.Fatalf("failed to fetch runsc (%d) parent PID: %v, out:\n%s", sandPID, err, string(parent)) - } - parentPID, err := strconv.Atoi(strings.TrimSpace(string(parent))) - if err != nil { - t.Fatalf("failed to parse PPID %q: %v", string(parent), err) + t.Fatalf("failed to fetch runsc parent PID: %v", err) } // Get all children from parent. - childrenOut, err := exec.Command("/usr/bin/pgrep", "-P", strconv.Itoa(parentPID)).CombinedOutput() + var childrenPIDs []int + procfsRoot := procPath() + procDirs, err := os.ReadDir(procfsRoot) if err != nil { - t.Fatalf("failed to fetch containerd-shim children: %v", err) + t.Fatalf("cannot list processes in %s: %s", procfsRoot, err) + } + for _, procDir := range procDirs { + if !procDir.IsDir() { + continue + } + procPID, err := strconv.Atoi(procDir.Name()) + if err != nil { + // We only care about directories that are PIDs. + continue + } + // Now check if it is a child of parentPID. + parent, err := getParentPID(procPID) + if err != nil { + // Skip, this may be a race condition with a process that has since gone away. + t.Logf("Non-fatal warning: cannot get parent PID of %d (process likely gone): %v", procPID, err) + continue + } + if parent == parentPID { + t.Logf("runsc parent PID %d has child PID %d", parentPID, procPID) + childrenPIDs = append(childrenPIDs, procPID) + } + } + // Ensure we have seen at least one child PID. + if len(childrenPIDs) == 0 { + t.Fatalf("Found no children of runsc parent PID %d", parentPID) } - children := strings.Split(strings.TrimSpace(string(childrenOut)), "\n") // This where the root directory is mapped on the host and that's where the // gofer must have chroot'd to. root := "/root" - for _, child := range children { - childPID, err := strconv.Atoi(child) - if err != nil { - t.Fatalf("failed to parse child PID %q: %v", child, err) - } + for _, childPID := range childrenPIDs { if childPID == sandPID { // Skip the sandbox, all other immediate children are gofers. continue } // Check that gofer is chroot'ed. - chroot, err := filepath.EvalSymlinks(filepath.Join("/proc", child, "root")) + chroot, err := filepath.EvalSymlinks(procPath(strconv.Itoa(childPID), "root")) if err != nil { t.Fatalf("error resolving /proc//root symlink: %v", err) } @@ -142,7 +173,7 @@ func TestChrootGofer(t *testing.T) { t.Errorf("gofer chroot is wrong, want: %q, got: %q", root, chroot) } - path, err := filepath.EvalSymlinks(filepath.Join("/proc", child, "cwd")) + path, err := filepath.EvalSymlinks(procPath(strconv.Itoa(childPID), "cwd")) if err != nil { t.Fatalf("error resolving /proc//cwd symlink: %v", err) } diff --git a/tools/bazel.mk b/tools/bazel.mk index 4d3a71bc8..2ae9d8843 100644 --- a/tools/bazel.mk +++ b/tools/bazel.mk @@ -111,16 +111,6 @@ DOCKER_RUN_OPTIONS += -v "$(KERNEL_HEADERS_DIR_LINKED):$(KERNEL_HEADERS_DIR_LINK endif endif -# Same for systemd-related files and directories. This allows control of systemd -# from within the container, which is useful for tests that need to e.g. restart -# docker. -ifneq (,$(wildcard /run/systemd/system)) -DOCKER_RUN_OPTIONS += -v "/run/systemd/system:/run/systemd/system" -endif -ifneq (,$(wildcard /var/run/dbus/system_bus_socket)) -DOCKER_RUN_OPTIONS += -v "/var/run/dbus/system_bus_socket:/var/run/dbus/system_bus_socket" -endif - # Add basic UID/GID options. # # Note that USERADD_DOCKER and GROUPADD_DOCKER are both defined as "deferred" @@ -215,6 +205,7 @@ endif @docker run -d --name $(DOCKER_NAME) --hostname $(DOCKER_HOSTNAME) \ -v "$(CURDIR):$(CURDIR)" \ --workdir "$(CURDIR)" \ + --pid=host --cgroupns=host \ $(DOCKER_RUN_OPTIONS) \ gvisor.dev/images/builder \ bash -c "set -x; tail -f --pid=\$$($(BAZEL) info server_pid) /dev/null" diff --git a/tools/install_containerd.sh b/tools/install_containerd.sh index af5e50c62..f70677018 100755 --- a/tools/install_containerd.sh +++ b/tools/install_containerd.sh @@ -27,7 +27,8 @@ if [[ "${CONTAINERD_MAJOR}" -eq 1 ]] && [[ "${CONTAINERD_MINOR}" -le 4 ]]; then fi # containerd < 1.4 doesn't work with cgroupv2 setup, so we check for that here -if [[ "$(stat -f -c %T /sys/fs/cgroup 2>/dev/null)" == "cgroup2fs" && "${CONTAINERD_MAJOR}" -eq 1 && "${CONTAINERD_MINOR}" -lt 4 ]]; then +SYSFS_ROOT=/sys/fs/cgroup +if [[ "$(stat -f -c %T "$SYSFS_ROOT" 2>/dev/null)" == "cgroup2fs" && "${CONTAINERD_MAJOR}" -eq 1 && "${CONTAINERD_MINOR}" -lt 4 ]]; then echo "containerd < 1.4 does not work with cgroup2" exit 1 fi