Add additional unit-tests for Conditions (#92)

* Add additional unit-tests for Conditions

Make sure each condition is defined (as best we can)
Enhancements to #91

Signed-off-by: Todd Short <[email protected]>

Author: tmshort

Dockerfile

@@ -15,6 +15,7 @@ RUN go mod download
 COPY main.go main.go
 COPY api/ api/
 COPY controllers/ controllers/
+COPY internal/ internal/
 
 # Build
 # the GOARCH has not a default value to allow the binary be built according to the host where the command

api/v1alpha1/operator_types.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1alpha1
 
 import (
+	operatorutil "github.com/operator-framework/operator-controller/internal/util"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -28,13 +29,24 @@ type OperatorSpec struct {
 }
 
 const (
-	// TODO(user): add more Types
+	// TODO(user): add more Types, here and into init()
 	TypeReady = "Ready"
 
-	// TODO(user): add more Reasons
+	// TODO(user): add more Reasons, here and into init()
 	ReasonNotImplemented = "NotImplemented"
 )
 
+func init() {
+	// TODO(user): add Types from above
+	operatorutil.ConditionTypes = append(operatorutil.ConditionTypes,
+		TypeReady,
+	)
+	// TODO(user): add Reasons from above
+	operatorutil.ConditionReasons = append(operatorutil.ConditionReasons,
+		ReasonNotImplemented,
+	)
+}
+
 // OperatorStatus defines the observed state of Operator
 type OperatorStatus struct {
 	// +patchMergeKey=type

controllers/operator_controller.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2022.
+Copyright 2023.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -66,20 +66,15 @@ func (r *OperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	// Do checks before any Update()s, as Update() may modify the resource structure!
 	updateStatus := !equality.Semantic.DeepEqual(existingOp.Status, reconciledOp.Status)
 	updateFinalizers := !equality.Semantic.DeepEqual(existingOp.Finalizers, reconciledOp.Finalizers)
-
-	// Compare resources - ignoring status & metadata.finalizers
-	compareOp := reconciledOp.DeepCopy()
-	existingOp.Status, compareOp.Status = operatorsv1alpha1.OperatorStatus{}, operatorsv1alpha1.OperatorStatus{}
-	existingOp.Finalizers, compareOp.Finalizers = []string{}, []string{}
-	specDiffers := !equality.Semantic.DeepEqual(existingOp, compareOp)
+	unexpectedFieldsChanged := checkForUnexpectedFieldChange(*existingOp, *reconciledOp)
 
 	if updateStatus {
 		if updateErr := r.Status().Update(ctx, reconciledOp); updateErr != nil {
 			return res, utilerrors.NewAggregate([]error{reconcileErr, updateErr})
 		}
 	}
 
-	if specDiffers {
+	if unexpectedFieldsChanged {
 		panic("spec or metadata changed by reconciler")
 	}
 
@@ -92,6 +87,13 @@ func (r *OperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	return res, reconcileErr
 }
 
+// Compare resources - ignoring status & metadata.finalizers
+func checkForUnexpectedFieldChange(a, b operatorsv1alpha1.Operator) bool {
+	a.Status, b.Status = operatorsv1alpha1.OperatorStatus{}, operatorsv1alpha1.OperatorStatus{}
+	a.Finalizers, b.Finalizers = []string{}, []string{}
+	return !equality.Semantic.DeepEqual(a, b)
+}
+
 // Helper function to do the actual reconcile
 func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha1.Operator) (ctrl.Result, error) {
 

controllers/suite_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2022.
+Copyright 2023.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/rand"
@@ -38,6 +39,7 @@ import (
 
 	operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
 	"github.com/operator-framework/operator-controller/controllers"
+	operatorutil "github.com/operator-framework/operator-controller/internal/util"
 	//+kubebuilder:scaffold:imports
 )
 
@@ -127,18 +129,47 @@ var _ = Describe("Reconcile Test", func() {
 			err = k8sClient.Delete(ctx, operator)
 			Expect(err).To(Not(HaveOccurred()))
 		})
-		It("has a Condition created", func() {
-			getOperator := &operatorsv1alpha1.Operator{}
+		It("has all Conditions created", func() {
+			op := &operatorsv1alpha1.Operator{}
 
 			err = k8sClient.Get(ctx, client.ObjectKey{
 				Name: opName,
-			}, getOperator)
+			}, op)
 			Expect(err).To(Not(HaveOccurred()))
 
-			// There should always be a "Ready" condition, regardless of Status.
-			conds := getOperator.Status.Conditions
+			// All defined condition Types MUST exist after reconciliation
+			conds := op.Status.Conditions
 			Expect(conds).To(Not(BeEmpty()))
-			Expect(conds).To(ContainElement(HaveField("Type", operatorsv1alpha1.TypeReady)))
+			Expect(conds).To(HaveLen(len(operatorutil.ConditionTypes)))
+			for _, t := range operatorutil.ConditionTypes {
+				Expect(apimeta.FindStatusCondition(conds, t)).To(Not(BeNil()))
+			}
+		})
+		It("has matching generations in Conditions", func() {
+			op := &operatorsv1alpha1.Operator{}
+
+			err = k8sClient.Get(ctx, client.ObjectKey{
+				Name: opName,
+			}, op)
+			Expect(err).To(Not(HaveOccurred()))
+
+			// The ObservedGeneration MUST match the resource generation after reconciliation
+			for _, c := range op.Status.Conditions {
+				Expect(c.ObservedGeneration).To(Equal(op.GetGeneration()))
+			}
+		})
+		It("has only pre-defined Reasons", func() {
+			op := &operatorsv1alpha1.Operator{}
+
+			err = k8sClient.Get(ctx, client.ObjectKey{
+				Name: opName,
+			}, op)
+			Expect(err).To(Not(HaveOccurred()))
+
+			// A given Reason MUST be in the list of ConditionReasons
+			for _, c := range op.Status.Conditions {
+				Expect(c.Reason).To(BeElementOf(operatorutil.ConditionReasons))
+			}
 		})
 	})
 })

internal/util/util.go

@@ -0,0 +1,24 @@
+/*
+Copyright 2023.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+// ConditionTypes is the full set of Operator condition Types.
+// ConditionReasons is the full set of Operator condition Reasons.
+//
+// NOTE: These are populated by init() in api/v1alpha1/operator_types.go
+var ConditionTypes []string
+var ConditionReasons []string