Merge pull request #56607 from justinsb/aws_nvme_2

pkg/cloudprovider/providers/aws/aws.go

@@ -1662,7 +1662,7 @@ type awsDisk struct {
 }
 
 func newAWSDisk(aws *Cloud, name KubernetesVolumeID) (*awsDisk, error) {
-	awsID, err := name.mapToAWSVolumeID()
+	awsID, err := name.MapToAWSVolumeID()
 	if err != nil {
 		return nil, err
 	}
@@ -2022,7 +2022,6 @@ func (c *Cloud) AttachDisk(diskName KubernetesVolumeID, nodeName types.NodeName,
 // DetachDisk implements Volumes.DetachDisk
 func (c *Cloud) DetachDisk(diskName KubernetesVolumeID, nodeName types.NodeName) (string, error) {
 	diskInfo, attached, err := c.checkIfAttachedToNode(diskName, nodeName)
-
 	if diskInfo == nil {
 		return "", err
 	}
@@ -2320,7 +2319,6 @@ func (c *Cloud) GetDiskPath(volumeName KubernetesVolumeID) (string, error) {
 // DiskIsAttached implements Volumes.DiskIsAttached
 func (c *Cloud) DiskIsAttached(diskName KubernetesVolumeID, nodeName types.NodeName) (bool, error) {
 	diskInfo, attached, err := c.checkIfAttachedToNode(diskName, nodeName)
-
 	if diskInfo == nil {
 		return true, err
 	}
@@ -2378,7 +2376,7 @@ func (c *Cloud) DisksAreAttached(nodeDisks map[types.NodeName][]KubernetesVolume
 
 		idToDiskName := make(map[awsVolumeID]KubernetesVolumeID)
 		for _, diskName := range diskNames {
-			volumeID, err := diskName.mapToAWSVolumeID()
+			volumeID, err := diskName.MapToAWSVolumeID()
 			if err != nil {
 				return nil, fmt.Errorf("error mapping volume spec %q to aws id: %v", diskName, err)
 			}

pkg/cloudprovider/providers/aws/volumes.go

@@ -59,8 +59,8 @@ type diskInfo struct {
 	disk            *awsDisk
 }
 
-// mapToAWSVolumeID extracts the awsVolumeID from the KubernetesVolumeID
-func (name KubernetesVolumeID) mapToAWSVolumeID() (awsVolumeID, error) {
+// MapToAWSVolumeID extracts the awsVolumeID from the KubernetesVolumeID
+func (name KubernetesVolumeID) MapToAWSVolumeID() (awsVolumeID, error) {
 	// name looks like aws://availability-zone/awsVolumeId
 
 	// The original idea of the URL-style name was to put the AZ into the
@@ -101,7 +101,7 @@ func (name KubernetesVolumeID) mapToAWSVolumeID() (awsVolumeID, error) {
 
 func GetAWSVolumeID(kubeVolumeID string) (string, error) {
 	kid := KubernetesVolumeID(kubeVolumeID)
-	awsID, err := kid.mapToAWSVolumeID()
+	awsID, err := kid.MapToAWSVolumeID()
 	return string(awsID), err
 }
 

pkg/volume/aws_ebs/attacher.go

@@ -169,7 +169,7 @@ func (attacher *awsElasticBlockStoreAttacher) WaitForAttach(spec *volume.Spec, d
 		case <-ticker.C:
 			glog.V(5).Infof("Checking AWS Volume %q is attached.", volumeID)
 			if devicePath != "" {
-				devicePaths := getDiskByIdPaths(partition, devicePath)
+				devicePaths := getDiskByIdPaths(aws.KubernetesVolumeID(volumeSource.VolumeID), partition, devicePath)
 				path, err := verifyDevicePath(devicePaths)
 				if err != nil {
 					// Log error, if any, and continue checking periodically. See issue #11321

pkg/volume/aws_ebs/aws_util.go

@@ -18,6 +18,8 @@ package aws_ebs
 
 import (
 	"fmt"
+	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"time"
@@ -178,7 +180,7 @@ func verifyAllPathsRemoved(devicePaths []string) (bool, error) {
 // Returns list of all paths for given EBS mount
 // This is more interesting on GCE (where we are able to identify volumes under /dev/disk-by-id)
 // Here it is mostly about applying the partition path
-func getDiskByIdPaths(partition string, devicePath string) []string {
+func getDiskByIdPaths(volumeID aws.KubernetesVolumeID, partition string, devicePath string) []string {
 	devicePaths := []string{}
 	if devicePath != "" {
 		devicePaths = append(devicePaths, devicePath)
@@ -190,6 +192,23 @@ func getDiskByIdPaths(partition string, devicePath string) []string {
 		}
 	}
 
+	// We need to find NVME volumes, which are mounted on a "random" nvme path ("/dev/nvme0n1"),
+	// and we have to get the volume id from the nvme interface
+	awsVolumeID, err := volumeID.MapToAWSVolumeID()
+	if err != nil {
+		glog.Warningf("error mapping volume %q to AWS volume: %v", volumeID, err)
+	} else {
+		// This is the magic name on which AWS presents NVME devices under /dev/disk/by-id/
+		// For example, vol-0fab1d5e3f72a5e23 creates a symlink at /dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol0fab1d5e3f72a5e23
+		nvmeName := "nvme-Amazon_Elastic_Block_Store_" + strings.Replace(string(awsVolumeID), "-", "", -1)
+		nvmePath, err := findNvmeVolume(nvmeName)
+		if err != nil {
+			glog.Warningf("error looking for nvme volume %q: %v", volumeID, err)
+		} else if nvmePath != "" {
+			devicePaths = append(devicePaths, nvmePath)
+		}
+	}
+
 	return devicePaths
 }
 
@@ -202,3 +221,35 @@ func getCloudProvider(cloudProvider cloudprovider.Interface) (*aws.Cloud, error)
 
 	return awsCloudProvider, nil
 }
+
+// findNvmeVolume looks for the nvme volume with the specified name
+// It follows the symlink (if it exists) and returns the absolute path to the device
+func findNvmeVolume(findName string) (device string, err error) {
+	p := filepath.Join("/dev/disk/by-id/", findName)
+	stat, err := os.Lstat(p)
+	if err != nil {
+		if os.IsNotExist(err) {
+			glog.V(6).Infof("nvme path not found %q", p)
+			return "", nil
+		}
+		return "", fmt.Errorf("error getting stat of %q: %v", p, err)
+	}
+
+	if stat.Mode()&os.ModeSymlink != os.ModeSymlink {
+		glog.Warningf("nvme file %q found, but was not a symlink", p)
+		return "", nil
+	}
+
+	// Find the target, resolving to an absolute path
+	// For example, /dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol0fab1d5e3f72a5e23 -> ../../nvme2n1
+	resolved, err := filepath.EvalSymlinks(p)
+	if err != nil {
+		return "", fmt.Errorf("error reading target of symlink %q: %v", p, err)
+	}
+
+	if !strings.HasPrefix(resolved, "/dev") {
+		return "", fmt.Errorf("resolved symlink for %q was unexpected: %q", p, resolved)
+	}
+
+	return resolved, nil
+}