Browse code

Fix ulimits in `docker inspect` when daemon default exists

This fix tries to fix 26326 where `docker inspect` will not show
ulimit even when daemon default ulimit has been set.

This fix merge the HostConfig's ulimit with daemon default in
`docker inspect`, so that when daemon is started with `default-ulimit`
and HostConfig's ulimit is not set, `docker inspect` will output
the daemon default.

An integration test has been added to cover the changes.

This fix fixes 26326.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

Yong Tang authored on 2016/09/08 13:23:56
Showing 5 changed files
... ...
@@ -110,6 +110,9 @@ func (daemon *Daemon) getInspectData(container *container.Container, size bool)
110 110
 		hostConfig.Links = append(hostConfig.Links, fmt.Sprintf("%s:%s", child.Name, linkAlias))
111 111
 	}
112 112
 
113
+	// We merge the Ulimits from hostConfig with daemon default
114
+	daemon.mergeUlimits(&hostConfig)
115
+
113 116
 	var containerHealth *types.Health
114 117
 	if container.State.Health != nil {
115 118
 		containerHealth = &types.Health{
... ...
@@ -115,19 +115,11 @@ func setDevices(s *specs.Spec, c *container.Container) error {
115 115
 func setRlimits(daemon *Daemon, s *specs.Spec, c *container.Container) error {
116 116
 	var rlimits []specs.Rlimit
117 117
 
118
-	ulimits := c.HostConfig.Ulimits
119
-	// Merge ulimits with daemon defaults
120
-	ulIdx := make(map[string]struct{})
121
-	for _, ul := range ulimits {
122
-		ulIdx[ul.Name] = struct{}{}
123
-	}
124
-	for name, ul := range daemon.configStore.Ulimits {
125
-		if _, exists := ulIdx[name]; !exists {
126
-			ulimits = append(ulimits, ul)
127
-		}
128
-	}
129
-
130
-	for _, ul := range ulimits {
118
+	// We want to leave the original HostConfig alone so make a copy here
119
+	hostConfig := *c.HostConfig
120
+	// Merge with the daemon defaults
121
+	daemon.mergeUlimits(&hostConfig)
122
+	for _, ul := range hostConfig.Ulimits {
131 123
 		rlimits = append(rlimits, specs.Rlimit{
132 124
 			Type: "RLIMIT_" + strings.ToUpper(ul.Name),
133 125
 			Soft: uint64(ul.Soft),
... ...
@@ -709,3 +701,19 @@ func clearReadOnly(m *specs.Mount) {
709 709
 	}
710 710
 	m.Options = opt
711 711
 }
712
+
713
+// mergeUlimits merge the Ulimits from HostConfig with daemon defaults, and update HostConfig
714
+func (daemon *Daemon) mergeUlimits(c *containertypes.HostConfig) {
715
+	ulimits := c.Ulimits
716
+	// Merge ulimits with daemon defaults
717
+	ulIdx := make(map[string]struct{})
718
+	for _, ul := range ulimits {
719
+		ulIdx[ul.Name] = struct{}{}
720
+	}
721
+	for name, ul := range daemon.configStore.Ulimits {
722
+		if _, exists := ulIdx[name]; !exists {
723
+			ulimits = append(ulimits, ul)
724
+		}
725
+	}
726
+	c.Ulimits = ulimits
727
+}
... ...
@@ -1,6 +1,7 @@
1 1
 package daemon
2 2
 
3 3
 import (
4
+	containertypes "github.com/docker/docker/api/types/container"
4 5
 	"github.com/docker/docker/container"
5 6
 	"github.com/docker/docker/libcontainerd"
6 7
 	"github.com/docker/docker/oci"
... ...
@@ -10,3 +11,9 @@ func (daemon *Daemon) createSpec(c *container.Container) (*libcontainerd.Spec, e
10 10
 	s := oci.DefaultSpec()
11 11
 	return (*libcontainerd.Spec)(&s), nil
12 12
 }
13
+
14
+// mergeUlimits merge the Ulimits from HostConfig with daemon defaults, and update HostConfig
15
+// It will do nothing on non-Linux platform
16
+func (daemon *Daemon) mergeUlimits(c *containertypes.HostConfig) {
17
+	return
18
+}
... ...
@@ -7,6 +7,7 @@ import (
7 7
 	"path/filepath"
8 8
 	"syscall"
9 9
 
10
+	containertypes "github.com/docker/docker/api/types/container"
10 11
 	"github.com/docker/docker/container"
11 12
 	"github.com/docker/docker/image"
12 13
 	"github.com/docker/docker/layer"
... ...
@@ -192,3 +193,9 @@ func escapeArgs(args []string) []string {
192 192
 	}
193 193
 	return escapedArgs
194 194
 }
195
+
196
+// mergeUlimits merge the Ulimits from HostConfig with daemon defaults, and update HostConfig
197
+// It will do nothing on non-Linux platform
198
+func (daemon *Daemon) mergeUlimits(c *containertypes.HostConfig) {
199
+	return
200
+}
... ...
@@ -4518,3 +4518,25 @@ exec "$@"`,
4518 4518
 	c.Assert(err, check.NotNil)
4519 4519
 	c.Assert(err.Error(), checker.Contains, "No command specified")
4520 4520
 }
4521
+
4522
+func (s *DockerDaemonSuite) TestRunWithUlimitAndDaemonDefault(c *check.C) {
4523
+	c.Assert(s.d.StartWithBusybox("--debug", "--default-ulimit=nofile=65535"), checker.IsNil)
4524
+
4525
+	name := "test-A"
4526
+	_, err := s.d.Cmd("run", "--name", name, "-d", "busybox", "top")
4527
+	c.Assert(err, checker.IsNil)
4528
+	c.Assert(s.d.waitRun(name), check.IsNil)
4529
+
4530
+	out, err := s.d.Cmd("inspect", "--format", "{{.HostConfig.Ulimits}}", name)
4531
+	c.Assert(err, checker.IsNil)
4532
+	c.Assert(out, checker.Contains, "[nofile=65535:65535]")
4533
+
4534
+	name = "test-B"
4535
+	_, err = s.d.Cmd("run", "--name", name, "--ulimit=nofile=42", "-d", "busybox", "top")
4536
+	c.Assert(err, checker.IsNil)
4537
+	c.Assert(s.d.waitRun(name), check.IsNil)
4538
+
4539
+	out, err = s.d.Cmd("inspect", "--format", "{{.HostConfig.Ulimits}}", name)
4540
+	c.Assert(err, checker.IsNil)
4541
+	c.Assert(out, checker.Contains, "[nofile=42:42]")
4542
+}