diff --git a/internal/oci/container.go b/internal/oci/container.go index 78a9ba66b6da..dd60ed3046c5 100644 --- a/internal/oci/container.go +++ b/internal/oci/container.go @@ -586,20 +586,6 @@ func (c *Container) verifyPid() (string, error) { return state, nil } -// ShouldBeStopped checks whether the container state is in a place -// where attempting to stop it makes sense -// a container is not stoppable if it's paused or stopped -// if it's paused, that's an error, and is reported as such -func (c *Container) ShouldBeStopped() error { - switch c.State().Status { - case ContainerStateStopped: // no-op - return ErrContainerStopped - case ContainerStatePaused: - return errors.New("cannot stop paused container") - } - return nil -} - // Spoofed returns whether this container is spoofed. // A container should be spoofed when it doesn't have to exist in the container runtime, // but does need to exist in the storage. The main use of this is when an infra container diff --git a/internal/oci/container_test.go b/internal/oci/container_test.go index ecf1d25982bc..9cd5ecda4975 100644 --- a/internal/oci/container_test.go +++ b/internal/oci/container_test.go @@ -427,42 +427,6 @@ var _ = t.Describe("Container", func() { Expect(err).To(HaveOccurred()) }) }) - t.Describe("ShouldBeStopped", func() { - It("should fail to stop if already stopped", func() { - // Given - state := &oci.ContainerState{} - state.Status = oci.ContainerStateStopped - sut.SetState(state) - // When - err := sut.ShouldBeStopped() - - // Then - Expect(err).To(Equal(oci.ErrContainerStopped)) - }) - It("should fail to stop if paused", func() { - // Given - state := &oci.ContainerState{} - state.Status = oci.ContainerStatePaused - sut.SetState(state) - // When - err := sut.ShouldBeStopped() - - // Then - Expect(err).NotTo(Equal(oci.ErrContainerStopped)) - Expect(err).To(HaveOccurred()) - }) - It("should succeed to stop if started", func() { - // Given - state := &oci.ContainerState{} - state.Status = oci.ContainerStateRunning - sut.SetState(state) - // When - err := sut.ShouldBeStopped() - - // Then - Expect(err).ToNot(HaveOccurred()) - }) - }) t.Describe("Living", func() { It("should be false if pid uninitialized", func() { // Given diff --git a/internal/oci/runtime_oci.go b/internal/oci/runtime_oci.go index 3db835e845ac..b67908873fca 100644 --- a/internal/oci/runtime_oci.go +++ b/internal/oci/runtime_oci.go @@ -830,13 +830,6 @@ func (r *runtimeOCI) StopContainer(ctx context.Context, c *Container, timeout in return nil } - if err := c.ShouldBeStopped(); err != nil { - if errors.Is(err, ErrContainerStopped) { - err = nil - } - return err - } - // The initial container process either doesn't exist, or isn't ours. if err := c.Living(); err != nil { c.state.Finished = time.Now() diff --git a/internal/oci/runtime_oci_test.go b/internal/oci/runtime_oci_test.go index ea25bfeebe73..31852ed6ff53 100644 --- a/internal/oci/runtime_oci_test.go +++ b/internal/oci/runtime_oci_test.go @@ -73,20 +73,6 @@ var _ = t.Describe("Oci", func() { cmdrunner.ResetPrependedCmd() }) - It("should fail to stop if container paused", func() { - state := &oci.ContainerState{} - state.Status = oci.ContainerStatePaused - sut.SetState(state) - - Expect(sut.ShouldBeStopped()).NotTo(Succeed()) - }) - It("should fail to stop if container stopped", func() { - state := &oci.ContainerState{} - state.Status = oci.ContainerStateStopped - sut.SetState(state) - - Expect(sut.ShouldBeStopped()).To(Equal(oci.ErrContainerStopped)) - }) It("should return early if runtime command fails and process stopped", func() { // Given gomock.InOrder( diff --git a/internal/oci/runtime_vm.go b/internal/oci/runtime_vm.go index 68765b4400f5..94f6979a88e5 100644 --- a/internal/oci/runtime_vm.go +++ b/internal/oci/runtime_vm.go @@ -608,7 +608,7 @@ func (r *runtimeVM) StopContainer(ctx context.Context, c *Container, timeout int log.Debugf(ctx, "RuntimeVM.StopContainer() start") defer log.Debugf(ctx, "RuntimeVM.StopContainer() end") - if err := c.ShouldBeStopped(); err != nil { + if err := shouldBeStopped(c); err != nil { if errors.Is(err, ErrContainerStopped) { err = nil } @@ -669,6 +669,21 @@ func (r *runtimeVM) StopContainer(ctx context.Context, c *Container, timeout int return nil } +// shouldBeStopped checks whether the container's state permits +// stopping. It determines if stopping the container makes sense +// based on its current state. A container cannot be stopped if +// it is already stopped or paused. If the container is paused, +// the function attempts to unpause it and update its status. +func shouldBeStopped(c *Container) error { + switch c.State().Status { + case ContainerStateStopped: // no-op + return ErrContainerStopped + case ContainerStatePaused: + return errors.New("cannot stop paused container") + } + return nil +} + func (r *runtimeVM) waitCtrTerminate(sig syscall.Signal, stopCh chan error, timeout time.Duration) error { select { case err := <-stopCh: