Remove container lifecycle image ref dependency.

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

pkg/server/container_create.go

@@ -187,6 +187,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
 		opts = append(opts, customopts.WithVolumes(mountMap))
 	}
 	meta.ImageRef = image.ID
+	meta.StopSignal = image.ImageSpec.Config.StopSignal
 
 	// Get container log path.
 	if config.GetLogPath() != "" {

pkg/server/container_status.go

@@ -24,6 +24,7 @@ import (
 	"golang.org/x/net/context"
 	runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
 
+	"github.com/containerd/cri/pkg/store"
 	containerstore "github.com/containerd/cri/pkg/store/container"
 )
 
@@ -43,17 +44,20 @@ func (c *criService) ContainerStatus(ctx context.Context, r *runtime.ContainerSt
 	imageRef := container.ImageRef
 	image, err := c.imageStore.Get(imageRef)
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed to get image %q", imageRef)
-	}
-	repoTags, repoDigests := parseImageReferences(image.References)
-	if len(repoTags) > 0 {
-		// Based on current behavior of dockershim, this field should be
-		// image tag.
-		spec = &runtime.ImageSpec{Image: repoTags[0]}
-	}
-	if len(repoDigests) > 0 {
-		// Based on the CRI definition, this field will be consumed by user.
-		imageRef = repoDigests[0]
+		if err != store.ErrNotExist {
+			return nil, errors.Wrapf(err, "failed to get image %q", imageRef)
+		}
+	} else {
+		repoTags, repoDigests := parseImageReferences(image.References)
+		if len(repoTags) > 0 {
+			// Based on current behavior of dockershim, this field should be
+			// image tag.
+			spec = &runtime.ImageSpec{Image: repoTags[0]}
+		}
+		if len(repoDigests) > 0 {
+			// Based on the CRI definition, this field will be consumed by user.
+			imageRef = repoDigests[0]
+		}
 	}
 	status := toCRIContainerStatus(container, spec, imageRef)
 	info, err := toCRIContainerInfo(ctx, container, r.GetVerbose())

pkg/server/container_stop.go

@@ -27,6 +27,7 @@ import (
 	"golang.org/x/sys/unix"
 	runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
 
+	"github.com/containerd/cri/pkg/store"
 	containerstore "github.com/containerd/cri/pkg/store/container"
 )
 
@@ -76,24 +77,36 @@ func (c *criService) stopContainer(ctx context.Context, container containerstore
 	// We only need to kill the task. The event handler will Delete the
 	// task from containerd after it handles the Exited event.
 	if timeout > 0 {
-		stopSignal := unix.SIGTERM
-		image, err := c.imageStore.Get(container.ImageRef)
-		if err != nil {
-			// NOTE(random-liu): It's possible that the container is stopped,
-			// deleted and image is garbage collected before this point. However,
-			// the chance is really slim, even it happens, it's still fine to return
-			// an error here.
-			return errors.Wrapf(err, "failed to get image metadata %q", container.ImageRef)
-		}
-		if image.ImageSpec.Config.StopSignal != "" {
-			stopSignal, err = signal.ParseSignal(image.ImageSpec.Config.StopSignal)
+		stopSignal := "SIGTERM"
+		if container.StopSignal != "" {
+			stopSignal = container.StopSignal
+		} else {
+			// The image may have been deleted, and the `StopSignal` field is
+			// just introduced to handle that.
+			// However, for containers created before the `StopSignal` field is
+			// introduced, still try to get the stop signal from the image config.
+			// If the image has been deleted, logging an error and using the
+			// default SIGTERM is still better than returning error and leaving
+			// the container unstoppable. (See issue #990)
+			// TODO(random-liu): Remove this logic when containerd 1.2 is deprecated.
+			image, err := c.imageStore.Get(container.ImageRef)
 			if err != nil {
-				return errors.Wrapf(err, "failed to parse stop signal %q",
-					image.ImageSpec.Config.StopSignal)
+				if err != store.ErrNotExist {
+					return errors.Wrapf(err, "failed to get image %q", container.ImageRef)
+				}
+				logrus.Warningf("Image %q not found, stop container with signal %q", container.ImageRef, stopSignal)
+			} else {
+				if image.ImageSpec.Config.StopSignal != "" {
+					stopSignal = image.ImageSpec.Config.StopSignal
+				}
 			}
 		}
-		logrus.Infof("Stop container %q with signal %v", id, stopSignal)
-		if err = task.Kill(ctx, stopSignal); err != nil && !errdefs.IsNotFound(err) {
+		sig, err := signal.ParseSignal(stopSignal)
+		if err != nil {
+			return errors.Wrapf(err, "failed to parse stop signal %q", stopSignal)
+		}
+		logrus.Infof("Stop container %q with signal %v", id, sig)
+		if err = task.Kill(ctx, sig); err != nil && !errdefs.IsNotFound(err) {
 			return errors.Wrapf(err, "failed to stop container %q", id)
 		}
 

pkg/store/container/container_test.go

@@ -39,8 +39,9 @@ func TestContainerStore(t *testing.T) {
 					Attempt: 1,
 				},
 			},
-			ImageRef: "TestImage-1",
-			LogPath:  "/test/log/path/1",
+			ImageRef:   "TestImage-1",
+			StopSignal: "SIGTERM",
+			LogPath:    "/test/log/path/1",
 		},
 		"2abcd": {
 			ID:        "2abcd",
@@ -52,8 +53,9 @@ func TestContainerStore(t *testing.T) {
 					Attempt: 2,
 				},
 			},
-			ImageRef: "TestImage-2",
-			LogPath:  "/test/log/path/2",
+			StopSignal: "SIGTERM",
+			ImageRef:   "TestImage-2",
+			LogPath:    "/test/log/path/2",
 		},
 		"4a333": {
 			ID:        "4a333",
@@ -65,8 +67,9 @@ func TestContainerStore(t *testing.T) {
 					Attempt: 3,
 				},
 			},
-			ImageRef: "TestImage-3",
-			LogPath:  "/test/log/path/3",
+			StopSignal: "SIGTERM",
+			ImageRef:   "TestImage-3",
+			LogPath:    "/test/log/path/3",
 		},
 		"4abcd": {
 			ID:        "4abcd",
@@ -78,7 +81,8 @@ func TestContainerStore(t *testing.T) {
 					Attempt: 1,
 				},
 			},
-			ImageRef: "TestImage-4abcd",
+			StopSignal: "SIGTERM",
+			ImageRef:   "TestImage-4abcd",
 		},
 	}
 	statuses := map[string]Status{
@@ -182,8 +186,9 @@ func TestWithContainerIO(t *testing.T) {
 				Attempt: 1,
 			},
 		},
-		ImageRef: "TestImage-1",
-		LogPath:  "/test/log/path",
+		ImageRef:   "TestImage-1",
+		StopSignal: "SIGTERM",
+		LogPath:    "/test/log/path",
 	}
 	status := Status{
 		Pid:        1,

pkg/store/container/metadata.go

@@ -27,7 +27,7 @@ import (
 // 1) Metadata is immutable after created.
 // 2) Metadata is checkpointed as containerd container label.
 
-// metadataVersion  is current version of container metadata.
+// metadataVersion is current version of container metadata.
 const metadataVersion = "v1" // nolint
 
 // versionedMetadata is the internal versioned container metadata.
@@ -58,6 +58,9 @@ type Metadata struct {
 	ImageRef string
 	// LogPath is the container log path.
 	LogPath string
+	// StopSignal is the system call signal that will be sent to the container to exit.
+	// TODO(random-liu): Add integration test for stop signal.
+	StopSignal string
 }
 
 // MarshalJSON encodes Metadata into bytes in json format.