From b8bc54a03f320a4d0a2a88c67d5ec13010016a80 Mon Sep 17 00:00:00 2001
From: hatfieldbrian <bhatfiel@redhat.com>
Date: Thu, 19 Aug 2021 13:50:20 -0700
Subject: [PATCH] return a bool from AddFinalizer and RemoveFinalizer

Signed-off-by: hatfieldbrian <bhatfiel@redhat.com>
---
 .../controllerutil/controllerutil.go          | 11 +++-
 .../controllerutil/controllerutil_test.go     | 57 +++++++++++++++++++
 2 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/pkg/controller/controllerutil/controllerutil.go b/pkg/controller/controllerutil/controllerutil.go
index 13f14a7ed3..bacbea3544 100644
--- a/pkg/controller/controllerutil/controllerutil.go
+++ b/pkg/controller/controllerutil/controllerutil.go
@@ -349,26 +349,31 @@ func mutate(f MutateFn, key client.ObjectKey, obj client.Object) error {
 type MutateFn func() error
 
 // AddFinalizer accepts an Object and adds the provided finalizer if not present.
-func AddFinalizer(o client.Object, finalizer string) {
+// It returns an indication of whether it updated the object's list of finalizers.
+func AddFinalizer(o client.Object, finalizer string) (finalizersUpdated bool) {
 	f := o.GetFinalizers()
 	for _, e := range f {
 		if e == finalizer {
-			return
+			return false
 		}
 	}
 	o.SetFinalizers(append(f, finalizer))
+	return true
 }
 
 // RemoveFinalizer accepts an Object and removes the provided finalizer if present.
-func RemoveFinalizer(o client.Object, finalizer string) {
+// It returns an indication of whether it updated the object's list of finalizers.
+func RemoveFinalizer(o client.Object, finalizer string) (finalizersUpdated bool) {
 	f := o.GetFinalizers()
 	for i := 0; i < len(f); i++ {
 		if f[i] == finalizer {
 			f = append(f[:i], f[i+1:]...)
 			i--
+			finalizersUpdated = true
 		}
 	}
 	o.SetFinalizers(f)
+	return
 }
 
 // ContainsFinalizer checks an Object that the provided finalizer is present.
diff --git a/pkg/controller/controllerutil/controllerutil_test.go b/pkg/controller/controllerutil/controllerutil_test.go
index d47c691465..e2464b2953 100644
--- a/pkg/controller/controllerutil/controllerutil_test.go
+++ b/pkg/controller/controllerutil/controllerutil_test.go
@@ -700,6 +700,62 @@ var _ = Describe("Controllerutil", func() {
 			})
 		})
 
+		Describe("AddFinalizer, which returns an indication of whether it modified the object's list of finalizers,", func() {
+			deploy = &appsv1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Finalizers: []string{},
+				},
+			}
+
+			When("the object's list of finalizers has no instances of the input finalizer", func() {
+				It("should return true", func() {
+					Expect(controllerutil.AddFinalizer(deploy, testFinalizer)).To(BeTrue())
+				})
+				It("should add the input finalizer to the object's list of finalizers", func() {
+					Expect(deploy.ObjectMeta.GetFinalizers()).To(Equal([]string{testFinalizer}))
+				})
+			})
+
+			When("the object's list of finalizers has an instance of the input finalizer", func() {
+				It("should return false", func() {
+					Expect(controllerutil.AddFinalizer(deploy, testFinalizer)).To(BeFalse())
+				})
+				It("should not modify the object's list of finalizers", func() {
+					Expect(deploy.ObjectMeta.GetFinalizers()).To(Equal([]string{testFinalizer}))
+				})
+			})
+		})
+
+		Describe("RemoveFinalizer, which returns an indication of whether it modified the object's list of finalizers,", func() {
+			When("the object's list of finalizers has no instances of the input finalizer", func() {
+				It("should return false", func() {
+					Expect(controllerutil.RemoveFinalizer(deploy, testFinalizer1)).To(BeFalse())
+				})
+				It("should not modify the object's list of finalizers", func() {
+					Expect(deploy.ObjectMeta.GetFinalizers()).To(Equal([]string{testFinalizer}))
+				})
+			})
+
+			When("the object's list of finalizers has one instance of the input finalizer", func() {
+				It("should return true", func() {
+					Expect(controllerutil.RemoveFinalizer(deploy, testFinalizer)).To(BeTrue())
+				})
+				It("should remove the instance of the input finalizer from the object's list of finalizers", func() {
+					Expect(deploy.ObjectMeta.GetFinalizers()).To(Equal([]string{}))
+				})
+			})
+
+			When("the object's list of finalizers has multiple instances of the input finalizer", func() {
+				It("should return true", func() {
+					deploy.SetFinalizers(append(deploy.Finalizers, testFinalizer, testFinalizer))
+					Expect(controllerutil.RemoveFinalizer(deploy, testFinalizer)).To(BeTrue())
+				})
+				It("should remove each instance of the input finalizer from the object's list of finalizers", func() {
+					Expect(deploy.ObjectMeta.GetFinalizers()).To(Equal([]string{}))
+				})
+			})
+		})
+
 		Describe("ContainsFinalizer", func() {
 			It("should check that finalizer is present", func() {
 				controllerutil.AddFinalizer(deploy, testFinalizer)
@@ -715,6 +771,7 @@ var _ = Describe("Controllerutil", func() {
 })
 
 const testFinalizer = "foo.bar.baz"
+const testFinalizer1 = testFinalizer + "1"
 
 var _ runtime.Object = &errRuntimeObj{}
 var _ metav1.Object = &errMetaObj{}