diff --git a/src/runtime/pkg/resourcecontrol/cgroups.go b/src/runtime/pkg/resourcecontrol/cgroups.go index be8e9dc973..42006f4949 100644 --- a/src/runtime/pkg/resourcecontrol/cgroups.go +++ b/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 } diff --git a/src/runtime/pkg/resourcecontrol/utils_linux.go b/src/runtime/pkg/resourcecontrol/utils_linux.go index 0acbc6c6af..52ac68f5af 100644 --- a/src/runtime/pkg/resourcecontrol/utils_linux.go +++ b/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 systemdCgroup { - 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 isCgroupV2 { + return filepath.Join("/", path), nil + } else { + return path, nil + } } if systemdCgroup { diff --git a/src/runtime/pkg/resourcecontrol/utils_linux_test.go b/src/runtime/pkg/resourcecontrol/utils_linux_test.go index 804f6253a8..6ed70fbcb6 100644 --- a/src/runtime/pkg/resourcecontrol/utils_linux_test.go +++ b/src/runtime/pkg/resourcecontrol/utils_linux_test.go @@ -41,74 +41,15 @@ func TestIsSystemdCgroup(t *testing.T) { } } -func TestValidCgroupPathV1(t *testing.T) { - assert := assert.New(t) - - 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) - } - } +func TestValidCgroupPath(t *testing.T) { + // test with cgroup v1 + runValidCgroupPathTest(t, false) + // 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) {