Skip to content

Commit aa30ae5

Browse files
clintonkclintonk
authored
Improved economy driver performance
1 parent 2d46487 commit aa30ae5

29 files changed

+336
-120
lines changed

‎core/concurrent_core.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1753,7 +1753,7 @@ func (o *ConcurrentTridentOrchestrator) updateBackendVolumes(ctx context.Context
17531753

17541754
vol.BackendUUID = backend.BackendUUID()
17551755
updatePersistentStore := false
1756-
volumeExists := backend.Driver().Get(ctx, vol.Config.InternalName) == nil
1756+
volumeExists := backend.Driver().Get(ctx, vol.Config) == nil
17571757
if !volumeExists {
17581758
if !vol.Orphaned {
17591759
vol.Orphaned = true

‎core/concurrent_core_test.go‎

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12838,6 +12838,15 @@ func TestValidateBackendUpdateConcurrentCore(t *testing.T) {
1283812838
}
1283912839

1284012840
func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
12841+
volConfig1 := &storage.VolumeConfig{
12842+
Name: "testVolume1",
12843+
InternalName: "testVolume1",
12844+
}
12845+
volConfig2 := &storage.VolumeConfig{
12846+
Name: "testVolume2",
12847+
InternalName: "testVolume2",
12848+
}
12849+
1284112850
tests := []struct {
1284212851
name string
1284312852
setupMocks func(mockCtrl *gomock.Controller, mockStoreClient *mockpersistentstore.MockStoreClient, o *ConcurrentTridentOrchestrator) storage.Backend
@@ -12852,10 +12861,7 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1285212861

1285312862
// Setup volume that exists in cache but not on backend (should become orphaned)
1285412863
vol := &storage.Volume{
12855-
Config: &storage.VolumeConfig{
12856-
Name: "testVolume1",
12857-
InternalName: "testVolume1",
12858-
},
12864+
Config: volConfig1,
1285912865
BackendUUID: "backend-uuid1",
1286012866
Orphaned: false,
1286112867
}
@@ -12864,7 +12870,7 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1286412870
addBackendsToCache(t, mockBackend)
1286512871

1286612872
mockBackend.EXPECT().Driver().Return(mockDriver).Times(1)
12867-
mockDriver.EXPECT().Get(gomock.Any(), "testVolume1").Return(errors.New("volume not found")).Times(1)
12873+
mockDriver.EXPECT().Get(gomock.Any(), volConfig1).Return(errors.New("volume not found")).Times(1)
1286812874

1286912875
mockStoreClient.EXPECT().UpdateVolume(gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, volume *storage.Volume) error {
1287012876
assert.True(t, volume.Orphaned)
@@ -12893,10 +12899,7 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1289312899

1289412900
// Setup orphaned volume that now exists on backend (should be recovered)
1289512901
vol := &storage.Volume{
12896-
Config: &storage.VolumeConfig{
12897-
Name: "testVolume1",
12898-
InternalName: "testVolume1",
12899-
},
12902+
Config: volConfig1,
1290012903
BackendUUID: "backend-uuid1",
1290112904
Orphaned: true,
1290212905
}
@@ -12905,7 +12908,7 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1290512908
addBackendsToCache(t, mockBackend)
1290612909

1290712910
mockBackend.EXPECT().Driver().Return(mockDriver).Times(1)
12908-
mockDriver.EXPECT().Get(gomock.Any(), "testVolume1").Return(nil).Times(1)
12911+
mockDriver.EXPECT().Get(gomock.Any(), volConfig1).Return(nil).Times(1)
1290912912

1291012913
mockStoreClient.EXPECT().UpdateVolume(gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, volume *storage.Volume) error {
1291112914
assert.False(t, volume.Orphaned)
@@ -12934,10 +12937,7 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1293412937

1293512938
// Setup volume that exists and is not orphaned (no change needed)
1293612939
vol := &storage.Volume{
12937-
Config: &storage.VolumeConfig{
12938-
Name: "testVolume1",
12939-
InternalName: "testVolume1",
12940-
},
12940+
Config: volConfig1,
1294112941
BackendUUID: "backend-uuid1",
1294212942
Orphaned: false,
1294312943
}
@@ -12946,7 +12946,7 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1294612946
addBackendsToCache(t, mockBackend)
1294712947

1294812948
mockBackend.EXPECT().Driver().Return(mockDriver).Times(1)
12949-
mockDriver.EXPECT().Get(gomock.Any(), "testVolume1").Return(nil).Times(1)
12949+
mockDriver.EXPECT().Get(gomock.Any(), volConfig1).Return(nil).Times(1)
1295012950

1295112951
// No UpdateVolume call expected since state doesn't change
1295212952
mockBackend.EXPECT().Volumes().Return(&sync.Map{}).Times(1)
@@ -12971,18 +12971,12 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1297112971

1297212972
// Setup multiple volumes with different states
1297312973
vol1 := &storage.Volume{
12974-
Config: &storage.VolumeConfig{
12975-
Name: "testVolume1",
12976-
InternalName: "testVolume1",
12977-
},
12974+
Config: volConfig1,
1297812975
BackendUUID: "backend-uuid1",
1297912976
Orphaned: false,
1298012977
}
1298112978
vol2 := &storage.Volume{
12982-
Config: &storage.VolumeConfig{
12983-
Name: "testVolume2",
12984-
InternalName: "testVolume2",
12985-
},
12979+
Config: volConfig2,
1298612980
BackendUUID: "backend-uuid1",
1298712981
Orphaned: true,
1298812982
}
@@ -12992,9 +12986,9 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1299212986

1299312987
mockBackend.EXPECT().Driver().Return(mockDriver).Times(2)
1299412988
// Vol1 not found on backend (becomes orphaned)
12995-
mockDriver.EXPECT().Get(gomock.Any(), "testVolume1").Return(errors.New("not found")).Times(1)
12989+
mockDriver.EXPECT().Get(gomock.Any(), volConfig1).Return(errors.New("not found")).Times(1)
1299612990
// Vol2 found on backend (recovered)
12997-
mockDriver.EXPECT().Get(gomock.Any(), "testVolume2").Return(nil).Times(1)
12991+
mockDriver.EXPECT().Get(gomock.Any(), volConfig2).Return(nil).Times(1)
1299812992

1299912993
// Expect two update calls
1300012994
gomock.InOrder(
@@ -13035,10 +13029,7 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1303513029
mockBackend := getMockBackend(mockCtrl, "testBackend1", "backend-uuid1")
1303613030

1303713031
vol := &storage.Volume{
13038-
Config: &storage.VolumeConfig{
13039-
Name: "testVolume1",
13040-
InternalName: "testVolume1",
13041-
},
13032+
Config: volConfig1,
1304213033
BackendUUID: "backend-uuid1",
1304313034
Orphaned: false,
1304413035
}
@@ -13061,10 +13052,7 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1306113052
mockBackend := getMockBackend(mockCtrl, "testBackend1", "backend-uuid1")
1306213053

1306313054
vol := &storage.Volume{
13064-
Config: &storage.VolumeConfig{
13065-
Name: "testVolume1",
13066-
InternalName: "testVolume1",
13067-
},
13055+
Config: volConfig1,
1306813056
BackendUUID: "backend-uuid1",
1306913057
Orphaned: false,
1307013058
}
@@ -13091,10 +13079,7 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1309113079
mockDriver := mockstorage.NewMockDriver(mockCtrl)
1309213080

1309313081
vol := &storage.Volume{
13094-
Config: &storage.VolumeConfig{
13095-
Name: "testVolume1",
13096-
InternalName: "testVolume1",
13097-
},
13082+
Config: volConfig1,
1309813083
BackendUUID: "backend-uuid1",
1309913084
Orphaned: false,
1310013085
}
@@ -13103,7 +13088,7 @@ func TestUpdateBackendVolumesConcurrentCore(t *testing.T) {
1310313088
addBackendsToCache(t, mockBackend)
1310413089

1310513090
mockBackend.EXPECT().Driver().Return(mockDriver).Times(1)
13106-
mockDriver.EXPECT().Get(gomock.Any(), "testVolume1").Return(errors.New("not found")).Times(1)
13091+
mockDriver.EXPECT().Get(gomock.Any(), volConfig1).Return(errors.New("not found")).Times(1)
1310713092

1310813093
// Simulate store update failure
1310913094
mockStoreClient.EXPECT().UpdateVolume(gomock.Any(), gomock.Any()).Return(errors.New("store error")).Times(1)

‎core/orchestrator_core.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1448,7 +1448,7 @@ func (o *TridentOrchestrator) updateBackendByBackendUUID(
14481448
if vol.BackendUUID == originalBackend.BackendUUID() {
14491449
vol.BackendUUID = backend.BackendUUID()
14501450
updatePersistentStore := false
1451-
volumeExists := backend.Driver().Get(ctx, vol.Config.InternalName) == nil
1451+
volumeExists := backend.Driver().Get(ctx, vol.Config) == nil
14521452
if !volumeExists {
14531453
if !vol.Orphaned {
14541454
vol.Orphaned = true

‎mocks/mock_storage/mock_storage_driver.go‎

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

‎storage/backend.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ type Driver interface {
4747
Destroy(ctx context.Context, volConfig *VolumeConfig) error
4848
Rename(ctx context.Context, name, newName string) error
4949
Resize(ctx context.Context, volConfig *VolumeConfig, sizeBytes uint64) error
50-
Get(ctx context.Context, name string) error
50+
Get(ctx context.Context, volConfig *VolumeConfig) error
5151
GetInternalVolumeName(ctx context.Context, volConfig *VolumeConfig, storagePool Pool) string
5252
GetStorageBackendSpecs(ctx context.Context, backend Backend) error
5353
GetStorageBackendPhysicalPoolNames(ctx context.Context) []string
@@ -615,7 +615,7 @@ func (b *StorageBackend) CloneVolume(
615615
if !cloneVolConfig.ReadOnlyClone {
616616
// The clone may not be fully created when the clone API returns, so wait here until it exists.
617617
checkCloneExists := func() error {
618-
return b.driver.Get(ctx, cloneVolConfig.InternalName)
618+
return b.driver.Get(ctx, cloneVolConfig)
619619
}
620620
cloneExistsNotify := func(err error, duration time.Duration) {
621621
Logc(ctx).WithField("increment", duration).Debug("Clone not yet present, waiting.")
@@ -823,7 +823,7 @@ func (b *StorageBackend) RenameVolume(ctx context.Context, volConfig *VolumeConf
823823
return err
824824
}
825825

826-
if err := b.driver.Get(ctx, oldName); err != nil {
826+
if err := b.driver.Get(ctx, volConfig); err != nil {
827827
return fmt.Errorf("volume %s not found on backend %s; %v", oldName, b.name, err)
828828
}
829829
if err := b.driver.Rename(ctx, oldName, newName); err != nil {

‎storage_drivers/azure/azure_anf.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2174,7 +2174,9 @@ func (d *NASStorageDriver) List(ctx context.Context) ([]string, error) {
21742174
}
21752175

21762176
// Get tests for the existence of a volume.
2177-
func (d *NASStorageDriver) Get(ctx context.Context, name string) error {
2177+
func (d *NASStorageDriver) Get(ctx context.Context, volConfig *storage.VolumeConfig) error {
2178+
name := volConfig.InternalName
2179+
21782180
fields := LogFields{"Method": "Get", "Type": "NASStorageDriver"}
21792181
Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace(">>>> Get")
21802182
defer Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< Get")

‎storage_drivers/azure/azure_anf_test.go‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7771,7 +7771,12 @@ func TestGet(t *testing.T) {
77717771
mockAPI.EXPECT().RefreshAzureResources(ctx).Return(nil).Times(1)
77727772
mockAPI.EXPECT().VolumeByCreationToken(ctx, "volume1").Return(filesystem, nil).Times(1)
77737773

7774-
result := driver.Get(ctx, "volume1")
7774+
volConfig1 := &storage.VolumeConfig{
7775+
Name: "volume1",
7776+
InternalName: "volume1",
7777+
}
7778+
7779+
result := driver.Get(ctx, volConfig1)
77757780

77767781
assert.NoError(t, result, "expected no error")
77777782
}
@@ -7782,7 +7787,12 @@ func TestGet_DiscoveryFailed(t *testing.T) {
77827787
mockAPI.EXPECT().RefreshAzureResources(ctx).Return(errFailed).Times(1)
77837788
mockAPI.EXPECT().VolumeByCreationToken(ctx, "volume1").Times(0)
77847789

7785-
result := driver.Get(ctx, "volume1")
7790+
volConfig1 := &storage.VolumeConfig{
7791+
Name: "volume1",
7792+
InternalName: "volume1",
7793+
}
7794+
7795+
result := driver.Get(ctx, volConfig1)
77867796

77877797
assert.Error(t, result, "expected error")
77887798
}
@@ -7793,7 +7803,12 @@ func TestGet_NotFound(t *testing.T) {
77937803
mockAPI.EXPECT().RefreshAzureResources(ctx).Return(nil).Times(1)
77947804
mockAPI.EXPECT().VolumeByCreationToken(ctx, "volume1").Return(nil, errFailed).Times(1)
77957805

7796-
result := driver.Get(ctx, "volume1")
7806+
volConfig1 := &storage.VolumeConfig{
7807+
Name: "volume1",
7808+
InternalName: "volume1",
7809+
}
7810+
7811+
result := driver.Get(ctx, volConfig1)
77977812

77987813
assert.Error(t, result, "expected error")
77997814
}

‎storage_drivers/fake/fake.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1199,7 +1199,8 @@ func (d *StorageDriver) ConstructGroupSnapshot(
11991199
return groupSnapshot.ConstructClone(), nil
12001200
}
12011201

1202-
func (d *StorageDriver) Get(_ context.Context, name string) error {
1202+
func (d *StorageDriver) Get(_ context.Context, volConfig *storage.VolumeConfig) error {
1203+
name := volConfig.InternalName
12031204
_, ok := d.Volumes[name]
12041205
if !ok {
12051206
return fmt.Errorf("could not find volume %s", name)

‎storage_drivers/gcp/gcp_gcnv.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2021,7 +2021,8 @@ func (d *NASStorageDriver) List(ctx context.Context) ([]string, error) {
20212021
}
20222022

20232023
// Get tests for the existence of a volume.
2024-
func (d *NASStorageDriver) Get(ctx context.Context, name string) error {
2024+
func (d *NASStorageDriver) Get(ctx context.Context, volConfig *storage.VolumeConfig) error {
2025+
name := volConfig.InternalName
20252026
fields := LogFields{"Method": "Get", "Type": "NASStorageDriver"}
20262027
Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace(">>>> Get")
20272028
defer Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< Get")

‎storage_drivers/gcp/gcp_gcnv_san.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1477,7 +1477,8 @@ func (d *SANStorageDriver) Import(ctx context.Context, volConfig *storage.Volume
14771477
}
14781478

14791479
// Get tests for the existence of a volume.
1480-
func (d *SANStorageDriver) Get(ctx context.Context, name string) error {
1480+
func (d *SANStorageDriver) Get(ctx context.Context, volConfig *storage.VolumeConfig) error {
1481+
name := volConfig.InternalName
14811482
fields := LogFields{"Method": "Get", "Type": "SANStorageDriver", "name": name}
14821483
Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace(">>>> Get")
14831484
defer Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< Get")

0 commit comments

Comments
 (0)