Skip to content

Commit 30dda05

Browse files
MiciahMiciah
committed
Check capabilities before watching OLM resource
Only try to watch subscriptions and installplans if the "marketplace" and "OperatorLifecycleManager" capabilities are enabled. If the "OperatorLifecycleManager" capability is not enabled, the installplans and subscriptions resources do not exist, and if the "marketplace" capability is not enabled, the default catalog does not exist. Before this commit, the operator would fail to initialize when these capabilities were not enabled as the status and gatewayclass controllers would try and fail to watch the non-existent resources. This commit fixes OCPBUGS-55317 https://issues.redhat.com/browse/OCPBUGS-55317 * pkg/operator/controller/gatewayapi/controller.go (Config): Add MarketplaceEnabled and OperatorLifecycleManagerEnabled fields to indicate whether the associated capabilities are enabled. (Reconcile): Don't enable dependent controllers if the marketplace and OLM capabilities are not enabled. * pkg/operator/controller/gatewayapi/controller_test.go (Test_Reconcile): Add a test case in which the featuregates are enabled but the marketplace and OLM capabilities are not enabled. Indicate that the capabilities are enabled for the existing test cases. (TestReconcileOnlyStartsControllerOnce): Set MarketplaceEnabled and OperatorLifecycleManagerEnabled to true for the test. * pkg/operator/controller/status/controller.go (New): Only watch subscriptions if the required capabilities are enabled. (Config): Add MarketplaceEnabled and OperatorLifecycleManagerEnabled fields. (getOperatorState): Only try to get the subscription if the required capabilities are enabled. * pkg/operator/operator.go (New): Read the capabilities from the cluster ClusterVersion, and set the MarketplaceEnabled and OperatorLifecycleManagerEnabled fields in the status controller's and gatewayapi controller's configs accordingly.
1 parent 2e6bd66 commit 30dda05

File tree

4 files changed

+117
-28
lines changed

4 files changed

+117
-28
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,12 @@ type Config struct {
104104
GatewayAPIEnabled bool
105105
// GatewayAPIControllerEnabled indicates that the "GatewayAPIController" featuregate is enabled.
106106
GatewayAPIControllerEnabled bool
107+
// MarketplaceEnabled indicates whether the "marketplace" capability is
108+
// enabled.
109+
MarketplaceEnabled bool
110+
// OperatorLifecycleManagerEnabled indicates whether the
111+
// "OperatorLifecycleManager" capability is enabled.
112+
OperatorLifecycleManagerEnabled bool
107113

108114
// DependentControllers is a list of controllers that watch Gateway API
109115
// resources. The gatewayapi controller starts these controllers once
@@ -148,6 +154,16 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
148154
return reconcile.Result{}, nil
149155
}
150156

157+
// The subscriptions resource only exists if the
158+
// "OperatorLifecycleManager" capability is enabled, and the default
159+
// catalog only exists if the "marketplace" capability is enabled. We
160+
// cannot install OSSM if the subscriptions resource or default catalog
161+
// does not exist, and accordingly, we should not start these
162+
// controllers if the capabilities are not enabled.
163+
if !r.config.MarketplaceEnabled || !r.config.OperatorLifecycleManagerEnabled {
164+
return reconcile.Result{}, nil
165+
}
166+
151167
r.startControllers.Do(func() {
152168
for i := range r.config.DependentControllers {
153169
c := &r.config.DependentControllers[i]

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

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ func Test_Reconcile(t *testing.T) {
5757
name string
5858
gatewayAPIEnabled bool
5959
gatewayAPIControllerEnabled bool
60+
marketplaceEnabled bool
61+
olmEnabled bool
6062
existingObjects []runtime.Object
6163
// existingStatusSubresource contains the original version of objects
6264
// whose status will updated by Reconcile function.
@@ -72,8 +74,10 @@ func Test_Reconcile(t *testing.T) {
7274
expectStartCtrl bool
7375
}{
7476
{
75-
name: "gateway API disabled",
76-
gatewayAPIEnabled: false,
77+
name: "gateway API disabled",
78+
gatewayAPIEnabled: false,
79+
marketplaceEnabled: true,
80+
olmEnabled: true,
7781
existingObjects: []runtime.Object{
7882
co("ingress"),
7983
},
@@ -86,6 +90,8 @@ func Test_Reconcile(t *testing.T) {
8690
name: "gateway API enabled",
8791
gatewayAPIEnabled: true,
8892
gatewayAPIControllerEnabled: true,
93+
marketplaceEnabled: true,
94+
olmEnabled: true,
8995
existingObjects: []runtime.Object{
9096
co("ingress"),
9197
},
@@ -106,6 +112,30 @@ func Test_Reconcile(t *testing.T) {
106112
name: "gateway API enabled, gateway API controller disabled",
107113
gatewayAPIEnabled: true,
108114
gatewayAPIControllerEnabled: false,
115+
marketplaceEnabled: true,
116+
olmEnabled: true,
117+
existingObjects: []runtime.Object{
118+
co("ingress"),
119+
},
120+
expectCreate: []client.Object{
121+
crd("gatewayclasses.gateway.networking.k8s.io"),
122+
crd("gateways.gateway.networking.k8s.io"),
123+
crd("grpcroutes.gateway.networking.k8s.io"),
124+
crd("httproutes.gateway.networking.k8s.io"),
125+
crd("referencegrants.gateway.networking.k8s.io"),
126+
clusterRole("system:openshift:gateway-api:aggregate-to-admin"),
127+
clusterRole("system:openshift:gateway-api:aggregate-to-view"),
128+
},
129+
expectUpdate: []client.Object{},
130+
expectDelete: []client.Object{},
131+
expectStartCtrl: false,
132+
},
133+
{
134+
name: "GatewayAPI enabled, GatewayAPIController enabled, marketplace and OLM capabilities disabled",
135+
gatewayAPIEnabled: true,
136+
gatewayAPIControllerEnabled: true,
137+
marketplaceEnabled: false,
138+
olmEnabled: false,
109139
existingObjects: []runtime.Object{
110140
co("ingress"),
111141
},
@@ -126,6 +156,8 @@ func Test_Reconcile(t *testing.T) {
126156
name: "unmanaged gateway API CRDs created",
127157
gatewayAPIEnabled: true,
128158
gatewayAPIControllerEnabled: true,
159+
marketplaceEnabled: true,
160+
olmEnabled: true,
129161
existingObjects: []runtime.Object{
130162
co("ingress"),
131163
crd("listenersets.gateway.networking.x-k8s.io"),
@@ -154,6 +186,8 @@ func Test_Reconcile(t *testing.T) {
154186
name: "unmanaged gateway API CRDs removed",
155187
gatewayAPIEnabled: true,
156188
gatewayAPIControllerEnabled: true,
189+
marketplaceEnabled: true,
190+
olmEnabled: true,
157191
existingObjects: []runtime.Object{
158192
coWithExtension("ingress", `{"unmanagedGatewayAPICRDNames":"listenersets.gateway.networking.x-k8s.io"}`),
159193
},
@@ -180,6 +214,8 @@ func Test_Reconcile(t *testing.T) {
180214
name: "third party CRDs",
181215
gatewayAPIEnabled: true,
182216
gatewayAPIControllerEnabled: true,
217+
marketplaceEnabled: true,
218+
olmEnabled: true,
183219
existingObjects: []runtime.Object{
184220
co("ingress"),
185221
crd("thirdpartycrd1.openshift.io"),
@@ -241,9 +277,11 @@ func Test_Reconcile(t *testing.T) {
241277
client: cl,
242278
cache: cache,
243279
config: Config{
244-
GatewayAPIEnabled: tc.gatewayAPIEnabled,
245-
GatewayAPIControllerEnabled: tc.gatewayAPIControllerEnabled,
246-
DependentControllers: []controller.Controller{ctrl},
280+
GatewayAPIEnabled: tc.gatewayAPIEnabled,
281+
GatewayAPIControllerEnabled: tc.gatewayAPIControllerEnabled,
282+
MarketplaceEnabled: tc.marketplaceEnabled,
283+
OperatorLifecycleManagerEnabled: tc.olmEnabled,
284+
DependentControllers: []controller.Controller{ctrl},
247285
},
248286
}
249287
req := reconcile.Request{
@@ -316,9 +354,11 @@ func TestReconcileOnlyStartsControllerOnce(t *testing.T) {
316354
client: cl,
317355
cache: cache,
318356
config: Config{
319-
GatewayAPIEnabled: true,
320-
GatewayAPIControllerEnabled: true,
321-
DependentControllers: []controller.Controller{ctrl},
357+
GatewayAPIEnabled: true,
358+
GatewayAPIControllerEnabled: true,
359+
MarketplaceEnabled: true,
360+
OperatorLifecycleManagerEnabled: true,
361+
DependentControllers: []controller.Controller{ctrl},
322362
},
323363
}
324364
req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "cluster"}}

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

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,15 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
9999
return nil, err
100100
}
101101

102-
if config.GatewayAPIEnabled {
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 {
103111
if err := c.Watch(source.Kind[client.Object](operatorCache, &operatorsv1alpha1.Subscription{}, handler.EnqueueRequestsFromMapFunc(toDefaultIngressController), predicate.Funcs{
104112
CreateFunc: func(e event.CreateEvent) bool {
105113
return e.Object.GetNamespace() == operatorcontroller.OpenshiftOperatorNamespace
@@ -124,11 +132,17 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
124132
// Config holds all the things necessary for the controller to run.
125133
type Config struct {
126134
// GatewayAPIEnabled indicates that the "GatewayAPI" featuregate is enabled.
127-
GatewayAPIEnabled bool
128-
IngressControllerImage string
129-
CanaryImage string
130-
OperatorReleaseVersion string
131-
Namespace string
135+
GatewayAPIEnabled bool
136+
// MarketplaceEnabled indicates whether the "marketplace" capability is
137+
// enabled.
138+
MarketplaceEnabled bool
139+
// OperatorLifecycleManagerEnabled indicates whether the
140+
// "OperatorLifecycleManager" capability is enabled.
141+
OperatorLifecycleManagerEnabled bool
142+
IngressControllerImage string
143+
CanaryImage string
144+
OperatorReleaseVersion string
145+
Namespace string
132146
}
133147

134148
// IngressOperatorStatusExtension holds status extensions of the ingress cluster operator.
@@ -338,14 +352,17 @@ func (r *reconciler) getOperatorState(ingressNamespace, canaryNamespace string,
338352
}
339353

340354
if r.config.GatewayAPIEnabled {
341-
var subscription operatorsv1alpha1.Subscription
342-
subscriptionName := operatorcontroller.ServiceMeshOperatorSubscriptionName()
343-
if err := r.cache.Get(context.TODO(), subscriptionName, &subscription); err != nil {
344-
if !errors.IsNotFound(err) {
345-
return state, fmt.Errorf("failed to get subscription %q: %v", subscriptionName, err)
355+
if r.config.MarketplaceEnabled && r.config.OperatorLifecycleManagerEnabled {
356+
var subscription operatorsv1alpha1.Subscription
357+
subscriptionName := operatorcontroller.ServiceMeshOperatorSubscriptionName()
358+
if err := r.cache.Get(context.TODO(), subscriptionName, &subscription); err != nil {
359+
if !errors.IsNotFound(err) {
360+
return state, fmt.Errorf("failed to get subscription %q: %v", subscriptionName, err)
361+
}
362+
} else {
363+
state.haveOSSMSubscription = true
364+
346365
}
347-
} else {
348-
state.haveOSSMSubscription = true
349366
}
350367

351368
if len(co.Status.Extension.Raw) > 0 {

‎pkg/operator/operator.go‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import (
4646
"k8s.io/apimachinery/pkg/api/errors"
4747
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
4848
"k8s.io/apimachinery/pkg/types"
49+
"k8s.io/apimachinery/pkg/util/sets"
4950
"k8s.io/apimachinery/pkg/util/wait"
5051
"k8s.io/client-go/rest"
5152
"k8s.io/client-go/util/retry"
@@ -140,6 +141,17 @@ func New(config operatorconfig.Config, kubeConfig *rest.Config) (*Operator, erro
140141
ingressControllerDCMEnabled := featureGates.Enabled(features.FeatureGateIngressControllerDynamicConfigurationManager)
141142
gcpCustomEndpointsEnabled := featureGates.Enabled(features.FeatureGateGCPCustomAPIEndpoints)
142143

144+
cv, err := configClient.ConfigV1().ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
145+
if err != nil {
146+
return nil, fmt.Errorf("failed fetching clusterversion: %w", err)
147+
}
148+
enabledCapabilities := sets.New[configv1.ClusterVersionCapability]()
149+
for _, cap := range cv.Status.Capabilities.EnabledCapabilities {
150+
enabledCapabilities.Insert(cap)
151+
}
152+
marketplaceEnabled := enabledCapabilities.Has(configv1.ClusterVersionCapabilityMarketplace)
153+
olmEnabled := enabledCapabilities.Has(configv1.ClusterVersionCapabilityOperatorLifecycleManager)
154+
143155
// Set up an operator manager for the operator namespace.
144156
mgr, err := manager.New(kubeConfig, manager.Options{
145157
Scheme: scheme,
@@ -208,11 +220,13 @@ func New(config operatorconfig.Config, kubeConfig *rest.Config) (*Operator, erro
208220

209221
// Set up the status controller.
210222
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,
223+
Namespace: config.Namespace,
224+
IngressControllerImage: config.IngressControllerImage,
225+
CanaryImage: config.CanaryImage,
226+
OperatorReleaseVersion: config.OperatorReleaseVersion,
227+
GatewayAPIEnabled: gatewayAPIEnabled,
228+
MarketplaceEnabled: marketplaceEnabled,
229+
OperatorLifecycleManagerEnabled: olmEnabled,
216230
}); err != nil {
217231
return nil, fmt.Errorf("failed to create status controller: %v", err)
218232
}
@@ -324,8 +338,10 @@ func New(config operatorconfig.Config, kubeConfig *rest.Config) (*Operator, erro
324338

325339
// Set up the gatewayapi controller.
326340
if _, err := gatewayapicontroller.New(mgr, gatewayapicontroller.Config{
327-
GatewayAPIEnabled: gatewayAPIEnabled,
328-
GatewayAPIControllerEnabled: gatewayAPIControllerEnabled,
341+
GatewayAPIEnabled: gatewayAPIEnabled,
342+
GatewayAPIControllerEnabled: gatewayAPIControllerEnabled,
343+
MarketplaceEnabled: marketplaceEnabled,
344+
OperatorLifecycleManagerEnabled: olmEnabled,
329345
DependentControllers: []controller.Controller{
330346
gatewayClassController,
331347
gatewayServiceDNSController,

0 commit comments

Comments
 (0)