Validate the existence of filesystem before attempting to mount it (linux)

mount_linux.go

@@ -19,7 +19,6 @@ limitations under the License.
 package mount
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -289,29 +288,25 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
 		}
 	}
 
-	// Try to mount the disk
+	// Check if the disk is already formatted
-	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
 	}
-		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"
 	}
 
+	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)
+		}
+
+		// Disk is unformatted so format it.
+		args := []string{source}
 		if fstype == "ext4" || fstype == "ext3" {
 			args = []string{
 				"-F",  // Force flag
@@ -319,25 +314,27 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
 				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 {
+		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.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 {
+		klog.Infof("Disk successfully formatted (mkfs): %s - %s %s", fstype, source, target)
-			// This is mount error
+	} else if fstype != existingFormat {
-			return mountErr
+		// 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)
 	}
-		// 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)
+	// 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 mountErr
+
+	return nil
 }
 
 // GetDiskFormat uses 'blkid' to see if the given disk is unformatted

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",
+			fstype:      "ext4",
-			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\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)",