Add support for updating restart policy

This is something Docker does, and we did not do until now. Most
difficult/annoying part was the REST API, where I did not really
want to modify the struct being sent, so I made the new restart
policy parameters query parameters instead.

Testing was also a bit annoying, because testing restart policy
always is.

Signed-off-by: Matt Heon <mheon@redhat.com>

cmd/podman/common/create.go

@@ -643,7 +643,8 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 			`If a container with the same name exists, replace it`,
 		)
 	}
-	if mode == entities.InfraMode || (mode == entities.CreateMode) { // infra container flags, create should also pick these up
+	// Restart is allowed for created, updated, and infra ctr
+	if mode == entities.InfraMode || mode == entities.CreateMode || mode == entities.UpdateMode {
 		restartFlagName := "restart"
 		createFlags.StringVar(
 			&cf.Restart,
@@ -651,7 +652,8 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 			`Restart policy to apply when a container exits ("always"|"no"|"never"|"on-failure"|"unless-stopped")`,
 		)
 		_ = cmd.RegisterFlagCompletionFunc(restartFlagName, AutocompleteRestartOption)
-
+	}
+	if mode == entities.InfraMode || (mode == entities.CreateMode) { // infra container flags, create should also pick these up
 		shmSizeFlagName := "shm-size"
 		createFlags.String(
 			shmSizeFlagName, shmSize(),

cmd/podman/containers/update.go

@@ -7,9 +7,11 @@ import (
 
 	"github.com/containers/podman/v5/cmd/podman/common"
 	"github.com/containers/podman/v5/cmd/podman/registry"
+	"github.com/containers/podman/v5/libpod/define"
 	"github.com/containers/podman/v5/pkg/domain/entities"
 	"github.com/containers/podman/v5/pkg/specgen"
 	"github.com/containers/podman/v5/pkg/specgenutil"
+	"github.com/containers/podman/v5/pkg/util"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/spf13/cobra"
 )
@@ -70,6 +72,17 @@ func update(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	if updateOpts.Restart != "" {
+		policy, retries, err := util.ParseRestartPolicy(updateOpts.Restart)
+		if err != nil {
+			return err
+		}
+		s.RestartPolicy = policy
+		if policy == define.RestartPolicyOnFailure {
+			s.RestartRetries = &retries
+		}
+	}
+
 	// we need to pass the whole specgen since throttle devices are parsed later due to cross compat.
 	s.ResourceLimits, err = specgenutil.GetResources(s, &updateOpts)
 	if err != nil {

docs/source/markdown/options/restart.md

@@ -1,5 +1,5 @@
 ####> This option file is used in:
-####>   podman create, pod clone, pod create, run
+####>   podman create, pod clone, pod create, run, update
 ####> If file is edited, make sure the changes
 ####> are applicable to all of those.
 #### **--restart**=*policy*

docs/source/markdown/podman-update.1.md.in

@@ -53,6 +53,8 @@ The currently supported options are a subset of the podman create/run resource l
 
 @@option pids-limit
 
+@@option restart
+
 
 ## EXAMPLEs
 

libpod/container_api.go

@@ -119,8 +119,10 @@ func (c *Container) Start(ctx context.Context, recursive bool) (finalErr error)
 }
 
 // Update updates the given container.
-// only the cgroup config can be updated and therefore only a linux resource spec is passed.
-func (c *Container) Update(res *spec.LinuxResources) error {
+// Either resource limits or restart policy can be updated.
+// Either resourcs or restartPolicy must not be nil.
+// If restartRetries is not nil, restartPolicy must be set and must be "on-failure".
+func (c *Container) Update(resources *spec.LinuxResources, restartPolicy *string, restartRetries *uint) error {
 	if !c.batched {
 		c.lock.Lock()
 		defer c.lock.Unlock()
@@ -134,7 +136,7 @@ func (c *Container) Update(res *spec.LinuxResources) error {
 		return fmt.Errorf("container %s is being removed, cannot update: %w", c.ID(), define.ErrCtrStateInvalid)
 	}
 
-	return c.update(res)
+	return c.update(resources, restartPolicy, restartRetries)
 }
 
 // StartAndAttach starts a container and attaches to it.

libpod/container_inspect.go

@@ -467,6 +467,9 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named
 
 	restartPolicy := new(define.InspectRestartPolicy)
 	restartPolicy.Name = c.config.RestartPolicy
+	if restartPolicy.Name == "" {
+		restartPolicy.Name = define.RestartPolicyNo
+	}
 	restartPolicy.MaximumRetryCount = c.config.RestartRetries
 	hostConfig.RestartPolicy = restartPolicy
 	if c.config.NoCgroups {

libpod/container_internal.go

@@ -2514,22 +2514,55 @@ func (c *Container) extractSecretToCtrStorage(secr *ContainerSecret) error {
 	return nil
 }
 
-// Update a container's resources after creation
-func (c *Container) update(resources *spec.LinuxResources) error {
-	oldResources := c.config.Spec.Linux.Resources
-
-	if c.config.Spec.Linux == nil {
-		c.config.Spec.Linux = new(spec.Linux)
+// Update a container's resources or restart policy after creation.
+// At least one of resources or restartPolicy must not be nil.
+func (c *Container) update(resources *spec.LinuxResources, restartPolicy *string, restartRetries *uint) error {
+	if resources == nil && restartPolicy == nil {
+		return fmt.Errorf("must provide at least one of resources and restartPolicy to update a container: %w", define.ErrInvalidArg)
+	}
+	if restartRetries != nil && restartPolicy == nil {
+		return fmt.Errorf("must provide restart policy if updating restart retries: %w", define.ErrInvalidArg)
+	}
+
+	oldResources := c.config.Spec.Linux.Resources
+	oldRestart := c.config.RestartPolicy
+	oldRetries := c.config.RestartRetries
+
+	if restartPolicy != nil {
+		if err := define.ValidateRestartPolicy(*restartPolicy); err != nil {
+			return err
+		}
+
+		if restartRetries != nil {
+			if *restartPolicy != define.RestartPolicyOnFailure {
+				return fmt.Errorf("cannot set restart policy retries unless policy is on-failure: %w", define.ErrInvalidArg)
+			}
+		}
+
+		c.config.RestartPolicy = *restartPolicy
+		if restartRetries != nil {
+			c.config.RestartRetries = *restartRetries
+		} else {
+			c.config.RestartRetries = 0
+		}
+	}
+
+	if resources != nil {
+		if c.config.Spec.Linux == nil {
+			c.config.Spec.Linux = new(spec.Linux)
+		}
+		c.config.Spec.Linux.Resources = resources
 	}
-	c.config.Spec.Linux.Resources = resources
 
 	if err := c.runtime.state.SafeRewriteContainerConfig(c, "", "", c.config); err != nil {
 		// Assume DB write failed, revert to old resources block
 		c.config.Spec.Linux.Resources = oldResources
+		c.config.RestartPolicy = oldRestart
+		c.config.RestartRetries = oldRetries
 		return err
 	}
 
-	if c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStatePaused) {
+	if c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStatePaused) && resources != nil {
 		// So `podman inspect` on running containers sources its OCI spec from disk.
 		// To keep inspect accurate we need to update the on-disk OCI spec.
 		onDiskSpec, err := c.specFromState()

libpod/define/container.go

@@ -1,5 +1,9 @@
 package define
 
+import (
+	"fmt"
+)
+
 // Valid restart policy types.
 const (
 	// RestartPolicyNone indicates that no restart policy has been requested
@@ -27,6 +31,16 @@ var RestartPolicyMap = map[string]string{
 	RestartPolicyUnlessStopped: RestartPolicyUnlessStopped,
 }
 
+// Validate that the given string is a valid restart policy.
+func ValidateRestartPolicy(policy string) error {
+	switch policy {
+	case RestartPolicyNone, RestartPolicyNo, RestartPolicyOnFailure, RestartPolicyAlways, RestartPolicyUnlessStopped:
+		return nil
+	default:
+		return fmt.Errorf("%q is not a valid restart policy: %w", policy, ErrInvalidArg)
+	}
+}
+
 // InitContainerTypes
 const (
 	// AlwaysInitContainer is an init container that runs on each

libpod/options.go

@@ -1392,13 +1392,12 @@ func WithRestartPolicy(policy string) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		switch policy {
-		case define.RestartPolicyNone, define.RestartPolicyNo, define.RestartPolicyOnFailure, define.RestartPolicyAlways, define.RestartPolicyUnlessStopped:
-			ctr.config.RestartPolicy = policy
-		default:
-			return fmt.Errorf("%q is not a valid restart policy: %w", policy, define.ErrInvalidArg)
+		if err := define.ValidateRestartPolicy(policy); err != nil {
+			return err
 		}
 
+		ctr.config.RestartPolicy = policy
+
 		return nil
 	}
 }

pkg/api/handlers/compat/containers.go

@@ -774,8 +774,18 @@ func UpdateContainer(w http.ResponseWriter, r *http.Request) {
 		resources.BlockIO.Weight = &options.BlkioWeight
 	}
 
-	if err := ctr.Update(resources); err != nil {
-		utils.Error(w, http.StatusInternalServerError, fmt.Errorf("updating resources: %w", err))
+	// Restart policy
+	localPolicy := string(options.RestartPolicy.Name)
+	restartPolicy := &localPolicy
+
+	var restartRetries *uint
+	if options.RestartPolicy.MaximumRetryCount != 0 {
+		localRetries := uint(options.RestartPolicy.MaximumRetryCount)
+		restartRetries = &localRetries
+	}
+
+	if err := ctr.Update(resources, restartPolicy, restartRetries); err != nil {
+		utils.Error(w, http.StatusInternalServerError, fmt.Errorf("updating container: %w", err))
 		return
 	}
 

pkg/api/handlers/libpod/containers.go

@@ -402,18 +402,46 @@ func InitContainer(w http.ResponseWriter, r *http.Request) {
 func UpdateContainer(w http.ResponseWriter, r *http.Request) {
 	name := utils.GetName(r)
 	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
+	decoder := utils.GetDecoder(r)
+	query := struct {
+		RestartPolicy  string `schema:"restartPolicy"`
+		RestartRetries uint   `schema:"restartRetries"`
+	}{
+		// override any golang type defaults
+	}
+
+	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
+		utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse parameters for %s: %w", r.URL.String(), err))
+		return
+	}
+
 	ctr, err := runtime.LookupContainer(name)
 	if err != nil {
 		utils.ContainerNotFound(w, name, err)
 		return
 	}
 
+	var restartPolicy *string
+	var restartRetries *uint
+	if query.RestartPolicy != "" {
+		restartPolicy = &query.RestartPolicy
+		if query.RestartPolicy == define.RestartPolicyOnFailure {
+			restartRetries = &query.RestartRetries
+		} else if query.RestartRetries != 0 {
+			utils.Error(w, http.StatusBadRequest, errors.New("cannot set restart retries unless restart policy is on-failure"))
+			return
+		}
+	} else if query.RestartRetries != 0 {
+		utils.Error(w, http.StatusBadRequest, errors.New("cannot set restart retries unless restart policy is set"))
+		return
+	}
+
 	options := &handlers.UpdateEntities{Resources: &specs.LinuxResources{}}
 	if err := json.NewDecoder(r.Body).Decode(&options.Resources); err != nil {
 		utils.Error(w, http.StatusInternalServerError, fmt.Errorf("decode(): %w", err))
 		return
 	}
-	err = ctr.Update(options.Resources)
+	err = ctr.Update(options.Resources, restartPolicy, restartRetries)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return

pkg/api/handlers/swagger/models.go

@@ -4,6 +4,7 @@ package swagger
 import (
 	"github.com/containers/podman/v5/pkg/domain/entities"
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
 )
 
 // Details for creating a volume
@@ -48,3 +49,7 @@ type networkConnectRequestLibpod entities.NetworkConnectOptions
 // Network update
 // swagger:model
 type networkUpdateRequestLibpod entities.NetworkUpdateOptions
+
+// Container update
+// swagger:model
+type containerUpdateRequest container.UpdateConfig

pkg/api/server/register_containers.go

@@ -689,9 +689,10 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error {
 	//    description: Full or partial ID or full name of the container to rename
 	//  - in: body
 	//    name: resources
+	//    required: false
 	//    description: attributes for updating the container
 	//    schema:
-	//      $ref: "#/definitions/UpdateConfig"
+	//      $ref: "#/definitions/containerUpdateRequest"
 	// produces:
 	// - application/json
 	// responses:
@@ -1783,8 +1784,18 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error {
 	//    type: string
 	//    required: true
 	//    description: Full or partial ID or full name of the container to update
+	//  - in: query
+	//    name: restartPolicy
+	//    type: string
+	//    required: false
+	//    description: New restart policy for the container.
+	//  - in: query
+	//    name: restartRetries
+	//    type: integer
+	//    required: false
+	//    description: New amount of retries for the container's restart policy. Only allowed if restartPolicy is set to on-failure
 	//  - in: body
-	//    name: resources
+	//    name: config
 	//    description: attributes for updating the container
 	//    schema:
 	//      $ref: "#/definitions/UpdateEntities"
@@ -1794,6 +1805,8 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error {
 	//   responses:
 	//     201:
 	//       $ref: "#/responses/containerUpdateResponse"
+	//   400:
+	//     $ref: "#/responses/badParamError"
 	//   404:
 	//     $ref: "#/responses/containerNotFound"
 	//   500:

pkg/bindings/containers/update.go

@@ -3,6 +3,8 @@ package containers
 import (
 	"context"
 	"net/http"
+	"net/url"
+	"strconv"
 	"strings"
 
 	"github.com/containers/podman/v5/pkg/bindings"
@@ -16,12 +18,20 @@ func Update(ctx context.Context, options *types.ContainerUpdateOptions) (string,
 		return "", err
 	}
 
+	params := url.Values{}
+	if options.Specgen.RestartPolicy != "" {
+		params.Set("restartPolicy", options.Specgen.RestartPolicy)
+		if options.Specgen.RestartRetries != nil {
+			params.Set("restartRetries", strconv.Itoa(int(*options.Specgen.RestartRetries)))
+		}
+	}
+
 	resources, err := jsoniter.MarshalToString(options.Specgen.ResourceLimits)
 	if err != nil {
 		return "", err
 	}
 	stringReader := strings.NewReader(resources)
-	response, err := conn.DoRequest(ctx, stringReader, http.MethodPost, "/containers/%s/update", nil, nil, options.NameOrID)
+	response, err := conn.DoRequest(ctx, stringReader, http.MethodPost, "/containers/%s/update", params, nil, options.NameOrID)
 	if err != nil {
 		return "", err
 	}

pkg/domain/infra/abi/containers.go

@@ -1767,7 +1767,12 @@ func (ic *ContainerEngine) ContainerUpdate(ctx context.Context, updateOptions *e
 		return "", fmt.Errorf("container not found")
 	}
 
-	if err = containers[0].Update(updateOptions.Specgen.ResourceLimits); err != nil {
+	var restartPolicy *string
+	if updateOptions.Specgen.RestartPolicy != "" {
+		restartPolicy = &updateOptions.Specgen.RestartPolicy
+	}
+
+	if err = containers[0].Update(updateOptions.Specgen.ResourceLimits, restartPolicy, updateOptions.Specgen.RestartRetries); err != nil {
 		return "", err
 	}
 	return containers[0].ID(), nil

pkg/specgen/generate/container_create.go

@@ -619,7 +619,12 @@ func createContainerOptions(rt *libpod.Runtime, s *specgen.SpecGenerator, pod *l
 		}
 		restartPolicy = s.RestartPolicy
 	}
-	options = append(options, libpod.WithRestartRetries(retries), libpod.WithRestartPolicy(restartPolicy))
+	if restartPolicy != "" {
+		options = append(options, libpod.WithRestartPolicy(restartPolicy))
+	}
+	if retries != 0 {
+		options = append(options, libpod.WithRestartRetries(retries))
+	}
 
 	healthCheckSet := false
 	if s.ContainerHealthCheckConfig.HealthConfig != nil {

test/e2e/common_test.go

@@ -454,6 +454,31 @@ func (p *PodmanTestIntegration) InspectContainer(name string) []define.InspectCo
 	return session.InspectContainerToJSON()
 }
 
+// Pull a single field from a container using `podman inspect --format {{ field }}`,
+// and verify it against the given expected value.
+func (p *PodmanTestIntegration) CheckContainerSingleField(name, field, expected string) {
+	inspect := p.Podman([]string{"inspect", "--format", fmt.Sprintf("{{ %s }}", field), name})
+	inspect.WaitWithDefaultTimeout()
+	ExpectWithOffset(1, inspect).Should(Exit(0))
+	ExpectWithOffset(1, inspect.OutputToString()).To(Equal(expected))
+}
+
+// Check that the contents of a single file in the given container matches the expected value.
+func (p *PodmanTestIntegration) CheckFileInContainer(name, filepath, expected string) {
+	exec := p.Podman([]string{"exec", name, "cat", filepath})
+	exec.WaitWithDefaultTimeout()
+	ExpectWithOffset(1, exec).Should(Exit(0))
+	ExpectWithOffset(1, exec.OutputToString()).To(Equal(expected))
+}
+
+// Check that the contents of a single file in the given container containers the given value.
+func (p *PodmanTestIntegration) CheckFileInContainerSubstring(name, filepath, expected string) {
+	exec := p.Podman([]string{"exec", name, "cat", filepath})
+	exec.WaitWithDefaultTimeout()
+	ExpectWithOffset(1, exec).Should(Exit(0))
+	ExpectWithOffset(1, exec.OutputToString()).To(ContainSubstring(expected))
+}
+
 // StopContainer stops a container with no timeout, ensuring a fast test.
 func (p *PodmanTestIntegration) StopContainer(nameOrID string) {
 	stop := p.Podman([]string{"stop", "-t0", nameOrID})

test/e2e/update_test.go

@@ -35,47 +35,25 @@ var _ = Describe("Podman update", func() {
 		Expect(session).Should(ExitCleanly())
 
 		// checking cpu quota from --cpus
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/cpu/cpu.cfs_quota_us"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("500000"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/cpu/cpu.cfs_quota_us", "500000")
 
 		// checking cpuset-cpus
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/cpuset/cpuset.cpus"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(Equal("0"))
+		podmanTest.CheckFileInContainer(ctrID, "/sys/fs/cgroup/cpuset/cpuset.cpus", "0")
 
 		// checking cpuset-mems
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/cpuset/cpuset.mems"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(Equal("0"))
+		podmanTest.CheckFileInContainer(ctrID, "/sys/fs/cgroup/cpuset/cpuset.mems", "0")
 
 		// checking memory limit
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/memory/memory.limit_in_bytes"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("1073741824"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/memory/memory.limit_in_bytes", "1073741824")
 
 		// checking memory-swap
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("2147483648"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes", "2147483648")
 
 		// checking cpu-shares
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/cpu/cpu.shares"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("123"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/cpu/cpu.shares", "123")
 
 		// checking pids-limit
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/pids/pids.max"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("123"))
-
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/pids/pids.max", "123")
 	})
 
 	It("podman update container unspecified pid limit", func() {
@@ -99,10 +77,7 @@ var _ = Describe("Podman update", func() {
 		ctrID = session.OutputToString()
 
 		// checking pids-limit was not changed after update when not specified as an option
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/pids.max"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("max"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/pids.max", "max")
 	})
 
 	It("podman update container all options v2", func() {
@@ -138,58 +113,31 @@ var _ = Describe("Podman update", func() {
 		ctrID = session.OutputToString()
 
 		// checking cpu quota and period
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/cpu.max"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("500000"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/cpu.max", "500000")
 
 		// checking blkio weight
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/io.bfq.weight"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("123"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/io.bfq.weight", "123")
 
 		// checking device-read/write-bps/iops
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/io.max"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("rbps=10485760 wbps=10485760 riops=1000 wiops=1000"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/io.max", "rbps=10485760 wbps=10485760 riops=1000 wiops=1000")
 
 		// checking cpuset-cpus
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/cpuset.cpus"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(Equal("0"))
+		podmanTest.CheckFileInContainer(ctrID, "/sys/fs/cgroup/cpuset.cpus", "0")
 
 		// checking cpuset-mems
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/cpuset.mems"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(Equal("0"))
+		podmanTest.CheckFileInContainer(ctrID, "/sys/fs/cgroup/cpuset.mems", "0")
 
 		// checking memory limit
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/memory.max"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("1073741824"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/memory.max", "1073741824")
 
 		// checking memory-swap
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/memory.swap.max"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("1073741824"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/memory.swap.max", "1073741824")
 
 		// checking cpu-shares
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/cpu.weight"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("5"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/cpu.weight", "5")
 
 		// checking pids-limit
-		session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/pids.max"})
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("123"))
+		podmanTest.CheckFileInContainerSubstring(ctrID, "/sys/fs/cgroup/pids.max", "123")
 	})
 
 	It("podman update keep original resources if not overridden", func() {
@@ -209,62 +157,44 @@ var _ = Describe("Podman update", func() {
 
 		ctrID := session.OutputToString()
 
+		path := "/sys/fs/cgroup/cpu/cpu.cfs_quota_us"
 		if v2, _ := cgroupv2.Enabled(); v2 {
-			session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/cpu.max"})
-		} else {
-			session = podmanTest.Podman([]string{"exec", ctrID, "cat", "/sys/fs/cgroup/cpu/cpu.cfs_quota_us"})
+			path = "/sys/fs/cgroup/cpu.max"
 		}
-		session.WaitWithDefaultTimeout()
-		Expect(session).Should(ExitCleanly())
-		Expect(session.OutputToString()).Should(ContainSubstring("500000"))
+
+		podmanTest.CheckFileInContainerSubstring(ctrID, path, "500000")
 	})
 
 	It("podman update persists changes", func() {
 		SkipIfCgroupV1("testing flags that only work in cgroup v2")
 		SkipIfRootless("many of these handlers are not enabled while rootless in CI")
 
+		memoryInspect := ".HostConfig.Memory"
+		memoryCgroup := "/sys/fs/cgroup/memory.max"
+		mem512m := "536870912"
+		mem256m := "268435456"
+
 		testCtr := "test-ctr-name"
 		ctr1 := podmanTest.Podman([]string{"run", "-d", "--name", testCtr, "-m", "512m", ALPINE, "top"})
 		ctr1.WaitWithDefaultTimeout()
 		Expect(ctr1).Should(ExitCleanly())
 
-		inspect1 := podmanTest.Podman([]string{"inspect", "--format", "{{ .HostConfig.Memory }}", testCtr})
-		inspect1.WaitWithDefaultTimeout()
-		Expect(inspect1).Should(ExitCleanly())
-		Expect(inspect1.OutputToString()).To(Equal("536870912"))
-
-		exec1 := podmanTest.Podman([]string{"exec", testCtr, "cat", "/sys/fs/cgroup/memory.max"})
-		exec1.WaitWithDefaultTimeout()
-		Expect(exec1).Should(ExitCleanly())
-		Expect(exec1.OutputToString()).Should(ContainSubstring("536870912"))
+		podmanTest.CheckContainerSingleField(testCtr, memoryInspect, mem512m)
+		podmanTest.CheckFileInContainer(testCtr, memoryCgroup, mem512m)
 
 		update := podmanTest.Podman([]string{"update", "-m", "256m", testCtr})
 		update.WaitWithDefaultTimeout()
 		Expect(update).Should(ExitCleanly())
 
-		inspect2 := podmanTest.Podman([]string{"inspect", "--format", "{{ .HostConfig.Memory }}", testCtr})
-		inspect2.WaitWithDefaultTimeout()
-		Expect(inspect2).Should(ExitCleanly())
-		Expect(inspect2.OutputToString()).To(Equal("268435456"))
-
-		exec2 := podmanTest.Podman([]string{"exec", testCtr, "cat", "/sys/fs/cgroup/memory.max"})
-		exec2.WaitWithDefaultTimeout()
-		Expect(exec2).Should(ExitCleanly())
-		Expect(exec2.OutputToString()).Should(ContainSubstring("268435456"))
+		podmanTest.CheckContainerSingleField(testCtr, memoryInspect, mem256m)
+		podmanTest.CheckFileInContainer(testCtr, memoryCgroup, mem256m)
 
 		restart := podmanTest.Podman([]string{"restart", testCtr})
 		restart.WaitWithDefaultTimeout()
 		Expect(restart).Should(ExitCleanly())
 
-		inspect3 := podmanTest.Podman([]string{"inspect", "--format", "{{ .HostConfig.Memory }}", testCtr})
-		inspect3.WaitWithDefaultTimeout()
-		Expect(inspect3).Should(ExitCleanly())
-		Expect(inspect3.OutputToString()).To(Equal("268435456"))
-
-		exec3 := podmanTest.Podman([]string{"exec", testCtr, "cat", "/sys/fs/cgroup/memory.max"})
-		exec3.WaitWithDefaultTimeout()
-		Expect(exec3).Should(ExitCleanly())
-		Expect(exec3.OutputToString()).Should(ContainSubstring("268435456"))
+		podmanTest.CheckContainerSingleField(testCtr, memoryInspect, mem256m)
+		podmanTest.CheckFileInContainer(testCtr, memoryCgroup, mem256m)
 
 		pause := podmanTest.Podman([]string{"pause", testCtr})
 		pause.WaitWithDefaultTimeout()
@@ -278,14 +208,34 @@ var _ = Describe("Podman update", func() {
 		unpause.WaitWithDefaultTimeout()
 		Expect(unpause).Should(ExitCleanly())
 
-		inspect4 := podmanTest.Podman([]string{"inspect", "--format", "{{ .HostConfig.Memory }}", testCtr})
-		inspect4.WaitWithDefaultTimeout()
-		Expect(inspect4).Should(ExitCleanly())
-		Expect(inspect4.OutputToString()).To(Equal("536870912"))
+		podmanTest.CheckContainerSingleField(testCtr, memoryInspect, mem512m)
+		podmanTest.CheckFileInContainer(testCtr, memoryCgroup, mem512m)
+	})
 
-		exec4 := podmanTest.Podman([]string{"exec", testCtr, "cat", "/sys/fs/cgroup/memory.max"})
-		exec4.WaitWithDefaultTimeout()
-		Expect(exec4).Should(ExitCleanly())
-		Expect(exec4.OutputToString()).Should(ContainSubstring("536870912"))
+	It("podman update sets restart policy", func() {
+		restartPolicyName := ".HostConfig.RestartPolicy.Name"
+		restartPolicyRetries := ".HostConfig.RestartPolicy.MaximumRetryCount"
+
+		testCtr := "test-ctr-name"
+		ctr1 := podmanTest.Podman([]string{"run", "-dt", "--name", testCtr, ALPINE, "top"})
+		ctr1.WaitWithDefaultTimeout()
+		Expect(ctr1).Should(ExitCleanly())
+
+		podmanTest.CheckContainerSingleField(testCtr, restartPolicyName, "no")
+		podmanTest.CheckContainerSingleField(testCtr, restartPolicyRetries, "0")
+
+		update1 := podmanTest.Podman([]string{"update", "--restart", "on-failure:5", testCtr})
+		update1.WaitWithDefaultTimeout()
+		Expect(update1).Should(ExitCleanly())
+
+		podmanTest.CheckContainerSingleField(testCtr, restartPolicyName, "on-failure")
+		podmanTest.CheckContainerSingleField(testCtr, restartPolicyRetries, "5")
+
+		update2 := podmanTest.Podman([]string{"update", "--restart", "always", testCtr})
+		update2.WaitWithDefaultTimeout()
+		Expect(update2).Should(ExitCleanly())
+
+		podmanTest.CheckContainerSingleField(testCtr, restartPolicyName, "always")
+		podmanTest.CheckContainerSingleField(testCtr, restartPolicyRetries, "0")
 	})
 })

test/system/280-update.bats

@@ -127,4 +127,26 @@ device-write-iops   = /dev/zero:4000 | - | -
     fi
 }
 
+@test "podman update - set restart policy" {
+    touch ${PODMAN_TMPDIR}/sentinel
+    run_podman run --security-opt label=disable --name testctr -v ${PODMAN_TMPDIR}:/testdir -d $IMAGE sh -c "touch /testdir/alive; while test -e /testdir/sentinel; do sleep 0.1; done;"
+
+    run_podman container inspect testctr --format "{{ .HostConfig.RestartPolicy.Name }}"
+    is "$output" "no"
+
+    run_podman update --restart always testctr
+
+    run_podman container inspect testctr --format "{{ .HostConfig.RestartPolicy.Name }}"
+    is "$output" "always"
+
+    # Ensure the container is alive
+    wait_for_file ${PODMAN_TMPDIR}/alive
+
+    rm -f ${PODMAN_TMPDIR}/alive
+    rm -f ${PODMAN_TMPDIR}/sentinel
+
+    # Restart should ensure that the container comes back up and recreates the file
+    wait_for_file ${PODMAN_TMPDIR}/alive
+}
+
 # vim: filetype=sh