From 7d71d24440afbf30689c53c2c69205072e4b029f Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 28 May 2020 13:27:23 +0200 Subject: podman-pod{rm,start,stop}: support --pod-id-file Support the `--pod-id-file` flag in the rm, start and stop pod commands. This completes the already support flag in pod-create and is another prerequisite for generating generic systemd unit files for pods. Also add completions, docs and tests. Signed-off-by: Valentin Rothberg --- completions/bash/podman | 3 +++ 1 file changed, 3 insertions(+) (limited to 'completions/bash') diff --git a/completions/bash/podman b/completions/bash/podman index a58becaf0..6528281ba 100644 --- a/completions/bash/podman +++ b/completions/bash/podman @@ -3223,6 +3223,7 @@ _podman_pod_restart() { _podman_pod_rm() { local options_with_args=" + --pod-id-file " local boolean_options=" @@ -3250,6 +3251,7 @@ _podman_pod_rm() { _podman_pod_start() { local options_with_args=" + --pod-id-file " local boolean_options=" @@ -3275,6 +3277,7 @@ _podman_pod_stop() { local options_with_args=" -t --time + --pod-id-file " local boolean_options=" -- cgit v1.2.3-54-g00ecf From cf89bb671184e453c4ba5f27e26d02216d8fc491 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 29 May 2020 10:35:22 +0200 Subject: container-{create,run}: add `--pod-id-file` Allow containers to join an existing pod via the `--pod-id-file` which is already supported by a number of `podman-pod` subcommands. Also add tests to make sure it's working and to prevent future regressions. Signed-off-by: Valentin Rothberg --- cmd/podman/common/create.go | 5 +++++ cmd/podman/common/create_opts.go | 1 + cmd/podman/common/specgen.go | 11 ++++++++++ cmd/podman/common/util.go | 25 ++++++++++++++++++++++ cmd/podman/pods/common.go | 23 -------------------- cmd/podman/pods/rm.go | 3 ++- cmd/podman/pods/start.go | 3 ++- cmd/podman/pods/stop.go | 3 ++- completions/bash/podman | 3 ++- docs/source/markdown/podman-create.1.md | 4 ++++ docs/source/markdown/podman-run.1.md | 4 ++++ test/e2e/create_test.go | 37 +++++++++++++++++++++++++++++++++ 12 files changed, 95 insertions(+), 27 deletions(-) delete mode 100644 cmd/podman/pods/common.go (limited to 'completions/bash') diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index 86cd51643..e79c5c20b 100644 --- a/cmd/podman/common/create.go +++ b/cmd/podman/common/create.go @@ -338,6 +338,11 @@ func GetCreateFlags(cf *ContainerCLIOpts) *pflag.FlagSet { "pod", "", "Run container in an existing pod", ) + createFlags.StringVar( + &cf.PodIDFile, + "pod-id-file", "", + "Read the pod ID from the file", + ) createFlags.BoolVar( &cf.Privileged, "privileged", false, diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go index 4cba5daf7..98dc6744c 100644 --- a/cmd/podman/common/create_opts.go +++ b/cmd/podman/common/create_opts.go @@ -68,6 +68,7 @@ type ContainerCLIOpts struct { PID string PIDsLimit int64 Pod string + PodIDFile string Privileged bool PublishAll bool Pull string diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go index 2286e67de..fee9d8c7b 100644 --- a/cmd/podman/common/specgen.go +++ b/cmd/podman/common/specgen.go @@ -254,6 +254,17 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string s.PublishExposedPorts = c.PublishAll s.Pod = c.Pod + if len(c.PodIDFile) > 0 { + if len(s.Pod) > 0 { + return errors.New("Cannot specify both --pod and --pod-id-file") + } + podID, err := ReadPodIDFile(c.PodIDFile) + if err != nil { + return err + } + s.Pod = podID + } + expose, err := createExpose(c.Expose) if err != nil { return err diff --git a/cmd/podman/common/util.go b/cmd/podman/common/util.go index a3626b4e4..422e241af 100644 --- a/cmd/podman/common/util.go +++ b/cmd/podman/common/util.go @@ -1,6 +1,7 @@ package common import ( + "io/ioutil" "net" "strconv" "strings" @@ -10,6 +11,30 @@ import ( "github.com/sirupsen/logrus" ) +// ReadPodIDFile reads the specified file and returns its content (i.e., first +// line). +func ReadPodIDFile(path string) (string, error) { + content, err := ioutil.ReadFile(path) + if err != nil { + return "", errors.Wrap(err, "error reading pod ID file") + } + return strings.Split(string(content), "\n")[0], nil +} + +// ReadPodIDFiles reads the specified files and returns their content (i.e., +// first line). +func ReadPodIDFiles(files []string) ([]string, error) { + ids := []string{} + for _, file := range files { + id, err := ReadPodIDFile(file) + if err != nil { + return nil, err + } + ids = append(ids, id) + } + return ids, nil +} + // createExpose parses user-provided exposed port definitions and converts them // into SpecGen format. // TODO: The SpecGen format should really handle ranges more sanely - we could diff --git a/cmd/podman/pods/common.go b/cmd/podman/pods/common.go deleted file mode 100644 index 1c4195095..000000000 --- a/cmd/podman/pods/common.go +++ /dev/null @@ -1,23 +0,0 @@ -package pods - -import ( - "io/ioutil" - "strings" - - "github.com/pkg/errors" -) - -// readPodIDFiles reads the specified files and returns their content (i.e., -// first line). -func readPodIDFiles(files []string) ([]string, error) { - ids := []string{} - for _, podFile := range files { - content, err := ioutil.ReadFile(podFile) - if err != nil { - return nil, errors.Wrap(err, "error reading pod ID file") - } - id := strings.Split(string(content), "\n")[0] - ids = append(ids, id) - } - return ids, nil -} diff --git a/cmd/podman/pods/rm.go b/cmd/podman/pods/rm.go index ecceda32a..8de0bce9e 100644 --- a/cmd/podman/pods/rm.go +++ b/cmd/podman/pods/rm.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/containers/libpod/cmd/podman/common" "github.com/containers/libpod/cmd/podman/parse" "github.com/containers/libpod/cmd/podman/registry" "github.com/containers/libpod/cmd/podman/utils" @@ -61,7 +62,7 @@ func rm(cmd *cobra.Command, args []string) error { errs utils.OutputErrors ) - ids, err := readPodIDFiles(rmOptions.PodIDFiles) + ids, err := common.ReadPodIDFiles(rmOptions.PodIDFiles) if err != nil { return err } diff --git a/cmd/podman/pods/start.go b/cmd/podman/pods/start.go index 86517190d..97020b360 100644 --- a/cmd/podman/pods/start.go +++ b/cmd/podman/pods/start.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/containers/libpod/cmd/podman/common" "github.com/containers/libpod/cmd/podman/parse" "github.com/containers/libpod/cmd/podman/registry" "github.com/containers/libpod/cmd/podman/utils" @@ -61,7 +62,7 @@ func start(cmd *cobra.Command, args []string) error { errs utils.OutputErrors ) - ids, err := readPodIDFiles(startOptions.PodIDFiles) + ids, err := common.ReadPodIDFiles(startOptions.PodIDFiles) if err != nil { return err } diff --git a/cmd/podman/pods/stop.go b/cmd/podman/pods/stop.go index fd66488f9..628e8a536 100644 --- a/cmd/podman/pods/stop.go +++ b/cmd/podman/pods/stop.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/containers/libpod/cmd/podman/common" "github.com/containers/libpod/cmd/podman/parse" "github.com/containers/libpod/cmd/podman/registry" "github.com/containers/libpod/cmd/podman/utils" @@ -68,7 +69,7 @@ func stop(cmd *cobra.Command, args []string) error { stopOptions.Timeout = int(stopOptions.TimeoutCLI) } - ids, err := readPodIDFiles(stopOptions.PodIDFiles) + ids, err := common.ReadPodIDFiles(stopOptions.PodIDFiles) if err != nil { return err } diff --git a/completions/bash/podman b/completions/bash/podman index 6528281ba..0e4b60b14 100644 --- a/completions/bash/podman +++ b/completions/bash/podman @@ -2102,6 +2102,7 @@ _podman_container_run() { --pid --pids-limit --pod + --pod-id-file --publish -p --pull --runtime @@ -2206,7 +2207,7 @@ _podman_container_run() { __podman_complete_capabilities return ;; - --cidfile|--env-file|--init-path|--label-file) + --cidfile|--env-file|--init-path|--label-file|--pod-id-file) _filedir return ;; diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md index a69ef04d1..81c83369a 100644 --- a/docs/source/markdown/podman-create.1.md +++ b/docs/source/markdown/podman-create.1.md @@ -593,6 +593,10 @@ Tune the container's pids limit. Set `0` to have unlimited pids for the containe Run container in an existing pod. If you want Podman to make the pod for you, preference the pod name with `new:`. To make a pod with more granular options, use the `podman pod create` command before creating a container. +**--pod-id-file**=*path* + +Run container in an existing pod and read the pod's ID from the specified file. If a container is run with a pod, and the pod has an infra-container, the infra-container will be started before the container is. + **--privileged**=*true|false* Give extended privileges to this container. The default is *false*. diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index 02db8b205..4f43e4c19 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -605,6 +605,10 @@ Run container in an existing pod. If you want Podman to make the pod for you, pr To make a pod with more granular options, use the **podman pod create** command before creating a container. If a container is run with a pod, and the pod has an infra-container, the infra-container will be started before the container is. +**--pod-id-file**=*path* + +Run container in an existing pod and read the pod's ID from the specified file. If a container is run with a pod, and the pod has an infra-container, the infra-container will be started before the container is. + **--privileged**=**true**|**false** Give extended privileges to this container. The default is **false**. diff --git a/test/e2e/create_test.go b/test/e2e/create_test.go index f40472a7c..b9a1ff83d 100644 --- a/test/e2e/create_test.go +++ b/test/e2e/create_test.go @@ -2,6 +2,7 @@ package integration import ( "fmt" + "io/ioutil" "os" "path/filepath" @@ -221,6 +222,42 @@ var _ = Describe("Podman create", func() { Expect(match).To(BeTrue()) }) + It("podman create --pod-id-file", func() { + // First, make sure that --pod and --pod-id-file yield an error + // if used together. + session := podmanTest.Podman([]string{"create", "--pod", "foo", "--pod-id-file", "bar", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(125)) + + tmpDir, err := ioutil.TempDir("", "") + Expect(err).To(BeNil()) + defer os.RemoveAll(tmpDir) + + podName := "rudoplh" + ctrName := "prancer" + podIDFile := tmpDir + "pod-id-file" + + // Now, let's create a pod with --pod-id-file. + session = podmanTest.Podman([]string{"pod", "create", "--pod-id-file", podIDFile, "--name", podName}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"pod", "inspect", podName}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.IsJSONOutputValid()).To(BeTrue()) + podData := session.InspectPodToJSON() + + // Finally we can create a container with --pod-id-file and do + // some checks to make sure it's working as expected. + session = podmanTest.Podman([]string{"create", "--pod-id-file", podIDFile, "--name", ctrName, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + ctrJSON := podmanTest.InspectContainer(ctrName) + Expect(podData.ID).To(Equal(ctrJSON[0].Pod)) // Make sure the container's pod matches the pod's ID + }) + It("podman run entrypoint and cmd test", func() { name := "test101" create := podmanTest.Podman([]string{"create", "--name", name, redis}) -- cgit v1.2.3-54-g00ecf From 402c68b41d3fd8f354a4fd0ba4d4101920f8cfe6 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 29 May 2020 13:20:22 +0200 Subject: pod create: add `--infra-conmon-pidfile` Add an `--infra-conmon-pidfile` flag to `podman-pod-create` to write the infra container's conmon process ID to a specified path. Several container sub-commands already support `--conmon-pidfile` which is especially helpful to allow for systemd to access and track the conmon processes. This allows for easily tracking the conmon process of a pod's infra container. Signed-off-by: Valentin Rothberg --- cmd/podman/pods/create.go | 4 ++++ completions/bash/podman | 1 + docs/source/markdown/podman-pod-create.1.md | 4 ++++ libpod/options.go | 12 ++++++++++++ libpod/pod.go | 1 + libpod/runtime_pod_infra_linux.go | 3 +++ pkg/domain/entities/pods.go | 22 +++++++++++++--------- pkg/specgen/generate/pod_create.go | 3 +++ pkg/specgen/podspecgen.go | 3 +++ test/e2e/pod_start_test.go | 19 +++++++++++++++++++ 10 files changed, 63 insertions(+), 9 deletions(-) (limited to 'completions/bash') diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go index 5ed5fa57c..51b7a7d52 100644 --- a/cmd/podman/pods/create.go +++ b/cmd/podman/pods/create.go @@ -53,6 +53,7 @@ func init() { flags.AddFlagSet(common.GetNetFlags()) 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.StringSliceVar(&labelFile, "label-file", []string{}, "Read in a line delimited file of labels") @@ -83,6 +84,9 @@ func create(cmd *cobra.Command, args []string) error { if !createOptions.Infra { logrus.Debugf("Not creating an infra container") + if cmd.Flag("infra-conmon-pidfile").Changed { + return errors.New("cannot set infra-conmon-pid without an infra container") + } if cmd.Flag("infra-command").Changed { return errors.New("cannot set infra-command without an infra container") } diff --git a/completions/bash/podman b/completions/bash/podman index 0e4b60b14..6dbe645fe 100644 --- a/completions/bash/podman +++ b/completions/bash/podman @@ -3098,6 +3098,7 @@ _podman_pod_create() { --dns-opt --dns-search --infra-command + --infra-conmon-pidfile --infra-image --ip --label-file diff --git a/docs/source/markdown/podman-pod-create.1.md b/docs/source/markdown/podman-pod-create.1.md index 489c9b32e..de6b600f0 100644 --- a/docs/source/markdown/podman-pod-create.1.md +++ b/docs/source/markdown/podman-pod-create.1.md @@ -47,6 +47,10 @@ Set a hostname to the pod Create an infra container and associate it with the pod. An infra container is a lightweight container used to coordinate the shared kernel namespace of a pod. Default: true. +**--infra-conmon-pidfile**=*file* + +Write the pid of the infra container's **conmon** process to a file. As **conmon** runs in a separate process than Podman, this is necessary when using systemd to manage Podman containers and pods. + **--infra-command**=*command* The command that will be run to start the infra container. Default: "/pause". diff --git a/libpod/options.go b/libpod/options.go index 75d098815..5a0f60093 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1550,6 +1550,18 @@ func WithPodCreateCommand() PodCreateOption { } } +// WithInfraConmonPidFile sets the path to a custom conmon PID file for the +// infra container. +func WithInfraConmonPidFile(path string) PodCreateOption { + return func(pod *Pod) error { + if pod.valid { + return define.ErrPodFinalized + } + pod.config.InfraContainer.ConmonPidFile = path + return nil + } +} + // WithPodLabels sets the labels of a pod. func WithPodLabels(labels map[string]string) PodCreateOption { return func(pod *Pod) error { diff --git a/libpod/pod.go b/libpod/pod.go index 38fe1fd2c..7af78fa07 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -83,6 +83,7 @@ type podState struct { // InfraContainerConfig is the configuration for the pod's infra container type InfraContainerConfig struct { + ConmonPidFile string `json:"conmonPidFile"` HasInfraContainer bool `json:"makeInfraContainer"` HostNetwork bool `json:"infraHostNetwork,omitempty"` PortBindings []ocicni.PortMapping `json:"infraPortBindings"` diff --git a/libpod/runtime_pod_infra_linux.go b/libpod/runtime_pod_infra_linux.go index 06a7b3936..a0dee3aa1 100644 --- a/libpod/runtime_pod_infra_linux.go +++ b/libpod/runtime_pod_infra_linux.go @@ -130,6 +130,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 len(p.config.InfraContainer.ConmonPidFile) > 0 { + options = append(options, WithConmonPidFile(p.config.InfraContainer.ConmonPidFile)) + } return r.newContainer(ctx, g.Config, options...) } diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go index a85333c75..fc76ddd41 100644 --- a/pkg/domain/entities/pods.go +++ b/pkg/domain/entities/pods.go @@ -103,15 +103,16 @@ type PodRmReport struct { } type PodCreateOptions struct { - CGroupParent string - Hostname string - Infra bool - InfraImage string - InfraCommand string - Labels map[string]string - Name string - Net *NetOptions - Share []string + CGroupParent string + Hostname string + Infra bool + InfraImage string + InfraCommand string + InfraConmonPidFile string + Labels map[string]string + Name string + Net *NetOptions + Share []string } type PodCreateReport struct { @@ -127,6 +128,9 @@ func (p PodCreateOptions) ToPodSpecGen(s *specgen.PodSpecGenerator) { if len(p.InfraCommand) > 0 { s.InfraCommand = strings.Split(p.InfraCommand, " ") } + if len(p.InfraConmonPidFile) > 0 { + s.InfraConmonPidFile = p.InfraConmonPidFile + } s.InfraImage = p.InfraImage s.SharedNamespaces = p.Share diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go index 51b7835b2..5ccb1ba80 100644 --- a/pkg/specgen/generate/pod_create.go +++ b/pkg/specgen/generate/pod_create.go @@ -94,5 +94,8 @@ func createPodOptions(p *specgen.PodSpecGenerator) ([]libpod.PodCreateOption, er } options = append(options, libpod.WithPodCgroups()) options = append(options, libpod.WithPodCreateCommand()) + if len(p.InfraConmonPidFile) > 0 { + options = append(options, libpod.WithInfraConmonPidFile(p.InfraConmonPidFile)) + } return options, nil } diff --git a/pkg/specgen/podspecgen.go b/pkg/specgen/podspecgen.go index 11976233a..600d27004 100644 --- a/pkg/specgen/podspecgen.go +++ b/pkg/specgen/podspecgen.go @@ -25,6 +25,9 @@ type PodBasicConfig struct { // InfraCommand and InfraImages in this struct. // Optional. NoInfra bool `json:"no_infra,omitempty"` + // InfraConmonPidFile is a custom path to store the infra container's + // conmon PID. + InfraConmonPidFile string `json:"infra_conmon_pid_file,omitempty"` // InfraCommand sets the command that will be used to start the infra // container. // If not set, the default set in the Libpod configuration file will be diff --git a/test/e2e/pod_start_test.go b/test/e2e/pod_start_test.go index 4502a76ed..99285d1e1 100644 --- a/test/e2e/pod_start_test.go +++ b/test/e2e/pod_start_test.go @@ -193,4 +193,23 @@ var _ = Describe("Podman pod start", func() { Expect(session.ExitCode()).To(Equal(0)) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(20)) // 10*(infra+top) }) + + It("podman pod create --infra-conmon-pod create + start", func() { + tmpDir, err := ioutil.TempDir("", "") + Expect(err).To(BeNil()) + tmpFile := tmpDir + "podID" + defer os.RemoveAll(tmpDir) + + podName := "rudolph" + // Create a pod with --infra-conmon-pid. + session := podmanTest.Podman([]string{"pod", "create", "--name", podName, "--infra-conmon-pidfile", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"pod", "start", podName}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1)) // infra + }) + }) -- cgit v1.2.3-54-g00ecf