controller: unit tests for overlapping and recreate deployments

pkg/controller/deployment/deployment_controller.go

@@ -73,6 +73,8 @@ type DeploymentController struct {
 
 	// To allow injection of syncDeployment for testing.
 	syncHandler func(dKey string) error
+	// used for unit testing
+	enqueueDeployment func(deployment *extensions.Deployment)
 
 	// A store of deployments, populated by the dController
 	dLister *cache.StoreToDeploymentLister
@@ -134,6 +136,8 @@ func NewDeploymentController(dInformer informers.DeploymentInformer, rsInformer
 	})
 
 	dc.syncHandler = dc.syncDeployment
+	dc.enqueueDeployment = dc.enqueue
+
 	dc.dLister = dInformer.Lister()
 	dc.rsLister = rsInformer.Lister()
 	dc.podLister = podInformer.Lister()
@@ -343,7 +347,7 @@ func (dc *DeploymentController) deletePod(obj interface{}) {
 	}
 }
 
-func (dc *DeploymentController) enqueueDeployment(deployment *extensions.Deployment) {
+func (dc *DeploymentController) enqueue(deployment *extensions.Deployment) {
 	key, err := controller.KeyFunc(deployment)
 	if err != nil {
 		utilruntime.HandleError(fmt.Errorf("Couldn't get key for object %#v: %v", deployment, err))

pkg/controller/deployment/deployment_controller_test.go

@@ -19,6 +19,7 @@ package deployment
 import (
 	"fmt"
 	"testing"
+	"time"
 
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/apimachinery/registered"
@@ -28,6 +29,7 @@ import (
 	"k8s.io/kubernetes/pkg/client/record"
 	"k8s.io/kubernetes/pkg/client/testing/core"
 	"k8s.io/kubernetes/pkg/controller"
+	"k8s.io/kubernetes/pkg/controller/deployment/util"
 	"k8s.io/kubernetes/pkg/controller/informers"
 	"k8s.io/kubernetes/pkg/runtime"
 	"k8s.io/kubernetes/pkg/runtime/schema"
@@ -67,9 +69,10 @@ func newDeployment(name string, replicas int, revisionHistoryLimit *int32, maxSu
 	d := extensions.Deployment{
 		TypeMeta: metav1.TypeMeta{APIVersion: registered.GroupOrDie(extensions.GroupName).GroupVersion.String()},
 		ObjectMeta: v1.ObjectMeta{
-			UID:       uuid.NewUUID(),
+			UID:         uuid.NewUUID(),
-			Name:      name,
+			Name:        name,
-			Namespace: v1.NamespaceDefault,
+			Namespace:   v1.NamespaceDefault,
+			Annotations: make(map[string]string),
 		},
 		Spec: extensions.DeploymentSpec{
 			Strategy: extensions.DeploymentStrategy{
@@ -110,8 +113,10 @@ func newReplicaSet(d *extensions.Deployment, name string, replicas int) *extensi
 		ObjectMeta: v1.ObjectMeta{
 			Name:      name,
 			Namespace: v1.NamespaceDefault,
+			Labels:    d.Spec.Selector.MatchLabels,
 		},
 		Spec: extensions.ReplicaSetSpec{
+			Selector: d.Spec.Selector,
 			Replicas: func() *int32 { i := int32(replicas); return &i }(),
 			Template: d.Spec.Template,
 		},
@@ -163,7 +168,7 @@ func newFixture(t *testing.T) *fixture {
 	return f
 }
 
-func (f *fixture) run(deploymentName string) {
+func (f *fixture) newController() (*DeploymentController, informers.SharedInformerFactory) {
 	f.client = fake.NewSimpleClientset(f.objects...)
 	informers := informers.NewSharedInformerFactory(f.client, nil, controller.NoResyncPeriodFunc())
 	c := NewDeploymentController(informers.Deployments(), informers.ReplicaSets(), informers.Pods(), f.client)
@@ -180,6 +185,11 @@ func (f *fixture) run(deploymentName string) {
 	for _, pod := range f.podLister {
 		c.podLister.Indexer.Add(pod)
 	}
+	return c, informers
+}
+
+func (f *fixture) run(deploymentName string) {
+	c, informers := f.newController()
 	stopCh := make(chan struct{})
 	defer close(stopCh)
 	informers.Start(stopCh)
@@ -286,3 +296,218 @@ func filterInformerActions(actions []core.Action) []core.Action {
 
 	return ret
 }
+
+// TestOverlappingDeployment ensures that an overlapping deployment will not be synced by
+// the controller.
+func TestOverlappingDeployment(t *testing.T) {
+	f := newFixture(t)
+	now := metav1.Now()
+	later := metav1.Time{Time: now.Add(time.Minute)}
+
+	foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
+	foo.CreationTimestamp = now
+	bar := newDeployment("bar", 1, nil, nil, nil, map[string]string{"foo": "bar", "app": "baz"})
+	bar.CreationTimestamp = later
+
+	f.dLister = append(f.dLister, foo, bar)
+	f.objects = append(f.objects, foo, bar)
+
+	f.expectUpdateDeploymentStatusAction(bar)
+	f.run(getKey(bar, t))
+
+	actions := f.client.Actions()
+	d := actions[0].(core.UpdateAction).GetObject().(*extensions.Deployment)
+	if len(d.Annotations[util.OverlapAnnotation]) == 0 {
+		t.Errorf("annotations weren't updated for the overlapping deployment: %v", d.Annotations)
+	}
+}
+
+// TestSyncOverlappedDeployment ensures that from two overlapping deployments, the older
+// one will be synced and the newer will be marked as overlapping. Note that in reality it's
+// not always the older deployment that is the one that works vs the rest but the one which
+// has the selector unchanged for longer time.
+func TestSyncOverlappedDeployment(t *testing.T) {
+	f := newFixture(t)
+	now := metav1.Now()
+	later := metav1.Time{Time: now.Add(time.Minute)}
+
+	foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
+	foo.CreationTimestamp = now
+	bar := newDeployment("bar", 1, nil, nil, nil, map[string]string{"foo": "bar", "app": "baz"})
+	bar.CreationTimestamp = later
+
+	f.dLister = append(f.dLister, foo, bar)
+	f.objects = append(f.objects, foo, bar)
+
+	f.expectUpdateDeploymentStatusAction(bar)
+	f.expectCreateRSAction(newReplicaSet(foo, "foo-rs", 1))
+	f.expectUpdateDeploymentAction(foo)
+	f.expectUpdateDeploymentStatusAction(foo)
+	f.run(getKey(foo, t))
+
+	actions := f.client.Actions()
+	d := actions[0].(core.UpdateAction).GetObject().(*extensions.Deployment)
+	if d.Annotations[util.OverlapAnnotation] != "foo" {
+		t.Errorf("annotations weren't updated for the overlapping deployment: %v", d.Annotations)
+	}
+}
+
+// TestDeletedDeploymentShouldCleanupOverlaps ensures that the deletion of a deployment
+// will cleanup any deployments that overlap with it.
+func TestDeletedDeploymentShouldCleanupOverlaps(t *testing.T) {
+	f := newFixture(t)
+	now := metav1.Now()
+	earlier := metav1.Time{Time: now.Add(-time.Minute)}
+	later := metav1.Time{Time: now.Add(time.Minute)}
+
+	foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
+	foo.CreationTimestamp = earlier
+	foo.DeletionTimestamp = &now
+	bar := newDeployment("bar", 1, nil, nil, nil, map[string]string{"foo": "bar"})
+	bar.CreationTimestamp = later
+	bar.Annotations = map[string]string{util.OverlapAnnotation: "foo"}
+
+	f.dLister = append(f.dLister, foo, bar)
+	f.objects = append(f.objects, foo, bar)
+
+	f.expectUpdateDeploymentStatusAction(bar)
+	f.expectUpdateDeploymentStatusAction(foo)
+	f.run(getKey(foo, t))
+
+	for _, a := range f.client.Actions() {
+		action, ok := a.(core.UpdateAction)
+		if !ok {
+			continue
+		}
+		d := action.GetObject().(*extensions.Deployment)
+		if d.Name != "bar" {
+			continue
+		}
+
+		if len(d.Annotations[util.OverlapAnnotation]) > 0 {
+			t.Errorf("annotations weren't cleaned up for the overlapping deployment: %v", d.Annotations)
+		}
+	}
+}
+
+// TestDeletedDeploymentShouldNotCleanupOtherOverlaps ensures that the deletion of
+// a deployment will not cleanup deployments that overlap with another deployment.
+func TestDeletedDeploymentShouldNotCleanupOtherOverlaps(t *testing.T) {
+	f := newFixture(t)
+	now := metav1.Now()
+	earlier := metav1.Time{Time: now.Add(-time.Minute)}
+	later := metav1.Time{Time: now.Add(time.Minute)}
+
+	foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
+	foo.CreationTimestamp = earlier
+	foo.DeletionTimestamp = &now
+	bar := newDeployment("bar", 1, nil, nil, nil, map[string]string{"bla": "bla"})
+	bar.CreationTimestamp = later
+	// Notice this deployment is overlapping with another deployment
+	bar.Annotations = map[string]string{util.OverlapAnnotation: "baz"}
+
+	f.dLister = append(f.dLister, foo, bar)
+	f.objects = append(f.objects, foo, bar)
+
+	f.expectUpdateDeploymentStatusAction(foo)
+	f.run(getKey(foo, t))
+
+	for _, a := range f.client.Actions() {
+		action, ok := a.(core.UpdateAction)
+		if !ok {
+			continue
+		}
+		d := action.GetObject().(*extensions.Deployment)
+		if d.Name != "bar" {
+			continue
+		}
+
+		if len(d.Annotations[util.OverlapAnnotation]) == 0 {
+			t.Errorf("overlapping annotation should not be cleaned up for bar: %v", d.Annotations)
+		}
+	}
+}
+
+// TestPodDeletionEnqueuesRecreateDeployment ensures that the deletion of a pod
+// will requeue a Recreate deployment iff there is no other pod returned from the
+// client.
+func TestPodDeletionEnqueuesRecreateDeployment(t *testing.T) {
+	f := newFixture(t)
+
+	foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
+	foo.Spec.Strategy.Type = extensions.RecreateDeploymentStrategyType
+	rs := newReplicaSet(foo, "foo-1", 1)
+	pod := generatePodFromRS(rs)
+
+	f.dLister = append(f.dLister, foo)
+	f.rsLister = append(f.rsLister, rs)
+	f.objects = append(f.objects, foo, rs)
+
+	c, informers := f.newController()
+	enqueued := false
+	c.enqueueDeployment = func(d *extensions.Deployment) {
+		if d.Name == "foo" {
+			enqueued = true
+		}
+	}
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+	informers.Start(stopCh)
+
+	c.deletePod(pod)
+
+	if !enqueued {
+		t.Errorf("expected deployment %q to be queued after pod deletion", foo.Name)
+	}
+}
+
+// TestPodDeletionDoesntEnqueueRecreateDeployment ensures that the deletion of a pod
+// will not requeue a Recreate deployment iff there are other pods returned from the
+// client.
+func TestPodDeletionDoesntEnqueueRecreateDeployment(t *testing.T) {
+	f := newFixture(t)
+
+	foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
+	foo.Spec.Strategy.Type = extensions.RecreateDeploymentStrategyType
+	rs := newReplicaSet(foo, "foo-1", 1)
+	pod := generatePodFromRS(rs)
+
+	f.dLister = append(f.dLister, foo)
+	f.rsLister = append(f.rsLister, rs)
+	// Let's pretend this is a different pod. The gist is that the pod lister needs to
+	// return a non-empty list.
+	f.podLister = append(f.podLister, pod)
+
+	c, informers := f.newController()
+	enqueued := false
+	c.enqueueDeployment = func(d *extensions.Deployment) {
+		if d.Name == "foo" {
+			enqueued = true
+		}
+	}
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+	informers.Start(stopCh)
+
+	c.deletePod(pod)
+
+	if enqueued {
+		t.Errorf("expected deployment %q not to be queued after pod deletion", foo.Name)
+	}
+}
+
+// generatePodFromRS creates a pod, with the input ReplicaSet's selector and its template
+func generatePodFromRS(rs *extensions.ReplicaSet) *v1.Pod {
+	trueVar := true
+	return &v1.Pod{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      rs.Name + "-pod",
+			Namespace: rs.Namespace,
+			Labels:    rs.Spec.Selector.MatchLabels,
+			OwnerReferences: []metav1.OwnerReference{
+				{UID: rs.UID, APIVersion: "v1beta1", Kind: "ReplicaSet", Name: rs.Name, Controller: &trueVar},
+			},
+		},
+		Spec: rs.Spec.Template.Spec,
+	}
+}