Combine template copy and download into a single asynchronous operation

This commit is contained in:
Harikrishna Patnala 2026-01-09 13:22:59 +05:30
parent 7ccaf4e0bc
commit 55e211c3c5
6 changed files with 108 additions and 62 deletions

View File

@ -31,7 +31,5 @@ public interface StorageOrchestrationService {
MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreId, List<Long> templateIdList, List<Long> snapshotIdList);
Future<TemplateApiResult> orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore);
Future<TemplateApiResult> orchestrateTemplateCopyAcrossZones(TemplateInfo source, DataStore destStore);
Future<TemplateApiResult> orchestrateTemplateCopyFromSecondaryStores(long templateId, DataStore destStore);
}

View File

@ -80,4 +80,6 @@ public interface TemplateService {
List<DatadiskTO> getTemplateDatadisksOnImageStore(TemplateInfo templateInfo, String configurationId);
AsyncCallFuture<TemplateApiResult> copyTemplateToImageStore(DataObject source, DataStore destStore);
}
void handleTemplateCopyFromSecondaryStores(long templateId, DataStore destStore);
}

View File

@ -44,7 +44,6 @@ import com.cloud.storage.VMTemplateVO;
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.template.TemplateManager;
import org.apache.cloudstack.api.response.MigrationResponse;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
@ -53,6 +52,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SecondaryStorageServic
import org.apache.cloudstack.engine.subsystem.api.storage.SecondaryStorageService.DataObjectResult;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService;
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult;
@ -116,6 +116,8 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra
@Inject
VMTemplateDao templateDao;
@Inject
TemplateDataFactory templateDataFactory;
@Inject
DataCenterDao dcDao;
@ -324,11 +326,9 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra
}
@Override
public Future<TemplateApiResult> orchestrateTemplateCopyAcrossZones(TemplateInfo templateInfo, DataStore destStore) {
public Future<TemplateApiResult> orchestrateTemplateCopyFromSecondaryStores(long srcTemplateId, DataStore destStore) {
Long dstZoneId = destStore.getScope().getScopeId();
DataCenterVO dstZone = dcDao.findById(dstZoneId);
long userId = CallContext.current().getCallingUserId();
return submit(dstZoneId, new CrossZoneCopyTemplateTask(userId, templateInfo, dstZone));
return submit(dstZoneId, new CopyTemplateFromSecondaryStorageTask(srcTemplateId, destStore));
}
protected Pair<String, Boolean> migrateCompleted(Long destDatastoreId, DataStore srcDatastore, List<DataObject> files, MigrationPolicy migrationPolicy, int skipped) {
@ -677,16 +677,14 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra
}
}
private class CrossZoneCopyTemplateTask implements Callable<TemplateApiResult> {
private final long userId;
private final TemplateInfo sourceTmpl;
private final DataCenterVO dstZone;
private class CopyTemplateFromSecondaryStorageTask implements Callable<TemplateApiResult> {
private final long srcTemplateId;
private final DataStore destStore;
private final String logid;
CrossZoneCopyTemplateTask(long userId, TemplateInfo sourceTmpl, DataCenterVO dstZone) {
this.userId = userId;
this.sourceTmpl = sourceTmpl;
this.dstZone = dstZone;
CopyTemplateFromSecondaryStorageTask(long srcTemplateId, DataStore destStore) {
this.srcTemplateId = srcTemplateId;
this.destStore = destStore;
this.logid = ThreadContext.get(LOGCONTEXTID);
}
@ -694,25 +692,13 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra
public TemplateApiResult call() {
ThreadContext.put(LOGCONTEXTID, logid);
TemplateApiResult result;
VMTemplateVO template = templateDao.findById(sourceTmpl.getId());
long destZoneId = destStore.getScope().getScopeId();
TemplateInfo sourceTmpl = templateDataFactory.getTemplate(srcTemplateId, DataStoreRole.Image);
try {
DataStore sourceStore = sourceTmpl.getDataStore();
boolean success = templateManager.copy(userId, template, sourceStore, dstZone);
templateService.handleTemplateCopyFromSecondaryStores(srcTemplateId, destStore);
result = new TemplateApiResult(sourceTmpl);
if (!success) {
result.setResult("Cross-zone template copy failed");
}
} catch (StorageUnavailableException | ResourceAllocationException e) {
logger.error("Exception while copying template [{}] from zone [{}] to zone [{}]",
template,
sourceTmpl.getDataStore().getScope().getScopeId(),
dstZone.getId(),
e);
result = new TemplateApiResult(sourceTmpl);
result.setResult(e.getMessage());
} finally {
tryCleaningUpExecutor(dstZone.getId());
tryCleaningUpExecutor(destZoneId);
ThreadContext.clearAll();
}

View File

@ -31,6 +31,8 @@ import java.util.concurrent.ExecutionException;
import javax.inject.Inject;
import com.cloud.exception.StorageUnavailableException;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
@ -70,6 +72,7 @@ import org.apache.cloudstack.storage.to.TemplateObjectTO;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.ThreadContext;
import org.springframework.stereotype.Component;
import com.cloud.agent.api.Answer;
@ -548,10 +551,7 @@ public class TemplateServiceImpl implements TemplateService {
}
if (availHypers.contains(tmplt.getHypervisorType())) {
boolean copied = imageStoreDetailsUtil.isCopyTemplatesFromOtherStoragesEnabled(storeId, zoneId) && tryCopyingTemplateToImageStore(tmplt, store);
if (!copied) {
tryDownloadingTemplateToImageStore(tmplt, store);
}
storageOrchestrator.orchestrateTemplateCopyFromSecondaryStores(tmplt.getId(), store);
} else {
logger.info("Skip downloading template {} since current data center does not have hypervisor {}", tmplt, tmplt.getHypervisorType());
}
@ -598,6 +598,16 @@ public class TemplateServiceImpl implements TemplateService {
}
@Override
public void handleTemplateCopyFromSecondaryStores(long templateId, DataStore destStore) {
VMTemplateVO template = _templateDao.findById(templateId);
long zoneId = destStore.getScope().getScopeId();
boolean copied = imageStoreDetailsUtil.isCopyTemplatesFromOtherStoragesEnabled(destStore.getId(), zoneId) && tryCopyingTemplateToImageStore(template, destStore);
if (!copied) {
tryDownloadingTemplateToImageStore(template, destStore);
}
}
protected void tryDownloadingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) {
if (tmplt.getUrl() == null) {
logger.info("Not downloading template [{}] to image store [{}], as it has no URL.", tmplt.getUniqueName(),
@ -686,8 +696,17 @@ public class TemplateServiceImpl implements TemplateService {
return false;
}
storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore);
return true;
TemplateApiResult result;
AsyncCallFuture<TemplateApiResult> future = copyTemplateToImageStore(sourceTmpl, destStore);
try {
result = future.get();
} catch (ExecutionException | InterruptedException e) {
logger.warn("Exception while copying template [{}] from image store [{}] to image store [{}]: {}",
sourceTmpl.getUniqueName(), sourceTmpl.getDataStore().getName(), destStore.getName(), e.toString());
result = new TemplateApiResult(sourceTmpl);
result.setResult(e.getMessage());
}
return result.isSuccess();
}
private boolean copyTemplateAcrossZones(DataStore destStore, TemplateObject sourceTmpl) {
@ -699,9 +718,29 @@ public class TemplateServiceImpl implements TemplateService {
return false;
}
TemplateApiResult result;
try {
storageOrchestrator.orchestrateTemplateCopyAcrossZones(sourceTmpl, destStore);
return true;
VMTemplateVO template = _templateDao.findById(sourceTmpl.getId());
try {
DataStore sourceStore = sourceTmpl.getDataStore();
long userId = CallContext.current().getCallingUserId();
boolean success = _tmpltMgr.copy(userId, template, sourceStore, dstZone);
result = new TemplateApiResult(sourceTmpl);
if (!success) {
result.setResult("Cross-zone template copy failed");
}
} catch (StorageUnavailableException | ResourceAllocationException e) {
logger.error("Exception while copying template [{}] from zone [{}] to zone [{}]",
template,
sourceTmpl.getDataStore().getScope().getScopeId(),
dstZone.getId(),
e);
result = new TemplateApiResult(sourceTmpl);
result.setResult(e.getMessage());
} finally {
ThreadContext.clearAll();
}
} catch (Exception e) {
logger.error("Failed to copy template [{}] from zone [{}] to zone [{}].",
sourceTmpl.getUniqueName(),
@ -710,6 +749,8 @@ public class TemplateServiceImpl implements TemplateService {
e);
return false;
}
return result.isSuccess();
}
@Override

View File

@ -20,13 +20,18 @@ package org.apache.cloudstack.storage.image;
import com.cloud.dc.DataCenterVO;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.StorageUnavailableException;
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.storage.template.TemplateProp;
import com.cloud.template.TemplateManager;
import com.cloud.user.Account;
import com.cloud.user.User;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
import org.apache.cloudstack.framework.async.AsyncCallFuture;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
import org.apache.cloudstack.storage.image.store.TemplateObject;
@ -48,6 +53,8 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import static org.mockito.Mockito.mock;
@RunWith(MockitoJUnitRunner.class)
public class TemplateServiceImplTest {
@ -88,6 +95,9 @@ public class TemplateServiceImplTest {
@Mock
DataCenterDao _dcDao;
@Mock
VMTemplateDao templateDao;
@Mock
TemplateManager _tmpltMgr;
@ -103,7 +113,6 @@ public class TemplateServiceImplTest {
Mockito.doReturn(List.of(sourceStoreMock, destStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(zoneId);
Mockito.doReturn(templatesInSourceStore).when(templateService).listTemplate(sourceStoreMock);
Mockito.doReturn(null).when(templateService).listTemplate(destStoreMock);
Mockito.doReturn("install-path").when(templateInfoMock).getInstallPath();
Mockito.doReturn(templateInfoMock).when(templateDataFactoryMock).getTemplate(2L, sourceStoreMock);
}
@ -168,7 +177,7 @@ public class TemplateServiceImplTest {
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
Assert.assertFalse(result);
Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any());
Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyFromSecondaryStores(Mockito.anyLong(), Mockito.any());
}
@Test
@ -184,22 +193,11 @@ public class TemplateServiceImplTest {
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
Assert.assertFalse(result);
Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any());
Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyFromSecondaryStores(Mockito.anyLong(), Mockito.any());
}
@Test
public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherStorageAndTaskWasScheduled() {
templatesInSourceStore.put(tmpltMock.getUniqueName(), tmpltPropMock);
Mockito.doReturn(new AsyncCallFuture<>()).when(storageOrchestrator).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any());
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
Assert.assertTrue(result);
Mockito.verify(storageOrchestrator).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any());
}
@Test
public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherZone() {
public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherZone() throws StorageUnavailableException, ResourceAllocationException {
Scope scopeMock = Mockito.mock(Scope.class);
Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
Mockito.doReturn(1L).when(scopeMock).getScopeId();
@ -222,6 +220,7 @@ public class TemplateServiceImplTest {
DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class);
Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L);
Mockito.doReturn(true).when(_tmpltMgr).copy(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
@ -256,7 +255,7 @@ public class TemplateServiceImplTest {
}
@Test
public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenCrossZoneCopyTaskIsScheduled() {
public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenCrossZoneCopyTaskIsScheduled() throws StorageUnavailableException, ResourceAllocationException {
Scope scopeMock = Mockito.mock(Scope.class);
Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
Mockito.doReturn(1L).when(scopeMock).getScopeId();
@ -270,15 +269,27 @@ public class TemplateServiceImplTest {
Map<String, TemplateProp> templates = new HashMap<>();
templates.put("unique-name", tmpltPropMock);
Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock);
TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class);
Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock);
Mockito.doReturn("/mnt/secondary/template.qcow2").when(sourceTmplMock).getInstallPath();
Mockito.doReturn(100L).when(sourceTmplMock).getId();
DataStore sourceStoreMock = Mockito.mock(DataStore.class);
Scope sourceScopeMock = Mockito.mock(Scope.class);
Mockito.doReturn(sourceStoreMock).when(sourceTmplMock).getDataStore();
DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class);
Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L);
VMTemplateVO templateVoMock = Mockito.mock(VMTemplateVO.class);
Mockito.doReturn(templateVoMock).when(templateDao).findById(100L);
Mockito.doReturn(true).when(_tmpltMgr).copy(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
Account account = mock(Account.class);
User user = mock(User.class);
CallContext callContext = mock(CallContext.class);
boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);

View File

@ -838,6 +838,9 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
// Copy will just find one eligible image store for the destination zone
// and copy template there, not propagate to all image stores
// for that zone
boolean copied = false;
for (DataStore dstSecStore : dstSecStores) {
TemplateDataStoreVO dstTmpltStore = _tmplStoreDao.findByStoreTemplate(dstSecStore.getId(), tmpltId);
if (dstTmpltStore != null && dstTmpltStore.getDownloadState() == Status.DOWNLOADED) {
@ -852,9 +855,12 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
TemplateApiResult result = future.get();
if (result.isFailed()) {
logger.debug("Copy Template failed for image store {}: {}", dstSecStore, result.getResult());
_tmplStoreDao.removeByTemplateStore(tmpltId, dstSecStore.getId());
continue; // try next image store
}
copied = true;
_tmpltDao.addTemplateToZone(template, dstZoneId);
if (account.getId() != Account.ACCOUNT_ID_SYSTEM) {
@ -882,12 +888,14 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
}
}
}
return true;
} catch (Exception ex) {
logger.debug("Failed to copy Template to image store:{} ,will try next one", dstSecStore);
logger.debug("Failed to copy Template to image store:{} ,will try next one", dstSecStore, ex);
}
}
return true;
return copied;
}
@Override