Merge pull request #2959 from mheon/merge_volume_flags

Merge volume flags implementation

4 files changed, 22 insertions, 388 deletions

diff --git a/cmd/podman/play_kube.go b/cmd/podman/play_kube.go
index 967798399..e778bafb9 100644
--- a/cmd/podman/play_kube.go
+++ b/cmd/podman/play_kube.go
@@ -205,7 +205,8 @@ func playKubeYAMLCmd(c *cliconfig.KubePlayValues, ctx context.Context, runtime *
 				return pod, errors.Errorf("Directories are the only supported HostPath type")
 			}
 		}
-		if err := shared.ValidateVolumeHostDir(hostPath.Path); err != nil {
+
+		if err := createconfig.ValidateVolumeHostDir(hostPath.Path); err != nil {
 			return pod, errors.Wrapf(err, "Error in parsing HostPath in YAML")
 		}
 		volumes[volume.Name] = hostPath.Path
@@ -281,7 +282,6 @@ func kubeContainerToCreateConfig(ctx context.Context, containerYAML v1.Container
 	// The default for MemorySwappiness is -1, not 0
 	containerConfig.Resources.MemorySwappiness = -1
 
-	containerConfig.Runtime = runtime
 	containerConfig.Image = containerYAML.Image
 	containerConfig.ImageID = newImage.ID()
 	containerConfig.Name = containerYAML.Name
@@ -352,7 +352,7 @@ func kubeContainerToCreateConfig(ctx context.Context, containerYAML v1.Container
 		if !exists {
 			return nil, errors.Errorf("Volume mount %s specified for container but not configured in volumes", volume.Name)
 		}
-		if err := shared.ValidateVolumeCtrDir(volume.MountPath); err != nil {
+		if err := createconfig.ValidateVolumeCtrDir(volume.MountPath); err != nil {
 			return nil, errors.Wrapf(err, "error in parsing MountPath")
 		}
 		containerConfig.Volumes = append(containerConfig.Volumes, fmt.Sprintf("%s:%s", host_path, volume.MountPath))
diff --git a/cmd/podman/run_test.go b/cmd/podman/run_test.go
deleted file mode 100644
index af9e6923c..000000000
--- a/cmd/podman/run_test.go
+++ /dev/null
@@ -1,162 +0,0 @@
-package main
-
-import (
-	"runtime"
-	"testing"
-
-	"github.com/containers/libpod/cmd/podman/cliconfig"
-	"github.com/containers/libpod/cmd/podman/shared"
-	"github.com/containers/libpod/pkg/inspect"
-	cc "github.com/containers/libpod/pkg/spec"
-	"github.com/containers/libpod/pkg/sysinfo"
-	"github.com/docker/go-units"
-	ociv1 "github.com/opencontainers/image-spec/specs-go/v1"
-	spec "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/spf13/cobra"
-	"github.com/stretchr/testify/assert"
-)
-
-var (
-	sysInfo = sysinfo.New(true)
-	cmd     = []string{"podman", "test", "alpine"}
-	CLI     *cliconfig.PodmanCommand
-)
-
-// generates a mocked ImageData structure based on alpine
-func generateAlpineImageData() *inspect.ImageData {
-	config := &ociv1.ImageConfig{
-		User:         "",
-		ExposedPorts: nil,
-		Env:          []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"},
-		Entrypoint:   []string{},
-		Cmd:          []string{"/bin/sh"},
-		Volumes:      nil,
-		WorkingDir:   "",
-		Labels:       nil,
-		StopSignal:   "",
-	}
-
-	data := &inspect.ImageData{
-		ID:           "e21c333399e0aeedfd70e8827c9fba3f8e9b170ef8a48a29945eb7702bf6aa5f",
-		RepoTags:     []string{"docker.io/library/alpine:latest"},
-		RepoDigests:  []string{"docker.io/library/alpine@sha256:5cb04fce748f576d7b72a37850641de8bd725365519673c643ef2d14819b42c6"},
-		Comment:      "Created:2017-12-01 18:48:48.949613376 +0000",
-		Author:       "",
-		Architecture: "amd64",
-		Os:           "linux",
-		Version:      "17.06.2-ce",
-		Config:       config,
-	}
-	return data
-}
-
-// sets a global CLI
-func testCmd(c *cobra.Command) error {
-	CLI = &cliconfig.PodmanCommand{Command: c}
-	return nil
-}
-
-// creates the mocked cli pointing to our create flags
-// global flags like log-level are not implemented
-func createCLI(args []string) *cliconfig.PodmanCommand {
-	var testCommand = &cliconfig.PodmanCommand{
-		Command: &cobra.Command{
-			Use: "test",
-			RunE: func(cmd *cobra.Command, args []string) error {
-				return testCmd(cmd)
-			},
-		},
-	}
-	rootCmd := testCommand
-	getCreateFlags(rootCmd)
-	rootCmd.ParseFlags(args)
-	return rootCmd
-}
-
-func getRuntimeSpec(c *cliconfig.PodmanCommand) (*spec.Spec, error) {
-	/*
-		TODO: This test has never worked. Need to install content
-		runtime, err := getRuntime(c)
-		if err != nil {
-		return nil, err
-		}
-		createConfig, err := parseCreateOpts(c, runtime, "alpine", generateAlpineImageData())
-	*/
-	ctx := getContext()
-	genericResults := shared.NewIntermediateLayer(c, false)
-	createConfig, err := shared.ParseCreateOpts(ctx, &genericResults, nil, "alpine", generateAlpineImageData())
-	if err != nil {
-		return nil, err
-	}
-	runtimeSpec, err := cc.CreateConfigToOCISpec(createConfig)
-	if err != nil {
-		return nil, err
-	}
-	return runtimeSpec, nil
-}
-
-// TestPIDsLimit verifies the inputted pid-limit is correctly defined in the spec
-func TestPIDsLimit(t *testing.T) {
-	// The default configuration of podman enables seccomp, which is not available on non-Linux systems.
-	// Thus, any tests that use the default seccomp setting would fail.
-	// Skip the tests on non-Linux platforms rather than explicitly disable seccomp in the test and possibly affect the test result.
-	if runtime.GOOS != "linux" {
-		t.Skip("seccomp, which is enabled by default, is only supported on Linux")
-	}
-	if !sysInfo.PidsLimit {
-		t.Skip("running test not supported by the host system")
-	}
-	args := []string{"--pids-limit", "22"}
-	a := createCLI(args)
-	a.InputArgs = args
-	//a.Run(append(cmd, args...))
-	runtimeSpec, err := getRuntimeSpec(a)
-	if err != nil {
-		t.Fatalf(err.Error())
-	}
-	assert.Equal(t, runtimeSpec.Linux.Resources.Pids.Limit, int64(22))
-}
-
-// TestBLKIOWeightDevice verifies the inputted blkio weigh device is correctly defined in the spec
-func TestBLKIOWeightDevice(t *testing.T) {
-	// The default configuration of podman enables seccomp, which is not available on non-Linux systems.
-	// Thus, any tests that use the default seccomp setting would fail.
-	// Skip the tests on non-Linux platforms rather than explicitly disable seccomp in the test and possibly affect the test result.
-	if runtime.GOOS != "linux" {
-		t.Skip("seccomp, which is enabled by default, is only supported on Linux")
-	}
-	if !sysInfo.BlkioWeightDevice {
-		t.Skip("running test not supported by the host system")
-	}
-	args := []string{"--blkio-weight-device", "/dev/zero:100"}
-	a := createCLI(args)
-	a.InputArgs = args
-	runtimeSpec, err := getRuntimeSpec(a)
-	if err != nil {
-		t.Fatalf(err.Error())
-	}
-	assert.Equal(t, *runtimeSpec.Linux.Resources.BlockIO.WeightDevice[0].Weight, uint16(100))
-}
-
-// TestMemorySwap verifies that the inputted memory swap is correctly defined in the spec
-func TestMemorySwap(t *testing.T) {
-	// The default configuration of podman enables seccomp, which is not available on non-Linux systems.
-	// Thus, any tests that use the default seccomp setting would fail.
-	// Skip the tests on non-Linux platforms rather than explicitly disable seccomp in the test and possibly affect the test result.
-	if runtime.GOOS != "linux" {
-		t.Skip("seccomp, which is enabled by default, is only supported on Linux")
-	}
-	if !sysInfo.SwapLimit {
-		t.Skip("running test not supported by the host system")
-	}
-	args := []string{"--memory-swap", "45m", "--memory", "40m"}
-	a := createCLI(args)
-	a.InputArgs = args
-	//a.Run(append(cmd, args...))
-	runtimeSpec, err := getRuntimeSpec(a)
-	if err != nil {
-		t.Fatalf(err.Error())
-	}
-	mem, _ := units.RAMInBytes("45m")
-	assert.Equal(t, *runtimeSpec.Linux.Resources.Memory.Swap, mem)
-}
diff --git a/cmd/podman/shared/create.go b/cmd/podman/shared/create.go
index eac2d044d..81566326b 100644
--- a/cmd/podman/shared/create.go
+++ b/cmd/podman/shared/create.go
@@ -25,7 +25,6 @@ import (
 	"github.com/docker/go-connections/nat"
 	"github.com/docker/go-units"
 	"github.com/google/shlex"
-	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/opentracing/opentracing-go"
 	"github.com/pkg/errors"
@@ -114,6 +113,7 @@ func CreateContainer(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 			}
 		}
 	}
+
 	createConfig, err := ParseCreateOpts(ctx, c, runtime, imageName, data)
 	if err != nil {
 		return nil, nil, err
@@ -123,7 +123,16 @@ func CreateContainer(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 	// at this point. The rest is done by WithOptions.
 	createConfig.HealthCheck = healthCheck
 
-	ctr, err := CreateContainerFromCreateConfig(runtime, createConfig, ctx, nil)
+	// TODO: Should be able to return this from ParseCreateOpts
+	var pod *libpod.Pod
+	if createConfig.Pod != "" {
+		pod, err = runtime.LookupPod(createConfig.Pod)
+		if err != nil {
+			return nil, nil, errors.Wrapf(err, "error looking up pod to join")
+		}
+	}
+
+	ctr, err := CreateContainerFromCreateConfig(runtime, createConfig, ctx, pod)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -139,7 +148,7 @@ func CreateContainer(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 	return ctr, createConfig, nil
 }
 
-func parseSecurityOpt(config *cc.CreateConfig, securityOpts []string) error {
+func parseSecurityOpt(config *cc.CreateConfig, securityOpts []string, runtime *libpod.Runtime) error {
 	var (
 		labelOpts []string
 	)
@@ -147,7 +156,7 @@ func parseSecurityOpt(config *cc.CreateConfig, securityOpts []string) error {
 	if config.PidMode.IsHost() {
 		labelOpts = append(labelOpts, label.DisableSecOpt()...)
 	} else if config.PidMode.IsContainer() {
-		ctr, err := config.Runtime.LookupContainer(config.PidMode.Container())
+		ctr, err := runtime.LookupContainer(config.PidMode.Container())
 		if err != nil {
 			return errors.Wrapf(err, "container %q not found", config.PidMode.Container())
 		}
@@ -161,7 +170,7 @@ func parseSecurityOpt(config *cc.CreateConfig, securityOpts []string) error {
 	if config.IpcMode.IsHost() {
 		labelOpts = append(labelOpts, label.DisableSecOpt()...)
 	} else if config.IpcMode.IsContainer() {
-		ctr, err := config.Runtime.LookupContainer(config.IpcMode.Container())
+		ctr, err := runtime.LookupContainer(config.IpcMode.Container())
 		if err != nil {
 			return errors.Wrapf(err, "container %q not found", config.IpcMode.Container())
 		}
@@ -331,18 +340,6 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 		}
 		blkioWeight = uint16(u)
 	}
-	var mountList []spec.Mount
-	if mountList, err = parseMounts(c.StringArray("mount")); err != nil {
-		return nil, err
-	}
-
-	if err = parseVolumes(c.StringArray("volume")); err != nil {
-		return nil, err
-	}
-
-	if err = parseVolumesFrom(c.StringSlice("volumes-from")); err != nil {
-		return nil, err
-	}
 
 	tty := c.Bool("tty")
 
@@ -604,7 +601,6 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 	memorySwappiness := c.Int64("memory-swappiness")
 
 	config := &cc.CreateConfig{
-		Runtime:           runtime,
 		Annotations:       annotations,
 		BuiltinImgVolumes: ImageVolumes,
 		ConmonPidFile:     c.String("conmon-pidfile"),
@@ -627,6 +623,8 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 		HTTPProxy:   c.Bool("http-proxy"),
 		NoHosts:     c.Bool("no-hosts"),
 		IDMappings:  idmappings,
+		Init:        c.Bool("init"),
+		InitPath:    c.String("init-path"),
 		Image:       imageName,
 		ImageID:     imageID,
 		Interactive: c.Bool("interactive"),
@@ -687,31 +685,18 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 		Tty:         tty,
 		User:        user,
 		UsernsMode:  usernsMode,
-		Mounts:      mountList,
+		MountsFlag:  c.StringArray("mount"),
 		Volumes:     c.StringArray("volume"),
 		WorkDir:     workDir,
 		Rootfs:      rootfs,
 		VolumesFrom: c.StringSlice("volumes-from"),
 		Syslog:      c.Bool("syslog"),
 	}
-	if c.Bool("init") {
-		initPath := c.String("init-path")
-		if initPath == "" {
-			rtc, err := runtime.GetConfig()
-			if err != nil {
-				return nil, err
-			}
-			initPath = rtc.InitPath
-		}
-		if err := config.AddContainerInitBinary(initPath); err != nil {
-			return nil, err
-		}
-	}
 
 	if config.Privileged {
 		config.LabelOpts = label.DisableSecOpt()
 	} else {
-		if err := parseSecurityOpt(config, c.StringArray("security-opt")); err != nil {
+		if err := parseSecurityOpt(config, c.StringArray("security-opt"), runtime); err != nil {
 			return nil, err
 		}
 	}
@@ -727,12 +712,7 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 }
 
 func CreateContainerFromCreateConfig(r *libpod.Runtime, createConfig *cc.CreateConfig, ctx context.Context, pod *libpod.Pod) (*libpod.Container, error) {
-	runtimeSpec, err := cc.CreateConfigToOCISpec(createConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	options, err := createConfig.GetContainerCreateOptions(r, pod)
+	runtimeSpec, options, err := createConfig.MakeContainerConfig(r, pod)
 	if err != nil {
 		return nil, err
 	}
diff --git a/cmd/podman/shared/create_cli.go b/cmd/podman/shared/create_cli.go
index 4f9cb1699..f731e8db5 100644
--- a/cmd/podman/shared/create_cli.go
+++ b/cmd/podman/shared/create_cli.go
@@ -2,15 +2,11 @@ package shared
 
 import (
 	"fmt"
-	"os"
-	"path/filepath"
 	"strings"
 
 	"github.com/containers/libpod/cmd/podman/shared/parse"
 	cc "github.com/containers/libpod/pkg/spec"
 	"github.com/containers/libpod/pkg/sysinfo"
-	"github.com/docker/go-units"
-	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -78,186 +74,6 @@ func addWarning(warnings []string, msg string) []string {
 	return append(warnings, msg)
 }
 
-// Format supported.
-// podman run --mount type=bind,src=/etc/resolv.conf,target=/etc/resolv.conf ...
-// podman run --mount type=tmpfs,target=/dev/shm ..
-func parseMounts(mounts []string) ([]spec.Mount, error) {
-	// TODO(vrothberg): the manual parsing can be replaced with a regular expression
-	//                  to allow a more robust parsing of the mount format and to give
-	//                  precise errors regarding supported format versus suppored options.
-	var mountList []spec.Mount
-	errInvalidSyntax := errors.Errorf("incorrect mount format: should be --mount type=<bind|tmpfs>,[src=<host-dir>,]target=<ctr-dir>[,options]")
-	for _, mount := range mounts {
-		var tokenCount int
-		var mountInfo spec.Mount
-
-		arr := strings.SplitN(mount, ",", 2)
-		if len(arr) < 2 {
-			return nil, errors.Wrapf(errInvalidSyntax, "%q", mount)
-		}
-		kv := strings.Split(arr[0], "=")
-		if kv[0] != "type" {
-			return nil, errors.Wrapf(errInvalidSyntax, "%q", mount)
-		}
-		switch kv[1] {
-		case "bind":
-			mountInfo.Type = string(cc.TypeBind)
-		case "tmpfs":
-			mountInfo.Type = string(cc.TypeTmpfs)
-			mountInfo.Source = string(cc.TypeTmpfs)
-			mountInfo.Options = append(mountInfo.Options, []string{"rprivate", "noexec", "nosuid", "nodev", "size=65536k"}...)
-
-		default:
-			return nil, errors.Errorf("invalid filesystem type %q", kv[1])
-		}
-
-		tokens := strings.Split(arr[1], ",")
-		for i, val := range tokens {
-			if i == (tokenCount - 1) {
-				//Parse tokens before options.
-				break
-			}
-			kv := strings.Split(val, "=")
-			switch kv[0] {
-			case "ro", "nosuid", "nodev", "noexec":
-				mountInfo.Options = append(mountInfo.Options, kv[0])
-			case "shared", "rshared", "private", "rprivate", "slave", "rslave", "Z", "z":
-				if mountInfo.Type != "bind" {
-					return nil, errors.Errorf("%s can only be used with bind mounts", kv[0])
-				}
-				mountInfo.Options = append(mountInfo.Options, kv[0])
-			case "tmpfs-mode":
-				if mountInfo.Type != "tmpfs" {
-					return nil, errors.Errorf("%s can only be used with tmpfs mounts", kv[0])
-				}
-				mountInfo.Options = append(mountInfo.Options, fmt.Sprintf("mode=%s", kv[1]))
-			case "tmpfs-size":
-				if mountInfo.Type != "tmpfs" {
-					return nil, errors.Errorf("%s can only be used with tmpfs mounts", kv[0])
-				}
-				shmSize, err := units.FromHumanSize(kv[1])
-				if err != nil {
-					return nil, errors.Wrapf(err, "unable to translate tmpfs-size")
-				}
-
-				mountInfo.Options = append(mountInfo.Options, fmt.Sprintf("size=%d", shmSize))
-
-			case "bind-propagation":
-				if mountInfo.Type != "bind" {
-					return nil, errors.Errorf("%s can only be used with bind mounts", kv[0])
-				}
-				mountInfo.Options = append(mountInfo.Options, kv[1])
-			case "src", "source":
-				if mountInfo.Type == "tmpfs" {
-					return nil, errors.Errorf("cannot use src= on a tmpfs file system")
-				}
-				if err := ValidateVolumeHostDir(kv[1]); err != nil {
-					return nil, err
-				}
-				mountInfo.Source = kv[1]
-			case "target", "dst", "destination":
-				if err := ValidateVolumeCtrDir(kv[1]); err != nil {
-					return nil, err
-				}
-				mountInfo.Destination = kv[1]
-			default:
-				return nil, errors.Errorf("incorrect mount option : %s", kv[0])
-			}
-		}
-		mountList = append(mountList, mountInfo)
-	}
-	return mountList, nil
-}
-
-func parseVolumes(volumes []string) error {
-	for _, volume := range volumes {
-		arr := strings.SplitN(volume, ":", 3)
-		if len(arr) < 2 {
-			return errors.Errorf("incorrect volume format %q, should be host-dir:ctr-dir[:option]", volume)
-		}
-		if err := ValidateVolumeHostDir(arr[0]); err != nil {
-			return err
-		}
-		if err := ValidateVolumeCtrDir(arr[1]); err != nil {
-			return err
-		}
-		if len(arr) > 2 {
-			if err := validateVolumeOpts(arr[2]); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
-func parseVolumesFrom(volumesFrom []string) error {
-	for _, vol := range volumesFrom {
-		arr := strings.SplitN(vol, ":", 2)
-		if len(arr) == 2 {
-			if strings.Contains(arr[1], "Z") || strings.Contains(arr[1], "private") || strings.Contains(arr[1], "slave") || strings.Contains(arr[1], "shared") {
-				return errors.Errorf("invalid options %q, can only specify 'ro', 'rw', and 'z", arr[1])
-			}
-			if err := validateVolumeOpts(arr[1]); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
-// ValidateVolumeHostDir ...
-func ValidateVolumeHostDir(hostDir string) error {
-	if len(hostDir) == 0 {
-		return errors.Errorf("host directory cannot be empty")
-	}
-	if filepath.IsAbs(hostDir) {
-		if _, err := os.Stat(hostDir); err != nil {
-			return errors.Wrapf(err, "error checking path %q", hostDir)
-		}
-	}
-	// If hostDir is not an absolute path, that means the user wants to create a
-	// named volume. This will be done later on in the code.
-	return nil
-}
-
-// ValidateVolumeCtrDir ...
-func ValidateVolumeCtrDir(ctrDir string) error {
-	if len(ctrDir) == 0 {
-		return errors.Errorf("container directory cannot be empty")
-	}
-	if !filepath.IsAbs(ctrDir) {
-		return errors.Errorf("invalid container path, must be an absolute path %q", ctrDir)
-	}
-	return nil
-}
-
-func validateVolumeOpts(option string) error {
-	var foundRootPropagation, foundRWRO, foundLabelChange int
-	options := strings.Split(option, ",")
-	for _, opt := range options {
-		switch opt {
-		case "rw", "ro":
-			foundRWRO++
-			if foundRWRO > 1 {
-				return errors.Errorf("invalid options %q, can only specify 1 'rw' or 'ro' option", option)
-			}
-		case "z", "Z":
-			foundLabelChange++
-			if foundLabelChange > 1 {
-				return errors.Errorf("invalid options %q, can only specify 1 'z' or 'Z' option", option)
-			}
-		case "private", "rprivate", "shared", "rshared", "slave", "rslave":
-			foundRootPropagation++
-			if foundRootPropagation > 1 {
-				return errors.Errorf("invalid options %q, can only specify 1 '[r]shared', '[r]private' or '[r]slave' option", option)
-			}
-		default:
-			return errors.Errorf("invalid option type %q", option)
-		}
-	}
-	return nil
-}
-
 func verifyContainerResources(config *cc.CreateConfig, update bool) ([]string, error) {
 	warnings := []string{}
 	sysInfo := sysinfo.New(true)