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 }