From f620349837ce05dd12d284f42c4920db985f4a0f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 12 Feb 2019 16:07:07 +0100 Subject: [PATCH] Add systctl support for services Signed-off-by: Sebastiaan van Stijn --- cli/command/service/create.go | 2 + cli/command/service/formatter.go | 13 +++ cli/command/service/opts.go | 6 ++ cli/command/service/opts_test.go | 13 +++ cli/command/service/update.go | 25 +++++ cli/command/service/update_test.go | 97 ++++++++++++++++++++ cli/compose/convert/service.go | 1 + cli/compose/loader/full-example.yml | 4 + cli/compose/loader/full-struct_test.go | 15 ++- cli/compose/loader/loader.go | 1 + cli/compose/loader/loader_test.go | 41 +++++++++ cli/compose/types/types.go | 9 +- contrib/completion/bash/docker | 3 + docs/reference/commandline/service_create.md | 1 + docs/reference/commandline/service_update.md | 2 + 15 files changed, 229 insertions(+), 4 deletions(-) diff --git a/cli/command/service/create.go b/cli/command/service/create.go index c045682e03..fb8f26c995 100644 --- a/cli/command/service/create.go +++ b/cli/command/service/create.go @@ -60,6 +60,8 @@ func newCreateCommand(dockerCli command.Cli) *cobra.Command { flags.SetAnnotation(flagHost, "version", []string{"1.25"}) flags.BoolVar(&opts.init, flagInit, false, "Use an init inside each service container to forward signals and reap processes") flags.SetAnnotation(flagInit, "version", []string{"1.37"}) + flags.Var(&opts.sysctls, flagSysCtl, "Sysctl options") + flags.SetAnnotation(flagSysCtl, "version", []string{"1.40"}) flags.Var(cliopts.NewListOptsRef(&opts.resources.resGenericResources, ValidateSingleGenericResource), "generic-resource", "User defined resources") flags.SetAnnotation(flagHostAdd, "version", []string{"1.32"}) diff --git a/cli/command/service/formatter.go b/cli/command/service/formatter.go index d814ddbd13..3ebe52d520 100644 --- a/cli/command/service/formatter.go +++ b/cli/command/service/formatter.go @@ -96,6 +96,11 @@ ContainerSpec: {{- if .ContainerUser }} User: {{ .ContainerUser }} {{- end }} +{{- if .ContainerSysCtls }} +SysCtls: +{{- range $k, $v := .ContainerSysCtls }} + {{ $k }}{{if $v }}: {{ $v }}{{ end }} +{{- end }}{{ end }} {{- if .ContainerMounts }} Mounts: {{- end }} @@ -415,6 +420,14 @@ func (ctx *serviceInspectContext) ContainerMounts() []mounttypes.Mount { return ctx.Service.Spec.TaskTemplate.ContainerSpec.Mounts } +func (ctx *serviceInspectContext) ContainerSysCtls() map[string]string { + return ctx.Service.Spec.TaskTemplate.ContainerSpec.Sysctls +} + +func (ctx *serviceInspectContext) HasContainerSysCtls() bool { + return len(ctx.Service.Spec.TaskTemplate.ContainerSpec.Sysctls) > 0 +} + func (ctx *serviceInspectContext) HasResources() bool { return ctx.Service.Spec.TaskTemplate.Resources != nil } diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index e2620ada35..c51d37c1de 100644 --- a/cli/command/service/opts.go +++ b/cli/command/service/opts.go @@ -490,6 +490,7 @@ type serviceOptions struct { dnsSearch opts.ListOpts dnsOption opts.ListOpts hosts opts.ListOpts + sysctls opts.ListOpts resources resourceOptions stopGrace opts.DurationOpt @@ -531,6 +532,7 @@ func newServiceOptions() *serviceOptions { dnsOption: opts.NewListOpts(nil), dnsSearch: opts.NewListOpts(opts.ValidateDNSSearch), hosts: opts.NewListOpts(opts.ValidateExtraHost), + sysctls: opts.NewListOpts(nil), } } @@ -643,6 +645,7 @@ func (options *serviceOptions) ToService(ctx context.Context, apiClient client.N StopGracePeriod: options.ToStopGracePeriod(flags), Healthcheck: healthConfig, Isolation: container.Isolation(options.isolation), + Sysctls: opts.ConvertKVStringsToMap(options.sysctls.GetAll()), }, Networks: networks, Resources: resources, @@ -890,6 +893,9 @@ const ( flagRollbackOrder = "rollback-order" flagRollbackParallelism = "rollback-parallelism" flagInit = "init" + flagSysCtl = "sysctl" + flagSysCtlAdd = "sysctl-add" + flagSysCtlRemove = "sysctl-rm" flagStopGracePeriod = "stop-grace-period" flagStopSignal = "stop-signal" flagTTY = "tty" diff --git a/cli/command/service/opts_test.go b/cli/command/service/opts_test.go index a128e24152..9493a82303 100644 --- a/cli/command/service/opts_test.go +++ b/cli/command/service/opts_test.go @@ -233,3 +233,16 @@ func TestToServiceMaxReplicasGlobalModeConflict(t *testing.T) { _, err := opt.ToServiceMode() assert.Error(t, err, "replicas-max-per-node can only be used with replicated mode") } + +func TestToServiceSysCtls(t *testing.T) { + o := newServiceOptions() + o.mode = "replicated" + o.sysctls.Set("net.ipv4.ip_forward=1") + o.sysctls.Set("kernel.shmmax=123456") + + expected := map[string]string{"net.ipv4.ip_forward": "1", "kernel.shmmax": "123456"} + flags := newCreateCommand(nil).Flags() + service, err := o.ToService(context.Background(), &fakeClient{}, flags) + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(service.TaskTemplate.ContainerSpec.Sysctls, expected)) +} diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 12ac6ed118..8269adcd00 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -96,6 +96,10 @@ func newUpdateCommand(dockerCli command.Cli) *cobra.Command { flags.SetAnnotation(flagHostAdd, "version", []string{"1.25"}) flags.BoolVar(&options.init, flagInit, false, "Use an init inside each service container to forward signals and reap processes") flags.SetAnnotation(flagInit, "version", []string{"1.37"}) + flags.Var(&options.sysctls, flagSysCtlAdd, "Add or update a Sysctl option") + flags.SetAnnotation(flagSysCtlAdd, "version", []string{"1.40"}) + flags.Var(newListOptsVar(), flagSysCtlRemove, "Remove a Sysctl option") + flags.SetAnnotation(flagSysCtlRemove, "version", []string{"1.40"}) // Add needs parsing, Remove only needs the key flags.Var(newListOptsVar(), flagGenericResourcesRemove, "Remove a Generic resource") @@ -328,6 +332,8 @@ func updateService(ctx context.Context, apiClient client.NetworkAPIClient, flags return err } + updateSysCtls(flags, &task.ContainerSpec.Sysctls) + if anyChanged(flags, flagLimitCPU, flagLimitMemory) { taskResources().Limits = spec.TaskTemplate.Resources.Limits updateInt64Value(flagLimitCPU, &task.Resources.Limits.NanoCPUs) @@ -661,6 +667,25 @@ func updateLabels(flags *pflag.FlagSet, field *map[string]string) { } } +func updateSysCtls(flags *pflag.FlagSet, field *map[string]string) { + if *field != nil && flags.Changed(flagSysCtlRemove) { + values := flags.Lookup(flagSysCtlRemove).Value.(*opts.ListOpts).GetAll() + for key := range opts.ConvertKVStringsToMap(values) { + delete(*field, key) + } + } + if flags.Changed(flagSysCtlAdd) { + if *field == nil { + *field = map[string]string{} + } + + values := flags.Lookup(flagSysCtlAdd).Value.(*opts.ListOpts).GetAll() + for key, value := range opts.ConvertKVStringsToMap(values) { + (*field)[key] = value + } + } +} + func updateEnvironment(flags *pflag.FlagSet, field *[]string) { if flags.Changed(flagEnvAdd) { envSet := map[string]string{} diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index 6ece18195c..bd35750217 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -828,3 +828,100 @@ func TestUpdateMaxReplicas(t *testing.T) { assert.DeepEqual(t, svc.TaskTemplate.Placement, &swarm.Placement{MaxReplicas: uint64(2)}) } + +func TestUpdateSysCtls(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + spec map[string]string + add []string + rm []string + expected map[string]string + }{ + { + name: "from scratch", + add: []string{"sysctl.zet=value-99", "sysctl.alpha=value-1"}, + expected: map[string]string{"sysctl.zet": "value-99", "sysctl.alpha": "value-1"}, + }, + { + name: "append new", + spec: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2"}, + add: []string{"new.sysctl=newvalue"}, + expected: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2", "new.sysctl": "newvalue"}, + }, + { + name: "append duplicate is a no-op", + spec: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2"}, + add: []string{"sysctl.one=value-1"}, + expected: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2"}, + }, + { + name: "remove and append existing is a no-op", + spec: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2"}, + add: []string{"sysctl.one=value-1"}, + rm: []string{"sysctl.one=value-1"}, + expected: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2"}, + }, + { + name: "remove and append new should append", + spec: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2"}, + add: []string{"new.sysctl=newvalue"}, + rm: []string{"new.sysctl=newvalue"}, + expected: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2", "new.sysctl": "newvalue"}, + }, + { + name: "update existing", + spec: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2"}, + add: []string{"sysctl.one=newvalue"}, + expected: map[string]string{"sysctl.one": "newvalue", "sysctl.two": "value-2"}, + }, + { + name: "update existing twice", + spec: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2"}, + add: []string{"sysctl.one=newvalue", "sysctl.one=evennewervalue"}, + expected: map[string]string{"sysctl.one": "evennewervalue", "sysctl.two": "value-2"}, + }, + { + name: "remove all", + spec: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2"}, + rm: []string{"sysctl.one=value-1", "sysctl.two=value-2"}, + expected: map[string]string{}, + }, + { + name: "remove by key", + spec: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2"}, + rm: []string{"sysctl.one"}, + expected: map[string]string{"sysctl.two": "value-2"}, + }, + { + name: "remove by key and different value", + spec: map[string]string{"sysctl.one": "value-1", "sysctl.two": "value-2"}, + rm: []string{"sysctl.one=anyvalueyoulike"}, + expected: map[string]string{"sysctl.two": "value-2"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + svc := swarm.ServiceSpec{ + TaskTemplate: swarm.TaskSpec{ + ContainerSpec: &swarm.ContainerSpec{Sysctls: tc.spec}, + }, + } + flags := newUpdateCommand(nil).Flags() + for _, v := range tc.add { + assert.NilError(t, flags.Set(flagSysCtlAdd, v)) + } + for _, v := range tc.rm { + assert.NilError(t, flags.Set(flagSysCtlRemove, v)) + } + err := updateService(ctx, &fakeClient{}, flags, &svc) + assert.NilError(t, err) + if !assert.Check(t, is.DeepEqual(svc.TaskTemplate.ContainerSpec.Sysctls, tc.expected)) { + t.Logf("expected: %v", tc.expected) + t.Logf("actual: %v", svc.TaskTemplate.ContainerSpec.Sysctls) + } + }) + } +} diff --git a/cli/compose/convert/service.go b/cli/compose/convert/service.go index 7c98244d8e..21f4850c87 100644 --- a/cli/compose/convert/service.go +++ b/cli/compose/convert/service.go @@ -151,6 +151,7 @@ func Service( Privileges: &privileges, Isolation: container.Isolation(service.Isolation), Init: service.Init, + Sysctls: service.Sysctls, }, LogDriver: logDriver, Resources: resources, diff --git a/cli/compose/loader/full-example.yml b/cli/compose/loader/full-example.yml index 518c5d8c3d..f452812a69 100644 --- a/cli/compose/loader/full-example.yml +++ b/cli/compose/loader/full-example.yml @@ -240,6 +240,10 @@ services: stop_signal: SIGUSR1 + sysctls: + net.core.somaxconn: 1024 + net.ipv4.tcp_syncookies: 0 + # String or list # tmpfs: /run tmpfs: diff --git a/cli/compose/loader/full-struct_test.go b/cli/compose/loader/full-struct_test.go index 355c389447..427f2e8e06 100644 --- a/cli/compose/loader/full-struct_test.go +++ b/cli/compose/loader/full-struct_test.go @@ -346,8 +346,12 @@ func services(workingDir, homeDir string) []types.ServiceConfig { StdinOpen: true, StopSignal: "SIGUSR1", StopGracePeriod: durationPtr(20 * time.Second), - Tmpfs: []string{"/run", "/tmp"}, - Tty: true, + Sysctls: map[string]string{ + "net.core.somaxconn": "1024", + "net.ipv4.tcp_syncookies": "0", + }, + Tmpfs: []string{"/run", "/tmp"}, + Tty: true, Ulimits: map[string]*types.UlimitsConfig{ "nproc": { Single: 65535, @@ -756,6 +760,9 @@ services: stdin_open: true stop_grace_period: 20s stop_signal: SIGUSR1 + sysctls: + net.core.somaxconn: "1024" + net.ipv4.tcp_syncookies: "0" tmpfs: - /run - /tmp @@ -1325,6 +1332,10 @@ func fullExampleJSON(workingDir string) string { "stdin_open": true, "stop_grace_period": "20s", "stop_signal": "SIGUSR1", + "sysctls": { + "net.core.somaxconn": "1024", + "net.ipv4.tcp_syncookies": "0" + }, "tmpfs": [ "/run", "/tmp" diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go index 1a920ee2fe..a3df518b31 100644 --- a/cli/compose/loader/loader.go +++ b/cli/compose/loader/loader.go @@ -304,6 +304,7 @@ func createTransformHook(additionalTransformers ...Transformer) mapstructure.Dec reflect.TypeOf(types.ServiceConfigObjConfig{}): transformStringSourceMap, reflect.TypeOf(types.StringOrNumberList{}): transformStringOrNumberList, reflect.TypeOf(map[string]*types.ServiceNetworkConfig{}): transformServiceNetworkMap, + reflect.TypeOf(types.Mapping{}): transformMappingOrListFunc("=", false), reflect.TypeOf(types.MappingWithEquals{}): transformMappingOrListFunc("=", true), reflect.TypeOf(types.Labels{}): transformMappingOrListFunc("=", false), reflect.TypeOf(types.MappingWithColon{}): transformMappingOrListFunc(":", false), diff --git a/cli/compose/loader/loader_test.go b/cli/compose/loader/loader_test.go index 651b04655a..51eb4fb461 100644 --- a/cli/compose/loader/loader_test.go +++ b/cli/compose/loader/loader_test.go @@ -1461,6 +1461,47 @@ services: } } +func TestLoadSysctls(t *testing.T) { + config, err := loadYAML(` +version: "3.8" +services: + web: + image: busybox + sysctls: + - net.core.somaxconn=1024 + - net.ipv4.tcp_syncookies=0 + - testing.one.one= + - testing.one.two +`) + assert.NilError(t, err) + + expected := types.Mapping{ + "net.core.somaxconn": "1024", + "net.ipv4.tcp_syncookies": "0", + "testing.one.one": "", + "testing.one.two": "", + } + + assert.Assert(t, is.Len(config.Services, 1)) + assert.Check(t, is.DeepEqual(expected, config.Services[0].Sysctls)) + + config, err = loadYAML(` +version: "3.8" +services: + web: + image: busybox + sysctls: + net.core.somaxconn: 1024 + net.ipv4.tcp_syncookies: 0 + testing.one.one: "" + testing.one.two: +`) + assert.NilError(t, err) + + assert.Assert(t, is.Len(config.Services, 1)) + assert.Check(t, is.DeepEqual(expected, config.Services[0].Sysctls)) +} + func TestTransform(t *testing.T) { var source = []interface{}{ "80-82:8080-8082", diff --git a/cli/compose/types/types.go b/cli/compose/types/types.go index 6ca27579ce..8185f836c8 100644 --- a/cli/compose/types/types.go +++ b/cli/compose/types/types.go @@ -24,7 +24,6 @@ var UnsupportedProperties = []string{ "restart", "security_opt", "shm_size", - "sysctls", "ulimits", "userns_mode", } @@ -200,7 +199,7 @@ type ServiceConfig struct { StdinOpen bool `mapstructure:"stdin_open" yaml:"stdin_open,omitempty" json:"stdin_open,omitempty"` StopGracePeriod *Duration `mapstructure:"stop_grace_period" yaml:"stop_grace_period,omitempty" json:"stop_grace_period,omitempty"` StopSignal string `mapstructure:"stop_signal" yaml:"stop_signal,omitempty" json:"stop_signal,omitempty"` - Sysctls StringList `yaml:",omitempty" json:"sysctls,omitempty"` + Sysctls Mapping `yaml:",omitempty" json:"sysctls,omitempty"` Tmpfs StringList `yaml:",omitempty" json:"tmpfs,omitempty"` Tty bool `mapstructure:"tty" yaml:"tty,omitempty" json:"tty,omitempty"` Ulimits map[string]*UlimitsConfig `yaml:",omitempty" json:"ulimits,omitempty"` @@ -240,6 +239,12 @@ type StringOrNumberList []string // For the key without value (`key`), the mapped value is set to nil. type MappingWithEquals map[string]*string +// Mapping is a mapping type that can be converted from a list of +// key[=value] strings. +// For the key with an empty value (`key=`), or key without value (`key`), the +// mapped value is set to an empty string `""`. +type Mapping map[string]string + // Labels is a mapping type for labels type Labels map[string]string diff --git a/contrib/completion/bash/docker b/contrib/completion/bash/docker index 4e09dd5842..50adc690e3 100644 --- a/contrib/completion/bash/docker +++ b/contrib/completion/bash/docker @@ -3696,6 +3696,7 @@ _docker_service_update_and_create() { --placement-pref --publish -p --secret + --sysctl " case "$prev" in @@ -3746,6 +3747,8 @@ _docker_service_update_and_create() { --rollback --secret-add --secret-rm + --sysctl-add + --sysctl-rm " boolean_options="$boolean_options diff --git a/docs/reference/commandline/service_create.md b/docs/reference/commandline/service_create.md index daf7107540..0b66badf48 100644 --- a/docs/reference/commandline/service_create.md +++ b/docs/reference/commandline/service_create.md @@ -77,6 +77,7 @@ Options: --secret secret Specify secrets to expose to the service --stop-grace-period duration Time to wait before force killing a container (ns|us|ms|s|m|h) (default 10s) --stop-signal string Signal to stop the container + --sysctl list Sysctl options -t, --tty Allocate a pseudo-TTY --update-delay duration Delay between updates (ns|us|ms|s|m|h) (default 0s) --update-failure-action string Action on update failure ("pause"|"continue"|"rollback") (default "pause") diff --git a/docs/reference/commandline/service_update.md b/docs/reference/commandline/service_update.md index f3d70495b3..ed11080fc5 100644 --- a/docs/reference/commandline/service_update.md +++ b/docs/reference/commandline/service_update.md @@ -93,6 +93,8 @@ Options: --secret-rm list Remove a secret --stop-grace-period duration Time to wait before force killing a container (ns|us|ms|s|m|h) --stop-signal string Signal to stop the container + --sysctl-add list Add or update a Sysctl option + --sysctl-rm list Remove a Sysctl option -t, --tty Allocate a pseudo-TTY --update-delay duration Delay between updates (ns|us|ms|s|m|h) --update-failure-action string Action on update failure ("pause"|"continue"|"rollback")