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 <umohnani@redhat.com>

Closes: #221
Approved by: mheon

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",
 	},

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"),

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 {
+	validSysctlMap := map[string]bool{
-		values := strings.Split(inputSysctl, delimiter)
+		"kernel.msgmax":          true,
-		if len(values) < 2 {
+		"kernel.msgmnb":          true,
-			return sysctl, errors.Errorf("%s in an invalid sysctl value", inputSysctl)
+		"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
 }

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) {
+func TestValidateSysctl(t *testing.T) {
-	strSlice := []string{"BLAU=BLUE", "GELB=YELLOW"}
+	strSlice := []string{"net.core.test1=4", "kernel.msgmax=2"}
-	result, _ := convertStringSliceToMap(strSlice, "=")
+	result, _ := validateSysctl(strSlice)
-	assert.Equal(t, result["BLAU"], "BLUE")
+	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)
 }
 

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)
+}

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 sysctlKey, sysctlVal := range config.Sysctl {
-	for _, sysctl := range config.Sysctl {
+		g.AddLinuxSysctl(sysctlKey, sysctlVal)
-		s := strings.SplitN(sysctl, "=", 2)
-		g.AddLinuxSysctl(s[0], s[1])
 	}
 
 	// 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 != "" {
+	if config.Resources.CPUsetCPUs != "" {
-		g.SetLinuxResourcesCPUCpus(config.Resources.CPUs)
+		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) {
+func (c *createConfig) CreateBlockIO() (*spec.LinuxBlockIO, error) {
-	bio := spec.LinuxBlockIO{}
+	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
 }
 

docs/podman-run.1.md

@@ -56,13 +56,6 @@ each of stdin, stdout, and stderr.
 **--cidfile**=""
    Write the container ID to the file
 
-**--cpu-count**=*0*
-    Limit the number of CPUs available for execution by the container.
-
-    On Windows Server containers, this is approximated as a percentage of total CPU usage.
-
-    On Windows Server containers, the processor resource controls are mutually exclusive, the order of precedence is CPUCount first, then CPUShares, and CPUPercent last.
-
 **--cpu-period**=*0*
     Limit the CPU CFS (Completely Fair Scheduler) period
 

test/podman_run.bats

@@ -110,6 +110,11 @@ IMAGE="docker.io/library/fedora:latest"
     [ "$status" -eq 0 ]
     [ "$output" = 2048 ]
 
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --ulimit nofile=1024:1028 ${IMAGE} ulimit -n | tr -d '\r'"
+    echo $output
+    [ "$status" -eq 0 ]
+    [ "$output" = 1024 ]
+
     run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --oom-kill-disable=true ${IMAGE} echo memory-hog"
     echo $output
     [ "$status" -eq 0 ]
@@ -141,3 +146,17 @@ IMAGE="docker.io/library/fedora:latest"
     echo "$output"
     [ "$status" -eq 0 ]
 }
+
+@test "podman run sysctl test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --sysctl net.core.somaxconn=65535 ${ALPINE} sysctl net.core.somaxconn | tr -d '\r'"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    [ "$output" = "net.core.somaxconn = 65535" ]
+}
+
+@test "podman run blkio-weight test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --blkio-weight=15 ${ALPINE} cat /sys/fs/cgroup/blkio/blkio.weight | tr -d '\r'"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    [ "$output" = 15 ]
+}

test/podman_run_cpu.bats

@@ -0,0 +1,71 @@
+#!/usr/bin/env bats
+
+load helpers
+
+function teardown() {
+    cleanup_test
+}
+
+function setup() {
+    copy_images
+}
+
+@test "run cpu-period test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --cpu-period=5000 ${ALPINE} cat /sys/fs/cgroup/cpu/cpu.cfs_period_us | tr -d '\r'"
+    echo $output
+    [ "$status" -eq 0 ]
+    [ "$output" = 5000 ]
+}
+
+@test "run cpu-quota test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --cpu-quota=5000 ${ALPINE} cat /sys/fs/cgroup/cpu/cpu.cfs_quota_us | tr -d '\r'"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    [ "$output" = 5000 ]
+}
+
+@test "run cpus test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --cpus=0.5 ${ALPINE} cat /sys/fs/cgroup/cpu/cpu.cfs_period_us | tr -d '\r'"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    [ "$output" = 100000 ]
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --cpus=0.5 ${ALPINE} cat /sys/fs/cgroup/cpu/cpu.cfs_quota_us | tr -d '\r'"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    [ "$output" = 50000 ]
+}
+
+@test "run cpu-shares test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --cpu-shares=2 ${ALPINE} cat /sys/fs/cgroup/cpu/cpu.shares | tr -d '\r'"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    [ "$output" = 2 ]
+}
+
+@test "run cpuset-cpus test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --cpuset-cpus=0 ${ALPINE} cat /sys/fs/cgroup/cpuset/cpuset.cpus | tr -d '\r'"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    [ "$output" = 0 ]
+}
+
+@test "run cpuset-mems test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --cpuset-mems=0 ${ALPINE} cat /sys/fs/cgroup/cpuset/cpuset.mems | tr -d '\r'"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    [ "$output" = 0 ]
+}
+
+@test "run failure if cpus and cpu-period set together test" {
+    # skip, error code incorrect with bash -c and will fail centos test without bash -c
+    run ${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --cpu-period=5000 --cpus=0.5 ${ALPINE} /bin/bash
+    echo "$output"
+    [ "$status" -ne 0 ]
+}
+
+@test "run failure if cpus and cpu-quota set together test" {
+    # skip, error code incorrect with bash -c and will fail centos test without bash -c
+    run ${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --cpu-quota=5000 --cpus=0.5 ${ALPINE} /bin/bash
+    echo "$output"
+    [ "$status" -ne 0 ]
+}

test/podman_run_memory.bats

@@ -0,0 +1,39 @@
+#!/usr/bin/env bats
+
+load helpers
+
+function teardown() {
+    cleanup_test
+}
+
+function setup() {
+    copy_images
+}
+
+@test "run memory test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --memory=40m ${ALPINE}  cat /sys/fs/cgroup/memory/memory.limit_in_bytes | tr -d '\r'"
+    echo $output
+    [ "$status" -eq 0 ]
+    [ "$output" = 41943040 ]
+}
+
+@test "run memory-reservation test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --memory-reservation=40m ${ALPINE} cat /sys/fs/cgroup/memory/memory.soft_limit_in_bytes | tr -d '\r'"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    [ "$output" = 41943040 ]
+}
+
+@test "run memory-swappiness test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --memory-swappiness=15 ${ALPINE} cat /sys/fs/cgroup/memory/memory.swappiness | tr -d '\r'"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    [ "$output" = 15 ]
+}
+
+@test "run kernel-memory test" {
+    run bash -c "${PODMAN_BINARY} ${PODMAN_OPTIONS} run --rm --kernel-memory=40m ${ALPINE} cat /sys/fs/cgroup/memory/memory.kmem.limit_in_bytes | tr -d '\r'"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    [ "$output" = 41943040 ]
+}