Merge pull request #2712 from thaJeztah/carry_2660_ulimits

Add ulimits support to docker service and docker stack deploy (carry 2660)

cli/command/service/create.go

@@ -64,6 +64,8 @@ func newCreateCommand(dockerCli command.Cli) *cobra.Command {
 	flags.SetAnnotation(flagInit, "version", []string{"1.37"})
 	flags.Var(&opts.sysctls, flagSysCtl, "Sysctl options")
 	flags.SetAnnotation(flagSysCtl, "version", []string{"1.40"})
+	flags.Var(&opts.ulimits, flagUlimit, "Ulimit options")
+	flags.SetAnnotation(flagUlimit, "version", []string{"1.41"})
 
 	flags.Var(cliopts.NewListOptsRef(&opts.resources.resGenericResources, ValidateSingleGenericResource), "generic-resource", "User defined resources")
 	flags.SetAnnotation(flagHostAdd, "version", []string{"1.32"})

cli/command/service/formatter.go

@@ -111,6 +111,11 @@ SysCtls:
 {{- range $k, $v := .ContainerSysCtls }}
  {{ $k }}{{if $v }}: {{ $v }}{{ end }}
 {{- end }}{{ end }}
+{{- if .ContainerUlimits }}
+Ulimits:
+{{- range $k, $v := .ContainerUlimits }}
+ {{ $k }}: {{ $v }}
+{{- end }}{{ end }}
 {{- if .ContainerMounts }}
 Mounts:
 {{- end }}
@@ -467,6 +472,20 @@ func (ctx *serviceInspectContext) HasContainerSysCtls() bool {
 	return len(ctx.Service.Spec.TaskTemplate.ContainerSpec.Sysctls) > 0
 }
 
+func (ctx *serviceInspectContext) ContainerUlimits() map[string]string {
+	ulimits := map[string]string{}
+
+	for _, u := range ctx.Service.Spec.TaskTemplate.ContainerSpec.Ulimits {
+		ulimits[u.Name] = fmt.Sprintf("%d:%d", u.Soft, u.Hard)
+	}
+
+	return ulimits
+}
+
+func (ctx *serviceInspectContext) HasContainerUlimits() bool {
+	return len(ctx.Service.Spec.TaskTemplate.ContainerSpec.Ulimits) > 0
+}
+
 func (ctx *serviceInspectContext) HasResources() bool {
 	return ctx.Service.Spec.TaskTemplate.Resources != nil
 }

cli/command/service/opts.go

@@ -508,6 +508,7 @@ type serviceOptions struct {
 	sysctls         opts.ListOpts
 	capAdd          opts.ListOpts
 	capDrop         opts.ListOpts
+	ulimits         opts.UlimitOpt
 
 	resources resourceOptions
 	stopGrace opts.DurationOpt
@@ -553,6 +554,7 @@ func newServiceOptions() *serviceOptions {
 		sysctls:         opts.NewListOpts(nil),
 		capAdd:          opts.NewListOpts(nil),
 		capDrop:         opts.NewListOpts(nil),
+		ulimits:         *opts.NewUlimitOpt(nil),
 	}
 }
 
@@ -724,6 +726,7 @@ func (options *serviceOptions) ToService(ctx context.Context, apiClient client.N
 				Sysctls:         opts.ConvertKVStringsToMap(options.sysctls.GetAll()),
 				CapabilityAdd:   capAdd,
 				CapabilityDrop:  capDrop,
+				Ulimits:         options.ulimits.GetList(),
 			},
 			Networks:      networks,
 			Resources:     resources,
@@ -1015,6 +1018,9 @@ const (
 	flagIsolation               = "isolation"
 	flagCapAdd                  = "cap-add"
 	flagCapDrop                 = "cap-drop"
+	flagUlimit                  = "ulimit"
+	flagUlimitAdd               = "ulimit-add"
+	flagUlimitRemove            = "ulimit-rm"
 )
 
 func validateAPIVersion(c swarm.ServiceSpec, serverAPIVersion string) error {

cli/command/service/update.go

@@ -16,6 +16,7 @@ import (
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/api/types/versions"
 	"github.com/docker/docker/client"
+	units "github.com/docker/go-units"
 	"github.com/docker/swarmkit/api/defaults"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -100,6 +101,10 @@ func newUpdateCommand(dockerCli command.Cli) *cobra.Command {
 	flags.SetAnnotation(flagSysCtlAdd, "version", []string{"1.40"})
 	flags.Var(newListOptsVar(), flagSysCtlRemove, "Remove a Sysctl option")
 	flags.SetAnnotation(flagSysCtlRemove, "version", []string{"1.40"})
+	flags.Var(&options.ulimits, flagUlimitAdd, "Add or update a ulimit option")
+	flags.SetAnnotation(flagUlimitAdd, "version", []string{"1.41"})
+	flags.Var(newListOptsVar(), flagUlimitRemove, "Remove a ulimit option")
+	flags.SetAnnotation(flagUlimitRemove, "version", []string{"1.41"})
 
 	// Add needs parsing, Remove only needs the key
 	flags.Var(newListOptsVar(), flagGenericResourcesRemove, "Remove a Generic resource")
@@ -344,6 +349,7 @@ func updateService(ctx context.Context, apiClient client.NetworkAPIClient, flags
 	}
 
 	updateSysCtls(flags, &task.ContainerSpec.Sysctls)
+	task.ContainerSpec.Ulimits = updateUlimits(flags, task.ContainerSpec.Ulimits)
 
 	if anyChanged(flags, flagLimitCPU, flagLimitMemory, flagLimitPids) {
 		taskResources().Limits = spec.TaskTemplate.Resources.Limits
@@ -700,6 +706,35 @@ func updateSysCtls(flags *pflag.FlagSet, field *map[string]string) {
 	}
 }
 
+func updateUlimits(flags *pflag.FlagSet, ulimits []*units.Ulimit) []*units.Ulimit {
+	newUlimits := make(map[string]*units.Ulimit)
+
+	for _, ulimit := range ulimits {
+		newUlimits[ulimit.Name] = ulimit
+	}
+	if flags.Changed(flagUlimitRemove) {
+		values := flags.Lookup(flagUlimitRemove).Value.(*opts.ListOpts).GetAll()
+		for key := range opts.ConvertKVStringsToMap(values) {
+			delete(newUlimits, key)
+		}
+	}
+
+	if flags.Changed(flagUlimitAdd) {
+		for _, ulimit := range flags.Lookup(flagUlimitAdd).Value.(*opts.UlimitOpt).GetList() {
+			newUlimits[ulimit.Name] = ulimit
+		}
+	}
+
+	var limits []*units.Ulimit
+	for _, ulimit := range newUlimits {
+		limits = append(limits, ulimit)
+	}
+	sort.SliceStable(limits, func(i, j int) bool {
+		return limits[i].Name < limits[j].Name
+	})
+	return limits
+}
+
 func updateEnvironment(flags *pflag.FlagSet, field *[]string) {
 	toRemove := buildToRemoveSet(flags, flagEnvRemove)
 	*field = removeItems(*field, toRemove, envKey)

cli/command/service/update_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/docker/docker/api/types/container"
 	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/api/types/swarm"
+	"github.com/docker/go-units"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
@@ -1585,3 +1586,120 @@ func TestUpdateCaps(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateUlimits(t *testing.T) {
+	ctx := context.Background()
+
+	tests := []struct {
+		name     string
+		spec     []*units.Ulimit
+		rm       []string
+		add      []string
+		expected []*units.Ulimit
+	}{
+		{
+			name: "from scratch",
+			add:  []string{"nofile=512:1024", "core=1024:1024"},
+			expected: []*units.Ulimit{
+				{Name: "core", Hard: 1024, Soft: 1024},
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+		},
+		{
+			name: "append new",
+			spec: []*units.Ulimit{
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+			add: []string{"core=1024:1024"},
+			expected: []*units.Ulimit{
+				{Name: "core", Hard: 1024, Soft: 1024},
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+		},
+		{
+			name: "remove and append new should append",
+			spec: []*units.Ulimit{
+				{Name: "core", Hard: 1024, Soft: 1024},
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+			rm:  []string{"nofile=512:1024"},
+			add: []string{"nofile=512:1024"},
+			expected: []*units.Ulimit{
+				{Name: "core", Hard: 1024, Soft: 1024},
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+		},
+		{
+			name: "update existing",
+			spec: []*units.Ulimit{
+				{Name: "nofile", Hard: 2048, Soft: 1024},
+			},
+			add: []string{"nofile=512:1024"},
+			expected: []*units.Ulimit{
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+		},
+		{
+			name: "update existing twice",
+			spec: []*units.Ulimit{
+				{Name: "nofile", Hard: 2048, Soft: 1024},
+			},
+			add: []string{"nofile=256:512", "nofile=512:1024"},
+			expected: []*units.Ulimit{
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+		},
+		{
+			name: "remove all",
+			spec: []*units.Ulimit{
+				{Name: "core", Hard: 1024, Soft: 1024},
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+			rm:       []string{"nofile=512:1024", "core=1024:1024"},
+			expected: nil,
+		},
+		{
+			name: "remove by key",
+			spec: []*units.Ulimit{
+				{Name: "core", Hard: 1024, Soft: 1024},
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+			rm: []string{"core"},
+			expected: []*units.Ulimit{
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+		},
+		{
+			name: "remove by key and different value",
+			spec: []*units.Ulimit{
+				{Name: "core", Hard: 1024, Soft: 1024},
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+			rm: []string{"core=1234:5678"},
+			expected: []*units.Ulimit{
+				{Name: "nofile", Hard: 1024, Soft: 512},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			svc := swarm.ServiceSpec{
+				TaskTemplate: swarm.TaskSpec{
+					ContainerSpec: &swarm.ContainerSpec{Ulimits: tc.spec},
+				},
+			}
+			flags := newUpdateCommand(nil).Flags()
+			for _, v := range tc.add {
+				assert.NilError(t, flags.Set(flagUlimitAdd, v))
+			}
+			for _, v := range tc.rm {
+				assert.NilError(t, flags.Set(flagUlimitRemove, v))
+			}
+			err := updateService(ctx, &fakeClient{}, flags, &svc)
+			assert.NilError(t, err)
+			assert.DeepEqual(t, svc.TaskTemplate.ContainerSpec.Ulimits, tc.expected)
+		})
+	}
+}

cli/compose/convert/service.go

@@ -14,6 +14,7 @@ import (
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/api/types/versions"
 	"github.com/docker/docker/client"
+	"github.com/docker/go-units"
 	"github.com/pkg/errors"
 )
 
@@ -151,6 +152,7 @@ func Service(
 				Sysctls:         service.Sysctls,
 				CapabilityAdd:   capAdd,
 				CapabilityDrop:  capDrop,
+				Ulimits:         convertUlimits(service.Ulimits),
 			},
 			LogDriver:     logDriver,
 			Resources:     resources,
@@ -680,3 +682,30 @@ func convertCredentialSpec(namespace Namespace, spec composetypes.CredentialSpec
 	}
 	return &swarmCredSpec, nil
 }
+
+func convertUlimits(origUlimits map[string]*composetypes.UlimitsConfig) []*units.Ulimit {
+	newUlimits := make(map[string]*units.Ulimit)
+	for name, u := range origUlimits {
+		if u.Single != 0 {
+			newUlimits[name] = &units.Ulimit{
+				Name: name,
+				Soft: int64(u.Single),
+				Hard: int64(u.Single),
+			}
+		} else {
+			newUlimits[name] = &units.Ulimit{
+				Name: name,
+				Soft: int64(u.Soft),
+				Hard: int64(u.Hard),
+			}
+		}
+	}
+	var ulimits []*units.Ulimit
+	for _, ulimit := range newUlimits {
+		ulimits = append(ulimits, ulimit)
+	}
+	sort.SliceStable(ulimits, func(i, j int) bool {
+		return ulimits[i].Name < ulimits[j].Name
+	})
+	return ulimits
+}

cli/compose/types/types.go

@@ -23,7 +23,6 @@ var UnsupportedProperties = []string{
 	"restart",
 	"security_opt",
 	"shm_size",
-	"ulimits",
 	"userns_mode",
 }
 

contrib/completion/bash/docker

@@ -3756,6 +3756,7 @@ _docker_service_update_and_create() {
 			--publish -p
 			--secret
 			--sysctl
+			--ulimit
 		"
 
 		case "$prev" in
@@ -3808,6 +3809,8 @@ _docker_service_update_and_create() {
 			--secret-rm
 			--sysctl-add
 			--sysctl-rm
+			--ulimit-add
+			--ulimit-rm
 		"
 
 		boolean_options="$boolean_options

docs/reference/commandline/service_create.md

@@ -74,6 +74,7 @@ Options:
       --stop-signal string                 Signal to stop the container
       --sysctl list                        Sysctl options
   -t, --tty                                Allocate a pseudo-TTY
+      --ulimit ulimit                      Ulimit options (default [])
       --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")
       --update-max-failure-ratio float     Failure rate to tolerate during an update (default 0)

docs/reference/commandline/service_update.md

@@ -91,6 +91,8 @@ Options:
       --sysctl-add list                    Add or update a Sysctl option
       --sysctl-rm list                     Remove a Sysctl option
   -t, --tty                                Allocate a pseudo-TTY
+      --ulimit-add ulimit                  Add or update a ulimit option (default [])
+      --ulimit-rm list                     Remove a ulimit option
       --update-delay duration              Delay between updates (ns|us|ms|s|m|h)
       --update-failure-action string       Action on update failure ("pause"|"continue"|"rollback")
       --update-max-failure-ratio float     Failure rate to tolerate during an update

opts/ulimit.go

@@ -2,6 +2,7 @@ package opts
 
 import (
 	"fmt"
+	"sort"
 
 	"github.com/docker/go-units"
 )
@@ -11,7 +12,7 @@ type UlimitOpt struct {
 	values *map[string]*units.Ulimit
 }
 
-// NewUlimitOpt creates a new UlimitOpt
+// NewUlimitOpt creates a new UlimitOpt. Ulimits are not validated.
 func NewUlimitOpt(ref *map[string]*units.Ulimit) *UlimitOpt {
 	if ref == nil {
 		ref = &map[string]*units.Ulimit{}
@@ -31,23 +32,25 @@ func (o *UlimitOpt) Set(val string) error {
 	return nil
 }
 
-// String returns Ulimit values as a string.
+// String returns Ulimit values as a string. Values are sorted by name.
 func (o *UlimitOpt) String() string {
 	var out []string
 	for _, v := range *o.values {
 		out = append(out, v.String())
 	}
-
+	sort.Strings(out)
 	return fmt.Sprintf("%v", out)
 }
 
-// GetList returns a slice of pointers to Ulimits.
+// GetList returns a slice of pointers to Ulimits. Values are sorted by name.
 func (o *UlimitOpt) GetList() []*units.Ulimit {
 	var ulimits []*units.Ulimit
 	for _, v := range *o.values {
 		ulimits = append(ulimits, v)
 	}
-
+	sort.SliceStable(ulimits, func(i, j int) bool {
+		return ulimits[i].Name < ulimits[j].Name
+	})
 	return ulimits
 }
 

opts/ulimit_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/docker/go-units"
+	"gotest.tools/v3/assert"
 )
 
 func TestUlimitOpt(t *testing.T) {
@@ -14,29 +15,40 @@ func TestUlimitOpt(t *testing.T) {
 	ulimitOpt := NewUlimitOpt(&ulimitMap)
 
 	expected := "[nofile=512:1024]"
-	if ulimitOpt.String() != expected {
+	assert.Equal(t, ulimitOpt.String(), expected)
-		t.Fatalf("Expected %v, got %v", expected, ulimitOpt)
-	}
 
 	// Valid ulimit append to opts
-	if err := ulimitOpt.Set("core=1024:1024"); err != nil {
+	err := ulimitOpt.Set("core=1024:1024")
-		t.Fatal(err)
+	assert.NilError(t, err)
-	}
+
+	err = ulimitOpt.Set("nofile")
+	assert.ErrorContains(t, err, "invalid ulimit argument")
 
 	// Invalid ulimit type returns an error and do not append to opts
-	if err := ulimitOpt.Set("notavalidtype=1024:1024"); err == nil {
+	err = ulimitOpt.Set("notavalidtype=1024:1024")
-		t.Fatalf("Expected error on invalid ulimit type")
+	assert.ErrorContains(t, err, "invalid ulimit type")
-	}
+
-	expected = "[nofile=512:1024 core=1024:1024]"
+	expected = "[core=1024:1024 nofile=512:1024]"
-	expected2 := "[core=1024:1024 nofile=512:1024]"
+	assert.Equal(t, ulimitOpt.String(), expected)
-	result := ulimitOpt.String()
-	if result != expected && result != expected2 {
-		t.Fatalf("Expected %v or %v, got %v", expected, expected2, ulimitOpt)
-	}
 
 	// And test GetList
 	ulimits := ulimitOpt.GetList()
-	if len(ulimits) != 2 {
+	assert.Equal(t, len(ulimits), 2)
-		t.Fatalf("Expected a ulimit list of 2, got %v", ulimits)
+}
-	}
+
+func TestUlimitOptSorting(t *testing.T) {
+	ulimitOpt := NewUlimitOpt(&map[string]*units.Ulimit{
+		"nofile": {Name: "nofile", Hard: 1024, Soft: 512},
+		"core":   {Name: "core", Hard: 1024, Soft: 1024},
+	})
+
+	expected := []*units.Ulimit{
+		{Name: "core", Hard: 1024, Soft: 1024},
+		{Name: "nofile", Hard: 1024, Soft: 512},
+	}
+
+	ulimits := ulimitOpt.GetList()
+	assert.DeepEqual(t, ulimits, expected)
+
+	assert.Equal(t, ulimitOpt.String(), "[core=1024:1024 nofile=512:1024]")
 }