fix reconcile (#410)

pravega · Feb 14, 2023 · edc3bef · edc3bef
1 parent a4a345f
commit edc3bef
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 20 deletions.
diff --git a/controllers/zookeepercluster_controller.go b/controllers/zookeepercluster_controller.go
@@ -100,8 +100,11 @@ func (r *ZookeeperClusterReconciler) Reconcile(_ context.Context, request ctrl.R
 		}
 		return reconcile.Result{Requeue: true}, nil
 	}
+	if finalized, err := r.reconcileFinalizers(instance); err != nil || finalized {
+		return reconcile.Result{}, err
+	}
+
 	for _, fun := range []reconcileFun{
-		r.reconcileFinalizers,
 		r.reconcileConfigMap,
 		r.reconcileStatefulSet,
 		r.reconcileClientService,
@@ -671,30 +674,30 @@ func (r *ZookeeperClusterReconciler) yamlConfigMap(instance *zookeeperv1beta1.Zo
 	return yamlexporter.GenerateOutputYAMLFile(subdir, cm.Kind, cm)
 }
 
-func (r *ZookeeperClusterReconciler) reconcileFinalizers(instance *zookeeperv1beta1.ZookeeperCluster) (err error) {
+func (r *ZookeeperClusterReconciler) reconcileFinalizers(instance *zookeeperv1beta1.ZookeeperCluster) (finalized bool, err error) {
 	if instance.Spec.Persistence != nil && instance.Spec.Persistence.VolumeReclaimPolicy != zookeeperv1beta1.VolumeReclaimPolicyDelete {
-		return nil
+		return false, nil
 	}
 	if instance.DeletionTimestamp.IsZero() {
 		if !utils.ContainsString(instance.ObjectMeta.Finalizers, utils.ZkFinalizer) && !config.DisableFinalizer {
 			instance.ObjectMeta.Finalizers = append(instance.ObjectMeta.Finalizers, utils.ZkFinalizer)
 			if err = r.Client.Update(context.TODO(), instance); err != nil {
-				return err
+				return false, err
 			}
 		}
-		return r.cleanupOrphanPVCs(instance)
+		return false, r.cleanupOrphanPVCs(instance)
 	} else {
 		if utils.ContainsString(instance.ObjectMeta.Finalizers, utils.ZkFinalizer) {
 			if err = r.cleanUpAllPVCs(instance); err != nil {
-				return err
+				return false, err
 			}
 			instance.ObjectMeta.Finalizers = utils.RemoveString(instance.ObjectMeta.Finalizers, utils.ZkFinalizer)
 			if err = r.Client.Update(context.TODO(), instance); err != nil {
-				return err
+				return false, err
 			}
 		}
 	}
-	return nil
+	return true, nil
 }
 
 func (r *ZookeeperClusterReconciler) getPVCCount(instance *zookeeperv1beta1.ZookeeperCluster) (pvcCount int, err error) {

diff --git a/controllers/zookeepercluster_controller_test.go b/controllers/zookeepercluster_controller_test.go
@@ -495,9 +495,10 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 
 		Context("Checking client", func() {
 			var (
-				cl    client.Client
-				err   error
-				count int
+				cl        client.Client
+				err       error
+				count     int
+				finalized bool
 			)
 
 			BeforeEach(func() {
@@ -537,8 +538,9 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 				_ = cl.Get(context.TODO(), req.NamespacedName, z)
 				z.Spec.Persistence.VolumeReclaimPolicy = v1beta1.VolumeReclaimPolicyDelete
 				cl.Update(context.TODO(), z)
-				err = r.reconcileFinalizers(z)
+				finalized, err = r.reconcileFinalizers(z)
 				Ω(err).To(BeNil())
+				Ω(finalized).To(BeFalse())
 			})
 
 			It("should delete pvc", func() {
@@ -588,32 +590,42 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 
 		Context("reconcileFinalizers", func() {
 			var (
-				cl  client.Client
-				err error
+				cl        client.Client
+				err       error
+				finalized bool
 			)
 			BeforeEach(func() {
 				z.WithDefaults()
 				z.Spec.Persistence = nil
 				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
-				err = r.reconcileFinalizers(z)
+				finalized, err = r.reconcileFinalizers(z)
 				// update deletion timestamp
 				_ = cl.Get(context.TODO(), req.NamespacedName, z)
 				now := metav1.Now()
 				z.SetDeletionTimestamp(&now)
 				cl.Update(context.TODO(), z)
-				err = r.reconcileFinalizers(z)
 			})
 			It("should not raise an error", func() {
+				finalized, err = r.reconcileFinalizers(z)
 				Ω(err).To(BeNil())
+				Ω(finalized).To(BeFalse())
+			})
+			It("should not raise an error", func() {
+				z.Spec.Persistence.VolumeReclaimPolicy = v1beta1.VolumeReclaimPolicyDelete
+				cl.Update(context.TODO(), z)
+				finalized, err = r.reconcileFinalizers(z)
+				Ω(err).To(BeNil())
+				Ω(finalized).To(BeTrue())
 			})
 		})
 
 		Context("reconcileFinalizers", func() {
 			var (
-				cl  client.Client
-				err error
+				cl        client.Client
+				err       error
+				finalized bool
 			)
 			BeforeEach(func() {
 				z.WithDefaults()
@@ -623,16 +635,18 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 			It("should have 1 finalizer, should not raise an error", func() {
 				config.DisableFinalizer = false
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
-				err = r.reconcileFinalizers(z)
+				finalized, err = r.reconcileFinalizers(z)
 				Expect(z.ObjectMeta.Finalizers).To(HaveLen(1))
 				Ω(err).To(BeNil())
+				Ω(finalized).To(BeFalse())
 			})
 			It("should have 0 finalizer, should not raise an error", func() {
 				config.DisableFinalizer = true
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
-				err = r.reconcileFinalizers(z)
+				finalized, err = r.reconcileFinalizers(z)
 				Expect(z.ObjectMeta.Finalizers).To(HaveLen(0))
 				Ω(err).To(BeNil())
+				Ω(finalized).To(BeFalse())
 			})
 		})