Merge pull request #17533 from Random-Liu/fix-fake-docker-client

cmd/integration/integration.go

@@ -73,7 +73,8 @@ import (
 )
 
 var (
-	fakeDocker1, fakeDocker2 dockertools.FakeDockerClient
+	fakeDocker1 = dockertools.NewFakeDockerClient()
+	fakeDocker2 = dockertools.NewFakeDockerClient()
 	// Limit the number of concurrent tests.
 	maxConcurrency int
 
@@ -220,7 +221,7 @@ func startComponents(firstManifestURL, secondManifestURL string) (string, string
 	cm := cm.NewStubContainerManager()
 	kcfg := kubeletapp.SimpleKubelet(
 		cl,
-		&fakeDocker1,
+		fakeDocker1,
 		"localhost",
 		testRootDir,
 		firstManifestURL,
@@ -252,7 +253,7 @@ func startComponents(firstManifestURL, secondManifestURL string) (string, string
 
 	kcfg = kubeletapp.SimpleKubelet(
 		cl,
-		&fakeDocker2,
+		fakeDocker2,
 		"127.0.0.1",
 		testRootDir,
 		secondManifestURL,

cmd/kubemark/hollow-node.go

@@ -96,9 +96,8 @@ func main() {
 		cadvisorInterface := new(cadvisor.Fake)
 		containerManager := cm.NewStubContainerManager()
 
-		fakeDockerClient := &dockertools.FakeDockerClient{}
+		fakeDockerClient := dockertools.NewFakeDockerClient()
 		fakeDockerClient.VersionInfo = docker.Env{"ApiVersion=1.18"}
-		fakeDockerClient.ContainerMap = make(map[string]*docker.Container)
 		fakeDockerClient.EnableSleep = true
 
 		hollowKubelet := kubemark.NewHollowKubelet(

pkg/kubelet/dockertools/container_gc_test.go

@@ -28,6 +28,7 @@ import (
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 )
 
+// TODO (random-liu) Cleanup the test soon
 func newTestContainerGC(t *testing.T) (*containerGC, *FakeDockerClient) {
 	fakeDocker := new(FakeDockerClient)
 	gc := NewContainerGC(fakeDocker, "")

pkg/kubelet/dockertools/docker_test.go

@@ -79,19 +79,16 @@ func findPodContainer(dockerContainers []*docker.APIContainers, podFullName stri
 
 func TestGetContainerID(t *testing.T) {
 	fakeDocker := &FakeDockerClient{}
-	fakeDocker.ContainerList = []docker.APIContainers{
+	fakeDocker.SetFakeRunningContainers([]*docker.Container{
 		{
-			ID:    "foobar",
-			Names: []string{"/k8s_foo_qux_ns_1234_42"},
+			ID:   "foobar",
+			Name: "/k8s_foo_qux_ns_1234_42",
 		},
 		{
-			ID:    "barbar",
-			Names: []string{"/k8s_bar_qux_ns_2565_42"},
+			ID:   "barbar",
+			Name: "/k8s_bar_qux_ns_2565_42",
 		},
-	}
-	fakeDocker.Container = &docker.Container{
-		ID: "foobar",
-	}
+	})
 
 	dockerContainers, err := GetKubeletDockerContainers(fakeDocker, false)
 	if err != nil {

pkg/kubelet/dockertools/fake_docker_client.go

@@ -33,27 +33,35 @@ import (
 )
 
 // FakeDockerClient is a simple fake docker client, so that kubelet can be run for testing without requiring a real docker setup.
-// TODO: create a proper constructor for FakeDockerClient, so we won't need to check if ContainerMap is not nil.
 type FakeDockerClient struct {
 	sync.Mutex
 	ContainerList       []docker.APIContainers
 	ExitedContainerList []docker.APIContainers
-	Container           *docker.Container
 	ContainerMap        map[string]*docker.Container
 	Image               *docker.Image
 	Images              []docker.APIImages
 	Errors              map[string]error
 	called              []string
-	Stopped             []string
 	pulled              []string
-	Created             []string
-	Removed             []string
-	RemovedImages       sets.String
-	VersionInfo         docker.Env
-	Information         docker.Env
-	ExecInspect         *docker.ExecInspect
-	execCmd             []string
-	EnableSleep         bool
+	// Created, Stopped and Removed all container docker ID
+	Created       []string
+	Stopped       []string
+	Removed       []string
+	RemovedImages sets.String
+	VersionInfo   docker.Env
+	Information   docker.Env
+	ExecInspect   *docker.ExecInspect
+	execCmd       []string
+	EnableSleep   bool
+}
+
+func NewFakeDockerClient() *FakeDockerClient {
+	return &FakeDockerClient{
+		VersionInfo:   docker.Env{"Version=1.1.3", "ApiVersion=1.15"},
+		Errors:        make(map[string]error),
+		RemovedImages: sets.String{},
+		ContainerMap:  make(map[string]*docker.Container),
+	}
 }
 
 func (f *FakeDockerClient) ClearCalls() {
@@ -199,12 +207,10 @@ func (f *FakeDockerClient) InspectContainer(id string) (*docker.Container, error
 	defer f.Unlock()
 	f.called = append(f.called, "inspect_container")
 	err := f.popError("inspect_container")
-	if f.ContainerMap != nil {
-		if container, ok := f.ContainerMap[id]; ok {
-			return container, err
-		}
+	if container, ok := f.ContainerMap[id]; ok {
+		return container, err
 	}
-	return f.Container, err
+	return nil, err
 }
 
 // InspectImage is a test-spy implementation of DockerInterface.InspectImage.
@@ -240,19 +246,17 @@ func (f *FakeDockerClient) CreateContainer(c docker.CreateContainerOptions) (*do
 	if err := f.popError("create"); err != nil {
 		return nil, err
 	}
-	f.Created = append(f.Created, c.Name)
 	// This is not a very good fake. We'll just add this container's name to the list.
 	// Docker likes to add a '/', so copy that behavior.
 	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{
 		{ID: name, Names: []string{name}, Image: c.Config.Image, Labels: c.Config.Labels},
 	}, f.ContainerList...)
 	container := docker.Container{ID: name, Name: name, Config: c.Config}
-	if f.ContainerMap != nil {
-		containerCopy := container
-		f.ContainerMap[name] = &containerCopy
-	}
+	containerCopy := container
+	f.ContainerMap[name] = &containerCopy
 	f.normalSleep(200, 50, 50)
 	return &container, nil
 }
@@ -266,32 +270,18 @@ func (f *FakeDockerClient) StartContainer(id string, hostConfig *docker.HostConf
 	if err := f.popError("start"); err != nil {
 		return err
 	}
-	f.Container = &docker.Container{
-		ID:         id,
-		Name:       id, // For testing purpose, we set name to id
-		Config:     &docker.Config{Image: "testimage"},
-		HostConfig: hostConfig,
-		State: docker.State{
-			Running:   true,
-			Pid:       os.Getpid(),
-			StartedAt: time.Now(),
-		},
-		NetworkSettings: &docker.NetworkSettings{IPAddress: "1.2.3.4"},
+	container, ok := f.ContainerMap[id]
+	if !ok {
+		container = &docker.Container{ID: id, Name: id}
 	}
-	if f.ContainerMap != nil {
-		container, ok := f.ContainerMap[id]
-		if !ok {
-			container = &docker.Container{ID: id, Name: id}
-		}
-		container.HostConfig = hostConfig
-		container.State = docker.State{
-			Running:   true,
-			Pid:       os.Getpid(),
-			StartedAt: time.Now(),
-		}
-		container.NetworkSettings = &docker.NetworkSettings{IPAddress: "2.3.4.5"}
-		f.ContainerMap[id] = container
+	container.HostConfig = hostConfig
+	container.State = docker.State{
+		Running:   true,
+		Pid:       os.Getpid(),
+		StartedAt: time.Now(),
 	}
+	container.NetworkSettings = &docker.NetworkSettings{IPAddress: "2.3.4.5"}
+	f.ContainerMap[id] = container
 	f.normalSleep(200, 50, 50)
 	return nil
 }
@@ -316,24 +306,22 @@ func (f *FakeDockerClient) StopContainer(id string, timeout uint) error {
 		newList = append(newList, container)
 	}
 	f.ContainerList = newList
-	if f.ContainerMap != nil {
-		container, ok := f.ContainerMap[id]
-		if !ok {
-			container = &docker.Container{
-				ID:   id,
-				Name: id,
-				State: docker.State{
-					Running:    false,
-					StartedAt:  time.Now().Add(-time.Second),
-					FinishedAt: time.Now(),
-				},
-			}
-		} else {
-			container.State.FinishedAt = time.Now()
-			container.State.Running = false
+	container, ok := f.ContainerMap[id]
+	if !ok {
+		container = &docker.Container{
+			ID:   id,
+			Name: id,
+			State: docker.State{
+				Running:    false,
+				StartedAt:  time.Now().Add(-time.Second),
+				FinishedAt: time.Now(),
+			},
 		}
-		f.ContainerMap[id] = container
+	} else {
+		container.State.FinishedAt = time.Now()
+		container.State.Running = false
 	}
+	f.ContainerMap[id] = container
 	f.normalSleep(200, 50, 50)
 	return nil
 }
@@ -346,9 +334,7 @@ func (f *FakeDockerClient) RemoveContainer(opts docker.RemoveContainerOptions) e
 	if err == nil {
 		f.Removed = append(f.Removed, opts.ID)
 	}
-	if f.ContainerMap != nil {
-		delete(f.ContainerMap, opts.ID)
-	}
+	delete(f.ContainerMap, opts.ID)
 	return err
 }
 

pkg/kubelet/network/cni/cni_test.go

@@ -38,7 +38,6 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/dockertools"
 	"k8s.io/kubernetes/pkg/kubelet/network"
 	proberesults "k8s.io/kubernetes/pkg/kubelet/prober/results"
-	"k8s.io/kubernetes/pkg/util/sets"
 )
 
 // The temp dir where test plugins will be stored.
@@ -137,15 +136,17 @@ func (fnh *fakeNetworkHost) GetKubeClient() client.Interface {
 
 func (nh *fakeNetworkHost) GetRuntime() kubecontainer.Runtime {
 	dm, fakeDockerClient := newTestDockerManager()
-	fakeDockerClient.Container = &docker.Container{
-		ID:    "foobar",
-		State: docker.State{Pid: 12345},
-	}
+	fakeDockerClient.SetFakeRunningContainers([]*docker.Container{
+		{
+			ID:    "test_infra_container",
+			State: docker.State{Pid: 12345},
+		},
+	})
 	return dm
 }
 
 func newTestDockerManager() (*dockertools.DockerManager, *dockertools.FakeDockerClient) {
-	fakeDocker := &dockertools.FakeDockerClient{VersionInfo: docker.Env{"Version=1.1.3", "ApiVersion=1.15"}, Errors: make(map[string]error), RemovedImages: sets.String{}}
+	fakeDocker := dockertools.NewFakeDockerClient()
 	fakeRecorder := &record.FakeRecorder{}
 	containerRefManager := kubecontainer.NewRefManager()
 	networkPlugin, _ := network.InitNetworkPlugin([]network.NetworkPlugin{}, "", network.NewFakeHost(nil))
@@ -183,7 +184,7 @@ func TestCNIPlugin(t *testing.T) {
 		t.Fatalf("Failed to select the desired plugin: %v", err)
 	}
 
-	err = plug.SetUpPod("podNamespace", "podName", "dockerid2345")
+	err = plug.SetUpPod("podNamespace", "podName", "test_infra_container")
 	if err != nil {
 		t.Errorf("Expected nil: %v", err)
 	}
@@ -194,16 +195,16 @@ func TestCNIPlugin(t *testing.T) {
 	if err != nil {
 		t.Errorf("Failed to read output file %s: %v (env %s err %v)", outputFile, err, eo, eerr)
 	}
-	expectedOutput := "ADD /proc/12345/ns/net podNamespace podName dockerid2345"
+	expectedOutput := "ADD /proc/12345/ns/net podNamespace podName test_infra_container"
 	if string(output) != expectedOutput {
 		t.Errorf("Mismatch in expected output for setup hook. Expected '%s', got '%s'", expectedOutput, string(output))
 	}
-	err = plug.TearDownPod("podNamespace", "podName", "dockerid4545454")
+	err = plug.TearDownPod("podNamespace", "podName", "test_infra_container")
 	if err != nil {
 		t.Errorf("Expected nil: %v", err)
 	}
 	output, err = ioutil.ReadFile(path.Join(testNetworkConfigPath, pluginName, pluginName+".out"))
-	expectedOutput = "DEL /proc/12345/ns/net podNamespace podName dockerid4545454"
+	expectedOutput = "DEL /proc/12345/ns/net podNamespace podName test_infra_container"
 	if string(output) != expectedOutput {
 		t.Errorf("Mismatch in expected output for setup hook. Expected '%s', got '%s'", expectedOutput, string(output))
 	}