Add nil case to lease event handler

gardener · Nov 15, 2021 · 8734edf · 8734edf
1 parent df61842
commit 8734edf
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 23 deletions.
diff --git a/controllers/compaction_lease_controller_test.go b/controllers/compaction_lease_controller_test.go
@@ -70,29 +70,7 @@ var _ = Describe("Lease Controller", func() {
 			svc = &corev1.Service{}
 			Eventually(func() error { return serviceIsCorrectlyReconciled(c, instance, svc) }, timeout, pollingInterval).Should(BeNil())
 		})
-		It("no jobs will be scheduled because no store details are provided", func() {
-			// Verufy if the statefulset updated the specs
-			validateEtcdWithDefaults(instance, s, cm, svc)
 
-			setStatefulSetReady(s)
-			err = c.Status().Update(context.TODO(), s)
-			Expect(err).NotTo(HaveOccurred())
-
-			// Verify if that the job is not created even if the holder identity in delta-snapshot-revision is greater than 1M
-			deltaLease := &coordinationv1.Lease{}
-			Eventually(func() error { return deltaLeaseIsCorrectlyReconciled(c, instance, deltaLease) }, timeout, pollingInterval).Should(BeNil())
-			err = kutil.TryUpdate(context.TODO(), retry.DefaultBackoff, c, deltaLease, func() error {
-				deltaLease.Spec.HolderIdentity = pointer.StringPtr("1000000")
-				renewedTime := time.Now()
-				deltaLease.Spec.RenewTime = &metav1.MicroTime{Time: renewedTime}
-				return nil
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			j := &batchv1.Job{}
-			Eventually(func() error { return jobIsCorrectlyReconciled(c, instance, j) }, time.Duration(30*time.Second), pollingInterval).ShouldNot(BeNil())
-
-		})
 		AfterEach(func() {
 			Expect(c.Delete(context.TODO(), instance)).To(Succeed())
 			Eventually(func() error { return statefulSetRemoved(c, s) }, timeout, pollingInterval).Should(BeNil())
@@ -218,6 +196,16 @@ var _ = Describe("Lease Controller", func() {
 			})
 			Expect(err).NotTo(HaveOccurred())
 
+			// Deliberately update the full lease
+			fullLease := &coordinationv1.Lease{}
+			Eventually(func() error { return fullLeaseIsCorrectlyReconciled(c, instance, fullLease) }, timeout, pollingInterval).Should(BeNil())
+			err = kutil.TryUpdate(context.TODO(), retry.DefaultBackoff, c, fullLease, func() error {
+				fullLease.Spec.HolderIdentity = pointer.StringPtr("0")
+				renewedTime := time.Now()
+				fullLease.Spec.RenewTime = &metav1.MicroTime{Time: renewedTime}
+				return nil
+			})
+
 			// Deliberately update the delta lease
 			deltaLease := &coordinationv1.Lease{}
 			Eventually(func() error { return deltaLeaseIsCorrectlyReconciled(c, instance, deltaLease) }, timeout, pollingInterval).Should(BeNil())
@@ -284,6 +272,26 @@ var _ = Describe("Lease Controller", func() {
 			})
 			Expect(err).NotTo(HaveOccurred())
 
+			// Deliberately update the full lease
+			fullLease := &coordinationv1.Lease{}
+			Eventually(func() error { return fullLeaseIsCorrectlyReconciled(c, instance, fullLease) }, timeout, pollingInterval).Should(BeNil())
+			err = kutil.TryUpdate(context.TODO(), retry.DefaultBackoff, c, fullLease, func() error {
+				fullLease.Spec.HolderIdentity = pointer.StringPtr("0")
+				renewedTime := time.Now()
+				fullLease.Spec.RenewTime = &metav1.MicroTime{Time: renewedTime}
+				return nil
+			})
+
+			Eventually(func() error {
+				if err := c.Get(ctx, client.ObjectKeyFromObject(fullLease), fullLease); err != nil {
+					return err
+				}
+				if fullLease.Spec.HolderIdentity != nil {
+					return nil
+				}
+				return fmt.Errorf("no HolderIdentity")
+			}, 10*time.Second, pollingInterval)
+
 			// Deliberately update the delta lease
 			deltaLease := &coordinationv1.Lease{}
 			Eventually(func() error { return deltaLeaseIsCorrectlyReconciled(c, instance, deltaLease) }, timeout, pollingInterval).Should(BeNil())

diff --git a/pkg/predicate/predicate.go b/pkg/predicate/predicate.go
@@ -124,7 +124,8 @@ func LeaseHolderIdentityChange() predicate.Predicate {
 		if !ok {
 			return false
 		}
-		return *leaseOld.Spec.HolderIdentity != *leaseNew.Spec.HolderIdentity
+
+		return !reflect.DeepEqual(leaseOld.Spec.HolderIdentity, leaseNew.Spec.HolderIdentity)
 	}
 
 	return predicate.Funcs{

diff --git a/pkg/predicate/predicate_test.go b/pkg/predicate/predicate_test.go
@@ -116,6 +116,20 @@ var _ = Describe("Druid Predicate", func() {
 			pred = LeaseHolderIdentityChange()
 		})
 
+		Context("when holder identity is nil", func() {
+			BeforeEach(func() {
+				obj = &coordinationv1.Lease{}
+				oldObj = &coordinationv1.Lease{}
+			})
+
+			It("should return false", func() {
+				gomega.Expect(pred.Create(createEvent)).To(gomega.BeTrue())
+				gomega.Expect(pred.Update(updateEvent)).To(gomega.BeFalse())
+				gomega.Expect(pred.Delete(deleteEvent)).To(gomega.BeTrue())
+				gomega.Expect(pred.Generic(genericEvent)).To(gomega.BeTrue())
+			})
+		})
+
 		Context("when holder identity matches", func() {
 			BeforeEach(func() {
 				obj = &coordinationv1.Lease{