runonce: list kubelet container between each iteration

pkg/kubelet/runonce.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools"
 	"github.com/golang/glog"
 )
@@ -34,7 +33,6 @@ const (
 
 type RunPodResult struct {
 	Pod *Pod
-	Info api.PodInfo
 	Err error
 }
 
@@ -62,8 +60,8 @@ func (kl *Kubelet) runOnce(pods []Pod) (results []RunPodResult, err error) {
 	for i := range pods {
 		pod := pods[i] // Make a copy
 		go func() {
-			info, err := kl.runPod(pod)
+			err := kl.runPod(pod)
-			ch <- RunPodResult{&pod, info, err}
+			ch <- RunPodResult{&pod, err}
 		}()
 	}
 
@@ -77,7 +75,7 @@ func (kl *Kubelet) runOnce(pods []Pod) (results []RunPodResult, err error) {
 			glog.Infof("failed to start pod %q: %v", res.Pod.Name, res.Err)
 			failedPods = append(failedPods, res.Pod.Name)
 		} else {
-			glog.Infof("started pod %q: %#v", res.Pod.Name, res.Info)
+			glog.Infof("started pod %q", res.Pod.Name)
 		}
 	}
 	if len(failedPods) > 0 {
@@ -87,31 +85,29 @@ func (kl *Kubelet) runOnce(pods []Pod) (results []RunPodResult, err error) {
 	return results, err
 }
 
-// Run a single pod and wait until all containers are running.
+// runPod runs a single pod and wait until all containers are running.
-func (kl *Kubelet) runPod(pod Pod) (api.PodInfo, error) {
+func (kl *Kubelet) runPod(pod Pod) error {
-	dockerContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient, false)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get kubelet docker containers: %v", err)
-	}
-
 	delay := RunOnceRetryDelay
 	retry := 0
 	for {
-		running, info, err := kl.isPodRunning(pod)
+		dockerContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient, false)
 		if err != nil {
-			return nil, fmt.Errorf("error checking pod status: %v", err)
+			return fmt.Errorf("failed to get kubelet docker containers: %v", err)
+		}
+		running, err := kl.isPodRunning(pod, dockerContainers)
+		if err != nil {
+			return fmt.Errorf("error checking pod status: %v", err)
 		}
 		if running {
 			glog.Infof("pod %q containers running", pod.Name)
-			return info, nil
+			return nil
 		}
 		glog.Infof("pod %q containers not running: syncing", pod.Name)
-		err = kl.syncPod(&pod, dockerContainers)
+		if err = kl.syncPod(&pod, dockerContainers); err != nil {
-		if err != nil {
+			return fmt.Errorf("error syncing pod: %v", err)
-			return nil, fmt.Errorf("error syncing pod: %v", err)
 		}
 		if retry >= RunOnceMaxRetries {
-			return info, fmt.Errorf("timeout error: pod %q containers not running after %d retries", pod.Name, RunOnceMaxRetries)
+			return fmt.Errorf("timeout error: pod %q containers not running after %d retries", pod.Name, RunOnceMaxRetries)
 		}
 		// TODO(proppy): health checking would be better than waiting + checking the state at the next iteration.
 		glog.Infof("pod %q containers synced, waiting for %v", pod.Name, delay)
@@ -121,31 +117,13 @@ func (kl *Kubelet) runPod(pod Pod) (api.PodInfo, error) {
 	}
 }
 
-// Check if all containers of a manifest are running.
+// isPodRunning returns true if all containers of a manifest are running.
-func (kl *Kubelet) isPodRunning(pod Pod) (bool, api.PodInfo, error) {
+func (kl *Kubelet) isPodRunning(pod Pod, dockerContainers dockertools.DockerContainers) (bool, error) {
-	info, err := kl.GetPodInfo(GetPodFullName(&pod), pod.Manifest.UUID)
-	if err != nil {
-		return false, nil, fmt.Errorf("error getting pod info: %v", err)
-	}
 	for _, container := range pod.Manifest.Containers {
-		running := podInfo(info).isContainerRunning(container)
+		if dockerContainer, found, _ := dockerContainers.FindPodContainer(GetPodFullName(&pod), pod.Manifest.UUID, container.Name); !found || dockerContainer.Status != "running" {
-		glog.Infof("container %q running: %v", container.Name, running)
+			glog.Infof("container %q not found (%v) or not running: %#v", container.Name, found, dockerContainer)
-		if !running {
+			return false, nil
-			return false, info, nil
 		}
 	}
-	return true, info, nil
+	return true, nil
-}
-
-// Alias PodInfo for internal usage.
-type podInfo api.PodInfo
-
-func (info podInfo) isContainerRunning(container api.Container) bool {
-	for name, status := range info {
-		glog.Infof("container %q status: %#v", name, status)
-		if name == container.Name && status.State.Running != nil {
-			return true
-		}
-	}
-	return false
 }

pkg/kubelet/runonce_test.go

@@ -67,22 +67,25 @@ func (d *testDocker) InspectContainer(id string) (*docker.Container, error) {
 
 func TestRunOnce(t *testing.T) {
 	kb := &Kubelet{}
-	container := api.Container{Name: "bar"}
+	podContainers := []docker.APIContainers{
-	kb.dockerClient = &testDocker{
-		listContainersResults: []listContainersResult{
-			{label: "pre syncPod", containers: []docker.APIContainers{}},
-			{label: "syncPod #1", containers: []docker.APIContainers{}},
-			{label: "syncPod #2", containers: []docker.APIContainers{}},
-			{label: "post syncPod", containers: []docker.APIContainers{
 		{
-					Names: []string{"/k8s_bar." + strconv.FormatUint(dockertools.HashContainer(&container), 16) + "_foo.test"},
+			Names:  []string{"/k8s_bar." + strconv.FormatUint(dockertools.HashContainer(&api.Container{Name: "bar"}), 16) + "_foo.test"},
 			ID:     "1234",
+			Status: "running",
 		},
 		{
 			Names:  []string{"/k8s_net_foo.test_"},
 			ID:     "9876",
+			Status: "running",
 		},
-			}},
+	}
+	kb.dockerClient = &testDocker{
+		listContainersResults: []listContainersResult{
+			{label: "list pod container", containers: []docker.APIContainers{}},
+			{label: "syncPod", containers: []docker.APIContainers{}},
+			{label: "list pod container", containers: []docker.APIContainers{}},
+			{label: "syncPod", containers: podContainers},
+			{label: "list pod container", containers: podContainers},
 		},
 		inspectContainersResults: []inspectContainersResult{
 			{