diff --git a/cli/create.go b/cli/create.go index 9a09448591..dbbf2e4455 100644 --- a/cli/create.go +++ b/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 } diff --git a/cli/create_test.go b/cli/create_test.go index d09ac4f5ef..930f6832f0 100644 --- a/cli/create_test.go +++ b/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{}}, - {"", "", "", "", true, oci.RuntimeConfig{}}, - {"foo", "", "", "", true, oci.RuntimeConfig{}}, - {testContainerID, bundlePath, testConsole, pidFilePath, false, runtimeConfig}, - {testContainerID, bundlePath, testConsole, pidFilePath, true, runtimeConfig}, + {"", "", "", "", false, false, oci.RuntimeConfig{}}, + {"", "", "", "", true, true, oci.RuntimeConfig{}}, + {"foo", "", "", "", true, false, oci.RuntimeConfig{}}, + {testContainerID, bundlePath, testConsole, pidFilePath, false, false, 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: "a, } - _, 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)) } diff --git a/cli/main.go b/cli/main.go index 546062b955..588137427a 100644 --- a/cli/main.go +++ b/cli/main.go @@ -105,6 +105,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-) diff --git a/cli/run.go b/cli/run.go index ac0198d0f2..13a08620ca 100644 --- a/cli/run.go +++ b/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 } diff --git a/cli/run_test.go b/cli/run_test.go index f77c27453e..362e563897 100644 --- a/cli/run_test.go +++ b/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{}}, - {"", "", "", "", "", false, oci.RuntimeConfig{}}, - {"", "", "", "", "", true, runtimeConfig}, - {"", "", "", "", "", false, runtimeConfig}, - {"", "", "", "", pidFilePath, false, runtimeConfig}, - {"", "", "", "", inexistentPath, false, runtimeConfig}, - {"", "", "", "", pidFilePath, false, runtimeConfig}, - {"", "", "", inexistentPath, pidFilePath, false, runtimeConfig}, - {"", "", inexistentPath, inexistentPath, pidFilePath, false, runtimeConfig}, - {"", "", inexistentPath, "", pidFilePath, false, runtimeConfig}, - {"", "", consolePath, "", pidFilePath, false, runtimeConfig}, - {"", bundlePath, consolePath, "", pidFilePath, false, runtimeConfig}, - {testContainerID, inexistentPath, consolePath, "", pidFilePath, false, oci.RuntimeConfig{}}, - {testContainerID, inexistentPath, consolePath, "", inexistentPath, false, oci.RuntimeConfig{}}, - {testContainerID, bundlePath, consolePath, "", pidFilePath, false, oci.RuntimeConfig{}}, - {testContainerID, inexistentPath, consolePath, "", pidFilePath, false, runtimeConfig}, - {testContainerID, inexistentPath, consolePath, "", inexistentPath, false, runtimeConfig}, - {testContainerID, bundlePath, consolePath, "", pidFilePath, false, runtimeConfig}, + {"", "", "", "", "", true, true, oci.RuntimeConfig{}}, + {"", "", "", "", "", false, false, oci.RuntimeConfig{}}, + {"", "", "", "", "", true, false, runtimeConfig}, + {"", "", "", "", "", false, true, runtimeConfig}, + {"", "", "", "", pidFilePath, false, false, runtimeConfig}, + {"", "", "", "", inexistentPath, false, false, runtimeConfig}, + {"", "", "", "", pidFilePath, false, true, runtimeConfig}, + {"", "", "", inexistentPath, pidFilePath, false, true, runtimeConfig}, + {"", "", inexistentPath, inexistentPath, pidFilePath, false, false, runtimeConfig}, + {"", "", inexistentPath, "", pidFilePath, false, false, runtimeConfig}, + {"", "", consolePath, "", pidFilePath, false, true, runtimeConfig}, + {"", bundlePath, consolePath, "", pidFilePath, false, true, runtimeConfig}, + {testContainerID, inexistentPath, consolePath, "", pidFilePath, false, true, oci.RuntimeConfig{}}, + {testContainerID, inexistentPath, consolePath, "", inexistentPath, false, true, oci.RuntimeConfig{}}, + {testContainerID, bundlePath, consolePath, "", pidFilePath, false, false, oci.RuntimeConfig{}}, + {testContainerID, inexistentPath, consolePath, "", pidFilePath, false, false, runtimeConfig}, + {testContainerID, inexistentPath, consolePath, "", inexistentPath, false, true, 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)) } diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index e5c03fdda7..3f0a2916c7 100644 --- a/virtcontainers/kata_agent.go +++ b/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) diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index 7c8c762e09..d62d916c7f 100644 --- a/virtcontainers/kata_agent_test.go +++ b/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) { diff --git a/virtcontainers/pkg/oci/utils.go b/virtcontainers/pkg/oci/utils.go index f9f97fb172..7978be9cd3 100644 --- a/virtcontainers/pkg/oci/utils.go +++ b/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) diff --git a/virtcontainers/pkg/oci/utils_test.go b/virtcontainers/pkg/oci/utils_test.go index 349c119b83..042a02f49e 100644 --- a/virtcontainers/pkg/oci/utils_test.go +++ b/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) } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 073ece5272..4913bcd943 100644 --- a/virtcontainers/sandbox.go +++ b/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) {