Replaced mount-manager with well tested k8s mount utilities

Author: davidz627

cmd/main.go

@@ -22,7 +22,6 @@ import (
 
 	gce "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider"
 	driver "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-pd-csi-driver"
-	mountmanager "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/mount-manager"
 )
 
 func init() {
@@ -56,10 +55,7 @@ func handle() {
 		glog.Fatalf("Failed to get cloud provider: %v", err)
 	}
 
-	mounter, err := mountmanager.CreateMounter()
-	if err != nil {
-		glog.Fatalf("Failed to get mounter: %v", err)
-	}
+	mounter := driver.NewSafeFormatAndMounter()
 
 	err = gceDriver.SetupGCEDriver(cloudProvider, mounter, *driverName, *nodeID, vendorVersion)
 	if err != nil {

pkg/gce-pd-csi-driver/gce-pd-driver.go

@@ -21,8 +21,8 @@ import (
 	"github.com/golang/glog"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"k8s.io/kubernetes/pkg/util/mount"
 	gce "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider"
-	mountmanager "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/mount-manager"
 )
 
 type GCEDriver struct {
@@ -43,7 +43,7 @@ func GetGCEDriver() *GCEDriver {
 	return &GCEDriver{}
 }
 
-func (gceDriver *GCEDriver) SetupGCEDriver(cloudProvider gce.GCECompute, mounter mountmanager.Mounter, name, nodeID, vendorVersion string) error {
+func (gceDriver *GCEDriver) SetupGCEDriver(cloudProvider gce.GCECompute, mounter mount.SafeFormatAndMount, name, nodeID, vendorVersion string) error {
 	if name == "" {
 		return fmt.Errorf("Driver name missing")
 	}
@@ -129,7 +129,7 @@ func NewIdentityServer(gceDriver *GCEDriver) *GCEIdentityServer {
 	}
 }
 
-func NewNodeServer(gceDriver *GCEDriver, mounter mountmanager.Mounter) *GCENodeServer {
+func NewNodeServer(gceDriver *GCEDriver, mounter mount.SafeFormatAndMount) *GCENodeServer {
 	return &GCENodeServer{
 		Driver:  gceDriver,
 		Mounter: mounter,
@@ -143,6 +143,45 @@ func NewControllerServer(gceDriver *GCEDriver, cloudProvider gce.GCECompute) *GC
 	}
 }
 
+func NewSafeFormatAndMounter() mount.SafeFormatAndMount {
+	realMounter := mount.New("")
+	realExec := mount.NewOsExec()
+	return mount.SafeFormatAndMount{
+		Interface: realMounter,
+		Exec:      realExec,
+	}
+}
+
+func NewFakeSafeFormatAndMounter(mountErrs []error) mount.SafeFormatAndMount {
+	execCallback := func(cmd string, args ...string) ([]byte, error) {
+		return nil, nil
+		// TODO: Fill out exec callback for errors
+		/*
+			if len(test.execScripts) <= execCallCount {
+				t.Errorf("Unexpected command: %s %v", cmd, args)
+				return nil, nil
+			}
+			script := test.execScripts[execCallCount]
+			execCallCount++
+			if script.command != cmd {
+				t.Errorf("Unexpected command %s. Expecting %s", cmd, script.command)
+			}
+			for j := range args {
+				if args[j] != script.args[j] {
+					t.Errorf("Unexpected args %v. Expecting %v", args, script.args)
+				}
+			}
+			return []byte(script.output), script.err
+		*/
+	}
+	fakeMounter := mount.ErrorMounter{&mount.FakeMounter{}, 0, mountErrs}
+	fakeExec := mount.NewFakeExec(execCallback)
+	return mount.SafeFormatAndMount{
+		Interface: &fakeMounter,
+		Exec:      fakeExec,
+	}
+}
+
 func (gceDriver *GCEDriver) Run(endpoint string) {
 	glog.Infof("Driver: %v", gceDriver.name)
 

pkg/gce-pd-csi-driver/node.go

@@ -17,20 +17,48 @@ package gceGCEDriver
 import (
 	"fmt"
 	"os"
+	"path"
+	"path/filepath"
+	"strings"
 	"sync"
 
 	csi "github.com/container-storage-interface/spec/lib/go/csi/v0"
 	"github.com/golang/glog"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
-	mountmanager "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/mount-manager"
+	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/kubernetes/pkg/util/mount"
+	"k8s.io/utils/exec"
 	utils "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/utils"
 )
 
+const (
+	diskByHostIdPath     = "/host/dev/disk/by-id/"
+	diskByIdPath         = "/dev/disk/by-id/"
+	diskGooglePrefix     = "google-"
+	diskScsiGooglePrefix = "scsi-0Google_PersistentDisk_"
+	diskPartitionSuffix  = "-part"
+	diskSDPath           = "/host/dev/sd"
+	diskSDPattern        = "/host/dev/sd*"
+	// How many times to retry for a consistent read of /proc/mounts.
+	maxListTries = 3
+	// Number of fields per line in /proc/mounts as per the fstab man page.
+	expectedNumFieldsPerLine = 6
+	// Location of the mount file to use
+	procMountsPath = "/proc/mounts"
+	// Location of the mountinfo file
+	procMountInfoPath = "/proc/self/mountinfo"
+	// 'fsck' found errors and corrected them
+	fsckErrorsCorrected = 1
+	// 'fsck' found errors but exited without correcting them
+	fsckErrorsUncorrected = 4
+	defaultMountCommand   = "mount"
+)
+
 type GCENodeServer struct {
 	Driver  *GCEDriver
-	Mounter mountmanager.Mounter
+	Mounter mount.SafeFormatAndMount
 	// TODO: Only lock mutually exclusive calls and make locking more fine grained
 	mux sync.Mutex
 }
@@ -71,7 +99,7 @@ func (ns *GCENodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePub
 		return nil, nil
 	}
 
-	if err := ns.Mounter.MkdirAll(targetPath, 0750); err != nil {
+	if err := ns.Mounter.MakeDir(targetPath); err != nil {
 		glog.Errorf("mkdir failed on disk %s (%v)", targetPath, err)
 		return nil, err
 	}
@@ -82,15 +110,15 @@ func (ns *GCENodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePub
 		options = append(options, "ro")
 	}
 
-	err = ns.Mounter.DoMount(stagingTargetPath, targetPath, "ext4", options)
+	err = ns.Mounter.Mount(stagingTargetPath, targetPath, "ext4", options)
 	if err != nil {
 		notMnt, mntErr := ns.Mounter.IsLikelyNotMountPoint(targetPath)
 		if mntErr != nil {
 			glog.Errorf("IsLikelyNotMountPoint check failed: %v", mntErr)
 			return nil, status.Error(codes.Internal, fmt.Sprintf("TODO: %v", err))
 		}
 		if !notMnt {
-			if _, mntErr = ns.Mounter.UnmountVolume(targetPath); mntErr != nil {
+			if mntErr = ns.Mounter.Unmount(targetPath); mntErr != nil {
 				glog.Errorf("Failed to unmount: %v", mntErr)
 				return nil, status.Error(codes.Internal, fmt.Sprintf("TODO: %v", err))
 			}
@@ -105,7 +133,7 @@ func (ns *GCENodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePub
 				return nil, status.Error(codes.Internal, fmt.Sprintf("TODO: %v", err))
 			}
 		}
-		ns.Mounter.Remove(targetPath)
+		os.Remove(targetPath)
 		glog.Errorf("Mount of disk %s failed: %v", targetPath, err)
 		return nil, status.Error(codes.Internal, fmt.Sprintf("TODO: %v", err))
 	}
@@ -130,9 +158,9 @@ func (ns *GCENodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeU
 
 	// TODO: Check volume still exists
 
-	output, err := ns.Mounter.UnmountVolume(targetPath)
+	err := ns.Mounter.Unmount(targetPath)
 	if err != nil {
-		return nil, status.Error(codes.Internal, fmt.Sprintf("Unmount failed: %v\nUnmounting arguments: %s\nOutput: %s\n", err, targetPath, output))
+		return nil, status.Error(codes.Internal, fmt.Sprintf("Unmount failed: %v\nUnmounting arguments: %s\n", err, targetPath))
 	}
 
 	return &csi.NodeUnpublishVolumeResponse{}, nil
@@ -168,8 +196,8 @@ func (ns *GCENodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStage
 	// TODO: Get real partitions
 	partition := ""
 
-	devicePaths := ns.Mounter.GetDiskByIdPaths(volumeName, partition)
-	devicePath, err := ns.Mounter.VerifyDevicePath(devicePaths)
+	devicePaths := getDiskByIdPaths(volumeName, partition)
+	devicePath, err := verifyDevicePath(devicePaths)
 
 	if err != nil {
 		return nil, status.Error(codes.Internal, fmt.Sprintf("Error verifying GCE PD (%q) is attached: %v", volumeName, err))
@@ -184,7 +212,7 @@ func (ns *GCENodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStage
 	notMnt, err := ns.Mounter.IsLikelyNotMountPoint(stagingTargetPath)
 	if err != nil {
 		if os.IsNotExist(err) {
-			if err := ns.Mounter.MkdirAll(stagingTargetPath, 0750); err != nil {
+			if err := ns.Mounter.MakeDir(stagingTargetPath); err != nil {
 				return nil, status.Error(codes.Internal, fmt.Sprintf("Failed to create directory (%q): %v", stagingTargetPath, err))
 			}
 			notMnt = true
@@ -242,7 +270,7 @@ func (ns *GCENodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUns
 		return nil, status.Error(codes.InvalidArgument, "NodeUnstageVolume Staging Target Path must be provided")
 	}
 
-	_, err := ns.Mounter.UnmountVolume(stagingTargetPath)
+	err := ns.Mounter.Unmount(stagingTargetPath)
 	if err != nil {
 		return nil, status.Error(codes.Internal, fmt.Sprintf("NodeUnstageVolume failed to unmount at path %s: %v", stagingTargetPath, err))
 	}
@@ -277,3 +305,107 @@ func (ns *GCENodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoRe
 	}
 	return resp, nil
 }
+
+// Returns list of all /dev/disk/by-id/* paths for given PD.
+func getDiskByIdPaths(pdName string, partition string) []string {
+	devicePaths := []string{
+		path.Join(diskByIdPath, diskGooglePrefix+pdName),
+		path.Join(diskByIdPath, diskScsiGooglePrefix+pdName),
+		path.Join(diskByHostIdPath, diskScsiGooglePrefix+pdName),
+		path.Join(diskByHostIdPath, diskScsiGooglePrefix+pdName),
+	}
+
+	if partition != "" {
+		for i, path := range devicePaths {
+			devicePaths[i] = path + diskPartitionSuffix + partition
+		}
+	}
+
+	return devicePaths
+}
+
+// Returns the first path that exists, or empty string if none exist.
+func verifyDevicePath(devicePaths []string) (string, error) {
+	sdBefore, err := filepath.Glob(diskSDPattern)
+	if err != nil {
+		// Seeing this error means that the diskSDPattern is malformed.
+		glog.Errorf("Error filepath.Glob(\"%s\"): %v\r\n", diskSDPattern, err)
+	}
+	sdBeforeSet := sets.NewString(sdBefore...)
+	// TODO: Remove this udevadm stuff. Not applicable because can't access /dev/sd* from container
+	if err := udevadmChangeToNewDrives(sdBeforeSet); err != nil {
+		// udevadm errors should not block disk detachment, log and continue
+		glog.Errorf("udevadmChangeToNewDrives failed with: %v", err)
+	}
+
+	for _, path := range devicePaths {
+		if pathExists, err := pathExists(path); err != nil {
+			return "", fmt.Errorf("Error checking if path exists: %v", err)
+		} else if pathExists {
+			return path, nil
+		}
+	}
+
+	return "", nil
+}
+
+// Triggers the application of udev rules by calling "udevadm trigger
+// --action=change" for newly created "/dev/sd*" drives (exist only in
+// after set). This is workaround for Issue #7972. Once the underlying
+// issue has been resolved, this may be removed.
+func udevadmChangeToNewDrives(sdBeforeSet sets.String) error {
+	sdAfter, err := filepath.Glob(diskSDPattern)
+	if err != nil {
+		return fmt.Errorf("Error filepath.Glob(\"%s\"): %v\r\n", diskSDPattern, err)
+	}
+
+	for _, sd := range sdAfter {
+		if !sdBeforeSet.Has(sd) {
+			return udevadmChangeToDrive(sd)
+		}
+	}
+
+	return nil
+}
+
+// Calls "udevadm trigger --action=change" on the specified drive.
+// drivePath must be the block device path to trigger on, in the format "/dev/sd*", or a symlink to it.
+// This is workaround for Issue #7972. Once the underlying issue has been resolved, this may be removed.
+func udevadmChangeToDrive(drivePath string) error {
+	glog.V(5).Infof("udevadmChangeToDrive: drive=%q", drivePath)
+
+	// Evaluate symlink, if any
+	drive, err := filepath.EvalSymlinks(drivePath)
+	if err != nil {
+		return fmt.Errorf("udevadmChangeToDrive: filepath.EvalSymlinks(%q) failed with %v.", drivePath, err)
+	}
+	glog.V(5).Infof("udevadmChangeToDrive: symlink path is %q", drive)
+
+	// Check to make sure input is "/dev/sd*"
+	if !strings.Contains(drive, diskSDPath) {
+		return fmt.Errorf("udevadmChangeToDrive: expected input in the form \"%s\" but drive is %q.", diskSDPattern, drive)
+	}
+
+	// Call "udevadm trigger --action=change --property-match=DEVNAME=/dev/sd..."
+	_, err = exec.New().Command(
+		"udevadm",
+		"trigger",
+		"--action=change",
+		fmt.Sprintf("--property-match=DEVNAME=%s", drive)).CombinedOutput()
+	if err != nil {
+		return fmt.Errorf("udevadmChangeToDrive: udevadm trigger failed for drive %q with %v.", drive, err)
+	}
+	return nil
+}
+
+// PathExists returns true if the specified path exists.
+func pathExists(path string) (bool, error) {
+	_, err := os.Stat(path)
+	if err == nil {
+		return true, nil
+	} else if os.IsNotExist(err) {
+		return false, nil
+	} else {
+		return false, err
+	}
+}