OCPBUGS-54745: status: Conditionally add CRDs to relatedObjects

pkg/operator/controller/status/controller.go

@@ -24,9 +24,11 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
 	utilclock "k8s.io/utils/clock"
 
 	gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
@@ -50,13 +52,27 @@ const (
 
 	ingressesEqualConditionMessage = "desired and current number of IngressControllers are equal"
 
+	// gatewaysResourceName is the name of the Gateway API gateways CRD.
+	gatewaysResourceName = "gateways.gateway.networking.k8s.io"
+	// gatewayclassesResourceName is the name of the Gateway API
+	// gatewayclasses CRD.
+	gatewayclassesResourceName = "gatewayclasses.gateway.networking.k8s.io"
+	// istiosResourceName is the name of the Sail Operator istios CRD.
+	istiosResourceName = "istios.sailoperator.io"
+
 	controllerName = "status_controller"
 )
 
-var log = logf.Logger.WithName(controllerName)
+var (
+	log = logf.Logger.WithName(controllerName)
+
+	// clock is to enable unit testing
+	clock utilclock.Clock = utilclock.RealClock{}
 
-// clock is to enable unit testing
-var clock utilclock.Clock = utilclock.RealClock{}
+	// relatedObjectsCRDs is a set of names of CRDs that we add to
+	// relatedObjects if they exist.
+	relatedObjectsCRDs = sets.New[string](gatewaysResourceName, gatewayclassesResourceName, istiosResourceName)
+)
 
 // New creates the status controller. This is the controller that handles all
 // the logic for creating the ClusterOperator operator and updating its status.
@@ -99,15 +115,15 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 		return nil, err
 	}
 
-	// If the "GatewayAPI" controller featuregate is enabled, watch
-	// subscriptions so that this controller can update status when the OSSM
-	// subscription is created or updated.  Note that the subscriptions
-	// resource only exists if the "OperatorLifecycleManager" capability is
-	// enabled, so we cannot watch it if the capability is not enabled.
-	// Additionally, the default catalog only exists if the "marketplace"
-	// capability is enabled, so we cannot install OSSM without that
-	// capability.
-	if config.GatewayAPIEnabled && config.MarketplaceEnabled && config.OperatorLifecycleManagerEnabled {
+	// If the "GatewayAPI" and "GatewayAPIController" featuregates are
+	// enabled, watch subscriptions so that this controller can update
+	// status when the OSSM subscription is created or updated.  Note that
+	// the subscriptions resource only exists if the
+	// "OperatorLifecycleManager" capability is enabled, so we cannot watch
+	// it if the capability is not enabled.  Additionally, the default
+	// catalog only exists if the "marketplace" capability is enabled, so we
+	// cannot install OSSM without that capability.
+	if config.GatewayAPIEnabled && config.GatewayAPIControllerEnabled && config.MarketplaceEnabled && config.OperatorLifecycleManagerEnabled {
 		if err := c.Watch(source.Kind[client.Object](operatorCache, &operatorsv1alpha1.Subscription{}, handler.EnqueueRequestsFromMapFunc(toDefaultIngressController), predicate.Funcs{
 			CreateFunc: func(e event.CreateEvent) bool {
 				return e.Object.GetNamespace() == operatorcontroller.OpenshiftOperatorNamespace
@@ -124,6 +140,22 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 		})); err != nil {
 			return nil, err
 		}
+		if err := c.Watch(source.Kind[client.Object](operatorCache, &apiextensionsv1.CustomResourceDefinition{}, handler.EnqueueRequestsFromMapFunc(toDefaultIngressController), predicate.Funcs{
+			CreateFunc: func(e event.CreateEvent) bool {
+				return relatedObjectsCRDs.Has(e.Object.GetName())
+			},
+			UpdateFunc: func(e event.UpdateEvent) bool {
+				return false
+			},
+			DeleteFunc: func(e event.DeleteEvent) bool {
+				return relatedObjectsCRDs.Has(e.Object.GetName())
+			},
+			GenericFunc: func(e event.GenericEvent) bool {
+				return false
+			},
+		})); err != nil {
+			return nil, err
+		}
 	}
 
 	return c, nil
@@ -133,6 +165,9 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 type Config struct {
 	// GatewayAPIEnabled indicates that the "GatewayAPI" featuregate is enabled.
 	GatewayAPIEnabled bool
+	// GatewayAPIControllerEnabled indicates that the "GatewayAPIController"
+	// featuregate is enabled.
+	GatewayAPIControllerEnabled bool
 	// MarketplaceEnabled indicates whether the "marketplace" capability is
 	// enabled.
 	MarketplaceEnabled bool
@@ -183,7 +218,7 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
 	}
 	oldStatus := co.Status.DeepCopy()
 
-	state, err := r.getOperatorState(ingressNamespace, canaryNamespace, co)
+	state, err := r.getOperatorState(ctx, ingressNamespace, canaryNamespace, co)
 	if err != nil {
 		return reconcile.Result{}, fmt.Errorf("failed to get operator state: %v", err)
 	}
@@ -220,11 +255,7 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
 			Name:     state.CanaryNamespace.Name,
 		})
 	}
-	if r.config.GatewayAPIEnabled {
-		related = append(related, configv1.ObjectReference{
-			Group:    gatewayapiv1.GroupName,
-			Resource: "gatewayclasses",
-		})
+	if r.config.GatewayAPIEnabled && r.config.GatewayAPIControllerEnabled {
 		if state.haveOSSMSubscription {
 			subscriptionName := operatorcontroller.ServiceMeshOperatorSubscriptionName()
 			related = append(related, configv1.ObjectReference{
@@ -233,18 +264,25 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
 				Namespace: subscriptionName.Namespace,
 				Name:      subscriptionName.Name,
 			})
-			if state.IngressNamespace != nil {
-				related = append(related, configv1.ObjectReference{
-					Group:     sailv1.GroupVersion.Group,
-					Resource:  "istios",
-					Namespace: state.IngressNamespace.Name,
-				})
-				related = append(related, configv1.ObjectReference{
-					Group:     gatewayapiv1.GroupName,
-					Resource:  "gateways",
-					Namespace: state.IngressNamespace.Name,
-				})
-			}
+		}
+		if state.haveIstiosResource {
+			related = append(related, configv1.ObjectReference{
+				Group:    sailv1.GroupVersion.Group,
+				Resource: "istios",
+			})
+		}
+		if state.haveGatewayclassesResource {
+			related = append(related, configv1.ObjectReference{
+				Group:    gatewayapiv1.GroupName,
+				Resource: "gatewayclasses",
+			})
+		}
+		if state.haveGatewaysResource {
+			related = append(related, configv1.ObjectReference{
+				Group:     gatewayapiv1.GroupName,
+				Resource:  "gateways",
+				Namespace: "", // Include all namespaces.
+			})
 		}
 	}
 
@@ -317,17 +355,26 @@ type operatorState struct {
 	IngressControllers []operatorv1.IngressController
 	DNSRecords         []iov1.DNSRecord
 
-	haveOSSMSubscription        bool
 	unmanagedGatewayAPICRDNames string
+	// haveOSSMSubscription means that the subscription for OSSM 3 exists.
+	haveOSSMSubscription bool
+	// haveIstiosResource means that the "istios.sailproject.io" CRD exists.
+	haveIstiosResource bool
+	// haveGatewaysResource means that the
+	// "gateways.gateway.networking.k8s.io" CRD exists.
+	haveGatewaysResource bool
+	// haveGatewayclassesResource means that the
+	// "gatewayclasses.gateway.networking.k8s.io" CRD exists.
+	haveGatewayclassesResource bool
 }
 
 // getOperatorState gets and returns the resources necessary to compute the
 // operator's current state.
-func (r *reconciler) getOperatorState(ingressNamespace, canaryNamespace string, co *configv1.ClusterOperator) (operatorState, error) {
+func (r *reconciler) getOperatorState(ctx context.Context, ingressNamespace, canaryNamespace string, co *configv1.ClusterOperator) (operatorState, error) {
 	state := operatorState{}
 
 	ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ingressNamespace}}
-	if err := r.client.Get(context.TODO(), types.NamespacedName{Name: ingressNamespace}, ns); err != nil {
+	if err := r.client.Get(ctx, types.NamespacedName{Name: ingressNamespace}, ns); err != nil {
 		if !errors.IsNotFound(err) {
 			return state, fmt.Errorf("failed to get namespace %q: %v", ingressNamespace, err)
 		}
@@ -336,7 +383,7 @@ func (r *reconciler) getOperatorState(ingressNamespace, canaryNamespace string,
 	}
 
 	ns = &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: canaryNamespace}}
-	if err := r.client.Get(context.TODO(), types.NamespacedName{Name: canaryNamespace}, ns); err != nil {
+	if err := r.client.Get(ctx, types.NamespacedName{Name: canaryNamespace}, ns); err != nil {
 		if !errors.IsNotFound(err) {
 			return state, fmt.Errorf("failed to get namespace %q: %v", canaryNamespace, err)
 		}
@@ -345,32 +392,61 @@ func (r *reconciler) getOperatorState(ingressNamespace, canaryNamespace string,
 	}
 
 	ingressList := &operatorv1.IngressControllerList{}
-	if err := r.cache.List(context.TODO(), ingressList, client.InNamespace(r.config.Namespace)); err != nil {
+	if err := r.cache.List(ctx, ingressList, client.InNamespace(r.config.Namespace)); err != nil {
 		return state, fmt.Errorf("failed to list ingresscontrollers in %q: %v", r.config.Namespace, err)
 	} else {
 		state.IngressControllers = ingressList.Items
 	}
 
 	if r.config.GatewayAPIEnabled {
-		if r.config.MarketplaceEnabled && r.config.OperatorLifecycleManagerEnabled {
+		if len(co.Status.Extension.Raw) > 0 {
+			extension := &IngressOperatorStatusExtension{}
+			if err := json.Unmarshal(co.Status.Extension.Raw, extension); err != nil {
+				return state, fmt.Errorf("failed to unmarshal status extension of cluster operator %q: %w", co.Name, err)
+			}
+			state.unmanagedGatewayAPICRDNames = extension.UnmanagedGatewayAPICRDNames
+		}
+
+		if r.config.GatewayAPIControllerEnabled && r.config.MarketplaceEnabled && r.config.OperatorLifecycleManagerEnabled {
 			var subscription operatorsv1alpha1.Subscription
 			subscriptionName := operatorcontroller.ServiceMeshOperatorSubscriptionName()
-			if err := r.cache.Get(context.TODO(), subscriptionName, &subscription); err != nil {
+			if err := r.cache.Get(ctx, subscriptionName, &subscription); err != nil {
 				if !errors.IsNotFound(err) {
 					return state, fmt.Errorf("failed to get subscription %q: %v", subscriptionName, err)
 				}
 			} else {
 				state.haveOSSMSubscription = true
 
 			}
-		}
 
-		if len(co.Status.Extension.Raw) > 0 {
-			extension := &IngressOperatorStatusExtension{}
-			if err := json.Unmarshal(co.Status.Extension.Raw, extension); err != nil {
-				return state, fmt.Errorf("failed to unmarshal status extension of cluster operator %q: %w", co.Name, err)
+			var (
+				crd                                  apiextensionsv1.CustomResourceDefinition
+				gatewaysResourceNamespacedName       = types.NamespacedName{Name: gatewaysResourceName}
+				gatewayclassesResourceNamespacedName = types.NamespacedName{Name: gatewayclassesResourceName}
+				istiosResourceNamespacedName         = types.NamespacedName{Name: istiosResourceName}
+			)
+
+			if err := r.cache.Get(ctx, gatewaysResourceNamespacedName, &crd); err != nil {
+				if !errors.IsNotFound(err) {
+					return state, fmt.Errorf("failed to get CRD %q: %v", gatewaysResourceName, err)
+				}
+			} else {
+				state.haveGatewaysResource = true
+			}
+			if err := r.cache.Get(ctx, gatewayclassesResourceNamespacedName, &crd); err != nil {
+				if !errors.IsNotFound(err) {
+					return state, fmt.Errorf("failed to get CRD %q: %v", gatewayclassesResourceName, err)
+				}
+			} else {
+				state.haveGatewayclassesResource = true
+			}
+			if err := r.cache.Get(ctx, istiosResourceNamespacedName, &crd); err != nil {
+				if !errors.IsNotFound(err) {
+					return state, fmt.Errorf("failed to get CRD %q: %v", istiosResourceName, err)
+				}
+			} else {
+				state.haveIstiosResource = true
 			}
-			state.unmanagedGatewayAPICRDNames = extension.UnmanagedGatewayAPICRDNames
 		}
 	}
 

pkg/operator/operator.go

@@ -225,6 +225,7 @@ func New(config operatorconfig.Config, kubeConfig *rest.Config) (*Operator, erro
 		CanaryImage:                     config.CanaryImage,
 		OperatorReleaseVersion:          config.OperatorReleaseVersion,
 		GatewayAPIEnabled:               gatewayAPIEnabled,
+		GatewayAPIControllerEnabled:     gatewayAPIControllerEnabled,
 		MarketplaceEnabled:              marketplaceEnabled,
 		OperatorLifecycleManagerEnabled: olmEnabled,
 	}); err != nil {

test/e2e/operator_test.go

@@ -233,12 +233,23 @@ func TestClusterOperatorStatusRelatedObjects(t *testing.T) {
 	if gatewayAPIEnabled, err := isFeatureGateEnabled(features.FeatureGateGatewayAPI); err != nil {
 		t.Fatalf("Failed to look up %q featuregate: %v", features.FeatureGateGatewayAPI, err)
 	} else if gatewayAPIEnabled {
-		expected = append(expected, configv1.ObjectReference{
-			Group:    "gateway.networking.k8s.io",
-			Resource: "gatewayclasses",
-		})
-		// This test runs before TestGatewayAPI, so we do *not* expect
-		// to see subscriptions, istios, or gateways in relatedObjects.
+		if gatewayAPIControllerEnabled, err := isFeatureGateEnabled(features.FeatureGateGatewayAPIController); err != nil {
+			t.Fatalf("Failed to look up %q featuregate: %v", features.FeatureGateGatewayAPIController, err)
+		} else if gatewayAPIControllerEnabled {
+			// This test runs before TestGatewayAPI creates the
+			// subscription to install OSSM, so we do *not* expect
+			// to see subscriptions or istios in relatedObjects.
+			// However, we *do* expect to see gatewayclasses and
+			// gateways whenever the GatewayAPI and
+			// GatewayAPIController featuregates are enabled.
+			expected = append(expected, configv1.ObjectReference{
+				Group:    "gateway.networking.k8s.io",
+				Resource: "gatewayclasses",
+			}, configv1.ObjectReference{
+				Group:    "gateway.networking.k8s.io",
+				Resource: "gateways",
+			})
+		}
 	}
 
 	coName := controller.IngressClusterOperatorName()