Skip to content

Commit b1c01bd

Browse files
jwebster7jwebster7
authored
Ensure all expected iSCSI devices are present before initiating rescans in CSI NodeExpandVolume
This change updates CSI NodeExpandVolume to restore any missing iSCSI paths and verify all devices are available before proceeding with volume resize. This prevents multipath devices from shrinking due to incomplete device sets after Trident rescans, ensuring reliable and consistent volume expansion.
1 parent 8dd9616 commit b1c01bd

File tree

5 files changed

+558
-54
lines changed

5 files changed

+558
-54
lines changed

‎frontend/csi/node_server.go‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ const (
4949
tridentDeviceInfoPath = "/var/lib/trident/tracking"
5050
nodeLockID = "csi_node_server"
5151
AttachISCSIVolumeTimeoutShort = 20 * time.Second
52+
ResizeISCSIVolumeTimeout = 20 * time.Second
5253
AttachFCPVolumeTimeoutShort = 20 * time.Second
5354
iSCSINodeUnstageMaxDuration = 15 * time.Second
5455
iSCSILoginTimeout = 10 * time.Second
@@ -733,7 +734,6 @@ func (p *Plugin) nodePrepareISCSIVolumeForExpansion(
733734
ctx context.Context, publishInfo *models.VolumePublishInfo, requiredBytes int64,
734735
) error {
735736
lunID := int(publishInfo.IscsiLunNumber)
736-
737737
Logc(ctx).WithFields(LogFields{
738738
"targetIQN": publishInfo.IscsiTargetIQN,
739739
"lunID": lunID,
@@ -742,24 +742,16 @@ func (p *Plugin) nodePrepareISCSIVolumeForExpansion(
742742
"filesystemType": publishInfo.FilesystemType,
743743
}).Debug("PublishInfo for block device to expand.")
744744

745-
var err error
746-
747-
// Make sure device is ready.
748-
if p.iscsi.IsAlreadyAttached(ctx, lunID, publishInfo.IscsiTargetIQN) {
749-
// Rescan device to detect increased size.
750-
if err = p.iscsi.RescanDevices(
751-
ctx, publishInfo.IscsiTargetIQN, publishInfo.IscsiLunNumber, requiredBytes); err != nil {
752-
Logc(ctx).WithField("device", publishInfo.DevicePath).WithError(err).
753-
Error("Unable to scan device.")
754-
err = status.Error(codes.Internal, err.Error())
755-
}
756-
} else {
757-
err = fmt.Errorf("device %s to expand is not attached", publishInfo.DevicePath)
758-
Logc(ctx).WithField("devicePath", publishInfo.DevicePath).WithError(err).Error(
759-
"Unable to expand volume.")
745+
// Resize the volume.
746+
if err := p.iscsi.ResizeVolumeRetry(ctx, publishInfo, requiredBytes, ResizeISCSIVolumeTimeout); err != nil {
747+
Logc(ctx).WithFields(LogFields{
748+
"lunID": publishInfo.IscsiLunNumber,
749+
"devicePath": publishInfo.DevicePath,
750+
}).WithError(err).Error("Unable to resize device(s) for LUN.")
760751
return status.Error(codes.Internal, err.Error())
761752
}
762-
return err
753+
754+
return nil
763755
}
764756

765757
func (p *Plugin) NodeGetCapabilities(

‎frontend/csi/node_server_test.go‎

Lines changed: 29 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,9 @@ func TestNodeStageVolume(t *testing.T) {
7575
mockISCSIClient.EXPECT().AttachVolumeRetry(
7676
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
7777
).Return(int64(1), nil)
78-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
79-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
78+
mockISCSIClient.EXPECT().ResizeVolumeRetry(
79+
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
80+
).Return(nil)
8081
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
8182
return mockISCSIClient
8283
},
@@ -252,8 +253,9 @@ func TestNodeStageISCSIVolume(t *testing.T) {
252253
mockISCSIClient.EXPECT().AttachVolumeRetry(
253254
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
254255
).Return(int64(1), nil)
255-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
256-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
256+
mockISCSIClient.EXPECT().ResizeVolumeRetry(
257+
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
258+
).Return(nil)
257259
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
258260
return mockISCSIClient
259261
},
@@ -520,7 +522,9 @@ func TestNodeStageISCSIVolume(t *testing.T) {
520522
mockISCSIClient.EXPECT().AttachVolumeRetry(
521523
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
522524
).Return(int64(1), nil)
523-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(false)
525+
mockISCSIClient.EXPECT().ResizeVolumeRetry(
526+
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
527+
).Return(errors.New("volume not attached"))
524528
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
525529
return mockISCSIClient
526530
},
@@ -538,9 +542,9 @@ func TestNodeStageISCSIVolume(t *testing.T) {
538542
mockISCSIClient.EXPECT().AttachVolumeRetry(
539543
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
540544
).Return(int64(1), nil)
541-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
542-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(),
543-
gomock.Any()).Return(errors.New("some error"))
545+
mockISCSIClient.EXPECT().ResizeVolumeRetry(
546+
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
547+
).Return(errors.New("volume resize failed"))
544548
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
545549
return mockISCSIClient
546550
},
@@ -558,8 +562,9 @@ func TestNodeStageISCSIVolume(t *testing.T) {
558562
mockISCSIClient.EXPECT().AttachVolumeRetry(
559563
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
560564
).Return(int64(1), nil)
561-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
562-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
565+
mockISCSIClient.EXPECT().ResizeVolumeRetry(
566+
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
567+
).Return(nil)
563568
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
564569
return mockISCSIClient
565570
},
@@ -2733,8 +2738,7 @@ func TestNodeStageVolume_Multithreaded(t *testing.T) {
27332738
mockISCSIClient.EXPECT().AttachVolumeRetry(
27342739
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
27352740
).Return(int64(1), nil)
2736-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
2737-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
2741+
mockISCSIClient.EXPECT().ResizeVolumeRetry(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
27382742
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
27392743
mockTrackingClient.EXPECT().WriteTrackingInfo(gomock.Any(), gomock.Any(), gomock.Any()).Times(2).Return(nil)
27402744
}
@@ -2881,8 +2885,7 @@ func TestNodeStageVolume_Multithreaded(t *testing.T) {
28812885
mockISCSIClient.EXPECT().AttachVolumeRetry(
28822886
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
28832887
).Return(int64(1), nil)
2884-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
2885-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
2888+
mockISCSIClient.EXPECT().ResizeVolumeRetry(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
28862889
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
28872890
mockTrackingClient.EXPECT().WriteTrackingInfo(gomock.Any(), gomock.Any(), gomock.Any()).Times(2).Return(nil)
28882891
}
@@ -11540,7 +11543,7 @@ func TestNodeExpandVolume(t *testing.T) {
1154011543
expErrCode: codes.Internal,
1154111544
},
1154211545
{
11543-
name: "Node expand volume failed, volume tracking file not found",
11546+
name: "Node expand volume failed, volume tracking file not found",
1154411547
req: &csi.NodeExpandVolumeRequest{
1154511548
VolumeId: "test-vol",
1154611549
VolumePath: "/vol/path",
@@ -11662,8 +11665,7 @@ func TestNodeExpandVolume(t *testing.T) {
1166211665
},
1166311666
setupISCSIMock: func() iscsi.ISCSI {
1166411667
mockISCSIClient := mock_iscsi.NewMockISCSI(gomock.NewController(t))
11665-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true).AnyTimes()
11666-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
11668+
mockISCSIClient.EXPECT().ResizeVolumeRetry(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
1166711669
return mockISCSIClient
1166811670
},
1166911671
mockFilesystem: func() filesystem.Filesystem {
@@ -11703,8 +11705,8 @@ func TestNodeExpandVolume(t *testing.T) {
1170311705
},
1170411706
setupISCSIMock: func() iscsi.ISCSI {
1170511707
mockISCSIClient := mock_iscsi.NewMockISCSI(gomock.NewController(t))
11706-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(false).AnyTimes()
11707-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
11708+
mockISCSIClient.EXPECT().ResizeVolumeRetry(gomock.Any(), gomock.Any(), gomock.Any(),
11709+
gomock.Any()).Return(errors.New("failure")).AnyTimes()
1170811710
return mockISCSIClient
1170911711
},
1171011712

@@ -11738,8 +11740,7 @@ func TestNodeExpandVolume(t *testing.T) {
1173811740
},
1173911741
setupISCSIMock: func() iscsi.ISCSI {
1174011742
mockISCSIClient := mock_iscsi.NewMockISCSI(gomock.NewController(t))
11741-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true).AnyTimes()
11742-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("")).AnyTimes()
11743+
mockISCSIClient.EXPECT().ResizeVolumeRetry(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
1174311744
return mockISCSIClient
1174411745
},
1174511746

@@ -11773,8 +11774,7 @@ func TestNodeExpandVolume(t *testing.T) {
1177311774
},
1177411775
setupISCSIMock: func() iscsi.ISCSI {
1177511776
mockISCSIClient := mock_iscsi.NewMockISCSI(gomock.NewController(t))
11776-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true).AnyTimes()
11777-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
11777+
mockISCSIClient.EXPECT().ResizeVolumeRetry(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
1177811778
return mockISCSIClient
1177911779
},
1178011780
mockFilesystem: func() filesystem.Filesystem {
@@ -11924,8 +11924,7 @@ func TestNodeExpandVolume(t *testing.T) {
1192411924
},
1192511925
setupISCSIMock: func() iscsi.ISCSI {
1192611926
mockISCSIClient := mock_iscsi.NewMockISCSI(gomock.NewController(t))
11927-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true).AnyTimes()
11928-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
11927+
mockISCSIClient.EXPECT().ResizeVolumeRetry(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
1192911928
return mockISCSIClient
1193011929
},
1193111930
mockFilesystem: func() filesystem.Filesystem {
@@ -11972,8 +11971,7 @@ func TestNodeExpandVolume(t *testing.T) {
1197211971
},
1197311972
setupISCSIMock: func() iscsi.ISCSI {
1197411973
mockISCSIClient := mock_iscsi.NewMockISCSI(gomock.NewController(t))
11975-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true).AnyTimes()
11976-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
11974+
mockISCSIClient.EXPECT().ResizeVolumeRetry(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
1197711975
return mockISCSIClient
1197811976
},
1197911977
mockFilesystem: func() filesystem.Filesystem {
@@ -12020,8 +12018,8 @@ func TestNodeExpandVolume(t *testing.T) {
1202012018
},
1202112019
setupISCSIMock: func() iscsi.ISCSI {
1202212020
mockISCSIClient := mock_iscsi.NewMockISCSI(gomock.NewController(t))
12023-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true).AnyTimes()
12024-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
12021+
mockISCSIClient.EXPECT().ResizeVolumeRetry(gomock.Any(), gomock.Any(), gomock.Any(),
12022+
gomock.Any()).Return(nil).AnyTimes()
1202512023
return mockISCSIClient
1202612024
},
1202712025
mockFilesystem: func() filesystem.Filesystem {
@@ -12068,8 +12066,8 @@ func TestNodeExpandVolume(t *testing.T) {
1206812066
},
1206912067
setupISCSIMock: func() iscsi.ISCSI {
1207012068
mockISCSIClient := mock_iscsi.NewMockISCSI(gomock.NewController(t))
12071-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true).AnyTimes()
12072-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
12069+
mockISCSIClient.EXPECT().ResizeVolumeRetry(gomock.Any(), gomock.Any(), gomock.Any(),
12070+
gomock.Any()).Return(errors.New("failure")).AnyTimes()
1207312071
return mockISCSIClient
1207412072
},
1207512073
mockFilesystem: func() filesystem.Filesystem {

‎mocks/mock_utils/mock_iscsi/mock_iscsi_client.go‎

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)