Merge pull request #14200 from swagiaal/run-fsck-before-format

pkg/util/mount/mount_linux.go

@@ -30,6 +30,7 @@ import (
 	"syscall"
 
 	"github.com/golang/glog"
+	utilExec "k8s.io/kubernetes/pkg/util/exec"
 )
 
 const (
@@ -41,6 +42,13 @@ const (
 	procMountsPath = "/proc/mounts"
 )
 
+const (
+	// 'fsck' found errors and corrected them
+	fsckErrorsCorrected = 1
+	// 'fsck' found errors but exited without correcting them
+	fsckErrorsUncorrected = 4
+)
+
 // Mounter provides the default implementation of mount.Interface
 // for the linux platform.  This implementation assumes that the
 // kubelet is running in the host's root mount namespace.
@@ -240,13 +248,31 @@ func readProcMountsFrom(file io.Reader, out *[]MountPoint) (uint32, error) {
 func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error {
 	options = append(options, "defaults")
 
+	// Run fsck on the disk to fix repairable issues
+	args := []string{"-a", source}
+	cmd := mounter.Runner.Command("fsck", args...)
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		ee, isExitError := err.(utilExec.ExitError)
+		switch {
+		case err == utilExec.ErrExecutableNotFound:
+			glog.Warningf("'fsck' not found on system; continuing mount without running 'fsck'.")
+		case isExitError && ee.ExitStatus() == fsckErrorsCorrected:
+			glog.Infof("Device %s has errors which were corrected by fsck.", source)
+		case isExitError && ee.ExitStatus() == fsckErrorsUncorrected:
+			return fmt.Errorf("'fsck' found errors on device %s but could not correct them: %s.", source, string(out))
+		case isExitError && ee.ExitStatus() > fsckErrorsUncorrected:
+			glog.Infof("`fsck` error %s", string(out))
+		}
+	}
+
 	// Try to mount the disk
-	err := mounter.Interface.Mount(source, target, fstype, options)
+	err = mounter.Interface.Mount(source, target, fstype, options)
 	if err != nil {
 		// It is possible that this disk is not formatted. Double check using diskLooksUnformatted
 		notFormatted, err := mounter.diskLooksUnformatted(source)
 		if err == nil && notFormatted {
-			args := []string{source}
+			args = []string{source}
 			// Disk is unformatted so format it.
 			// Use 'ext4' as the default
 			if len(fstype) == 0 {

pkg/util/mount/safe_format_and_mount_test.go

@@ -47,69 +47,107 @@ type ExecArgs struct {
 
 func TestSafeFormatAndMount(t *testing.T) {
 	tests := []struct {
+		description   string
 		fstype        string
 		mountOptions  []string
 		execScripts   []ExecArgs
 		mountErrs     []error
 		expectedError error
 	}{
-		{ // Test a read only mount
+		{
+			description:  "Test a read only mount",
 			fstype:       "ext4",
 			mountOptions: []string{"ro"},
 		},
-		{ // Test a normal mount
-			fstype: "ext4",
+		{
+			description: "Test a normal mount",
+			fstype:      "ext4",
+			execScripts: []ExecArgs{
+				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
+			},
 		},
-
-		{ // Test that 'lsblk' is called and fails
-			fstype:    "ext4",
-			mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")},
+		{
+			description: "Test 'fsck' fails with exit status 4",
+			fstype:      "ext4",
+			execScripts: []ExecArgs{
+				{"fsck", []string{"-a", "/dev/foo"}, "", &exec.FakeExitError{Status: 4}},
+			},
+			expectedError: fmt.Errorf("'fsck' found errors on device /dev/foo but could not correct them: ."),
+		},
+		{
+			description: "Test 'fsck' fails with exit status 1 (errors found and corrected)",
+			fstype:      "ext4",
+			execScripts: []ExecArgs{
+				{"fsck", []string{"-a", "/dev/foo"}, "", &exec.FakeExitError{Status: 1}},
+			},
+		},
+		{
+			description: "Test 'fsck' fails with exit status other than 1 and 4 (likely unformatted device)",
+			fstype:      "ext4",
+			execScripts: []ExecArgs{
+				{"fsck", []string{"-a", "/dev/foo"}, "", &exec.FakeExitError{Status: 8}},
+			},
+		},
+		{
+			description: "Test that 'lsblk' is called and fails",
+			fstype:      "ext4",
+			mountErrs:   []error{fmt.Errorf("unknown filesystem type '(null)'")},
 			execScripts: []ExecArgs{
+				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "ext4", nil},
 			},
 			expectedError: fmt.Errorf("unknown filesystem type '(null)'"),
 		},
-		{ // Test that 'lsblk' is called and confirms unformatted disk, format fails
-			fstype:    "ext4",
-			mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")},
+		{
+			description: "Test that 'lsblk' is called and confirms unformatted disk, format fails",
+			fstype:      "ext4",
+			mountErrs:   []error{fmt.Errorf("unknown filesystem type '(null)'")},
 			execScripts: []ExecArgs{
+				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "", nil},
 				{"mkfs.ext4", []string{"-E", "lazy_itable_init=0,lazy_journal_init=0", "-F", "/dev/foo"}, "", fmt.Errorf("formatting failed")},
 			},
 			expectedError: fmt.Errorf("formatting failed"),
 		},
-		{ // Test that 'lsblk' 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")},
+		{
+			description: "Test that 'lsblk' 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")},
 			execScripts: []ExecArgs{
+				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "", nil},
 				{"mkfs.ext4", []string{"-E", "lazy_itable_init=0,lazy_journal_init=0", "-F", "/dev/foo"}, "", nil},
 			},
 			expectedError: fmt.Errorf("Still cannot mount"),
 		},
-		{ // Test that 'lsblk' is called and confirms unformatted disk, format passes, second mount passes
-			fstype:    "ext4",
-			mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil},
+		{
+			description: "Test that 'lsblk' is called and confirms unformatted disk, format passes, second mount passes",
+			fstype:      "ext4",
+			mountErrs:   []error{fmt.Errorf("unknown filesystem type '(null)'"), nil},
 			execScripts: []ExecArgs{
+				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "", nil},
 				{"mkfs.ext4", []string{"-E", "lazy_itable_init=0,lazy_journal_init=0", "-F", "/dev/foo"}, "", nil},
 			},
 			expectedError: nil,
 		},
-		{ // Test that 'lsblk' is called and confirms unformatted disk, format passes, second mount passes with ext3
-			fstype:    "ext3",
-			mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil},
+		{
+			description: "Test that 'lsblk' is called and confirms unformatted disk, format passes, second mount passes with ext3",
+			fstype:      "ext3",
+			mountErrs:   []error{fmt.Errorf("unknown filesystem type '(null)'"), nil},
 			execScripts: []ExecArgs{
+				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "", nil},
 				{"mkfs.ext3", []string{"-E", "lazy_itable_init=0,lazy_journal_init=0", "-F", "/dev/foo"}, "", nil},
 			},
 			expectedError: nil,
 		},
-		{ // Test that 'lsblk' is called and confirms unformatted disk, format passes, second mount passes
-			// test that none ext4 fs does not get called with ext4 options.
-			fstype:    "xfs",
-			mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil},
+		{
+			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},
 				{"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "", nil},
 				{"mkfs.xfs", []string{"/dev/foo"}, "", nil},
 			},
@@ -159,23 +197,23 @@ func TestSafeFormatAndMount(t *testing.T) {
 		err := mounter.FormatAndMount(device, dest, test.fstype, test.mountOptions)
 		if test.expectedError == nil {
 			if err != nil {
-				t.Errorf("unexpected non-error: %v", err)
+				t.Errorf("test \"%s\" unexpected non-error: %v", test.description, err)
 			}
 
 			// Check that something was mounted on the directory
 			isNotMountPoint, err := fakeMounter.IsLikelyNotMountPoint(dest)
 			if err != nil || isNotMountPoint {
-				t.Errorf("the directory was not mounted")
+				t.Errorf("test \"%s\" the directory was not mounted", test.description)
 			}
 
 			//check that the correct device was mounted
 			mountedDevice, _, err := GetDeviceNameFromMount(fakeMounter.FakeMounter, dest)
 			if err != nil || mountedDevice != device {
-				t.Errorf("the correct device was not mounted")
+				t.Errorf("test \"%s\" the correct device was not mounted", test.description)
 			}
 		} else {
 			if err == nil || test.expectedError.Error() != err.Error() {
-				t.Errorf("unexpected error: %v. Expecting %v", err, test.expectedError)
+				t.Errorf("test \"%s\" unexpected error: \n          [%v]. \nExpecting [%v]", test.description, err, test.expectedError)
 			}
 		}
 	}