Checked container spec when killing container.

pkg/kubelet/kuberuntime/kuberuntime_container.go

@@ -155,7 +155,10 @@ func (m *kubeGenericRuntimeManager) startContainer(podSandboxID string, podSandb
 		msg, handlerErr := m.runner.Run(kubeContainerID, pod, container, container.Lifecycle.PostStart)
 		if handlerErr != nil {
 			m.recordContainerEvent(pod, container, kubeContainerID.ID, v1.EventTypeWarning, events.FailedPostStartHook, msg)
-			m.killContainer(pod, kubeContainerID, container.Name, "FailedPostStartHook", nil)
+			if err := m.killContainer(pod, kubeContainerID, container.Name, "FailedPostStartHook", nil); err != nil {
+				glog.Errorf("Failed to kill container %q(id=%q) in pod %q: %v, %v",
+					container.Name, kubeContainerID.String(), format.Pod(pod), ErrPostStartHook, err)
+			}
 			return msg, ErrPostStartHook
 		}
 	}
@@ -547,7 +550,10 @@ func (m *kubeGenericRuntimeManager) restoreSpecsFromContainerLabels(containerID
 func (m *kubeGenericRuntimeManager) killContainer(pod *v1.Pod, containerID kubecontainer.ContainerID, containerName string, reason string, gracePeriodOverride *int64) error {
 	var containerSpec *v1.Container
 	if pod != nil {
-		containerSpec = kubecontainer.GetContainerSpec(pod, containerName)
+		if containerSpec = kubecontainer.GetContainerSpec(pod, containerName); containerSpec == nil {
+			return fmt.Errorf("failed to get containerSpec %q(id=%q) in pod %q when killing container for reason %q",
+				containerName, containerID.String(), format.Pod(pod), reason)
+		}
 	} else {
 		// Restore necessary information if one of the specs is nil.
 		restoredPod, restoredContainer, err := m.restoreSpecsFromContainerLabels(containerID)
@@ -556,6 +562,7 @@ func (m *kubeGenericRuntimeManager) killContainer(pod *v1.Pod, containerID kubec
 		}
 		pod, containerSpec = restoredPod, restoredContainer
 	}
+
 	// From this point , pod and container must be non-nil.
 	gracePeriod := int64(minimumGracePeriodInSeconds)
 	switch {

pkg/kubelet/kuberuntime/kuberuntime_container_test.go

@@ -69,6 +69,41 @@ func TestRemoveContainer(t *testing.T) {
 	assert.Empty(t, containers)
 }
 
+// TestKillContainer tests killing the container in a Pod.
+func TestKillContainer(t *testing.T) {
+	_, _, m, _ := createTestRuntimeManager()
+
+	tests := []struct {
+		caseName            string
+		pod                 *v1.Pod
+		containerID         kubecontainer.ContainerID
+		containerName       string
+		reason              string
+		gracePeriodOverride int64
+		succeed             bool
+	}{
+		{
+			caseName: "Failed to find container in pods, expect to return error",
+			pod: &v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{UID: "pod1_id", Name: "pod1", Namespace: "default"},
+				Spec:       v1.PodSpec{Containers: []v1.Container{{Name: "empty_container"}}},
+			},
+			containerID:         kubecontainer.ContainerID{Type: "docker", ID: "not_exist_container_id"},
+			containerName:       "not_exist_container",
+			reason:              "unknown reason",
+			gracePeriodOverride: 0,
+			succeed:             false,
+		},
+	}
+
+	for _, test := range tests {
+		err := m.killContainer(test.pod, test.containerID, test.containerName, test.reason, &test.gracePeriodOverride)
+		if test.succeed != (err == nil) {
+			t.Errorf("%s: expected %v, got %v (%v)", test.caseName, test.succeed, (err == nil), err)
+		}
+	}
+}
+
 // TestToKubeContainerStatus tests the converting the CRI container status to
 // the internal type (i.e., toKubeContainerStatus()) for containers in
 // different states.