Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search for nvme device path even if non-nvme exists #1082

Merged
merged 5 commits into from
Oct 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ word-hyphen = $(word $2,$(subst -, ,$1))

.EXPORT_ALL_VARIABLES:

.PHONY: linux/$(ARCH)
.PHONY: linux/$(ARCH) bin/aws-ebs-csi-driver
linux/$(ARCH): bin/aws-ebs-csi-driver
bin/aws-ebs-csi-driver: | bin
CGO_ENABLED=0 GOOS=linux GOARCH=$(ARCH) go build -mod=vendor -ldflags ${LDFLAGS} -o bin/aws-ebs-csi-driver ./cmd/

.PHONY: windows/$(ARCH)
.PHONY: windows/$(ARCH) bin/aws-ebs-csi-driver.exe
windows/$(ARCH): bin/aws-ebs-csi-driver.exe
bin/aws-ebs-csi-driver.exe: | bin
CGO_ENABLED=0 GOOS=windows GOARCH=$(ARCH) go build -mod=vendor -ldflags ${LDFLAGS} -o bin/aws-ebs-csi-driver.exe ./cmd/
Expand Down
2 changes: 1 addition & 1 deletion hack/e2e/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ REGION=${AWS_REGION:-us-west-2}
ZONES=${AWS_AVAILABILITY_ZONES:-us-west-2a,us-west-2b,us-west-2c}
FIRST_ZONE=$(echo "${ZONES}" | cut -d, -f1)
NODE_COUNT=${NODE_COUNT:-3}
INSTANCE_TYPE=${INSTANCE_TYPE:-c4.large}
INSTANCE_TYPE=${INSTANCE_TYPE:-c5.large}

AWS_ACCOUNT_ID=$(aws sts get-caller-identity --query Account --output text)
IMAGE_NAME=${IMAGE_NAME:-${AWS_ACCOUNT_ID}.dkr.ecr.${REGION}.amazonaws.com/${DRIVER_NAME}}
Expand Down
54 changes: 54 additions & 0 deletions pkg/driver/mock_mount.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 27 additions & 0 deletions pkg/driver/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ limitations under the License.
package driver

import (
"os"
"path/filepath"

"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/mounter"
mountutils "k8s.io/mount-utils"
)
Expand Down Expand Up @@ -54,3 +57,27 @@ func newNodeMounter() (Mounter, error) {
}
return &NodeMounter{safeMounter}, nil
}

// DeviceIdentifier is for mocking os io functions used for the driver to
// identify an EBS volume's corresponding device (in Linux, the path under
// /dev; in Windows, the volume number) so that it can mount it. For volumes
// already mounted, see GetDeviceNameFromMount.
// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/nvme-ebs-volumes.html#identify-nvme-ebs-device
type DeviceIdentifier interface {
Lstat(name string) (os.FileInfo, error)
EvalSymlinks(path string) (string, error)
}

type nodeDeviceIdentifier struct{}

func newNodeDeviceIdentifier() DeviceIdentifier {
return &nodeDeviceIdentifier{}
}

func (i *nodeDeviceIdentifier) Lstat(name string) (os.FileInfo, error) {
return os.Lstat(name)
}

func (i *nodeDeviceIdentifier) EvalSymlinks(path string) (string, error) {
return filepath.EvalSymlinks(path)
}
18 changes: 10 additions & 8 deletions pkg/driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,11 @@ var (

// nodeService represents the node service of CSI driver
type nodeService struct {
metadata cloud.MetadataService
mounter Mounter
inFlight *internal.InFlight
driverOptions *DriverOptions
metadata cloud.MetadataService
mounter Mounter
deviceIdentifier DeviceIdentifier
inFlight *internal.InFlight
driverOptions *DriverOptions
}

// newNodeService creates a new node service
Expand All @@ -94,10 +95,11 @@ func newNodeService(driverOptions *DriverOptions) nodeService {
}

return nodeService{
metadata: metadata,
mounter: nodeMounter,
inFlight: internal.NewInFlight(),
driverOptions: driverOptions,
metadata: metadata,
mounter: nodeMounter,
deviceIdentifier: newNodeDeviceIdentifier(),
inFlight: internal.NewInFlight(),
driverOptions: driverOptions,
}
}

Expand Down
53 changes: 37 additions & 16 deletions pkg/driver/node_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,41 +33,62 @@ import (
// findDevicePath finds path of device and verifies its existence
// if the device is not nvme, return the path directly
// if the device is nvme, finds and returns the nvme device path eg. /dev/nvme1n1
func (d *nodeService) findDevicePath(devicePath, volumeID string, partition string) (string, error) {
func (d *nodeService) findDevicePath(devicePath, volumeID, partition string) (string, error) {
canonicalDevicePath := ""

// If the given path exists, the device MAY be nvme. Further, it MAY be a
// symlink to the nvme device path like:
// | $ stat /dev/xvdba
// | File: ‘/dev/xvdba’ -> ‘nvme1n1’
// Since these are maybes, not guarantees, the search for the nvme device
// path below must happen and must rely on volume ID
exists, err := d.mounter.PathExists(devicePath)
if err != nil {
return "", err
return "", fmt.Errorf("failed to check if path %q exists: %v", devicePath, err)
}

// If the path exists, assume it is not nvme device
if exists {
if partition != "" {
devicePath = devicePath + diskPartitionSuffix + partition
}
return devicePath, nil
canonicalDevicePath = devicePath
}

// Else find the nvme device path using volume ID
// This is the magic name on which AWS presents NVME devices under /dev/disk/by-id/
// For example, vol-0fab1d5e3f72a5e23 creates a symlink at
// AWS recommends identifying devices by volume ID
// (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/nvme-ebs-volumes.html),
// so find the nvme device path using volume ID. 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(volumeID, "-", "", -1)

nvmeDevicePath, err := findNvmeVolume(nvmeName)
if err != nil {
return "", err
nvmeDevicePath, err := findNvmeVolume(d.deviceIdentifier, nvmeName)

if err == nil {
if partition != "" {
nvmeDevicePath = nvmeDevicePath + nvmeDiskPartitionSuffix + partition
}
canonicalDevicePath = nvmeDevicePath
} else {
klog.V(5).Infof("[Debug] error searching for nvme path %q: %v", nvmeName, err)
}
if partition != "" {
nvmeDevicePath = nvmeDevicePath + nvmeDiskPartitionSuffix + partition

if canonicalDevicePath == "" {
return "", errNoDevicePathFound(devicePath, volumeID)
}
return nvmeDevicePath, nil

return canonicalDevicePath, nil
}

func errNoDevicePathFound(devicePath, volumeID string) error {
return fmt.Errorf("no device path for device %q volume %q found!", devicePath, volumeID)
}

// 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) {
func findNvmeVolume(deviceIdentifier DeviceIdentifier, findName string) (device string, err error) {
p := filepath.Join("/dev/disk/by-id/", findName)
stat, err := os.Lstat(p)
stat, err := deviceIdentifier.Lstat(p)
if err != nil {
if os.IsNotExist(err) {
klog.V(5).Infof("[Debug] nvme path %q not found", p)
Expand All @@ -82,7 +103,7 @@ func findNvmeVolume(findName string) (device string, err error) {
}
// 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)
resolved, err := deviceIdentifier.EvalSymlinks(p)
if err != nil {
return "", fmt.Errorf("error reading target of symlink %q: %v", p, err)
}
Expand Down
Loading