Skip to content

Commit

Permalink
Merge pull request #477 from davidz627/fix/regional
Browse files Browse the repository at this point in the history
Stop regional PD test from failing silently and fix verification path to use correct name
  • Loading branch information
k8s-ci-robot authored Feb 21, 2020
2 parents faf69fd + 7c3dde6 commit 2a39852
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 14 deletions.
2 changes: 1 addition & 1 deletion pkg/gce-pd-csi-driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ func (ns *GCENodeServer) getDevicePath(volumeID string, partition string) (strin
}

devicePaths := ns.DeviceUtils.GetDiskByIdPaths(deviceName, partition)
devicePath, err := ns.DeviceUtils.VerifyDevicePath(devicePaths, volumeKey.Name)
devicePath, err := ns.DeviceUtils.VerifyDevicePath(devicePaths, deviceName)

if err != nil {
return "", fmt.Errorf("error verifying GCE PD (%q) is attached: %v", volumeKey.Name, err)
Expand Down
24 changes: 12 additions & 12 deletions pkg/mount-manager/device-utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ type DeviceUtils interface {

// VerifyDevicePath returns the first of the list of device paths that
// exists on the machine, or an empty string if none exists
VerifyDevicePath(devicePaths []string, diskName string) (string, error)
VerifyDevicePath(devicePaths []string, deviceName string) (string, error)
}

type deviceUtils struct {
Expand Down Expand Up @@ -138,7 +138,7 @@ func parseScsiSerial(output string) (string, error) {
// candidate devicePaths or an empty string if none is found. If
// /lib/udev_containerized/scsi_id exists it will attempt to fix any issues
// caused by missing paths or mismatched devices by running a udevadm --trigger.
func (m *deviceUtils) VerifyDevicePath(devicePaths []string, diskName string) (string, error) {
func (m *deviceUtils) VerifyDevicePath(devicePaths []string, deviceName string) (string, error) {
var devicePath string
var err error
const (
Expand Down Expand Up @@ -167,9 +167,9 @@ func (m *deviceUtils) VerifyDevicePath(devicePaths []string, diskName string) (s

if len(devicePath) == 0 {
// Couldn't find the path so we need to find a /dev/sdx with the SCSI
// serial that matches diskName. Then we run udevadm trigger on that
// serial that matches deviceName. Then we run udevadm trigger on that
// device to get the device to show up in /dev/by-id/
innerErr := udevadmTriggerForDiskIfExists(diskName)
innerErr := udevadmTriggerForDiskIfExists(deviceName)
if innerErr != nil {
return false, fmt.Errorf("failed to trigger udevadm fix: %v", innerErr)
}
Expand All @@ -188,16 +188,16 @@ func (m *deviceUtils) VerifyDevicePath(devicePaths []string, diskName string) (s
if strings.Contains(devSDX, diskSDPath) {
scsiSerial, innerErr := getScsiSerial(devSDX)
if innerErr != nil {
return false, fmt.Errorf("couldn't get SCSI serial number for disk %s: %v", diskName, innerErr)
return false, fmt.Errorf("couldn't get SCSI serial number for disk %s: %v", deviceName, innerErr)
}
// SUCCESS! devicePath points to a /dev/sdx that has a SCSI serial
// equivilant to our disk name
if scsiSerial == diskName {
if scsiSerial == deviceName {
return true, nil
}
}
// The devicePath is not mapped to the correct disk
innerErr = udevadmTriggerForDiskIfExists(diskName)
innerErr = udevadmTriggerForDiskIfExists(deviceName)
if innerErr != nil {
return false, fmt.Errorf("failed to trigger udevadm fix: %v", innerErr)
}
Expand All @@ -207,13 +207,13 @@ func (m *deviceUtils) VerifyDevicePath(devicePaths []string, diskName string) (s
})

if err != nil {
return "", fmt.Errorf("failed to find and re-link disk %s with udevadm after retrying for %v: %v", diskName, pollTimeout, err)
return "", fmt.Errorf("failed to find and re-link disk %s with udevadm after retrying for %v: %v", deviceName, pollTimeout, err)
}

return devicePath, nil
}

func udevadmTriggerForDiskIfExists(diskName string) error {
func udevadmTriggerForDiskIfExists(deviceName string) error {
devToSCSI := map[string]string{}
sds, err := filepath.Glob(diskSDPattern)
if err != nil {
Expand All @@ -225,7 +225,7 @@ func udevadmTriggerForDiskIfExists(diskName string) error {
return fmt.Errorf("failed to get SCSI Serial num: %v", err)
}
devToSCSI[devSDX] = scsiSerial
if scsiSerial == diskName {
if scsiSerial == deviceName {
// Found the disk that we're looking for so run a trigger on it
// to resolve its /dev/by-id/ path
klog.Warningf("udevadm --trigger running to fix disk at path %s which has SCSI ID %s", devSDX, scsiSerial)
Expand All @@ -236,8 +236,8 @@ func udevadmTriggerForDiskIfExists(diskName string) error {
return nil
}
}
klog.Warningf("udevadm --trigger requested to fix disk %s but no such disk was found in %v", diskName, devToSCSI)
return fmt.Errorf("udevadm --trigger requested to fix disk %s but no such disk was found", diskName)
klog.Warningf("udevadm --trigger requested to fix disk %s but no such disk was found in %v", deviceName, devToSCSI)
return fmt.Errorf("udevadm --trigger requested to fix disk %s but no such disk was found", deviceName)
}

// Calls "udevadm trigger --action=change" on the specified drive. drivePath
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/tests/multi_zone_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ var _ = Describe("GCE PD CSI Driver Multi-Zone", func() {
if i >= 1 {
readOnly = true
}
testAttachWriteReadDetach(volID, volName, testContext.Instance, testContext.Client, readOnly)
err = testAttachWriteReadDetach(volID, volName, testContext.Instance, testContext.Client, readOnly)
Expect(err).To(BeNil(), "failed volume lifecycle checks")
i = i + 1
}
})
Expand Down

0 comments on commit 2a39852

Please sign in to comment.