diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index 78fd1d7216..c85de4d127 100644 --- a/cmd/podman/common/create.go +++ b/cmd/podman/common/create.go @@ -386,14 +386,6 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions, ) _ = cmd.RegisterFlagCompletionFunc(requiresFlagName, AutocompleteContainers) - restartFlagName := "restart" - createFlags.StringVar( - &cf.Restart, - restartFlagName, "", - `Restart policy to apply when a container exits ("always"|"no"|"on-failure"|"unless-stopped")`, - ) - _ = cmd.RegisterFlagCompletionFunc(restartFlagName, AutocompleteRestartOption) - createFlags.BoolVar( &cf.Rm, "rm", false, @@ -635,6 +627,14 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions, ) } if mode == entities.InfraMode || (mode == entities.CreateMode) { // infra container flags, create should also pick these up + restartFlagName := "restart" + createFlags.StringVar( + &cf.Restart, + restartFlagName, "", + `Restart policy to apply when a container exits ("always"|"no"|"never"|"on-failure"|"unless-stopped")`, + ) + _ = cmd.RegisterFlagCompletionFunc(restartFlagName, AutocompleteRestartOption) + shmSizeFlagName := "shm-size" createFlags.String( shmSizeFlagName, shmSize(), diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go index e25c90f4d7..d6a6ff3773 100644 --- a/cmd/podman/containers/create.go +++ b/cmd/podman/containers/create.go @@ -137,6 +137,7 @@ func create(cmd *cobra.Command, args []string) error { } cliVals.InitContainerType = initctr } + // TODO: v5.0 block users from setting restart policy for a container if the container is in a pod cliVals, err := CreateInit(cmd, cliVals, false) if err != nil { @@ -405,6 +406,7 @@ func createPodIfNecessary(cmd *cobra.Command, s *specgen.SpecGenerator, netOpts CpusetCpus: cliVals.CPUSetCPUs, Pid: cliVals.PID, Userns: uns, + Restart: cliVals.Restart, } // Unset config values we passed to the pod to prevent them being used twice for the container and pod. s.ContainerBasicConfig.Hostname = "" diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go index db1eec07b6..5cc4505ff2 100644 --- a/cmd/podman/pods/create.go +++ b/cmd/podman/pods/create.go @@ -259,6 +259,7 @@ func create(cmd *cobra.Command, args []string) error { podSpec.InfraContainerSpec = specgen.NewSpecGenerator(imageName, false) podSpec.InfraContainerSpec.RawImageName = rawImageName podSpec.InfraContainerSpec.NetworkOptions = podSpec.NetworkOptions + podSpec.InfraContainerSpec.RestartPolicy = podSpec.RestartPolicy err = specgenutil.FillOutSpecGen(podSpec.InfraContainerSpec, &infraOptions, []string{}) if err != nil { return err diff --git a/docs/source/markdown/options/restart.md b/docs/source/markdown/options/restart.md index 3a5e665c12..7d0219600e 100644 --- a/docs/source/markdown/options/restart.md +++ b/docs/source/markdown/options/restart.md @@ -1,5 +1,5 @@ ####> This option file is used in: -####> podman create, run +####> podman create, pod clone, pod create, run ####> If file is edited, make sure the changes ####> are applicable to all of those. #### **--restart**=*policy* @@ -10,6 +10,7 @@ Restart policy will not take effect if a container is stopped via the **podman k Valid _policy_ values are: - `no` : Do not restart containers on exit +- `never` : Synonym for **no**; do not restart containers on exit - `on-failure[:max_retries]` : Restart containers when they exit with a non-zero exit code, retrying indefinitely or until the optional *max_retries* count is hit - `always` : Restart containers when they exit, regardless of status, retrying indefinitely - `unless-stopped` : Identical to **always** diff --git a/docs/source/markdown/podman-pod-clone.1.md.in b/docs/source/markdown/podman-pod-clone.1.md.in index a474436ee4..a442be80c9 100644 --- a/docs/source/markdown/podman-pod-clone.1.md.in +++ b/docs/source/markdown/podman-pod-clone.1.md.in @@ -67,6 +67,10 @@ Set a custom name for the cloned pod. The default if not specified is of the syn @@option pid.pod +@@option restart + +Default restart policy for all the containers in a pod. + @@option security-opt @@option shm-size diff --git a/docs/source/markdown/podman-pod-create.1.md.in b/docs/source/markdown/podman-pod-create.1.md.in index 322c637813..28ce755316 100644 --- a/docs/source/markdown/podman-pod-create.1.md.in +++ b/docs/source/markdown/podman-pod-create.1.md.in @@ -143,6 +143,10 @@ but only by the pod itself. @@option replace +@@option restart + +Default restart policy for all the containers in a pod. + @@option security-opt #### **--share**=*namespace* diff --git a/docs/source/markdown/podman-pod-inspect.1.md.in b/docs/source/markdown/podman-pod-inspect.1.md.in index b43f6977ce..e4697dcf1f 100644 --- a/docs/source/markdown/podman-pod-inspect.1.md.in +++ b/docs/source/markdown/podman-pod-inspect.1.md.in @@ -50,6 +50,7 @@ Valid placeholders for the Go template are listed below: | .Name | Pod name | | .Namespace | Namespace | | .NumContainers | Number of containers in the pod | +| .RestartPolicy | Restart policy of the pod | | .SecurityOpts | Security options | | .SharedNamespaces | Pod shared namespaces | | .State | Pod state | diff --git a/libpod/define/pod_inspect.go b/libpod/define/pod_inspect.go index dc82af2201..9607bf868d 100644 --- a/libpod/define/pod_inspect.go +++ b/libpod/define/pod_inspect.go @@ -83,6 +83,8 @@ type InspectPodData struct { BlkioWeight uint64 `json:"blkio_weight,omitempty"` // BlkioWeightDevice contains the blkio weight device limits for the pod BlkioWeightDevice []InspectBlkioWeightDevice `json:"blkio_weight_device,omitempty"` + // RestartPolicy of the pod. + RestartPolicy string `json:"RestartPolicy,omitempty"` } // InspectPodInfraConfig contains the configuration of the pod's infra diff --git a/libpod/options.go b/libpod/options.go index bc70e4a32c..a974caeebf 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -2020,6 +2020,40 @@ func WithPodExitPolicy(policy string) PodCreateOption { } } +// WithPodRestartPolicy sets the restart policy of the pod. +func WithPodRestartPolicy(policy string) PodCreateOption { + return func(pod *Pod) error { + if pod.valid { + return define.ErrPodFinalized + } + + switch policy { + //TODO: v5.0 if no restart policy is set, follow k8s convention and default to Always + case define.RestartPolicyNone, define.RestartPolicyNo, define.RestartPolicyOnFailure, define.RestartPolicyAlways, define.RestartPolicyUnlessStopped: + pod.config.RestartPolicy = policy + default: + return fmt.Errorf("%q is not a valid restart policy: %w", policy, define.ErrInvalidArg) + } + + return nil + } +} + +// WithPodRestartRetries sets the number of retries to use when restarting a +// container with the "on-failure" restart policy. +// 0 is an allowed value, and indicates infinite retries. +func WithPodRestartRetries(tries uint) PodCreateOption { + return func(pod *Pod) error { + if pod.valid { + return define.ErrPodFinalized + } + + pod.config.RestartRetries = &tries + + return nil + } +} + // WithPodHostname sets the hostname of the pod. func WithPodHostname(hostname string) PodCreateOption { return func(pod *Pod) error { diff --git a/libpod/pod.go b/libpod/pod.go index e9ce876373..cdd0f3d715 100644 --- a/libpod/pod.go +++ b/libpod/pod.go @@ -81,6 +81,12 @@ type PodConfig struct { // The pod's exit policy. ExitPolicy config.PodExitPolicy `json:"ExitPolicy,omitempty"` + // The pod's restart policy + RestartPolicy string `json:"RestartPolicy,omitempty"` + + // The max number of retries for a pod based on restart policy + RestartRetries *uint `json:"RestartRetries,omitempty"` + // ID of the pod's lock LockID uint32 `json:"lockID"` @@ -522,3 +528,10 @@ func (p *Pod) Config() (*PodConfig, error) { return conf, err } + +// ConfigNoCopy returns the configuration used by the pod. +// Note that the returned value is not a copy and must hence +// only be used in a reading fashion. +func (p *Pod) ConfigNoCopy() *PodConfig { + return p.config +} diff --git a/libpod/pod_api.go b/libpod/pod_api.go index b5b676b40e..f8e70cc8f5 100644 --- a/libpod/pod_api.go +++ b/libpod/pod_api.go @@ -741,6 +741,7 @@ func (p *Pod) Inspect() (*define.InspectPodData, error) { CPUSetMems: p.CPUSetMems(), BlkioDeviceWriteBps: p.BlkiThrottleWriteBps(), CPUShares: p.CPUShares(), + RestartPolicy: p.config.RestartPolicy, } return &inspectData, nil diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go index b0ff5cdaf4..eeda81ef64 100644 --- a/pkg/domain/entities/pods.go +++ b/pkg/domain/entities/pods.go @@ -135,6 +135,7 @@ type PodCreateOptions struct { Net *NetOptions `json:"net,omitempty"` Share []string `json:"share,omitempty"` ShareParent *bool `json:"share_parent,omitempty"` + Restart string `json:"restart,omitempty"` Pid string `json:"pid,omitempty"` Cpus float64 `json:"cpus,omitempty"` CpusetCpus string `json:"cpuset_cpus,omitempty"` @@ -375,6 +376,14 @@ func ToPodSpecGen(s specgen.PodSpecGenerator, p *PodCreateOptions) (*specgen.Pod s.ShareParent = p.ShareParent s.PodCreateCommand = p.CreateCommand s.VolumesFrom = p.VolumesFrom + if p.Restart != "" { + policy, retries, err := util.ParseRestartPolicy(p.Restart) + if err != nil { + return nil, err + } + s.RestartPolicy = policy + s.RestartRetries = &retries + } // Networking config diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index 5964607fa1..266c4c5883 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -555,12 +555,24 @@ func createContainerOptions(rt *libpod.Runtime, s *specgen.SpecGenerator, pod *l } // Default used if not overridden on command line - if s.RestartPolicy != "" { - if s.RestartRetries != nil { - options = append(options, libpod.WithRestartRetries(*s.RestartRetries)) + var ( + restartPolicy string + retries uint + ) + // If the container is running in a pod, use the pod's restart policy for all the containers + if pod != nil { + podConfig := pod.ConfigNoCopy() + if podConfig.RestartRetries != nil { + retries = *podConfig.RestartRetries } - options = append(options, libpod.WithRestartPolicy(s.RestartPolicy)) + restartPolicy = podConfig.RestartPolicy + } else if s.RestartPolicy != "" { + if s.RestartRetries != nil { + retries = *s.RestartRetries + } + restartPolicy = s.RestartPolicy } + options = append(options, libpod.WithRestartRetries(retries), libpod.WithRestartPolicy(restartPolicy)) if s.ContainerHealthCheckConfig.HealthConfig != nil { options = append(options, libpod.WithHealthCheck(s.ContainerHealthCheckConfig.HealthConfig)) diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go index 6d45a728e1..03f15f1903 100644 --- a/pkg/specgen/generate/pod_create.go +++ b/pkg/specgen/generate/pod_create.go @@ -165,6 +165,10 @@ func createPodOptions(p *specgen.PodSpecGenerator) ([]libpod.PodCreateOption, er } options = append(options, libpod.WithPodExitPolicy(p.ExitPolicy)) + options = append(options, libpod.WithPodRestartPolicy(p.RestartPolicy)) + if p.RestartRetries != nil { + options = append(options, libpod.WithPodRestartRetries(*p.RestartRetries)) + } return options, nil } diff --git a/pkg/specgen/podspecgen.go b/pkg/specgen/podspecgen.go index 2d855bfae1..5c8e561ecc 100644 --- a/pkg/specgen/podspecgen.go +++ b/pkg/specgen/podspecgen.go @@ -64,6 +64,17 @@ type PodBasicConfig struct { // Conflicts with NoInfra=true. // Optional. SharedNamespaces []string `json:"shared_namespaces,omitempty"` + // RestartPolicy is the pod's restart policy - an action which + // will be taken when one or all the containers in the pod exits. + // If not given, the default policy will be set to Always, which + // restarts the containers in the pod when they exit indefinitely. + // Optional. + RestartPolicy string `json:"restart_policy,omitempty"` + // RestartRetries is the number of attempts that will be made to restart + // the container. + // Only available when RestartPolicy is set to "on-failure". + // Optional. + RestartRetries *uint `json:"restart_tries,omitempty"` // PodCreateCommand is the command used to create this pod. // This will be shown in the output of Inspect() on the pod, and may // also be used by some tools that wish to recreate the pod diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go index 409018086d..903232e448 100644 --- a/pkg/specgenutil/specgen.go +++ b/pkg/specgenutil/specgen.go @@ -808,27 +808,12 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions s.OOMScoreAdj = c.OOMScoreAdj } if c.Restart != "" { - splitRestart := strings.Split(c.Restart, ":") - switch len(splitRestart) { - case 1: - // No retries specified - case 2: - if strings.ToLower(splitRestart[0]) != "on-failure" { - return errors.New("restart policy retries can only be specified with on-failure restart policy") - } - retries, err := strconv.Atoi(splitRestart[1]) - if err != nil { - return fmt.Errorf("parsing restart policy retry count: %w", err) - } - if retries < 0 { - return errors.New("must specify restart policy retry count as a number greater than 0") - } - var retriesUint = uint(retries) - s.RestartRetries = &retriesUint - default: - return errors.New("invalid restart policy: may specify retries at most once") + policy, retries, err := util.ParseRestartPolicy(c.Restart) + if err != nil { + return err } - s.RestartPolicy = splitRestart[0] + s.RestartPolicy = policy + s.RestartRetries = &retries } if len(s.Secrets) == 0 || len(c.Secrets) != 0 { diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 69e23fcd01..4aab28964c 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -21,6 +21,7 @@ import ( "github.com/containers/image/v5/types" encconfig "github.com/containers/ocicrypt/config" enchelpers "github.com/containers/ocicrypt/helpers" + "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/pkg/errorhandling" "github.com/containers/podman/v4/pkg/namespaces" "github.com/containers/podman/v4/pkg/rootless" @@ -665,3 +666,37 @@ func DecryptConfig(decryptionKeys []string) (*encconfig.DecryptConfig, error) { return decryptConfig, nil } + +// ParseRestartPolicy parses the value given to the --restart flag and returns the policy +// and restart retries value +func ParseRestartPolicy(policy string) (string, uint, error) { + var ( + retriesUint uint + policyType string + ) + splitRestart := strings.Split(policy, ":") + switch len(splitRestart) { + case 1: + // No retries specified + policyType = splitRestart[0] + if strings.ToLower(splitRestart[0]) == "never" { + policyType = define.RestartPolicyNo + } + case 2: + if strings.ToLower(splitRestart[0]) != "on-failure" { + return "", 0, errors.New("restart policy retries can only be specified with on-failure restart policy") + } + retries, err := strconv.Atoi(splitRestart[1]) + if err != nil { + return "", 0, fmt.Errorf("parsing restart policy retry count: %w", err) + } + if retries < 0 { + return "", 0, errors.New("must specify restart policy retry count as a number greater than 0") + } + retriesUint = uint(retries) + policyType = splitRestart[0] + default: + return "", 0, errors.New("invalid restart policy: may specify retries at most once") + } + return policyType, retriesUint, nil +} diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go index 008eb3543f..bff398721f 100644 --- a/test/e2e/pod_create_test.go +++ b/test/e2e/pod_create_test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strconv" "strings" + "time" "github.com/containers/common/pkg/apparmor" "github.com/containers/common/pkg/seccomp" @@ -1212,4 +1213,115 @@ ENTRYPOINT ["sleep","99999"] Expect(inspect).Should(Exit(0)) Expect(inspect.OutputToString()).Should(Equal(pod2Name)) }) + + It("podman pod create --restart set to default", func() { + // When the --restart flag is not set, the default value is No + // TODO: v5.0 change this so that the default value is Always + podName := "mypod" + testCtr := "ctr1" + session := podmanTest.Podman([]string{"pod", "create", podName}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + // add container to pod + ctrRun := podmanTest.Podman([]string{"run", "--name", testCtr, "-d", "--pod", podName, ALPINE, "echo", "hello"}) + ctrRun.WaitWithDefaultTimeout() + Expect(ctrRun).Should(Exit(0)) + // Wait about 1 second, so we can check the number of restarts as default restart policy is set to No + time.Sleep(1 * time.Second) + ps := podmanTest.Podman([]string{"ps", "-a", "--filter", "name=" + testCtr, "--format", "{{.Restarts}}"}) + ps.WaitWithDefaultTimeout() + Expect(ps).Should(Exit(0)) + restarts, err := strconv.Atoi(ps.OutputToString()) + Expect(err).ToNot(HaveOccurred()) + Expect(restarts).To(BeNumerically("==", 0)) + ps = podmanTest.Podman([]string{"ps", "-a", "--filter", "name=" + testCtr, "--format", "{{.Status}}"}) + ps.WaitWithDefaultTimeout() + Expect(ps).Should(Exit(0)) + Expect(ps.OutputToString()).To(ContainSubstring("Exited")) + }) + + It("podman pod create --restart=on-failure", func() { + // Restart policy set to on-failure with max 2 retries + podName := "mypod" + runningCtr := "ctr1" + testCtr := "ctr2" + session := podmanTest.Podman([]string{"pod", "create", "--restart", "on-failure:2", podName}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + // add container to pod + ctrRun := podmanTest.Podman([]string{"run", "--name", runningCtr, "-d", "--pod", podName, ALPINE, "sleep", "100"}) + ctrRun.WaitWithDefaultTimeout() + Expect(ctrRun).Should(Exit(0)) + ctrRun = podmanTest.Podman([]string{"run", "--name", testCtr, "-d", "--pod", podName, ALPINE, "sh", "-c", "echo hello && exit 1"}) + ctrRun.WaitWithDefaultTimeout() + Expect(ctrRun).Should(Exit(0)) + // Wait about 2 seconds, so we can check the number of restarts after failure + time.Sleep(2 * time.Second) + ps := podmanTest.Podman([]string{"ps", "-a", "--filter", "name=" + testCtr, "--format", "{{.Restarts}}"}) + ps.WaitWithDefaultTimeout() + Expect(ps).Should(Exit(0)) + restarts, err := strconv.Atoi(ps.OutputToString()) + Expect(err).ToNot(HaveOccurred()) + Expect(restarts).To(BeNumerically("==", 2)) + ps = podmanTest.Podman([]string{"ps", "-a", "--filter", "name=" + testCtr, "--format", "{{.Status}}"}) + ps.WaitWithDefaultTimeout() + Expect(ps).Should(Exit(0)) + Expect(ps.OutputToString()).To(ContainSubstring("Exited")) + ps = podmanTest.Podman([]string{"ps", "-a", "--filter", "name=" + runningCtr, "--format", "{{.Status}}"}) + ps.WaitWithDefaultTimeout() + Expect(ps).Should(Exit(0)) + Expect(ps.OutputToString()).To(ContainSubstring("Up")) + }) + + It("podman pod create --restart=no/never", func() { + // never and no are the same, just different words to do the same thing + policy := []string{"no", "never"} + for _, p := range policy { + podName := "mypod-" + p + runningCtr := "ctr1-" + p + testCtr := "ctr2-" + p + testCtr2 := "ctr3-" + p + session := podmanTest.Podman([]string{"pod", "create", "--restart", p, podName}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + // add container to pod + ctrRun := podmanTest.Podman([]string{"run", "--name", runningCtr, "-d", "--pod", podName, ALPINE, "sleep", "100"}) + ctrRun.WaitWithDefaultTimeout() + Expect(ctrRun).Should(Exit(0)) + ctrRun = podmanTest.Podman([]string{"run", "--name", testCtr, "-d", "--pod", podName, ALPINE, "echo", "hello"}) + ctrRun.WaitWithDefaultTimeout() + Expect(ctrRun).Should(Exit(0)) + ctrRun = podmanTest.Podman([]string{"run", "--name", testCtr2, "-d", "--pod", podName, ALPINE, "sh", "-c", "echo hello && exit 1"}) + ctrRun.WaitWithDefaultTimeout() + Expect(ctrRun).Should(Exit(0)) + // Wait 1 second, so we can check the number of restarts and make sure the container has actually ran + time.Sleep(1 * time.Second) + // check first test container - container exits with exit code 0 + ps := podmanTest.Podman([]string{"ps", "-a", "--filter", "name=" + testCtr, "--format", "{{.Restarts}}"}) + ps.WaitWithDefaultTimeout() + Expect(ps).Should(Exit(0)) + restarts, err := strconv.Atoi(ps.OutputToString()) + Expect(err).ToNot(HaveOccurred()) + Expect(restarts).To(BeNumerically("==", 0)) + ps = podmanTest.Podman([]string{"ps", "-a", "--filter", "name=" + testCtr, "--format", "{{.Status}}"}) + ps.WaitWithDefaultTimeout() + Expect(ps).Should(Exit(0)) + Expect(ps.OutputToString()).To(ContainSubstring("Exited")) + // Check second test container - container exits with non-zero exit code + ps = podmanTest.Podman([]string{"ps", "-a", "--filter", "name=" + testCtr2, "--format", "{{.Restarts}}"}) + ps.WaitWithDefaultTimeout() + Expect(ps).Should(Exit(0)) + restarts, err = strconv.Atoi(ps.OutputToString()) + Expect(err).ToNot(HaveOccurred()) + Expect(restarts).To(BeNumerically("==", 0)) + ps = podmanTest.Podman([]string{"ps", "-a", "--filter", "name=" + testCtr2, "--format", "{{.Status}}"}) + ps.WaitWithDefaultTimeout() + Expect(ps).Should(Exit(0)) + Expect(ps.OutputToString()).To(ContainSubstring("Exited")) + ps = podmanTest.Podman([]string{"ps", "-a", "--filter", "name=" + runningCtr, "--format", "{{.Status}}"}) + ps.WaitWithDefaultTimeout() + Expect(ps).Should(Exit(0)) + Expect(ps.OutputToString()).To(ContainSubstring("Up")) + } + }) })