Refactr ListContainers.

pkg/kubelet/dockertools/convert.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"strings"
 
+	dockertypes "github.com/docker/engine-api/types"
 	docker "github.com/fsouza/go-dockerclient"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 )
@@ -32,7 +33,7 @@ const (
 )
 
 func mapState(state string) kubecontainer.ContainerState {
-	// Parse the state string in docker.APIContainers. This could break when
+	// Parse the state string in dockertypes.Container. This could break when
 	// we upgrade docker.
 	switch {
 	case strings.HasPrefix(state, statusRunningPrefix):
@@ -44,8 +45,8 @@ func mapState(state string) kubecontainer.ContainerState {
 	}
 }
 
-// Converts docker.APIContainers to kubecontainer.Container.
+// Converts dockertypes.Container to kubecontainer.Container.
-func toRuntimeContainer(c *docker.APIContainers) (*kubecontainer.Container, error) {
+func toRuntimeContainer(c *dockertypes.Container) (*kubecontainer.Container, error) {
 	if c == nil {
 		return nil, fmt.Errorf("unable to convert a nil pointer to a runtime container")
 	}

pkg/kubelet/dockertools/convert_test.go

@@ -20,6 +20,7 @@ import (
 	"reflect"
 	"testing"
 
+	dockertypes "github.com/docker/engine-api/types"
 	docker "github.com/fsouza/go-dockerclient"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 )
@@ -43,7 +44,7 @@ func TestMapState(t *testing.T) {
 }
 
 func TestToRuntimeContainer(t *testing.T) {
-	original := &docker.APIContainers{
+	original := &dockertypes.Container{
 		ID:      "ab2cdf",
 		Image:   "bar_image",
 		Created: 12345,

pkg/kubelet/dockertools/docker.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/docker/docker/pkg/jsonmessage"
 	dockerapi "github.com/docker/engine-api/client"
+	dockertypes "github.com/docker/engine-api/types"
 	docker "github.com/fsouza/go-dockerclient"
 	"github.com/golang/glog"
 	"k8s.io/kubernetes/pkg/api"
@@ -57,7 +58,7 @@ const (
 
 // DockerInterface is an abstract interface for testability.  It abstracts the interface of docker.Client.
 type DockerInterface interface {
-	ListContainers(options docker.ListContainersOptions) ([]docker.APIContainers, error)
+	ListContainers(options dockertypes.ContainerListOptions) ([]dockertypes.Container, error)
 	InspectContainer(id string) (*docker.Container, error)
 	CreateContainer(docker.CreateContainerOptions) (*docker.Container, error)
 	StartContainer(id string, hostConfig *docker.HostConfig) error
@@ -346,9 +347,9 @@ func milliCPUToShares(milliCPU int64) int64 {
 // GetKubeletDockerContainers lists all container or just the running ones.
 // Returns a list of docker containers that we manage
 // TODO: Move this function with dockerCache to DockerManager.
-func GetKubeletDockerContainers(client DockerInterface, allContainers bool) ([]*docker.APIContainers, error) {
+func GetKubeletDockerContainers(client DockerInterface, allContainers bool) ([]*dockertypes.Container, error) {
-	result := []*docker.APIContainers{}
+	result := []*dockertypes.Container{}
-	containers, err := client.ListContainers(docker.ListContainersOptions{All: allContainers})
+	containers, err := client.ListContainers(dockertypes.ContainerListOptions{All: allContainers})
 	if err != nil {
 		return nil, err
 	}

pkg/kubelet/dockertools/docker_test.go

@@ -27,6 +27,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/pkg/jsonmessage"
+	dockertypes "github.com/docker/engine-api/types"
 	docker "github.com/fsouza/go-dockerclient"
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	"k8s.io/kubernetes/cmd/kubelet/app/options"
@@ -62,7 +63,7 @@ func verifyStringArrayEquals(t *testing.T, actual, expected []string) {
 	}
 }
 
-func findPodContainer(dockerContainers []*docker.APIContainers, podFullName string, uid types.UID, containerName string) (*docker.APIContainers, bool, uint64) {
+func findPodContainer(dockerContainers []*dockertypes.Container, podFullName string, uid types.UID, containerName string) (*dockertypes.Container, bool, uint64) {
 	for _, dockerContainer := range dockerContainers {
 		if len(dockerContainer.Names) == 0 {
 			continue
@@ -98,7 +99,7 @@ func TestGetContainerID(t *testing.T) {
 		t.Errorf("Expected no error, Got %#v", err)
 	}
 	if len(dockerContainers) != 2 {
-		t.Errorf("Expected %#v, Got %#v", fakeDocker.ContainerList, dockerContainers)
+		t.Errorf("Expected %#v, Got %#v", fakeDocker.RunningContainerList, dockerContainers)
 	}
 	verifyCalls(t, fakeDocker, []string{"list"})
 
@@ -516,14 +517,14 @@ func (b containersByID) Less(i, j int) bool { return b[i].ID.ID < b[j].ID.ID }
 
 func TestFindContainersByPod(t *testing.T) {
 	tests := []struct {
-		containerList       []docker.APIContainers
+		runningContainerList []dockertypes.Container
-		exitedContainerList []docker.APIContainers
+		exitedContainerList  []dockertypes.Container
-		all                 bool
+		all                  bool
-		expectedPods        []*kubecontainer.Pod
+		expectedPods         []*kubecontainer.Pod
 	}{
 
 		{
-			[]docker.APIContainers{
+			[]dockertypes.Container{
 				{
 					ID:    "foobar",
 					Names: []string{"/k8s_foobar.1234_qux_ns_1234_42"},
@@ -537,7 +538,7 @@ func TestFindContainersByPod(t *testing.T) {
 					Names: []string{"/k8s_baz.1234_qux_ns_1234_42"},
 				},
 			},
-			[]docker.APIContainers{
+			[]dockertypes.Container{
 				{
 					ID:    "barfoo",
 					Names: []string{"/k8s_barfoo.1234_qux_ns_1234_42"},
@@ -584,7 +585,7 @@ func TestFindContainersByPod(t *testing.T) {
 			},
 		},
 		{
-			[]docker.APIContainers{
+			[]dockertypes.Container{
 				{
 					ID:    "foobar",
 					Names: []string{"/k8s_foobar.1234_qux_ns_1234_42"},
@@ -598,7 +599,7 @@ func TestFindContainersByPod(t *testing.T) {
 					Names: []string{"/k8s_baz.1234_qux_ns_1234_42"},
 				},
 			},
-			[]docker.APIContainers{
+			[]dockertypes.Container{
 				{
 					ID:    "barfoo",
 					Names: []string{"/k8s_barfoo.1234_qux_ns_1234_42"},
@@ -664,8 +665,8 @@ func TestFindContainersByPod(t *testing.T) {
 			},
 		},
 		{
-			[]docker.APIContainers{},
+			[]dockertypes.Container{},
-			[]docker.APIContainers{},
+			[]dockertypes.Container{},
 			true,
 			nil,
 		},
@@ -675,7 +676,7 @@ func TestFindContainersByPod(t *testing.T) {
 	// image back-off is set to nil, this test should not pull images
 	containerManager := NewFakeDockerManager(fakeClient, &record.FakeRecorder{}, nil, nil, &cadvisorapi.MachineInfo{}, options.GetDefaultPodInfraContainerImage(), 0, 0, "", containertest.FakeOS{}, np, nil, nil, nil)
 	for i, test := range tests {
-		fakeClient.ContainerList = test.containerList
+		fakeClient.RunningContainerList = test.runningContainerList
 		fakeClient.ExitedContainerList = test.exitedContainerList
 
 		result, _ := containerManager.GetPods(test.all)

pkg/kubelet/dockertools/fake_docker_client.go

@@ -26,6 +26,7 @@ import (
 	"sync"
 	"time"
 
+	dockertypes "github.com/docker/engine-api/types"
 	docker "github.com/fsouza/go-dockerclient"
 
 	"k8s.io/kubernetes/pkg/api"
@@ -35,14 +36,14 @@ import (
 // FakeDockerClient is a simple fake docker client, so that kubelet can be run for testing without requiring a real docker setup.
 type FakeDockerClient struct {
 	sync.Mutex
-	ContainerList       []docker.APIContainers
+	RunningContainerList []dockertypes.Container
-	ExitedContainerList []docker.APIContainers
+	ExitedContainerList  []dockertypes.Container
-	ContainerMap        map[string]*docker.Container
+	ContainerMap         map[string]*docker.Container
-	Image               *docker.Image
+	Image                *docker.Image
-	Images              []docker.APIImages
+	Images               []docker.APIImages
-	Errors              map[string]error
+	Errors               map[string]error
-	called              []string
+	called               []string
-	pulled              []string
+	pulled               []string
 	// Created, Stopped and Removed all container docker ID
 	Created       []string
 	Stopped       []string
@@ -107,8 +108,8 @@ func (f *FakeDockerClient) SetFakeContainers(containers []*docker.Container) {
 	defer f.Unlock()
 	// Reset the lists and the map.
 	f.ContainerMap = map[string]*docker.Container{}
-	f.ContainerList = []docker.APIContainers{}
+	f.RunningContainerList = []dockertypes.Container{}
-	f.ExitedContainerList = []docker.APIContainers{}
+	f.ExitedContainerList = []dockertypes.Container{}
 
 	for i := range containers {
 		c := containers[i]
@@ -116,14 +117,14 @@ func (f *FakeDockerClient) SetFakeContainers(containers []*docker.Container) {
 			c.Config = &docker.Config{}
 		}
 		f.ContainerMap[c.ID] = c
-		apiContainer := docker.APIContainers{
+		container := dockertypes.Container{
 			Names: []string{c.Name},
 			ID:    c.ID,
 		}
 		if c.State.Running {
-			f.ContainerList = append(f.ContainerList, apiContainer)
+			f.RunningContainerList = append(f.RunningContainerList, container)
 		} else {
-			f.ExitedContainerList = append(f.ExitedContainerList, apiContainer)
+			f.ExitedContainerList = append(f.ExitedContainerList, container)
 		}
 	}
 }
@@ -210,12 +211,12 @@ func (f *FakeDockerClient) popError(op string) error {
 
 // ListContainers is a test-spy implementation of DockerInterface.ListContainers.
 // It adds an entry "list" to the internal method call record.
-func (f *FakeDockerClient) ListContainers(options docker.ListContainersOptions) ([]docker.APIContainers, error) {
+func (f *FakeDockerClient) ListContainers(options dockertypes.ContainerListOptions) ([]dockertypes.Container, error) {
 	f.Lock()
 	defer f.Unlock()
 	f.called = append(f.called, "list")
 	err := f.popError("list")
-	containerList := append([]docker.APIContainers{}, f.ContainerList...)
+	containerList := append([]dockertypes.Container{}, f.RunningContainerList...)
 	if options.All {
 		// Although the container is not sorted, but the container with the same name should be in order,
 		// that is enough for us now.
@@ -276,9 +277,9 @@ func (f *FakeDockerClient) CreateContainer(c docker.CreateContainerOptions) (*do
 	name := "/" + c.Name
 	f.Created = append(f.Created, name)
 	// The newest container should be in front, because we assume so in GetPodStatus()
-	f.ContainerList = append([]docker.APIContainers{
+	f.RunningContainerList = append([]dockertypes.Container{
 		{ID: name, Names: []string{name}, Image: c.Config.Image, Labels: c.Config.Labels},
-	}, f.ContainerList...)
+	}, f.RunningContainerList...)
 	container := docker.Container{ID: name, Name: name, Config: c.Config, HostConfig: c.HostConfig}
 	containerCopy := container
 	f.ContainerMap[name] = &containerCopy
@@ -327,16 +328,16 @@ func (f *FakeDockerClient) StopContainer(id string, timeout uint) error {
 	f.Stopped = append(f.Stopped, id)
 	// Container status should be Updated before container moved to ExitedContainerList
 	f.updateContainerStatus(id, statusExitedPrefix)
-	var newList []docker.APIContainers
+	var newList []dockertypes.Container
-	for _, container := range f.ContainerList {
+	for _, container := range f.RunningContainerList {
 		if container.ID == id {
 			// The newest exited container should be in front. Because we assume so in GetPodStatus()
-			f.ExitedContainerList = append([]docker.APIContainers{container}, f.ExitedContainerList...)
+			f.ExitedContainerList = append([]dockertypes.Container{container}, f.ExitedContainerList...)
 			continue
 		}
 		newList = append(newList, container)
 	}
-	f.ContainerList = newList
+	f.RunningContainerList = newList
 	container, ok := f.ContainerMap[id]
 	if !ok {
 		container = &docker.Container{
@@ -455,9 +456,9 @@ func (f *FakeDockerClient) RemoveImage(image string) error {
 }
 
 func (f *FakeDockerClient) updateContainerStatus(id, status string) {
-	for i := range f.ContainerList {
+	for i := range f.RunningContainerList {
-		if f.ContainerList[i].ID == id {
+		if f.RunningContainerList[i].ID == id {
-			f.ContainerList[i].Status = status
+			f.RunningContainerList[i].Status = status
 		}
 	}
 }

pkg/kubelet/dockertools/instrumented_docker.go

@@ -19,6 +19,7 @@ package dockertools
 import (
 	"time"
 
+	dockertypes "github.com/docker/engine-api/types"
 	docker "github.com/fsouza/go-dockerclient"
 	"k8s.io/kubernetes/pkg/kubelet/metrics"
 )
@@ -48,7 +49,7 @@ func recordError(operation string, err error) {
 	}
 }
 
-func (in instrumentedDockerInterface) ListContainers(options docker.ListContainersOptions) ([]docker.APIContainers, error) {
+func (in instrumentedDockerInterface) ListContainers(options dockertypes.ContainerListOptions) ([]dockertypes.Container, error) {
 	const operation = "list_containers"
 	defer recordOperation(operation, time.Now())
 

pkg/kubelet/dockertools/kube_docker_client.go

@@ -100,21 +100,14 @@ func convertEnv(src interface{}) (*docker.Env, error) {
 	return env, nil
 }
 
-func (k *kubeDockerClient) ListContainers(options docker.ListContainersOptions) ([]docker.APIContainers, error) {
+func (k *kubeDockerClient) ListContainers(options dockertypes.ContainerListOptions) ([]dockertypes.Container, error) {
-	containers, err := k.client.ContainerList(getDefaultContext(), dockertypes.ContainerListOptions{
+	containers, err := k.client.ContainerList(getDefaultContext(), options)
-		Size:   options.Size,
-		All:    options.All,
-		Limit:  options.Limit,
-		Since:  options.Since,
-		Before: options.Before,
-		Filter: convertFilters(options.Filters),
-	})
 	if err != nil {
 		return nil, err
 	}
-	apiContainers := []docker.APIContainers{}
+	apiContainers := []dockertypes.Container{}
-	if err := convertType(&containers, &apiContainers); err != nil {
+	for _, c := range containers {
-		return nil, err
+		apiContainers = append(apiContainers, dockertypes.Container(c))
 	}
 	return apiContainers, nil
 }

pkg/kubelet/dockertools/manager.go

@@ -32,6 +32,7 @@ import (
 	"time"
 
 	"github.com/coreos/go-semver/semver"
+	dockertypes "github.com/docker/engine-api/types"
 	docker "github.com/fsouza/go-dockerclient"
 	"github.com/golang/glog"
 	cadvisorapi "github.com/google/cadvisor/info/v1"
@@ -695,7 +696,7 @@ func setEntrypointAndCommand(container *api.Container, opts *kubecontainer.RunCo
 
 // A helper function to get the KubeletContainerName and hash from a docker
 // container.
-func getDockerContainerNameInfo(c *docker.APIContainers) (*KubeletContainerName, uint64, error) {
+func getDockerContainerNameInfo(c *dockertypes.Container) (*KubeletContainerName, uint64, error) {
 	if len(c.Names) == 0 {
 		return nil, 0, fmt.Errorf("cannot parse empty docker container name: %#v", c.Names)
 	}
@@ -707,7 +708,7 @@ func getDockerContainerNameInfo(c *docker.APIContainers) (*KubeletContainerName,
 }
 
 // Get pod UID, name, and namespace by examining the container names.
-func getPodInfoFromContainer(c *docker.APIContainers) (types.UID, string, string, error) {
+func getPodInfoFromContainer(c *dockertypes.Container) (types.UID, string, string, error) {
 	dockerName, _, err := getDockerContainerNameInfo(c)
 	if err != nil {
 		return types.UID(""), "", "", err
@@ -779,8 +780,8 @@ func (dm *DockerManager) GetPods(all bool) ([]*kubecontainer.Pod, error) {
 	}
 
 	// Convert map to list.
-	for _, c := range pods {
+	for _, p := range pods {
-		result = append(result, c)
+		result = append(result, p)
 	}
 	return result, nil
 }
@@ -2150,7 +2151,7 @@ func (dm *DockerManager) GetPodStatus(uid types.UID, name, namespace string) (*k
 	// However, there may be some old containers without these labels, so at least now we can't do that.
 	// TODO(random-liu): Do only one list and pass in the list result in the future
 	// TODO(random-liu): Add filter when we are sure that all the containers have the labels
-	containers, err := dm.client.ListContainers(docker.ListContainersOptions{All: true})
+	containers, err := dm.client.ListContainers(dockertypes.ContainerListOptions{All: true})
 	if err != nil {
 		return podStatus, err
 	}

pkg/kubelet/dockertools/manager_test.go

@@ -29,6 +29,7 @@ import (
 	"testing"
 	"time"
 
+	dockertypes "github.com/docker/engine-api/types"
 	docker "github.com/fsouza/go-dockerclient"
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	"github.com/stretchr/testify/assert"
@@ -338,7 +339,7 @@ func TestGetPods(t *testing.T) {
 	// because the conversion is tested separately in convert_test.go
 	containers := make([]*kubecontainer.Container, len(dockerContainers))
 	for i := range containers {
-		c, err := toRuntimeContainer(&docker.APIContainers{
+		c, err := toRuntimeContainer(&dockertypes.Container{
 			ID:    dockerContainers[i].ID,
 			Names: []string{dockerContainers[i].Name},
 		})
@@ -394,40 +395,6 @@ func TestListImages(t *testing.T) {
 	}
 }
 
-func apiContainerToContainer(c docker.APIContainers) kubecontainer.Container {
-	dockerName, hash, err := ParseDockerName(c.Names[0])
-	if err != nil {
-		return kubecontainer.Container{}
-	}
-	return kubecontainer.Container{
-		ID:   kubecontainer.ContainerID{Type: "docker", ID: c.ID},
-		Name: dockerName.ContainerName,
-		Hash: hash,
-	}
-}
-
-func dockerContainersToPod(containers []*docker.APIContainers) 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:    kubecontainer.ContainerID{Type: "docker", ID: 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 := newTestDockerManager()
 
@@ -634,13 +601,13 @@ func TestSyncPodCreateNetAndContainer(t *testing.T) {
 	fakeDocker.Lock()
 
 	found := false
-	for _, c := range fakeDocker.ContainerList {
+	for _, c := range fakeDocker.RunningContainerList {
 		if c.Image == "pod_infra_image" && strings.HasPrefix(c.Names[0], "/k8s_POD") {
 			found = true
 		}
 	}
 	if !found {
-		t.Errorf("Custom pod infra container not found: %v", fakeDocker.ContainerList)
+		t.Errorf("Custom pod infra container not found: %v", fakeDocker.RunningContainerList)
 	}
 
 	if len(fakeDocker.Created) != 2 ||
@@ -1199,7 +1166,7 @@ func TestGetRestartCount(t *testing.T) {
 	verifyRestartCount(&pod, 3)
 
 	// All exited containers have been garbage collected, restart count should be got from old api pod status
-	fakeDocker.ExitedContainerList = []docker.APIContainers{}
+	fakeDocker.ExitedContainerList = []dockertypes.Container{}
 	verifyRestartCount(&pod, 3)
 	killOneContainer(&pod)
 
@@ -1228,7 +1195,7 @@ func TestGetTerminationMessagePath(t *testing.T) {
 
 	runSyncPod(t, dm, fakeDocker, pod, nil, false)
 
-	containerList := fakeDocker.ContainerList
+	containerList := fakeDocker.RunningContainerList
 	if len(containerList) != 2 {
 		// One for infra container, one for container "bar"
 		t.Fatalf("unexpected container list length %d", len(containerList))