Skip to content

Commit df734b2

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #2731 from linoyaslan/fix_expect_ingresscontroller_when_ingress_disabled
OCPBUGS-58155: CNO must skip watching IngressController if Ingress capability is disabled in HyperShift
2 parents 29122af + 98f46a7 commit df734b2

File tree

3 files changed

+237
-0
lines changed

3 files changed

+237
-0
lines changed

‎pkg/controller/ingressconfig/ingressconfig_controller.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,17 @@ package ingressconfig
22

33
import (
44
"context"
5+
"fmt"
56
"log"
67
"time"
78

89
operv1 "github.com/openshift/api/operator/v1"
910
cnoclient "github.com/openshift/cluster-network-operator/pkg/client"
1011
"github.com/openshift/cluster-network-operator/pkg/controller/statusmanager"
12+
"github.com/openshift/cluster-network-operator/pkg/hypershift"
1113
"github.com/openshift/cluster-network-operator/pkg/names"
1214
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
15+
"k8s.io/apimachinery/pkg/api/meta"
1316

1417
corev1 "k8s.io/api/core/v1"
1518
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -46,8 +49,55 @@ func newIngressConfigReconciler(client crclient.Client, status *statusmanager.St
4649
return &ReconcileIngressConfigs{client: client, status: status}
4750
}
4851

52+
// isIngressCapabilityEnabled checks if the Ingress capability is enabled in the cluster
53+
func isIngressCapabilityEnabled(client crclient.Client) (bool, error) {
54+
hcpCfg := hypershift.NewHyperShiftConfig()
55+
return isIngressCapabilityEnabledWithConfig(client, hcpCfg)
56+
}
57+
58+
// isIngressCapabilityEnabledWithConfig checks if the Ingress capability is enabled in the cluster
59+
func isIngressCapabilityEnabledWithConfig(client crclient.Client, hcpCfg *hypershift.HyperShiftConfig) (bool, error) {
60+
// Handle nil client
61+
if client == nil {
62+
return false, fmt.Errorf("client cannot be nil")
63+
}
64+
65+
// Check if this is a hypershift cluster
66+
if !hcpCfg.Enabled {
67+
// For non-hypershift clusters, assume ingress capability is always enabled
68+
return true, nil
69+
}
70+
71+
// For hypershift clusters, check if the IngressController CRD exists
72+
gvk := operv1.SchemeGroupVersion.WithKind("IngressController")
73+
mapping, err := client.RESTMapper().RESTMapping(gvk.GroupKind(), gvk.Version)
74+
if err != nil {
75+
if meta.IsNoMatchError(err) {
76+
log.Printf("IngressController CRD not found, assuming Ingress capability is disabled: %v", err)
77+
return false, nil
78+
}
79+
return false, err
80+
}
81+
82+
// If we got a mapping, the CRD exists
83+
log.Printf("IngressController CRD found, Ingress capability is enabled (Resource: %v)", mapping.Resource)
84+
return true, nil
85+
}
86+
4987
// add adds a new Controller to mgr with r as the reconcile.Reconciler
5088
func add(mgr manager.Manager, r *ReconcileIngressConfigs) error {
89+
// Check if Ingress capability is enabled before setting up the controller
90+
ingressEnabled, err := isIngressCapabilityEnabled(mgr.GetClient())
91+
if err != nil {
92+
log.Printf("Error checking if Ingress capability is enabled: %v", err)
93+
return err
94+
}
95+
96+
if !ingressEnabled {
97+
log.Printf("Ingress capability is disabled, skipping ingress-config-controller creation entirely")
98+
return nil
99+
}
100+
51101
// create a controller and register watcher for ingresscontroller resource
52102
c, err := controller.New("ingress-config-controller", mgr, controller.Options{Reconciler: r})
53103
if err != nil {

‎pkg/controller/ingressconfig/ingressconfig_controller_test.go‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
package ingressconfig
2+
3+
import (
4+
"testing"
5+
6+
. "github.com/onsi/gomega"
7+
8+
"github.com/openshift/cluster-network-operator/pkg/client/fake"
9+
"github.com/openshift/cluster-network-operator/pkg/hypershift"
10+
"k8s.io/apimachinery/pkg/api/meta"
11+
"k8s.io/apimachinery/pkg/runtime/schema"
12+
)
13+
14+
func TestIsIngressCapabilityEnabled(t *testing.T) {
15+
testCases := []struct {
16+
name string
17+
hypershiftEnabled bool
18+
crdExists bool
19+
restMapperError error
20+
expectedResult bool
21+
expectedError bool
22+
}{
23+
{
24+
name: "Non-hypershift cluster should always return true (IngressController CRD doesn't exists)",
25+
hypershiftEnabled: false,
26+
crdExists: false, // doesn't matter for non-hypershift
27+
expectedResult: true,
28+
expectedError: false,
29+
},
30+
{
31+
name: "Non-hypershift cluster should always return true (even with IngressController CRD)",
32+
hypershiftEnabled: false,
33+
crdExists: true, // doesn't matter for non-hypershift
34+
expectedResult: true,
35+
expectedError: false,
36+
},
37+
{
38+
name: "Hypershift cluster with IngressController CRD should return true",
39+
hypershiftEnabled: true,
40+
crdExists: true,
41+
expectedResult: true,
42+
expectedError: false,
43+
},
44+
{
45+
name: "Hypershift cluster without IngressController CRD should return false",
46+
hypershiftEnabled: true,
47+
crdExists: false,
48+
restMapperError: &meta.NoKindMatchError{
49+
GroupKind: schema.GroupKind{
50+
Group: "operator.openshift.io",
51+
Kind: "IngressController",
52+
},
53+
SearchedVersions: []string{"v1"},
54+
},
55+
expectedResult: false,
56+
expectedError: false,
57+
},
58+
}
59+
60+
for _, tc := range testCases {
61+
t.Run(tc.name, func(t *testing.T) {
62+
g := NewGomegaWithT(t)
63+
64+
// Create mock REST mapper
65+
restMapper := &mockRESTMapper{
66+
shouldReturnError: !tc.crdExists,
67+
errorToReturn: tc.restMapperError,
68+
}
69+
70+
// Create mock client with the REST mapper
71+
fakeClient := fake.NewFakeClient()
72+
mockClient := &mockClient{
73+
Client: fakeClient.Default().CRClient(),
74+
restMapper: restMapper,
75+
}
76+
77+
// Create hypershift config with the desired enabled state
78+
hcpCfg := &hypershift.HyperShiftConfig{
79+
Enabled: tc.hypershiftEnabled,
80+
}
81+
82+
result, err := isIngressCapabilityEnabledWithConfig(mockClient, hcpCfg)
83+
84+
if tc.expectedError {
85+
g.Expect(err).To(HaveOccurred())
86+
} else {
87+
g.Expect(err).NotTo(HaveOccurred())
88+
}
89+
g.Expect(result).To(Equal(tc.expectedResult))
90+
})
91+
}
92+
}
93+
94+
func TestIsIngressCapabilityEnabledEdgeCases(t *testing.T) {
95+
g := NewGomegaWithT(t)
96+
97+
// Test with nil client - should handle gracefully
98+
hcpCfg := &hypershift.HyperShiftConfig{Enabled: false}
99+
result, err := isIngressCapabilityEnabledWithConfig(nil, hcpCfg)
100+
g.Expect(err).To(HaveOccurred())
101+
g.Expect(result).To(BeFalse())
102+
103+
// Test REST mapper error handling (in non-hypershift environment)
104+
// This simulates what would happen if there were issues with the REST mapper
105+
restMapper := &mockRESTMapper{
106+
shouldReturnError: true,
107+
errorToReturn: &meta.NoKindMatchError{
108+
GroupKind: schema.GroupKind{
109+
Group: "operator.openshift.io",
110+
Kind: "IngressController",
111+
},
112+
SearchedVersions: []string{"v1"},
113+
},
114+
}
115+
116+
fakeClient := fake.NewFakeClient()
117+
mockClient := &mockClient{
118+
Client: fakeClient.Default().CRClient(),
119+
restMapper: restMapper,
120+
}
121+
122+
// In non-hypershift mode, this should still return true regardless of REST mapper errors
123+
result, err = isIngressCapabilityEnabledWithConfig(mockClient, hcpCfg)
124+
g.Expect(err).NotTo(HaveOccurred())
125+
g.Expect(result).To(BeTrue()) // Non-hypershift always returns true
126+
}

‎pkg/controller/ingressconfig/mocks_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package ingressconfig
2+
3+
import (
4+
"k8s.io/apimachinery/pkg/api/meta"
5+
"k8s.io/apimachinery/pkg/runtime/schema"
6+
crclient "sigs.k8s.io/controller-runtime/pkg/client"
7+
)
8+
9+
// mockRESTMapper is a fake RESTMapper for testing
10+
type mockRESTMapper struct {
11+
shouldReturnError bool
12+
errorToReturn error
13+
}
14+
15+
func (m *mockRESTMapper) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) {
16+
return schema.GroupVersionKind{}, nil
17+
}
18+
19+
func (m *mockRESTMapper) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
20+
return nil, nil
21+
}
22+
23+
func (m *mockRESTMapper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, error) {
24+
return schema.GroupVersionResource{}, nil
25+
}
26+
27+
func (m *mockRESTMapper) ResourcesFor(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) {
28+
return nil, nil
29+
}
30+
31+
func (m *mockRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) {
32+
if m.shouldReturnError {
33+
return nil, m.errorToReturn
34+
}
35+
// Return a successful mapping indicating CRD exists
36+
return &meta.RESTMapping{
37+
Resource: schema.GroupVersionResource{
38+
Group: "operator.openshift.io",
39+
Version: "v1",
40+
Resource: "ingresscontrollers",
41+
},
42+
}, nil
43+
}
44+
45+
func (m *mockRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
46+
return nil, nil
47+
}
48+
49+
func (m *mockRESTMapper) ResourceSingularizer(resource string) (singular string, err error) {
50+
return "", nil
51+
}
52+
53+
// mockClient wraps the fake client with a custom RESTMapper
54+
type mockClient struct {
55+
crclient.Client
56+
restMapper meta.RESTMapper
57+
}
58+
59+
func (m *mockClient) RESTMapper() meta.RESTMapper {
60+
return m.restMapper
61+
}

0 commit comments

Comments
 (0)