Skip to content

Commit 77ed69d

Browse files
jharrodjharrod
authored
Revert "Derive device path in unstage"
1 parent 3e0c838 commit 77ed69d

File tree

6 files changed

+1409
-1464
lines changed

6 files changed

+1409
-1464
lines changed

‎frontend/csi/node_server.go‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package csi
55
import (
66
"context"
77
"crypto/rand"
8-
"encoding/hex"
98
"fmt"
109
"math/big"
1110
"os"
@@ -1960,20 +1959,6 @@ func (p *Plugin) updateChapInfoFromController(
19601959
func (p *Plugin) nodeUnstageISCSIVolume(
19611960
ctx context.Context, req *csi.NodeUnstageVolumeRequest, publishInfo *models.VolumePublishInfo, force bool,
19621961
) 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-
19771962
hostSessionMap := iscsiUtils.GetISCSIHostSessionMapForTarget(ctx, publishInfo.IscsiTargetIQN)
19781963
// TODO: (jharrod) This is a temporary fix to handle the case where the hostSessionMap is empty. We need to
19791964
// refactor nodeUnstageISCSIVolume to handle this case more gracefully and not rely on the hostSessionMap.

‎frontend/csi/node_server_test.go‎

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2074,7 +2074,6 @@ 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())
20782077
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
20792078
gomock.Any(), gomock.Any()).Return(nil)
20802079
return mockDeviceClient
@@ -2115,7 +2114,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
21152114
},
21162115
getDeviceClient: func() devices.Devices {
21172116
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2118-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
21192117
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
21202118
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).Return(nil)
21212119
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosed(gomock.Any(), mockDevicePath).Return(nil)
@@ -2162,7 +2160,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
21622160
},
21632161
getDeviceClient: func() devices.Devices {
21642162
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2165-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
21662163
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
21672164
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
21682165
Return(nil)
@@ -2207,7 +2204,7 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
22072204
},
22082205
getDeviceClient: func() devices.Devices {
22092206
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2210-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
2207+
22112208
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
22122209
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
22132210
Return(fmt.Errorf("mock error"))
@@ -2226,7 +2223,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
22262223
},
22272224
getDeviceClient: func() devices.Devices {
22282225
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2229-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
22302226
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
22312227
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
22322228
Return(nil)
@@ -2269,7 +2265,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
22692265
},
22702266
getDeviceClient: func() devices.Devices {
22712267
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2272-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
22732268
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
22742269
gomock.Any(), gomock.Any()).Return(nil)
22752270
return mockDeviceClient
@@ -2298,7 +2293,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
22982293
},
22992294
getDeviceClient: func() devices.Devices {
23002295
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2301-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
23022296
return mockDeviceClient
23032297
},
23042298
},
@@ -2321,7 +2315,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
23212315
},
23222316
getDeviceClient: func() devices.Devices {
23232317
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2324-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
23252318
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return("", fmt.Errorf(
23262319
"mock error"))
23272320
return mockDeviceClient
@@ -2342,7 +2335,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
23422335
},
23432336
getDeviceClient: func() devices.Devices {
23442337
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2345-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
23462338
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
23472339
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
23482340
Return(nil)
@@ -2376,7 +2368,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
23762368
},
23772369
getDeviceClient: func() devices.Devices {
23782370
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2379-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
23802371
mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
23812372
mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
23822373
Return(nil)
@@ -2420,7 +2411,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
24202411
},
24212412
getDeviceClient: func() devices.Devices {
24222413
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2423-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
24242414
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
24252415
gomock.Any(), gomock.Any()).Return(nil)
24262416
return mockDeviceClient
@@ -2459,7 +2449,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
24592449
},
24602450
getDeviceClient: func() devices.Devices {
24612451
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2462-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
24632452
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
24642453
gomock.Any(), gomock.Any()).Return(nil)
24652454
return mockDeviceClient
@@ -2500,7 +2489,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
25002489
},
25012490
getDeviceClient: func() devices.Devices {
25022491
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2503-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
25042492
return mockDeviceClient
25052493
},
25062494
getMountClient: func() mount.Mount {
@@ -2535,7 +2523,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
25352523
},
25362524
getDeviceClient: func() devices.Devices {
25372525
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2538-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
25392526
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
25402527
gomock.Any(), gomock.Any()).Return(fmt.Errorf("mock error"))
25412528
return mockDeviceClient
@@ -2571,7 +2558,6 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
25712558
},
25722559
getDeviceClient: func() devices.Devices {
25732560
mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
2574-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
25752561
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
25762562
gomock.Any(), gomock.Any()).Return(nil)
25772563
return mockDeviceClient
@@ -3522,7 +3508,6 @@ func TestNodeUnstageVolume_Multithreaded(t *testing.T) {
35223508
mockISCSIClient.EXPECT().GetDeviceInfoForLUN(gomock.Any(), gomock.Any(),
35233509
gomock.Any(), gomock.Any(), false).Return(mockDevice, nil)
35243510

3525-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
35263511
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
35273512
gomock.Any(), gomock.Any()).Return(nil)
35283513

@@ -3686,7 +3671,6 @@ func TestNodeUnstageVolume_Multithreaded(t *testing.T) {
36863671
gomock.Any(), false, false).Return("", nil)
36873672
mockISCSIClient.EXPECT().GetDeviceInfoForLUN(gomock.Any(), gomock.Any(),
36883673
gomock.Any(), gomock.Any(), false).Return(mockDevice, nil)
3689-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
36903674
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
36913675
gomock.Any(), gomock.Any()).Return(nil)
36923676
mockMountClient.EXPECT().UmountAndRemoveTemporaryMountPoint(gomock.Any(), gomock.Any()).Return(nil)
@@ -5744,7 +5728,6 @@ func Test_NodeStage_NodeUnstage_Multithreaded(t *testing.T) {
57445728
gomock.Any(), false, false).Return("", nil).Times(numOfRequests).Times(2)
57455729
mockISCSIClient.EXPECT().RemovePortalsFromSession(gomock.Any(), gomock.Any(), gomock.Any()).Times(numOfRequests).Times(2)
57465730
mockISCSIClient.EXPECT().Logout(gomock.Any(), gomock.Any(), gomock.Any()).Times(numOfRequests).Times(2)
5747-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any()).Times(2)
57485731
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
57495732
gomock.Any(), gomock.Any()).Return(nil)
57505733
volumeTrackingInfo := &models.VolumeTrackingInfo{
@@ -6117,7 +6100,6 @@ func TestNodeUnstageISCSIVolume_Multithreaded(t *testing.T) {
61176100
mockISCSIClient.EXPECT().Logout(gomock.Any(), gomock.Any(), gomock.Any())
61186101
mockISCSIClient.EXPECT().PrepareDeviceForRemoval(gomock.Any(), gomock.Any(), gomock.Any(),
61196102
gomock.Any(), false, false).Return("", nil)
6120-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
61216103
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
61226104
gomock.Any(), gomock.Any()).Return(nil)
61236105
mockIscsiReconcileUtilsClient.EXPECT().GetISCSIHostSessionMapForTarget(gomock.Any(),
@@ -6205,7 +6187,6 @@ func TestNodeUnstageISCSIVolume_Multithreaded(t *testing.T) {
62056187
mockISCSIClient.EXPECT().TargetHasMountedDevice(gomock.Any(), gomock.Any()).Return(false, nil)
62066188
mockISCSIClient.EXPECT().SafeToLogOut(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
62076189
mockISCSIClient.EXPECT().RemovePortalsFromSession(gomock.Any(), gomock.Any(), gomock.Any())
6208-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
62096190
mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
62106191
gomock.Any(), gomock.Any()).Return(nil)
62116192
mockISCSIClient.EXPECT().Logout(gomock.Any(), gomock.Any(), gomock.Any())
@@ -6223,7 +6204,6 @@ func TestNodeUnstageISCSIVolume_Multithreaded(t *testing.T) {
62236204

62246205
// Setting up expectations for remaining requests
62256206
for i := numOfParallelRequestsAllowed; i < numOfRequests; i++ {
6226-
mockDeviceClient.EXPECT().GetMultipathDeviceBySerial(gomock.Any(), gomock.Any())
62276207
mockISCSIClient.EXPECT().GetDeviceInfoForLUN(gomock.Any(), gomock.Any(),
62286208
gomock.Any(), gomock.Any(), false).Return(mockDevice, nil)
62296209
mockISCSIClient.EXPECT().RemoveLUNFromSessions(gomock.Any(), gomock.Any(), gomock.Any())

0 commit comments

Comments
 (0)