-
Notifications
You must be signed in to change notification settings - Fork 216
OCPSTRAT-2485: Introduce feature gate based inclusion/exclusion of manifests #1273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
37b1cd3
226437e
61c52fb
10b7a6c
85b12d0
84a0a3a
f09b811
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import ( | |
| "time" | ||
|
|
||
| "github.com/spf13/cobra" | ||
| "k8s.io/apimachinery/pkg/util/sets" | ||
|
|
||
| "github.com/openshift/cluster-version-operator/pkg/payload" | ||
| ) | ||
|
|
@@ -30,7 +31,7 @@ func newTaskGraphCmd() *cobra.Command { | |
|
|
||
| func runTaskGraphCmd(cmd *cobra.Command, args []string) error { | ||
| manifestDir := args[0] | ||
| release, err := payload.LoadUpdate(manifestDir, "", "", "", payload.DefaultClusterProfile, nil) | ||
| release, err := payload.LoadUpdate(manifestDir, "", "", "", payload.DefaultClusterProfile, nil, sets.Set[string]{}) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't handle feature sets today, does it need to handle feature gates?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not think so. It would be nice, but as you have stated, the file is not being kept up-to-date with new functionalities, so I believe it's okay not to implement it for now. |
||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,7 @@ import ( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/apimachinery/pkg/types" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| utilruntime "k8s.io/apimachinery/pkg/util/runtime" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/apimachinery/pkg/util/sets" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/apimachinery/pkg/util/wait" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| informerscorev1 "k8s.io/client-go/informers/core/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/client-go/kubernetes" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -120,6 +121,7 @@ type Operator struct { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmConfigLister listerscorev1.ConfigMapNamespaceLister | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmConfigManagedLister listerscorev1.ConfigMapNamespaceLister | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| proxyLister configlistersv1.ProxyLister | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| featureGateLister configlistersv1.FeatureGateLister | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cacheSynced []cache.InformerSynced | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // queue tracks applying updates to a cluster. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -178,16 +180,23 @@ type Operator struct { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // to select the manifests that will be applied in the cluster. The starting value cannot be changed in the executing | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // CVO but the featurechangestopper controller will detect a feature set change in the cluster and shutdown the CVO. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Enforcing featuresets is a standard GA CVO behavior that supports the feature gating functionality across the whole | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // cluster, as opposed to the enabledFeatureGates which controls what gated behaviors of CVO itself are enabled by | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // cluster, as opposed to the enabledCVOFeatureGates which controls what gated behaviors of CVO itself are enabled by | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // the cluster feature gates. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // See: https://github.com/openshift/enhancements/blob/master/enhancements/update/cvo-techpreview-manifests.md | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| requiredFeatureSet configv1.FeatureSet | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // enabledFeatureGates is the checker for what gated CVO behaviors are enabled or disabled by specific cluster-level | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // enabledCVOFeatureGates is the checker for what gated CVO behaviors are enabled or disabled by specific cluster-level | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // feature gates. It allows multiplexing the cluster-level feature gates to more granular CVO-level gates. Similarly | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // to the requiredFeatureSet, the enabledFeatureGates cannot be changed in the executing CVO but the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // to the requiredFeatureSet, the enabledCVOFeatureGates cannot be changed in the executing CVO but the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // featurechangestopper controller will detect when cluster feature gate config changes and shutdown the CVO. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enabledFeatureGates featuregates.CvoGateChecker | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enabledCVOFeatureGates featuregates.CvoGateChecker | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // featureGatesMutex protects access to enabledManifestFeatureGates | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| featureGatesMutex sync.RWMutex | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // enabledManifestFeatureGates is the set of feature gates that are currently enabled for the manifests that are applied to the cluster. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // This is the full set of enabled feature gates extracted from the FeatureGate object. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // We use this set as a filter to determine which of the manifests from the payload should or should not be applied to the cluster. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enabledManifestFeatureGates sets.Set[string] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clusterProfile string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uid types.UID | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -213,6 +222,7 @@ func New( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmConfigManagedInformer informerscorev1.ConfigMapInformer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| proxyInformer configinformersv1.ProxyInformer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| operatorInformerFactory operatorexternalversions.SharedInformerFactory, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| featureGateInformer configinformersv1.FeatureGateInformer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client clientset.Interface, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| kubeClient kubernetes.Interface, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| operatorClient operatorclientset.Interface, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -225,6 +235,7 @@ func New( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| alwaysEnableCapabilities []configv1.ClusterVersionCapability, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| featureSet configv1.FeatureSet, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cvoGates featuregates.CvoGateChecker, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| startingEnabledManifestFeatureGates sets.Set[string], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) (*Operator, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eventBroadcaster := record.NewBroadcaster() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eventBroadcaster.StartLogging(klog.Infof) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -248,18 +259,19 @@ func New( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| kubeClient: kubeClient, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| operatorClient: operatorClient, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eventRecorder: eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: namespace}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| queue: workqueue.NewTypedRateLimitingQueueWithConfig[any](workqueue.DefaultTypedControllerRateLimiter[any](), workqueue.TypedRateLimitingQueueConfig[any]{Name: "clusterversion"}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| availableUpdatesQueue: workqueue.NewTypedRateLimitingQueueWithConfig[any](workqueue.DefaultTypedControllerRateLimiter[any](), workqueue.TypedRateLimitingQueueConfig[any]{Name: "availableupdates"}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| upgradeableQueue: workqueue.NewTypedRateLimitingQueueWithConfig[any](workqueue.DefaultTypedControllerRateLimiter[any](), workqueue.TypedRateLimitingQueueConfig[any]{Name: "upgradeable"}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| queue: workqueue.NewTypedRateLimitingQueueWithConfig(workqueue.DefaultTypedControllerRateLimiter[any](), workqueue.TypedRateLimitingQueueConfig[any]{Name: "clusterversion"}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| availableUpdatesQueue: workqueue.NewTypedRateLimitingQueueWithConfig(workqueue.DefaultTypedControllerRateLimiter[any](), workqueue.TypedRateLimitingQueueConfig[any]{Name: "availableupdates"}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| upgradeableQueue: workqueue.NewTypedRateLimitingQueueWithConfig(workqueue.DefaultTypedControllerRateLimiter[any](), workqueue.TypedRateLimitingQueueConfig[any]{Name: "upgradeable"}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hypershift: hypershift, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exclude: exclude, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clusterProfile: clusterProfile, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| conditionRegistry: standard.NewConditionRegistry(promqlTarget), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| injectClusterIdIntoPromQL: injectClusterIdIntoPromQL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| requiredFeatureSet: featureSet, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enabledFeatureGates: cvoGates, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| requiredFeatureSet: featureSet, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enabledCVOFeatureGates: cvoGates, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enabledManifestFeatureGates: startingEnabledManifestFeatureGates, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| alwaysEnableCapabilities: alwaysEnableCapabilities, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -276,6 +288,9 @@ func New( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if _, err := coInformer.Informer().AddEventHandler(optr.clusterOperatorEventHandler()); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if _, err := featureGateInformer.Informer().AddEventHandler(optr.featureGateEventHandler()); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.coLister = coInformer.Lister() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.cacheSynced = append(optr.cacheSynced, coInformer.Informer().HasSynced) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -287,6 +302,9 @@ func New( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.cmConfigLister = cmConfigInformer.Lister().ConfigMaps(internal.ConfigNamespace) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.cmConfigManagedLister = cmConfigManagedInformer.Lister().ConfigMaps(internal.ConfigManagedNamespace) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.featureGateLister = featureGateInformer.Lister() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.cacheSynced = append(optr.cacheSynced, featureGateInformer.Informer().HasSynced) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // make sure this is initialized after all the listers are initialized | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.upgradeableChecks = optr.defaultUpgradeableChecks() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -318,7 +336,7 @@ func (optr *Operator) LoadInitialPayload(ctx context.Context, restConfig *rest.C | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| update, err := payload.LoadUpdate(optr.defaultPayloadDir(), optr.release.Image, optr.exclude, string(optr.requiredFeatureSet), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
JoelSpeed marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.clusterProfile, configv1.KnownClusterVersionCapabilities) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.clusterProfile, configv1.KnownClusterVersionCapabilities, optr.getEnabledFeatureGates()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, fmt.Errorf("the local release contents are invalid - no current version can be determined from disk: %v", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -779,7 +797,7 @@ func (optr *Operator) sync(ctx context.Context, key string) error { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // inform the config sync loop about our desired state | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| status := optr.configSync.Update(ctx, config.Generation, desired, config, state) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| status := optr.configSync.Update(ctx, config.Generation, desired, config, state, optr.getEnabledFeatureGates()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // write cluster version status | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return optr.syncStatus(ctx, original, config, status, errs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1084,12 +1102,78 @@ func (optr *Operator) HTTPClient() (*http.Client, error) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // featureGateEventHandler handles changes to FeatureGate objects and updates the cluster feature gates | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (optr *Operator) featureGateEventHandler() cache.ResourceEventHandler { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| workQueueKey := optr.queueKey() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return cache.ResourceEventHandlerFuncs{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
JoelSpeed marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AddFunc: func(obj interface{}) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if optr.updateEnabledFeatureGates(obj) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.queue.Add(workQueueKey) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| UpdateFunc: func(old, new interface{}) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if optr.updateEnabledFeatureGates(new) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.queue.Add(workQueueKey) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1105
to
+1120
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's look at the exact code at the specified lines
sed -n '1105,1120p' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 589 🏁 Script executed: # Get more context around the featureGateEventHandler function
sed -n '1100,1125p' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 970 🏁 Script executed: # Search for the Operator struct definition to find the fields mentioned
rg -A 30 'type Operator struct'Repository: openshift/cluster-version-operator Length of output: 3877 🏁 Script executed: # Search for featureGatesMutex and enabledManifestFeatureGates fields
rg 'featureGatesMutex|enabledManifestFeatureGates'Repository: openshift/cluster-version-operator Length of output: 1182 🏁 Script executed: # Look for other uses of featureGateEventHandler to understand context
rg -B 3 -A 3 'featureGateEventHandler'Repository: openshift/cluster-version-operator Length of output: 858 🏁 Script executed: # Search for how FeatureGate objects are used and created
rg -B 3 -A 3 'FeatureGate' pkg/cvo/cvo.go | head -60Repository: openshift/cluster-version-operator Length of output: 3193 🏁 Script executed: # Look at the featureGateEventHandler tests
rg -B 5 -A 20 'featureGateEventHandler' pkg/cvo/cvo_featuregates_test.goRepository: openshift/cluster-version-operator Length of output: 60 🏁 Script executed: # Check if there are any tests for DeleteFunc in ResourceEventHandler scenarios
rg -B 2 -A 5 'DeleteFunc' pkg/cvo/Repository: openshift/cluster-version-operator Length of output: 2181 🏁 Script executed: # Search for updateEnabledFeatureGates to understand what happens on update
rg -B 2 -A 15 'func.*updateEnabledFeatureGates' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 753 🏁 Script executed: # Check how many FeatureGate objects are typically managed
rg 'FeatureGate' pkg/cvo/ -A 2 -B 2 | grep -i 'single\|multiple\|list\|watch'Repository: openshift/cluster-version-operator Length of output: 1703 🏁 Script executed: # Look at the featureGateEventHandler and surrounding code to understand the full context
sed -n '1090,1140p' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 1713 🏁 Script executed: # Search for any tests around feature gate deletion
rg -B 5 -A 10 'featureGate.*[Dd]elete\|[Dd]elete.*featureGate' pkg/cvo/Repository: openshift/cluster-version-operator Length of output: 60 🏁 Script executed: # Check if there's documentation or comments about the FeatureGate lifecycle
rg -B 3 -A 5 'FeatureGate.*delete\|delete.*FeatureGate'Repository: openshift/cluster-version-operator Length of output: 60 🏁 Script executed: # Look at how clusterOperatorEventHandler implements DeleteFunc for comparison
rg -B 5 -A 10 'clusterOperatorEventHandler' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 1189 🏁 Script executed: # Check if enabledManifestFeatureGates is ever reset or cleared anywhere
rg 'enabledManifestFeatureGates.*=' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 123 🏁 Script executed: # Check how FeatureGate is configured/managed in the cluster - is deletion a realistic scenario?
rg -B 3 -A 3 'FeatureGate' pkg/cvo/sync_worker.go | head -40Repository: openshift/cluster-version-operator Length of output: 1664 🏁 Script executed: # Look for tests related to feature gate deletion or resets
rg -B 5 -A 10 'Delete\|Reset' pkg/cvo/cvo_featuregates_test.goRepository: openshift/cluster-version-operator Length of output: 60 🏁 Script executed: # Check the featuregate_integration_test to see scenarios
cat pkg/cvo/featuregate_integration_test.go | head -100Repository: openshift/cluster-version-operator Length of output: 3275 🏁 Script executed: # Verify the initialization and understand what "starting" gates mean
rg -B 10 'startingEnabledManifestFeatureGates' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 1074 🏁 Script executed: # Check how extractEnabledGates works to understand the semantics better
rg -B 3 -A 15 'func.*extractEnabledGates' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 1066 🏁 Script executed: # Look for any comments or documentation about what happens if FeatureGate is missing/deleted
rg -B 5 -A 5 'FeatureGate.*missing\|FeatureGate.*delete\|missing.*FeatureGate' pkg/cvo/Repository: openshift/cluster-version-operator Length of output: 60 🏁 Script executed: # Check if there's a default/empty state for feature gates
rg -B 3 -A 3 'sets\.New\[string\]\(\)' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 60 🏁 Script executed: # Look at what happens in the main sync loop regarding feature gates
rg -B 5 -A 5 'enabledManifestFeatureGates' pkg/cvo/cvo.go | tail -50Repository: openshift/cluster-version-operator Length of output: 1982 🏁 Script executed: # Verify if FeatureGate is a cluster-scoped singleton or if there can be multiple
rg -B 3 -A 3 'featureGateLister\|FeatureGateLister' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 60 🏁 Script executed: # Check the configv1.FeatureGate type to understand its scope
fd -e go -path '*/api/*' | xargs grep -l 'type FeatureGate struct' | head -3Repository: openshift/cluster-version-operator Length of output: 246 🏁 Script executed: # Look for any existing issue handling or comment about FeatureGate deletion
rg -B 5 -A 5 'FeatureGate' pkg/cvo/sync_worker.go | tail -30Repository: openshift/cluster-version-operator Length of output: 1310 🏁 Script executed: # Check what happens in the sync loop when enabledManifestFeatureGates is used
rg -B 3 -A 3 'getEnabledFeatureGates\|enabledManifestFeatureGates' pkg/cvo/cvo.go | grep -A 5 -B 5 'func (optr'Repository: openshift/cluster-version-operator Length of output: 60 🏁 Script executed: # Look at how other informers handle deletion in the codebase to understand patterns
rg -B 2 'DeleteFunc.*func' pkg/cvo/cvo.go | head -20Repository: openshift/cluster-version-operator Length of output: 161 🏁 Script executed: # Verify that FeatureGate is indeed cluster-scoped (singular) by checking the lister usage
rg 'featureGateLister\.' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 60 🏁 Script executed: # Search for where getEnabledFeatureGates is called in the main sync loop
rg -B 5 -A 5 'getEnabledFeatureGates' pkg/cvo/Repository: openshift/cluster-version-operator Length of output: 5793 🏁 Script executed: # Look at the clusterOperatorEventHandler to understand the pattern better
sed -n '1050,1090p' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 1299 🏁 Script executed: # Check if there are any other ResourceEventHandlers that handle DeleteFunc as a pattern
rg -B 3 -A 8 'ResourceEventHandlerFuncs{' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 193 🏁 Script executed: # Verify that the proposed fix uses the correct approach by looking at similar patterns
rg -B 2 -A 6 'featureGatesMutex.Lock' pkg/cvo/cvo.goRepository: openshift/cluster-version-operator Length of output: 384 Handle FeatureGate deletion to prevent stale manifest gates. The Suggested fix func (optr *Operator) featureGateEventHandler() cache.ResourceEventHandler {
workQueueKey := optr.queueKey()
return cache.ResourceEventHandlerFuncs{
AddFunc: func(obj interface{}) {
if optr.updateEnabledFeatureGates(obj) {
optr.queue.Add(workQueueKey)
}
},
UpdateFunc: func(old, new interface{}) {
if optr.updateEnabledFeatureGates(new) {
optr.queue.Add(workQueueKey)
}
},
+ DeleteFunc: func(_ interface{}) {
+ optr.featureGatesMutex.Lock()
+ optr.enabledManifestFeatureGates = sets.New[string]()
+ optr.featureGatesMutex.Unlock()
+ optr.queue.Add(workQueueKey)
+ },
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // updateEnabledFeatureGates updates the cluster feature gates based on a FeatureGate object. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Returns true or false based on whether or not the gates were actually updated. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // This allows us to avoid unnecessary work if the gates have not changed. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (optr *Operator) updateEnabledFeatureGates(obj interface{}) bool { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| featureGate, ok := obj.(*configv1.FeatureGate) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if !ok { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| klog.Warningf("Expected FeatureGate object but got %T", obj) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| newGates := optr.extractEnabledGates(featureGate) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.featureGatesMutex.Lock() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| defer optr.featureGatesMutex.Unlock() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check if gates actually changed to avoid unnecessary work | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if !optr.enabledManifestFeatureGates.Equal(newGates) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| klog.V(2).Infof("Cluster feature gates changed from %v to %v", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sets.List(optr.enabledManifestFeatureGates), sets.List(newGates)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.enabledManifestFeatureGates = newGates | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // getEnabledFeatureGates returns a copy of the current cluster feature gates for safe consumption | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (optr *Operator) getEnabledFeatureGates() sets.Set[string] { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| optr.featureGatesMutex.RLock() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| defer optr.featureGatesMutex.RUnlock() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Return a copy to prevent external modification | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return optr.enabledManifestFeatureGates.Clone() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // extractEnabledGates extracts the list of enabled feature gates for the current cluster version | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (optr *Operator) extractEnabledGates(featureGate *configv1.FeatureGate) sets.Set[string] { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Find the feature gate details for the current loaded payload version. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentVersion := optr.currentVersion().Version | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if currentVersion == "" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| klog.Warningf("Payload has not been initialized yet, using the operator version %s", optr.enabledCVOFeatureGates.DesiredVersion()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentVersion = optr.enabledCVOFeatureGates.DesiredVersion() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return featuregates.ExtractEnabledGates(featureGate, currentVersion) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // shouldReconcileCVOConfiguration returns whether the CVO should reconcile its configuration using the API server. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // enabledFeatureGates must be initialized before the function is called. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // enabledCVOFeatureGates must be initialized before the function is called. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (optr *Operator) shouldReconcileCVOConfiguration() bool { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // The relevant CRD and CR are not applied in HyperShift, which configures the CVO via a configuration file | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return optr.enabledFeatureGates.CVOConfiguration() && !optr.hypershift | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return optr.enabledCVOFeatureGates.CVOConfiguration() && !optr.hypershift | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // shouldReconcileAcceptRisks returns whether the CVO should reconcile spec.desiredUpdate.acceptRisks and populate the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1098,5 +1182,5 @@ func (optr *Operator) shouldReconcileCVOConfiguration() bool { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // enabledFeatureGates must be initialized before the function is called. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (optr *Operator) shouldReconcileAcceptRisks() bool { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // HyperShift will be supported later if needed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return optr.enabledFeatureGates.AcceptRisks() && !optr.hypershift | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return optr.enabledCVOFeatureGates.AcceptRisks() && !optr.hypershift | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.