From 7fedb14c2796e9a110fab8a172f564ba94a104f4 Mon Sep 17 00:00:00 2001 From: Yifan Gu Date: Mon, 13 Apr 2015 15:25:14 -0700 Subject: [PATCH] kubelet: Remove unused docker functions. Remove kubelet.getPodInfraContainer(). Remove dockertools.RemoveContainerWithID(). Remove dockertools.FindContainersByPod(). Also replace the useless test with a test for GetPods(). --- pkg/kubelet/dockertools/docker.go | 24 --- pkg/kubelet/dockertools/docker_test.go | 256 ++++++++++++++----------- pkg/kubelet/kubelet.go | 11 -- 3 files changed, 148 insertions(+), 143 deletions(-) diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index ad3e58934cc..03ae96d88d9 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -409,30 +409,6 @@ func (c DockerContainers) FindPodContainer(podFullName string, uid types.UID, co return nil, false, 0 } -// RemoveContainerWithID removes the container with the given containerID. -func (c DockerContainers) RemoveContainerWithID(containerID DockerID) { - delete(c, containerID) -} - -// FindContainersByPod returns the containers that belong to the pod. -func (c DockerContainers) FindContainersByPod(podUID types.UID, podFullName string) DockerContainers { - containers := make(DockerContainers) - for _, dockerContainer := range c { - if len(dockerContainer.Names) == 0 { - continue - } - dockerName, _, err := ParseDockerName(dockerContainer.Names[0]) - if err != nil { - continue - } - if podUID == dockerName.PodUID || - (podUID == "" && podFullName == dockerName.PodFullName) { - containers[DockerID(dockerContainer.ID)] = dockerContainer - } - } - return containers -} - const containerNamePrefix = "k8s" func HashContainer(container *api.Container) uint64 { diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index dec82f3bce0..5771d42b346 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -20,11 +20,13 @@ import ( "fmt" "hash/adler32" "reflect" + "sort" "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/client/record" "github.com/GoogleCloudPlatform/kubernetes/pkg/credentialprovider" + kubecontainer "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/container" "github.com/GoogleCloudPlatform/kubernetes/pkg/types" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" docker "github.com/fsouza/go-dockerclient" @@ -500,145 +502,183 @@ func TestGetRunningContainers(t *testing.T) { } } +type podsByID []*kubecontainer.Pod + +func (b podsByID) Len() int { return len(b) } +func (b podsByID) Swap(i, j int) { b[i], b[j] = b[j], b[i] } +func (b podsByID) Less(i, j int) bool { return b[i].ID < b[j].ID } + +type containersByID []*kubecontainer.Container + +func (b containersByID) Len() int { return len(b) } +func (b containersByID) Swap(i, j int) { b[i], b[j] = b[j], b[i] } +func (b containersByID) Less(i, j int) bool { return b[i].ID < b[j].ID } + func TestFindContainersByPod(t *testing.T) { tests := []struct { - testContainers DockerContainers - inputPodID types.UID - inputPodFullName string - expectedContainers DockerContainers + containerList []docker.APIContainers + exitedContainerList []docker.APIContainers + all bool + expectedPods []*kubecontainer.Pod }{ + { - DockerContainers{ - "foobar": &docker.APIContainers{ + []docker.APIContainers{ + { ID: "foobar", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, + Names: []string{"/k8s_foobar.1234_qux_ns_1234_42"}, }, - "barbar": &docker.APIContainers{ + { ID: "barbar", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, + Names: []string{"/k8s_barbar.1234_qux_ns_2343_42"}, }, - "baz": &docker.APIContainers{ + { ID: "baz", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, + Names: []string{"/k8s_baz.1234_qux_ns_1234_42"}, }, }, - types.UID("1234"), - "", - DockerContainers{ - "foobar": &docker.APIContainers{ - ID: "foobar", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, + []docker.APIContainers{ + { + ID: "barfoo", + Names: []string{"/k8s_barfoo.1234_qux_ns_1234_42"}, }, - "barbar": &docker.APIContainers{ - ID: "barbar", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, + { + ID: "bazbaz", + Names: []string{"/k8s_bazbaz.1234_qux_ns_5678_42"}, }, - "baz": &docker.APIContainers{ - ID: "baz", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, + }, + false, + []*kubecontainer.Pod{ + { + ID: "1234", + Name: "qux", + Namespace: "ns", + Containers: []*kubecontainer.Container{ + { + ID: "foobar", + Name: "foobar", + Hash: 0x1234, + }, + { + ID: "baz", + Name: "baz", + Hash: 0x1234, + }, + }, + }, + { + ID: "2343", + Name: "qux", + Namespace: "ns", + Containers: []*kubecontainer.Container{ + { + ID: "barbar", + Name: "barbar", + Hash: 0x1234, + }, + }, }, }, }, { - DockerContainers{ - "foobar": &docker.APIContainers{ + []docker.APIContainers{ + { ID: "foobar", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, + Names: []string{"/k8s_foobar.1234_qux_ns_1234_42"}, }, - "barbar": &docker.APIContainers{ + { ID: "barbar", - Names: []string{"/k8s_foo_qux_ns_2343_42"}, + Names: []string{"/k8s_barbar.1234_qux_ns_2343_42"}, }, - "baz": &docker.APIContainers{ + { ID: "baz", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, + Names: []string{"/k8s_baz.1234_qux_ns_1234_42"}, }, }, - types.UID("1234"), - "", - DockerContainers{ - "foobar": &docker.APIContainers{ - ID: "foobar", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, + []docker.APIContainers{ + { + ID: "barfoo", + Names: []string{"/k8s_barfoo.1234_qux_ns_1234_42"}, }, - "baz": &docker.APIContainers{ - ID: "baz", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, + { + ID: "bazbaz", + Names: []string{"/k8s_bazbaz.1234_qux_ns_5678_42"}, + }, + }, + true, + []*kubecontainer.Pod{ + { + ID: "1234", + Name: "qux", + Namespace: "ns", + Containers: []*kubecontainer.Container{ + { + ID: "foobar", + Name: "foobar", + Hash: 0x1234, + }, + { + ID: "barfoo", + Name: "barfoo", + Hash: 0x1234, + }, + { + ID: "baz", + Name: "baz", + Hash: 0x1234, + }, + }, + }, + { + ID: "2343", + Name: "qux", + Namespace: "ns", + Containers: []*kubecontainer.Container{ + { + ID: "barbar", + Name: "barbar", + Hash: 0x1234, + }, + }, + }, + { + ID: "5678", + Name: "qux", + Namespace: "ns", + Containers: []*kubecontainer.Container{ + { + ID: "bazbaz", + Name: "bazbaz", + Hash: 0x1234, + }, + }, }, }, }, { - DockerContainers{ - "foobar": &docker.APIContainers{ - ID: "foobar", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, - }, - "barbar": &docker.APIContainers{ - ID: "barbar", - Names: []string{"/k8s_foo_qux_ns_2343_42"}, - }, - "baz": &docker.APIContainers{ - ID: "baz", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, - }, - }, - types.UID("5678"), - "", - DockerContainers{}, - }, - { - DockerContainers{ - "foobar": &docker.APIContainers{ - ID: "foobar", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, - }, - "barbar": &docker.APIContainers{ - ID: "barbar", - Names: nil, - }, - "baz": &docker.APIContainers{ - ID: "baz", - Names: []string{"/k8s_foo_qux_ns_5678_42"}, - }, - }, - types.UID("5678"), - "", - DockerContainers{ - "baz": &docker.APIContainers{ - ID: "baz", - Names: []string{"/k8s_foo_qux_ns_5678_42"}, - }, - }, - }, - { - DockerContainers{ - "foobar": &docker.APIContainers{ - ID: "foobar", - Names: []string{"/k8s_foo_qux_ns_1234_42"}, - }, - "barbar": &docker.APIContainers{ - ID: "barbar", - Names: []string{"/k8s_foo_abc_ns_5678_42"}, - }, - "baz": &docker.APIContainers{ - ID: "baz", - Names: []string{"/k8s_foo_qux_ns_5678_42"}, - }, - }, - "", - "abc_ns", - DockerContainers{ - "barbar": &docker.APIContainers{ - ID: "barbar", - Names: []string{"/k8s_foo_abc_ns_5678_42"}, - }, - }, + []docker.APIContainers{}, + []docker.APIContainers{}, + true, + nil, }, } - for _, test := range tests { - result := test.testContainers.FindContainersByPod(test.inputPodID, test.inputPodFullName) - if !reflect.DeepEqual(result, test.expectedContainers) { - t.Errorf("expected: %v, saw: %v", test.expectedContainers, result) + fakeClient := &FakeDockerClient{} + containerManager := NewDockerManager(fakeClient, &record.FakeRecorder{}, PodInfraContainerImage, 0, 0) + for i, test := range tests { + fakeClient.ContainerList = test.containerList + fakeClient.ExitedContainerList = test.exitedContainerList + + result, _ := containerManager.GetPods(test.all) + for i := range result { + sort.Sort(containersByID(result[i].Containers)) + } + for i := range test.expectedPods { + sort.Sort(containersByID(test.expectedPods[i].Containers)) + } + sort.Sort(podsByID(result)) + sort.Sort(podsByID(test.expectedPods)) + if !reflect.DeepEqual(test.expectedPods, result) { + t.Errorf("%d: expected: %#v, saw: %#v", i, test.expectedPods, result) } } } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index e22b3d1d1ad..9078d462188 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1001,17 +1001,6 @@ func (kl *Kubelet) shouldContainerBeRestarted(container *api.Container, pod *api return true } -// Finds an infra container for a pod given by podFullName and UID in dockerContainers. If there is an infra container -// return its ID and true, otherwise it returns empty ID and false. -func (kl *Kubelet) getPodInfraContainer(podFullName string, uid types.UID, - dockerContainers dockertools.DockerContainers) (dockertools.DockerID, bool) { - if podInfraDockerContainer, found, _ := dockerContainers.FindPodContainer(podFullName, uid, dockertools.PodInfraContainerName); found { - podInfraContainerID := dockertools.DockerID(podInfraDockerContainer.ID) - return podInfraContainerID, true - } - return "", false -} - // Attempts to start a container pulling the image before that if necessary. It returns DockerID of a started container // if it was successful, and a non-nil error otherwise. func (kl *Kubelet) pullImageAndRunContainer(pod *api.Pod, container *api.Container, podVolumes *volumeMap,