Skip to content
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.

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

Cleanup FeatureGate skippers #105428

Merged
merged 5 commits into from
Oct 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions test/e2e/common/node/container_probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/uuid"
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
kubefeatures "k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/kubelet/events"
"k8s.io/kubernetes/test/e2e/framework"
e2eevents "k8s.io/kubernetes/test/e2e/framework/events"
Expand Down Expand Up @@ -217,7 +218,7 @@ var _ = SIGDescribe("Probing container", func() {
ginkgo.It("should be restarted with an exec liveness probe with timeout [MinimumKubeletVersion:1.20] [NodeConformance]", func() {
// The ExecProbeTimeout feature gate exists to allow backwards-compatibility with pre-1.20 cluster behaviors, where livenessProbe timeouts were ignored
// If ExecProbeTimeout feature gate is disabled, timeout enforcement for exec livenessProbes is disabled, so we should skip this test
e2eskipper.SkipUnlessExecProbeTimeoutEnabled()
e2eskipper.SkipUnlessFeatureGateEnabled(kubefeatures.ExecProbeTimeout)
cmd := []string{"/bin/sh", "-c", "sleep 600"}
livenessProbe := &v1.Probe{
Handler: execHandler([]string{"/bin/sh", "-c", "sleep 10"}),
Expand All @@ -237,7 +238,7 @@ var _ = SIGDescribe("Probing container", func() {
ginkgo.It("should not be ready with an exec readiness probe timeout [MinimumKubeletVersion:1.20] [NodeConformance]", func() {
// The ExecProbeTimeout feature gate exists to allow backwards-compatibility with pre-1.20 cluster behaviors, where readiness probe timeouts were ignored
// If ExecProbeTimeout feature gate is disabled, timeout enforcement for exec readiness probe is disabled, so we should skip this test
e2eskipper.SkipUnlessExecProbeTimeoutEnabled()
e2eskipper.SkipUnlessFeatureGateEnabled(kubefeatures.ExecProbeTimeout)

cmd := []string{"/bin/sh", "-c", "sleep 600"}
readinessProbe := &v1.Probe{
Expand All @@ -259,7 +260,7 @@ var _ = SIGDescribe("Probing container", func() {
// The ExecProbeTimeout feature gate exists to allow backwards-compatibility with pre-1.20 cluster behaviors using dockershim, where livenessProbe timeouts were ignored
// If ExecProbeTimeout feature gate is disabled on a dockershim cluster, timeout enforcement for exec livenessProbes is disabled, but a failing liveness probe MUST still trigger a restart
// Note ExecProbeTimeout=false is not recommended for non-dockershim clusters (e.g., containerd), and this test will fail if run against such a configuration
e2eskipper.SkipUnlessExecProbeTimeoutEnabled()
e2eskipper.SkipUnlessFeatureGateEnabled(kubefeatures.ExecProbeTimeout)

cmd := []string{"/bin/sh", "-c", "sleep 600"}
livenessProbe := &v1.Probe{
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/common/node/downwardapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/uuid"
kubefeatures "k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/test/e2e/framework"
e2enetwork "k8s.io/kubernetes/test/e2e/framework/network"
e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
Expand Down Expand Up @@ -289,7 +290,7 @@ var _ = SIGDescribe("Downward API [Serial] [Disruptive] [NodeFeature:DownwardAPI

ginkgo.Context("Downward API tests for hugepages", func() {
ginkgo.BeforeEach(func() {
e2eskipper.SkipUnlessDownwardAPIHugePagesEnabled()
e2eskipper.SkipUnlessFeatureGateEnabled(kubefeatures.DownwardAPIHugePages)
})

ginkgo.It("should provide container's limits.hugepages-<pagesize> and requests.hugepages-<pagesize> as env vars", func() {
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/common/storage/downwardapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/uuid"
kubefeatures "k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/test/e2e/framework"
e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
imageutils "k8s.io/kubernetes/test/utils/image"
Expand All @@ -35,7 +36,7 @@ var _ = SIGDescribe("Downward API [Serial] [Disruptive] [NodeFeature:EphemeralSt

ginkgo.Context("Downward API tests for local ephemeral storage", func() {
ginkgo.BeforeEach(func() {
e2eskipper.SkipUnlessLocalEphemeralStorageEnabled()
e2eskipper.SkipUnlessFeatureGateEnabled(kubefeatures.LocalStorageCapacityIsolation)
})

ginkgo.It("should provide container's limits.ephemeral-storage and requests.ephemeral-storage as env vars", func() {
Expand Down
5 changes: 1 addition & 4 deletions test/e2e/common/storage/empty_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/uuid"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/test/e2e/framework"
e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
Expand Down Expand Up @@ -297,9 +296,7 @@ var _ = SIGDescribe("EmptyDir volumes", func() {
*/
ginkgo.It("pod should support memory backed volumes of specified size", func() {
// skip if feature gate is not enabled, this could be elevated to conformance in future if on Linux.
if !utilfeature.DefaultFeatureGate.Enabled(features.SizeMemoryBackedVolumes) {
return
}
e2eskipper.SkipUnlessFeatureGateEnabled(features.SizeMemoryBackedVolumes)

var (
volumeName = "shared-data"
Expand Down
36 changes: 8 additions & 28 deletions test/e2e/framework/skipper/skipper.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,6 @@ import (
e2essh "k8s.io/kubernetes/test/e2e/framework/ssh"
)

// New local storage types to support local storage capacity isolation
var localStorageCapacityIsolation featuregate.Feature = "LocalStorageCapacityIsolation"

var (
downwardAPIHugePages featuregate.Feature = "DownwardAPIHugePages"
execProbeTimeout featuregate.Feature = "ExecProbeTimeout"
csiMigration featuregate.Feature = "CSIMigration"
)

func skipInternalf(caller int, format string, args ...interface{}) {
msg := fmt.Sprintf(format, args...)
framework.Logf(msg)
Expand Down Expand Up @@ -136,28 +127,17 @@ func SkipUnlessAtLeast(value int, minValue int, message string) {
}
}

// SkipUnlessLocalEphemeralStorageEnabled skips if the LocalStorageCapacityIsolation is not enabled.
func SkipUnlessLocalEphemeralStorageEnabled() {
if !utilfeature.DefaultFeatureGate.Enabled(localStorageCapacityIsolation) {
skipInternalf(1, "Only supported when %v feature is enabled", localStorageCapacityIsolation)
}
}

func SkipUnlessDownwardAPIHugePagesEnabled() {
if !utilfeature.DefaultFeatureGate.Enabled(downwardAPIHugePages) {
skipInternalf(1, "Only supported when %v feature is enabled", downwardAPIHugePages)
}
}

func SkipUnlessExecProbeTimeoutEnabled() {
if !utilfeature.DefaultFeatureGate.Enabled(execProbeTimeout) {
skipInternalf(1, "Only supported when %v feature is enabled", execProbeTimeout)
// SkipUnlessFeatureGateEnabled skips if the feature is disabled
func SkipUnlessFeatureGateEnabled(gate featuregate.Feature) {
if !utilfeature.DefaultFeatureGate.Enabled(gate) {
skipInternalf(1, "Only supported when %v feature is enabled", gate)
}
}

func SkipIfCSIMigrationEnabled() {
if utilfeature.DefaultFeatureGate.Enabled(csiMigration) {
skipInternalf(1, "Only supported when %v feature is disabled", csiMigration)
// SkipIfFeatureGateEnabled skips if the feature is enabled
func SkipIfFeatureGateEnabled(gate featuregate.Feature) {
Git-Jiro marked this conversation as resolved.
Show resolved Hide resolved
if utilfeature.DefaultFeatureGate.Enabled(gate) {
skipInternalf(1, "Only supported when %v feature is disabled", gate)
}
}

Expand Down
3 changes: 2 additions & 1 deletion test/e2e/storage/volume_limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"k8s.io/api/core/v1"
clientset "k8s.io/client-go/kubernetes"
v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
kubefeatures "k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/test/e2e/framework"
e2enode "k8s.io/kubernetes/test/e2e/framework/node"
e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
Expand All @@ -35,7 +36,7 @@ var _ = utils.SIGDescribe("Volume limits", func() {
ginkgo.BeforeEach(func() {
e2eskipper.SkipUnlessProviderIs("aws", "gce", "gke")
// If CSIMigration is enabled, then the limits should be on CSINodes, not Nodes, and another test checks this
e2eskipper.SkipIfCSIMigrationEnabled()
e2eskipper.SkipIfFeatureGateEnabled(kubefeatures.CSIMigration)
c = f.ClientSet
framework.ExpectNoError(framework.WaitForAllNodesSchedulable(c, framework.TestContext.NodeSchedulableTimeout))
})
Expand Down
4 changes: 1 addition & 3 deletions test/e2e_node/podresources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -626,9 +626,7 @@ var _ = SIGDescribe("POD Resources [Serial] [Feature:PodResources][NodeFeature:P
})

ginkgo.It("should return the expected error with the feature gate disabled", func() {
if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.KubeletPodResourcesGetAllocatable) {
e2eskipper.Skipf("this test is meant to run with the POD Resources Extensions feature gate disabled")
}
e2eskipper.SkipIfFeatureGateEnabled(kubefeatures.KubeletPodResourcesGetAllocatable)

endpoint, err := util.LocalEndpoint(defaultPodResourcesPath, podresources.Socket)
framework.ExpectNoError(err)
Expand Down