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

6 files changed, 98 insertions, 29 deletions

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
 }