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/containers/ps.go b/cmd/podman/containers/ps.go index 09f73ffd22..621cc23e5e 100644 --- a/cmd/podman/containers/ps.go +++ b/cmd/podman/containers/ps.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "strconv" "strings" "time" @@ -300,6 +301,7 @@ func createPsOut() ([]map[string]string, string) { "PIDNS": "pidns", "Pod": "pod id", "PodName": "podname", // undo camelcase space break + "Restarts": "restarts", "RunningFor": "running for", "UTS": "uts", "User": "userns", @@ -377,6 +379,10 @@ func (l psReporter) Status() string { return state } +func (l psReporter) Restarts() string { + return strconv.Itoa(int(l.ListContainer.Restarts)) +} + func (l psReporter) RunningFor() string { return l.CreatedHuman() } 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/cmd/podman/pods/ps.go b/cmd/podman/pods/ps.go index 7ba9403f41..405d7cdc03 100644 --- a/cmd/podman/pods/ps.go +++ b/cmd/podman/pods/ps.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "sort" + "strconv" "strings" "time" @@ -147,6 +148,7 @@ func pods(cmd *cobra.Command, _ []string) error { "ContainerStatuses": "STATUS", "Cgroup": "CGROUP", "Namespace": "NAMESPACES", + "Restarts": "RESTARTS", }) if err := rpt.Execute(headers); err != nil { @@ -178,6 +180,7 @@ func podPsFormat() string { if !psInput.CtrStatus && !psInput.CtrNames && !psInput.CtrIds { row = append(row, "{{.NumberOfContainers}}") } + return "{{range . }}" + strings.Join(row, "\t") + "\n" + "{{end -}}" } @@ -265,6 +268,15 @@ func (l ListPodReporter) ContainerStatuses() string { return strings.Join(statuses, ",") } +// Restarts returns the total number of restarts for all the containers in the pod +func (l ListPodReporter) Restarts() string { + restarts := 0 + for _, c := range l.Containers { + restarts += int(c.RestartCount) + } + return strconv.Itoa(restarts) +} + func sortPodPsOutput(sortBy string, lprs []*entities.ListPodsReport) error { switch sortBy { case "created": 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/docs/source/markdown/podman-pod-ps.1.md.in b/docs/source/markdown/podman-pod-ps.1.md.in index a0d99a722a..6e369119ff 100644 --- a/docs/source/markdown/podman-pod-ps.1.md.in +++ b/docs/source/markdown/podman-pod-ps.1.md.in @@ -77,20 +77,21 @@ Pretty-print containers to JSON or using a Go template Valid placeholders for the Go template are listed below: -| **Placeholder** | **Description** | -|---------------------|----------------------------------------------------| -| .Cgroup | Cgroup path of pod | -| .ContainerIds | Comma-separated list of container IDs in the pod | -| .ContainerNames | Comma-separated list of container names in the pod | -| .ContainerStatuses | Comma-separated list of container statuses | -| .Created | Creation time of pod | -| .ID | Container ID | -| .InfraID | Pod infra container ID | -| .Labels | All the labels assigned to the pod | -| .Name | Name of pod | -| .Networks | Show all networks connected to the infra container | -| .NumberOfContainers | Show the number of containers attached to pod | -| .Status | Status of pod | +| **Placeholder** | **Description** | +|---------------------|------------------------------------------------------| +| .Cgroup | Cgroup path of pod | +| .ContainerIds | Comma-separated list of container IDs in the pod | +| .ContainerNames | Comma-separated list of container names in the pod | +| .ContainerStatuses | Comma-separated list of container statuses | +| .Created | Creation time of pod | +| .ID | Container ID | +| .InfraID | Pod infra container ID | +| .Labels | All the labels assigned to the pod | +| .Name | Name of pod | +| .Networks | Show all networks connected to the infra container | +| .NumberOfContainers | Show the number of containers attached to pod | +| .Restarts | Show the total number of container restarts in a pod | +| .Status | Status of pod | #### **--help**, **-h** diff --git a/docs/source/markdown/podman-ps.1.md b/docs/source/markdown/podman-ps.1.md index 43a5eb82cb..efe7a15c27 100644 --- a/docs/source/markdown/podman-ps.1.md +++ b/docs/source/markdown/podman-ps.1.md @@ -89,6 +89,7 @@ Valid placeholders for the Go template are listed below: | .Pod | Pod the container is associated with (SHA) | | .PodName | Seems to be empty no matter what | | .Ports | Exposed ports | +| .Restarts | Display the container restart count | | .RunningFor | Time elapsed since container was started | | .Size | Size of container | | .StartedAt | Time (epoch seconds) the container started | diff --git a/libpod/container.go b/libpod/container.go index b92255d7ed..640e24a5f9 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -203,7 +203,6 @@ type ContainerState struct { // restart policy. This is NOT incremented by normal container restarts // (only by restart policy). RestartCount uint `json:"restartCount,omitempty"` - // StartupHCPassed indicates that the startup healthcheck has // succeeded and the main healthcheck can begin. StartupHCPassed bool `json:"startupHCPassed,omitempty"` @@ -730,6 +729,18 @@ func (c *Container) State() (define.ContainerStatus, error) { return c.state.State, nil } +func (c *Container) RestartCount() (uint, error) { + if !c.batched { + c.lock.Lock() + defer c.lock.Unlock() + + if err := c.syncContainer(); err != nil { + return 0, err + } + } + return c.state.RestartCount, nil +} + // Mounted returns whether the container is mounted and the path it is mounted // at (if it is mounted). // If the container is not mounted, no error is returned, and the mountpoint 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/kube.go b/libpod/kube.go index 11fb8da856..093760de72 100644 --- a/libpod/kube.go +++ b/libpod/kube.go @@ -97,23 +97,8 @@ func (p *Pod) GenerateForKube(ctx context.Context, getService bool) (*v1.Pod, [] } pod.Spec.HostAliases = extraHost - // vendor/k8s.io/api/core/v1/types.go: v1.Container cannot save restartPolicy - // so set it at here - for _, ctr := range allContainers { - if !ctr.IsInfra() { - switch ctr.config.RestartPolicy { - case define.RestartPolicyAlways: - pod.Spec.RestartPolicy = v1.RestartPolicyAlways - case define.RestartPolicyOnFailure: - pod.Spec.RestartPolicy = v1.RestartPolicyOnFailure - case define.RestartPolicyNo: - pod.Spec.RestartPolicy = v1.RestartPolicyNever - default: // some pod create from cmdline, such as "", so set it to "" as k8s automatically defaults to always - pod.Spec.RestartPolicy = "" - } - break - } - } + // Set the pod's restart policy + pod.Spec.RestartPolicy = getPodRestartPolicy(p.config.RestartPolicy) if p.SharesPID() { // unfortunately, go doesn't have a nice way to specify a pointer to a bool @@ -136,7 +121,7 @@ func (p *Pod) getInfraContainer() (*Container, error) { // kind YAML. func GenerateForKubeDeployment(ctx context.Context, pod *YAMLPod, options entities.GenerateKubeOptions) (*YAMLDeployment, error) { // Restart policy for Deployments can only be set to Always - if options.Type == define.K8sKindDeployment && !(pod.Spec.RestartPolicy == "" || pod.Spec.RestartPolicy == define.RestartPolicyAlways) { + if options.Type == define.K8sKindDeployment && !(pod.Spec.RestartPolicy == "" || pod.Spec.RestartPolicy == v1.RestartPolicyAlways) { return nil, fmt.Errorf("k8s Deployments can only have restartPolicy set to Always") } @@ -599,6 +584,7 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container, getServic kubeAnnotations := make(map[string]string) ctrNames := make([]string, 0, len(ctrs)) var hostname string + var restartPolicy *string for _, ctr := range ctrs { ctrNames = append(ctrNames, removeUnderscores(ctr.Name())) for k, v := range ctr.config.Spec.Annotations { @@ -623,6 +609,11 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container, getServic } } + // Use the restart policy of the first non-init container + if !isInit && restartPolicy == nil { + restartPolicy = &ctr.config.RestartPolicy + } + if ctr.config.Spec.Process != nil { var ulimitArr []string defaultUlimits := util.DefaultContainerConfig().Ulimits() @@ -700,7 +691,7 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container, getServic podName += "-pod" } - return newPodObject( + pod := newPodObject( podName, kubeAnnotations, kubeInitCtrs, @@ -709,7 +700,30 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container, getServic &podDNS, hostNetwork, hostUsers, - hostname), nil + hostname) + + // Set the pod's restart policy + policy := "" + if restartPolicy != nil { + policy = *restartPolicy + } + pod.Spec.RestartPolicy = getPodRestartPolicy(policy) + + return pod, nil +} + +// getPodRestartPolicy returns the pod restart policy to be set in the generated kube yaml +func getPodRestartPolicy(policy string) v1.RestartPolicy { + switch policy { + case define.RestartPolicyNo: + return v1.RestartPolicyNever + case define.RestartPolicyAlways: + return v1.RestartPolicyAlways + case define.RestartPolicyOnFailure: + return v1.RestartPolicyOnFailure + default: // some pod/ctr create from cmdline, such as "" - set it to "" and let k8s handle the defaults + return "" + } } // containerToV1Container converts information we know about a libpod container 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/container_ps.go b/pkg/domain/entities/container_ps.go index 519d2b7da7..93fdae7470 100644 --- a/pkg/domain/entities/container_ps.go +++ b/pkg/domain/entities/container_ps.go @@ -55,6 +55,10 @@ type ListContainer struct { PodName string // Port mappings Ports []types.PortMapping + // Restarts is how many times the container was restarted by its + // restart policy. This is NOT incremented by normal container restarts + // (only by restart policy). + Restarts uint // Size of the container rootfs. Requires the size boolean to be true Size *define.ContainerSize // Time when container started diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go index b0ff5cdaf4..585b0e6ca5 100644 --- a/pkg/domain/entities/pods.go +++ b/pkg/domain/entities/pods.go @@ -38,9 +38,10 @@ type ListPodsReport struct { } type ListPodContainer struct { - Id string //nolint:revive,stylecheck - Names string - Status string + Id string //nolint:revive,stylecheck + Names string + Status string + RestartCount uint } type PodPauseOptions struct { @@ -135,6 +136,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 +377,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/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index dfcdb0ce70..ce602b5e74 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -592,16 +592,16 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY return nil, nil, err } - var ctrRestartPolicy string + // Set the restart policy from the kube yaml at the pod level in podman switch podYAML.Spec.RestartPolicy { case v1.RestartPolicyAlways: - ctrRestartPolicy = define.RestartPolicyAlways + podSpec.PodSpecGen.RestartPolicy = define.RestartPolicyAlways case v1.RestartPolicyOnFailure: - ctrRestartPolicy = define.RestartPolicyOnFailure + podSpec.PodSpecGen.RestartPolicy = define.RestartPolicyOnFailure case v1.RestartPolicyNever: - ctrRestartPolicy = define.RestartPolicyNo + podSpec.PodSpecGen.RestartPolicy = define.RestartPolicyNo default: // Default to Always - ctrRestartPolicy = define.RestartPolicyAlways + podSpec.PodSpecGen.RestartPolicy = define.RestartPolicyAlways } if podOpt.Infra { @@ -775,7 +775,6 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY PodName: podName, PodSecurityContext: podYAML.Spec.SecurityContext, ReadOnly: readOnly, - RestartPolicy: ctrRestartPolicy, SeccompPaths: seccompPaths, SecretsManager: secretsManager, UserNSIsHost: p.Userns.IsHost(), diff --git a/pkg/domain/infra/abi/pods.go b/pkg/domain/infra/abi/pods.go index 4b269d9258..e54e287d7a 100644 --- a/pkg/domain/infra/abi/pods.go +++ b/pkg/domain/infra/abi/pods.go @@ -429,10 +429,15 @@ func (ic *ContainerEngine) listPodReportFromPod(p *libpod.Pod) (*entities.ListPo if err != nil { return nil, err } + restartCount, err := c.RestartCount() + if err != nil { + return nil, err + } lpcs[i] = &entities.ListPodContainer{ - Id: c.ID(), - Names: c.Name(), - Status: state.String(), + Id: c.ID(), + Names: c.Name(), + Status: state.String(), + RestartCount: restartCount, } } infraID, err := p.InfraContainerID() diff --git a/pkg/ps/ps.go b/pkg/ps/ps.go index 44a1568806..abbe38f1c5 100644 --- a/pkg/ps/ps.go +++ b/pkg/ps/ps.go @@ -145,6 +145,7 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities portMappings []libnetworkTypes.PortMapping networks []string healthStatus string + restartCount uint ) batchErr := ctr.Batch(func(c *libpod.Container) error { @@ -193,6 +194,11 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities return err } + restartCount, err = c.RestartCount() + if err != nil { + return err + } + if !opts.Size && !opts.Namespace { return nil } @@ -251,6 +257,7 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities StartedAt: startedTime.Unix(), State: conState.String(), Status: healthStatus, + Restarts: restartCount, } if opts.Pod && len(conConfig.Pod) > 0 { podName, err := rt.GetPodName(conConfig.Pod) 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 != "" { + 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 + } + restartPolicy = podConfig.RestartPolicy + } else if s.RestartPolicy != "" { if s.RestartRetries != nil { - options = append(options, libpod.WithRestartRetries(*s.RestartRetries)) + retries = *s.RestartRetries } - options = append(options, libpod.WithRestartPolicy(s.RestartPolicy)) + 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/generate_kube_test.go b/test/e2e/generate_kube_test.go index ee7339c7e9..12ae1ff340 100644 --- a/test/e2e/generate_kube_test.go +++ b/test/e2e/generate_kube_test.go @@ -528,18 +528,21 @@ var _ = Describe("Podman kube generate", func() { Expect(err).ToNot(HaveOccurred()) }) - It("podman generate kube on pod with restartPolicy", func() { + It("podman generate kube on pod with restartPolicy set for container in a pod", func() { + //TODO: v5.0 - change/remove test once we block --restart on container when it is in a pod // podName, set, expect testSli := [][]string{ {"testPod1", "", ""}, // some pod create from cmdline, so set it to an empty string and let k8s default it to Always {"testPod2", "always", "Always"}, {"testPod3", "on-failure", "OnFailure"}, {"testPod4", "no", "Never"}, + {"testPod5", "never", "Never"}, } for k, v := range testSli { podName := v[0] - podSession := podmanTest.Podman([]string{"pod", "create", "--name", podName}) + // Need to set --restart during pod creation as gen kube only picks up the pod's restart policy + podSession := podmanTest.Podman([]string{"pod", "create", "--restart", v[1], "--name", podName}) podSession.WaitWithDefaultTimeout() Expect(podSession).Should(Exit(0)) @@ -561,6 +564,67 @@ var _ = Describe("Podman kube generate", func() { } }) + It("podman generate kube on pod with restartPolicy", func() { + // podName, set, expect + testSli := [][]string{ + {"testPod1", "", ""}, + {"testPod2", "always", "Always"}, + {"testPod3", "on-failure", "OnFailure"}, + {"testPod4", "no", "Never"}, + {"testPod5", "never", "Never"}, + } + + for k, v := range testSli { + podName := v[0] + podSession := podmanTest.Podman([]string{"pod", "create", "--restart", v[1], podName}) + podSession.WaitWithDefaultTimeout() + Expect(podSession).Should(Exit(0)) + + ctrName := "ctr" + strconv.Itoa(k) + ctr1Session := podmanTest.Podman([]string{"create", "--name", ctrName, "--pod", podName, ALPINE, "top"}) + ctr1Session.WaitWithDefaultTimeout() + Expect(ctr1Session).Should(Exit(0)) + + kube := podmanTest.Podman([]string{"generate", "kube", podName}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + pod := new(v1.Pod) + err := yaml.Unmarshal(kube.Out.Contents(), pod) + Expect(err).ToNot(HaveOccurred()) + + Expect(string(pod.Spec.RestartPolicy)).To(Equal(v[2])) + } + }) + + It("podman generate kube on ctr with restartPolicy", func() { + // podName, set, expect + testSli := [][]string{ + {"", ""}, // some ctr created from cmdline, set it to "" and let k8s default it to Always + {"always", "Always"}, + {"on-failure", "OnFailure"}, + {"no", "Never"}, + {"never", "Never"}, + } + + for k, v := range testSli { + ctrName := "ctr" + strconv.Itoa(k) + ctrSession := podmanTest.Podman([]string{"create", "--restart", v[0], "--name", ctrName, ALPINE, "top"}) + ctrSession.WaitWithDefaultTimeout() + Expect(ctrSession).Should(Exit(0)) + + kube := podmanTest.Podman([]string{"generate", "kube", ctrName}) + kube.WaitWithDefaultTimeout() + Expect(kube).Should(Exit(0)) + + pod := new(v1.Pod) + err := yaml.Unmarshal(kube.Out.Contents(), pod) + Expect(err).ToNot(HaveOccurred()) + + Expect(string(pod.Spec.RestartPolicy)).To(Equal(v[1])) + } + }) + It("podman generate kube on pod with memory limit", func() { SkipIfRootlessCgroupsV1("Not supported for rootless + CgroupsV1") podName := "testMemoryLimit" @@ -1465,13 +1529,12 @@ USER test1` }) It("podman generate kube on pod with --type=deployment and --restart=no should fail", func() { - // TODO: When we add --restart for pods, fix this test to reflect that podName := "test-pod" - session := podmanTest.Podman([]string{"pod", "create", podName}) + session := podmanTest.Podman([]string{"pod", "create", "--restart", "no", podName}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - session = podmanTest.Podman([]string{"create", "--pod", podName, "--restart", "no", ALPINE, "top"}) + session = podmanTest.Podman([]string{"create", "--pod", podName, ALPINE, "top"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index dc2e07555f..b3d5ffe18f 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -2382,7 +2382,7 @@ var _ = Describe("Podman play kube", func() { kube.WaitWithDefaultTimeout() Expect(kube).Should(Exit(0)) - inspect := podmanTest.Podman([]string{"inspect", getCtrNameInPod(pod), "--format", "{{.HostConfig.RestartPolicy.Name}}"}) + inspect := podmanTest.Podman([]string{"inspect", pod.Name, "--format", "{{.RestartPolicy}}"}) inspect.WaitWithDefaultTimeout() Expect(inspect).Should(Exit(0)) Expect(inspect.OutputToString()).To(Equal(v[2])) 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")) + } + }) })