Merge pull request #1224 from Random-Liu/infer-systemd-cgroup

Infer systemd cgroup
containerd · Aug 6, 2019 · c7b48c0 · c7b48c0
2 parents f9760af + eae5fc3
commit c7b48c0
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 15 deletions.
diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go
@@ -408,8 +408,7 @@ func (c *criService) generateContainerSpec(id string, sandboxID string, sandboxP
 	} else {
 		specOpts = append(specOpts, customopts.WithResources(config.GetLinux().GetResources()))
 		if sandboxConfig.GetLinux().GetCgroupParent() != "" {
-			cgroupsPath := getCgroupsPath(sandboxConfig.GetLinux().GetCgroupParent(), id,
-				c.config.SystemdCgroup)
+			cgroupsPath := getCgroupsPath(sandboxConfig.GetLinux().GetCgroupParent(), id)
 			specOpts = append(specOpts, oci.WithCgroup(cgroupsPath))
 		}
 	}

diff --git a/pkg/server/container_create_test.go b/pkg/server/container_create_test.go
@@ -161,7 +161,7 @@ func getCreateContainerTestData() (*runtime.ContainerConfig, *runtime.PodSandbox
 		assert.Equal(t, spec.Process.NoNewPrivileges, true)
 
 		t.Logf("Check cgroup path")
-		assert.Equal(t, getCgroupsPath("/test/cgroup/parent", id, false), spec.Linux.CgroupsPath)
+		assert.Equal(t, getCgroupsPath("/test/cgroup/parent", id), spec.Linux.CgroupsPath)
 
 		t.Logf("Check namespaces")
 		assert.Contains(t, spec.Linux.Namespaces, runtimespec.LinuxNamespace{

diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go
@@ -142,12 +142,12 @@ func makeContainerName(c *runtime.ContainerMetadata, s *runtime.PodSandboxMetada
 }
 
 // getCgroupsPath generates container cgroups path.
-func getCgroupsPath(cgroupsParent, id string, systemdCgroup bool) string {
-	if systemdCgroup {
-		// Convert a.slice/b.slice/c.slice to c.slice.
-		p := path.Base(cgroupsParent)
+func getCgroupsPath(cgroupsParent, id string) string {
+	base := path.Base(cgroupsParent)
+	if strings.HasSuffix(base, ".slice") {
+		// For a.slice/b.slice/c.slice, base is c.slice.
 		// runc systemd cgroup path format is "slice:prefix:name".
-		return strings.Join([]string{p, "cri-containerd", id}, ":")
+		return strings.Join([]string{base, "cri-containerd", id}, ":")
 	}
 	return filepath.Join(cgroupsParent, id)
 }

diff --git a/pkg/server/helpers_test.go b/pkg/server/helpers_test.go
@@ -117,22 +117,31 @@ func TestGetCgroupsPath(t *testing.T) {
 	testID := "test-id"
 	for desc, test := range map[string]struct {
 		cgroupsParent string
-		systemdCgroup bool
 		expected      string
 	}{
 		"should support regular cgroup path": {
 			cgroupsParent: "/a/b",
-			systemdCgroup: false,
 			expected:      "/a/b/test-id",
 		},
 		"should support systemd cgroup path": {
 			cgroupsParent: "/a.slice/b.slice",
-			systemdCgroup: true,
 			expected:      "b.slice:cri-containerd:test-id",
 		},
+		"should support tailing slash for regular cgroup path": {
+			cgroupsParent: "/a/b/",
+			expected:      "/a/b/test-id",
+		},
+		"should support tailing slash for systemd cgroup path": {
+			cgroupsParent: "/a.slice/b.slice/",
+			expected:      "b.slice:cri-containerd:test-id",
+		},
+		"should treat root cgroup as regular cgroup path": {
+			cgroupsParent: "/",
+			expected:      "/test-id",
+		},
 	} {
 		t.Logf("TestCase %q", desc)
-		got := getCgroupsPath(test.cgroupsParent, testID, test.systemdCgroup)
+		got := getCgroupsPath(test.cgroupsParent, testID)
 		assert.Equal(t, test.expected, got)
 	}
 }

diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go
@@ -352,8 +352,7 @@ func (c *criService) generateSandboxContainerSpec(id string, config *runtime.Pod
 		specOpts = append(specOpts, customopts.WithDisabledCgroups)
 	} else {
 		if config.GetLinux().GetCgroupParent() != "" {
-			cgroupsPath := getCgroupsPath(config.GetLinux().GetCgroupParent(), id,
-				c.config.SystemdCgroup)
+			cgroupsPath := getCgroupsPath(config.GetLinux().GetCgroupParent(), id)
 			specOpts = append(specOpts, oci.WithCgroup(cgroupsPath))
 		}
 	}

diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go
@@ -61,7 +61,7 @@ func getRunPodSandboxTestData() (*runtime.PodSandboxConfig, *imagespec.ImageConf
 	}
 	specCheck := func(t *testing.T, id string, spec *runtimespec.Spec) {
 		assert.Equal(t, "test-hostname", spec.Hostname)
-		assert.Equal(t, getCgroupsPath("/test/cgroup/parent", id, false), spec.Linux.CgroupsPath)
+		assert.Equal(t, getCgroupsPath("/test/cgroup/parent", id), spec.Linux.CgroupsPath)
 		assert.Equal(t, relativeRootfsPath, spec.Root.Path)
 		assert.Equal(t, true, spec.Root.Readonly)
 		assert.Contains(t, spec.Process.Env, "a=b", "c=d")