Skip to content

Commit 1acac06

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #946 from alebedev87/required-capabilities
NE-1318: Add always-enable-capabilities flag and set Ingress as always enabled
2 parents 5e73deb + ade334a commit 1acac06

File tree

9 files changed

+116
-30
lines changed

9 files changed

+116
-30
lines changed

‎cmd/start.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,5 +47,6 @@ func init() {
4747
cmd.PersistentFlags().StringVar(&opts.PrometheusURLString, "metrics-url", opts.PrometheusURLString, "The URL used to access the remote PromQL query service.")
4848
cmd.PersistentFlags().BoolVar(&opts.InjectClusterIdIntoPromQL, "hypershift", opts.InjectClusterIdIntoPromQL, "This options indicates whether the CVO is running inside a hosted control plane.")
4949
cmd.PersistentFlags().StringVar(&opts.UpdateService, "update-service", opts.UpdateService, "The preferred update service. If set, this option overrides any upstream value configured in ClusterVersion spec.")
50+
cmd.PersistentFlags().StringSliceVar(&opts.AlwaysEnableCapabilities, "always-enable-capabilities", opts.AlwaysEnableCapabilities, "List of the cluster capabilities which will always be implicitly enabled.")
5051
rootCmd.AddCommand(cmd)
5152
}

‎install/0000_00_cluster-version-operator_03_deployment.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ spec:
3535
- "--serving-cert-file=/etc/tls/serving-cert/tls.crt"
3636
- "--serving-key-file=/etc/tls/serving-cert/tls.key"
3737
- "--v=2"
38+
- "--always-enable-capabilities=Ingress"
3839
resources:
3940
requests:
4041
cpu: 20m

‎lib/capability/capability.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,13 @@ func (caps capabilitiesSort) Less(i, j int) bool { return string(caps[i]) < stri
3737
// SetCapabilities populates and returns cluster capabilities from ClusterVersion capabilities spec. This method also
3838
// ensures that no previousily enabled capability is now disabled and returns any such implicitly enabled capabilities.
3939
func SetCapabilities(config *configv1.ClusterVersion,
40-
existingEnabled map[configv1.ClusterVersionCapability]struct{}) ClusterCapabilities {
40+
existingEnabled, alwaysEnabled map[configv1.ClusterVersionCapability]struct{}) ClusterCapabilities {
4141

4242
var capabilities ClusterCapabilities
4343
capabilities.KnownCapabilities = setKnownCapabilities()
4444

4545
capabilities.EnabledCapabilities, capabilities.ImplicitlyEnabledCapabilities = setEnabledCapabilities(config.Spec.Capabilities,
46-
existingEnabled)
46+
existingEnabled, alwaysEnabled)
4747

4848
return capabilities
4949
}
@@ -150,8 +150,9 @@ func setKnownCapabilities() map[configv1.ClusterVersionCapability]struct{} {
150150
// DefaultCapabilitySet is used if a baseline capability set is not defined by ClusterVersion. A check is then made to
151151
// ensure that no previousily enabled capability is now disabled and if any such capabilities are found each is enabled,
152152
// saved, and returned.
153+
// The required capabilities are added to the implicitly enabled.
153154
func setEnabledCapabilities(capabilitiesSpec *configv1.ClusterVersionCapabilitiesSpec,
154-
priorEnabled map[configv1.ClusterVersionCapability]struct{}) (map[configv1.ClusterVersionCapability]struct{},
155+
priorEnabled, alwaysEnabled map[configv1.ClusterVersionCapability]struct{}) (map[configv1.ClusterVersionCapability]struct{},
155156
[]configv1.ClusterVersionCapability) {
156157

157158
capSet := DefaultCapabilitySet
@@ -176,6 +177,12 @@ func setEnabledCapabilities(capabilitiesSpec *configv1.ClusterVersionCapabilitie
176177
enabled[k] = struct{}{}
177178
}
178179
}
180+
for k := range alwaysEnabled {
181+
if _, ok := enabled[k]; !ok {
182+
implicitlyEnabled = append(implicitlyEnabled, k)
183+
enabled[k] = struct{}{}
184+
}
185+
}
179186
sort.Sort(capabilitiesSort(implicitlyEnabled))
180187
return enabled, implicitlyEnabled
181188
}

‎lib/capability/capability_test.go

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func TestSetCapabilities(t *testing.T) {
101101
}
102102
for _, test := range tests {
103103
t.Run(test.name, func(t *testing.T) {
104-
caps := SetCapabilities(test.config, nil)
104+
caps := SetCapabilities(test.config, nil, nil)
105105
if test.config.Spec.Capabilities == nil || (test.config.Spec.Capabilities != nil &&
106106
len(test.config.Spec.Capabilities.BaselineCapabilitySet) == 0) {
107107

@@ -130,26 +130,50 @@ func TestSetCapabilities(t *testing.T) {
130130

131131
func TestSetCapabilitiesWithImplicitlyEnabled(t *testing.T) {
132132
tests := []struct {
133-
name string
134-
config *configv1.ClusterVersion
135-
wantImplicit []string
136-
priorEnabled map[configv1.ClusterVersionCapability]struct{}
133+
name string
134+
config *configv1.ClusterVersion
135+
wantImplicit []string
136+
priorEnabled map[configv1.ClusterVersionCapability]struct{}
137+
alwaysEnabled map[configv1.ClusterVersionCapability]struct{}
137138
}{
138-
{name: "set capabilities 4_11 with additional",
139+
{name: "set baseline capabilities with additional",
139140
config: &configv1.ClusterVersion{
140141
Spec: configv1.ClusterVersionSpec{
141142
Capabilities: &configv1.ClusterVersionCapabilitiesSpec{
142-
BaselineCapabilitySet: configv1.ClusterVersionCapabilitySet4_11,
143+
BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetCurrent,
143144
},
144145
},
145146
},
146147
priorEnabled: map[configv1.ClusterVersionCapability]struct{}{"cap1": {}, "cap2": {}, "cap3": {}},
147148
wantImplicit: []string{"cap1", "cap2", "cap3"},
148149
},
150+
{name: "set baseline capabilities with always enabled",
151+
config: &configv1.ClusterVersion{
152+
Spec: configv1.ClusterVersionSpec{
153+
Capabilities: &configv1.ClusterVersionCapabilitiesSpec{
154+
BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetCurrent,
155+
},
156+
},
157+
},
158+
alwaysEnabled: map[configv1.ClusterVersionCapability]struct{}{"cap1": {}, "cap2": {}, "cap3": {}},
159+
wantImplicit: []string{"cap1", "cap2", "cap3"},
160+
},
161+
{name: "set baseline capabilities with additional and always enabled",
162+
config: &configv1.ClusterVersion{
163+
Spec: configv1.ClusterVersionSpec{
164+
Capabilities: &configv1.ClusterVersionCapabilitiesSpec{
165+
BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetCurrent,
166+
},
167+
},
168+
},
169+
priorEnabled: map[configv1.ClusterVersionCapability]struct{}{"cap1": {}, "cap2": {}, "cap3": {}},
170+
alwaysEnabled: map[configv1.ClusterVersionCapability]struct{}{"cap1": {}, "cap2": {}, "cap4": {}},
171+
wantImplicit: []string{"cap1", "cap2", "cap3", "cap4"},
172+
},
149173
}
150174
for _, test := range tests {
151175
t.Run(test.name, func(t *testing.T) {
152-
caps := SetCapabilities(test.config, test.priorEnabled)
176+
caps := SetCapabilities(test.config, test.priorEnabled, test.alwaysEnabled)
153177
if len(caps.ImplicitlyEnabledCapabilities) != len(test.wantImplicit) {
154178
t.Errorf("Incorrect number of implicitly enabled keys, wanted: %q. ImplicitlyEnabledCapabilities returned: %v", test.wantImplicit, caps.ImplicitlyEnabledCapabilities)
155179
}

‎pkg/cvo/cvo.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ type Operator struct {
172172

173173
clusterProfile string
174174
uid types.UID
175+
176+
// alwaysEnableCapabilities is a list of the cluster capabilities which should
177+
// always be implicitly enabled.
178+
alwaysEnableCapabilities []configv1.ClusterVersionCapability
175179
}
176180

177181
// New returns a new cluster version operator.
@@ -193,6 +197,7 @@ func New(
193197
promqlTarget clusterconditions.PromQLTarget,
194198
injectClusterIdIntoPromQL bool,
195199
updateService string,
200+
alwaysEnableCapabilities []configv1.ClusterVersionCapability,
196201
) (*Operator, error) {
197202
eventBroadcaster := record.NewBroadcaster()
198203
eventBroadcaster.StartLogging(klog.Infof)
@@ -228,7 +233,8 @@ func New(
228233
// Because of OCPBUGS-30080, we can only detect the enabled feature gates after Operator loads the initial payload
229234
// from disk via LoadInitialPayload. We must not have any gate-checking code until that happens, so we initialize
230235
// this field with a checker that panics when used.
231-
enabledFeatureGates: featuregates.PanicOnUsageBeforeInitialization,
236+
enabledFeatureGates: featuregates.PanicOnUsageBeforeInitialization,
237+
alwaysEnableCapabilities: alwaysEnableCapabilities,
232238
}
233239

234240
if _, err := cvInformer.Informer().AddEventHandler(optr.clusterVersionEventHandler()); err != nil {
@@ -341,6 +347,7 @@ func (optr *Operator) InitializeFromPayload(update *payload.Update, requiredFeat
341347
requiredFeatureSet,
342348
optr.eventRecorder,
343349
optr.clusterProfile,
350+
optr.alwaysEnableCapabilities,
344351
)
345352
}
346353

‎pkg/cvo/cvo_scenarios_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ func setupCVOTest(payloadDir string) (*Operator, map[string]apiruntime.Object, *
135135
"",
136136
record.NewFakeRecorder(100),
137137
o.clusterProfile,
138+
[]configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityIngress},
138139
)
139140
o.configSync = worker
140141

@@ -1971,7 +1972,7 @@ func TestCVO_UpgradeFailedPayloadLoadWithCapsChanges(t *testing.T) {
19711972

19721973
// confirm capabilities are updated
19731974
checkStatus(t, actions[1], "update", "clusterversions", "status", "", configv1.ClusterVersionCapabilitiesStatus{
1974-
EnabledCapabilities: []configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityBaremetal, configv1.ClusterVersionCapabilityMarketplace},
1975+
EnabledCapabilities: []configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityIngress, configv1.ClusterVersionCapabilityBaremetal, configv1.ClusterVersionCapabilityMarketplace},
19751976
KnownCapabilities: sortedCaps,
19761977
})
19771978
}

‎pkg/cvo/sync_worker.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,16 @@ type SyncWorker struct {
181181
requiredFeatureSet configv1.FeatureSet
182182

183183
clusterProfile string
184+
185+
// alwaysEnableCapabilities is a list of cluster capabilities which should
186+
// always be implicitly enabled.
187+
// This contributes to whether or not some manifests are included for reconciliation.
188+
alwaysEnableCapabilities []configv1.ClusterVersionCapability
184189
}
185190

186191
// NewSyncWorker initializes a ConfigSyncWorker that will retrieve payloads to disk, apply them via builder
187192
// to a server, and obey limits about how often to reconcile or retry on errors.
188-
func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, requiredFeatureSet configv1.FeatureSet, eventRecorder record.EventRecorder, clusterProfile string) *SyncWorker {
193+
func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, requiredFeatureSet configv1.FeatureSet, eventRecorder record.EventRecorder, clusterProfile string, alwaysEnableCapabilities []configv1.ClusterVersionCapability) *SyncWorker {
189194
return &SyncWorker{
190195
retriever: retriever,
191196
builder: builder,
@@ -200,18 +205,18 @@ func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder,
200205
// if the reader is not fast enough.
201206
report: make(chan SyncWorkerStatus, 500),
202207

203-
exclude: exclude,
204-
requiredFeatureSet: requiredFeatureSet,
205-
206-
clusterProfile: clusterProfile,
208+
exclude: exclude,
209+
requiredFeatureSet: requiredFeatureSet,
210+
clusterProfile: clusterProfile,
211+
alwaysEnableCapabilities: alwaysEnableCapabilities,
207212
}
208213
}
209214

210215
// NewSyncWorkerWithPreconditions initializes a ConfigSyncWorker that will retrieve payloads to disk, apply them via builder
211216
// to a server, and obey limits about how often to reconcile or retry on errors.
212217
// It allows providing preconditions for loading payload.
213-
func NewSyncWorkerWithPreconditions(retriever PayloadRetriever, builder payload.ResourceBuilder, preconditions precondition.List, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, requiredFeatureSet configv1.FeatureSet, eventRecorder record.EventRecorder, clusterProfile string) *SyncWorker {
214-
worker := NewSyncWorker(retriever, builder, reconcileInterval, backoff, exclude, requiredFeatureSet, eventRecorder, clusterProfile)
218+
func NewSyncWorkerWithPreconditions(retriever PayloadRetriever, builder payload.ResourceBuilder, preconditions precondition.List, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, requiredFeatureSet configv1.FeatureSet, eventRecorder record.EventRecorder, clusterProfile string, alwaysEnableCapabilities []configv1.ClusterVersionCapability) *SyncWorker {
219+
worker := NewSyncWorker(retriever, builder, reconcileInterval, backoff, exclude, requiredFeatureSet, eventRecorder, clusterProfile, alwaysEnableCapabilities)
215220
worker.preconditions = preconditions
216221
return worker
217222
}
@@ -462,7 +467,7 @@ func (w *SyncWorker) Update(ctx context.Context, generation int64, desired confi
462467
return w.status.DeepCopy()
463468
}
464469

465-
work.Capabilities = capability.SetCapabilities(config, priorCaps)
470+
work.Capabilities = capability.SetCapabilities(config, priorCaps, capability.GetCapabilitiesAsMap(w.alwaysEnableCapabilities))
466471

467472
versionEqual, overridesEqual, capabilitiesEqual :=
468473
equalSyncWork(w.work, work, fmt.Sprintf("considering cluster version generation %d", generation))

‎pkg/start/start.go

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ type Options struct {
8080

8181
ClusterProfile string
8282

83+
// AlwaysEnableCapabilities is a list of cluster version capabilities
84+
// which will always be implicitly enabled.
85+
AlwaysEnableCapabilities []string
86+
8387
// for testing only
8488
Name string
8589
Namespace string
@@ -148,6 +152,10 @@ func (o *Options) Run(ctx context.Context) error {
148152
if len(o.Exclude) > 0 {
149153
klog.Infof("Excluding manifests for %q", o.Exclude)
150154
}
155+
alwaysEnableCaps, unknownCaps := parseAlwaysEnableCapabilities(o.AlwaysEnableCapabilities)
156+
if len(unknownCaps) > 0 {
157+
return fmt.Errorf("--always-enable-capabilities was set with unknown capabilities: %v", unknownCaps)
158+
}
151159

152160
// parse the prometheus url
153161
var err error
@@ -168,7 +176,7 @@ func (o *Options) Run(ctx context.Context) error {
168176
}
169177

170178
// initialize the controllers and attempt to load the payload information
171-
controllerCtx, err := o.NewControllerContext(cb)
179+
controllerCtx, err := o.NewControllerContext(cb, alwaysEnableCaps)
172180
if err != nil {
173181
return err
174182
}
@@ -447,7 +455,7 @@ type Context struct {
447455

448456
// NewControllerContext initializes the default Context for the current Options. It does
449457
// not start any background processes.
450-
func (o *Options) NewControllerContext(cb *ClientBuilder) (*Context, error) {
458+
func (o *Options) NewControllerContext(cb *ClientBuilder, alwaysEnableCapabilities []configv1.ClusterVersionCapability) (*Context, error) {
451459
client := cb.ClientOrDie("shared-informer")
452460
kubeClient := cb.KubeClientOrDie(internal.ConfigNamespace, useProtobuf)
453461

@@ -481,6 +489,7 @@ func (o *Options) NewControllerContext(cb *ClientBuilder) (*Context, error) {
481489
o.PromQLTarget,
482490
o.InjectClusterIdIntoPromQL,
483491
o.UpdateService,
492+
alwaysEnableCapabilities,
484493
)
485494
if err != nil {
486495
return nil, err
@@ -585,3 +594,26 @@ func (c *Context) InitializeFromPayload(ctx context.Context, restConfig *rest.Co
585594

586595
return nil
587596
}
597+
598+
// parseAlwaysEnableCapabilities parses the string list of capabilities
599+
// into two lists of configv1.ClusterVersionCapability: known and unknown.
600+
func parseAlwaysEnableCapabilities(caps []string) ([]configv1.ClusterVersionCapability, []configv1.ClusterVersionCapability) {
601+
var (
602+
knownCaps []configv1.ClusterVersionCapability
603+
unknownCaps []configv1.ClusterVersionCapability
604+
)
605+
for _, c := range caps {
606+
known := false
607+
for _, kc := range configv1.KnownClusterVersionCapabilities {
608+
if configv1.ClusterVersionCapability(c) == kc {
609+
knownCaps = append(knownCaps, kc)
610+
known = true
611+
break
612+
}
613+
}
614+
if !known {
615+
unknownCaps = append(unknownCaps, configv1.ClusterVersionCapability(c))
616+
}
617+
}
618+
return knownCaps, unknownCaps
619+
}

‎pkg/start/start_integration_test.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,12 +184,15 @@ func TestIntegrationCVO_initializeAndUpgrade(t *testing.T) {
184184
options.ReleaseImage = payloadImage1
185185
options.PayloadOverride = filepath.Join(dir, "0.0.1")
186186
options.leaderElection = getLeaderElectionConfig(ctx, cfg)
187-
controllers, err := options.NewControllerContext(cb)
187+
alwaysEnableCapabilities := []configv1.ClusterVersionCapability{
188+
configv1.ClusterVersionCapabilityIngress,
189+
}
190+
controllers, err := options.NewControllerContext(cb, alwaysEnableCapabilities)
188191
if err != nil {
189192
t.Fatal(err)
190193
}
191194

192-
worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile)
195+
worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile, alwaysEnableCapabilities)
193196
controllers.CVO.SetSyncWorkerForTesting(worker)
194197

195198
lock, err := createResourceLock(cb, options.Namespace, options.Name)
@@ -315,12 +318,15 @@ func TestIntegrationCVO_gracefulStepDown(t *testing.T) {
315318
options.ReleaseImage = payloadImage1
316319
options.PayloadOverride = filepath.Join(dir, "0.0.1")
317320
options.leaderElection = getLeaderElectionConfig(ctx, cfg)
318-
controllers, err := options.NewControllerContext(cb)
321+
alwaysEnableCapabilities := []configv1.ClusterVersionCapability{
322+
configv1.ClusterVersionCapabilityIngress,
323+
}
324+
controllers, err := options.NewControllerContext(cb, alwaysEnableCapabilities)
319325
if err != nil {
320326
t.Fatal(err)
321327
}
322328

323-
worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile)
329+
worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile, alwaysEnableCapabilities)
324330
controllers.CVO.SetSyncWorkerForTesting(worker)
325331

326332
lock, err := createResourceLock(cb, options.Namespace, options.Name)
@@ -508,13 +514,15 @@ metadata:
508514
options.ReleaseImage = payloadImage1
509515
options.PayloadOverride = payloadDir
510516
options.leaderElection = getLeaderElectionConfig(ctx, cfg)
511-
512-
controllers, err := options.NewControllerContext(cb)
517+
alwaysEnableCapabilities := []configv1.ClusterVersionCapability{
518+
configv1.ClusterVersionCapabilityIngress,
519+
}
520+
controllers, err := options.NewControllerContext(cb, alwaysEnableCapabilities)
513521
if err != nil {
514522
t.Fatal(err)
515523
}
516524

517-
worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile)
525+
worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile, alwaysEnableCapabilities)
518526
controllers.CVO.SetSyncWorkerForTesting(worker)
519527

520528
arch := runtime.GOARCH

0 commit comments

Comments
 (0)