Option handling has become large and should be a shared function

Everytime we add a new option for create, we end up having to also
add it to run, this makes it error prone.  Moving these to the same
function makes it easier to develop and prevents user mistakes.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

Closes: #975
Approved by: mheon

cmd/podman/create.go

@@ -110,29 +110,17 @@ func createCmd(c *cli.Context) error {
 	if err != nil {
 		return err
 	}
-	useImageVolumes := createConfig.ImageVolumeType == "bind"
 
 	runtimeSpec, err := cc.CreateConfigToOCISpec(createConfig)
 	if err != nil {
 		return err
 	}
+
 	options, err := createConfig.GetContainerCreateOptions()
 	if err != nil {
-		return errors.Wrapf(err, "unable to parse new container options")
+		return err
-	}
-	// Gather up the options for NewContainer which consist of With... funcs
-	options = append(options, libpod.WithRootFSFromImage(createConfig.ImageID, createConfig.Image, useImageVolumes))
-	options = append(options, libpod.WithSELinuxLabels(createConfig.ProcessLabel, createConfig.MountLabel))
-	options = append(options, libpod.WithConmonPidFile(createConfig.ConmonPidFile))
-	options = append(options, libpod.WithLabels(createConfig.Labels))
-	options = append(options, libpod.WithUser(createConfig.User))
-	options = append(options, libpod.WithShmDir(createConfig.ShmDir))
-	options = append(options, libpod.WithShmSize(createConfig.Resources.ShmSize))
-	options = append(options, libpod.WithGroups(createConfig.GroupAdd))
-	options = append(options, libpod.WithIDMappings(*createConfig.IDMappings))
-	if createConfig.Rootfs != "" {
-		options = append(options, libpod.WithRootFS(createConfig.Rootfs))
 	}
+
 	ctr, err := runtime.NewContainer(ctx, runtimeSpec, options...)
 	if err != nil {
 		return err

cmd/podman/run.go

@@ -104,7 +104,6 @@ func runCmd(c *cli.Context) error {
 	if err != nil {
 		return err
 	}
-	useImageVolumes := createConfig.ImageVolumeType == "bind"
 
 	runtimeSpec, err := cc.CreateConfigToOCISpec(createConfig)
 	if err != nil {
@@ -113,27 +112,7 @@ func runCmd(c *cli.Context) error {
 
 	options, err := createConfig.GetContainerCreateOptions()
 	if err != nil {
-		return errors.Wrapf(err, "unable to parse new container options")
+		return err
-	}
-
-	// Gather up the options for NewContainer which consist of With... funcs
-	options = append(options, libpod.WithRootFSFromImage(createConfig.ImageID, createConfig.Image, useImageVolumes))
-	options = append(options, libpod.WithSELinuxLabels(createConfig.ProcessLabel, createConfig.MountLabel))
-	options = append(options, libpod.WithConmonPidFile(createConfig.ConmonPidFile))
-	options = append(options, libpod.WithLabels(createConfig.Labels))
-	options = append(options, libpod.WithUser(createConfig.User))
-	options = append(options, libpod.WithShmDir(createConfig.ShmDir))
-	options = append(options, libpod.WithShmSize(createConfig.Resources.ShmSize))
-	options = append(options, libpod.WithGroups(createConfig.GroupAdd))
-	options = append(options, libpod.WithIDMappings(*createConfig.IDMappings))
-	if createConfig.Rootfs != "" {
-		options = append(options, libpod.WithRootFS(createConfig.Rootfs))
-	}
-
-	// Default used if not overridden on command line
-
-	if createConfig.CgroupParent != "" {
-		options = append(options, libpod.WithCgroupParent(createConfig.CgroupParent))
 	}
 
 	ctr, err := runtime.NewContainer(ctx, runtimeSpec, options...)

pkg/spec/createconfig.go

@@ -413,6 +413,27 @@ func (c *CreateConfig) GetContainerCreateOptions() ([]libpod.CtrCreateOption, er
 	}
 
 	options = append(options, libpod.WithPrivileged(c.Privileged))
+
+	useImageVolumes := c.ImageVolumeType == "bind"
+	// Gather up the options for NewContainer which consist of With... funcs
+	options = append(options, libpod.WithRootFSFromImage(c.ImageID, c.Image, useImageVolumes))
+	options = append(options, libpod.WithSELinuxLabels(c.ProcessLabel, c.MountLabel))
+	options = append(options, libpod.WithConmonPidFile(c.ConmonPidFile))
+	options = append(options, libpod.WithLabels(c.Labels))
+	options = append(options, libpod.WithUser(c.User))
+	options = append(options, libpod.WithShmDir(c.ShmDir))
+	options = append(options, libpod.WithShmSize(c.Resources.ShmSize))
+	options = append(options, libpod.WithGroups(c.GroupAdd))
+	options = append(options, libpod.WithIDMappings(*c.IDMappings))
+	if c.Rootfs != "" {
+		options = append(options, libpod.WithRootFS(c.Rootfs))
+	}
+	// Default used if not overridden on command line
+
+	if c.CgroupParent != "" {
+		options = append(options, libpod.WithCgroupParent(c.CgroupParent))
+	}
+
 	return options, nil
 }
 

pkg/varlinkapi/containers_create.go

@@ -41,7 +41,6 @@ func (i *LibpodAPI) CreateContainer(call ioprojectatomicpodman.VarlinkCall, conf
 	if err != nil {
 		return call.ReplyErrorOccurred(err.Error())
 	}
-	useImageVolumes := createConfig.ImageVolumeType == "bind"
 
 	runtimeSpec, err := cc.CreateConfigToOCISpec(createConfig)
 	if err != nil {
@@ -52,16 +51,7 @@ func (i *LibpodAPI) CreateContainer(call ioprojectatomicpodman.VarlinkCall, conf
 	if err != nil {
 		return call.ReplyErrorOccurred(err.Error())
 	}
-	// Gather up the options for NewContainer which consist of With... funcs
+
-	options = append(options, libpod.WithRootFSFromImage(createConfig.ImageID, createConfig.Image, useImageVolumes))
-	options = append(options, libpod.WithSELinuxLabels(createConfig.ProcessLabel, createConfig.MountLabel))
-	options = append(options, libpod.WithConmonPidFile(createConfig.ConmonPidFile))
-	options = append(options, libpod.WithLabels(createConfig.Labels))
-	options = append(options, libpod.WithUser(createConfig.User))
-	options = append(options, libpod.WithShmDir(createConfig.ShmDir))
-	options = append(options, libpod.WithShmSize(createConfig.Resources.ShmSize))
-	options = append(options, libpod.WithGroups(createConfig.GroupAdd))
-	options = append(options, libpod.WithIDMappings(*createConfig.IDMappings))
 	ctr, err := runtime.NewContainer(ctx, runtimeSpec, options...)
 	if err != nil {
 		return call.ReplyErrorOccurred(err.Error())