Merge pull request #8931 from yaoyinnan/8930/feat/merge-ValidCgroupPath

runtime: merged ValidCgroupPath method

src/runtime/pkg/resourcecontrol/cgroups.go

@@ -136,7 +136,7 @@ func NewResourceController(path string, resources *specs.LinuxResources) (Resour
 	var cgroupPath string
 
 	if cgroups.Mode() == cgroups.Legacy || cgroups.Mode() == cgroups.Hybrid {
-		cgroupPath, err = ValidCgroupPathV1(path, IsSystemdCgroup(path))
+		cgroupPath, err = ValidCgroupPath(path, false, IsSystemdCgroup(path))
 		if err != nil {
 			return nil, err
 		}
@@ -145,7 +145,7 @@ func NewResourceController(path string, resources *specs.LinuxResources) (Resour
 			return nil, err
 		}
 	} else if cgroups.Mode() == cgroups.Unified {
-		cgroupPath, err = ValidCgroupPathV2(path, IsSystemdCgroup(path))
+		cgroupPath, err = ValidCgroupPath(path, true, IsSystemdCgroup(path))
 		if err != nil {
 			return nil, err
 		}

src/runtime/pkg/resourcecontrol/utils_linux.go

@@ -21,35 +21,15 @@ import (
 // DefaultResourceControllerID runtime-determined location in the cgroups hierarchy.
 const DefaultResourceControllerID = "/vc"
 
-// ValidCgroupPathV1 returns a valid cgroup path for cgroup v1.
+// ValidCgroupPath returns a valid cgroup path.
 // see https://github.com/opencontainers/runtime-spec/blob/master/config-linux.md#cgroups-path
-func ValidCgroupPathV1(path string, systemdCgroup bool) (string, error) {
+func ValidCgroupPath(path string, isCgroupV2 bool, systemdCgroup bool) (string, error) {
 	if IsSystemdCgroup(path) {
-		return path, nil
+		if isCgroupV2 {
-	}
+			return filepath.Join("/", path), nil
-
+		} else {
-	if systemdCgroup {
+			return path, nil
-		return "", fmt.Errorf("malformed systemd path '%v': expected to be of form 'slice:prefix:name'", path)
+		}
-	}
-
-	// In the case of an absolute path (starting with /), the runtime MUST
-	// take the path to be relative to the cgroups mount point.
-	if filepath.IsAbs(path) {
-		return filepath.Clean(path), nil
-	}
-
-	// In the case of a relative path (not starting with /), the runtime MAY
-	// interpret the path relative to a runtime-determined location in the cgroups hierarchy.
-	// clean up path and return a new path relative to DefaultResourceControllerID
-	return filepath.Join(DefaultResourceControllerID, filepath.Clean("/"+path)), nil
-}
-
-// ValidCgroupPathV2 returns a valid cgroup path for cgroup v2.
-// see https://github.com/opencontainers/runtime-spec/blob/master/config-linux.md#cgroups-path
-func ValidCgroupPathV2(path string, systemdCgroup bool) (string, error) {
-	// In cgroup v2，path must be a "clean" absolute path starts with "/".
-	if IsSystemdCgroup(path) {
-		return filepath.Join("/", path), nil
 	}
 
 	if systemdCgroup {

src/runtime/pkg/resourcecontrol/utils_linux_test.go

@@ -41,74 +41,15 @@ func TestIsSystemdCgroup(t *testing.T) {
 	}
 }
 
-func TestValidCgroupPathV1(t *testing.T) {
+func TestValidCgroupPath(t *testing.T) {
-	assert := assert.New(t)
+	// test with cgroup v1
-
+	runValidCgroupPathTest(t, false)
-	for _, t := range []struct {
-		path          string
-		systemdCgroup bool
-		error         bool
-	}{
-		// empty paths
-		{"../../../", false, false},
-		{"../", false, false},
-		{".", false, false},
-		{"../../../", false, false},
-		{"./../", false, false},
-
-		// valid no-systemd paths
-		{"../../../foo", false, false},
-		{"/../hi", false, false},
-		{"/../hi/foo", false, false},
-		{"o / m /../ g", false, false},
-		{"/overhead/foobar", false, false},
-		{"/kata/afhts2e5d4g5s", false, false},
-		{"/kubepods/besteffort/podxxx-afhts2e5d4g5s/kata_afhts2e5d4g5s", false, false},
-		{"/sys/fs/cgroup/cpu/sandbox/kata_foobar", false, false},
-		{"kata_overhead/afhts2e5d4g5s", false, false},
-
-		// invalid systemd paths
-		{"o / m /../ g", true, true},
-		{"slice:kata", true, true},
-		{"a:b:c:d", true, true},
-		{":::", true, true},
-		{"", true, true},
-		{":", true, true},
-		{"::", true, true},
-		{":::", true, true},
-		{"a:b", true, true},
-		{"a:b:", true, true},
-		{":a:b", true, true},
-		{"@:@:@", true, true},
-
-		// valid systemd paths
-		{"x.slice:kata:55555", true, false},
-		{"system.slice:kata:afhts2e5d4g5s", true, false},
-	} {
-		path, err := ValidCgroupPathV1(t.path, t.systemdCgroup)
-		if t.error {
-			assert.Error(err)
-			continue
-		} else {
-			assert.NoError(err)
-		}
-
-		if filepath.IsAbs(t.path) {
-			cleanPath := filepath.Dir(filepath.Clean(t.path))
-			assert.True(strings.HasPrefix(path, cleanPath),
-				"%v should have prefix %v", path, cleanPath)
-		} else if t.systemdCgroup {
-			assert.Equal(t.path, path)
-		} else {
-			assert.True(
-				strings.HasPrefix(path, DefaultResourceControllerID),
-				"%v should have prefix /%v", path, DefaultResourceControllerID)
-		}
-	}
 
+	// test with cgroup v2
+	runValidCgroupPathTest(t, true)
 }
 
-func TestValidCgroupPathV2(t *testing.T) {
+func runValidCgroupPathTest(t *testing.T, isCgroupV2 bool) {
 	assert := assert.New(t)
 
 	for _, t := range []struct {
@@ -152,7 +93,7 @@ func TestValidCgroupPathV2(t *testing.T) {
 		{"x.slice:kata:55555", true, false},
 		{"system.slice:kata:afhts2e5d4g5s", true, false},
 	} {
-		path, err := ValidCgroupPathV2(t.path, t.systemdCgroup)
+		path, err := ValidCgroupPath(t.path, isCgroupV2, t.systemdCgroup)
 		if t.error {
 			assert.Error(err)
 			continue
@@ -165,14 +106,17 @@ func TestValidCgroupPathV2(t *testing.T) {
 			assert.True(strings.HasPrefix(path, cleanPath),
 				"%v should have prefix %v", path, cleanPath)
 		} else if t.systemdCgroup {
-			assert.Equal(filepath.Join("/", t.path), path)
+			if isCgroupV2 {
+				assert.Equal(filepath.Join("/", t.path), path)
+			} else {
+				assert.Equal(t.path, path)
+			}
 		} else {
 			assert.True(
 				strings.HasPrefix(path, DefaultResourceControllerID),
 				"%v should have prefix /%v", path, DefaultResourceControllerID)
 		}
 	}
-
 }
 
 func TestDeviceToCgroupDeviceRule(t *testing.T) {