fixed unit tests

trifork · Aug 21, 2024 · 33ab8f9 · 33ab8f9
1 parent a0aad9d
commit 33ab8f9
Show file tree

Hide file tree

Showing 3 changed files with 140 additions and 59 deletions.
diff --git a/opensearch-operator/pkg/reconcilers/ismpolicy.go b/opensearch-operator/pkg/reconcilers/ismpolicy.go
@@ -120,7 +120,7 @@ func (r *IsmPolicyReconciler) Reconcile() (retResult ctrl.Result, retErr error)
 	// Check cluster ref has not changed
 	managedCluster := r.instance.Status.ManagedCluster
 	if managedCluster != nil && *managedCluster != r.cluster.UID {
-		reason = "cannot change the cluster a role refers to"
+		reason = "cannot change the cluster a resource refers to"
 		retErr = fmt.Errorf("%s", reason)
 		r.recorder.Event(r.instance, "Warning", opensearchRefMismatch, reason)
 		return ctrl.Result{
@@ -250,6 +250,7 @@ func (r *IsmPolicyReconciler) Reconcile() (retResult ctrl.Result, retErr error)
 	// Return if there are no changes
 	if r.instance.Spec.PolicyID == existingPolicy.PolicyID && cmp.Equal(*newPolicy, existingPolicy.Policy, cmpopts.EquateEmpty()) {
 		r.logger.V(1).Info(fmt.Sprintf("user %s is in sync", r.instance.Name))
+		r.recorder.Event(r.instance, "Normal", opensearchAPIUnchanged, "policy is in sync")
 		return ctrl.Result{
 			Requeue:      true,
 			RequeueAfter: defaultRequeueAfter,
@@ -271,7 +272,6 @@ func (r *IsmPolicyReconciler) Reconcile() (retResult ctrl.Result, retErr error)
 	}
 
 	r.recorder.Event(r.instance, "Normal", opensearchAPIUpdated, "policy updated in opensearch")
-
 	return ctrl.Result{
 		Requeue:      true,
 		RequeueAfter: defaultRequeueAfter,

diff --git a/opensearch-operator/pkg/reconcilers/ismpolicy_test.go b/opensearch-operator/pkg/reconcilers/ismpolicy_test.go
@@ -116,7 +116,7 @@ var _ = Describe("ism policy reconciler", func() {
 			recorder = record.NewFakeRecorder(1)
 			mockClient.EXPECT().GetOpenSearchCluster(mock.Anything, mock.Anything).Return(*cluster, nil)
 		})
-		It("should wait for the cluster to be running", func() {
+		It("should emit a unit test event and requeue", func() {
 			go func() {
 				defer GinkgoRecover()
 				defer close(recorder.Events)
@@ -162,6 +162,29 @@ var _ = Describe("ism policy reconciler", func() {
 			)
 		})
 
+		When("cluster reference mismatch", func() {
+			BeforeEach(func() {
+				managedCluster := types.UID("different-uid")
+				instance.Status.ManagedCluster = &managedCluster
+			})
+
+			It("should emit a unit test event and not requeue", func() {
+				go func() {
+					defer GinkgoRecover()
+					defer close(recorder.Events)
+					result, err := reconciler.Reconcile()
+					Expect(err).To(HaveOccurred())
+					Expect(result.Requeue).To(BeFalse())
+				}()
+				var events []string
+				for msg := range recorder.Events {
+					events = append(events, msg)
+				}
+				Expect(len(events)).To(Equal(1))
+				Expect(events[0]).To(Equal(fmt.Sprintf("Warning %s cannot change the cluster a resource refers to", opensearchRefMismatch)))
+			})
+		})
+
 		When("policy does not exist in opensearch", func() {
 			BeforeEach(func() {
 				mockClient.EXPECT().UdateObjectStatus(mock.Anything, mock.Anything).Return(nil)
@@ -187,7 +210,7 @@ var _ = Describe("ism policy reconciler", func() {
 					httpmock.NewStringResponder(200, "OK").Once(),
 				)
 			})
-			It("should create the policy", func() {
+			It("should create the policy, emit a unit test event, and requeue", func() {
 				go func() {
 					defer GinkgoRecover()
 					defer close(recorder.Events)
@@ -234,19 +257,20 @@ var _ = Describe("ism policy reconciler", func() {
 			})
 		})
 
-		When("policy exists in opensearch and existing status is nil", func() {
+		When("policy exists in opensearch", func() {
 			BeforeEach(func() {
-				mockClient.EXPECT().UdateObjectStatus(mock.Anything, mock.Anything).Return(nil)
+				instance.Spec.PolicyID = "test-policy-id"
 
 				transport.RegisterResponder(
 					http.MethodGet,
 					fmt.Sprintf(
 						"https://%s.%s.svc.cluster.local:9200/_plugins/_ism/policies/%s",
 						cluster.Spec.General.ServiceName,
 						cluster.Namespace,
-						instance.Name,
+						instance.Spec.PolicyID,
 					),
 					httpmock.NewJsonResponderOrPanic(200, responses.GetISMPolicyResponse{
+						PolicyID: "test-policy-id",
 						Policy: requests.ISMPolicySpec{
 							DefaultState: "test-state",
 							Description:  "test-policy",
@@ -255,63 +279,119 @@ var _ = Describe("ism policy reconciler", func() {
 				)
 			})
 
-			It("should requeue and emit a unit test event", func() {
-				go func() {
-					defer GinkgoRecover()
-					defer close(recorder.Events)
-					result, err := reconciler.Reconcile()
-					Expect(err).ToNot(HaveOccurred())
-					Expect(result.Requeue).To(BeTrue())
-					// Confirm all responders have been called
-					Expect(transport.GetTotalCallCount()).To(Equal(transport.NumResponders() + extraContextCalls))
-				}()
-				var events []string
-				for msg := range recorder.Events {
-					events = append(events, msg)
-				}
-				Expect(len(events)).To(Equal(1))
-				Expect(events[0]).To(Equal(fmt.Sprintf("Warning %s the ISM policy already exists in the OpenSearch cluster", opensearchIsmPolicyExists)))
+			When("existing status is nil", func() {
+				BeforeEach(func() {
+					mockClient.EXPECT().UdateObjectStatus(mock.Anything, mock.Anything).Return(nil)
+					instance.Status.ExistingISMPolicy = nil
+				})
+
+				It("should emit a unit test event and requeue", func() {
+					go func() {
+						defer GinkgoRecover()
+						defer close(recorder.Events)
+						result, err := reconciler.Reconcile()
+						Expect(err).ToNot(HaveOccurred())
+						Expect(result.Requeue).To(BeTrue())
+						// Confirm all responders have been called
+						Expect(transport.GetTotalCallCount()).To(Equal(transport.NumResponders() + extraContextCalls))
+					}()
+					var events []string
+					for msg := range recorder.Events {
+						events = append(events, msg)
+					}
+					Expect(len(events)).To(Equal(1))
+					Expect(events[0]).To(Equal(fmt.Sprintf("Warning %s the ISM policy already exists in the OpenSearch cluster", opensearchIsmPolicyExists)))
+				})
 			})
-		})
 
-		When("policy exists in opensearch and existing status is true", func() {
-			BeforeEach(func() {
-				instance.Status.ExistingISMPolicy = pointer.Bool(true)
-				mockClient.EXPECT().UdateObjectStatus(mock.Anything, mock.Anything).Return(nil)
+			When("existing status is true", func() {
+				BeforeEach(func() {
+					mockClient.EXPECT().UdateObjectStatus(mock.Anything, mock.Anything).Return(nil)
+					instance.Status.ExistingISMPolicy = pointer.Bool(true)
+				})
 
-				transport.RegisterResponder(
-					http.MethodGet,
-					fmt.Sprintf(
-						"https://%s.%s.svc.cluster.local:9200/_plugins/_ism/policies/%s",
-						cluster.Spec.General.ServiceName,
-						cluster.Namespace,
-						instance.Name,
-					),
-					httpmock.NewJsonResponderOrPanic(200, responses.GetISMPolicyResponse{
-						Policy: requests.ISMPolicySpec{
-							DefaultState: "test-state",
-							Description:  "test-policy",
-						},
-					}).Once(),
-				)
+				It("should emit a unit test event and requeue", func() {
+					go func() {
+						defer GinkgoRecover()
+						defer close(recorder.Events)
+						result, err := reconciler.Reconcile()
+						Expect(err).ToNot(HaveOccurred())
+						Expect(result.Requeue).To(BeTrue())
+						// Confirm all responders have been called
+						Expect(transport.GetTotalCallCount()).To(Equal(transport.NumResponders() + extraContextCalls))
+					}()
+					var events []string
+					for msg := range recorder.Events {
+						events = append(events, msg)
+					}
+					Expect(len(events)).To(Equal(1))
+					Expect(events[0]).To(Equal(fmt.Sprintf("Warning %s the ISM policy already exists in the OpenSearch cluster", opensearchIsmPolicyExists)))
+				})
 			})
 
-			It("should requeue and emit a unit test event", func() {
-				go func() {
-					defer GinkgoRecover()
-					defer close(recorder.Events)
-					result, err := reconciler.Reconcile()
-					Expect(err).ToNot(HaveOccurred())
-					Expect(result.Requeue).To(BeTrue())
-					// Confirm all responders have been called
-					Expect(transport.GetTotalCallCount()).To(Equal(transport.NumResponders() + extraContextCalls))
-				}()
-				var events []string
-				for msg := range recorder.Events {
-					events = append(events, msg)
-				}
-				Expect(len(events)).To(Equal(1))
-				Expect(events[0]).To(Equal(fmt.Sprintf("Warning %s the ISM policy already exists in the OpenSearch cluster", opensearchIsmPolicyExists)))
+			When("existing status is false", func() {
+				BeforeEach(func() {
+					instance.Status.ExistingISMPolicy = pointer.Bool(false)
+				})
+
+				When("policy is the same", func() {
+					BeforeEach(func() {
+						instance.Spec.DefaultState = "test-state"
+						instance.Spec.Description = "test-policy"
+					})
+					It("should emit a unit test event and requeue", func() {
+						go func() {
+							defer GinkgoRecover()
+							defer close(recorder.Events)
+							result, err := reconciler.Reconcile()
+							Expect(err).ToNot(HaveOccurred())
+							Expect(result.Requeue).To(BeTrue())
+							// Confirm all responders have been called
+							Expect(transport.GetTotalCallCount()).To(Equal(transport.NumResponders() + extraContextCalls))
+						}()
+						var events []string
+						for msg := range recorder.Events {
+							events = append(events, msg)
+						}
+						Expect(len(events)).To(Equal(1))
+						Expect(events[0]).To(Equal(fmt.Sprintf("Normal %s policy is in sync", opensearchAPIUnchanged)))
+					})
+				})
+
+				When("policy is not the same", func() {
+					BeforeEach(func() {
+						instance.Spec.DefaultState = "test-state2"
+						instance.Spec.Description = "test-policy2"
+
+						transport.RegisterResponder(
+							http.MethodPut,
+							fmt.Sprintf(
+								"https://%s.%s.svc.cluster.local:9200/_plugins/_ism/policies/%s",
+								cluster.Spec.General.ServiceName,
+								cluster.Namespace,
+								instance.Spec.PolicyID,
+							),
+							httpmock.NewStringResponder(200, "OK").Once(),
+						)
+					})
+					It("should update ism policy, emit a unit test event, and requeue", func() {
+						go func() {
+							defer GinkgoRecover()
+							defer close(recorder.Events)
+							result, err := reconciler.Reconcile()
+							Expect(err).ToNot(HaveOccurred())
+							Expect(result.Requeue).To(BeTrue())
+							// Confirm all responders have been called
+							Expect(transport.GetTotalCallCount()).To(Equal(transport.NumResponders() + extraContextCalls))
+						}()
+						var events []string
+						for msg := range recorder.Events {
+							events = append(events, msg)
+						}
+						Expect(len(events)).To(Equal(1))
+						Expect(events[0]).To(Equal(fmt.Sprintf("Normal %s policy updated in opensearch", opensearchAPIUpdated)))
+					})
+				})
 			})
 		})
 	})

diff --git a/opensearch-operator/pkg/reconcilers/reconcilers.go b/opensearch-operator/pkg/reconcilers/reconcilers.go
@@ -19,6 +19,7 @@ const (
 	opensearchAPIError            = "OpensearchAPIError"
 	opensearchRefMismatch         = "OpensearchRefMismatch"
 	opensearchAPIUpdated          = "OpensearchAPIUpdated"
+	opensearchAPIUnchanged        = "OpensearchAPIUnchanged"
 	opensearchCustomResourceError = "OpensearchCustomResourceError"
 	passwordError                 = "PasswordError"
 	statusError                   = "StatusUpdateError"