Merge pull request #7601 from yujuhong/kill_container

pkg/kubelet/dockertools/manager.go

@@ -1023,7 +1023,7 @@ func (dm *DockerManager) KillPod(pod kubecontainer.Pod) error {
 					errs <- err
 				}
 			}
-			err := dm.KillContainer(container.ID)
+			err := dm.killContainer(container.ID)
 			if err != nil {
 				glog.Errorf("Failed to delete container: %v; Skipping pod %q", err, pod.ID)
 				errs <- err
@@ -1043,10 +1043,29 @@ func (dm *DockerManager) KillPod(pod kubecontainer.Pod) error {
 	return nil
 }
 
+// KillContainerInPod kills a container in the pod.
+func (dm *DockerManager) KillContainerInPod(container api.Container, pod *api.Pod) error {
+	// Locate the container.
+	pods, err := dm.GetPods(false)
+	if err != nil {
+		return err
+	}
+	targetPod := kubecontainer.Pods(pods).FindPod(kubecontainer.GetPodFullName(pod), pod.UID)
+	targetContainer := targetPod.FindContainerByName(container.Name)
+	if targetContainer == nil {
+		return fmt.Errorf("unable to find container %q in pod %q", container.Name, targetPod.Name)
+	}
+	return dm.killContainer(targetContainer.ID)
+}
+
 // KillContainer kills a container identified by containerID.
 // Internally, it invokes docker's StopContainer API with a timeout of 10s.
-// TODO(yifan): Use new ContainerID type.
+// TODO: Deprecate this function in favor of KillContainerInPod.
 func (dm *DockerManager) KillContainer(containerID types.UID) error {
+	return dm.killContainer(containerID)
+}
+
+func (dm *DockerManager) killContainer(containerID types.UID) error {
 	ID := string(containerID)
 	glog.V(2).Infof("Killing container with id %q", ID)
 	dm.readinessManager.RemoveReadiness(ID)
@@ -1087,7 +1106,7 @@ func (dm *DockerManager) RunContainer(pod *api.Pod, container *api.Container, ge
 	if container.Lifecycle != nil && container.Lifecycle.PostStart != nil {
 		handlerErr := runner.Run(id, pod, container, container.Lifecycle.PostStart)
 		if handlerErr != nil {
-			dm.KillContainer(types.UID(id))
+			dm.killContainer(types.UID(id))
 			return kubeletTypes.DockerID(""), fmt.Errorf("failed to call event handler: %v", handlerErr)
 		}
 	}

pkg/kubelet/dockertools/manager_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package dockertools
 
 import (
+	"fmt"
 	"reflect"
 	"sort"
 	"testing"
@@ -210,3 +211,127 @@ func TestListImages(t *testing.T) {
 		t.Errorf("expected %#v, got %#v", expected.List(), actual.List())
 	}
 }
+
+func apiContainerToContainer(c docker.APIContainers) kubecontainer.Container {
+	dockerName, hash, err := ParseDockerName(c.Names[0])
+	if err != nil {
+		return kubecontainer.Container{}
+	}
+	return kubecontainer.Container{
+		ID:   types.UID(c.ID),
+		Name: dockerName.ContainerName,
+		Hash: hash,
+	}
+}
+
+func dockerContainersToPod(containers DockerContainers) kubecontainer.Pod {
+	var pod kubecontainer.Pod
+	for _, c := range containers {
+		dockerName, hash, err := ParseDockerName(c.Names[0])
+		if err != nil {
+			continue
+		}
+		pod.Containers = append(pod.Containers, &kubecontainer.Container{
+			ID:    types.UID(c.ID),
+			Name:  dockerName.ContainerName,
+			Hash:  hash,
+			Image: c.Image,
+		})
+		// TODO(yifan): Only one evaluation is enough.
+		pod.ID = dockerName.PodUID
+		name, namespace, _ := kubecontainer.ParsePodFullName(dockerName.PodFullName)
+		pod.Name = name
+		pod.Namespace = namespace
+	}
+	return pod
+}
+
+func TestKillContainerInPod(t *testing.T) {
+	manager, fakeDocker := NewFakeDockerManager()
+
+	pod := &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			UID:       "12345678",
+			Name:      "qux",
+			Namespace: "new",
+		},
+		Spec: api.PodSpec{Containers: []api.Container{{Name: "foo"}, {Name: "bar"}}},
+	}
+	containers := []docker.APIContainers{
+		{
+			ID:    "1111",
+			Names: []string{"/k8s_foo_qux_new_1234_42"},
+		},
+		{
+			ID:    "2222",
+			Names: []string{"/k8s_bar_qux_new_1234_42"},
+		},
+	}
+	containerToKill := &containers[0]
+	containerToSpare := &containers[1]
+	fakeDocker.ContainerList = containers
+	// Set all containers to ready.
+	for _, c := range fakeDocker.ContainerList {
+		manager.readinessManager.SetReadiness(c.ID, true)
+	}
+
+	if err := manager.KillContainerInPod(pod.Spec.Containers[0], pod); err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	// Assert the container has been stopped.
+	if err := fakeDocker.AssertStopped([]string{containerToKill.ID}); err != nil {
+		t.Errorf("container was not stopped correctly: %v", err)
+	}
+
+	// Verify that the readiness has been removed for the stopped container.
+	if ready := manager.readinessManager.GetReadiness(containerToKill.ID); ready {
+		t.Errorf("exepcted container entry ID '%v' to not be found. states: %+v", containerToKill.ID, ready)
+	}
+	if ready := manager.readinessManager.GetReadiness(containerToSpare.ID); !ready {
+		t.Errorf("exepcted container entry ID '%v' to be found. states: %+v", containerToSpare.ID, ready)
+	}
+}
+
+func TestKillContainerInPodWithError(t *testing.T) {
+	manager, fakeDocker := NewFakeDockerManager()
+
+	pod := &api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			UID:       "12345678",
+			Name:      "qux",
+			Namespace: "new",
+		},
+		Spec: api.PodSpec{Containers: []api.Container{{Name: "foo"}, {Name: "bar"}}},
+	}
+	containers := []docker.APIContainers{
+		{
+			ID:    "1111",
+			Names: []string{"/k8s_foo_qux_new_1234_42"},
+		},
+		{
+			ID:    "2222",
+			Names: []string{"/k8s_bar_qux_new_1234_42"},
+		},
+	}
+	containerToKill := &containers[0]
+	containerToSpare := &containers[1]
+	fakeDocker.ContainerList = containers
+	fakeDocker.Errors["stop"] = fmt.Errorf("sample error")
+
+	// Set all containers to ready.
+	for _, c := range fakeDocker.ContainerList {
+		manager.readinessManager.SetReadiness(c.ID, true)
+	}
+
+	if err := manager.KillContainerInPod(pod.Spec.Containers[0], pod); err == nil {
+		t.Errorf("expected error, found nil")
+	}
+
+	// Verify that the readiness has been removed even though the stop failed.
+	if ready := manager.readinessManager.GetReadiness(containerToKill.ID); ready {
+		t.Errorf("exepcted container entry ID '%v' to not be found. states: %+v", containerToKill.ID, ready)
+	}
+	if ready := manager.readinessManager.GetReadiness(containerToSpare.ID); !ready {
+		t.Errorf("exepcted container entry ID '%v' to be found. states: %+v", containerToSpare.ID, ready)
+	}
+}

pkg/kubelet/kubelet_test.go

@@ -355,107 +355,6 @@ func apiContainerToContainer(c docker.APIContainers) container.Container {
 	}
 }
 
-func dockerContainersToPod(containers dockertools.DockerContainers) container.Pod {
-	var pod container.Pod
-	for _, c := range containers {
-		dockerName, hash, err := dockertools.ParseDockerName(c.Names[0])
-		if err != nil {
-			continue
-		}
-		pod.Containers = append(pod.Containers, &container.Container{
-			ID:    types.UID(c.ID),
-			Name:  dockerName.ContainerName,
-			Hash:  hash,
-			Image: c.Image,
-		})
-		// TODO(yifan): Only one evaluation is enough.
-		pod.ID = dockerName.PodUID
-		name, namespace, _ := kubecontainer.ParsePodFullName(dockerName.PodFullName)
-		pod.Name = name
-		pod.Namespace = namespace
-	}
-	return pod
-}
-
-func TestKillContainerWithError(t *testing.T) {
-	containers := []docker.APIContainers{
-		{
-			ID:    "1234",
-			Names: []string{"/k8s_foo_qux_new_1234_42"},
-		},
-		{
-			ID:    "5678",
-			Names: []string{"/k8s_bar_qux_new_5678_42"},
-		},
-	}
-	testKubelet := newTestKubelet(t)
-	kubelet := testKubelet.kubelet
-	fakeDocker := testKubelet.fakeDocker
-	fakeDocker.ContainerList = containers
-
-	for _, c := range fakeDocker.ContainerList {
-		kubelet.readinessManager.SetReadiness(c.ID, true)
-	}
-	kubelet.dockerClient = fakeDocker
-	c := apiContainerToContainer(fakeDocker.ContainerList[0])
-	fakeDocker.Errors["stop"] = fmt.Errorf("sample error")
-	err := kubelet.containerManager.KillContainer(c.ID)
-	if err == nil {
-		t.Errorf("expected error, found nil")
-	}
-	verifyCalls(t, fakeDocker, []string{"stop"})
-	killedContainer := containers[0]
-	liveContainer := containers[1]
-	ready := kubelet.readinessManager.GetReadiness(killedContainer.ID)
-	if ready {
-		t.Errorf("exepcted container entry ID '%v' to not be found. states: %+v", killedContainer.ID, ready)
-	}
-	ready = kubelet.readinessManager.GetReadiness(liveContainer.ID)
-	if !ready {
-		t.Errorf("exepcted container entry ID '%v' to be found. states: %+v", liveContainer.ID, ready)
-	}
-}
-
-func TestKillContainer(t *testing.T) {
-	containers := []docker.APIContainers{
-		{
-			ID:    "1234",
-			Names: []string{"/k8s_foo_qux_new_1234_42"},
-		},
-		{
-			ID:    "5678",
-			Names: []string{"/k8s_bar_qux_new_5678_42"},
-		},
-	}
-	testKubelet := newTestKubelet(t)
-	kubelet := testKubelet.kubelet
-	fakeDocker := testKubelet.fakeDocker
-	fakeDocker.ContainerList = append([]docker.APIContainers{}, containers...)
-	fakeDocker.Container = &docker.Container{
-		Name: "foobar",
-	}
-	for _, c := range fakeDocker.ContainerList {
-		kubelet.readinessManager.SetReadiness(c.ID, true)
-	}
-
-	c := apiContainerToContainer(fakeDocker.ContainerList[0])
-	err := kubelet.containerManager.KillContainer(c.ID)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	verifyCalls(t, fakeDocker, []string{"stop"})
-	killedContainer := containers[0]
-	liveContainer := containers[1]
-	ready := kubelet.readinessManager.GetReadiness(killedContainer.ID)
-	if ready {
-		t.Errorf("exepcted container entry ID '%v' to not be found. states: %+v", killedContainer.ID, ready)
-	}
-	ready = kubelet.readinessManager.GetReadiness(liveContainer.ID)
-	if !ready {
-		t.Errorf("exepcted container entry ID '%v' to be found. states: %+v", liveContainer.ID, ready)
-	}
-}
-
 var emptyPodUIDs map[types.UID]metrics.SyncPodType
 
 func generatePodInfraContainerHash(pod *api.Pod) uint64 {