Merge pull request #747 from devimc/topic/systemdCgroup

cli: add systemd-cgroup option

cli/create.go

@@ -83,6 +83,7 @@ var createCLICommand = cli.Command{
 			console,
 			context.String("pid-file"),
 			true,
+			context.Bool("systemd-cgroup"),
 			runtimeConfig,
 		)
 	},
@@ -91,7 +92,7 @@ var createCLICommand = cli.Command{
 // Use a variable to allow tests to modify its value
 var getKernelParamsFunc = getKernelParams
 
-func create(ctx context.Context, containerID, bundlePath, console, pidFilePath string, detach bool,
+func create(ctx context.Context, containerID, bundlePath, console, pidFilePath string, detach, systemdCgroup bool,
 	runtimeConfig oci.RuntimeConfig) error {
 	var err error
 
@@ -146,7 +147,7 @@ func create(ctx context.Context, containerID, bundlePath, console, pidFilePath s
 	var process vc.Process
 	switch containerType {
 	case vc.PodSandbox:
-		process, err = createSandbox(ctx, ociSpec, runtimeConfig, containerID, bundlePath, console, disableOutput)
+		process, err = createSandbox(ctx, ociSpec, runtimeConfig, containerID, bundlePath, console, disableOutput, systemdCgroup)
 		if err != nil {
 			return err
 		}
@@ -252,7 +253,7 @@ func setKernelParams(containerID string, runtimeConfig *oci.RuntimeConfig) error
 }
 
 func createSandbox(ctx context.Context, ociSpec oci.CompatOCISpec, runtimeConfig oci.RuntimeConfig,
-	containerID, bundlePath, console string, disableOutput bool) (vc.Process, error) {
+	containerID, bundlePath, console string, disableOutput, systemdCgroup bool) (vc.Process, error) {
 	span, ctx := trace(ctx, "createSandbox")
 	defer span.Finish()
 
@@ -261,7 +262,7 @@ func createSandbox(ctx context.Context, ociSpec oci.CompatOCISpec, runtimeConfig
 		return vc.Process{}, err
 	}
 
-	sandboxConfig, err := oci.SandboxConfig(ociSpec, runtimeConfig, bundlePath, containerID, console, disableOutput)
+	sandboxConfig, err := oci.SandboxConfig(ociSpec, runtimeConfig, bundlePath, containerID, console, disableOutput, systemdCgroup)
 	if err != nil {
 		return vc.Process{}, err
 	}

cli/create_test.go

@@ -326,19 +326,20 @@ func TestCreateInvalidArgs(t *testing.T) {
 		console       string
 		pidFilePath   string
 		detach        bool
+		systemdCgroup bool
 		runtimeConfig oci.RuntimeConfig
 	}
 
 	data := []testData{
-		{"", "", "", "", false, oci.RuntimeConfig{}},
+		{"", "", "", "", false, false, oci.RuntimeConfig{}},
-		{"", "", "", "", true, oci.RuntimeConfig{}},
+		{"", "", "", "", true, true, oci.RuntimeConfig{}},
-		{"foo", "", "", "", true, oci.RuntimeConfig{}},
+		{"foo", "", "", "", true, false, oci.RuntimeConfig{}},
-		{testContainerID, bundlePath, testConsole, pidFilePath, false, runtimeConfig},
+		{testContainerID, bundlePath, testConsole, pidFilePath, false, false, runtimeConfig},
-		{testContainerID, bundlePath, testConsole, pidFilePath, true, runtimeConfig},
+		{testContainerID, bundlePath, testConsole, pidFilePath, true, true, runtimeConfig},
 	}
 
 	for i, d := range data {
-		err := create(context.Background(), d.containerID, d.bundlePath, d.console, d.pidFilePath, d.detach, d.runtimeConfig)
+		err := create(context.Background(), d.containerID, d.bundlePath, d.console, d.pidFilePath, d.detach, d.systemdCgroup, d.runtimeConfig)
 		assert.Errorf(err, "test %d (%+v)", i, d)
 	}
 }
@@ -377,7 +378,7 @@ func TestCreateInvalidConfigJSON(t *testing.T) {
 	f.Close()
 
 	for detach := range []bool{true, false} {
-		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, runtimeConfig)
+		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, true, runtimeConfig)
 		assert.Errorf(err, "%+v", detach)
 		assert.False(vcmock.IsMockError(err))
 		os.RemoveAll(path)
@@ -421,7 +422,7 @@ func TestCreateInvalidContainerType(t *testing.T) {
 	assert.NoError(err)
 
 	for detach := range []bool{true, false} {
-		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, runtimeConfig)
+		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, true, runtimeConfig)
 		assert.Errorf(err, "%+v", detach)
 		assert.False(vcmock.IsMockError(err))
 		os.RemoveAll(path)
@@ -466,7 +467,7 @@ func TestCreateContainerInvalid(t *testing.T) {
 	assert.NoError(err)
 
 	for detach := range []bool{true, false} {
-		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, runtimeConfig)
+		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, true, runtimeConfig)
 		assert.Errorf(err, "%+v", detach)
 		assert.False(vcmock.IsMockError(err))
 		os.RemoveAll(path)
@@ -561,7 +562,7 @@ func TestCreateProcessCgroupsPathSuccessful(t *testing.T) {
 	assert.NoError(err)
 
 	for _, detach := range []bool{true, false} {
-		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, detach, runtimeConfig)
+		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, detach, true, runtimeConfig)
 		assert.NoError(err, "detached: %+v", detach)
 		os.RemoveAll(path)
 	}
@@ -645,7 +646,7 @@ func TestCreateCreateCgroupsFilesFail(t *testing.T) {
 	assert.NoError(err)
 
 	for detach := range []bool{true, false} {
-		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, runtimeConfig)
+		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, true, runtimeConfig)
 		assert.Errorf(err, "%+v", detach)
 		assert.False(vcmock.IsMockError(err))
 		os.RemoveAll(path)
@@ -721,7 +722,7 @@ func TestCreateCreateCreatePidFileFail(t *testing.T) {
 	assert.NoError(err)
 
 	for detach := range []bool{true, false} {
-		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, runtimeConfig)
+		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, true, runtimeConfig)
 		assert.Errorf(err, "%+v", detach)
 		assert.False(vcmock.IsMockError(err))
 		os.RemoveAll(path)
@@ -791,7 +792,7 @@ func TestCreate(t *testing.T) {
 	assert.NoError(err)
 
 	for detach := range []bool{true, false} {
-		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, runtimeConfig)
+		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, true, runtimeConfig)
 		assert.NoError(err, "%+v", detach)
 		os.RemoveAll(path)
 	}
@@ -848,7 +849,7 @@ func TestCreateInvalidKernelParams(t *testing.T) {
 	}
 
 	for detach := range []bool{true, false} {
-		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, runtimeConfig)
+		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, true, runtimeConfig)
 		assert.Errorf(err, "%+v", detach)
 		assert.False(vcmock.IsMockError(err))
 		os.RemoveAll(path)
@@ -893,7 +894,7 @@ func TestCreateSandboxConfigFail(t *testing.T) {
 		Quota: &quota,
 	}
 
-	_, err = createSandbox(context.Background(), spec, runtimeConfig, testContainerID, bundlePath, testConsole, true)
+	_, err = createSandbox(context.Background(), spec, runtimeConfig, testContainerID, bundlePath, testConsole, true, true)
 	assert.Error(err)
 	assert.False(vcmock.IsMockError(err))
 }
@@ -928,7 +929,7 @@ func TestCreateCreateSandboxFail(t *testing.T) {
 	spec, err := readOCIConfigFile(ociConfigFile)
 	assert.NoError(err)
 
-	_, err = createSandbox(context.Background(), spec, runtimeConfig, testContainerID, bundlePath, testConsole, true)
+	_, err = createSandbox(context.Background(), spec, runtimeConfig, testContainerID, bundlePath, testConsole, true, true)
 	assert.Error(err)
 	assert.True(vcmock.IsMockError(err))
 }

cli/main.go

@@ -106,6 +106,10 @@ var runtimeFlags = []cli.Flag{
 		Name:  showConfigPathsOption,
 		Usage: "show config file paths that will be checked for (in order)",
 	},
+	cli.BoolFlag{
+		Name:  "systemd-cgroup",
+		Usage: "enable systemd cgroup support, expects cgroupsPath to be of form \"slice:prefix:name\" for e.g. \"system.slice:runc:434234\"",
+	},
 }
 
 // runtimeCommands is the list of supported command-line (sub-)

cli/run.go

@@ -75,11 +75,12 @@ var runCLICommand = cli.Command{
 			context.String("console-socket"),
 			context.String("pid-file"),
 			context.Bool("detach"),
+			context.Bool("systemd-cgroup"),
 			runtimeConfig)
 	},
 }
 
-func run(ctx context.Context, containerID, bundle, console, consoleSocket, pidFile string, detach bool,
+func run(ctx context.Context, containerID, bundle, console, consoleSocket, pidFile string, detach, systemdCgroup bool,
 	runtimeConfig oci.RuntimeConfig) error {
 	span, ctx := trace(ctx, "run")
 	defer span.Finish()
@@ -89,7 +90,7 @@ func run(ctx context.Context, containerID, bundle, console, consoleSocket, pidFi
 		return err
 	}
 
-	if err := create(ctx, containerID, bundle, consolePath, pidFile, detach, runtimeConfig); err != nil {
+	if err := create(ctx, containerID, bundle, consolePath, pidFile, detach, systemdCgroup, runtimeConfig); err != nil {
 		return err
 	}
 

cli/run_test.go

@@ -118,32 +118,33 @@ func TestRunInvalidArgs(t *testing.T) {
 		consoleSocket string
 		pidFile       string
 		detach        bool
+		systemdCgroup bool
 		runtimeConfig oci.RuntimeConfig
 	}
 
 	args := []testArgs{
-		{"", "", "", "", "", true, oci.RuntimeConfig{}},
+		{"", "", "", "", "", true, true, oci.RuntimeConfig{}},
-		{"", "", "", "", "", false, oci.RuntimeConfig{}},
+		{"", "", "", "", "", false, false, oci.RuntimeConfig{}},
-		{"", "", "", "", "", true, runtimeConfig},
+		{"", "", "", "", "", true, false, runtimeConfig},
-		{"", "", "", "", "", false, runtimeConfig},
+		{"", "", "", "", "", false, true, runtimeConfig},
-		{"", "", "", "", pidFilePath, false, runtimeConfig},
+		{"", "", "", "", pidFilePath, false, false, runtimeConfig},
-		{"", "", "", "", inexistentPath, false, runtimeConfig},
+		{"", "", "", "", inexistentPath, false, false, runtimeConfig},
-		{"", "", "", "", pidFilePath, false, runtimeConfig},
+		{"", "", "", "", pidFilePath, false, true, runtimeConfig},
-		{"", "", "", inexistentPath, pidFilePath, false, runtimeConfig},
+		{"", "", "", inexistentPath, pidFilePath, false, true, runtimeConfig},
-		{"", "", inexistentPath, inexistentPath, pidFilePath, false, runtimeConfig},
+		{"", "", inexistentPath, inexistentPath, pidFilePath, false, false, runtimeConfig},
-		{"", "", inexistentPath, "", pidFilePath, false, runtimeConfig},
+		{"", "", inexistentPath, "", pidFilePath, false, false, runtimeConfig},
-		{"", "", consolePath, "", pidFilePath, false, runtimeConfig},
+		{"", "", consolePath, "", pidFilePath, false, true, runtimeConfig},
-		{"", bundlePath, consolePath, "", pidFilePath, false, runtimeConfig},
+		{"", bundlePath, consolePath, "", pidFilePath, false, true, runtimeConfig},
-		{testContainerID, inexistentPath, consolePath, "", pidFilePath, false, oci.RuntimeConfig{}},
+		{testContainerID, inexistentPath, consolePath, "", pidFilePath, false, true, oci.RuntimeConfig{}},
-		{testContainerID, inexistentPath, consolePath, "", inexistentPath, false, oci.RuntimeConfig{}},
+		{testContainerID, inexistentPath, consolePath, "", inexistentPath, false, true, oci.RuntimeConfig{}},
-		{testContainerID, bundlePath, consolePath, "", pidFilePath, false, oci.RuntimeConfig{}},
+		{testContainerID, bundlePath, consolePath, "", pidFilePath, false, false, oci.RuntimeConfig{}},
-		{testContainerID, inexistentPath, consolePath, "", pidFilePath, false, runtimeConfig},
+		{testContainerID, inexistentPath, consolePath, "", pidFilePath, false, false, runtimeConfig},
-		{testContainerID, inexistentPath, consolePath, "", inexistentPath, false, runtimeConfig},
+		{testContainerID, inexistentPath, consolePath, "", inexistentPath, false, true, runtimeConfig},
-		{testContainerID, bundlePath, consolePath, "", pidFilePath, false, runtimeConfig},
+		{testContainerID, bundlePath, consolePath, "", pidFilePath, false, true, runtimeConfig},
 	}
 
 	for i, a := range args {
-		err := run(context.Background(), a.containerID, a.bundle, a.console, a.consoleSocket, a.pidFile, a.detach, a.runtimeConfig)
+		err := run(context.Background(), a.containerID, a.bundle, a.console, a.consoleSocket, a.pidFile, a.detach, a.systemdCgroup, a.runtimeConfig)
 		assert.Errorf(err, "test %d (%+v)", i, a)
 	}
 }
@@ -289,7 +290,7 @@ func TestRunContainerSuccessful(t *testing.T) {
 		testingImpl.DeleteContainerFunc = nil
 	}()
 
-	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, false, d.runtimeConfig)
+	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, false, true, d.runtimeConfig)
 
 	// should return ExitError with the message and exit code
 	e, ok := err.(*cli.ExitError)
@@ -367,7 +368,7 @@ func TestRunContainerDetachSuccessful(t *testing.T) {
 		testingImpl.DeleteContainerFunc = nil
 	}()
 
-	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, true, d.runtimeConfig)
+	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, true, true, d.runtimeConfig)
 
 	// should not return ExitError
 	assert.NoError(err)
@@ -440,7 +441,7 @@ func TestRunContainerDeleteFail(t *testing.T) {
 		testingImpl.DeleteContainerFunc = nil
 	}()
 
-	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, false, d.runtimeConfig)
+	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, false, true, d.runtimeConfig)
 
 	// should not return ExitError
 	err, ok := err.(*cli.ExitError)
@@ -517,7 +518,7 @@ func TestRunContainerWaitFail(t *testing.T) {
 		testingImpl.DeleteContainerFunc = nil
 	}()
 
-	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, false, d.runtimeConfig)
+	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, false, true, d.runtimeConfig)
 
 	// should not return ExitError
 	err, ok := err.(*cli.ExitError)
@@ -575,7 +576,7 @@ func TestRunContainerStartFail(t *testing.T) {
 		testingImpl.StatusContainerFunc = nil
 	}()
 
-	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, false, d.runtimeConfig)
+	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, false, true, d.runtimeConfig)
 
 	// should not return ExitError
 	err, ok := err.(*cli.ExitError)
@@ -630,7 +631,7 @@ func TestRunContainerStartFailExistingContainer(t *testing.T) {
 		testingImpl.StartSandboxFunc = nil
 	}()
 
-	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, false, d.runtimeConfig)
+	err = run(context.Background(), d.sandbox.ID(), d.bundlePath, d.consolePath, "", d.pidFilePath, false, true, d.runtimeConfig)
 	assert.Error(err)
 	assert.False(vcmock.IsMockError(err))
 }

virtcontainers/kata_agent.go

@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"strings"
 	"sync"
@@ -714,7 +715,7 @@ func (k *kataAgent) replaceOCIMountsForStorages(spec *specs.Spec, volumeStorages
 	return nil
 }
 
-func constraintGRPCSpec(grpcSpec *grpc.Spec) {
+func constraintGRPCSpec(grpcSpec *grpc.Spec, systemdCgroup bool) {
 	// Disable Hooks since they have been handled on the host and there is
 	// no reason to send them to the agent. It would make no sense to try
 	// to apply them on the guest.
@@ -734,6 +735,24 @@ func constraintGRPCSpec(grpcSpec *grpc.Spec) {
 	grpcSpec.Linux.Resources.HugepageLimits = nil
 	grpcSpec.Linux.Resources.Network = nil
 
+	// There are three main reasons to do not apply systemd cgroups in the VM
+	// - 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 systemdCgroup {
+		// Convert systemd cgroup to cgroupfs
+		// systemd cgroup path: slice:prefix:name
+		re := regexp.MustCompile(`([[:alnum:]]|.)+:([[:alnum:]]|.)+:([[:alnum:]]|.)+`)
+		systemdCgroupPath := re.FindString(grpcSpec.Linux.CgroupsPath)
+		if systemdCgroupPath != "" {
+			slice := strings.Split(systemdCgroupPath, ":")
+			// 0 - slice: system.slice
+			// 1 - prefix: docker
+			// 2 - name: abc123
+			grpcSpec.Linux.CgroupsPath = filepath.Join("/", slice[1], slice[2])
+		}
+	}
+
 	// Disable network namespace since it is already handled on the host by
 	// virtcontainers. The network is a complex part which cannot be simply
 	// passed to the agent.
@@ -964,7 +983,7 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process,
 
 	// We need to constraint the spec to make sure we're not passing
 	// irrelevant information to the agent.
-	constraintGRPCSpec(grpcSpec)
+	constraintGRPCSpec(grpcSpec, sandbox.config.SystemdCgroup)
 
 	k.handleShm(grpcSpec, sandbox)
 

virtcontainers/kata_agent_test.go

@@ -421,6 +421,7 @@ func TestAppendDevices(t *testing.T) {
 
 func TestConstraintGRPCSpec(t *testing.T) {
 	assert := assert.New(t)
+	expectedCgroupPath := "/foo/bar"
 
 	g := &pb.Spec{
 		Hooks: &pb.Hooks{},
@@ -448,10 +449,11 @@ func TestConstraintGRPCSpec(t *testing.T) {
 				HugepageLimits: []pb.LinuxHugepageLimit{},
 				Network:        &pb.LinuxNetwork{},
 			},
+			CgroupsPath: "system.slice:foo:bar",
 		},
 	}
 
-	constraintGRPCSpec(g)
+	constraintGRPCSpec(g, true)
 
 	// check nil fields
 	assert.Nil(g.Hooks)
@@ -470,6 +472,9 @@ func TestConstraintGRPCSpec(t *testing.T) {
 
 	// check mounts
 	assert.Len(g.Mounts, 1)
+
+	// check cgroup path
+	assert.Equal(expectedCgroupPath, g.Linux.CgroupsPath)
 }
 
 func TestHandleShm(t *testing.T) {

virtcontainers/pkg/oci/utils.go

@@ -453,7 +453,7 @@ func addAssetAnnotations(ocispec CompatOCISpec, config *vc.SandboxConfig) {
 
 // SandboxConfig converts an OCI compatible runtime configuration file
 // to a virtcontainers sandbox configuration structure.
-func SandboxConfig(ocispec CompatOCISpec, runtime RuntimeConfig, bundlePath, cid, console string, detach bool) (vc.SandboxConfig, error) {
+func SandboxConfig(ocispec CompatOCISpec, runtime RuntimeConfig, bundlePath, cid, console string, detach, systemdCgroup bool) (vc.SandboxConfig, error) {
 	containerConfig, err := ContainerConfig(ocispec, bundlePath, cid, console, detach)
 	if err != nil {
 		return vc.SandboxConfig{}, err
@@ -507,6 +507,8 @@ func SandboxConfig(ocispec CompatOCISpec, runtime RuntimeConfig, bundlePath, cid
 		},
 
 		ShmSize: shmSize,
+
+		SystemdCgroup: systemdCgroup,
 	}
 
 	addAssetAnnotations(ocispec, &sandboxConfig)

virtcontainers/pkg/oci/utils_test.go

@@ -231,6 +231,8 @@ func TestMinimalSandboxConfig(t *testing.T) {
 			vcAnnotations.ConfigJSONKey: string(ociSpecJSON),
 			vcAnnotations.BundlePathKey: tempBundlePath,
 		},
+
+		SystemdCgroup: true,
 	}
 
 	ociSpec, err := ParseConfigJSON(tempBundlePath)
@@ -238,7 +240,7 @@ func TestMinimalSandboxConfig(t *testing.T) {
 		t.Fatalf("Could not parse config.json: %v", err)
 	}
 
-	sandboxConfig, err := SandboxConfig(ociSpec, runtimeConfig, tempBundlePath, containerID, consolePath, false)
+	sandboxConfig, err := SandboxConfig(ociSpec, runtimeConfig, tempBundlePath, containerID, consolePath, false, true)
 	if err != nil {
 		t.Fatalf("Could not create Sandbox configuration %v", err)
 	}

virtcontainers/sandbox.go

@@ -352,6 +352,9 @@ type SandboxConfig struct {
 	// Stateful keeps sandbox resources in memory across APIs. Users will be responsible
 	// for calling Release() to release the memory resources.
 	Stateful bool
+
+	// SystemdCgroup enables systemd cgroup support
+	SystemdCgroup bool
 }
 
 func (s *Sandbox) trace(name string) (opentracing.Span, context.Context) {