Skip to content

Commit 654b681

Browse files
alebedev87alebedev87
committed
NE-1969: Set Degraded=True if unmanaged Gateway API CRDs exist
Set the ingress cluster operator’s `Degraded` status to `True` if unmanaged Gateway API CRDs exist on the cluster. An unmanaged Gateway API CRD is one with "gateway.networking.k8s.io" or "gateway.networking.x-k8s.io" in its `spec.group` field, and not managed by the gatewayapi controller. This commit uses the cluster operator’s `status.extension` field to store the names of unmanaged CRDs. The gatewayapi controller writes to this field, while the status controller reads it and updates the ingress cluster operator’s `Degraded` status accordingly.
1 parent 04bd19b commit 654b681

File tree

10 files changed

+691
-60
lines changed

10 files changed

+691
-60
lines changed

‎pkg/operator/controller/gateway-service-dns/controller_test.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,13 @@ func Test_Reconcile(t *testing.T) {
254254
WithScheme(scheme).
255255
WithRuntimeObjects(tc.existingObjects...).
256256
Build()
257-
cl := &testutil.FakeClientRecorder{fakeClient, t, []client.Object{}, []client.Object{}, []client.Object{}}
257+
cl := &testutil.FakeClientRecorder{
258+
Client: fakeClient,
259+
T: t,
260+
Added: []client.Object{},
261+
Updated: []client.Object{},
262+
Deleted: []client.Object{},
263+
}
258264
informer := informertest.FakeInformers{Scheme: scheme}
259265
cache := testutil.FakeCache{Informers: &informer, Reader: cl}
260266
reconciler := &reconciler{

‎pkg/operator/controller/gatewayapi/controller.go‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package gatewayapi
22

33
import (
44
"context"
5+
"fmt"
56
"sync"
67

78
logf "github.com/openshift/cluster-ingress-operator/pkg/log"
@@ -14,6 +15,7 @@ import (
1415
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1516
"k8s.io/apimachinery/pkg/types"
1617

18+
"sigs.k8s.io/controller-runtime/pkg/cache"
1719
"sigs.k8s.io/controller-runtime/pkg/client"
1820
"sigs.k8s.io/controller-runtime/pkg/controller"
1921
"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -25,7 +27,10 @@ import (
2527
)
2628

2729
const (
28-
controllerName = "gatewayapi_controller"
30+
controllerName = "gatewayapi_controller"
31+
experimentalGatewayAPIGroupName = "gateway.networking.x-k8s.io"
32+
gatewayAPICRDIndexFieldName = "gatewayAPICRD"
33+
unmanagedGatewayAPICRDIndexFieldValue = "unmanaged"
2934
)
3035

3136
var log = logf.Logger.WithName(controllerName)
@@ -36,6 +41,7 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
3641
operatorCache := mgr.GetCache()
3742
reconciler := &reconciler{
3843
client: mgr.GetClient(),
44+
cache: operatorCache,
3945
config: config,
4046
}
4147
c, err := controller.New(controllerName, mgr, controller.Options{Reconciler: reconciler})
@@ -60,14 +66,34 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
6066
}}
6167
}
6268

63-
// watch for CRDs
64-
crdPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
65-
return o.(*apiextensionsv1.CustomResourceDefinition).Spec.Group == gatewayapiv1.GroupName
66-
})
69+
isGatewayAPICRD := func(o client.Object) bool {
70+
crd := o.(*apiextensionsv1.CustomResourceDefinition)
71+
return crd.Spec.Group == gatewayapiv1.GroupName || crd.Spec.Group == experimentalGatewayAPIGroupName
72+
}
73+
crdPredicate := predicate.NewPredicateFuncs(isGatewayAPICRD)
6774

75+
// watch for CRDs
6876
if err := c.Watch(source.Kind[client.Object](operatorCache, &apiextensionsv1.CustomResourceDefinition{}, handler.EnqueueRequestsFromMapFunc(toFeatureGate), crdPredicate)); err != nil {
6977
return nil, err
7078
}
79+
80+
// Index unmanaged Gateway API CRDs to enable efficient filtering
81+
// during list operations.
82+
if err := mgr.GetFieldIndexer().IndexField(
83+
context.Background(),
84+
&apiextensionsv1.CustomResourceDefinition{},
85+
gatewayAPICRDIndexFieldName,
86+
client.IndexerFunc(func(o client.Object) []string {
87+
if isGatewayAPICRD(o) {
88+
if _, found := managedCRDMap[o.GetName()]; !found {
89+
return []string{unmanagedGatewayAPICRDIndexFieldValue}
90+
}
91+
}
92+
return []string{}
93+
})); err != nil {
94+
return nil, fmt.Errorf("failed to create index for custom resource definitions: %w", err)
95+
}
96+
7197
return c, nil
7298
}
7399

@@ -90,6 +116,7 @@ type reconciler struct {
90116
config Config
91117

92118
client client.Client
119+
cache cache.Cache
93120
recorder record.EventRecorder
94121
startControllers sync.Once
95122
}
@@ -111,6 +138,12 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
111138
return reconcile.Result{}, err
112139
}
113140

141+
if crdNames, err := r.listUnmanagedGatewayAPICRDs(ctx); err != nil {
142+
return reconcile.Result{}, fmt.Errorf("failed to list unmanaged gateway CRDs: %w", err)
143+
} else if err = r.setUnmanagedGatewayAPICRDNamesStatus(ctx, crdNames); err != nil {
144+
return reconcile.Result{}, fmt.Errorf("failed to update the ingress cluster operator status: %w", err)
145+
}
146+
114147
if !r.config.GatewayAPIControllerEnabled {
115148
return reconcile.Result{}, nil
116149
}

‎pkg/operator/controller/gatewayapi/controller_test.go‎

Lines changed: 168 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package gatewayapi
22

33
import (
44
"context"
5+
"strings"
56
"testing"
67
"time"
78

@@ -17,6 +18,7 @@ import (
1718
"k8s.io/apimachinery/pkg/runtime"
1819
"k8s.io/apimachinery/pkg/types"
1920

21+
"sigs.k8s.io/controller-runtime/pkg/cache/informertest"
2022
"sigs.k8s.io/controller-runtime/pkg/client"
2123
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2224
"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -36,28 +38,57 @@ func Test_Reconcile(t *testing.T) {
3638
ObjectMeta: metav1.ObjectMeta{Name: name},
3739
}
3840
}
41+
co := func(name string) *configv1.ClusterOperator {
42+
return &configv1.ClusterOperator{
43+
ObjectMeta: metav1.ObjectMeta{Name: name},
44+
}
45+
}
46+
coWithExtension := func(name, extension string) *configv1.ClusterOperator {
47+
co := co(name)
48+
co.Status = configv1.ClusterOperatorStatus{
49+
Extension: runtime.RawExtension{
50+
Raw: []byte(extension),
51+
},
52+
}
53+
return co
54+
}
55+
3956
tests := []struct {
4057
name string
4158
gatewayAPIEnabled bool
4259
gatewayAPIControllerEnabled bool
4360
existingObjects []runtime.Object
44-
expectCreate []client.Object
45-
expectUpdate []client.Object
46-
expectDelete []client.Object
47-
expectStartCtrl bool
61+
// existingStatusSubresource contains the original version of objects
62+
// whose status will updated by Reconcile function.
63+
// This field is similar to `existingObjects` but is specifically used
64+
// for objects where status updates are performed using `Status().Update()` call.
65+
existingStatusSubresource []client.Object
66+
expectCreate []client.Object
67+
expectUpdate []client.Object
68+
expectDelete []client.Object
69+
// expectStatusUpdate contains the updated versions of objects
70+
// whose status is expected to be updated by the test.
71+
expectStatusUpdate []client.Object
72+
expectStartCtrl bool
4873
}{
4974
{
5075
name: "gateway API disabled",
5176
gatewayAPIEnabled: false,
52-
expectCreate: []client.Object{},
53-
expectUpdate: []client.Object{},
54-
expectDelete: []client.Object{},
55-
expectStartCtrl: false,
77+
existingObjects: []runtime.Object{
78+
co("ingress"),
79+
},
80+
expectCreate: []client.Object{},
81+
expectUpdate: []client.Object{},
82+
expectDelete: []client.Object{},
83+
expectStartCtrl: false,
5684
},
5785
{
5886
name: "gateway API enabled",
5987
gatewayAPIEnabled: true,
6088
gatewayAPIControllerEnabled: true,
89+
existingObjects: []runtime.Object{
90+
co("ingress"),
91+
},
6192
expectCreate: []client.Object{
6293
crd("gatewayclasses.gateway.networking.k8s.io"),
6394
crd("gateways.gateway.networking.k8s.io"),
@@ -75,6 +106,9 @@ func Test_Reconcile(t *testing.T) {
75106
name: "gateway API enabled, gateway API controller disabled",
76107
gatewayAPIEnabled: true,
77108
gatewayAPIControllerEnabled: false,
109+
existingObjects: []runtime.Object{
110+
co("ingress"),
111+
},
78112
expectCreate: []client.Object{
79113
crd("gatewayclasses.gateway.networking.k8s.io"),
80114
crd("gateways.gateway.networking.k8s.io"),
@@ -88,6 +122,87 @@ func Test_Reconcile(t *testing.T) {
88122
expectDelete: []client.Object{},
89123
expectStartCtrl: false,
90124
},
125+
{
126+
name: "unmanaged gateway API CRDs created",
127+
gatewayAPIEnabled: true,
128+
gatewayAPIControllerEnabled: true,
129+
existingObjects: []runtime.Object{
130+
co("ingress"),
131+
crd("listenersets.gateway.networking.x-k8s.io"),
132+
crd("backendtrafficpolicies.gateway.networking.x-k8s.io"),
133+
},
134+
existingStatusSubresource: []client.Object{
135+
co("ingress"),
136+
},
137+
expectCreate: []client.Object{
138+
crd("gatewayclasses.gateway.networking.k8s.io"),
139+
crd("gateways.gateway.networking.k8s.io"),
140+
crd("grpcroutes.gateway.networking.k8s.io"),
141+
crd("httproutes.gateway.networking.k8s.io"),
142+
crd("referencegrants.gateway.networking.k8s.io"),
143+
clusterRole("system:openshift:gateway-api:aggregate-to-admin"),
144+
clusterRole("system:openshift:gateway-api:aggregate-to-view"),
145+
},
146+
expectUpdate: []client.Object{},
147+
expectDelete: []client.Object{},
148+
expectStatusUpdate: []client.Object{
149+
coWithExtension("ingress", `{"unmanagedGatewayAPICRDNames":"backendtrafficpolicies.gateway.networking.x-k8s.io,listenersets.gateway.networking.x-k8s.io"}`),
150+
},
151+
expectStartCtrl: true,
152+
},
153+
{
154+
name: "unmanaged gateway API CRDs removed",
155+
gatewayAPIEnabled: true,
156+
gatewayAPIControllerEnabled: true,
157+
existingObjects: []runtime.Object{
158+
coWithExtension("ingress", `{"unmanagedGatewayAPICRDNames":"listenersets.gateway.networking.x-k8s.io"}`),
159+
},
160+
existingStatusSubresource: []client.Object{
161+
co("ingress"),
162+
},
163+
expectCreate: []client.Object{
164+
crd("gatewayclasses.gateway.networking.k8s.io"),
165+
crd("gateways.gateway.networking.k8s.io"),
166+
crd("grpcroutes.gateway.networking.k8s.io"),
167+
crd("httproutes.gateway.networking.k8s.io"),
168+
crd("referencegrants.gateway.networking.k8s.io"),
169+
clusterRole("system:openshift:gateway-api:aggregate-to-admin"),
170+
clusterRole("system:openshift:gateway-api:aggregate-to-view"),
171+
},
172+
expectUpdate: []client.Object{},
173+
expectDelete: []client.Object{},
174+
expectStatusUpdate: []client.Object{
175+
coWithExtension("ingress", `{}`),
176+
},
177+
expectStartCtrl: true,
178+
},
179+
{
180+
name: "third party CRDs",
181+
gatewayAPIEnabled: true,
182+
gatewayAPIControllerEnabled: true,
183+
existingObjects: []runtime.Object{
184+
co("ingress"),
185+
crd("thirdpartycrd1.openshift.io"),
186+
crd("thirdpartycrd2.openshift.io"),
187+
},
188+
existingStatusSubresource: []client.Object{
189+
co("ingress"),
190+
},
191+
expectCreate: []client.Object{
192+
crd("gatewayclasses.gateway.networking.k8s.io"),
193+
crd("gateways.gateway.networking.k8s.io"),
194+
crd("grpcroutes.gateway.networking.k8s.io"),
195+
crd("httproutes.gateway.networking.k8s.io"),
196+
crd("referencegrants.gateway.networking.k8s.io"),
197+
clusterRole("system:openshift:gateway-api:aggregate-to-admin"),
198+
clusterRole("system:openshift:gateway-api:aggregate-to-view"),
199+
},
200+
expectUpdate: []client.Object{},
201+
expectDelete: []client.Object{},
202+
// Third party CRDs have no impact on cluster operator status.
203+
expectStatusUpdate: []client.Object{},
204+
expectStartCtrl: true,
205+
},
91206
}
92207

93208
scheme := runtime.NewScheme()
@@ -100,11 +215,31 @@ func Test_Reconcile(t *testing.T) {
100215
fakeClient := fake.NewClientBuilder().
101216
WithScheme(scheme).
102217
WithRuntimeObjects(tc.existingObjects...).
218+
WithStatusSubresource(tc.existingStatusSubresource...).
219+
WithIndex(&apiextensionsv1.CustomResourceDefinition{}, "gatewayAPICRD", client.IndexerFunc(func(o client.Object) []string {
220+
// Assume that all experimental CRDs are unmanaged.
221+
if strings.Contains(o.GetName(), "gateway.networking.x-k8s.io") {
222+
return []string{"unmanaged"}
223+
}
224+
return []string{}
225+
})).
103226
Build()
104-
cl := &testutil.FakeClientRecorder{fakeClient, t, []client.Object{}, []client.Object{}, []client.Object{}}
227+
cl := &testutil.FakeClientRecorder{
228+
Client: fakeClient,
229+
T: t,
230+
Added: []client.Object{},
231+
Updated: []client.Object{},
232+
Deleted: []client.Object{},
233+
StatusWriter: &testutil.FakeStatusWriter{
234+
StatusWriter: fakeClient.Status(),
235+
},
236+
}
105237
ctrl := &testutil.FakeController{t, false, nil}
238+
informer := informertest.FakeInformers{Scheme: scheme}
239+
cache := &testutil.FakeCache{Informers: &informer, Reader: fakeClient}
106240
reconciler := &reconciler{
107241
client: cl,
242+
cache: cache,
108243
config: Config{
109244
GatewayAPIEnabled: tc.gatewayAPIEnabled,
110245
GatewayAPIControllerEnabled: tc.gatewayAPIControllerEnabled,
@@ -144,6 +279,9 @@ func Test_Reconcile(t *testing.T) {
144279
if diff := cmp.Diff(tc.expectDelete, cl.Deleted, cmpOpts...); diff != "" {
145280
t.Fatalf("found diff between expected and actual deletes: %s", diff)
146281
}
282+
if diff := cmp.Diff(tc.expectStatusUpdate, cl.StatusWriter.Updated, cmpOpts...); diff != "" {
283+
t.Fatalf("found diff between expected and actual status updates: %s", diff)
284+
}
147285
})
148286
}
149287
}
@@ -153,11 +291,30 @@ func TestReconcileOnlyStartsControllerOnce(t *testing.T) {
153291
configv1.Install(scheme)
154292
apiextensionsv1.AddToScheme(scheme)
155293
rbacv1.AddToScheme(scheme)
156-
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects().Build()
157-
cl := &testutil.FakeClientRecorder{fakeClient, t, []client.Object{}, []client.Object{}, []client.Object{}}
294+
fakeClient := fake.NewClientBuilder().
295+
WithScheme(scheme).
296+
WithRuntimeObjects(
297+
&configv1.ClusterOperator{
298+
ObjectMeta: metav1.ObjectMeta{Name: "ingress"},
299+
}).
300+
WithIndex(&apiextensionsv1.CustomResourceDefinition{}, "gatewayAPICRD", client.IndexerFunc(func(o client.Object) []string {
301+
// Assume that there are no unmanaged CRDs.
302+
return []string{}
303+
})).
304+
Build()
305+
cl := &testutil.FakeClientRecorder{
306+
Client: fakeClient,
307+
T: t,
308+
Added: []client.Object{},
309+
Updated: []client.Object{},
310+
Deleted: []client.Object{},
311+
}
158312
ctrl := &testutil.FakeController{t, false, make(chan struct{})}
313+
informer := informertest.FakeInformers{Scheme: scheme}
314+
cache := &testutil.FakeCache{Informers: &informer, Reader: fakeClient}
159315
reconciler := &reconciler{
160316
client: cl,
317+
cache: cache,
161318
config: Config{
162319
GatewayAPIEnabled: true,
163320
GatewayAPIControllerEnabled: true,

0 commit comments

Comments
 (0)