return a bool from AddFinalizer and RemoveFinalizer

Signed-off-by: hatfieldbrian <[email protected]>

Author: hatfieldbrian

pkg/controller/controllerutil/controllerutil.go

@@ -348,27 +348,30 @@ func mutate(f MutateFn, key client.ObjectKey, obj client.Object) error {
 // MutateFn is a function which mutates the existing object into it's desired state.
 type MutateFn func() error
 
-// AddFinalizer accepts an Object and adds the provided finalizer if not present.
-func AddFinalizer(o client.Object, finalizer string) {
+// AddFinalizer accepts an Object and adds the provided finalizer and returns true if not present.
+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) {
+// RemoveFinalizer accepts an Object and removes the provided finalizer and returns true if present.
+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.

pkg/controller/controllerutil/controllerutil_test.go

@@ -700,6 +700,42 @@ var _ = Describe("Controllerutil", func() {
 			})
 		})
 
+		Describe("AddFinalizer", func() {
+			deploy = &appsv1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Finalizers: []string{},
+				},
+			}
+
+			It("should add the finalizer when not present", func() {
+				Expect(controllerutil.AddFinalizer(deploy, testFinalizer)).To(BeTrue())
+				Expect(deploy.ObjectMeta.GetFinalizers()).To(Equal([]string{testFinalizer}))
+			})
+
+			It("should not add the finalizer when already present", func() {
+				Expect(controllerutil.AddFinalizer(deploy, testFinalizer)).To(BeFalse())
+				Expect(deploy.ObjectMeta.GetFinalizers()).To(Equal([]string{testFinalizer}))
+			})
+		})
+
+		Describe("RemoveFinalizer", func() {
+			It("should not remove a finalizer not present", func() {
+				Expect(controllerutil.RemoveFinalizer(deploy, testFinalizer1)).To(BeFalse())
+				Expect(deploy.ObjectMeta.GetFinalizers()).To(Equal([]string{testFinalizer}))
+			})
+
+			It("should remove finalizer if present", func() {
+				Expect(controllerutil.RemoveFinalizer(deploy, testFinalizer)).To(BeTrue())
+				Expect(deploy.ObjectMeta.GetFinalizers()).To(Equal([]string{}))
+			})
+
+			It("should remove all equal finalizers if present", func() {
+				deploy.SetFinalizers(append(deploy.Finalizers, testFinalizer, testFinalizer))
+				Expect(controllerutil.RemoveFinalizer(deploy, testFinalizer)).To(BeTrue())
+				Expect(deploy.ObjectMeta.GetFinalizers()).To(Equal([]string{}))
+			})
+		})
+
 		Describe("ContainsFinalizer", func() {
 			It("should check that finalizer is present", func() {
 				controllerutil.AddFinalizer(deploy, testFinalizer)
@@ -715,6 +751,7 @@ var _ = Describe("Controllerutil", func() {
 })
 
 const testFinalizer = "foo.bar.baz"
+const testFinalizer1 = testFinalizer + "1"
 
 var _ runtime.Object = &errRuntimeObj{}
 var _ metav1.Object = &errMetaObj{}