From e2695d9d05053443dd3b1941a33504a9f6b98e80 Mon Sep 17 00:00:00 2001 From: Michail Kargakis Date: Wed, 21 Dec 2016 14:46:07 +0100 Subject: [PATCH] controller: unit tests for overlapping and recreate deployments --- .../deployment/deployment_controller.go | 6 +- .../deployment/deployment_controller_test.go | 249 +++++++++++++++++- 2 files changed, 250 insertions(+), 5 deletions(-) diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index ff91be56e3..736d62275b 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/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)) diff --git a/pkg/controller/deployment/deployment_controller_test.go b/pkg/controller/deployment/deployment_controller_test.go index 12f9312022..f2efd16e08 100644 --- a/pkg/controller/deployment/deployment_controller_test.go +++ b/pkg/controller/deployment/deployment_controller_test.go @@ -19,6 +19,7 @@ package deployment import ( "fmt" "testing" + "time" "k8s.io/apimachinery/pkg/apimachinery/registered" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -30,6 +31,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/util/intstr" "k8s.io/kubernetes/pkg/util/uuid" @@ -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(), - Name: name, - Namespace: v1.NamespaceDefault, + UID: uuid.NewUUID(), + Name: name, + 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,234 @@ 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)) + + for _, a := range filterInformerActions(f.client.Actions()) { + action, ok := a.(core.UpdateAction) + if !ok { + continue + } + d, ok := action.GetObject().(*extensions.Deployment) + if !ok { + continue + } + if d.Name == "bar" && d.Annotations[util.OverlapAnnotation] != "foo" { + 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)) + + for _, a := range filterInformerActions(f.client.Actions()) { + action, ok := a.(core.UpdateAction) + if !ok { + continue + } + d, ok := action.GetObject().(*extensions.Deployment) + if !ok { + continue + } + if d.Name == "bar" && 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, + } +}