Skip to content

Commit 6f42089

Browse files
MiciahMiciah
committed
status: Conditionally add CRDs to relatedObjects
Check whether the gatewayclasses, gateways, and istios CRDs actually exist before adding them to relatedObjects. Watch customresourcedefinitions in the status controller so that it updates relatedObjects as these CRDs are created. Check the "GatewayAPIController" featuregate to determine whether to add the gatewayclasses, gateways, istios, and subscriptions resources to relatedObjects, in addition to checking 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): Check the GatewayAPIControllerEnabled field in the controller config in addition to checking GatewayAPIEnabled to determine whether to watch subscriptions and customresourcedefinitions. Add a watch on customresourcedefinitions, with a predicate for CRDs with names that are in relatedObjectsCRDs. (Config): Add GatewayAPIControllerEnabled. (Reconcile): Check the GatewayAPIControllerEnabled field in the controller config as well as the haveIstiosResource, haveGatewayclassesResource, and haveGatewaysResource fields in the operatorState object, and conditionally add the corresponding resources to relatedObjects. (operatorState): Add haveIstiosResource, haveGatewaysResource, and haveGatewayclassesResource fields. (getOperatorState): Check GatewayAPIControllerEnabled in addition to GatewayAPIEnabled before checking for the OSSM subscription. Set haveGatewaysResource, haveGatewayclassesResource, and haveIstiosResource. * pkg/operator/operator.go (New): Specify GatewayAPIControllerEnabled in the status controller config. * test/e2e/operator_test.go (TestClusterOperatorStatusRelatedObjects): Expect to see "gateways" and "gatewayclasses" in relatedObjects if the "GatewayAPI" and "GatewayAPIController" featuregates are enabled.
1 parent 751bb76 commit 6f42089

File tree

3 files changed

+122
-31
lines changed

3 files changed

+122
-31
lines changed

‎pkg/operator/controller/status/controller.go‎

Lines changed: 104 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@ import (
2424

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

27+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2728
"k8s.io/apimachinery/pkg/api/errors"
2829
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2930
"k8s.io/apimachinery/pkg/types"
31+
"k8s.io/apimachinery/pkg/util/sets"
3032
utilclock "k8s.io/utils/clock"
3133

3234
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
@@ -50,13 +52,27 @@ const (
5052

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

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

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

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

6177
// New creates the status controller. This is the controller that handles all
6278
// 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) {
99115
return nil, err
100116
}
101117

102-
// If the "GatewayAPI" controller featuregate is enabled, watch
103-
// subscriptions so that this controller can update status when the OSSM
104-
// subscription is created or updated. Note that the subscriptions
105-
// resource only exists if the "OperatorLifecycleManager" capability is
106-
// enabled, so we cannot watch it if the capability is not enabled.
107-
// Additionally, the default catalog only exists if the "marketplace"
108-
// capability is enabled, so we cannot install OSSM without that
109-
// capability.
110-
if config.GatewayAPIEnabled && config.MarketplaceEnabled && config.OperatorLifecycleManagerEnabled {
118+
// If the "GatewayAPI" and "GatewayAPIController" featuregates are
119+
// enabled, watch subscriptions so that this controller can update
120+
// status when the OSSM subscription is created or updated. Note that
121+
// the subscriptions resource only exists if the
122+
// "OperatorLifecycleManager" capability is enabled, so we cannot watch
123+
// it if the capability is not enabled. Additionally, the default
124+
// catalog only exists if the "marketplace" capability is enabled, so we
125+
// cannot install OSSM without that capability.
126+
if config.GatewayAPIEnabled && config.GatewayAPIControllerEnabled && config.MarketplaceEnabled && config.OperatorLifecycleManagerEnabled {
111127
if err := c.Watch(source.Kind[client.Object](operatorCache, &operatorsv1alpha1.Subscription{}, handler.EnqueueRequestsFromMapFunc(toDefaultIngressController), predicate.Funcs{
112128
CreateFunc: func(e event.CreateEvent) bool {
113129
return e.Object.GetNamespace() == operatorcontroller.OpenshiftOperatorNamespace
@@ -124,6 +140,22 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
124140
})); err != nil {
125141
return nil, err
126142
}
143+
if err := c.Watch(source.Kind[client.Object](operatorCache, &apiextensionsv1.CustomResourceDefinition{}, handler.EnqueueRequestsFromMapFunc(toDefaultIngressController), predicate.Funcs{
144+
CreateFunc: func(e event.CreateEvent) bool {
145+
return relatedObjectsCRDs.Has(e.Object.GetName())
146+
},
147+
UpdateFunc: func(e event.UpdateEvent) bool {
148+
return false
149+
},
150+
DeleteFunc: func(e event.DeleteEvent) bool {
151+
return relatedObjectsCRDs.Has(e.Object.GetName())
152+
},
153+
GenericFunc: func(e event.GenericEvent) bool {
154+
return false
155+
},
156+
})); err != nil {
157+
return nil, err
158+
}
127159
}
128160

129161
return c, nil
@@ -133,6 +165,9 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
133165
type Config struct {
134166
// GatewayAPIEnabled indicates that the "GatewayAPI" featuregate is enabled.
135167
GatewayAPIEnabled bool
168+
// GatewayAPIControllerEnabled indicates that the "GatewayAPIController"
169+
// featuregate is enabled.
170+
GatewayAPIControllerEnabled bool
136171
// MarketplaceEnabled indicates whether the "marketplace" capability is
137172
// enabled.
138173
MarketplaceEnabled bool
@@ -220,11 +255,7 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
220255
Name: state.CanaryNamespace.Name,
221256
})
222257
}
223-
if r.config.GatewayAPIEnabled {
224-
related = append(related, configv1.ObjectReference{
225-
Group: gatewayapiv1.GroupName,
226-
Resource: "gatewayclasses",
227-
})
258+
if r.config.GatewayAPIEnabled && r.config.GatewayAPIControllerEnabled {
228259
if state.haveOSSMSubscription {
229260
subscriptionName := operatorcontroller.ServiceMeshOperatorSubscriptionName()
230261
related = append(related, configv1.ObjectReference{
@@ -233,10 +264,20 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
233264
Namespace: subscriptionName.Namespace,
234265
Name: subscriptionName.Name,
235266
})
267+
}
268+
if state.haveIstiosResource {
236269
related = append(related, configv1.ObjectReference{
237270
Group: sailv1.GroupVersion.Group,
238271
Resource: "istios",
239272
})
273+
}
274+
if state.haveGatewayclassesResource {
275+
related = append(related, configv1.ObjectReference{
276+
Group: gatewayapiv1.GroupName,
277+
Resource: "gatewayclasses",
278+
})
279+
}
280+
if state.haveGatewaysResource {
240281
related = append(related, configv1.ObjectReference{
241282
Group: gatewayapiv1.GroupName,
242283
Resource: "gateways",
@@ -314,8 +355,17 @@ type operatorState struct {
314355
IngressControllers []operatorv1.IngressController
315356
DNSRecords []iov1.DNSRecord
316357

317-
haveOSSMSubscription bool
318358
unmanagedGatewayAPICRDNames string
359+
// haveOSSMSubscription means that the subscription for OSSM 3 exists.
360+
haveOSSMSubscription bool
361+
// haveIstiosResource means that the "istios.sailproject.io" CRD exists.
362+
haveIstiosResource bool
363+
// haveGatewaysResource means that the
364+
// "gateways.gateway.networking.k8s.io" CRD exists.
365+
haveGatewaysResource bool
366+
// haveGatewayclassesResource means that the
367+
// "gatewayclasses.gateway.networking.k8s.io" CRD exists.
368+
haveGatewayclassesResource bool
319369
}
320370

321371
// getOperatorState gets and returns the resources necessary to compute the
@@ -349,7 +399,15 @@ func (r *reconciler) getOperatorState(ctx context.Context, ingressNamespace, can
349399
}
350400

351401
if r.config.GatewayAPIEnabled {
352-
if r.config.MarketplaceEnabled && r.config.OperatorLifecycleManagerEnabled {
402+
if len(co.Status.Extension.Raw) > 0 {
403+
extension := &IngressOperatorStatusExtension{}
404+
if err := json.Unmarshal(co.Status.Extension.Raw, extension); err != nil {
405+
return state, fmt.Errorf("failed to unmarshal status extension of cluster operator %q: %w", co.Name, err)
406+
}
407+
state.unmanagedGatewayAPICRDNames = extension.UnmanagedGatewayAPICRDNames
408+
}
409+
410+
if r.config.GatewayAPIControllerEnabled && r.config.MarketplaceEnabled && r.config.OperatorLifecycleManagerEnabled {
353411
var subscription operatorsv1alpha1.Subscription
354412
subscriptionName := operatorcontroller.ServiceMeshOperatorSubscriptionName()
355413
if err := r.cache.Get(ctx, subscriptionName, &subscription); err != nil {
@@ -360,14 +418,35 @@ func (r *reconciler) getOperatorState(ctx context.Context, ingressNamespace, can
360418
state.haveOSSMSubscription = true
361419

362420
}
363-
}
364421

365-
if len(co.Status.Extension.Raw) > 0 {
366-
extension := &IngressOperatorStatusExtension{}
367-
if err := json.Unmarshal(co.Status.Extension.Raw, extension); err != nil {
368-
return state, fmt.Errorf("failed to unmarshal status extension of cluster operator %q: %w", co.Name, err)
422+
var (
423+
crd apiextensionsv1.CustomResourceDefinition
424+
gatewaysResourceNamespacedName = types.NamespacedName{Name: gatewaysResourceName}
425+
gatewayclassesResourceNamespacedName = types.NamespacedName{Name: gatewayclassesResourceName}
426+
istiosResourceNamespacedName = types.NamespacedName{Name: istiosResourceName}
427+
)
428+
429+
if err := r.cache.Get(ctx, gatewaysResourceNamespacedName, &crd); err != nil {
430+
if !errors.IsNotFound(err) {
431+
return state, fmt.Errorf("failed to get CRD %q: %v", gatewaysResourceName, err)
432+
}
433+
} else {
434+
state.haveGatewaysResource = true
435+
}
436+
if err := r.cache.Get(ctx, gatewayclassesResourceNamespacedName, &crd); err != nil {
437+
if !errors.IsNotFound(err) {
438+
return state, fmt.Errorf("failed to get CRD %q: %v", gatewayclassesResourceName, err)
439+
}
440+
} else {
441+
state.haveGatewayclassesResource = true
442+
}
443+
if err := r.cache.Get(ctx, istiosResourceNamespacedName, &crd); err != nil {
444+
if !errors.IsNotFound(err) {
445+
return state, fmt.Errorf("failed to get CRD %q: %v", istiosResourceName, err)
446+
}
447+
} else {
448+
state.haveIstiosResource = true
369449
}
370-
state.unmanagedGatewayAPICRDNames = extension.UnmanagedGatewayAPICRDNames
371450
}
372451
}
373452

‎pkg/operator/operator.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ func New(config operatorconfig.Config, kubeConfig *rest.Config) (*Operator, erro
225225
CanaryImage: config.CanaryImage,
226226
OperatorReleaseVersion: config.OperatorReleaseVersion,
227227
GatewayAPIEnabled: gatewayAPIEnabled,
228+
GatewayAPIControllerEnabled: gatewayAPIControllerEnabled,
228229
MarketplaceEnabled: marketplaceEnabled,
229230
OperatorLifecycleManagerEnabled: olmEnabled,
230231
}); err != nil {

‎test/e2e/operator_test.go‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -233,12 +233,23 @@ func TestClusterOperatorStatusRelatedObjects(t *testing.T) {
233233
if gatewayAPIEnabled, err := isFeatureGateEnabled(features.FeatureGateGatewayAPI); err != nil {
234234
t.Fatalf("Failed to look up %q featuregate: %v", features.FeatureGateGatewayAPI, err)
235235
} else if gatewayAPIEnabled {
236-
expected = append(expected, configv1.ObjectReference{
237-
Group: "gateway.networking.k8s.io",
238-
Resource: "gatewayclasses",
239-
})
240-
// This test runs before TestGatewayAPI, so we do *not* expect
241-
// to see subscriptions, istios, or gateways in relatedObjects.
236+
if gatewayAPIControllerEnabled, err := isFeatureGateEnabled(features.FeatureGateGatewayAPIController); err != nil {
237+
t.Fatalf("Failed to look up %q featuregate: %v", features.FeatureGateGatewayAPIController, err)
238+
} else if gatewayAPIControllerEnabled {
239+
// This test runs before TestGatewayAPI creates the
240+
// subscription to install OSSM, so we do *not* expect
241+
// to see subscriptions or istios in relatedObjects.
242+
// However, we *do* expect to see gatewayclasses and
243+
// gateways whenever the GatewayAPI and
244+
// GatewayAPIController featuregates are enabled.
245+
expected = append(expected, configv1.ObjectReference{
246+
Group: "gateway.networking.k8s.io",
247+
Resource: "gatewayclasses",
248+
}, configv1.ObjectReference{
249+
Group: "gateway.networking.k8s.io",
250+
Resource: "gateways",
251+
})
252+
}
242253
}
243254

244255
coName := controller.IngressClusterOperatorName()

0 commit comments

Comments
 (0)