From bd4b87d7c217391c15f41dd0f713b326b6642a80 Mon Sep 17 00:00:00 2001 From: torredil Date: Wed, 10 Apr 2024 18:46:01 +0000 Subject: [PATCH] Add unit tests for mount_linux.go Signed-off-by: torredil --- pkg/driver/node_test.go | 80 --------- pkg/mounter/mount_linux.go | 67 +------ pkg/mounter/mount_test.go | 352 +++++++++++++------------------------ 3 files changed, 121 insertions(+), 378 deletions(-) diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index 53580cd4f9..84c952c093 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -1572,71 +1572,6 @@ func TestNodePublishVolume(t *testing.T) { }, expectedErr: status.Error(codes.Internal, "Failed to find device path /dev/xvdba. device path error"), }, - { - name: "nodePublishVolumeForBlock_path_exists_check", - req: &csi.NodePublishVolumeRequest{ - VolumeId: "vol-test", - StagingTargetPath: "/staging/path", - TargetPath: "/target/path", - VolumeCapability: &csi.VolumeCapability{ - AccessType: &csi.VolumeCapability_Block{ - Block: &csi.VolumeCapability_BlockVolume{}, - }, - AccessMode: &csi.VolumeCapability_AccessMode{ - Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, - }, - }, - PublishContext: map[string]string{ - DevicePathKey: "/dev/xvdba", - }, - }, - mounterMock: func(ctrl *gomock.Controller) *mockmounter.MockMounter { - m := mockmounter.NewMockMounter(ctrl) - - m.EXPECT().FindDevicePath(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("/dev/xvdba", nil) - m.EXPECT().PathExists(gomock.Any()).Return(false, errors.New("path exists error")) - return m - }, - metadataMock: func(ctrl *gomock.Controller) *metadata.MockMetadataService { - m := metadata.NewMockMetadataService(ctrl) - m.EXPECT().GetRegion().Return("us-west-2") - return m - }, - expectedErr: status.Error(codes.Internal, "Could not check if path exists \"/target\": path exists error"), - }, - { - name: "nodePublishVolumeForBlock_make_file_failure", - req: &csi.NodePublishVolumeRequest{ - VolumeId: "vol-test", - StagingTargetPath: "/staging/path", - TargetPath: "/target/path", - VolumeCapability: &csi.VolumeCapability{ - AccessType: &csi.VolumeCapability_Block{ - Block: &csi.VolumeCapability_BlockVolume{}, - }, - AccessMode: &csi.VolumeCapability_AccessMode{ - Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, - }, - }, - PublishContext: map[string]string{ - DevicePathKey: "/dev/xvdba", - }, - }, - mounterMock: func(ctrl *gomock.Controller) *mockmounter.MockMounter { - m := mockmounter.NewMockMounter(ctrl) - - m.EXPECT().FindDevicePath(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("/dev/xvdba", nil) - m.EXPECT().PathExists(gomock.Any()).Return(false, nil) - m.EXPECT().MakeDir(gomock.Any()).Return(errors.New("make file error")) - return m - }, - metadataMock: func(ctrl *gomock.Controller) *metadata.MockMetadataService { - m := metadata.NewMockMetadataService(ctrl) - m.EXPECT().GetRegion().Return("us-west-2") - return m - }, - expectedErr: status.Error(codes.Internal, "Could not create dir \"/target\": make file error"), - }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -2352,21 +2287,6 @@ func TestNodeGetVolumeStats(t *testing.T) { return nil }, }, - { - name: "metrics_stat_error", - validVolId: true, - validPath: true, - mounterMock: func(ctrl *gomock.Controller, dir string) *mockmounter.MockMounter { - m := mockmounter.NewMockMounter(ctrl) - m.EXPECT().PathExists(gomock.Any()).Return(true, nil) - m.EXPECT().IsBlockDevice(gomock.Any()).Return(false, nil) - return m - }, - expectedErr: func(dir string) error { - return status.Errorf(codes.Internal, "failed to get fs info on path %s: %v", "fake-path", "failed to get FsInfo due to error no such file or directory") - }, - metricsStatErr: true, - }, } for _, tc := range testCases { diff --git a/pkg/mounter/mount_linux.go b/pkg/mounter/mount_linux.go index d354cbc8fb..a248f66543 100644 --- a/pkg/mounter/mount_linux.go +++ b/pkg/mounter/mount_linux.go @@ -92,17 +92,13 @@ func (m *NodeMounter) FindDevicePath(devicePath, volumeID, partition, region str } if canonicalDevicePath == "" { - return "", errNoDevicePathFound(devicePath, volumeID) + return "", fmt.Errorf("no device path for device %q volume %q found", devicePath, volumeID) } canonicalDevicePath = m.appendPartition(canonicalDevicePath, partition) return canonicalDevicePath, nil } -func errNoDevicePathFound(devicePath, volumeID string) error { - return fmt.Errorf("no device path for device %q volume %q found", devicePath, volumeID) -} - // findNvmeVolume looks for the nvme volume with the specified name // It follows the symlink (if it exists) and returns the absolute path to the device func findNvmeVolume(findName string) (device string, err error) { @@ -259,67 +255,6 @@ func (m *NodeMounter) NeedResize(devicePath string, deviceMountPath string) (boo return mountutils.NewResizeFs(m.Exec).NeedResize(devicePath, deviceMountPath) } -func (m *NodeMounter) getExtSize(devicePath string) (uint64, uint64, error) { - output, err := m.SafeFormatAndMount.Exec.Command("dumpe2fs", "-h", devicePath).CombinedOutput() - if err != nil { - return 0, 0, fmt.Errorf("failed to read size of filesystem on %s: %w: %s", devicePath, err, string(output)) - } - - blockSize, blockCount, _ := m.parseFsInfoOutput(string(output), ":", "block size", "block count") - - if blockSize == 0 { - return 0, 0, fmt.Errorf("could not find block size of device %s", devicePath) - } - if blockCount == 0 { - return 0, 0, fmt.Errorf("could not find block count of device %s", devicePath) - } - return blockSize, blockSize * blockCount, nil -} - -func (m *NodeMounter) getXFSSize(devicePath string) (uint64, uint64, error) { - output, err := m.SafeFormatAndMount.Exec.Command("xfs_io", "-c", "statfs", devicePath).CombinedOutput() - if err != nil { - return 0, 0, fmt.Errorf("failed to read size of filesystem on %s: %w: %s", devicePath, err, string(output)) - } - - blockSize, blockCount, _ := m.parseFsInfoOutput(string(output), "=", "geom.bsize", "geom.datablocks") - - if blockSize == 0 { - return 0, 0, fmt.Errorf("could not find block size of device %s", devicePath) - } - if blockCount == 0 { - return 0, 0, fmt.Errorf("could not find block count of device %s", devicePath) - } - return blockSize, blockSize * blockCount, nil -} - -func (m *NodeMounter) parseFsInfoOutput(cmdOutput string, spliter string, blockSizeKey string, blockCountKey string) (uint64, uint64, error) { - lines := strings.Split(cmdOutput, "\n") - var blockSize, blockCount uint64 - var err error - - for _, line := range lines { - tokens := strings.Split(line, spliter) - if len(tokens) != 2 { - continue - } - key, value := strings.ToLower(strings.TrimSpace(tokens[0])), strings.ToLower(strings.TrimSpace(tokens[1])) - if key == blockSizeKey { - blockSize, err = strconv.ParseUint(value, 10, 64) - if err != nil { - return 0, 0, fmt.Errorf("failed to parse block size %s: %w", value, err) - } - } - if key == blockCountKey { - blockCount, err = strconv.ParseUint(value, 10, 64) - if err != nil { - return 0, 0, fmt.Errorf("failed to parse block count %s: %w", value, err) - } - } - } - return blockSize, blockCount, err -} - func (m *NodeMounter) Unpublish(path string) error { // On linux, unpublish and unstage both perform an unmount return m.Unstage(path) diff --git a/pkg/mounter/mount_test.go b/pkg/mounter/mount_test.go index 8ef2e7e012..0e9c3b3415 100644 --- a/pkg/mounter/mount_test.go +++ b/pkg/mounter/mount_test.go @@ -20,248 +20,18 @@ limitations under the License. package mounter import ( + "fmt" "os" "path/filepath" "testing" + "github.com/stretchr/testify/assert" "k8s.io/mount-utils" utilexec "k8s.io/utils/exec" fakeexec "k8s.io/utils/exec/testing" ) -func TestGetFileSystemSize(t *testing.T) { - cmdOutputSuccessXfs := - ` - statfs.f_bsize = 4096 - statfs.f_blocks = 1832448 - statfs.f_bavail = 1822366 - statfs.f_files = 3670016 - statfs.f_ffree = 3670012 - statfs.f_flags = 0x1020 - geom.bsize = 4096 - geom.agcount = 4 - geom.agblocks = 458752 - geom.datablocks = 1835008 - geom.rtblocks = 0 - geom.rtextents = 0 - geom.rtextsize = 1 - geom.sunit = 0 - geom.swidth = 0 - counts.freedata = 1822372 - counts.freertx = 0 - counts.freeino = 61 - counts.allocino = 64 -` - cmdOutputNoDataXfs := - ` - statfs.f_bsize = 4096 - statfs.f_blocks = 1832448 - statfs.f_bavail = 1822366 - statfs.f_files = 3670016 - statfs.f_ffree = 3670012 - statfs.f_flags = 0x1020 - geom.agcount = 4 - geom.agblocks = 458752 - geom.rtblocks = 0 - geom.rtextents = 0 - geom.rtextsize = 1 - geom.sunit = 0 - geom.swidth = 0 - counts.freedata = 1822372 - counts.freertx = 0 - counts.freeino = 61 - counts.allocino = 64 -` - cmdOutputSuccessExt4 := - ` -Filesystem volume name: cloudimg-rootfs -Last mounted on: / -Filesystem UUID: testUUID -Filesystem magic number: 0xEF53 -Filesystem revision #: 1 (dynamic) -Filesystem features: has_journal ext_attr resize_inode dir_index filetype needs_recovery extent 64bit -Default mount options: user_xattr acl -Filesystem state: clean -Errors behavior: Continue -Filesystem OS type: Linux -Inode count: 3840000 -Block count: 5242880 -Reserved block count: 0 -Free blocks: 5514413 -Free inodes: 3677492 -First block: 0 -Block size: 4096 -Fragment size: 4096 -Group descriptor size: 64 -Reserved GDT blocks: 252 -Blocks per group: 32768 -Fragments per group: 32768 -Inodes per group: 16000 -Inode blocks per group: 1000 -Flex block group size: 16 -Mount count: 2 -Maximum mount count: -1 -Check interval: 0 () -Lifetime writes: 180 GB -Reserved blocks uid: 0 (user root) -Reserved blocks gid: 0 (group root) -First inode: 11 -Inode size: 256 -Required extra isize: 32 -Desired extra isize: 32 -Journal inode: 8 -Default directory hash: half_md4 -Directory Hash Seed: Test Hashing -Journal backup: inode blocks -Checksum type: crc32c -Checksum: 0x57705f62 -Journal features: journal_incompat_revoke journal_64bit journal_checksum_v3 -Journal size: 64M -Journal length: 16384 -Journal sequence: 0x00037109 -Journal start: 1 -Journal checksum type: crc32c -Journal checksum: 0xb7df3c6e -` - cmdOutputNoDataExt4 := - `Filesystem volume name: cloudimg-rootfs -Last mounted on: / -Filesystem UUID: testUUID -Filesystem magic number: 0xEF53 -Filesystem revision #: 1 (dynamic) -Filesystem features: has_journal ext_attr resize_inode dir_index filetype needs_recovery extent 64bit -Default mount options: user_xattr acl -Filesystem state: clean -Errors behavior: Continue -Filesystem OS type: Linux -Inode count: 3840000 -Reserved block count: 0 -Free blocks: 5514413 -Free inodes: 3677492 -First block: 0 -Fragment size: 4096 -Group descriptor size: 64 -Reserved GDT blocks: 252 -Blocks per group: 32768 -Fragments per group: 32768 -Inodes per group: 16000 -Inode blocks per group: 1000 -Flex block group size: 16 -Mount count: 2 -Maximum mount count: -1 -Check interval: 0 () -Lifetime writes: 180 GB -Reserved blocks uid: 0 (user root) -Reserved blocks gid: 0 (group root) -First inode: 11 -Inode size: 256 -Required extra isize: 32 -Desired extra isize: 32 -Journal inode: 8 -Default directory hash: half_md4 -Directory Hash Seed: Test Hashing -Journal backup: inode blocks -Checksum type: crc32c -Checksum: 0x57705f62 -Journal features: journal_incompat_revoke journal_64bit journal_checksum_v3 -Journal size: 64M -Journal length: 16384 -Journal sequence: 0x00037109 -Journal start: 1 -Journal checksum type: crc32c -Journal checksum: 0xb7df3c6e -` - testcases := []struct { - name string - devicePath string - blocksize uint64 - blockCount uint64 - cmdOutput string - expectError bool - fsType string - }{ - { - name: "success parse xfs info", - devicePath: "/dev/test1", - blocksize: 4096, - blockCount: 1835008, - cmdOutput: cmdOutputSuccessXfs, - expectError: false, - fsType: "xfs", - }, - { - name: "block size not present - xfs", - devicePath: "/dev/test1", - blocksize: 0, - blockCount: 0, - cmdOutput: cmdOutputNoDataXfs, - expectError: true, - fsType: "xfs", - }, - { - name: "success parse ext info", - devicePath: "/dev/test1", - blocksize: 4096, - blockCount: 5242880, - cmdOutput: cmdOutputSuccessExt4, - expectError: false, - fsType: "ext4", - }, - { - name: "block size not present - ext4", - devicePath: "/dev/test1", - blocksize: 0, - blockCount: 0, - cmdOutput: cmdOutputNoDataExt4, - expectError: true, - fsType: "ext4", - }, - } - - for _, test := range testcases { - t.Run(test.name, func(t *testing.T) { - fcmd := fakeexec.FakeCmd{ - CombinedOutputScript: []fakeexec.FakeAction{ - func() ([]byte, []byte, error) { return []byte(test.cmdOutput), nil, nil }, - }, - } - fexec := fakeexec.FakeExec{ - CommandScript: []fakeexec.FakeCommandAction{ - func(cmd string, args ...string) utilexec.Cmd { - return fakeexec.InitFakeCmd(&fcmd, cmd, args...) - }, - }, - } - safe := mount.SafeFormatAndMount{ - Interface: mount.New(""), - Exec: &fexec, - } - fakeMounter := NodeMounter{&safe} - - var blockSize uint64 - var fsSize uint64 - var err error - switch test.fsType { - case "xfs": - blockSize, fsSize, err = fakeMounter.getXFSSize(test.devicePath) - case "ext4": - blockSize, fsSize, err = fakeMounter.getExtSize(test.devicePath) - } - - if blockSize != test.blocksize { - t.Fatalf("Parse wrong block size value, expect %d, but got %d", test.blocksize, blockSize) - } - if fsSize != test.blocksize*test.blockCount { - t.Fatalf("Parse wrong fs size value, expect %d, but got %d", test.blocksize*test.blockCount, fsSize) - } - if !test.expectError && err != nil { - t.Fatalf("Expect no error but got %v", err) - } - }) - } -} - func TestNeedResize(t *testing.T) { testcases := []struct { name string @@ -418,3 +188,121 @@ func TestGetDeviceName(t *testing.T) { } } + +func TestFindDevicePath(t *testing.T) { + testCases := []struct { + name string + volumeID string + partition string + region string + createTempDir bool + symlink bool + verifyErr error + deviceSize string + cmdOutputFsType string + expectedErr error + }{ + { + name: "Device path exists and matches volume ID", + volumeID: "vol-1234567890abcdef0", + partition: "1", + createTempDir: true, + symlink: false, + verifyErr: nil, + deviceSize: "1024", + cmdOutputFsType: "ext4", + expectedErr: nil, + }, + { + name: "Device path doesn't exist", + volumeID: "vol-1234567890abcdef0", + partition: "1", + createTempDir: false, + symlink: false, + verifyErr: nil, + deviceSize: "1024", + cmdOutputFsType: "ext4", + expectedErr: fmt.Errorf("no device path for device %q volume %q found", "/temp/vol-1234567890abcdef0", "vol-1234567890abcdef0"), + }, + { + name: "SBE region fallback", + volumeID: "vol-1234567890abcdef0", + partition: "1", + region: "snow", + createTempDir: false, + symlink: false, + verifyErr: nil, + deviceSize: "1024", + cmdOutputFsType: "ext4", + expectedErr: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var tmpDir string + var err error + + if tc.createTempDir { + tmpDir, err = os.MkdirTemp("", "temp-test-device-path") + if err != nil { + t.Fatalf("Failed to create temporary directory: %v", err) + } + defer os.RemoveAll(tmpDir) + } else { + tmpDir = "/temp" + } + + devicePath := filepath.Join(tmpDir, tc.volumeID) + expectedResult := devicePath + tc.partition + + fcmd := fakeexec.FakeCmd{ + CombinedOutputScript: []fakeexec.FakeAction{ + func() ([]byte, []byte, error) { return []byte(tc.deviceSize), nil, nil }, + func() ([]byte, []byte, error) { return []byte(tc.cmdOutputFsType), nil, nil }, + }, + } + fexec := fakeexec.FakeExec{ + CommandScript: []fakeexec.FakeCommandAction{ + func(cmd string, args ...string) utilexec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + func(cmd string, args ...string) utilexec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + }, + } + + safe := mount.SafeFormatAndMount{ + Interface: mount.New(""), + Exec: &fexec, + } + + fakeMounter := NodeMounter{&safe} + + if tc.createTempDir { + if tc.symlink { + symlinkErr := os.Symlink(devicePath, devicePath) + if symlinkErr != nil { + t.Fatalf("Failed to create symlink: %v", err) + } + } else { + _, osCreateErr := os.Create(devicePath) + if osCreateErr != nil { + t.Fatalf("Failed to create device path: %v", err) + } + } + } + + result, err := fakeMounter.FindDevicePath(devicePath, tc.volumeID, tc.partition, tc.region) + + if tc.region == "snow" { + expectedResult = "/dev/vd" + tc.volumeID[len(tc.volumeID)-1:] + tc.partition + } + + if tc.expectedErr == nil { + assert.Equal(t, expectedResult, result) + assert.NoError(t, err) + } else { + assert.Empty(t, result) + assert.EqualError(t, err, tc.expectedErr.Error()) + } + }) + } +}