From e43a97cd386bd5dab092b19ebbd037a8202d353d Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 6 Mar 2017 16:01:04 -0500 Subject: [PATCH] Use opts.MemBytes for flags. Signed-off-by: Daniel Nephin --- command/container/opts.go | 79 +++++++--------------------------- command/container/opts_test.go | 33 +++++++------- command/container/update.go | 62 ++++++-------------------- command/image/build.go | 34 +++------------ 4 files changed, 49 insertions(+), 159 deletions(-) diff --git a/command/container/opts.go b/command/container/opts.go index 0413ae5f71..72d416379d 100644 --- a/command/container/opts.go +++ b/command/container/opts.go @@ -18,7 +18,6 @@ import ( "github.com/docker/docker/pkg/signal" runconfigopts "github.com/docker/docker/runconfig/opts" "github.com/docker/go-connections/nat" - units "github.com/docker/go-units" "github.com/spf13/pflag" ) @@ -72,10 +71,10 @@ type containerOptions struct { containerIDFile string entrypoint string hostname string - memoryString string - memoryReservation string - memorySwap string - kernelMemory string + memory opts.MemBytes + memoryReservation opts.MemBytes + memorySwap opts.MemSwapBytes + kernelMemory opts.MemBytes user string workingDir string cpuCount int64 @@ -89,7 +88,7 @@ type containerOptions struct { cpusetCpus string cpusetMems string blkioWeight uint16 - ioMaxBandwidth string + ioMaxBandwidth opts.MemBytes ioMaxIOps uint64 swappiness int64 netMode string @@ -254,12 +253,12 @@ func addFlags(flags *pflag.FlagSet) *containerOptions { flags.Var(&copts.deviceReadIOps, "device-read-iops", "Limit read rate (IO per second) from a device") flags.Var(&copts.deviceWriteBps, "device-write-bps", "Limit write rate (bytes per second) to a device") flags.Var(&copts.deviceWriteIOps, "device-write-iops", "Limit write rate (IO per second) to a device") - flags.StringVar(&copts.ioMaxBandwidth, "io-maxbandwidth", "", "Maximum IO bandwidth limit for the system drive (Windows only)") + flags.Var(&copts.ioMaxBandwidth, "io-maxbandwidth", "Maximum IO bandwidth limit for the system drive (Windows only)") flags.Uint64Var(&copts.ioMaxIOps, "io-maxiops", 0, "Maximum IOps limit for the system drive (Windows only)") - flags.StringVar(&copts.kernelMemory, "kernel-memory", "", "Kernel memory limit") - flags.StringVarP(&copts.memoryString, "memory", "m", "", "Memory limit") - flags.StringVar(&copts.memoryReservation, "memory-reservation", "", "Memory soft limit") - flags.StringVar(&copts.memorySwap, "memory-swap", "", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap") + flags.Var(&copts.kernelMemory, "kernel-memory", "Kernel memory limit") + flags.VarP(&copts.memory, "memory", "m", "Memory limit") + flags.Var(&copts.memoryReservation, "memory-reservation", "Memory soft limit") + flags.Var(&copts.memorySwap, "memory-swap", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap") flags.Int64Var(&copts.swappiness, "memory-swappiness", -1, "Tune container memory swappiness (0 to 100)") flags.BoolVar(&copts.oomKillDisable, "oom-kill-disable", false, "Disable OOM Killer") 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 var err error - var memory int64 - if copts.memoryString != "" { - memory, err = units.RAMInBytes(copts.memoryString) - if err != nil { - return nil, nil, nil, err - } - } - - var memoryReservation int64 - if copts.memoryReservation != "" { - memoryReservation, err = units.RAMInBytes(copts.memoryReservation) - if err != nil { - return nil, nil, nil, err - } - } - - var memorySwap int64 - if copts.memorySwap != "" { - if copts.memorySwap == "-1" { - memorySwap = -1 - } else { - memorySwap, err = units.RAMInBytes(copts.memorySwap) - if err != nil { - return nil, nil, nil, err - } - } - } - - var kernelMemory int64 - if copts.kernelMemory != "" { - kernelMemory, err = units.RAMInBytes(copts.kernelMemory) - if err != nil { - return nil, nil, nil, err - } - } - swappiness := copts.swappiness if swappiness != -1 && (swappiness < 0 || swappiness > 100) { return nil, nil, nil, fmt.Errorf("invalid value: %d. Valid memory swappiness range is 0-100", swappiness) } - // TODO FIXME units.RAMInBytes should have a uint64 version - var maxIOBandwidth int64 - if copts.ioMaxBandwidth != "" { - maxIOBandwidth, err = units.RAMInBytes(copts.ioMaxBandwidth) - if err != nil { - return nil, nil, nil, err - } - if maxIOBandwidth < 0 { - return nil, nil, nil, fmt.Errorf("invalid value: %s. Maximum IO Bandwidth must be positive", copts.ioMaxBandwidth) - } - } - var binds []string volumes := copts.volumes.GetMap() // add any bind targets to the list of container volumes @@ -530,11 +481,11 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*container.Config, *c resources := container.Resources{ CgroupParent: copts.cgroupParent, - Memory: memory, - MemoryReservation: memoryReservation, - MemorySwap: memorySwap, + Memory: copts.memory.Value(), + MemoryReservation: copts.memoryReservation.Value(), + MemorySwap: copts.memorySwap.Value(), MemorySwappiness: &copts.swappiness, - KernelMemory: kernelMemory, + KernelMemory: copts.kernelMemory.Value(), OomKillDisable: &copts.oomKillDisable, NanoCPUs: copts.cpus.Value(), CPUCount: copts.cpuCount, @@ -554,7 +505,7 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*container.Config, *c BlkioDeviceReadIOps: copts.deviceReadIOps.GetList(), BlkioDeviceWriteIOps: copts.deviceWriteIOps.GetList(), IOMaximumIOps: copts.ioMaxIOps, - IOMaximumBandwidth: uint64(maxIOBandwidth), + IOMaximumBandwidth: uint64(copts.ioMaxBandwidth), Ulimits: copts.ulimits.GetList(), DeviceCgroupRules: copts.deviceCgroupRules.GetAll(), Devices: deviceMappings, diff --git a/command/container/opts_test.go b/command/container/opts_test.go index 6ba83c29d1..1448dae8db 100644 --- a/command/container/opts_test.go +++ b/command/container/opts_test.go @@ -13,6 +13,7 @@ import ( "github.com/docker/docker/api/types/container" networktypes "github.com/docker/docker/api/types/network" + "github.com/docker/docker/pkg/testutil/assert" "github.com/docker/docker/runconfig" "github.com/docker/go-connections/nat" "github.com/spf13/pflag" @@ -235,28 +236,24 @@ func TestParseWithMacAddress(t *testing.T) { func TestParseWithMemory(t *testing.T) { invalidMemory := "--memory=invalid" - validMemory := "--memory=1G" - if _, _, _, err := parseRun([]string{invalidMemory, "img", "cmd"}); err != nil && err.Error() != "invalid size: 'invalid'" { - t.Fatalf("Expected an error with '%v' Memory, got '%v'", invalidMemory, err) - } - if _, hostconfig := mustParse(t, validMemory); hostconfig.Memory != 1073741824 { - t.Fatalf("Expected the config to have '1G' as Memory, got '%v'", hostconfig.Memory) - } + _, _, _, err := parseRun([]string{invalidMemory, "img", "cmd"}) + assert.Error(t, err, invalidMemory) + + _, hostconfig := mustParse(t, "--memory=1G") + assert.Equal(t, hostconfig.Memory, int64(1073741824)) } func TestParseWithMemorySwap(t *testing.T) { invalidMemory := "--memory-swap=invalid" - validMemory := "--memory-swap=1G" - anotherValidMemory := "--memory-swap=-1" - if _, _, _, err := parseRun([]string{invalidMemory, "img", "cmd"}); err == nil || err.Error() != "invalid size: 'invalid'" { - t.Fatalf("Expected an error with '%v' MemorySwap, got '%v'", invalidMemory, err) - } - if _, hostconfig := mustParse(t, validMemory); hostconfig.MemorySwap != 1073741824 { - t.Fatalf("Expected the config to have '1073741824' as MemorySwap, got '%v'", hostconfig.MemorySwap) - } - if _, hostconfig := mustParse(t, anotherValidMemory); hostconfig.MemorySwap != -1 { - t.Fatalf("Expected the config to have '-1' as MemorySwap, got '%v'", hostconfig.MemorySwap) - } + + _, _, _, err := parseRun([]string{invalidMemory, "img", "cmd"}) + assert.Error(t, err, invalidMemory) + + _, hostconfig := mustParse(t, "--memory-swap=1G") + assert.Equal(t, hostconfig.MemorySwap, int64(1073741824)) + + _, hostconfig = mustParse(t, "--memory-swap=-1") + assert.Equal(t, hostconfig.MemorySwap, int64(-1)) } func TestParseHostname(t *testing.T) { diff --git a/command/container/update.go b/command/container/update.go index 4a1220a262..b2a44975b3 100644 --- a/command/container/update.go +++ b/command/container/update.go @@ -8,8 +8,8 @@ import ( containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/cli" "github.com/docker/docker/cli/command" + "github.com/docker/docker/opts" runconfigopts "github.com/docker/docker/runconfig/opts" - "github.com/docker/go-units" "github.com/spf13/cobra" "golang.org/x/net/context" ) @@ -23,10 +23,10 @@ type updateOptions struct { cpusetCpus string cpusetMems string cpuShares int64 - memoryString string - memoryReservation string - memorySwap string - kernelMemory string + memory opts.MemBytes + memoryReservation opts.MemBytes + memorySwap opts.MemSwapBytes + kernelMemory opts.MemBytes restartPolicy string nFlag int @@ -60,10 +60,10 @@ func NewUpdateCommand(dockerCli *command.DockerCli) *cobra.Command { flags.StringVar(&opts.cpusetCpus, "cpuset-cpus", "", "CPUs in which to allow execution (0-3, 0,1)") flags.StringVar(&opts.cpusetMems, "cpuset-mems", "", "MEMs in which to allow execution (0-3, 0,1)") flags.Int64VarP(&opts.cpuShares, "cpu-shares", "c", 0, "CPU shares (relative weight)") - flags.StringVarP(&opts.memoryString, "memory", "m", "", "Memory limit") - flags.StringVar(&opts.memoryReservation, "memory-reservation", "", "Memory soft limit") - flags.StringVar(&opts.memorySwap, "memory-swap", "", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap") - flags.StringVar(&opts.kernelMemory, "kernel-memory", "", "Kernel memory limit") + flags.VarP(&opts.memory, "memory", "m", "Memory limit") + flags.Var(&opts.memoryReservation, "memory-reservation", "Memory soft limit") + flags.Var(&opts.memorySwap, "memory-swap", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap") + flags.Var(&opts.kernelMemory, "kernel-memory", "Kernel memory limit") flags.StringVar(&opts.restartPolicy, "restart", "", "Restart policy to apply when a container exits") return cmd @@ -76,42 +76,6 @@ func runUpdate(dockerCli *command.DockerCli, opts *updateOptions) error { return errors.New("You must provide one or more flags when using this command.") } - var memory int64 - if opts.memoryString != "" { - memory, err = units.RAMInBytes(opts.memoryString) - if err != nil { - return err - } - } - - var memoryReservation int64 - if opts.memoryReservation != "" { - memoryReservation, err = units.RAMInBytes(opts.memoryReservation) - if err != nil { - return err - } - } - - var memorySwap int64 - if opts.memorySwap != "" { - if opts.memorySwap == "-1" { - memorySwap = -1 - } else { - memorySwap, err = units.RAMInBytes(opts.memorySwap) - if err != nil { - return err - } - } - } - - var kernelMemory int64 - if opts.kernelMemory != "" { - kernelMemory, err = units.RAMInBytes(opts.kernelMemory) - if err != nil { - return err - } - } - var restartPolicy containertypes.RestartPolicy if opts.restartPolicy != "" { restartPolicy, err = runconfigopts.ParseRestartPolicy(opts.restartPolicy) @@ -125,10 +89,10 @@ func runUpdate(dockerCli *command.DockerCli, opts *updateOptions) error { CpusetCpus: opts.cpusetCpus, CpusetMems: opts.cpusetMems, CPUShares: opts.cpuShares, - Memory: memory, - MemoryReservation: memoryReservation, - MemorySwap: memorySwap, - KernelMemory: kernelMemory, + Memory: opts.memory.Value(), + MemoryReservation: opts.memoryReservation.Value(), + MemorySwap: opts.memorySwap.Value(), + KernelMemory: opts.kernelMemory.Value(), CPUPeriod: opts.cpuPeriod, CPUQuota: opts.cpuQuota, CPURealtimePeriod: opts.cpuRealtimePeriod, diff --git a/command/image/build.go b/command/image/build.go index 9fde671418..5f7d5d07a8 100644 --- a/command/image/build.go +++ b/command/image/build.go @@ -40,8 +40,8 @@ type buildOptions struct { buildArgs opts.ListOpts extraHosts opts.ListOpts ulimits *opts.UlimitOpt - memory string - memorySwap string + memory opts.MemBytes + memorySwap opts.MemSwapBytes shmSize opts.MemBytes cpuShares int64 cpuPeriod int64 @@ -89,8 +89,8 @@ func NewBuildCommand(dockerCli *command.DockerCli) *cobra.Command { flags.Var(&options.buildArgs, "build-arg", "Set build-time variables") flags.Var(options.ulimits, "ulimit", "Ulimit options") flags.StringVarP(&options.dockerfileName, "file", "f", "", "Name of the Dockerfile (Default is 'PATH/Dockerfile')") - flags.StringVarP(&options.memory, "memory", "m", "", "Memory limit") - flags.StringVar(&options.memorySwap, "memory-swap", "", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap") + flags.VarP(&options.memory, "memory", "m", "Memory limit") + flags.Var(&options.memorySwap, "memory-swap", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap") flags.Var(&options.shmSize, "shm-size", "Size of /dev/shm") flags.Int64VarP(&options.cpuShares, "cpu-shares", "c", 0, "CPU shares (relative weight)") 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 { var body io.Reader = progress.NewProgressReader(buildCtx, progressOutput, 0, "", "Sending build context to Docker daemon") - var memory int64 - if options.memory != "" { - parsedMemory, err := units.RAMInBytes(options.memory) - if err != nil { - return err - } - memory = parsedMemory - } - - var memorySwap int64 - if options.memorySwap != "" { - if options.memorySwap == "-1" { - memorySwap = -1 - } else { - parsedMemorySwap, err := units.RAMInBytes(options.memorySwap) - if err != nil { - return err - } - memorySwap = parsedMemorySwap - } - } - authConfigs, _ := dockerCli.GetAllCredentials() buildOptions := types.ImageBuildOptions{ - Memory: memory, - MemorySwap: memorySwap, + Memory: options.memory.Value(), + MemorySwap: options.memorySwap.Value(), Tags: options.tags.GetAll(), SuppressOutput: options.quiet, NoCache: options.noCache,