Skip to content

Commit 9ee017c

Browse files
committed
fix: use nsenter-aware iscsiadmCmd for iSCSI session recovery
The health monitor's recovery path was calling iscsiadm directly via exec.CommandContext instead of the nsenter-aware iscsiadmCmd wrapper, causing all recovery commands to silently fail in containerized environments (the CSI node pod). Additionally, getISCSISessionInfo could not correlate devices to their target IQN when sessions were in transport-offline/blocked state, because the kernel removes sysfs iscsi_session symlinks for those sessions. The fallback parsed `iscsiadm -m session` output but returned the first IQN found with no device correlation. Fix both issues: - Replace all bare exec.CommandContext("iscsiadm") calls in node_recovery.go and node_health.go with iscsiadmCmd() - Rewrite the iscsiadm fallback to use `-m session -P 3` which includes "Attached scsi disk" lines, allowing correct device-to-IQN correlation even for offline sessions
1 parent 8ad7425 commit 9ee017c

2 files changed

Lines changed: 68 additions & 16 deletions

File tree

pkg/driver/node_health.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ func getISCSISessionState(ctx context.Context, devicePath string) (string, error
338338
}
339339

340340
// Alternative: use iscsiadm to check session state
341-
cmd := exec.CommandContext(ctx, "iscsiadm", "-m", "session", "-P", "1")
341+
cmd := iscsiadmCmd(ctx, "-m", "session", "-P", "1")
342342
output, cmdErr := cmd.CombinedOutput()
343343
if cmdErr != nil {
344344
return "", fmt.Errorf("iscsiadm failed: %w", cmdErr)

pkg/driver/node_recovery.go

Lines changed: 67 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,12 @@ func (s *NodeService) recoverNVMeOFConnections(ctx context.Context) int {
377377
}
378378

379379
// getISCSISessionInfo extracts the IQN and portal from sysfs for a given iSCSI device.
380+
// It first tries the direct sysfs path (works for healthy sessions), then falls back
381+
// to parsing `iscsiadm -m session -P 3` output to correlate the device name to its
382+
// target IQN (works even when the session is transport-offline/blocked and sysfs
383+
// session symlinks are gone).
380384
func getISCSISessionInfo(ctx context.Context, sysBlockPath string) (iqn, portal string) {
385+
// Phase 1: Try sysfs — fast path for sessions that still have their symlinks.
381386
// The target IQN is in /sys/block/sdX/device/../../iscsi_session/session*/targetname
382387
sessionGlob := sysBlockPath + "/device/../../iscsi_session/session*"
383388
sessions, err := filepath.Glob(sessionGlob)
@@ -409,21 +414,68 @@ func getISCSISessionInfo(ctx context.Context, sysBlockPath string) (iqn, portal
409414
}
410415
}
411416

412-
// Fallback: parse iscsiadm session output
413-
cmd := exec.CommandContext(ctx, "iscsiadm", "-m", "session")
417+
// Phase 2: sysfs failed (common for transport-offline/blocked sessions where the
418+
// kernel has removed the iscsi_session symlinks). Use iscsiadm -m session -P 3 to
419+
// get the full session detail including attached disk names, then correlate the
420+
// device name back to the target IQN.
421+
devName := filepath.Base(sysBlockPath) // e.g. "sda"
422+
return getISCSISessionInfoFromIscsiadm(ctx, devName)
423+
}
424+
425+
// getISCSISessionInfoFromIscsiadm uses `iscsiadm -m session -P 3` to find the IQN
426+
// and portal for a specific device name. The -P 3 output includes "Attached scsi disk"
427+
// lines that let us correlate a device back to its target, even when sysfs session
428+
// symlinks are gone (transport-offline/blocked sessions).
429+
func getISCSISessionInfoFromIscsiadm(ctx context.Context, devName string) (iqn, portal string) {
430+
sessionCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
431+
defer cancel()
432+
433+
cmd := iscsiadmCmd(sessionCtx, "-m", "session", "-P", "3")
414434
output, err := cmd.CombinedOutput()
415435
if err != nil {
436+
klog.V(4).Infof("iscsiadm -m session -P 3 failed: %v", err)
416437
return "", ""
417438
}
418439

419-
// Parse lines like: "tcp: [3] 10.10.20.100:3260,1 iqn.2137-04.storage.nasty:vol-name (non-flash)"
440+
// Parse the -P 3 output which has sections per target:
441+
// Target: iqn.2137-04.storage.nasty:vol-name (non-flash)
442+
// Current Portal: 10.10.20.100:3260,1
443+
// ...
444+
// Attached scsi disk sda State: transport-offline
445+
var currentIQN, currentPortal string
420446
for _, line := range strings.Split(string(output), "\n") {
421-
fields := strings.Fields(line)
422-
if len(fields) >= 4 {
423-
candidateIQN := strings.TrimSpace(fields[3])
424-
if strings.HasPrefix(candidateIQN, "iqn.") {
425-
// Return first match — caller will need to correlate with device
426-
return candidateIQN, strings.TrimSuffix(fields[2], ",1")
447+
trimmed := strings.TrimSpace(line)
448+
449+
if strings.HasPrefix(trimmed, "Target:") {
450+
targetLine := strings.TrimPrefix(trimmed, "Target:")
451+
targetLine = strings.TrimSpace(targetLine)
452+
// IQN may be followed by "(non-flash)" etc — take first field
453+
if fields := strings.Fields(targetLine); len(fields) > 0 {
454+
currentIQN = fields[0]
455+
}
456+
currentPortal = ""
457+
continue
458+
}
459+
460+
if strings.HasPrefix(trimmed, "Current Portal:") {
461+
portalLine := strings.TrimPrefix(trimmed, "Current Portal:")
462+
portalLine = strings.TrimSpace(portalLine)
463+
// Strip ",tpg" suffix from "10.10.20.100:3260,1"
464+
if idx := strings.LastIndex(portalLine, ","); idx != -1 {
465+
currentPortal = portalLine[:idx]
466+
} else {
467+
currentPortal = portalLine
468+
}
469+
continue
470+
}
471+
472+
if strings.Contains(trimmed, "Attached scsi disk") {
473+
// "Attached scsi disk sda State: transport-offline"
474+
parts := strings.Fields(trimmed)
475+
for i, part := range parts {
476+
if part == "disk" && i+1 < len(parts) && parts[i+1] == devName {
477+
return currentIQN, currentPortal
478+
}
427479
}
428480
}
429481
}
@@ -440,9 +492,9 @@ func recoverISCSISession(ctx context.Context, iqn, portal string) error {
440492

441493
var rescanCmd *exec.Cmd
442494
if portal != "" {
443-
rescanCmd = exec.CommandContext(rescanCtx, "iscsiadm", "-m", "session", "-r", iqn, "--rescan")
495+
rescanCmd = iscsiadmCmd(rescanCtx, "-m", "session", "-r", iqn, "--rescan")
444496
} else {
445-
rescanCmd = exec.CommandContext(rescanCtx, "iscsiadm", "-m", "session", "--rescan")
497+
rescanCmd = iscsiadmCmd(rescanCtx, "-m", "session", "--rescan")
446498
}
447499
if output, err := rescanCmd.CombinedOutput(); err != nil {
448500
klog.V(4).Infof("Session rescan for %s: %v (%s)", iqn, err, strings.TrimSpace(string(output)))
@@ -454,7 +506,7 @@ func recoverISCSISession(ctx context.Context, iqn, portal string) error {
454506
// Check session state after rescan using iscsiadm
455507
checkCtx, checkCancel := context.WithTimeout(ctx, 5*time.Second)
456508
defer checkCancel()
457-
checkCmd := exec.CommandContext(checkCtx, "iscsiadm", "-m", "session", "-P", "1")
509+
checkCmd := iscsiadmCmd(checkCtx, "-m", "session", "-P", "1")
458510
checkOutput, checkErr := checkCmd.CombinedOutput()
459511
if checkErr == nil && strings.Contains(string(checkOutput), iscsiSessionStateLoggedIn) {
460512
return nil // Recovered via rescan
@@ -465,7 +517,7 @@ func recoverISCSISession(ctx context.Context, iqn, portal string) error {
465517

466518
logoutCtx, logoutCancel := context.WithTimeout(ctx, 15*time.Second)
467519
defer logoutCancel()
468-
logoutCmd := exec.CommandContext(logoutCtx, "iscsiadm", "-m", "node", "-T", iqn, "--logout")
520+
logoutCmd := iscsiadmCmd(logoutCtx, "-m", "node", "-T", iqn, "--logout")
469521
if output, err := logoutCmd.CombinedOutput(); err != nil {
470522
klog.V(4).Infof("Logout for recovery of %s: %v (%s)", iqn, err, strings.TrimSpace(string(output)))
471523
}
@@ -479,9 +531,9 @@ func recoverISCSISession(ctx context.Context, iqn, portal string) error {
479531

480532
var loginCmd *exec.Cmd
481533
if portal != "" {
482-
loginCmd = exec.CommandContext(loginCtx, "iscsiadm", "-m", "node", "-T", iqn, "-p", portal, "--login")
534+
loginCmd = iscsiadmCmd(loginCtx, "-m", "node", "-T", iqn, "-p", portal, "--login")
483535
} else {
484-
loginCmd = exec.CommandContext(loginCtx, "iscsiadm", "-m", "node", "-T", iqn, "--login")
536+
loginCmd = iscsiadmCmd(loginCtx, "-m", "node", "-T", iqn, "--login")
485537
}
486538
output, err := loginCmd.CombinedOutput()
487539
if err != nil {

0 commit comments

Comments
 (0)