Skip to content

Commit cd88132

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 e521883 commit cd88132

File tree

5 files changed

+369
-32
lines changed

5 files changed

+369
-32
lines changed

‎core/concurrent_core.go‎

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

2186-
// 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.
2186+
// Check if the source volume's backend is honored by the target storage class, only if the orchestrator
2187+
// is not in Docker plugin mode. In Docker plugin mode, the storage class of source and clone volume
2188+
// will be different at times.
21872189
if !isDockerPluginMode() && volConfig.StorageClass != sourceVolume.Config.StorageClass {
2188-
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s with different storage classes is not allowed",
2189-
volConfig.Name, volConfig.CloneSourceVolume)
2190+
poolMap := o.GetStorageClassPoolMap()
2191+
if !poolMap.BackendMatchesStorageClass(ctx, backend.Name(), volConfig.StorageClass) {
2192+
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s with "+
2193+
"different storage classes that have no common backends is not allowed",
2194+
volConfig.Name, volConfig.CloneSourceVolume)
2195+
}
21902196
}
21912197

21922198
if volConfig.Size != "" {
@@ -2373,12 +2379,6 @@ func (o *ConcurrentTridentOrchestrator) cloneVolumeRetry(
23732379

23742380
Logc(ctx).WithFields(logFields).Debug("Cloning volume.")
23752381

2376-
// 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.
2377-
if !isDockerPluginMode() && cloneConfig.StorageClass != sourceVolConfig.StorageClass {
2378-
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s with different storage classes is not allowed",
2379-
cloneConfig.Name, cloneConfig.CloneSourceVolume)
2380-
}
2381-
23822382
// Create the volume
23832383
createVolume := func() error {
23842384
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
@@ -4330,6 +4330,171 @@ func TestCloneVolumeConcurrentCore(t *testing.T) {
43304330
assert.Nil(t, volume)
43314331
},
43324332
},
4333+
{
4334+
name: "CloneVolumeFromDifferentStorageClassSameBackendSuccess",
4335+
bootstrapErr: nil,
4336+
setupMocks: func(mockCtrl *gomock.Controller, mockStoreClient *mockpersistentstore.MockStoreClient, o *ConcurrentTridentOrchestrator) {
4337+
mockBackend := getMockBackend(mockCtrl, "testBackend", "backend-uuid")
4338+
4339+
fakePool := storage.NewStoragePool(nil, "pool1")
4340+
fakePool.AddStorageClass("sc-source")
4341+
fakePool.AddStorageClass("sc-dest")
4342+
fakePool.SetBackend(mockBackend)
4343+
4344+
mockBackend.EXPECT().StoragePools().Return(
4345+
func() *sync.Map {
4346+
m := sync.Map{}
4347+
m.Store("pool1", fakePool)
4348+
return &m
4349+
}(),
4350+
).AnyTimes()
4351+
mockBackend.EXPECT().CreatePrepare(gomock.Any(), gomock.Any(), gomock.Any())
4352+
mockBackend.EXPECT().CloneVolume(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), false).Return(
4353+
&storage.Volume{
4354+
Config: &storage.VolumeConfig{
4355+
InternalName: "cloneVolume",
4356+
Name: "cloneVolume",
4357+
Size: "1073741824",
4358+
VolumeMode: config.Filesystem,
4359+
CloneSourceVolume: "sourceVolume",
4360+
CloneSourceVolumeInternal: "sourceVolume",
4361+
StorageClass: "sc-dest",
4362+
},
4363+
BackendUUID: "backend-uuid",
4364+
}, nil).Times(1)
4365+
4366+
sourceVolumeDiffSC := &storage.Volume{
4367+
Config: &storage.VolumeConfig{
4368+
InternalName: "sourceVolume",
4369+
Name: "sourceVolume",
4370+
Size: "1073741824",
4371+
VolumeMode: config.Filesystem,
4372+
StorageClass: "sc-source",
4373+
},
4374+
BackendUUID: "backend-uuid",
4375+
}
4376+
4377+
scSource := storageclass.New(&storageclass.Config{
4378+
Name: "sc-source",
4379+
AdditionalPools: map[string][]string{"testBackend": {"pool1"}},
4380+
})
4381+
scDest := storageclass.New(&storageclass.Config{
4382+
Name: "sc-dest",
4383+
AdditionalPools: map[string][]string{"testBackend": {"pool1"}},
4384+
})
4385+
4386+
addBackendsToCache(t, mockBackend)
4387+
addVolumesToCache(t, sourceVolumeDiffSC)
4388+
addStorageClassesToCache(t, scSource, scDest)
4389+
4390+
o.RebuildStorageClassPoolMap(testCtx)
4391+
4392+
mockStoreClient.EXPECT().GetVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
4393+
mockStoreClient.EXPECT().AddVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil).Times(1)
4394+
mockStoreClient.EXPECT().UpdateVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil).Times(1)
4395+
mockStoreClient.EXPECT().DeleteVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil).Times(1)
4396+
mockStoreClient.EXPECT().AddVolume(gomock.Any(), gomock.Any()).Return(nil).Times(1)
4397+
},
4398+
volumeConfig: &storage.VolumeConfig{
4399+
InternalName: "cloneVolume",
4400+
Name: "cloneVolume",
4401+
Size: "1073741824",
4402+
VolumeMode: config.Filesystem,
4403+
CloneSourceVolume: "sourceVolume",
4404+
CloneSourceVolumeInternal: "sourceVolume",
4405+
StorageClass: "sc-dest",
4406+
},
4407+
verifyError: func(err error) {
4408+
assert.NoError(t, err)
4409+
},
4410+
verifyResult: func(result *storage.VolumeExternal) {
4411+
require.NotNil(t, result)
4412+
assert.Equal(t, "cloneVolume", result.Config.Name)
4413+
4414+
// Additionally verify the volume is added to the cache
4415+
volume := getVolumeByNameFromCache(t, "cloneVolume")
4416+
assert.NotNil(t, volume)
4417+
},
4418+
},
4419+
{
4420+
name: "CloneVolumeFromDifferentStorageClassDifferentBackendFailed",
4421+
bootstrapErr: nil,
4422+
setupMocks: func(mockCtrl *gomock.Controller, mockStoreClient *mockpersistentstore.MockStoreClient, o *ConcurrentTridentOrchestrator) {
4423+
mockBackend1 := getMockBackend(mockCtrl, "testBackend1", "backend-uuid-1")
4424+
mockBackend2 := getMockBackend(mockCtrl, "testBackend2", "backend-uuid-2")
4425+
4426+
fakePool1 := storage.NewStoragePool(nil, "pool1")
4427+
fakePool1.AddStorageClass("sc-source")
4428+
fakePool1.SetBackend(mockBackend1)
4429+
4430+
fakePool2 := storage.NewStoragePool(nil, "pool2")
4431+
fakePool2.AddStorageClass("sc-dest")
4432+
fakePool2.SetBackend(mockBackend2)
4433+
4434+
mockBackend1.EXPECT().StoragePools().Return(
4435+
func() *sync.Map {
4436+
m := sync.Map{}
4437+
m.Store("pool1", fakePool1)
4438+
return &m
4439+
}(),
4440+
).AnyTimes()
4441+
mockBackend2.EXPECT().StoragePools().Return(
4442+
func() *sync.Map {
4443+
m := sync.Map{}
4444+
m.Store("pool2", fakePool2)
4445+
return &m
4446+
}(),
4447+
).AnyTimes()
4448+
4449+
sourceVolumeDiffBackend := &storage.Volume{
4450+
Config: &storage.VolumeConfig{
4451+
InternalName: "sourceVolume",
4452+
Name: "sourceVolume",
4453+
Size: "1073741824",
4454+
VolumeMode: config.Filesystem,
4455+
StorageClass: "sc-source",
4456+
},
4457+
BackendUUID: "backend-uuid-1",
4458+
}
4459+
4460+
scSource := storageclass.New(&storageclass.Config{
4461+
Name: "sc-source",
4462+
AdditionalPools: map[string][]string{"testBackend1": {"pool1"}},
4463+
})
4464+
scDest := storageclass.New(&storageclass.Config{
4465+
Name: "sc-dest",
4466+
AdditionalPools: map[string][]string{"testBackend2": {"pool2"}},
4467+
})
4468+
4469+
addBackendsToCache(t, mockBackend1, mockBackend2)
4470+
addVolumesToCache(t, sourceVolumeDiffBackend)
4471+
addStorageClassesToCache(t, scSource, scDest)
4472+
4473+
o.RebuildStorageClassPoolMap(testCtx)
4474+
4475+
mockStoreClient.EXPECT().GetVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
4476+
mockStoreClient.EXPECT().AddVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil).Times(1)
4477+
mockStoreClient.EXPECT().DeleteVolumeTransaction(gomock.Any(), gomock.Any()).Return(nil).Times(1)
4478+
},
4479+
volumeConfig: &storage.VolumeConfig{
4480+
InternalName: "cloneVolume",
4481+
Name: "cloneVolume",
4482+
Size: "1073741824",
4483+
VolumeMode: config.Filesystem,
4484+
CloneSourceVolume: "sourceVolume",
4485+
CloneSourceVolumeInternal: "sourceVolume",
4486+
StorageClass: "sc-dest",
4487+
},
4488+
verifyError: func(err error) {
4489+
assert.True(t, errors.IsMismatchedStorageClassError(err))
4490+
},
4491+
verifyResult: func(result *storage.VolumeExternal) {
4492+
require.Nil(t, result)
4493+
// Additionally verify the volume is not added to the cache
4494+
volume := getVolumeByNameFromCache(t, "cloneVolume")
4495+
assert.Nil(t, volume)
4496+
},
4497+
},
43334498
}
43344499

43354500
for _, tt := range tests {

‎core/orchestrator_core.go‎

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

2293-
// 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.
2293+
// Check if the source volume's backend is honored by the target storage class, only if the orchestrator
2294+
// is not in Docker plugin mode. In Docker plugin mode, the storage class of source and clone volume
2295+
// will be different.
22942296
if !isDockerPluginMode() && volumeConfig.StorageClass != sourceVolume.Config.StorageClass {
2295-
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s with different storage classes is not allowed", volumeConfig.Name,
2296-
volumeConfig.CloneSourceVolume)
2297+
srcBackend, srcBackendFound := o.backends[sourceVolume.BackendUUID]
2298+
dstSC, dstSCFound := o.storageClasses[volumeConfig.StorageClass]
2299+
// If the source backend is not found in the cache, or the destination storage class is not found in the cache,
2300+
// or the destination storage class is not added to the source backend,
2301+
// then return an error as cloning across different storage classes that have no common backends is not allowed.
2302+
if !srcBackendFound || !dstSCFound || !dstSC.IsAddedToBackend(srcBackend, volumeConfig.StorageClass) {
2303+
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s with"+
2304+
" different storage classes that have no common backends is not allowed",
2305+
volumeConfig.Name, volumeConfig.CloneSourceVolume)
2306+
}
22972307
}
22982308

22992309
Logc(ctx).WithFields(LogFields{
@@ -2531,12 +2541,6 @@ func (o *TridentOrchestrator) cloneVolumeRetry(
25312541
return nil, errors.NotFoundError("source volume not found: %s", cloneConfig.CloneSourceVolume)
25322542
}
25332543

2534-
// 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.
2535-
if !isDockerPluginMode() && cloneConfig.StorageClass != sourceVolume.Config.StorageClass {
2536-
return nil, errors.MismatchedStorageClassError("clone volume %s from source volume %s with different storage classes is not allowed",
2537-
cloneConfig.Name, cloneConfig.CloneSourceVolume)
2538-
}
2539-
25402544
backend, found = o.backends[txn.VolumeCreatingConfig.BackendUUID]
25412545
if !found {
25422546
// Should never get here but just to be safe

0 commit comments

Comments
 (0)