From 75359c0b94d2f3d9f487a47e174c1d9ce7ccf36e Mon Sep 17 00:00:00 2001 From: Yecheng Fu Date: Tue, 30 Jan 2018 23:58:44 +0800 Subject: [PATCH 1/3] Use `blkid` to get fs type of device. `lsblk` reads fs type info from udev files. If udev rules are not installed. `lsblk` could not get correct fs type. This will cause problems, e.g. expanding volume depends on fs type of disk. --- pkg/util/mount/mount_linux.go | 25 ++++--------- pkg/util/mount/safe_format_and_mount_test.go | 38 ++++++++------------ 2 files changed, 21 insertions(+), 42 deletions(-) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index 71064f321e..a8d99963ff 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -536,11 +536,11 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, return mountErr } -// GetDiskFormat uses 'lsblk' to see if the given disk is unformated +// GetDiskFormat uses 'blkid' to see if the given disk is unformated func (mounter *SafeFormatAndMount) GetDiskFormat(disk string) (string, error) { - args := []string{"-n", "-o", "FSTYPE", disk} - glog.V(4).Infof("Attempting to determine if disk %q is formatted using lsblk with args: (%v)", disk, args) - dataOut, err := mounter.Exec.Run("lsblk", args...) + args := []string{"-p", "-s", "TYPE", "-o", "value", disk} + glog.V(4).Infof("Attempting to determine if disk %q is formatted using blkid with args: (%v)", disk, args) + dataOut, err := mounter.Exec.Run("blkid", args...) output := string(dataOut) glog.V(4).Infof("Output: %q", output) @@ -549,23 +549,12 @@ func (mounter *SafeFormatAndMount) GetDiskFormat(disk string) (string, error) { return "", err } - // Split lsblk output into lines. Unformatted devices should contain only - // "\n". Beware of "\n\n", that's a device with one empty partition. - output = strings.TrimSuffix(output, "\n") // Avoid last empty line - lines := strings.Split(output, "\n") - if lines[0] != "" { - // The device is formatted - return lines[0], nil - } - - if len(lines) == 1 { - // The device is unformatted and has no dependent devices + if len(output) <= 0 { + // no fs type return "", nil } - // The device has dependent devices, most probably partitions (LVM, LUKS - // and MD RAID are reported as FSTYPE and caught above). - return "unknown data, probably partitions", nil + return strings.TrimSpace(output), nil } // isShared returns true, if given path is on a mount point that has shared diff --git a/pkg/util/mount/safe_format_and_mount_test.go b/pkg/util/mount/safe_format_and_mount_test.go index a7e7cc29a2..0e505622c6 100644 --- a/pkg/util/mount/safe_format_and_mount_test.go +++ b/pkg/util/mount/safe_format_and_mount_test.go @@ -100,55 +100,55 @@ func TestSafeFormatAndMount(t *testing.T) { }, }, { - description: "Test that 'lsblk' is called and fails", + description: "Test that 'blkid' is called and fails", fstype: "ext4", mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "ext4\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-o", "value", "/dev/foo"}, "ext4\n", nil}, }, expectedError: fmt.Errorf("unknown filesystem type '(null)'"), }, { - description: "Test that 'lsblk' is called and confirms unformatted disk, format fails", + description: "Test that 'blkid' 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{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-o", "value", "/dev/foo"}, "", nil}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", fmt.Errorf("formatting failed")}, }, expectedError: fmt.Errorf("formatting failed"), }, { - description: "Test that 'lsblk' is called and confirms unformatted disk, format passes, second mount fails", + 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")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-o", "value", "/dev/foo"}, "", nil}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: fmt.Errorf("Still cannot mount"), }, { - description: "Test that 'lsblk' is called and confirms unformatted disk, format passes, second mount passes", + description: "Test that 'blkid' 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{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-o", "value", "/dev/foo"}, "", nil}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: nil, }, { - description: "Test that 'lsblk' 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, 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{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-o", "value", "/dev/foo"}, "", nil}, {"mkfs.ext3", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: nil, @@ -159,30 +159,20 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-o", "value", "/dev/foo"}, "", nil}, {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, }, expectedError: nil, }, { - description: "Test that 'lsblk' is called and reports ext4 partition", + description: "Test that 'blkid' is called and reports ext4 partition", fstype: "ext3", mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "\next4\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-o", "value", "/dev/foo"}, "LVM2_member\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 'lsblk' is called and reports empty partition", - fstype: "ext3", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, - execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n\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)'"), + expectedError: fmt.Errorf("failed to mount the volume as \"ext3\", it already contains LVM2_member. Mount error: unknown filesystem type '(null)'"), }, } From 5136938ff6eef20adbccc6385b019daf66fd994b Mon Sep 17 00:00:00 2001 From: Yecheng Fu Date: Fri, 2 Feb 2018 18:47:24 +0800 Subject: [PATCH 2/3] Use `blkid` to get fs type of device. If a parition table type is detected, returns a special non-empty string as filesystem type. --- pkg/util/mount/mount_linux.go | 33 +++++++++++++++++--- pkg/util/mount/safe_format_and_mount_test.go | 16 +++++----- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index a8d99963ff..78a48c4a78 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -538,7 +538,7 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, // GetDiskFormat uses 'blkid' to see if the given disk is unformated func (mounter *SafeFormatAndMount) GetDiskFormat(disk string) (string, error) { - args := []string{"-p", "-s", "TYPE", "-o", "value", disk} + args := []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", disk} glog.V(4).Infof("Attempting to determine if disk %q is formatted using blkid with args: (%v)", disk, args) dataOut, err := mounter.Exec.Run("blkid", args...) output := string(dataOut) @@ -549,12 +549,35 @@ func (mounter *SafeFormatAndMount) GetDiskFormat(disk string) (string, error) { return "", err } - if len(output) <= 0 { - // no fs type - return "", nil + var fstype, pttype string + + lines := strings.Split(output, "\n") + for _, l := range lines { + if len(l) <= 0 { + // Ignore empty line. + continue + } + cs := strings.Split(l, "=") + if len(cs) != 2 { + return "", fmt.Errorf("blkid returns invalid output: %s", output) + } + // TYPE is filesystem type, and PTTYPE is partition table type, according + // to https://www.kernel.org/pub/linux/utils/util-linux/v2.21/libblkid-docs/. + if cs[0] == "TYPE" { + fstype = cs[1] + } else if cs[0] == "PTTYPE" { + pttype = cs[1] + } } - return strings.TrimSpace(output), nil + if len(pttype) > 0 { + glog.V(4).Infof("Disk %s detected partition table type: %s", pttype) + // Returns a special non-empty string as filesystem type, then kubelet + // will not format it. + return "unknown data, probably partitions", nil + } + + return fstype, nil } // isShared returns true, if given path is on a mount point that has shared diff --git a/pkg/util/mount/safe_format_and_mount_test.go b/pkg/util/mount/safe_format_and_mount_test.go index 0e505622c6..e09af825c9 100644 --- a/pkg/util/mount/safe_format_and_mount_test.go +++ b/pkg/util/mount/safe_format_and_mount_test.go @@ -105,7 +105,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", "-o", "value", "/dev/foo"}, "ext4\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, }, expectedError: fmt.Errorf("unknown filesystem type '(null)'"), }, @@ -115,7 +115,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", "-o", "value", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", nil}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", fmt.Errorf("formatting failed")}, }, expectedError: fmt.Errorf("formatting failed"), @@ -126,7 +126,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), fmt.Errorf("Still cannot mount")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"blkid", []string{"-p", "-s", "TYPE", "-o", "value", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", nil}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: fmt.Errorf("Still cannot mount"), @@ -137,7 +137,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"blkid", []string{"-p", "-s", "TYPE", "-o", "value", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", nil}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: nil, @@ -148,7 +148,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"blkid", []string{"-p", "-s", "TYPE", "-o", "value", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", nil}, {"mkfs.ext3", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: nil, @@ -159,7 +159,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"blkid", []string{"-p", "-s", "TYPE", "-o", "value", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", nil}, {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, }, expectedError: nil, @@ -170,9 +170,9 @@ 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", "-o", "value", "/dev/foo"}, "LVM2_member\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nPTTYPE=dos\n", nil}, }, - expectedError: fmt.Errorf("failed to mount the volume as \"ext3\", it already contains LVM2_member. Mount error: unknown filesystem type '(null)'"), + expectedError: fmt.Errorf("failed to mount the volume as \"ext3\", it already contains unknown data, probably partitions. Mount error: unknown filesystem type '(null)'"), }, } From 322c09484169d32b3f0fe0a5f9c57d7a6b7d65fc Mon Sep 17 00:00:00 2001 From: Yecheng Fu Date: Mon, 5 Feb 2018 11:21:35 +0800 Subject: [PATCH 3/3] Use `blkid` to get fs type of device. For `blkid`, if the specified token (TYPE/PTTYPE, etc) was not found, or no (specified) devices could be identified, an exit code of 2 is returned. --- pkg/util/mount/mount_linux.go | 11 +++++++++- pkg/util/mount/safe_format_and_mount_test.go | 21 +++++++++++++++----- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index 78a48c4a78..f5e168b5b9 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -542,9 +542,18 @@ func (mounter *SafeFormatAndMount) GetDiskFormat(disk string) (string, error) { glog.V(4).Infof("Attempting to determine if disk %q is formatted using blkid with args: (%v)", disk, args) dataOut, err := mounter.Exec.Run("blkid", args...) output := string(dataOut) - glog.V(4).Infof("Output: %q", output) + glog.V(4).Infof("Output: %q, err: %v", output, err) if err != nil { + if exit, ok := err.(utilexec.ExitError); ok { + if exit.ExitStatus() == 2 { + // Disk device is unformatted. + // For `blkid`, if the specified token (TYPE/PTTYPE, etc) was + // not found, or no (specified) devices could be identified, an + // exit code of 2 is returned. + return "", nil + } + } glog.Errorf("Could not determine if disk %q is formatted (%v)", disk, err) return "", err } diff --git a/pkg/util/mount/safe_format_and_mount_test.go b/pkg/util/mount/safe_format_and_mount_test.go index e09af825c9..20bfeb9f62 100644 --- a/pkg/util/mount/safe_format_and_mount_test.go +++ b/pkg/util/mount/safe_format_and_mount_test.go @@ -115,7 +115,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"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &fakeexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", fmt.Errorf("formatting failed")}, }, expectedError: fmt.Errorf("formatting failed"), @@ -126,7 +126,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), fmt.Errorf("Still cannot mount")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &fakeexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: fmt.Errorf("Still cannot mount"), @@ -137,7 +137,7 @@ func TestSafeFormatAndMount(t *testing.T) { 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"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &fakeexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: nil, @@ -148,7 +148,7 @@ func TestSafeFormatAndMount(t *testing.T) { 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"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &fakeexec.FakeExitError{Status: 2}}, {"mkfs.ext3", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: nil, @@ -159,7 +159,7 @@ func TestSafeFormatAndMount(t *testing.T) { 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"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &fakeexec.FakeExitError{Status: 2}}, {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, }, expectedError: nil, @@ -174,6 +174,17 @@ func TestSafeFormatAndMount(t *testing.T) { }, 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)", + 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"}, "", &fakeexec.FakeExitError{Status: 4}}, + {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, + }, + expectedError: fmt.Errorf("exit 4"), + }, } for _, test := range tests {