Merge pull request #9279 from yifan-gu/refactor

pkg/api/testing/fuzzer.go

@@ -29,7 +29,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
-	"github.com/fsouza/go-dockerclient"
+	docker "github.com/fsouza/go-dockerclient"
 	"github.com/google/gofuzz"
 
 	"speter.net/go/exp/math/dec/inf"

pkg/kubelet/container/runtime_cache.go

@@ -32,8 +32,6 @@ type RuntimeCache interface {
 	ForceUpdateIfOlder(time.Time) error
 }
 
-// TODO(yifan): This interface can be removed once docker manager has implemented
-// all the runtime interfaces, (thus we can pass the runtime directly).
 type podsGetter interface {
 	GetPods(bool) ([]*Pod, error)
 }

pkg/kubelet/container_gc.go

@@ -23,7 +23,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
-	"github.com/fsouza/go-dockerclient"
+	docker "github.com/fsouza/go-dockerclient"
 	"github.com/golang/glog"
 )
 

pkg/kubelet/container_gc_test.go

@@ -22,7 +22,7 @@ import (
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
-	"github.com/fsouza/go-dockerclient"
+	docker "github.com/fsouza/go-dockerclient"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )

pkg/kubelet/dockertools/exec.go

@@ -24,7 +24,7 @@ import (
 	"time"
 
 	kubecontainer "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/container"
-	"github.com/fsouza/go-dockerclient"
+	docker "github.com/fsouza/go-dockerclient"
 )
 
 // ExecHandler knows how to execute a command in a running Docker container.

pkg/kubelet/dockertools/fake_docker_client.go

@@ -24,7 +24,7 @@ import (
 	"sort"
 	"sync"
 
-	"github.com/fsouza/go-dockerclient"
+	docker "github.com/fsouza/go-dockerclient"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"

pkg/kubelet/dockertools/fake_manager.go

@@ -41,7 +41,7 @@ func NewFakeDockerManager(
 
 	dm := NewDockerManager(client, recorder, readinessManager, containerRefManager, podInfraContainerImage, qps,
 		burst, containerLogsDir, osInterface, networkPlugin, generator, httpClient, runtimeHooks, &NativeExecHandler{})
-	dm.Puller = &FakeDockerPuller{}
+	dm.puller = &FakeDockerPuller{}
 	dm.prober = prober.New(nil, readinessManager, containerRefManager, recorder)
 	return dm
 }

pkg/kubelet/dockertools/instrumented_docker.go

@@ -20,7 +20,7 @@ import (
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/metrics"
-	"github.com/fsouza/go-dockerclient"
+	docker "github.com/fsouza/go-dockerclient"
 )
 
 type instrumentedDockerInterface struct {

pkg/kubelet/dockertools/manager.go

@@ -68,9 +68,8 @@ type DockerManager struct {
 	containerRefManager *kubecontainer.RefManager
 	os                  kubecontainer.OSInterface
 
-	// TODO(yifan): PodInfraContainerImage can be unexported once
-	// we move createPodInfraContainer into dockertools.
-	PodInfraContainerImage string
+	// The image name of the pod infra container.
+	podInfraContainerImage string
 	// reasonCache stores the failure reason of the last container creation
 	// and/or start in a string, keyed by <pod_UID>_<container_name>. The goal
 	// is to propagate this reason to the container status. This endeavor is
@@ -80,11 +79,9 @@ type DockerManager struct {
 	//      means that some entries may be recycled before a pod has been
 	//      deleted.
 	reasonCache stringCache
-	// TODO(yifan): We export this for testability, so when we have a fake
-	// container manager, then we can unexport this. Also at that time, we
-	// use the concrete type so that we can record the pull failure and eliminate
-	// the image checking in GetPodStatus().
-	Puller DockerPuller
+	// TODO(yifan): Record the pull failure so we can  eliminate the image checking
+	// in GetPodStatus()?
+	puller DockerPuller
 
 	// Root of the Docker runtime.
 	dockerRoot string
@@ -164,9 +161,9 @@ func NewDockerManager(
 		readinessManager:    readinessManager,
 		containerRefManager: containerRefManager,
 		os:                  osInterface,
-		PodInfraContainerImage: podInfraContainerImage,
+		podInfraContainerImage: podInfraContainerImage,
 		reasonCache:            reasonCache,
-		Puller:                 newDockerPuller(client, qps, burst),
+		puller:                 newDockerPuller(client, qps, burst),
 		dockerRoot:             dockerRoot,
 		containerLogsDir:       containerLogsDir,
 		networkPlugin:          networkPlugin,
@@ -784,12 +781,12 @@ func (dm *DockerManager) ListImages() ([]kubecontainer.Image, error) {
 // TODO(vmarmol): Consider unexporting.
 // PullImage pulls an image from network to local storage.
 func (dm *DockerManager) PullImage(image kubecontainer.ImageSpec, secrets []api.Secret) error {
-	return dm.Puller.Pull(image.Image, secrets)
+	return dm.puller.Pull(image.Image, secrets)
 }
 
 // IsImagePresent checks whether the container image is already in the local storage.
 func (dm *DockerManager) IsImagePresent(image kubecontainer.ImageSpec) (bool, error) {
-	return dm.Puller.IsImagePresent(image.Image)
+	return dm.puller.IsImagePresent(image.Image)
 }
 
 // Removes the specified image.
@@ -825,7 +822,7 @@ func (dm *DockerManager) podInfraContainerChanged(pod *api.Pod, podInfraContaine
 	}
 	expectedPodInfraContainer := &api.Container{
 		Name:  PodInfraContainerName,
-		Image: dm.PodInfraContainerImage,
+		Image: dm.podInfraContainerImage,
 		Ports: ports,
 	}
 	return podInfraContainer.Hash != kubecontainer.HashContainer(expectedPodInfraContainer), nil
@@ -1203,7 +1200,7 @@ func (dm *DockerManager) createPodInfraContainer(pod *api.Pod) (kubeletTypes.Doc
 
 	container := &api.Container{
 		Name:  PodInfraContainerName,
-		Image: dm.PodInfraContainerImage,
+		Image: dm.podInfraContainerImage,
 		Ports: ports,
 	}
 	ref, err := kubecontainer.GenerateContainerRef(pod, container)

pkg/kubelet/dockertools/manager_test.go

@@ -38,7 +38,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	uexec "github.com/GoogleCloudPlatform/kubernetes/pkg/util/exec"
-	"github.com/fsouza/go-dockerclient"
+	docker "github.com/fsouza/go-dockerclient"
 )
 
 type fakeHTTP struct {
@@ -876,7 +876,7 @@ func runSyncPod(t *testing.T, dm *DockerManager, fakeDocker *FakeDockerClient, p
 
 func TestSyncPodCreateNetAndContainer(t *testing.T) {
 	dm, fakeDocker := newTestDockerManager()
-	dm.PodInfraContainerImage = "custom_image_name"
+	dm.podInfraContainerImage = "custom_image_name"
 	fakeDocker.ContainerList = []docker.APIContainers{}
 	pod := &api.Pod{
 		ObjectMeta: api.ObjectMeta{
@@ -921,10 +921,10 @@ func TestSyncPodCreateNetAndContainer(t *testing.T) {
 
 func TestSyncPodCreatesNetAndContainerPullsImage(t *testing.T) {
 	dm, fakeDocker := newTestDockerManager()
-	dm.PodInfraContainerImage = "custom_image_name"
-	puller := dm.Puller.(*FakeDockerPuller)
+	dm.podInfraContainerImage = "custom_image_name"
+	puller := dm.puller.(*FakeDockerPuller)
 	puller.HasImages = []string{}
-	dm.PodInfraContainerImage = "custom_image_name"
+	dm.podInfraContainerImage = "custom_image_name"
 	fakeDocker.ContainerList = []docker.APIContainers{}
 	pod := &api.Pod{
 		ObjectMeta: api.ObjectMeta{
@@ -1282,9 +1282,9 @@ func TestSyncPodsDoesNothing(t *testing.T) {
 
 func TestSyncPodWithPullPolicy(t *testing.T) {
 	dm, fakeDocker := newTestDockerManager()
-	puller := dm.Puller.(*FakeDockerPuller)
+	puller := dm.puller.(*FakeDockerPuller)
 	puller.HasImages = []string{"existing_one", "want:latest"}
-	dm.PodInfraContainerImage = "custom_image_name"
+	dm.podInfraContainerImage = "custom_image_name"
 	fakeDocker.ContainerList = []docker.APIContainers{}
 
 	pod := &api.Pod{
@@ -1649,7 +1649,7 @@ func TestGetPodPullImageFailureReason(t *testing.T) {
 	dm, fakeDocker := newTestDockerManager()
 	// Initialize the FakeDockerPuller so that it'd try to pull non-existent
 	// images.
-	puller := dm.Puller.(*FakeDockerPuller)
+	puller := dm.puller.(*FakeDockerPuller)
 	puller.HasImages = []string{}
 	// Inject the pull image failure error.
 	failureReason := "pull image faiulre"

pkg/kubelet/kubelet.go

@@ -2124,7 +2124,6 @@ func (kl *Kubelet) ServeLogs(w http.ResponseWriter, req *http.Request) {
 
 // findContainer finds and returns the container with the given pod ID, full name, and container name.
 // It returns nil if not found.
-// TODO(yifan): Move this to runtime once the runtime interface has been all implemented.
 func (kl *Kubelet) findContainer(podFullName string, podUID types.UID, containerName string) (*kubecontainer.Container, error) {
 	pods, err := kl.containerRuntime.GetPods(false)
 	if err != nil {

pkg/kubelet/kubelet_test.go

@@ -50,7 +50,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/version"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/volume"
 	_ "github.com/GoogleCloudPlatform/kubernetes/pkg/volume/host_path"
-	"github.com/fsouza/go-dockerclient"
+	docker "github.com/fsouza/go-dockerclient"
 	cadvisorApi "github.com/google/cadvisor/info/v1"
 	cadvisorApiv2 "github.com/google/cadvisor/info/v2"
 )
@@ -2176,17 +2176,12 @@ func TestPortForward(t *testing.T) {
 	var port uint16 = 5000
 	stream := &fakeReadWriteCloser{}
 
-	podInfraContainerImage := "POD"
 	infraContainerID := "infra"
-	// TODO: Move this test to dockertools so that we don't have to do the hacky
-	// type assertion here.
-	dm := kubelet.containerRuntime.(*dockertools.DockerManager)
-	dm.PodInfraContainerImage = podInfraContainerImage
 
 	fakeDocker.ContainerList = []docker.APIContainers{
 		{
 			ID:    infraContainerID,
-			Names: []string{"/k8s_" + podInfraContainerImage + "_" + podName + "_" + podNamespace + "_12345678_42"},
+			Names: []string{"/k8s_POD" + "_" + podName + "_" + podNamespace + "_12345678_42"},
 		},
 		{
 			ID:    containerID,

pkg/kubelet/pod_workers_test.go

@@ -29,7 +29,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/network"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
-	"github.com/fsouza/go-dockerclient"
+	docker "github.com/fsouza/go-dockerclient"
 )
 
 func newPod(uid, name string) *api.Pod {