Skip to content

Commit 0fa1281

Browse files
committed
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 for the gateways and istios resources, rather than 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 haveGatewayclassesResource, haveGatewaysResource, and haveIstiosResource in the operatorState object and conditionally add the corresponding resources to relatedObjectsCRDs. (operatorState): Add haveGatewaysResource, haveGatewayclassesResource, and haveIstiosResource fields. (getOperatorState): Set haveGatewaysResource, haveGatewayclassesResource, and haveIstiosResource. * pkg/operator/operator.go (New): Specify GatewayAPIControllerEnabled in the status controller config.
1 parent 500f13e commit 0fa1281

File tree

2 files changed

+100
-17
lines changed

2 files changed

+100
-17
lines changed

pkg/operator/controller/status/controller.go

Lines changed: 94 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ import (
2323

2424
corev1 "k8s.io/api/core/v1"
2525

26+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2627
"k8s.io/apimachinery/pkg/api/errors"
2728
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2829
"k8s.io/apimachinery/pkg/types"
30+
"k8s.io/apimachinery/pkg/util/sets"
2931
utilclock "k8s.io/utils/clock"
3032

3133
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
@@ -49,13 +51,27 @@ const (
4951

5052
ingressesEqualConditionMessage = "desired and current number of IngressControllers are equal"
5153

54+
// gatewaysResourceName is the name of the Gateway API gateways CRD.
55+
gatewaysResourceName = "gateways.gateway.networking.k8s.io"
56+
// gatewayclassesResourceName is the name of the Gateway API
57+
// gatewayclasses CRD.
58+
gatewayclassesResourceName = "gatewayclasses.gateway.networking.k8s.io"
59+
// istiosResourceName is the name of the Sail Operator istios CRD.
60+
istiosResourceName = "istios.sailoperator.io"
61+
5262
controllerName = "status_controller"
5363
)
5464

55-
var log = logf.Logger.WithName(controllerName)
65+
var (
66+
log = logf.Logger.WithName(controllerName)
67+
68+
// clock is to enable unit testing
69+
clock utilclock.Clock = utilclock.RealClock{}
5670

57-
// clock is to enable unit testing
58-
var clock utilclock.Clock = utilclock.RealClock{}
71+
// relatedObjectsCRDs is a set of names of CRDs that we add to
72+
// relatedObjects if they exist.
73+
relatedObjectsCRDs = sets.New[string](gatewaysResourceName, gatewayclassesResourceName, istiosResourceName)
74+
)
5975

6076
// New creates the status controller. This is the controller that handles all
6177
// 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) {
115131
})); err != nil {
116132
return nil, err
117133
}
134+
if config.GatewayAPIControllerEnabled {
135+
if err := c.Watch(source.Kind[client.Object](operatorCache, &apiextensionsv1.CustomResourceDefinition{}, handler.EnqueueRequestsFromMapFunc(toDefaultIngressController), predicate.Funcs{
136+
CreateFunc: func(e event.CreateEvent) bool {
137+
return relatedObjectsCRDs.Has(e.Object.GetName())
138+
},
139+
UpdateFunc: func(e event.UpdateEvent) bool {
140+
return false
141+
},
142+
DeleteFunc: func(e event.DeleteEvent) bool {
143+
return relatedObjectsCRDs.Has(e.Object.GetName())
144+
},
145+
GenericFunc: func(e event.GenericEvent) bool {
146+
return false
147+
},
148+
})); err != nil {
149+
return nil, err
150+
}
151+
}
118152
}
119153

120154
return c, nil
@@ -123,7 +157,10 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
123157
// Config holds all the things necessary for the controller to run.
124158
type Config struct {
125159
// GatewayAPIEnabled indicates that the "GatewayAPI" featuregate is enabled.
126-
GatewayAPIEnabled bool
160+
GatewayAPIEnabled bool
161+
// GatewayAPIControllerEnabled indicates that the "GatewayAPIControllerEnabled" featuregate is enabled.
162+
GatewayAPIControllerEnabled bool
163+
127164
IngressControllerImage string
128165
CanaryImage string
129166
OperatorReleaseVersion string
@@ -199,10 +236,12 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
199236
})
200237
}
201238
if r.config.GatewayAPIEnabled {
202-
related = append(related, configv1.ObjectReference{
203-
Group: gatewayapiv1.GroupName,
204-
Resource: "gatewayclasses",
205-
})
239+
if state.haveGatewayclassesResource {
240+
related = append(related, configv1.ObjectReference{
241+
Group: gatewayapiv1.GroupName,
242+
Resource: "gatewayclasses",
243+
})
244+
}
206245
if state.haveOSSMSubscription {
207246
subscriptionName := operatorcontroller.ServiceMeshOperatorSubscriptionName()
208247
related = append(related, configv1.ObjectReference{
@@ -211,12 +250,15 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
211250
Namespace: subscriptionName.Namespace,
212251
Name: subscriptionName.Name,
213252
})
214-
if state.IngressNamespace != nil {
253+
}
254+
if r.config.GatewayAPIControllerEnabled {
255+
if state.haveIstiosResource {
215256
related = append(related, configv1.ObjectReference{
216-
Group: sailv1.GroupVersion.Group,
217-
Resource: "istios",
218-
Namespace: state.IngressNamespace.Name,
257+
Group: sailv1.GroupVersion.Group,
258+
Resource: "istios",
219259
})
260+
}
261+
if state.haveGatewaysResource && state.IngressNamespace != nil {
220262
related = append(related, configv1.ObjectReference{
221263
Group: gatewayapiv1.GroupName,
222264
Resource: "gateways",
@@ -295,7 +337,16 @@ type operatorState struct {
295337
IngressControllers []operatorv1.IngressController
296338
DNSRecords []iov1.DNSRecord
297339

340+
// haveOSSMSubscription means that the subscription for OSSM 3 exists.
298341
haveOSSMSubscription bool
342+
// haveIstiosResource means that the "istios.sailproject.io" CRD exists.
343+
haveIstiosResource bool
344+
// haveGatewaysResource means that the
345+
// "gateways.gateway.networking.k8s.io" CRD exists.
346+
haveGatewaysResource bool
347+
// haveGatewayclassesResource means that the
348+
// "gatewayclasses.gateway.networking.k8s.io" CRD exists.
349+
haveGatewayclassesResource bool
299350
}
300351

301352
// getOperatorState gets and returns the resources necessary to compute the
@@ -338,6 +389,37 @@ func (r *reconciler) getOperatorState(ctx context.Context, ingressNamespace, can
338389
} else {
339390
state.haveOSSMSubscription = true
340391
}
392+
393+
if r.config.GatewayAPIControllerEnabled {
394+
var (
395+
crd apiextensionsv1.CustomResourceDefinition
396+
gatewaysResourceNamespacedName = types.NamespacedName{Name: gatewaysResourceName}
397+
gatewayclassesResourceNamespacedName = types.NamespacedName{Name: gatewayclassesResourceName}
398+
istiosResourceNamespacedName = types.NamespacedName{Name: istiosResourceName}
399+
)
400+
401+
if err := r.cache.Get(ctx, gatewaysResourceNamespacedName, &crd); err != nil {
402+
if !errors.IsNotFound(err) {
403+
return state, fmt.Errorf("failed to get CRD %q: %v", gatewaysResourceName, err)
404+
}
405+
} else {
406+
state.haveGatewaysResource = true
407+
}
408+
if err := r.cache.Get(ctx, gatewayclassesResourceNamespacedName, &crd); err != nil {
409+
if !errors.IsNotFound(err) {
410+
return state, fmt.Errorf("failed to get CRD %q: %v", gatewayclassesResourceName, err)
411+
}
412+
} else {
413+
state.haveGatewayclassesResource = true
414+
}
415+
if err := r.cache.Get(ctx, istiosResourceNamespacedName, &crd); err != nil {
416+
if !errors.IsNotFound(err) {
417+
return state, fmt.Errorf("failed to get CRD %q: %v", istiosResourceName, err)
418+
}
419+
} else {
420+
state.haveIstiosResource = true
421+
}
422+
}
341423
}
342424

343425
return state, nil

pkg/operator/operator.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,11 +208,12 @@ func New(config operatorconfig.Config, kubeConfig *rest.Config) (*Operator, erro
208208

209209
// Set up the status controller.
210210
if _, err := statuscontroller.New(mgr, statuscontroller.Config{
211-
Namespace: config.Namespace,
212-
IngressControllerImage: config.IngressControllerImage,
213-
CanaryImage: config.CanaryImage,
214-
OperatorReleaseVersion: config.OperatorReleaseVersion,
215-
GatewayAPIEnabled: gatewayAPIEnabled,
211+
Namespace: config.Namespace,
212+
IngressControllerImage: config.IngressControllerImage,
213+
CanaryImage: config.CanaryImage,
214+
OperatorReleaseVersion: config.OperatorReleaseVersion,
215+
GatewayAPIEnabled: gatewayAPIEnabled,
216+
GatewayAPIControllerEnabled: gatewayAPIControllerEnabled,
216217
}); err != nil {
217218
return nil, fmt.Errorf("failed to create status controller: %v", err)
218219
}

0 commit comments

Comments
 (0)