Update ISBServiceRollout controller unit tests (#68)

Signed-off-by: Ali Ibrahim <[email protected]> Co-authored-by: xdevxy <[email protected]>
numaproj · Jun 20, 2024 · 58d4a53 · 58d4a53
1 parent 2e9fd0e
commit 58d4a53
Showing 1 changed file with 97 additions and 1 deletion.
diff --git a/internal/controller/isbservicerollout_controller_test.go b/internal/controller/isbservicerollout_controller_test.go
@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 
 	numaflowv1 "github.com/numaproj/numaflow/pkg/apis/numaflow/v1alpha1"
+	"github.com/numaproj/numaplane/internal/util"
 	apiv1 "github.com/numaproj/numaplane/pkg/apis/numaplane/v1alpha1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -84,6 +85,19 @@ var _ = Describe("ISBServiceRollout Controller", func() {
 
 			By("Verifying the content of the ISBServiceRollout spec field")
 			Expect(createdInterStepBufferServiceSpec).Should(Equal(isbsSpec))
+
+			By("Verifying the spec hash stored in the ISBServiceRollout annotations after creation")
+			var isbServiceSpecAsMap map[string]any
+			Expect(json.Unmarshal(isbsSpecRaw, &isbServiceSpecAsMap)).ToNot(HaveOccurred())
+			isbServiceSpecHash := util.MustHash(isbServiceSpecAsMap)
+			Eventually(func() (string, error) {
+				createdResource := &apiv1.ISBServiceRollout{}
+				err := k8sClient.Get(ctx, resourceLookupKey, createdResource)
+				if err != nil {
+					return "", err
+				}
+				return createdResource.Annotations[apiv1.KeyHash], nil
+			}, timeout, interval).Should(Equal(isbServiceSpecHash))
 		})
 
 		It("Should have created an InterStepBufferService ", func() {
@@ -108,12 +122,30 @@ var _ = Describe("ISBServiceRollout Controller", func() {
 			}, timeout, interval).Should(Equal(apiv1.PhaseRunning))
 		})
 
-		It("Should update the ISBServiceRollout", func() {
+		It("Should update the ISBServiceRollout and InterStepBufferService", func() {
 			By("updating the ISBServiceRollout")
 
 			currentISBServiceRollout := &apiv1.ISBServiceRollout{}
 			Expect(k8sClient.Get(ctx, resourceLookupKey, currentISBServiceRollout)).ToNot(HaveOccurred())
 
+			var lastTransitionTime time.Time
+			Eventually(func() (time.Time, error) {
+				currentResource := &apiv1.ISBServiceRollout{}
+				err := k8sClient.Get(ctx, resourceLookupKey, currentResource)
+				if err != nil {
+					return time.Time{}, err
+				}
+
+				for _, cond := range currentISBServiceRollout.Status.Conditions {
+					if cond.Type == string(apiv1.ConditionConfigured) {
+						lastTransitionTime = cond.LastTransitionTime.Time
+						return lastTransitionTime, nil
+					}
+				}
+
+				return time.Time{}, nil
+			}, timeout, interval).Should(Not(Equal(time.Time{})))
+
 			// Prepare a new spec for update
 			newIsbsSpec := numaflowv1.InterStepBufferServiceSpec{
 				Redis: &numaflowv1.RedisBufferService{},
@@ -157,6 +189,70 @@ var _ = Describe("ISBServiceRollout Controller", func() {
 				return updatedChildResource.Spec, nil
 			}, timeout, interval).Should(Equal(newIsbsSpec))
 
+			By("Verifying the spec hash stored in the ISBServiceRollout annotations after update")
+			var isbServiceSpecAsMap map[string]any
+			Expect(json.Unmarshal(newIsbsSpecRaw, &isbServiceSpecAsMap)).ToNot(HaveOccurred())
+			isbServiceSpecHash := util.MustHash(isbServiceSpecAsMap)
+			Eventually(func() (string, error) {
+				updatedResource := &apiv1.ISBServiceRollout{}
+				err := k8sClient.Get(ctx, resourceLookupKey, updatedResource)
+				if err != nil {
+					return "", err
+				}
+				return updatedResource.Annotations[apiv1.KeyHash], nil
+			}, timeout, interval).Should(Equal(isbServiceSpecHash))
+
+			By("Verifying the LastTransitionTime of the Configured condition of the ISBServiceRollout is after the time of the initial configuration")
+			Eventually(func() (bool, error) {
+				updatedResource := &apiv1.ISBServiceRollout{}
+				err := k8sClient.Get(ctx, resourceLookupKey, updatedResource)
+				if err != nil {
+					return false, err
+				}
+
+				for _, cond := range updatedResource.Status.Conditions {
+					if cond.Type == string(apiv1.ConditionConfigured) {
+						isAfter := cond.LastTransitionTime.Time.After(lastTransitionTime)
+						lastTransitionTime = cond.LastTransitionTime.Time
+						return isAfter, nil
+					}
+				}
+
+				return false, nil
+			}, time.Second, interval).Should(BeTrue())
+
+			By("Verifying that the ISBServiceRollout Status Phase is Running")
+			Consistently(func() (apiv1.Phase, error) {
+				updatedResource := &apiv1.ISBServiceRollout{}
+				err := k8sClient.Get(ctx, resourceLookupKey, updatedResource)
+				if err != nil {
+					return apiv1.Phase(""), err
+				}
+				return updatedResource.Status.Phase, nil
+			}, timeout, interval).Should(Equal(apiv1.PhaseRunning))
+
+			By("Verifying that the same ISBServiceRollout should not perform and update (no Configuration condition LastTransitionTime change) and the hash spec annotation should not change")
+			Expect(k8sClient.Get(ctx, resourceLookupKey, currentISBServiceRollout)).ToNot(HaveOccurred())
+			Expect(k8sClient.Update(ctx, currentISBServiceRollout)).ToNot(HaveOccurred())
+			Eventually(func() (bool, error) {
+				updatedResource := &apiv1.ISBServiceRollout{}
+				err := k8sClient.Get(ctx, resourceLookupKey, updatedResource)
+				if err != nil {
+					return false, err
+				}
+
+				equalHash := updatedResource.Annotations[apiv1.KeyHash] == isbServiceSpecHash
+
+				for _, cond := range updatedResource.Status.Conditions {
+					if cond.Type == string(apiv1.ConditionConfigured) {
+						equalTime := cond.LastTransitionTime.Time.Equal(lastTransitionTime)
+						return equalTime && equalHash, nil
+					}
+				}
+
+				return false, nil
+			}, timeout, interval).Should(BeTrue())
+
 		})
 
 		It("Should delete the ISBServiceRollout and InterStepBufferService", func() {