Update: Improve get kuadrant resource (#910)

* Update: Improve get kuadrant resource Updates the getting of the kuadrant CR to use the topology as the input data structure. This adds consistency to how others will get the kuadrant CR. --------- Signed-off-by: Jim Fitzpatrick <jfitzpat@redhat.com>
Kuadrant · Oct 14, 2024 · 60d5d43 · 60d5d43
1 parent cd7f9c2
commit 60d5d43
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 178 deletions.
diff --git a/controllers/authorino_reconciler.go b/controllers/authorino_reconciler.go
@@ -2,14 +2,14 @@ package controllers
 
 import (
 	"context"
-	"strings"
+	"errors"
 	"sync"
 
 	v1beta2 "github.com/kuadrant/authorino-operator/api/v1beta1"
 	"github.com/kuadrant/policy-machinery/controller"
 	"github.com/kuadrant/policy-machinery/machinery"
 	"github.com/samber/lo"
-	"k8s.io/apimachinery/pkg/api/errors"
+	apiErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/utils/ptr"
@@ -40,16 +40,9 @@ func (r *AuthorinoReconciler) Reconcile(ctx context.Context, _ []controller.Reso
 	logger.Info("reconciling authorino resource", "status", "started")
 	defer logger.Info("reconciling authorino resource", "status", "completed")
 
-	kobjs := lo.FilterMap(topology.Objects().Roots(), func(item machinery.Object, _ int) (*v1beta1.Kuadrant, bool) {
-		if item.GroupVersionKind().Kind == v1beta1.KuadrantGroupKind.Kind {
-			return item.(*v1beta1.Kuadrant), true
-		}
-		return nil, false
-	})
-
-	kobj, err := GetOldestKuadrant(kobjs)
+	kobj, err := GetKuadrant(topology)
 	if err != nil {
-		if strings.Contains(err.Error(), "empty list passed") {
+		if errors.Is(err, ErrNoKandrantResource) {
 			logger.Info("kuadrant resource not found, ignoring", "status", "skipping")
 			return err
 		}
@@ -111,7 +104,7 @@ func (r *AuthorinoReconciler) Reconcile(ctx context.Context, _ []controller.Reso
 	logger.Info("creating authorino resource", "status", "processing")
 	_, err = r.Client.Resource(v1beta1.AuthorinosResource).Namespace(authorino.Namespace).Create(ctx, unstructuredAuthorino, metav1.CreateOptions{})
 	if err != nil {
-		if errors.IsAlreadyExists(err) {
+		if apiErrors.IsAlreadyExists(err) {
 			logger.Info("already created authorino resource", "status", "acceptable")
 		} else {
 			logger.Error(err, "failed to create authorino resource", "status", "error")

diff --git a/controllers/limitador_reconciler.go b/controllers/limitador_reconciler.go
@@ -2,14 +2,14 @@ package controllers
 
 import (
 	"context"
-	"strings"
+	"errors"
 	"sync"
 
 	limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1"
 	"github.com/kuadrant/policy-machinery/controller"
 	"github.com/kuadrant/policy-machinery/machinery"
 	"github.com/samber/lo"
-	"k8s.io/apimachinery/pkg/api/errors"
+	apiErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/utils/ptr"
@@ -41,20 +41,13 @@ func (r *LimitadorReconciler) Reconcile(ctx context.Context, _ []controller.Reso
 	logger.Info("reconciling limtador resource", "status", "started")
 	defer logger.Info("reconciling limitador resource", "status", "completed")
 
-	kobjs := lo.FilterMap(topology.Objects().Roots(), func(item machinery.Object, _ int) (*v1beta1.Kuadrant, bool) {
-		if item.GroupVersionKind().Kind == v1beta1.KuadrantGroupKind.Kind {
-			return item.(*v1beta1.Kuadrant), true
-		}
-		return nil, false
-	})
-
-	kobj, err := GetOldestKuadrant(kobjs)
+	kobj, err := GetKuadrant(topology)
 	if err != nil {
-		if strings.Contains(err.Error(), "empty list passed") {
+		if errors.Is(err, ErrNoKandrantResource) {
 			logger.Info("kuadrant resource not found, ignoring", "status", "skipping")
-			return nil
+			return err
 		}
-		logger.Error(err, "cannont find kuadrant resource", "status", "error")
+		logger.Error(err, "cannot find Kuadrant resource", "status", "error")
 		return err
 	}
 
@@ -100,7 +93,7 @@ func (r *LimitadorReconciler) Reconcile(ctx context.Context, _ []controller.Reso
 	logger.Info("creating limitador resource", "status", "processing")
 	_, err = r.Client.Resource(v1beta1.LimitadorsResource).Namespace(limitador.Namespace).Create(ctx, unstructuredLimitador, metav1.CreateOptions{})
 	if err != nil {
-		if errors.IsAlreadyExists(err) {
+		if apiErrors.IsAlreadyExists(err) {
 			logger.Info("already created limitador resource", "status", "acceptable")
 		} else {
 			logger.Error(err, "failed to create limitador resource", "status", "error")

diff --git a/controllers/state_of_the_world.go b/controllers/state_of_the_world.go
@@ -3,14 +3,17 @@ package controllers
 import (
 	"fmt"
 	"reflect"
+	"sort"
 
 	certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	egv1alpha1 "github.com/envoyproxy/gateway/api/v1alpha1"
 	"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"
 	consolev1 "github.com/openshift/api/console/v1"
+	"github.com/samber/lo"
 	istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1"
 	istioclientnetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
 	istioclientgosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
@@ -404,31 +407,23 @@ func finalStepsWorkflow(client *dynamic.DynamicClient, isIstioInstalled, isEnvoy
 	return workflow
 }
 
-// GetOldestKuadrant returns the oldest kuadrant resource from a list of kuadrant resources that is not marked for deletion.
-func GetOldestKuadrant(kuadrants []*kuadrantv1beta1.Kuadrant) (*kuadrantv1beta1.Kuadrant, error) {
-	if len(kuadrants) == 1 {
-		return kuadrants[0], nil
-	}
-	if len(kuadrants) == 0 {
-		return nil, fmt.Errorf("empty list passed")
-	}
-	oldest := kuadrants[0]
-	for _, k := range kuadrants[1:] {
-		if k == nil || k.DeletionTimestamp != nil {
-			continue
-		}
-		if oldest == nil {
-			oldest = k
-			continue
-		}
-		if k.CreationTimestamp.Before(&oldest.CreationTimestamp) {
-			oldest = k
+var ErrNoKandrantResource = fmt.Errorf("no kuadrant resources in topology")
+
+// GetKuadrant returns the oldest Kuadrant from the root objects in the topology
+func GetKuadrant(topology *machinery.Topology) (*kuadrantv1beta1.Kuadrant, error) {
+	kuadrantList := lo.FilterMap(topology.Objects().Roots(), func(item machinery.Object, _ int) (controller.Object, bool) {
+		k, ok := item.(controller.Object)
+		if ok && k.GetObjectKind().GroupVersionKind().GroupKind() == kuadrantv1beta1.KuadrantGroupKind && k.GetDeletionTimestamp() == nil {
+			return k, true
 		}
+		return nil, false
+	})
+	if len(kuadrantList) == 0 {
+		return nil, ErrNoKandrantResource
 	}
-	if oldest == nil {
-		return nil, fmt.Errorf("only nil pointers in list")
-	}
-	return oldest, nil
+	sort.Sort(controller.ObjectsByCreationTimestamp(kuadrantList))
+	k, _ := kuadrantList[0].(*kuadrantv1beta1.Kuadrant)
+	return k, nil
 }
 
 func isObjectOwnedByGroupKind(o client.Object, groupKind schema.GroupKind) bool {