status: Conditionally add CRDs to relatedObjects

Check whether the gatewayclasses, gateways, and istios CRDs actually
exist before adding them to relatedObjects.

Also, use the "GatewayAPIController" featuregate to determine whether to
add the gatewayclasses, gateways, istios, and subscriptions resources to
relatedObjects, rather than using the "GatewayAPI" featuregate.

Before this change, the operator could add istios to relatedObjects even
if the OSSM subscription failed to install.  By convention, an operator
should only add resources to relatedObjects if those resources exist.

This commit fixes OCPBUGS-54745.

https://issues.redhat.com/browse/OCPBUGS-54745

* pkg/operator/controller/status/controller.go
(gatewaysResourceName, gatewayclassesResourceName, istiosResourceName):
New consts for the CRD names.
(relatedObjectsCRDs): New var for a string set that contains
gatewaysResourceName, gatewayclassesResourceName, and
istiosResourceName.
(New): Add a watch on CRDs, with a predicate for CRDs with names that
are in relatedObjectsCRDs.
(Config): Add GatewayAPIControllerEnabled.
(Reconcile): Check the GatewayAPIControllerEnabled field in the operator
config as well as the haveIstiosResource, haveGatewayclassesResource,
and haveGatewaysResource fields in the operatorState object and
conditionally add the corresponding resources to relatedObjects.
(operatorState): Add haveIstiosResource, haveGatewaysResource, and
haveGatewayclassesResource fields.
(getOperatorState): Set haveGatewaysResource,
haveGatewayclassesResource, and haveIstiosResource.
* pkg/operator/operator.go (New): Specify GatewayAPIControllerEnabled in
the status controller config.
* test/e2e/operator_test.go (TestClusterOperatorStatusRelatedObjects):
Expect to see "gateways" and "gatewayclasses" in relatedObjects if the
"GatewayAPI" and "GatewayAPIController" featuregates are enabled.

Author: Miciah

pkg/operator/controller/status/controller.go

@@ -23,9 +23,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"
@@ -49,13 +51,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.
@@ -115,6 +131,24 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 		})); err != nil {
 			return nil, err
 		}
+		if config.GatewayAPIControllerEnabled {
+			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
@@ -123,7 +157,10 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 // Config holds all the things necessary for the controller to run.
 type Config struct {
 	// GatewayAPIEnabled indicates that the "GatewayAPI" featuregate is enabled.
-	GatewayAPIEnabled      bool
+	GatewayAPIEnabled bool
+	// GatewayAPIControllerEnabled indicates that the "GatewayAPIControllerEnabled" featuregate is enabled.
+	GatewayAPIControllerEnabled bool
+
 	IngressControllerImage string
 	CanaryImage            string
 	OperatorReleaseVersion string
@@ -198,11 +235,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{
@@ -211,17 +244,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",
-				})
-				related = append(related, configv1.ObjectReference{
-					Group:     gatewayapiv1.GroupName,
-					Resource:  "gateways",
-					Namespace: "", // Include all namespaces.
-				})
-			}
+		}
+		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 && state.IngressNamespace != nil {
+			related = append(related, configv1.ObjectReference{
+				Group:     gatewayapiv1.GroupName,
+				Resource:  "gateways",
+				Namespace: "", // Include all namespaces.
+			})
 		}
 	}
 
@@ -294,7 +335,16 @@ type operatorState struct {
 	IngressControllers []operatorv1.IngressController
 	DNSRecords         []iov1.DNSRecord
 
+	// 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
@@ -337,6 +387,37 @@ func (r *reconciler) getOperatorState(ctx context.Context, ingressNamespace, can
 		} else {
 			state.haveOSSMSubscription = true
 		}
+
+		if r.config.GatewayAPIControllerEnabled {
+			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
+			}
+		}
 	}
 
 	return state, nil

pkg/operator/operator.go

@@ -208,11 +208,12 @@ func New(config operatorconfig.Config, kubeConfig *rest.Config) (*Operator, erro
 
 	// Set up the status controller.
 	if _, err := statuscontroller.New(mgr, statuscontroller.Config{
-		Namespace:              config.Namespace,
-		IngressControllerImage: config.IngressControllerImage,
-		CanaryImage:            config.CanaryImage,
-		OperatorReleaseVersion: config.OperatorReleaseVersion,
-		GatewayAPIEnabled:      gatewayAPIEnabled,
+		Namespace:                   config.Namespace,
+		IngressControllerImage:      config.IngressControllerImage,
+		CanaryImage:                 config.CanaryImage,
+		OperatorReleaseVersion:      config.OperatorReleaseVersion,
+		GatewayAPIEnabled:           gatewayAPIEnabled,
+		GatewayAPIControllerEnabled: gatewayAPIControllerEnabled,
 	}); err != nil {
 		return nil, fmt.Errorf("failed to create status controller: %v", err)
 	}

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()