WIP: unit tests

Signed-off-by: Rashmi Gottipati <[email protected]>

Author: rashmigottipati

pkg/finalizer/finalizer.go

@@ -21,8 +21,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
-func NewFinalizers() (Finalizers, error) {
-	return finalizers(map[string]Finalizer{}), nil
+// NewFinalizers returns the Finalizers interface
+func NewFinalizers() Finalizers {
+	return finalizers(map[string]Finalizer{})
 }
 
 func (f finalizers) Register(key string, finalizer Finalizer) error {
@@ -36,13 +37,13 @@ func (f finalizers) Register(key string, finalizer Finalizer) error {
 func (f finalizers) Finalize(ctx context.Context, obj client.Object) (bool, error) {
 	needsUpdate := false
 	for key, finalizer := range f {
-		if obj.GetDeletionTimestamp() == nil && !controllerutil.ContainsFinalizer(obj, key) {
+		if obj.GetDeletionTimestamp().IsZero() && !controllerutil.ContainsFinalizer(obj, key) {
 			controllerutil.AddFinalizer(obj, key)
 			needsUpdate = true
 		} else if obj.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(obj, key) {
-			_, err := finalizer.Finalize(ctx, obj)
+			needsUpdate, err := finalizer.Finalize(ctx, obj)
 			if err != nil {
-				return true, fmt.Errorf("finalize failed for key %q: %v", key, err)
+				return needsUpdate, fmt.Errorf("finalize failed for key %q: %v", key, err)
 			}
 			controllerutil.RemoveFinalizer(obj, key)
 			needsUpdate = true

pkg/finalizer/finalizer_test.go

@@ -0,0 +1,92 @@
+package finalizer
+
+import (
+	"context"
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
+)
+
+type mockFinalizer struct{}
+
+func (f mockFinalizer) Finalize(context.Context, client.Object) (needsUpdate bool, err error) {
+	return true, nil
+}
+func TestFinalizer(t *testing.T) {
+	RegisterFailHandler(Fail)
+	suiteName := "Finalizer Suite"
+	RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)})
+}
+
+var _ = Describe("TestFinalizer", func() {
+	var err error
+	var pod *corev1.Pod
+	var finalizers Finalizers
+	var f mockFinalizer
+	BeforeEach(func() {
+		pod = &corev1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Finalizers: []string{"finalizers.sigs.k8s.io/testfinalizer"},
+			},
+		}
+		Expect(pod).NotTo(BeNil())
+
+		finalizers = NewFinalizers()
+		Expect(finalizers).NotTo(BeNil())
+
+		f := mockFinalizer{}
+		Expect(f).NotTo(BeNil())
+
+	})
+	Describe("Finalizer helper library", func() {
+		It("Register should successfully register a finalizer", func() {
+			err = finalizers.Register("finalizers.sigs.k8s.io/testfinalizer", f)
+			Expect(err).To(BeNil())
+		})
+
+		It("Register should return an error when it is called twice with the same key", func() {
+			err = finalizers.Register("finalizers.sigs.k8s.io/testfinalizer", f)
+			Expect(err).To(BeNil())
+
+			// calling Register again with the same key should return an error
+			err = finalizers.Register("finalizers.sigs.k8s.io/testfinalizer", f)
+			Expect(err).NotTo(BeNil())
+			Expect(err.Error()).To(ContainSubstring("already registered"))
+
+		})
+
+		It("Finalize should return no error and indicate false for needsUpdate if a finalizer is not registered", func() {
+			ret, err := finalizers.Finalize(context.TODO(), pod)
+			Expect(err).To(BeNil())
+			Expect(ret).To(BeFalse())
+		})
+
+		It("Finalize should return no error when deletion timestamp is not nil and the finalizer exists", func() {
+			now := metav1.Now()
+			pod.DeletionTimestamp = &now
+
+			err = finalizers.Register("finalizers.sigs.k8s.io/testfinalizer", f)
+			Expect(err).To(BeNil())
+
+			_, err := finalizers.Finalize(context.TODO(), pod)
+			Expect(err).To(BeNil())
+		})
+
+		It("Finalize should return no error when deletion timestamp is nil and finalizer does not exist", func() {
+			err = finalizers.Register("finalizers.sigs.k8s.io/testfinalizer", f)
+			Expect(err).To(BeNil())
+
+			pod.DeletionTimestamp = nil
+			pod.Finalizers = []string{}
+
+			ret, err := finalizers.Finalize(context.TODO(), pod)
+			Expect(err).To(BeNil())
+			Expect(ret).To(BeTrue())
+		})
+	})
+})

pkg/finalizer/types.go

@@ -19,21 +19,26 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+// Registerer holds Register that will check if a key is already registered
+// and error out and it does; and if not registered, it will add the finalizer
+// to the finalizers map as the value for the provided key
 type Registerer interface {
 	Register(key string, f Finalizer) error
 }
 
+// Finalizer holds Finalize that will add/remove a finalizer based on the
+// deletion timestamp being set and return an indication of whether the
+// obj needs an update or not
 type Finalizer interface {
 	Finalize(context.Context, client.Object) (needsUpdate bool, err error)
 }
 
 type finalizers map[string]Finalizer
 
+// Finalizers implements Registerer and Finalizer to finalize all registered
+// finalizers if the provided object has a deletion timestamp or set all
+// registered finalizers if it does not
 type Finalizers interface {
-	// implements Registerer and Finalizer to finalize
-	// all registered finalizers if the provided object
-	// has a deletion timestamp or set all registered
-	// finalizers if it doesn't
 	Registerer
 	Finalizer
 }