diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java index 481d0ebbc769..8be2015bfef6 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java @@ -18,12 +18,18 @@ package org.apache.cloudstack.engine.orchestration.service; import java.util.List; +import java.util.concurrent.Future; import org.apache.cloudstack.api.response.MigrationResponse; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult; import org.apache.cloudstack.storage.ImageStoreService.MigrationPolicy; public interface StorageOrchestrationService { MigrationResponse migrateData(Long srcDataStoreId, List destDatastores, MigrationPolicy migrationPolicy); MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreId, List templateIdList, List snapshotIdList); + + Future orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore); } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java index 115cf024617f..a8861d5acc68 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java @@ -78,4 +78,6 @@ public TemplateInfo getTemplate() { AsyncCallFuture createDatadiskTemplateAsync(TemplateInfo parentTemplate, TemplateInfo dataDiskTemplate, String path, String diskId, long fileSize, boolean bootable); List getTemplateDatadisksOnImageStore(TemplateInfo templateInfo, String configurationId); + + AsyncCallFuture copyTemplateToImageStore(DataObject source, DataStore destStore); } diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index 7b31ec6a81b9..4196d98daee7 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -220,6 +220,10 @@ public interface StorageManager extends StorageService { "storage.pool.host.connect.workers", "1", "Number of worker threads to be used to connect hosts to a primary storage", true); + ConfigKey COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES = new ConfigKey<>(Boolean.class, "copy.public.templates.from.other.storages", + "Storage", "true", "Allow SSVMs to try copying public templates from one secondary storage to another instead of downloading them from the source.", + true, ConfigKey.Scope.Zone, null); + /** * should we execute in sequence not involving any storages? * @return tru if commands should execute in sequence diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java index c260f48dcf8c..89fc75415b16 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java @@ -299,10 +299,9 @@ protected List getAllReadyVolumes(DataStore srcDataStore) { /** Returns the count of active SSVMs - SSVM with agents in connected state, so as to dynamically increase the thread pool * size when SSVMs scale */ - protected int activeSSVMCount(DataStore dataStore) { - long datacenterId = dataStore.getScope().getScopeId(); + protected int activeSSVMCount(Long zoneId) { List ssvms = - secStorageVmDao.getSecStorageVmListInStates(null, datacenterId, VirtualMachine.State.Running, VirtualMachine.State.Migrating); + secStorageVmDao.getSecStorageVmListInStates(null, zoneId, VirtualMachine.State.Running, VirtualMachine.State.Migrating); int activeSSVMs = 0; for (SecondaryStorageVmVO vm : ssvms) { String name = "s-"+vm.getId()+"-VM"; diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java index 0773c20b6b98..f9366c2df7fe 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java @@ -46,6 +46,8 @@ 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.TemplateInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult; import org.apache.cloudstack.framework.async.AsyncCallFuture; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; @@ -91,6 +93,8 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra @Inject private SecondaryStorageService secStgSrv; @Inject + TemplateService templateService; + @Inject TemplateDataStoreDao templateDataStoreDao; @Inject VolumeDataStoreDao volumeDataStoreDao; @@ -106,6 +110,8 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra Integer numConcurrentCopyTasksPerSSVM = 2; + private final Map zoneExecutorMap = new HashMap<>(); + @Override public String getConfigComponentName() { return StorageOrchestrationService.class.getName(); @@ -167,8 +173,6 @@ public MigrationResponse migrateData(Long srcDataStoreId, List destDatasto double meanstddev = getStandardDeviation(storageCapacities); double threshold = ImageStoreImbalanceThreshold.value(); MigrationResponse response = null; - ThreadPoolExecutor executor = new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM , numConcurrentCopyTasksPerSSVM, 30, - TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM)); Date start = new Date(); if (meanstddev < threshold && migrationPolicy == MigrationPolicy.BALANCE) { logger.debug("mean std deviation of the image stores is below threshold, no migration required"); @@ -177,7 +181,7 @@ public MigrationResponse migrateData(Long srcDataStoreId, List destDatasto } int skipped = 0; - List>> futures = new ArrayList<>(); + List> futures = new ArrayList<>(); while (true) { DataObject chosenFileForMigration = null; if (files.size() > 0) { @@ -206,7 +210,7 @@ public MigrationResponse migrateData(Long srcDataStoreId, List destDatasto } if (shouldMigrate(chosenFileForMigration, srcDatastore.getId(), destDatastoreId, storageCapacities, snapshotChains, childTemplates, migrationPolicy)) { - storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destDatastoreId, executor, futures); + storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destDatastoreId, futures); } else { if (migrationPolicy == MigrationPolicy.BALANCE) { continue; @@ -217,7 +221,7 @@ public MigrationResponse migrateData(Long srcDataStoreId, List destDatasto } } Date end = new Date(); - handleSnapshotMigration(srcDataStoreId, start, end, migrationPolicy, futures, storageCapacities, executor); + handleSnapshotMigration(srcDataStoreId, start, end, migrationPolicy, futures, storageCapacities); return handleResponse(futures, migrationPolicy, message, success); } @@ -250,9 +254,7 @@ public MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreI storageCapacities = getStorageCapacities(storageCapacities, srcImgStoreId); storageCapacities = getStorageCapacities(storageCapacities, destImgStoreId); - ThreadPoolExecutor executor = new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM, numConcurrentCopyTasksPerSSVM, 30, - TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM)); - List>> futures = new ArrayList<>(); + List> futures = new ArrayList<>(); Date start = new Date(); while (true) { @@ -272,7 +274,7 @@ public MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreI } if (storageCapacityBelowThreshold(storageCapacities, destImgStoreId)) { - storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destImgStoreId, executor, futures); + storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destImgStoreId, futures); } else { message = "Migration failed. Destination store doesn't have enough capacity for migration"; success = false; @@ -289,7 +291,7 @@ public MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreI SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(snap.getSnapshotId(), snap.getDataStoreId(), DataStoreRole.Image); SnapshotInfo parentSnapshot = snapshotInfo.getParent(); if (snapshotInfo.getDataStore().getId() == srcImgStoreId && parentSnapshot != null && migratedSnapshotIdList.contains(parentSnapshot.getSnapshotId())) { - futures.add(executor.submit(new MigrateDataTask(snapshotInfo, srcDatastore, dataStoreManager.getDataStore(destImgStoreId, DataStoreRole.Image)))); + futures.add(submit(srcDatastore.getScope().getScopeId(), new MigrateDataTask(snapshotInfo, srcDatastore, dataStoreManager.getDataStore(destImgStoreId, DataStoreRole.Image)))); } }); } @@ -297,6 +299,11 @@ public MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreI return handleResponse(futures, null, message, success); } + @Override + public Future orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore) { + return submit(destStore.getScope().getScopeId(), new CopyTemplateTask(source, destStore)); + } + protected Pair migrateCompleted(Long destDatastoreId, DataStore srcDatastore, List files, MigrationPolicy migrationPolicy, int skipped) { String message = ""; boolean success = true; @@ -332,19 +339,10 @@ protected Map> migrateAway( Map, Long>> templateChains, DataStore srcDatastore, Long destDatastoreId, - ThreadPoolExecutor executor, - List>> futures) { + List> futures) { Long fileSize = migrationHelper.getFileSize(chosenFileForMigration, snapshotChains, templateChains); - storageCapacities = assumeMigrate(storageCapacities, srcDatastore.getId(), destDatastoreId, fileSize); - long activeSsvms = migrationHelper.activeSSVMCount(srcDatastore); - long totalJobs = activeSsvms * numConcurrentCopyTasksPerSSVM; - // Increase thread pool size with increase in number of SSVMs - if ( totalJobs > executor.getCorePoolSize()) { - executor.setMaximumPoolSize((int) (totalJobs)); - executor.setCorePoolSize((int) (totalJobs)); - } MigrateDataTask task = new MigrateDataTask(chosenFileForMigration, srcDatastore, dataStoreManager.getDataStore(destDatastoreId, DataStoreRole.Image)); if (chosenFileForMigration instanceof SnapshotInfo ) { @@ -353,19 +351,56 @@ protected Map> migrateAway( if (chosenFileForMigration instanceof TemplateInfo) { task.setTemplateChain(templateChains); } - futures.add((executor.submit(task))); + futures.add(submit(srcDatastore.getScope().getScopeId(), task)); logger.debug("Migration of {}: {} is initiated.", chosenFileForMigration.getType().name(), chosenFileForMigration.getUuid()); return storageCapacities; } + protected synchronized Future submit(Long zoneId, Callable task) { + if (!zoneExecutorMap.containsKey(zoneId)) { + zoneExecutorMap.put(zoneId, new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM, numConcurrentCopyTasksPerSSVM, + 30, TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM))); + } + scaleExecutorIfNecessary(zoneId); + return zoneExecutorMap.get(zoneId).submit(task); + } + + protected void scaleExecutorIfNecessary(Long zoneId) { + long activeSsvms = migrationHelper.activeSSVMCount(zoneId); + long totalJobs = activeSsvms * numConcurrentCopyTasksPerSSVM; + ThreadPoolExecutor executor = zoneExecutorMap.get(zoneId); + if (totalJobs > executor.getCorePoolSize()) { + logger.debug("Scaling up executor of zone [{}] from [{}] to [{}] threads.", zoneId, executor.getCorePoolSize(), + totalJobs); + executor.setMaximumPoolSize((int) (totalJobs)); + executor.setCorePoolSize((int) (totalJobs)); + } + } + + protected synchronized void tryCleaningUpExecutor(Long zoneId) { + if (!zoneExecutorMap.containsKey(zoneId)) { + logger.debug("No executor exists for zone [{}].", zoneId); + return; + } + ThreadPoolExecutor executor = zoneExecutorMap.get(zoneId); + int activeTasks = executor.getActiveCount(); + if (activeTasks > 1) { + logger.debug("Not cleaning executor of zone [{}] yet, as there are [{}] active tasks.", zoneId, activeTasks); + return; + } - private MigrationResponse handleResponse(List>> futures, MigrationPolicy migrationPolicy, String message, boolean success) { + logger.debug("Cleaning executor of zone [{}].", zoneId); + zoneExecutorMap.remove(zoneId); + executor.shutdown(); + } + + private MigrationResponse handleResponse(List> futures, MigrationPolicy migrationPolicy, String message, boolean success) { int successCount = 0; - for (Future> future : futures) { + for (Future future : futures) { try { - AsyncCallFuture res = future.get(); - if (res.get().isSuccess()) { + DataObjectResult res = future.get(); + if (res.isSuccess()) { successCount++; } } catch ( InterruptedException | ExecutionException e) { @@ -379,7 +414,7 @@ private MigrationResponse handleResponse(List>> futures, Map> storageCapacities, ThreadPoolExecutor executor) { + List> futures, Map> storageCapacities) { DataStore srcDatastore = dataStoreManager.getDataStore(srcDataStoreId, DataStoreRole.Image); List snaps = snapshotDataStoreDao.findSnapshots(srcDataStoreId, start, end); if (!snaps.isEmpty()) { @@ -395,12 +430,12 @@ private void handleSnapshotMigration(Long srcDataStoreId, Date start, Date end, storeId = dstores.get(1); } DataStore datastore = dataStoreManager.getDataStore(storeId, DataStoreRole.Image); - futures.add(executor.submit(new MigrateDataTask(snapshotInfo, srcDatastore, datastore))); + futures.add(submit(srcDatastore.getScope().getScopeId(), new MigrateDataTask(snapshotInfo, srcDatastore, datastore))); } if (parentSnapshot != null) { DataStore parentDS = dataStoreManager.getDataStore(parentSnapshot.getDataStore().getId(), DataStoreRole.Image); if (parentDS.getId() != snapshotInfo.getDataStore().getId()) { - futures.add(executor.submit(new MigrateDataTask(snapshotInfo, srcDatastore, parentDS))); + futures.add(submit(srcDatastore.getScope().getScopeId(), new MigrateDataTask(snapshotInfo, srcDatastore, parentDS))); } } } @@ -527,7 +562,7 @@ private double calculateStorageStandardDeviation(double[] metricValues, double m return standardDeviation.evaluate(metricValues, mean); } - private class MigrateDataTask implements Callable> { + private class MigrateDataTask implements Callable { private DataObject file; private DataStore srcDataStore; private DataStore destDataStore; @@ -557,8 +592,44 @@ public DataObject getFile() { } @Override - public AsyncCallFuture call() throws Exception { - return secStgSrv.migrateData(file, srcDataStore, destDataStore, snapshotChain, templateChain); + public DataObjectResult call() { + DataObjectResult result; + AsyncCallFuture future = secStgSrv.migrateData(file, srcDataStore, destDataStore, snapshotChain, templateChain); + try { + result = future.get(); + } catch (ExecutionException | InterruptedException e) { + logger.warn("Exception while migrating data to another secondary storage: {}", e.toString()); + result = new DataObjectResult(file); + result.setResult(e.toString()); + } + tryCleaningUpExecutor(srcDataStore.getScope().getScopeId()); + return result; + } + } + + private class CopyTemplateTask implements Callable { + private TemplateInfo sourceTmpl; + private DataStore destStore; + + public CopyTemplateTask(TemplateInfo sourceTmpl, DataStore destStore) { + this.sourceTmpl = sourceTmpl; + this.destStore = destStore; + } + + @Override + public TemplateApiResult call() { + TemplateApiResult result; + AsyncCallFuture future = templateService.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()); + } + tryCleaningUpExecutor(destStore.getScope().getScopeId()); + return result; } } } diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java index 38e0d0d081cb..aa09c924775a 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java @@ -31,6 +31,7 @@ import javax.inject.Inject; +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; import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionService; @@ -42,7 +43,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.State; -import org.apache.cloudstack.engine.subsystem.api.storage.Scope; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory; @@ -77,7 +77,6 @@ import com.cloud.agent.api.to.DatadiskTO; import com.cloud.alert.AlertManager; import com.cloud.configuration.Config; -import com.cloud.configuration.Resource; import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.ClusterDao; @@ -157,6 +156,8 @@ public class TemplateServiceImpl implements TemplateService { ImageStoreDetailsUtil imageStoreDetailsUtil; @Inject TemplateDataFactory imageFactory; + @Inject + StorageOrchestrationService storageOrchestrator; class TemplateOpContext extends AsyncRpcContext { final TemplateObject template; @@ -320,7 +321,6 @@ public void handleTemplateSync(DataStore store) { if (syncLock.lock(3)) { try { Long zoneId = store.getScope().getScopeId(); - Map templateInfos = listTemplate(store); if (templateInfos == null) { return; @@ -529,10 +529,6 @@ public void handleTemplateSync(DataStore store) { availHypers.add(HypervisorType.None); // bug 9809: resume ISO // download. for (VMTemplateVO tmplt : toBeDownloaded) { - if (tmplt.getUrl() == null) { // If url is null, skip downloading - logger.info("Skip downloading template {} since no url is specified.", tmplt); - continue; - } // if this is private template, skip sync to a new image store if (isSkipTemplateStoreDownload(tmplt, zoneId)) { logger.info("Skip sync downloading private template {} to a new image store", tmplt); @@ -551,14 +547,10 @@ public void handleTemplateSync(DataStore store) { } if (availHypers.contains(tmplt.getHypervisorType())) { - logger.info("Downloading template {} to image store {}", tmplt, store); - associateTemplateToZone(tmplt.getId(), zoneId); - TemplateInfo tmpl = _templateFactory.getTemplate(tmplt.getId(), store); - TemplateOpContext context = new TemplateOpContext<>(null,(TemplateObject)tmpl, null); - AsyncCallbackDispatcher caller = AsyncCallbackDispatcher.create(this); - caller.setCallback(caller.getTarget().createTemplateAsyncCallBack(null, null)); - caller.setContext(context); - createTemplateAsync(tmpl, store, caller); + boolean copied = isCopyFromOtherStoragesEnabled(zoneId) && tryCopyingTemplateToImageStore(tmplt, store); + if (!copied) { + tryDownloadingTemplateToImageStore(tmplt, store); + } } else { logger.info("Skip downloading template {} since current data center does not have hypervisor {}", tmplt, tmplt.getHypervisorType()); } @@ -605,6 +597,118 @@ public void handleTemplateSync(DataStore store) { } + 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(), + destStore.getName()); + return; + } + logger.info("Downloading template [{}] to image store [{}].", tmplt.getUniqueName(), destStore.getName()); + associateTemplateToZone(tmplt.getId(), destStore.getScope().getScopeId()); + TemplateInfo tmpl = _templateFactory.getTemplate(tmplt.getId(), destStore); + TemplateOpContext context = new TemplateOpContext<>(null,(TemplateObject)tmpl, null); + AsyncCallbackDispatcher caller = AsyncCallbackDispatcher.create(this); + caller.setCallback(caller.getTarget().createTemplateAsyncCallBack(null, null)); + caller.setContext(context); + createTemplateAsync(tmpl, destStore, caller); + } + + protected boolean tryCopyingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) { + Long zoneId = destStore.getScope().getScopeId(); + List storesInZone = _storeMgr.getImageStoresByZoneIds(zoneId); + for (DataStore sourceStore : storesInZone) { + Map existingTemplatesInSourceStore = listTemplate(sourceStore); + if (existingTemplatesInSourceStore == null || !existingTemplatesInSourceStore.containsKey(tmplt.getUniqueName())) { + logger.debug("Template [{}] does not exist on image store [{}]; searching on another one.", + tmplt.getUniqueName(), sourceStore.getName()); + continue; + } + TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore); + if (sourceTmpl.getInstallPath() == null) { + logger.warn("Can not copy template [{}] from image store [{}], as it returned a null install path.", tmplt.getUniqueName(), + sourceStore.getName()); + continue; + } + storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore); + return true; + } + logger.debug("Can't copy template [{}] from another image store.", tmplt.getUniqueName()); + return false; + } + + @Override + public AsyncCallFuture copyTemplateToImageStore(DataObject source, DataStore destStore) { + TemplateObject sourceTmpl = (TemplateObject) source; + logger.debug("Copying template [{}] from image store [{}] to [{}].", sourceTmpl.getUniqueName(), sourceTmpl.getDataStore().getName(), + destStore.getName()); + TemplateObject destTmpl = (TemplateObject) destStore.create(sourceTmpl); + destTmpl.processEvent(Event.CreateOnlyRequested); + + AsyncCallFuture future = new AsyncCallFuture<>(); + TemplateOpContext context = new TemplateOpContext<>(null, destTmpl, future); + AsyncCallbackDispatcher caller = AsyncCallbackDispatcher.create(this); + caller.setCallback(caller.getTarget().copyTemplateToImageStoreCallback(null, null)).setContext(context); + _motionSrv.copyAsync(sourceTmpl, destTmpl, caller); + return future; + } + + protected Void copyTemplateToImageStoreCallback(AsyncCallbackDispatcher callback, TemplateOpContext context) { + TemplateInfo tmplt = context.getTemplate(); + CopyCommandResult result = callback.getResult(); + AsyncCallFuture future = context.getFuture(); + TemplateApiResult res = new TemplateApiResult(tmplt); + if (result.isSuccess()) { + logger.info("Copied template [{}] to image store [{}].", tmplt.getUniqueName(), tmplt.getDataStore().getName()); + tmplt.processEvent(Event.OperationSuccessed, result.getAnswer()); + publishTemplateCreation(tmplt); + } else { + logger.warn("Failed to copy template [{}] to image store [{}].", tmplt.getUniqueName(), tmplt.getDataStore().getName()); + res.setResult(result.getResult()); + tmplt.processEvent(Event.OperationFailed); + } + future.complete(res); + return null; + } + + protected boolean isCopyFromOtherStoragesEnabled(Long zoneId) { + return StorageManager.COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES.valueIn(zoneId); + } + + protected void publishTemplateCreation(TemplateInfo tmplt) { + VMTemplateVO tmpltVo = _templateDao.findById(tmplt.getId()); + + if (tmpltVo.isPublicTemplate()) { + _messageBus.publish(null, TemplateManager.MESSAGE_REGISTER_PUBLIC_TEMPLATE_EVENT, PublishScope.LOCAL, tmpltVo.getId()); + } + + Long size = tmplt.getSize(); + if (size == null) { + return; + } + + DataStore store = tmplt.getDataStore(); + TemplateDataStoreVO tmpltStore = _vmTemplateStoreDao.findByStoreTemplate(store.getId(), tmpltVo.getId()); + + long physicalSize = 0; + if (tmpltStore != null) { + physicalSize = tmpltStore.getPhysicalSize(); + } else { + logger.warn("No entry found in template_store_ref for template [{}] and image store [{}] at the end of registering template!", + tmpltVo.getUniqueName(), store.getName()); + } + + Long zoneId = store.getScope().getScopeId(); + if (zoneId != null) { + String usageEvent = tmplt.getFormat() == ImageFormat.ISO ? EventTypes.EVENT_ISO_CREATE : EventTypes.EVENT_TEMPLATE_CREATE; + UsageEventUtils.publishUsageEvent(usageEvent, tmpltVo.getAccountId(), zoneId, tmpltVo.getId(), tmpltVo.getName(), + null, null, physicalSize, size, VirtualMachineTemplate.class.getName(), tmpltVo.getUuid()); + } else { + logger.warn("Zone-wide image store [{}] has a null scope ID.", store); + } + + _resourceLimitMgr.incrementResourceCount(tmpltVo.getAccountId(), ResourceType.secondary_storage, size); + } + // persist entry in template_zone_ref table. zoneId can be empty for // region-wide image store, in that case, // we will associate the template to all the zones. @@ -650,45 +754,14 @@ public void associateCrosszoneTemplatesToZone(long dcId) { protected Void createTemplateAsyncCallBack(AsyncCallbackDispatcher callback, TemplateOpContext context) { - TemplateInfo template = context.template; TemplateApiResult result = callback.getResult(); if (result.isSuccess()) { - VMTemplateVO tmplt = _templateDao.findById(template.getId()); - // need to grant permission for public templates - if (tmplt.isPublicTemplate()) { - _messageBus.publish(null, TemplateManager.MESSAGE_REGISTER_PUBLIC_TEMPLATE_EVENT, PublishScope.LOCAL, tmplt.getId()); - } - long accountId = tmplt.getAccountId(); - if (template.getSize() != null) { - // publish usage event - String etype = EventTypes.EVENT_TEMPLATE_CREATE; - if (tmplt.getFormat() == ImageFormat.ISO) { - etype = EventTypes.EVENT_ISO_CREATE; - } - // get physical size from template_store_ref table - long physicalSize = 0; - DataStore ds = template.getDataStore(); - TemplateDataStoreVO tmpltStore = _vmTemplateStoreDao.findByStoreTemplate(ds.getId(), template.getId()); - if (tmpltStore != null) { - physicalSize = tmpltStore.getPhysicalSize(); - } else { - logger.warn("No entry found in template_store_ref for template: {} and image store: {} at the end of registering template!", template, ds); - } - Scope dsScope = ds.getScope(); - if (dsScope.getScopeId() != null) { - UsageEventUtils.publishUsageEvent(etype, template.getAccountId(), dsScope.getScopeId(), template.getId(), template.getName(), null, null, - physicalSize, template.getSize(), VirtualMachineTemplate.class.getName(), template.getUuid()); - } else { - logger.warn("Zone scope image store {} has a null scope id", ds); - } - _resourceLimitMgr.incrementResourceCount(accountId, Resource.ResourceType.secondary_storage, template.getSize()); - } + publishTemplateCreation(context.template); } - return null; } - private Map listTemplate(DataStore ssStore) { + protected Map listTemplate(DataStore ssStore) { String nfsVersion = imageStoreDetailsUtil.getNfsVersion(ssStore.getId()); ListTemplateCommand cmd = new ListTemplateCommand(ssStore.getTO(), nfsVersion); EndPoint ep = _epSelector.select(ssStore); diff --git a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java index bc6f37b201a6..276581e2e482 100644 --- a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java +++ b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java @@ -18,9 +18,17 @@ */ package org.apache.cloudstack.storage.image; +import com.cloud.storage.template.TemplateProp; +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; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -33,6 +41,10 @@ import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateVO; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + @RunWith(MockitoJUnitRunner.class) public class TemplateServiceImplTest { @@ -43,6 +55,49 @@ public class TemplateServiceImplTest { @Mock TemplateDataStoreDao templateDataStoreDao; + @Mock + TemplateDataFactoryImpl templateDataFactoryMock; + + @Mock + DataStoreManager dataStoreManagerMock; + + @Mock + VMTemplateVO tmpltMock; + + @Mock + TemplateProp tmpltPropMock; + + @Mock + TemplateObject templateInfoMock; + + @Mock + DataStore sourceStoreMock; + + @Mock + DataStore destStoreMock; + + @Mock + Scope zoneScopeMock; + + @Mock + StorageOrchestrationService storageOrchestrator; + + Map templatesInSourceStore = new HashMap<>(); + + @Before + public void setUp() { + Long zoneId = 1L; + Mockito.doReturn(2L).when(tmpltMock).getId(); + Mockito.doReturn("unique-name").when(tmpltMock).getUniqueName(); + Mockito.doReturn(zoneId).when(zoneScopeMock).getScopeId(); + Mockito.doReturn(zoneScopeMock).when(destStoreMock).getScope(); + 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); + } + @Test public void testIsSkipTemplateStoreDownloadPublicTemplate() { VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class); @@ -81,4 +136,51 @@ public void testIsSkipTemplateStoreDownloadPrivateExistingTemplate() { Mockito.when(templateDataStoreDao.findByTemplateZone(id, id, DataStoreRole.Image)).thenReturn(Mockito.mock(TemplateDataStoreVO.class)); Assert.assertTrue(templateService.isSkipTemplateStoreDownload(templateVO, id)); } + + @Test + public void tryDownloadingTemplateToImageStoreTestDownloadsTemplateWhenUrlIsNotNull() { + Mockito.doReturn("url").when(tmpltMock).getUrl(); + Mockito.doNothing().when(templateService).associateTemplateToZone(Mockito.anyLong(), Mockito.any(Long.class)); + + templateService.tryDownloadingTemplateToImageStore(tmpltMock, destStoreMock); + + Mockito.verify(templateService).createTemplateAsync(Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void tryDownloadingTemplateToImageStoreTestDoesNothingWhenUrlIsNull() { + templateService.tryDownloadingTemplateToImageStore(tmpltMock, destStoreMock); + + Mockito.verify(templateService, Mockito.never()).createTemplateAsync(Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenTemplateDoesNotExistOnAnotherImageStore() { + boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); + + Assert.assertFalse(result); + Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); + } + + @Test + public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenInstallPathIsNull() { + templatesInSourceStore.put(tmpltMock.getUniqueName(), tmpltPropMock); + Mockito.doReturn(null).when(templateInfoMock).getInstallPath(); + + boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); + + Assert.assertFalse(result); + Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyToImageStore(Mockito.any(), 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()); + } } diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 74b7f6f358b5..a9947f087b63 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -4168,7 +4168,8 @@ public ConfigKey[] getConfigKeys() { VmwareAllowParallelExecution, DataStoreDownloadFollowRedirects, AllowVolumeReSizeBeyondAllocation, - StoragePoolHostConnectWorkers + StoragePoolHostConnectWorkers, + COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES }; }