cleanup-policy

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