Skip to content

Commit 79445cf

Browse files
clintonkVinayKumarHavanur
clintonk
and
VinayKumarHavanur
authored
Allow clone across storage classes if backend is common
Co-authored-by: VinayKumarHavanur <54576364+VinayKumarHavanur@users.noreply.github.com>
1 parent 769bfbb commit 79445cf

5 files changed

Lines changed: 369 additions & 33 deletions

File tree

‎core/concurrent_core.go‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2349,10 +2349,16 @@ func (o *ConcurrentTridentOrchestrator) cloneVolume(
23492349
return nil, errors.NotFoundError("backend for source volume %s not found", volConfig.CloneSourceVolume)
23502350
}
23512351

2352-
// Check if the storage class of source and clone volume is different, only if the orchestrator is not in Docker plugin mode. In Docker plugin mode, the storage class of source and clone volume will be different at times.
2352+
// Check if the source volume's backend is honored by the target storage class, only if the orchestrator
2353+
// is not in Docker plugin mode. In Docker plugin mode, the storage class of source and clone volume
2354+
// will be different at times.
23532355
if !isDockerPluginMode() && volConfig.StorageClass != sourceVolume.Config.StorageClass {
2354-
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s with "+
2355-
"different storage classes is not allowed", volConfig.Name, volConfig.CloneSourceVolume)
2356+
poolMap := o.GetStorageClassPoolMap()
2357+
if !poolMap.BackendMatchesStorageClass(ctx, backend.Name(), volConfig.StorageClass) {
2358+
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s with "+
2359+
"different storage classes that have no common backends is not allowed",
2360+
volConfig.Name, volConfig.CloneSourceVolume)
2361+
}
23562362
}
23572363

23582364
if volConfig.Size != "" {
@@ -2539,13 +2545,6 @@ func (o *ConcurrentTridentOrchestrator) cloneVolumeRetry(
25392545

25402546
Logc(ctx).WithFields(logFields).Debug("Cloning volume.")
25412547

2542-
// Check if the storage class of source and clone volume is different, only if the orchestrator is not in Docker
2543-
// plugin mode. In Docker plugin mode, the storage class of source and clone volume will be different at times.
2544-
if !isDockerPluginMode() && cloneConfig.StorageClass != sourceVolConfig.StorageClass {
2545-
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s "+
2546-
"with different storage classes is not allowed", cloneConfig.Name, cloneConfig.CloneSourceVolume)
2547-
}
2548-
25492548
// Create the volume
25502549
createVolume := func() error {
25512550
if volume, err = backend.CloneVolume(ctx, sourceVolConfig, cloneConfig, pool, false); err != nil {

‎core/concurrent_core_test.go‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5785,6 +5785,171 @@ func TestCloneVolumeConcurrentCore(t *testing.T) {
57855785
assert.Nil(t, volume)
57865786
},
57875787
},
5788+
{
5789+
name: "CloneVolumeFromDifferentStorageClassSameBackendSuccess",
5790+
bootstrapErr: nil,
5791+
setupMocks: func(mockCtrl *gomock.Controller, mockStoreClient *mockpersistentstore.MockStoreClient, o *ConcurrentTridentOrchestrator) {
5792+
mockBackend := getMockBackend(mockCtrl, "testBackend", "backend-uuid")
5793+
5794+
fakePool := storage.NewStoragePool(nil, "pool1")
5795+
fakePool.AddStorageClass("sc-source")
5796+
fakePool.AddStorageClass("sc-dest")
5797+
fakePool.SetBackend(mockBackend)
5798+
5799+
mockBackend.EXPECT().StoragePools().Return(
5800+
func() *sync.Map {
5801+
m := sync.Map{}
5802+
m.Store("pool1", fakePool)
5803+
return &m
5804+
}(),
5805+
).AnyTimes()
5806+
mockBackend.EXPECT().CreatePrepare(gomock.Any(), gomock.Any(), gomock.Any())
5807+
mockBackend.EXPECT().CloneVolume(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), false).Return(
5808+
&storage.Volume{
5809+
Config: &storage.VolumeConfig{
5810+
InternalName: "cloneVolume",
5811+
Name: "cloneVolume",
5812+
Size: "1073741824",
5813+
VolumeMode: config.Filesystem,
5814+
CloneSourceVolume: "sourceVolume",
5815+
CloneSourceVolumeInternal: "sourceVolume",
5816+
StorageClass: "sc-dest",
5817+
},
5818+
BackendUUID: "backend-uuid",
5819+
}, nil).Times(1)
5820+
5821+
sourceVolumeDiffSC := &storage.Volume{
5822+
Config: &storage.VolumeConfig{
5823+
InternalName: "sourceVolume",
5824+
Name: "sourceVolume",
5825+
Size: "1073741824",
5826+
VolumeMode: config.Filesystem,
5827+
StorageClass: "sc-source",
5828+
},
5829+
BackendUUID: "backend-uuid",
5830+
}
5831+
5832+
scSource := storageclass.New(&storageclass.Config{
5833+
Name: "sc-source",
5834+
AdditionalPools: map[string][]string{"testBackend": {"pool1"}},
5835+
})
5836+
scDest := storageclass.New(&storageclass.Config{
5837+
Name: "sc-dest",
5838+
AdditionalPools: map[string][]string{"testBackend": {"pool1"}},
5839+
})
5840+
5841+
addBackendsToCache(t, mockBackend)
5842+
addVolumesToCache(t, sourceVolumeDiffSC)
5843+
addStorageClassesToCache(t, scSource, scDest)
5844+
5845+
o.RebuildStorageClassPoolMap(testCtx)
5846+
5847+
mockStoreClient.EXPECT().GetVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
5848+
mockStoreClient.EXPECT().AddVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil).Times(1)
5849+
mockStoreClient.EXPECT().UpdateVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil).Times(1)
5850+
mockStoreClient.EXPECT().DeleteVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil).Times(1)
5851+
mockStoreClient.EXPECT().AddVolume(gomock.Any(), gomock.Any()).Return(nil).Times(1)
5852+
},
5853+
volumeConfig: &storage.VolumeConfig{
5854+
InternalName: "cloneVolume",
5855+
Name: "cloneVolume",
5856+
Size: "1073741824",
5857+
VolumeMode: config.Filesystem,
5858+
CloneSourceVolume: "sourceVolume",
5859+
CloneSourceVolumeInternal: "sourceVolume",
5860+
StorageClass: "sc-dest",
5861+
},
5862+
verifyError: func(err error) {
5863+
assert.NoError(t, err)
5864+
},
5865+
verifyResult: func(result *storage.VolumeExternal) {
5866+
require.NotNil(t, result)
5867+
assert.Equal(t, "cloneVolume", result.Config.Name)
5868+
5869+
// Additionally verify the volume is added to the cache
5870+
volume := getVolumeByNameFromCache(t, "cloneVolume")
5871+
assert.NotNil(t, volume)
5872+
},
5873+
},
5874+
{
5875+
name: "CloneVolumeFromDifferentStorageClassDifferentBackendFailed",
5876+
bootstrapErr: nil,
5877+
setupMocks: func(mockCtrl *gomock.Controller, mockStoreClient *mockpersistentstore.MockStoreClient, o *ConcurrentTridentOrchestrator) {
5878+
mockBackend1 := getMockBackend(mockCtrl, "testBackend1", "backend-uuid-1")
5879+
mockBackend2 := getMockBackend(mockCtrl, "testBackend2", "backend-uuid-2")
5880+
5881+
fakePool1 := storage.NewStoragePool(nil, "pool1")
5882+
fakePool1.AddStorageClass("sc-source")
5883+
fakePool1.SetBackend(mockBackend1)
5884+
5885+
fakePool2 := storage.NewStoragePool(nil, "pool2")
5886+
fakePool2.AddStorageClass("sc-dest")
5887+
fakePool2.SetBackend(mockBackend2)
5888+
5889+
mockBackend1.EXPECT().StoragePools().Return(
5890+
func() *sync.Map {
5891+
m := sync.Map{}
5892+
m.Store("pool1", fakePool1)
5893+
return &m
5894+
}(),
5895+
).AnyTimes()
5896+
mockBackend2.EXPECT().StoragePools().Return(
5897+
func() *sync.Map {
5898+
m := sync.Map{}
5899+
m.Store("pool2", fakePool2)
5900+
return &m
5901+
}(),
5902+
).AnyTimes()
5903+
5904+
sourceVolumeDiffBackend := &storage.Volume{
5905+
Config: &storage.VolumeConfig{
5906+
InternalName: "sourceVolume",
5907+
Name: "sourceVolume",
5908+
Size: "1073741824",
5909+
VolumeMode: config.Filesystem,
5910+
StorageClass: "sc-source",
5911+
},
5912+
BackendUUID: "backend-uuid-1",
5913+
}
5914+
5915+
scSource := storageclass.New(&storageclass.Config{
5916+
Name: "sc-source",
5917+
AdditionalPools: map[string][]string{"testBackend1": {"pool1"}},
5918+
})
5919+
scDest := storageclass.New(&storageclass.Config{
5920+
Name: "sc-dest",
5921+
AdditionalPools: map[string][]string{"testBackend2": {"pool2"}},
5922+
})
5923+
5924+
addBackendsToCache(t, mockBackend1, mockBackend2)
5925+
addVolumesToCache(t, sourceVolumeDiffBackend)
5926+
addStorageClassesToCache(t, scSource, scDest)
5927+
5928+
o.RebuildStorageClassPoolMap(testCtx)
5929+
5930+
mockStoreClient.EXPECT().GetVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
5931+
mockStoreClient.EXPECT().AddVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil).Times(1)
5932+
mockStoreClient.EXPECT().DeleteVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil).Times(1)
5933+
},
5934+
volumeConfig: &storage.VolumeConfig{
5935+
InternalName: "cloneVolume",
5936+
Name: "cloneVolume",
5937+
Size: "1073741824",
5938+
VolumeMode: config.Filesystem,
5939+
CloneSourceVolume: "sourceVolume",
5940+
CloneSourceVolumeInternal: "sourceVolume",
5941+
StorageClass: "sc-dest",
5942+
},
5943+
verifyError: func(err error) {
5944+
assert.True(t, errors.IsMismatchedStorageClassError(err))
5945+
},
5946+
verifyResult: func(result *storage.VolumeExternal) {
5947+
require.Nil(t, result)
5948+
// Additionally verify the volume is not added to the cache
5949+
volume := getVolumeByNameFromCache(t, "cloneVolume")
5950+
assert.Nil(t, volume)
5951+
},
5952+
},
57885953
}
57895954

57905955
for _, tt := range tests {

‎core/orchestrator_core.go‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2374,10 +2374,20 @@ func (o *TridentOrchestrator) cloneVolumeInitial(
23742374
return nil, errors.NotFoundError("source volume not found: %s", volumeConfig.CloneSourceVolume)
23752375
}
23762376

2377-
// Check if the storage class of source and clone volume is different, only if the orchestrator is not in Docker plugin mode. In Docker plugin mode, the storage class of source and clone volume will be different.
2377+
// Check if the source volume's backend is honored by the target storage class, only if the orchestrator
2378+
// is not in Docker plugin mode. In Docker plugin mode, the storage class of source and clone volume
2379+
// will be different.
23782380
if !isDockerPluginMode() && volumeConfig.StorageClass != sourceVolume.Config.StorageClass {
2379-
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s with"+
2380-
" different storage classes is not allowed", volumeConfig.Name, volumeConfig.CloneSourceVolume)
2381+
srcBackend, srcBackendFound := o.backends[sourceVolume.BackendUUID]
2382+
dstSC, dstSCFound := o.storageClasses[volumeConfig.StorageClass]
2383+
// If the source backend is not found in the cache, or the destination storage class is not found in the cache,
2384+
// or the destination storage class is not added to the source backend,
2385+
// then return an error as cloning across different storage classes that have no common backends is not allowed.
2386+
if !srcBackendFound || !dstSCFound || !dstSC.IsAddedToBackend(srcBackend, volumeConfig.StorageClass) {
2387+
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s with"+
2388+
" different storage classes that have no common backends is not allowed",
2389+
volumeConfig.Name, volumeConfig.CloneSourceVolume)
2390+
}
23812391
}
23822392

23832393
Logc(ctx).WithFields(LogFields{
@@ -2615,12 +2625,6 @@ func (o *TridentOrchestrator) cloneVolumeRetry(
26152625
return nil, errors.NotFoundError("source volume not found: %s", cloneConfig.CloneSourceVolume)
26162626
}
26172627

2618-
// Check if the storage class of source and clone volume is different, only if the orchestrator is not in Docker plugin mode. In Docker plugin mode, the storage class of source and clone volume will be different at times.
2619-
if !isDockerPluginMode() && cloneConfig.StorageClass != sourceVolume.Config.StorageClass {
2620-
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s with "+
2621-
"different storage classes is not allowed", cloneConfig.Name, cloneConfig.CloneSourceVolume)
2622-
}
2623-
26242628
backend, found = o.backends[txn.VolumeCreatingConfig.BackendUUID]
26252629
if !found {
26262630
// Should never get here but just to be safe

0 commit comments

Comments
 (0)