Shut down container runtime when pod stops

When a pod using the VM runtime type stops, the actual runtime process
should also be stopped.

Previously, the runtime process was killed when the pod was deleted. This
works well for many workloads, but causes process leaks when large
numbers of one-shot pods are created (e.g. pods that enter the
"Completed" state in Kubernetes). Those pods will eventually be cleaned
up, but until then a large number of runtime processes will hang around.
The situation is made worse when the runtime process enters a bad state
after its container dies (see
kata-containers/runtime#2719).

Initially, this problem was addressed in
cri-o#3998
However, that PR worked by actually deleting VM runtime containers on
stop, which led to issues with pods being stuck in a NotReady state
indefinitely.

This PR re-addresses the issue solved by 3998 by sending a shutdown task
to the runtime on pod stop.

Signed-off-by: Evan Foster <efoster@adobe.com>

internal/oci/oci.go

@@ -75,6 +75,7 @@ type RuntimeImpl interface {
  PortForwardContainer(*Container, int32, io.ReadWriter) error
  ReopenContainerLog(*Container) error
  WaitContainerStateStopped(context.Context, *Container) error
+ ShutdownContainerRuntime(*Container) error
 }
 
 // New creates a new Runtime with options provided
@@ -394,6 +395,16 @@ func (r *Runtime) PortForwardContainer(c *Container, port int32, stream io.ReadW
  return impl.PortForwardContainer(c, port, stream)
 }
 
+// ShutdownContainerRuntime kills the runtime process when using the VM runtime type.
+func (r *Runtime) ShutdownContainerRuntime(c *Container) error {
+ impl, err := r.RuntimeImpl(c)
+ if err != nil {
+ return err
+ }
+
+ return impl.ShutdownContainerRuntime(c)
+}
+
 // ReopenContainerLog reopens the log file of a container.
 func (r *Runtime) ReopenContainerLog(c *Container) error {
  impl, err := r.RuntimeImpl(c)

internal/oci/runtime_oci.go

@@ -994,6 +994,11 @@ func (r *runtimeOCI) ReopenContainerLog(c *Container) error {
  return nil
 }
 
+// ShutdownContainerRuntime is a no-op for OCI containers.
+func (r *runtimeOCI) ShutdownContainerRuntime(c *Container) error {
+ return nil
+}
+
 // prepareProcessExec returns the path of the process.json used in runc exec -p
 // caller is responsible to close the returned *os.File if needed.
 func prepareProcessExec(c *Container, cmd []string, tty bool) (*os.File, error) {

internal/oci/runtime_vm.go

@@ -276,12 +276,6 @@ func (r *runtimeVM) StartContainer(c *Container) error {
  if err1 := r.updateContainerStatus(c); err1 != nil {
  logrus.Warningf("error updating container status %v", err1)
  }
-
- if c.state.Status == ContainerStateStopped {
- if err1 := r.deleteContainer(c, true); err1 != nil {
- logrus.WithError(err1).Infof("deleteContainer failed for container %s", c.ID())
- }
- }
  }
  }()
 
@@ -590,6 +584,14 @@ func (r *runtimeVM) deleteContainer(c *Container, force bool) error {
  return nil
 }
 
+func (r *runtimeVM) ShutdownContainerRuntime(c *Container) error {
+ _, err := r.task.Shutdown(r.ctx, &task.ShutdownRequest{ID: c.ID()})
+ if err != nil && err != ttrpc.ErrClosed {
+ return err
+ }
+ return nil
+}
+
 // UpdateContainerStatus refreshes the status of the container.
 func (r *runtimeVM) UpdateContainerStatus(c *Container) error {
  logrus.Debug("runtimeVM.UpdateContainerStatus() start")
@@ -622,6 +624,7 @@ func (r *runtimeVM) updateContainerStatus(c *Container) error {
  if errors.Cause(err) != ttrpc.ErrClosed {
  return errdefs.FromGRPC(err)
  }
+ logrus.Debugf("Updating container status caused %v", err)
  return errdefs.ErrNotFound
  }
 

server/sandbox_stop_linux.go

@@ -5,6 +5,8 @@ package server
 import (
  "fmt"
 
+ "github.com/containerd/ttrpc"
+
  "github.com/containers/storage"
  "github.com/cri-o/cri-o/internal/lib/sandbox"
  "github.com/cri-o/cri-o/internal/oci"
@@ -78,6 +80,10 @@ func (s *Server) stopPodSandbox(ctx context.Context, req *pb.StopPodSandboxReque
  // assume container already umounted
  log.Warnf(ctx, "failed to stop container %s in pod sandbox %s: %v", c.Name(), sb.ID(), err)
  }
+ log.Debugf(ctx, "shutting down runtime for container %s in pod sandbox %s", c.Name(), sb.ID())
+ if err := s.Runtime().ShutdownContainerRuntime(c); err != nil && err != ttrpc.ErrClosed {
+ log.Warnf(ctx, "failed to shut down container %s runtime in pod sandbox %s: %v", c.Name(), sb.ID(), err)
+ }
  if err := s.ContainerStateToDisk(c); err != nil {
  return errors.Wrapf(err, "write container %q state do disk", c.Name())
  }
@@ -89,6 +95,10 @@ func (s *Server) stopPodSandbox(ctx context.Context, req *pb.StopPodSandboxReque
  return nil, err
  }
  }
+ log.Debugf(ctx, "shutting down runtime for container %s in pod sandbox %s", containers[0].Name(), sb.ID())
+ if err := s.Runtime().ShutdownContainerRuntime(containers[0]); err != nil && err != ttrpc.ErrClosed {
+ log.Warnf(ctx, "failed to shut down container %s runtime in pod sandbox %s: %v", containers[0].Name(), sb.ID(), err)
+ }
 
  podInfraStatus := podInfraContainer.State()
  if podInfraStatus.Status != oci.ContainerStateStopped {