Address comments; fix test failure

pkg/util/deployment/deployment.go

@@ -191,7 +191,7 @@ func addHashKeyToRSAndPods(deployment extensions.Deployment, c clientset.Interfa
 		}
 	}
 	// Make sure rs pod template is updated so that it won't create pods without the new label (orphaned pods).
-	if updatedRS.Generation != updatedRS.Status.ObservedGeneration {
+	if updatedRS.Generation > updatedRS.Status.ObservedGeneration {
 		if err = waitForReplicaSetUpdated(c, updatedRS.Generation, namespace, rs.Name); err != nil {
 			return nil, err
 		}

pkg/util/deployment/deployment_test.go

@@ -22,14 +22,11 @@ import (
 	"time"
 
 	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/kubernetes/pkg/api/testapi"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/apis/extensions"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
 	"k8s.io/kubernetes/pkg/client/testing/core"
-	"k8s.io/kubernetes/pkg/client/testing/fake"
 	"k8s.io/kubernetes/pkg/client/unversioned/testclient"
-	"k8s.io/kubernetes/pkg/client/unversioned/testclient/simple"
 	"k8s.io/kubernetes/pkg/runtime"
 )
 
@@ -223,22 +220,27 @@ func TestGetNewRC(t *testing.T) {
 
 	tests := []struct {
 		test     string
-		rsList   extensions.ReplicaSetList
+		objs     []runtime.Object
 		expected *extensions.ReplicaSet
 	}{
 		{
 			"No new ReplicaSet",
-			extensions.ReplicaSetList{
+			[]runtime.Object{
+				&api.PodList{},
+				&extensions.ReplicaSetList{
 					Items: []extensions.ReplicaSet{
 						generateRS(generateDeployment("foo")),
 						generateRS(generateDeployment("bar")),
 					},
 				},
+			},
 			nil,
 		},
 		{
 			"Has new ReplicaSet",
-			extensions.ReplicaSetList{
+			[]runtime.Object{
+				&api.PodList{},
+				&extensions.ReplicaSetList{
 					Items: []extensions.ReplicaSet{
 						generateRS(generateDeployment("foo")),
 						generateRS(generateDeployment("bar")),
@@ -247,23 +249,18 @@ func TestGetNewRC(t *testing.T) {
 						generateRS(generateDeployment("xyz")),
 					},
 				},
+			},
 			&newRC,
 		},
 	}
 
-	ns := api.NamespaceDefault
 	for _, test := range tests {
-		c := &simple.Client{
-			Request: simple.Request{
-				Method: "GET",
-				Path:   testapi.Default.ResourcePath("replicaSets", ns, ""),
-			},
-			Response: simple.Response{
-				StatusCode: 200,
-				Body:       &test.rsList,
-			},
-		}
-		rs, err := GetNewReplicaSet(newDeployment, c.Setup(t).Clientset)
+		fakeClient := &fake.Clientset{}
+		fakeClient = addListPodsReactor(fakeClient, test.objs[0])
+		fakeClient = addListRSReactor(fakeClient, test.objs[1])
+		fakeClient = addUpdatePodsReactor(fakeClient)
+		fakeClient = addUpdateRSReactor(fakeClient)
+		rs, err := GetNewReplicaSet(newDeployment, fakeClient)
 		if err != nil {
 			t.Errorf("In test case %s, got unexpected error %v", test.test, err)
 		}

test/e2e/deployment.go

@@ -849,6 +849,7 @@ func testDeploymentLabelAdopted(f *Framework) {
 		Expect(c.Extensions().ReplicaSets(ns).Delete(newRS.Name, nil)).NotTo(HaveOccurred())
 	}()
 
+	// The RS and pods should be relabeled before the status is updated by syncRollingUpdateDeployment
 	err = waitForDeploymentStatus(c, ns, deploymentName, replicas, replicas-1, replicas+1, 0)
 	Expect(err).NotTo(HaveOccurred())