From 0c2136ab546fc24ca2ab3c0cedb50d582de48db1 Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Mon, 13 Feb 2023 10:39:38 +0100 Subject: [PATCH] Graduate `DownwardAPIHugePages` feature to stable / GA This update updates the feature documentation for its GA graduation. Signed-off-by: Sascha Grunert --- pkg/api/pod/util.go | 43 --------------- pkg/apis/core/validation/validation.go | 14 +---- pkg/apis/core/validation/validation_test.go | 59 +-------------------- pkg/features/kube_features.go | 3 +- 4 files changed, 5 insertions(+), 114 deletions(-) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 9cc7aeb1156..28404f43263 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -295,37 +295,6 @@ func UpdatePodCondition(status *api.PodStatus, condition *api.PodCondition) bool return !isEqual } -// usesHugePagesInProjectedVolume returns true if hugepages are used in downward api for volume -func usesHugePagesInProjectedVolume(podSpec *api.PodSpec) bool { - // determine if any container is using hugepages in downward api volume - for _, volumeSource := range podSpec.Volumes { - if volumeSource.DownwardAPI != nil { - for _, item := range volumeSource.DownwardAPI.Items { - if item.ResourceFieldRef != nil { - if strings.HasPrefix(item.ResourceFieldRef.Resource, "requests.hugepages-") || strings.HasPrefix(item.ResourceFieldRef.Resource, "limits.hugepages-") { - return true - } - } - } - } - } - return false -} - -// usesHugePagesInProjectedEnv returns true if hugepages are used in downward api for volume -func usesHugePagesInProjectedEnv(item api.Container) bool { - for _, env := range item.Env { - if env.ValueFrom != nil { - if env.ValueFrom.ResourceFieldRef != nil { - if strings.HasPrefix(env.ValueFrom.ResourceFieldRef.Resource, "requests.hugepages-") || strings.HasPrefix(env.ValueFrom.ResourceFieldRef.Resource, "limits.hugepages-") { - return true - } - } - } - } - return false -} - func checkContainerUseIndivisibleHugePagesValues(container api.Container) bool { for resourceName, quantity := range container.Resources.Limits { if helper.IsHugePageResourceName(resourceName) { @@ -418,8 +387,6 @@ func hasInvalidTopologySpreadConstraintLabelSelector(spec *api.PodSpec) bool { func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, podMeta, oldPodMeta *metav1.ObjectMeta) apivalidation.PodValidationOptions { // default pod validation options based on feature gate opts := apivalidation.PodValidationOptions{ - // Allow pod spec to use hugepages in downward API if feature is enabled - AllowDownwardAPIHugePages: utilfeature.DefaultFeatureGate.Enabled(features.DownwardAPIHugePages), AllowInvalidPodDeletionCost: !utilfeature.DefaultFeatureGate.Enabled(features.PodDeletionCost), // Do not allow pod spec to use non-integer multiple of huge page unit size default AllowIndivisibleHugePagesValues: false, @@ -430,16 +397,6 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po } if oldPodSpec != nil { - // if old spec used hugepages in downward api, we must allow it - opts.AllowDownwardAPIHugePages = opts.AllowDownwardAPIHugePages || usesHugePagesInProjectedVolume(oldPodSpec) - // determine if any container is using hugepages in env var - if !opts.AllowDownwardAPIHugePages { - VisitContainers(oldPodSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool { - opts.AllowDownwardAPIHugePages = opts.AllowDownwardAPIHugePages || usesHugePagesInProjectedEnv(*c) - return !opts.AllowDownwardAPIHugePages - }) - } - // if old spec used non-integer multiple of huge page unit size, we must allow it opts.AllowIndivisibleHugePagesValues = usesIndivisibleHugePagesValues(oldPodSpec) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 74655ce7cbc..8ccd27ced14 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1044,10 +1044,7 @@ func validateDownwardAPIVolumeFile(file *core.DownwardAPIVolumeFile, fldPath *fi allErrs = append(allErrs, field.Invalid(fldPath, "resource", "fieldRef and resourceFieldRef can not be specified simultaneously")) } } else if file.ResourceFieldRef != nil { - localValidContainerResourceFieldPathPrefixes := validContainerResourceFieldPathPrefixes - if opts.AllowDownwardAPIHugePages { - localValidContainerResourceFieldPathPrefixes = validContainerResourceFieldPathPrefixesWithDownwardAPIHugePages - } + localValidContainerResourceFieldPathPrefixes := validContainerResourceFieldPathPrefixesWithDownwardAPIHugePages allErrs = append(allErrs, validateContainerResourceFieldSelector(file.ResourceFieldRef, &validContainerResourceFieldPathExpressions, &localValidContainerResourceFieldPathPrefixes, fldPath.Child("resourceFieldRef"), true)...) } else { allErrs = append(allErrs, field.Required(fldPath, "one of fieldRef and resourceFieldRef is required")) @@ -2404,8 +2401,6 @@ var validEnvDownwardAPIFieldPathExpressions = sets.NewString( var validContainerResourceFieldPathExpressions = sets.NewString("limits.cpu", "limits.memory", "limits.ephemeral-storage", "requests.cpu", "requests.memory", "requests.ephemeral-storage") -// NOTE: this is only valid with DownwardAPIHugePages enabled -var validContainerResourceFieldPathPrefixes = sets.NewString() var validContainerResourceFieldPathPrefixesWithDownwardAPIHugePages = sets.NewString(hugepagesRequestsPrefixDownwardAPI, hugepagesLimitsPrefixDownwardAPI) const hugepagesRequestsPrefixDownwardAPI string = `requests.hugepages-` @@ -2426,10 +2421,7 @@ func validateEnvVarValueFrom(ev core.EnvVar, fldPath *field.Path, opts PodValida } if ev.ValueFrom.ResourceFieldRef != nil { numSources++ - localValidContainerResourceFieldPathPrefixes := validContainerResourceFieldPathPrefixes - if opts.AllowDownwardAPIHugePages { - localValidContainerResourceFieldPathPrefixes = validContainerResourceFieldPathPrefixesWithDownwardAPIHugePages - } + localValidContainerResourceFieldPathPrefixes := validContainerResourceFieldPathPrefixesWithDownwardAPIHugePages allErrs = append(allErrs, validateContainerResourceFieldSelector(ev.ValueFrom.ResourceFieldRef, &validContainerResourceFieldPathExpressions, &localValidContainerResourceFieldPathPrefixes, fldPath.Child("resourceFieldRef"), false)...) } if ev.ValueFrom.ConfigMapKeyRef != nil { @@ -3625,8 +3617,6 @@ func validateContainerOnlyForPod(ctr *core.Container, path *field.Path) field.Er // PodValidationOptions contains the different settings for pod validation type PodValidationOptions struct { - // Allow pod spec to use hugepages in downward API - AllowDownwardAPIHugePages bool // Allow invalid pod-deletion-cost annotation value for backward compatibility. AllowInvalidPodDeletionCost bool // Allow invalid label-value in LabelSelector diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 1ccdaa20901..2f453037e00 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -4269,53 +4269,6 @@ func TestValidateVolumes(t *testing.T) { }, }, }, - opts: PodValidationOptions{AllowDownwardAPIHugePages: true}, - }, - { - name: "hugepages-downwardAPI-requests-disabled", - vol: core.Volume{ - Name: "downwardapi", - VolumeSource: core.VolumeSource{ - DownwardAPI: &core.DownwardAPIVolumeSource{ - Items: []core.DownwardAPIVolumeFile{ - { - Path: "hugepages_request", - ResourceFieldRef: &core.ResourceFieldSelector{ - ContainerName: "test-container", - Resource: "requests.hugepages-2Mi", - }, - }, - }, - }, - }, - }, - errs: []verr{{ - etype: field.ErrorTypeNotSupported, - field: "downwardAPI.resourceFieldRef.resource", - }}, - }, - { - name: "hugepages-downwardAPI-limits-disabled", - vol: core.Volume{ - Name: "downwardapi", - VolumeSource: core.VolumeSource{ - DownwardAPI: &core.DownwardAPIVolumeSource{ - Items: []core.DownwardAPIVolumeFile{ - { - Path: "hugepages_limit", - ResourceFieldRef: &core.ResourceFieldSelector{ - ContainerName: "test-container", - Resource: "limits.hugepages-2Mi", - }, - }, - }, - }, - }, - }, - errs: []verr{{ - etype: field.ErrorTypeNotSupported, - field: "downwardAPI.resourceFieldRef.resource", - }}, }, { name: "downapi valid defaultMode", @@ -5555,22 +5508,12 @@ func TestHugePagesEnv(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DownwardAPIHugePages, true)() - opts := PodValidationOptions{AllowDownwardAPIHugePages: true} + opts := PodValidationOptions{} if errs := validateEnvVarValueFrom(testCase, field.NewPath("field"), opts); len(errs) != 0 { t.Errorf("expected success, got: %v", errs) } }) } - // disable gate - for _, testCase := range testCases { - t.Run(testCase.Name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DownwardAPIHugePages, false)() - opts := PodValidationOptions{AllowDownwardAPIHugePages: false} - if errs := validateEnvVarValueFrom(testCase, field.NewPath("field"), opts); len(errs) == 0 { - t.Errorf("expected failure") - } - }) - } } func TestValidateEnv(t *testing.T) { diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 5a9ef0ce375..fc41224653f 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -240,6 +240,7 @@ const ( // owner: @derekwaynecarr // alpha: v1.20 // beta: v1.21 (off by default until 1.22) + // ga: v1.27 // // Enables usage of hugepages- in downward API. DownwardAPIHugePages featuregate.Feature = "DownwardAPIHugePages" @@ -900,7 +901,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS DisableKubeletCloudCredentialProviders: {Default: false, PreRelease: featuregate.Alpha}, - DownwardAPIHugePages: {Default: true, PreRelease: featuregate.Beta}, // on by default in 1.22 + DownwardAPIHugePages: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in v1.29 EndpointSliceTerminatingCondition: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in v1.28