diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 6e69d886901..868b0168769 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -583,9 +583,10 @@ type StorageMedium string // These are the valid value for StorageMedium const ( - StorageMediumDefault StorageMedium = "" // use whatever the default is for the node - StorageMediumMemory StorageMedium = "Memory" // use memory (tmpfs) - StorageMediumHugePages StorageMedium = "HugePages" // use hugepages + StorageMediumDefault StorageMedium = "" // use whatever the default is for the node + StorageMediumMemory StorageMedium = "Memory" // use memory (tmpfs) + StorageMediumHugePages StorageMedium = "HugePages" // use hugepages + StorageMediumHugePagesPrefix StorageMedium = "HugePages-" // prefix for full medium notation HugePages- ) // Protocol defines network protocols supported for things like container ports. diff --git a/pkg/apis/core/v1/helper/helpers.go b/pkg/apis/core/v1/helper/helpers.go index a930520f1b1..ac3018c9992 100644 --- a/pkg/apis/core/v1/helper/helpers.go +++ b/pkg/apis/core/v1/helper/helpers.go @@ -103,6 +103,27 @@ func HugePageUnitSizeFromByteSize(size int64) (string, error) { return fmt.Sprintf("%d%s", size, hugePageSizeUnitList[idx]), nil } +// IsHugePageMedium returns true if the volume medium is in 'HugePages[-size]' format +func IsHugePageMedium(medium v1.StorageMedium) bool { + if medium == v1.StorageMediumHugePages { + return true + } + return strings.HasPrefix(string(medium), string(v1.StorageMediumHugePagesPrefix)) +} + +// HugePageSizeFromMedium returns the page size for the specified huge page medium. +// If the specified input is not a valid huge page medium an error is returned. +func HugePageSizeFromMedium(medium v1.StorageMedium) (resource.Quantity, error) { + if !IsHugePageMedium(medium) { + return resource.Quantity{}, fmt.Errorf("medium: %s is not a hugepage medium", medium) + } + if medium == v1.StorageMediumHugePages { + return resource.Quantity{}, fmt.Errorf("medium: %s doesn't have size information", medium) + } + pageSize := strings.TrimPrefix(string(medium), string(v1.StorageMediumHugePagesPrefix)) + return resource.ParseQuantity(pageSize) +} + // IsOvercommitAllowed returns true if the resource is in the default // namespace and is not hugepages. func IsOvercommitAllowed(name v1.ResourceName) bool { diff --git a/pkg/apis/core/v1/helper/helpers_test.go b/pkg/apis/core/v1/helper/helpers_test.go index 27d5b157903..c418e82b4fb 100644 --- a/pkg/apis/core/v1/helper/helpers_test.go +++ b/pkg/apis/core/v1/helper/helpers_test.go @@ -113,6 +113,73 @@ func TestHugePageSizeFromResourceName(t *testing.T) { } } +func TestHugePageSizeFromMedium(t *testing.T) { + testCases := []struct { + description string + medium v1.StorageMedium + expectVal resource.Quantity + expectErr bool + }{ + { + description: "Invalid hugepages medium", + medium: "Memory", + expectVal: resource.Quantity{}, + expectErr: true, + }, + { + description: "Invalid hugepages medium", + medium: "Memory", + expectVal: resource.Quantity{}, + expectErr: true, + }, + { + description: "Invalid: HugePages without size", + medium: "HugePages", + expectVal: resource.Quantity{}, + expectErr: true, + }, + { + description: "Invalid: HugePages without size", + medium: "HugePages", + expectVal: resource.Quantity{}, + expectErr: true, + }, + { + description: "Valid: HugePages-1Gi", + medium: "HugePages-1Gi", + expectVal: resource.MustParse("1Gi"), + expectErr: false, + }, + { + description: "Valid: HugePages-2Mi", + medium: "HugePages-2Mi", + expectVal: resource.MustParse("2Mi"), + expectErr: false, + }, + { + description: "Valid: HugePages-64Ki", + medium: "HugePages-64Ki", + expectVal: resource.MustParse("64Ki"), + expectErr: false, + }, + } + for i, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + t.Parallel() + v, err := HugePageSizeFromMedium(tc.medium) + if err == nil && tc.expectErr { + t.Errorf("[%v]expected error but got none.", i) + } + if err != nil && !tc.expectErr { + t.Errorf("[%v]did not expect error but got: %v", i, err) + } + if v != tc.expectVal { + t.Errorf("Got %v but expected %v", v, tc.expectVal) + } + }) + } +} + func TestIsOvercommitAllowed(t *testing.T) { testCases := []struct { resourceName v1.ResourceName diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index cec7ebf1d32..6c13172208c 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -3083,8 +3083,33 @@ func validateContainersOnlyForPod(containers []core.Container, fldPath *field.Pa return allErrs } +// 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 +} + +// 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{} + hugePageResources := sets.NewString() + for i := range pod.Spec.Containers { + resourceSet := toContainerResourcesSet(&pod.Spec.Containers[i]) + 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 +} + // ValidatePod tests if required fields in the pod are set. -func ValidatePod(pod *core.Pod) field.ErrorList { +func ValidatePod(pod *core.Pod, opts PodValidationOptions) field.ErrorList { fldPath := field.NewPath("metadata") allErrs := ValidateObjectMeta(&pod.ObjectMeta, true, ValidatePodName, fldPath) allErrs = append(allErrs, ValidatePodSpecificAnnotations(pod.ObjectMeta.Annotations, &pod.Spec, fldPath.Child("annotations"))...) @@ -3111,17 +3136,8 @@ func ValidatePod(pod *core.Pod) field.ErrorList { allErrs = append(allErrs, validateContainersOnlyForPod(pod.Spec.Containers, specPath.Child("containers"))...) allErrs = append(allErrs, validateContainersOnlyForPod(pod.Spec.InitContainers, specPath.Child("initContainers"))...) - hugePageResources := sets.NewString() - for i := range pod.Spec.Containers { - resourceSet := toContainerResourcesSet(&pod.Spec.Containers[i]) - for resourceStr := range resourceSet { - if v1helper.IsHugePageResourceName(v1.ResourceName(resourceStr)) { - hugePageResources.Insert(resourceStr) - } - } - } - if len(hugePageResources) > 1 { - allErrs = append(allErrs, field.Invalid(specPath, hugePageResources, "must use a single hugepage size in a pod spec")) + if !opts.AllowMultipleHugePageResources { + allErrs = append(allErrs, ValidatePodSingleHugePageResources(pod, specPath)...) } podIPsField := field.NewPath("status", "podIPs") @@ -3679,8 +3695,8 @@ func ValidateContainerUpdates(newContainers, oldContainers []core.Container, fld } // ValidatePodCreate validates a pod in the context of its initial create -func ValidatePodCreate(pod *core.Pod) field.ErrorList { - allErrs := ValidatePod(pod) +func ValidatePodCreate(pod *core.Pod, opts PodValidationOptions) field.ErrorList { + allErrs := ValidatePod(pod, opts) fldPath := field.NewPath("spec") // EphemeralContainers can only be set on update using the ephemeralcontainers subresource @@ -3693,12 +3709,16 @@ func ValidatePodCreate(pod *core.Pod) field.ErrorList { // ValidatePodUpdate tests to see if the update is legal for an end user to make. newPod is updated with fields // that cannot be changed. -func ValidatePodUpdate(newPod, oldPod *core.Pod) field.ErrorList { +func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) field.ErrorList { fldPath := field.NewPath("metadata") allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath) allErrs = append(allErrs, ValidatePodSpecificAnnotationUpdates(newPod, oldPod, fldPath.Child("annotations"))...) 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 1e44306a4d9..bfb1cc2026d 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -24,7 +24,7 @@ import ( "strings" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" @@ -3908,114 +3908,186 @@ func TestValidateVolumes(t *testing.T) { } func TestHugePagesIsolation(t *testing.T) { - successCases := []core.Pod{ - { // Basic fields. - ObjectMeta: metav1.ObjectMeta{Name: "123", 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"), - }, - Limits: core.ResourceList{ - core.ResourceName(core.ResourceCPU): resource.MustParse("10"), - core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), - core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), + testCases := map[string]struct { + pod *core.Pod + enableHugePageStorageMediumSize bool + expectError bool + }{ + "Valid: request hugepages-2Mi": { + pod: &core.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "123", 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"), + }, + Limits: core.ResourceList{ + core.ResourceName(core.ResourceCPU): resource.MustParse("10"), + core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), + core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), + }, }, }, }, + RestartPolicy: core.RestartPolicyAlways, + DNSPolicy: core.DNSClusterFirst, }, - RestartPolicy: core.RestartPolicyAlways, - DNSPolicy: core.DNSClusterFirst, }, }, + "Valid: HugePageStorageMediumSize enabled: 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("2Gi"), + }, + 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("2Gi"), + }, + }, + }, + }, + RestartPolicy: core.RestartPolicyAlways, + DNSPolicy: core.DNSClusterFirst, + }, + }, + enableHugePageStorageMediumSize: true, + expectError: false, + }, + "Invalid: HugePageStorageMediumSize disabled: request hugepages-1Gi, limit hugepages-2Mi and hugepages-1Gi": { + pod: &core.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "hugepages-multiple", 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-1Gi"): resource.MustParse("2Gi"), + }, + 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("2Gi"), + }, + }, + }, + }, + RestartPolicy: core.RestartPolicyAlways, + DNSPolicy: core.DNSClusterFirst, + }, + }, + expectError: true, + }, + "Invalid: not requesting cpu and memory": { + pod: &core.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "hugepages-requireCpuOrMemory", Namespace: "ns"}, + Spec: core.PodSpec{ + Containers: []core.Container{ + { + Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), + }, + Limits: core.ResourceList{ + core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), + }, + }, + }, + }, + RestartPolicy: core.RestartPolicyAlways, + DNSPolicy: core.DNSClusterFirst, + }, + }, + expectError: true, + }, + "Invalid: request 1Gi hugepages-2Mi but limit 2Gi": { + 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"), + }, + Limits: core.ResourceList{ + core.ResourceName(core.ResourceCPU): resource.MustParse("10"), + core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), + core.ResourceName("hugepages-2Mi"): resource.MustParse("2Gi"), + }, + }, + }, + }, + RestartPolicy: core.RestartPolicyAlways, + DNSPolicy: core.DNSClusterFirst, + }, + }, + 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, + }, } - failureCases := []core.Pod{ - { // Basic fields. - ObjectMeta: metav1.ObjectMeta{Name: "hugepages-requireCpuOrMemory", Namespace: "ns"}, - Spec: core.PodSpec{ - Containers: []core.Container{ - { - Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", - Resources: core.ResourceRequirements{ - Requests: core.ResourceList{ - core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), - }, - Limits: core.ResourceList{ - core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), - }, - }, - }, - }, - RestartPolicy: core.RestartPolicyAlways, - DNSPolicy: core.DNSClusterFirst, - }, - }, - { // Basic fields. - 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"), - }, - Limits: core.ResourceList{ - core.ResourceName(core.ResourceCPU): resource.MustParse("10"), - core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), - core.ResourceName("hugepages-2Mi"): resource.MustParse("2Gi"), - }, - }, - }, - }, - RestartPolicy: core.RestartPolicyAlways, - DNSPolicy: core.DNSClusterFirst, - }, - }, - { // Basic fields. - ObjectMeta: metav1.ObjectMeta{Name: "hugepages-multiple", 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-1Gi"): resource.MustParse("2Gi"), - }, - 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("2Gi"), - }, - }, - }, - }, - RestartPolicy: core.RestartPolicyAlways, - DNSPolicy: core.DNSClusterFirst, - }, - }, - } - for i := range successCases { - pod := &successCases[i] - if errs := ValidatePod(pod); len(errs) != 0 { - t.Errorf("Unexpected error for case[%d], err: %v", i, errs) - } - } - for i := range failureCases { - pod := &failureCases[i] - if errs := ValidatePod(pod); len(errs) == 0 { - t.Errorf("Expected error for case[%d], pod: %v", i, pod.Name) - } + for tcName, tc := range testCases { + t.Run(tcName, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HugePageStorageMediumSize, tc.enableHugePageStorageMediumSize)() + errs := ValidatePod(tc.pod, PodValidationOptions{tc.enableHugePageStorageMediumSize}) + if tc.expectError && len(errs) == 0 { + t.Errorf("Unexpected success") + } + if !tc.expectError && len(errs) != 0 { + t.Errorf("Unexpected error(s): %v", errs) + } + }) } } @@ -7375,7 +7447,7 @@ func TestValidatePod(t *testing.T) { }, } for _, pod := range successCases { - if errs := ValidatePod(&pod); len(errs) != 0 { + if errs := ValidatePod(&pod, PodValidationOptions{}); len(errs) != 0 { t.Errorf("expected success: %v", errs) } } @@ -8225,7 +8297,7 @@ func TestValidatePod(t *testing.T) { }, } for k, v := range errorCases { - if errs := ValidatePod(&v.spec); len(errs) == 0 { + if errs := ValidatePod(&v.spec, PodValidationOptions{}); len(errs) == 0 { t.Errorf("expected failure for %q", k) } else if v.expectedError == "" { t.Errorf("missing expectedError for %q, got %q", k, errs.ToAggregate().Error()) @@ -8965,7 +9037,7 @@ func TestValidatePodUpdate(t *testing.T) { for _, test := range tests { test.new.ObjectMeta.ResourceVersion = "1" test.old.ObjectMeta.ResourceVersion = "1" - errs := ValidatePodUpdate(&test.new, &test.old) + errs := ValidatePodUpdate(&test.new, &test.old, PodValidationOptions{}) if test.err == "" { if len(errs) != 0 { t.Errorf("unexpected invalid: %s (%+v)\nA: %+v\nB: %+v", test.test, errs, test.new, test.old) @@ -14876,7 +14948,7 @@ func TestPodIPsValidation(t *testing.T) { } for _, testCase := range testCases { - errs := ValidatePod(&testCase.pod) + errs := ValidatePod(&testCase.pod, PodValidationOptions{}) if len(errs) == 0 && testCase.expectError { t.Errorf("expected failure for %s, but there were none", testCase.pod.Name) return diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index f32e2a2f144..c58e4f49b2a 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -540,6 +540,14 @@ const ( // // Enables a feature to make secrets and configmaps data immutable. ImmutableEphemeralVolumes featuregate.Feature = "ImmutableEphemeralVolumes" + + // owner: @bart0sh + // alpha: v1.18 + // + // Enables usage of HugePages- in a volume medium, + // e.g. emptyDir: + // medium: HugePages-1Gi + HugePageStorageMediumSize featuregate.Feature = "HugePageStorageMediumSize" ) func init() { @@ -625,6 +633,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS PodDisruptionBudget: {Default: true, PreRelease: featuregate.Beta}, ServiceTopology: {Default: false, PreRelease: featuregate.Alpha}, ImmutableEphemeralVolumes: {Default: false, PreRelease: featuregate.Alpha}, + HugePageStorageMediumSize: {Default: false, PreRelease: featuregate.Alpha}, // inherited features from generic apiserver, relisted here to get a conflict if it is changed // unintentionally on either side: diff --git a/pkg/kubelet/config/BUILD b/pkg/kubelet/config/BUILD index 25595bd3e9b..b34fb86f199 100644 --- a/pkg/kubelet/config/BUILD +++ b/pkg/kubelet/config/BUILD @@ -24,6 +24,7 @@ go_library( "//pkg/apis/core/install:go_default_library", "//pkg/apis/core/v1:go_default_library", "//pkg/apis/core/validation:go_default_library", + "//pkg/features:go_default_library", "//pkg/kubelet/checkpoint:go_default_library", "//pkg/kubelet/checkpointmanager:go_default_library", "//pkg/kubelet/container:go_default_library", @@ -40,6 +41,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/yaml:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/tools/cache:go_default_library", "//staging/src/k8s.io/client-go/tools/record:go_default_library", diff --git a/pkg/kubelet/config/common.go b/pkg/kubelet/config/common.go index cd557c28147..6ddf219b4f1 100644 --- a/pkg/kubelet/config/common.go +++ b/pkg/kubelet/config/common.go @@ -27,8 +27,10 @@ import ( "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 @@ -133,7 +135,10 @@ func tryDecodeSinglePod(data []byte, defaultFn defaultFunc) (parsed bool, pod *v if err = defaultFn(newPod); err != nil { return true, pod, err } - if errs := validation.ValidatePod(newPod); len(errs) > 0 { + opts := validation.PodValidationOptions{ + AllowMultipleHugePageResources: utilfeature.DefaultFeatureGate.Enabled(features.HugePageStorageMediumSize), + } + if errs := validation.ValidatePod(newPod, opts); len(errs) > 0 { return true, pod, fmt.Errorf("invalid pod: %v", errs) } v1Pod := &v1.Pod{} @@ -157,13 +162,17 @@ 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.ValidatePod(newPod); len(errs) > 0 { + if errs := validation.ValidatePod(newPod, opts); len(errs) > 0 { err = fmt.Errorf("invalid pod: %v", errs) return true, pods, err } diff --git a/pkg/kubelet/config/common_test.go b/pkg/kubelet/config/common_test.go index c220a14e504..b29b83d080f 100644 --- a/pkg/kubelet/config/common_test.go +++ b/pkg/kubelet/config/common_test.go @@ -251,7 +251,7 @@ func TestStaticPodNameGenerate(t *testing.T) { if c.overwrite != "" { pod.Name = c.overwrite } - errs := validation.ValidatePod(pod) + errs := validation.ValidatePod(pod, validation.PodValidationOptions{}) if c.shouldErr { specNameErrored := false for _, err := range errs { diff --git a/pkg/kubelet/config/file_linux_test.go b/pkg/kubelet/config/file_linux_test.go index ee9f1517b75..4c7a7ba8f34 100644 --- a/pkg/kubelet/config/file_linux_test.go +++ b/pkg/kubelet/config/file_linux_test.go @@ -90,7 +90,7 @@ func TestReadPodsFromFileExistAlready(t *testing.T) { if err := k8s_api_v1.Convert_v1_Pod_To_core_Pod(pod, internalPod, nil); err != nil { t.Fatalf("%s: Cannot convert pod %#v, %#v", testCase.desc, pod, err) } - if errs := validation.ValidatePod(internalPod); len(errs) > 0 { + if errs := validation.ValidatePod(internalPod, validation.PodValidationOptions{}); len(errs) > 0 { t.Fatalf("%s: Invalid pod %#v, %#v", testCase.desc, internalPod, errs) } } @@ -369,7 +369,7 @@ func expectUpdate(t *testing.T, ch chan interface{}, testCase *testCase) { if err := k8s_api_v1.Convert_v1_Pod_To_core_Pod(pod, internalPod, nil); err != nil { t.Fatalf("%s: Cannot convert pod %#v, %#v", testCase.desc, pod, err) } - if errs := validation.ValidatePod(internalPod); len(errs) > 0 { + if errs := validation.ValidatePod(internalPod, validation.PodValidationOptions{}); len(errs) > 0 { t.Fatalf("%s: Invalid pod %#v, %#v", testCase.desc, internalPod, errs) } } diff --git a/pkg/kubelet/config/http_test.go b/pkg/kubelet/config/http_test.go index 398dbc8e679..2778bfc535b 100644 --- a/pkg/kubelet/config/http_test.go +++ b/pkg/kubelet/config/http_test.go @@ -319,7 +319,7 @@ func TestExtractPodsFromHTTP(t *testing.T) { if err := k8s_api_v1.Convert_v1_Pod_To_core_Pod(pod, internalPod, nil); err != nil { t.Fatalf("%s: Cannot convert pod %#v, %#v", testCase.desc, pod, err) } - if errs := validation.ValidatePod(internalPod); len(errs) != 0 { + if errs := validation.ValidatePod(internalPod, validation.PodValidationOptions{}); len(errs) != 0 { t.Errorf("%s: Expected no validation errors on %#v, Got %v", testCase.desc, pod, errs.ToAggregate()) } } diff --git a/pkg/registry/core/pod/strategy.go b/pkg/registry/core/pod/strategy.go index 917769e400c..2f9e8235685 100644 --- a/pkg/registry/core/pod/strategy.go +++ b/pkg/registry/core/pod/strategy.go @@ -88,7 +88,11 @@ func (podStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object // Validate validates a new pod. func (podStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { pod := obj.(*api.Pod) - allErrs := validation.ValidatePodCreate(pod) + opts := validation.PodValidationOptions{ + // Allow multiple huge pages on pod create if feature is enabled + AllowMultipleHugePageResources: utilfeature.DefaultFeatureGate.Enabled(features.HugePageStorageMediumSize), + } + allErrs := validation.ValidatePodCreate(pod, opts) allErrs = append(allErrs, validation.ValidateConditionalPod(pod, nil, field.NewPath(""))...) return allErrs } @@ -104,8 +108,13 @@ func (podStrategy) AllowCreateOnUpdate() bool { // ValidateUpdate is the default update validation for an end user. func (podStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - errorList := validation.ValidatePod(obj.(*api.Pod)) - errorList = append(errorList, validation.ValidatePodUpdate(obj.(*api.Pod), old.(*api.Pod))...) + oldFailsSingleHugepagesValidation := len(validation.ValidatePodSingleHugePageResources(old.(*api.Pod), field.NewPath("spec"))) > 0 + opts := validation.PodValidationOptions{ + // Allow multiple huge pages on pod create if feature is enabled or if the old pod already has multiple hugepages specified + AllowMultipleHugePageResources: oldFailsSingleHugepagesValidation || utilfeature.DefaultFeatureGate.Enabled(features.HugePageStorageMediumSize), + } + errorList := validation.ValidatePod(obj.(*api.Pod), opts) + errorList = append(errorList, validation.ValidatePodUpdate(obj.(*api.Pod), old.(*api.Pod), opts)...) errorList = append(errorList, validation.ValidateConditionalPod(obj.(*api.Pod), old.(*api.Pod), field.NewPath(""))...) return errorList } diff --git a/pkg/volume/emptydir/BUILD b/pkg/volume/emptydir/BUILD index c4a58fda540..b220a50107e 100644 --- a/pkg/volume/emptydir/BUILD +++ b/pkg/volume/emptydir/BUILD @@ -44,6 +44,7 @@ go_test( embed = [":go_default_library"], deps = select({ "@io_bazel_rules_go//go/platform:android": [ + "//pkg/features:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/testing:go_default_library", "//pkg/volume/util:go_default_library", @@ -51,10 +52,13 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/util/testing:go_default_library", + "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//vendor/k8s.io/utils/mount:go_default_library", ], "@io_bazel_rules_go//go/platform:linux": [ + "//pkg/features:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/testing:go_default_library", "//pkg/volume/util:go_default_library", @@ -62,7 +66,9 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/util/testing:go_default_library", + "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//vendor/k8s.io/utils/mount:go_default_library", ], "//conditions:default": [], diff --git a/pkg/volume/emptydir/empty_dir.go b/pkg/volume/emptydir/empty_dir.go index a890a34802c..2ab16a93636 100644 --- a/pkg/volume/emptydir/empty_dir.go +++ b/pkg/volume/emptydir/empty_dir.go @@ -25,7 +25,7 @@ import ( "k8s.io/utils/mount" utilstrings "k8s.io/utils/strings" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -160,7 +160,7 @@ type mountDetector interface { // returns (v1.StorageMediumMemory, false, nil), the caller knows that the path is // on a memory FS (tmpfs on Linux) but is not the root mountpoint of // that tmpfs. - GetMountMedium(path string) (v1.StorageMedium, bool, error) + GetMountMedium(path string, requestedMedium v1.StorageMedium) (v1.StorageMedium, bool, *resource.Quantity, error) } // EmptyDir volumes are temporary directories exposed to the pod. @@ -216,12 +216,12 @@ func (ed *emptyDir) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { } } - switch ed.medium { - case v1.StorageMediumDefault: + switch { + case ed.medium == v1.StorageMediumDefault: err = ed.setupDir(dir) - case v1.StorageMediumMemory: + case ed.medium == v1.StorageMediumMemory: err = ed.setupTmpfs(dir) - case v1.StorageMediumHugePages: + case v1helper.IsHugePageMedium(ed.medium): err = ed.setupHugepages(dir) default: err = fmt.Errorf("unknown storage medium %q", ed.medium) @@ -261,7 +261,7 @@ func (ed *emptyDir) setupTmpfs(dir string) error { return err } // Make SetUp idempotent. - medium, isMnt, err := ed.mountDetector.GetMountMedium(dir) + medium, isMnt, _, err := ed.mountDetector.GetMountMedium(dir, ed.medium) if err != nil { return err } @@ -284,17 +284,34 @@ func (ed *emptyDir) setupHugepages(dir string) error { return err } // Make SetUp idempotent. - medium, isMnt, err := ed.mountDetector.GetMountMedium(dir) + medium, isMnt, mountPageSize, err := ed.mountDetector.GetMountMedium(dir, ed.medium) + klog.V(3).Infof("pod %v: setupHugepages: medium: %s, isMnt: %v, dir: %s, err: %v", ed.pod.UID, medium, isMnt, dir, err) if err != nil { return err } - // If the directory is a mountpoint with medium hugepages, there is no - // work to do since we are already in the desired state. - if isMnt && medium == v1.StorageMediumHugePages { + // If the directory is a mountpoint with medium hugepages of the same page size, + // there is no work to do since we are already in the desired state. + if isMnt && v1helper.IsHugePageMedium(medium) { + // Medium is: Hugepages + if ed.medium == v1.StorageMediumHugePages { + return nil + } + if mountPageSize == nil { + return fmt.Errorf("pod %v: mounted dir %s pagesize is not determined", ed.pod.UID, dir) + } + // Medium is: Hugepages- + // Mounted page size and medium size must be equal + mediumSize, err := v1helper.HugePageSizeFromMedium(ed.medium) + if err != nil { + return err + } + if mountPageSize == nil || mediumSize.Cmp(*mountPageSize) != 0 { + return fmt.Errorf("pod %v: mounted dir %s pagesize '%s' and requested medium size '%s' differ", ed.pod.UID, dir, mountPageSize.String(), mediumSize.String()) + } return nil } - pageSizeMountOption, err := getPageSizeMountOptionFromPod(ed.pod) + pageSizeMountOption, err := getPageSizeMountOption(ed.medium, ed.pod) if err != nil { return err } @@ -303,33 +320,52 @@ func (ed *emptyDir) setupHugepages(dir string) error { return ed.mounter.Mount("nodev", dir, "hugetlbfs", []string{pageSizeMountOption}) } -// getPageSizeMountOptionFromPod retrieves pageSize mount option from Pod's resources -// and validates pageSize options in all containers of given Pod. -func getPageSizeMountOptionFromPod(pod *v1.Pod) (string, error) { +// getPageSizeMountOption retrieves pageSize mount option from Pod's resources +// and medium and validates pageSize options in all containers of given Pod. +func getPageSizeMountOption(medium v1.StorageMedium, pod *v1.Pod) (string, error) { pageSizeFound := false pageSize := resource.Quantity{} - // In some rare cases init containers can also consume Huge pages. - containers := append(pod.Spec.Containers, pod.Spec.InitContainers...) - for _, container := range containers { + + var mediumPageSize resource.Quantity + if medium != v1.StorageMediumHugePages { + // medium is: Hugepages- + var err error + mediumPageSize, err = v1helper.HugePageSizeFromMedium(medium) + if err != nil { + return "", err + } + } + + // In some rare cases init containers can also consume Huge pages + for _, container := range append(pod.Spec.Containers, pod.Spec.InitContainers...) { // We can take request because limit and requests must match. for requestName := range container.Resources.Requests { - if v1helper.IsHugePageResourceName(requestName) { - currentPageSize, err := v1helper.HugePageSizeFromResourceName(requestName) - if err != nil { - return "", err - } - // PageSize for all volumes in a POD are equal, except for the first one discovered. + if !v1helper.IsHugePageResourceName(requestName) { + continue + } + currentPageSize, err := v1helper.HugePageSizeFromResourceName(requestName) + if err != nil { + return "", err + } + if medium == v1.StorageMediumHugePages { // medium is: Hugepages, size is not specified + // PageSize for all volumes in a POD must be equal if medium is "Hugepages" if pageSizeFound && pageSize.Cmp(currentPageSize) != 0 { - return "", fmt.Errorf("multiple pageSizes for huge pages in a single PodSpec") + return "", fmt.Errorf("medium: %s can't be used if container requests multiple huge page sizes", medium) } - pageSize = currentPageSize + pageSizeFound = true + pageSize = currentPageSize + } else { // medium is: Hugepages- + if currentPageSize.Cmp(mediumPageSize) == 0 { + pageSizeFound = true + pageSize = currentPageSize + } } } } if !pageSizeFound { - return "", fmt.Errorf("hugePages storage requested, but there is no resource request for huge pages") + return "", fmt.Errorf("medium %s: hugePages storage requested, but there is no resource request for huge pages", medium) } return fmt.Sprintf("%s=%s", hugePagesPageSizeMountOption, pageSize.String()), nil @@ -393,7 +429,7 @@ func (ed *emptyDir) TearDownAt(dir string) error { } // Figure out the medium. - medium, isMnt, err := ed.mountDetector.GetMountMedium(dir) + medium, isMnt, _, err := ed.mountDetector.GetMountMedium(dir, ed.medium) if err != nil { return err } diff --git a/pkg/volume/emptydir/empty_dir_linux.go b/pkg/volume/emptydir/empty_dir_linux.go index 93b48e27dc6..63a25dc4ed0 100644 --- a/pkg/volume/emptydir/empty_dir_linux.go +++ b/pkg/volume/emptydir/empty_dir_linux.go @@ -20,12 +20,14 @@ package emptydir import ( "fmt" + "strings" "golang.org/x/sys/unix" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/klog" "k8s.io/utils/mount" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" ) // Defined by Linux - the type number for tmpfs mounts. @@ -39,22 +41,71 @@ type realMountDetector struct { mounter mount.Interface } -func (m *realMountDetector) GetMountMedium(path string) (v1.StorageMedium, bool, error) { +// getPageSize obtains page size from the 'pagesize' mount option of the +// mounted volume +func getPageSize(path string, mounter mount.Interface) (*resource.Quantity, error) { + // Get mount point data for the path + mountPoints, err := mounter.List() + if err != nil { + return nil, fmt.Errorf("error listing mount points: %v", err) + } + // Find mount point for the path + mountPoint, err := func(mps []mount.MountPoint, mpPath string) (*mount.MountPoint, error) { + for _, mp := range mps { + if mp.Path == mpPath { + return &mp, nil + } + } + return nil, fmt.Errorf("mount point for %s not found", mpPath) + }(mountPoints, path) + if err != nil { + return nil, err + } + // Get page size from the 'pagesize' option value + for _, opt := range mountPoint.Opts { + opt = strings.TrimSpace(opt) + prefix := "pagesize=" + if strings.HasPrefix(opt, prefix) { + // NOTE: Adding suffix 'i' as result should be comparable with a medium size. + // pagesize mount option is specified without a suffix, + // e.g. pagesize=2M or pagesize=1024M for x86 CPUs + pageSize, err := resource.ParseQuantity(strings.TrimPrefix(opt, prefix) + "i") + if err != nil { + return nil, fmt.Errorf("error getting page size from '%s' mount option: %v", opt, err) + } + return &pageSize, nil + } + } + return nil, fmt.Errorf("no pagesize option specified for %s mount", mountPoint.Path) +} + +func (m *realMountDetector) GetMountMedium(path string, requestedMedium v1.StorageMedium) (v1.StorageMedium, bool, *resource.Quantity, error) { klog.V(5).Infof("Determining mount medium of %v", path) notMnt, err := m.mounter.IsLikelyNotMountPoint(path) if err != nil { - return v1.StorageMediumDefault, false, fmt.Errorf("IsLikelyNotMountPoint(%q): %v", path, err) - } - buf := unix.Statfs_t{} - if err := unix.Statfs(path, &buf); err != nil { - return v1.StorageMediumDefault, false, fmt.Errorf("statfs(%q): %v", path, err) + return v1.StorageMediumDefault, false, nil, fmt.Errorf("IsLikelyNotMountPoint(%q): %v", path, err) } - klog.V(5).Infof("Statfs_t of %v: %+v", path, buf) - if buf.Type == linuxTmpfsMagic { - return v1.StorageMediumMemory, !notMnt, nil - } else if int64(buf.Type) == linuxHugetlbfsMagic { - return v1.StorageMediumHugePages, !notMnt, nil + buf := unix.Statfs_t{} + if err := unix.Statfs(path, &buf); err != nil { + return v1.StorageMediumDefault, false, nil, fmt.Errorf("statfs(%q): %v", path, err) } - return v1.StorageMediumDefault, !notMnt, nil + + klog.V(3).Infof("Statfs_t of %v: %+v", path, buf) + + if buf.Type == linuxTmpfsMagic { + return v1.StorageMediumMemory, !notMnt, nil, nil + } else if int64(buf.Type) == linuxHugetlbfsMagic { + // Skip page size detection if requested medium doesn't have size specified + if requestedMedium == v1.StorageMediumHugePages { + return v1.StorageMediumHugePages, !notMnt, nil, nil + } + // Get page size for the volume mount + pageSize, err := getPageSize(path, m.mounter) + if err != nil { + return v1.StorageMediumHugePages, !notMnt, nil, err + } + return v1.StorageMediumHugePages, !notMnt, pageSize, nil + } + return v1.StorageMediumDefault, !notMnt, nil, nil } diff --git a/pkg/volume/emptydir/empty_dir_test.go b/pkg/volume/emptydir/empty_dir_test.go index 892ec1ec342..bd813926c2c 100644 --- a/pkg/volume/emptydir/empty_dir_test.go +++ b/pkg/volume/emptydir/empty_dir_test.go @@ -19,20 +19,24 @@ limitations under the License. package emptydir import ( + "fmt" + "io/ioutil" "os" "path/filepath" "testing" - "k8s.io/utils/mount" - - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + utilfeature "k8s.io/apiserver/pkg/util/feature" utiltesting "k8s.io/client-go/util/testing" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" volumetest "k8s.io/kubernetes/pkg/volume/testing" "k8s.io/kubernetes/pkg/volume/util" + "k8s.io/utils/mount" ) // Construct an instance of a plugin, by name. @@ -71,8 +75,8 @@ type fakeMountDetector struct { isMount bool } -func (fake *fakeMountDetector) GetMountMedium(path string) (v1.StorageMedium, bool, error) { - return fake.medium, fake.isMount, nil +func (fake *fakeMountDetector) GetMountMedium(path string, requestedMedium v1.StorageMedium) (v1.StorageMedium, bool, *resource.Quantity, error) { + return fake.medium, fake.isMount, nil, nil } func TestPluginEmptyRootContext(t *testing.T) { @@ -83,12 +87,33 @@ func TestPluginEmptyRootContext(t *testing.T) { } func TestPluginHugetlbfs(t *testing.T) { - doTestPlugin(t, pluginTestConfig{ - medium: v1.StorageMediumHugePages, - expectedSetupMounts: 1, - expectedTeardownMounts: 0, - shouldBeMountedBeforeTeardown: true, - }) + testCases := map[string]struct { + medium v1.StorageMedium + enableHugePageStorageMediumSize bool + }{ + "HugePageStorageMediumSize enabled: medium without size": { + medium: "HugePages", + enableHugePageStorageMediumSize: true, + }, + "HugePageStorageMediumSize disabled: medium without size": { + medium: "HugePages", + }, + "HugePageStorageMediumSize enabled: medium with size": { + medium: "HugePages-2Mi", + enableHugePageStorageMediumSize: true, + }, + } + 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, + expectedTeardownMounts: 0, + shouldBeMountedBeforeTeardown: true, + }) + }) + } } type pluginTestConfig struct { @@ -307,11 +332,13 @@ func TestMetrics(t *testing.T) { func TestGetHugePagesMountOptions(t *testing.T) { testCases := map[string]struct { - pod *v1.Pod - shouldFail bool - expectedResult string + pod *v1.Pod + medium v1.StorageMedium + shouldFail bool + expectedResult string + enableHugePageStorageMediumSize bool }{ - "testWithProperValues": { + "ProperValues": { pod: &v1.Pod{ Spec: v1.PodSpec{ Containers: []v1.Container{ @@ -325,10 +352,11 @@ func TestGetHugePagesMountOptions(t *testing.T) { }, }, }, + medium: v1.StorageMediumHugePages, shouldFail: false, expectedResult: "pagesize=2Mi", }, - "testWithProperValuesAndDifferentPageSize": { + "ProperValuesAndDifferentPageSize": { pod: &v1.Pod{ Spec: v1.PodSpec{ Containers: []v1.Container{ @@ -349,6 +377,7 @@ func TestGetHugePagesMountOptions(t *testing.T) { }, }, }, + medium: v1.StorageMediumHugePages, shouldFail: false, expectedResult: "pagesize=1Gi", }, @@ -373,6 +402,7 @@ func TestGetHugePagesMountOptions(t *testing.T) { }, }, }, + medium: v1.StorageMediumHugePages, shouldFail: false, expectedResult: "pagesize=1Gi", }, @@ -397,6 +427,7 @@ func TestGetHugePagesMountOptions(t *testing.T) { }, }, }, + medium: v1.StorageMediumHugePages, shouldFail: true, expectedResult: "", }, @@ -421,24 +452,463 @@ func TestGetHugePagesMountOptions(t *testing.T) { }, }, }, - shouldFail: true, - expectedResult: "", + medium: v1.StorageMediumHugePages, + shouldFail: true, + expectedResult: "", + enableHugePageStorageMediumSize: true, }, "PodWithNoHugePagesRequest": { pod: &v1.Pod{}, + medium: v1.StorageMediumHugePages, + shouldFail: true, + expectedResult: "", + }, + "ProperValuesMultipleSizes": { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + v1.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, + }, + }, + }, + medium: v1.StorageMediumHugePagesPrefix + "1Gi", + shouldFail: false, + expectedResult: "pagesize=1Gi", + enableHugePageStorageMediumSize: true, + }, + "InitContainerAndContainerHasProperValuesMultipleSizes": { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + InitContainers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + }, + }, + }, + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-1Gi"): resource.MustParse("4Gi"), + v1.ResourceName("hugepages-2Mi"): resource.MustParse("50Mi"), + }, + }, + }, + }, + }, + }, + medium: v1.StorageMediumHugePagesPrefix + "2Mi", + shouldFail: false, + expectedResult: "pagesize=2Mi", + enableHugePageStorageMediumSize: true, + }, + "MediumWithoutSizeMultipleSizes": { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + v1.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, + }, + }, + }, + medium: v1.StorageMediumHugePagesPrefix, + shouldFail: true, + expectedResult: "", + }, + "IncorrectMediumFormatMultipleSizes": { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + v1.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, + }, + }, + }, + medium: "foo", + shouldFail: true, + expectedResult: "", + }, + "MediumSizeDoesntMatchResourcesMultipleSizes": { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + v1.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, + }, + }, + }, + medium: v1.StorageMediumHugePagesPrefix + "1Mi", shouldFail: true, expectedResult: "", }, } for testCaseName, testCase := range testCases { - value, err := getPageSizeMountOptionFromPod(testCase.pod) - if testCase.shouldFail && err == nil { - t.Errorf("Expected an error in %v", testCaseName) - } else if !testCase.shouldFail && err != nil { - t.Errorf("Unexpected error in %v, got %v", testCaseName, err) - } else if testCase.expectedResult != value { - t.Errorf("Unexpected mountOptions for Pod. Expected %v, got %v", testCase.expectedResult, value) - } + 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) + } else if !testCase.shouldFail && err != nil { + t.Errorf("%s: Unexpected error: %v", testCaseName, err) + } else if testCase.expectedResult != value { + t.Errorf("%s: Unexpected mountOptions for Pod. Expected %v, got %v", testCaseName, testCase.expectedResult, value) + } + }) + } +} + +type testMountDetector struct { + pageSize *resource.Quantity + isMnt bool + err error +} + +func (md *testMountDetector) GetMountMedium(path string, requestedMedium v1.StorageMedium) (v1.StorageMedium, bool, *resource.Quantity, error) { + return v1.StorageMediumHugePages, md.isMnt, md.pageSize, md.err +} + +func TestSetupHugepages(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "TestSetupHugepages") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpdir) + + pageSize2Mi := resource.MustParse("2Mi") + + testCases := map[string]struct { + path string + ed *emptyDir + shouldFail bool + }{ + "Valid: mount expected": { + path: tmpdir, + ed: &emptyDir{ + medium: v1.StorageMediumHugePages, + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + }, + }, + }, + }, + }, + }, + mounter: &mount.FakeMounter{}, + mountDetector: &testMountDetector{ + pageSize: &resource.Quantity{}, + isMnt: false, + err: nil, + }, + }, + shouldFail: false, + }, + "Valid: already mounted with correct pagesize": { + path: tmpdir, + ed: &emptyDir{ + medium: "HugePages-2Mi", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + }, + }, + }, + }, + }, + }, + mounter: mount.NewFakeMounter([]mount.MountPoint{{Path: tmpdir, Opts: []string{"rw", "pagesize=2M", "realtime"}}}), + mountDetector: &testMountDetector{ + pageSize: &pageSize2Mi, + isMnt: true, + err: nil, + }, + }, + shouldFail: false, + }, + "Valid: already mounted": { + path: tmpdir, + ed: &emptyDir{ + medium: "HugePages", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + }, + }, + }, + }, + }, + }, + mounter: mount.NewFakeMounter([]mount.MountPoint{{Path: tmpdir, Opts: []string{"rw", "pagesize=2M", "realtime"}}}), + mountDetector: &testMountDetector{ + pageSize: nil, + isMnt: true, + err: nil, + }, + }, + shouldFail: false, + }, + "Invalid: mounter is nil": { + path: tmpdir, + ed: &emptyDir{ + medium: "HugePages-2Mi", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + }, + }, + }, + }, + }, + }, + mounter: nil, + }, + shouldFail: true, + }, + "Invalid: GetMountMedium error": { + path: tmpdir, + ed: &emptyDir{ + medium: "HugePages-2Mi", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + }, + }, + }, + }, + }, + }, + mounter: mount.NewFakeMounter([]mount.MountPoint{{Path: tmpdir, Opts: []string{"rw", "pagesize=2M", "realtime"}}}), + mountDetector: &testMountDetector{ + pageSize: &pageSize2Mi, + isMnt: true, + err: fmt.Errorf("GetMountMedium error"), + }, + }, + shouldFail: true, + }, + "Invalid: medium and page size differ": { + path: tmpdir, + ed: &emptyDir{ + medium: "HugePages-1Gi", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, + }, + }, + }, + mounter: mount.NewFakeMounter([]mount.MountPoint{{Path: tmpdir, Opts: []string{"rw", "pagesize=2M", "realtime"}}}), + mountDetector: &testMountDetector{ + pageSize: &pageSize2Mi, + isMnt: true, + err: nil, + }, + }, + shouldFail: true, + }, + "Invalid medium": { + path: tmpdir, + ed: &emptyDir{ + medium: "HugePages-NN", + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + }, + }, + }, + }, + }, + }, + mounter: &mount.FakeMounter{}, + mountDetector: &testMountDetector{ + pageSize: &resource.Quantity{}, + isMnt: false, + err: nil, + }, + }, + shouldFail: true, + }, + "Invalid: setupDir fails": { + path: "", + ed: &emptyDir{ + medium: v1.StorageMediumHugePages, + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + }, + }, + }, + }, + }, + }, + mounter: &mount.FakeMounter{}, + }, + shouldFail: true, + }, + } + + for testCaseName, testCase := range testCases { + t.Run(testCaseName, func(t *testing.T) { + err := testCase.ed.setupHugepages(testCase.path) + if testCase.shouldFail && err == nil { + t.Errorf("%s: Unexpected success", testCaseName) + } else if !testCase.shouldFail && err != nil { + t.Errorf("%s: Unexpected error: %v", testCaseName, err) + } + }) + } +} + +func TestGetPageSize(t *testing.T) { + mounter := &mount.FakeMounter{ + MountPoints: []mount.MountPoint{ + { + Device: "/dev/sda2", + Type: "ext4", + Path: "/", + Opts: []string{"rw", "relatime", "errors=remount-ro"}, + }, + { + Device: "/dev/hugepages", + Type: "hugetlbfs", + Path: "/mnt/hugepages-2Mi", + Opts: []string{"rw", "relatime", "pagesize=2M"}, + }, + { + Device: "sysfs", + Type: "sysfs", + Path: "/sys", + Opts: []string{"rw", "nosuid", "nodev", "noexec", "relatime"}, + }, + { + Device: "/dev/hugepages", + Type: "hugetlbfs", + Path: "/mnt/hugepages-1Gi", + Opts: []string{"rw", "relatime", "pagesize=1024M"}, + }, + { + Device: "/dev/hugepages", + Type: "hugetlbfs", + Path: "/mnt/noopt", + Opts: []string{"rw", "relatime"}, + }, + { + Device: "/dev/hugepages", + Type: "hugetlbfs", + Path: "/mnt/badopt", + Opts: []string{"rw", "relatime", "pagesize=NN"}, + }, + }, + } + + testCases := map[string]struct { + path string + mounter mount.Interface + expectedResult resource.Quantity + shouldFail bool + }{ + "Valid: existing 2Mi mount": { + path: "/mnt/hugepages-2Mi", + mounter: mounter, + shouldFail: false, + expectedResult: resource.MustParse("2Mi"), + }, + "Valid: existing 1Gi mount": { + path: "/mnt/hugepages-1Gi", + mounter: mounter, + shouldFail: false, + expectedResult: resource.MustParse("1Gi"), + }, + "Invalid: mount point doesn't exist": { + path: "/mnt/nomp", + mounter: mounter, + shouldFail: true, + }, + "Invalid: no pagesize option": { + path: "/mnt/noopt", + mounter: mounter, + shouldFail: true, + }, + "Invalid: incorrect pagesize option": { + path: "/mnt/badopt", + mounter: mounter, + shouldFail: true, + }, + } + + for testCaseName, testCase := range testCases { + t.Run(testCaseName, func(t *testing.T) { + pageSize, err := getPageSize(testCase.path, testCase.mounter) + if testCase.shouldFail && err == nil { + t.Errorf("%s: Unexpected success", testCaseName) + } else if !testCase.shouldFail && err != nil { + t.Errorf("%s: Unexpected error: %v", testCaseName, err) + } + if err == nil && pageSize.Cmp(testCase.expectedResult) != 0 { + t.Errorf("%s: Unexpected result: %s, expected: %s", testCaseName, pageSize.String(), testCase.expectedResult.String()) + } + }) } } diff --git a/pkg/volume/emptydir/empty_dir_unsupported.go b/pkg/volume/emptydir/empty_dir_unsupported.go index b048f76081a..45dc0defa47 100644 --- a/pkg/volume/emptydir/empty_dir_unsupported.go +++ b/pkg/volume/emptydir/empty_dir_unsupported.go @@ -19,6 +19,7 @@ limitations under the License. package emptydir import ( + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/utils/mount" v1 "k8s.io/api/core/v1" @@ -29,6 +30,6 @@ type realMountDetector struct { mounter mount.Interface } -func (m *realMountDetector) GetMountMedium(path string) (v1.StorageMedium, bool, error) { - return v1.StorageMediumDefault, false, nil +func (m *realMountDetector) GetMountMedium(path string, requestedMedium v1.StorageMedium) (v1.StorageMedium, bool, *resource.Quantity, error) { + return v1.StorageMediumDefault, false, nil, nil } diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 85865211019..91cebce9acc 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -887,9 +887,10 @@ type FlockerVolumeSource struct { type StorageMedium string const ( - StorageMediumDefault StorageMedium = "" // use whatever the default is for the node, assume anything we don't explicitly handle is this - StorageMediumMemory StorageMedium = "Memory" // use memory (e.g. tmpfs on linux) - StorageMediumHugePages StorageMedium = "HugePages" // use hugepages + StorageMediumDefault StorageMedium = "" // use whatever the default is for the node, assume anything we don't explicitly handle is this + StorageMediumMemory StorageMedium = "Memory" // use memory (e.g. tmpfs on linux) + StorageMediumHugePages StorageMedium = "HugePages" // use hugepages + StorageMediumHugePagesPrefix StorageMedium = "HugePages-" // prefix for full medium notation HugePages- ) // Protocol defines network protocols supported for things like container ports.