Merge pull request #19590 from mqliang/cleanup-policy

Auto commit by PR queue bot

pkg/apis/extensions/types.go

@@ -219,6 +219,10 @@ type DeploymentSpec struct {
 	// The deployment strategy to use to replace existing pods with new ones.
 	Strategy DeploymentStrategy `json:"strategy,omitempty"`
 
+	// The number of old ReplicationControllers to retain to allow rollback.
+	// This is a pointer to distinguish between explicit zero and not specified.
+	RevisionHistoryLimit *int `json:"revisionHistoryLimit,omitempty"`
+
 	// Key of the selector that is added to existing RCs (and label key that is
 	// added to its pods) to prevent the existing RCs to select new pods (and old
 	// pods being selected by new RC).

pkg/apis/extensions/v1beta1/conversion.go

@@ -260,6 +260,10 @@ func Convert_extensions_DeploymentSpec_To_v1beta1_DeploymentSpec(in *extensions.
 	if err := Convert_extensions_DeploymentStrategy_To_v1beta1_DeploymentStrategy(&in.Strategy, &out.Strategy, s); err != nil {
 		return err
 	}
+	if in.RevisionHistoryLimit != nil {
+		out.RevisionHistoryLimit = new(int32)
+		*out.RevisionHistoryLimit = int32(*in.RevisionHistoryLimit)
+	}
 	out.UniqueLabelKey = new(string)
 	*out.UniqueLabelKey = in.UniqueLabelKey
 	out.Paused = in.Paused
@@ -287,6 +291,10 @@ func Convert_v1beta1_DeploymentSpec_To_extensions_DeploymentSpec(in *DeploymentS
 	if err := Convert_v1beta1_DeploymentStrategy_To_extensions_DeploymentStrategy(&in.Strategy, &out.Strategy, s); err != nil {
 		return err
 	}
+	if in.RevisionHistoryLimit != nil {
+		out.RevisionHistoryLimit = new(int)
+		*out.RevisionHistoryLimit = int(*in.RevisionHistoryLimit)
+	}
 	if in.UniqueLabelKey != nil {
 		out.UniqueLabelKey = *in.UniqueLabelKey
 	}

pkg/apis/extensions/v1beta1/conversion_generated.go

@@ -2699,6 +2699,12 @@ func autoConvert_extensions_DeploymentSpec_To_v1beta1_DeploymentSpec(in *extensi
 	if err := Convert_extensions_DeploymentStrategy_To_v1beta1_DeploymentStrategy(&in.Strategy, &out.Strategy, s); err != nil {
 		return err
 	}
+	if in.RevisionHistoryLimit != nil {
+		out.RevisionHistoryLimit = new(int32)
+		*out.RevisionHistoryLimit = int32(*in.RevisionHistoryLimit)
+	} else {
+		out.RevisionHistoryLimit = nil
+	}
 	if err := s.Convert(&in.UniqueLabelKey, &out.UniqueLabelKey, 0); err != nil {
 		return err
 	}
@@ -3819,6 +3825,12 @@ func autoConvert_v1beta1_DeploymentSpec_To_extensions_DeploymentSpec(in *Deploym
 	if err := s.Convert(&in.Strategy, &out.Strategy, 0); err != nil {
 		return err
 	}
+	if in.RevisionHistoryLimit != nil {
+		out.RevisionHistoryLimit = new(int)
+		*out.RevisionHistoryLimit = int(*in.RevisionHistoryLimit)
+	} else {
+		out.RevisionHistoryLimit = nil
+	}
 	// in.UniqueLabelKey has no peer in out
 	out.Paused = in.Paused
 	return nil

pkg/apis/extensions/v1beta1/deep_copy_generated.go

@@ -1147,6 +1147,12 @@ func deepCopy_v1beta1_DeploymentSpec(in DeploymentSpec, out *DeploymentSpec, c *
 	if err := deepCopy_v1beta1_DeploymentStrategy(in.Strategy, &out.Strategy, c); err != nil {
 		return err
 	}
+	if in.RevisionHistoryLimit != nil {
+		out.RevisionHistoryLimit = new(int32)
+		*out.RevisionHistoryLimit = *in.RevisionHistoryLimit
+	} else {
+		out.RevisionHistoryLimit = nil
+	}
 	if in.UniqueLabelKey != nil {
 		out.UniqueLabelKey = new(string)
 		*out.UniqueLabelKey = *in.UniqueLabelKey

pkg/apis/extensions/v1beta1/types.go

@@ -204,6 +204,10 @@ type DeploymentSpec struct {
 	// The deployment strategy to use to replace existing pods with new ones.
 	Strategy DeploymentStrategy `json:"strategy,omitempty"`
 
+	// The number of old ReplicationControllers to retain to allow rollback.
+	// This is a pointer to distinguish between explicit zero and not specified.
+	RevisionHistoryLimit *int32 `json:"revisionHistoryLimit,omitempty"`
+
 	// Key of the selector that is added to existing RCs (and label key that is
 	// added to its pods) to prevent the existing RCs to select new pods (and old
 	// pods being selected by new RC).

pkg/apis/extensions/v1beta1/types_swagger_doc_generated.go

@@ -149,13 +149,14 @@ func (DeploymentList) SwaggerDoc() map[string]string {
 }
 
 var map_DeploymentSpec = map[string]string{
-	"":               "DeploymentSpec is the specification of the desired behavior of the Deployment.",
-	"replicas":       "Number of desired pods. This is a pointer to distinguish between explicit zero and not specified. Defaults to 1.",
-	"selector":       "Label selector for pods. Existing ReplicationControllers whose pods are selected by this will be the ones affected by this deployment.",
-	"template":       "Template describes the pods that will be created.",
-	"strategy":       "The deployment strategy to use to replace existing pods with new ones.",
-	"uniqueLabelKey": "Key of the selector that is added to existing RCs (and label key that is added to its pods) to prevent the existing RCs to select new pods (and old pods being selected by new RC). Users can set this to an empty string to indicate that the system should not add any selector and label. If unspecified, system uses DefaultDeploymentUniqueLabelKey(\"deployment.kubernetes.io/podTemplateHash\"). Value of this key is hash of DeploymentSpec.PodTemplateSpec. No label is added if this is set to empty string.",
-	"paused":         "Indicates that the deployment is paused and will not be processed by the deployment controller.",
+	"":                     "DeploymentSpec is the specification of the desired behavior of the Deployment.",
+	"replicas":             "Number of desired pods. This is a pointer to distinguish between explicit zero and not specified. Defaults to 1.",
+	"selector":             "Label selector for pods. Existing ReplicationControllers whose pods are selected by this will be the ones affected by this deployment.",
+	"template":             "Template describes the pods that will be created.",
+	"strategy":             "The deployment strategy to use to replace existing pods with new ones.",
+	"revisionHistoryLimit": "The number of old ReplicationControllers to retain to allow rollback. This is a pointer to distinguish between explicit zero and not specified.",
+	"uniqueLabelKey":       "Key of the selector that is added to existing RCs (and label key that is added to its pods) to prevent the existing RCs to select new pods (and old pods being selected by new RC). Users can set this to an empty string to indicate that the system should not add any selector and label. If unspecified, system uses DefaultDeploymentUniqueLabelKey(\"deployment.kubernetes.io/podTemplateHash\"). Value of this key is hash of DeploymentSpec.PodTemplateSpec. No label is added if this is set to empty string.",
+	"paused":               "Indicates that the deployment is paused and will not be processed by the deployment controller.",
 }
 
 func (DeploymentSpec) SwaggerDoc() map[string]string {

pkg/apis/extensions/validation/validation.go

@@ -315,6 +315,10 @@ func ValidateDeploymentSpec(spec *extensions.DeploymentSpec, fldPath *field.Path
 	allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(spec.Replicas), fldPath.Child("replicas"))...)
 	allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpecForRC(&spec.Template, spec.Selector, spec.Replicas, fldPath.Child("template"))...)
 	allErrs = append(allErrs, ValidateDeploymentStrategy(&spec.Strategy, fldPath.Child("strategy"))...)
+	if spec.RevisionHistoryLimit != nil {
+		// zero is a valid RevisionHistoryLimit
+		allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.RevisionHistoryLimit), fldPath.Child("revisionHistoryLimit"))...)
+	}
 	// empty string is a valid UniqueLabelKey
 	if len(spec.UniqueLabelKey) > 0 {
 		allErrs = append(allErrs, apivalidation.ValidateLabelName(spec.UniqueLabelKey, fldPath.Child("uniqueLabel"))...)

pkg/controller/controller_utils.go

@@ -464,3 +464,16 @@ func SyncAllPodsWithStore(kubeClient client.Interface, store cache.Store) {
 	store.Replace(pods, allPods.ResourceVersion)
 	return
 }
+
+// ControllersByCreationTimestamp sorts a list of ReplicationControllers by creation timestamp, using their names as a tie breaker.
+type ControllersByCreationTimestamp []*api.ReplicationController
+
+func (o ControllersByCreationTimestamp) Len() int      { return len(o) }
+func (o ControllersByCreationTimestamp) Swap(i, j int) { o[i], o[j] = o[j], o[i] }
+
+func (o ControllersByCreationTimestamp) Less(i, j int) bool {
+	if o[i].CreationTimestamp.Equal(o[j].CreationTimestamp) {
+		return o[i].Name < o[j].Name
+	}
+	return o[i].CreationTimestamp.Before(o[j].CreationTimestamp)
+}

pkg/controller/deployment/deployment_controller.go

@@ -19,11 +19,12 @@ package deployment
 import (
 	"fmt"
 	"math"
+	"sort"
 	"time"
 
 	"github.com/golang/glog"
-
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/apis/extensions"
 	"k8s.io/kubernetes/pkg/client/cache"
 	"k8s.io/kubernetes/pkg/client/record"
@@ -34,6 +35,7 @@ import (
 	"k8s.io/kubernetes/pkg/runtime"
 	"k8s.io/kubernetes/pkg/util"
 	deploymentutil "k8s.io/kubernetes/pkg/util/deployment"
+	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 	labelsutil "k8s.io/kubernetes/pkg/util/labels"
 	podutil "k8s.io/kubernetes/pkg/util/pod"
 	"k8s.io/kubernetes/pkg/util/workqueue"
@@ -457,6 +459,11 @@ func (dc *DeploymentController) syncRecreateDeployment(deployment extensions.Dep
 		return dc.updateDeploymentStatus(allRCs, newRC, deployment)
 	}
 
+	if deployment.Spec.RevisionHistoryLimit != nil {
+		// Cleanup old RCs
+		dc.cleanupOldRcs(oldRCs, deployment)
+	}
+
 	// Sync deployment status
 	return dc.syncDeploymentStatus(allRCs, newRC, deployment)
 
@@ -496,6 +503,11 @@ func (dc *DeploymentController) syncRollingUpdateDeployment(deployment extension
 		return dc.updateDeploymentStatus(allRCs, newRC, deployment)
 	}
 
+	if deployment.Spec.RevisionHistoryLimit != nil {
+		// Cleanup old RCs
+		dc.cleanupOldRcs(oldRCs, deployment)
+	}
+
 	// Sync deployment status
 	return dc.syncDeploymentStatus(allRCs, newRC, deployment)
 
@@ -706,6 +718,31 @@ func (dc *DeploymentController) scaleUpNewRCForRecreate(newRC *api.ReplicationCo
 	return true, err
 }
 
+func (dc *DeploymentController) cleanupOldRcs(oldRCs []*api.ReplicationController, deployment extensions.Deployment) error {
+	diff := len(oldRCs) - *deployment.Spec.RevisionHistoryLimit
+	if diff <= 0 {
+		return nil
+	}
+
+	sort.Sort(controller.ControllersByCreationTimestamp(oldRCs))
+
+	var errList []error
+	// TODO: This should be parallelized.
+	for i := 0; i < diff; i++ {
+		controller := oldRCs[i]
+		// Avoid delete rc with non-zero replica counts
+		if controller.Spec.Replicas != 0 || controller.Generation > controller.Status.ObservedGeneration {
+			continue
+		}
+		if err := dc.client.ReplicationControllers(controller.Namespace).Delete(controller.Name); err != nil && !errors.IsNotFound(err) {
+			glog.V(2).Infof("Failed deleting old rc %v for deployment %v: %v", controller.Name, deployment.Name, err)
+			errList = append(errList, err)
+		}
+	}
+
+	return utilerrors.NewAggregate(errList)
+}
+
 func (dc *DeploymentController) updateDeploymentStatus(allRCs []*api.ReplicationController, newRC *api.ReplicationController, deployment extensions.Deployment) error {
 	totalReplicas, updatedReplicas, availableReplicas, unavailableReplicas, err := dc.calculateStatus(allRCs, newRC, deployment)
 	if err != nil {

pkg/controller/deployment/deployment_controller_test.go

@@ -85,8 +85,8 @@ func TestDeploymentController_reconcileNewRC(t *testing.T) {
 
 	for i, test := range tests {
 		t.Logf("executing scenario %d", i)
-		newRc := rc("foo-v2", test.newReplicas)
-		oldRc := rc("foo-v2", test.oldReplicas)
+		newRc := rc("foo-v2", test.newReplicas, nil)
+		oldRc := rc("foo-v2", test.oldReplicas, nil)
 		allRcs := []*api.ReplicationController{newRc, oldRc}
 		deployment := deployment("foo", test.deploymentReplicas, test.maxSurge, intstr.FromInt(0))
 		fake := &testclient.Fake{}
@@ -162,7 +162,7 @@ func TestDeploymentController_reconcileOldRCs(t *testing.T) {
 
 	for i, test := range tests {
 		t.Logf("executing scenario %d", i)
-		oldRc := rc("foo-v2", test.oldReplicas)
+		oldRc := rc("foo-v2", test.oldReplicas, nil)
 		allRcs := []*api.ReplicationController{oldRc}
 		oldRcs := []*api.ReplicationController{oldRc}
 		deployment := deployment("foo", test.deploymentReplicas, intstr.FromInt(0), test.maxUnavailable)
@@ -233,13 +233,76 @@ func TestDeploymentController_reconcileOldRCs(t *testing.T) {
 	}
 }
 
-func rc(name string, replicas int) *api.ReplicationController {
+func TestDeploymentController_cleanupOldRCs(t *testing.T) {
+	selector := map[string]string{"foo": "bar"}
+
+	tests := []struct {
+		oldRCs               []*api.ReplicationController
+		revisionHistoryLimit int
+		expectedDeletions    int
+	}{
+		{
+			oldRCs: []*api.ReplicationController{
+				rc("foo-1", 0, selector),
+				rc("foo-2", 0, selector),
+				rc("foo-3", 0, selector),
+			},
+			revisionHistoryLimit: 1,
+			expectedDeletions:    2,
+		},
+		{
+			oldRCs: []*api.ReplicationController{
+				rc("foo-1", 0, selector),
+				rc("foo-2", 0, selector),
+			},
+			revisionHistoryLimit: 0,
+			expectedDeletions:    2,
+		},
+		{
+			oldRCs: []*api.ReplicationController{
+				rc("foo-1", 1, selector),
+				rc("foo-2", 1, selector),
+			},
+			revisionHistoryLimit: 0,
+			expectedDeletions:    0,
+		},
+	}
+
+	for i, test := range tests {
+		fake := &testclient.Fake{}
+		controller := NewDeploymentController(fake, controller.NoResyncPeriodFunc)
+
+		controller.eventRecorder = &record.FakeRecorder{}
+		controller.rcStoreSynced = alwaysReady
+		controller.podStoreSynced = alwaysReady
+		for _, rc := range test.oldRCs {
+			controller.rcStore.Add(rc)
+		}
+
+		d := newDeployment(1, &tests[i].revisionHistoryLimit)
+		controller.cleanupOldRcs(test.oldRCs, *d)
+
+		gotDeletions := 0
+		for _, action := range fake.Actions() {
+			if "delete" == action.GetVerb() {
+				gotDeletions++
+			}
+		}
+		if gotDeletions != test.expectedDeletions {
+			t.Errorf("expect %v old rcs been deleted, but got %v", test.expectedDeletions, gotDeletions)
+			continue
+		}
+	}
+}
+
+func rc(name string, replicas int, selector map[string]string) *api.ReplicationController {
 	return &api.ReplicationController{
 		ObjectMeta: api.ObjectMeta{
 			Name: name,
 		},
 		Spec: api.ReplicationControllerSpec{
 			Replicas: replicas,
+			Selector: selector,
 			Template: &api.PodTemplateSpec{},
 		},
 	}
@@ -265,7 +328,7 @@ func deployment(name string, replicas int, maxSurge, maxUnavailable intstr.IntOr
 
 var alwaysReady = func() bool { return true }
 
-func newDeployment(replicas int) *exp.Deployment {
+func newDeployment(replicas int, revisionHistoryLimit *int) *exp.Deployment {
 	d := exp.Deployment{
 		TypeMeta: unversioned.TypeMeta{APIVersion: testapi.Default.GroupVersion().String()},
 		ObjectMeta: api.ObjectMeta{
@@ -296,6 +359,7 @@ func newDeployment(replicas int) *exp.Deployment {
 					},
 				},
 			},
+			RevisionHistoryLimit: revisionHistoryLimit,
 		},
 	}
 	return &d
@@ -413,7 +477,7 @@ func (f *fixture) run(deploymentName string) {
 func TestSyncDeploymentCreatesRC(t *testing.T) {
 	f := newFixture(t)
 
-	d := newDeployment(1)
+	d := newDeployment(1, nil)
 	f.dStore = append(f.dStore, d)
 
 	// expect that one rc with zero replicas is created

test/e2e/deployment.go

@@ -45,6 +45,9 @@ var _ = Describe("Deployment [Feature:Deployment]", func() {
 	It("RecreateDeployment should delete old pods and create new ones", func() {
 		testRecreateDeployment(f)
 	})
+	It("deployment should delete old rcs", func() {
+		testDeploymentCleanUpPolicy(f)
+	})
 	It("deployment should support rollover [Flaky]", func() {
 		testRolloverDeployment(f)
 	})
@@ -78,7 +81,7 @@ func newRC(rcName string, replicas int, rcPodLabels map[string]string, imageName
 	}
 }
 
-func newDeployment(deploymentName string, replicas int, podLabels map[string]string, imageName string, image string, strategyType extensions.DeploymentStrategyType) *extensions.Deployment {
+func newDeployment(deploymentName string, replicas int, podLabels map[string]string, imageName string, image string, strategyType extensions.DeploymentStrategyType, revisionHistoryLimit *int) *extensions.Deployment {
 	return &extensions.Deployment{
 		ObjectMeta: api.ObjectMeta{
 			Name: deploymentName,
@@ -89,7 +92,8 @@ func newDeployment(deploymentName string, replicas int, podLabels map[string]str
 			Strategy: extensions.DeploymentStrategy{
 				Type: strategyType,
 			},
-			UniqueLabelKey: extensions.DefaultDeploymentUniqueLabelKey,
+			RevisionHistoryLimit: revisionHistoryLimit,
+			UniqueLabelKey:       extensions.DefaultDeploymentUniqueLabelKey,
 			Template: api.PodTemplateSpec{
 				ObjectMeta: api.ObjectMeta{
 					Labels: podLabels,
@@ -114,7 +118,7 @@ func testNewDeployment(f *Framework) {
 	podLabels := map[string]string{"name": "nginx"}
 	replicas := 1
 	Logf("Creating simple deployment %s", deploymentName)
-	_, err := c.Deployments(ns).Create(newDeployment(deploymentName, replicas, podLabels, "nginx", "nginx", extensions.RollingUpdateDeploymentStrategyType))
+	_, err := c.Deployments(ns).Create(newDeployment(deploymentName, replicas, podLabels, "nginx", "nginx", extensions.RollingUpdateDeploymentStrategyType, nil))
 	Expect(err).NotTo(HaveOccurred())
 	defer func() {
 		deployment, err := c.Deployments(ns).Get(deploymentName)
@@ -171,7 +175,7 @@ func testRollingUpdateDeployment(f *Framework) {
 	// Create a deployment to delete nginx pods and instead bring up redis pods.
 	deploymentName := "redis-deployment"
 	Logf("Creating deployment %s", deploymentName)
-	_, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis", extensions.RollingUpdateDeploymentStrategyType))
+	_, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis", extensions.RollingUpdateDeploymentStrategyType, nil))
 	Expect(err).NotTo(HaveOccurred())
 	defer func() {
 		deployment, err := c.Deployments(ns).Get(deploymentName)
@@ -215,7 +219,7 @@ func testRollingUpdateDeploymentEvents(f *Framework) {
 	// Create a deployment to delete nginx pods and instead bring up redis pods.
 	deploymentName := "redis-deployment-2"
 	Logf("Creating deployment %s", deploymentName)
-	_, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis", extensions.RollingUpdateDeploymentStrategyType))
+	_, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis", extensions.RollingUpdateDeploymentStrategyType, nil))
 	Expect(err).NotTo(HaveOccurred())
 	defer func() {
 		deployment, err := c.Deployments(ns).Get(deploymentName)
@@ -276,7 +280,7 @@ func testRecreateDeployment(f *Framework) {
 	// Create a deployment to delete nginx pods and instead bring up redis pods.
 	deploymentName := "redis-deployment-3"
 	Logf("Creating deployment %s", deploymentName)
-	_, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis", extensions.RecreateDeploymentStrategyType))
+	_, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis", extensions.RecreateDeploymentStrategyType, nil))
 	Expect(err).NotTo(HaveOccurred())
 	defer func() {
 		deployment, err := c.Deployments(ns).Get(deploymentName)
@@ -314,6 +318,50 @@ func testRecreateDeployment(f *Framework) {
 	Expect(events.Items[1].Message).Should(Equal(fmt.Sprintf("Scaled up rc %s to 3", newRC.Name)))
 }
 
+// testDeploymentCleanUpPolicy tests that deployment supports cleanup policy
+func testDeploymentCleanUpPolicy(f *Framework) {
+	ns := f.Namespace.Name
+	c := f.Client
+	// Create nginx pods.
+	deploymentPodLabels := map[string]string{"name": "cleanup-pod"}
+	rcPodLabels := map[string]string{
+		"name": "cleanup-pod",
+		"pod":  "nginx",
+	}
+	rcName := "nginx-controller"
+	replicas := 1
+	revisionHistoryLimit := new(int)
+	*revisionHistoryLimit = 0
+	_, err := c.ReplicationControllers(ns).Create(newRC(rcName, replicas, rcPodLabels, "nginx", "nginx"))
+	Expect(err).NotTo(HaveOccurred())
+
+	// Verify that the required pods have come up.
+	err = verifyPods(c, ns, "cleanup-pod", false, 1)
+	if err != nil {
+		Logf("error in waiting for pods to come up: %s", err)
+		Expect(err).NotTo(HaveOccurred())
+	}
+
+	// Create a deployment to delete nginx pods and instead bring up redis pods.
+	deploymentName := "redis-deployment"
+	Logf("Creating deployment %s", deploymentName)
+	_, err = c.Deployments(ns).Create(newDeployment(deploymentName, replicas, deploymentPodLabels, "redis", "redis", extensions.RollingUpdateDeploymentStrategyType, revisionHistoryLimit))
+	Expect(err).NotTo(HaveOccurred())
+	defer func() {
+		deployment, err := c.Deployments(ns).Get(deploymentName)
+		Expect(err).NotTo(HaveOccurred())
+		Logf("deleting deployment %s", deploymentName)
+		Expect(c.Deployments(ns).Delete(deploymentName, nil)).NotTo(HaveOccurred())
+		// TODO: remove this once we can delete rcs with deployment
+		newRC, err := deploymentutil.GetNewRC(*deployment, c)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(c.ReplicationControllers(ns).Delete(newRC.Name)).NotTo(HaveOccurred())
+	}()
+
+	err = waitForDeploymentOldRCsNum(c, ns, deploymentName, *revisionHistoryLimit)
+	Expect(err).NotTo(HaveOccurred())
+}
+
 // testRolloverDeployment tests that deployment supports rollover.
 // i.e. we can change desired state and kick off rolling update, then change desired state again before it finishes.
 func testRolloverDeployment(f *Framework) {
@@ -348,7 +396,7 @@ func testRolloverDeployment(f *Framework) {
 	deploymentMinReadySeconds := 5
 	deploymentStrategyType := extensions.RollingUpdateDeploymentStrategyType
 	Logf("Creating deployment %s", deploymentName)
-	newDeployment := newDeployment(deploymentName, deploymentReplicas, deploymentPodLabels, deploymentImageName, deploymentImage, deploymentStrategyType)
+	newDeployment := newDeployment(deploymentName, deploymentReplicas, deploymentPodLabels, deploymentImageName, deploymentImage, deploymentStrategyType, nil)
 	newDeployment.Spec.Strategy.RollingUpdate = &extensions.RollingUpdateDeployment{
 		MaxUnavailable:  intstr.FromInt(1),
 		MaxSurge:        intstr.FromInt(1),
@@ -401,7 +449,7 @@ func testPausedDeployment(f *Framework) {
 	c := f.Client
 	deploymentName := "nginx"
 	podLabels := map[string]string{"name": "nginx"}
-	d := newDeployment(deploymentName, 1, podLabels, "nginx", "nginx", extensions.RollingUpdateDeploymentStrategyType)
+	d := newDeployment(deploymentName, 1, podLabels, "nginx", "nginx", extensions.RollingUpdateDeploymentStrategyType, nil)
 	d.Spec.Paused = true
 	Logf("Creating paused deployment %s", deploymentName)
 	_, err := c.Deployments(ns).Create(d)

test/e2e/util.go

@@ -1978,6 +1978,22 @@ func waitForDeploymentStatus(c *client.Client, ns, deploymentName string, desire
 	})
 }
 
+// Waits for the deployment to clean up old rcs.
+func waitForDeploymentOldRCsNum(c *client.Client, ns, deploymentName string, desiredRCNum int) error {
+	return wait.Poll(poll, 5*time.Minute, func() (bool, error) {
+
+		deployment, err := c.Deployments(ns).Get(deploymentName)
+		if err != nil {
+			return false, err
+		}
+		oldRCs, err := deploymentutil.GetOldRCs(*deployment, c)
+		if err != nil {
+			return false, err
+		}
+		return len(oldRCs) == desiredRCNum, nil
+	})
+}
+
 func logRCsOfDeployment(deploymentName string, oldRCs []*api.ReplicationController, newRC *api.ReplicationController) {
 	for i := range oldRCs {
 		Logf("Old RCs (%d/%d) of deployment %s: %+v", i+1, len(oldRCs), deploymentName, oldRCs[i])