e2e framework: clarify Alpha/Beta requirement for feature gates
We want: - To keep test annotations simple, using both WithFeatureGate and WithFeature should only be necessary when a test really has requirements that go beyond "feature gate needs to be enabled". - To run tests which depend only on feature gates being enabled in the ci-kubernetes-e2e-kind-alpha-features resp. ci-kubernetes-e2e-kind-beta-features, because otherwise we may have a proliferation of many bespoke jobs which only run very few tests. This would make testing more expensive for Kubernetes. - To enable those tests only once in the ci-kubernetes-e2e-kind-alpha-features and ci-kubernetes-e2e-kind-beta-features definition instead of having to update those each time feature gates change. This can be achieved by adding `Feature:Alpha` resp. `Feature:Beta` as Ginkgo labels instead of just `Alpha` and `Beta`. Then jobs which are configured to skip tests with feature dependencies via --label-filter=!/Feature:.+/ will skip tests which are labeled with just WithFeatureGate. The ci-kubernetes jobs can select to include such tests with a special regexp that mimicks a negative lookahead (see k8s.io/community/contributors/devel/sig-testing/e2e-tests.md) Note that removing WithFeature depends on first updating job definitions to use --label-filter or to skip based on the inline `[Alpha]` or `[Beta]` text, otherwise tests that were previously skipped because of WithFeature might start to run in jobs which don't have the feature gate enabled.
This commit is contained in:
		@@ -209,8 +209,9 @@ func registerInSuite(ginkgoCall func(string, ...interface{}) bool, args []interf
 | 
				
			|||||||
		case label:
 | 
							case label:
 | 
				
			||||||
			fullLabel := strings.Join(arg.parts, ":")
 | 
								fullLabel := strings.Join(arg.parts, ":")
 | 
				
			||||||
			addLabel(fullLabel)
 | 
								addLabel(fullLabel)
 | 
				
			||||||
			if arg.extra != "" {
 | 
								if arg.extraFeature != "" {
 | 
				
			||||||
				addLabel(arg.extra)
 | 
									texts = append(texts, fmt.Sprintf("[%s]", arg.extraFeature))
 | 
				
			||||||
 | 
									ginkgoArgs = append(ginkgoArgs, ginkgo.Label("Feature:"+arg.extraFeature))
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
			if fullLabel == "Serial" {
 | 
								if fullLabel == "Serial" {
 | 
				
			||||||
				ginkgoArgs = append(ginkgoArgs, ginkgo.Serial)
 | 
									ginkgoArgs = append(ginkgoArgs, ginkgo.Serial)
 | 
				
			||||||
@@ -309,6 +310,10 @@ func validateText(location types.CodeLocation, text string, labels []string) {
 | 
				
			|||||||
			recordTextBug(location, fmt.Sprintf("[%s] in plain text is deprecated and must be added through With%s instead", tag, tag))
 | 
								recordTextBug(location, fmt.Sprintf("[%s] in plain text is deprecated and must be added through With%s instead", tag, tag))
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
		if deprecatedStability.Has(tag) {
 | 
							if deprecatedStability.Has(tag) {
 | 
				
			||||||
 | 
								if slices.Contains(labels, "Feature:"+tag) {
 | 
				
			||||||
 | 
									// Okay, was also set as label.
 | 
				
			||||||
 | 
									continue
 | 
				
			||||||
 | 
								}
 | 
				
			||||||
			recordTextBug(location, fmt.Sprintf("[%s] in plain text is deprecated and must be added by defining the feature gate through WithFeatureGate instead", tag))
 | 
								recordTextBug(location, fmt.Sprintf("[%s] in plain text is deprecated and must be added by defining the feature gate through WithFeatureGate instead", tag))
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
		if index := strings.Index(tag, ":"); index > 0 {
 | 
							if index := strings.Index(tag, ":"); index > 0 {
 | 
				
			||||||
@@ -353,6 +358,16 @@ func withFeature(name Feature) interface{} {
 | 
				
			|||||||
// [k8s.io/apiserver/pkg/util/feature.DefaultMutableFeatureGate]. Once a
 | 
					// [k8s.io/apiserver/pkg/util/feature.DefaultMutableFeatureGate]. Once a
 | 
				
			||||||
// feature gate gets removed from there, the WithFeatureGate calls using it
 | 
					// feature gate gets removed from there, the WithFeatureGate calls using it
 | 
				
			||||||
// also need to be removed.
 | 
					// also need to be removed.
 | 
				
			||||||
 | 
					//
 | 
				
			||||||
 | 
					// [Alpha] resp. [Beta] get added to the test name automatically depending
 | 
				
			||||||
 | 
					// on the current stability level of the feature. Feature:Alpha resp.
 | 
				
			||||||
 | 
					// Feature:Beta get added to the Ginkgo labels because this is a special
 | 
				
			||||||
 | 
					// requirement for how the cluster needs to be configured.
 | 
				
			||||||
 | 
					//
 | 
				
			||||||
 | 
					// If the test can run in any cluster that has alpha resp. beta features and
 | 
				
			||||||
 | 
					// API groups enabled, then annotating it with just WithFeatureGate is
 | 
				
			||||||
 | 
					// sufficient. Otherwise, WithFeature has to be used to define the additional
 | 
				
			||||||
 | 
					// requirements.
 | 
				
			||||||
func WithFeatureGate(featureGate featuregate.Feature) interface{} {
 | 
					func WithFeatureGate(featureGate featuregate.Feature) interface{} {
 | 
				
			||||||
	return withFeatureGate(featureGate)
 | 
						return withFeatureGate(featureGate)
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
@@ -376,7 +391,7 @@ func withFeatureGate(featureGate featuregate.Feature) interface{} {
 | 
				
			|||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	l := newLabel("FeatureGate", string(featureGate))
 | 
						l := newLabel("FeatureGate", string(featureGate))
 | 
				
			||||||
	l.extra = level
 | 
						l.extraFeature = level
 | 
				
			||||||
	return l
 | 
						return l
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -544,8 +559,9 @@ func withFlaky() interface{} {
 | 
				
			|||||||
type label struct {
 | 
					type label struct {
 | 
				
			||||||
	// parts get concatenated with ":" to build the full label.
 | 
						// parts get concatenated with ":" to build the full label.
 | 
				
			||||||
	parts []string
 | 
						parts []string
 | 
				
			||||||
	// extra is an optional fully-formed extra label.
 | 
						// extra is an optional feature name. It gets added as [<extraFeature>]
 | 
				
			||||||
	extra string
 | 
						// to the test name and as Feature:<extraFeature> to the labels.
 | 
				
			||||||
 | 
						extraFeature string
 | 
				
			||||||
	// explanation gets set for each label to help developers
 | 
						// explanation gets set for each label to help developers
 | 
				
			||||||
	// who pass a label to a ginkgo function. They need to use
 | 
						// who pass a label to a ginkgo function. They need to use
 | 
				
			||||||
	// the corresponding framework function instead.
 | 
						// the corresponding framework function instead.
 | 
				
			||||||
@@ -572,7 +588,7 @@ func TagsEqual(a, b interface{}) bool {
 | 
				
			|||||||
	if !ok {
 | 
						if !ok {
 | 
				
			||||||
		return false
 | 
							return false
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	if al.extra != bl.extra {
 | 
						if al.extraFeature != bl.extraFeature {
 | 
				
			||||||
		return false
 | 
							return false
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	return slices.Equal(al.parts, bl.parts)
 | 
						return slices.Equal(al.parts, bl.parts)
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -133,12 +133,12 @@ ERROR: some/relative/path/buggy.go:200: with spaces
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
	// Used by unittests/list-labels.
 | 
						// Used by unittests/list-labels.
 | 
				
			||||||
	ListLabelsOutput = `The following labels can be used with 'ginkgo run --label-filter':
 | 
						ListLabelsOutput = `The following labels can be used with 'ginkgo run --label-filter':
 | 
				
			||||||
    Alpha
 | 
					 | 
				
			||||||
    Beta
 | 
					 | 
				
			||||||
    Conformance
 | 
					    Conformance
 | 
				
			||||||
    Disruptive
 | 
					    Disruptive
 | 
				
			||||||
    Environment:Linux
 | 
					    Environment:Linux
 | 
				
			||||||
    Environment:no-such-env
 | 
					    Environment:no-such-env
 | 
				
			||||||
 | 
					    Feature:Alpha
 | 
				
			||||||
 | 
					    Feature:Beta
 | 
				
			||||||
    Feature:feature-foo
 | 
					    Feature:feature-foo
 | 
				
			||||||
    Feature:no-such-feature
 | 
					    Feature:no-such-feature
 | 
				
			||||||
    FeatureGate:TestAlphaFeature
 | 
					    FeatureGate:TestAlphaFeature
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -26,7 +26,7 @@ import (
 | 
				
			|||||||
	"k8s.io/kubernetes/test/e2e/framework/internal/unittests/bugs"
 | 
						"k8s.io/kubernetes/test/e2e/framework/internal/unittests/bugs"
 | 
				
			||||||
)
 | 
					)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func TestListTests(t *testing.T) {
 | 
					func TestListLabels(t *testing.T) {
 | 
				
			||||||
	bugs.Describe()
 | 
						bugs.Describe()
 | 
				
			||||||
	framework.CheckForBugs = false
 | 
						framework.CheckForBugs = false
 | 
				
			||||||
	output, code := unittests.GetFrameworkOutput(t, map[string]string{"list-labels": "true"})
 | 
						output, code := unittests.GetFrameworkOutput(t, map[string]string{"list-labels": "true"})
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user