Addressed the review comments. Included tidying up the code up to

standard and refactoring some fucntions.

Author: Ali Syed

pkg/operator/controller/gatewayapi-upgradeable/controller.go

@@ -10,7 +10,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
-	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
@@ -30,11 +29,9 @@ var (
 )
 
 // New function initializes the controller and sets up the watch for the ConfigMap
-func New(mgr manager.Manager, config Config) (controller.Controller, error) {
-	operatorCache := mgr.GetCache()
+func New(mgr manager.Manager) (controller.Controller, error) {
 	reconciler := &reconciler{
-		cache:  config.Cache,
-		client: mgr.GetClient(),
+		cache: mgr.GetCache(), // Directly using mgr.GetCache()
 	}
 
 	//Create a new controller with given reconciler
@@ -46,50 +43,33 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 	}
 
 	// Define a predicate filter to watch for specific changes in the ConfigMap
-	adminGateFilter := predicate.Funcs{
-		CreateFunc: func(e event.CreateEvent) bool {
-			// Watch for creation of the "admin-gates" ConfigMap in "openshift-config-managed" namespace
-			configmap := e.Object.(*corev1.ConfigMap)
-			return configmap.Name == operatorcontroller.AdminGatesConfigMapName().Name && configmap.Namespace == operatorcontroller.AdminGatesConfigMapName().Namespace
-		},
-		DeleteFunc: func(deleteEvent event.DeleteEvent) bool { return false }, // Ignore delete events
-		UpdateFunc: func(e event.UpdateEvent) bool {
-			// Watch for updates to the "admin-gates" ConfigMap to check if the admin key changes
-			configmapOld := e.ObjectOld.(*corev1.ConfigMap)
-			configmapNew := e.ObjectNew.(*corev1.ConfigMap)
-			if configmapOld.Name != operatorcontroller.AdminGatesConfigMapName().Name || configmapOld.Namespace != operatorcontroller.AdminGatesConfigMapName().Namespace {
-				return false
-			}
-			oldVal, oldExists := configmapOld.Data[GatewayAPIAdminKey]
-			newVal, newExists := configmapNew.Data[GatewayAPIAdminKey]
-			return oldExists != newExists || oldVal != newVal
-		},
-		GenericFunc: func(genericEvent event.GenericEvent) bool { return false }, // Ignore generic events
-	}
+	// Check that ConfigMap's name and namespace match expected values
+	adminGatePredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
+		cm := o.(*corev1.ConfigMap)
+		return cm.Name == operatorcontroller.AdminGatesConfigMapName().Name && cm.Namespace == operatorcontroller.AdminGatesConfigMapName().Namespace
+	})
 
+	//Map events to the admin gate ConfigMap
 	toAdminGateConfigMap := func(_ context.Context, _ client.Object) []reconcile.Request {
 		return []reconcile.Request{{
 			NamespacedName: operatorcontroller.AdminGatesConfigMapName(),
 		}}
 	}
+
 	// Define the CRD predicate
 	crdPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
 		return o.(*apiextensionsv1.CustomResourceDefinition).Spec.Group == gatewayapiv1.GroupName
 	})
 
-	//TODO: too many args to c.watch and not enough to source.Kind
-	if err := c.Watch(source.Kind(operatorCache, &corev1.ConfigMap{}),
-		handler.EnqueueRequestsFromMapFunc(toAdminGateConfigMap),
-		adminGateFilter); err != nil {
+	// Set up a single watch for CRD events and map to admin gate ConfigMap
+	if err := c.Watch(source.Kind[client.Object](mgr.GetCache(), &apiextensionsv1.CustomResourceDefinition{}, handler.EnqueueRequestsFromMapFunc(toAdminGateConfigMap), crdPredicate)); err != nil {
 		return nil, err
 	}
 
-	// Watch for CRD updates using the reconciler cache and apply the CRD predicate
-	if err := c.Watch(source.Kind(reconciler.cache, &apiextensionsv1.CustomResourceDefinition{}),
-		&handler.EnqueueRequestForObject{},
-		crdPredicate); err != nil {
+	if err := c.Watch(source.Kind[client.Object](mgr.GetCache(), &corev1.ConfigMap{}, &handler.EnqueueRequestForObject{}, adminGatePredicate)); err != nil {
 		return nil, err
 	}
+
 	return c, nil
 }
 
@@ -99,21 +79,16 @@ type reconciler struct {
 	cache  cache.Cache
 }
 
-// Config struct holds the cache information
-type Config struct {
-	//namespace referes to namespace where admin gates controller resides
-	Namespace string
-	//cache should be a namespace global cache
-	Cache cache.Cache
-}
-
 // Reconcile function implements the logic to check conditions and manage the admin gate
 func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
 	log.Info("reconciling", "request", request)
+
+	// Check if the admin gate should be added or removed
 	adminGateConditionExists, err := r.adminGateConditionExists(ctx)
 	if err != nil {
-		return reconcile.Result{}, fmt.Errorf("failed to determine if UnservableInFutureVersionsRoute routes exist: %w", err)
+		return reconcile.Result{}, fmt.Errorf("failed to determine if admin gate exist: %w", err)
 	}
+
 	if adminGateConditionExists {
 		if err := r.addAdminGate(ctx); err != nil {
 			return reconcile.Result{}, fmt.Errorf("failed to add admin gate: %w", err)
@@ -127,15 +102,31 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
 	return reconcile.Result{}, nil
 }
 
-// adminGateConditionExists is a placeholder for now,
-// TODO: adminGatesConditionExists needs to be implemented....
-func (r *reconciler) adminGateConditionExists(ctx context.Context, condition string) (bool, error) {
+// adminGateConditionExists checks if the admin gate condition exists based on both ConfigMap and CRDs
+func (r *reconciler) adminGateConditionExists(ctx context.Context) (bool, error) {
+	adminGateConfigMap := &corev1.ConfigMap{}
+	if err := r.cache.Get(ctx, operatorcontroller.AdminGatesConfigMapName(), adminGateConfigMap); err != nil {
+		return false, fmt.Errorf("failed to get configmap %s: %w", operatorcontroller.AdminGatesConfigMapName(), err)
+	}
+
+	// Check if the admin key exists and is set to the expected message
+	if val, ok := adminGateConfigMap.Data[GatewayAPIAdminKey]; ok && val == GatewayAPIAdminMsg {
+		return true, nil // Exists as expected
+	}
 
-	// Placeholder: Attempt to list ConfigMaps
-	// Placeholder: Iterate through ConfigMap list and check for the condition (to be implemented).
+	// Check for the presence of Gateway API CRDs
+	crds := &apiextensionsv1.CustomResourceDefinitionList{}
+	if err := r.client.List(ctx, crds); err != nil {
+		return false, fmt.Errorf("failed to list CRDs: %w", err)
+	}
+
+	for _, crd := range crds.Items {
+		if crd.Spec.Group == gatewayapiv1.GroupName {
+			return true, nil
+		}
+	}
 
-	// Placeholder return statement, to be replaced with actual logic.
-	return false, nil
+	return false, nil // No Gateway API CRDs found
 }
 
 // addAdminGate function adds the admin gate to the ConfigMap

pkg/operator/controller/gatewayapi/controller.go

@@ -83,9 +83,6 @@ type Config struct {
 	// resources.  The gatewayapi controller starts these controllers once
 	// the Gateway API CRDs have been created.
 	DependentControllers []controller.Controller
-
-	//AdminConsentRequired indicates
-	AdminConsentRequired bool
 }
 
 // reconciler reconciles gatewayclasses.

pkg/operator/controller/names.go

@@ -52,7 +52,7 @@ const (
 func AdminGatesConfigMapName() types.NamespacedName {
 	return types.NamespacedName{
 		Name:      "admin-gates",
-		Namespace: "openshift-config",
+		Namespace: GlobalMachineSpecifiedConfigNamespace,
 	}
 }
 

pkg/operator/operator.go

@@ -3,7 +3,6 @@ package operator
 import (
 	"context"
 	"fmt"
-	gatewayapi_upgradeable "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/gatewayapi-upgradeable"
 	"time"
 
 	configclient "github.com/openshift/client-go/config/clientset/versioned"
@@ -36,6 +35,7 @@ import (
 	dnscontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/dns"
 	gatewayservicednscontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/gateway-service-dns"
 	gatewayapicontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/gatewayapi"
+	gatewayapi_upgradeable "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/gatewayapi-upgradeable"
 	gatewayclasscontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/gatewayclass"
 	ingress "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/ingress"
 	ingresscontroller "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/ingress"
@@ -320,10 +320,8 @@ func New(config operatorconfig.Config, kubeConfig *rest.Config) (*Operator, erro
 		return nil, fmt.Errorf("failed to create gatewayapi controller: %w", err)
 	}
 
-	//set up the gateway api upgradeable controller
-	if _, err := gatewayapi_upgradeable.New(mgr, gatewayapi_upgradeable.Config{
-		Cache: mgr.GetCache(),
-	}); err != nil {
+	// Set up the gateway api upgradeable controller
+	if _, err := gatewayapi_upgradeable.New(mgr); err != nil {
 		return nil, fmt.Errorf("failed to create gatewayapi upgradeable controller: %w", err)
 	}