Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
import com.cloud.exception.DiscoveryException;
import com.cloud.storage.ImageStore;
import com.cloud.user.Account;
import org.apache.commons.collections.MapUtils;

import java.util.Collection;
import java.util.HashMap;
import java.util.Map;

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

@Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, description = "Details in key/value pairs using format details[i].keyname=keyvalue. Example: details[0].copytemplatesfromothersecondarystorages=true")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually prefer having parameters instead of passing things as a detail, so that it is automatically added to the API's documentation, but I'm ok if you keep it like this seeing that there's already documentation and a UI change for it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes we do have both doc and UI changes for this. The reason I didnt keep it as parameter is, even though it is not passed, the config value will be used.

protected Map details;

/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
Expand All @@ -56,6 +64,20 @@ public Long getZoneId() {
return zoneId;
}

public Map<String, String> getDetails() {
Map<String, String> detailsMap = new HashMap<>();
if (MapUtils.isNotEmpty(details)) {
Collection<?> props = details.values();
for (Object prop : props) {
HashMap<String, String> detail = (HashMap<String, String>) prop;
for (Map.Entry<String, String> entry: detail.entrySet()) {
detailsMap.put(entry.getKey(),entry.getValue());
}
}
}
return detailsMap;
}

/////////////////////////////////////////////////////
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////
Expand All @@ -68,7 +90,7 @@ public long getEntityOwnerId() {
@Override
public void execute(){
try{
ImageStore result = _storageService.discoverImageStore(null, getUrl(), "NFS", getZoneId(), null);
ImageStore result = _storageService.discoverImageStore(null, getUrl(), "NFS", getZoneId(), getDetails());
ImageStoreResponse storeResponse = null;
if (result != null ) {
storeResponse = _responseGenerator.createImageStoreResponse(result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,6 @@ 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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,9 @@ 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<Boolean> 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.",
ConfigKey<Boolean> COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES = new ConfigKey<>(Boolean.class, "copy.templates.from.other.secondary.storages",
"Storage", "true", "Allow templates to be copied from existing Secondary Storage servers (within the same zone or across zones) " +
"when adding a new Secondary Storage, instead of downloading them from the source URL.",
true, ConfigKey.Scope.Zone, null);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,15 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

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.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;
Expand Down Expand Up @@ -103,6 +111,13 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra
VolumeDataStoreDao volumeDataStoreDao;
@Inject
DataMigrationUtility migrationHelper;
@Inject
TemplateManager templateManager;
@Inject
VMTemplateDao templateDao;
@Inject
DataCenterDao dcDao;


ConfigKey<Double> ImageStoreImbalanceThreshold = new ConfigKey<>("Advanced", Double.class,
"image.store.imbalance.threshold",
Expand Down Expand Up @@ -308,6 +323,14 @@ public Future<TemplateApiResult> orchestrateTemplateCopyToImageStore(TemplateInf
return submit(destStore.getScope().getScopeId(), new CopyTemplateTask(source, destStore));
}

@Override
public Future<TemplateApiResult> orchestrateTemplateCopyAcrossZones(TemplateInfo templateInfo, 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));
}

protected Pair<String, Boolean> migrateCompleted(Long destDatastoreId, DataStore srcDatastore, List<DataObject> files, MigrationPolicy migrationPolicy, int skipped) {
String message = "";
boolean success = true;
Expand Down Expand Up @@ -653,4 +676,47 @@ public TemplateApiResult call() {
return result;
}
}

private class CrossZoneCopyTemplateTask implements Callable<TemplateApiResult> {
private final long userId;
private final TemplateInfo sourceTmpl;
private final DataCenterVO dstZone;
private final String logid;

CrossZoneCopyTemplateTask(long userId, TemplateInfo sourceTmpl, DataCenterVO dstZone) {
this.userId = userId;
this.sourceTmpl = sourceTmpl;
this.dstZone = dstZone;
this.logid = ThreadContext.get(LOGCONTEXTID);
}

@Override
public TemplateApiResult call() {
ThreadContext.put(LOGCONTEXTID, logid);
TemplateApiResult result;
VMTemplateVO template = templateDao.findById(sourceTmpl.getId());
try {
DataStore sourceStore = sourceTmpl.getDataStore();
boolean success = templateManager.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 {
tryCleaningUpExecutor(dstZone.getId());
ThreadContext.clearAll();
}
Comment on lines +714 to +717
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would place these 2 lines outside the finally so that there's less idented code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept it finally to handle any runtime exceptions as well.


return result;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ public void handleTemplateSync(DataStore store) {
}

if (availHypers.contains(tmplt.getHypervisorType())) {
boolean copied = isCopyFromOtherStoragesEnabled(zoneId) && tryCopyingTemplateToImageStore(tmplt, store);
boolean copied = imageStoreDetailsUtil.isCopyTemplatesFromOtherStoragesEnabled(storeId, zoneId) && tryCopyingTemplateToImageStore(tmplt, store);
if (!copied) {
tryDownloadingTemplateToImageStore(tmplt, store);
}
Expand Down Expand Up @@ -615,28 +615,111 @@ protected void tryDownloadingTemplateToImageStore(VMTemplateVO tmplt, DataStore
}

protected boolean tryCopyingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) {
Long zoneId = destStore.getScope().getScopeId();
List<DataStore> storesInZone = _storeMgr.getImageStoresByZoneIds(zoneId);
for (DataStore sourceStore : storesInZone) {
if (searchAndCopyWithinZone(tmplt, destStore)) {
return true;
}

Long destZoneId = destStore.getScope().getScopeId();
logger.debug("Template [{}] not found in any image store of zone [{}]. Checking other zones.",
tmplt.getUniqueName(), destZoneId);

return searchAndCopyAcrossZones(tmplt, destStore, destZoneId);
}

private boolean searchAndCopyAcrossZones(VMTemplateVO tmplt, DataStore destStore, Long destZoneId) {
List<Long> allZoneIds = _dcDao.listAllIds();
for (Long otherZoneId : allZoneIds) {
if (otherZoneId.equals(destZoneId)) {
continue;
}

List<DataStore> storesInOtherZone = _storeMgr.getImageStoresByZoneIds(otherZoneId);
logger.debug("Checking zone [{}] for template [{}]...", otherZoneId, tmplt.getUniqueName());

if (storesInOtherZone == null || storesInOtherZone.isEmpty()) {
logger.debug("Zone [{}] has no image stores. Skipping.", otherZoneId);
continue;
}

TemplateObject sourceTmpl = findUsableTemplate(tmplt, storesInOtherZone);
if (sourceTmpl == null) {
logger.debug("Template [{}] not found with a valid install path in any image store of zone [{}].",
tmplt.getUniqueName(), otherZoneId);
continue;
}

logger.info("Template [{}] found in zone [{}]. Initiating cross-zone copy to zone [{}].",
tmplt.getUniqueName(), otherZoneId, destZoneId);

return copyTemplateAcrossZones(destStore, sourceTmpl);
}

logger.debug("Template [{}] was not found in any zone. Cannot perform zone-to-zone copy.", tmplt.getUniqueName());
return false;
}

protected TemplateObject findUsableTemplate(VMTemplateVO tmplt, List<DataStore> imageStores) {
for (DataStore store : imageStores) {
TemplateObject tmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), store);
if (tmpl == null) {
continue;
}

if (tmpl.getInstallPath() == null) {
logger.debug("Template [{}] found in image store [{}] but install path is null. Skipping.",
tmplt.getUniqueName(), store.getName());
continue;
}
return tmpl;
}
return null;
}

private boolean searchAndCopyWithinZone(VMTemplateVO tmplt, DataStore destStore) {
Long destZoneId = destStore.getScope().getScopeId();
List<DataStore> storesInSameZone = _storeMgr.getImageStoresByZoneIds(destZoneId);
for (DataStore sourceStore : storesInSameZone) {
Map<String, TemplateProp> 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());
if (existingTemplatesInSourceStore == null ||
!existingTemplatesInSourceStore.containsKey(tmplt.getUniqueName())) {
logger.debug("Template [{}] does not exist on image store [{}]; searching another.", 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());
logger.warn("Cannot copy template [{}] from image store [{}]; install path is null.", 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;
}

private boolean copyTemplateAcrossZones(DataStore destStore, TemplateObject sourceTmpl) {
Long dstZoneId = destStore.getScope().getScopeId();
DataCenterVO dstZone = _dcDao.findById(dstZoneId);

if (dstZone == null) {
logger.warn("Destination zone [{}] not found for template [{}].", dstZoneId, sourceTmpl.getUniqueName());
return false;
}

try {
storageOrchestrator.orchestrateTemplateCopyAcrossZones(sourceTmpl, destStore);
return true;
} catch (Exception e) {
logger.error("Failed to copy template [{}] from zone [{}] to zone [{}].",
sourceTmpl.getUniqueName(),
sourceTmpl.getDataStore().getScope().getScopeId(),
dstZoneId,
e);
return false;
}
}

@Override
public AsyncCallFuture<TemplateApiResult> copyTemplateToImageStore(DataObject source, DataStore destStore) {
TemplateObject sourceTmpl = (TemplateObject) source;
Expand Down Expand Up @@ -680,10 +763,6 @@ protected Void copyTemplateToImageStoreCallback(AsyncCallbackDispatcher<Template
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());

Expand Down
Loading
Loading