Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Commit

Permalink
cli: add systemd-cgroup option
Browse files Browse the repository at this point in the history 
Add support for cgroup driver systemd.
systemd cgroup is not applied in the VM since in some cases like initrd images
there is no systemd running and nobody can update a systemd cgroup using
systemctl.

fixes #596

Signed-off-by: Julio Montes <[email protected]>
  • Loading branch information
Julio Montes committed Sep 18, 2018
1 parent a8284f8 commit 0075175
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 52 deletions.
9 changes: 5 additions & 4 deletions cli/create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ var createCLICommand = cli.Command{
console,
context.String("pid-file"),
true,
context.Bool("systemd-cgroup"),
runtimeConfig,
)
},
Expand All @@ -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

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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()

Expand All @@ -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
}
Expand Down
33 changes: 17 additions & 16 deletions cli/create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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))
}
Expand Down
4 changes: 4 additions & 0 deletions cli/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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-)
Expand Down
5 changes: 3 additions & 2 deletions cli/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
}

Expand Down
51 changes: 26 additions & 25 deletions cli/run_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
}
23 changes: 21 additions & 2 deletions virtcontainers/kata_agent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"fmt"
"os"
"path/filepath"
"regexp"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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)

Expand Down
7 changes: 6 additions & 1 deletion virtcontainers/kata_agent_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
Loading

0 comments on commit 0075175

Please sign in to comment.