From 062b9f86b92436082ea98a3af81aa7cfae55dd28 Mon Sep 17 00:00:00 2001
From: Janet Kuo <chiachenk@google.com>
Date: Wed, 24 Feb 2016 10:59:51 -0800
Subject: [PATCH] Fix the incorrect deployment.status.unavailableReplicas

---
 pkg/controller/deployment/deployment_controller.go | 13 +++++++------
 pkg/util/deployment/deployment.go                  | 14 ++++++++++----
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go
index 1476a8701fe..2172ef9c2a7 100644
--- a/pkg/controller/deployment/deployment_controller.go
+++ b/pkg/controller/deployment/deployment_controller.go
@@ -578,11 +578,11 @@ func (dc *DeploymentController) syncRollingUpdateDeployment(deployment extension
 
 // syncDeploymentStatus checks if the status is up-to-date and sync it if necessary
 func (dc *DeploymentController) syncDeploymentStatus(allRSs []*extensions.ReplicaSet, newRS *extensions.ReplicaSet, d extensions.Deployment) error {
-	totalReplicas, updatedReplicas, availableReplicas, _, err := dc.calculateStatus(allRSs, newRS, d)
+	totalActualReplicas, updatedReplicas, availableReplicas, _, err := dc.calculateStatus(allRSs, newRS, d)
 	if err != nil {
 		return err
 	}
-	if d.Generation > d.Status.ObservedGeneration || d.Status.Replicas != totalReplicas || d.Status.UpdatedReplicas != updatedReplicas || d.Status.AvailableReplicas != availableReplicas {
+	if d.Generation > d.Status.ObservedGeneration || d.Status.Replicas != totalActualReplicas || d.Status.UpdatedReplicas != updatedReplicas || d.Status.AvailableReplicas != availableReplicas {
 		return dc.updateDeploymentStatus(allRSs, newRS, d)
 	}
 	return nil
@@ -1029,7 +1029,7 @@ func (dc *DeploymentController) cleanupOldReplicaSets(oldRSs []*extensions.Repli
 }
 
 func (dc *DeploymentController) updateDeploymentStatus(allRSs []*extensions.ReplicaSet, newRS *extensions.ReplicaSet, deployment extensions.Deployment) error {
-	totalReplicas, updatedReplicas, availableReplicas, unavailableReplicas, err := dc.calculateStatus(allRSs, newRS, deployment)
+	totalActualReplicas, updatedReplicas, availableReplicas, unavailableReplicas, err := dc.calculateStatus(allRSs, newRS, deployment)
 	if err != nil {
 		return err
 	}
@@ -1038,7 +1038,7 @@ func (dc *DeploymentController) updateDeploymentStatus(allRSs []*extensions.Repl
 	newDeployment.Status = extensions.DeploymentStatus{
 		// TODO: Ensure that if we start retrying status updates, we won't pick up a new Generation value.
 		ObservedGeneration:  deployment.Generation,
-		Replicas:            totalReplicas,
+		Replicas:            totalActualReplicas,
 		UpdatedReplicas:     updatedReplicas,
 		AvailableReplicas:   availableReplicas,
 		UnavailableReplicas: unavailableReplicas,
@@ -1047,8 +1047,8 @@ func (dc *DeploymentController) updateDeploymentStatus(allRSs []*extensions.Repl
 	return err
 }
 
-func (dc *DeploymentController) calculateStatus(allRSs []*extensions.ReplicaSet, newRS *extensions.ReplicaSet, deployment extensions.Deployment) (totalReplicas, updatedReplicas, availableReplicas, unavailableReplicas int, err error) {
-	totalReplicas = deploymentutil.GetActualReplicaCountForReplicaSets(allRSs)
+func (dc *DeploymentController) calculateStatus(allRSs []*extensions.ReplicaSet, newRS *extensions.ReplicaSet, deployment extensions.Deployment) (totalActualReplicas, updatedReplicas, availableReplicas, unavailableReplicas int, err error) {
+	totalActualReplicas = deploymentutil.GetActualReplicaCountForReplicaSets(allRSs)
 	updatedReplicas = deploymentutil.GetActualReplicaCountForReplicaSets([]*extensions.ReplicaSet{newRS})
 	minReadySeconds := deployment.Spec.MinReadySeconds
 	availableReplicas, err = deploymentutil.GetAvailablePodsForReplicaSets(dc.client, allRSs, minReadySeconds)
@@ -1056,6 +1056,7 @@ func (dc *DeploymentController) calculateStatus(allRSs []*extensions.ReplicaSet,
 		err = fmt.Errorf("failed to count available pods: %v", err)
 		return
 	}
+	totalReplicas := deploymentutil.GetReplicaCountForReplicaSets(allRSs)
 	unavailableReplicas = totalReplicas - availableReplicas
 	return
 }
diff --git a/pkg/util/deployment/deployment.go b/pkg/util/deployment/deployment.go
index 77e8cc709c8..b673cdedf14 100644
--- a/pkg/util/deployment/deployment.go
+++ b/pkg/util/deployment/deployment.go
@@ -377,7 +377,7 @@ func IsPodAvailable(pod *api.Pod, minReadySeconds int) bool {
 }
 
 func GetPodsForReplicaSets(c clientset.Interface, replicaSets []*extensions.ReplicaSet) ([]api.Pod, error) {
-	allPods := []api.Pod{}
+	allPods := map[string]api.Pod{}
 	for _, rs := range replicaSets {
 		selector, err := unversioned.LabelSelectorAsSelector(rs.Spec.Selector)
 		if err != nil {
@@ -386,11 +386,17 @@ func GetPodsForReplicaSets(c clientset.Interface, replicaSets []*extensions.Repl
 		options := api.ListOptions{LabelSelector: selector}
 		podList, err := c.Core().Pods(rs.ObjectMeta.Namespace).List(options)
 		if err != nil {
-			return allPods, fmt.Errorf("error listing pods: %v", err)
+			return nil, fmt.Errorf("error listing pods: %v", err)
+		}
+		for _, pod := range podList.Items {
+			allPods[pod.Name] = pod
 		}
-		allPods = append(allPods, podList.Items...)
 	}
-	return allPods, nil
+	requiredPods := []api.Pod{}
+	for _, pod := range allPods {
+		requiredPods = append(requiredPods, pod)
+	}
+	return requiredPods, nil
 }
 
 // Revision returns the revision number of the input replica set