-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Allow copy of templates from secondary storages of other zone when adding a new secondary storage #12296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.20
Are you sure you want to change the base?
Allow copy of templates from secondary storages of other zone when adding a new secondary storage #12296
Changes from all commits
1c448d3
a6d8bf6
b3232a4
7a2a9d5
1797f46
3556d74
b3916fa
fbbbbd0
d277218
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -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", | ||
|
|
@@ -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; | ||
|
|
@@ -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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kept it finally to handle any runtime exceptions as well.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, that's better in fact. Could you also add the finally to |
||
|
|
||
| return result; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
| } | ||
|
|
@@ -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; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice to keep the check with |
||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private boolean searchAndCopyWithinZone(VMTemplateVO tmplt, DataStore destStore) { | ||
| Long destZoneId = destStore.getScope().getScopeId(); | ||
| List<DataStore> storesInSameZone = _storeMgr.getImageStoresByZoneIds(destZoneId); | ||
| for (DataStore sourceStore : storesInSameZone) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This for loop can be replaced with a call to |
||
| 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; | ||
|
|
@@ -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()); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.