diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 98123b128a6..4da2170d7c3 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/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) } } diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index 1d6d14c5678..c048d06b5b2 100644 --- a/pkg/kubelet/dockertools/manager_test.go +++ b/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) + } +} diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 124eecdd2f4..94fe518ccf9 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/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 {