Merge pull request #7271 from yifan-gu/run

kubelet/dockertools: Move RunContainer into container runtime.

pkg/kubelet/container/runtime.go

@@ -72,14 +72,6 @@ type Runtime interface {
 	IsImagePresent(image string) (bool, error)
 }
 
-// Container runner is a narrow interface to consume in the Kubelet
-// before there is a full implementation of Runtime.
-//
-// TODO: eventually include this interface in Runtime
-type ContainerRunner interface {
-	RunContainer(pod *api.Pod, container *api.Container, opts *RunContainerOptions) (string, error)
-}
-
 // Pod is a group of containers, with the status of the pod.
 type Pod struct {
 	// The ID of the pod, which can be used to retrieve a particular pod

pkg/kubelet/dockertools/manager.go

@@ -45,9 +45,8 @@ const (
 	maxReasonCacheEntries = 200
 )
 
-// Implements kubecontainer.ContainerRunner.
 // TODO: Eventually DockerManager should implement kubecontainer.Runtime
-// interface, and it should also add a cache to replace dockerCache.
+// interface.
 type DockerManager struct {
 	client              DockerInterface
 	recorder            record.EventRecorder
@@ -73,9 +72,6 @@ type DockerManager struct {
 	Puller DockerPuller
 }
 
-// Ensures DockerManager implements ConatinerRunner.
-var _ kubecontainer.ContainerRunner = new(DockerManager)
-
 func NewDockerManager(
 	client DockerInterface,
 	recorder record.EventRecorder,
@@ -408,8 +404,8 @@ func (dm *DockerManager) GetRunningContainers(ids []string) ([]*docker.Container
 	return result, nil
 }
 
-func (dm *DockerManager) RunContainer(pod *api.Pod, container *api.Container, opts *kubecontainer.RunContainerOptions) (string, error) {
-	dockerID, err := dm.runContainer(pod, container, opts)
+func (dm *DockerManager) runContainerRecordErrorReason(pod *api.Pod, container *api.Container, opts *kubecontainer.RunContainerOptions, ref *api.ObjectReference) (string, error) {
+	dockerID, err := dm.runContainer(pod, container, opts, ref)
 	if err != nil {
 		errString := err.Error()
 		if errString != "" {
@@ -421,12 +417,7 @@ func (dm *DockerManager) RunContainer(pod *api.Pod, container *api.Container, op
 	return dockerID, err
 }
 
-func (dm *DockerManager) runContainer(pod *api.Pod, container *api.Container, opts *kubecontainer.RunContainerOptions) (string, error) {
-	ref, err := kubecontainer.GenerateContainerRef(pod, container)
-	if err != nil {
-		glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err)
-	}
-
+func (dm *DockerManager) runContainer(pod *api.Pod, container *api.Container, opts *kubecontainer.RunContainerOptions, ref *api.ObjectReference) (string, error) {
 	dockerName := KubeletContainerName{
 		PodFullName:   kubecontainer.GetPodFullName(pod),
 		PodUID:        pod.UID,
@@ -911,3 +902,35 @@ func (dm *DockerManager) KillContainer(containerID types.UID) error {
 	}
 	return err
 }
+
+// Run a single container from a pod. Returns the docker container ID
+func (dm *DockerManager) RunContainer(pod *api.Pod, container *api.Container, generator kubecontainer.RunContainerOptionsGenerator, runner kubecontainer.HandlerRunner, netMode, ipcMode string) (DockerID, error) {
+	ref, err := kubecontainer.GenerateContainerRef(pod, container)
+	if err != nil {
+		glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err)
+	}
+
+	opts, err := generator.GenerateRunContainerOptions(pod, container, netMode, ipcMode)
+	if err != nil {
+		return "", err
+	}
+
+	id, err := dm.runContainerRecordErrorReason(pod, container, opts, ref)
+	if err != nil {
+		return "", err
+	}
+
+	// Remember this reference so we can report events about this container
+	if ref != nil {
+		dm.containerRefManager.SetRef(id, ref)
+	}
+
+	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))
+			return DockerID(""), fmt.Errorf("failed to call event handler: %v", handlerErr)
+		}
+	}
+	return DockerID(id), err
+}

pkg/kubelet/kubelet.go

@@ -728,38 +728,6 @@ func (kl *Kubelet) GenerateRunContainerOptions(pod *api.Pod, container *api.Cont
 	return opts, nil
 }
 
-// Run a single container from a pod. Returns the docker container ID
-func (kl *Kubelet) runContainer(pod *api.Pod, container *api.Container, netMode, ipcMode string) (dockertools.DockerID, error) {
-	ref, err := kubecontainer.GenerateContainerRef(pod, container)
-	if err != nil {
-		glog.Errorf("Couldn't make a ref to pod %v, container %v: '%v'", pod.Name, container.Name, err)
-	}
-
-	opts, err := kl.GenerateRunContainerOptions(pod, container, netMode, ipcMode)
-	if err != nil {
-		return "", err
-	}
-
-	id, err := kl.containerManager.RunContainer(pod, container, opts)
-	if err != nil {
-		return "", err
-	}
-
-	// Remember this reference so we can report events about this container
-	if ref != nil {
-		kl.containerRefManager.SetRef(id, ref)
-	}
-
-	if container.Lifecycle != nil && container.Lifecycle.PostStart != nil {
-		handlerErr := kl.handlerRunner.Run(id, pod, container, container.Lifecycle.PostStart)
-		if handlerErr != nil {
-			kl.containerManager.KillContainer(types.UID(id))
-			return dockertools.DockerID(""), fmt.Errorf("failed to call event handler: %v", handlerErr)
-		}
-	}
-	return dockertools.DockerID(id), err
-}
-
 var masterServices = util.NewStringSet("kubernetes", "kubernetes-ro")
 
 // getServiceEnvVarMap makes a map[string]string of env vars for services a pod in namespace ns should see
@@ -952,7 +920,7 @@ func (kl *Kubelet) createPodInfraContainer(pod *api.Pod) (dockertools.DockerID,
 		kl.recorder.Eventf(ref, "pulled", "Successfully pulled image %q", container.Image)
 	}
 
-	id, err := kl.runContainer(pod, container, netNamespace, "")
+	id, err := kl.containerManager.RunContainer(pod, container, kl, kl.handlerRunner, netNamespace, "")
 	if err != nil {
 		return "", err
 	}
@@ -1107,7 +1075,7 @@ func (kl *Kubelet) pullImageAndRunContainer(pod *api.Pod, container *api.Contain
 	}
 	// TODO(dawnchen): Check RestartPolicy.DelaySeconds before restart a container
 	namespaceMode := fmt.Sprintf("container:%v", podInfraContainerID)
-	containerID, err := kl.runContainer(pod, container, namespaceMode, namespaceMode)
+	containerID, err := kl.containerManager.RunContainer(pod, container, kl, kl.handlerRunner, namespaceMode, namespaceMode)
 	if err != nil {
 		// TODO(bburns) : Perhaps blacklist a container after N failures?
 		glog.Errorf("Error running pod %q container %q: %v", podFullName, container.Name, err)

pkg/kubelet/kubelet_test.go

@@ -4140,7 +4140,7 @@ func TestGetPodCreationFailureReason(t *testing.T) {
 	pods := []*api.Pod{pod}
 	kubelet.podManager.SetPods(pods)
 	kubelet.volumeManager.SetVolumes(pod.UID, volumeMap{})
-	_, err := kubelet.runContainer(pod, &pod.Spec.Containers[0], "", "")
+	_, err := kubelet.containerManager.RunContainer(pod, &pod.Spec.Containers[0], kubelet, kubelet.handlerRunner, "", "")
 	if err == nil {
 		t.Errorf("expected error, found nil")
 	}