OCPBUGS-58155: CNO must skip watching IngressController if Ingress capability is disabled in HyperShift

This commit updates the CNO to conditionally watch the IngressController resource only when the Ingress capability is enabled. When Ingress is disabled in HyperShift, the IngressController CRD is not available. Without this change, CNO attempts to watch a non-existent resource and repeatedly restarts. This update prevents that by skipping the watch when the capability is disabled.

Author: linoyaslan

pkg/controller/ingressconfig/ingressconfig_controller.go

@@ -2,14 +2,17 @@ package ingressconfig
 
 import (
 	"context"
+	"fmt"
 	"log"
 	"time"
 
 	operv1 "github.com/openshift/api/operator/v1"
 	cnoclient "github.com/openshift/cluster-network-operator/pkg/client"
 	"github.com/openshift/cluster-network-operator/pkg/controller/statusmanager"
+	"github.com/openshift/cluster-network-operator/pkg/hypershift"
 	"github.com/openshift/cluster-network-operator/pkg/names"
 	"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
+	"k8s.io/apimachinery/pkg/api/meta"
 
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -46,8 +49,55 @@ func newIngressConfigReconciler(client crclient.Client, status *statusmanager.St
 	return &ReconcileIngressConfigs{client: client, status: status}
 }
 
+// isIngressCapabilityEnabled checks if the Ingress capability is enabled in the cluster
+func isIngressCapabilityEnabled(client crclient.Client) (bool, error) {
+	hcpCfg := hypershift.NewHyperShiftConfig()
+	return isIngressCapabilityEnabledWithConfig(client, hcpCfg)
+}
+
+// isIngressCapabilityEnabledWithConfig checks if the Ingress capability is enabled in the cluster
+func isIngressCapabilityEnabledWithConfig(client crclient.Client, hcpCfg *hypershift.HyperShiftConfig) (bool, error) {
+	// Handle nil client
+	if client == nil {
+		return false, fmt.Errorf("client cannot be nil")
+	}
+
+	// Check if this is a hypershift cluster
+	if !hcpCfg.Enabled {
+		// For non-hypershift clusters, assume ingress capability is always enabled
+		return true, nil
+	}
+
+	// For hypershift clusters, check if the IngressController CRD exists
+	gvk := operv1.SchemeGroupVersion.WithKind("IngressController")
+	mapping, err := client.RESTMapper().RESTMapping(gvk.GroupKind(), gvk.Version)
+	if err != nil {
+		if meta.IsNoMatchError(err) {
+			log.Printf("IngressController CRD not found, assuming Ingress capability is disabled: %v", err)
+			return false, nil
+		}
+		return false, err
+	}
+
+	// If we got a mapping, the CRD exists
+	log.Printf("IngressController CRD found, Ingress capability is enabled (Resource: %v)", mapping.Resource)
+	return true, nil
+}
+
 // add adds a new Controller to mgr with r as the reconcile.Reconciler
 func add(mgr manager.Manager, r *ReconcileIngressConfigs) error {
+	// Check if Ingress capability is enabled before setting up the controller
+	ingressEnabled, err := isIngressCapabilityEnabled(mgr.GetClient())
+	if err != nil {
+		log.Printf("Error checking if Ingress capability is enabled: %v", err)
+		return err
+	}
+
+	if !ingressEnabled {
+		log.Printf("Ingress capability is disabled, skipping ingress-config-controller creation entirely")
+		return nil
+	}
+
 	// create a controller and register watcher for ingresscontroller resource
 	c, err := controller.New("ingress-config-controller", mgr, controller.Options{Reconciler: r})
 	if err != nil {

pkg/controller/ingressconfig/ingressconfig_controller_test.go

@@ -0,0 +1,126 @@
+package ingressconfig
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+
+	"github.com/openshift/cluster-network-operator/pkg/client/fake"
+	"github.com/openshift/cluster-network-operator/pkg/hypershift"
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+func TestIsIngressCapabilityEnabled(t *testing.T) {
+	testCases := []struct {
+		name              string
+		hypershiftEnabled bool
+		crdExists         bool
+		restMapperError   error
+		expectedResult    bool
+		expectedError     bool
+	}{
+		{
+			name:              "Non-hypershift cluster should always return true (IngressController CRD doesn't exists)",
+			hypershiftEnabled: false,
+			crdExists:         false, // doesn't matter for non-hypershift
+			expectedResult:    true,
+			expectedError:     false,
+		},
+		{
+			name:              "Non-hypershift cluster should always return true (even with IngressController CRD)",
+			hypershiftEnabled: false,
+			crdExists:         true, // doesn't matter for non-hypershift
+			expectedResult:    true,
+			expectedError:     false,
+		},
+		{
+			name:              "Hypershift cluster with IngressController CRD should return true",
+			hypershiftEnabled: true,
+			crdExists:         true,
+			expectedResult:    true,
+			expectedError:     false,
+		},
+		{
+			name:              "Hypershift cluster without IngressController CRD should return false",
+			hypershiftEnabled: true,
+			crdExists:         false,
+			restMapperError: &meta.NoKindMatchError{
+				GroupKind: schema.GroupKind{
+					Group: "operator.openshift.io",
+					Kind:  "IngressController",
+				},
+				SearchedVersions: []string{"v1"},
+			},
+			expectedResult: false,
+			expectedError:  false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewGomegaWithT(t)
+
+			// Create mock REST mapper
+			restMapper := &mockRESTMapper{
+				shouldReturnError: !tc.crdExists,
+				errorToReturn:     tc.restMapperError,
+			}
+
+			// Create mock client with the REST mapper
+			fakeClient := fake.NewFakeClient()
+			mockClient := &mockClient{
+				Client:     fakeClient.Default().CRClient(),
+				restMapper: restMapper,
+			}
+
+			// Create hypershift config with the desired enabled state
+			hcpCfg := &hypershift.HyperShiftConfig{
+				Enabled: tc.hypershiftEnabled,
+			}
+
+			result, err := isIngressCapabilityEnabledWithConfig(mockClient, hcpCfg)
+
+			if tc.expectedError {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).NotTo(HaveOccurred())
+			}
+			g.Expect(result).To(Equal(tc.expectedResult))
+		})
+	}
+}
+
+func TestIsIngressCapabilityEnabledEdgeCases(t *testing.T) {
+	g := NewGomegaWithT(t)
+
+	// Test with nil client - should handle gracefully
+	hcpCfg := &hypershift.HyperShiftConfig{Enabled: false}
+	result, err := isIngressCapabilityEnabledWithConfig(nil, hcpCfg)
+	g.Expect(err).To(HaveOccurred())
+	g.Expect(result).To(BeFalse())
+
+	// Test REST mapper error handling (in non-hypershift environment)
+	// This simulates what would happen if there were issues with the REST mapper
+	restMapper := &mockRESTMapper{
+		shouldReturnError: true,
+		errorToReturn: &meta.NoKindMatchError{
+			GroupKind: schema.GroupKind{
+				Group: "operator.openshift.io",
+				Kind:  "IngressController",
+			},
+			SearchedVersions: []string{"v1"},
+		},
+	}
+
+	fakeClient := fake.NewFakeClient()
+	mockClient := &mockClient{
+		Client:     fakeClient.Default().CRClient(),
+		restMapper: restMapper,
+	}
+
+	// In non-hypershift mode, this should still return true regardless of REST mapper errors
+	result, err = isIngressCapabilityEnabledWithConfig(mockClient, hcpCfg)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(result).To(BeTrue()) // Non-hypershift always returns true
+}

pkg/controller/ingressconfig/mocks_test.go

@@ -0,0 +1,61 @@
+package ingressconfig
+
+import (
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	crclient "sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// mockRESTMapper is a fake RESTMapper for testing
+type mockRESTMapper struct {
+	shouldReturnError bool
+	errorToReturn     error
+}
+
+func (m *mockRESTMapper) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) {
+	return schema.GroupVersionKind{}, nil
+}
+
+func (m *mockRESTMapper) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
+	return nil, nil
+}
+
+func (m *mockRESTMapper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, error) {
+	return schema.GroupVersionResource{}, nil
+}
+
+func (m *mockRESTMapper) ResourcesFor(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) {
+	return nil, nil
+}
+
+func (m *mockRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) {
+	if m.shouldReturnError {
+		return nil, m.errorToReturn
+	}
+	// Return a successful mapping indicating CRD exists
+	return &meta.RESTMapping{
+		Resource: schema.GroupVersionResource{
+			Group:    "operator.openshift.io",
+			Version:  "v1",
+			Resource: "ingresscontrollers",
+		},
+	}, nil
+}
+
+func (m *mockRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
+	return nil, nil
+}
+
+func (m *mockRESTMapper) ResourceSingularizer(resource string) (singular string, err error) {
+	return "", nil
+}
+
+// mockClient wraps the fake client with a custom RESTMapper
+type mockClient struct {
+	crclient.Client
+	restMapper meta.RESTMapper
+}
+
+func (m *mockClient) RESTMapper() meta.RESTMapper {
+	return m.restMapper
+}