Skip to content

featuregate for catalogd metas web api #2202

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 20, 2025
Merged

featuregate for catalogd metas web api #2202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d0f74a9
featuregate for catalogd metas web api
grokspawn Feb 12, 2025
cedb085
review resolution
grokspawn Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
| MultiArchInstallAzure| | | | | | |
| ClusterVersionOperatorConfiguration| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | |
| Example2| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | |
| NewOLMCatalogdAPIV1Metas| | | | <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> |
| NewOLM| | <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> |
| AWSClusterHostedDNS| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> |
| AutomatedEtcdBackup| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> |
Expand Down
8 changes: 8 additions & 0 deletions features/features.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,14 @@ var (
enableForClusterProfile(SelfManaged, configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade, configv1.Default).
mustRegister()

FeatureGateNewOLMCatalogdMetas = newFeatureGate("NewOLMCatalogdAPIV1Metas").
reportProblemsToJiraComponent("olm").
contactPerson("jordank").
productScope(ocpSpecific).
enhancementPR("https://github.com/openshift/enhancements/pull/1749").
enableForClusterProfile(SelfManaged, configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is deliberately not being enabled for HyperShift clusters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I think this is just where I'm uncertain about how to go about this. There are Hypershift and SelfManaged options, and I need the latter, but it's unclear how to also enable the former.

Do I list the feature twice (once for each)? Is Hypershift a superset of SelfManaged?

Without some feedback from SNO reviewers I don't think we want this landing as a surprise there, but otherwise this feature should be everywhere else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh. Having played with it a bit, it appears that Hypershift is a superset of SelfManaged, so I /think/ I've fixed it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NewOLM is not enabled on Hypershift (yet), therefore any new subfeature of NewOLM (like this one) should also not be enabled on hypershift.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you just want to enable everywhere and not make a distinction between cluster profiles

Suggested change
enableForClusterProfile(SelfManaged, configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade).
enableIn(configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade).

Copy link
Contributor Author

@grokspawn grokspawn Feb 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would enable us for hypershift, and the new feature flag can't be enable there because its "parent" NewOLM cannot be enabled there yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, in which case you have it correct already!

mustRegister()

FeatureGateInsightsOnDemandDataGather = newFeatureGate("InsightsOnDemandDataGather").
reportProblemsToJiraComponent("insights").
contactPerson("tremes").
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-Hypershift-Default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@
{
"name": "NewOLM"
},
{
"name": "NewOLMCatalogdAPIV1Metas"
},
{
"name": "NodeSwap"
},
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
},
{
"name": "NewOLM"
},
{
"name": "NewOLMCatalogdAPIV1Metas"
}
],
"enabled": [
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
},
{
"name": "NewOLM"
},
{
"name": "NewOLMCatalogdAPIV1Metas"
}
],
"enabled": [
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@
{
"name": "MultiArchInstallAzure"
},
{
"name": "NewOLMCatalogdAPIV1Metas"
},
{
"name": "NodeSwap"
},
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@
{
"name": "NewOLM"
},
{
"name": "NewOLMCatalogdAPIV1Metas"
},
{
"name": "NodeDisruptionPolicy"
},
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@
{
"name": "NewOLM"
},
{
"name": "NewOLMCatalogdAPIV1Metas"
},
{
"name": "NodeDisruptionPolicy"
},
Expand Down