kubelet: Refactor computePodContainerChanges().

Pull generatePodStatus() and makePodDataDirs() out as they are the
common part for container runtimes.

pkg/kubelet/kubelet.go

@@ -1060,20 +1060,16 @@ type podContainerChangesSpec struct {
 	containersToKeep    map[dockertools.DockerID]int
 }
 
-func (kl *Kubelet) computePodContainerChanges(pod *api.Pod, runningPod kubecontainer.Pod) (podContainerChangesSpec, error) {
+func (kl *Kubelet) computePodContainerChanges(pod *api.Pod, runningPod kubecontainer.Pod, podStatus api.PodStatus) (podContainerChangesSpec, error) {
 	podFullName := kubecontainer.GetPodFullName(pod)
 	uid := pod.UID
 	glog.V(4).Infof("Syncing Pod %+v, podFullName: %q, uid: %q", pod, podFullName, uid)
 
-	err := kl.makePodDataDirs(pod)
-	if err != nil {
-		return podContainerChangesSpec{}, err
-	}
-
 	containersToStart := make(map[int]empty)
 	containersToKeep := make(map[dockertools.DockerID]int)
 	createPodInfraContainer := false
 
+	var err error
 	var podInfraContainerID dockertools.DockerID
 	var changed bool
 	podInfraContainer := runningPod.FindContainerByName(dockertools.PodInfraContainerName)
@@ -1097,18 +1093,6 @@ func (kl *Kubelet) computePodContainerChanges(pod *api.Pod, runningPod kubeconta
 		containersToKeep[podInfraContainerID] = -1
 	}
 
-	// Do not use the cache here since we need the newest status to check
-	// if we need to restart the container below.
-	pod, found := kl.GetPodByFullName(podFullName)
-	if !found {
-		return podContainerChangesSpec{}, fmt.Errorf("couldn't find pod %q", podFullName)
-	}
-	podStatus, err := kl.generatePodStatus(pod)
-	if err != nil {
-		glog.Errorf("Unable to get pod with name %q and uid %q info with error(%v)", podFullName, uid, err)
-		return podContainerChangesSpec{}, err
-	}
-
 	for index, container := range pod.Spec.Containers {
 		expectedHash := dockertools.HashContainer(&container)
 
@@ -1213,7 +1197,18 @@ func (kl *Kubelet) syncPod(pod *api.Pod, mirrorPod *api.Pod, runningPod kubecont
 		return err
 	}
 
-	containerChanges, err := kl.computePodContainerChanges(pod, runningPod)
+	if err := kl.makePodDataDirs(pod); err != nil {
+		glog.Errorf("Unable to make pod data directories for pod %q (uid %q): %v", podFullName, uid, err)
+		return err
+	}
+
+	podStatus, err := kl.generatePodStatus(pod)
+	if err != nil {
+		glog.Errorf("Unable to get status for pod %q (uid %q): %v", podFullName, uid, err)
+		return err
+	}
+
+	containerChanges, err := kl.computePodContainerChanges(pod, runningPod, podStatus)
 	glog.V(3).Infof("Got container changes for pod %q: %+v", podFullName, containerChanges)
 	if err != nil {
 		return err

pkg/kubelet/kubelet_test.go

@@ -511,10 +511,10 @@ func TestSyncPodsDoesNothing(t *testing.T) {
 	waitGroup.Wait()
 	verifyCalls(t, fakeDocker, []string{
 		"list", "list",
-		// Check the pod infra contianer.
-		"inspect_container",
 		// Get pod status.
 		"list", "inspect_container", "inspect_container",
+		// Check the pod infra contianer.
+		"inspect_container",
 		// Get pod status.
 		"list", "inspect_container", "inspect_container"})
 }
@@ -743,10 +743,10 @@ func TestSyncPodsWithPodInfraCreatesContainer(t *testing.T) {
 
 	verifyCalls(t, fakeDocker, []string{
 		"list", "list",
-		// Check the pod infra container.
-		"inspect_container",
 		// Get pod status.
 		"list", "inspect_container", "inspect_image",
+		// Check the pod infra container.
+		"inspect_container",
 		// Create container.
 		"create", "start",
 		// Get pod status.
@@ -818,10 +818,10 @@ func TestSyncPodsWithPodInfraCreatesContainerCallsHandler(t *testing.T) {
 
 	verifyCalls(t, fakeDocker, []string{
 		"list", "list",
-		// Check the pod infra container.
-		"inspect_container",
 		// Get pod status.
 		"list", "inspect_container", "inspect_image",
+		// Check the pod infra container.
+		"inspect_container",
 		// Create container.
 		"create", "start",
 		// Get pod status.
@@ -1104,10 +1104,10 @@ func TestSyncPodsDeletesDuplicate(t *testing.T) {
 
 	verifyCalls(t, fakeDocker, []string{
 		"list", "list",
-		// Check the pod infra container.
-		"inspect_container",
 		// Get pod status.
 		"list", "inspect_container", "inspect_container", "inspect_container",
+		// Check the pod infra container.
+		"inspect_container",
 		// Kill the duplicated container.
 		"stop",
 		// Get pod status.
@@ -1175,10 +1175,10 @@ func TestSyncPodsBadHash(t *testing.T) {
 
 	verifyCalls(t, fakeDocker, []string{
 		"list", "list",
-		// Check the pod infra container.
-		"inspect_container",
 		// Get pod status.
 		"list", "inspect_container", "inspect_container",
+		// Check the pod infra container.
+		"inspect_container",
 		// Kill and restart the bad hash container.
 		"stop", "create", "start",
 		// Get pod status.
@@ -1249,10 +1249,10 @@ func TestSyncPodsUnhealthy(t *testing.T) {
 
 	verifyCalls(t, fakeDocker, []string{
 		"list", "list",
-		// Check the pod infra container.
-		"inspect_container",
 		// Get pod status.
 		"list", "inspect_container", "inspect_container",
+		// Check the pod infra container.
+		"inspect_container",
 		// Kill the unhealthy container.
 		"stop",
 		// Restart the unhealthy container.
@@ -1868,10 +1868,10 @@ func TestSyncPodEventHandlerFails(t *testing.T) {
 
 	verifyCalls(t, fakeDocker, []string{
 		"list", "list",
-		// Check the pod infra container.
-		"inspect_container",
 		// Get pod status.
 		"list", "inspect_container", "inspect_image",
+		// Check the pod infra container.
+		"inspect_container",
 		// Create the container.
 		"create", "start",
 		// Kill the container since event handler fails.
@@ -3871,10 +3871,10 @@ func TestSyncPodsWithRestartPolicy(t *testing.T) {
 		{
 			api.RestartPolicyAlways,
 			[]string{"list", "list",
-				// Check the pod infra container.
-				"inspect_container",
 				// Get pod status.
 				"list", "inspect_container", "inspect_container", "inspect_container",
+				// Check the pod infra container.
+				"inspect_container",
 				// Restart both containers.
 				"create", "start", "create", "start",
 				// Get pod status.
@@ -3885,10 +3885,10 @@ func TestSyncPodsWithRestartPolicy(t *testing.T) {
 		{
 			api.RestartPolicyOnFailure,
 			[]string{"list", "list",
-				// Check the pod infra container.
-				"inspect_container",
 				// Get pod status.
 				"list", "inspect_container", "inspect_container", "inspect_container",
+				// Check the pod infra container.
+				"inspect_container",
 				// Restart the failed container.
 				"create", "start",
 				// Get pod status.
@@ -3899,10 +3899,10 @@ func TestSyncPodsWithRestartPolicy(t *testing.T) {
 		{
 			api.RestartPolicyNever,
 			[]string{"list", "list",
-				// Check the pod infra container.
-				"inspect_container",
 				// Get pod status.
 				"list", "inspect_container", "inspect_container", "inspect_container",
+				// Check the pod infra container.
+				"inspect_container",
 				// Stop the last pod infra container.
 				"stop",
 				// Get pod status.