Skip to content

Commit b806fd6

Browse files
jharrodjharrod
authored
Derive device path in unstage
Derive the device path in iSCSI unstage in case the path has changed since tracking file creation.
1 parent 7ba0814 commit b806fd6

File tree

6 files changed

+1464
-1409
lines changed

6 files changed

+1464
-1409
lines changed

‎frontend/csi/node_server.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package csi
55
import (
66
"context"
77
"crypto/rand"
8+
"encoding/hex"
89
"fmt"
910
"math/big"
1011
"os"
@@ -1959,6 +1960,20 @@ func (p *Plugin) updateChapInfoFromController(
19591960
func (p *Plugin) nodeUnstageISCSIVolume(
19601961
ctx context.Context, req *csi.NodeUnstageVolumeRequest, publishInfo *models.VolumePublishInfo, force bool,
19611962
) error {
1963+
// Derive the device path using the LunSerial. The publishInfo.DevicePath may be incorrect due to Kernel actions.
1964+
// Fallback to using the publishInfo.DevicePath if the multipath device cannot be derived.
1965+
multipathDevice, err := p.devices.GetMultipathDeviceBySerial(ctx, hex.EncodeToString([]byte(publishInfo.IscsiLunSerial)))
1966+
if err != nil {
1967+
Logc(ctx).WithError(err).WithField("LunSerial", publishInfo.IscsiLunSerial).Debug(
1968+
"Error finding multipath device by serial.")
1969+
} else {
1970+
Logc(ctx).WithFields(LogFields{
1971+
"multipathDevice": multipathDevice,
1972+
"LunSerial": publishInfo.IscsiLunSerial,
1973+
}).Debug("Found multipath device by serial.")
1974+
publishInfo.DevicePath = iscsi.DevPrefix + multipathDevice
1975+
}
1976+
19621977
hostSessionMap := iscsiUtils.GetISCSIHostSessionMapForTarget(ctx, publishInfo.IscsiTargetIQN)
19631978
// TODO: (jharrod) This is a temporary fix to handle the case where the hostSessionMap is empty. We need to
19641979
// refactor nodeUnstageISCSIVolume to handle this case more gracefully and not rely on the hostSessionMap.

‎frontend/csi/node_server_test.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2074,6 +2074,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
20742074
},
20752075
getDeviceClient: func() devices.Devices {
20762076
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2077+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
20772078
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
20782079
gomock.Any(), gomock.Any()).Return(nil)
20792080
return mockDeviceClient
@@ -2114,6 +2115,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
21142115
},
21152116
getDeviceClient: func() devices.Devices {
21162117
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2118+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
21172119
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
21182120
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).Return(nil)
21192121
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosed(gomock.Any(), mockDevicePath).Return(nil)
@@ -2160,6 +2162,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
21602162
},
21612163
getDeviceClient: func() devices.Devices {
21622164
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2165+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
21632166
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
21642167
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
21652168
Return(nil)
@@ -2204,7 +2207,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
22042207
},
22052208
getDeviceClient: func() devices.Devices {
22062209
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2207-
2210+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
22082211
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
22092212
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
22102213
Return(fmt.Errorf("mock error"))
@@ -2223,6 +2226,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
22232226
},
22242227
getDeviceClient: func() devices.Devices {
22252228
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2229+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
22262230
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
22272231
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
22282232
Return(nil)
@@ -2265,6 +2269,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
22652269
},
22662270
getDeviceClient: func() devices.Devices {
22672271
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2272+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
22682273
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
22692274
gomock.Any(), gomock.Any()).Return(nil)
22702275
return mockDeviceClient
@@ -2293,6 +2298,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
22932298
},
22942299
getDeviceClient: func() devices.Devices {
22952300
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2301+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
22962302
return mockDeviceClient
22972303
},
22982304
},
@@ -2315,6 +2321,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
23152321
},
23162322
getDeviceClient: func() devices.Devices {
23172323
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2324+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
23182325
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return("", fmt.Errorf(
23192326
"mock error"))
23202327
return mockDeviceClient
@@ -2335,6 +2342,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
23352342
},
23362343
getDeviceClient: func() devices.Devices {
23372344
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2345+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
23382346
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
23392347
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
23402348
Return(nil)
@@ -2368,6 +2376,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
23682376
},
23692377
getDeviceClient: func() devices.Devices {
23702378
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2379+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
23712380
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
23722381
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
23732382
Return(nil)
@@ -2411,6 +2420,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
24112420
},
24122421
getDeviceClient: func() devices.Devices {
24132422
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2423+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
24142424
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
24152425
gomock.Any(), gomock.Any()).Return(nil)
24162426
return mockDeviceClient
@@ -2449,6 +2459,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
24492459
},
24502460
getDeviceClient: func() devices.Devices {
24512461
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2462+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
24522463
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
24532464
gomock.Any(), gomock.Any()).Return(nil)
24542465
return mockDeviceClient
@@ -2489,6 +2500,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
24892500
},
24902501
getDeviceClient: func() devices.Devices {
24912502
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2503+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
24922504
return mockDeviceClient
24932505
},
24942506
getMountClient: func() mount.Mount {
@@ -2523,6 +2535,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
25232535
},
25242536
getDeviceClient: func() devices.Devices {
25252537
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2538+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
25262539
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
25272540
gomock.Any(), gomock.Any()).Return(fmt.Errorf("mock error"))
25282541
return mockDeviceClient
@@ -2558,6 +2571,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
25582571
},
25592572
getDeviceClient: func() devices.Devices {
25602573
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2574+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
25612575
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
25622576
gomock.Any(), gomock.Any()).Return(nil)
25632577
return mockDeviceClient
@@ -3508,6 +3522,7 @@ func TestNodeUnstageVolume_Multithreaded(t *testing.T) {
35083522
mockISCSIClient.EXPECT().GetDeviceInfoForLUN(gomock.Any(), gomock.Any(),
35093523
gomock.Any(), gomock.Any(), false).Return(mockDevice, nil)
35103524

3525+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
35113526
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
35123527
gomock.Any(), gomock.Any()).Return(nil)
35133528

@@ -3671,6 +3686,7 @@ func TestNodeUnstageVolume_Multithreaded(t *testing.T) {
36713686
gomock.Any(), false, false).Return("", nil)
36723687
mockISCSIClient.EXPECT().GetDeviceInfoForLUN(gomock.Any(), gomock.Any(),
36733688
gomock.Any(), gomock.Any(), false).Return(mockDevice, nil)
3689+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
36743690
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
36753691
gomock.Any(), gomock.Any()).Return(nil)
36763692
mockMountClient.EXPECT().UmountAndRemoveTemporaryMountPoint(gomock.Any(), gomock.Any()).Return(nil)
@@ -5728,6 +5744,7 @@ func Test_NodeStage_NodeUnstage_Multithreaded(t *testing.T) {
57285744
gomock.Any(), false, false).Return("", nil).Times(numOfRequests).Times(2)
57295745
mockISCSIClient.EXPECT().RemovePortalsFromSession(gomock.Any(), gomock.Any(), gomock.Any()).Times(numOfRequests).Times(2)
57305746
mockISCSIClient.EXPECT().Logout(gomock.Any(), gomock.Any(), gomock.Any()).Times(numOfRequests).Times(2)
5747+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any()).Times(2)
57315748
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
57325749
gomock.Any(), gomock.Any()).Return(nil)
57335750
volumeTrackingInfo := &models.VolumeTrackingInfo{
@@ -6100,6 +6117,7 @@ func TestNodeUnstageISCSIVolume_Multithreaded(t *testing.T) {
61006117
mockISCSIClient.EXPECT().Logout(gomock.Any(), gomock.Any(), gomock.Any())
61016118
mockISCSIClient.EXPECT().PrepareDeviceForRemoval(gomock.Any(), gomock.Any(), gomock.Any(),
61026119
gomock.Any(), false, false).Return("", nil)
6120+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
61036121
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
61046122
gomock.Any(), gomock.Any()).Return(nil)
61056123
mockIscsiReconcileUtilsClient.EXPECT().GetISCSIHostSessionMapForTarget(gomock.Any(),
@@ -6187,6 +6205,7 @@ func TestNodeUnstageISCSIVolume_Multithreaded(t *testing.T) {
61876205
mockISCSIClient.EXPECT().TargetHasMountedDevice(gomock.Any(), gomock.Any()).Return(false, nil)
61886206
mockISCSIClient.EXPECT().SafeToLogOut(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
61896207
mockISCSIClient.EXPECT().RemovePortalsFromSession(gomock.Any(), gomock.Any(), gomock.Any())
6208+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
61906209
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
61916210
gomock.Any(), gomock.Any()).Return(nil)
61926211
mockISCSIClient.EXPECT().Logout(gomock.Any(), gomock.Any(), gomock.Any())
@@ -6204,6 +6223,7 @@ func TestNodeUnstageISCSIVolume_Multithreaded(t *testing.T) {
62046223

62056224
// Setting up expectations for remaining requests
62066225
for i := numOfParallelRequestsAllowed; i < numOfRequests; i++ {
6226+
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
62076227
mockISCSIClient.EXPECT().GetDeviceInfoForLUN(gomock.Any(), gomock.Any(),
62086228
gomock.Any(), gomock.Any(), false).Return(mockDevice, nil)
62096229
mockISCSIClient.EXPECT().RemoveLUNFromSessions(gomock.Any(), gomock.Any(), gomock.Any())

0 commit comments

Comments
 (0)