From 636881ece56f77df4c6a25dc5faa68a740607e3f Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 28 May 2020 10:54:17 +0200 Subject: [PATCH 01/14] pod config: add a `CreateCommand` field Add a `CreateCommand` field to the pod config which includes the entire `os.Args` at pod-creation. Similar to the already existing field in a container config, we need this information to properly generate generic systemd unit files for pods. It's a prerequisite to support the `--new` flag for pods. Also add the `CreateCommand` to the pod-inspect data, which can come in handy for debugging, general inspection and certainly for the tests that are added along with the other changes. Signed-off-by: Valentin Rothberg --- libpod/define/pod_inspect.go | 3 +++ libpod/options.go | 12 ++++++++++++ libpod/pod.go | 4 ++++ libpod/pod_api.go | 1 + pkg/specgen/generate/pod_create.go | 1 + test/e2e/pod_inspect_test.go | 22 ++++++++++++++++++++++ 6 files changed, 43 insertions(+) diff --git a/libpod/define/pod_inspect.go b/libpod/define/pod_inspect.go index 26fd2cab49f3..7f06e16fc3f7 100644 --- a/libpod/define/pod_inspect.go +++ b/libpod/define/pod_inspect.go @@ -18,6 +18,9 @@ type InspectPodData struct { Namespace string `json:"Namespace,omitempty"` // Created is the time when the pod was created. Created time.Time + // CreateCommand is the full command plus arguments of the process the + // container has been created with. + CreateCommand []string `json:"CreateCommand,omitempty"` // State represents the current state of the pod. State string `json:"State"` // Hostname is the hostname that the pod will set. diff --git a/libpod/options.go b/libpod/options.go index 8e0d3df86e12..75d09881580c 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1538,6 +1538,18 @@ func WithPodHostname(hostname string) PodCreateOption { } } +// WithPodCreateCommand adds the full command plus arguments of the current +// process to the pod config. +func WithPodCreateCommand() PodCreateOption { + return func(pod *Pod) error { + if pod.valid { + return define.ErrPodFinalized + } + pod.config.CreateCommand = os.Args + 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 8afaa6052154..38fe1fd2c07c 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -64,6 +64,10 @@ type PodConfig struct { // Time pod was created CreatedTime time.Time `json:"created"` + // CreateCommand is the full command plus arguments of the process the + // container has been created with. + CreateCommand []string `json:"CreateCommand,omitempty"` + // ID of the pod's lock LockID uint32 `json:"lockID"` } diff --git a/libpod/pod_api.go b/libpod/pod_api.go index e2c4b515ded8..c8605eb69b46 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -489,6 +489,7 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) { Name: p.Name(), Namespace: p.Namespace(), Created: p.CreatedTime(), + CreateCommand: p.config.CreateCommand, State: podState, Hostname: p.config.Hostname, Labels: p.Labels(), diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go index cd2d69cfb7da..51b7835b2095 100644 --- a/pkg/specgen/generate/pod_create.go +++ b/pkg/specgen/generate/pod_create.go @@ -93,5 +93,6 @@ func createPodOptions(p *specgen.PodSpecGenerator) ([]libpod.PodCreateOption, er options = append(options, libpod.WithInfraContainerPorts(ports)) } options = append(options, libpod.WithPodCgroups()) + options = append(options, libpod.WithPodCreateCommand()) return options, nil } diff --git a/test/e2e/pod_inspect_test.go b/test/e2e/pod_inspect_test.go index 8040adf1e450..f1acd37500d3 100644 --- a/test/e2e/pod_inspect_test.go +++ b/test/e2e/pod_inspect_test.go @@ -57,4 +57,26 @@ var _ = Describe("Podman pod inspect", func() { podData := inspect.InspectPodToJSON() Expect(podData.ID).To(Equal(podid)) }) + + It("podman pod inspect (CreateCommand)", func() { + podName := "myTestPod" + createCommand := []string{"pod", "create", "--name", podName, "--hostname", "rudolph", "--share", "net"} + + // Create the pod. + session := podmanTest.Podman(createCommand) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Inspect the pod and make sure that the create command is + // exactly how we created the pod. + inspect := podmanTest.Podman([]string{"pod", "inspect", podName}) + inspect.WaitWithDefaultTimeout() + Expect(inspect.ExitCode()).To(Equal(0)) + Expect(inspect.IsJSONOutputValid()).To(BeTrue()) + podData := inspect.InspectPodToJSON() + // Let's get the last len(createCommand) items in the command. + inspectCreateCommand := podData.CreateCommand + index := len(inspectCreateCommand) - len(createCommand) + Expect(inspectCreateCommand[index:]).To(Equal(createCommand)) + }) }) From 7f5aabb08389f9baee49bd76ab21f876e0bb70b9 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 28 May 2020 11:05:41 +0200 Subject: [PATCH 02/14] systemd/generate: remove unused infra container field Signed-off-by: Valentin Rothberg --- pkg/systemd/generate/systemdgen.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/systemd/generate/systemdgen.go b/pkg/systemd/generate/systemdgen.go index 73fe52c0ef1c..538e54ec9cd2 100644 --- a/pkg/systemd/generate/systemdgen.go +++ b/pkg/systemd/generate/systemdgen.go @@ -27,8 +27,6 @@ type ContainerInfo struct { ServiceName string // Name or ID of the container. ContainerName string - // InfraContainer of the pod. - InfraContainer string // StopTimeout sets the timeout Podman waits before killing the container // during service stop. StopTimeout uint From 7d71d24440afbf30689c53c2c69205072e4b029f Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 28 May 2020 13:27:23 +0200 Subject: [PATCH 03/14] 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 --- cmd/podman/parse/common.go | 55 +++++++++++++++++ cmd/podman/pods/common.go | 23 ++++++++ cmd/podman/pods/rm.go | 24 ++++++-- cmd/podman/pods/start.go | 21 ++++++- cmd/podman/pods/stop.go | 34 +++++++---- completions/bash/podman | 3 + docs/source/markdown/podman-pod-rm.1.md | 6 ++ docs/source/markdown/podman-pod-start.1.md | 5 ++ docs/source/markdown/podman-pod-stop.1.md | 11 ++++ pkg/domain/infra/abi/pods.go | 4 ++ test/e2e/pod_rm_test.go | 69 ++++++++++++++++++++++ test/e2e/pod_start_test.go | 57 ++++++++++++++++++ test/e2e/pod_stop_test.go | 69 ++++++++++++++++++++++ 13 files changed, 361 insertions(+), 20 deletions(-) create mode 100644 cmd/podman/pods/common.go diff --git a/cmd/podman/parse/common.go b/cmd/podman/parse/common.go index 13f425b6dcfe..b3aa88da2ca8 100644 --- a/cmd/podman/parse/common.go +++ b/cmd/podman/parse/common.go @@ -5,6 +5,10 @@ import ( "github.com/spf13/cobra" ) +// TODO: the two functions here are almost identical. It may be worth looking +// into generalizing the two a bit more and share code but time is scarce and +// we only live once. + // CheckAllLatestAndCIDFile checks that --all and --latest are used correctly. // If cidfile is set, also check for the --cidfile flag. func CheckAllLatestAndCIDFile(c *cobra.Command, args []string, ignoreArgLen bool, cidfile bool) error { @@ -55,3 +59,54 @@ func CheckAllLatestAndCIDFile(c *cobra.Command, args []string, ignoreArgLen bool } return nil } + +// CheckAllLatestAndPodIDFile checks that --all and --latest are used correctly. +// If withIDFile is set, also check for the --pod-id-file flag. +func CheckAllLatestAndPodIDFile(c *cobra.Command, args []string, ignoreArgLen bool, withIDFile bool) error { + argLen := len(args) + if c.Flags().Lookup("all") == nil || c.Flags().Lookup("latest") == nil { + if !withIDFile { + return errors.New("unable to lookup values for 'latest' or 'all'") + } else if c.Flags().Lookup("pod-id-file") == nil { + return errors.New("unable to lookup values for 'latest', 'all' or 'pod-id-file'") + } + } + + specifiedAll, _ := c.Flags().GetBool("all") + specifiedLatest, _ := c.Flags().GetBool("latest") + specifiedPodIDFile := false + if pid, _ := c.Flags().GetStringArray("pod-id-file"); len(pid) > 0 { + specifiedPodIDFile = true + } + + if specifiedPodIDFile && (specifiedAll || specifiedLatest) { + return errors.Errorf("--all, --latest and --pod-id-file cannot be used together") + } else if specifiedAll && specifiedLatest { + return errors.Errorf("--all and --latest cannot be used together") + } + + if (argLen > 0) && specifiedAll { + return errors.Errorf("no arguments are needed with --all") + } + + if ignoreArgLen { + return nil + } + + if argLen > 0 { + if specifiedLatest { + return errors.Errorf("no arguments are needed with --latest") + } else if withIDFile && (specifiedLatest || specifiedPodIDFile) { + return errors.Errorf("no arguments are needed with --latest or --pod-id-file") + } + } + + if specifiedPodIDFile { + return nil + } + + if argLen < 1 && !specifiedAll && !specifiedLatest && !specifiedPodIDFile { + return errors.Errorf("you must provide at least one name or id") + } + return nil +} diff --git a/cmd/podman/pods/common.go b/cmd/podman/pods/common.go new file mode 100644 index 000000000000..1c4195095e49 --- /dev/null +++ b/cmd/podman/pods/common.go @@ -0,0 +1,23 @@ +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 4b9882f8a69a..ecceda32aa18 100644 --- a/cmd/podman/pods/rm.go +++ b/cmd/podman/pods/rm.go @@ -11,7 +11,15 @@ import ( "github.com/spf13/cobra" ) +// allows for splitting API and CLI-only options +type podRmOptionsWrapper struct { + entities.PodRmOptions + + PodIDFiles []string +} + var ( + rmOptions = podRmOptionsWrapper{} podRmDescription = fmt.Sprintf(`podman rm will remove one or more stopped pods and their containers from the host. The pod name or ID can be used. A pod with containers will not be removed without --force. If --force is specified, all containers will be stopped, then removed.`) @@ -21,7 +29,7 @@ var ( Long: podRmDescription, RunE: rm, Args: func(cmd *cobra.Command, args []string) error { - return parse.CheckAllLatestAndCIDFile(cmd, args, false, false) + return parse.CheckAllLatestAndPodIDFile(cmd, args, false, true) }, Example: `podman pod rm mywebserverpod podman pod rm -f 860a4b23 @@ -29,10 +37,6 @@ var ( } ) -var ( - rmOptions = entities.PodRmOptions{} -) - func init() { registry.Commands = append(registry.Commands, registry.CliCommand{ Mode: []entities.EngineMode{entities.ABIMode, entities.TunnelMode}, @@ -45,6 +49,7 @@ func init() { flags.BoolVarP(&rmOptions.Force, "force", "f", false, "Force removal of a running pod by first stopping all containers, then removing all containers in the pod. The default is false") flags.BoolVarP(&rmOptions.Ignore, "ignore", "i", false, "Ignore errors when a specified pod is missing") flags.BoolVarP(&rmOptions.Latest, "latest", "l", false, "Remove the latest pod podman is aware of") + flags.StringArrayVarP(&rmOptions.PodIDFiles, "pod-id-file", "", nil, "Read the pod ID from the file") if registry.IsRemote() { _ = flags.MarkHidden("latest") _ = flags.MarkHidden("ignore") @@ -55,7 +60,14 @@ func rm(cmd *cobra.Command, args []string) error { var ( errs utils.OutputErrors ) - responses, err := registry.ContainerEngine().PodRm(context.Background(), args, rmOptions) + + ids, err := readPodIDFiles(rmOptions.PodIDFiles) + if err != nil { + return err + } + args = append(args, ids...) + + responses, err := registry.ContainerEngine().PodRm(context.Background(), args, rmOptions.PodRmOptions) if err != nil { return err } diff --git a/cmd/podman/pods/start.go b/cmd/podman/pods/start.go index d0150a3c26e3..86517190db12 100644 --- a/cmd/podman/pods/start.go +++ b/cmd/podman/pods/start.go @@ -11,6 +11,13 @@ import ( "github.com/spf13/cobra" ) +// allows for splitting API and CLI-only options +type podStartOptionsWrapper struct { + entities.PodStartOptions + + PodIDFiles []string +} + var ( podStartDescription = `The pod name or ID can be used. @@ -21,7 +28,7 @@ var ( Long: podStartDescription, RunE: start, Args: func(cmd *cobra.Command, args []string) error { - return parse.CheckAllLatestAndCIDFile(cmd, args, false, false) + return parse.CheckAllLatestAndPodIDFile(cmd, args, false, true) }, Example: `podman pod start podID podman pod start --latest @@ -30,7 +37,7 @@ var ( ) var ( - startOptions = entities.PodStartOptions{} + startOptions = podStartOptionsWrapper{} ) func init() { @@ -43,6 +50,7 @@ func init() { flags := startCommand.Flags() flags.BoolVarP(&startOptions.All, "all", "a", false, "Restart all running pods") flags.BoolVarP(&startOptions.Latest, "latest", "l", false, "Restart the latest pod podman is aware of") + flags.StringArrayVarP(&startOptions.PodIDFiles, "pod-id-file", "", nil, "Read the pod ID from the file") if registry.IsRemote() { _ = flags.MarkHidden("latest") } @@ -52,7 +60,14 @@ func start(cmd *cobra.Command, args []string) error { var ( errs utils.OutputErrors ) - responses, err := registry.ContainerEngine().PodStart(context.Background(), args, startOptions) + + ids, err := readPodIDFiles(startOptions.PodIDFiles) + if err != nil { + return err + } + args = append(args, ids...) + + responses, err := registry.ContainerEngine().PodStart(context.Background(), args, startOptions.PodStartOptions) if err != nil { return err } diff --git a/cmd/podman/pods/stop.go b/cmd/podman/pods/stop.go index daf05d640c59..fd66488f9ba4 100644 --- a/cmd/podman/pods/stop.go +++ b/cmd/podman/pods/stop.go @@ -11,7 +11,18 @@ import ( "github.com/spf13/cobra" ) +// allows for splitting API and CLI-only options +type podStopOptionsWrapper struct { + entities.PodStopOptions + + PodIDFiles []string + TimeoutCLI uint +} + var ( + stopOptions = podStopOptionsWrapper{ + PodStopOptions: entities.PodStopOptions{Timeout: -1}, + } podStopDescription = `The pod name or ID can be used. This command will stop all running containers in each of the specified pods.` @@ -22,7 +33,7 @@ var ( Long: podStopDescription, RunE: stop, Args: func(cmd *cobra.Command, args []string) error { - return parse.CheckAllLatestAndCIDFile(cmd, args, false, false) + return parse.CheckAllLatestAndPodIDFile(cmd, args, false, true) }, Example: `podman pod stop mywebserverpod podman pod stop --latest @@ -30,13 +41,6 @@ var ( } ) -var ( - stopOptions = entities.PodStopOptions{ - Timeout: -1, - } - timeout uint -) - func init() { registry.Commands = append(registry.Commands, registry.CliCommand{ Mode: []entities.EngineMode{entities.ABIMode, entities.TunnelMode}, @@ -47,7 +51,8 @@ func init() { flags.BoolVarP(&stopOptions.All, "all", "a", false, "Stop all running pods") flags.BoolVarP(&stopOptions.Ignore, "ignore", "i", false, "Ignore errors when a specified pod is missing") flags.BoolVarP(&stopOptions.Latest, "latest", "l", false, "Stop the latest pod podman is aware of") - flags.UintVarP(&timeout, "time", "t", containerConfig.Engine.StopTimeout, "Seconds to wait for pod stop before killing the container") + flags.UintVarP(&stopOptions.TimeoutCLI, "time", "t", containerConfig.Engine.StopTimeout, "Seconds to wait for pod stop before killing the container") + flags.StringArrayVarP(&stopOptions.PodIDFiles, "pod-id-file", "", nil, "Read the pod ID from the file") if registry.IsRemote() { _ = flags.MarkHidden("latest") _ = flags.MarkHidden("ignore") @@ -60,9 +65,16 @@ func stop(cmd *cobra.Command, args []string) error { errs utils.OutputErrors ) if cmd.Flag("time").Changed { - stopOptions.Timeout = int(timeout) + stopOptions.Timeout = int(stopOptions.TimeoutCLI) + } + + ids, err := readPodIDFiles(stopOptions.PodIDFiles) + if err != nil { + return err } - responses, err := registry.ContainerEngine().PodStop(context.Background(), args, stopOptions) + args = append(args, ids...) + + responses, err := registry.ContainerEngine().PodStop(context.Background(), args, stopOptions.PodStopOptions) if err != nil { return err } diff --git a/completions/bash/podman b/completions/bash/podman index a58becaf0a4b..6528281ba813 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=" diff --git a/docs/source/markdown/podman-pod-rm.1.md b/docs/source/markdown/podman-pod-rm.1.md index 14da2071ff7a..95e7ab00224d 100644 --- a/docs/source/markdown/podman-pod-rm.1.md +++ b/docs/source/markdown/podman-pod-rm.1.md @@ -31,6 +31,10 @@ The latest option is not supported on the remote client. Stop running containers and delete all stopped containers before removal of pod. +**--pod-id-file** + +Read pod ID from the specified file and remove the pod. Can be specified multiple times. + ## EXAMPLE podman pod rm mywebserverpod @@ -43,6 +47,8 @@ podman pod rm -f -a podman pod rm -fa +podman pod rm --pod-id-file /path/to/id/file + ## SEE ALSO podman-pod(1) diff --git a/docs/source/markdown/podman-pod-start.1.md b/docs/source/markdown/podman-pod-start.1.md index 29960d6aa23e..6c6cfa2cf7ed 100644 --- a/docs/source/markdown/podman-pod-start.1.md +++ b/docs/source/markdown/podman-pod-start.1.md @@ -22,6 +22,10 @@ Instead of providing the pod name or ID, start the last created pod. The latest option is not supported on the remote client. +**--pod-id-file** + +Read pod ID from the specified file and start the pod. Can be specified multiple times. + ## EXAMPLE podman pod start mywebserverpod @@ -32,6 +36,7 @@ podman pod start --latest podman pod start --all +podman pod start --pod-id-file /path/to/id/file ## SEE ALSO podman-pod(1), podman-pod-stop(1), podman-start(1) diff --git a/docs/source/markdown/podman-pod-stop.1.md b/docs/source/markdown/podman-pod-stop.1.md index b5e7aef7d278..7ce9ff941013 100644 --- a/docs/source/markdown/podman-pod-stop.1.md +++ b/docs/source/markdown/podman-pod-stop.1.md @@ -31,6 +31,10 @@ The latest option is not supported on the remote client. Timeout to wait before forcibly stopping the containers in the pod. +**--pod-id-file** + +Read pod ID from the specified file and stop the pod. Can be specified multiple times. + ## EXAMPLE Stop a pod called *mywebserverpod* @@ -62,6 +66,13 @@ $ podman pod stop --all cc8f0bea67b1a1a11aec1ecd38102a1be4b145577f21fc843c7c83b77fc28907 ``` +Stop two pods via --pod-id-file +``` +$ podman pod stop --pod-id-file file1 --pod-id-file file2 +19456b4cd557eaf9629825113a552681a6013f8c8cad258e36ab825ef536e818 +cc8f0bea67b1a1a11aec1ecd38102a1be4b145577f21fc843c7c83b77fc28907 +``` + Stop all pods with a timeout of 1 second. ``` $ podman pod stop -a -t 1 diff --git a/pkg/domain/infra/abi/pods.go b/pkg/domain/infra/abi/pods.go index eb6f1e191d99..054b59b0673d 100644 --- a/pkg/domain/infra/abi/pods.go +++ b/pkg/domain/infra/abi/pods.go @@ -144,6 +144,7 @@ func (ic *ContainerEngine) PodStop(ctx context.Context, namesOrIds []string, opt var ( reports []*entities.PodStopReport ) + pods, err := getPodsByContext(options.All, options.Latest, namesOrIds, ic.Libpod) if err != nil && !(options.Ignore && errors.Cause(err) == define.ErrNoSuchPod) { return nil, err @@ -199,10 +200,12 @@ func (ic *ContainerEngine) PodStart(ctx context.Context, namesOrIds []string, op var ( reports []*entities.PodStartReport ) + pods, err := getPodsByContext(options.All, options.Latest, namesOrIds, ic.Libpod) if err != nil { return nil, err } + for _, p := range pods { report := entities.PodStartReport{Id: p.ID()} errs, err := p.Start(ctx) @@ -227,6 +230,7 @@ func (ic *ContainerEngine) PodRm(ctx context.Context, namesOrIds []string, optio var ( reports []*entities.PodRmReport ) + pods, err := getPodsByContext(options.All, options.Latest, namesOrIds, ic.Libpod) if err != nil && !(options.Ignore && errors.Cause(err) == define.ErrNoSuchPod) { return nil, err diff --git a/test/e2e/pod_rm_test.go b/test/e2e/pod_rm_test.go index 4060e1268933..d0ece7b53e30 100644 --- a/test/e2e/pod_rm_test.go +++ b/test/e2e/pod_rm_test.go @@ -2,6 +2,7 @@ package integration import ( "fmt" + "io/ioutil" "os" "path/filepath" "strings" @@ -229,4 +230,72 @@ var _ = Describe("Podman pod rm", func() { session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) }) + + It("podman pod start/remove single pod via --pod-id-file", func() { + tmpDir, err := ioutil.TempDir("", "") + Expect(err).To(BeNil()) + tmpFile := tmpDir + "podID" + defer os.RemoveAll(tmpDir) + + podName := "rudolph" + + // Create a pod with --pod-id-file. + session := podmanTest.Podman([]string{"pod", "create", "--name", podName, "--pod-id-file", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Create container inside the pod. + session = podmanTest.Podman([]string{"create", "--pod", podName, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"pod", "start", "--pod-id-file", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(2)) // infra+top + + session = podmanTest.Podman([]string{"pod", "rm", "--pod-id-file", tmpFile, "--force"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) + }) + + It("podman pod start/remove multiple pods via --pod-id-file", func() { + tmpDir, err := ioutil.TempDir("", "") + Expect(err).To(BeNil()) + defer os.RemoveAll(tmpDir) + + podIDFiles := []string{} + for _, i := range "0123456789" { + tmpFile := tmpDir + "cid" + string(i) + podName := "rudolph" + string(i) + // Create a pod with --pod-id-file. + session := podmanTest.Podman([]string{"pod", "create", "--name", podName, "--pod-id-file", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Create container inside the pod. + session = podmanTest.Podman([]string{"create", "--pod", podName, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Append the id files along with the command. + podIDFiles = append(podIDFiles, "--pod-id-file") + podIDFiles = append(podIDFiles, tmpFile) + } + + cmd := []string{"pod", "start"} + cmd = append(cmd, podIDFiles...) + session := podmanTest.Podman(cmd) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(20)) // 10*(infra+top) + + cmd = []string{"pod", "rm", "--force"} + cmd = append(cmd, podIDFiles...) + session = podmanTest.Podman(cmd) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) + }) }) diff --git a/test/e2e/pod_start_test.go b/test/e2e/pod_start_test.go index 8e78cadfd374..4502a76ed51e 100644 --- a/test/e2e/pod_start_test.go +++ b/test/e2e/pod_start_test.go @@ -1,6 +1,7 @@ package integration import ( + "io/ioutil" "os" . "github.com/containers/libpod/test/utils" @@ -136,4 +137,60 @@ var _ = Describe("Podman pod start", func() { session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(125)) }) + + It("podman pod start single pod via --pod-id-file", func() { + tmpDir, err := ioutil.TempDir("", "") + Expect(err).To(BeNil()) + tmpFile := tmpDir + "podID" + defer os.RemoveAll(tmpDir) + + podName := "rudolph" + + // Create a pod with --pod-id-file. + session := podmanTest.Podman([]string{"pod", "create", "--name", podName, "--pod-id-file", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Create container inside the pod. + session = podmanTest.Podman([]string{"create", "--pod", podName, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"pod", "start", "--pod-id-file", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(2)) // infra+top + }) + + It("podman pod start multiple pods via --pod-id-file", func() { + tmpDir, err := ioutil.TempDir("", "") + Expect(err).To(BeNil()) + defer os.RemoveAll(tmpDir) + + podIDFiles := []string{} + for _, i := range "0123456789" { + tmpFile := tmpDir + "cid" + string(i) + podName := "rudolph" + string(i) + // Create a pod with --pod-id-file. + session := podmanTest.Podman([]string{"pod", "create", "--name", podName, "--pod-id-file", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Create container inside the pod. + session = podmanTest.Podman([]string{"create", "--pod", podName, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Append the id files along with the command. + podIDFiles = append(podIDFiles, "--pod-id-file") + podIDFiles = append(podIDFiles, tmpFile) + } + + cmd := []string{"pod", "start"} + cmd = append(cmd, podIDFiles...) + session := podmanTest.Podman(cmd) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(20)) // 10*(infra+top) + }) }) diff --git a/test/e2e/pod_stop_test.go b/test/e2e/pod_stop_test.go index 0a46b07c93ad..0fe580921202 100644 --- a/test/e2e/pod_stop_test.go +++ b/test/e2e/pod_stop_test.go @@ -1,6 +1,7 @@ package integration import ( + "io/ioutil" "os" . "github.com/containers/libpod/test/utils" @@ -175,4 +176,72 @@ var _ = Describe("Podman pod stop", func() { session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(125)) }) + + It("podman pod start/stop single pod via --pod-id-file", func() { + tmpDir, err := ioutil.TempDir("", "") + Expect(err).To(BeNil()) + tmpFile := tmpDir + "podID" + defer os.RemoveAll(tmpDir) + + podName := "rudolph" + + // Create a pod with --pod-id-file. + session := podmanTest.Podman([]string{"pod", "create", "--name", podName, "--pod-id-file", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Create container inside the pod. + session = podmanTest.Podman([]string{"create", "--pod", podName, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"pod", "start", "--pod-id-file", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(2)) // infra+top + + session = podmanTest.Podman([]string{"pod", "stop", "--pod-id-file", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) + }) + + It("podman pod start/stop multiple pods via --pod-id-file", func() { + tmpDir, err := ioutil.TempDir("", "") + Expect(err).To(BeNil()) + defer os.RemoveAll(tmpDir) + + podIDFiles := []string{} + for _, i := range "0123456789" { + tmpFile := tmpDir + "cid" + string(i) + podName := "rudolph" + string(i) + // Create a pod with --pod-id-file. + session := podmanTest.Podman([]string{"pod", "create", "--name", podName, "--pod-id-file", tmpFile}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Create container inside the pod. + session = podmanTest.Podman([]string{"create", "--pod", podName, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Append the id files along with the command. + podIDFiles = append(podIDFiles, "--pod-id-file") + podIDFiles = append(podIDFiles, tmpFile) + } + + cmd := []string{"pod", "start"} + cmd = append(cmd, podIDFiles...) + session := podmanTest.Podman(cmd) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(20)) // 10*(infra+top) + + cmd = []string{"pod", "stop"} + cmd = append(cmd, podIDFiles...) + session = podmanTest.Podman(cmd) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) + }) }) From cf89bb671184e453c4ba5f27e26d02216d8fc491 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 29 May 2020 10:35:22 +0200 Subject: [PATCH 04/14] 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 diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index 86cd51643e3e..e79c5c20b048 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 4cba5daf774b..98dc6744cb84 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 2286e67de714..fee9d8c7bbea 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 a3626b4e4545..422e241af7a3 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 1c4195095e49..000000000000 --- 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 ecceda32aa18..8de0bce9e3dc 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 86517190db12..97020b360d93 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 fd66488f9ba4..628e8a536629 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 6528281ba813..0e4b60b14956 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 a69ef04d10dd..81c83369affd 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 02db8b205a9c..4f43e4c191a3 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 f40472a7c396..b9a1ff83d6df 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}) From 144c6bb76d7652f53167d4bc7130cf897f5903a1 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 29 May 2020 11:40:28 +0200 Subject: [PATCH 05/14] generate systemd: rename "cid" to "ctr-id" Rename the container ID file from "cid" to "ctr-id" to make the generated unit files a) easier to read and to b) pro-actively avoid any confusion when pod ID files are being added in the future. Signed-off-by: Valentin Rothberg --- pkg/systemd/generate/systemdgen.go | 8 ++++---- pkg/systemd/generate/systemdgen_test.go | 24 ++++++++++++------------ test/e2e/generate_systemd_test.go | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/pkg/systemd/generate/systemdgen.go b/pkg/systemd/generate/systemdgen.go index 538e54ec9cd2..fb0ea5cf9490 100644 --- a/pkg/systemd/generate/systemdgen.go +++ b/pkg/systemd/generate/systemdgen.go @@ -101,10 +101,10 @@ Before={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ Environment={{.EnvVariable}}=%n Restart={{.RestartPolicy}} {{- if .New}} -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-cid +ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id ExecStart={{.RunCommand}} -ExecStop={{.Executable}} stop --ignore --cidfile %t/%n-cid {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} -ExecStopPost={{.Executable}} rm --ignore -f --cidfile %t/%n-cid +ExecStop={{.Executable}} stop --ignore --cidfile %t/%n-ctr-id {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} +ExecStopPost={{.Executable}} rm --ignore -f --cidfile %t/%n-ctr-id PIDFile=%t/%n-pid {{- else}} ExecStart={{.Executable}} start {{.ContainerName}} @@ -168,7 +168,7 @@ func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, erro info.Executable, "run", "--conmon-pidfile", "%t/%n-pid", - "--cidfile", "%t/%n-cid", + "--cidfile", "%t/%n-ctr-id", "--cgroups=no-conmon", } diff --git a/pkg/systemd/generate/systemdgen_test.go b/pkg/systemd/generate/systemdgen_test.go index cc5db5e244ee..9da261a69aaa 100644 --- a/pkg/systemd/generate/systemdgen_test.go +++ b/pkg/systemd/generate/systemdgen_test.go @@ -136,10 +136,10 @@ After=network-online.target [Service] Environment=PODMAN_SYSTEMD_UNIT=%n Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-cid -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-cid --cgroups=no-conmon -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-cid -t 42 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-cid +ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 42 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id PIDFile=%t/%n-pid KillMode=none Type=forking @@ -159,10 +159,10 @@ After=network-online.target [Service] Environment=PODMAN_SYSTEMD_UNIT=%n Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-cid -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-cid --cgroups=no-conmon --detach --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-cid -t 42 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-cid +ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon --detach --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 42 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id PIDFile=%t/%n-pid KillMode=none Type=forking @@ -182,10 +182,10 @@ After=network-online.target [Service] Environment=PODMAN_SYSTEMD_UNIT=%n Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-cid -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-cid --cgroups=no-conmon -d awesome-image:latest -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-cid -t 10 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-cid +ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon -d awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 10 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id PIDFile=%t/%n-pid KillMode=none Type=forking diff --git a/test/e2e/generate_systemd_test.go b/test/e2e/generate_systemd_test.go index d5ae441e222e..64d220776409 100644 --- a/test/e2e/generate_systemd_test.go +++ b/test/e2e/generate_systemd_test.go @@ -191,7 +191,7 @@ var _ = Describe("Podman generate systemd", func() { found, _ := session.GrepString("# container-foo.service") Expect(found).To(BeTrue()) - found, _ = session.GrepString("stop --ignore --cidfile %t/%n-cid -t 42") + found, _ = session.GrepString("stop --ignore --cidfile %t/%n-ctr-id -t 42") Expect(found).To(BeTrue()) }) From 402c68b41d3fd8f354a4fd0ba4d4101920f8cfe6 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 29 May 2020 13:20:22 +0200 Subject: [PATCH 06/14] 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(-) diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go index 5ed5fa57ca8a..51b7a7d52080 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 0e4b60b14956..6dbe645feb66 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 489c9b32ee5b..de6b600f09d9 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 75d09881580c..5a0f600931a7 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 38fe1fd2c07c..7af78fa07bda 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 06a7b3936ac7..a0dee3aa1211 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 a85333c753fa..fc76ddd4168c 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 51b7835b2095..5ccb1ba80786 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 11976233a393..600d27004859 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 4502a76ed51e..99285d1e1f0d 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 + }) + }) From 9232d4e44365e0fa344fc5432dfd4843cb4c200c Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Jun 2020 14:57:11 +0200 Subject: [PATCH 07/14] generate systemd: rephrase lookup error Rephrase the lookup error when the specified name or ID does not refer to a container or pod. Until, only the pod-lookup error has been returned which can be confusing when actually looking for a container; a user might have just mistyped the ID or name. Signed-off-by: Valentin Rothberg --- pkg/domain/infra/abi/generate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/domain/infra/abi/generate.go b/pkg/domain/infra/abi/generate.go index abb5e291112c..08cb87287b7b 100644 --- a/pkg/domain/infra/abi/generate.go +++ b/pkg/domain/infra/abi/generate.go @@ -40,7 +40,7 @@ func (ic *ContainerEngine) GenerateSystemd(ctx context.Context, nameOrID string, // We're either having a pod or garbage. pod, err := ic.Libpod.LookupPod(nameOrID) if err != nil { - return nil, err + return nil, errors.Errorf("%q does not refer to a container or pod", nameOrID) } // Error out if the pod has no infra container, which we require to be the From 557a9c12111f8bebaf1466bc267ef8cb992d1c19 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Jun 2020 15:00:57 +0200 Subject: [PATCH 08/14] generate systemd: rename source files Rename to `containers{_test}.go` to make some place for the upcoming pod changes. Signed-off-by: Valentin Rothberg --- pkg/systemd/generate/{systemdgen.go => containers.go} | 0 pkg/systemd/generate/{systemdgen_test.go => containers_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename pkg/systemd/generate/{systemdgen.go => containers.go} (100%) rename pkg/systemd/generate/{systemdgen_test.go => containers_test.go} (100%) diff --git a/pkg/systemd/generate/systemdgen.go b/pkg/systemd/generate/containers.go similarity index 100% rename from pkg/systemd/generate/systemdgen.go rename to pkg/systemd/generate/containers.go diff --git a/pkg/systemd/generate/systemdgen_test.go b/pkg/systemd/generate/containers_test.go similarity index 100% rename from pkg/systemd/generate/systemdgen_test.go rename to pkg/systemd/generate/containers_test.go From b4a410215ef0d6ebac6e5b61921f2cbe5d9ac8bd Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Jun 2020 15:34:04 +0200 Subject: [PATCH 09/14] add (*Pod).CreateCommand() Add a method to Pod to easily access its .config.CreateCommand. Signed-off-by: Valentin Rothberg --- libpod/pod.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libpod/pod.go b/libpod/pod.go index 7af78fa07bda..db87e60f217e 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -129,6 +129,12 @@ func (p *Pod) CreatedTime() time.Time { return p.config.CreatedTime } +// CreateCommand returns the os.Args of the process with which the pod has been +// created. +func (p *Pod) CreateCommand() []string { + return p.config.CreateCommand +} + // CgroupParent returns the pod's CGroup parent func (p *Pod) CgroupParent() string { return p.config.CgroupParent From 35ae53067f01c0194dc13513656e57293de95004 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Jun 2020 16:57:58 +0200 Subject: [PATCH 10/14] generate systemd: refactor Refactor the systemd-unit generation code and move all the logic into `pkg/systemd/generate`. The code was already hard to maintain but I found it impossible to wire the `--new` logic for pods in all the chaos. The code refactoring in this commit will make maintaining the code easier and should make it easier to extend as well. Further changes and refactorings may still be needed but they will easier. Signed-off-by: Valentin Rothberg --- libpod/pod.go | 14 ++ pkg/domain/infra/abi/generate.go | 158 +-------------- pkg/systemd/generate/common.go | 36 ++++ pkg/systemd/generate/containers.go | 109 ++++++----- .../{containers_test.go => generate_test.go} | 182 +++++++++--------- pkg/systemd/generate/pods.go | 130 +++++++++++++ 6 files changed, 347 insertions(+), 282 deletions(-) create mode 100644 pkg/systemd/generate/common.go rename pkg/systemd/generate/{containers_test.go => generate_test.go} (57%) create mode 100644 pkg/systemd/generate/pods.go diff --git a/libpod/pod.go b/libpod/pod.go index db87e60f217e..f53290876c2c 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -257,6 +257,20 @@ func (p *Pod) InfraContainerID() (string, error) { return p.state.InfraContainerID, nil } +// InfraContainer returns the infra container. +func (p *Pod) InfraContainer() (*Container, error) { + if !p.HasInfraContainer() { + return nil, errors.New("pod has no infra container") + } + + id, err := p.InfraContainerID() + if err != nil { + return nil, err + } + + return p.runtime.state.Container(id) +} + // TODO add pod batching // Lock pod to avoid lock contention // Store and lock all containers (no RemoveContainer in batch guarantees cache will not become stale) diff --git a/pkg/domain/infra/abi/generate.go b/pkg/domain/infra/abi/generate.go index 08cb87287b7b..fa0cfb389847 100644 --- a/pkg/domain/infra/abi/generate.go +++ b/pkg/domain/infra/abi/generate.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "fmt" - "strings" "github.com/containers/libpod/libpod" "github.com/containers/libpod/libpod/define" @@ -16,165 +15,28 @@ import ( ) func (ic *ContainerEngine) GenerateSystemd(ctx context.Context, nameOrID string, options entities.GenerateSystemdOptions) (*entities.GenerateSystemdReport, error) { - opts := generate.Options{ - Files: options.Files, - New: options.New, - } - // First assume it's a container. - if info, found, err := ic.generateSystemdgenContainerInfo(nameOrID, nil, options); found && err != nil { - return nil, err - } else if found && err == nil { - output, err := generate.CreateContainerSystemdUnit(info, opts) - if err != nil { - return nil, err + ctr, err := ic.Libpod.LookupContainer(nameOrID) + if err == nil { + // Generate the unit for the container. + s, err := generate.ContainerUnit(ctr, options) + if err == nil { + return &entities.GenerateSystemdReport{Output: s}, nil } - return &entities.GenerateSystemdReport{Output: output}, nil - } - - // --new does not support pods. - if options.New { - return nil, errors.Errorf("error generating systemd unit files: cannot generate generic files for a pod") } - // We're either having a pod or garbage. + // If it's not a container, we either have a pod or garbage. pod, err := ic.Libpod.LookupPod(nameOrID) if err != nil { return nil, errors.Errorf("%q does not refer to a container or pod", nameOrID) } - // Error out if the pod has no infra container, which we require to be the - // main service. - if !pod.HasInfraContainer() { - return nil, fmt.Errorf("error generating systemd unit files: Pod %q has no infra container", pod.Name()) - } - - // Generate a systemdgen.ContainerInfo for the infra container. This - // ContainerInfo acts as the main service of the pod. - infraID, err := pod.InfraContainerID() - if err != nil { - return nil, nil - } - podInfo, _, err := ic.generateSystemdgenContainerInfo(infraID, pod, options) + // Generate the units for the pod and all its containers. + s, err := generate.PodUnits(pod, options) if err != nil { return nil, err } - - // Compute the container-dependency graph for the Pod. - containers, err := pod.AllContainers() - if err != nil { - return nil, err - } - if len(containers) == 0 { - return nil, fmt.Errorf("error generating systemd unit files: Pod %q has no containers", pod.Name()) - } - graph, err := libpod.BuildContainerGraph(containers) - if err != nil { - return nil, err - } - - // Traverse the dependency graph and create systemdgen.ContainerInfo's for - // each container. - containerInfos := []*generate.ContainerInfo{podInfo} - for ctr, dependencies := range graph.DependencyMap() { - // Skip the infra container as we already generated it. - if ctr.ID() == infraID { - continue - } - ctrInfo, _, err := ic.generateSystemdgenContainerInfo(ctr.ID(), nil, options) - if err != nil { - return nil, err - } - // Now add the container's dependencies and at the container as a - // required service of the infra container. - for _, dep := range dependencies { - if dep.ID() == infraID { - ctrInfo.BoundToServices = append(ctrInfo.BoundToServices, podInfo.ServiceName) - } else { - _, serviceName := generateServiceName(dep, nil, options) - ctrInfo.BoundToServices = append(ctrInfo.BoundToServices, serviceName) - } - } - podInfo.RequiredServices = append(podInfo.RequiredServices, ctrInfo.ServiceName) - containerInfos = append(containerInfos, ctrInfo) - } - - // Now generate the systemd service for all containers. - builder := strings.Builder{} - for i, info := range containerInfos { - if i > 0 { - builder.WriteByte('\n') - } - out, err := generate.CreateContainerSystemdUnit(info, opts) - if err != nil { - return nil, err - } - builder.WriteString(out) - } - - return &entities.GenerateSystemdReport{Output: builder.String()}, nil -} - -// generateSystemdgenContainerInfo is a helper to generate a -// systemdgen.ContainerInfo for `GenerateSystemd`. -func (ic *ContainerEngine) generateSystemdgenContainerInfo(nameOrID string, pod *libpod.Pod, options entities.GenerateSystemdOptions) (*generate.ContainerInfo, bool, error) { - ctr, err := ic.Libpod.LookupContainer(nameOrID) - if err != nil { - return nil, false, err - } - - timeout := ctr.StopTimeout() - if options.StopTimeout != nil { - timeout = *options.StopTimeout - } - - config := ctr.Config() - conmonPidFile := config.ConmonPidFile - if conmonPidFile == "" { - return nil, true, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") - } - - createCommand := []string{} - if config.CreateCommand != nil { - createCommand = config.CreateCommand - } else if options.New { - return nil, true, errors.Errorf("cannot use --new on container %q: no create command found", nameOrID) - } - - name, serviceName := generateServiceName(ctr, pod, options) - info := &generate.ContainerInfo{ - ServiceName: serviceName, - ContainerName: name, - RestartPolicy: options.RestartPolicy, - PIDFile: conmonPidFile, - StopTimeout: timeout, - GenerateTimestamp: true, - CreateCommand: createCommand, - } - - return info, true, nil -} - -// generateServiceName generates the container name and the service name for systemd service. -func generateServiceName(ctr *libpod.Container, pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, string) { - var kind, name, ctrName string - if pod == nil { - kind = options.ContainerPrefix //defaults to container - name = ctr.ID() - if options.Name { - name = ctr.Name() - } - ctrName = name - } else { - kind = options.PodPrefix //defaults to pod - name = pod.ID() - ctrName = ctr.ID() - if options.Name { - name = pod.Name() - ctrName = ctr.Name() - } - } - return ctrName, fmt.Sprintf("%s%s%s", kind, options.Separator, name) + return &entities.GenerateSystemdReport{Output: s}, nil } func (ic *ContainerEngine) GenerateKube(ctx context.Context, nameOrID string, options entities.GenerateKubeOptions) (*entities.GenerateKubeReport, error) { diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go new file mode 100644 index 000000000000..e809b483762c --- /dev/null +++ b/pkg/systemd/generate/common.go @@ -0,0 +1,36 @@ +package generate + +import ( + "github.com/pkg/errors" +) + +// EnvVariable "PODMAN_SYSTEMD_UNIT" is set in all generated systemd units and +// is set to the unit's (unique) name. +const EnvVariable = "PODMAN_SYSTEMD_UNIT" + +// restartPolicies includes all valid restart policies to be used in a unit +// file. +var restartPolicies = []string{"no", "on-success", "on-failure", "on-abnormal", "on-watchdog", "on-abort", "always"} + +// validateRestartPolicy checks that the user-provided policy is valid. +func validateRestartPolicy(restart string) error { + for _, i := range restartPolicies { + if i == restart { + return nil + } + } + return errors.Errorf("%s is not a valid restart policy", restart) +} + +const headerTemplate = `# {{.ServiceName}}.service +# autogenerated by Podman {{.PodmanVersion}} +{{- if .TimeStamp}} +# {{.TimeStamp}} +{{- end}} + +[Unit] +Description=Podman {{.ServiceName}}.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +` diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index fb0ea5cf9490..f316d44524d9 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -11,22 +11,20 @@ import ( "text/template" "time" + "github.com/containers/libpod/libpod" + "github.com/containers/libpod/pkg/domain/entities" "github.com/containers/libpod/version" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) -// EnvVariable "PODMAN_SYSTEMD_UNIT" is set in all generated systemd units and -// is set to the unit's (unique) name. -const EnvVariable = "PODMAN_SYSTEMD_UNIT" - -// ContainerInfo contains data required for generating a container's systemd +// containerInfo contains data required for generating a container's systemd // unit file. -type ContainerInfo struct { +type containerInfo struct { // ServiceName of the systemd service. ServiceName string // Name or ID of the container. - ContainerName string + ContainerNameOrID string // StopTimeout sets the timeout Podman waits before killing the container // during service stop. StopTimeout uint @@ -63,29 +61,7 @@ type ContainerInfo struct { EnvVariable string } -var restartPolicies = []string{"no", "on-success", "on-failure", "on-abnormal", "on-watchdog", "on-abort", "always"} - -// validateRestartPolicy checks that the user-provided policy is valid. -func validateRestartPolicy(restart string) error { - for _, i := range restartPolicies { - if i == restart { - return nil - } - } - return errors.Errorf("%s is not a valid restart policy", restart) -} - -const containerTemplate = `# {{.ServiceName}}.service -# autogenerated by Podman {{.PodmanVersion}} -{{- if .TimeStamp}} -# {{.TimeStamp}} -{{- end}} - -[Unit] -Description=Podman {{.ServiceName}}.service -Documentation=man:podman-generate-systemd(1) -Wants=network.target -After=network-online.target +const containerTemplate = headerTemplate + ` {{- if .BoundToServices}} RefuseManualStart=yes RefuseManualStop=yes @@ -107,8 +83,8 @@ ExecStop={{.Executable}} stop --ignore --cidfile %t/%n-ctr-id {{if (ge .StopTime ExecStopPost={{.Executable}} rm --ignore -f --cidfile %t/%n-ctr-id PIDFile=%t/%n-pid {{- else}} -ExecStart={{.Executable}} start {{.ContainerName}} -ExecStop={{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerName}} +ExecStart={{.Executable}} start {{.ContainerNameOrID}} +ExecStop={{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}} PIDFile={{.PIDFile}} {{- end}} KillMode=none @@ -117,18 +93,19 @@ Type=forking [Install] WantedBy=multi-user.target default.target` -// Options include different options to control the unit file generation. -type Options struct { - // When set, generate service files in the current working directory and - // return the paths to these files instead of returning all contents in one - // big string. - Files bool - // New controls if a new container is created or if an existing one is started. - New bool +// ContainerUnit generates a systemd unit for the specified container. Based +// on the options, the return value might be the entire unit or a file it has +// been written to. +func ContainerUnit(ctr *libpod.Container, options entities.GenerateSystemdOptions) (string, error) { + info, err := generateContainerInfo(ctr, options) + if err != nil { + return "", err + } + return createContainerSystemdUnit(info, options) } -// CreateContainerSystemdUnit creates a systemd unit file for a container. -func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, error) { +// createContainerSystemdUnit creates a systemd unit file for a container. +func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSystemdOptions) (string, error) { if err := validateRestartPolicy(info.RestartPolicy); err != nil { return "", err } @@ -152,7 +129,7 @@ func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, erro // been created via a Python script, which would certainly yield an // invalid `info.CreateCommand`. Hence, we're doing a best effort unit // generation and don't try aiming at completeness. - if opts.New { + if options.New { // The create command must at least have three arguments: // /usr/bin/podman run $IMAGE index := 2 @@ -218,7 +195,7 @@ func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, erro return "", err } - if !opts.Files { + if !options.Files { return buf.String(), nil } @@ -233,3 +210,47 @@ func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, erro } return path, nil } + +func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSystemdOptions) (*containerInfo, error) { + timeout := ctr.StopTimeout() + if options.StopTimeout != nil { + timeout = *options.StopTimeout + } + + config := ctr.Config() + conmonPidFile := config.ConmonPidFile + if conmonPidFile == "" { + return nil, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") + } + + createCommand := []string{} + if config.CreateCommand != nil { + createCommand = config.CreateCommand + } else if options.New { + return nil, errors.Errorf("cannot use --new on container %q: no create command found", ctr.ID()) + } + + nameOrID, serviceName := containerServiceName(ctr, options) + + info := containerInfo{ + ServiceName: serviceName, + ContainerNameOrID: nameOrID, + RestartPolicy: options.RestartPolicy, + PIDFile: conmonPidFile, + StopTimeout: timeout, + GenerateTimestamp: true, + CreateCommand: createCommand, + } + return &info, nil +} + +// containerServiceName returns the nameOrID and the service name of the +// container. +func containerServiceName(ctr *libpod.Container, options entities.GenerateSystemdOptions) (string, string) { + nameOrID := ctr.ID() + if options.Name { + nameOrID = ctr.Name() + } + serviceName := fmt.Sprintf("%s%s%s", options.ContainerPrefix, options.Separator, nameOrID) + return nameOrID, serviceName +} diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/generate_test.go similarity index 57% rename from pkg/systemd/generate/containers_test.go rename to pkg/systemd/generate/generate_test.go index 9da261a69aaa..11cabb463145 100644 --- a/pkg/systemd/generate/containers_test.go +++ b/pkg/systemd/generate/generate_test.go @@ -2,31 +2,33 @@ package generate import ( "testing" + + "github.com/containers/libpod/pkg/domain/entities" ) func TestValidateRestartPolicy(t *testing.T) { - type ContainerInfo struct { + type containerInfo struct { restart string } tests := []struct { name string - ContainerInfo ContainerInfo + containerInfo containerInfo wantErr bool }{ - {"good-on", ContainerInfo{restart: "no"}, false}, - {"good-on-success", ContainerInfo{restart: "on-success"}, false}, - {"good-on-failure", ContainerInfo{restart: "on-failure"}, false}, - {"good-on-abnormal", ContainerInfo{restart: "on-abnormal"}, false}, - {"good-on-watchdog", ContainerInfo{restart: "on-watchdog"}, false}, - {"good-on-abort", ContainerInfo{restart: "on-abort"}, false}, - {"good-always", ContainerInfo{restart: "always"}, false}, - {"fail", ContainerInfo{restart: "foobar"}, true}, - {"failblank", ContainerInfo{restart: ""}, true}, + {"good-on", containerInfo{restart: "no"}, false}, + {"good-on-success", containerInfo{restart: "on-success"}, false}, + {"good-on-failure", containerInfo{restart: "on-failure"}, false}, + {"good-on-abnormal", containerInfo{restart: "on-abnormal"}, false}, + {"good-on-watchdog", containerInfo{restart: "on-watchdog"}, false}, + {"good-on-abort", containerInfo{restart: "on-abort"}, false}, + {"good-always", containerInfo{restart: "always"}, false}, + {"fail", containerInfo{restart: "foobar"}, true}, + {"failblank", containerInfo{restart: ""}, true}, } for _, tt := range tests { test := tt t.Run(tt.name, func(t *testing.T) { - if err := validateRestartPolicy(test.ContainerInfo.restart); (err != nil) != test.wantErr { + if err := validateRestartPolicy(test.containerInfo.restart); (err != nil) != test.wantErr { t.Errorf("ValidateRestartPolicy() error = %v, wantErr %v", err, test.wantErr) } }) @@ -195,67 +197,67 @@ WantedBy=multi-user.target default.target` tests := []struct { name string - info ContainerInfo + info containerInfo want string wantErr bool }{ {"good with id", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - ContainerName: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + ContainerNameOrID: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", }, goodID, false, }, {"good with name", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-foobar", - ContainerName: "foobar", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-foobar", + ContainerNameOrID: "foobar", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", }, goodName, false, }, {"good with name and bound to", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-foobar", - ContainerName: "foobar", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - BoundToServices: []string{"pod", "a", "b", "c"}, + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-foobar", + ContainerNameOrID: "foobar", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + BoundToServices: []string{"pod", "a", "b", "c"}, }, goodNameBoundTo, false, }, {"pod", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "pod-123abc", - ContainerName: "jadda-jadda-infra", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - RequiredServices: []string{"container-1", "container-2"}, + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "pod-123abc", + ContainerNameOrID: "jadda-jadda-infra", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + RequiredServices: []string{"container-1", "container-2"}, }, podGoodName, false, }, {"bad restart policy", - ContainerInfo{ + containerInfo{ Executable: "/usr/bin/podman", ServiceName: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", RestartPolicy: "never", @@ -267,61 +269,61 @@ WantedBy=multi-user.target default.target` true, }, {"good with name and generic", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "jadda-jadda", - ContainerName: "jadda-jadda", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 42, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + New: true, + CreateCommand: []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, }, goodNameNew, false, }, {"good with explicit short detach param", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "jadda-jadda", - ContainerName: "jadda-jadda", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 42, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + New: true, + CreateCommand: []string{"I'll get stripped", "container", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, }, goodNameNew, false, }, {"good with explicit full detach param", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "jadda-jadda", - ContainerName: "jadda-jadda", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 42, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "--detach", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + New: true, + CreateCommand: []string{"I'll get stripped", "container", "run", "--detach", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, }, goodNameNewDetach, false, }, {"good with id and no param", - ContainerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - ContainerName: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - New: true, - CreateCommand: []string{"I'll get stripped", "container", "run", "awesome-image:latest"}, + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + ContainerNameOrID: "639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + New: true, + CreateCommand: []string{"I'll get stripped", "container", "run", "awesome-image:latest"}, }, goodIDNew, false, @@ -330,11 +332,11 @@ WantedBy=multi-user.target default.target` for _, tt := range tests { test := tt t.Run(tt.name, func(t *testing.T) { - opts := Options{ + opts := entities.GenerateSystemdOptions{ Files: false, New: test.info.New, } - got, err := CreateContainerSystemdUnit(&test.info, opts) + got, err := createContainerSystemdUnit(&test.info, opts) if (err != nil) != test.wantErr { t.Errorf("CreateContainerSystemdUnit() error = \n%v, wantErr \n%v", err, test.wantErr) return diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go new file mode 100644 index 000000000000..16f324f7859b --- /dev/null +++ b/pkg/systemd/generate/pods.go @@ -0,0 +1,130 @@ +package generate + +import ( + "fmt" + "strings" + + "github.com/containers/libpod/libpod" + "github.com/containers/libpod/pkg/domain/entities" + "github.com/pkg/errors" +) + +// PodUnits generates systemd units for the specified pod and its containers. +// Based on the options, the return value might be the content of all units or +// the files they been written to. +func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, error) { + if options.New { + return "", errors.New("--new is not supported for pods") + } + + // Error out if the pod has no infra container, which we require to be the + // main service. + if !pod.HasInfraContainer() { + return "", fmt.Errorf("error generating systemd unit files: Pod %q has no infra container", pod.Name()) + } + + podInfo, err := generatePodInfo(pod, options) + if err != nil { + return "", err + } + + infraID, err := pod.InfraContainerID() + if err != nil { + return "", err + } + + // Compute the container-dependency graph for the Pod. + containers, err := pod.AllContainers() + if err != nil { + return "", err + } + if len(containers) == 0 { + return "", fmt.Errorf("error generating systemd unit files: Pod %q has no containers", pod.Name()) + } + graph, err := libpod.BuildContainerGraph(containers) + if err != nil { + return "", err + } + + // Traverse the dependency graph and create systemdgen.containerInfo's for + // each container. + containerInfos := []*containerInfo{podInfo} + for ctr, dependencies := range graph.DependencyMap() { + // Skip the infra container as we already generated it. + if ctr.ID() == infraID { + continue + } + ctrInfo, err := generateContainerInfo(ctr, options) + if err != nil { + return "", err + } + // Now add the container's dependencies and at the container as a + // required service of the infra container. + for _, dep := range dependencies { + if dep.ID() == infraID { + ctrInfo.BoundToServices = append(ctrInfo.BoundToServices, podInfo.ServiceName) + } else { + _, serviceName := containerServiceName(dep, options) + ctrInfo.BoundToServices = append(ctrInfo.BoundToServices, serviceName) + } + } + podInfo.RequiredServices = append(podInfo.RequiredServices, ctrInfo.ServiceName) + containerInfos = append(containerInfos, ctrInfo) + } + + // Now generate the systemd service for all containers. + builder := strings.Builder{} + for i, info := range containerInfos { + if i > 0 { + builder.WriteByte('\n') + } + out, err := createContainerSystemdUnit(info, options) + if err != nil { + return "", err + } + builder.WriteString(out) + } + + return builder.String(), nil +} + +func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) (*containerInfo, error) { + // Generate a systemdgen.containerInfo for the infra container. This + // containerInfo acts as the main service of the pod. + infraCtr, err := pod.InfraContainer() + if err != nil { + return nil, errors.Wrap(err, "could not find infra container") + } + + timeout := infraCtr.StopTimeout() + if options.StopTimeout != nil { + timeout = *options.StopTimeout + } + + config := infraCtr.Config() + conmonPidFile := config.ConmonPidFile + if conmonPidFile == "" { + return nil, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") + } + + createCommand := []string{} + + nameOrID := pod.ID() + ctrNameOrID := infraCtr.ID() + if options.Name { + nameOrID = pod.Name() + ctrNameOrID = infraCtr.Name() + } + serviceName := fmt.Sprintf("%s%s%s", options.PodPrefix, options.Separator, nameOrID) + + info := containerInfo{ + ServiceName: serviceName, + ContainerNameOrID: ctrNameOrID, + RestartPolicy: options.RestartPolicy, + PIDFile: conmonPidFile, + StopTimeout: timeout, + GenerateTimestamp: true, + CreateCommand: createCommand, + } + return &info, nil +} From 8d8746adeeab8a39ccedb5b06fe8d0a785a97190 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Jun 2020 18:23:12 +0200 Subject: [PATCH 11/14] generate systemd: create pod template Create a new template for generating a pod unit file. Eventually, this allows for treating and extending pod and container generation seprately. The `--new` flag now also works on pods. Signed-off-by: Valentin Rothberg --- .../markdown/podman-generate-systemd.1.md | 5 +- libpod/pod.go | 2 +- pkg/systemd/generate/common.go | 14 + pkg/systemd/generate/common_test.go | 25 ++ pkg/systemd/generate/containers.go | 189 ++++++++------ .../{generate_test.go => containers_test.go} | 105 ++++---- pkg/systemd/generate/pods.go | 239 +++++++++++++++++- pkg/systemd/generate/pods_test.go | 100 ++++++++ test/e2e/generate_systemd_test.go | 50 +++- 9 files changed, 586 insertions(+), 143 deletions(-) create mode 100644 pkg/systemd/generate/common_test.go rename pkg/systemd/generate/{generate_test.go => containers_test.go} (75%) create mode 100644 pkg/systemd/generate/pods_test.go diff --git a/docs/source/markdown/podman-generate-systemd.1.md b/docs/source/markdown/podman-generate-systemd.1.md index 72031b19be60..2facd754c239 100644 --- a/docs/source/markdown/podman-generate-systemd.1.md +++ b/docs/source/markdown/podman-generate-systemd.1.md @@ -26,10 +26,7 @@ Use the name of the container for the start, stop, and description in the unit f **--new** -Create a new container via podman-run instead of starting an existing one. This option relies on container configuration files, which may not map directly to podman CLI flags; please review the generated output carefully before placing in production. -Since we use systemd `Type=forking` service, using this option will force the container run with the detached param `-d`. - -Note: Generating systemd unit files with `--new` flag is not yet supported for pods. +Using this flag will yield unit files that do not expect containers and pods to exist. Instead, new containers and pods are created based on their configuration files. The unit files are created best effort and may need to be further edited; please review the generated files carefully before using them in production. **--time**, **-t**=*value* diff --git a/libpod/pod.go b/libpod/pod.go index f53290876c2c..bf0d7a397fda 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -260,7 +260,7 @@ func (p *Pod) InfraContainerID() (string, error) { // InfraContainer returns the infra container. func (p *Pod) InfraContainer() (*Container, error) { if !p.HasInfraContainer() { - return nil, errors.New("pod has no infra container") + return nil, errors.Wrap(define.ErrNoSuchCtr, "pod has no infra container") } id, err := p.InfraContainerID() diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go index e809b483762c..4f995be9681f 100644 --- a/pkg/systemd/generate/common.go +++ b/pkg/systemd/generate/common.go @@ -34,3 +34,17 @@ Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target ` + +// filterPodFlags removes --pod and --pod-id-file from the specified command. +func filterPodFlags(command []string) []string { + processed := []string{} + for i := 0; i < len(command); i++ { + s := command[i] + if s == "--pod" || s == "--pod-id-file" { + i += 1 + continue + } + processed = append(processed, s) + } + return processed +} diff --git a/pkg/systemd/generate/common_test.go b/pkg/systemd/generate/common_test.go new file mode 100644 index 000000000000..f53bb7828409 --- /dev/null +++ b/pkg/systemd/generate/common_test.go @@ -0,0 +1,25 @@ +package generate + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFilterPodFlags(t *testing.T) { + + tests := []struct { + input []string + }{ + {[]string{"podman", "pod", "create"}}, + {[]string{"podman", "pod", "create", "--name", "foo"}}, + {[]string{"podman", "pod", "create", "--pod-id-file", "foo"}}, + {[]string{"podman", "run", "--pod", "foo"}}, + } + + for _, test := range tests { + processed := filterPodFlags(test.input) + assert.NotContains(t, processed, "--pod-id-file") + assert.NotContains(t, processed, "--pod") + } +} diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index f316d44524d9..dced1a3da573 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -33,14 +33,13 @@ type containerInfo struct { // PIDFile of the service. Required for forking services. Must point to the // PID of the associated conmon process. PIDFile string + // ContainerIDFile to be used in the unit. + ContainerIDFile string // GenerateTimestamp, if set the generated unit file has a time stamp. GenerateTimestamp bool // BoundToServices are the services this service binds to. Note that this // service runs after them. BoundToServices []string - // RequiredServices are services this service requires. Note that this - // service runs before them. - RequiredServices []string // PodmanVersion for the header. Will be set internally. Will be auto-filled // if left empty. PodmanVersion string @@ -49,16 +48,23 @@ type containerInfo struct { Executable string // TimeStamp at the time of creating the unit file. Will be set internally. TimeStamp string - // New controls if a new container is created or if an existing one is started. - New bool // CreateCommand is the full command plus arguments of the process the // container has been created with. CreateCommand []string - // RunCommand is a post-processed variant of CreateCommand and used for - // the ExecStart field in generic unit files. - RunCommand string // EnvVariable is generate.EnvVariable and must not be set. EnvVariable string + // ExecStartPre of the unit. + ExecStartPre string + // ExecStart of the unit. + ExecStart string + // ExecStop of the unit. + ExecStop string + // ExecStopPost of the unit. + ExecStopPost string + + // If not nil, the container is part of the pod. We can use the + // podInfo to extract the relevant data. + pod *podInfo } const containerTemplate = headerTemplate + ` @@ -68,25 +74,19 @@ RefuseManualStop=yes BindsTo={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} After={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} {{- end}} -{{- if .RequiredServices}} -Requires={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} -Before={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} -{{- end}} [Service] Environment={{.EnvVariable}}=%n Restart={{.RestartPolicy}} -{{- if .New}} -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id -ExecStart={{.RunCommand}} -ExecStop={{.Executable}} stop --ignore --cidfile %t/%n-ctr-id {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} -ExecStopPost={{.Executable}} rm --ignore -f --cidfile %t/%n-ctr-id -PIDFile=%t/%n-pid -{{- else}} -ExecStart={{.Executable}} start {{.ContainerNameOrID}} -ExecStop={{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}} -PIDFile={{.PIDFile}} +{{- if .ExecStartPre}} +ExecStartPre={{.ExecStartPre}} +{{- end}} +ExecStart={{.ExecStart}} +ExecStop={{.ExecStop}} +{{- if .ExecStopPost}} +ExecStopPost={{.ExecStopPost}} {{- end}} +PIDFile={{.PIDFile}} KillMode=none Type=forking @@ -101,11 +101,58 @@ func ContainerUnit(ctr *libpod.Container, options entities.GenerateSystemdOption if err != nil { return "", err } - return createContainerSystemdUnit(info, options) + return executeContainerTemplate(info, options) +} + +func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSystemdOptions) (*containerInfo, error) { + timeout := ctr.StopTimeout() + if options.StopTimeout != nil { + timeout = *options.StopTimeout + } + + config := ctr.Config() + conmonPidFile := config.ConmonPidFile + if conmonPidFile == "" { + return nil, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") + } + + createCommand := []string{} + if config.CreateCommand != nil { + createCommand = config.CreateCommand + } else if options.New { + return nil, errors.Errorf("cannot use --new on container %q: no create command found", ctr.ID()) + } + + nameOrID, serviceName := containerServiceName(ctr, options) + + info := containerInfo{ + ServiceName: serviceName, + ContainerNameOrID: nameOrID, + RestartPolicy: options.RestartPolicy, + PIDFile: conmonPidFile, + StopTimeout: timeout, + GenerateTimestamp: true, + CreateCommand: createCommand, + } + + return &info, nil +} + +// containerServiceName returns the nameOrID and the service name of the +// container. +func containerServiceName(ctr *libpod.Container, options entities.GenerateSystemdOptions) (string, string) { + nameOrID := ctr.ID() + if options.Name { + nameOrID = ctr.Name() + } + serviceName := fmt.Sprintf("%s%s%s", options.ContainerPrefix, options.Separator, nameOrID) + return nameOrID, serviceName } -// createContainerSystemdUnit creates a systemd unit file for a container. -func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSystemdOptions) (string, error) { +// executeContainerTemplate executes the container template on the specified +// containerInfo. Note that the containerInfo is also post processed and +// completed, which allows for an easier unit testing. +func executeContainerTemplate(info *containerInfo, options entities.GenerateSystemdOptions) (string, error) { if err := validateRestartPolicy(info.RestartPolicy); err != nil { return "", err } @@ -121,6 +168,8 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy } info.EnvVariable = EnvVariable + info.ExecStart = "{{.Executable}} start {{.ContainerNameOrID}}" + info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}}" // Assemble the ExecStart command when creating a new container. // @@ -130,6 +179,8 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy // invalid `info.CreateCommand`. Hence, we're doing a best effort unit // generation and don't try aiming at completeness. if options.New { + info.PIDFile = "%t/" + info.ServiceName + ".pid" + info.ContainerIDFile = "%t/" + info.ServiceName + ".ctr-id" // The create command must at least have three arguments: // /usr/bin/podman run $IMAGE index := 2 @@ -141,13 +192,20 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy } // We're hard-coding the first five arguments and append the // CreateCommand with a stripped command and subcomand. - command := []string{ + startCommand := []string{ info.Executable, "run", - "--conmon-pidfile", "%t/%n-pid", - "--cidfile", "%t/%n-ctr-id", + "--conmon-pidfile", "{{.PIDFile}}", + "--cidfile", "{{.ContainerIDFile}}", "--cgroups=no-conmon", } + // If the container is in a pod, make sure that the + // --pod-id-file is set correctly. + if info.pod != nil { + podFlags := []string{"--pod-id-file", info.pod.PodIDFile} + startCommand = append(startCommand, podFlags...) + info.CreateCommand = filterPodFlags(info.CreateCommand) + } // Enforce detaching // @@ -165,12 +223,14 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy } } if !hasDetachParam { - command = append(command, "-d") + startCommand = append(startCommand, "-d") } + startCommand = append(startCommand, info.CreateCommand[index:]...) - command = append(command, info.CreateCommand[index:]...) - info.RunCommand = strings.Join(command, " ") - info.New = true + info.ExecStartPre = "/usr/bin/rm -f {{.PIDFile}} {{.ContainerIDFile}}" + info.ExecStart = strings.Join(startCommand, " ") + info.ExecStop = "{{.Executable}} stop --ignore --cidfile {{.ContainerIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}" + info.ExecStopPost = "{{.Executable}} rm --ignore -f --cidfile {{.ContainerIDFile}}" } if info.PodmanVersion == "" { @@ -181,11 +241,17 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy } // Sort the slices to assure a deterministic output. - sort.Strings(info.RequiredServices) sort.Strings(info.BoundToServices) // Generate the template and compile it. - templ, err := template.New("systemd_service_file").Parse(containerTemplate) + // + // Note that we need a two-step generation process to allow for fields + // embedding other fields. This way we can replace `A -> B -> C` and + // make the code easier to maintain at the cost of a slightly slower + // generation. That's especially needed for embedding the PID and ID + // files in other fields which will eventually get replaced in the 2nd + // template execution. + templ, err := template.New("container_template").Parse(containerTemplate) if err != nil { return "", errors.Wrap(err, "error parsing systemd service template") } @@ -195,6 +261,17 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy return "", err } + // Now parse the generated template (i.e., buf) and execute it. + templ, err = template.New("container_template").Parse(buf.String()) + if err != nil { + return "", errors.Wrap(err, "error parsing systemd service template") + } + + buf = bytes.Buffer{} + if err := templ.Execute(&buf, info); err != nil { + return "", err + } + if !options.Files { return buf.String(), nil } @@ -210,47 +287,3 @@ func createContainerSystemdUnit(info *containerInfo, options entities.GenerateSy } return path, nil } - -func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSystemdOptions) (*containerInfo, error) { - timeout := ctr.StopTimeout() - if options.StopTimeout != nil { - timeout = *options.StopTimeout - } - - config := ctr.Config() - conmonPidFile := config.ConmonPidFile - if conmonPidFile == "" { - return nil, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") - } - - createCommand := []string{} - if config.CreateCommand != nil { - createCommand = config.CreateCommand - } else if options.New { - return nil, errors.Errorf("cannot use --new on container %q: no create command found", ctr.ID()) - } - - nameOrID, serviceName := containerServiceName(ctr, options) - - info := containerInfo{ - ServiceName: serviceName, - ContainerNameOrID: nameOrID, - RestartPolicy: options.RestartPolicy, - PIDFile: conmonPidFile, - StopTimeout: timeout, - GenerateTimestamp: true, - CreateCommand: createCommand, - } - return &info, nil -} - -// containerServiceName returns the nameOrID and the service name of the -// container. -func containerServiceName(ctr *libpod.Container, options entities.GenerateSystemdOptions) (string, string) { - nameOrID := ctr.ID() - if options.Name { - nameOrID = ctr.Name() - } - serviceName := fmt.Sprintf("%s%s%s", options.ContainerPrefix, options.Separator, nameOrID) - return nameOrID, serviceName -} diff --git a/pkg/systemd/generate/generate_test.go b/pkg/systemd/generate/containers_test.go similarity index 75% rename from pkg/systemd/generate/generate_test.go rename to pkg/systemd/generate/containers_test.go index 11cabb463145..8365ecd7a5b0 100644 --- a/pkg/systemd/generate/generate_test.go +++ b/pkg/systemd/generate/containers_test.go @@ -6,7 +6,7 @@ import ( "github.com/containers/libpod/pkg/domain/entities" ) -func TestValidateRestartPolicy(t *testing.T) { +func TestValidateRestartPolicyContainer(t *testing.T) { type containerInfo struct { restart string } @@ -103,30 +103,30 @@ Type=forking [Install] WantedBy=multi-user.target default.target` - podGoodName := `# pod-123abc.service + goodNameNew := `# jadda-jadda.service # autogenerated by Podman CI [Unit] -Description=Podman pod-123abc.service +Description=Podman jadda-jadda.service Documentation=man:podman-generate-systemd(1) Wants=network.target After=network-online.target -Requires=container-1.service container-2.service -Before=container-1.service container-2.service [Service] Environment=PODMAN_SYSTEMD_UNIT=%n Restart=always -ExecStart=/usr/bin/podman start jadda-jadda-infra -ExecStop=/usr/bin/podman stop -t 10 jadda-jadda-infra -PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +ExecStartPre=/usr/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id +PIDFile=%t/jadda-jadda.pid KillMode=none Type=forking [Install] WantedBy=multi-user.target default.target` - goodNameNew := `# jadda-jadda.service + goodNameNewWithPodFile := `# jadda-jadda.service # autogenerated by Podman CI [Unit] @@ -138,17 +138,16 @@ After=network-online.target [Service] Environment=PODMAN_SYSTEMD_UNIT=%n Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 42 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id -PIDFile=%t/%n-pid +ExecStartPre=/usr/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon --pod-id-file /tmp/pod-foobar.pod-id-file -d --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id +PIDFile=%t/jadda-jadda.pid KillMode=none Type=forking [Install] WantedBy=multi-user.target default.target` - goodNameNewDetach := `# jadda-jadda.service # autogenerated by Podman CI @@ -161,11 +160,11 @@ After=network-online.target [Service] Environment=PODMAN_SYSTEMD_UNIT=%n Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon --detach --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 42 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id -PIDFile=%t/%n-pid +ExecStartPre=/usr/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon --detach --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id +PIDFile=%t/jadda-jadda.pid KillMode=none Type=forking @@ -184,11 +183,11 @@ After=network-online.target [Service] Environment=PODMAN_SYSTEMD_UNIT=%n Restart=always -ExecStartPre=/usr/bin/rm -f %t/%n-pid %t/%n-ctr-id -ExecStart=/usr/bin/podman run --conmon-pidfile %t/%n-pid --cidfile %t/%n-ctr-id --cgroups=no-conmon -d awesome-image:latest -ExecStop=/usr/bin/podman stop --ignore --cidfile %t/%n-ctr-id -t 10 -ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/%n-ctr-id -PIDFile=%t/%n-pid +ExecStartPre=/usr/bin/rm -f %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id +ExecStart=/usr/bin/podman run --conmon-pidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid --cidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id --cgroups=no-conmon -d awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id -t 10 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.ctr-id +PIDFile=%t/container-639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid KillMode=none Type=forking @@ -199,6 +198,7 @@ WantedBy=multi-user.target default.target` name string info containerInfo want string + new bool wantErr bool }{ @@ -211,9 +211,11 @@ WantedBy=multi-user.target default.target` PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 10, PodmanVersion: "CI", + EnvVariable: EnvVariable, }, goodID, false, + false, }, {"good with name", containerInfo{ @@ -224,9 +226,11 @@ WantedBy=multi-user.target default.target` PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 10, PodmanVersion: "CI", + EnvVariable: EnvVariable, }, goodName, false, + false, }, {"good with name and bound to", containerInfo{ @@ -238,22 +242,10 @@ WantedBy=multi-user.target default.target` StopTimeout: 10, PodmanVersion: "CI", BoundToServices: []string{"pod", "a", "b", "c"}, + EnvVariable: EnvVariable, }, goodNameBoundTo, false, - }, - {"pod", - containerInfo{ - Executable: "/usr/bin/podman", - ServiceName: "pod-123abc", - ContainerNameOrID: "jadda-jadda-infra", - RestartPolicy: "always", - PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", - StopTimeout: 10, - PodmanVersion: "CI", - RequiredServices: []string{"container-1", "container-2"}, - }, - podGoodName, false, }, {"bad restart policy", @@ -264,8 +256,10 @@ WantedBy=multi-user.target default.target` PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 10, PodmanVersion: "CI", + EnvVariable: EnvVariable, }, "", + false, true, }, {"good with name and generic", @@ -277,10 +271,11 @@ WantedBy=multi-user.target default.target` PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 42, PodmanVersion: "CI", - New: true, CreateCommand: []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + EnvVariable: EnvVariable, }, goodNameNew, + true, false, }, {"good with explicit short detach param", @@ -292,10 +287,30 @@ WantedBy=multi-user.target default.target` PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 42, PodmanVersion: "CI", - New: true, CreateCommand: []string{"I'll get stripped", "container", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + EnvVariable: EnvVariable, }, goodNameNew, + true, + false, + }, + {"good with explicit short detach param and podInfo", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 42, + PodmanVersion: "CI", + CreateCommand: []string{"I'll get stripped", "container", "run", "-d", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + EnvVariable: EnvVariable, + pod: &podInfo{ + PodIDFile: "/tmp/pod-foobar.pod-id-file", + }, + }, + goodNameNewWithPodFile, + true, false, }, {"good with explicit full detach param", @@ -307,10 +322,11 @@ WantedBy=multi-user.target default.target` PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 42, PodmanVersion: "CI", - New: true, CreateCommand: []string{"I'll get stripped", "container", "run", "--detach", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + EnvVariable: EnvVariable, }, goodNameNewDetach, + true, false, }, {"good with id and no param", @@ -322,10 +338,11 @@ WantedBy=multi-user.target default.target` PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 10, PodmanVersion: "CI", - New: true, CreateCommand: []string{"I'll get stripped", "container", "run", "awesome-image:latest"}, + EnvVariable: EnvVariable, }, goodIDNew, + true, false, }, } @@ -334,9 +351,9 @@ WantedBy=multi-user.target default.target` t.Run(tt.name, func(t *testing.T) { opts := entities.GenerateSystemdOptions{ Files: false, - New: test.info.New, + New: test.new, } - got, err := createContainerSystemdUnit(&test.info, opts) + got, err := executeContainerTemplate(&test.info, opts) if (err != nil) != test.wantErr { t.Errorf("CreateContainerSystemdUnit() error = \n%v, wantErr \n%v", err, test.wantErr) return diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go index 16f324f7859b..355103df8dc2 100644 --- a/pkg/systemd/generate/pods.go +++ b/pkg/systemd/generate/pods.go @@ -1,22 +1,101 @@ package generate import ( + "bytes" "fmt" + "io/ioutil" + "os" + "path/filepath" + "sort" "strings" + "text/template" + "time" "github.com/containers/libpod/libpod" "github.com/containers/libpod/pkg/domain/entities" + "github.com/containers/libpod/version" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) +// podInfo contains data required for generating a pod's systemd +// unit file. +type podInfo struct { + // ServiceName of the systemd service. + ServiceName string + // Name or ID of the infra container. + InfraNameOrID string + // StopTimeout sets the timeout Podman waits before killing the container + // during service stop. + StopTimeout uint + // RestartPolicy of the systemd unit (e.g., no, on-failure, always). + RestartPolicy string + // PIDFile of the service. Required for forking services. Must point to the + // PID of the associated conmon process. + PIDFile string + // PodIDFile of the unit. + PodIDFile string + // GenerateTimestamp, if set the generated unit file has a time stamp. + GenerateTimestamp bool + // RequiredServices are services this service requires. Note that this + // service runs before them. + RequiredServices []string + // PodmanVersion for the header. Will be set internally. Will be auto-filled + // if left empty. + PodmanVersion string + // Executable is the path to the podman executable. Will be auto-filled if + // left empty. + Executable string + // TimeStamp at the time of creating the unit file. Will be set internally. + TimeStamp string + // CreateCommand is the full command plus arguments of the process the + // container has been created with. + CreateCommand []string + // PodCreateCommand - a post-processed variant of CreateCommand to use + // when creating the pod. + PodCreateCommand string + // EnvVariable is generate.EnvVariable and must not be set. + EnvVariable string + // ExecStartPre1 of the unit. + ExecStartPre1 string + // ExecStartPre2 of the unit. + ExecStartPre2 string + // ExecStart of the unit. + ExecStart string + // ExecStop of the unit. + ExecStop string + // ExecStopPost of the unit. + ExecStopPost string +} + +const podTemplate = headerTemplate + `Requires={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} +Before={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} + +[Service] +Environment={{.EnvVariable}}=%n +Restart={{.RestartPolicy}} +{{- if .ExecStartPre1}} +ExecStartPre={{.ExecStartPre1}} +{{- end}} +{{- if .ExecStartPre2}} +ExecStartPre={{.ExecStartPre2}} +{{- end}} +ExecStart={{.ExecStart}} +ExecStop={{.ExecStop}} +{{- if .ExecStopPost}} +ExecStopPost={{.ExecStopPost}} +{{- end}} +PIDFile={{.PIDFile}} +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + // PodUnits generates systemd units for the specified pod and its containers. // Based on the options, the return value might be the content of all units or // the files they been written to. func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, error) { - if options.New { - return "", errors.New("--new is not supported for pods") - } - // Error out if the pod has no infra container, which we require to be the // main service. if !pod.HasInfraContainer() { @@ -48,7 +127,7 @@ func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, // Traverse the dependency graph and create systemdgen.containerInfo's for // each container. - containerInfos := []*containerInfo{podInfo} + containerInfos := []*containerInfo{} for ctr, dependencies := range graph.DependencyMap() { // Skip the infra container as we already generated it. if ctr.ID() == infraID { @@ -74,11 +153,15 @@ func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, // Now generate the systemd service for all containers. builder := strings.Builder{} - for i, info := range containerInfos { - if i > 0 { - builder.WriteByte('\n') - } - out, err := createContainerSystemdUnit(info, options) + out, err := executePodTemplate(podInfo, options) + if err != nil { + return "", err + } + builder.WriteString(out) + for _, info := range containerInfos { + info.pod = podInfo + builder.WriteByte('\n') + out, err := executeContainerTemplate(info, options) if err != nil { return "", err } @@ -88,7 +171,7 @@ func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, return builder.String(), nil } -func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) (*containerInfo, error) { +func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) (*podInfo, error) { // Generate a systemdgen.containerInfo for the infra container. This // containerInfo acts as the main service of the pod. infraCtr, err := pod.InfraContainer() @@ -107,7 +190,10 @@ func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) ( return nil, errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag") } - createCommand := []string{} + createCommand := pod.CreateCommand() + if options.New && len(createCommand) == 0 { + return nil, errors.Errorf("cannot use --new on pod %q: no create command found", pod.ID()) + } nameOrID := pod.ID() ctrNameOrID := infraCtr.ID() @@ -117,9 +203,9 @@ func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) ( } serviceName := fmt.Sprintf("%s%s%s", options.PodPrefix, options.Separator, nameOrID) - info := containerInfo{ + info := podInfo{ ServiceName: serviceName, - ContainerNameOrID: ctrNameOrID, + InfraNameOrID: ctrNameOrID, RestartPolicy: options.RestartPolicy, PIDFile: conmonPidFile, StopTimeout: timeout, @@ -128,3 +214,128 @@ func generatePodInfo(pod *libpod.Pod, options entities.GenerateSystemdOptions) ( } return &info, nil } + +// executePodTemplate executes the pod template on the specified podInfo. Note +// that the podInfo is also post processed and completed, which allows for an +// easier unit testing. +func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) (string, error) { + if err := validateRestartPolicy(info.RestartPolicy); err != nil { + return "", err + } + + // Make sure the executable is set. + if info.Executable == "" { + executable, err := os.Executable() + if err != nil { + executable = "/usr/bin/podman" + logrus.Warnf("Could not obtain podman executable location, using default %s", executable) + } + info.Executable = executable + } + + info.EnvVariable = EnvVariable + info.ExecStart = "{{.Executable}} start {{.InfraNameOrID}}" + info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.InfraNameOrID}}" + + // Assemble the ExecStart command when creating a new pod. + // + // Note that we cannot catch all corner cases here such that users + // *must* manually check the generated files. A pod might have been + // created via a Python script, which would certainly yield an invalid + // `info.CreateCommand`. Hence, we're doing a best effort unit + // generation and don't try aiming at completeness. + if options.New { + info.PIDFile = "%t/" + info.ServiceName + ".pid" + info.PodIDFile = "%t/" + info.ServiceName + ".pod-id" + + podCreateIndex := 0 + var podRootArgs, podCreateArgs []string + switch len(info.CreateCommand) { + case 0, 1, 2: + return "", errors.Errorf("pod does not appear to be created via `podman pod create`: %v", info.CreateCommand) + default: + // Make sure that pod was created with `pod create` and + // not something else, such as `run --pod new`. + for i := 1; i < len(info.CreateCommand); i++ { + if info.CreateCommand[i-1] == "pod" && info.CreateCommand[i] == "create" { + podCreateIndex = i + break + } + } + if podCreateIndex == 0 { + return "", errors.Errorf("pod does not appear to be created via `podman pod create`: %v", info.CreateCommand) + } + podRootArgs = info.CreateCommand[1 : podCreateIndex-2] + podCreateArgs = filterPodFlags(info.CreateCommand[podCreateIndex+1:]) + } + // We're hard-coding the first five arguments and append the + // CreateCommand with a stripped command and subcomand. + startCommand := []string{info.Executable} + startCommand = append(startCommand, podRootArgs...) + startCommand = append(startCommand, + []string{"pod", "create", + "--infra-conmon-pidfile", "{{.PIDFile}}", + "--pod-id-file", "{{.PodIDFile}}"}...) + + startCommand = append(startCommand, podCreateArgs...) + + info.ExecStartPre1 = "/usr/bin/rm -f {{.PIDFile}} {{.PodIDFile}}" + info.ExecStartPre2 = strings.Join(startCommand, " ") + info.ExecStart = "{{.Executable}} pod start --pod-id-file {{.PodIDFile}}" + info.ExecStop = "{{.Executable}} pod stop --ignore --pod-id-file {{.PodIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}" + info.ExecStopPost = "{{.Executable}} pod rm --ignore -f --pod-id-file {{.PodIDFile}}" + } + if info.PodmanVersion == "" { + info.PodmanVersion = version.Version + } + if info.GenerateTimestamp { + info.TimeStamp = fmt.Sprintf("%v", time.Now().Format(time.UnixDate)) + } + + // Sort the slices to assure a deterministic output. + sort.Strings(info.RequiredServices) + + // Generate the template and compile it. + // + // Note that we need a two-step generation process to allow for fields + // embedding other fields. This way we can replace `A -> B -> C` and + // make the code easier to maintain at the cost of a slightly slower + // generation. That's especially needed for embedding the PID and ID + // files in other fields which will eventually get replaced in the 2nd + // template execution. + templ, err := template.New("pod_template").Parse(podTemplate) + if err != nil { + return "", errors.Wrap(err, "error parsing systemd service template") + } + + var buf bytes.Buffer + if err := templ.Execute(&buf, info); err != nil { + return "", err + } + + // Now parse the generated template (i.e., buf) and execute it. + templ, err = template.New("pod_template").Parse(buf.String()) + if err != nil { + return "", errors.Wrap(err, "error parsing systemd service template") + } + + buf = bytes.Buffer{} + if err := templ.Execute(&buf, info); err != nil { + return "", err + } + + if !options.Files { + return buf.String(), nil + } + + buf.WriteByte('\n') + cwd, err := os.Getwd() + if err != nil { + return "", errors.Wrap(err, "error getting current working directory") + } + path := filepath.Join(cwd, fmt.Sprintf("%s.service", info.ServiceName)) + if err := ioutil.WriteFile(path, buf.Bytes(), 0644); err != nil { + return "", errors.Wrap(err, "error generating systemd unit") + } + return path, nil +} diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go new file mode 100644 index 000000000000..f6e225c355af --- /dev/null +++ b/pkg/systemd/generate/pods_test.go @@ -0,0 +1,100 @@ +package generate + +import ( + "testing" + + "github.com/containers/libpod/pkg/domain/entities" +) + +func TestValidateRestartPolicyPod(t *testing.T) { + type podInfo struct { + restart string + } + tests := []struct { + name string + podInfo podInfo + wantErr bool + }{ + {"good-on", podInfo{restart: "no"}, false}, + {"good-on-success", podInfo{restart: "on-success"}, false}, + {"good-on-failure", podInfo{restart: "on-failure"}, false}, + {"good-on-abnormal", podInfo{restart: "on-abnormal"}, false}, + {"good-on-watchdog", podInfo{restart: "on-watchdog"}, false}, + {"good-on-abort", podInfo{restart: "on-abort"}, false}, + {"good-always", podInfo{restart: "always"}, false}, + {"fail", podInfo{restart: "foobar"}, true}, + {"failblank", podInfo{restart: ""}, true}, + } + for _, tt := range tests { + test := tt + t.Run(tt.name, func(t *testing.T) { + if err := validateRestartPolicy(test.podInfo.restart); (err != nil) != test.wantErr { + t.Errorf("ValidateRestartPolicy() error = %v, wantErr %v", err, test.wantErr) + } + }) + } +} + +func TestCreatePodSystemdUnit(t *testing.T) { + podGoodName := `# pod-123abc.service +# autogenerated by Podman CI + +[Unit] +Description=Podman pod-123abc.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +Requires=container-1.service container-2.service +Before=container-1.service container-2.service + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Restart=always +ExecStart=/usr/bin/podman start jadda-jadda-infra +ExecStop=/usr/bin/podman stop -t 10 jadda-jadda-infra +PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid +KillMode=none +Type=forking + +[Install] +WantedBy=multi-user.target default.target` + + tests := []struct { + name string + info podInfo + want string + wantErr bool + }{ + {"pod", + podInfo{ + Executable: "/usr/bin/podman", + ServiceName: "pod-123abc", + InfraNameOrID: "jadda-jadda-infra", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + RequiredServices: []string{"container-1", "container-2"}, + }, + podGoodName, + false, + }, + } + + for _, tt := range tests { + test := tt + t.Run(tt.name, func(t *testing.T) { + opts := entities.GenerateSystemdOptions{ + Files: false, + } + got, err := executePodTemplate(&test.info, opts) + if (err != nil) != test.wantErr { + t.Errorf("CreatePodSystemdUnit() error = \n%v, wantErr \n%v", err, test.wantErr) + return + } + if got != test.want { + t.Errorf("CreatePodSystemdUnit() = \n%v\n---------> want\n%v", got, test.want) + } + }) + } +} diff --git a/test/e2e/generate_systemd_test.go b/test/e2e/generate_systemd_test.go index 64d220776409..497e8f71e287 100644 --- a/test/e2e/generate_systemd_test.go +++ b/test/e2e/generate_systemd_test.go @@ -3,6 +3,7 @@ package integration import ( + "io/ioutil" "os" . "github.com/containers/libpod/test/utils" @@ -191,7 +192,7 @@ var _ = Describe("Podman generate systemd", func() { found, _ := session.GrepString("# container-foo.service") Expect(found).To(BeTrue()) - found, _ = session.GrepString("stop --ignore --cidfile %t/%n-ctr-id -t 42") + found, _ = session.GrepString("stop --ignore --cidfile %t/container-foo.ctr-id -t 42") Expect(found).To(BeTrue()) }) @@ -230,7 +231,7 @@ var _ = Describe("Podman generate systemd", func() { session := podmanTest.Podman([]string{"generate", "systemd", "--time", "42", "--name", "--new", "foo"}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(Equal(125)) + Expect(session.ExitCode()).To(Equal(0)) }) It("podman generate systemd --container-prefix con", func() { @@ -325,4 +326,49 @@ var _ = Describe("Podman generate systemd", func() { found, _ = session.GrepString("BindsTo=p_foo.service") Expect(found).To(BeTrue()) }) + + It("podman generate systemd pod with containers --new", func() { + tmpDir, err := ioutil.TempDir("", "") + Expect(err).To(BeNil()) + tmpFile := tmpDir + "podID" + defer os.RemoveAll(tmpDir) + + n := podmanTest.Podman([]string{"pod", "create", "--pod-id-file", tmpFile, "--name", "foo"}) + n.WaitWithDefaultTimeout() + Expect(n.ExitCode()).To(Equal(0)) + + n = podmanTest.Podman([]string{"create", "--pod", "foo", "--name", "foo-1", "alpine", "top"}) + n.WaitWithDefaultTimeout() + Expect(n.ExitCode()).To(Equal(0)) + + n = podmanTest.Podman([]string{"create", "--pod", "foo", "--name", "foo-2", "alpine", "top"}) + n.WaitWithDefaultTimeout() + Expect(n.ExitCode()).To(Equal(0)) + + session := podmanTest.Podman([]string{"generate", "systemd", "--new", "--name", "foo"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Grepping the output (in addition to unit tests) + found, _ := session.GrepString("# pod-foo.service") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("Requires=container-foo-1.service container-foo-2.service") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("BindsTo=pod-foo.service") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("pod create --infra-conmon-pidfile %t/pod-foo.pid --pod-id-file %t/pod-foo.pod-id --name foo") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("ExecStartPre=/usr/bin/rm -f %t/pod-foo.pid %t/pod-foo.pod-id") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("pod stop --ignore --pod-id-file %t/pod-foo.pod-id -t 10") + Expect(found).To(BeTrue()) + + found, _ = session.GrepString("pod rm --ignore -f --pod-id-file %t/pod-foo.pod-id") + Expect(found).To(BeTrue()) + }) }) From 139f82933deceda569c30d3364bba585fa925f5a Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 10 Jun 2020 11:00:31 +0200 Subject: [PATCH 12/14] docs: create/run fix --pod-id-file description Signed-off-by: Valentin Rothberg --- docs/source/markdown/podman-create.1.md | 2 +- docs/source/markdown/podman-run.1.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md index 81c83369affd..dbc835920930 100644 --- a/docs/source/markdown/podman-create.1.md +++ b/docs/source/markdown/podman-create.1.md @@ -595,7 +595,7 @@ To make a pod with more granular options, use the `podman pod create` command be **--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. +Run container in an existing pod and read the pod's ID from the specified file. If a container is run within a pod, and the pod has an infra-container, the infra-container will be started before the container is. **--privileged**=*true|false* diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index 4f43e4c191a3..22f7cae09efa 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -607,7 +607,7 @@ If a container is run with a pod, and the pod has an infra-container, the infra- **--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. +Run container in an existing pod and read the pod's ID from the specified file. If a container is run within a pod, and the pod has an infra-container, the infra-container will be started before the container is. **--privileged**=**true**|**false** From 05713fbbf30a8c10f9aa290723efec5f3846a113 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 10 Jun 2020 11:05:41 +0200 Subject: [PATCH 13/14] generate systemd: wrap pod/ctr lookup errors Signed-off-by: Valentin Rothberg --- pkg/domain/infra/abi/generate.go | 7 ++++--- pkg/systemd/generate/common.go | 2 +- pkg/systemd/generate/pods.go | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/domain/infra/abi/generate.go b/pkg/domain/infra/abi/generate.go index fa0cfb389847..8853303d5905 100644 --- a/pkg/domain/infra/abi/generate.go +++ b/pkg/domain/infra/abi/generate.go @@ -16,8 +16,8 @@ import ( func (ic *ContainerEngine) GenerateSystemd(ctx context.Context, nameOrID string, options entities.GenerateSystemdOptions) (*entities.GenerateSystemdReport, error) { // First assume it's a container. - ctr, err := ic.Libpod.LookupContainer(nameOrID) - if err == nil { + ctr, ctrErr := ic.Libpod.LookupContainer(nameOrID) + if ctrErr == nil { // Generate the unit for the container. s, err := generate.ContainerUnit(ctr, options) if err == nil { @@ -28,7 +28,8 @@ func (ic *ContainerEngine) GenerateSystemd(ctx context.Context, nameOrID string, // If it's not a container, we either have a pod or garbage. pod, err := ic.Libpod.LookupPod(nameOrID) if err != nil { - return nil, errors.Errorf("%q does not refer to a container or pod", nameOrID) + err = errors.Wrap(ctrErr, err.Error()) + return nil, errors.Wrapf(err, "%s does not refer to a container or pod", nameOrID) } // Generate the units for the pod and all its containers. diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go index 4f995be9681f..fe56dc874b4d 100644 --- a/pkg/systemd/generate/common.go +++ b/pkg/systemd/generate/common.go @@ -41,7 +41,7 @@ func filterPodFlags(command []string) []string { for i := 0; i < len(command); i++ { s := command[i] if s == "--pod" || s == "--pod-id-file" { - i += 1 + i++ continue } processed = append(processed, s) diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go index 355103df8dc2..5cfd5ab0a1c7 100644 --- a/pkg/systemd/generate/pods.go +++ b/pkg/systemd/generate/pods.go @@ -99,7 +99,7 @@ func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, // Error out if the pod has no infra container, which we require to be the // main service. if !pod.HasInfraContainer() { - return "", fmt.Errorf("error generating systemd unit files: Pod %q has no infra container", pod.Name()) + return "", errors.Errorf("error generating systemd unit files: Pod %q has no infra container", pod.Name()) } podInfo, err := generatePodInfo(pod, options) @@ -118,7 +118,7 @@ func PodUnits(pod *libpod.Pod, options entities.GenerateSystemdOptions) (string, return "", err } if len(containers) == 0 { - return "", fmt.Errorf("error generating systemd unit files: Pod %q has no containers", pod.Name()) + return "", errors.Errorf("error generating systemd unit files: Pod %q has no containers", pod.Name()) } graph, err := libpod.BuildContainerGraph(containers) if err != nil { From c7c81a8c081f6f7458345027f315a796d6ca5eda Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 11 Jun 2020 10:59:43 +0200 Subject: [PATCH 14/14] e2e: sanity check --infra-conmon-pidfile Signed-off-by: Valentin Rothberg --- test/e2e/pod_start_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/e2e/pod_start_test.go b/test/e2e/pod_start_test.go index 99285d1e1f0d..d7d623d6eda2 100644 --- a/test/e2e/pod_start_test.go +++ b/test/e2e/pod_start_test.go @@ -1,8 +1,11 @@ package integration import ( + "fmt" "io/ioutil" "os" + "strconv" + "strings" . "github.com/containers/libpod/test/utils" . "github.com/onsi/ginkgo" @@ -210,6 +213,21 @@ var _ = Describe("Podman pod start", func() { session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1)) // infra + + readFirstLine := func(path string) string { + content, err := ioutil.ReadFile(path) + Expect(err).To(BeNil()) + return strings.Split(string(content), "\n")[0] + } + + // Read the infra-conmon-pidfile and perform some sanity checks + // on the pid. + infraConmonPID := readFirstLine(tmpFile) + _, err = strconv.Atoi(infraConmonPID) // Make sure it's a proper integer + Expect(err).To(BeNil()) + + cmdline := readFirstLine(fmt.Sprintf("/proc/%s/cmdline", infraConmonPID)) + Expect(cmdline).To(ContainSubstring("/conmon")) }) })