Add unit test for get old rcs

pkg/util/deployment/deployment.go

@@ -60,11 +60,8 @@ func GetOldRCs(deployment extensions.Deployment, c client.Interface) ([]*api.Rep
 		}
 	}
 	requiredRCs := []*api.ReplicationController{}
-	// Note that go reuses the same memory location for every iteration,
-	// which means the 'value' returned from range will have the same address.
-	// Therefore, we should use the returned 'index' instead.
-	for i := range oldRCs {
-		value := oldRCs[i]
+	for key := range oldRCs {
+		value := oldRCs[key]
 		requiredRCs = append(requiredRCs, &value)
 	}
 	return requiredRCs, nil

pkg/util/deployment/deployment_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package deployment
 
 import (
+	"reflect"
 	"testing"
 	"time"
 
@@ -24,7 +25,9 @@ import (
 	"k8s.io/kubernetes/pkg/api/testapi"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/apis/extensions"
+	"k8s.io/kubernetes/pkg/client/unversioned/testclient"
 	"k8s.io/kubernetes/pkg/client/unversioned/testclient/simple"
+	"k8s.io/kubernetes/pkg/runtime"
 )
 
 func newPod(now time.Time, ready bool, beforeSec int) api.Pod {
@@ -79,10 +82,65 @@ func TestGetReadyPodsCount(t *testing.T) {
 	}
 }
 
+// generatePodFromRC creates a pod, with the input rc's selector and its template
+func generatePodFromRC(rc api.ReplicationController) api.Pod {
+	return api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			Labels: rc.Spec.Selector,
+		},
+		Spec: rc.Spec.Template.Spec,
+	}
+}
+
+func generatePod(labels map[string]string, image string) api.Pod {
+	return api.Pod{
+		ObjectMeta: api.ObjectMeta{
+			Labels: labels,
+		},
+		Spec: api.PodSpec{
+			Containers: []api.Container{
+				{
+					Name:                   image,
+					Image:                  image,
+					ImagePullPolicy:        api.PullAlways,
+					TerminationMessagePath: api.TerminationMessagePathDefault,
+				},
+			},
+		},
+	}
+}
+
+func generateRCWithLabel(labels map[string]string, image string) api.ReplicationController {
+	return api.ReplicationController{
+		ObjectMeta: api.ObjectMeta{
+			Name:   api.SimpleNameGenerator.GenerateName("rc"),
+			Labels: labels,
+		},
+		Spec: api.ReplicationControllerSpec{
+			Replicas: 1,
+			Selector: labels,
+			Template: &api.PodTemplateSpec{
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{
+							Name:                   image,
+							Image:                  image,
+							ImagePullPolicy:        api.PullAlways,
+							TerminationMessagePath: api.TerminationMessagePathDefault,
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+// generateRC creates a replication controller, with the input deployment's template as its template
 func generateRC(deployment extensions.Deployment) api.ReplicationController {
 	template := GetNewRCTemplate(deployment)
 	return api.ReplicationController{
 		ObjectMeta: api.ObjectMeta{
+			Name:   api.SimpleNameGenerator.GenerateName("rc"),
 			Labels: template.Labels,
 		},
 		Spec: api.ReplicationControllerSpec{
@@ -92,6 +150,7 @@ func generateRC(deployment extensions.Deployment) api.ReplicationController {
 	}
 }
 
+// generateDeployment creates a deployment, with the input image as its template
 func generateDeployment(image string) extensions.Deployment {
 	podLabels := map[string]string{"name": image}
 	terminationSec := int64(30)
@@ -181,3 +240,106 @@ func TestGetNewRC(t *testing.T) {
 		}
 	}
 }
+
+func TestGetOldRCs(t *testing.T) {
+	newDeployment := generateDeployment("nginx")
+	newRC := generateRC(newDeployment)
+	newPod := generatePodFromRC(newRC)
+
+	// create 2 old deployments and related rcs/pods, with the same labels but different template
+	oldDeployment := generateDeployment("nginx")
+	oldDeployment.Spec.Template.Spec.Containers[0].Name = "nginx-old-1"
+	oldRC := generateRC(oldDeployment)
+	oldPod := generatePodFromRC(oldRC)
+	oldDeployment2 := generateDeployment("nginx")
+	oldDeployment2.Spec.Template.Spec.Containers[0].Name = "nginx-old-2"
+	oldRC2 := generateRC(oldDeployment2)
+	oldPod2 := generatePodFromRC(oldRC2)
+
+	// create 1 rc that existed before the deployment, with the same labels as the deployment
+	existedPod := generatePod(newDeployment.Spec.Selector, "foo")
+	existedRC := generateRCWithLabel(newDeployment.Spec.Selector, "foo")
+
+	tests := []struct {
+		test     string
+		objs     []runtime.Object
+		expected []*api.ReplicationController
+	}{
+		{
+			"No old RCs",
+			[]runtime.Object{
+				&api.PodList{
+					Items: []api.Pod{
+						generatePod(newDeployment.Spec.Selector, "foo"),
+						generatePod(newDeployment.Spec.Selector, "bar"),
+						newPod,
+					},
+				},
+				&api.ReplicationControllerList{
+					Items: []api.ReplicationController{
+						generateRC(generateDeployment("foo")),
+						newRC,
+						generateRC(generateDeployment("bar")),
+					},
+				},
+			},
+			[]*api.ReplicationController{},
+		},
+		{
+			"Has old RC",
+			[]runtime.Object{
+				&api.PodList{
+					Items: []api.Pod{
+						oldPod,
+						oldPod2,
+						generatePod(map[string]string{"name": "bar"}, "bar"),
+						generatePod(map[string]string{"name": "xyz"}, "xyz"),
+						existedPod,
+						generatePod(newDeployment.Spec.Selector, "abc"),
+					},
+				},
+				&api.ReplicationControllerList{
+					Items: []api.ReplicationController{
+						oldRC2,
+						oldRC,
+						existedRC,
+						newRC,
+						generateRCWithLabel(map[string]string{"name": "xyz"}, "xyz"),
+						generateRCWithLabel(map[string]string{"name": "bar"}, "bar"),
+					},
+				},
+			},
+			[]*api.ReplicationController{&oldRC, &oldRC2, &existedRC},
+		},
+	}
+
+	for _, test := range tests {
+		rcs, err := GetOldRCs(newDeployment, testclient.NewSimpleFake(test.objs...))
+		if err != nil {
+			t.Errorf("In test case %s, got unexpected error %v", test.test, err)
+		}
+		if !equal(rcs, test.expected) {
+			t.Errorf("In test case %q, expected %v, got %v", test.test, test.expected, rcs)
+		}
+	}
+}
+
+// equal compares the equality of two rc slices regardless of their ordering
+func equal(rcs1, rcs2 []*api.ReplicationController) bool {
+	if reflect.DeepEqual(rcs1, rcs2) {
+		return true
+	}
+	if rcs1 == nil || rcs2 == nil || len(rcs1) != len(rcs2) {
+		return false
+	}
+	count := 0
+	for _, rc1 := range rcs1 {
+		for _, rc2 := range rcs2 {
+			if reflect.DeepEqual(rc1, rc2) {
+				count++
+				break
+			}
+		}
+	}
+	return count == len(rcs1)
+}