From 03ecc20b19972ecb4825ecf4ce6868bc2a45f3c0 Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Mon, 17 Feb 2020 17:21:50 +0200 Subject: [PATCH] empty_dir: Check if hugetlbfs volume is mounted with a correct pagesize Extended GetMountMedium function to check if hugetlbfs volume is mounted with the page size equal to the medium size. Page size is obtained from the 'pagesize' mount option of the mounted hugetlbfs volume. --- pkg/volume/emptydir/empty_dir.go | 31 +- pkg/volume/emptydir/empty_dir_linux.go | 77 ++++- pkg/volume/emptydir/empty_dir_test.go | 334 ++++++++++++++++++- pkg/volume/emptydir/empty_dir_unsupported.go | 5 +- 4 files changed, 423 insertions(+), 24 deletions(-) diff --git a/pkg/volume/emptydir/empty_dir.go b/pkg/volume/emptydir/empty_dir.go index 1102262f766..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. @@ -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,13 +284,30 @@ 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 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 } @@ -412,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 12e5820f2f0..bd813926c2c 100644 --- a/pkg/volume/emptydir/empty_dir_test.go +++ b/pkg/volume/emptydir/empty_dir_test.go @@ -19,6 +19,8 @@ limitations under the License. package emptydir import ( + "fmt" + "io/ioutil" "os" "path/filepath" "testing" @@ -73,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) { @@ -582,3 +584,331 @@ func TestGetHugePagesMountOptions(t *testing.T) { }) } } + +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 }