Merge pull request #81180 from codenrhoden/mount-pkg-lint

Fix most linting issues in pkg/util/mount

pkg/util/mount/exec.go

@@ -18,11 +18,12 @@ package mount
 
 import "k8s.io/utils/exec"
 
+// NewOsExec returns a new Exec interface implementation based on exec()
 func NewOsExec() Exec {
 	return &osExec{}
 }
 
-// Real implementation of Exec interface that uses simple util.Exec
+// Real implementation of Exec interface that uses simple utils.Exec
 type osExec struct{}
 
 var _ Exec = &osExec{}
@@ -32,16 +33,18 @@ func (e *osExec) Run(cmd string, args ...string) ([]byte, error) {
 	return exe.Command(cmd, args...).CombinedOutput()
 }
 
+// NewFakeExec returns a new FakeExec
 func NewFakeExec(run runHook) *FakeExec {
 	return &FakeExec{runHook: run}
 }
 
-// Fake for testing.
+// FakeExec for testing.
 type FakeExec struct {
 	runHook runHook
 }
 type runHook func(cmd string, args ...string) ([]byte, error)
 
+// Run executes the command using the optional runhook, if given
 func (f *FakeExec) Run(cmd string, args ...string) ([]byte, error) {
 	if f.runHook != nil {
 		return f.runHook(cmd, args...)

pkg/util/mount/fake.go

@@ -38,9 +38,12 @@ type FakeMounter struct {
 
 var _ Interface = &FakeMounter{}
 
-// Values for FakeAction.Action
+const (
-const FakeActionMount = "mount"
+	// FakeActionMount is the string for specifying mount as FakeAction.Action
-const FakeActionUnmount = "unmount"
+	FakeActionMount = "mount"
+	// FakeActionUnmount is the string for specifying unmount as FakeAction.Action
+	FakeActionUnmount = "unmount"
+)
 
 // FakeAction objects are logged every time a fake mount or unmount is called.
 type FakeAction struct {
@@ -50,6 +53,7 @@ type FakeAction struct {
 	FSType string // applies only to "mount" actions
 }
 
+// ResetLog clears all the log entries in FakeMounter
 func (f *FakeMounter) ResetLog() {
 	f.mutex.Lock()
 	defer f.mutex.Unlock()
@@ -57,6 +61,7 @@ func (f *FakeMounter) ResetLog() {
 	f.Log = []FakeAction{}
 }
 
+// Mount records the mount event and updates the in-memory mount points for FakeMounter
 func (f *FakeMounter) Mount(source string, target string, fstype string, options []string) error {
 	f.mutex.Lock()
 	defer f.mutex.Unlock()
@@ -99,6 +104,7 @@ func (f *FakeMounter) Mount(source string, target string, fstype string, options
 	return nil
 }
 
+// Unmount records the unmount event and updates the in-memory mount points for FakeMounter
 func (f *FakeMounter) Unmount(target string) error {
 	f.mutex.Lock()
 	defer f.mutex.Unlock()
@@ -124,6 +130,7 @@ func (f *FakeMounter) Unmount(target string) error {
 	return nil
 }
 
+// List returns all the in-memory mountpoints for FakeMounter
 func (f *FakeMounter) List() ([]MountPoint, error) {
 	f.mutex.Lock()
 	defer f.mutex.Unlock()
@@ -131,10 +138,13 @@ func (f *FakeMounter) List() ([]MountPoint, error) {
 	return f.MountPoints, nil
 }
 
+// IsMountPointMatch tests if dir and mp are the same path
 func (f *FakeMounter) IsMountPointMatch(mp MountPoint, dir string) bool {
 	return mp.Path == dir
 }
 
+// IsLikelyNotMountPoint determines whether a path is a mountpoint by checking
+// if the absolute path to file is in the in-memory mountpoints
 func (f *FakeMounter) IsLikelyNotMountPoint(file string) (bool, error) {
 	f.mutex.Lock()
 	defer f.mutex.Unlock()
@@ -165,6 +175,8 @@ func (f *FakeMounter) IsLikelyNotMountPoint(file string) (bool, error) {
 	return true, nil
 }
 
+// GetMountRefs finds all mount references to the path, returns a
+// list of paths.
 func (f *FakeMounter) GetMountRefs(pathname string) ([]string, error) {
 	realpath, err := filepath.EvalSymlinks(pathname)
 	if err != nil {
@@ -174,6 +186,7 @@ func (f *FakeMounter) GetMountRefs(pathname string) ([]string, error) {
 	return getMountRefsByDev(f, realpath)
 }
 
+// FakeHostUtil is a fake mount.HostUtils implementation for testing
 type FakeHostUtil struct {
 	MountPoints []MountPoint
 	Filesystem  map[string]FileType
@@ -183,6 +196,8 @@ type FakeHostUtil struct {
 
 var _ HostUtils = &FakeHostUtil{}
 
+// DeviceOpened checks if block device referenced by pathname is in use by
+// checking if is listed as a device in the in-memory mountpoint table.
 func (hu *FakeHostUtil) DeviceOpened(pathname string) (bool, error) {
 	hu.mutex.Lock()
 	defer hu.mutex.Unlock()
@@ -195,18 +210,24 @@ func (hu *FakeHostUtil) DeviceOpened(pathname string) (bool, error) {
 	return false, nil
 }
 
+// PathIsDevice always returns true
 func (hu *FakeHostUtil) PathIsDevice(pathname string) (bool, error) {
 	return true, nil
 }
 
+// GetDeviceNameFromMount given a mount point, find the volume id
 func (hu *FakeHostUtil) GetDeviceNameFromMount(mounter Interface, mountPath, pluginMountDir string) (string, error) {
 	return getDeviceNameFromMount(mounter, mountPath, pluginMountDir)
 }
 
+// MakeRShared checks if path is shared and bind-mounts it as rshared if needed.
+// No-op for testing
 func (hu *FakeHostUtil) MakeRShared(path string) error {
 	return nil
 }
 
+// GetFileType checks for file/directory/socket/block/character devices.
+// Defaults to Directory if otherwise unspecified.
 func (hu *FakeHostUtil) GetFileType(pathname string) (FileType, error) {
 	if t, ok := hu.Filesystem[pathname]; ok {
 		return t, nil
@@ -214,14 +235,19 @@ func (hu *FakeHostUtil) GetFileType(pathname string) (FileType, error) {
 	return FileType("Directory"), nil
 }
 
+// MakeDir creates a new directory.
+// No-op for testing
 func (hu *FakeHostUtil) MakeDir(pathname string) error {
 	return nil
 }
 
+// MakeFile creates a new file.
+// No-op for testing
 func (hu *FakeHostUtil) MakeFile(pathname string) error {
 	return nil
 }
 
+// ExistsPath checks if pathname exists.
 func (hu *FakeHostUtil) ExistsPath(pathname string) (bool, error) {
 	if _, ok := hu.Filesystem[pathname]; ok {
 		return true, nil
@@ -229,18 +255,26 @@ func (hu *FakeHostUtil) ExistsPath(pathname string) (bool, error) {
 	return false, nil
 }
 
+// EvalHostSymlinks returns the path name after evaluating symlinks.
+// No-op for testing
 func (hu *FakeHostUtil) EvalHostSymlinks(pathname string) (string, error) {
 	return pathname, nil
 }
 
+// GetFSGroup returns FSGroup of pathname.
+// Not implemented for testing
 func (hu *FakeHostUtil) GetFSGroup(pathname string) (int64, error) {
 	return -1, errors.New("GetFSGroup not implemented")
 }
 
+// GetSELinuxSupport tests if pathname is on a mount that supports SELinux.
+// Not implemented for testing
 func (hu *FakeHostUtil) GetSELinuxSupport(pathname string) (bool, error) {
 	return false, errors.New("GetSELinuxSupport not implemented")
 }
 
+// GetMode returns permissions of pathname.
+// Not implemented for testing
 func (hu *FakeHostUtil) GetMode(pathname string) (os.FileMode, error) {
 	return 0, errors.New("not implemented")
 }

pkg/util/mount/mount.go

@@ -16,6 +16,7 @@ limitations under the License.
 
 // TODO(thockin): This whole pkg is pretty linux-centric.  As soon as we have
 // an alternate platform, we will need to abstract further.
+
 package mount
 
 import (
@@ -25,32 +26,42 @@ import (
 	"strings"
 )
 
+// FileType enumerates the known set of possible file types.
 type FileType string
 
 const (
-	// Default mount command if mounter path is not specified
+	// Default mount command if mounter path is not specified.
-	defaultMountCommand          = "mount"
+	defaultMountCommand = "mount"
-	FileTypeDirectory   FileType = "Directory"
+
-	FileTypeFile        FileType = "File"
+	// FileTypeBlockDev defines a constant for the block device FileType.
-	FileTypeSocket      FileType = "Socket"
+	FileTypeBlockDev FileType = "BlockDevice"
-	FileTypeCharDev     FileType = "CharDevice"
+	// FileTypeCharDev defines a constant for the character device FileType.
-	FileTypeBlockDev    FileType = "BlockDevice"
+	FileTypeCharDev FileType = "CharDevice"
+	// FileTypeDirectory defines a constant for the directory FileType.
+	FileTypeDirectory FileType = "Directory"
+	// FileTypeFile defines a constant for the file FileType.
+	FileTypeFile FileType = "File"
+	// FileTypeSocket defines a constant for the socket FileType.
+	FileTypeSocket FileType = "Socket"
+	// FileTypeUnknown defines a constant for an unknown FileType.
+	FileTypeUnknown FileType = ""
 )
 
+// Interface defines the set of methods to allow for mount operations on a system.
 type Interface interface {
 	// Mount mounts source to target as fstype with given options.
 	Mount(source string, target string, fstype string, options []string) error
 	// Unmount unmounts given target.
 	Unmount(target string) error
 	// List returns a list of all mounted filesystems.  This can be large.
-	// On some platforms, reading mounts is not guaranteed consistent (i.e.
+	// On some platforms, reading mounts directly from the OS is not guaranteed
-	// it could change between chunked reads). This is guaranteed to be
+	// consistent (i.e. it could change between chunked reads). This is guaranteed
-	// consistent.
+	// to be consistent.
 	List() ([]MountPoint, error)
-	// IsMountPointMatch determines if the mountpoint matches the dir
+	// IsMountPointMatch determines if the mountpoint matches the dir.
 	IsMountPointMatch(mp MountPoint, dir string) bool
 	// IsLikelyNotMountPoint uses heuristics to determine if a directory
-	// is a mountpoint.
+	// is not a mountpoint.
 	// It should return ErrNotExist when the directory does not exist.
 	// IsLikelyNotMountPoint does NOT properly detect all mountpoint types
 	// most notably linux bind mounts and symbolic link.
@@ -61,32 +72,28 @@ type Interface interface {
 	GetMountRefs(pathname string) ([]string, error)
 }
 
+// HostUtils defines the set of methods for interacting with paths on a host.
 type HostUtils interface {
-	// DeviceOpened determines if the device is in use elsewhere
+	// DeviceOpened determines if the device (e.g. /dev/sdc) is in use elsewhere
 	// on the system, i.e. still mounted.
 	DeviceOpened(pathname string) (bool, error)
 	// PathIsDevice determines if a path is a device.
 	PathIsDevice(pathname string) (bool, error)
 	// GetDeviceNameFromMount finds the device name by checking the mount path
-	// to get the global mount path within its plugin directory
+	// to get the global mount path within its plugin directory.
 	GetDeviceNameFromMount(mounter Interface, mountPath, pluginMountDir string) (string, error)
 	// MakeRShared checks that given path is on a mount with 'rshared' mount
 	// propagation. If not, it bind-mounts the path as rshared.
 	MakeRShared(path string) error
 	// GetFileType checks for file/directory/socket/block/character devices.
-	// Will operate in the host mount namespace if kubelet is running in a container
 	GetFileType(pathname string) (FileType, error)
 	// MakeFile creates an empty file.
-	// Will operate in the host mount namespace if kubelet is running in a container
 	MakeFile(pathname string) error
 	// MakeDir creates a new directory.
-	// Will operate in the host mount namespace if kubelet is running in a container
 	MakeDir(pathname string) error
-	// Will operate in the host mount namespace if kubelet is running in a container.
 	// Error is returned on any other error than "file not found".
 	ExistsPath(pathname string) (bool, error)
 	// EvalHostSymlinks returns the path name after evaluating symlinks.
-	// Will operate in the host mount namespace if kubelet is running in a container.
 	EvalHostSymlinks(pathname string) (string, error)
 	// GetFSGroup returns FSGroup of the path.
 	GetFSGroup(pathname string) (int64, error)
@@ -97,25 +104,7 @@ type HostUtils interface {
 	GetMode(pathname string) (os.FileMode, error)
 }
 
-type Subpath struct {
+// Exec is an interface for executing commands on systems.
-	// index of the VolumeMount for this container
-	VolumeMountIndex int
-	// Full path to the subpath directory on the host
-	Path string
-	// name of the volume that is a valid directory name.
-	VolumeName string
-	// Full path to the volume path
-	VolumePath string
-	// Path to the pod's directory, including pod UID
-	PodDir string
-	// Name of the container
-	ContainerName string
-}
-
-// Exec executes command where mount utilities are. This can be either the host,
-// container where kubelet runs or even a remote pod with mount utilities.
-// Usual k8s.io/utils/exec interface is not used because kubelet.RunInContainer does
-// not provide stdin/stdout/stderr streams.
 type Exec interface {
 	// Run executes a command and returns its stdout + stderr combined in one
 	// stream.
@@ -123,14 +112,14 @@ type Exec interface {
 }
 
 // Compile-time check to ensure all Mounter implementations satisfy
-// the mount interface
+// the mount interface.
 var _ Interface = &Mounter{}
 
 // Compile-time check to ensure all HostUtil implementations satisfy
-// the HostUtils Interface
+// the HostUtils Interface.
 var _ HostUtils = &hostUtil{}
 
-// This represents a single line in /proc/mounts or /etc/fstab.
+// MountPoint represents a single line in /proc/mounts or /etc/fstab.
 type MountPoint struct {
 	Device string
 	Path   string
@@ -166,7 +155,7 @@ func getMountRefsByDev(mounter Interface, mountPath string) ([]string, error) {
 		return nil, err
 	}
 
-	// Finding the device mounted to mountPath
+	// Finding the device mounted to mountPath.
 	diskDev := ""
 	for i := range mps {
 		if mountPath == mps[i].Path {
@@ -187,8 +176,8 @@ func getMountRefsByDev(mounter Interface, mountPath string) ([]string, error) {
 	return refs, nil
 }
 
-// GetDeviceNameFromMount: given a mnt point, find the device from /proc/mounts
+// GetDeviceNameFromMount given a mnt point, find the device from /proc/mounts
-// returns the device name, reference count, and error code
+// returns the device name, reference count, and error code.
 func GetDeviceNameFromMount(mounter Interface, mountPath string) (string, int, error) {
 	mps, err := mounter.List()
 	if err != nil {
@@ -196,7 +185,7 @@ func GetDeviceNameFromMount(mounter Interface, mountPath string) (string, int, e
 	}
 
 	// Find the device name.
-	// FIXME if multiple devices mounted on the same mount path, only the first one is returned
+	// FIXME if multiple devices mounted on the same mount path, only the first one is returned.
 	device := ""
 	// If mountPath is symlink, need get its target path.
 	slTarget, err := filepath.EvalSymlinks(mountPath)
@@ -226,10 +215,10 @@ func GetDeviceNameFromMount(mounter Interface, mountPath string) (string, int, e
 // IsNotMountPoint detects bind mounts in linux.
 // IsNotMountPoint enumerates all the mountpoints using List() and
 // the list of mountpoints may be large, then it uses
-// IsMountPointMatch to evaluate whether the directory is a mountpoint
+// IsMountPointMatch to evaluate whether the directory is a mountpoint.
 func IsNotMountPoint(mounter Interface, file string) (bool, error) {
 	// IsLikelyNotMountPoint provides a quick check
-	// to determine whether file IS A mountpoint
+	// to determine whether file IS A mountpoint.
 	notMnt, notMntErr := mounter.IsLikelyNotMountPoint(file)
 	if notMntErr != nil && os.IsPermission(notMntErr) {
 		// We were not allowed to do the simple stat() check, e.g. on NFS with
@@ -240,12 +229,12 @@ func IsNotMountPoint(mounter Interface, file string) (bool, error) {
 	if notMntErr != nil {
 		return notMnt, notMntErr
 	}
-	// identified as mountpoint, so return this fact
+	// identified as mountpoint, so return this fact.
 	if notMnt == false {
 		return notMnt, nil
 	}
 
-	// Resolve any symlinks in file, kernel would do the same and use the resolved path in /proc/mounts
+	// Resolve any symlinks in file, kernel would do the same and use the resolved path in /proc/mounts.
 	hu := NewHostUtil()
 	resolvedFile, err := hu.EvalHostSymlinks(file)
 	if err != nil {
@@ -253,7 +242,7 @@ func IsNotMountPoint(mounter Interface, file string) (bool, error) {
 	}
 
 	// check all mountpoints since IsLikelyNotMountPoint
-	// is not reliable for some mountpoint types
+	// is not reliable for some mountpoint types.
 	mountPoints, mountPointsErr := mounter.List()
 	if mountPointsErr != nil {
 		return notMnt, mountPointsErr
@@ -312,6 +301,7 @@ func checkForNetDev(options []string) bool {
 	return false
 }
 
+// HasMountRefs checks if the given mountPath has mountRefs.
 // TODO: this is a workaround for the unmount device issue caused by gci mounter.
 // In GCI cluster, if gci mounter is used for mounting, the container started by mounter
 // script will cause additional mounts created in the container. Since these mounts are
@@ -319,7 +309,6 @@ func checkForNetDev(options []string) bool {
 // mount references. Current solution is to filter out those mount paths that contain
 // the string of original mount path.
 // Plan to work on better approach to solve this issue.
-
 func HasMountRefs(mountPath string, mountRefs []string) bool {
 	for _, ref := range mountRefs {
 		if !strings.Contains(ref, mountPath) {
@@ -336,19 +325,19 @@ func PathWithinBase(fullPath, basePath string) bool {
 		return false
 	}
 	if StartsWithBackstep(rel) {
-		// Needed to escape the base path
+		// Needed to escape the base path.
 		return false
 	}
 	return true
 }
 
-// StartsWithBackstep checks if the given path starts with a backstep segment
+// StartsWithBackstep checks if the given path starts with a backstep segment.
 func StartsWithBackstep(rel string) bool {
 	// normalize to / and check for ../
 	return rel == ".." || strings.HasPrefix(filepath.ToSlash(rel), "../")
 }
 
-// getFileType checks for file/directory/socket and block/character devices
+// getFileType checks for file/directory/socket and block/character devices.
 func getFileType(pathname string) (FileType, error) {
 	var pathType FileType
 	info, err := os.Stat(pathname)
@@ -360,7 +349,7 @@ func getFileType(pathname string) (FileType, error) {
 		return pathType, err
 	}
 
-	// checks whether the mode is the target mode
+	// checks whether the mode is the target mode.
 	isSpecificMode := func(mode, targetMode os.FileMode) bool {
 		return mode&targetMode == targetMode
 	}

pkg/util/mount/mount_helper_common.go

@@ -23,11 +23,10 @@ import (
 	"k8s.io/klog"
 )
 
-// CleanupMountPoint unmounts the given path and
+// CleanupMountPoint unmounts the given path and deletes the remaining directory
-// deletes the remaining directory if successful.
+// if successful. If extensiveMountPointCheck is true IsNotMountPoint will be
-// if extensiveMountPointCheck is true
+// called instead of IsLikelyNotMountPoint. IsNotMountPoint is more expensive
-// IsNotMountPoint will be called instead of IsLikelyNotMountPoint.
+// but properly handles bind mounts within the same fs.
-// IsNotMountPoint is more expensive but properly handles bind mounts within the same fs.
 func CleanupMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool) error {
 	// mounter.ExistsPath cannot be used because for containerized kubelet, we need to check
 	// the path in the kubelet container, not on the host.
@@ -87,8 +86,8 @@ func doCleanupMountPoint(mountPath string, mounter Interface, extensiveMountPoin
 	return fmt.Errorf("Failed to unmount path %v", mountPath)
 }
 
-// TODO: clean this up to use pkg/util/file/FileExists
 // PathExists returns true if the specified path exists.
+// TODO: clean this up to use pkg/util/file/FileExists
 func PathExists(path string) (bool, error) {
 	_, err := os.Stat(path)
 	if err == nil {
@@ -97,7 +96,6 @@ func PathExists(path string) (bool, error) {
 		return false, nil
 	} else if IsCorruptedMnt(err) {
 		return true, err
-	} else {
-		return false, err
 	}
+	return false, err
 }

pkg/util/mount/mount_linux.go

@@ -102,14 +102,14 @@ func (mounter *Mounter) Mount(source string, target string, fstype string, optio
 }
 
 // doMount runs the mount command. mounterPath is the path to mounter binary if containerized mounter is used.
-func (m *Mounter) doMount(mounterPath string, mountCmd string, source string, target string, fstype string, options []string) error {
+func (mounter *Mounter) doMount(mounterPath string, mountCmd string, source string, target string, fstype string, options []string) error {
 	mountArgs := MakeMountArgs(source, target, fstype, options)
 	if len(mounterPath) > 0 {
 		mountArgs = append([]string{mountCmd}, mountArgs...)
 		mountCmd = mounterPath
 	}
 
-	if m.withSystemd {
+	if mounter.withSystemd {
 		// Try to run mount via systemd-run --scope. This will escape the
 		// service where kubelet runs and any fuse daemons will be started in a
 		// specific scope. kubelet service than can be restarted without killing
@@ -145,7 +145,7 @@ func (m *Mounter) doMount(mounterPath string, mountCmd string, source string, ta
 	if err != nil {
 		args := strings.Join(mountArgs, " ")
 		klog.Errorf("Mount failed: %v\nMounting command: %s\nMounting arguments: %s\nOutput: %s\n", err, mountCmd, args, string(output))
-		return fmt.Errorf("mount failed: %v\nMounting command: %s\nMounting arguments: %s\nOutput: %s\n",
+		return fmt.Errorf("mount failed: %v\nMounting command: %s\nMounting arguments: %s\nOutput: %s",
 			err, mountCmd, args, string(output))
 	}
 	return err
@@ -210,7 +210,7 @@ func (mounter *Mounter) Unmount(target string) error {
 	command := exec.Command("umount", target)
 	output, err := command.CombinedOutput()
 	if err != nil {
-		return fmt.Errorf("Unmount failed: %v\nUnmounting arguments: %s\nOutput: %s\n", err, target, string(output))
+		return fmt.Errorf("unmount failed: %v\nUnmounting arguments: %s\nOutput: %s", err, target, string(output))
 	}
 	return nil
 }
@@ -220,6 +220,7 @@ func (*Mounter) List() ([]MountPoint, error) {
 	return ListProcMounts(procMountsPath)
 }
 
+// IsMountPointMatch returns true if the path in mp is the same as dir
 func (mounter *Mounter) IsMountPointMatch(mp MountPoint, dir string) bool {
 	deletedDir := fmt.Sprintf("%s\\040(deleted)", dir)
 	return ((mp.Path == dir) || (mp.Path == deletedDir))
@@ -249,6 +250,9 @@ func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) {
 	return true, nil
 }
 
+// GetMountRefs finds all mount references to pathname, returns a
+// list of paths. Path could be a mountpoint path, device or a normal
+// directory (for bind mount).
 func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) {
 	pathExists, pathErr := PathExists(pathname)
 	if !pathExists {
@@ -291,7 +295,7 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
 			case isExitError && ee.ExitStatus() == fsckErrorsCorrected:
 				klog.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))
+				return fmt.Errorf("'fsck' found errors on device %s but could not correct them: %s", source, string(out))
 			case isExitError && ee.ExitStatus() > fsckErrorsUncorrected:
 				klog.Infof("`fsck` error %s", string(out))
 			}
@@ -337,16 +341,14 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
 			}
 			klog.Errorf("format of disk %q failed: type:(%q) target:(%q) options:(%q)error:(%v)", source, fstype, target, options, err)
 			return err
-		} else {
-			// Disk is already formatted and failed to mount
-			if len(fstype) == 0 || fstype == existingFormat {
-				// This is mount error
-				return mountErr
-			} else {
-				// 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)
-			}
 		}
+		// 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)
 	}
 	return mountErr
 }
@@ -453,6 +455,8 @@ func parseProcMounts(content []byte) ([]MountPoint, error) {
 type hostUtil struct {
 }
 
+// NewHostUtil returns a struct that implements the HostUtils interface on
+// linux platforms
 func NewHostUtil() HostUtils {
 	return &hostUtil{}
 }
@@ -668,7 +672,7 @@ func parseMountInfo(filename string) ([]mountInfo, error) {
 			info.optionalFields = append(info.optionalFields, fields[i])
 		}
 		// Parse the rest 3 fields.
-		i += 1
+		i++
 		if len(fields)-i < 3 {
 			return nil, fmt.Errorf("expect 3 fields in %s, got %d", line, len(fields)-i)
 		}

pkg/util/mount/mount_unsupported.go

@@ -23,11 +23,12 @@ import (
 	"os"
 )
 
+// Mounter implements mount.Interface for unsupported platforms
 type Mounter struct {
 	mounterPath string
 }
 
-var unsupportedErr = errors.New("util/mount on this platform is not supported")
+var errUnsupported = errors.New("util/mount on this platform is not supported")
 
 // New returns a mount.Interface for the current system.
 // It provides options to override the default mounter behavior.
@@ -38,28 +39,34 @@ func New(mounterPath string) Interface {
 	}
 }
 
+// Mount always returns an error on unsupported platforms
 func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error {
-	return unsupportedErr
+	return errUnsupported
 }
 
+// Unmount always returns an error on unsupported platforms
 func (mounter *Mounter) Unmount(target string) error {
-	return unsupportedErr
+	return errUnsupported
 }
 
+// List always returns an error on unsupported platforms
 func (mounter *Mounter) List() ([]MountPoint, error) {
-	return []MountPoint{}, unsupportedErr
+	return []MountPoint{}, errUnsupported
 }
 
+// IsMountPointMatch returns true if the path in mp is the same as dir
 func (mounter *Mounter) IsMountPointMatch(mp MountPoint, dir string) bool {
 	return (mp.Path == dir)
 }
 
+// IsLikelyNotMountPoint always returns an error on unsupported platforms
 func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) {
-	return true, unsupportedErr
+	return true, errUnsupported
 }
 
+// GetMountRefs always returns an error on unsupported platforms
 func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) {
-	return nil, errors.New("not implemented")
+	return nil, errUnsupported
 }
 
 func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error {
@@ -67,68 +74,70 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
 }
 
 func (mounter *SafeFormatAndMount) diskLooksUnformatted(disk string) (bool, error) {
-	return true, unsupportedErr
+	return true, errUnsupported
 }
 
 func getDeviceNameFromMount(mounter Interface, mountPath, pluginMountDir string) (string, error) {
-	return "", unsupportedErr
+	return "", errUnsupported
 }
 
 type hostUtil struct{}
 
+// NewHostUtil returns a struct that implements the HostUtils interface on
+// unsupported platforms
 func NewHostUtil() HostUtils {
 	return &hostUtil{}
 }
 
 // DeviceOpened determines if the device is in use elsewhere
 func (hu *hostUtil) DeviceOpened(pathname string) (bool, error) {
-	return false, unsupportedErr
+	return false, errUnsupported
 }
 
 // PathIsDevice determines if a path is a device.
 func (hu *hostUtil) PathIsDevice(pathname string) (bool, error) {
-	return true, unsupportedErr
+	return true, errUnsupported
 }
 
 // GetDeviceNameFromMount finds the device name by checking the mount path
 // to get the global mount path within its plugin directory
 func (hu *hostUtil) GetDeviceNameFromMount(mounter Interface, mountPath, pluginMountDir string) (string, error) {
-	return "", unsupportedErr
+	return "", errUnsupported
 }
 
 func (hu *hostUtil) MakeRShared(path string) error {
-	return unsupportedErr
+	return errUnsupported
 }
 
 func (hu *hostUtil) GetFileType(pathname string) (FileType, error) {
-	return FileType("fake"), unsupportedErr
+	return FileType("fake"), errUnsupported
 }
 
 func (hu *hostUtil) MakeFile(pathname string) error {
-	return unsupportedErr
+	return errUnsupported
 }
 
 func (hu *hostUtil) MakeDir(pathname string) error {
-	return unsupportedErr
+	return errUnsupported
 }
 
 func (hu *hostUtil) ExistsPath(pathname string) (bool, error) {
-	return true, unsupportedErr
+	return true, errUnsupported
 }
 
 // EvalHostSymlinks returns the path name after evaluating symlinks
 func (hu *hostUtil) EvalHostSymlinks(pathname string) (string, error) {
-	return "", unsupportedErr
+	return "", errUnsupported
 }
 
 func (hu *hostUtil) GetFSGroup(pathname string) (int64, error) {
-	return -1, unsupportedErr
+	return -1, errUnsupported
 }
 
 func (hu *hostUtil) GetSELinuxSupport(pathname string) (bool, error) {
-	return false, unsupportedErr
+	return false, errUnsupported
 }
 
 func (hu *hostUtil) GetMode(pathname string) (os.FileMode, error) {
-	return 0, unsupportedErr
+	return 0, errUnsupported
 }

pkg/util/mount/mount_windows.go

@@ -311,6 +311,8 @@ func getAllParentLinks(path string) ([]string, error) {
 
 type hostUtil struct{}
 
+// NewHostUtil returns a struct that implements the HostUtils interface on
+// windows platforms
 func NewHostUtil() HostUtils {
 	return &hostUtil{}
 }

pkg/util/mount/safe_format_and_mount_test.go

@@ -21,6 +21,7 @@ import (
 	"io/ioutil"
 	"os"
 	"runtime"
+	"strings"
 	"testing"
 
 	fakeexec "k8s.io/utils/exec/testing"
@@ -83,7 +84,7 @@ func TestSafeFormatAndMount(t *testing.T) {
 			execScripts: []ExecArgs{
 				{"fsck", []string{"-a", "/dev/foo"}, "", &fakeexec.FakeExitError{Status: 4}},
 			},
-			expectedError: fmt.Errorf("'fsck' found errors on device /dev/foo but could not correct them: ."),
+			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)",
@@ -234,7 +235,7 @@ func TestSafeFormatAndMount(t *testing.T) {
 				t.Errorf("test \"%s\" the correct device was not mounted", test.description)
 			}
 		} else {
-			if err == nil || test.expectedError.Error() != err.Error() {
+			if err == nil || !strings.HasPrefix(err.Error(), test.expectedError.Error()) {
 				t.Errorf("test \"%s\" unexpected error: \n          [%v]. \nExpecting [%v]", test.description, err, test.expectedError)
 			}
 		}