NE-1318: Add always-enable-capabilities flag and set Ingress as always enabled

cmd/start.go

@@ -47,5 +47,6 @@ func init() {
 	cmd.PersistentFlags().StringVar(&opts.PrometheusURLString, "metrics-url", opts.PrometheusURLString, "The URL used to access the remote PromQL query service.")
 	cmd.PersistentFlags().BoolVar(&opts.InjectClusterIdIntoPromQL, "hypershift", opts.InjectClusterIdIntoPromQL, "This options indicates whether the CVO is running inside a hosted control plane.")
 	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.")
+	cmd.PersistentFlags().StringSliceVar(&opts.AlwaysEnableCapabilities, "always-enable-capabilities", opts.AlwaysEnableCapabilities, "List of the cluster capabilities which will always be implicitly enabled.")
 	rootCmd.AddCommand(cmd)
 }

install/0000_00_cluster-version-operator_03_deployment.yaml

@@ -34,6 +34,7 @@ spec:
           - "--serving-cert-file=/etc/tls/serving-cert/tls.crt"
           - "--serving-key-file=/etc/tls/serving-cert/tls.key"
           - "--v=2"
+          - "--always-enable-capabilities=Ingress"
         resources:
           requests:
             cpu: 20m

lib/capability/capability.go

@@ -37,13 +37,13 @@ func (caps capabilitiesSort) Less(i, j int) bool { return string(caps[i]) < stri
 // SetCapabilities populates and returns cluster capabilities from ClusterVersion capabilities spec. This method also
 // ensures that no previousily enabled capability is now disabled and returns any such implicitly enabled capabilities.
 func SetCapabilities(config *configv1.ClusterVersion,
-	existingEnabled map[configv1.ClusterVersionCapability]struct{}) ClusterCapabilities {
+	existingEnabled, alwaysEnabled map[configv1.ClusterVersionCapability]struct{}) ClusterCapabilities {
 
 	var capabilities ClusterCapabilities
 	capabilities.KnownCapabilities = setKnownCapabilities()
 
 	capabilities.EnabledCapabilities, capabilities.ImplicitlyEnabledCapabilities = setEnabledCapabilities(config.Spec.Capabilities,
-		existingEnabled)
+		existingEnabled, alwaysEnabled)
 
 	return capabilities
 }
@@ -150,8 +150,9 @@ func setKnownCapabilities() map[configv1.ClusterVersionCapability]struct{} {
 // DefaultCapabilitySet is used if a baseline capability set is not defined by ClusterVersion. A check is then made to
 // ensure that no previousily enabled capability is now disabled and if any such capabilities are found each is enabled,
 // saved, and returned.
+// The required capabilities are added to the implicitly enabled.
 func setEnabledCapabilities(capabilitiesSpec *configv1.ClusterVersionCapabilitiesSpec,
-	priorEnabled map[configv1.ClusterVersionCapability]struct{}) (map[configv1.ClusterVersionCapability]struct{},
+	priorEnabled, alwaysEnabled map[configv1.ClusterVersionCapability]struct{}) (map[configv1.ClusterVersionCapability]struct{},
 	[]configv1.ClusterVersionCapability) {
 
 	capSet := DefaultCapabilitySet
@@ -176,6 +177,12 @@ func setEnabledCapabilities(capabilitiesSpec *configv1.ClusterVersionCapabilitie
 			enabled[k] = struct{}{}
 		}
 	}
+	for k := range alwaysEnabled {
+		if _, ok := enabled[k]; !ok {
+			implicitlyEnabled = append(implicitlyEnabled, k)
+			enabled[k] = struct{}{}
+		}
+	}
 	sort.Sort(capabilitiesSort(implicitlyEnabled))
 	return enabled, implicitlyEnabled
 }

lib/capability/capability_test.go

@@ -101,7 +101,7 @@ func TestSetCapabilities(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			caps := SetCapabilities(test.config, nil)
+			caps := SetCapabilities(test.config, nil, nil)
 			if test.config.Spec.Capabilities == nil || (test.config.Spec.Capabilities != nil &&
 				len(test.config.Spec.Capabilities.BaselineCapabilitySet) == 0) {
 
@@ -130,26 +130,50 @@ func TestSetCapabilities(t *testing.T) {
 
 func TestSetCapabilitiesWithImplicitlyEnabled(t *testing.T) {
 	tests := []struct {
-		name         string
-		config       *configv1.ClusterVersion
-		wantImplicit []string
-		priorEnabled map[configv1.ClusterVersionCapability]struct{}
+		name          string
+		config        *configv1.ClusterVersion
+		wantImplicit  []string
+		priorEnabled  map[configv1.ClusterVersionCapability]struct{}
+		alwaysEnabled map[configv1.ClusterVersionCapability]struct{}
 	}{
-		{name: "set capabilities 4_11 with additional",
+		{name: "set baseline capabilities with additional",
 			config: &configv1.ClusterVersion{
 				Spec: configv1.ClusterVersionSpec{
 					Capabilities: &configv1.ClusterVersionCapabilitiesSpec{
-						BaselineCapabilitySet: configv1.ClusterVersionCapabilitySet4_11,
+						BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetCurrent,
 					},
 				},
 			},
 			priorEnabled: map[configv1.ClusterVersionCapability]struct{}{"cap1": {}, "cap2": {}, "cap3": {}},
 			wantImplicit: []string{"cap1", "cap2", "cap3"},
 		},
+		{name: "set baseline capabilities with always enabled",
+			config: &configv1.ClusterVersion{
+				Spec: configv1.ClusterVersionSpec{
+					Capabilities: &configv1.ClusterVersionCapabilitiesSpec{
+						BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetCurrent,
+					},
+				},
+			},
+			alwaysEnabled: map[configv1.ClusterVersionCapability]struct{}{"cap1": {}, "cap2": {}, "cap3": {}},
+			wantImplicit:  []string{"cap1", "cap2", "cap3"},
+		},
+		{name: "set baseline capabilities with additional and always enabled",
+			config: &configv1.ClusterVersion{
+				Spec: configv1.ClusterVersionSpec{
+					Capabilities: &configv1.ClusterVersionCapabilitiesSpec{
+						BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetCurrent,
+					},
+				},
+			},
+			priorEnabled:  map[configv1.ClusterVersionCapability]struct{}{"cap1": {}, "cap2": {}, "cap3": {}},
+			alwaysEnabled: map[configv1.ClusterVersionCapability]struct{}{"cap1": {}, "cap2": {}, "cap4": {}},
+			wantImplicit:  []string{"cap1", "cap2", "cap3", "cap4"},
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			caps := SetCapabilities(test.config, test.priorEnabled)
+			caps := SetCapabilities(test.config, test.priorEnabled, test.alwaysEnabled)
 			if len(caps.ImplicitlyEnabledCapabilities) != len(test.wantImplicit) {
 				t.Errorf("Incorrect number of implicitly enabled keys, wanted: %q. ImplicitlyEnabledCapabilities returned: %v", test.wantImplicit, caps.ImplicitlyEnabledCapabilities)
 			}

pkg/cvo/cvo.go

@@ -172,6 +172,10 @@ type Operator struct {
 
 	clusterProfile string
 	uid            types.UID
+
+	// alwaysEnableCapabilities is a list of the cluster capabilities which should
+	// always be implicitly enabled.
+	alwaysEnableCapabilities []configv1.ClusterVersionCapability
 }
 
 // New returns a new cluster version operator.
@@ -193,6 +197,7 @@ func New(
 	promqlTarget clusterconditions.PromQLTarget,
 	injectClusterIdIntoPromQL bool,
 	updateService string,
+	alwaysEnableCapabilities []configv1.ClusterVersionCapability,
 ) (*Operator, error) {
 	eventBroadcaster := record.NewBroadcaster()
 	eventBroadcaster.StartLogging(klog.Infof)
@@ -228,7 +233,8 @@ func New(
 		// Because of OCPBUGS-30080, we can only detect the enabled feature gates after Operator loads the initial payload
 		// from disk via LoadInitialPayload. We must not have any gate-checking code until that happens, so we initialize
 		// this field with a checker that panics when used.
-		enabledFeatureGates: featuregates.PanicOnUsageBeforeInitialization,
+		enabledFeatureGates:      featuregates.PanicOnUsageBeforeInitialization,
+		alwaysEnableCapabilities: alwaysEnableCapabilities,
 	}
 
 	if _, err := cvInformer.Informer().AddEventHandler(optr.clusterVersionEventHandler()); err != nil {
@@ -341,6 +347,7 @@ func (optr *Operator) InitializeFromPayload(update *payload.Update, requiredFeat
 		requiredFeatureSet,
 		optr.eventRecorder,
 		optr.clusterProfile,
+		optr.alwaysEnableCapabilities,
 	)
 }
 

pkg/cvo/cvo_scenarios_test.go

@@ -135,6 +135,7 @@ func setupCVOTest(payloadDir string) (*Operator, map[string]apiruntime.Object, *
 		"",
 		record.NewFakeRecorder(100),
 		o.clusterProfile,
+		[]configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityIngress},
 	)
 	o.configSync = worker
 
@@ -1971,7 +1972,7 @@ func TestCVO_UpgradeFailedPayloadLoadWithCapsChanges(t *testing.T) {
 
 	// confirm capabilities are updated
 	checkStatus(t, actions[1], "update", "clusterversions", "status", "", configv1.ClusterVersionCapabilitiesStatus{
-		EnabledCapabilities: []configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityBaremetal, configv1.ClusterVersionCapabilityMarketplace},
+		EnabledCapabilities: []configv1.ClusterVersionCapability{configv1.ClusterVersionCapabilityIngress, configv1.ClusterVersionCapabilityBaremetal, configv1.ClusterVersionCapabilityMarketplace},
 		KnownCapabilities:   sortedCaps,
 	})
 }

pkg/cvo/sync_worker.go

@@ -181,11 +181,16 @@ type SyncWorker struct {
 	requiredFeatureSet configv1.FeatureSet
 
 	clusterProfile string
+
+	// alwaysEnableCapabilities is a list of cluster capabilities which should
+	// always be implicitly enabled.
+	// This contributes to whether or not some manifests are included for reconciliation.
+	alwaysEnableCapabilities []configv1.ClusterVersionCapability
 }
 
 // NewSyncWorker initializes a ConfigSyncWorker that will retrieve payloads to disk, apply them via builder
 // to a server, and obey limits about how often to reconcile or retry on errors.
-func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, requiredFeatureSet configv1.FeatureSet, eventRecorder record.EventRecorder, clusterProfile string) *SyncWorker {
+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 {
 	return &SyncWorker{
 		retriever:     retriever,
 		builder:       builder,
@@ -200,18 +205,18 @@ func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder,
 		// if the reader is not fast enough.
 		report: make(chan SyncWorkerStatus, 500),
 
-		exclude:            exclude,
-		requiredFeatureSet: requiredFeatureSet,
-
-		clusterProfile: clusterProfile,
+		exclude:                  exclude,
+		requiredFeatureSet:       requiredFeatureSet,
+		clusterProfile:           clusterProfile,
+		alwaysEnableCapabilities: alwaysEnableCapabilities,
 	}
 }
 
 // NewSyncWorkerWithPreconditions initializes a ConfigSyncWorker that will retrieve payloads to disk, apply them via builder
 // to a server, and obey limits about how often to reconcile or retry on errors.
 // It allows providing preconditions for loading payload.
-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 {
-	worker := NewSyncWorker(retriever, builder, reconcileInterval, backoff, exclude, requiredFeatureSet, eventRecorder, clusterProfile)
+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 {
+	worker := NewSyncWorker(retriever, builder, reconcileInterval, backoff, exclude, requiredFeatureSet, eventRecorder, clusterProfile, alwaysEnableCapabilities)
 	worker.preconditions = preconditions
 	return worker
 }
@@ -462,7 +467,7 @@ func (w *SyncWorker) Update(ctx context.Context, generation int64, desired confi
 		return w.status.DeepCopy()
 	}
 
-	work.Capabilities = capability.SetCapabilities(config, priorCaps)
+	work.Capabilities = capability.SetCapabilities(config, priorCaps, capability.GetCapabilitiesAsMap(w.alwaysEnableCapabilities))
 
 	versionEqual, overridesEqual, capabilitiesEqual :=
 		equalSyncWork(w.work, work, fmt.Sprintf("considering cluster version generation %d", generation))

pkg/start/start.go

@@ -80,6 +80,10 @@ type Options struct {
 
 	ClusterProfile string
 
+	// AlwaysEnableCapabilities is a list of cluster version capabilities
+	// which will always be implicitly enabled.
+	AlwaysEnableCapabilities []string
+
 	// for testing only
 	Name            string
 	Namespace       string
@@ -148,6 +152,10 @@ func (o *Options) Run(ctx context.Context) error {
 	if len(o.Exclude) > 0 {
 		klog.Infof("Excluding manifests for %q", o.Exclude)
 	}
+	alwaysEnableCaps, unknownCaps := parseAlwaysEnableCapabilities(o.AlwaysEnableCapabilities)
+	if len(unknownCaps) > 0 {
+		return fmt.Errorf("--always-enable-capabilities was set with unknown capabilities: %v", unknownCaps)
+	}
 
 	// parse the prometheus url
 	var err error
@@ -168,7 +176,7 @@ func (o *Options) Run(ctx context.Context) error {
 	}
 
 	// initialize the controllers and attempt to load the payload information
-	controllerCtx, err := o.NewControllerContext(cb)
+	controllerCtx, err := o.NewControllerContext(cb, alwaysEnableCaps)
 	if err != nil {
 		return err
 	}
@@ -446,7 +454,7 @@ type Context struct {
 
 // NewControllerContext initializes the default Context for the current Options. It does
 // not start any background processes.
-func (o *Options) NewControllerContext(cb *ClientBuilder) (*Context, error) {
+func (o *Options) NewControllerContext(cb *ClientBuilder, alwaysEnableCapabilities []configv1.ClusterVersionCapability) (*Context, error) {
 	client := cb.ClientOrDie("shared-informer")
 	kubeClient := cb.KubeClientOrDie(internal.ConfigNamespace, useProtobuf)
 
@@ -480,6 +488,7 @@ func (o *Options) NewControllerContext(cb *ClientBuilder) (*Context, error) {
 		o.PromQLTarget,
 		o.InjectClusterIdIntoPromQL,
 		o.UpdateService,
+		alwaysEnableCapabilities,
 	)
 	if err != nil {
 		return nil, err
@@ -584,3 +593,26 @@ func (c *Context) InitializeFromPayload(ctx context.Context, restConfig *rest.Co
 
 	return nil
 }
+
+// parseAlwaysEnableCapabilities parses the string list of capabilities
+// into two lists of configv1.ClusterVersionCapability: known and unknown.
+func parseAlwaysEnableCapabilities(caps []string) ([]configv1.ClusterVersionCapability, []configv1.ClusterVersionCapability) {
+	var (
+		knownCaps   []configv1.ClusterVersionCapability
+		unknownCaps []configv1.ClusterVersionCapability
+	)
+	for _, c := range caps {
+		known := false
+		for _, kc := range configv1.KnownClusterVersionCapabilities {
+			if configv1.ClusterVersionCapability(c) == kc {
+				knownCaps = append(knownCaps, kc)
+				known = true
+				break
+			}
+		}
+		if !known {
+			unknownCaps = append(unknownCaps, configv1.ClusterVersionCapability(c))
+		}
+	}
+	return knownCaps, unknownCaps
+}

pkg/start/start_integration_test.go

@@ -184,12 +184,15 @@ func TestIntegrationCVO_initializeAndUpgrade(t *testing.T) {
 	options.ReleaseImage = payloadImage1
 	options.PayloadOverride = filepath.Join(dir, "0.0.1")
 	options.leaderElection = getLeaderElectionConfig(ctx, cfg)
-	controllers, err := options.NewControllerContext(cb)
+	alwaysEnableCapabilities := []configv1.ClusterVersionCapability{
+		configv1.ClusterVersionCapabilityIngress,
+	}
+	controllers, err := options.NewControllerContext(cb, alwaysEnableCapabilities)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile)
+	worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile, alwaysEnableCapabilities)
 	controllers.CVO.SetSyncWorkerForTesting(worker)
 
 	lock, err := createResourceLock(cb, options.Namespace, options.Name)
@@ -315,12 +318,15 @@ func TestIntegrationCVO_gracefulStepDown(t *testing.T) {
 	options.ReleaseImage = payloadImage1
 	options.PayloadOverride = filepath.Join(dir, "0.0.1")
 	options.leaderElection = getLeaderElectionConfig(ctx, cfg)
-	controllers, err := options.NewControllerContext(cb)
+	alwaysEnableCapabilities := []configv1.ClusterVersionCapability{
+		configv1.ClusterVersionCapabilityIngress,
+	}
+	controllers, err := options.NewControllerContext(cb, alwaysEnableCapabilities)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile)
+	worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile, alwaysEnableCapabilities)
 	controllers.CVO.SetSyncWorkerForTesting(worker)
 
 	lock, err := createResourceLock(cb, options.Namespace, options.Name)
@@ -508,13 +514,15 @@ metadata:
 	options.ReleaseImage = payloadImage1
 	options.PayloadOverride = payloadDir
 	options.leaderElection = getLeaderElectionConfig(ctx, cfg)
-
-	controllers, err := options.NewControllerContext(cb)
+	alwaysEnableCapabilities := []configv1.ClusterVersionCapability{
+		configv1.ClusterVersionCapabilityIngress,
+	}
+	controllers, err := options.NewControllerContext(cb, alwaysEnableCapabilities)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile)
+	worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", "", record.NewFakeRecorder(100), payload.DefaultClusterProfile, alwaysEnableCapabilities)
 	controllers.CVO.SetSyncWorkerForTesting(worker)
 
 	arch := runtime.GOARCH