From f6637a326e88118a5f14ce1b890a121845ade4d1 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Tue, 6 Jun 2017 07:45:56 -0700 Subject: [PATCH] server: Don't stop a sandbox or a container in "created" state There is no point in trying to stop a container that is not currently running. More than that, the OCI specification is very clear that trying to issue a "kill" command on a container which is not running must be considered as an error. Refer to OCI specification here: https://github.com/opencontainers/runtime-spec/blob/master/runtime.md#kill In case of a pod sandbox, OCI is not forcing anything, but I think it does make sense not to try to stop a pod which is not running. It could lead to unexpected behaviors, and thinking about semantics, we should not be able to stop something which has not been started first. Signed-off-by: Sebastien Boeuf --- server/container_remove.go | 2 +- server/container_stop.go | 2 +- server/sandbox_remove.go | 2 +- server/sandbox_stop.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/container_remove.go b/server/container_remove.go index 125bf246..a153bc60 100644 --- a/server/container_remove.go +++ b/server/container_remove.go @@ -23,7 +23,7 @@ func (s *Server) RemoveContainer(ctx context.Context, req *pb.RemoveContainerReq } cState := s.runtime.ContainerStatus(c) - if cState.Status == oci.ContainerStateCreated || cState.Status == oci.ContainerStateRunning { + if cState.Status == oci.ContainerStateRunning { if err := s.runtime.StopContainer(c, -1); err != nil { return nil, fmt.Errorf("failed to stop container %s: %v", c.ID(), err) } diff --git a/server/container_stop.go b/server/container_stop.go index 5ea53bf7..7228f3ad 100644 --- a/server/container_stop.go +++ b/server/container_stop.go @@ -21,7 +21,7 @@ func (s *Server) StopContainer(ctx context.Context, req *pb.StopContainerRequest return nil, err } cStatus := s.runtime.ContainerStatus(c) - if cStatus.Status != oci.ContainerStateStopped { + if cStatus.Status == oci.ContainerStateRunning { if err := s.runtime.StopContainer(c, req.Timeout); err != nil { return nil, fmt.Errorf("failed to stop container %s: %v", c.ID(), err) } diff --git a/server/sandbox_remove.go b/server/sandbox_remove.go index 763ffb03..34fdc63e 100644 --- a/server/sandbox_remove.go +++ b/server/sandbox_remove.go @@ -44,7 +44,7 @@ func (s *Server) RemovePodSandbox(ctx context.Context, req *pb.RemovePodSandboxR } cState := s.runtime.ContainerStatus(c) - if cState.Status == oci.ContainerStateCreated || cState.Status == oci.ContainerStateRunning { + if cState.Status == oci.ContainerStateRunning { if err := s.runtime.StopContainer(c, -1); err != nil { // Assume container is already stopped logrus.Warnf("failed to stop container %s: %v", c.Name(), err) diff --git a/server/sandbox_stop.go b/server/sandbox_stop.go index fb998b1d..86847786 100644 --- a/server/sandbox_stop.go +++ b/server/sandbox_stop.go @@ -57,7 +57,7 @@ func (s *Server) StopPodSandbox(ctx context.Context, req *pb.StopPodSandboxReque return nil, err } cStatus := s.runtime.ContainerStatus(c) - if cStatus.Status != oci.ContainerStateStopped { + if cStatus.Status == oci.ContainerStateRunning { if err := s.runtime.StopContainer(c, -1); err != nil { return nil, fmt.Errorf("failed to stop container %s in pod sandbox %s: %v", c.Name(), sb.id, err) }