Feature gates for platforms, architecture, and installs #1895

stbenjam · 2024-05-16T15:37:09Z

When validating a feature gate has tests, try to determine if the
feature gate is specific to a platform or architecture and only query
those results.

If it's an installer feature gate, then we don't require specific tests;
use the existing cluster install tests.

openshift-ci · 2024-05-16T15:37:15Z

Hello @stbenjam! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

openshift-ci · 2024-05-16T15:37:16Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

stbenjam · 2024-05-16T15:37:17Z

/test verify

stbenjam · 2024-05-16T15:50:12Z

Testing locally with #1892. The platforms TRT cares about pass. Nutanix is missing a variant in Sippy (but is healthy), and OpenStack pass rates are low because the techpreview jobs aren't showing up (they lack a version in their job names)

Query component readiness for all test run results for feature gate "ClusterAPIInstallAWS" on clusterProfile ["Hypershift" "SelfManagedHA"]
Query component readiness for all test run results for pattern "install should succeed" on variant main.JobVariant{Cloud:"aws", Architecture:"amd64", Topology:"hypershift"}
Query component readiness for all test run results for pattern "install should succeed" on variant main.JobVariant{Cloud:"aws", Architecture:"amd64", Topology:"ha"}
Sufficient CI testing for "ClusterAPIInstallAWS".
Query component readiness for all test run results for feature gate "ClusterAPIInstallNutanix" on clusterProfile ["Hypershift" "SelfManagedHA"]
Query component readiness for all test run results for pattern "install should succeed" on variant main.JobVariant{Cloud:"nutanix", Architecture:"amd64", Topology:"ha"}
INSUFFICIENT CI testing for "ClusterAPIInstallNutanix".
Query component readiness for all test run results for feature gate "ClusterAPIInstallOpenStack" on clusterProfile ["Hypershift" "SelfManagedHA"]
Query component readiness for all test run results for pattern "install should succeed" on variant main.JobVariant{Cloud:"openstack", Architecture:"amd64", Topology:"ha"}
INSUFFICIENT CI testing for "ClusterAPIInstallOpenStack".
Query component readiness for all test run results for feature gate "ClusterAPIInstallVSphere" on clusterProfile ["Hypershift" "SelfManagedHA"]
Query component readiness for all test run results for pattern "install should succeed" on variant main.JobVariant{Cloud:"vsphere-ipi", Architecture:"amd64", Topology:"ha"}
Sufficient CI testing for "ClusterAPIInstallVSphere".
F0516 12:19:14.329322    5899 root.go:64] Error running codegen: error: only 0 tests found, need at least 5 for "ClusterAPIInstallNutanix" on {nutanix amd64 ha}
error: "install should succeed: cluster bootstrap" only passed 91%, need at least 95% for "ClusterAPIInstallOpenStack" on {openstack amd64 ha}
error: "install should succeed: cluster creation" only passed 92%, need at least 95% for "ClusterAPIInstallOpenStack" on {openstack amd64 ha}
error: "install should succeed: overall" only passed 83%, need at least 95% for "ClusterAPIInstallOpenStack" on {openstack amd64 ha}

tools/codegen/cmd/featuregate-test-analyzer.go

deads2k · 2024-05-23T18:27:09Z

tools/codegen/pkg/sippy/json_types.go

@@ -101,7 +105,7 @@ func QueriesFor(cloud, architecture, topology, featureGate string) []*SippyQuery
 					ColumnField:   "name",
 					Not:           false,
 					OperatorValue: "contains",
-					Value:         fmt.Sprintf("[FeatureGate:%s]", featureGate),


This is what allowed us to handle both kube and openshift FeatureGate markers in tests. Can you refactor to ensure we get OCPFeatureGate and FeatureGate tags?

I fixed to use the common suffix to both, FeatureGate:%s, as the test pattern, is that ok?

deads2k · 2024-05-23T18:28:39Z

tools/codegen/cmd/featuregate-test-analyzer.go

-	if len(jobVariantsToCheck) == 0 {
-		jobVariantsToCheck = append(jobVariantsToCheck, requiredSelfManagedJobVariants...)
+		// See if the feature gate is specific to any optional platform
+		optionalPlatformVariants := filterVariants(featureGate, optionalSelfManagedPlatformVariants, true)


is this actually, "platform variants that a featuregate is targeted for" versus "all platforms that we support"?

deads2k · 2024-05-23T18:29:40Z

tools/codegen/cmd/featuregate-test-analyzer.go

-	if len(jobVariantsToCheck) == 0 {
-		jobVariantsToCheck = append(jobVariantsToCheck, requiredSelfManagedJobVariants...)
+		// See if the feature gate is specific to any optional platform
+		optionalPlatformVariants := filterVariants(featureGate, optionalSelfManagedPlatformVariants, true)


oh it isn't. I've misunderstood. This is a sometimes list versus the rest. Thinking about the factoring here.

deads2k · 2024-05-23T18:30:26Z

tools/codegen/cmd/featuregate-test-analyzer.go

@@ -363,6 +365,21 @@ var (
 		//},
 	}

+	// These are only checked if the feature gate is platform specific


Why shouldn't we always checks and if a featuregate is targeted at a particular variant, then restrict to just that one?

Do you really care about enforcing on nutanix and openstack? These were new jobs added for install, and I didn't think they were proven stable enough to check every new feature gate unless it was specifically for them

deads2k · 2024-05-23T18:31:15Z

tools/codegen/cmd/featuregate-test-analyzer.go

@@ -456,8 +476,32 @@ func listTestResultFor(featureGate string, clusterProfiles sets.Set[string]) (ma
 	return results, nil
 }

+func filterVariants(featureGate string, variants []JobVariant, optional bool) []JobVariant {


if we looked at everything by default, then I think this doesn't need the "optional" flag.

When validating a feature gate has tests, try to determine if the feature gate is specific to a platform or architecture and only query those results. If it's an installer feature gate, then we don't require specific tests; use the existing `cluster install` tests.

openshift-ci · 2024-05-29T14:30:39Z

@stbenjam: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

deads2k · 2024-05-29T19:16:27Z

/lgtm
/approve

deads2k · 2024-05-29T19:16:53Z

/cherry-pick release-4.16

openshift-cherrypick-robot · 2024-05-29T19:16:55Z

@deads2k: once the present PR merges, I will cherry-pick it on top of release-4.16 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.16

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-ci · 2024-05-29T19:18:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, stbenjam

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [deads2k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-cherrypick-robot · 2024-05-29T19:24:29Z

@deads2k: new pull request created: #1910

In response to this:

/cherry-pick release-4.16

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-bot · 2024-05-30T04:31:42Z

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-cluster-config-api-container-v4.17.0-202405300213.p0.g16d44e6.assembly.stream.el9 for distgit ose-cluster-config-api.
All builds following this will include this PR.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 16, 2024

openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 16, 2024

stbenjam force-pushed the platform-feature-gates branch from 4d04ad3 to 86132e8 Compare May 16, 2024 15:49

stbenjam marked this pull request as ready for review May 16, 2024 15:49

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 16, 2024

openshift-ci bot requested review from deads2k and sjenning May 16, 2024 15:51

stbenjam mentioned this pull request May 16, 2024

Move CAPI install platforms to default feature set #1892

Merged

stbenjam force-pushed the platform-feature-gates branch from 86132e8 to c9141fa Compare May 16, 2024 16:13

deads2k reviewed May 23, 2024

View reviewed changes

tools/codegen/cmd/featuregate-test-analyzer.go Show resolved Hide resolved

deads2k reviewed May 23, 2024

View reviewed changes

stbenjam force-pushed the platform-feature-gates branch from ba6715b to 18265e8 Compare May 29, 2024 13:59

Address feedback

4a42830

stbenjam force-pushed the platform-feature-gates branch from 18265e8 to 4a42830 Compare May 29, 2024 14:01

openshift-ci bot assigned deads2k May 29, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 29, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 29, 2024

openshift-merge-bot bot merged commit 16d44e6 into openshift:master May 29, 2024
9 checks passed

openshift-cherrypick-robot mentioned this pull request May 29, 2024

[release-4.16] Feature gates for platforms, architecture, and installs #1910

Merged

stbenjam deleted the platform-feature-gates branch May 30, 2024 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature gates for platforms, architecture, and installs #1895

Feature gates for platforms, architecture, and installs #1895

stbenjam commented May 16, 2024

openshift-ci bot commented May 16, 2024

openshift-ci bot commented May 16, 2024

stbenjam commented May 16, 2024

stbenjam commented May 16, 2024 •

edited

deads2k May 23, 2024

stbenjam May 29, 2024

deads2k May 23, 2024

deads2k May 23, 2024

deads2k May 23, 2024

stbenjam May 29, 2024

deads2k May 23, 2024

openshift-ci bot commented May 29, 2024

deads2k commented May 29, 2024

deads2k commented May 29, 2024

openshift-cherrypick-robot commented May 29, 2024

openshift-ci bot commented May 29, 2024

openshift-cherrypick-robot commented May 29, 2024

openshift-bot commented May 30, 2024

Feature gates for platforms, architecture, and installs #1895

Feature gates for platforms, architecture, and installs #1895

Conversation

stbenjam commented May 16, 2024

openshift-ci bot commented May 16, 2024

openshift-ci bot commented May 16, 2024

stbenjam commented May 16, 2024

stbenjam commented May 16, 2024 • edited

deads2k May 23, 2024

Choose a reason for hiding this comment

stbenjam May 29, 2024

Choose a reason for hiding this comment

deads2k May 23, 2024

Choose a reason for hiding this comment

deads2k May 23, 2024

Choose a reason for hiding this comment

deads2k May 23, 2024

Choose a reason for hiding this comment

stbenjam May 29, 2024

Choose a reason for hiding this comment

deads2k May 23, 2024

Choose a reason for hiding this comment

openshift-ci bot commented May 29, 2024

deads2k commented May 29, 2024

deads2k commented May 29, 2024

openshift-cherrypick-robot commented May 29, 2024

openshift-ci bot commented May 29, 2024

openshift-cherrypick-robot commented May 29, 2024

openshift-bot commented May 30, 2024

stbenjam commented May 16, 2024 •

edited