diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go index 27b5b55e86..c3eaf657ec 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go +++ b/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 { diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go index c6bfbd10b7..6152a1974e 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go +++ b/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.