Node restart idempotency

Handle node cleanup upon a node restart for ontap-san/iSCSI

Author: jharrod

frontend/csi/node_server.go

@@ -1940,16 +1940,15 @@ func (p *Plugin) nodeUnstageISCSIVolume(
 		Logc(ctx).WithError(err).Debug("Could not find devices.")
 		return fmt.Errorf("could not get device info: %v", err)
 	} else if deviceInfo == nil {
-		Logc(ctx).Debug("Could not find devices, nothing to do.")
-		return nil
+		Logc(ctx).Debug("No devices found.")
 	}
 
 	// Remove Portal/LUN entries in self-healing map.
 	p.iscsi.RemoveLUNFromSessions(ctx, publishInfo, &publishedISCSISessions)
 
 	var luksMapperPath string
 	// If the multipath device is not present, the LUKS device should not exist.
-	if convert.ToBool(publishInfo.LUKSEncryption) && deviceInfo.MultipathDevice != "" {
+	if convert.ToBool(publishInfo.LUKSEncryption) && deviceInfo != nil && deviceInfo.MultipathDevice != "" {
 		fields := LogFields{
 			"lunID":           publishInfo.IscsiLunNumber,
 			"publishedDevice": publishInfo.DevicePath,
@@ -1985,18 +1984,21 @@ func (p *Plugin) nodeUnstageISCSIVolume(
 	}
 
 	// Delete the device from the host.
-	unmappedMpathDevice, err := p.iscsi.PrepareDeviceForRemoval(ctx, deviceInfo, publishInfo, nil, p.unsafeDetach,
-		force)
-	if err != nil {
-		if errors.IsISCSISameLunNumberError(err) {
-			// There is a need to pass all the publish infos this time
-			unmappedMpathDevice, err = p.iscsi.PrepareDeviceForRemoval(ctx, deviceInfo, publishInfo,
-				p.readAllTrackingFiles(ctx),
-				p.unsafeDetach, force)
-		}
+	var unmappedMpathDevice string
+	if deviceInfo != nil {
+		unmappedMpathDevice, err = p.iscsi.PrepareDeviceForRemoval(ctx, deviceInfo, publishInfo, nil, p.unsafeDetach,
+			force)
+		if err != nil {
+			if errors.IsISCSISameLunNumberError(err) {
+				// There is a need to pass all the publish infos this time
+				unmappedMpathDevice, err = p.iscsi.PrepareDeviceForRemoval(ctx, deviceInfo, publishInfo,
+					p.readAllTrackingFiles(ctx),
+					p.unsafeDetach, force)
+			}
 
-		if err != nil && !p.unsafeDetach {
-			return status.Error(codes.Internal, err.Error())
+			if err != nil && !p.unsafeDetach {
+				return status.Error(codes.Internal, err.Error())
+			}
 		}
 	}
 

frontend/csi/node_server_test.go

@@ -2379,12 +2379,30 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
 				mockISCSIClient := mock_iscsi.NewMockISCSI(gomock.NewController(t))
 				mockISCSIClient.EXPECT().GetDeviceInfoForLUN(gomock.Any(), gomock.Any(),
 					gomock.Any(), gomock.Any(), false).Return(nil, nil)
+				mockISCSIClient.EXPECT().RemoveLUNFromSessions(gomock.Any(), gomock.Any(), gomock.Any())
+				mockISCSIClient.EXPECT().TargetHasMountedDevice(gomock.Any(),
+					"iqn.1992-08.com.netapp:sn.a0e6b50f49e611efa8b5005056b33c0d:vs.2").Return(false, nil)
+				mockISCSIClient.EXPECT().SafeToLogOut(gomock.Any(), 6, 3).Return(true)
+				mockISCSIClient.EXPECT().RemovePortalsFromSession(gomock.Any(), gomock.Any(), gomock.Any())
+				mockISCSIClient.EXPECT().Logout(gomock.Any(), gomock.Any(), gomock.Any())
 				return mockISCSIClient
 			},
+			getMountClient: func() mount.Mount {
+				mockMountClient := mock_mount.NewMockMount(gomock.NewController(t))
+				mockMountClient.EXPECT().UmountAndRemoveTemporaryMountPoint(gomock.Any(), gomock.Any()).Return(nil)
+				return mockMountClient
+			},
 			getDeviceClient: func() devices.Devices {
 				mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
+				mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
+					gomock.Any(), gomock.Any()).Return(nil)
 				return mockDeviceClient
 			},
+			getNodeHelper: func() nodehelpers.NodeHelper {
+				mockNodeHelper := mockNodeHelpers.NewMockNodeHelper(gomock.NewController(t))
+				mockNodeHelper.EXPECT().DeleteTrackingInfo(gomock.Any(), gomock.Any()).Return(nil)
+				return mockNodeHelper
+			},
 		},
 		"SAN: iSCSI unstage: GetDeviceInfoForLUN error": {
 			assertError: assert.Error,

utils/iscsi/iscsi.go

@@ -796,7 +796,9 @@ func (client *Client) GetDeviceInfoForLUN(
 	if err != nil {
 		return nil, err
 	} else if len(devicesForLUN) == 0 {
-		return nil, fmt.Errorf("scan not completed for LUN %d on target %s", lunID, iSCSINodeName)
+		// No devices found may be due to a node reboot
+		Logc(ctx).WithFields(fields).Info("No devices found for LUN.")
+		return nil, nil
 	}
 
 	multipathDevice := ""

utils/iscsi/iscsi_test.go

@@ -2997,7 +2997,7 @@ func TestClient_getDeviceInfoForLUN(t *testing.T) {
 			getFileSystemUtils: func() afero.Fs {
 				return afero.NewMemMapFs()
 			},
-			assertError:        assert.Error,
+			assertError:        assert.NoError,
 			expectedDeviceInfo: nil,
 		},
 		"error getting devices for LUN": {
@@ -3035,7 +3035,7 @@ func TestClient_getDeviceInfoForLUN(t *testing.T) {
 			getFileSystemUtils: func() afero.Fs {
 				return afero.NewMemMapFs()
 			},
-			assertError:        assert.Error,
+			assertError:        assert.NoError,
 			expectedDeviceInfo: nil,
 		},
 		"no multipath device found": {