From 5bdc66ac5a179c133c605c7ed71608906b65eea2 Mon Sep 17 00:00:00 2001 From: jharrod Date: Thu, 19 Dec 2024 09:39:49 -0700 Subject: [PATCH] add device utils unit tests Add unit tests fpr device utils package --- .../mock_devices/mock_size_getter_client.go | 55 ++ utils/devices/devices.go | 35 +- utils/devices/devices_darwin.go | 2 +- utils/devices/devices_darwin_test.go | 4 +- utils/devices/devices_linux.go | 21 +- utils/devices/devices_linux_test.go | 427 ++++++++++- utils/devices/devices_test.go | 691 +++++++++++++++--- utils/devices/devices_windows.go | 2 +- utils/devices/devices_windows_test.go | 4 +- utils/devices/luks/luks_linux.go | 3 + utils/devices/luks/luks_linux_test.go | 81 +- utils/devices/luks/utils_test.go | 31 + utils/devices/utils.go | 6 +- utils/iscsi.go | 3 - 14 files changed, 1222 insertions(+), 143 deletions(-) create mode 100644 mocks/mock_utils/mock_devices/mock_size_getter_client.go create mode 100644 utils/devices/luks/utils_test.go diff --git a/mocks/mock_utils/mock_devices/mock_size_getter_client.go b/mocks/mock_utils/mock_devices/mock_size_getter_client.go new file mode 100644 index 000000000..5e83e993d --- /dev/null +++ b/mocks/mock_utils/mock_devices/mock_size_getter_client.go @@ -0,0 +1,55 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/netapp/trident/utils/devices (interfaces: SizeGetter) +// +// Generated by this command: +// +// mockgen -destination=../../mocks/mock_utils/mock_devices/mock_size_getter_client.go github.com/netapp/trident/utils/devices SizeGetter +// + +// Package mock_devices is a generated GoMock package. +package mock_devices + +import ( + context "context" + reflect "reflect" + + gomock "go.uber.org/mock/gomock" +) + +// MockSizeGetter is a mock of SizeGetter interface. +type MockSizeGetter struct { + ctrl *gomock.Controller + recorder *MockSizeGetterMockRecorder +} + +// MockSizeGetterMockRecorder is the mock recorder for MockSizeGetter. +type MockSizeGetterMockRecorder struct { + mock *MockSizeGetter +} + +// NewMockSizeGetter creates a new mock instance. +func NewMockSizeGetter(ctrl *gomock.Controller) *MockSizeGetter { + mock := &MockSizeGetter{ctrl: ctrl} + mock.recorder = &MockSizeGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockSizeGetter) EXPECT() *MockSizeGetterMockRecorder { + return m.recorder +} + +// GetDiskSize mocks base method. +func (m *MockSizeGetter) GetDiskSize(arg0 context.Context, arg1 string) (int64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetDiskSize", arg0, arg1) + ret0, _ := ret[0].(int64) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetDiskSize indicates an expected call of GetDiskSize. +func (mr *MockSizeGetterMockRecorder) GetDiskSize(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDiskSize", reflect.TypeOf((*MockSizeGetter)(nil).GetDiskSize), arg0, arg1) +} diff --git a/utils/devices/devices.go b/utils/devices/devices.go index 1c359db72..475ad3c09 100644 --- a/utils/devices/devices.go +++ b/utils/devices/devices.go @@ -1,6 +1,7 @@ // Copyright 2024 NetApp, Inc. All Rights Reserved. //go:generate mockgen -destination=../../mocks/mock_utils/mock_devices/mock_devices_client.go github.com/netapp/trident/utils/devices Devices +//go:generate mockgen -destination=../../mocks/mock_utils/mock_devices/mock_size_getter_client.go github.com/netapp/trident/utils/devices SizeGetter package devices @@ -74,17 +75,28 @@ type Devices interface { ) error } +type SizeGetter interface { + GetDiskSize(ctx context.Context, devicePath string) (int64, error) +} + +type DiskSizeGetter struct{} + +func NewDiskSizeGetter() *DiskSizeGetter { + return &DiskSizeGetter{} +} + type Client struct { chrootPathPrefix string command exec.Command osFs afero.Afero + SizeGetter } func New() *Client { - return NewDetailed(exec.NewCommand(), afero.NewOsFs()) + return NewDetailed(exec.NewCommand(), afero.NewOsFs(), NewDiskSizeGetter()) } -func NewDetailed(command exec.Command, osFs afero.Fs) *Client { +func NewDetailed(command exec.Command, osFs afero.Fs, diskSizeGetter SizeGetter) *Client { chrootPathPrefix := "" if os.Getenv("DOCKER_PLUGIN_MODE") != "" { chrootPathPrefix = "/host" @@ -93,6 +105,7 @@ func NewDetailed(command exec.Command, osFs afero.Fs) *Client { chrootPathPrefix: chrootPathPrefix, command: command, osFs: afero.Afero{Fs: osFs}, + SizeGetter: diskSizeGetter, } } @@ -297,7 +310,7 @@ func (c *Client) FindDevicesForMultipathDevice(ctx context.Context, device strin devices := make([]string, 0) slavesDir := c.chrootPathPrefix + "/sys/block/" + device + "/slaves" - if dirs, err := os.ReadDir(slavesDir); err == nil { + if dirs, err := c.osFs.ReadDir(slavesDir); err == nil { for _, f := range dirs { name := f.Name() if strings.HasPrefix(name, "sd") { @@ -585,7 +598,7 @@ func (c *Client) GetMultipathDeviceDisks( multipathDevice := strings.TrimPrefix(multipathDevicePath, "/dev/") diskPath := c.chrootPathPrefix + fmt.Sprintf("/sys/block/%s/slaves/", multipathDevice) - diskDirs, err := os.ReadDir(diskPath) + diskDirs, err := c.osFs.ReadDir(diskPath) if err != nil { Logc(ctx).WithError(err).Errorf("Could not read %s", diskPath) return nil, fmt.Errorf("failed to identify multipath device disks; unable to read '%s'", diskPath) @@ -607,7 +620,7 @@ func (c *Client) GetMultipathDeviceDisks( func (c *Client) GetMultipathDeviceBySerial(ctx context.Context, hexSerial string) (string, error) { sysPath := c.chrootPathPrefix + "/sys/block/" - blockDirs, err := os.ReadDir(sysPath) + blockDirs, err := c.osFs.ReadDir(sysPath) if err != nil { Logc(ctx).WithError(err).Errorf("Could not read %s", sysPath) return "", fmt.Errorf("failed to find multipath device by serial; unable to read '%s'", sysPath) @@ -647,12 +660,12 @@ func (c *Client) GetMultipathDeviceUUID(multipathDevicePath string) (string, err deviceUUIDPath := c.chrootPathPrefix + fmt.Sprintf("/sys/block/%s/dm/uuid", multipathDevice) - exists, err := PathExists(deviceUUIDPath) + exists, err := PathExists(c.osFs, deviceUUIDPath) if !exists || err != nil { return "", errors.NotFoundError("multipath device '%s' UUID not found", multipathDevice) } - UUID, err := os.ReadFile(deviceUUIDPath) + UUID, err := c.osFs.ReadFile(deviceUUIDPath) if err != nil { return "", err } @@ -666,7 +679,7 @@ func (c *Client) RemoveDevice(ctx context.Context, devices []string, ignoreError defer Logc(ctx).Debug("<<<< devices.removeDevice") var ( - f *os.File + f afero.File err error ) @@ -674,7 +687,7 @@ func (c *Client) RemoveDevice(ctx context.Context, devices []string, ignoreError for _, deviceName := range devices { filename := fmt.Sprintf(c.chrootPathPrefix+"/sys/block/%s/device/delete", deviceName) - if f, err = os.OpenFile(filename, os.O_APPEND|os.O_WRONLY, 0o200); err != nil { + if f, err = c.osFs.OpenFile(filename, os.O_APPEND|os.O_WRONLY, 0o200); err != nil { Logc(ctx).WithField("file", filename).Warning("Could not open file for writing.") if ignoreErrors { continue @@ -746,7 +759,7 @@ func (c *Client) GetLUKSDeviceForMultipathDevice(multipathDevice string) (string dmDevice := strings.TrimSuffix(strings.TrimPrefix(multipathDevice, "/dev/"), "/") // Get holder of mpath device - dirents, err := os.ReadDir(fmt.Sprintf("/sys/block/%s/holders/", dmDevice)) + dirents, err := c.osFs.ReadDir(fmt.Sprintf("/sys/block/%s/holders/", dmDevice)) if err != nil { return "", err } @@ -759,7 +772,7 @@ func (c *Client) GetLUKSDeviceForMultipathDevice(multipathDevice string) (string holder := dirents[0].Name() // Verify holder is LUKS device - b, err := os.ReadFile(fmt.Sprintf("/sys/block/%s/dm/uuid", holder)) + b, err := c.osFs.ReadFile(fmt.Sprintf("/sys/block/%s/dm/uuid", holder)) if err != nil { return "", err } diff --git a/utils/devices/devices_darwin.go b/utils/devices/devices_darwin.go index 6a2ea135b..8641a1eef 100644 --- a/utils/devices/devices_darwin.go +++ b/utils/devices/devices_darwin.go @@ -37,7 +37,7 @@ func (c *Client) VerifyMultipathDeviceSize(ctx context.Context, multipathDevice, } // GetDiskSize unused stub function -func (c *Client) GetDiskSize(ctx context.Context, _ string) (int64, error) { +func (c *DiskSizeGetter) GetDiskSize(ctx context.Context, _ string) (int64, error) { Logc(ctx).Debug(">>>> devices_darwin.GetDiskSize") defer Logc(ctx).Debug("<<<< devices_darwin.GetDiskSize") return 0, errors.UnsupportedError("GetDiskSize is not supported for darwin") diff --git a/utils/devices/devices_darwin_test.go b/utils/devices/devices_darwin_test.go index 33375f6a7..a9d1b6379 100644 --- a/utils/devices/devices_darwin_test.go +++ b/utils/devices/devices_darwin_test.go @@ -16,7 +16,7 @@ import ( func TestFlushOneDevice(t *testing.T) { ctx := context.Background() - devices := NewDetailed(exec.NewCommand(), afero.NewMemMapFs()) + devices := NewDetailed(exec.NewCommand(), afero.NewMemMapFs(), nil) result := devices.FlushOneDevice(ctx, "/test/path") assert.Error(t, result, "no error") assert.True(t, errors.IsUnsupportedError(result), "not UnsupportedError") @@ -25,7 +25,7 @@ func TestFlushOneDevice(t *testing.T) { func TestGetISCSIDiskSize(t *testing.T) { ctx := context.Background() - devices := NewDetailed(exec.NewCommand(), afero.NewMemMapFs()) + devices := NewDetailed(exec.NewCommand(), afero.NewMemMapFs(), NewDiskSizeGetter()) result, err := devices.GetDiskSize(ctx, "/test/path") assert.Equal(t, result, int64(0), "received disk size") assert.Error(t, err, "no error") diff --git a/utils/devices/devices_linux.go b/utils/devices/devices_linux.go index 232e20827..ad214f5e3 100644 --- a/utils/devices/devices_linux.go +++ b/utils/devices/devices_linux.go @@ -7,7 +7,6 @@ package devices import ( "fmt" "os" - "os/exec" "strings" "syscall" "time" @@ -19,22 +18,12 @@ import ( "github.com/netapp/trident/internal/fiji" . "github.com/netapp/trident/logging" "github.com/netapp/trident/utils/errors" + execCmd "github.com/netapp/trident/utils/exec" "github.com/netapp/trident/utils/filesystem" ) const ( - luksCloseTimeout = 30 * time.Second - luksCypherMode = "aes-xts-plain64" - luksType = "luks2" - - // Return codes for `cryptsetup status` - cryptsetupStatusDeviceDoesExistStatusCode = 0 - cryptsetupStatusDeviceDoesNotExistStatusCode = 4 - - // Return codes for `cryptsetup isLuks` - cryptsetupIsLuksDeviceIsLuksStatusCode = 0 - cryptsetupIsLuksDeviceIsNotLuksStatusCode = 1 - + luksCloseTimeout = 30 * time.Second luksCloseMaxWaitDuration = 2 * time.Minute ) @@ -71,7 +60,7 @@ func (c *Client) FlushOneDevice(ctx context.Context, devicePath string) error { } // GetDiskSize queries the current block size in bytes -func (c *Client) GetDiskSize(ctx context.Context, devicePath string) (int64, error) { +func (c *DiskSizeGetter) GetDiskSize(ctx context.Context, devicePath string) (int64, error) { fields := LogFields{"rawDevicePath": devicePath} Logc(ctx).WithFields(fields).Debug(">>>> devices_linux.GetDiskSize") defer Logc(ctx).WithFields(fields).Debug("<<<< devices_linux.GetDiskSize") @@ -203,7 +192,7 @@ func (c *Client) GetDeviceFSType(ctx context.Context, device string) (string, er if errors.IsTimeoutError(err) { c.ListAllDevices(ctx) return "", err - } else if exitErr, ok := err.(*exec.ExitError); ok && exitErr.ExitCode() == 2 { + } else if exitErr, ok := err.(execCmd.ExitError); ok && exitErr.ExitCode() == 2 { // EITHER: Disk device is unformatted. // OR: For 'blkid', if the specified token (TYPE/PTTYPE, etc) was // not found, or no (specified) devices could be identified, an @@ -293,7 +282,7 @@ func (c *Client) WaitForDevice(ctx context.Context, device string) error { Logc(ctx).WithFields(fields).Debug(">>>> devices.waitForDevice") defer Logc(ctx).WithFields(fields).Debug("<<<< devices.waitForDevice") - exists, err := PathExists(device) + exists, err := PathExists(c.osFs, device) if !exists || err != nil { return errors.New("device not yet present") } else { diff --git a/utils/devices/devices_linux_test.go b/utils/devices/devices_linux_test.go index 60cbe1a51..6c545c55a 100644 --- a/utils/devices/devices_linux_test.go +++ b/utils/devices/devices_linux_test.go @@ -15,9 +15,12 @@ import ( "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" + "github.com/netapp/trident/mocks/mock_utils/mock_devices" mockexec "github.com/netapp/trident/mocks/mock_utils/mock_exec" "github.com/netapp/trident/utils/errors" "github.com/netapp/trident/utils/exec" + "github.com/netapp/trident/utils/filesystem" + "github.com/netapp/trident/utils/models" ) func mockCryptsetupLuksClose(mock *mockexec.MockCommand) *gomock.Call { @@ -31,7 +34,7 @@ func TestEnsureLUKSDeviceClosedWithMaxWaitLimit(t *testing.T) { luksDevicePath := "/dev/mapper/luks-test" osFs.Create(luksDevicePath) client := mockexec.NewMockCommand(gomock.NewController(t)) - deviceClient := NewDetailed(client, osFs) + deviceClient := NewDetailed(client, osFs, NewDiskSizeGetter()) type testCase struct { name string @@ -86,7 +89,7 @@ func TestEnsureLUKSDeviceClosedWithMaxWaitLimit(t *testing.T) { func Test_CloseLUKSDevice(t *testing.T) { mockCtrl := gomock.NewController(t) mockCommand := mockexec.NewMockCommand(mockCtrl) - deviceClient := NewDetailed(mockCommand, afero.NewMemMapFs()) + deviceClient := NewDetailed(mockCommand, afero.NewMemMapFs(), NewDiskSizeGetter()) // Setup mock calls and reassign any clients to their mock counterparts. mockCryptsetupLuksClose(mockCommand).Return([]byte(""), nil) @@ -96,7 +99,7 @@ func Test_CloseLUKSDevice(t *testing.T) { } func TestEnsureLUKSDeviceClosed_DeviceDoesNotExist(t *testing.T) { - deviceClient := NewDetailed(exec.NewCommand(), afero.NewMemMapFs()) + deviceClient := NewDetailed(exec.NewCommand(), afero.NewMemMapFs(), NewDiskSizeGetter()) err := deviceClient.EnsureLUKSDeviceClosed(context.Background(), "/dev/mapper/luks-test-dev") assert.NoError(t, err) } @@ -109,7 +112,7 @@ func TestEnsureLUKSDeviceClosed_FailsToDetectDevice(t *testing.T) { b.WriteByte('a') } s := b.String() - deviceClient := NewDetailed(exec.NewCommand(), osFs) + deviceClient := NewDetailed(exec.NewCommand(), osFs, NewDiskSizeGetter()) err := deviceClient.EnsureLUKSDeviceClosed(context.Background(), "/dev/mapper/"+s) assert.Error(t, err) } @@ -120,7 +123,7 @@ func TestEnsureLUKSDeviceClosed_FailsToCloseDevice(t *testing.T) { osFs := afero.NewMemMapFs() osFs.Create(devicePath) - deviceClient := NewDetailed(exec.NewCommand(), osFs) + deviceClient := NewDetailed(exec.NewCommand(), osFs, NewDiskSizeGetter()) err := deviceClient.EnsureLUKSDeviceClosed(ctx, devicePath) assert.Error(t, err) } @@ -139,7 +142,419 @@ func TestEnsureLUKSDeviceClosed_ClosesDevice(t *testing.T) { mockCryptsetupLuksClose(mockCommand).Return([]byte(""), nil), ) - deviceClient := NewDetailed(mockCommand, osFs) + deviceClient := NewDetailed(mockCommand, osFs, NewDiskSizeGetter()) err := deviceClient.EnsureLUKSDeviceClosed(ctx, devicePath) assert.NoError(t, err) } + +func TestFlushOneDevice(t *testing.T) { + tests := map[string]struct { + name string + devicePath string + mockSetup func(*mockexec.MockCommand) + expectedError bool + }{ + "Flush successful": { + devicePath: "/dev/sda", + mockSetup: func(mockCommand *mockexec.MockCommand) { + mockCommand.EXPECT().ExecuteWithTimeout( + gomock.Any(), "blockdev", deviceOperationsTimeout, true, "--flushbufs", "/dev/sda", + ).Return([]byte(""), nil) + }, + expectedError: false, + }, + "Flush failed": { + devicePath: "/dev/sda", + mockSetup: func(mockCommand *mockexec.MockCommand) { + mockCommand.EXPECT().ExecuteWithTimeout( + gomock.Any(), "blockdev", deviceOperationsTimeout, true, "--flushbufs", "/dev/sda", + ).Return([]byte(""), fmt.Errorf("flush error")) + }, + expectedError: true, + }, + } + + for name, params := range tests { + t.Run(name, func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockCommand := mockexec.NewMockCommand(mockCtrl) + params.mockSetup(mockCommand) + + client := &Client{ + command: mockCommand, + osFs: afero.Afero{Fs: afero.NewMemMapFs()}, + } + + err := client.FlushOneDevice(context.Background(), params.devicePath) + if params.expectedError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestGetDeviceFSType(t *testing.T) { + tests := map[string]struct { + name string + device string + getMockCommand func() exec.Command + getMockFs func() afero.Fs + expectedFSType string + expectedError bool + }{ + "SuccessfulGetDeviceFSType": { + device: "/dev/sda", + getMockCommand: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout( + gomock.Any(), "blkid", 5*time.Second, true, "/dev/sda", + ).Return([]byte(`TYPE="ext4"`), nil) + return mockCommand + }, + getMockFs: func() afero.Fs { + mockFs := afero.NewMemMapFs() + mockFs.Create("/dev/sda") + return mockFs + }, + expectedFSType: "ext4", + expectedError: false, + }, + "DeviceNotFound": { + device: "/dev/sdb", + getMockCommand: func() exec.Command { + return mockexec.NewMockCommand(gomock.NewController(t)) + }, + getMockFs: func() afero.Fs { + return afero.NewMemMapFs() + }, + expectedFSType: "", + expectedError: true, + }, + "UnformattedDevice": { + device: "/dev/sda", + getMockCommand: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout( + gomock.Any(), "blkid", 5*time.Second, true, "/dev/sda", + ).Return([]byte(""), mockexec.NewMockExitError(2, "mock error")) + return mockCommand + }, + getMockFs: func() afero.Fs { + mockFs := afero.NewMemMapFs() + mockFs.Create("/dev/sda") + return mockFs + }, + expectedFSType: "", + expectedError: false, + }, + "UnknownFsType": { + device: "/dev/sda", + getMockCommand: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout( + gomock.Any(), "blkid", 5*time.Second, true, "/dev/sda", + ).Return([]byte("TYPE="), nil) + mockCommand.EXPECT().ExecuteWithTimeout( + context.Background(), "dd", 5*time.Second, false, "if=/dev/sda", "bs=4096", + "count=512", "status=none") + return mockCommand + }, + getMockFs: func() afero.Fs { + mockFs := afero.NewMemMapFs() + mockFs.Create("/dev/sda") + return mockFs + }, + expectedFSType: "", + expectedError: true, + }, + "UnknownFormattedFsType": { + device: "/dev/sda", + getMockCommand: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout( + gomock.Any(), "blkid", 5*time.Second, true, "/dev/sda", + ).Return([]byte("TYPE="), nil) + + dataSize := 2097152 + out := make([]byte, dataSize) + for i := range dataSize { + out[i] = 1 + } + mockCommand.EXPECT().ExecuteWithTimeout( + context.Background(), "dd", 5*time.Second, false, "if=/dev/sda", "bs=4096", + "count=512", "status=none").Return(out, nil) + return mockCommand + }, + getMockFs: func() afero.Fs { + mockFs := afero.NewMemMapFs() + mockFs.Create("/dev/sda") + return mockFs + }, + expectedFSType: filesystem.UnknownFstype, + expectedError: false, + }, + "TimeoutError": { + device: "/dev/sda", + getMockCommand: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout( + gomock.Any(), "blkid", 5*time.Second, true, "/dev/sda", + ).Return(nil, errors.TimeoutError("timeout")) + return mockCommand + }, + getMockFs: func() afero.Fs { + mockFs := afero.NewMemMapFs() + mockFs.Create("/dev/sda") + return mockFs + }, + expectedFSType: "", + expectedError: true, + }, + } + + for name, params := range tests { + t.Run(name, func(t *testing.T) { + client := &Client{ + command: params.getMockCommand(), + osFs: afero.Afero{Fs: params.getMockFs()}, + } + + fsType, err := client.GetDeviceFSType(context.Background(), params.device) + if params.expectedError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, params.expectedFSType, fsType) + } + }) + } +} + +func TestClient_verifyMultipathDeviceSize(t *testing.T) { + type parameters struct { + getDevicesClient func(controller *gomock.Controller) Devices + getMockDiskSizeGetter func(controller *gomock.Controller) SizeGetter + assertError assert.ErrorAssertionFunc + assertValid assert.BoolAssertionFunc + expectedDeviceSize int64 + } + + const deviceName = "sda" + const multipathDeviceName = "dm-0" + + tests := map[string]parameters{ + "error getting device size": { + getMockDiskSizeGetter: func(controller *gomock.Controller) SizeGetter { + mockGetter := mock_devices.NewMockSizeGetter(controller) + mockGetter.EXPECT().GetDiskSize(context.TODO(), gomock.Any()).Return(int64(0), + errors.New("some error")) + return mockGetter + }, + assertError: assert.Error, + assertValid: assert.False, + expectedDeviceSize: 0, + }, + "error getting multipath device size": { + getMockDiskSizeGetter: func(controller *gomock.Controller) SizeGetter { + mockGetter := mock_devices.NewMockSizeGetter(controller) + mockGetter.EXPECT().GetDiskSize(context.TODO(), gomock.Any()).Return(int64(1), nil) + mockGetter.EXPECT().GetDiskSize(context.TODO(), gomock.Any()).Return(int64(0), + errors.New("some error")) + return mockGetter + }, + assertError: assert.Error, + assertValid: assert.False, + expectedDeviceSize: 0, + }, + "device size != multipath device size": { + getMockDiskSizeGetter: func(controller *gomock.Controller) SizeGetter { + mockGetter := mock_devices.NewMockSizeGetter(controller) + mockGetter.EXPECT().GetDiskSize(context.TODO(), gomock.Any()).Return(int64(1), nil) + mockGetter.EXPECT().GetDiskSize(context.TODO(), gomock.Any()).Return(int64(0), nil) + return mockGetter + }, + assertError: assert.NoError, + assertValid: assert.False, + expectedDeviceSize: 1, + }, + "happy path": { + getMockDiskSizeGetter: func(controller *gomock.Controller) SizeGetter { + mockGetter := mock_devices.NewMockSizeGetter(controller) + mockGetter.EXPECT().GetDiskSize(context.TODO(), gomock.Any()).Return(int64(1), nil) + mockGetter.EXPECT().GetDiskSize(context.TODO(), gomock.Any()).Return(int64(1), nil) + return mockGetter + }, + assertError: assert.NoError, + assertValid: assert.True, + expectedDeviceSize: 0, + }, + } + + for name, params := range tests { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + client := NewDetailed(exec.NewCommand(), afero.NewMemMapFs(), params.getMockDiskSizeGetter(ctrl)) + deviceSize, valid, err := client.VerifyMultipathDeviceSize(context.TODO(), multipathDeviceName, deviceName) + if params.assertError != nil { + params.assertError(t, err) + } + if params.assertValid != nil { + params.assertValid(t, valid) + } + assert.Equal(t, params.expectedDeviceSize, deviceSize) + }) + } +} + +func TestMultipathFlushDevice(t *testing.T) { + devicePath := "/dev/dm-0" + tests := map[string]struct { + getMockCmd func() exec.Command + deviceInfo *models.ScsiDeviceInfo + expectError bool + }{ + "Happy Path": { + getMockCmd: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "multipath", 5*time.Second, true, + "-C", devicePath).Return([]byte(""), nil) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "blockdev", 5*time.Second, true, "--flushbufs", + devicePath).Return([]byte(""), nil) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "multipath", 10*time.Second, false, "-f", + devicePath).Return([]byte(""), nil) + return mockCommand + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "dm-0", + }, + expectError: false, + }, + "Missing MultipathDevice": { + getMockCmd: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + return mockCommand + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "", + }, + expectError: false, + }, + "Can Flush Error": { + getMockCmd: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "multipath", 5*time.Second, true, + "-C", "/dev/dm-0").Return([]byte(""), errors.ISCSIDeviceFlushError("error")) + return mockCommand + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "dm-0", + }, + expectError: true, + }, + "Timeout Error": { + getMockCmd: func() exec.Command { + volumeFlushExceptions[devicePath] = time.Now().Add(-1 * time.Hour) + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "multipath", 5*time.Second, true, + "-C", "/dev/dm-0").Return([]byte("no usable paths found"), fmt.Errorf("error")) + return mockCommand + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "dm-0", + }, + expectError: true, + }, + "Flush Error": { + getMockCmd: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "multipath", 5*time.Second, true, + "-C", devicePath).Return([]byte(""), nil) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "blockdev", 5*time.Second, true, "--flushbufs", + devicePath).Return([]byte(""), fmt.Errorf("error")) + return mockCommand + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "dm-0", + }, + expectError: true, + }, + "Multipath Remove Error": { + getMockCmd: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "multipath", 5*time.Second, true, + "-C", devicePath).Return([]byte(""), nil) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "blockdev", 5*time.Second, true, "--flushbufs", + devicePath).Return([]byte(""), nil) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "multipath", 10*time.Second, false, "-f", + devicePath).Return([]byte(""), fmt.Errorf("error")) + return mockCommand + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "dm-0", + }, + expectError: true, + }, + } + for name, params := range tests { + t.Run(name, func(t *testing.T) { + deviceClient := NewDetailed(params.getMockCmd(), afero.NewMemMapFs(), NewDiskSizeGetter()) + err := deviceClient.MultipathFlushDevice(context.TODO(), params.deviceInfo) + delete(volumeFlushExceptions, devicePath) + if params.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestFlushDevice(t *testing.T) { + device := "sda" + tests := map[string]struct { + getMockCmd func() exec.Command + deviceInfo *models.ScsiDeviceInfo + force bool + expectError bool + }{ + "Happy Path": { + deviceInfo: &models.ScsiDeviceInfo{ + Devices: []string{device}, + }, + getMockCmd: func() exec.Command { + mockCmd := mockexec.NewMockCommand(gomock.NewController(t)) + mockCmd.EXPECT().ExecuteWithTimeout(gomock.Any(), "blockdev", 5*time.Second, true, "--flushbufs", + DevPrefix+device).Return([]byte(""), nil) + return mockCmd + }, + expectError: false, + }, + "No Multipath Device Error": { + deviceInfo: &models.ScsiDeviceInfo{ + Devices: []string{device}, + }, + getMockCmd: func() exec.Command { + mockCmd := mockexec.NewMockCommand(gomock.NewController(t)) + mockCmd.EXPECT().ExecuteWithTimeout(gomock.Any(), "blockdev", 5*time.Second, true, "--flushbufs", + DevPrefix+device).Return([]byte(""), fmt.Errorf("error")) + return mockCmd + }, + expectError: true, + }, + } + for name, params := range tests { + t.Run(name, func(t *testing.T) { + deviceClient := NewDetailed(params.getMockCmd(), afero.NewMemMapFs(), NewDiskSizeGetter()) + err := deviceClient.FlushDevice(context.TODO(), params.deviceInfo, params.force) + if params.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/utils/devices/devices_test.go b/utils/devices/devices_test.go index 0be6b2ac6..56e2aacec 100644 --- a/utils/devices/devices_test.go +++ b/utils/devices/devices_test.go @@ -17,6 +17,8 @@ import ( mockexec "github.com/netapp/trident/mocks/mock_utils/mock_exec" tridentError "github.com/netapp/trident/utils/errors" + "github.com/netapp/trident/utils/exec" + "github.com/netapp/trident/utils/models" ) func TestRemoveMultipathDeviceMapping(t *testing.T) { @@ -66,7 +68,7 @@ func TestRemoveMultipathDeviceMapping(t *testing.T) { Return(tt.mockReturn, tt.mockError) } - deviceClient := NewDetailed(mockCommand, afero.NewMemMapFs()) + deviceClient := NewDetailed(mockCommand, afero.NewMemMapFs(), NewDiskSizeGetter()) err := deviceClient.RemoveMultipathDeviceMapping(context.TODO(), tt.devicePath) if tt.expectError { assert.Error(t, err) @@ -181,7 +183,7 @@ func TestClient_scanTargetLUN(t *testing.T) { for name, params := range tests { t.Run(name, func(t *testing.T) { - client := NewDetailed(nil, afero.Afero{Fs: params.getFileSystemUtils()}) + client := NewDetailed(nil, afero.Afero{Fs: params.getFileSystemUtils()}, NewDiskSizeGetter()) err := client.ScanTargetLUN(context.TODO(), lunID, []int{host1, host2}) if params.assertError != nil { @@ -266,7 +268,7 @@ func TestClient_getLunSerial(t *testing.T) { for name, params := range tests { t.Run(name, func(t *testing.T) { - client := NewDetailed(nil, afero.Afero{Fs: params.getFileSystemUtils()}) + client := NewDetailed(nil, afero.Afero{Fs: params.getFileSystemUtils()}, NewDiskSizeGetter()) response, err := client.GetLunSerial(context.TODO(), devicePath) if params.assertError != nil { params.assertError(t, err) @@ -276,94 +278,599 @@ func TestClient_getLunSerial(t *testing.T) { } } -// NOTE: Since this is now in the devices package, -// we cannot unit test this without finding a way to mock file's Fd() function. -// Afero does not support this. -//func TestClient_verifyMultipathDeviceSize(t *testing.T) { -// type parameters struct { -// getDevicesClient func(controller *gomock.Controller) Devices -// getFs func(controller *gomock.Controller) afero.Fs -// assertError assert.ErrorAssertionFunc -// assertValid assert.BoolAssertionFunc -// expectedDeviceSize int64 -// } -// -// const deviceName = "sda" -// const multipathDeviceName = "dm-0" -// -// tests := map[string]parameters{ -// "error getting device size": { -// getDevicesClient: func(controller *gomock.Controller) Devices { -// mockDevices := mock_devices.NewMockDevices(controller) -// mockDevices.EXPECT().GetISCSIDiskSize(context.TODO(), gomock.Any()).Return(int64(0), -// errors.New("some error")) -// return mockDevices -// }, -// assertError: assert.Error, -// assertValid: assert.False, -// expectedDeviceSize: 0, -// }, -// "error getting multipath device size": { -// getDevicesClient: func(controller *gomock.Controller) Devices { -// mockDevices := mock_devices.NewMockDevices(controller) -// mockDevices.EXPECT().GetISCSIDiskSize(context.TODO(), gomock.Any()).Return(int64(1), nil) -// mockDevices.EXPECT().GetISCSIDiskSize(context.TODO(), gomock.Any()).Return(int64(0), -// errors.New("some error")) -// return mockDevices -// }, -// assertError: assert.Error, -// assertValid: assert.False, -// expectedDeviceSize: 0, -// }, -// "device size != multipath device size": { -// getDevicesClient: func(controller *gomock.Controller) Devices { -// mockDevices := mock_devices.NewMockDevices(controller) -// mockDevices.EXPECT().GetISCSIDiskSize(context.TODO(), gomock.Any()).Return(int64(1), nil) -// mockDevices.EXPECT().GetISCSIDiskSize(context.TODO(), gomock.Any()).Return(int64(0), nil) -// return mockDevices -// }, -// assertError: assert.NoError, -// assertValid: assert.False, -// expectedDeviceSize: 1, -// }, -// "happy path": { -// getDevicesClient: func(controller *gomock.Controller) Devices { -// mockDevices := mock_devices.NewMockDevices(controller) -// mockDevices.EXPECT().GetISCSIDiskSize(context.TODO(), gomock.Any()).Return(int64(1), nil) -// mockDevices.EXPECT().GetISCSIDiskSize(context.TODO(), gomock.Any()).Return(int64(1), nil) -// return mockDevices -// }, -// getFs: func(controller *gomock.Controller) afero.Fs { -// fs := afero.NewMemMapFs() -// fs.Create(DevPrefix + deviceName) -// fs.Create(multipathDeviceName) -// return fs -// }, -// assertError: assert.NoError, -// assertValid: assert.True, -// expectedDeviceSize: 0, -// }, -// } -// -// for name, params := range tests { -// t.Run(name, func(t *testing.T) { -// ctrl := gomock.NewController(t) -// var fs afero.Fs -// if params.getFs != nil { -// fs = params.getFs(ctrl) -// } -// client := NewDetailed(execCmd.NewCommand(), fs) -// deviceSize, valid, err := client.VerifyMultipathDeviceSize(context.TODO(), multipathDeviceName, deviceName) -// if params.assertError != nil { -// params.assertError(t, err) -// } -// if params.assertValid != nil { -// params.assertValid(t, valid) -// } -// assert.Equal(t, params.expectedDeviceSize, deviceSize) -// }) -// } -//} +func TestClient_EnsureDeviceReadable(t *testing.T) { + devicePath := "/dev/mock-0" + tests := map[string]struct { + getMockCmd func() exec.Command + expectError bool + }{ + "Happy Path": { + getMockCmd: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + outBytes := 4096 + out := make([]byte, outBytes) + for i := range outBytes { + out[i] = 0 + } + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "dd", 5*time.Second, false, "if="+devicePath, + "bs=4096", "count=1", "status=none").Return(out, nil) + return mockCommand + }, + expectError: false, + }, + "Fail to read device": { + getMockCmd: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "dd", 5*time.Second, false, "if="+devicePath, + "bs=4096", "count=1", "status=none").Return([]byte(""), fmt.Errorf("error")) + return mockCommand + }, + expectError: true, + }, + "NoDataRead": { + getMockCmd: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "dd", 5*time.Second, false, "if="+devicePath, + "bs=4096", "count=1", "status=none").Return([]byte(""), nil) + return mockCommand + }, + expectError: true, + }, + } + + for name, params := range tests { + t.Run(name, func(t *testing.T) { + deviceClient := NewDetailed(params.getMockCmd(), afero.NewMemMapFs(), NewDiskSizeGetter()) + err := deviceClient.EnsureDeviceReadable(context.TODO(), devicePath) + if params.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestCanFlushMultipathDevice(t *testing.T) { + devicePath := "/dev/mock-0" + tests := map[string]struct { + getMockCmd func() exec.Command + expectError bool + }{ + "Happy Path": { + getMockCmd: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "multipath", 5*time.Second, true, + "-C", devicePath).Return([]byte(""), nil) + return mockCommand + }, + expectError: false, + }, + "Device Not Ready For Flush": { + getMockCmd: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "multipath", 5*time.Second, true, + "-C", devicePath).Return([]byte(""), fmt.Errorf("error")) + return mockCommand + }, + expectError: true, + }, + "Device Unavailable": { + getMockCmd: func() exec.Command { + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "multipath", 5*time.Second, true, + "-C", devicePath).Return([]byte("no usable paths found"), fmt.Errorf("error")) + return mockCommand + }, + expectError: true, + }, + "Flush timeout exceeded": { + getMockCmd: func() exec.Command { + volumeFlushExceptions[devicePath] = time.Now().Add(-1 * time.Hour) + mockCommand := mockexec.NewMockCommand(gomock.NewController(t)) + mockCommand.EXPECT().ExecuteWithTimeout(gomock.Any(), "multipath", 5*time.Second, true, + "-C", devicePath).Return([]byte("no usable paths found"), fmt.Errorf("error")) + return mockCommand + }, + expectError: true, + }, + } + for name, params := range tests { + t.Run(name, func(t *testing.T) { + deviceClient := NewDetailed(params.getMockCmd(), afero.NewMemMapFs(), NewDiskSizeGetter()) + err := deviceClient.canFlushMultipathDevice(context.TODO(), devicePath) + delete(volumeFlushExceptions, devicePath) + if params.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestFindDevicesForMultipathDevice(t *testing.T) { + dm := "dm-0" + device := "sda" + tests := map[string]struct { + getFs func() afero.Fs + expect []string + }{ + "Happy Path": { + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + fs.Create(fmt.Sprintf("/sys/block/%s/slaves/%s", dm, device)) + return fs + }, + expect: []string{device}, + }, + "Device Not Found": { + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + return fs + }, + expect: []string{}, + }, + } + for name, params := range tests { + t.Run(name, func(t *testing.T) { + deviceClient := NewDetailed(nil, params.getFs(), nil) + s := deviceClient.FindDevicesForMultipathDevice(context.TODO(), dm) + assert.Equal(t, params.expect, s) + }) + } +} + +func TestVerifyMultipathDevice(t *testing.T) { + tests := map[string]struct { + getFs func() afero.Fs + publishInfo *models.VolumePublishInfo + deviceInfo *models.ScsiDeviceInfo + allPublishInfos []models.VolumePublishInfo + expectError bool + }{ + "CompareWithPublishedDevicePath Happy Path": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "/dev/dm-0", + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-0", + }, + getFs: func() afero.Fs { + return afero.NewMemMapFs() + }, + expectError: false, + }, + "CompareWithPublishedDevicePath Incorrect Multipath": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "/dev/dm-0", + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-1", + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + fs.Mkdir("/sys/block/dm-0/slaves/sda", 0o755) + return fs + }, + expectError: false, + }, + "CompareWithPublishedDevicePath Incorrect Multipath, Ghost Device": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "/dev/dm-0", + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-1", + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + fs.Mkdir("/sys/block/dm-0/slaves", 0o755) + return fs + }, + expectError: false, + }, + "CompareWithPublishedSerialNumber Happy Path": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "", + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunSerial: "1234", + }, + }, + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-0", + DevicePaths: []string{"/dev/sda"}, + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + afero.WriteFile(fs, "/dev/sda/vpd_pg80", []byte{ + 0, 128, 0, 12, 121, 111, 99, 119, 66, 63, 87, 108, + 55, 120, 50, 108, + }, 0o755) + return fs + }, + expectError: false, + }, + "CompareWithPublishedSerialNumber GetMultipathDeviceUUID Error": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "", + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunSerial: "yocwB?Wl7x2l", + }, + }, + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-0", + DevicePaths: []string{"/dev/sda"}, + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + afero.WriteFile(fs, "/dev/sda/vpd_pg80", []byte{ + 0, 128, 0, 12, 121, 111, 99, 119, 66, 63, 87, 108, + 55, 120, 50, 108, + }, 0o755) + fs.Mkdir("/sys/block/dm-0", 0o755) + return fs + }, + expectError: true, + }, + "CompareWithPublishedSerialNumber Missing Multipath Device": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "", + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunSerial: "yocwB?Wl7x2l", + }, + }, + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-0", + DevicePaths: []string{"/dev/sda"}, + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + afero.WriteFile(fs, "/dev/sda/vpd_pg80", []byte{ + 0, 128, 0, 12, 121, 111, 99, 119, 66, 63, 87, 108, + 55, 120, 50, 108, + }, 0o755) + return fs + }, + expectError: true, + }, + "CompareWithPublishedSerialNumber Fail Getting LUN Serial": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "", + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunSerial: "yocwB?Wl7x2l", + }, + }, + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-0", + DevicePaths: []string{"/dev/sda"}, + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + // LUN serial is not defined + return fs + }, + expectError: true, + }, + "CompareWithPublishedSerialNumber Ghost Device": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "", + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunSerial: "yocwB?Wl7x2l", + }, + }, + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-0", + DevicePaths: []string{"/dev/sda"}, + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + // Defines LUN serial + afero.WriteFile(fs, "/dev/sda/vpd_pg80", []byte{ + 0, 128, 0, 12, 121, 111, 99, 119, 66, 63, 87, 108, + 55, 120, 50, 108, + }, 0o755) + afero.WriteFile(fs, "/sys/block/dm-1/dm/uuid", []byte("mpath-3600a0980796f6377423f576c3778326c"), 0o755) + fs.Mkdir("/sys/block/dm-1/slaves/sdb", 0o755) + return fs + }, + expectError: false, + }, + "CompareWithPublishedSerialNumber Not Ghost Device": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "", + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunSerial: "yocwB?Wl7x2l", + }, + }, + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-0", + DevicePaths: []string{"/dev/sda"}, + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + afero.WriteFile(fs, "/dev/sda/vpd_pg80", []byte{ + 0, 128, 0, 12, 121, 111, 99, 119, 66, 63, 87, 108, + 55, 120, 50, 108, + }, 0o755) + afero.WriteFile(fs, "/sys/block/dm-1/dm/uuid", []byte("mpath-3600a0980796f6377423f576c3778326c"), 0o755) + fs.Mkdir("/sys/block/dm-1/slaves/", 0o755) + return fs + }, + expectError: false, + }, + "CompareWithAllPublishInfos Happy Path": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "", + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunSerial: "", + }, + }, + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-0", + DevicePaths: []string{"/dev/sda"}, + }, + allPublishInfos: []models.VolumePublishInfo{ + { + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunNumber: 1, + }, + }, + }, + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + afero.WriteFile(fs, "/dev/sda/vpd_pg80", []byte{ + 0, 128, 0, 12, 121, 111, 99, 119, 66, 63, 87, 108, + 55, 120, 50, 108, + }, 0o755) + afero.WriteFile(fs, "/sys/block/dm-1/dm/uuid", []byte("mpath-3600a0980796f6377423f576c3778326c"), 0o755) + fs.Mkdir("/sys/block/dm-1/slaves/", 0o755) + return fs + }, + expectError: false, + }, + "CompareWithAllPublishInfos Missing All Publish Info": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "", + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunSerial: "", + }, + }, + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-0", + DevicePaths: []string{"/dev/sda"}, + }, + allPublishInfos: []models.VolumePublishInfo{}, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + afero.WriteFile(fs, "/dev/sda/vpd_pg80", []byte{ + 0, 128, 0, 12, 121, 111, 99, 119, 66, 63, 87, 108, + 55, 120, 50, 108, + }, 0o755) + afero.WriteFile(fs, "/sys/block/dm-1/dm/uuid", []byte("mpath-3600a0980796f6377423f576c3778326c"), 0o755) + fs.Mkdir("/sys/block/dm-1/slaves/", 0o755) + return fs + }, + expectError: true, + }, + "CompareWithAllPublishInfos Duplicate LUNs": { + publishInfo: &models.VolumePublishInfo{ + DevicePath: "", + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunSerial: "", + IscsiLunNumber: 2, + }, + }, + }, + deviceInfo: &models.ScsiDeviceInfo{ + MultipathDevice: "/dev/dm-0", + DevicePaths: []string{"/dev/sda"}, + }, + allPublishInfos: []models.VolumePublishInfo{ + { + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunNumber: 2, + }, + }, + }, + { + VolumeAccessInfo: models.VolumeAccessInfo{ + IscsiAccessInfo: models.IscsiAccessInfo{ + IscsiLunNumber: 2, + }, + }, + }, + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + afero.WriteFile(fs, "/dev/sda/vpd_pg80", []byte{ + 0, 128, 0, 12, 121, 111, 99, 119, 66, 63, 87, 108, + 55, 120, 50, 108, + }, 0o755) + afero.WriteFile(fs, "/sys/block/dm-1/dm/uuid", []byte("mpath-3600a0980796f6377423f576c3778326c"), 0o755) + fs.Mkdir("/sys/block/dm-1/slaves/", 0o755) + return fs + }, + expectError: true, + }, + } + for name, params := range tests { + t.Run(name, func(t *testing.T) { + deviceClient := NewDetailed(nil, params.getFs(), nil) + _, err := deviceClient.VerifyMultipathDevice(context.TODO(), params.publishInfo, + params.allPublishInfos, + params.deviceInfo) + if params.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestRemoveDevice(t *testing.T) { + tests := map[string]struct { + getFs func() afero.Fs + deviceInfo *models.ScsiDeviceInfo + expectError bool + }{ + "Happy Path": { + deviceInfo: &models.ScsiDeviceInfo{ + Devices: []string{"sda"}, + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + fs.Create("/sys/block/sda/device/delete") + return fs + }, + expectError: false, + }, + "Error Opening File": { + deviceInfo: &models.ScsiDeviceInfo{ + Devices: []string{"sda"}, + }, + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + return fs + }, + expectError: true, + }, + } + for name, params := range tests { + t.Run(name, func(t *testing.T) { + deviceClient := NewDetailed(nil, params.getFs(), NewDiskSizeGetter()) + err := deviceClient.RemoveDevice(context.TODO(), params.deviceInfo.Devices, false) + if params.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestGetLUKSDeviceForMultipathDevice(t *testing.T) { + multipathDevice := "/dev/dm-0" + tests := map[string]struct { + getFs func() afero.Fs + expectError bool + }{ + "Happy Path": { + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + fs.Create("/sys/block/dm-0/holders/dm-1") + afero.WriteFile(fs, "/sys/block/dm-1/dm/uuid", []byte("CRYPT-LUKS2-b600e061186e46ac8b05590f389da249-luks-trident_pvc_4b7874ba_58d7_4d93_8d36_09a09b837f81"), 0o755) + return fs + }, + expectError: false, + }, + "No Holders Found": { + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + fs.Mkdir("/sys/block/dm-0/holders/", 0o755) + // afero.WriteFile(fs, "/sys/block/dm-1/dm/uuid", []byte("CRYPT-LUKS2-b600e061186e46ac8b05590f389da249-luks-trident_pvc_4b7874ba_58d7_4d93_8d36_09a09b837f81"), 0755) + return fs + }, + expectError: true, + }, + "Multiple Holders Found": { + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + fs.Create("/sys/block/dm-0/holders/dm-1") + fs.Create("/sys/block/dm-0/holders/dm-2") + // afero.WriteFile(fs, "/sys/block/dm-1/dm/uuid", []byte("CRYPT-LUKS2-b600e061186e46ac8b05590f389da249-luks-trident_pvc_4b7874ba_58d7_4d93_8d36_09a09b837f81"), 0755) + return fs + }, + expectError: true, + }, + "Not LUKS Device": { + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + fs.Create("/sys/block/dm-0/holders/dm-1") + afero.WriteFile(fs, "/sys/block/dm-1/dm/uuid", []byte("not-a-luks-uuid"), 0o755) + return fs + }, + expectError: true, + }, + "Error Reading Holders Dir": { + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + return fs + }, + expectError: true, + }, + "Error Reading UUID": { + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + fs.Create("/sys/block/dm-0/holders/dm-1") + return fs + }, + expectError: true, + }, + } + for name, params := range tests { + t.Run(name, func(t *testing.T) { + deviceClient := NewDetailed(nil, params.getFs(), NewDiskSizeGetter()) + _, err := deviceClient.GetLUKSDeviceForMultipathDevice(multipathDevice) + if params.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestFindMultipathDeviceForDevice(t *testing.T) { + device := "sda" + tests := map[string]struct { + getFs func() afero.Fs + expect string + }{ + "Happy Path": { + getFs: func() afero.Fs { + fs := afero.NewMemMapFs() + fs.Create(fmt.Sprintf("/sys/block/%s/holders/dm-1", device)) + afero.WriteFile(fs, "/sys/block/dm-1/dm/uuid", []byte("CRYPT-LUKS2-b600e061186e46ac8b05590f389da249-luks-trident_pvc_4b7874ba_58d7_4d93_8d36_09a09b837f81"), 0o755) + return fs + }, + expect: "dm-1", + }, + "Not Found": { + getFs: func() afero.Fs { + return afero.NewMemMapFs() + }, + expect: "", + }, + } + for name, params := range tests { + t.Run(name, func(t *testing.T) { + deviceClient := NewDetailed(nil, params.getFs(), NewDiskSizeGetter()) + foundDevice := deviceClient.FindMultipathDeviceForDevice(context.TODO(), device) + assert.Equal(t, params.expect, foundDevice) + }) + } +} func TestWaitForDevicesRemoval(t *testing.T) { errMsg := "timed out waiting for devices to be removed" @@ -436,7 +943,7 @@ func TestWaitForDevicesRemoval(t *testing.T) { t.Run(name, func(t *testing.T) { fs, err := params.getOsFs() assert.NoError(t, err) - devices := NewDetailed(nil, fs) + devices := NewDetailed(nil, fs, nil) err = devices.WaitForDevicesRemoval(context.Background(), params.devicePathPrefix, params.deviceNames, params.maxWaitTime) if params.expectedError != nil { diff --git a/utils/devices/devices_windows.go b/utils/devices/devices_windows.go index 54a759358..974c082fd 100644 --- a/utils/devices/devices_windows.go +++ b/utils/devices/devices_windows.go @@ -37,7 +37,7 @@ func (c *Client) VerifyMultipathDeviceSize(ctx context.Context, multipathDevice, } // GetDiskSize unused stub function -func (c *Client) GetDiskSize(ctx context.Context, _ string) (int64, error) { +func (c *DiskSizeGetter) GetDiskSize(ctx context.Context, _ string) (int64, error) { Logc(ctx).Debug(">>>> devices_windows.GetDiskSize") defer Logc(ctx).Debug("<<<< devices_windows.GetDiskSize") return 0, errors.UnsupportedError("GetDiskSize is not supported for windows") diff --git a/utils/devices/devices_windows_test.go b/utils/devices/devices_windows_test.go index 64bd50cf9..e6cd6e86f 100644 --- a/utils/devices/devices_windows_test.go +++ b/utils/devices/devices_windows_test.go @@ -16,7 +16,7 @@ import ( func TestFlushOneDevice(t *testing.T) { ctx := context.Background() - devices := NewDetailed(exec.NewCommand(), afero.NewMemMapFs()) + devices := NewDetailed(exec.NewCommand(), afero.NewMemMapFs(), nil) result := devices.FlushOneDevice(ctx, "/test/path") assert.Error(t, result, "no error") assert.True(t, errors.IsUnsupportedError(result), "not UnsupportedError") @@ -25,7 +25,7 @@ func TestFlushOneDevice(t *testing.T) { func TestGetDiskSize(t *testing.T) { ctx := context.Background() - devices := NewDetailed(exec.NewCommand(), afero.NewMemMapFs()) + devices := NewDetailed(exec.NewCommand(), afero.NewMemMapFs(), NewDiskSizeGetter()) result, err := devices.GetDiskSize(ctx, "/test/path") assert.Equal(t, result, int64(0), "received disk size") assert.Error(t, err, "no error") diff --git a/utils/devices/luks/luks_linux.go b/utils/devices/luks/luks_linux.go index 9473557ce..ce84636ec 100644 --- a/utils/devices/luks/luks_linux.go +++ b/utils/devices/luks/luks_linux.go @@ -21,6 +21,9 @@ import ( const ( luksCommandTimeout time.Duration = time.Second * 30 + luksCypherMode = "aes-xts-plain64" + luksType = "luks2" + // Return code for "no permission (bad passphrase)" from cryptsetup command luksCryptsetupBadPassphraseReturnCode = 2 diff --git a/utils/devices/luks/luks_linux_test.go b/utils/devices/luks/luks_linux_test.go index 8f541dc4c..578729e24 100644 --- a/utils/devices/luks/luks_linux_test.go +++ b/utils/devices/luks/luks_linux_test.go @@ -102,17 +102,54 @@ func TestLUKSDevice_LUKSFormat(t *testing.T) { mockCtrl := gomock.NewController(t) mockCommand := mockexec.NewMockCommand(mockCtrl) - luksDevice := NewDetailed("/dev/sdb", "pvc-test", mockCommand, devices.New(), afero.NewMemMapFs()) - assert.Equal(t, "/dev/mapper/pvc-test", luksDevice.MappedDevicePath()) - assert.Equal(t, "/dev/sdb", luksDevice.RawDevicePath()) - - // Setup mock calls and reassign any clients to their mock counterparts. + mockCryptsetupIsLuks(mockCommand).Return([]byte(""), + mock_exec.NewMockExitError(cryptsetupIsLuksDeviceIsNotLuksStatusCode, "not LUKS")) + mockCryptsetupLuksFormat(mockCommand).Return([]byte(""), nil) mockCryptsetupIsLuks(mockCommand).Return([]byte(""), nil) + mockDevices := mock_devices.NewMockDevices(mockCtrl) + mockDevices.EXPECT().IsDeviceUnformatted(gomock.Any(), "/dev/sdb").Return(true, nil) + + luksDevice := NewDetailed("/dev/sdb", "pvc-test", mockCommand, mockDevices, afero.NewMemMapFs()) err := luksDevice.lUKSFormat(context.Background(), "passphrase") assert.NoError(t, err) } +func TestLUKSFormat_UnformattedCheckError(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockCommand := mockexec.NewMockCommand(mockCtrl) + + mockCryptsetupIsLuks(mockCommand).Return([]byte(""), + mock_exec.NewMockExitError(cryptsetupIsLuksDeviceIsNotLuksStatusCode, "not LUKS")) + + // Setup mock calls and reassign any clients to their mock counterparts. + mockDevices := mock_devices.NewMockDevices(mockCtrl) + mockDevices.EXPECT().IsDeviceUnformatted(gomock.Any(), "/dev/sdb").Return(false, errors.New("mock error")) + + luksDevice := NewDetailed("/dev/sdb", "pvc-test", mockCommand, mockDevices, afero.NewMemMapFs()) + err := luksDevice.lUKSFormat(context.Background(), "passphrase") + assert.Error(t, err) +} + +func TestLUKSFormat_SecondFormatCheckError(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockCommand := mockexec.NewMockCommand(mockCtrl) + + // Setup mock calls and reassign any clients to their mock counterparts. + mockCryptsetupIsLuks(mockCommand).Return([]byte(""), + mock_exec.NewMockExitError(cryptsetupIsLuksDeviceIsNotLuksStatusCode, "not LUKS")) + mockCryptsetupLuksFormat(mockCommand).Return([]byte(""), nil) + mockCryptsetupIsLuks(mockCommand).Return([]byte(""), + mock_exec.NewMockExitError(cryptsetupIsLuksDeviceIsNotLuksStatusCode, "not LUKS")) + + mockDevices := mock_devices.NewMockDevices(mockCtrl) + mockDevices.EXPECT().IsDeviceUnformatted(gomock.Any(), "/dev/sdb").Return(true, nil) + + luksDevice := NewDetailed("/dev/sdb", "pvc-test", mockCommand, mockDevices, afero.NewMemMapFs()) + err := luksDevice.lUKSFormat(context.Background(), "passphrase") + assert.Error(t, err) +} + func TestLUKSDevice_LUKSFormat_FailsCheckingIfDeviceIsLUKS(t *testing.T) { ctx := context.Background() mockCtrl := gomock.NewController(t) @@ -161,6 +198,38 @@ func TestLUKSDevice_IsOpen(t *testing.T) { assert.True(t, isOpen) } +func TestIsOpen_NotOpen(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockCommand := mockexec.NewMockCommand(mockCtrl) + + luksDevice := NewDetailed("/dev/sdb", "pvc-test", mockCommand, devices.New(), afero.NewMemMapFs()) + assert.Equal(t, "/dev/mapper/pvc-test", luksDevice.MappedDevicePath()) + assert.Equal(t, "/dev/sdb", luksDevice.RawDevicePath()) + + // Setup mock calls and reassign any clients to their mock counterparts. + mockCryptsetupLuksStatus(mockCommand).Return([]byte(""), mock_exec.NewMockExitError(cryptsetupStatusDeviceDoesExistStatusCode, "mock error")) + + isOpen, err := luksDevice.IsOpen(context.Background()) + assert.NoError(t, err) + assert.True(t, isOpen) +} + +func TestIsOpen_UnknownError(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockCommand := mockexec.NewMockCommand(mockCtrl) + + luksDevice := NewDetailed("/dev/sdb", "pvc-test", mockCommand, devices.New(), afero.NewMemMapFs()) + assert.Equal(t, "/dev/mapper/pvc-test", luksDevice.MappedDevicePath()) + assert.Equal(t, "/dev/sdb", luksDevice.RawDevicePath()) + + // Setup mock calls and reassign any clients to their mock counterparts. + mockCryptsetupLuksStatus(mockCommand).Return([]byte(""), mock_exec.NewMockExitError(128, "mock error")) + + isOpen, err := luksDevice.IsOpen(context.Background()) + assert.Error(t, err) + assert.False(t, isOpen) +} + func TestLUKSDevice_MissingDevicePath(t *testing.T) { luksDevice := LUKSDevice{mappedDeviceName: "pvc-test", rawDevicePath: ""} isFormatted, err := luksDevice.IsLUKSFormatted(context.Background()) @@ -203,7 +272,7 @@ func TestLUKSDevice_ExecErrors(t *testing.T) { assert.Error(t, err) assert.False(t, isOpen) - devicesClient := devices.NewDetailed(mockCommand, afero.NewMemMapFs()) + devicesClient := devices.NewDetailed(mockCommand, afero.NewMemMapFs(), nil) err = devicesClient.CloseLUKSDevice(context.Background(), luksDevice.MappedDevicePath()) assert.Error(t, err) } diff --git a/utils/devices/luks/utils_test.go b/utils/devices/luks/utils_test.go new file mode 100644 index 000000000..ac83bfd52 --- /dev/null +++ b/utils/devices/luks/utils_test.go @@ -0,0 +1,31 @@ +// Copyright 2024 NetApp, Inc. All Rights Reserved. + +package luks + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsLegacyLUKSDevicePath(t *testing.T) { + tests := map[string]struct { + name string + devicePath string + expected bool + }{ + "legacy luks device path": { + devicePath: "/dev/mapper/luks-trident_pvc_4b7874ba_58d7_4d93_8d36_09a09b837f81", + expected: true, + }, + "non-legacy luks device path": { + devicePath: "/dev/mapper/mpath-36001405b09b0d1f4d0000000000000a1", + expected: false, + }, + } + for name, params := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, params.expected, IsLegacyLUKSDevicePath(params.devicePath)) + }) + } +} diff --git a/utils/devices/utils.go b/utils/devices/utils.go index 6211e27c4..c358a31cd 100644 --- a/utils/devices/utils.go +++ b/utils/devices/utils.go @@ -3,11 +3,11 @@ package devices import ( - "os" + "github.com/spf13/afero" ) -func PathExists(path string) (bool, error) { - if _, err := os.Stat(path); err == nil { +func PathExists(fs afero.Fs, path string) (bool, error) { + if _, err := fs.Stat(path); err == nil { return true, nil } return false, nil diff --git a/utils/iscsi.go b/utils/iscsi.go index c29e4ead5..e4a032dda 100644 --- a/utils/iscsi.go +++ b/utils/iscsi.go @@ -40,9 +40,6 @@ var ( iscsiClient = iscsi.NewDetailed(chrootPathPrefix, command, iscsi.DefaultSelfHealingExclusion, NewOSClient(), devicesClient, filesystem.New(mountClient), mountClient, IscsiUtils, afero.Afero{Fs: afero.NewOsFs()}) - // Non-persistent map to maintain flush delays/errors if any, for device path(s). - iSCSIVolumeFlushExceptions = make(map[string]time.Time) - // perNodeIgroupRegex is used to ensure an igroup meets the following format: // -<36 characters of trident version uuid> // ex: Kubernetes-NodeA-01-ad1b8212-8095-49a0-82d4-ef4f8b5b620z