Do not log out of stale portals during iSCSI self-healing

This commit changes iSCSI self-healing such that the logout remediation is never performed for stale portals. Instead, if a session is stale for an extended period of time, Trident will gather additional session and connection state then log a warning to administrators to indicate which portals are potentially unhealthy.

Author: jwebster7

utils/iscsi/iscsi.go

@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"regexp"
 	"sort"
 	"strconv"
@@ -42,6 +43,7 @@ const (
 	SessionInfoSource          = "sessionSource"
 	SessionSourceCurrentStatus = "currentStatus"
 	SessionSourceNodeStage     = "nodeStage"
+	sessionConnectionStateUp   = "up"
 
 	iscsiadmLoginTimeoutValue = 10
 	iscsiadmLoginTimeout      = iscsiadmLoginTimeoutValue * time.Second
@@ -1115,7 +1117,109 @@ func (client *Client) portalsToLogin(ctx context.Context, targetIQN string, port
 	return portalsNotLoggedIn, loggedIn, nil
 }
 
-// IsSessionStale - reads /sys/class/iscsi_session/session<sid>/state and returns true if it is not "LOGGED_IN".
+// getSessionConnectionsState returns the state of iscsi session connections stored in:
+// '/sys/class/iscsi_session/session<ID>/device/connection<ID>:0/iscsi_connection/connection<ID>:0.
+func (client *Client) getSessionConnectionsState(ctx context.Context, sessionID string) []string {
+	Logc(ctx).WithField("sessionID", sessionID).Debug(">>>> iscsi.getSessionConnectionsState")
+	defer Logc(ctx).Debug("<<<< iscsi.getSessionConnectionsState")
+
+	// Find the session device dirs under: '/sys/class/iscsi_session/session<ID>/device/'.
+	sessionName := fmt.Sprintf("session%s", sessionID)
+	sessionDevicePath := filepath.Join(client.chrootPathPrefix, "sys", "class", "iscsi_session", sessionName, "device")
+	sessionDeviceEntries, err := client.os.ReadDir(sessionDevicePath)
+	if err != nil {
+		Logc(ctx).WithField("path", sessionDevicePath).WithError(err).Error("Could not read session dirs.")
+		return nil
+	}
+
+	const notFound = "<NOT FOUND>"
+	var errs error
+
+	// Dynamically discover the 'state' for all underlying connections and return them.
+	connectionStates := make([]string, 0)
+	for _, entry := range sessionDeviceEntries {
+		// Only consider: `/sys/class/iscsi_session/session<ID>/device/connection<ID>:0`
+		connection := entry.Name()
+		if !strings.HasPrefix(connection, "connection") {
+			continue
+		}
+
+		// At this point, we know we're looking at something like:
+		// '/sys/class/iscsi_session/session<ID>/device/connection<ID>:0' but we need:
+		// '/sys/class/iscsi_session/session<ID>/device/connection<ID>:0/iscsi_connection/connection<ID>:0'
+		state := notFound
+		statePath := filepath.Join(sessionDevicePath, connection, "iscsi_connection", connection, "state")
+		rawState, err := client.os.ReadFile(statePath)
+		if err != nil {
+			errs = errors.Join(errs, fmt.Errorf("failed to read session state at: '%s'; %w", statePath, err))
+		} else if len(rawState) != 0 {
+			state = strings.TrimSpace(string(rawState))
+		}
+
+		// If the connection state is "up" or not found, further inspection won't be helpful. Ignore this and move on.
+		if state == sessionConnectionStateUp || state == notFound {
+			continue
+		}
+
+		// Get the persistent address. This is the IP associated with a session.
+		address := notFound
+		addrPath := filepath.Join(sessionDevicePath, connection, "iscsi_connection", connection, "persistent_address")
+		rawAddress, err := client.os.ReadFile(addrPath)
+		if err != nil {
+			errs = errors.Join(errs, fmt.Errorf("failed to read connection IP at: '%s'; %w", addrPath, err))
+		} else if len(rawAddress) != 0 {
+			address = strings.TrimSpace(string(rawAddress))
+		}
+
+		// Get the persistent port. This is the port associated with a session.
+		port := notFound
+		portPath := filepath.Join(sessionDevicePath, connection, "iscsi_connection", connection, "persistent_port")
+		rawPort, err := client.os.ReadFile(portPath)
+		if err != nil {
+			errs = errors.Join(errs, fmt.Errorf("failed to read connection port at: '%s'; %w", portPath, err))
+		} else if len(rawPort) != 0 {
+			port = strings.TrimSpace(string(rawPort))
+		}
+
+		portal := fmt.Sprintf("%s:%s", address, port)
+
+		// This will allow Trident to communicate which portals have bad connections.
+		connectionState := fmt.Sprintf("\"portal:'%s'; connection:'%s'; state:'%s'\"", portal, connection, state)
+		connectionStates = append(connectionStates, connectionState)
+	}
+
+	if errs != nil {
+		Logc(ctx).WithError(errs).Error("Could not discover state of iSCSI connections.")
+	}
+
+	return connectionStates
+}
+
+// getSessionState returns the state stored in /sys/class/iscsi_session/session<sid>/state.
+// If no state is found, an empty string is returned.
+func (client *Client) getSessionState(ctx context.Context, sessionID string) string {
+	Logc(ctx).WithField("sessionID", sessionID).Debug(">>>> iscsi.getSessionState")
+	defer Logc(ctx).Debug("<<<< iscsi.getSessionState")
+
+	// Find the session state from the session at /sys/class/iscsi_session/sessionXXX/state
+	filename := fmt.Sprintf(client.chrootPathPrefix+"/sys/class/iscsi_session/session%s/state", sessionID)
+	sessionStateBytes, err := client.os.ReadFile(filename)
+	if err != nil {
+		Logc(ctx).WithField("path", filename).WithError(err).Error("Could not read session state file.")
+		return ""
+	}
+
+	sessionState := strings.TrimSpace(string(sessionStateBytes))
+	Logc(ctx).WithFields(LogFields{
+		"sessionID":    sessionID,
+		"sessionState": sessionState,
+		"sysfsFile":    filename,
+	}).Debug("Found iSCSI session state.")
+
+	return sessionState
+}
+
+// isSessionStale - reads /sys/class/iscsi_session/session<sid>/state and returns true if it is not "LOGGED_IN".
 // Looks that the state of an already established session to identify if it is
 // logged in or not, if it is not logged in then it could be a stale session.
 // For now, we are relying on the sysfs files
@@ -1127,10 +1231,7 @@ func (client *Client) isSessionStale(ctx context.Context, sessionID string) bool
 	filename := fmt.Sprintf(client.chrootPathPrefix+"/sys/class/iscsi_session/session%s/state", sessionID)
 	sessionStateBytes, err := client.os.ReadFile(filename)
 	if err != nil {
-		Logc(ctx).WithFields(LogFields{
-			"path":  filename,
-			"error": err,
-		}).Error("Could not read session state file")
+		Logc(ctx).WithField("path", filename).WithError(err).Error("Could not read session state file.")
 		return false
 	}
 
@@ -2655,7 +2756,6 @@ func (client *Client) InspectAllISCSISessions(
 	var candidateStalePortals, candidateNonStalePortal []string
 
 	for portal, publishedSessionData := range publishedSessions.Info {
-
 		logFields := LogFields{"portal": portal}
 
 		var publishedPortalInfo, currentPortalInfo *models.PortalInfo
@@ -2725,6 +2825,14 @@ func (client *Client) InspectAllISCSISessions(
 
 			if action != models.NoAction {
 				candidateStalePortals = append(candidateStalePortals, portal)
+
+				// At this point we know the iSCSI session has been stale for some time but do not know why.
+				// Retrieve additional state from sysfs and inform the admin of an issue.
+				Logc(ctx).WithFields(LogFields{
+					"portal":          portal,
+					"sessionState":    client.getSessionState(ctx, currentPortalInfo.SessionNumber),
+					"connectionState": client.getSessionConnectionsState(ctx, currentPortalInfo.SessionNumber),
+				}).Warn("Portal requires manual intervention; storage network connection may be unstable.")
 			}
 			continue
 		}
@@ -2794,7 +2902,10 @@ func isStalePortal(
 	} else if timeNow.Sub(publishedPortalInfo.FirstIdentifiedStaleAt) >= iSCSISessionWaitTime {
 		Logc(ctx).WithFields(logFields).Warningf("Portal exceeded stale wait time at %v; adding to stale portals list.",
 			timeNow)
-		return models.LogoutLoginScan
+		// Things like storage platform upgrades or extended network outages may result in a FREE or FAILED state on the
+		// session. At this point in time, there isn't a reliable mechanism to know when it would be safe to perform a
+		// Logout remediation step, so only ever perform a LoginScan.
+		return models.LoginScan
 	} else {
 		Logc(ctx).WithFields(logFields).Warningf("Portal has not exceeded stale wait time at %v.", timeNow)
 	}

utils/iscsi/iscsi_test.go

@@ -5879,7 +5879,7 @@ func TestIsStalePortal(t *testing.T) {
 			SessionWaitTime: 10 * time.Second,
 			TimeNow:         time.Now().Add(20 * time.Second),
 			Portal:          ipList[0],
-			ResultAction:    models.LogoutLoginScan,
+			ResultAction:    models.LoginScan,
 			SimulateConditions: func(publishedSessions, currentSessions *models.ISCSISessions, portal string) {
 				publishedSessions.Info[portal].PortalInfo.FirstIdentifiedStaleAt = time.Now()
 			},
@@ -5943,7 +5943,7 @@ func TestIsStalePortal(t *testing.T) {
 			SessionWaitTime: 10 * time.Second,
 			TimeNow:         time.Now().Add(20 * time.Second),
 			Portal:          ipList[0],
-			ResultAction:    models.LogoutLoginScan,
+			ResultAction:    models.LoginScan,
 			SimulateConditions: func(publishedSessions, currentSessions *models.ISCSISessions, portal string) {
 				publishedSessions.Info[portal].PortalInfo.Credentials = chapCredentials[0]
 				publishedSessions.Info[portal].PortalInfo.FirstIdentifiedStaleAt = time.Now()
@@ -5954,7 +5954,6 @@ func TestIsStalePortal(t *testing.T) {
 	for _, input := range inputs {
 		t.Run(input.TestName, func(t *testing.T) {
 			portal := input.Portal
-
 			input.SimulateConditions(input.PublishedPortals, input.CurrentPortals, portal)
 
 			publishedPortalData, _ := input.PublishedPortals.Info[portal]

utils/iscsi_test.go

@@ -1,4 +1,4 @@
-// Copyright 2024 NetApp, Inc. All Rights Reserved.
+// Copyright 2025 NetApp, Inc. All Rights Reserved.
 
 package utils