Forcibly stop running containers before removal

Signed-off-by: Abhishek Kulkarni <abd.kulkarni@gmail.com>

pkg/server/container_remove.go

@@ -21,6 +21,7 @@ import (
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/log"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"golang.org/x/net/context"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
 
@@ -29,7 +30,6 @@ import (
 )
 
 // RemoveContainer removes the container.
-// TODO(random-liu): Forcibly stop container if it's running.
 func (c *criService) RemoveContainer(ctx context.Context, r *runtime.RemoveContainerRequest) (_ *runtime.RemoveContainerResponse, retErr error) {
 	container, err := c.containerStore.Get(r.GetContainerId())
 	if err != nil {
@@ -42,6 +42,17 @@ func (c *criService) RemoveContainer(ctx context.Context, r *runtime.RemoveConta
 	}
 	id := container.ID
 
+	// Forcibly stop the containers if they are in running or unknown state
+	state := container.Status.Get().State()
+	if state == runtime.ContainerState_CONTAINER_RUNNING ||
+		state == runtime.ContainerState_CONTAINER_UNKNOWN {
+		logrus.Infof("Forcibly stopping container %q", id)
+		if err := c.stopContainer(ctx, container, 0); err != nil {
+			return nil, errors.Wrapf(err, "failed to forcibly stop container %q", id)
+		}
+
+	}
+
 	// Set removing state to prevent other start/remove operations against this container
 	// while it's being removed.
 	if err := setContainerRemoving(container); err != nil {

pkg/server/sandbox_remove.go

@@ -21,6 +21,7 @@ import (
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/log"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"golang.org/x/net/context"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
 
@@ -48,7 +49,10 @@ func (c *criService) RemovePodSandbox(ctx context.Context, r *runtime.RemovePodS
 	// Return error if sandbox container is still running or unknown.
 	state := sandbox.Status.Get().State
 	if state == sandboxstore.StateReady || state == sandboxstore.StateUnknown {
-		return nil, errors.Errorf("sandbox container %q is not fully stopped", id)
+		logrus.Infof("Forcibly stopping sandbox %q", id)
+		if err := c.stopPodSandbox(ctx, sandbox); err != nil {
+			return nil, errors.Wrapf(err, "failed to forcibly stop sandbox %q", id)
+		}
 	}
 
 	// Return error if sandbox network namespace is not closed yet.

pkg/server/sandbox_stop.go

@@ -39,6 +39,15 @@ func (c *criService) StopPodSandbox(ctx context.Context, r *runtime.StopPodSandb
 		return nil, errors.Wrapf(err, "an error occurred when try to find sandbox %q",
 			r.GetPodSandboxId())
 	}
+
+	if err := c.stopPodSandbox(ctx, sandbox); err != nil {
+		return nil, err
+	}
+
+	return &runtime.StopPodSandboxResponse{}, nil
+}
+
+func (c *criService) stopPodSandbox(ctx context.Context, sandbox sandboxstore.Sandbox) error {
 	// Use the full sandbox id.
 	id := sandbox.ID
 
@@ -52,20 +61,20 @@ func (c *criService) StopPodSandbox(ctx context.Context, r *runtime.StopPodSandb
 		}
 		// Forcibly stop the container. Do not use `StopContainer`, because it introduces a race
 		// if a container is removed after list.
-		if err = c.stopContainer(ctx, container, 0); err != nil {
-			return nil, errors.Wrapf(err, "failed to stop container %q", container.ID)
+		if err := c.stopContainer(ctx, container, 0); err != nil {
+			return errors.Wrapf(err, "failed to stop container %q", container.ID)
 		}
 	}
 
 	if err := c.cleanupSandboxFiles(id, sandbox.Config); err != nil {
-		return nil, errors.Wrap(err, "failed to cleanup sandbox files")
+		return errors.Wrap(err, "failed to cleanup sandbox files")
 	}
 
 	// Only stop sandbox container when it's running or unknown.
 	state := sandbox.Status.Get().State
 	if state == sandboxstore.StateReady || state == sandboxstore.StateUnknown {
 		if err := c.stopSandboxContainer(ctx, sandbox); err != nil {
-			return nil, errors.Wrapf(err, "failed to stop sandbox container %q in %q state", id, state)
+			return errors.Wrapf(err, "failed to stop sandbox container %q in %q state", id, state)
 		}
 	}
 
@@ -74,21 +83,21 @@ func (c *criService) StopPodSandbox(ctx context.Context, r *runtime.StopPodSandb
 		// Use empty netns path if netns is not available. This is defined in:
 		// https://github.com/containernetworking/cni/blob/v0.7.0-alpha1/SPEC.md
 		if closed, err := sandbox.NetNS.Closed(); err != nil {
-			return nil, errors.Wrap(err, "failed to check network namespace closed")
+			return errors.Wrap(err, "failed to check network namespace closed")
 		} else if closed {
 			sandbox.NetNSPath = ""
 		}
 		if err := c.teardownPodNetwork(ctx, sandbox); err != nil {
-			return nil, errors.Wrapf(err, "failed to destroy network for sandbox %q", id)
+			return errors.Wrapf(err, "failed to destroy network for sandbox %q", id)
 		}
-		if err = sandbox.NetNS.Remove(); err != nil {
-			return nil, errors.Wrapf(err, "failed to remove network namespace for sandbox %q", id)
+		if err := sandbox.NetNS.Remove(); err != nil {
+			return errors.Wrapf(err, "failed to remove network namespace for sandbox %q", id)
 		}
 	}
 
 	log.G(ctx).Infof("TearDown network for sandbox %q successfully", id)
 
-	return &runtime.StopPodSandboxResponse{}, nil
+	return nil
 }
 
 // stopSandboxContainer kills the sandbox container.