From 5d34e5006ab85290ce8c820645c9ee91f3b2d2dd Mon Sep 17 00:00:00 2001 From: David Zhu Date: Thu, 23 Jan 2020 19:25:03 -0800 Subject: [PATCH] FormatAndMount unit test only checks for MountErrorValue now and closed gaps for some error values --- mount.go | 8 ++++--- mount_linux.go | 4 ++-- safe_format_and_mount_test.go | 44 +++++++++++++++++------------------ 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/mount.go b/mount.go index 0e7d1087044..46d77327fbf 100644 --- a/mount.go +++ b/mount.go @@ -84,6 +84,8 @@ const ( HasFilesystemErrors MountErrorType = "HasFilesystemErrors" UnformattedReadOnly MountErrorType = "UnformattedReadOnly" FormatFailed MountErrorType = "FormatFailed" + GetDiskFormatFailed MountErrorType = "GetDiskFormatFailed" + UnknownMountError MountErrorType = "UnknownMountError" ) type MountError struct { @@ -91,16 +93,16 @@ type MountError struct { Message string } -func (mountError *MountError) String() string { +func (mountError MountError) String() string { return mountError.Message } -func (mountError *MountError) Error() string { +func (mountError MountError) Error() string { return mountError.Message } func NewMountError(mountErrorValue MountErrorType, format string, args ...interface{}) error { - mountError := &MountError{ + mountError := MountError{ Type: mountErrorValue, Message: fmt.Sprintf(format, args...), } diff --git a/mount_linux.go b/mount_linux.go index c4485162987..4dc696d69b0 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -315,12 +315,12 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, } options = append(options, "defaults") - var mountErrorValue MountErrorType + mountErrorValue := UnknownMountError // Check if the disk is already formatted existingFormat, err := mounter.GetDiskFormat(source) if err != nil { - return err + return NewMountError(GetDiskFormatFailed, "failed to get disk format of disk %s: %v", source, err) } // Use 'ext4' as the default diff --git a/safe_format_and_mount_test.go b/safe_format_and_mount_test.go index 5a8727134bd..a0869293656 100644 --- a/safe_format_and_mount_test.go +++ b/safe_format_and_mount_test.go @@ -21,7 +21,6 @@ import ( "io/ioutil" "os" "runtime" - "strings" "testing" "k8s.io/utils/exec" @@ -60,12 +59,12 @@ func TestSafeFormatAndMount(t *testing.T) { } defer os.RemoveAll(mntDir) tests := []struct { - description string - fstype string - mountOptions []string - execScripts []ExecArgs - mountErrs []error - expectedError error + description string + fstype string + mountOptions []string + execScripts []ExecArgs + mountErrs []error + expErrorType MountErrorType }{ { description: "Test a read only mount of an already formatted device", @@ -90,7 +89,7 @@ func TestSafeFormatAndMount(t *testing.T) { execScripts: []ExecArgs{ {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, }, - expectedError: fmt.Errorf("cannot mount unformatted disk /dev/foo as we are manipulating it in read-only mode"), + expErrorType: UnformattedReadOnly, }, { description: "Test a normal mount of unformatted device", @@ -107,7 +106,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}}, }, - expectedError: fmt.Errorf("'fsck' found errors on device /dev/foo but could not correct them"), + expErrorType: HasFilesystemErrors, }, { description: "Test 'fsck' fails with exit status 1 (errors found and corrected)", @@ -133,7 +132,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nPTTYPE=dos\n", nil}, {"fsck", []string{"-a", "/dev/foo"}, "", nil}, }, - expectedError: fmt.Errorf("unknown filesystem type '(null)'"), + expErrorType: FilesystemMismatch, }, { description: "Test that 'blkid' is called and confirms unformatted disk, format fails", @@ -143,7 +142,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", fmt.Errorf("formatting failed")}, }, - expectedError: fmt.Errorf("formatting failed"), + expErrorType: FormatFailed, }, { description: "Test that 'blkid' is called and confirms unformatted disk, format passes, second mount fails", @@ -153,7 +152,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, - expectedError: fmt.Errorf("unknown filesystem type '(null)'"), + expErrorType: UnknownMountError, }, { description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes", @@ -162,7 +161,6 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, - expectedError: nil, }, { description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes with ext3", @@ -171,7 +169,6 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext3", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, - expectedError: nil, }, { description: "test that none ext4 fs does not get called with ext4 options.", @@ -180,7 +177,6 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, }, - expectedError: nil, }, { description: "Test that 'blkid' is called and reports ext4 partition", @@ -198,7 +194,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}}, {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, }, - expectedError: fmt.Errorf("exit 4"), + expErrorType: GetDiskFormatFailed, }, { description: "Test that 'xfs_repair' is called only once, no need to repair the filesystem", @@ -207,7 +203,6 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil}, {"xfs_repair", []string{"-n", "/dev/foo"}, "", nil}, }, - expectedError: nil, }, { description: "Test that 'xfs_repair' is called twice and repair the filesystem", @@ -217,7 +212,6 @@ func TestSafeFormatAndMount(t *testing.T) { {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, {"xfs_repair", []string{"/dev/foo"}, "\ndone\n", nil}, }, - expectedError: nil, }, { description: "Test that 'xfs_repair' is called twice and repair the filesystem, but mount failed", @@ -228,7 +222,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, {"xfs_repair", []string{"/dev/foo"}, "\ndone\n", nil}, }, - expectedError: fmt.Errorf("unknown filesystem type '(null)'"), + expErrorType: UnknownMountError, }, { description: "Test that 'xfs_repair' is called twice but could not repair the filesystem", @@ -238,7 +232,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, {"xfs_repair", []string{"/dev/foo"}, "\nAn error occurred\n", &testingexec.FakeExitError{Status: 1}}, }, - expectedError: fmt.Errorf("'xfs_repair' found errors on device %s but could not correct them: %v", "/dev/foo", "\nAn error occurred\n"), + expErrorType: HasFilesystemErrors, }, } @@ -260,7 +254,7 @@ func TestSafeFormatAndMount(t *testing.T) { device := "/dev/foo" dest := mntDir err := mounter.FormatAndMount(device, dest, test.fstype, test.mountOptions) - if test.expectedError == nil { + if len(test.expErrorType) == 0 { if err != nil { t.Errorf("test \"%s\" unexpected non-error: %v", test.description, err) } @@ -277,8 +271,12 @@ func TestSafeFormatAndMount(t *testing.T) { t.Errorf("test \"%s\" the correct device was not mounted", test.description) } } else { - if err == nil || !strings.HasPrefix(err.Error(), test.expectedError.Error()) { - t.Errorf("test \"%s\" unexpected error: \n [%v]. \nExpecting [%v]", test.description, err, test.expectedError) + mntErr, ok := err.(MountError) + if !ok { + t.Errorf("mount error not of mount error type: %v", err) + } + if mntErr.Type != test.expErrorType { + t.Errorf("test \"%s\" unexpected error: \n [%v]. \nExpecting err type[%v]", test.description, err, test.expErrorType) } } }