Browse code

Update opts.MemBytes to disable default, and move `docker run/create/build` to use opts.MemBytes

This fix made several updates:
1. Update opts.MemBytes so that default value will not show up.
The reason is that in case a default value is decided by daemon,
instead of client, we actually want to not show default value.
2. Move `docker run/create/build` to use opts.MemBytes for `--shm-size`
This is to bring consistency between daemon and docker run
3. docs updates.

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

Yong Tang authored on 2016/12/29 07:44:07
Showing 10 changed files
... ...
@@ -100,7 +100,7 @@ type containerOptions struct {
100 100
 	stopSignal         string
101 101
 	stopTimeout        int
102 102
 	isolation          string
103
-	shmSize            string
103
+	shmSize            opts.MemBytes
104 104
 	noHealthcheck      bool
105 105
 	healthCmd          string
106 106
 	healthInterval     time.Duration
... ...
@@ -259,7 +259,7 @@ func addFlags(flags *pflag.FlagSet) *containerOptions {
259 259
 	flags.StringVar(&copts.ipcMode, "ipc", "", "IPC namespace to use")
260 260
 	flags.StringVar(&copts.isolation, "isolation", "", "Container isolation technology")
261 261
 	flags.StringVar(&copts.pidMode, "pid", "", "PID namespace to use")
262
-	flags.StringVar(&copts.shmSize, "shm-size", "", "Size of /dev/shm, default value is 64MB")
262
+	flags.Var(&copts.shmSize, "shm-size", "Size of /dev/shm")
263 263
 	flags.StringVar(&copts.utsMode, "uts", "", "UTS namespace to use")
264 264
 	flags.StringVar(&copts.runtime, "runtime", "", "Runtime to use for this container")
265 265
 
... ...
@@ -336,14 +336,6 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*container.Config, *c
336 336
 		return nil, nil, nil, fmt.Errorf("invalid value: %d. Valid memory swappiness range is 0-100", swappiness)
337 337
 	}
338 338
 
339
-	var shmSize int64
340
-	if copts.shmSize != "" {
341
-		shmSize, err = units.RAMInBytes(copts.shmSize)
342
-		if err != nil {
343
-			return nil, nil, nil, err
344
-		}
345
-	}
346
-
347 339
 	// TODO FIXME units.RAMInBytes should have a uint64 version
348 340
 	var maxIOBandwidth int64
349 341
 	if copts.ioMaxBandwidth != "" {
... ...
@@ -615,7 +607,7 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*container.Config, *c
615 615
 		LogConfig:      container.LogConfig{Type: copts.loggingDriver, Config: loggingOpts},
616 616
 		VolumeDriver:   copts.volumeDriver,
617 617
 		Isolation:      container.Isolation(copts.isolation),
618
-		ShmSize:        shmSize,
618
+		ShmSize:        copts.shmSize.Value(),
619 619
 		Resources:      resources,
620 620
 		Tmpfs:          tmpfs,
621 621
 		Sysctls:        copts.sysctls.GetAll(),
... ...
@@ -411,8 +411,9 @@ func TestParseModes(t *testing.T) {
411 411
 		t.Fatalf("Expected a valid UTSMode, got %v", hostconfig.UTSMode)
412 412
 	}
413 413
 	// shm-size ko
414
-	if _, _, _, err = parseRun([]string{"--shm-size=a128m", "img", "cmd"}); err == nil || err.Error() != "invalid size: 'a128m'" {
415
-		t.Fatalf("Expected an error with message 'invalid size: a128m', got %v", err)
414
+	expectedErr := `invalid argument "a128m" for --shm-size=a128m: invalid size: 'a128m'`
415
+	if _, _, _, err = parseRun([]string{"--shm-size=a128m", "img", "cmd"}); err == nil || err.Error() != expectedErr {
416
+		t.Fatalf("Expected an error with message '%v', got %v", expectedErr, err)
416 417
 	}
417 418
 	// shm-size ok
418 419
 	_, hostconfig, _, err = parseRun([]string{"--shm-size=128m", "img", "cmd"})
... ...
@@ -41,7 +41,7 @@ type buildOptions struct {
41 41
 	ulimits        *opts.UlimitOpt
42 42
 	memory         string
43 43
 	memorySwap     string
44
-	shmSize        string
44
+	shmSize        opts.MemBytes
45 45
 	cpuShares      int64
46 46
 	cpuPeriod      int64
47 47
 	cpuQuota       int64
... ...
@@ -89,7 +89,7 @@ func NewBuildCommand(dockerCli *command.DockerCli) *cobra.Command {
89 89
 	flags.StringVarP(&options.dockerfileName, "file", "f", "", "Name of the Dockerfile (Default is 'PATH/Dockerfile')")
90 90
 	flags.StringVarP(&options.memory, "memory", "m", "", "Memory limit")
91 91
 	flags.StringVar(&options.memorySwap, "memory-swap", "", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap")
92
-	flags.StringVar(&options.shmSize, "shm-size", "", "Size of /dev/shm, default value is 64MB")
92
+	flags.Var(&options.shmSize, "shm-size", "Size of /dev/shm")
93 93
 	flags.Int64VarP(&options.cpuShares, "cpu-shares", "c", 0, "CPU shares (relative weight)")
94 94
 	flags.Int64Var(&options.cpuPeriod, "cpu-period", 0, "Limit the CPU CFS (Completely Fair Scheduler) period")
95 95
 	flags.Int64Var(&options.cpuQuota, "cpu-quota", 0, "Limit the CPU CFS (Completely Fair Scheduler) quota")
... ...
@@ -271,14 +271,6 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error {
271 271
 		}
272 272
 	}
273 273
 
274
-	var shmSize int64
275
-	if options.shmSize != "" {
276
-		shmSize, err = units.RAMInBytes(options.shmSize)
277
-		if err != nil {
278
-			return err
279
-		}
280
-	}
281
-
282 274
 	authConfigs, _ := dockerCli.GetAllCredentials()
283 275
 	buildOptions := types.ImageBuildOptions{
284 276
 		Memory:         memory,
... ...
@@ -297,7 +289,7 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error {
297 297
 		CPUPeriod:      options.cpuPeriod,
298 298
 		CgroupParent:   options.cgroupParent,
299 299
 		Dockerfile:     relDockerfile,
300
-		ShmSize:        shmSize,
300
+		ShmSize:        options.shmSize.Value(),
301 301
 		Ulimits:        options.ulimits.GetList(),
302 302
 		BuildArgs:      runconfigopts.ConvertKVStringsToMapWithNil(options.buildArgs.GetAll()),
303 303
 		AuthConfigs:    authConfigs,
... ...
@@ -49,7 +49,7 @@ Options:
49 49
   -q, --quiet                   Suppress the build output and print image ID on success
50 50
       --rm                      Remove intermediate containers after a successful build (default true)
51 51
       --security-opt value      Security Options (default [])
52
-      --shm-size string         Size of /dev/shm, default value is 64MB.
52
+      --shm-size bytes          Size of /dev/shm
53 53
                                 The format is `<number><unit>`. `number` must be greater than `0`.
54 54
                                 Unit is optional and can be `b` (bytes), `k` (kilobytes), `m` (megabytes),
55 55
                                 or `g` (gigabytes). If you omit the unit, the system uses bytes.
... ...
@@ -106,7 +106,7 @@ Options:
106 106
       --rm                          Automatically remove the container when it exits
107 107
       --runtime string              Runtime to use for this container
108 108
       --security-opt value          Security Options (default [])
109
-      --shm-size string             Size of /dev/shm, default value is 64MB.
109
+      --shm-size bytes              Size of /dev/shm
110 110
                                     The format is `<number><unit>`. `number` must be greater than `0`.
111 111
                                     Unit is optional and can be `b` (bytes), `k` (kilobytes), `m` (megabytes),
112 112
                                     or `g` (gigabytes). If you omit the unit, the system uses bytes.
... ...
@@ -116,7 +116,7 @@ Options:
116 116
       --rm                          Automatically remove the container when it exits
117 117
       --runtime string              Runtime to use for this container
118 118
       --security-opt value          Security Options (default [])
119
-      --shm-size string             Size of /dev/shm, default value is 64MB.
119
+      --shm-size bytes              Size of /dev/shm
120 120
                                     The format is `<number><unit>`. `number` must be greater than `0`.
121 121
                                     Unit is optional and can be `b` (bytes), `k` (kilobytes), `m` (megabytes),
122 122
                                     or `g` (gigabytes). If you omit the unit, the system uses bytes.
... ...
@@ -39,7 +39,7 @@ Options:
39 39
       --hostname string                  Container hostname
40 40
   -l, --label list                       Service labels (default [])
41 41
       --limit-cpu decimal                Limit CPUs (default 0.000)
42
-      --limit-memory bytes               Limit Memory (default 0 B)
42
+      --limit-memory bytes               Limit Memory
43 43
       --log-driver string                Logging driver for service
44 44
       --log-opt list                     Logging driver options (default [])
45 45
       --mode string                      Service mode (replicated or global) (default "replicated")
... ...
@@ -50,7 +50,7 @@ Options:
50 50
   -p, --publish port                     Publish a port as a node port
51 51
       --replicas uint                    Number of tasks
52 52
       --reserve-cpu decimal              Reserve CPUs (default 0.000)
53
-      --reserve-memory bytes             Reserve Memory (default 0 B)
53
+      --reserve-memory bytes             Reserve Memory
54 54
       --restart-condition string         Restart when condition is met (none, on-failure, or any)
55 55
       --restart-delay duration           Delay between restart attempts (ns|us|ms|s|m|h)
56 56
       --restart-max-attempts uint        Maximum number of restarts before giving up
... ...
@@ -50,7 +50,7 @@ Options:
50 50
       --label-add list                   Add or update a service label (default [])
51 51
       --label-rm list                    Remove a label by its key (default [])
52 52
       --limit-cpu decimal                Limit CPUs (default 0.000)
53
-      --limit-memory bytes               Limit Memory (default 0 B)
53
+      --limit-memory bytes               Limit Memory
54 54
       --log-driver string                Logging driver for service
55 55
       --log-opt list                     Logging driver options (default [])
56 56
       --mount-add mount                  Add or update a mount on a service
... ...
@@ -60,7 +60,7 @@ Options:
60 60
       --publish-rm port                  Remove a published port by its target port
61 61
       --replicas uint                    Number of tasks
62 62
       --reserve-cpu decimal              Reserve CPUs (default 0.000)
63
-      --reserve-memory bytes             Reserve Memory (default 0 B)
63
+      --reserve-memory bytes             Reserve Memory
64 64
       --restart-condition string         Restart when condition is met (none, on-failure, or any)
65 65
       --restart-delay duration           Delay between restart attempts (ns|us|ms|s|m|h)
66 66
       --restart-max-attempts uint        Maximum number of restarts before giving up
... ...
@@ -427,7 +427,7 @@ func (s *DockerDaemonSuite) TestDaemonEvents(c *check.C) {
427 427
 	out, err = s.d.Cmd("events", "--since=0", "--until", daemonUnixTime(c))
428 428
 	c.Assert(err, checker.IsNil)
429 429
 
430
-	c.Assert(out, checker.Contains, fmt.Sprintf("daemon reload %s (cluster-advertise=, cluster-store=, cluster-store-opts={}, debug=true, default-runtime=runc, default-shm-size=67108864, insecure-registries=[], labels=[\"bar=foo\"], live-restore=false, max-concurrent-downloads=1, max-concurrent-uploads=5, name=%s, runtimes=runc:{docker-runc []}, shutdown-timeout=10)", daemonID, daemonName))
430
+	c.Assert(out, checker.Contains, fmt.Sprintf("daemon reload %s (cluster-advertise=, cluster-store=, cluster-store-opts={}, debug=true, default-runtime=runc, default-shm-size=67108864, insecure-registries=[], labels=[\"bar=foo\"], live-restore=false, max-concurrent-downloads=1, max-concurrent-uploads=5, name=%s, registry-mirrors=[], runtimes=runc:{docker-runc []}, shutdown-timeout=10)", daemonID, daemonName))
431 431
 }
432 432
 
433 433
 func (s *DockerDaemonSuite) TestDaemonEventsWithFilters(c *check.C) {
... ...
@@ -409,7 +409,13 @@ type MemBytes int64
409 409
 
410 410
 // String returns the string format of the human readable memory bytes
411 411
 func (m *MemBytes) String() string {
412
-	return units.BytesSize(float64(m.Value()))
412
+	// NOTE: In spf13/pflag/flag.go, "0" is considered as "zero value" while "0 B" is not.
413
+	// We return "0" in case value is 0 here so that the default value is hidden.
414
+	// (Sometimes "default 0 B" is actually misleading)
415
+	if m.Value() != 0 {
416
+		return units.BytesSize(float64(m.Value()))
417
+	}
418
+	return "0"
413 419
 }
414 420
 
415 421
 // Set sets the value of the MemBytes by passing a string