From 10688f1a114869bf0b01128774a794c014953af2 Mon Sep 17 00:00:00 2001 From: Sami Wagiaalla Date: Fri, 18 Sep 2015 17:25:22 -0400 Subject: [PATCH] Run fsck before formatting disk Signed-off-by: Sami Wagiaalla --- pkg/util/mount/mount_linux.go | 23 ++++- pkg/util/mount/safe_format_and_mount_test.go | 94 ++++++++++++++------ 2 files changed, 87 insertions(+), 30 deletions(-) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index cf1f1fa6d50..ee75022fbd6 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -30,6 +30,7 @@ import ( "syscall" "github.com/golang/glog" + utilExec "k8s.io/kubernetes/pkg/util/exec" ) const ( @@ -240,13 +241,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() == 1: + glog.Infof("Device %s has errors which were corrected by fsck.", source) + case isExitError && ee.ExitStatus() == 4: + return fmt.Errorf("'fsck' found errors on device %s but could not correct them: %s.", source, string(out)) + case isExitError && ee.ExitStatus() > 4: + 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 { diff --git a/pkg/util/mount/safe_format_and_mount_test.go b/pkg/util/mount/safe_format_and_mount_test.go index d859b1e6b92..ad9bd91c506 100644 --- a/pkg/util/mount/safe_format_and_mount_test.go +++ b/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", - }, - - { // Test that 'lsblk' is called and fails - fstype: "ext4", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, + { + description: "Test a normal mount", + fstype: "ext4", execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + }, + }, + { + 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) } } }