Merge pull request #6491 from yifan-gu/depreciate_getkubeletdockercontainers

kubelet: Refactor RunInContainer/ExecInContainer/PortForward.

pkg/kubelet/container/runtime.go

@@ -124,9 +124,8 @@ type RunContainerOptions struct {
 
 type Pods []*Pod
 
-// FindPodByID returns a pod in the pod list by UID. It will return an empty pod
+// FindPodByID finds and returns a pod in the pod list by UID. It will return an empty pod
 // if not found.
-// TODO(yifan): Use a map?
 func (p Pods) FindPodByID(podUID types.UID) Pod {
 	for i := range p {
 		if p[i].ID == podUID {
@@ -136,6 +135,27 @@ func (p Pods) FindPodByID(podUID types.UID) Pod {
 	return Pod{}
 }
 
+// FindPodByFullName finds and returns a pod in the pod list by the full name.
+// It will return an empty pod if not found.
+func (p Pods) FindPodByFullName(podFullName string) Pod {
+	for i := range p {
+		if BuildPodFullName(p[i].Name, p[i].Namespace) == podFullName {
+			return *p[i]
+		}
+	}
+	return Pod{}
+}
+
+// FindPod combines FindPodByID and FindPodByFullName, it finds and returns a pod in the
+// pod list either by the full name or the pod ID. It will return an empty pod
+// if not found.
+func (p Pods) FindPod(podFullName string, podUID types.UID) Pod {
+	if len(podFullName) > 0 {
+		return p.FindPodByFullName(podFullName)
+	}
+	return p.FindPodByID(podUID)
+}
+
 // FindContainerByName returns a container in the pod with the given name.
 // When there are multiple containers with the same name, the first match will
 // be returned.

pkg/kubelet/dockertools/docker.go

@@ -173,6 +173,7 @@ func (d *dockerContainerCommandRunner) runInContainerUsingNsinit(containerID str
 }
 
 // RunInContainer uses nsinit to run the command inside the container identified by containerID
+// TODO(yifan): Use strong type for containerID.
 func (d *dockerContainerCommandRunner) RunInContainer(containerID string, cmd []string) ([]byte, error) {
 	// If native exec support does not exist in the local docker daemon use nsinit.
 	useNativeExec, err := d.nativeExecSupportExists()
@@ -217,6 +218,7 @@ func (d *dockerContainerCommandRunner) RunInContainer(containerID string, cmd []
 //  - match cgroups of container
 //  - should we support `docker exec`?
 //  - should we support nsenter in a container, running with elevated privs and --pid=host?
+//  - use strong type for containerId
 func (d *dockerContainerCommandRunner) ExecInContainer(containerId string, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool) error {
 	container, err := d.client.InspectContainer(containerId)
 	if err != nil {
@@ -303,8 +305,12 @@ func (d *dockerContainerCommandRunner) ExecInContainer(containerId string, cmd [
 //  - match cgroups of container
 //  - should we support nsenter + socat on the host? (current impl)
 //  - should we support nsenter + socat in a container, running with elevated privs and --pid=host?
-func (d *dockerContainerCommandRunner) PortForward(podInfraContainerID string, port uint16, stream io.ReadWriteCloser) error {
+func (d *dockerContainerCommandRunner) PortForward(pod *kubecontainer.Pod, port uint16, stream io.ReadWriteCloser) error {
-	container, err := d.client.InspectContainer(podInfraContainerID)
+	podInfraContainer := pod.FindContainerByName(PodInfraContainerName)
+	if podInfraContainer == nil {
+		return fmt.Errorf("cannot find pod infra container in pod %q", kubecontainer.BuildPodFullName(pod.Name, pod.Namespace))
+	}
+	container, err := d.client.InspectContainer(string(podInfraContainer.ID))
 	if err != nil {
 		return err
 	}
@@ -527,11 +533,12 @@ func ConnectToDockerOrDie(dockerEndpoint string) DockerInterface {
 	return client
 }
 
+// TODO(yifan): Move this to container.Runtime.
 type ContainerCommandRunner interface {
 	RunInContainer(containerID string, cmd []string) ([]byte, error)
 	GetDockerServerVersion() ([]uint, error)
 	ExecInContainer(containerID string, cmd []string, in io.Reader, out, err io.WriteCloser, tty bool) error
-	PortForward(podInfraContainerID string, port uint16, stream io.ReadWriteCloser) error
+	PortForward(pod *kubecontainer.Pod, port uint16, stream io.ReadWriteCloser) error
 }
 
 func milliCPUToShares(milliCPU int64) int64 {

pkg/kubelet/kubelet.go

@@ -1993,60 +1993,68 @@ func (kl *Kubelet) ServeLogs(w http.ResponseWriter, req *http.Request) {
 	kl.logServer.ServeHTTP(w, req)
 }
 
+// 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 GetPods() has the same signature as the runtime.GetPods().
+func (kl *Kubelet) findContainer(podFullName string, podUID types.UID, containerName string) (*kubecontainer.Container, error) {
+	pods, err := dockertools.GetPods(kl.dockerClient, false)
+	if err != nil {
+		return nil, err
+	}
+	pod := kubecontainer.Pods(pods).FindPod(podFullName, podUID)
+	return pod.FindContainerByName(containerName), nil
+}
+
 // Run a command in a container, returns the combined stdout, stderr as an array of bytes
-func (kl *Kubelet) RunInContainer(podFullName string, uid types.UID, container string, cmd []string) ([]byte, error) {
+func (kl *Kubelet) RunInContainer(podFullName string, podUID types.UID, containerName string, cmd []string) ([]byte, error) {
-	uid = kl.podManager.TranslatePodUID(uid)
+	podUID = kl.podManager.TranslatePodUID(podUID)
 
 	if kl.runner == nil {
 		return nil, fmt.Errorf("no runner specified.")
 	}
-	dockerContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient, false)
+	container, err := kl.findContainer(podFullName, podUID, containerName)
 	if err != nil {
 		return nil, err
 	}
-	dockerContainer, found, _ := dockerContainers.FindPodContainer(podFullName, uid, container)
+	if container == nil {
-	if !found {
+		return nil, fmt.Errorf("container not found (%q)", containerName)
-		return nil, fmt.Errorf("container not found (%q)", container)
 	}
-	return kl.runner.RunInContainer(dockerContainer.ID, cmd)
+	return kl.runner.RunInContainer(string(container.ID), cmd)
 }
 
 // ExecInContainer executes a command in a container, connecting the supplied
 // stdin/stdout/stderr to the command's IO streams.
-func (kl *Kubelet) ExecInContainer(podFullName string, uid types.UID, container string, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool) error {
+func (kl *Kubelet) ExecInContainer(podFullName string, podUID types.UID, containerName string, cmd []string, stdin io.Reader, stdout, stderr io.WriteCloser, tty bool) error {
-	uid = kl.podManager.TranslatePodUID(uid)
+	podUID = kl.podManager.TranslatePodUID(podUID)
 
 	if kl.runner == nil {
 		return fmt.Errorf("no runner specified.")
 	}
-	dockerContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient, false)
+	container, err := kl.findContainer(podFullName, podUID, containerName)
 	if err != nil {
 		return err
 	}
-	dockerContainer, found, _ := dockerContainers.FindPodContainer(podFullName, uid, container)
+	if container == nil {
-	if !found {
+		return fmt.Errorf("container not found (%q)", containerName)
-		return fmt.Errorf("container not found (%q)", container)
 	}
-	return kl.runner.ExecInContainer(dockerContainer.ID, cmd, stdin, stdout, stderr, tty)
+	return kl.runner.ExecInContainer(string(container.ID), cmd, stdin, stdout, stderr, tty)
 }
 
 // PortForward connects to the pod's port and copies data between the port
 // and the stream.
-func (kl *Kubelet) PortForward(podFullName string, uid types.UID, port uint16, stream io.ReadWriteCloser) error {
+func (kl *Kubelet) PortForward(podFullName string, podUID types.UID, port uint16, stream io.ReadWriteCloser) error {
-	uid = kl.podManager.TranslatePodUID(uid)
+	podUID = kl.podManager.TranslatePodUID(podUID)
 
 	if kl.runner == nil {
 		return fmt.Errorf("no runner specified.")
 	}
-	dockerContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient, false)
+
+	pods, err := dockertools.GetPods(kl.dockerClient, false)
 	if err != nil {
 		return err
 	}
-	podInfraContainer, found, _ := dockerContainers.FindPodContainer(podFullName, uid, dockertools.PodInfraContainerName)
+	pod := kubecontainer.Pods(pods).FindPod(podFullName, podUID)
-	if !found {
+	return kl.runner.PortForward(&pod, port, stream)
-		return fmt.Errorf("Unable to find pod infra container for pod %q, uid %v", podFullName, uid)
-	}
-	return kl.runner.PortForward(podInfraContainer.ID, port, stream)
 }
 
 // BirthCry sends an event that the kubelet has started up.

pkg/kubelet/kubelet_test.go

@@ -1425,8 +1425,12 @@ func (f *fakeContainerCommandRunner) ExecInContainer(id string, cmd []string, in
 	return f.E
 }
 
-func (f *fakeContainerCommandRunner) PortForward(podInfraContainerID string, port uint16, stream io.ReadWriteCloser) error {
+func (f *fakeContainerCommandRunner) PortForward(pod *kubecontainer.Pod, port uint16, stream io.ReadWriteCloser) error {
-	f.ID = podInfraContainerID
+	podInfraContainer := pod.FindContainerByName(dockertools.PodInfraContainerName)
+	if podInfraContainer == nil {
+		return fmt.Errorf("cannot find pod infra container in pod %q", kubecontainer.BuildPodFullName(pod.Name, pod.Namespace))
+	}
+	f.ID = string(podInfraContainer.ID)
 	f.Port = port
 	f.Stream = stream
 	return nil