Skip to content

Commit e521883

Browse files
jwebster7jwebster7
authored
Require healthy paths during iscsi volume expansion
Trident now checks the state of all paths to a LUN before rescanning or resizing the multipath map. If any path is unhealthy, expansion is deferred until all paths are remediated. Trident now also requires stable reads of path health, SCSI disk size, and multipath device size before returning success.
1 parent 4918dd9 commit e521883

17 files changed

Lines changed: 2297 additions & 880 deletions

‎frontend/csi/node_server.go‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -736,24 +736,16 @@ func (p *Plugin) nodePrepareISCSIVolumeForExpansion(
736736
"filesystemType": publishInfo.FilesystemType,
737737
}).Debug("PublishInfo for block device to expand.")
738738

739-
var err error
740-
741-
// Make sure device is ready.
742-
if p.iscsi.IsAlreadyAttached(ctx, lunID, publishInfo.IscsiTargetIQN) {
743-
// Rescan device to detect increased size.
744-
if err = p.iscsi.RescanDevices(
745-
ctx, publishInfo.IscsiTargetIQN, publishInfo.IscsiLunNumber, requiredBytes); err != nil {
746-
Logc(ctx).WithField("device", publishInfo.DevicePath).WithError(err).
747-
Error("Unable to scan device.")
748-
err = status.Error(codes.Internal, err.Error())
749-
}
750-
} else {
751-
err = fmt.Errorf("device %s to expand is not attached", publishInfo.DevicePath)
752-
Logc(ctx).WithField("devicePath", publishInfo.DevicePath).WithError(err).Error(
753-
"Unable to expand volume.")
739+
// Resize the volume.
740+
if err := p.iscsi.ExpandVolume(ctx, publishInfo, requiredBytes); err != nil {
741+
Logc(ctx).WithFields(LogFields{
742+
"lunID": publishInfo.IscsiLunNumber,
743+
"devicePath": publishInfo.DevicePath,
744+
}).WithError(err).Error("Unable to resize device(s) for LUN.")
754745
return status.Error(codes.Internal, err.Error())
755746
}
756-
return err
747+
748+
return nil
757749
}
758750

759751
func (p *Plugin) NodeGetCapabilities(

‎frontend/csi/node_server_test.go‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 NetApp, Inc. All Rights Reserved.
1+
// Copyright 2026 NetApp, Inc. All Rights Reserved.
22

33
package csi
44

@@ -65,8 +65,7 @@ func TestNodeStageVolume(t *testing.T) {
6565
mockISCSIClient.EXPECT().AttachVolumeRetry(
6666
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
6767
).Return(int64(1), nil)
68-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
69-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
68+
mockISCSIClient.EXPECT().ExpandVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
7069
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
7170
return mockISCSIClient
7271
},
@@ -240,8 +239,7 @@ func TestNodeStageISCSIVolume(t *testing.T) {
240239
mockISCSIClient.EXPECT().AttachVolumeRetry(
241240
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
242241
).Return(int64(1), nil)
243-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
244-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
242+
mockISCSIClient.EXPECT().ExpandVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
245243
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
246244
return mockISCSIClient
247245
},
@@ -508,7 +506,9 @@ func TestNodeStageISCSIVolume(t *testing.T) {
508506
mockISCSIClient.EXPECT().AttachVolumeRetry(
509507
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
510508
).Return(int64(1), nil)
511-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(false)
509+
mockISCSIClient.EXPECT().ExpandVolume(
510+
gomock.Any(), gomock.Any(), gomock.Any(),
511+
).Return(errors.New("volume not attached"))
512512
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
513513
return mockISCSIClient
514514
},
@@ -526,9 +526,9 @@ func TestNodeStageISCSIVolume(t *testing.T) {
526526
mockISCSIClient.EXPECT().AttachVolumeRetry(
527527
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
528528
).Return(int64(1), nil)
529-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
530-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(),
531-
gomock.Any()).Return(errors.New("some error"))
529+
mockISCSIClient.EXPECT().ExpandVolume(
530+
gomock.Any(), gomock.Any(), gomock.Any(),
531+
).Return(errors.New("volume resize failed"))
532532
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
533533
return mockISCSIClient
534534
},
@@ -546,8 +546,7 @@ func TestNodeStageISCSIVolume(t *testing.T) {
546546
mockISCSIClient.EXPECT().AttachVolumeRetry(
547547
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
548548
).Return(int64(1), nil)
549-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
550-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
549+
mockISCSIClient.EXPECT().ExpandVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
551550
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
552551
return mockISCSIClient
553552
},
@@ -2699,8 +2698,7 @@ func TestNodeStageVolume_Multithreaded(t *testing.T) {
26992698
mockISCSIClient.EXPECT().AttachVolumeRetry(
27002699
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
27012700
).Return(int64(1), nil)
2702-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
2703-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
2701+
mockISCSIClient.EXPECT().ExpandVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
27042702
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
27052703
mockTrackingClient.EXPECT().WriteTrackingInfo(gomock.Any(), gomock.Any(), gomock.Any()).Times(2).Return(nil)
27062704
}
@@ -2845,8 +2843,7 @@ func TestNodeStageVolume_Multithreaded(t *testing.T) {
28452843
mockISCSIClient.EXPECT().AttachVolumeRetry(
28462844
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
28472845
).Return(int64(1), nil)
2848-
mockISCSIClient.EXPECT().IsAlreadyAttached(gomock.Any(), gomock.Any(), gomock.Any()).Return(true)
2849-
mockISCSIClient.EXPECT().RescanDevices(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
2846+
mockISCSIClient.EXPECT().ExpandVolume(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
28502847
mockISCSIClient.EXPECT().AddSession(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
28512848
mockTrackingClient.EXPECT().WriteTrackingInfo(gomock.Any(), gomock.Any(), gomock.Any()).Times(2).Return(nil)
28522849
}

‎mocks/mock_utils/mock_devices/mock_devices_client.go‎

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

‎mocks/mock_utils/mock_iscsi/mock_iscsi_client.go‎

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

‎pkg/collection/list.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 NetApp, Inc. All Rights Reserved.
1+
// Copyright 2026 NetApp, Inc. All Rights Reserved.
22

33
package collection
44

@@ -98,7 +98,7 @@ func RemoveStringConditionally(slice []string, s string, fn func(string, string)
9898
}
9999
result = append(result, item)
100100
}
101-
return
101+
return result
102102
}
103103

104104
// ReplaceAtIndex returns a string with the rune at the specified index replaced.
@@ -142,3 +142,24 @@ func StringInSlice(s string, list []string) bool {
142142
}
143143
return false
144144
}
145+
146+
// EqualValues accepts 2 slices of any standard comparable type and returns whether their values are equivalent.
147+
func EqualValues[C comparable](s1, s2 []C) bool {
148+
if len(s1) != len(s2) {
149+
return false
150+
}
151+
152+
elemOccurrences := make(map[any]int, len(s1))
153+
for _, elem := range s1 {
154+
elemOccurrences[elem] += 1
155+
}
156+
157+
for _, v := range s2 {
158+
elemOccurrences[v]--
159+
if elemOccurrences[v] < 0 {
160+
return false
161+
}
162+
}
163+
164+
return true
165+
}

‎pkg/collection/list_test.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,102 @@ func TestReplaceAtIndex(t *testing.T) {
392392
assert.Equal(t, "boo", actual)
393393
}
394394

395+
func TestEqualValues(t *testing.T) {
396+
tests := map[string]struct {
397+
s1 []string
398+
s2 []string
399+
equal bool
400+
}{
401+
"identical slices": {
402+
s1: []string{"a", "b", "c"},
403+
s2: []string{"a", "b", "c"},
404+
equal: true,
405+
},
406+
"same elements different order": {
407+
s1: []string{"c", "a", "b"},
408+
s2: []string{"a", "b", "c"},
409+
equal: true,
410+
},
411+
"both nil": {
412+
s1: nil,
413+
s2: nil,
414+
equal: true,
415+
},
416+
"both empty": {
417+
s1: []string{},
418+
s2: []string{},
419+
equal: true,
420+
},
421+
"nil and empty are equal": {
422+
s1: nil,
423+
s2: []string{},
424+
equal: true,
425+
},
426+
"empty and nil are equal": {
427+
s1: []string{},
428+
s2: nil,
429+
equal: true,
430+
},
431+
"different lengths": {
432+
s1: []string{"a", "b"},
433+
s2: []string{"a", "b", "c"},
434+
equal: false,
435+
},
436+
"same length different values": {
437+
s1: []string{"a", "b", "c"},
438+
s2: []string{"a", "b", "d"},
439+
equal: false,
440+
},
441+
"one nil one populated": {
442+
s1: nil,
443+
s2: []string{"a"},
444+
equal: false,
445+
},
446+
"one empty one populated": {
447+
s1: []string{},
448+
s2: []string{"a"},
449+
equal: false,
450+
},
451+
"duplicate elements equal cardinality": {
452+
s1: []string{"a", "a", "b"},
453+
s2: []string{"a", "b", "a"},
454+
equal: true,
455+
},
456+
"duplicate elements unequal cardinality": {
457+
s1: []string{"a", "a", "b"},
458+
s2: []string{"a", "b", "b"},
459+
equal: false,
460+
},
461+
"single element equal": {
462+
s1: []string{"x"},
463+
s2: []string{"x"},
464+
equal: true,
465+
},
466+
"single element not equal": {
467+
s1: []string{"x"},
468+
s2: []string{"y"},
469+
equal: false,
470+
},
471+
}
472+
473+
for name, tc := range tests {
474+
t.Run(name, func(t *testing.T) {
475+
assert.Equal(t, tc.equal, EqualValues(tc.s1, tc.s2))
476+
})
477+
}
478+
479+
// Also test with int type to verify generics work.
480+
t.Run("int slices same elements different order", func(t *testing.T) {
481+
assert.True(t, EqualValues([]int{3, 1, 2}, []int{1, 2, 3}))
482+
})
483+
t.Run("int slices different values", func(t *testing.T) {
484+
assert.False(t, EqualValues([]int{1, 2, 3}, []int{1, 2, 4}))
485+
})
486+
t.Run("int nil and empty are equal", func(t *testing.T) {
487+
assert.True(t, EqualValues[int](nil, []int{}))
488+
})
489+
}
490+
395491
func TestAppendToStringList(t *testing.T) {
396492
tests := []struct {
397493
stringList string

‎utils/devices/devices.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 NetApp, Inc. All Rights Reserved.
1+
// Copyright 2026 NetApp, Inc. All Rights Reserved.
22

33
//go:generate mockgen -destination=../../mocks/mock_utils/mock_devices/mock_devices_client.go github.com/netapp/trident/utils/devices Devices
44
//go:generate mockgen -destination=../../mocks/mock_utils/mock_devices/mock_size_getter_client.go github.com/netapp/trident/utils/devices SizeGetter
@@ -79,6 +79,10 @@ type Devices interface {
7979
RemoveMultipathDeviceMappingWithRetries(ctx context.Context, devicePath string, retries uint64,
8080
sleep time.Duration) error
8181
ClearFormatting(ctx context.Context, devicePath string) error
82+
GetMultipathDeviceBySerial(ctx context.Context, hexSerial string) (string, error)
83+
// ExpandMultipathDevice expands a multipath device by resizing all dm-slaves then
84+
// resizing the multipath device-mapper and waiting for all devices sizes to converge.
85+
ExpandMultipathDevice(ctx context.Context, getter models.SCSIDeviceInfoGetter, targetSizeBytes int64) error
8286
}
8387

8488
type SizeGetter interface {

‎utils/devices/devices_darwin.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2024 NetApp, Inc. All Rights Reserved.
1+
// Copyright 2026 NetApp, Inc. All Rights Reserved.
22

33
// NOTE: This file should only contain functions for handling devices for Darwin flavor
44

@@ -9,6 +9,7 @@ import (
99

1010
. "github.com/netapp/trident/logging"
1111
"github.com/netapp/trident/utils/errors"
12+
"github.com/netapp/trident/utils/models"
1213
)
1314

1415
// flushOneDevice unused stub function
@@ -68,3 +69,11 @@ func (c *Client) CloseLUKSDevice(ctx context.Context, devicePath string) error {
6869
defer Logc(ctx).Debug("<<<< devices_darwin.CloseLUKSDevice")
6970
return errors.UnsupportedError("CloseLUKSDevice is not supported for darwin")
7071
}
72+
73+
func (c *Client) ExpandMultipathDevice(
74+
ctx context.Context, _ models.SCSIDeviceInfoGetter, _ int64,
75+
) error {
76+
Logc(ctx).Debug(">>>> devices_darwin.ExpandMultipathDevice")
77+
defer Logc(ctx).Debug("<<<< devices_darwin.ExpandMultipathDevice")
78+
return errors.UnsupportedError("ExpandMultipathDevice is not supported for darwin")
79+
}

0 commit comments

Comments
 (0)