diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index b88714529b..09a9cb9716 100644 --- a/cmd/podman/common/create.go +++ b/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(), diff --git a/cmd/podman/containers/update.go b/cmd/podman/containers/update.go index bcaa735a9c..9e8e28070a 100644 --- a/cmd/podman/containers/update.go +++ b/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 { diff --git a/docs/source/markdown/options/restart.md b/docs/source/markdown/options/restart.md index 6c84cd5ecd..74e4e7814f 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, 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* diff --git a/docs/source/markdown/podman-update.1.md.in b/docs/source/markdown/podman-update.1.md.in index 81357c98b7..9cce804aa9 100644 --- a/docs/source/markdown/podman-update.1.md.in +++ b/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 diff --git a/libpod/container_api.go b/libpod/container_api.go index 93bc623995..1b49266469 100644 --- a/libpod/container_api.go +++ b/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. diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index 1eb2fbb073..ef4bac14e4 100644 --- a/libpod/container_inspect.go +++ b/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 { diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 3e9446944c..862c72233d 100644 --- a/libpod/container_internal.go +++ b/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() diff --git a/libpod/define/container.go b/libpod/define/container.go index da2441fb9b..ce7605992a 100644 --- a/libpod/define/container.go +++ b/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 diff --git a/libpod/options.go b/libpod/options.go index b0b76a3870..3e50bdd863 100644 --- a/libpod/options.go +++ b/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 } } diff --git a/pkg/api/handlers/compat/containers.go b/pkg/api/handlers/compat/containers.go index f30600f26b..08061c1e48 100644 --- a/pkg/api/handlers/compat/containers.go +++ b/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 } diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go index e823cf0608..6d0f839f04 100644 --- a/pkg/api/handlers/libpod/containers.go +++ b/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 diff --git a/pkg/api/handlers/swagger/models.go b/pkg/api/handlers/swagger/models.go index a7b561df97..a58a6bd95a 100644 --- a/pkg/api/handlers/swagger/models.go +++ b/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 diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go index dbfc395394..b3a603af95 100644 --- a/pkg/api/server/register_containers.go +++ b/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: diff --git a/pkg/bindings/containers/update.go b/pkg/bindings/containers/update.go index 20b743c954..37cf74426e 100644 --- a/pkg/bindings/containers/update.go +++ b/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 } diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 9016398bb5..aa0d47236a 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/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 diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index db4f8cdb60..0887eb4c8c 100644 --- a/pkg/specgen/generate/container_create.go +++ b/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 { diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index d0bbdb0d86..86e18dccb7 100644 --- a/test/e2e/common_test.go +++ b/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}) diff --git a/test/e2e/update_test.go b/test/e2e/update_test.go index 3cd64e6c75..83ee481c3f 100644 --- a/test/e2e/update_test.go +++ b/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") }) }) diff --git a/test/system/280-update.bats b/test/system/280-update.bats index e35e541a3f..db335d0af1 100644 --- a/test/system/280-update.bats +++ b/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