From 82a948c04ec068acb9f0d47dc0f9e3bd05b4c90c Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 20 Jun 2018 17:08:18 -0400 Subject: 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 Closes: #975 Approved by: mheon --- cmd/podman/create.go | 18 +++--------------- cmd/podman/run.go | 23 +---------------------- pkg/spec/createconfig.go | 21 +++++++++++++++++++++ pkg/varlinkapi/containers_create.go | 12 +----------- 4 files changed, 26 insertions(+), 48 deletions(-) diff --git a/cmd/podman/create.go b/cmd/podman/create.go index 52d6e32f6..440406903 100644 --- a/cmd/podman/create.go +++ b/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") - } - // 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)) + return err } + ctr, err := runtime.NewContainer(ctx, runtimeSpec, options...) if err != nil { return err diff --git a/cmd/podman/run.go b/cmd/podman/run.go index b7516a233..421b868ad 100644 --- a/cmd/podman/run.go +++ b/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") - } - - // 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)) + return err } ctr, err := runtime.NewContainer(ctx, runtimeSpec, options...) diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go index 36a6e83f2..205e08c57 100644 --- a/pkg/spec/createconfig.go +++ b/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 } diff --git a/pkg/varlinkapi/containers_create.go b/pkg/varlinkapi/containers_create.go index 40bd92e9a..8268a3fdd 100644 --- a/pkg/varlinkapi/containers_create.go +++ b/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()) -- cgit v1.2.3-54-g00ecf