From 59a86e4cbc6dc1e6d5ce78c15d9cd09b02ddf105 Mon Sep 17 00:00:00 2001 From: PiotrProkop Date: Fri, 18 Aug 2017 12:33:38 +0200 Subject: [PATCH] Adding getHugePagesMountOptions function and tests --- pkg/api/types.go | 2 +- pkg/api/validation/validation.go | 3 + pkg/api/validation/validation_test.go | 22 ++++ pkg/volume/empty_dir/BUILD | 3 + pkg/volume/empty_dir/empty_dir.go | 50 +++++++- pkg/volume/empty_dir/empty_dir_test.go | 156 ++++++++++++++++++++++++- 6 files changed, 229 insertions(+), 7 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index 6cb987d0407..2c54fcf71fb 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -683,7 +683,7 @@ type StorageMedium string const ( StorageMediumDefault StorageMedium = "" // use whatever the default is for the node StorageMediumMemory StorageMedium = "Memory" // use memory (tmpfs) - StorageMediumHugepages StorageMedium = "Hugepages" // use hugepages + StorageMediumHugePages StorageMedium = "HugePages" // use hugepages ) // Protocol defines network protocols supported for things like container ports. diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index aac45af683b..81a9e01904f 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -394,6 +394,9 @@ func validateVolumeSource(source *api.VolumeSource, fldPath *field.Path, volName allErrs = append(allErrs, field.Forbidden(fldPath.Child("emptyDir").Child("sizeLimit"), "SizeLimit field must be a valid resource quantity")) } } + if !utilfeature.DefaultFeatureGate.Enabled(features.HugePages) && source.EmptyDir.Medium == api.StorageMediumHugePages { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("emptyDir").Child("medium"), "HugePages medium is disabled by feature-gate for EmptyDir volumes")) + } } if source.HostPath != nil { if numVolumes > 0 { diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 05491bcbaed..d0d51354ae0 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -2757,6 +2757,28 @@ func TestValidateVolumes(t *testing.T) { } else if errs[0].Type != field.ErrorTypeDuplicate { t.Errorf("expected error type %v, got %v", field.ErrorTypeDuplicate, errs[0].Type) } + + // Validate HugePages medium type for EmptyDir when HugePages feature is enabled/disabled + hugePagesCase := api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{Medium: api.StorageMediumHugePages}} + + // Enable alpha feature HugePages + err := utilfeature.DefaultFeatureGate.Set("HugePages=true") + if err != nil { + t.Errorf("Failed to enable feature gate for HugePages: %v", err) + } + if errs := validateVolumeSource(&hugePagesCase, field.NewPath("field").Index(0), "working"); len(errs) != 0 { + t.Errorf("Unexpected error when HugePages feature is enabled.") + } + + // Disable alpha feature HugePages + err = utilfeature.DefaultFeatureGate.Set("HugePages=false") + if err != nil { + t.Errorf("Failed to disable feature gate for HugePages: %v", err) + } + if errs := validateVolumeSource(&hugePagesCase, field.NewPath("field").Index(0), "failing"); len(errs) == 0 { + t.Errorf("Expected error when HugePages feature is disabled got nothing.") + } + } func TestAlphaHugePagesIsolation(t *testing.T) { diff --git a/pkg/volume/empty_dir/BUILD b/pkg/volume/empty_dir/BUILD index f16bc3941ba..3dfe1313647 100644 --- a/pkg/volume/empty_dir/BUILD +++ b/pkg/volume/empty_dir/BUILD @@ -19,12 +19,14 @@ go_library( "//conditions:default": [], }), deps = [ + "//pkg/api/v1/helper:go_default_library", "//pkg/util/mount:go_default_library", "//pkg/util/strings:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", "//vendor/github.com/golang/glog:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", ] + select({ @@ -51,6 +53,7 @@ go_test( "//pkg/volume/testing:go_default_library", "//pkg/volume/util:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/client-go/util/testing:go_default_library", diff --git a/pkg/volume/empty_dir/empty_dir.go b/pkg/volume/empty_dir/empty_dir.go index 575d0778a2e..c64ea2a180e 100644 --- a/pkg/volume/empty_dir/empty_dir.go +++ b/pkg/volume/empty_dir/empty_dir.go @@ -23,10 +23,12 @@ import ( "github.com/golang/glog" "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + v1helper "k8s.io/kubernetes/pkg/api/v1/helper" "k8s.io/kubernetes/pkg/util/mount" - "k8s.io/kubernetes/pkg/util/strings" + stringsutil "k8s.io/kubernetes/pkg/util/strings" "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" ) @@ -56,7 +58,7 @@ const ( ) func getPath(uid types.UID, volName string, host volume.VolumeHost) string { - return host.GetPodVolumeDir(uid, strings.EscapeQualifiedNameForDisk(emptyDirPluginName), volName) + return host.GetPodVolumeDir(uid, stringsutil.EscapeQualifiedNameForDisk(emptyDirPluginName), volName) } func (plugin *emptyDirPlugin) Init(host volume.VolumeHost) error { @@ -275,14 +277,52 @@ func (ed *emptyDir) setupHugepages(dir string) error { if err != nil { return err } - // If the directory is a mountpoint with medium memory, there is no + // 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 == mediumHugepages { return nil } + pageSizeMountOption, err := getPageSizeMountOptionFromPod(ed.pod) + if err != nil { + return err + } + glog.V(3).Infof("pod %v: mounting hugepages for volume %v", ed.pod.UID, ed.volName) - return ed.mounter.Mount("nodev", dir, "hugetlbfs", []string{}) + 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) { + 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 { + // 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 pageSizeFound && pageSize.Cmp(currentPageSize) != 0 { + return "", fmt.Errorf("multiple pageSizes for huge pages in a single PodSpec") + } + pageSize = currentPageSize + pageSizeFound = true + } + } + } + + if !pageSizeFound { + return "", fmt.Errorf("hugePages storage requested, but there is no resource request for huge pages.") + } + + return fmt.Sprintf("pageSize=%s", pageSize.String()), nil + } // setupDir creates the directory with the default permissions specified by the perm constant. @@ -383,7 +423,7 @@ func (ed *emptyDir) teardownTmpfsOrHugetlbfs(dir string) error { } func (ed *emptyDir) getMetaDir() string { - return path.Join(ed.plugin.host.GetPodPluginDir(ed.pod.UID, strings.EscapeQualifiedNameForDisk(emptyDirPluginName)), ed.volName) + return path.Join(ed.plugin.host.GetPodPluginDir(ed.pod.UID, stringsutil.EscapeQualifiedNameForDisk(emptyDirPluginName)), ed.volName) } func getVolumeSource(spec *volume.Spec) (*v1.EmptyDirVolumeSource, bool) { diff --git a/pkg/volume/empty_dir/empty_dir_test.go b/pkg/volume/empty_dir/empty_dir_test.go index 4a007a8e8a4..ffe1ce08612 100644 --- a/pkg/volume/empty_dir/empty_dir_test.go +++ b/pkg/volume/empty_dir/empty_dir_test.go @@ -24,6 +24,7 @@ import ( "testing" "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" utiltesting "k8s.io/client-go/util/testing" @@ -118,7 +119,22 @@ func doTestPlugin(t *testing.T, config pluginTestConfig) { physicalMounter = mount.FakeMounter{} mountDetector = fakeMountDetector{} - pod = &v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: types.UID("poduid")}} + pod = &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID("poduid"), + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + }, + }, + }, + }, + }, + } ) if config.idempotent { @@ -285,3 +301,141 @@ func TestMetrics(t *testing.T) { t.Errorf("Expected Available to be greater than 0") } } + +func TestGetHugePagesMountOptions(t *testing.T) { + testCases := map[string]struct { + pod *v1.Pod + shouldFail bool + expectedResult string + }{ + "testWithProperValues": { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + }, + }, + }, + }, + }, + }, + shouldFail: false, + expectedResult: "pageSize=2Mi", + }, + "testWithProperValuesAndDifferentPageSize": { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-1Gi"): resource.MustParse("4Gi"), + }, + }, + }, + }, + }, + }, + shouldFail: false, + expectedResult: "pageSize=1Gi", + }, + "InitContainerAndContainerHasProperValues": { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + InitContainers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-1Gi"): resource.MustParse("4Gi"), + }, + }, + }, + }, + }, + }, + shouldFail: false, + expectedResult: "pageSize=1Gi", + }, + "InitContainerAndContainerHasDifferentPageSizes": { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + InitContainers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("2Gi"), + }, + }, + }, + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-1Gi"): resource.MustParse("4Gi"), + }, + }, + }, + }, + }, + }, + shouldFail: true, + expectedResult: "", + }, + "ContainersWithMultiplePageSizes": { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, + { + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName("hugepages-2Mi"): resource.MustParse("100Mi"), + }, + }, + }, + }, + }, + }, + shouldFail: true, + expectedResult: "", + }, + "PodWithNoHugePagesRequest": { + pod: &v1.Pod{}, + 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) + } + } +}