Skip to content

Commit 8162736

Browse files
harikrishna-patnalaLocharla, Sandeep
harikrishna-patnala
authored and
Locharla, Sandeep
committed
Allow copy of templates from secondary storages of other zone when adding a new secondary storage (apache#12296)
* Allow copy of templates from secondary storages of other zone when adding a new secondary storage * Add API param and UI changes on add secondary storage page * Make copy template across zones non blocking * Code fixes * unused imports * Add copy template flag in zone wizard and remove NFS checks * Fix UI * Label fixes * code optimizations * code refactoring * missing changes * Combine template copy and download into a single asynchronous operation * unused import and fixed conflicts * unused code * update config message * Fix configuration setting value on add secondary storage page * Removed unused code * Update unit tests
1 parent 46d518b commit 8162736

File tree

9 files changed

+394
-51
lines changed

9 files changed

+394
-51
lines changed

‎api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddSecondaryStorageCmd.java‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@
2929
import com.cloud.exception.DiscoveryException;
3030
import com.cloud.storage.ImageStore;
3131
import com.cloud.user.Account;
32+
import org.apache.commons.collections.MapUtils;
33+
34+
import java.util.Collection;
35+
import java.util.HashMap;
36+
import java.util.Map;
3237

3338
@APICommand(name = "addSecondaryStorage", description = "Adds secondary storage.", responseObject = ImageStoreResponse.class,
3439
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
@@ -44,6 +49,9 @@ public class AddSecondaryStorageCmd extends BaseCmd {
4449
@Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, description = "The Zone ID for the secondary storage")
4550
protected Long zoneId;
4651

52+
@Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, description = "Details in key/value pairs using format details[i].keyname=keyvalue. Example: details[0].copytemplatesfromothersecondarystorages=true")
53+
protected Map details;
54+
4755
/////////////////////////////////////////////////////
4856
/////////////////// Accessors ///////////////////////
4957
/////////////////////////////////////////////////////
@@ -56,6 +64,20 @@ public Long getZoneId() {
5664
return zoneId;
5765
}
5866

67+
public Map<String, String> getDetails() {
68+
Map<String, String> detailsMap = new HashMap<>();
69+
if (MapUtils.isNotEmpty(details)) {
70+
Collection<?> props = details.values();
71+
for (Object prop : props) {
72+
HashMap<String, String> detail = (HashMap<String, String>) prop;
73+
for (Map.Entry<String, String> entry: detail.entrySet()) {
74+
detailsMap.put(entry.getKey(),entry.getValue());
75+
}
76+
}
77+
}
78+
return detailsMap;
79+
}
80+
5981
/////////////////////////////////////////////////////
6082
/////////////// API Implementation///////////////////
6183
/////////////////////////////////////////////////////
@@ -68,7 +90,7 @@ public long getEntityOwnerId() {
6890
@Override
6991
public void execute(){
7092
try{
71-
ImageStore result = _storageService.discoverImageStore(null, getUrl(), "NFS", getZoneId(), null);
93+
ImageStore result = _storageService.discoverImageStore(null, getUrl(), "NFS", getZoneId(), getDetails());
7294
ImageStoreResponse storeResponse = null;
7395
if (result != null ) {
7496
storeResponse = _responseGenerator.createImageStoreResponse(result);

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
import org.apache.cloudstack.api.response.MigrationResponse;
2424
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
25-
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
2625
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult;
2726
import org.apache.cloudstack.storage.ImageStoreService.MigrationPolicy;
2827

@@ -31,5 +30,5 @@ public interface StorageOrchestrationService {
3130

3231
MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreId, List<Long> templateIdList, List<Long> snapshotIdList);
3332

34-
Future<TemplateApiResult> orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore);
33+
Future<TemplateApiResult> orchestrateTemplateCopyFromSecondaryStores(long templateId, DataStore destStore);
3534
}

‎engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,6 @@ public TemplateInfo getTemplate() {
8080
List<DatadiskTO> getTemplateDatadisksOnImageStore(TemplateInfo templateInfo, String configurationId);
8181

8282
AsyncCallFuture<TemplateApiResult> copyTemplateToImageStore(DataObject source, DataStore destStore);
83-
}
83+
84+
void handleTemplateCopyFromSecondaryStores(long templateId, DataStore destStore);
85+
}

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

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@
3636
import javax.inject.Inject;
3737
import javax.naming.ConfigurationException;
3838

39+
import com.cloud.dc.dao.DataCenterDao;
40+
import com.cloud.storage.dao.VMTemplateDao;
41+
import com.cloud.template.TemplateManager;
3942
import org.apache.cloudstack.api.response.MigrationResponse;
4043
import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
4144
import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
@@ -45,6 +48,7 @@
4548
import org.apache.cloudstack.engine.subsystem.api.storage.SecondaryStorageService.DataObjectResult;
4649
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
4750
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
51+
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory;
4852
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
4953
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService;
5054
import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult;
@@ -103,6 +107,15 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra
103107
VolumeDataStoreDao volumeDataStoreDao;
104108
@Inject
105109
DataMigrationUtility migrationHelper;
110+
@Inject
111+
TemplateManager templateManager;
112+
@Inject
113+
VMTemplateDao templateDao;
114+
@Inject
115+
TemplateDataFactory templateDataFactory;
116+
@Inject
117+
DataCenterDao dcDao;
118+
106119

107120
ConfigKey<Double> ImageStoreImbalanceThreshold = new ConfigKey<>("Advanced", Double.class,
108121
"image.store.imbalance.threshold",
@@ -304,8 +317,9 @@ public MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreI
304317
}
305318

306319
@Override
307-
public Future<TemplateApiResult> orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore) {
308-
return submit(destStore.getScope().getScopeId(), new CopyTemplateTask(source, destStore));
320+
public Future<TemplateApiResult> orchestrateTemplateCopyFromSecondaryStores(long srcTemplateId, DataStore destStore) {
321+
Long dstZoneId = destStore.getScope().getScopeId();
322+
return submit(dstZoneId, new CopyTemplateFromSecondaryStorageTask(srcTemplateId, destStore));
309323
}
310324

311325
protected Pair<String, Boolean> migrateCompleted(Long destDatastoreId, DataStore srcDatastore, List<DataObject> files, MigrationPolicy migrationPolicy, int skipped) {
@@ -624,13 +638,13 @@ public DataObjectResult call() {
624638
}
625639
}
626640

627-
private class CopyTemplateTask implements Callable<TemplateApiResult> {
628-
private TemplateInfo sourceTmpl;
629-
private DataStore destStore;
630-
private String logid;
641+
private class CopyTemplateFromSecondaryStorageTask implements Callable<TemplateApiResult> {
642+
private final long srcTemplateId;
643+
private final DataStore destStore;
644+
private final String logid;
631645

632-
public CopyTemplateTask(TemplateInfo sourceTmpl, DataStore destStore) {
633-
this.sourceTmpl = sourceTmpl;
646+
CopyTemplateFromSecondaryStorageTask(long srcTemplateId, DataStore destStore) {
647+
this.srcTemplateId = srcTemplateId;
634648
this.destStore = destStore;
635649
this.logid = ThreadContext.get(LOGCONTEXTID);
636650
}
@@ -639,17 +653,16 @@ public CopyTemplateTask(TemplateInfo sourceTmpl, DataStore destStore) {
639653
public TemplateApiResult call() {
640654
ThreadContext.put(LOGCONTEXTID, logid);
641655
TemplateApiResult result;
642-
AsyncCallFuture<TemplateApiResult> future = templateService.copyTemplateToImageStore(sourceTmpl, destStore);
656+
long destZoneId = destStore.getScope().getScopeId();
657+
TemplateInfo sourceTmpl = templateDataFactory.getTemplate(srcTemplateId, DataStoreRole.Image);
643658
try {
644-
result = future.get();
645-
} catch (ExecutionException | InterruptedException e) {
646-
logger.warn("Exception while copying template [{}] from image store [{}] to image store [{}]: {}",
647-
sourceTmpl.getUniqueName(), sourceTmpl.getDataStore().getName(), destStore.getName(), e.toString());
659+
templateService.handleTemplateCopyFromSecondaryStores(srcTemplateId, destStore);
648660
result = new TemplateApiResult(sourceTmpl);
649-
result.setResult(e.getMessage());
661+
} finally {
662+
tryCleaningUpExecutor(destZoneId);
663+
ThreadContext.clearAll();
650664
}
651-
tryCleaningUpExecutor(destStore.getScope().getScopeId());
652-
ThreadContext.clearAll();
665+
653666
return result;
654667
}
655668
}

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

Lines changed: 135 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131

3232
import javax.inject.Inject;
3333

34+
import com.cloud.exception.StorageUnavailableException;
35+
import org.apache.cloudstack.context.CallContext;
3436
import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
3537
import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
3638
import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
@@ -67,9 +69,11 @@
6769
import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
6870
import org.apache.cloudstack.storage.image.store.TemplateObject;
6971
import org.apache.cloudstack.storage.to.TemplateObjectTO;
72+
import org.apache.commons.collections.CollectionUtils;
7073
import org.apache.commons.lang3.StringUtils;
7174
import org.apache.logging.log4j.Logger;
7275
import org.apache.logging.log4j.LogManager;
76+
import org.apache.logging.log4j.ThreadContext;
7377
import org.springframework.stereotype.Component;
7478

7579
import com.cloud.agent.api.Answer;
@@ -569,10 +573,7 @@ public void handleTemplateSync(DataStore store) {
569573
}
570574

571575
if (availHypers.contains(tmplt.getHypervisorType())) {
572-
boolean copied = isCopyFromOtherStoragesEnabled(zoneId) && tryCopyingTemplateToImageStore(tmplt, store);
573-
if (!copied) {
574-
tryDownloadingTemplateToImageStore(tmplt, store);
575-
}
576+
storageOrchestrator.orchestrateTemplateCopyFromSecondaryStores(tmplt.getId(), store);
576577
} else {
577578
logger.info("Skip downloading template {} since current data center does not have hypervisor {}", tmplt, tmplt.getHypervisorType());
578579
}
@@ -619,6 +620,16 @@ public void handleTemplateSync(DataStore store) {
619620

620621
}
621622

623+
@Override
624+
public void handleTemplateCopyFromSecondaryStores(long templateId, DataStore destStore) {
625+
VMTemplateVO template = _templateDao.findById(templateId);
626+
long zoneId = destStore.getScope().getScopeId();
627+
boolean copied = imageStoreDetailsUtil.isCopyTemplatesFromOtherStoragesEnabled(destStore.getId(), zoneId) && tryCopyingTemplateToImageStore(template, destStore);
628+
if (!copied) {
629+
tryDownloadingTemplateToImageStore(template, destStore);
630+
}
631+
}
632+
622633
protected void tryDownloadingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) {
623634
if (tmplt.getUrl() == null) {
624635
logger.info("Not downloading template [{}] to image store [{}], as it has no URL.", tmplt.getUniqueName(),
@@ -636,28 +647,134 @@ protected void tryDownloadingTemplateToImageStore(VMTemplateVO tmplt, DataStore
636647
}
637648

638649
protected boolean tryCopyingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) {
639-
Long zoneId = destStore.getScope().getScopeId();
640-
List<DataStore> storesInZone = _storeMgr.getImageStoresByZoneIds(zoneId);
641-
for (DataStore sourceStore : storesInZone) {
642-
Map<String, TemplateProp> existingTemplatesInSourceStore = listTemplate(sourceStore);
643-
if (existingTemplatesInSourceStore == null || !existingTemplatesInSourceStore.containsKey(tmplt.getUniqueName())) {
644-
logger.debug("Template [{}] does not exist on image store [{}]; searching on another one.",
645-
tmplt.getUniqueName(), sourceStore.getName());
650+
if (searchAndCopyWithinZone(tmplt, destStore)) {
651+
return true;
652+
}
653+
654+
Long destZoneId = destStore.getScope().getScopeId();
655+
logger.debug("Template [{}] not found in any image store of zone [{}]. Checking other zones.",
656+
tmplt.getUniqueName(), destZoneId);
657+
658+
return searchAndCopyAcrossZones(tmplt, destStore, destZoneId);
659+
}
660+
661+
private boolean searchAndCopyAcrossZones(VMTemplateVO tmplt, DataStore destStore, Long destZoneId) {
662+
List<Long> allZoneIds = _dcDao.listAllIds();
663+
for (Long otherZoneId : allZoneIds) {
664+
if (otherZoneId.equals(destZoneId)) {
646665
continue;
647666
}
648-
TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore);
649-
if (sourceTmpl.getInstallPath() == null) {
650-
logger.warn("Can not copy template [{}] from image store [{}], as it returned a null install path.", tmplt.getUniqueName(),
651-
sourceStore.getName());
667+
668+
List<DataStore> storesInOtherZone = _storeMgr.getImageStoresByZoneIds(otherZoneId);
669+
logger.debug("Checking zone [{}] for template [{}]...", otherZoneId, tmplt.getUniqueName());
670+
671+
if (CollectionUtils.isEmpty(storesInOtherZone)) {
672+
logger.debug("Zone [{}] has no image stores. Skipping.", otherZoneId);
652673
continue;
653674
}
654-
storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore);
655-
return true;
675+
676+
TemplateObject sourceTmpl = findUsableTemplate(tmplt, storesInOtherZone);
677+
if (sourceTmpl == null) {
678+
logger.debug("Template [{}] not found with a valid install path in any image store of zone [{}].",
679+
tmplt.getUniqueName(), otherZoneId);
680+
continue;
681+
}
682+
683+
logger.info("Template [{}] found in zone [{}]. Initiating cross-zone copy to zone [{}].",
684+
tmplt.getUniqueName(), otherZoneId, destZoneId);
685+
686+
return copyTemplateAcrossZones(destStore, sourceTmpl);
656687
}
657-
logger.debug("Can't copy template [{}] from another image store.", tmplt.getUniqueName());
688+
689+
logger.debug("Template [{}] was not found in any zone. Cannot perform zone-to-zone copy.", tmplt.getUniqueName());
658690
return false;
659691
}
660692

693+
protected TemplateObject findUsableTemplate(VMTemplateVO tmplt, List<DataStore> imageStores) {
694+
for (DataStore store : imageStores) {
695+
696+
Map<String, TemplateProp> templates = listTemplate(store);
697+
if (templates == null || !templates.containsKey(tmplt.getUniqueName())) {
698+
continue;
699+
}
700+
701+
TemplateObject tmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), store);
702+
if (tmpl.getInstallPath() == null) {
703+
logger.debug("Template [{}] found in image store [{}] but install path is null. Skipping.",
704+
tmplt.getUniqueName(), store.getName());
705+
continue;
706+
}
707+
return tmpl;
708+
}
709+
return null;
710+
}
711+
712+
private boolean searchAndCopyWithinZone(VMTemplateVO tmplt, DataStore destStore) {
713+
Long destZoneId = destStore.getScope().getScopeId();
714+
List<DataStore> storesInSameZone = _storeMgr.getImageStoresByZoneIds(destZoneId);
715+
716+
TemplateObject sourceTmpl = findUsableTemplate(tmplt, storesInSameZone);
717+
if (sourceTmpl == null) {
718+
return false;
719+
}
720+
721+
TemplateApiResult result;
722+
AsyncCallFuture<TemplateApiResult> future = copyTemplateToImageStore(sourceTmpl, destStore);
723+
try {
724+
result = future.get();
725+
} catch (ExecutionException | InterruptedException e) {
726+
logger.warn("Exception while copying template [{}] from image store [{}] to image store [{}]: {}",
727+
sourceTmpl.getUniqueName(), sourceTmpl.getDataStore().getName(), destStore.getName(), e.toString());
728+
result = new TemplateApiResult(sourceTmpl);
729+
result.setResult(e.getMessage());
730+
}
731+
return result.isSuccess();
732+
}
733+
734+
private boolean copyTemplateAcrossZones(DataStore destStore, TemplateObject sourceTmpl) {
735+
Long dstZoneId = destStore.getScope().getScopeId();
736+
DataCenterVO dstZone = _dcDao.findById(dstZoneId);
737+
738+
if (dstZone == null) {
739+
logger.warn("Destination zone [{}] not found for template [{}].", dstZoneId, sourceTmpl.getUniqueName());
740+
return false;
741+
}
742+
743+
TemplateApiResult result;
744+
try {
745+
VMTemplateVO template = _templateDao.findById(sourceTmpl.getId());
746+
try {
747+
DataStore sourceStore = sourceTmpl.getDataStore();
748+
long userId = CallContext.current().getCallingUserId();
749+
boolean success = _tmpltMgr.copy(userId, template, sourceStore, dstZone);
750+
751+
result = new TemplateApiResult(sourceTmpl);
752+
if (!success) {
753+
result.setResult("Cross-zone template copy failed");
754+
}
755+
} catch (StorageUnavailableException | ResourceAllocationException e) {
756+
logger.error("Exception while copying template [{}] from zone [{}] to zone [{}]",
757+
template,
758+
sourceTmpl.getDataStore().getScope().getScopeId(),
759+
dstZone.getId(),
760+
e);
761+
result = new TemplateApiResult(sourceTmpl);
762+
result.setResult(e.getMessage());
763+
} finally {
764+
ThreadContext.clearAll();
765+
}
766+
} catch (Exception e) {
767+
logger.error("Failed to copy template [{}] from zone [{}] to zone [{}].",
768+
sourceTmpl.getUniqueName(),
769+
sourceTmpl.getDataStore().getScope().getScopeId(),
770+
dstZoneId,
771+
e);
772+
return false;
773+
}
774+
775+
return result.isSuccess();
776+
}
777+
661778
@Override
662779
public AsyncCallFuture<TemplateApiResult> copyTemplateToImageStore(DataObject source, DataStore destStore) {
663780
TemplateObject sourceTmpl = (TemplateObject) source;
@@ -701,10 +818,6 @@ protected Void copyTemplateToImageStoreCallback(AsyncCallbackDispatcher<Template
701818
return null;
702819
}
703820

704-
protected boolean isCopyFromOtherStoragesEnabled(Long zoneId) {
705-
return StorageManager.COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES.valueIn(zoneId);
706-
}
707-
708821
protected void publishTemplateCreation(TemplateInfo tmplt) {
709822
VMTemplateVO tmpltVo = _templateDao.findById(tmplt.getId());
710823

0 commit comments

Comments
 (0)