Skip to content

Commit b3232a4

Browse files
harikrishna-patnalaharikrishna-patnala
committed
Make copy template across zones non blocking
1 parent a6d8bf6 commit b3232a4

File tree

4 files changed

+88
-16
lines changed

4 files changed

+88
-16
lines changed

‎engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,6 @@ public interface StorageOrchestrationService {
3232
MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreId, List<Long> templateIdList, List<Long> snapshotIdList);
3333

3434
Future<TemplateApiResult> orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore);
35+
36+
Future<TemplateApiResult> orchestrateTemplateCopyAcrossZones(TemplateInfo source, DataStore sourceStore, DataStore destStore);
3537
}

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,13 @@
3636
import javax.inject.Inject;
3737
import javax.naming.ConfigurationException;
3838

39+
import com.cloud.dc.DataCenterVO;
40+
import com.cloud.dc.dao.DataCenterDao;
41+
import com.cloud.storage.VMTemplateVO;
42+
import com.cloud.storage.dao.VMTemplateDao;
43+
import com.cloud.template.TemplateManager;
3944
import org.apache.cloudstack.api.response.MigrationResponse;
45+
import org.apache.cloudstack.context.CallContext;
4046
import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
4147
import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
4248
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
@@ -103,6 +109,13 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra
103109
VolumeDataStoreDao volumeDataStoreDao;
104110
@Inject
105111
DataMigrationUtility migrationHelper;
112+
@Inject
113+
TemplateManager templateManager;
114+
@Inject
115+
VMTemplateDao templateDao;
116+
@Inject
117+
DataCenterDao dcDao;
118+
106119

107120
ConfigKey<Double> ImageStoreImbalanceThreshold = new ConfigKey<>("Advanced", Double.class,
108121
"image.store.imbalance.threshold",
@@ -308,6 +321,14 @@ public Future<TemplateApiResult> orchestrateTemplateCopyToImageStore(TemplateInf
308321
return submit(destStore.getScope().getScopeId(), new CopyTemplateTask(source, destStore));
309322
}
310323

324+
@Override
325+
public Future<TemplateApiResult> orchestrateTemplateCopyAcrossZones(TemplateInfo templateInfo, DataStore sourceStore, DataStore destStore) {
326+
Long dstZoneId = destStore.getScope().getScopeId();
327+
DataCenterVO dstZone = dcDao.findById(dstZoneId);
328+
Long userId = CallContext.current().getCallingUserId();
329+
return submit(dstZoneId, new CrossZoneCopyTemplateTask(userId, templateInfo, sourceStore, dstZone));
330+
}
331+
311332
protected Pair<String, Boolean> migrateCompleted(Long destDatastoreId, DataStore srcDatastore, List<DataObject> files, MigrationPolicy migrationPolicy, int skipped) {
312333
String message = "";
313334
boolean success = true;
@@ -653,4 +674,51 @@ public TemplateApiResult call() {
653674
return result;
654675
}
655676
}
677+
678+
private class CrossZoneCopyTemplateTask implements Callable<TemplateApiResult> {
679+
private final long userId;
680+
private final TemplateInfo sourceTmpl;
681+
private final DataStore sourceStore;
682+
private final DataCenterVO dstZone;
683+
private final String logid;
684+
685+
CrossZoneCopyTemplateTask(long userId,
686+
TemplateInfo sourceTmpl,
687+
DataStore sourceStore,
688+
DataCenterVO dstZone) {
689+
this.userId = userId;
690+
this.sourceTmpl = sourceTmpl;
691+
this.sourceStore = sourceStore;
692+
this.dstZone = dstZone;
693+
this.logid = ThreadContext.get(LOGCONTEXTID);
694+
}
695+
696+
@Override
697+
public TemplateApiResult call() {
698+
ThreadContext.put(LOGCONTEXTID, logid);
699+
TemplateApiResult result;
700+
VMTemplateVO template = templateDao.findById(sourceTmpl.getId());
701+
try {
702+
boolean success = templateManager.copy(userId, template, sourceStore, dstZone);
703+
704+
result = new TemplateApiResult(sourceTmpl);
705+
if (!success) {
706+
result.setResult("Cross-zone template copy failed");
707+
}
708+
} catch (Exception e) {
709+
logger.error("Exception while copying template [{}] from zone [{}] to zone [{}]",
710+
template,
711+
sourceStore.getScope().getScopeId(),
712+
dstZone.getId(),
713+
e);
714+
result = new TemplateApiResult(sourceTmpl);
715+
result.setResult(e.getMessage());
716+
} finally {
717+
tryCleaningUpExecutor(dstZone.getId());
718+
ThreadContext.clearAll();
719+
}
720+
721+
return result;
722+
}
723+
}
656724
}

‎engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131

3232
import javax.inject.Inject;
3333

34-
import org.apache.cloudstack.context.CallContext;
3534
import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
3635
import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
3736
import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
@@ -644,7 +643,7 @@ private boolean searchAndCopyAcrossZones(VMTemplateVO tmplt, DataStore destStore
644643
continue;
645644
}
646645

647-
DataStore sourceStore = findTemplateInStores(tmplt, storesInOtherZone);
646+
DataStore sourceStore = findImageStorageHavingTemplate(tmplt, storesInOtherZone);
648647
if (sourceStore == null) {
649648
logger.debug("Template [{}] not found in any image store of zone [{}].",
650649
tmplt.getUniqueName(), otherZoneId);
@@ -685,7 +684,7 @@ private boolean searchAndCopyWithinZone(VMTemplateVO tmplt, DataStore destStore,
685684
return false;
686685
}
687686

688-
private DataStore findTemplateInStores(VMTemplateVO tmplt, List<DataStore> stores) {
687+
private DataStore findImageStorageHavingTemplate(VMTemplateVO tmplt, List<DataStore> stores) {
689688
for (DataStore store : stores) {
690689
Map<String, TemplateProp> templates = listTemplate(store);
691690
if (templates != null && templates.containsKey(tmplt.getUniqueName())) {
@@ -707,9 +706,10 @@ private boolean copyTemplateAcrossZones(DataStore sourceStore,
707706
return false;
708707
}
709708

709+
TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore);
710710
try {
711-
Long userId = CallContext.current().getCallingUserId();
712-
return _tmpltMgr.copy(userId, tmplt, sourceStore, dstZone);
711+
storageOrchestrator.orchestrateTemplateCopyAcrossZones(sourceTmpl, sourceStore, destStore);
712+
return true;
713713
} catch (Exception e) {
714714
logger.error("Failed to copy template [{}] from zone [{}] to zone [{}]",
715715
tmplt.getUniqueName(),

‎engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,6 @@ public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAno
218218

219219
DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class);
220220
Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L);
221-
Mockito.doReturn(true).when(_tmpltMgr).copy(Mockito.anyLong(), Mockito.eq(tmpltMock), Mockito.eq(otherZoneStoreMock), Mockito.eq(dstZoneMock));
222221

223222
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
224223

@@ -247,31 +246,34 @@ public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenDestinationZoneIsM
247246
}
248247

249248
@Test
250-
public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenCrossZoneCopyThrowsException() throws StorageUnavailableException, ResourceAllocationException {
249+
public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenCrossZoneCopyTaskIsScheduled() {
251250
Scope scopeMock = Mockito.mock(Scope.class);
252251
Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
253252
Mockito.doReturn(1L).when(scopeMock).getScopeId();
253+
254254
Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
255-
Mockito.doReturn(List.of()).when(dataStoreManagerMock).getImageStoresByZoneIds(1L);
255+
256+
Mockito.doReturn(List.of())
257+
.when(dataStoreManagerMock)
258+
.getImageStoresByZoneIds(1L);
256259

257260
DataStore otherZoneStoreMock = Mockito.mock(DataStore.class);
258-
Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L);
261+
Mockito.doReturn(List.of(otherZoneStoreMock))
262+
.when(dataStoreManagerMock)
263+
.getImageStoresByZoneIds(2L);
259264

260265
Map<String, TemplateProp> templates = new HashMap<>();
261266
templates.put(tmpltMock.getUniqueName(), tmpltPropMock);
262-
Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock);
267+
Mockito.doReturn(templates)
268+
.when(templateService)
269+
.listTemplate(otherZoneStoreMock);
263270

264271
DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class);
265272
Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L);
266-
Mockito.doThrow(new RuntimeException("copy failed")).when(_tmpltMgr).copy(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
267-
268-
Scope sourceScopeMock = Mockito.mock(Scope.class);
269-
Mockito.doReturn(sourceScopeMock).when(otherZoneStoreMock).getScope();
270-
Mockito.doReturn(2L).when(sourceScopeMock).getScopeId();
271273

272274
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
273275

274-
Assert.assertFalse(result);
276+
Assert.assertTrue(result);
275277
}
276278

277279
@Test

0 commit comments

Comments
 (0)