Skip to content

Commit 5cb146d

Browse files
petr-mullerpetr-muller
committed
refactor(start): process always enable caps in constructor
1 parent 09edded commit 5cb146d

File tree

2 files changed

+29
-31
lines changed

2 files changed

+29
-31
lines changed

‎pkg/start/start.go

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,11 @@ type Options struct {
8686

8787
HyperShift bool
8888

89-
// AlwaysEnableCapabilities is a list of cluster version capabilities
90-
// which will always be implicitly enabled.
89+
// AlwaysEnableCapabilities is user-provided list of cluster version capabilities to be always be implicitly enabled
9190
AlwaysEnableCapabilities []string
91+
// alwaysEnableCapabilities is the parsed list of cluster version capabilities to be always be implicitly enabled,
92+
// guaranteed to contain only known capabilities.
93+
alwaysEnableCapabilities []configv1.ClusterVersionCapability
9294

9395
// for testing only
9496
Name string
@@ -158,9 +160,8 @@ func (o *Options) Run(ctx context.Context) error {
158160
if len(o.Exclude) > 0 {
159161
klog.Infof("Excluding manifests for %q", o.Exclude)
160162
}
161-
alwaysEnableCaps, unknownCaps := parseAlwaysEnableCapabilities(o.AlwaysEnableCapabilities)
162-
if len(unknownCaps) > 0 {
163-
return fmt.Errorf("--always-enable-capabilities was set with unknown capabilities: %v", unknownCaps)
163+
if err := o.parseAlwaysEnableCapabilities(); err != nil {
164+
return fmt.Errorf("--always-enable-capability: %w", err)
164165
}
165166

166167
// Inject the cluster ID into PromQL queries in HyperShift
@@ -185,7 +186,7 @@ func (o *Options) Run(ctx context.Context) error {
185186
}
186187

187188
// initialize the controllers and attempt to load the payload information
188-
controllerCtx, err := o.NewControllerContext(cb, alwaysEnableCaps)
189+
controllerCtx, err := o.NewControllerContext(cb)
189190
if err != nil {
190191
return err
191192
}
@@ -470,7 +471,7 @@ type Context struct {
470471

471472
// NewControllerContext initializes the default Context for the current Options. It does
472473
// not start any background processes.
473-
func (o *Options) NewControllerContext(cb *ClientBuilder, alwaysEnableCapabilities []configv1.ClusterVersionCapability) (*Context, error) {
474+
func (o *Options) NewControllerContext(cb *ClientBuilder) (*Context, error) {
474475
client := cb.ClientOrDie("shared-informer")
475476
kubeClient := cb.KubeClientOrDie(internal.ConfigNamespace, useProtobuf)
476477
operatorClient := cb.OperatorClientOrDie("operator-client")
@@ -511,7 +512,7 @@ func (o *Options) NewControllerContext(cb *ClientBuilder, alwaysEnableCapabiliti
511512
o.PromQLTarget,
512513
o.InjectClusterIdIntoPromQL,
513514
o.UpdateService,
514-
alwaysEnableCapabilities,
515+
o.alwaysEnableCapabilities,
515516
)
516517
if err != nil {
517518
return nil, err
@@ -620,16 +621,14 @@ func (c *Context) InitializeFromPayload(ctx context.Context, restConfig *rest.Co
620621

621622
// parseAlwaysEnableCapabilities parses the string list of capabilities
622623
// into two lists of configv1.ClusterVersionCapability: known and unknown.
623-
func parseAlwaysEnableCapabilities(caps []string) ([]configv1.ClusterVersionCapability, []configv1.ClusterVersionCapability) {
624-
var (
625-
knownCaps []configv1.ClusterVersionCapability
626-
unknownCaps []configv1.ClusterVersionCapability
627-
)
628-
for _, c := range caps {
624+
func (o *Options) parseAlwaysEnableCapabilities() error {
625+
var unknownCaps []configv1.ClusterVersionCapability
626+
627+
for _, c := range o.AlwaysEnableCapabilities {
629628
known := false
630629
for _, kc := range configv1.KnownClusterVersionCapabilities {
631630
if configv1.ClusterVersionCapability(c) == kc {
632-
knownCaps = append(knownCaps, kc)
631+
o.alwaysEnableCapabilities = append(o.alwaysEnableCapabilities, kc)
633632
known = true
634633
break
635634
}
@@ -638,5 +637,10 @@ func parseAlwaysEnableCapabilities(caps []string) ([]configv1.ClusterVersionCapa
638637
unknownCaps = append(unknownCaps, configv1.ClusterVersionCapability(c))
639638
}
640639
}
641-
return knownCaps, unknownCaps
640+
641+
if len(unknownCaps) > 0 {
642+
return fmt.Errorf("unknown capabilities: %v", unknownCaps)
643+
}
644+
645+
return nil
642646
}

‎pkg/start/start_integration_test.go

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -184,15 +184,13 @@ 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-
alwaysEnableCapabilities := []configv1.ClusterVersionCapability{
188-
configv1.ClusterVersionCapabilityIngress,
189-
}
190-
controllers, err := options.NewControllerContext(cb, alwaysEnableCapabilities)
187+
options.alwaysEnableCapabilities = []configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityIngress}
188+
controllers, err := options.NewControllerContext(cb)
191189
if err != nil {
192190
t.Fatal(err)
193191
}
194192

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

198196
lock, err := createResourceLock(cb, options.Namespace, options.Name)
@@ -318,15 +316,13 @@ func TestIntegrationCVO_gracefulStepDown(t *testing.T) {
318316
options.ReleaseImage = payloadImage1
319317
options.PayloadOverride = filepath.Join(dir, "0.0.1")
320318
options.leaderElection = getLeaderElectionConfig(ctx, cfg)
321-
alwaysEnableCapabilities := []configv1.ClusterVersionCapability{
322-
configv1.ClusterVersionCapabilityIngress,
323-
}
324-
controllers, err := options.NewControllerContext(cb, alwaysEnableCapabilities)
319+
options.alwaysEnableCapabilities = []configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityIngress}
320+
controllers, err := options.NewControllerContext(cb)
325321
if err != nil {
326322
t.Fatal(err)
327323
}
328324

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

332328
lock, err := createResourceLock(cb, options.Namespace, options.Name)
@@ -514,15 +510,13 @@ metadata:
514510
options.ReleaseImage = payloadImage1
515511
options.PayloadOverride = payloadDir
516512
options.leaderElection = getLeaderElectionConfig(ctx, cfg)
517-
alwaysEnableCapabilities := []configv1.ClusterVersionCapability{
518-
configv1.ClusterVersionCapabilityIngress,
519-
}
520-
controllers, err := options.NewControllerContext(cb, alwaysEnableCapabilities)
513+
options.alwaysEnableCapabilities = []configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityIngress}
514+
controllers, err := options.NewControllerContext(cb)
521515
if err != nil {
522516
t.Fatal(err)
523517
}
524518

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

528522
lock, err := createResourceLock(cb, options.Namespace, options.Name)

0 commit comments

Comments
 (0)