Unstage multipath flush fix

Add retries to multipath flush
NetApp · Jan 7, 2025 · 27f7cdc · 27f7cdc
1 parent 06eadbb
commit 27f7cdc
Show file tree

Hide file tree

Showing 8 changed files with 598 additions and 468 deletions.
diff --git a/frontend/csi/node_server.go b/frontend/csi/node_server.go
@@ -69,6 +69,11 @@ var (
 	afterInitialTrackingInfoWrite  = fiji.Register("afterInitialTrackingInfoWrite", "node_server")
 )
 
+const (
+	removeMultipathDeviceMappingRetries    = 4
+	removeMultipathDeviceMappingRetryDelay = 500 * time.Millisecond
+)
+
 func attemptLock(ctx context.Context, lockContext string, lockTimeout time.Duration) bool {
 	startTime := time.Now()
 	utils.Lock(ctx, lockContext, lockID)
@@ -1779,7 +1784,8 @@ func (p *Plugin) nodeUnstageISCSIVolume(
 	}
 
 	// If there is multipath device, flush(remove) mappings
-	if err := p.devices.RemoveMultipathDeviceMapping(ctx, unmappedMpathDevice); err != nil {
+	if err := p.devices.RemoveMultipathDeviceMappingWithRetries(ctx, unmappedMpathDevice,
+		removeMultipathDeviceMappingRetries, removeMultipathDeviceMappingRetryDelay); err != nil {
 		return err
 	}
 

diff --git a/frontend/csi/node_server_test.go b/frontend/csi/node_server_test.go
@@ -1984,7 +1984,8 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
 			},
 			getDeviceClient: func() devices.Devices {
 				mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
-				mockDeviceClient.EXPECT().RemoveMultipathDeviceMapping(gomock.Any(), gomock.Any()).Return(nil)
+				mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
+					gomock.Any(), gomock.Any()).Return(nil)
 				return mockDeviceClient
 			},
 			getMountClient: func() mount.Mount {
@@ -2026,7 +2027,8 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
 				mockDeviceClient.EXPECT().GetLUKSDeviceForMultipathDevice(gomock.Any()).Return(mockDevicePath, nil)
 				mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).Return(nil)
 				mockDeviceClient.EXPECT().EnsureLUKSDeviceClosed(gomock.Any(), mockDevicePath).Return(nil)
-				mockDeviceClient.EXPECT().RemoveMultipathDeviceMapping(gomock.Any(), gomock.Any()).Return(nil)
+				mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
+					gomock.Any(), gomock.Any()).Return(nil)
 				return mockDeviceClient
 			},
 			getMountClient: func() mount.Mount {
@@ -2072,7 +2074,8 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
 				mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
 					Return(nil)
 				mockDeviceClient.EXPECT().EnsureLUKSDeviceClosed(gomock.Any(), mockDevicePath).Return(nil)
-				mockDeviceClient.EXPECT().RemoveMultipathDeviceMapping(gomock.Any(), gomock.Any()).Return(nil)
+				mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
+					gomock.Any(), gomock.Any()).Return(nil)
 				return mockDeviceClient
 			},
 			getMountClient: func() mount.Mount {
@@ -2247,7 +2250,8 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
 				mockDeviceClient.EXPECT().EnsureLUKSDeviceClosedWithMaxWaitLimit(gomock.Any(), mockDevicePath).
 					Return(nil)
 				mockDeviceClient.EXPECT().EnsureLUKSDeviceClosed(gomock.Any(), mockDevicePath).Return(nil)
-				mockDeviceClient.EXPECT().RemoveMultipathDeviceMapping(gomock.Any(), gomock.Any()).Return(nil)
+				mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
+					gomock.Any(), gomock.Any()).Return(nil)
 				return mockDeviceClient
 			},
 			getMountClient: func() mount.Mount {
@@ -2285,7 +2289,8 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
 			},
 			getDeviceClient: func() devices.Devices {
 				mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
-				mockDeviceClient.EXPECT().RemoveMultipathDeviceMapping(gomock.Any(), gomock.Any()).Return(nil)
+				mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
+					gomock.Any(), gomock.Any()).Return(nil)
 				return mockDeviceClient
 			},
 			getMountClient: func() mount.Mount {
@@ -2322,7 +2327,8 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
 			},
 			getDeviceClient: func() devices.Devices {
 				mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
-				mockDeviceClient.EXPECT().RemoveMultipathDeviceMapping(gomock.Any(), gomock.Any()).Return(nil)
+				mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
+					gomock.Any(), gomock.Any()).Return(nil)
 				return mockDeviceClient
 			},
 			getMountClient: func() mount.Mount {
@@ -2395,8 +2401,8 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
 			},
 			getDeviceClient: func() devices.Devices {
 				mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
-				mockDeviceClient.EXPECT().RemoveMultipathDeviceMapping(gomock.Any(), gomock.Any()).
-					Return(fmt.Errorf("mock error"))
+				mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
+					gomock.Any(), gomock.Any()).Return(fmt.Errorf("mock error"))
 				return mockDeviceClient
 			},
 			getMountClient: func() mount.Mount {
@@ -2430,7 +2436,8 @@ func TestNodeUnstageISCSIVolume(t *testing.T) {
 			},
 			getDeviceClient: func() devices.Devices {
 				mockDeviceClient := mock_devices.NewMockDevices(gomock.NewController(t))
-				mockDeviceClient.EXPECT().RemoveMultipathDeviceMapping(gomock.Any(), gomock.Any()).Return(nil)
+				mockDeviceClient.EXPECT().RemoveMultipathDeviceMappingWithRetries(gomock.Any(), gomock.Any(),
+					gomock.Any(), gomock.Any()).Return(nil)
 				return mockDeviceClient
 			},
 			getMountClient: func() mount.Mount {

diff --git a/mocks/mock_operator/mock_clients/mock_api.go b/mocks/mock_operator/mock_clients/mock_api.go