From c12aa0f6b7539c1a7bab8731ed68209b0f339907 Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Wed, 17 Feb 2021 08:27:02 +0200 Subject: [PATCH] promote HugePageStorageMediumSize to GA --- pkg/api/pod/util.go | 19 --------- pkg/apis/core/validation/validation.go | 28 ------------- pkg/apis/core/validation/validation_test.go | 46 ++++----------------- pkg/features/kube_features.go | 5 ++- pkg/kubelet/config/common.go | 15 ++----- pkg/volume/emptydir/empty_dir_test.go | 46 ++++++++------------- 6 files changed, 30 insertions(+), 129 deletions(-) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 725bd593553..6f45becf480 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -21,11 +21,9 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/sets" utilfeature "k8s.io/apiserver/pkg/util/feature" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/helper" - v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/features" ) @@ -331,19 +329,6 @@ func usesHugePagesInProjectedEnv(item api.Container) bool { return false } -// usesMultipleHugePageResources returns true if the pod spec uses more than -// one size of hugepage -func usesMultipleHugePageResources(podSpec *api.PodSpec) bool { - hugePageResources := sets.NewString() - resourceSet := helper.ToPodResourcesSet(podSpec) - for resourceStr := range resourceSet { - if v1helper.IsHugePageResourceName(v1.ResourceName(resourceStr)) { - hugePageResources.Insert(resourceStr) - } - } - return len(hugePageResources) > 1 -} - func checkContainerUseIndivisibleHugePagesValues(container api.Container) bool { for resourceName, quantity := range container.Resources.Limits { if helper.IsHugePageResourceName(resourceName) { @@ -394,8 +379,6 @@ func usesIndivisibleHugePagesValues(podSpec *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 multiple huge pages on pod create if feature is enabled - AllowMultipleHugePageResources: utilfeature.DefaultFeatureGate.Enabled(features.HugePageStorageMediumSize), // 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), @@ -404,8 +387,6 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po } if oldPodSpec != nil { - // if old spec used multiple huge page sizes, we must allow it - opts.AllowMultipleHugePageResources = opts.AllowMultipleHugePageResources || usesMultipleHugePageResources(oldPodSpec) // 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 diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 839c7b53582..360ff8b8167 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -48,7 +48,6 @@ import ( "k8s.io/kubernetes/pkg/apis/core/helper" podshelper "k8s.io/kubernetes/pkg/apis/core/pods" corev1 "k8s.io/kubernetes/pkg/apis/core/v1" - v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/capabilities" "k8s.io/kubernetes/pkg/cluster/ports" "k8s.io/kubernetes/pkg/features" @@ -3196,8 +3195,6 @@ func validateContainersOnlyForPod(containers []core.Container, fldPath *field.Pa // PodValidationOptions contains the different settings for pod validation type PodValidationOptions struct { - // Allow pod spec to have more than one huge page resource (with different sizes) - AllowMultipleHugePageResources bool // Allow pod spec to use hugepages in downward API AllowDownwardAPIHugePages bool // Allow invalid pod-deletion-cost annotation value for backward compatibility. @@ -3206,23 +3203,6 @@ type PodValidationOptions struct { AllowIndivisibleHugePagesValues bool } -// ValidatePodSingleHugePageResources checks if there are multiple huge -// pages resources in the pod object. -func ValidatePodSingleHugePageResources(pod *core.Pod, specPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} - resourceSet := helper.ToPodResourcesSet(&pod.Spec) - hugePageResources := sets.NewString() - for resourceStr := range resourceSet { - if v1helper.IsHugePageResourceName(v1.ResourceName(resourceStr)) { - hugePageResources.Insert(resourceStr) - } - } - if len(hugePageResources) > 1 { - allErrs = append(allErrs, field.Invalid(specPath, hugePageResources.List(), "must use a single hugepage size in a pod spec")) - } - return allErrs -} - // validatePodMetadataAndSpec tests if required fields in the pod.metadata and pod.spec are set, // and is called by ValidatePodCreate and ValidatePodUpdate. func validatePodMetadataAndSpec(pod *core.Pod, opts PodValidationOptions) field.ErrorList { @@ -3252,10 +3232,6 @@ func validatePodMetadataAndSpec(pod *core.Pod, opts PodValidationOptions) field. allErrs = append(allErrs, validateContainersOnlyForPod(pod.Spec.Containers, specPath.Child("containers"))...) allErrs = append(allErrs, validateContainersOnlyForPod(pod.Spec.InitContainers, specPath.Child("initContainers"))...) - if !opts.AllowMultipleHugePageResources { - allErrs = append(allErrs, ValidatePodSingleHugePageResources(pod, specPath)...) - } - return allErrs } @@ -3952,10 +3928,6 @@ func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel allErrs = append(allErrs, ValidatePodSpecificAnnotationUpdates(newPod, oldPod, fldPath.Child("annotations"), opts)...) specPath := field.NewPath("spec") - if !opts.AllowMultipleHugePageResources { - allErrs = append(allErrs, ValidatePodSingleHugePageResources(newPod, specPath)...) - } - // validate updateable fields: // 1. spec.containers[*].image // 2. spec.initContainers[*].image diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index d84a206eef4..13e5732f232 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -4113,9 +4113,8 @@ func TestValidateVolumes(t *testing.T) { func TestHugePagesIsolation(t *testing.T) { testCases := map[string]struct { - pod *core.Pod - enableHugePageStorageMediumSize bool - expectError bool + pod *core.Pod + expectError bool }{ "Valid: request hugepages-2Mi": { pod: &core.Pod{ @@ -4143,7 +4142,7 @@ func TestHugePagesIsolation(t *testing.T) { }, }, }, - "Valid: HugePageStorageMediumSize enabled: request more than one hugepages size": { + "Valid: request more than one hugepages size": { pod: &core.Pod{ ObjectMeta: metav1.ObjectMeta{Name: "hugepages-shared", Namespace: "ns"}, Spec: core.PodSpec{ @@ -4170,10 +4169,9 @@ func TestHugePagesIsolation(t *testing.T) { DNSPolicy: core.DNSClusterFirst, }, }, - enableHugePageStorageMediumSize: true, - expectError: false, + expectError: false, }, - "Invalid: HugePageStorageMediumSize disabled: request hugepages-1Gi, limit hugepages-2Mi and hugepages-1Gi": { + "Valid: request hugepages-1Gi, limit hugepages-2Mi and hugepages-1Gi": { pod: &core.Pod{ ObjectMeta: metav1.ObjectMeta{Name: "hugepages-multiple", Namespace: "ns"}, Spec: core.PodSpec{ @@ -4184,6 +4182,7 @@ func TestHugePagesIsolation(t *testing.T) { Requests: core.ResourceList{ core.ResourceName(core.ResourceCPU): resource.MustParse("10"), core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), + core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), }, Limits: core.ResourceList{ @@ -4199,7 +4198,6 @@ func TestHugePagesIsolation(t *testing.T) { DNSPolicy: core.DNSClusterFirst, }, }, - expectError: true, }, "Invalid: not requesting cpu and memory": { pod: &core.Pod{ @@ -4251,40 +4249,10 @@ func TestHugePagesIsolation(t *testing.T) { }, expectError: true, }, - "Invalid: HugePageStorageMediumSize disabled: request more than one hugepages size": { - pod: &core.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: "hugepages-shared", Namespace: "ns"}, - Spec: core.PodSpec{ - Containers: []core.Container{ - { - Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", - Resources: core.ResourceRequirements{ - Requests: core.ResourceList{ - core.ResourceName(core.ResourceCPU): resource.MustParse("10"), - core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), - core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), - core.ResourceName("hugepages-1Gi"): resource.MustParse("1Gi"), - }, - Limits: core.ResourceList{ - core.ResourceName(core.ResourceCPU): resource.MustParse("10"), - core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), - core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), - core.ResourceName("hugepages-1Gi"): resource.MustParse("1Gi"), - }, - }, - }, - }, - RestartPolicy: core.RestartPolicyAlways, - DNSPolicy: core.DNSClusterFirst, - }, - }, - expectError: true, - }, } for tcName, tc := range testCases { t.Run(tcName, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HugePageStorageMediumSize, tc.enableHugePageStorageMediumSize)() - errs := ValidatePodCreate(tc.pod, PodValidationOptions{AllowMultipleHugePageResources: tc.enableHugePageStorageMediumSize}) + errs := ValidatePodCreate(tc.pod, PodValidationOptions{}) if tc.expectError && len(errs) == 0 { t.Errorf("Unexpected success") } diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index bb2c3446471..2c24d6836dd 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -530,6 +530,7 @@ const ( // owner: @bart0sh // alpha: v1.18 // beta: v1.19 + // GA: 1.22 // // Enables usage of HugePages- in a volume medium, // e.g. emptyDir: @@ -799,8 +800,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS ServiceTopology: {Default: false, PreRelease: featuregate.Alpha}, ServiceAppProtocol: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, ImmutableEphemeralVolumes: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.24 - HugePageStorageMediumSize: {Default: true, PreRelease: featuregate.Beta}, - DownwardAPIHugePages: {Default: false, PreRelease: featuregate.Beta}, // on by default in 1.22 + HugePageStorageMediumSize: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.23 + DownwardAPIHugePages: {Default: false, PreRelease: featuregate.Beta}, // on by default in 1.22 AnyVolumeDataSource: {Default: false, PreRelease: featuregate.Alpha}, DefaultPodTopologySpread: {Default: true, PreRelease: featuregate.Beta}, SetHostnameAsFQDN: {Default: true, PreRelease: featuregate.Beta}, diff --git a/pkg/kubelet/config/common.go b/pkg/kubelet/config/common.go index 20456622a52..4dd515d33fe 100644 --- a/pkg/kubelet/config/common.go +++ b/pkg/kubelet/config/common.go @@ -22,15 +22,13 @@ import ( "fmt" "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" utilyaml "k8s.io/apimachinery/pkg/util/yaml" - utilfeature "k8s.io/apiserver/pkg/util/feature" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/helper" - "k8s.io/kubernetes/pkg/features" // TODO: remove this import if // api.Registry.GroupOrDie(v1.GroupName).GroupVersion.String() is changed @@ -137,10 +135,7 @@ func tryDecodeSinglePod(data []byte, defaultFn defaultFunc) (parsed bool, pod *v if err = defaultFn(newPod); err != nil { return true, pod, err } - opts := validation.PodValidationOptions{ - AllowMultipleHugePageResources: utilfeature.DefaultFeatureGate.Enabled(features.HugePageStorageMediumSize), - } - if errs := validation.ValidatePodCreate(newPod, opts); len(errs) > 0 { + if errs := validation.ValidatePodCreate(newPod, validation.PodValidationOptions{}); len(errs) > 0 { return true, pod, fmt.Errorf("invalid pod: %v", errs) } v1Pod := &v1.Pod{} @@ -164,17 +159,13 @@ func tryDecodePodList(data []byte, defaultFn defaultFunc) (parsed bool, pods v1. return false, pods, err } - opts := validation.PodValidationOptions{ - AllowMultipleHugePageResources: utilfeature.DefaultFeatureGate.Enabled(features.HugePageStorageMediumSize), - } - // Apply default values and validate pods. for i := range newPods.Items { newPod := &newPods.Items[i] if err = defaultFn(newPod); err != nil { return true, pods, err } - if errs := validation.ValidatePodCreate(newPod, opts); len(errs) > 0 { + if errs := validation.ValidatePodCreate(newPod, validation.PodValidationOptions{}); len(errs) > 0 { err = fmt.Errorf("invalid pod: %v", errs) return true, pods, err } diff --git a/pkg/volume/emptydir/empty_dir_test.go b/pkg/volume/emptydir/empty_dir_test.go index e62b54b7b5a..8625d5d2f4c 100644 --- a/pkg/volume/emptydir/empty_dir_test.go +++ b/pkg/volume/emptydir/empty_dir_test.go @@ -108,24 +108,17 @@ func TestPluginEmptyRootContext(t *testing.T) { func TestPluginHugetlbfs(t *testing.T) { testCases := map[string]struct { - medium v1.StorageMedium - enableHugePageStorageMediumSize bool + medium v1.StorageMedium }{ - "HugePageStorageMediumSize enabled: medium without size": { - medium: "HugePages", - enableHugePageStorageMediumSize: true, - }, - "HugePageStorageMediumSize disabled: medium without size": { + "medium without size": { medium: "HugePages", }, - "HugePageStorageMediumSize enabled: medium with size": { - medium: "HugePages-2Mi", - enableHugePageStorageMediumSize: true, + "medium with size": { + medium: "HugePages-2Mi", }, } for tcName, tc := range testCases { t.Run(tcName, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HugePageStorageMediumSize, tc.enableHugePageStorageMediumSize)() doTestPlugin(t, pluginTestConfig{ medium: tc.medium, expectedSetupMounts: 1, @@ -381,11 +374,10 @@ func TestMetrics(t *testing.T) { func TestGetHugePagesMountOptions(t *testing.T) { testCases := map[string]struct { - pod *v1.Pod - medium v1.StorageMedium - shouldFail bool - expectedResult string - enableHugePageStorageMediumSize bool + pod *v1.Pod + medium v1.StorageMedium + shouldFail bool + expectedResult string }{ "ProperValues": { pod: &v1.Pod{ @@ -501,10 +493,9 @@ func TestGetHugePagesMountOptions(t *testing.T) { }, }, }, - medium: v1.StorageMediumHugePages, - shouldFail: true, - expectedResult: "", - enableHugePageStorageMediumSize: true, + medium: v1.StorageMediumHugePages, + shouldFail: true, + expectedResult: "", }, "PodWithNoHugePagesRequest": { pod: &v1.Pod{}, @@ -527,10 +518,9 @@ func TestGetHugePagesMountOptions(t *testing.T) { }, }, }, - medium: v1.StorageMediumHugePagesPrefix + "1Gi", - shouldFail: false, - expectedResult: "pagesize=1Gi", - enableHugePageStorageMediumSize: true, + medium: v1.StorageMediumHugePagesPrefix + "1Gi", + shouldFail: false, + expectedResult: "pagesize=1Gi", }, "InitContainerAndContainerHasProperValuesMultipleSizes": { pod: &v1.Pod{ @@ -555,10 +545,9 @@ func TestGetHugePagesMountOptions(t *testing.T) { }, }, }, - medium: v1.StorageMediumHugePagesPrefix + "2Mi", - shouldFail: false, - expectedResult: "pagesize=2Mi", - enableHugePageStorageMediumSize: true, + medium: v1.StorageMediumHugePagesPrefix + "2Mi", + shouldFail: false, + expectedResult: "pagesize=2Mi", }, "MediumWithoutSizeMultipleSizes": { pod: &v1.Pod{ @@ -621,7 +610,6 @@ func TestGetHugePagesMountOptions(t *testing.T) { for testCaseName, testCase := range testCases { t.Run(testCaseName, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HugePageStorageMediumSize, testCase.enableHugePageStorageMediumSize)() value, err := getPageSizeMountOption(testCase.medium, testCase.pod) if testCase.shouldFail && err == nil { t.Errorf("%s: Unexpected success", testCaseName)