Merge pull request #2959 from mheon/merge_volume_flags

Merge volume flags implementation

.tool/lint

@@ -25,7 +25,6 @@ ${LINTER} \
 	--deadline=600s --disable-all\
 	--enable=deadcode\
 	--enable=errcheck\
-	--enable=goconst\
 	--enable=gofmt\
 	--enable=golint\
 	--enable=ineffassign\
@@ -41,7 +40,6 @@ ${LINTER} \
 	--exclude='duplicate of.*_test.go.*\(dupl\)$'\
 	--exclude='cmd\/client\/.*\.go.*\(dupl\)$'\
 	--exclude='libpod\/.*_easyjson.go:.*'\
-	--exclude='.* other occurrence\(s\) of "(container|host|tmpfs|unknown)" found in: .*\(goconst\)$'\
 	--exclude='vendor\/.*'\
 	--exclude='podman\/.*'\
 	--exclude='server\/seccomp\/.*\.go.*$'\

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

cmd/podman/run_test.go

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

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)
+	runtimeSpec, options, err := createConfig.MakeContainerConfig(r, pod)
-	if err != nil {
-		return nil, err
-	}
-
-	options, err := createConfig.GetContainerCreateOptions(r, pod)
 	if err != nil {
 		return nil, err
 	}

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)

libpod/options.go

@@ -10,6 +10,7 @@ import (
 	"github.com/containers/image/manifest"
 	"github.com/containers/libpod/pkg/namespaces"
 	"github.com/containers/libpod/pkg/rootless"
+	"github.com/containers/libpod/pkg/util"
 	"github.com/containers/storage"
 	"github.com/containers/storage/pkg/idtools"
 	"github.com/cri-o/ocicni/pkg/ocicni"
@@ -1273,7 +1274,7 @@ func WithNamedVolumes(volumes []*ContainerNamedVolume) CtrCreateOption {
 			ctr.config.NamedVolumes = append(ctr.config.NamedVolumes, &ContainerNamedVolume{
 				Name:    vol.Name,
 				Dest:    vol.Dest,
-				Options: vol.Options,
+				Options: util.ProcessOptions(vol.Options),
 			})
 		}
 

pkg/spec/containerconfig.go

@@ -0,0 +1,40 @@
+package createconfig
+
+import (
+	"github.com/containers/libpod/libpod"
+	spec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+)
+
+// MakeContainerConfig generates all configuration necessary to start a
+// container with libpod from a completed CreateConfig struct.
+func (config *CreateConfig) MakeContainerConfig(runtime *libpod.Runtime, pod *libpod.Pod) (*spec.Spec, []libpod.CtrCreateOption, error) {
+	if config.Pod != "" && pod == nil {
+		return nil, nil, errors.Wrapf(libpod.ErrInvalidArg, "pod was specified but no pod passed")
+	} else if config.Pod == "" && pod != nil {
+		return nil, nil, errors.Wrapf(libpod.ErrInvalidArg, "pod was given but no pod is specified")
+	}
+
+	// Parse volumes flag into OCI spec mounts and libpod Named Volumes.
+	// If there is an identical mount in the OCI spec, we will replace it
+	// with a mount generated here.
+	mounts, namedVolumes, err := config.parseVolumes(runtime)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	runtimeSpec, err := config.createConfigToOCISpec(runtime, mounts)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	options, err := config.getContainerCreateOptions(runtime, pod, mounts, namedVolumes)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	logrus.Debugf("created OCI spec and options for new container")
+
+	return runtimeSpec, options, nil
+}

pkg/spec/createconfig.go

@@ -1,7 +1,6 @@
 package createconfig
 
 import (
-	"fmt"
 	"net"
 	"os"
 	"strconv"
@@ -12,7 +11,6 @@ import (
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/namespaces"
 	"github.com/containers/storage"
-	"github.com/containers/storage/pkg/stringid"
 	"github.com/cri-o/ocicni/pkg/ocicni"
 	"github.com/docker/go-connections/nat"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
@@ -25,12 +23,6 @@ import (
 const (
 	bps = iota
 	iops
-	// TypeBind is the type for mounting host dir
-	TypeBind = "bind"
-	// TypeVolume is the type for remote storage volumes
-	// TypeVolume = "volume"  // re-enable upon use
-	// TypeTmpfs is the type for mounting tmpfs
-	TypeTmpfs = "tmpfs"
 )
 
 // CreateResourceConfig represents resource elements in CreateConfig
@@ -64,7 +56,6 @@ type CreateResourceConfig struct {
 
 // CreateConfig is a pre OCI spec structure.  It represents user input from varlink or the CLI
 type CreateConfig struct {
-	Runtime            *libpod.Runtime
 	Annotations        map[string]string
 	Args               []string
 	CapAdd             []string // cap-add
@@ -87,6 +78,8 @@ type CreateConfig struct {
 	HostAdd            []string //add-host
 	Hostname           string   //hostname
 	HTTPProxy          bool
+	Init               bool   // init
+	InitPath           string //init-path
 	Image              string
 	ImageID            string
 	BuiltinImgVolumes  map[string]struct{} // volumes defined in the image config
@@ -125,15 +118,16 @@ type CreateConfig struct {
 	UsernsMode         namespaces.UsernsMode //userns
 	User               string                //user
 	UtsMode            namespaces.UTSMode    //uts
-	Mounts             []spec.Mount          //mounts
+	Mounts             []spec.Mount
-	Volumes            []string              //volume
+	MountsFlag         []string // mounts
+	NamedVolumes       []*libpod.ContainerNamedVolume
+	Volumes            []string //volume
 	VolumesFrom        []string
-	NamedVolumes       []*libpod.ContainerNamedVolume // Filled in by CreateConfigToOCISpec
+	WorkDir            string   //workdir
-	WorkDir            string                         //workdir
+	LabelOpts          []string //SecurityOpts
-	LabelOpts          []string                       //SecurityOpts
+	NoNewPrivs         bool     //SecurityOpts
-	NoNewPrivs         bool                           //SecurityOpts
+	ApparmorProfile    string   //SecurityOpts
-	ApparmorProfile    string                         //SecurityOpts
+	SeccompProfilePath string   //SecurityOpts
-	SeccompProfilePath string                         //SecurityOpts
 	SecurityOpts       []string
 	Rootfs             string
 	Syslog             bool // Whether to enable syslog on exit commands
@@ -147,224 +141,8 @@ func (c *CreateConfig) CreateBlockIO() (*spec.LinuxBlockIO, error) {
 	return c.createBlockIO()
 }
 
-// AddContainerInitBinary adds the init binary specified by path iff the
+func (c *CreateConfig) createExitCommand(runtime *libpod.Runtime) ([]string, error) {
-// container will run in a private PID namespace that is not shared with the
+	config, err := runtime.GetConfig()
-// host or another pre-existing container, where an init-like process is
-// already running.
-//
-// Note that AddContainerInitBinary prepends "/dev/init" "--" to the command
-// to execute the bind-mounted binary as PID 1.
-func (c *CreateConfig) AddContainerInitBinary(path string) error {
-	if path == "" {
-		return fmt.Errorf("please specify a path to the container-init binary")
-	}
-	if !c.PidMode.IsPrivate() {
-		return fmt.Errorf("cannot add init binary as PID 1 (PID namespace isn't private)")
-	}
-	if c.Systemd {
-		return fmt.Errorf("cannot use container-init binary with systemd")
-	}
-	if _, err := os.Stat(path); os.IsNotExist(err) {
-		return errors.Wrap(err, "container-init binary not found on the host")
-	}
-	c.Command = append([]string{"/dev/init", "--"}, c.Command...)
-	c.Mounts = append(c.Mounts, spec.Mount{
-		Destination: "/dev/init",
-		Type:        TypeBind,
-		Source:      path,
-		Options:     []string{TypeBind, "ro"},
-	})
-	return nil
-}
-
-func processOptions(options []string) []string {
-	var (
-		foundrw, foundro bool
-		rootProp         string
-	)
-	options = append(options, "rbind")
-	for _, opt := range options {
-		switch opt {
-		case "rw":
-			foundrw = true
-		case "ro":
-			foundro = true
-		case "private", "rprivate", "slave", "rslave", "shared", "rshared":
-			rootProp = opt
-		}
-	}
-	if !foundrw && !foundro {
-		options = append(options, "rw")
-	}
-	if rootProp == "" {
-		options = append(options, "rprivate")
-	}
-	return options
-}
-
-func (c *CreateConfig) initFSMounts() []spec.Mount {
-	var mounts []spec.Mount
-	for _, m := range c.Mounts {
-		m.Options = processOptions(m.Options)
-		if m.Type == "tmpfs" {
-			m.Options = append(m.Options, "tmpcopyup")
-		} else {
-			mounts = append(mounts, m)
-		}
-	}
-	return mounts
-}
-
-// GetVolumeMounts takes user provided input for bind mounts and creates Mount structs
-func (c *CreateConfig) GetVolumeMounts(specMounts []spec.Mount) ([]spec.Mount, error) {
-	m := []spec.Mount{}
-	for _, i := range c.Volumes {
-		var options []string
-		spliti := strings.Split(i, ":")
-		if len(spliti) > 2 {
-			options = strings.Split(spliti[2], ",")
-		}
-
-		m = append(m, spec.Mount{
-			Destination: spliti[1],
-			Type:        string(TypeBind),
-			Source:      spliti[0],
-			Options:     processOptions(options),
-		})
-
-		logrus.Debugf("User mount %s:%s options %v", spliti[0], spliti[1], options)
-	}
-
-	if c.ImageVolumeType == "ignore" {
-		return m, nil
-	}
-
-	for vol := range c.BuiltinImgVolumes {
-		if libpod.MountExists(specMounts, vol) || libpod.MountExists(m, vol) {
-			continue
-		}
-
-		mount := spec.Mount{
-			Destination: vol,
-			Type:        c.ImageVolumeType,
-			Options:     []string{"rprivate", "rw", "nodev"},
-		}
-		if c.ImageVolumeType == "tmpfs" {
-			mount.Source = "tmpfs"
-			mount.Options = append(mount.Options, "tmpcopyup")
-		} else {
-			// TODO: Move support for this and tmpfs into libpod
-			// Should tmpfs also be handled as named volumes? Wouldn't be hard
-			// This will cause a new local Volume to be created on your system
-			mount.Source = stringid.GenerateNonCryptoID()
-			mount.Options = append(mount.Options, TypeBind)
-		}
-		m = append(m, mount)
-	}
-
-	return m, nil
-}
-
-// GetVolumesFrom reads the create-config artifact of the container to get volumes from
-// and adds it to c.Volumes of the current container.
-func (c *CreateConfig) GetVolumesFrom() error {
-	if os.Geteuid() != 0 {
-		return nil
-	}
-
-	for _, vol := range c.VolumesFrom {
-		options := ""
-		splitVol := strings.SplitN(vol, ":", 2)
-		if len(splitVol) == 2 {
-			options = splitVol[1]
-		}
-		ctr, err := c.Runtime.LookupContainer(splitVol[0])
-		if err != nil {
-			return errors.Wrapf(err, "error looking up container %q", splitVol[0])
-		}
-
-		logrus.Debugf("Adding volumes from container %s", ctr.ID())
-
-		// Look up the container's user volumes. This gets us the
-		// destinations of all mounts the user added to the container.
-		userVolumesArr := ctr.UserVolumes()
-
-		// We're going to need to access them a lot, so convert to a map
-		// to reduce looping.
-		// We'll also use the map to indicate if we missed any volumes along the way.
-		userVolumes := make(map[string]bool)
-		for _, dest := range userVolumesArr {
-			userVolumes[dest] = false
-		}
-
-		// Now we get the container's spec and loop through its volumes
-		// and append them in if we can find them.
-		spec := ctr.Spec()
-		if spec == nil {
-			return errors.Errorf("error retrieving container %s spec", ctr.ID())
-		}
-		for _, mnt := range spec.Mounts {
-			if mnt.Type != TypeBind {
-				continue
-			}
-			if _, exists := userVolumes[mnt.Destination]; exists {
-				userVolumes[mnt.Destination] = true
-				localOptions := options
-				if localOptions == "" {
-					localOptions = strings.Join(mnt.Options, ",")
-				}
-				c.Volumes = append(c.Volumes, fmt.Sprintf("%s:%s:%s", mnt.Source, mnt.Destination, localOptions))
-			}
-		}
-
-		// We're done with the spec mounts. Add named volumes.
-		// Add these unconditionally - none of them are automatically
-		// part of the container, as some spec mounts are.
-		namedVolumes := ctr.NamedVolumes()
-		for _, namedVol := range namedVolumes {
-			if _, exists := userVolumes[namedVol.Dest]; exists {
-				userVolumes[namedVol.Dest] = true
-			}
-			localOptions := options
-			if localOptions == "" {
-				localOptions = strings.Join(namedVol.Options, ",")
-			}
-			c.Volumes = append(c.Volumes, fmt.Sprintf("%s:%s:%s", namedVol.Name, namedVol.Dest, localOptions))
-		}
-
-		// Check if we missed any volumes
-		for volDest, found := range userVolumes {
-			if !found {
-				logrus.Warnf("Unable to match volume %s from container %s for volumes-from", volDest, ctr.ID())
-			}
-		}
-	}
-	return nil
-}
-
-//GetTmpfsMounts takes user provided input for Tmpfs mounts and creates Mount structs
-func (c *CreateConfig) GetTmpfsMounts() []spec.Mount {
-	var m []spec.Mount
-	for _, i := range c.Tmpfs {
-		// Default options if nothing passed
-		options := []string{"rprivate", "rw", "noexec", "nosuid", "nodev", "size=65536k"}
-		spliti := strings.Split(i, ":")
-		destPath := spliti[0]
-		if len(spliti) > 1 {
-			options = strings.Split(spliti[1], ",")
-		}
-		m = append(m, spec.Mount{
-			Destination: destPath,
-			Type:        string(TypeTmpfs),
-			Options:     options,
-			Source:      string(TypeTmpfs),
-		})
-	}
-	return m
-}
-
-func (c *CreateConfig) createExitCommand() ([]string, error) {
-	config, err := c.Runtime.GetConfig()
 	if err != nil {
 		return nil, err
 	}
@@ -396,7 +174,7 @@ func (c *CreateConfig) createExitCommand() ([]string, error) {
 }
 
 // GetContainerCreateOptions takes a CreateConfig and returns a slice of CtrCreateOptions
-func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *libpod.Pod) ([]libpod.CtrCreateOption, error) {
+func (c *CreateConfig) getContainerCreateOptions(runtime *libpod.Runtime, pod *libpod.Pod, mounts []spec.Mount, namedVolumes []*libpod.ContainerNamedVolume) ([]libpod.CtrCreateOption, error) {
 	var options []libpod.CtrCreateOption
 	var portBindings []ocicni.PortMapping
 	var err error
@@ -409,16 +187,10 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l
 		options = append(options, libpod.WithSystemd())
 	}
 	if c.Name != "" {
-		logrus.Debugf("appending name %s", c.Name)
+		logrus.Debugf("setting container name %s", c.Name)
 		options = append(options, libpod.WithName(c.Name))
 	}
-	if c.Pod != "" || pod != nil {
+	if c.Pod != "" {
-		if pod == nil {
-			pod, err = runtime.LookupPod(c.Pod)
-			if err != nil {
-				return nil, errors.Wrapf(err, "unable to add container to pod %s", c.Pod)
-			}
-		}
 		logrus.Debugf("adding container to pod %s", c.Pod)
 		options = append(options, runtime.WithPod(pod))
 	}
@@ -429,26 +201,22 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l
 		}
 	}
 
-	if len(c.Volumes) != 0 {
+	if len(mounts) != 0 || len(namedVolumes) != 0 {
-		// Volumes consist of multiple, comma-delineated fields
+		destinations := []string{}
-		// The image spec only includes one part of that, so drop the
+
-		// others, if they are included
+		// Take all mount and named volume destinations.
-		volumes := make([]string, 0, len(c.Volumes))
+		for _, mount := range mounts {
-		for _, vol := range c.Volumes {
+			destinations = append(destinations, mount.Destination)
-			// We always want the volume destination
+		}
-			splitVol := strings.SplitN(vol, ":", 3)
+		for _, volume := range namedVolumes {
-			if len(splitVol) > 1 {
+			destinations = append(destinations, volume.Dest)
-				volumes = append(volumes, splitVol[1])
-			} else {
-				volumes = append(volumes, splitVol[0])
-			}
 		}
 
-		options = append(options, libpod.WithUserVolumes(volumes))
+		options = append(options, libpod.WithUserVolumes(destinations))
 	}
 
-	if len(c.NamedVolumes) != 0 {
+	if len(namedVolumes) != 0 {
-		options = append(options, libpod.WithNamedVolumes(c.NamedVolumes))
+		options = append(options, libpod.WithNamedVolumes(namedVolumes))
 	}
 
 	if len(c.Command) != 0 {
@@ -484,7 +252,7 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l
 			return nil, err
 		}
 	} else if c.NetMode.IsContainer() {
-		connectedCtr, err := c.Runtime.LookupContainer(c.NetMode.Container())
+		connectedCtr, err := runtime.LookupContainer(c.NetMode.Container())
 		if err != nil {
 			return nil, errors.Wrapf(err, "container %q not found", c.NetMode.Container())
 		}
@@ -495,7 +263,7 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l
 	}
 
 	if c.PidMode.IsContainer() {
-		connectedCtr, err := c.Runtime.LookupContainer(c.PidMode.Container())
+		connectedCtr, err := runtime.LookupContainer(c.PidMode.Container())
 		if err != nil {
 			return nil, errors.Wrapf(err, "container %q not found", c.PidMode.Container())
 		}
@@ -504,7 +272,7 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l
 	}
 
 	if c.IpcMode.IsContainer() {
-		connectedCtr, err := c.Runtime.LookupContainer(c.IpcMode.Container())
+		connectedCtr, err := runtime.LookupContainer(c.IpcMode.Container())
 		if err != nil {
 			return nil, errors.Wrapf(err, "container %q not found", c.IpcMode.Container())
 		}
@@ -516,7 +284,7 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l
 		options = append(options, libpod.WithUTSNSFromPod(pod))
 	}
 	if c.UtsMode.IsContainer() {
-		connectedCtr, err := c.Runtime.LookupContainer(c.UtsMode.Container())
+		connectedCtr, err := runtime.LookupContainer(c.UtsMode.Container())
 		if err != nil {
 			return nil, errors.Wrapf(err, "container %q not found", c.UtsMode.Container())
 		}
@@ -592,7 +360,7 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l
 	}
 
 	// Always use a cleanup process to clean up Podman after termination
-	exitCmd, err := c.createExitCommand()
+	exitCmd, err := c.createExitCommand(runtime)
 	if err != nil {
 		return nil, err
 	}

pkg/spec/spec.go

@@ -2,13 +2,11 @@ package createconfig
 
 import (
 	"os"
-	"path"
 	"path/filepath"
 	"strings"
 
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/rootless"
-	"github.com/containers/storage/pkg/mount"
 	pmount "github.com/containers/storage/pkg/mount"
 	"github.com/docker/docker/oci/caps"
 	"github.com/docker/go-units"
@@ -21,61 +19,6 @@ import (
 
 const cpuPeriod = 100000
 
-func supercedeUserMounts(mounts []spec.Mount, configMount []spec.Mount) []spec.Mount {
-	if len(mounts) > 0 {
-		// If we have overlappings mounts, remove them from the spec in favor of
-		// the user-added volume mounts
-		destinations := make(map[string]bool)
-		for _, mount := range mounts {
-			destinations[path.Clean(mount.Destination)] = true
-		}
-		// Copy all mounts from spec to defaultMounts, except for
-		//  - mounts overridden by a user supplied mount;
-		//  - all mounts under /dev if a user supplied /dev is present;
-		mountDev := destinations["/dev"]
-		for _, mount := range configMount {
-			if _, ok := destinations[path.Clean(mount.Destination)]; !ok {
-				if mountDev && strings.HasPrefix(mount.Destination, "/dev/") {
-					// filter out everything under /dev if /dev is user-mounted
-					continue
-				}
-
-				logrus.Debugf("Adding mount %s", mount.Destination)
-				mounts = append(mounts, mount)
-			}
-		}
-		return mounts
-	}
-	return configMount
-}
-
-// Split named volumes from normal volumes
-func splitNamedVolumes(mounts []spec.Mount) ([]spec.Mount, []*libpod.ContainerNamedVolume) {
-	newMounts := make([]spec.Mount, 0)
-	namedVolumes := make([]*libpod.ContainerNamedVolume, 0)
-	for _, mount := range mounts {
-		// If it's not a named volume, append unconditionally
-		if mount.Type != TypeBind {
-			newMounts = append(newMounts, mount)
-			continue
-		}
-		// Volumes that are not named volumes must be an absolute or
-		// relative path.
-		// Volume names may not begin with a non-alphanumeric character
-		// so the HasPrefix() check is safe here.
-		if strings.HasPrefix(mount.Source, "/") || strings.HasPrefix(mount.Source, ".") {
-			newMounts = append(newMounts, mount)
-		} else {
-			namedVolume := new(libpod.ContainerNamedVolume)
-			namedVolume.Name = mount.Source
-			namedVolume.Dest = mount.Destination
-			namedVolume.Options = mount.Options
-			namedVolumes = append(namedVolumes, namedVolume)
-		}
-	}
-	return newMounts, namedVolumes
-}
-
 func getAvailableGids() (int64, error) {
 	idMap, err := user.ParseIDMapFile("/proc/self/gid_map")
 	if err != nil {
@@ -89,7 +32,7 @@ func getAvailableGids() (int64, error) {
 }
 
 // CreateConfigToOCISpec parses information needed to create a container into an OCI runtime spec
-func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint
+func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userMounts []spec.Mount) (*spec.Spec, error) {
 	cgroupPerm := "ro"
 	g, err := generate.New("linux")
 	if err != nil {
@@ -334,56 +277,6 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint
 		addedResources = true
 	}
 
-	for _, i := range config.Tmpfs {
-		// Default options if nothing passed
-		options := []string{"rw", "rprivate", "noexec", "nosuid", "nodev", "size=65536k"}
-		spliti := strings.SplitN(i, ":", 2)
-		if len(spliti) > 1 {
-			if _, _, err := mount.ParseTmpfsOptions(spliti[1]); err != nil {
-				return nil, err
-			}
-			options = strings.Split(spliti[1], ",")
-		}
-		tmpfsMnt := spec.Mount{
-			Destination: spliti[0],
-			Type:        "tmpfs",
-			Source:      "tmpfs",
-			Options:     append(options, "tmpcopyup"),
-		}
-		g.AddMount(tmpfsMnt)
-	}
-
-	for _, m := range config.Mounts {
-		if m.Type == "tmpfs" {
-			g.AddMount(m)
-		}
-	}
-
-	if config.ReadOnlyRootfs && config.ReadOnlyTmpfs {
-		options := []string{"rw", "rprivate", "nosuid", "nodev", "tmpcopyup"}
-		for _, i := range []string{"/tmp", "/var/tmp"} {
-			if libpod.MountExists(g.Config.Mounts, i) {
-				continue
-			}
-			// Default options if nothing passed
-			tmpfsMnt := spec.Mount{
-				Destination: i,
-				Type:        "tmpfs",
-				Source:      "tmpfs",
-				Options:     options,
-			}
-			g.AddMount(tmpfsMnt)
-		}
-		if !libpod.MountExists(g.Config.Mounts, "/run") {
-			tmpfsMnt := spec.Mount{
-				Destination: "/run",
-				Type:        "tmpfs",
-				Source:      "tmpfs",
-				Options:     append(options, "noexec", "size=65536k"),
-			}
-			g.AddMount(tmpfsMnt)
-		}
-	}
 	for name, val := range config.Env {
 		g.AddProcessEnv(name, val)
 	}
@@ -439,23 +332,9 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint
 	}
 
 	// BIND MOUNTS
-	if err := config.GetVolumesFrom(); err != nil {
+	configSpec.Mounts = supercedeUserMounts(userMounts, configSpec.Mounts)
-		return nil, errors.Wrap(err, "error getting volume mounts from --volumes-from flag")
+	// Process mounts to ensure correct options
-	}
+	configSpec.Mounts = initFSMounts(configSpec.Mounts)
-
-	volumeMounts, err := config.GetVolumeMounts(configSpec.Mounts)
-	if err != nil {
-		return nil, errors.Wrapf(err, "error getting volume mounts")
-	}
-
-	configSpec.Mounts = supercedeUserMounts(volumeMounts, configSpec.Mounts)
-	//--mount
-	configSpec.Mounts = supercedeUserMounts(config.initFSMounts(), configSpec.Mounts)
-
-	// Split normal mounts and named volumes
-	newMounts, namedVolumes := splitNamedVolumes(configSpec.Mounts)
-	configSpec.Mounts = newMounts
-	config.NamedVolumes = namedVolumes
 
 	// BLOCK IO
 	blkio, err := config.CreateBlockIO()

pkg/spec/spec_test.go

@@ -1,39 +1,98 @@
 package createconfig
 
 import (
-	"reflect"
+	"runtime"
 	"testing"
 
-	spec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/containers/libpod/pkg/sysinfo"
+	"github.com/containers/storage"
+	"github.com/containers/storage/pkg/idtools"
+	"github.com/docker/go-units"
 	"github.com/stretchr/testify/assert"
 )
 
-func TestCreateConfig_GetVolumeMounts(t *testing.T) {
+var (
-	data := spec.Mount{
+	sysInfo = sysinfo.New(true)
-		Destination: "/foobar",
+)
-		Type:        "bind",
+
-		Source:      "foobar",
+// Make createconfig to test with
-		Options:     []string{"ro", "rbind", "rprivate"},
+func makeTestCreateConfig() *CreateConfig {
+	cc := new(CreateConfig)
+	cc.Resources = CreateResourceConfig{}
+	cc.IDMappings = new(storage.IDMappingOptions)
+	cc.IDMappings.UIDMap = []idtools.IDMap{}
+	cc.IDMappings.GIDMap = []idtools.IDMap{}
+
+	return cc
+}
+
+// TestPIDsLimit verifies the given 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")
 	}
-	config := CreateConfig{
+	if !sysInfo.PidsLimit {
-		Volumes: []string{"foobar:/foobar:ro"},
+		t.Skip("running test not supported by the host system")
 	}
-	specMount, err := config.GetVolumeMounts([]spec.Mount{})
+
+	cc := makeTestCreateConfig()
+	cc.Resources.PidsLimit = 22
+
+	spec, err := cc.createConfigToOCISpec(nil, nil)
 	assert.NoError(t, err)
-	assert.True(t, reflect.DeepEqual(data, specMount[0]))
+
+	assert.Equal(t, spec.Linux.Resources.Pids.Limit, int64(22))
 }
 
-func TestCreateConfig_GetTmpfsMounts(t *testing.T) {
+// TestBLKIOWeightDevice verifies the given blkio weight is correctly set in the
-	data := spec.Mount{
+// spec.
-		Destination: "/homer",
+func TestBLKIOWeightDevice(t *testing.T) {
-		Type:        "tmpfs",
+	// The default configuration of podman enables seccomp, which is not available on non-Linux systems.
-		Source:      "tmpfs",
+	// Thus, any tests that use the default seccomp setting would fail.
-		Options:     []string{"rw", "size=787448k", "mode=1777"},
+	// 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")
 	}
-	config := CreateConfig{
+	if !sysInfo.BlkioWeightDevice {
-		Tmpfs: []string{"/homer:rw,size=787448k,mode=1777"},
+		t.Skip("running test not supported by the host system")
 	}
-	tmpfsMount := config.GetTmpfsMounts()
-	assert.True(t, reflect.DeepEqual(data, tmpfsMount[0]))
 
+	cc := makeTestCreateConfig()
+	cc.Resources.BlkioWeightDevice = []string{"/dev/zero:100"}
+
+	spec, err := cc.createConfigToOCISpec(nil, nil)
+	assert.NoError(t, err)
+
+	// /dev/zero is guaranteed 1,5 by the Linux kernel
+	assert.Equal(t, spec.Linux.Resources.BlockIO.WeightDevice[0].Major, int64(1))
+	assert.Equal(t, spec.Linux.Resources.BlockIO.WeightDevice[0].Minor, int64(5))
+	assert.Equal(t, *(spec.Linux.Resources.BlockIO.WeightDevice[0].Weight), uint16(100))
+}
+
+// TestMemorySwap verifies that the given swap memory limit is correctly set 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")
+	}
+
+	swapLimit, err := units.RAMInBytes("45m")
+	assert.NoError(t, err)
+
+	cc := makeTestCreateConfig()
+	cc.Resources.MemorySwap = swapLimit
+
+	spec, err := cc.createConfigToOCISpec(nil, nil)
+	assert.NoError(t, err)
+
+	assert.Equal(t, *(spec.Linux.Resources.Memory.Swap), swapLimit)
 }

pkg/spec/storage.go

@@ -0,0 +1,792 @@
+package createconfig
+
+import (
+	"fmt"
+	"os"
+	"path"
+	"path/filepath"
+	"strings"
+
+	"github.com/containers/libpod/libpod"
+	"github.com/containers/libpod/pkg/util"
+	"github.com/containers/storage/pkg/stringid"
+	spec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+)
+
+const (
+	// TypeBind is the type for mounting host dir
+	TypeBind = "bind"
+	// TypeVolume is the type for named volumes
+	TypeVolume = "volume"
+	// TypeTmpfs is the type for mounting tmpfs
+	TypeTmpfs = "tmpfs"
+)
+
+var (
+	errDuplicateDest = errors.Errorf("duplicate mount destination")
+	optionArgError   = errors.Errorf("must provide an argument for option")
+	noDestError      = errors.Errorf("must set volume destination")
+)
+
+// Parse all volume-related options in the create config into a set of mounts
+// and named volumes to add to the container.
+// Handles --volumes-from, --volumes, --tmpfs, --init, and --init-path flags.
+// TODO: Named volume options -  should we default to rprivate? It bakes into a
+// bind mount under the hood...
+// TODO: handle options parsing/processing via containers/storage/pkg/mount
+func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount, []*libpod.ContainerNamedVolume, error) {
+	// Add image volumes.
+	baseMounts, baseVolumes, err := config.getImageVolumes()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	// Add --volumes-from.
+	// Overrides image volumes unconditionally.
+	vFromMounts, vFromVolumes, err := config.getVolumesFrom(runtime)
+	if err != nil {
+		return nil, nil, err
+	}
+	for dest, mount := range vFromMounts {
+		baseMounts[dest] = mount
+	}
+	for dest, volume := range vFromVolumes {
+		baseVolumes[dest] = volume
+	}
+
+	// Next mounts from the --mounts flag.
+	// Do not override yet.
+	unifiedMounts, unifiedVolumes, err := config.getMounts()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	// Next --volumes flag.
+	// Do not override yet.
+	volumeMounts, volumeVolumes, err := config.getVolumeMounts()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	// Next --tmpfs flag.
+	// Do not override yet.
+	tmpfsMounts, err := config.getTmpfsMounts()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	// Unify mounts from --mount, --volume, --tmpfs.
+	// Also add mounts + volumes directly from createconfig.
+	// Start with --volume.
+	for dest, mount := range volumeMounts {
+		if _, ok := unifiedMounts[dest]; ok {
+			return nil, nil, errors.Wrapf(errDuplicateDest, dest)
+		}
+		unifiedMounts[dest] = mount
+	}
+	for dest, volume := range volumeVolumes {
+		if _, ok := unifiedVolumes[dest]; ok {
+			return nil, nil, errors.Wrapf(errDuplicateDest, dest)
+		}
+		unifiedVolumes[dest] = volume
+	}
+	// Now --tmpfs
+	for dest, tmpfs := range tmpfsMounts {
+		if _, ok := unifiedMounts[dest]; ok {
+			return nil, nil, errors.Wrapf(errDuplicateDest, dest)
+		}
+		unifiedMounts[dest] = tmpfs
+	}
+	// Now spec mounts and volumes
+	for _, mount := range config.Mounts {
+		dest := mount.Destination
+		if _, ok := unifiedMounts[dest]; ok {
+			return nil, nil, errors.Wrapf(errDuplicateDest, dest)
+		}
+		unifiedMounts[dest] = mount
+	}
+	for _, volume := range config.NamedVolumes {
+		dest := volume.Dest
+		if _, ok := unifiedVolumes[dest]; ok {
+			return nil, nil, errors.Wrapf(errDuplicateDest, dest)
+		}
+		unifiedVolumes[dest] = volume
+	}
+
+	// If requested, add container init binary
+	if config.Init {
+		initPath := config.InitPath
+		if initPath == "" {
+			rtc, err := runtime.GetConfig()
+			if err != nil {
+				return nil, nil, err
+			}
+			initPath = rtc.InitPath
+		}
+		initMount, err := config.addContainerInitBinary(initPath)
+		if err != nil {
+			return nil, nil, err
+		}
+		if _, ok := unifiedMounts[initMount.Destination]; ok {
+			return nil, nil, errors.Wrapf(errDuplicateDest, "conflict with mount added by --init to %q", initMount.Destination)
+		}
+		unifiedMounts[initMount.Destination] = initMount
+	}
+
+	// If requested, add tmpfs filesystems for read-only containers.
+	// Need to keep track of which we created, so we don't modify options
+	// for them later...
+	readonlyTmpfs := map[string]bool{
+		"/tmp":     false,
+		"/var/tmp": false,
+		"/run":     false,
+	}
+	if config.ReadOnlyRootfs && config.ReadOnlyTmpfs {
+		options := []string{"rw", "rprivate", "nosuid", "nodev", "tmpcopyup", "size=65536k"}
+		for dest := range readonlyTmpfs {
+			if _, ok := unifiedMounts[dest]; ok {
+				continue
+			}
+			localOpts := options
+			if dest == "/run" {
+				localOpts = append(localOpts, "noexec")
+			}
+			unifiedMounts[dest] = spec.Mount{
+				Destination: dest,
+				Type:        "tmpfs",
+				Source:      "tmpfs",
+				Options:     localOpts,
+			}
+			readonlyTmpfs[dest] = true
+		}
+	}
+
+	// Supercede volumes-from/image volumes with unified volumes from above.
+	// This is an unconditional replacement.
+	for dest, mount := range unifiedMounts {
+		baseMounts[dest] = mount
+	}
+	for dest, volume := range unifiedVolumes {
+		baseVolumes[dest] = volume
+	}
+
+	// Check for conflicts between named volumes and mounts
+	for dest := range baseMounts {
+		if _, ok := baseVolumes[dest]; ok {
+			return nil, nil, errors.Wrapf(errDuplicateDest, "conflict at mount destination %v", dest)
+		}
+	}
+	for dest := range baseVolumes {
+		if _, ok := baseMounts[dest]; ok {
+			return nil, nil, errors.Wrapf(errDuplicateDest, "conflict at mount destination %v", dest)
+		}
+	}
+
+	// Final step: maps to arrays
+	finalMounts := make([]spec.Mount, 0, len(baseMounts))
+	for _, mount := range baseMounts {
+		// All user-added tmpfs mounts need their options processed.
+		// Exception: mounts added by the ReadOnlyTmpfs option, which
+		// contain several exceptions to normal options rules.
+		if mount.Type == TypeTmpfs && !readonlyTmpfs[mount.Destination] {
+			opts, err := util.ProcessTmpfsOptions(mount.Options)
+			if err != nil {
+				return nil, nil, err
+			}
+			mount.Options = opts
+		}
+		finalMounts = append(finalMounts, mount)
+	}
+	finalVolumes := make([]*libpod.ContainerNamedVolume, 0, len(baseVolumes))
+	for _, volume := range baseVolumes {
+		finalVolumes = append(finalVolumes, volume)
+	}
+
+	logrus.Debugf("Got mounts: %v", finalMounts)
+	logrus.Debugf("Got volumes: %v", finalVolumes)
+
+	return finalMounts, finalVolumes, nil
+}
+
+// Parse volumes from - a set of containers whose volumes we will mount in.
+// Grab the containers, retrieve any user-created spec mounts and all named
+// volumes, and return a list of them.
+// Conflicts are resolved simply - the last container specified wins.
+// Container names may be suffixed by mount options after a colon.
+func (config *CreateConfig) getVolumesFrom(runtime *libpod.Runtime) (map[string]spec.Mount, map[string]*libpod.ContainerNamedVolume, error) {
+	// TODO: This can probably be disabled now
+	if os.Geteuid() != 0 {
+		return nil, nil, nil
+	}
+
+	// Both of these are maps of mount destination to mount type.
+	// We ensure that each destination is only mounted to once in this way.
+	finalMounts := make(map[string]spec.Mount)
+	finalNamedVolumes := make(map[string]*libpod.ContainerNamedVolume)
+
+	for _, vol := range config.VolumesFrom {
+		options := []string{}
+		splitVol := strings.SplitN(vol, ":", 2)
+		if len(splitVol) == 2 {
+			if strings.Contains(splitVol[1], "Z") ||
+				strings.Contains(splitVol[1], "private") ||
+				strings.Contains(splitVol[1], "slave") ||
+				strings.Contains(splitVol[1], "shared") {
+				return nil, nil, errors.Errorf("invalid options %q, can only specify 'ro', 'rw', and 'z", splitVol[1])
+			}
+			options = strings.Split(splitVol[1], ",")
+			if err := ValidateVolumeOpts(options); err != nil {
+				return nil, nil, err
+			}
+		}
+		ctr, err := runtime.LookupContainer(splitVol[0])
+		if err != nil {
+			return nil, nil, errors.Wrapf(err, "error looking up container %q for volumes-from", splitVol[0])
+		}
+
+		logrus.Debugf("Adding volumes from container %s", ctr.ID())
+
+		// Look up the container's user volumes. This gets us the
+		// destinations of all mounts the user added to the container.
+		userVolumesArr := ctr.UserVolumes()
+
+		// We're going to need to access them a lot, so convert to a map
+		// to reduce looping.
+		// We'll also use the map to indicate if we missed any volumes along the way.
+		userVolumes := make(map[string]bool)
+		for _, dest := range userVolumesArr {
+			userVolumes[dest] = false
+		}
+
+		// Now we get the container's spec and loop through its volumes
+		// and append them in if we can find them.
+		spec := ctr.Spec()
+		if spec == nil {
+			return nil, nil, errors.Errorf("error retrieving container %s spec for volumes-from", ctr.ID())
+		}
+		for _, mnt := range spec.Mounts {
+			if mnt.Type != TypeBind {
+				continue
+			}
+			if _, exists := userVolumes[mnt.Destination]; exists {
+				userVolumes[mnt.Destination] = true
+
+				if len(options) != 0 {
+					mnt.Options = options
+				}
+
+				if _, ok := finalMounts[mnt.Destination]; ok {
+					logrus.Debugf("Overriding mount to %s with new mount from container %s", mnt.Destination, ctr.ID())
+				}
+				finalMounts[mnt.Destination] = mnt
+			}
+		}
+
+		// We're done with the spec mounts. Add named volumes.
+		// Add these unconditionally - none of them are automatically
+		// part of the container, as some spec mounts are.
+		namedVolumes := ctr.NamedVolumes()
+		for _, namedVol := range namedVolumes {
+			if _, exists := userVolumes[namedVol.Dest]; exists {
+				userVolumes[namedVol.Dest] = true
+			}
+
+			if len(options) != 0 {
+				namedVol.Options = options
+			}
+
+			if _, ok := finalMounts[namedVol.Dest]; ok {
+				logrus.Debugf("Overriding named volume mount to %s with new named volume from container %s", namedVol.Dest, ctr.ID())
+			}
+			finalNamedVolumes[namedVol.Dest] = namedVol
+		}
+
+		// Check if we missed any volumes
+		for volDest, found := range userVolumes {
+			if !found {
+				logrus.Warnf("Unable to match volume %s from container %s for volumes-from", volDest, ctr.ID())
+			}
+		}
+	}
+
+	return finalMounts, finalNamedVolumes, nil
+}
+
+// getMounts takes user-provided input from the --mount flag and creates OCI
+// spec mounts and Libpod named volumes.
+// podman run --mount type=bind,src=/etc/resolv.conf,target=/etc/resolv.conf ...
+// podman run --mount type=tmpfs,target=/dev/shm ...
+// podman run --mount type=volume,source=test-volume, ...
+func (config *CreateConfig) getMounts() (map[string]spec.Mount, map[string]*libpod.ContainerNamedVolume, error) {
+	finalMounts := make(map[string]spec.Mount)
+	finalNamedVolumes := make(map[string]*libpod.ContainerNamedVolume)
+
+	errInvalidSyntax := errors.Errorf("incorrect mount format: should be --mount type=<bind|tmpfs|volume>,[src=<host-dir|volume-name>,]target=<ctr-dir>[,options]")
+
+	// 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.
+	for _, mount := range config.MountsFlag {
+		arr := strings.SplitN(mount, ",", 2)
+		if len(arr) < 2 {
+			return nil, nil, errors.Wrapf(errInvalidSyntax, "%q", mount)
+		}
+		kv := strings.Split(arr[0], "=")
+		// TODO: type is not explicitly required in Docker.
+		// If not specified, it defaults to "volume".
+		if len(kv) != 2 || kv[0] != "type" {
+			return nil, nil, errors.Wrapf(errInvalidSyntax, "%q", mount)
+		}
+
+		tokens := strings.Split(arr[1], ",")
+		switch kv[1] {
+		case TypeBind:
+			mount, err := getBindMount(tokens)
+			if err != nil {
+				return nil, nil, err
+			}
+			if _, ok := finalMounts[mount.Destination]; ok {
+				return nil, nil, errors.Wrapf(errDuplicateDest, mount.Destination)
+			}
+			finalMounts[mount.Destination] = mount
+		case TypeTmpfs:
+			mount, err := getTmpfsMount(tokens)
+			if err != nil {
+				return nil, nil, err
+			}
+			if _, ok := finalMounts[mount.Destination]; ok {
+				return nil, nil, errors.Wrapf(errDuplicateDest, mount.Destination)
+			}
+			finalMounts[mount.Destination] = mount
+		case "volume":
+			volume, err := getNamedVolume(tokens)
+			if err != nil {
+				return nil, nil, err
+			}
+			if _, ok := finalNamedVolumes[volume.Dest]; ok {
+				return nil, nil, errors.Wrapf(errDuplicateDest, volume.Dest)
+			}
+			finalNamedVolumes[volume.Dest] = volume
+		default:
+			return nil, nil, errors.Errorf("invalid fylesystem type %q", kv[1])
+		}
+	}
+
+	return finalMounts, finalNamedVolumes, nil
+}
+
+// Parse a single bind mount entry from the --mount flag.
+func getBindMount(args []string) (spec.Mount, error) {
+	newMount := spec.Mount{
+		Type: TypeBind,
+	}
+
+	setSource := false
+	setDest := false
+
+	for _, val := range args {
+		kv := strings.Split(val, "=")
+		switch kv[0] {
+		case "ro", "nosuid", "nodev", "noexec":
+			// TODO: detect duplication of these options.
+			// (Is this necessary?)
+			newMount.Options = append(newMount.Options, kv[0])
+		case "shared", "rshared", "private", "rprivate", "slave", "rslave", "Z", "z":
+			newMount.Options = append(newMount.Options, kv[0])
+		case "bind-propagation":
+			if len(kv) == 1 {
+				return newMount, errors.Wrapf(optionArgError, kv[0])
+			}
+			newMount.Options = append(newMount.Options, kv[1])
+		case "src", "source":
+			if len(kv) == 1 {
+				return newMount, errors.Wrapf(optionArgError, kv[0])
+			}
+			if err := ValidateVolumeHostDir(kv[1]); err != nil {
+				return newMount, err
+			}
+			newMount.Source = kv[1]
+			setSource = true
+		case "target", "dst", "destination":
+			if len(kv) == 1 {
+				return newMount, errors.Wrapf(optionArgError, kv[0])
+			}
+			if err := ValidateVolumeCtrDir(kv[1]); err != nil {
+				return newMount, err
+			}
+			newMount.Destination = kv[1]
+			setDest = true
+		default:
+			return newMount, errors.Wrapf(util.ErrBadMntOption, kv[0])
+		}
+	}
+
+	if !setDest {
+		return newMount, noDestError
+	}
+
+	if !setSource {
+		newMount.Source = newMount.Destination
+	}
+
+	if err := ValidateVolumeOpts(newMount.Options); err != nil {
+		return newMount, err
+	}
+
+	return newMount, nil
+}
+
+// Parse a single tmpfs mount entry from the --mount flag
+func getTmpfsMount(args []string) (spec.Mount, error) {
+	newMount := spec.Mount{
+		Type:   TypeTmpfs,
+		Source: TypeTmpfs,
+	}
+
+	setDest := false
+
+	for _, val := range args {
+		kv := strings.Split(val, "=")
+		switch kv[0] {
+		case "ro", "nosuid", "nodev", "noexec":
+			newMount.Options = append(newMount.Options, kv[0])
+		case "tmpfs-mode":
+			if len(kv) == 1 {
+				return newMount, errors.Wrapf(optionArgError, kv[0])
+			}
+			newMount.Options = append(newMount.Options, fmt.Sprintf("mode=%s", kv[1]))
+		case "tmpfs-size":
+			if len(kv) == 1 {
+				return newMount, errors.Wrapf(optionArgError, kv[0])
+			}
+			newMount.Options = append(newMount.Options, fmt.Sprintf("size=%s", kv[1]))
+		case "src", "source":
+			return newMount, errors.Errorf("source is not supported with tmpfs mounts")
+		case "target", "dst", "destination":
+			if len(kv) == 1 {
+				return newMount, errors.Wrapf(optionArgError, kv[0])
+			}
+			if err := ValidateVolumeCtrDir(kv[1]); err != nil {
+				return newMount, err
+			}
+			newMount.Destination = kv[1]
+			setDest = true
+		default:
+			return newMount, errors.Wrapf(util.ErrBadMntOption, kv[0])
+		}
+	}
+
+	if !setDest {
+		return newMount, noDestError
+	}
+
+	return newMount, nil
+}
+
+// Parse a single volume mount entry from the --mount flag.
+// Note that the volume-label option for named volumes is currently NOT supported.
+// TODO: add support for --volume-label
+func getNamedVolume(args []string) (*libpod.ContainerNamedVolume, error) {
+	newVolume := new(libpod.ContainerNamedVolume)
+
+	setSource := false
+	setDest := false
+
+	for _, val := range args {
+		kv := strings.Split(val, "=")
+		switch kv[0] {
+		case "ro", "nosuid", "nodev", "noexec":
+			// TODO: detect duplication of these options
+			newVolume.Options = append(newVolume.Options, kv[0])
+		case "volume-label":
+			return nil, errors.Errorf("the --volume-label option is not presently implemented")
+		case "src", "source":
+			if len(kv) == 1 {
+				return nil, errors.Wrapf(optionArgError, kv[0])
+			}
+			newVolume.Name = kv[1]
+			setSource = true
+		case "target", "dst", "destination":
+			if len(kv) == 1 {
+				return nil, errors.Wrapf(optionArgError, kv[0])
+			}
+			if err := ValidateVolumeCtrDir(kv[1]); err != nil {
+				return nil, err
+			}
+			newVolume.Dest = kv[1]
+			setDest = true
+		default:
+			return nil, errors.Wrapf(util.ErrBadMntOption, kv[0])
+		}
+	}
+
+	if !setSource {
+		return nil, errors.Errorf("must set source volume")
+	}
+	if !setDest {
+		return nil, noDestError
+	}
+
+	return newVolume, nil
+}
+
+// ValidateVolumeHostDir validates a volume mount's source directory
+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 validates a volume mount's destination directory.
+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 %q, must be an absolute path", ctrDir)
+	}
+	return nil
+}
+
+// ValidateVolumeOpts validates a volume's options
+func ValidateVolumeOpts(options []string) error {
+	var foundRootPropagation, foundRWRO, foundLabelChange int
+	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", strings.Join(options, ", "))
+			}
+		case "z", "Z":
+			foundLabelChange++
+			if foundLabelChange > 1 {
+				return errors.Errorf("invalid options %q, can only specify 1 'z' or 'Z' option", strings.Join(options, ", "))
+			}
+		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", strings.Join(options, ", "))
+			}
+		default:
+			return errors.Errorf("invalid option type %q", opt)
+		}
+	}
+	return nil
+}
+
+// GetVolumeMounts takes user provided input for bind mounts and creates Mount structs
+func (config *CreateConfig) getVolumeMounts() (map[string]spec.Mount, map[string]*libpod.ContainerNamedVolume, error) {
+	mounts := make(map[string]spec.Mount)
+	volumes := make(map[string]*libpod.ContainerNamedVolume)
+
+	volumeFormatErr := errors.Errorf("incorrect volume format, should be host-dir:ctr-dir[:option]")
+
+	for _, vol := range config.Volumes {
+		var (
+			options []string
+			src     string
+			dest    string
+		)
+
+		splitVol := strings.Split(vol, ":")
+		if len(splitVol) > 3 {
+			return nil, nil, errors.Wrapf(volumeFormatErr, vol)
+		}
+
+		src = splitVol[0]
+		if len(splitVol) == 1 {
+			dest = src
+		} else if len(splitVol) > 1 {
+			dest = splitVol[1]
+		}
+		if len(splitVol) > 2 {
+			options = strings.Split(splitVol[2], ",")
+			if err := ValidateVolumeOpts(options); err != nil {
+				return nil, nil, err
+			}
+		}
+
+		if err := ValidateVolumeHostDir(src); err != nil {
+			return nil, nil, err
+		}
+		if err := ValidateVolumeCtrDir(dest); err != nil {
+			return nil, nil, err
+		}
+
+		if strings.HasPrefix(src, "/") || strings.HasPrefix(src, ".") {
+			// This is not a named volume
+			newMount := spec.Mount{
+				Destination: dest,
+				Type:        string(TypeBind),
+				Source:      src,
+				Options:     options,
+			}
+			if _, ok := mounts[newMount.Destination]; ok {
+				return nil, nil, errors.Wrapf(errDuplicateDest, newMount.Destination)
+			}
+			mounts[newMount.Destination] = newMount
+		} else {
+			// This is a named volume
+			newNamedVol := new(libpod.ContainerNamedVolume)
+			newNamedVol.Name = src
+			newNamedVol.Dest = dest
+			newNamedVol.Options = options
+
+			if _, ok := volumes[newNamedVol.Dest]; ok {
+				return nil, nil, errors.Wrapf(errDuplicateDest, newNamedVol.Dest)
+			}
+			volumes[newNamedVol.Dest] = newNamedVol
+		}
+
+		logrus.Debugf("User mount %s:%s options %v", src, dest, options)
+	}
+
+	return mounts, volumes, nil
+}
+
+// Get mounts for container's image volumes
+func (config *CreateConfig) getImageVolumes() (map[string]spec.Mount, map[string]*libpod.ContainerNamedVolume, error) {
+	mounts := make(map[string]spec.Mount)
+	volumes := make(map[string]*libpod.ContainerNamedVolume)
+
+	if config.ImageVolumeType == "ignore" {
+		return mounts, volumes, nil
+	}
+
+	for vol := range config.BuiltinImgVolumes {
+		if config.ImageVolumeType == "tmpfs" {
+			// Tmpfs image volumes are handled as mounts
+			mount := spec.Mount{
+				Destination: vol,
+				Source:      TypeTmpfs,
+				Type:        TypeTmpfs,
+				Options:     []string{"rprivate", "rw", "nodev"},
+			}
+			mounts[vol] = mount
+		} else {
+			namedVolume := new(libpod.ContainerNamedVolume)
+			namedVolume.Name = stringid.GenerateNonCryptoID()
+			namedVolume.Options = []string{"rprivate", "rw", "nodev"}
+			namedVolume.Dest = vol
+			volumes[vol] = namedVolume
+		}
+	}
+
+	return mounts, volumes, nil
+}
+
+// GetTmpfsMounts creates spec.Mount structs for user-requested tmpfs mounts
+func (config *CreateConfig) getTmpfsMounts() (map[string]spec.Mount, error) {
+	m := make(map[string]spec.Mount)
+	for _, i := range config.Tmpfs {
+		// Default options if nothing passed
+		var options []string
+		spliti := strings.Split(i, ":")
+		destPath := spliti[0]
+		if len(spliti) > 1 {
+			options = strings.Split(spliti[1], ",")
+		}
+
+		if _, ok := m[destPath]; ok {
+			return nil, errors.Wrapf(errDuplicateDest, destPath)
+		}
+
+		mount := spec.Mount{
+			Destination: destPath,
+			Type:        string(TypeTmpfs),
+			Options:     options,
+			Source:      string(TypeTmpfs),
+		}
+		m[destPath] = mount
+	}
+	return m, nil
+}
+
+// AddContainerInitBinary adds the init binary specified by path iff the
+// container will run in a private PID namespace that is not shared with the
+// host or another pre-existing container, where an init-like process is
+// already running.
+//
+// Note that AddContainerInitBinary prepends "/dev/init" "--" to the command
+// to execute the bind-mounted binary as PID 1.
+func (config *CreateConfig) addContainerInitBinary(path string) (spec.Mount, error) {
+	mount := spec.Mount{
+		Destination: "/dev/init",
+		Type:        TypeBind,
+		Source:      path,
+		Options:     []string{TypeBind, "ro"},
+	}
+
+	if path == "" {
+		return mount, fmt.Errorf("please specify a path to the container-init binary")
+	}
+	if !config.PidMode.IsPrivate() {
+		return mount, fmt.Errorf("cannot add init binary as PID 1 (PID namespace isn't private)")
+	}
+	if config.Systemd {
+		return mount, fmt.Errorf("cannot use container-init binary with systemd")
+	}
+	if _, err := os.Stat(path); os.IsNotExist(err) {
+		return mount, errors.Wrap(err, "container-init binary not found on the host")
+	}
+	config.Command = append([]string{"/dev/init", "--"}, config.Command...)
+	return mount, nil
+}
+
+// Supercede existing mounts in the spec with new, user-specified mounts.
+// TODO: Should we unmount subtree mounts? E.g., if /tmp/ is mounted by
+// one mount, and we already have /tmp/a and /tmp/b, should we remove
+// the /tmp/a and /tmp/b mounts in favor of the more general /tmp?
+func supercedeUserMounts(mounts []spec.Mount, configMount []spec.Mount) []spec.Mount {
+	if len(mounts) > 0 {
+		// If we have overlappings mounts, remove them from the spec in favor of
+		// the user-added volume mounts
+		destinations := make(map[string]bool)
+		for _, mount := range mounts {
+			destinations[path.Clean(mount.Destination)] = true
+		}
+		// Copy all mounts from spec to defaultMounts, except for
+		//  - mounts overridden by a user supplied mount;
+		//  - all mounts under /dev if a user supplied /dev is present;
+		mountDev := destinations["/dev"]
+		for _, mount := range configMount {
+			if _, ok := destinations[path.Clean(mount.Destination)]; !ok {
+				if mountDev && strings.HasPrefix(mount.Destination, "/dev/") {
+					// filter out everything under /dev if /dev is user-mounted
+					continue
+				}
+
+				logrus.Debugf("Adding mount %s", mount.Destination)
+				mounts = append(mounts, mount)
+			}
+		}
+		return mounts
+	}
+	return configMount
+}
+
+// Ensure mount options on all mounts are correct
+func initFSMounts(inputMounts []spec.Mount) []spec.Mount {
+	var mounts []spec.Mount
+	for _, m := range inputMounts {
+		if m.Type == TypeBind {
+			m.Options = util.ProcessOptions(m.Options)
+		}
+		if m.Type == TypeTmpfs {
+			m.Options = append(m.Options, "tmpcopyup")
+		}
+		mounts = append(mounts, m)
+	}
+	return mounts
+}

pkg/spec/storage_test.go

@@ -0,0 +1,38 @@
+package createconfig
+
+import (
+	"testing"
+
+	spec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetVolumeMountsOneVolume(t *testing.T) {
+	data := spec.Mount{
+		Destination: "/foobar",
+		Type:        "bind",
+		Source:      "/tmp",
+		Options:     []string{"ro"},
+	}
+	config := CreateConfig{
+		Volumes: []string{"/tmp:/foobar:ro"},
+	}
+	specMount, _, err := config.getVolumeMounts()
+	assert.NoError(t, err)
+	assert.EqualValues(t, data, specMount[data.Destination])
+}
+
+func TestGetTmpfsMounts(t *testing.T) {
+	data := spec.Mount{
+		Destination: "/homer",
+		Type:        "tmpfs",
+		Source:      "tmpfs",
+		Options:     []string{"rw", "size=787448k", "mode=1777"},
+	}
+	config := CreateConfig{
+		Tmpfs: []string{"/homer:rw,size=787448k,mode=1777"},
+	}
+	tmpfsMount, err := config.getTmpfsMounts()
+	assert.NoError(t, err)
+	assert.EqualValues(t, data, tmpfsMount[data.Destination])
+}

pkg/util/mountOpts.go

@@ -0,0 +1,98 @@
+package util
+
+import (
+	"strings"
+
+	"github.com/pkg/errors"
+)
+
+var (
+	// ErrBadMntOption indicates that an invalid mount option was passed.
+	ErrBadMntOption = errors.Errorf("invalid mount option")
+	// ErrDupeMntOption indicates that a duplicate mount option was passed.
+	ErrDupeMntOption = errors.Errorf("duplicate option passed")
+)
+
+// ProcessOptions parses the options for a bind mount and ensures that they are
+// sensible and follow convention.
+func ProcessOptions(options []string) []string {
+	var (
+		foundrw, foundro bool
+		rootProp         string
+	)
+	options = append(options, "rbind")
+	for _, opt := range options {
+		switch opt {
+		case "rw":
+			foundrw = true
+		case "ro":
+			foundro = true
+		case "private", "rprivate", "slave", "rslave", "shared", "rshared":
+			rootProp = opt
+		}
+	}
+	if !foundrw && !foundro {
+		options = append(options, "rw")
+	}
+	if rootProp == "" {
+		options = append(options, "rprivate")
+	}
+	return options
+}
+
+// ProcessTmpfsOptions parses the options for a tmpfs mountpoint and ensures
+// that they are sensible and follow convention.
+func ProcessTmpfsOptions(options []string) ([]string, error) {
+	var (
+		foundWrite, foundSize, foundProp, foundMode bool
+	)
+
+	baseOpts := []string{"noexec", "nosuid", "nodev"}
+	for _, opt := range options {
+		// Some options have parameters - size, mode
+		splitOpt := strings.SplitN(opt, "=", 2)
+		switch splitOpt[0] {
+		case "rw", "ro":
+			if foundWrite {
+				return nil, errors.Wrapf(ErrDupeMntOption, "only one of rw and ro can be used")
+			}
+			foundWrite = true
+			baseOpts = append(baseOpts, opt)
+		case "private", "rprivate", "slave", "rslave", "shared", "rshared":
+			if foundProp {
+				return nil, errors.Wrapf(ErrDupeMntOption, "only one root propagation mode can be used")
+			}
+			foundProp = true
+			baseOpts = append(baseOpts, opt)
+		case "size":
+			if foundSize {
+				return nil, errors.Wrapf(ErrDupeMntOption, "only one tmpfs size can be specified")
+			}
+			foundSize = true
+			baseOpts = append(baseOpts, opt)
+		case "mode":
+			if foundMode {
+				return nil, errors.Wrapf(ErrDupeMntOption, "only one tmpfs mode can be specified")
+			}
+			foundMode = true
+			baseOpts = append(baseOpts, opt)
+		case "noexec", "nodev", "nosuid":
+			// Do nothing. We always include these even if they are
+			// not explicitly requested.
+		default:
+			return nil, errors.Wrapf(ErrBadMntOption, "unknown tmpfs option %q", opt)
+		}
+	}
+
+	if !foundWrite {
+		baseOpts = append(baseOpts, "rw")
+	}
+	if !foundSize {
+		baseOpts = append(baseOpts, "size=65536k")
+	}
+	if !foundProp {
+		baseOpts = append(baseOpts, "rprivate")
+	}
+
+	return baseOpts, nil
+}

test/e2e/run_test.go

@@ -212,63 +212,6 @@ var _ = Describe("Podman run", func() {
 		Expect(session.OutputToString()).To(ContainSubstring("100"))
 	})
 
-	It("podman run with volume flag", func() {
-		SkipIfRootless()
-		Skip("Skip until we diagnose the regression of volume mounts")
-		mountPath := filepath.Join(podmanTest.TempDir, "secrets")
-		os.Mkdir(mountPath, 0755)
-		session := podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/run/test", mountPath), ALPINE, "cat", "/proc/self/mountinfo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		Expect(session.OutputToString()).To(ContainSubstring("/run/test rw,relatime"))
-
-		mountPath = filepath.Join(podmanTest.TempDir, "secrets")
-		os.Mkdir(mountPath, 0755)
-		session = podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/run/test:ro", mountPath), ALPINE, "cat", "/proc/self/mountinfo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		Expect(session.OutputToString()).To(ContainSubstring("/run/test ro,relatime"))
-
-		mountPath = filepath.Join(podmanTest.TempDir, "secrets")
-		os.Mkdir(mountPath, 0755)
-		session = podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/run/test:shared", mountPath), ALPINE, "cat", "/proc/self/mountinfo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		Expect(session.OutputToString()).To(ContainSubstring("/run/test rw,relatime, shared"))
-	})
-
-	It("podman run with --mount flag", func() {
-		if podmanTest.Host.Arch == "ppc64le" {
-			Skip("skip failing test on ppc64le")
-		}
-		mountPath := filepath.Join(podmanTest.TempDir, "secrets")
-		os.Mkdir(mountPath, 0755)
-		session := podmanTest.Podman([]string{"run", "--rm", "--mount", fmt.Sprintf("type=bind,src=%s,target=/run/test", mountPath), ALPINE, "grep", "/run/test", "/proc/self/mountinfo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		Expect(session.OutputToString()).To(ContainSubstring("/run/test rw"))
-
-		session = podmanTest.Podman([]string{"run", "--rm", "--mount", fmt.Sprintf("type=bind,src=%s,target=/run/test,ro", mountPath), ALPINE, "grep", "/run/test", "/proc/self/mountinfo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		Expect(session.OutputToString()).To(ContainSubstring("/run/test ro"))
-
-		session = podmanTest.Podman([]string{"run", "--rm", "--mount", fmt.Sprintf("type=bind,src=%s,target=/run/test,shared", mountPath), ALPINE, "grep", "/run/test", "/proc/self/mountinfo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		found, matches := session.GrepString("/run/test")
-		Expect(found).Should(BeTrue())
-		Expect(matches[0]).To(ContainSubstring("rw"))
-		Expect(matches[0]).To(ContainSubstring("shared"))
-
-		mountPath = filepath.Join(podmanTest.TempDir, "scratchpad")
-		os.Mkdir(mountPath, 0755)
-		session = podmanTest.Podman([]string{"run", "--rm", "--mount", "type=tmpfs,target=/run/test", ALPINE, "grep", "/run/test", "/proc/self/mountinfo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		Expect(session.OutputToString()).To(ContainSubstring("/run/test rw,nosuid,nodev,noexec,relatime - tmpfs"))
-	})
-
 	It("podman run with cidfile", func() {
 		session := podmanTest.Podman([]string{"run", "--cidfile", tempdir + "cidfile", ALPINE, "ls"})
 		session.WaitWithDefaultTimeout()

test/e2e/run_volume_test.go

@@ -0,0 +1,107 @@
+// +build !remoteclient
+
+package integration
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+
+	. "github.com/containers/libpod/test/utils"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Podman run with volumes", func() {
+	var (
+		tempdir    string
+		err        error
+		podmanTest *PodmanTestIntegration
+	)
+
+	BeforeEach(func() {
+		tempdir, err = CreateTempDirInTempDir()
+		if err != nil {
+			os.Exit(1)
+		}
+		podmanTest = PodmanTestCreate(tempdir)
+		podmanTest.Setup()
+		podmanTest.RestoreAllArtifacts()
+	})
+
+	AfterEach(func() {
+		podmanTest.Cleanup()
+		f := CurrentGinkgoTestDescription()
+		processTestResult(f)
+	})
+
+	It("podman run with volume flag", func() {
+		mountPath := filepath.Join(podmanTest.TempDir, "secrets")
+		os.Mkdir(mountPath, 0755)
+		session := podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/run/test", mountPath), ALPINE, "grep", "/run/test", "/proc/self/mountinfo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		found, matches := session.GrepString("/run/test")
+		Expect(found).Should(BeTrue())
+		Expect(matches[0]).To(ContainSubstring("rw"))
+
+		mountPath = filepath.Join(podmanTest.TempDir, "secrets")
+		os.Mkdir(mountPath, 0755)
+		session = podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/run/test:ro", mountPath), ALPINE, "grep", "/run/test", "/proc/self/mountinfo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		found, matches = session.GrepString("/run/test")
+		Expect(found).Should(BeTrue())
+		Expect(matches[0]).To(ContainSubstring("ro"))
+
+		mountPath = filepath.Join(podmanTest.TempDir, "secrets")
+		os.Mkdir(mountPath, 0755)
+		session = podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/run/test:shared", mountPath), ALPINE, "grep", "/run/test", "/proc/self/mountinfo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		found, matches = session.GrepString("/run/test")
+		Expect(found).Should(BeTrue())
+		Expect(matches[0]).To(ContainSubstring("rw"))
+		Expect(matches[0]).To(ContainSubstring("shared"))
+	})
+
+	It("podman run with --mount flag", func() {
+		if podmanTest.Host.Arch == "ppc64le" {
+			Skip("skip failing test on ppc64le")
+		}
+		mountPath := filepath.Join(podmanTest.TempDir, "secrets")
+		os.Mkdir(mountPath, 0755)
+		session := podmanTest.Podman([]string{"run", "--rm", "--mount", fmt.Sprintf("type=bind,src=%s,target=/run/test", mountPath), ALPINE, "grep", "/run/test", "/proc/self/mountinfo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(ContainSubstring("/run/test rw"))
+
+		session = podmanTest.Podman([]string{"run", "--rm", "--mount", fmt.Sprintf("type=bind,src=%s,target=/run/test,ro", mountPath), ALPINE, "grep", "/run/test", "/proc/self/mountinfo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(ContainSubstring("/run/test ro"))
+
+		session = podmanTest.Podman([]string{"run", "--rm", "--mount", fmt.Sprintf("type=bind,src=%s,target=/run/test,shared", mountPath), ALPINE, "grep", "/run/test", "/proc/self/mountinfo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		found, matches := session.GrepString("/run/test")
+		Expect(found).Should(BeTrue())
+		Expect(matches[0]).To(ContainSubstring("rw"))
+		Expect(matches[0]).To(ContainSubstring("shared"))
+
+		mountPath = filepath.Join(podmanTest.TempDir, "scratchpad")
+		os.Mkdir(mountPath, 0755)
+		session = podmanTest.Podman([]string{"run", "--rm", "--mount", "type=tmpfs,target=/run/test", ALPINE, "grep", "/run/test", "/proc/self/mountinfo"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(ContainSubstring("/run/test rw,nosuid,nodev,noexec,relatime - tmpfs"))
+	})
+
+	It("podman run with conflicting volumes errors", func() {
+		mountPath := filepath.Join(podmanTest.TmpDir, "secrets")
+		os.Mkdir(mountPath, 0755)
+		session := podmanTest.Podman([]string{"run", "-v", fmt.Sprintf("%s:/run/test", mountPath), "-v", "/tmp:/run/test", ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(125))
+	})
+})