Skip to content

Commit f0e62c3

Browse files
Merge pull request #1232 from Miciah/OCPBUGS-55317-status-check-capabilities-for-subscriptions-watch
OCPBUGS-55317: Check capabilities before watching OLM resource
2 parents 2e6bd66 + 30dda05 commit f0e62c3

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)