From 03cdf6c4a997ba44f9568c7b2bd5e0f3d07e744a Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Wed, 5 Feb 2020 20:09:22 +0000 Subject: [PATCH] virtcontainers: add new package for cgroups virtcontainers/pkg/cgroups contains functions and structures needed to deal with cgroups and virtual containers Signed-off-by: Julio Montes --- virtcontainers/api_test.go | 11 +-- virtcontainers/cgroups.go | 103 ---------------------- virtcontainers/cgroups_test.go | 91 -------------------- virtcontainers/container.go | 3 +- virtcontainers/kata_agent.go | 3 +- virtcontainers/pkg/cgroups/utils.go | 65 ++++++++++++++ virtcontainers/pkg/cgroups/utils_test.go | 104 +++++++++++++++++++++++ 7 files changed, 179 insertions(+), 201 deletions(-) create mode 100644 virtcontainers/pkg/cgroups/utils.go create mode 100644 virtcontainers/pkg/cgroups/utils_test.go diff --git a/virtcontainers/api_test.go b/virtcontainers/api_test.go index 6d7061614b..ed954ecadb 100644 --- a/virtcontainers/api_test.go +++ b/virtcontainers/api_test.go @@ -18,6 +18,7 @@ import ( ktu "github.com/kata-containers/runtime/pkg/katatestutils" "github.com/kata-containers/runtime/virtcontainers/persist" "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" + vccgroups "github.com/kata-containers/runtime/virtcontainers/pkg/cgroups" "github.com/kata-containers/runtime/virtcontainers/pkg/mock" "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" @@ -48,7 +49,7 @@ func newEmptySpec() *specs.Spec { return &specs.Spec{ Linux: &specs.Linux{ Resources: &specs.LinuxResources{}, - CgroupsPath: defaultCgroupPath, + CgroupsPath: vccgroups.DefaultCgroupPath, }, Process: &specs.Process{ Capabilities: &specs.LinuxCapabilities{}, @@ -515,7 +516,7 @@ func TestStatusSandboxSuccessfulStateReady(t *testing.T) { assert := assert.New(t) config := newTestSandboxConfigNoop() - cgroupPath, err := renameCgroupPath(defaultCgroupPath) + cgroupPath, err := vccgroups.RenameCgroupPath(vccgroups.DefaultCgroupPath) assert.NoError(err) hypervisorConfig := HypervisorConfig{ @@ -574,7 +575,7 @@ func TestStatusSandboxSuccessfulStateRunning(t *testing.T) { assert := assert.New(t) config := newTestSandboxConfigNoop() - cgroupPath, err := renameCgroupPath(defaultCgroupPath) + cgroupPath, err := vccgroups.RenameCgroupPath(vccgroups.DefaultCgroupPath) assert.NoError(err) hypervisorConfig := HypervisorConfig{ @@ -1156,7 +1157,7 @@ func TestStatusContainerStateReady(t *testing.T) { contID := "101" config := newTestSandboxConfigNoop() - cgroupPath, err := renameCgroupPath(defaultCgroupPath) + cgroupPath, err := vccgroups.RenameCgroupPath(vccgroups.DefaultCgroupPath) assert.NoError(err) ctx := context.Background() @@ -1217,7 +1218,7 @@ func TestStatusContainerStateRunning(t *testing.T) { contID := "101" config := newTestSandboxConfigNoop() - cgroupPath, err := renameCgroupPath(defaultCgroupPath) + cgroupPath, err := vccgroups.RenameCgroupPath(vccgroups.DefaultCgroupPath) assert.NoError(err) ctx := context.Background() diff --git a/virtcontainers/cgroups.go b/virtcontainers/cgroups.go index 7c6cf78be1..4459df5c39 100644 --- a/virtcontainers/cgroups.go +++ b/virtcontainers/cgroups.go @@ -11,16 +11,9 @@ import ( "fmt" "os" "path/filepath" - "regexp" "strings" "github.com/containerd/cgroups" - "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" - libcontcgroups "github.com/opencontainers/runc/libcontainer/cgroups" - libcontcgroupsfs "github.com/opencontainers/runc/libcontainer/cgroups/fs" - libcontcgroupssystemd "github.com/opencontainers/runc/libcontainer/cgroups/systemd" - "github.com/opencontainers/runc/libcontainer/configs" - specconv "github.com/opencontainers/runc/libcontainer/specconv" specs "github.com/opencontainers/runtime-spec/specs-go" ) @@ -34,15 +27,6 @@ type cgroupPather interface { // where path is defined by the containers manager const cgroupKataPath = "/kata/" -// prepend a kata specific string to oci cgroup path to -// form a different cgroup path, thus cAdvisor couldn't -// find kata containers cgroup path on host to prevent it -// from grabbing the stats data. -const cgroupKataPrefix = "kata" - -// DefaultCgroupPath runtime-determined location in the cgroups hierarchy. -const defaultCgroupPath = "/vc" - var cgroupsLoadFunc = cgroups.Load var cgroupsNewFunc = cgroups.New @@ -181,90 +165,3 @@ func validCPUResources(cpuSpec *specs.LinuxCPU) *specs.LinuxCPU { return &cpu } - -func renameCgroupPath(path string) (string, error) { - if path == "" { - return "", fmt.Errorf("Cgroup path is empty") - } - - cgroupPathDir := filepath.Dir(path) - cgroupPathName := fmt.Sprintf("%s_%s", cgroupKataPrefix, filepath.Base(path)) - return filepath.Join(cgroupPathDir, cgroupPathName), nil - -} - -// validCgroupPath returns a valid cgroup path. -// see https://github.com/opencontainers/runtime-spec/blob/master/config-linux.md#cgroups-path -func validCgroupPath(path string, 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 renameCgroupPath(filepath.Clean(path)) - } - - // 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 defaultCgroupPath - return renameCgroupPath(filepath.Join(defaultCgroupPath, filepath.Clean("/"+path))) -} - -func isSystemdCgroup(cgroupPath string) bool { - // systemd cgroup path: slice:prefix:name - re := regexp.MustCompile(`([[:alnum:]]|\.)+:([[:alnum:]]|\.)+:([[:alnum:]]|\.)+`) - found := re.FindStringIndex(cgroupPath) - - // if found string is equal to cgroupPath then - // it's a correct systemd cgroup path. - return found != nil && cgroupPath[found[0]:found[1]] == cgroupPath -} - -func newCgroupManager(cgroups *configs.Cgroup, cgroupPaths map[string]string, spec *specs.Spec) (libcontcgroups.Manager, error) { - var err error - - rootless := rootless.IsRootless() - systemdCgroup := isSystemdCgroup(spec.Linux.CgroupsPath) - - // Create a new cgroup if the current one is nil - // this cgroups must be saved later - if cgroups == nil { - if cgroups, err = specconv.CreateCgroupConfig(&specconv.CreateOpts{ - // cgroup name is taken from spec - CgroupName: "", - UseSystemdCgroup: systemdCgroup, - Spec: spec, - RootlessCgroups: rootless, - }); err != nil { - return nil, fmt.Errorf("Could not create cgroup config: %v", err) - } - } - - // Set cgroupPaths to nil when the map is empty, it can and will be - // populated by `Manager.Apply()` when the runtime or any other process - // is moved to the cgroup. See sandbox.setupSandboxCgroup(). - if len(cgroupPaths) == 0 { - cgroupPaths = nil - } - - if systemdCgroup { - systemdCgroupFunc, err := libcontcgroupssystemd.NewSystemdCgroupsManager() - if err != nil { - return nil, fmt.Errorf("Could not create systemd cgroup manager: %v", err) - } - libcontcgroupssystemd.UseSystemd() - return systemdCgroupFunc(cgroups, cgroupPaths), nil - } - - return &libcontcgroupsfs.Manager{ - Cgroups: cgroups, - Rootless: rootless, - Paths: cgroupPaths, - }, nil -} diff --git a/virtcontainers/cgroups_test.go b/virtcontainers/cgroups_test.go index a1cca5f3b8..6fb73c4845 100644 --- a/virtcontainers/cgroups_test.go +++ b/virtcontainers/cgroups_test.go @@ -10,7 +10,6 @@ import ( "os" "os/exec" "path/filepath" - "strings" "testing" "github.com/containerd/cgroups" @@ -198,93 +197,3 @@ func TestUpdateCgroups(t *testing.T) { err = s.cgroupsDelete() assert.NoError(err) } - -func TestIsSystemdCgroup(t *testing.T) { - assert := assert.New(t) - - tests := []struct { - path string - expected bool - }{ - {"slice:kata:afhts2e5d4g5s", true}, - {"slice.system:kata:afhts2e5d4g5s", true}, - {"/kata/afhts2e5d4g5s", false}, - {"a:b:c:d", false}, - {":::", false}, - {"", false}, - {":", false}, - {"::", false}, - {":::", false}, - {"a:b", false}, - {"a:b:", false}, - {":a:b", false}, - {"@:@:@", false}, - } - - for _, t := range tests { - assert.Equal(t.expected, isSystemdCgroup(t.path), "invalid systemd cgroup path: %v", t.path) - } -} - -func TestValidCgroupPath(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}, - - // invalid systemd paths - {"o / m /../ g", true, true}, - {"slice:kata", true, true}, - {"/kata/afhts2e5d4g5s", 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 system paths - {"slice:kata:55555", true, false}, - {"slice.system:kata:afhts2e5d4g5s", true, false}, - } { - path, err := validCgroupPath(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", cleanPath) - } else if t.systemdCgroup { - assert.Equal(t.path, path) - } else { - assert.True(strings.HasPrefix(path, "/"+cgroupKataPrefix) || - strings.HasPrefix(path, defaultCgroupPath), - "%v should have prefix /%v or %v", path, cgroupKataPrefix, defaultCgroupPath) - } - } - -} diff --git a/virtcontainers/container.go b/virtcontainers/container.go index 2a9f2269ee..825a1f8a4e 100644 --- a/virtcontainers/container.go +++ b/virtcontainers/container.go @@ -17,6 +17,7 @@ import ( "time" "github.com/containerd/cgroups" + vccgroups "github.com/kata-containers/runtime/virtcontainers/pkg/cgroups" vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" "github.com/kata-containers/runtime/virtcontainers/types" "github.com/kata-containers/runtime/virtcontainers/utils" @@ -1409,7 +1410,7 @@ func (c *Container) cgroupsCreate() (err error) { resources.CPU = validCPUResources(spec.Linux.Resources.CPU) } - c.state.CgroupPath, err = validCgroupPath(spec.Linux.CgroupsPath, c.sandbox.config.SystemdCgroup) + c.state.CgroupPath, err = vccgroups.ValidCgroupPath(spec.Linux.CgroupsPath, c.sandbox.config.SystemdCgroup) if err != nil { return fmt.Errorf("Invalid cgroup path: %v", err) } diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 0bb4943a7a..305db6ecf9 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -25,6 +25,7 @@ import ( "github.com/kata-containers/runtime/virtcontainers/device/config" persistapi "github.com/kata-containers/runtime/virtcontainers/persist/api" vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" + vccgroups "github.com/kata-containers/runtime/virtcontainers/pkg/cgroups" ns "github.com/kata-containers/runtime/virtcontainers/pkg/nsenter" "github.com/kata-containers/runtime/virtcontainers/pkg/rootless" vcTypes "github.com/kata-containers/runtime/virtcontainers/pkg/types" @@ -1033,7 +1034,7 @@ func (k *kataAgent) constraintGRPCSpec(grpcSpec *grpc.Spec, passSeccomp bool) { // - Initrd image doesn't have systemd. // - Nobody will be able to modify the resources of a specific container by using systemctl set-property. // - docker is not running in the VM. - if isSystemdCgroup(grpcSpec.Linux.CgroupsPath) { + if vccgroups.IsSystemdCgroup(grpcSpec.Linux.CgroupsPath) { // Convert systemd cgroup to cgroupfs slice := strings.Split(grpcSpec.Linux.CgroupsPath, ":") // 0 - slice: system.slice diff --git a/virtcontainers/pkg/cgroups/utils.go b/virtcontainers/pkg/cgroups/utils.go new file mode 100644 index 0000000000..167d65554c --- /dev/null +++ b/virtcontainers/pkg/cgroups/utils.go @@ -0,0 +1,65 @@ +// Copyright (c) 2020 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package cgroups + +import ( + "fmt" + "path/filepath" + "regexp" +) + +// prepend a kata specific string to oci cgroup path to +// form a different cgroup path, thus cAdvisor couldn't +// find kata containers cgroup path on host to prevent it +// from grabbing the stats data. +const CgroupKataPrefix = "kata" + +// DefaultCgroupPath runtime-determined location in the cgroups hierarchy. +const DefaultCgroupPath = "/vc" + +func RenameCgroupPath(path string) (string, error) { + if path == "" { + return "", fmt.Errorf("Cgroup path is empty") + } + + cgroupPathDir := filepath.Dir(path) + cgroupPathName := fmt.Sprintf("%s_%s", CgroupKataPrefix, filepath.Base(path)) + return filepath.Join(cgroupPathDir, cgroupPathName), nil + +} + +// validCgroupPath returns a valid cgroup path. +// see https://github.com/opencontainers/runtime-spec/blob/master/config-linux.md#cgroups-path +func ValidCgroupPath(path string, 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 RenameCgroupPath(filepath.Clean(path)) + } + + // 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 DefaultCgroupPath + return RenameCgroupPath(filepath.Join(DefaultCgroupPath, filepath.Clean("/"+path))) +} + +func IsSystemdCgroup(cgroupPath string) bool { + // systemd cgroup path: slice:prefix:name + re := regexp.MustCompile(`([[:alnum:]]|\.)+:([[:alnum:]]|\.)+:([[:alnum:]]|\.)+`) + found := re.FindStringIndex(cgroupPath) + + // if found string is equal to cgroupPath then + // it's a correct systemd cgroup path. + return found != nil && cgroupPath[found[0]:found[1]] == cgroupPath +} diff --git a/virtcontainers/pkg/cgroups/utils_test.go b/virtcontainers/pkg/cgroups/utils_test.go new file mode 100644 index 0000000000..b920ce6e9c --- /dev/null +++ b/virtcontainers/pkg/cgroups/utils_test.go @@ -0,0 +1,104 @@ +// Copyright (c) 2020 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package cgroups + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsSystemdCgroup(t *testing.T) { + assert := assert.New(t) + + tests := []struct { + path string + expected bool + }{ + {"slice:kata:afhts2e5d4g5s", true}, + {"slice.system:kata:afhts2e5d4g5s", true}, + {"/kata/afhts2e5d4g5s", false}, + {"a:b:c:d", false}, + {":::", false}, + {"", false}, + {":", false}, + {"::", false}, + {":::", false}, + {"a:b", false}, + {"a:b:", false}, + {":a:b", false}, + {"@:@:@", false}, + } + + for _, t := range tests { + assert.Equal(t.expected, IsSystemdCgroup(t.path), "invalid systemd cgroup path: %v", t.path) + } +} + +func TestValidCgroupPath(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}, + + // invalid systemd paths + {"o / m /../ g", true, true}, + {"slice:kata", true, true}, + {"/kata/afhts2e5d4g5s", 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 system paths + {"slice:kata:55555", true, false}, + {"slice.system:kata:afhts2e5d4g5s", true, false}, + } { + path, err := ValidCgroupPath(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", cleanPath) + } else if t.systemdCgroup { + assert.Equal(t.path, path) + } else { + assert.True(strings.HasPrefix(path, "/"+CgroupKataPrefix) || + strings.HasPrefix(path, DefaultCgroupPath), + "%v should have prefix /%v or %v", path, CgroupKataPrefix, DefaultCgroupPath) + } + } + +}