From 2bfb31ddf4b7f28a67ef94b8b318536c367a663b Mon Sep 17 00:00:00 2001 From: umohnani8 Date: Fri, 12 Jan 2018 10:19:48 -0500 Subject: Implement and test the following flags for podman run and create memory, memory-reservation, memory-swap, memory-swappiness, kernel-memory, cpu-period, cou-quota, cpu-shares, cpus, cpuset-cpus, cpuset-mems, blkio-weight, blkio-weight-device, sysctl, and ulimit Signed-off-by: umohnani8 Closes: #221 Approved by: mheon --- cmd/podman/common.go | 2 +- cmd/podman/create.go | 16 +++++++++++----- cmd/podman/create_cli.go | 43 +++++++++++++++++++++++++++++++++++++------ cmd/podman/create_cli_test.go | 12 ++++++------ cmd/podman/run_test.go | 20 ++++++++++++++++++++ cmd/podman/spec.go | 34 +++++++++++++++++++++++----------- 6 files changed, 98 insertions(+), 29 deletions(-) (limited to 'cmd') diff --git a/cmd/podman/common.go b/cmd/podman/common.go index e0ef43782..8944e49a5 100644 --- a/cmd/podman/common.go +++ b/cmd/podman/common.go @@ -173,7 +173,7 @@ var createFlags = []cli.Flag{ Name: "cpu-shares", Usage: "CPU shares (relative weight)", }, - cli.StringFlag{ + cli.Float64Flag{ Name: "cpus", Usage: "Number of CPUs. The default is 0.000 which means no limit", }, diff --git a/cmd/podman/create.go b/cmd/podman/create.go index ead2f6735..14c520174 100644 --- a/cmd/podman/create.go +++ b/cmd/podman/create.go @@ -47,7 +47,7 @@ type createResourceConfig struct { CPURtPeriod uint64 // cpu-rt-period CPURtRuntime int64 // cpu-rt-runtime CPUShares uint64 // cpu-shares - CPUs string // cpus + CPUs float64 // cpus CPUsetCPUs string CPUsetMems string // cpuset-mems DeviceReadBps []string // device-read-bps @@ -382,9 +382,9 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime, imageName string, command = c.Args()[1:] } - sysctl, err := convertStringSliceToMap(c.StringSlice("sysctl"), "=") + sysctl, err := validateSysctl(c.StringSlice("sysctl")) if err != nil { - return nil, errors.Wrapf(err, "sysctl values must be in the form of KEY=VALUE") + return nil, errors.Wrapf(err, "invalid value for sysctl") } groupAdd, err := stringSlicetoUint32Slice(c.StringSlice("group-add")) @@ -444,6 +444,12 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime, imageName string, if c.Bool("detach") && c.Bool("rm") { return nil, errors.Errorf("--rm and --detach can not be specified together") } + if c.Int64("cpu-period") != 0 && c.Float64("cpus") > 0 { + return nil, errors.Errorf("--cpu-period and --cpus cannot be set together") + } + if c.Int64("cpu-quota") != 0 && c.Float64("cpus") > 0 { + return nil, errors.Errorf("--cpu-quota and --cpus cannot be set together") + } utsMode := container.UTSMode(c.String("uts")) if !utsMode.Valid() { @@ -582,12 +588,12 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime, imageName string, BlkioWeightDevice: c.StringSlice("blkio-weight-device"), CPUShares: c.Uint64("cpu-shares"), CPUPeriod: c.Uint64("cpu-period"), - CPUsetCPUs: c.String("cpu-period"), + CPUsetCPUs: c.String("cpuset-cpus"), CPUsetMems: c.String("cpuset-mems"), CPUQuota: c.Int64("cpu-quota"), CPURtPeriod: c.Uint64("cpu-rt-period"), CPURtRuntime: c.Int64("cpu-rt-runtime"), - CPUs: c.String("cpus"), + CPUs: c.Float64("cpus"), DeviceReadBps: c.StringSlice("device-read-bps"), DeviceReadIOps: c.StringSlice("device-read-iops"), DeviceWriteBps: c.StringSlice("device-write-bps"), diff --git a/cmd/podman/create_cli.go b/cmd/podman/create_cli.go index 0cc265e92..24856feb8 100644 --- a/cmd/podman/create_cli.go +++ b/cmd/podman/create_cli.go @@ -24,14 +24,45 @@ func getAllLabels(labelFile, inputLabels []string) (map[string]string, error) { return labels, nil } -func convertStringSliceToMap(strSlice []string, delimiter string) (map[string]string, error) { +// validateSysctl validates a sysctl and returns it. +func validateSysctl(strSlice []string) (map[string]string, error) { sysctl := make(map[string]string) - for _, inputSysctl := range strSlice { - values := strings.Split(inputSysctl, delimiter) - if len(values) < 2 { - return sysctl, errors.Errorf("%s in an invalid sysctl value", inputSysctl) + validSysctlMap := map[string]bool{ + "kernel.msgmax": true, + "kernel.msgmnb": true, + "kernel.msgmni": true, + "kernel.sem": true, + "kernel.shmall": true, + "kernel.shmmax": true, + "kernel.shmmni": true, + "kernel.shm_rmid_forced": true, + } + validSysctlPrefixes := []string{ + "net.", + "fs.mqueue.", + } + + for _, val := range strSlice { + foundMatch := false + arr := strings.Split(val, "=") + if len(arr) < 2 { + return nil, errors.Errorf("%s is invalid, sysctl values must be in the form of KEY=VALUE", val) + } + if validSysctlMap[arr[0]] { + sysctl[arr[0]] = arr[1] + continue + } + + for _, prefix := range validSysctlPrefixes { + if strings.HasPrefix(arr[0], prefix) { + sysctl[arr[0]] = arr[1] + foundMatch = true + break + } + } + if !foundMatch { + return nil, errors.Errorf("sysctl '%s' is not whitelisted", arr[0]) } - sysctl[values[0]] = values[1] } return sysctl, nil } diff --git a/cmd/podman/create_cli_test.go b/cmd/podman/create_cli_test.go index 63a1e5dd3..fa128c8e6 100644 --- a/cmd/podman/create_cli_test.go +++ b/cmd/podman/create_cli_test.go @@ -28,15 +28,15 @@ func createTmpFile(content []byte) (string, error) { return tmpfile.Name(), nil } -func TestConvertStringSliceToMap(t *testing.T) { - strSlice := []string{"BLAU=BLUE", "GELB=YELLOW"} - result, _ := convertStringSliceToMap(strSlice, "=") - assert.Equal(t, result["BLAU"], "BLUE") +func TestValidateSysctl(t *testing.T) { + strSlice := []string{"net.core.test1=4", "kernel.msgmax=2"} + result, _ := validateSysctl(strSlice) + assert.Equal(t, result["net.core.test1"], "4") } -func TestConvertStringSliceToMapBadData(t *testing.T) { +func TestValidateSysctlBadSysctl(t *testing.T) { strSlice := []string{"BLAU=BLUE", "GELB^YELLOW"} - _, err := convertStringSliceToMap(strSlice, "=") + _, err := validateSysctl(strSlice) assert.Error(t, err) } diff --git a/cmd/podman/run_test.go b/cmd/podman/run_test.go index 622e75d3e..f083b39af 100644 --- a/cmd/podman/run_test.go +++ b/cmd/podman/run_test.go @@ -3,6 +3,7 @@ package main import ( "testing" + units "github.com/docker/go-units" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/projectatomic/libpod/libpod" @@ -80,3 +81,22 @@ func TestPIDsLimit(t *testing.T) { runtimeSpec := getRuntimeSpec(CLI) assert.Equal(t, runtimeSpec.Linux.Resources.Pids.Limit, int64(22)) } + +// TestBLKIOWeightDevice verifies the inputed blkio weigh device is correctly defined in the spec +func TestBLKIOWeightDevice(t *testing.T) { + a := createCLI() + args := []string{"--blkio-weight-device", "/dev/sda:100"} + a.Run(append(cmd, args...)) + runtimeSpec := getRuntimeSpec(CLI) + assert.Equal(t, *runtimeSpec.Linux.Resources.BlockIO.WeightDevice[0].Weight, uint16(100)) +} + +// TestMemorySwap verifies that the inputed memory swap is correctly defined in the spec +func TestMemorySwap(t *testing.T) { + a := createCLI() + args := []string{"--memory-swap", "45m", "--memory", "40m"} + a.Run(append(cmd, args...)) + runtimeSpec := getRuntimeSpec(CLI) + mem, _ := units.RAMInBytes("45m") + assert.Equal(t, *runtimeSpec.Linux.Resources.Memory.Swap, mem) +} diff --git a/cmd/podman/spec.go b/cmd/podman/spec.go index d630b2f50..4c17bb62c 100644 --- a/cmd/podman/spec.go +++ b/cmd/podman/spec.go @@ -21,6 +21,8 @@ import ( "golang.org/x/sys/unix" ) +const cpuPeriod = 100000 + func blockAccessToKernelFilesystems(config *createConfig, g *generate.Generator) { if !config.Privileged { for _, mp := range []string{ @@ -143,7 +145,7 @@ func addRlimits(config *createConfig, g *generate.Generator) error { return errors.Wrapf(err, "ulimit option %q requires name=SOFT:HARD, failed to be parsed", u) } - g.AddProcessRlimits("RLIMIT_"+strings.ToUpper(ul.Name), uint64(ul.Soft), uint64(ul.Hard)) + g.AddProcessRlimits("RLIMIT_"+strings.ToUpper(ul.Name), uint64(ul.Hard), uint64(ul.Soft)) } return nil } @@ -210,10 +212,8 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) { if config.Hostname != "" { g.AddProcessEnv("HOSTNAME", config.Hostname) } - - for _, sysctl := range config.Sysctl { - s := strings.SplitN(sysctl, "=", 2) - g.AddLinuxSysctl(s[0], s[1]) + for sysctlKey, sysctlVal := range config.Sysctl { + g.AddLinuxSysctl(sysctlKey, sysctlVal) } // RESOURCES - MEMORY @@ -236,7 +236,6 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) { g.SetProcessOOMScoreAdj(config.Resources.OomScoreAdj) // RESOURCES - CPU - if config.Resources.CPUShares != 0 { g.SetLinuxResourcesCPUShares(config.Resources.CPUShares) } @@ -246,14 +245,18 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) { if config.Resources.CPUPeriod != 0 { g.SetLinuxResourcesCPUPeriod(config.Resources.CPUPeriod) } + if config.Resources.CPUs != 0 { + g.SetLinuxResourcesCPUPeriod(cpuPeriod) + g.SetLinuxResourcesCPUQuota(int64(config.Resources.CPUs * cpuPeriod)) + } if config.Resources.CPURtRuntime != 0 { g.SetLinuxResourcesCPURealtimeRuntime(config.Resources.CPURtRuntime) } if config.Resources.CPURtPeriod != 0 { g.SetLinuxResourcesCPURealtimePeriod(config.Resources.CPURtPeriod) } - if config.Resources.CPUs != "" { - g.SetLinuxResourcesCPUCpus(config.Resources.CPUs) + if config.Resources.CPUsetCPUs != "" { + g.SetLinuxResourcesCPUCpus(config.Resources.CPUsetCPUs) } if config.Resources.CPUsetMems != "" { g.SetLinuxResourcesCPUMems(config.Resources.CPUsetMems) @@ -356,6 +359,15 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) { return nil, err } + // BLOCK IO + blkio, err := config.CreateBlockIO() + if err != nil { + return nil, errors.Wrapf(err, "error creating block io") + } + if blkio != nil { + configSpec.Linux.Resources.BlockIO = blkio + } + /* Hooks: &configSpec.Hooks{}, //Annotations @@ -383,8 +395,8 @@ func createConfigToOCISpec(config *createConfig) (*spec.Spec, error) { return configSpec, nil } -func (c *createConfig) CreateBlockIO() (spec.LinuxBlockIO, error) { - bio := spec.LinuxBlockIO{} +func (c *createConfig) CreateBlockIO() (*spec.LinuxBlockIO, error) { + bio := &spec.LinuxBlockIO{} bio.Weight = &c.Resources.BlkioWeight if len(c.Resources.BlkioWeightDevice) > 0 { var lwds []spec.LinuxWeightDevice @@ -401,6 +413,7 @@ func (c *createConfig) CreateBlockIO() (spec.LinuxBlockIO, error) { lwd.Minor = int64(unix.Minor(wdStat.Rdev)) lwds = append(lwds, lwd) } + bio.WeightDevice = lwds } if len(c.Resources.DeviceReadBps) > 0 { readBps, err := makeThrottleArray(c.Resources.DeviceReadBps) @@ -430,7 +443,6 @@ func (c *createConfig) CreateBlockIO() (spec.LinuxBlockIO, error) { } bio.ThrottleWriteIOPSDevice = writeIOps } - return bio, nil } -- cgit v1.2.3-54-g00ecf