Fix cgroup when path is relative

This can happen when 'docker run --cgroup-parent=' flag is set.

PiperOrigin-RevId: 235645559
Change-Id: Ieea3ae66939abadab621053551bf7d62d412e7ee

6 files changed, 141 insertions, 41 deletions

diff --git a/runsc/cgroup/cgroup.go b/runsc/cgroup/cgroup.go
index 87f051e79..2b338b6c6 100644
--- a/runsc/cgroup/cgroup.go
+++ b/runsc/cgroup/cgroup.go
@@ -161,20 +161,59 @@ func countCpuset(cpuset string) (int, error) {
 	return count, nil
 }
 
+// LoadPaths loads cgroup paths for given 'pid', may be set to 'self'.
+func LoadPaths(pid string) (map[string]string, error) {
+	f, err := os.Open(filepath.Join("/proc", pid, "cgroup"))
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+
+	paths := make(map[string]string)
+	scanner := bufio.NewScanner(f)
+	for scanner.Scan() {
+		// Format: ID:controller1,controller2:path
+		// Example: 2:cpu,cpuacct:/user.slice
+		tokens := strings.Split(scanner.Text(), ":")
+		if len(tokens) != 3 {
+			return nil, fmt.Errorf("invalid cgroups file, line: %q", scanner.Text())
+		}
+		for _, ctrlr := range strings.Split(tokens[1], ",") {
+			paths[ctrlr] = tokens[2]
+		}
+	}
+	if err := scanner.Err(); err != nil {
+		return nil, err
+	}
+	return paths, nil
+}
+
 // Cgroup represents a group inside all controllers. For example: Name='/foo/bar'
 // maps to /sys/fs/cgroup/<controller>/foo/bar on all controllers.
 type Cgroup struct {
-	Name string `json:"name"`
-	Own  bool   `json:"own"`
+	Name    string            `json:"name"`
+	Parents map[string]string `json:"parents"`
+	Own     bool              `json:"own"`
 }
 
 // New creates a new Cgroup instance if the spec includes a cgroup path.
 // Returns nil otherwise.
-func New(spec *specs.Spec) *Cgroup {
+func New(spec *specs.Spec) (*Cgroup, error) {
 	if spec.Linux == nil || spec.Linux.CgroupsPath == "" {
-		return nil
+		return nil, nil
+	}
+	var parents map[string]string
+	if !filepath.IsAbs(spec.Linux.CgroupsPath) {
+		var err error
+		parents, err = LoadPaths("self")
+		if err != nil {
+			return nil, fmt.Errorf("finding current cgroups: %v", err)
+		}
 	}
-	return &Cgroup{Name: spec.Linux.CgroupsPath}
+	return &Cgroup{
+		Name:    spec.Linux.CgroupsPath,
+		Parents: parents,
+	}, nil
 }
 
 // Install creates and configures cgroups according to 'res'. If cgroup path
@@ -188,9 +227,11 @@ func (c *Cgroup) Install(res *specs.LinuxResources) error {
 		return nil
 	}
 
-	// Mark that cgroup resources are owned by me.
 	log.Debugf("Creating cgroup %q", c.Name)
+
+	// Mark that cgroup resources are owned by me.
 	c.Own = true
+
 	// The Cleanup object cleans up partially created cgroups when an error occurs.
 	// Errors occuring during cleanup itself are ignored.
 	clean := specutils.MakeCleanup(func() { _ = c.Uninstall() })
@@ -247,32 +288,19 @@ func (c *Cgroup) Uninstall() error {
 func (c *Cgroup) Join() (func(), error) {
 	// First save the current state so it can be restored.
 	undo := func() {}
-	f, err := os.Open("/proc/self/cgroup")
+	paths, err := LoadPaths("self")
 	if err != nil {
 		return undo, err
 	}
-	defer f.Close()
-
 	var undoPaths []string
-	scanner := bufio.NewScanner(f)
-	for scanner.Scan() {
-		// Format: ID:controller1,controller2:path
-		// Example: 2:cpu,cpuacct:/user.slice
-		tokens := strings.Split(scanner.Text(), ":")
-		if len(tokens) != 3 {
-			return undo, fmt.Errorf("formatting cgroups file, line: %q", scanner.Text())
-		}
-		for _, ctrlr := range strings.Split(tokens[1], ",") {
-			// Skip controllers we don't handle.
-			if _, ok := controllers[ctrlr]; ok {
-				undoPaths = append(undoPaths, filepath.Join(cgroupRoot, ctrlr, tokens[2]))
-				break
-			}
+	for ctrlr, path := range paths {
+		// Skip controllers we don't handle.
+		if _, ok := controllers[ctrlr]; ok {
+			fullPath := filepath.Join(cgroupRoot, ctrlr, path)
+			undoPaths = append(undoPaths, fullPath)
+			break
 		}
 	}
-	if err := scanner.Err(); err != nil {
-		return undo, err
-	}
 
 	// Replace empty undo with the real thing before changes are made to cgroups.
 	undo = func() {
@@ -316,7 +344,11 @@ func (c *Cgroup) MemoryLimit() (uint64, error) {
 }
 
 func (c *Cgroup) makePath(controllerName string) string {
-	return filepath.Join(cgroupRoot, controllerName, c.Name)
+	path := c.Name
+	if parent, ok := c.Parents[controllerName]; ok {
+		path = filepath.Join(parent, c.Name)
+	}
+	return filepath.Join(cgroupRoot, controllerName, path)
 }
 
 type controller interface {
diff --git a/runsc/container/container.go b/runsc/container/container.go
index 08a3725f5..6f092a5ce 100644
--- a/runsc/container/container.go
+++ b/runsc/container/container.go
@@ -295,7 +295,10 @@ func Create(id string, spec *specs.Spec, conf *boot.Config, bundleDir, consoleSo
 
 		// Create and join cgroup before processes are created to ensure they are
 		// part of the cgroup from the start (and all tneir children processes).
-		cg := cgroup.New(spec)
+		cg, err := cgroup.New(spec)
+		if err != nil {
+			return nil, err
+		}
 		if cg != nil {
 			// If there is cgroup config, install it before creating sandbox process.
 			if err := cg.Install(spec.Linux.Resources); err != nil {
diff --git a/runsc/test/root/BUILD b/runsc/test/root/BUILD
index 75826a521..7ded78baa 100644
--- a/runsc/test/root/BUILD
+++ b/runsc/test/root/BUILD
@@ -24,6 +24,7 @@ go_test(
         "local",
     ],
     deps = [
+        "//runsc/cgroup",
         "//runsc/specutils",
         "//runsc/test/root/testdata",
         "//runsc/test/testutil",
diff --git a/runsc/test/root/cgroup_test.go b/runsc/test/root/cgroup_test.go
index 0eabf9561..91839048c 100644
--- a/runsc/test/root/cgroup_test.go
+++ b/runsc/test/root/cgroup_test.go
@@ -15,16 +15,45 @@
 package root
 
 import (
+	"bufio"
+	"fmt"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
 
+	"gvisor.googlesource.com/gvisor/runsc/cgroup"
 	"gvisor.googlesource.com/gvisor/runsc/test/testutil"
 )
 
+func verifyPid(pid int, path string) error {
+	f, err := os.Open(path)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+
+	var gots []int
+	scanner := bufio.NewScanner(f)
+	for scanner.Scan() {
+		got, err := strconv.Atoi(scanner.Text())
+		if err != nil {
+			return err
+		}
+		if got == pid {
+			return nil
+		}
+		gots = append(gots, got)
+	}
+	if scanner.Err() != nil {
+		return scanner.Err()
+	}
+	return fmt.Errorf("got: %s, want: %d", gots, pid)
+}
+
 // TestCgroup sets cgroup options and checks that cgroup was properly configured.
 func TestCgroup(t *testing.T) {
 	if err := testutil.Pull("alpine"); err != nil {
@@ -161,12 +190,48 @@ func TestCgroup(t *testing.T) {
 	}
 	for _, ctrl := range controllers {
 		path := filepath.Join("/sys/fs/cgroup", ctrl, "docker", gid, "cgroup.procs")
-		out, err := ioutil.ReadFile(path)
-		if err != nil {
-			t.Fatalf("failed to read %q: %v", path, err)
-		}
-		if got := string(out); !strings.Contains(got, strconv.Itoa(pid)) {
-			t.Errorf("cgroup control %s processes, got: %q, want: %q", ctrl, got, pid)
+		if err := verifyPid(pid, path); err != nil {
+			t.Errorf("cgroup control %q processes: %v", ctrl, err)
 		}
 	}
 }
+
+func TestCgroupParent(t *testing.T) {
+	if err := testutil.Pull("alpine"); err != nil {
+		t.Fatal("docker pull failed:", err)
+	}
+	d := testutil.MakeDocker("cgroup-test")
+
+	parent := testutil.RandomName("runsc")
+	if err := d.Run("--cgroup-parent", parent, "alpine", "sleep", "10000"); err != nil {
+		t.Fatal("docker create failed:", err)
+	}
+	defer d.CleanUp()
+	gid, err := d.ID()
+	if err != nil {
+		t.Fatalf("Docker.ID() failed: %v", err)
+	}
+	t.Logf("cgroup ID: %s", gid)
+
+	// Check that sandbox is inside cgroup.
+	pid, err := d.SandboxPid()
+	if err != nil {
+		t.Fatalf("SandboxPid: %v", err)
+	}
+
+	// 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)
+	ppid, err := exec.Command("bash", "-c", cmd).CombinedOutput()
+	if err != nil {
+		t.Fatalf("Executing %q: %v", cmd, err)
+	}
+	cgroups, err := cgroup.LoadPaths(strings.TrimSpace(string(ppid)))
+	if err != nil {
+		t.Fatalf("cgroup.LoadPath(%s): %v", ppid, err)
+	}
+	path := filepath.Join("/sys/fs/cgroup/memory", cgroups["memory"], parent, gid, "cgroup.procs")
+	if err := verifyPid(pid, path); err != nil {
+		t.Errorf("cgroup control %q processes: %v", "memory", err)
+	}
+}
diff --git a/runsc/test/testutil/docker.go b/runsc/test/testutil/docker.go
index 5a92a5835..bce609061 100644
--- a/runsc/test/testutil/docker.go
+++ b/runsc/test/testutil/docker.go
@@ -18,7 +18,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"log"
-	"math/rand"
 	"os"
 	"os/exec"
 	"path"
@@ -31,10 +30,6 @@ import (
 	"github.com/kr/pty"
 )
 
-func init() {
-	rand.Seed(time.Now().UnixNano())
-}
-
 func getRuntime() string {
 	r := os.Getenv("RUNSC_RUNTIME")
 	if r == "" {
@@ -162,8 +157,7 @@ type Docker struct {
 // MakeDocker sets up the struct for a Docker container.
 // Names of containers will be unique.
 func MakeDocker(namePrefix string) Docker {
-	suffix := fmt.Sprintf("-%06d", rand.Int())[:7]
-	return Docker{Name: namePrefix + suffix, Runtime: getRuntime()}
+	return Docker{Name: RandomName(namePrefix), Runtime: getRuntime()}
 }
 
 // Create calls 'docker create' with the arguments provided.
diff --git a/runsc/test/testutil/testutil.go b/runsc/test/testutil/testutil.go
index a84530287..79f0a8b6b 100644
--- a/runsc/test/testutil/testutil.go
+++ b/runsc/test/testutil/testutil.go
@@ -461,3 +461,8 @@ func WriteTmpFile(pattern, text string) (string, error) {
 	}
 	return file.Name(), nil
 }
+
+// RandomName create a name with a 6 digit random number appended to it.
+func RandomName(prefix string) string {
+	return fmt.Sprintf("%s-%06d", prefix, rand.Int31n(1000000))
+}