Merge pull request kubernetes#130655 from BenTheElder/pr130580

Set [Feature:OffByDefault] when WithFeatureGate receives an off-by-default feature gate

Author: k8s-ci-robot

test/e2e/auth/projected_clustertrustbundle.go

@@ -39,7 +39,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/apimachinery/pkg/util/wait"
 	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
-	"k8s.io/kubernetes/test/e2e/feature"
+	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2epodoutput "k8s.io/kubernetes/test/e2e/framework/pod/output"
 	imageutils "k8s.io/kubernetes/test/utils/image"
@@ -57,7 +57,7 @@ const (
 	noSignerKey       = "no-signer"
 )
 
-var _ = SIGDescribe(feature.ClusterTrustBundle, feature.ClusterTrustBundleProjection, func() {
+var _ = SIGDescribe(framework.WithFeatureGate(features.ClusterTrustBundle), framework.WithFeatureGate(features.ClusterTrustBundleProjection), func() {
 	f := framework.NewDefaultFramework("projected-clustertrustbundle")
 	f.NamespacePodSecurityLevel = admissionapi.LevelBaseline
 

test/e2e/feature/feature.go

@@ -60,12 +60,6 @@ var (
 	// Owner: sig-autoscaling
 	ClusterSizeAutoscalingScaleUp = framework.WithFeature(framework.ValidFeatures.Add("ClusterSizeAutoscalingScaleUp"))
 
-	// TODO: document the feature (owning SIG, when to use this feature for a test)
-	ClusterTrustBundle = framework.WithFeature(framework.ValidFeatures.Add("ClusterTrustBundle"))
-
-	// TODO: document the feature (owning SIG, when to use this feature for a test)
-	ClusterTrustBundleProjection = framework.WithFeature(framework.ValidFeatures.Add("ClusterTrustBundleProjection"))
-
 	// TODO: document the feature (owning SIG, when to use this feature for a test)
 	ClusterUpgrade = framework.WithFeature(framework.ValidFeatures.Add("ClusterUpgrade"))
 

test/e2e/framework/ginkgowrapper.go

@@ -208,9 +208,14 @@ func registerInSuite(ginkgoCall func(string, ...interface{}) bool, args []interf
 		case label:
 			fullLabel := strings.Join(arg.parts, ":")
 			addLabel(fullLabel)
-			if arg.extraFeature != "" {
-				texts = append(texts, fmt.Sprintf("[%s]", arg.extraFeature))
-				ginkgoArgs = append(ginkgoArgs, ginkgo.Label("Feature:"+arg.extraFeature))
+			if arg.alphaBetaLevel != "" {
+				texts = append(texts, fmt.Sprintf("[%[1]s]", arg.alphaBetaLevel))
+				ginkgoArgs = append(ginkgoArgs, ginkgo.Label("Feature:"+arg.alphaBetaLevel))
+			}
+			if arg.offByDefault {
+				texts = append(texts, "[Feature:OffByDefault]")
+				// TODO: consider this once we have a plan to update the alpha/beta job filters
+				// ginkgoArgs = append(ginkgoArgs, ginkgo.Label("Feature:OffByDefault"))
 			}
 			if fullLabel == "Serial" {
 				ginkgoArgs = append(ginkgoArgs, ginkgo.Serial)
@@ -305,6 +310,12 @@ func validateText(location types.CodeLocation, text string, labels []string) {
 			// Okay, was also set as label.
 			continue
 		}
+		// TODO: we currently only set this as a text value
+		// We should probably reflect it into labels, but that could break some
+		// existing jobs and we're still setting on an exact plan
+		if tag == "Feature:OffByDefault" {
+			continue
+		}
 		if deprecatedTags.Has(tag) {
 			recordTextBug(location, fmt.Sprintf("[%s] in plain text is deprecated and must be added through With%s instead", tag, tag))
 		}
@@ -350,7 +361,8 @@ func withFeature(name Feature) interface{} {
 }
 
 // WithFeatureGate specifies that a certain test or group of tests depends on a
-// feature gate being enabled. The return value must be passed as additional
+// feature gate and the corresponding API group (if there is one)
+// being enabled. The return value must be passed as additional
 // argument to [framework.It], [framework.Describe], [framework.Context].
 //
 // The feature gate must be listed in
@@ -359,9 +371,15 @@ func withFeature(name Feature) interface{} {
 // 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.
+// on the current stability level of the feature, to emulate historic
+// usage of those tags.
+//
+// In addition, [Feature:Alpha] resp. [Feature:Beta] get added to support
+// skipping a test with a dependency on an alpha or beta feature gate in
+// jobs which use the traditional \[Feature:.*\] skip regular expression.
+//
+// For label filtering, Feature:Alpha resp. Feature:Beta get added to the
+// Ginkgo labels.
 //
 // 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
@@ -390,7 +408,8 @@ func withFeatureGate(featureGate featuregate.Feature) interface{} {
 	}
 
 	l := newLabel("FeatureGate", string(featureGate))
-	l.extraFeature = level
+	l.offByDefault = !spec.Default
+	l.alphaBetaLevel = level
 	return l
 }
 
@@ -536,13 +555,19 @@ func withFlaky() interface{} {
 type label struct {
 	// parts get concatenated with ":" to build the full label.
 	parts []string
-	// extra is an optional feature name. It gets added as [<extraFeature>]
-	// to the test name and as Feature:<extraFeature> to the labels.
-	extraFeature string
 	// explanation gets set for each label to help developers
 	// who pass a label to a ginkgo function. They need to use
 	// the corresponding framework function instead.
 	explanation string
+
+	// TODO: the fields below are only used for FeatureGates, we may want to refactor
+
+	// alphaBetaLevel is "Alpha", "Beta" or empty for GA features
+	// It gets added as [<level>] [Feature:<level>]
+	// to the test name and as Feature:<level> to the labels.
+	alphaBetaLevel string
+	// set based on featuregate default state
+	offByDefault bool
 }
 
 func newLabel(parts ...string) label {
@@ -565,7 +590,10 @@ func TagsEqual(a, b interface{}) bool {
 	if !ok {
 		return false
 	}
-	if al.extraFeature != bl.extraFeature {
+	if al.alphaBetaLevel != bl.alphaBetaLevel {
+		return false
+	}
+	if al.offByDefault != bl.offByDefault {
 		return false
 	}
 	return slices.Equal(al.parts, bl.parts)

test/e2e/framework/internal/unittests/bugs/bugs.go

@@ -122,8 +122,8 @@ ERROR: some/relative/path/buggy.go:200: with spaces
 `
 	// Used by unittests/list-tests. It's sorted by test name, not source code location.
 	ListTestsOutput = `The following spec names can be used with 'ginkgo run --focus/skip':
-    ../bugs/bugs.go:100: [sig-testing] abc   space1 space2  [Feature:no-such-feature] [Feature:feature-foo] [Environment:no-such-env] [Environment:Linux] [FeatureGate:no-such-feature-gate] [FeatureGate:TestAlphaFeature] [Alpha] [FeatureGate:TestBetaFeature] [Beta] [FeatureGate:TestGAFeature] [Conformance] [NodeConformance] [Slow] [Serial] [Disruptive] [custom-label] xyz x [foo] should [bar]
-    ../bugs/bugs.go:95: [sig-testing] abc   space1 space2  [Feature:no-such-feature] [Feature:feature-foo] [Environment:no-such-env] [Environment:Linux] [FeatureGate:no-such-feature-gate] [FeatureGate:TestAlphaFeature] [Alpha] [FeatureGate:TestBetaFeature] [Beta] [FeatureGate:TestGAFeature] [Conformance] [NodeConformance] [Slow] [Serial] [Disruptive] [custom-label] xyz y [foo] should [bar]
+    ../bugs/bugs.go:100: [sig-testing] abc   space1 space2  [Feature:no-such-feature] [Feature:feature-foo] [Environment:no-such-env] [Environment:Linux] [FeatureGate:no-such-feature-gate] [Feature:OffByDefault] [FeatureGate:TestAlphaFeature] [Alpha] [Feature:OffByDefault] [FeatureGate:TestBetaFeature] [Beta] [Feature:OffByDefault] [FeatureGate:TestGAFeature] [Feature:OffByDefault] [Conformance] [NodeConformance] [Slow] [Serial] [Disruptive] [custom-label] xyz x [foo] should [bar]
+    ../bugs/bugs.go:95: [sig-testing] abc   space1 space2  [Feature:no-such-feature] [Feature:feature-foo] [Environment:no-such-env] [Environment:Linux] [FeatureGate:no-such-feature-gate] [Feature:OffByDefault] [FeatureGate:TestAlphaFeature] [Alpha] [Feature:OffByDefault] [FeatureGate:TestBetaFeature] [Beta] [Feature:OffByDefault] [FeatureGate:TestGAFeature] [Feature:OffByDefault] [Conformance] [NodeConformance] [Slow] [Serial] [Disruptive] [custom-label] xyz y [foo] should [bar]
 
 `