fix: incorrect kuadrant status if limitador/authorino is not found (#…

…1000) * fix: incorrect kuadrant status if limitador/authorino is not found Signed-off-by: KevFan <[email protected]> * test: kuadrant status on bare k8s Signed-off-by: KevFan <[email protected]> * refactor: include ErrMissingAuthorino in kuadrant status Signed-off-by: KevFan <[email protected]> --------- Signed-off-by: KevFan <[email protected]>
Kuadrant · Nov 12, 2024 · ccd1a97 · ccd1a97
1 parent 365b170
commit ccd1a97
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 45 deletions.
diff --git a/controllers/kuadrant_status_updater.go b/controllers/kuadrant_status_updater.go
@@ -2,14 +2,11 @@ package controllers
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"slices"
 	"sync"
 
 	"github.com/go-logr/logr"
-	authorinov1beta1 "github.com/kuadrant/authorino-operator/api/v1beta1"
-	limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1"
 	"github.com/kuadrant/policy-machinery/controller"
 	"github.com/kuadrant/policy-machinery/machinery"
 	corev1 "k8s.io/api/core/v1"
@@ -128,43 +125,33 @@ func (r *KuadrantStatusUpdater) readyCondition(topology *machinery.Topology, log
 		return cond
 	}
 
-	limitadorObj, err := GetLimitadorFromTopology(topology)
-	if err != nil && !errors.Is(err, ErrMissingLimitador) {
-		logger.V(1).Error(err, "failed getting Limitador resource from topology", "status", "error")
-	}
-
-	if limitadorObj != nil {
-		reason := checkLimitadorReady(limitadorObj)
-		if reason != nil {
-			cond.Status = metav1.ConditionFalse
-			cond.Reason = "limitadornotready"
-			cond.Message = *reason
-			return cond
-		}
+	if reason := checkLimitadorReady(topology, logger); reason != nil {
+		cond.Status = metav1.ConditionFalse
+		cond.Reason = "LimitadorNotReady"
+		cond.Message = *reason
+		return cond
 	}
 
-	authorinoObj, err := GetAuthorinoFromTopology(topology)
-	if err != nil && !errors.Is(err, ErrMissingAuthorino) {
-		logger.V(1).Error(err, "failed getting Authorino resource from topology", "status", "error")
+	if reason := checkAuthorinoAvailable(topology, logger); reason != nil {
+		cond.Status = metav1.ConditionFalse
+		cond.Reason = "AuthorinoNotReady"
+		cond.Message = *reason
+		return cond
 	}
 
-	if authorinoObj != nil {
-		reason := checkAuthorinoAvailable(authorinoObj)
-		if reason != nil {
-			cond.Status = metav1.ConditionFalse
-			cond.Reason = "AuthorinoNotReady"
-			cond.Message = *reason
-			return cond
-		}
-	}
 	return cond
 }
 
-func checkLimitadorReady(limitadorObj *limitadorv1alpha1.Limitador) *string {
+func checkLimitadorReady(topology *machinery.Topology, logger logr.Logger) *string {
+	limitadorObj, err := GetLimitadorFromTopology(topology)
+	if err != nil {
+		logger.V(1).Error(err, "failed getting Limitador resource from topology", "status", "error")
+		return ptr.To(err.Error())
+	}
+
 	statusConditionReady := meta.FindStatusCondition(limitadorObj.Status.Conditions, "Ready")
 	if statusConditionReady == nil {
-		reason := "Ready condition not found"
-		return &reason
+		return ptr.To("Ready condition not found")
 	}
 	if statusConditionReady.Status != metav1.ConditionTrue {
 		return &statusConditionReady.Message
@@ -173,11 +160,16 @@ func checkLimitadorReady(limitadorObj *limitadorv1alpha1.Limitador) *string {
 	return nil
 }
 
-func checkAuthorinoAvailable(authorinoObj *authorinov1beta1.Authorino) *string {
+func checkAuthorinoAvailable(topology *machinery.Topology, logger logr.Logger) *string {
+	authorinoObj, err := GetAuthorinoFromTopology(topology)
+	if err != nil {
+		logger.V(1).Error(err, "failed getting Authorino resource from topology", "status", "error")
+		return ptr.To(err.Error())
+	}
+
 	readyCondition := authorino.FindAuthorinoStatusCondition(authorinoObj.Status.Conditions, "Ready")
 	if readyCondition == nil {
-		tmp := "Ready condition not found"
-		return &tmp
+		return ptr.To("Ready condition not found")
 	}
 
 	if readyCondition.Status != corev1.ConditionTrue {

diff --git a/tests/bare_k8s/kuadrant_controller_test.go b/tests/bare_k8s/kuadrant_controller_test.go
@@ -7,18 +7,20 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1"
+	"github.com/kuadrant/kuadrant-operator/controllers"
 	"github.com/kuadrant/kuadrant-operator/tests"
 )
 
-var _ = Describe("Kuadrant controller is disabled", func() {
+var _ = Describe("Controller", func() {
 	var (
 		testNamespace    string
-		kuadrantName     string = "local"
-		afterEachTimeOut        = NodeTimeout(3 * time.Minute)
+		testTimeOut      = SpecTimeout(15 * time.Second)
+		afterEachTimeOut = NodeTimeout(3 * time.Minute)
 	)
 
 	BeforeEach(func(ctx SpecContext) {
@@ -30,24 +32,28 @@ var _ = Describe("Kuadrant controller is disabled", func() {
 	}, afterEachTimeOut)
 
 	Context("when default kuadrant CR is created", func() {
-		It("Status is not populated", func(ctx SpecContext) {
+		It("Status is populated with missing GatewayProvide", func(ctx SpecContext) {
 			kuadrantCR := &kuadrantv1beta1.Kuadrant{
 				TypeMeta: metav1.TypeMeta{
 					Kind:       "Kuadrant",
 					APIVersion: kuadrantv1beta1.GroupVersion.String(),
 				},
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      kuadrantName,
+					Name:      "local",
 					Namespace: testNamespace,
 				},
 			}
 			Expect(testClient().Create(ctx, kuadrantCR)).ToNot(HaveOccurred())
 
-			kObj := &kuadrantv1beta1.Kuadrant{}
-			err := testClient().Get(ctx, client.ObjectKeyFromObject(kuadrantCR), kObj)
-			Expect(err).ToNot(HaveOccurred())
-			// expected empty. The controller should not have updated it
-			Expect(kObj.Status).To(Equal(kuadrantv1beta1.KuadrantStatus{}))
-		})
+			Eventually(func(g Gomega) {
+				g.Expect(testClient().Get(ctx, client.ObjectKeyFromObject(kuadrantCR), kuadrantCR)).To(Succeed())
+
+				cond := meta.FindStatusCondition(kuadrantCR.Status.Conditions, controllers.ReadyConditionType)
+				g.Expect(cond).ToNot(BeNil())
+				g.Expect(cond.Status).To(Equal(metav1.ConditionFalse))
+				g.Expect(cond.Reason).To(Equal("GatewayAPIProviderNotFound"))
+				g.Expect(cond.Message).To(Equal("GatewayAPI provider not found"))
+			}).WithContext(ctx).Should(Succeed())
+		}, testTimeOut)
 	})
 })