diff --git a/pkg/operator/controller/status/controller.go b/pkg/operator/controller/status/controller.go index 5bd956da9d..be802cca64 100644 --- a/pkg/operator/controller/status/controller.go +++ b/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,17 +392,25 @@ 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) } @@ -363,14 +418,35 @@ func (r *reconciler) getOperatorState(ingressNamespace, canaryNamespace string, 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 } } diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index f9d9ea3306..bf749eb849 100644 --- a/pkg/operator/operator.go +++ b/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 { diff --git a/test/e2e/operator_test.go b/test/e2e/operator_test.go index 09d997de97..eb5ad72328 100644 --- a/test/e2e/operator_test.go +++ b/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()