-
Notifications
You must be signed in to change notification settings - Fork 166
CONSOLE-5271: Set olmLifecycleEnabled based on OLMLifecycleAndCompatibility FeatureGate #1174
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
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 |
|---|---|---|
|
|
@@ -130,6 +130,12 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact | |
| return statusHandler.FlushAndReturn(techPreviewErr) | ||
| } | ||
|
|
||
| olmLifecycleMetadataEnabled, olmLifecycleMetadataErrReason, olmLifecycleMetadataErr := co.SyncOLMLifecycleMetadata() | ||
| statusHandler.AddConditions(status.HandleProgressingOrDegraded("OLMLifecycleMetadataSync", olmLifecycleMetadataErrReason, olmLifecycleMetadataErr)) | ||
| if olmLifecycleMetadataErr != nil { | ||
| return statusHandler.FlushAndReturn(olmLifecycleMetadataErr) | ||
| } | ||
|
|
||
| cm, cmErrReason, cmErr := co.SyncConfigMap( | ||
| ctx, | ||
| set.Operator, | ||
|
|
@@ -141,6 +147,7 @@ func (co *consoleOperator) sync_v400(ctx context.Context, controllerContext fact | |
| controllerContext.Recorder(), | ||
| consoleURL.Hostname(), | ||
| techPreviewEnabled, | ||
| olmLifecycleMetadataEnabled, | ||
| ) | ||
| statusHandler.AddConditions(status.HandleProgressingOrDegraded("ConfigMapSync", cmErrReason, cmErr)) | ||
| if cmErr != nil { | ||
|
|
@@ -352,6 +359,7 @@ func (co *consoleOperator) SyncConfigMap( | |
| recorder events.Recorder, | ||
| consoleHost string, | ||
| techPreviewEnabled bool, | ||
| olmLifecycleMetadataEnabled bool, | ||
| ) (consoleConfigMap *corev1.ConfigMap, reason string, err error) { | ||
|
|
||
| managedConfig, mcErr := co.managedNSConfigMapLister.ConfigMaps(api.OpenShiftConfigManagedNamespace).Get(api.OpenShiftConsoleConfigMapName) | ||
|
|
@@ -426,6 +434,7 @@ func (co *consoleOperator) SyncConfigMap( | |
| telemetryConfig, | ||
| consoleHost, | ||
| techPreviewEnabled, | ||
| olmLifecycleMetadataEnabled, | ||
| ) | ||
| if err != nil { | ||
| return nil, "FailedConsoleConfigBuilder", err | ||
|
|
@@ -600,7 +609,7 @@ func (co *consoleOperator) SyncTrustedCAConfigMap(ctx context.Context, operatorC | |
| func (co *consoleOperator) SyncTechPreview() (techPreviewEnabled bool, reason string, err error) { | ||
| featureGate, err := co.featureGateLister.Get(api.ConfigResourceName) | ||
| if err != nil { | ||
| klog.V(4).Infof("failed to get FeatureGate resource: %v.", err) | ||
| klog.V(4).Infof("failed to get FeatureGate resource: %v", err) | ||
| return false, "FailedGet", err | ||
| } | ||
|
|
||
|
|
@@ -612,6 +621,29 @@ func (co *consoleOperator) SyncTechPreview() (techPreviewEnabled bool, reason st | |
| return techPreviewEnabled, "", nil | ||
| } | ||
|
|
||
| // Replace with features.FeatureGateOLMLifecycleAndCompatibility once openshift/api is bumped. | ||
| const olmLifecycleAndCompatibilityFeatureGate configv1.FeatureGateName = "OLMLifecycleAndCompatibility" | ||
|
|
||
| // SyncOLMLifecycleMetadata determines if OLM lifecycle metadata features should be enabled | ||
| // by checking if the OLMLifecycleAndCompatibility feature gate is enabled in the cluster. | ||
| func (co *consoleOperator) SyncOLMLifecycleMetadata() (olmLifecycleMetadataEnabled bool, reason string, err error) { | ||
| featureGate, err := co.featureGateLister.Get(api.ConfigResourceName) | ||
| if err != nil { | ||
| klog.V(4).Infof("failed to get FeatureGate resource: %v", err) | ||
| return false, "FailedGet", err | ||
|
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. 633: ambiguous for anyone troubleshooting cluster statuses/events. strongly urge being more specific about what resource failed to load. --> return false, "FailedGetFeatureGate", err for ex.
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.
|
||
| } | ||
|
|
||
| for _, versionedGates := range featureGate.Status.FeatureGates { | ||
| for _, gate := range versionedGates.Enabled { | ||
| if gate.Name == olmLifecycleAndCompatibilityFeatureGate { | ||
| klog.V(4).Infoln("OLM lifecycle metadata features are enabled based on the OLMLifecycleAndCompatibility feature gate") | ||
| return true, "", nil | ||
| } | ||
| } | ||
| } | ||
| return false, "", nil | ||
| } | ||
|
|
||
| func (co *consoleOperator) SyncCustomLogos(operatorConfig *operatorv1.Console) (error, string) { | ||
| if operatorConfig.Spec.Customization.CustomLogoFile.Name != "" || operatorConfig.Spec.Customization.CustomLogoFile.Key != "" { | ||
| return co.SyncCustomLogoConfigMap(operatorConfig) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,6 +49,7 @@ func DefaultConfigMap( | |
| telemeterConfig map[string]string, | ||
|
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. GLOBAL compound modifiers preceding nouns should be hyphenated (ex., "user-defined").
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. Hyphenation is fixed. On the remediation hints — these are operator-internal |
||
| consoleHost string, | ||
| techPreviewEnabled bool, | ||
| olmLifecycleMetadataEnabled bool, | ||
| ) (consoleConfigMap *corev1.ConfigMap, unsupportedOverridesHaveMerged bool, err error) { | ||
|
|
||
| apiServerURL := infrastructuresub.GetAPIServerURL(infrastructureConfig) | ||
|
|
@@ -66,9 +67,10 @@ func DefaultConfigMap( | |
| NodeOperatingSystems(nodeOperatingSystems). | ||
| CopiedCSVsDisabled(copiedCSVsDisabled). | ||
| TechPreviewEnabled(techPreviewEnabled). | ||
| OLMLifecycleMetadataEnabled(olmLifecycleMetadataEnabled). | ||
| ConfigYAML() | ||
| if err != nil { | ||
| klog.Errorf("failed to generate default console-config config: %v", err) | ||
| klog.Errorf("failed to generate default console-config: %v", err) | ||
| return nil, false, err | ||
| } | ||
|
|
||
|
|
@@ -106,9 +108,10 @@ func DefaultConfigMap( | |
| AuthConfig(authConfig, apiServerURL). | ||
| Capabilities(operatorConfig.Spec.Customization.Capabilities). | ||
| TechPreviewEnabled(techPreviewEnabled). | ||
| OLMLifecycleMetadataEnabled(olmLifecycleMetadataEnabled). | ||
| ConfigYAML() | ||
| if err != nil { | ||
| klog.Errorf("failed to generate user defined console-config config: %v", err) | ||
| klog.Errorf("failed to generate user-defined console-config: %v", err) | ||
| return nil, false, err | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor terminology check: if status string visible to users, use full word "Synchronization"?
"OLMLifecycleMetadataSynchronization"
not a hill i'd 💀 on though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping
"Sync"— every existing condition name in this file uses that abbreviation:CustomLogoSync,TechPreviewSync,ConfigMapSync,ServiceCASync,TrustedCASync,DeploymentSync.