Browse code

Use opts.MemBytes for flags.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

Daniel Nephin authored on 2017/03/07 06:01:04
Showing 5 changed files
... ...
@@ -18,7 +18,6 @@ import (
18 18
 	"github.com/docker/docker/pkg/signal"
19 19
 	runconfigopts "github.com/docker/docker/runconfig/opts"
20 20
 	"github.com/docker/go-connections/nat"
21
-	units "github.com/docker/go-units"
22 21
 	"github.com/spf13/pflag"
23 22
 )
24 23
 
... ...
@@ -72,10 +71,10 @@ type containerOptions struct {
72 72
 	containerIDFile    string
73 73
 	entrypoint         string
74 74
 	hostname           string
75
-	memoryString       string
76
-	memoryReservation  string
77
-	memorySwap         string
78
-	kernelMemory       string
75
+	memory             opts.MemBytes
76
+	memoryReservation  opts.MemBytes
77
+	memorySwap         opts.MemSwapBytes
78
+	kernelMemory       opts.MemBytes
79 79
 	user               string
80 80
 	workingDir         string
81 81
 	cpuCount           int64
... ...
@@ -89,7 +88,7 @@ type containerOptions struct {
89 89
 	cpusetCpus         string
90 90
 	cpusetMems         string
91 91
 	blkioWeight        uint16
92
-	ioMaxBandwidth     string
92
+	ioMaxBandwidth     opts.MemBytes
93 93
 	ioMaxIOps          uint64
94 94
 	swappiness         int64
95 95
 	netMode            string
... ...
@@ -254,12 +253,12 @@ func addFlags(flags *pflag.FlagSet) *containerOptions {
254 254
 	flags.Var(&copts.deviceReadIOps, "device-read-iops", "Limit read rate (IO per second) from a device")
255 255
 	flags.Var(&copts.deviceWriteBps, "device-write-bps", "Limit write rate (bytes per second) to a device")
256 256
 	flags.Var(&copts.deviceWriteIOps, "device-write-iops", "Limit write rate (IO per second) to a device")
257
-	flags.StringVar(&copts.ioMaxBandwidth, "io-maxbandwidth", "", "Maximum IO bandwidth limit for the system drive (Windows only)")
257
+	flags.Var(&copts.ioMaxBandwidth, "io-maxbandwidth", "Maximum IO bandwidth limit for the system drive (Windows only)")
258 258
 	flags.Uint64Var(&copts.ioMaxIOps, "io-maxiops", 0, "Maximum IOps limit for the system drive (Windows only)")
259
-	flags.StringVar(&copts.kernelMemory, "kernel-memory", "", "Kernel memory limit")
260
-	flags.StringVarP(&copts.memoryString, "memory", "m", "", "Memory limit")
261
-	flags.StringVar(&copts.memoryReservation, "memory-reservation", "", "Memory soft limit")
262
-	flags.StringVar(&copts.memorySwap, "memory-swap", "", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap")
259
+	flags.Var(&copts.kernelMemory, "kernel-memory", "Kernel memory limit")
260
+	flags.VarP(&copts.memory, "memory", "m", "Memory limit")
261
+	flags.Var(&copts.memoryReservation, "memory-reservation", "Memory soft limit")
262
+	flags.Var(&copts.memorySwap, "memory-swap", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap")
263 263
 	flags.Int64Var(&copts.swappiness, "memory-swappiness", -1, "Tune container memory swappiness (0 to 100)")
264 264
 	flags.BoolVar(&copts.oomKillDisable, "oom-kill-disable", false, "Disable OOM Killer")
265 265
 	flags.IntVar(&copts.oomScoreAdj, "oom-score-adj", 0, "Tune host's OOM preferences (-1000 to 1000)")
... ...
@@ -308,59 +307,11 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*container.Config, *c
308 308
 
309 309
 	var err error
310 310
 
311
-	var memory int64
312
-	if copts.memoryString != "" {
313
-		memory, err = units.RAMInBytes(copts.memoryString)
314
-		if err != nil {
315
-			return nil, nil, nil, err
316
-		}
317
-	}
318
-
319
-	var memoryReservation int64
320
-	if copts.memoryReservation != "" {
321
-		memoryReservation, err = units.RAMInBytes(copts.memoryReservation)
322
-		if err != nil {
323
-			return nil, nil, nil, err
324
-		}
325
-	}
326
-
327
-	var memorySwap int64
328
-	if copts.memorySwap != "" {
329
-		if copts.memorySwap == "-1" {
330
-			memorySwap = -1
331
-		} else {
332
-			memorySwap, err = units.RAMInBytes(copts.memorySwap)
333
-			if err != nil {
334
-				return nil, nil, nil, err
335
-			}
336
-		}
337
-	}
338
-
339
-	var kernelMemory int64
340
-	if copts.kernelMemory != "" {
341
-		kernelMemory, err = units.RAMInBytes(copts.kernelMemory)
342
-		if err != nil {
343
-			return nil, nil, nil, err
344
-		}
345
-	}
346
-
347 311
 	swappiness := copts.swappiness
348 312
 	if swappiness != -1 && (swappiness < 0 || swappiness > 100) {
349 313
 		return nil, nil, nil, fmt.Errorf("invalid value: %d. Valid memory swappiness range is 0-100", swappiness)
350 314
 	}
351 315
 
352
-	// TODO FIXME units.RAMInBytes should have a uint64 version
353
-	var maxIOBandwidth int64
354
-	if copts.ioMaxBandwidth != "" {
355
-		maxIOBandwidth, err = units.RAMInBytes(copts.ioMaxBandwidth)
356
-		if err != nil {
357
-			return nil, nil, nil, err
358
-		}
359
-		if maxIOBandwidth < 0 {
360
-			return nil, nil, nil, fmt.Errorf("invalid value: %s. Maximum IO Bandwidth must be positive", copts.ioMaxBandwidth)
361
-		}
362
-	}
363
-
364 316
 	var binds []string
365 317
 	volumes := copts.volumes.GetMap()
366 318
 	// add any bind targets to the list of container volumes
... ...
@@ -530,11 +481,11 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*container.Config, *c
530 530
 
531 531
 	resources := container.Resources{
532 532
 		CgroupParent:         copts.cgroupParent,
533
-		Memory:               memory,
534
-		MemoryReservation:    memoryReservation,
535
-		MemorySwap:           memorySwap,
533
+		Memory:               copts.memory.Value(),
534
+		MemoryReservation:    copts.memoryReservation.Value(),
535
+		MemorySwap:           copts.memorySwap.Value(),
536 536
 		MemorySwappiness:     &copts.swappiness,
537
-		KernelMemory:         kernelMemory,
537
+		KernelMemory:         copts.kernelMemory.Value(),
538 538
 		OomKillDisable:       &copts.oomKillDisable,
539 539
 		NanoCPUs:             copts.cpus.Value(),
540 540
 		CPUCount:             copts.cpuCount,
... ...
@@ -554,7 +505,7 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*container.Config, *c
554 554
 		BlkioDeviceReadIOps:  copts.deviceReadIOps.GetList(),
555 555
 		BlkioDeviceWriteIOps: copts.deviceWriteIOps.GetList(),
556 556
 		IOMaximumIOps:        copts.ioMaxIOps,
557
-		IOMaximumBandwidth:   uint64(maxIOBandwidth),
557
+		IOMaximumBandwidth:   uint64(copts.ioMaxBandwidth),
558 558
 		Ulimits:              copts.ulimits.GetList(),
559 559
 		DeviceCgroupRules:    copts.deviceCgroupRules.GetAll(),
560 560
 		Devices:              deviceMappings,
... ...
@@ -13,6 +13,7 @@ import (
13 13
 
14 14
 	"github.com/docker/docker/api/types/container"
15 15
 	networktypes "github.com/docker/docker/api/types/network"
16
+	"github.com/docker/docker/pkg/testutil/assert"
16 17
 	"github.com/docker/docker/runconfig"
17 18
 	"github.com/docker/go-connections/nat"
18 19
 	"github.com/spf13/pflag"
... ...
@@ -235,28 +236,24 @@ func TestParseWithMacAddress(t *testing.T) {
235 235
 
236 236
 func TestParseWithMemory(t *testing.T) {
237 237
 	invalidMemory := "--memory=invalid"
238
-	validMemory := "--memory=1G"
239
-	if _, _, _, err := parseRun([]string{invalidMemory, "img", "cmd"}); err != nil && err.Error() != "invalid size: 'invalid'" {
240
-		t.Fatalf("Expected an error with '%v' Memory, got '%v'", invalidMemory, err)
241
-	}
242
-	if _, hostconfig := mustParse(t, validMemory); hostconfig.Memory != 1073741824 {
243
-		t.Fatalf("Expected the config to have '1G' as Memory, got '%v'", hostconfig.Memory)
244
-	}
238
+	_, _, _, err := parseRun([]string{invalidMemory, "img", "cmd"})
239
+	assert.Error(t, err, invalidMemory)
240
+
241
+	_, hostconfig := mustParse(t, "--memory=1G")
242
+	assert.Equal(t, hostconfig.Memory, int64(1073741824))
245 243
 }
246 244
 
247 245
 func TestParseWithMemorySwap(t *testing.T) {
248 246
 	invalidMemory := "--memory-swap=invalid"
249
-	validMemory := "--memory-swap=1G"
250
-	anotherValidMemory := "--memory-swap=-1"
251
-	if _, _, _, err := parseRun([]string{invalidMemory, "img", "cmd"}); err == nil || err.Error() != "invalid size: 'invalid'" {
252
-		t.Fatalf("Expected an error with '%v' MemorySwap, got '%v'", invalidMemory, err)
253
-	}
254
-	if _, hostconfig := mustParse(t, validMemory); hostconfig.MemorySwap != 1073741824 {
255
-		t.Fatalf("Expected the config to have '1073741824' as MemorySwap, got '%v'", hostconfig.MemorySwap)
256
-	}
257
-	if _, hostconfig := mustParse(t, anotherValidMemory); hostconfig.MemorySwap != -1 {
258
-		t.Fatalf("Expected the config to have '-1' as MemorySwap, got '%v'", hostconfig.MemorySwap)
259
-	}
247
+
248
+	_, _, _, err := parseRun([]string{invalidMemory, "img", "cmd"})
249
+	assert.Error(t, err, invalidMemory)
250
+
251
+	_, hostconfig := mustParse(t, "--memory-swap=1G")
252
+	assert.Equal(t, hostconfig.MemorySwap, int64(1073741824))
253
+
254
+	_, hostconfig = mustParse(t, "--memory-swap=-1")
255
+	assert.Equal(t, hostconfig.MemorySwap, int64(-1))
260 256
 }
261 257
 
262 258
 func TestParseHostname(t *testing.T) {
... ...
@@ -8,8 +8,8 @@ import (
8 8
 	containertypes "github.com/docker/docker/api/types/container"
9 9
 	"github.com/docker/docker/cli"
10 10
 	"github.com/docker/docker/cli/command"
11
+	"github.com/docker/docker/opts"
11 12
 	runconfigopts "github.com/docker/docker/runconfig/opts"
12
-	"github.com/docker/go-units"
13 13
 	"github.com/spf13/cobra"
14 14
 	"golang.org/x/net/context"
15 15
 )
... ...
@@ -23,10 +23,10 @@ type updateOptions struct {
23 23
 	cpusetCpus         string
24 24
 	cpusetMems         string
25 25
 	cpuShares          int64
26
-	memoryString       string
27
-	memoryReservation  string
28
-	memorySwap         string
29
-	kernelMemory       string
26
+	memory             opts.MemBytes
27
+	memoryReservation  opts.MemBytes
28
+	memorySwap         opts.MemSwapBytes
29
+	kernelMemory       opts.MemBytes
30 30
 	restartPolicy      string
31 31
 
32 32
 	nFlag int
... ...
@@ -60,10 +60,10 @@ func NewUpdateCommand(dockerCli *command.DockerCli) *cobra.Command {
60 60
 	flags.StringVar(&opts.cpusetCpus, "cpuset-cpus", "", "CPUs in which to allow execution (0-3, 0,1)")
61 61
 	flags.StringVar(&opts.cpusetMems, "cpuset-mems", "", "MEMs in which to allow execution (0-3, 0,1)")
62 62
 	flags.Int64VarP(&opts.cpuShares, "cpu-shares", "c", 0, "CPU shares (relative weight)")
63
-	flags.StringVarP(&opts.memoryString, "memory", "m", "", "Memory limit")
64
-	flags.StringVar(&opts.memoryReservation, "memory-reservation", "", "Memory soft limit")
65
-	flags.StringVar(&opts.memorySwap, "memory-swap", "", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap")
66
-	flags.StringVar(&opts.kernelMemory, "kernel-memory", "", "Kernel memory limit")
63
+	flags.VarP(&opts.memory, "memory", "m", "Memory limit")
64
+	flags.Var(&opts.memoryReservation, "memory-reservation", "Memory soft limit")
65
+	flags.Var(&opts.memorySwap, "memory-swap", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap")
66
+	flags.Var(&opts.kernelMemory, "kernel-memory", "Kernel memory limit")
67 67
 	flags.StringVar(&opts.restartPolicy, "restart", "", "Restart policy to apply when a container exits")
68 68
 
69 69
 	return cmd
... ...
@@ -76,42 +76,6 @@ func runUpdate(dockerCli *command.DockerCli, opts *updateOptions) error {
76 76
 		return errors.New("You must provide one or more flags when using this command.")
77 77
 	}
78 78
 
79
-	var memory int64
80
-	if opts.memoryString != "" {
81
-		memory, err = units.RAMInBytes(opts.memoryString)
82
-		if err != nil {
83
-			return err
84
-		}
85
-	}
86
-
87
-	var memoryReservation int64
88
-	if opts.memoryReservation != "" {
89
-		memoryReservation, err = units.RAMInBytes(opts.memoryReservation)
90
-		if err != nil {
91
-			return err
92
-		}
93
-	}
94
-
95
-	var memorySwap int64
96
-	if opts.memorySwap != "" {
97
-		if opts.memorySwap == "-1" {
98
-			memorySwap = -1
99
-		} else {
100
-			memorySwap, err = units.RAMInBytes(opts.memorySwap)
101
-			if err != nil {
102
-				return err
103
-			}
104
-		}
105
-	}
106
-
107
-	var kernelMemory int64
108
-	if opts.kernelMemory != "" {
109
-		kernelMemory, err = units.RAMInBytes(opts.kernelMemory)
110
-		if err != nil {
111
-			return err
112
-		}
113
-	}
114
-
115 79
 	var restartPolicy containertypes.RestartPolicy
116 80
 	if opts.restartPolicy != "" {
117 81
 		restartPolicy, err = runconfigopts.ParseRestartPolicy(opts.restartPolicy)
... ...
@@ -125,10 +89,10 @@ func runUpdate(dockerCli *command.DockerCli, opts *updateOptions) error {
125 125
 		CpusetCpus:         opts.cpusetCpus,
126 126
 		CpusetMems:         opts.cpusetMems,
127 127
 		CPUShares:          opts.cpuShares,
128
-		Memory:             memory,
129
-		MemoryReservation:  memoryReservation,
130
-		MemorySwap:         memorySwap,
131
-		KernelMemory:       kernelMemory,
128
+		Memory:             opts.memory.Value(),
129
+		MemoryReservation:  opts.memoryReservation.Value(),
130
+		MemorySwap:         opts.memorySwap.Value(),
131
+		KernelMemory:       opts.kernelMemory.Value(),
132 132
 		CPUPeriod:          opts.cpuPeriod,
133 133
 		CPUQuota:           opts.cpuQuota,
134 134
 		CPURealtimePeriod:  opts.cpuRealtimePeriod,
... ...
@@ -40,8 +40,8 @@ type buildOptions struct {
40 40
 	buildArgs      opts.ListOpts
41 41
 	extraHosts     opts.ListOpts
42 42
 	ulimits        *opts.UlimitOpt
43
-	memory         string
44
-	memorySwap     string
43
+	memory         opts.MemBytes
44
+	memorySwap     opts.MemSwapBytes
45 45
 	shmSize        opts.MemBytes
46 46
 	cpuShares      int64
47 47
 	cpuPeriod      int64
... ...
@@ -89,8 +89,8 @@ func NewBuildCommand(dockerCli *command.DockerCli) *cobra.Command {
89 89
 	flags.Var(&options.buildArgs, "build-arg", "Set build-time variables")
90 90
 	flags.Var(options.ulimits, "ulimit", "Ulimit options")
91 91
 	flags.StringVarP(&options.dockerfileName, "file", "f", "", "Name of the Dockerfile (Default is 'PATH/Dockerfile')")
92
-	flags.StringVarP(&options.memory, "memory", "m", "", "Memory limit")
93
-	flags.StringVar(&options.memorySwap, "memory-swap", "", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap")
92
+	flags.VarP(&options.memory, "memory", "m", "Memory limit")
93
+	flags.Var(&options.memorySwap, "memory-swap", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap")
94 94
 	flags.Var(&options.shmSize, "shm-size", "Size of /dev/shm")
95 95
 	flags.Int64VarP(&options.cpuShares, "cpu-shares", "c", 0, "CPU shares (relative weight)")
96 96
 	flags.Int64Var(&options.cpuPeriod, "cpu-period", 0, "Limit the CPU CFS (Completely Fair Scheduler) period")
... ...
@@ -254,32 +254,10 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error {
254 254
 
255 255
 	var body io.Reader = progress.NewProgressReader(buildCtx, progressOutput, 0, "", "Sending build context to Docker daemon")
256 256
 
257
-	var memory int64
258
-	if options.memory != "" {
259
-		parsedMemory, err := units.RAMInBytes(options.memory)
260
-		if err != nil {
261
-			return err
262
-		}
263
-		memory = parsedMemory
264
-	}
265
-
266
-	var memorySwap int64
267
-	if options.memorySwap != "" {
268
-		if options.memorySwap == "-1" {
269
-			memorySwap = -1
270
-		} else {
271
-			parsedMemorySwap, err := units.RAMInBytes(options.memorySwap)
272
-			if err != nil {
273
-				return err
274
-			}
275
-			memorySwap = parsedMemorySwap
276
-		}
277
-	}
278
-
279 257
 	authConfigs, _ := dockerCli.GetAllCredentials()
280 258
 	buildOptions := types.ImageBuildOptions{
281
-		Memory:         memory,
282
-		MemorySwap:     memorySwap,
259
+		Memory:         options.memory.Value(),
260
+		MemorySwap:     options.memorySwap.Value(),
283 261
 		Tags:           options.tags.GetAll(),
284 262
 		SuppressOutput: options.quiet,
285 263
 		NoCache:        options.noCache,
... ...
@@ -444,3 +444,39 @@ func (m *MemBytes) UnmarshalJSON(s []byte) error {
444 444
 	*m = MemBytes(val)
445 445
 	return err
446 446
 }
447
+
448
+// MemSwapBytes is a type for human readable memory bytes (like 128M, 2g, etc).
449
+// It differs from MemBytes in that -1 is valid and the default.
450
+type MemSwapBytes int64
451
+
452
+// Set sets the value of the MemSwapBytes by passing a string
453
+func (m *MemSwapBytes) Set(value string) error {
454
+	if value == "-1" {
455
+		*m = MemSwapBytes(-1)
456
+		return nil
457
+	}
458
+	val, err := units.RAMInBytes(value)
459
+	*m = MemSwapBytes(val)
460
+	return err
461
+}
462
+
463
+// Type returns the type
464
+func (m *MemSwapBytes) Type() string {
465
+	return "bytes"
466
+}
467
+
468
+// Value returns the value in int64
469
+func (m *MemSwapBytes) Value() int64 {
470
+	return int64(*m)
471
+}
472
+
473
+func (m *MemSwapBytes) String() string {
474
+	b := MemBytes(*m)
475
+	return b.String()
476
+}
477
+
478
+// UnmarshalJSON is the customized unmarshaler for MemSwapBytes
479
+func (m *MemSwapBytes) UnmarshalJSON(s []byte) error {
480
+	b := MemBytes(*m)
481
+	return b.UnmarshalJSON(s)
482
+}