From b3d6383f25e71748793535733a767c60ccfcbd82 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Sun, 13 Sep 2020 11:54:08 -0400 Subject: Fix podman pod create --infra-command and --infra-image Currently infr-command and --infra-image commands are ignored from the user. This PR instruments them and adds tests for each combination. Signed-off-by: Daniel J Walsh --- cmd/podman/pods/create.go | 19 ++++++++-- libpod/options.go | 30 +++++++++++++++ libpod/pod.go | 2 + libpod/runtime_pod_infra_linux.go | 25 +++++++++---- pkg/domain/infra/runtime_libpod.go | 17 --------- pkg/specgen/generate/pod_create.go | 9 +++++ pkg/specgen/pod_validate.go | 7 ---- test/e2e/pod_create_test.go | 76 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 151 insertions(+), 34 deletions(-) diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go index 603ddcaca..d8d32b930 100644 --- a/cmd/podman/pods/create.go +++ b/cmd/podman/pods/create.go @@ -55,8 +55,8 @@ func init() { flags.StringVar(&createOptions.CGroupParent, "cgroup-parent", "", "Set parent cgroup for the pod") flags.BoolVar(&createOptions.Infra, "infra", true, "Create an infra container associated with the pod to share namespaces with") flags.StringVar(&createOptions.InfraConmonPidFile, "infra-conmon-pidfile", "", "Path to the file that will receive the POD of the infra container's conmon") - flags.StringVar(&createOptions.InfraImage, "infra-image", containerConfig.Engine.InfraImage, "The image of the infra container to associate with the pod") - flags.StringVar(&createOptions.InfraCommand, "infra-command", containerConfig.Engine.InfraCommand, "The command to run on the infra container when the pod is started") + flags.String("infra-image", containerConfig.Engine.InfraImage, "The image of the infra container to associate with the pod") + flags.String("infra-command", containerConfig.Engine.InfraCommand, "The command to run on the infra container when the pod is started") flags.StringSliceVar(&labelFile, "label-file", []string{}, "Read in a line delimited file of labels") flags.StringSliceVarP(&labels, "label", "l", []string{}, "Set metadata on pod (default [])") flags.StringVarP(&createOptions.Name, "name", "n", "", "Assign a name to the pod") @@ -92,7 +92,6 @@ func create(cmd *cobra.Command, args []string) error { if cmd.Flag("infra-command").Changed { return errors.New("cannot set infra-command without an infra container") } - createOptions.InfraCommand = "" if cmd.Flag("infra-image").Changed { return errors.New("cannot set infra-image without an infra container") } @@ -104,6 +103,20 @@ func create(cmd *cobra.Command, args []string) error { createOptions.Share = nil } else { createOptions.Share = strings.Split(share, ",") + if cmd.Flag("infra-command").Changed { + // Only send content to server side if user changed defaults + createOptions.InfraCommand, err = cmd.Flags().GetString("infra-command") + if err != nil { + return err + } + } + if cmd.Flag("infra-image").Changed { + // Only send content to server side if user changed defaults + createOptions.InfraImage, err = cmd.Flags().GetString("infra-image") + if err != nil { + return err + } + } } if cmd.Flag("pod-id-file").Changed { diff --git a/libpod/options.go b/libpod/options.go index d592124bc..f7b3419e5 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1659,6 +1659,36 @@ func WithUmask(umask string) CtrCreateOption { // Pod Creation Options +// WithInfraImage sets the infra image for libpod. +// An infra image is used for inter-container kernel +// namespace sharing within a pod. Typically, an infra +// container is lightweight and is there to reap +// zombie processes within its pid namespace. +func WithInfraImage(img string) PodCreateOption { + return func(pod *Pod) error { + if pod.valid { + return define.ErrPodFinalized + } + + pod.config.InfraContainer.InfraImage = img + + return nil + } +} + +// WithInfraCommand sets the command to +// run on pause container start up. +func WithInfraCommand(cmd []string) PodCreateOption { + return func(pod *Pod) error { + if pod.valid { + return define.ErrPodFinalized + } + + pod.config.InfraContainer.InfraCommand = cmd + return nil + } +} + // WithPodName sets the name of the pod. func WithPodName(name string) PodCreateOption { return func(pod *Pod) error { diff --git a/libpod/pod.go b/libpod/pod.go index 422966b94..709184008 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -105,6 +105,8 @@ type InfraContainerConfig struct { HostAdd []string `json:"hostsAdd,omitempty"` Networks []string `json:"networks,omitempty"` ExitCommand []string `json:"exitCommand,omitempty"` + InfraImage string `json:"infraImage,omitempty"` + InfraCommand []string `json:"infraCommand,omitempty"` } // ID retrieves the pod's ID diff --git a/libpod/runtime_pod_infra_linux.go b/libpod/runtime_pod_infra_linux.go index b2f21d946..164068638 100644 --- a/libpod/runtime_pod_infra_linux.go +++ b/libpod/runtime_pod_infra_linux.go @@ -36,22 +36,26 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, rawIm isRootless := rootless.IsRootless() - entryCmd := []string{r.config.Engine.InfraCommand} + entrypointSet := len(p.config.InfraContainer.InfraCommand) > 0 + entryPoint := p.config.InfraContainer.InfraCommand + entryCmd := []string{} var options []CtrCreateOption // I've seen circumstances where config is being passed as nil. // Let's err on the side of safety and make sure it's safe to use. if config != nil { - setEntrypoint := false // default to entrypoint in image if there is one - if len(config.Entrypoint) > 0 { - entryCmd = config.Entrypoint - setEntrypoint = true + if !entrypointSet { + if len(config.Entrypoint) > 0 { + entrypointSet = true + entryPoint = config.Entrypoint + entryCmd = config.Entrypoint + } } if len(config.Cmd) > 0 { // We can't use the default pause command, since we're // sourcing from the image. If we didn't already set an // entrypoint, set one now. - if !setEntrypoint { + if !entrypointSet { // Use the Docker default "/bin/sh -c" // entrypoint, as we're overriding command. // If an image doesn't want this, it can @@ -136,6 +140,9 @@ func (r *Runtime) makeInfraContainer(ctx context.Context, p *Pod, imgName, rawIm options = append(options, WithRootFSFromImage(imgID, imgName, rawImageName)) options = append(options, WithName(containerName)) options = append(options, withIsInfra()) + if entrypointSet { + options = append(options, WithEntrypoint(entryPoint)) + } if len(p.config.InfraContainer.ConmonPidFile) > 0 { options = append(options, WithConmonPidFile(p.config.InfraContainer.ConmonPidFile)) } @@ -151,7 +158,11 @@ func (r *Runtime) createInfraContainer(ctx context.Context, p *Pod) (*Container, return nil, define.ErrRuntimeStopped } - newImage, err := r.ImageRuntime().New(ctx, r.config.Engine.InfraImage, "", "", nil, nil, image.SigningOptions{}, nil, util.PullImageMissing) + img := p.config.InfraContainer.InfraImage + if img == "" { + img = r.config.Engine.InfraImage + } + newImage, err := r.ImageRuntime().New(ctx, img, "", "", nil, nil, image.SigningOptions{}, nil, util.PullImageMissing) if err != nil { return nil, err } diff --git a/pkg/domain/infra/runtime_libpod.go b/pkg/domain/infra/runtime_libpod.go index f9b8106ef..26c9c7e2e 100644 --- a/pkg/domain/infra/runtime_libpod.go +++ b/pkg/domain/infra/runtime_libpod.go @@ -227,23 +227,6 @@ func getRuntime(ctx context.Context, fs *flag.FlagSet, opts *engineOpts) (*libpo // TODO flag to set CNI plugins dir? - // TODO I don't think these belong here? - // Will follow up with a different PR to address - // - // Pod create options - - infraImageFlag := fs.Lookup("infra-image") - if infraImageFlag != nil && infraImageFlag.Changed { - infraImage, _ := fs.GetString("infra-image") - options = append(options, libpod.WithDefaultInfraImage(infraImage)) - } - - infraCommandFlag := fs.Lookup("infra-command") - if infraCommandFlag != nil && infraImageFlag.Changed { - infraCommand, _ := fs.GetString("infra-command") - options = append(options, libpod.WithDefaultInfraCommand(infraCommand)) - } - if !opts.withFDS { options = append(options, libpod.WithEnableSDNotify()) } diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go index 0bd39d5a4..101201252 100644 --- a/pkg/specgen/generate/pod_create.go +++ b/pkg/specgen/generate/pod_create.go @@ -84,6 +84,15 @@ func createPodOptions(p *specgen.PodSpecGenerator, rt *libpod.Runtime) ([]libpod if len(p.CNINetworks) > 0 { options = append(options, libpod.WithPodNetworks(p.CNINetworks)) } + + if len(p.InfraImage) > 0 { + options = append(options, libpod.WithInfraImage(p.InfraImage)) + } + + if len(p.InfraCommand) > 0 { + options = append(options, libpod.WithInfraCommand(p.InfraCommand)) + } + switch p.NetNS.NSMode { case specgen.Bridge, specgen.Default, "": logrus.Debugf("Pod using default network mode") diff --git a/pkg/specgen/pod_validate.go b/pkg/specgen/pod_validate.go index d5e0aecf2..907c0bb69 100644 --- a/pkg/specgen/pod_validate.go +++ b/pkg/specgen/pod_validate.go @@ -95,12 +95,5 @@ func (p *PodSpecGenerator) Validate() error { return exclusivePodOptions("NoManageHosts", "HostAdd") } - // Set Defaults - if len(p.InfraImage) < 1 { - p.InfraImage = containerConfig.Engine.InfraImage - } - if len(p.InfraCommand) < 1 { - p.InfraCommand = []string{containerConfig.Engine.InfraCommand} - } return nil } diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go index f260a123a..ed62e8a4b 100644 --- a/test/e2e/pod_create_test.go +++ b/test/e2e/pod_create_test.go @@ -329,4 +329,80 @@ var _ = Describe("Podman pod create", func() { Expect(session.ExitCode()).To(Equal(0)) } }) + + It("podman create pod with defaults", func() { + name := "test" + session := podmanTest.Podman([]string{"pod", "create", "--name", name}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + check := podmanTest.Podman([]string{"pod", "inspect", name}) + check.WaitWithDefaultTimeout() + Expect(check.ExitCode()).To(Equal(0)) + data := check.InspectPodToJSON() + + check1 := podmanTest.Podman([]string{"container", "inspect", "--format", "{{.Config.Entrypoint}}", data.Containers[0].ID}) + check1.WaitWithDefaultTimeout() + Expect(check1.ExitCode()).To(Equal(0)) + Expect(check1.OutputToString()).To(Equal("/pause")) + }) + + It("podman create pod with --infra-command", func() { + name := "test" + session := podmanTest.Podman([]string{"pod", "create", "--infra-command", "/pause1", "--name", name}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + check := podmanTest.Podman([]string{"pod", "inspect", name}) + check.WaitWithDefaultTimeout() + Expect(check.ExitCode()).To(Equal(0)) + data := check.InspectPodToJSON() + + check1 := podmanTest.Podman([]string{"container", "inspect", "--format", "{{.Config.Entrypoint}}", data.Containers[0].ID}) + check1.WaitWithDefaultTimeout() + Expect(check1.ExitCode()).To(Equal(0)) + Expect(check1.OutputToString()).To(Equal("/pause1")) + }) + + It("podman create pod with --infra-image", func() { + dockerfile := `FROM docker.io/library/alpine:latest +entrypoint ["/fromimage"] +` + podmanTest.BuildImage(dockerfile, "localhost/infra", "false") + name := "test" + session := podmanTest.Podman([]string{"pod", "create", "--infra-image", "localhost/infra", "--name", name}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + check := podmanTest.Podman([]string{"pod", "inspect", name}) + check.WaitWithDefaultTimeout() + Expect(check.ExitCode()).To(Equal(0)) + data := check.InspectPodToJSON() + + check1 := podmanTest.Podman([]string{"container", "inspect", "--format", "{{.Config.Entrypoint}}", data.Containers[0].ID}) + check1.WaitWithDefaultTimeout() + Expect(check1.ExitCode()).To(Equal(0)) + Expect(check1.OutputToString()).To(Equal("/fromimage")) + }) + + It("podman create pod with --infra-command --infra-image", func() { + dockerfile := `FROM docker.io/library/alpine:latest +entrypoint ["/fromimage"] +` + podmanTest.BuildImage(dockerfile, "localhost/infra", "false") + name := "test" + session := podmanTest.Podman([]string{"pod", "create", "--infra-image", "localhost/infra", "--infra-command", "/fromcommand", "--name", name}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + check := podmanTest.Podman([]string{"pod", "inspect", name}) + check.WaitWithDefaultTimeout() + Expect(check.ExitCode()).To(Equal(0)) + data := check.InspectPodToJSON() + + check1 := podmanTest.Podman([]string{"container", "inspect", "--format", "{{.Config.Entrypoint}}", data.Containers[0].ID}) + check1.WaitWithDefaultTimeout() + Expect(check1.ExitCode()).To(Equal(0)) + Expect(check1.OutputToString()).To(Equal("/fromcommand")) + }) }) -- cgit v1.2.3-54-g00ecf