Skip to content

Commit c57be6f

Browse files
committed
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.
1 parent 79c61d5 commit c57be6f

File tree

3 files changed

+25
-9
lines changed

3 files changed

+25
-9
lines changed

test/e2e/framework/ginkgowrapper.go

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,9 @@ func registerInSuite(ginkgoCall func(string, ...interface{}) bool, args []interf
209209
case label:
210210
fullLabel := strings.Join(arg.parts, ":")
211211
addLabel(fullLabel)
212-
if arg.extra != "" {
213-
addLabel(arg.extra)
212+
if arg.extraFeature != "" {
213+
texts = append(texts, fmt.Sprintf("[%s]", arg.extraFeature))
214+
ginkgoArgs = append(ginkgoArgs, ginkgo.Label("Feature:"+arg.extraFeature))
214215
}
215216
if fullLabel == "Serial" {
216217
ginkgoArgs = append(ginkgoArgs, ginkgo.Serial)
@@ -309,6 +310,10 @@ func validateText(location types.CodeLocation, text string, labels []string) {
309310
recordTextBug(location, fmt.Sprintf("[%s] in plain text is deprecated and must be added through With%s instead", tag, tag))
310311
}
311312
if deprecatedStability.Has(tag) {
313+
if slices.Contains(labels, "Feature:"+tag) {
314+
// Okay, was also set as label.
315+
continue
316+
}
312317
recordTextBug(location, fmt.Sprintf("[%s] in plain text is deprecated and must be added by defining the feature gate through WithFeatureGate instead", tag))
313318
}
314319
if index := strings.Index(tag, ":"); index > 0 {
@@ -353,6 +358,16 @@ func withFeature(name Feature) interface{} {
353358
// [k8s.io/apiserver/pkg/util/feature.DefaultMutableFeatureGate]. Once a
354359
// feature gate gets removed from there, the WithFeatureGate calls using it
355360
// also need to be removed.
361+
//
362+
// [Alpha] resp. [Beta] get added to the test name automatically depending
363+
// on the current stability level of the feature. Feature:Alpha resp.
364+
// Feature:Beta get added to the Ginkgo labels because this is a special
365+
// requirement for how the cluster needs to be configured.
366+
//
367+
// If the test can run in any cluster that has alpha resp. beta features and
368+
// API groups enabled, then annotating it with just WithFeatureGate is
369+
// sufficient. Otherwise, WithFeature has to be used to define the additional
370+
// requirements.
356371
func WithFeatureGate(featureGate featuregate.Feature) interface{} {
357372
return withFeatureGate(featureGate)
358373
}
@@ -376,7 +391,7 @@ func withFeatureGate(featureGate featuregate.Feature) interface{} {
376391
}
377392

378393
l := newLabel("FeatureGate", string(featureGate))
379-
l.extra = level
394+
l.extraFeature = level
380395
return l
381396
}
382397

@@ -544,8 +559,9 @@ func withFlaky() interface{} {
544559
type label struct {
545560
// parts get concatenated with ":" to build the full label.
546561
parts []string
547-
// extra is an optional fully-formed extra label.
548-
extra string
562+
// extra is an optional feature name. It gets added as [<extraFeature>]
563+
// to the test name and as Feature:<extraFeature> to the labels.
564+
extraFeature string
549565
// explanation gets set for each label to help developers
550566
// who pass a label to a ginkgo function. They need to use
551567
// the corresponding framework function instead.
@@ -572,7 +588,7 @@ func TagsEqual(a, b interface{}) bool {
572588
if !ok {
573589
return false
574590
}
575-
if al.extra != bl.extra {
591+
if al.extraFeature != bl.extraFeature {
576592
return false
577593
}
578594
return slices.Equal(al.parts, bl.parts)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,12 @@ ERROR: some/relative/path/buggy.go:200: with spaces
133133

134134
// Used by unittests/list-labels.
135135
ListLabelsOutput = `The following labels can be used with 'ginkgo run --label-filter':
136-
Alpha
137-
Beta
138136
Conformance
139137
Disruptive
140138
Environment:Linux
141139
Environment:no-such-env
140+
Feature:Alpha
141+
Feature:Beta
142142
Feature:feature-foo
143143
Feature:no-such-feature
144144
FeatureGate:TestAlphaFeature

test/e2e/framework/internal/unittests/list-labels/listlabels_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
"k8s.io/kubernetes/test/e2e/framework/internal/unittests/bugs"
2727
)
2828

29-
func TestListTests(t *testing.T) {
29+
func TestListLabels(t *testing.T) {
3030
bugs.Describe()
3131
framework.CheckForBugs = false
3232
output, code := unittests.GetFrameworkOutput(t, map[string]string{"list-labels": "true"})

0 commit comments

Comments
 (0)