diff --git a/mount_linux.go b/mount_linux.go index 2a910024297..2e87f7169fd 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -19,7 +19,6 @@ limitations under the License. package mount import ( - "errors" "fmt" "os" "os/exec" @@ -289,55 +288,53 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, } } - // Try to mount the disk - klog.V(4).Infof("Attempting to mount disk: %s %s %s", fstype, source, target) - mountErr := mounter.Interface.Mount(source, target, fstype, options) - if mountErr != nil { - // Mount failed. This indicates either that the disk is unformatted or - // it contains an unexpected filesystem. - existingFormat, err := mounter.GetDiskFormat(source) - if err != nil { - return err + // Check if the disk is already formatted + existingFormat, err := mounter.GetDiskFormat(source) + if err != nil { + return err + } + + // Use 'ext4' as the default + if len(fstype) == 0 { + fstype = "ext4" + } + + if existingFormat == "" { + // Do not attempt to format the disk if mounting as readonly, return an error to reflect this. + if readOnly { + return fmt.Errorf("cannot mount unformatted disk %s as we are manipulating it in read-only mode", source) } - if existingFormat == "" { - if readOnly { - // Don't attempt to format if mounting as readonly, return an error to reflect this. - return errors.New("failed to mount unformatted volume as read only") - } - // Disk is unformatted so format it. - args := []string{source} - // Use 'ext4' as the default - if len(fstype) == 0 { - fstype = "ext4" + // Disk is unformatted so format it. + args := []string{source} + if fstype == "ext4" || fstype == "ext3" { + args = []string{ + "-F", // Force flag + "-m0", // Zero blocks reserved for super-user + source, } + } - if fstype == "ext4" || fstype == "ext3" { - args = []string{ - "-F", // Force flag - "-m0", // Zero blocks reserved for super-user - source, - } - } - klog.Infof("Disk %q appears to be unformatted, attempting to format as type: %q with options: %v", source, fstype, args) - _, err := mounter.Exec.Command("mkfs."+fstype, args...).CombinedOutput() - if err == nil { - // the disk has been formatted successfully try to mount it again. - klog.Infof("Disk successfully formatted (mkfs): %s - %s %s", fstype, source, target) - return mounter.Interface.Mount(source, target, fstype, options) - } + klog.Infof("Disk %q appears to be unformatted, attempting to format as type: %q with options: %v", source, fstype, args) + _, err := mounter.Exec.Command("mkfs."+fstype, args...).CombinedOutput() + if err != nil { klog.Errorf("format of disk %q failed: type:(%q) target:(%q) options:(%q)error:(%v)", source, fstype, target, options, err) return err } - // Disk is already formatted and failed to mount - if len(fstype) == 0 || fstype == existingFormat { - // This is mount error - return mountErr - } - // Block device is formatted with unexpected filesystem, let the user know - return fmt.Errorf("failed to mount the volume as %q, it already contains %s. Mount error: %v", fstype, existingFormat, mountErr) + + klog.Infof("Disk successfully formatted (mkfs): %s - %s %s", fstype, source, target) + } else if fstype != existingFormat { + // Verify that the disk is formatted with filesystem type we are expecting + klog.Warningf("Configured to mount disk %s as %s but current format is %s, things might break", source, existingFormat, fstype) } - return mountErr + + // Mount the disk + klog.V(4).Infof("Attempting to mount disk %s in %s format at %s", source, fstype, target) + if err := mounter.Interface.Mount(source, target, fstype, options); err != nil { + return err + } + + return nil } // GetDiskFormat uses 'blkid' to see if the given disk is unformatted diff --git a/safe_format_and_mount_test.go b/safe_format_and_mount_test.go index 34bf92fbc25..17de98b1d33 100644 --- a/safe_format_and_mount_test.go +++ b/safe_format_and_mount_test.go @@ -25,7 +25,7 @@ import ( "testing" "k8s.io/utils/exec" - "k8s.io/utils/exec/testing" + testingexec "k8s.io/utils/exec/testing" ) type ErrorMounter struct { @@ -68,15 +68,37 @@ func TestSafeFormatAndMount(t *testing.T) { expectedError error }{ { - description: "Test a read only mount", + description: "Test a read only mount of an already formatted device", fstype: "ext4", mountOptions: []string{"ro"}, + execScripts: []ExecArgs{ + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + }, }, { - description: "Test a normal mount", + description: "Test a normal mount of an already formatted device", fstype: "ext4", execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + }, + }, + { + description: "Test a read only mount of unformatted device", + fstype: "ext4", + mountOptions: []string{"ro"}, + 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"), + }, + { + description: "Test a normal mount of unformatted device", + fstype: "ext4", + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, + {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, }, { @@ -84,6 +106,7 @@ func TestSafeFormatAndMount(t *testing.T) { fstype: "ext4", execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, }, expectedError: fmt.Errorf("'fsck' found errors on device /dev/foo but could not correct them"), }, @@ -92,6 +115,7 @@ func TestSafeFormatAndMount(t *testing.T) { fstype: "ext4", execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, }, }, { @@ -99,6 +123,7 @@ func TestSafeFormatAndMount(t *testing.T) { fstype: "ext4", execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 8}}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, }, }, { @@ -107,7 +132,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nPTTYPE=dos\n", nil}, }, expectedError: fmt.Errorf("unknown filesystem type '(null)'"), }, @@ -125,18 +150,17 @@ func TestSafeFormatAndMount(t *testing.T) { { description: "Test that 'blkid' is called and confirms unformatted disk, format passes, second mount fails", fstype: "ext4", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), fmt.Errorf("Still cannot mount")}, + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"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("Still cannot mount"), + expectedError: fmt.Errorf("unknown filesystem type '(null)'"), }, { - description: "Test that 'blkid' is called and confirms unformatted disk, format passes, second mount passes", + description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes", fstype: "ext4", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, @@ -145,9 +169,8 @@ func TestSafeFormatAndMount(t *testing.T) { expectedError: nil, }, { - description: "Test that 'blkid' is called and confirms unformatted disk, format passes, second mount passes with ext3", + description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes with ext3", fstype: "ext3", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, @@ -158,7 +181,6 @@ func TestSafeFormatAndMount(t *testing.T) { { description: "test that none ext4 fs does not get called with ext4 options.", fstype: "xfs", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, @@ -168,13 +190,11 @@ func TestSafeFormatAndMount(t *testing.T) { }, { description: "Test that 'blkid' is called and reports ext4 partition", - fstype: "ext3", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, + fstype: "ext4", execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nPTTYPE=dos\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, }, - expectedError: fmt.Errorf("failed to mount the volume as \"ext3\", it already contains unknown data, probably partitions. Mount error: unknown filesystem type '(null)'"), }, { description: "Test that 'blkid' is called but has some usage or other errors (an exit code of 4 is returned)",