Support unknown state for sandbox and container

Signed-off-by: Lantao Liu <lantaol@google.com>

pkg/server/container_remove.go

@@ -98,9 +98,12 @@ func (c *criService) RemoveContainer(ctx context.Context, r *runtime.RemoveConta
 // container will not be started or removed again.
 func setContainerRemoving(container containerstore.Container) error {
 	return container.Status.Update(func(status containerstore.Status) (containerstore.Status, error) {
-		// Do not remove container if it's still running.
+		// Do not remove container if it's still running or unknown.
 		if status.State() == runtime.ContainerState_CONTAINER_RUNNING {
-			return status, errors.New("container is still running")
+			return status, errors.New("container is still running, to stop first")
+		}
+		if status.State() == runtime.ContainerState_CONTAINER_UNKNOWN {
+			return status, errors.New("container state is unknown, to stop first")
 		}
 		if status.Removing {
 			return status, errors.New("container is already in removing state")

pkg/server/container_status.go

@@ -60,6 +60,15 @@ func (c *criService) ContainerStatus(ctx context.Context, r *runtime.ContainerSt
 		}
 	}
 	status := toCRIContainerStatus(container, spec, imageRef)
+	if status.GetCreatedAt() == 0 {
+		// CRI doesn't allow CreatedAt == 0.
+		info, err := container.Container.Info(ctx)
+		if err != nil {
+			return nil, errors.Wrapf(err, "failed to get CreatedAt in %q state", status.State)
+		}
+		status.CreatedAt = info.CreatedAt.UnixNano()
+	}
+
 	info, err := toCRIContainerInfo(ctx, container, r.GetVerbose())
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get verbose container info")

pkg/server/container_stop.go

@@ -19,6 +19,8 @@ package server
 import (
 	"time"
 
+	"github.com/containerd/containerd"
+	eventtypes "github.com/containerd/containerd/api/events"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/docker/docker/pkg/signal"
 	"github.com/pkg/errors"
@@ -60,18 +62,49 @@ func (c *criService) stopContainer(ctx context.Context, container containerstore
 	// Return without error if container is not running. This makes sure that
 	// stop only takes real action after the container is started.
 	state := container.Status.Get().State()
-	if state != runtime.ContainerState_CONTAINER_RUNNING {
-		logrus.Infof("Container to stop %q is not running, current state %q",
+	if state != runtime.ContainerState_CONTAINER_RUNNING &&
+		state != runtime.ContainerState_CONTAINER_UNKNOWN {
+		logrus.Infof("Container to stop %q must be in running or unknown state, current state %q",
 			id, criContainerStateToString(state))
 		return nil
 	}
 
 	task, err := container.Container.Task(ctx, nil)
 	if err != nil {
 		if !errdefs.IsNotFound(err) {
-			return errors.Wrapf(err, "failed to stop container, task not found for container %q", id)
+			return errors.Wrapf(err, "failed to get task for container %q", id)
+		}
+		// Don't return for unknown state, some cleanup needs to be done.
+		if state != runtime.ContainerState_CONTAINER_UNKNOWN {
+			return nil
+		}
+		// Task is an interface, explicitly set it to nil just in case.
+		task = nil
+	}
+
+	// Handle unknown state.
+	if state == runtime.ContainerState_CONTAINER_UNKNOWN {
+		status, err := getTaskStatus(ctx, task)
+		if err != nil {
+			return errors.Wrapf(err, "failed to get task status for %q", id)
+		}
+		switch status.Status {
+		case containerd.Running, containerd.Created:
+			// The task is still running, continue stopping the task.
+		case containerd.Stopped:
+			// The task has exited. If the task exited after containerd
+			// started, the event monitor will receive its exit event; if it
+			// exited before containerd started, the event monitor will never
+			// receive its exit event.
+			// However, we can't tell that because the task state was not
+			// successfully loaded during containerd start (container is
+			// in UNKNOWN state).
+			// So always do cleanup here, just in case that we've missed the
+			// exit event.
+			return cleanupUnknownContainer(ctx, id, status, container)
+		default:
+			return errors.Wrapf(err, "unsupported task status %q", status.Status)
 		}
-		return nil
 	}
 
 	// We only need to kill the task. The event handler will Delete the
@@ -141,3 +174,21 @@ func (c *criService) waitContainerStop(ctx context.Context, container containers
 		return nil
 	}
 }
+
+// cleanupUnknownContainer cleanup stopped container in unknown state.
+func cleanupUnknownContainer(ctx context.Context, id string, status containerd.Status,
+	cntr containerstore.Container) error {
+	// Reuse handleContainerExit to do the cleanup.
+	// NOTE(random-liu): If the task did exit after containerd started, both
+	// the event monitor and the cleanup function would update the container
+	// state. The final container state will be whatever being updated first.
+	// There is no way to completely avoid this race condition, and for best
+	// effort unknown state container cleanup, this seems acceptable.
+	return handleContainerExit(ctx, &eventtypes.TaskExit{
+		ContainerID: id,
+		ID:          id,
+		Pid:         0,
+		ExitStatus:  status.ExitStatus,
+		ExitedAt:    status.ExitTime,
+	}, cntr)
+}

pkg/server/events.go

@@ -260,7 +260,16 @@ func handleContainerExit(ctx context.Context, e *eventtypes.TaskExit, cntr conta
 	// Attach container IO so that `Delete` could cleanup the stream properly.
 	task, err := cntr.Container.Task(ctx,
 		func(*containerdio.FIFOSet) (containerdio.IO, error) {
-			return cntr.IO, nil
+			// We can't directly return cntr.IO here, because
+			// even if cntr.IO is nil, the cio.IO interface
+			// is not.
+			// See https://tour.golang.org/methods/12:
+			//   Note that an interface value that holds a nil
+			//   concrete value is itself non-nil.
+			if cntr.IO != nil {
+				return cntr.IO, nil
+			}
+			return nil, nil
 		},
 	)
 	if err != nil {

pkg/server/helpers.go

@@ -25,9 +25,12 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/BurntSushi/toml"
+	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/containers"
+	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/runtime/linux/runctypes"
 	runcoptions "github.com/containerd/containerd/runtime/v2/runc/options"
 	"github.com/containerd/typeurl"
@@ -44,7 +47,9 @@ import (
 	runtimeoptions "github.com/containerd/cri/pkg/api/runtimeoptions/v1"
 	criconfig "github.com/containerd/cri/pkg/config"
 	"github.com/containerd/cri/pkg/store"
+	containerstore "github.com/containerd/cri/pkg/store/container"
 	imagestore "github.com/containerd/cri/pkg/store/image"
+	sandboxstore "github.com/containerd/cri/pkg/store/sandbox"
 	"github.com/containerd/cri/pkg/util"
 )
 
@@ -524,3 +529,53 @@ func restrictOOMScoreAdj(preferredOOMScoreAdj int) (int, error) {
 	}
 	return preferredOOMScoreAdj, nil
 }
+
+const (
+	// unknownExitCode is the exit code when exit reason is unknown.
+	unknownExitCode = 255
+	// unknownExitReason is the exit reason when exit reason is unknown.
+	unknownExitReason = "Unknown"
+)
+
+// unknownContainerStatus returns the default container status when its status is unknown.
+func unknownContainerStatus() containerstore.Status {
+	return containerstore.Status{
+		CreatedAt:  0,
+		StartedAt:  0,
+		FinishedAt: 0,
+		ExitCode:   unknownExitCode,
+		Reason:     unknownExitReason,
+	}
+}
+
+// unknownSandboxStatus returns the default sandbox status when its status is unknown.
+func unknownSandboxStatus() sandboxstore.Status {
+	return sandboxstore.Status{
+		State: sandboxstore.StateUnknown,
+	}
+}
+
+// unknownExitStatus generates containerd.Status for container exited with unknown exit code.
+func unknownExitStatus() containerd.Status {
+	return containerd.Status{
+		Status:     containerd.Stopped,
+		ExitStatus: unknownExitCode,
+		ExitTime:   time.Now(),
+	}
+}
+
+// getTaskStatus returns status for a given task. It returns unknown exit status if
+// the task is nil or not found.
+func getTaskStatus(ctx context.Context, task containerd.Task) (containerd.Status, error) {
+	if task == nil {
+		return unknownExitStatus(), nil
+	}
+	status, err := task.Status(ctx)
+	if err != nil {
+		if !errdefs.IsNotFound(err) {
+			return containerd.Status{}, err
+		}
+		return unknownExitStatus(), nil
+	}
+	return status, nil
+}