From 5fb5890ea38e0089bf9bb67b68d20bfde3487a0c Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 28 Feb 2018 11:37:09 -0300 Subject: [PATCH] Fix backport issue and retrying logic --- .../DirectTemplateDownloaderImpl.java | 2 +- .../directdownload/DirectDownloadAnswer.java | 8 +++- .../kvm/storage/KVMStorageProcessor.java | 45 ++++++++----------- .../download/DirectDownloadManagerImpl.java | 9 +++- utils/src/com/cloud/utils/UriUtils.java | 11 ++++- 5 files changed, 42 insertions(+), 33 deletions(-) diff --git a/agent/src/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java b/agent/src/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java index b0bf1848f5a..5f3c843d0c9 100644 --- a/agent/src/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java +++ b/agent/src/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java @@ -195,9 +195,9 @@ public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDown boolean valid = false; try { while (!valid && retry > 0) { + retry--; s_logger.info("Performing checksum validation for downloaded template " + templateId + ", retries left: " + retry); valid = DigestHelper.check(checksum, new FileInputStream(downloadedFilePath)); - retry--; if (!valid && retry > 0) { s_logger.info("Checksum validation failded, re-downloading template"); redownload = true; diff --git a/core/src/org/apache/cloudstack/agent/directdownload/DirectDownloadAnswer.java b/core/src/org/apache/cloudstack/agent/directdownload/DirectDownloadAnswer.java index 28b4dc7ac1c..0090173eb0c 100644 --- a/core/src/org/apache/cloudstack/agent/directdownload/DirectDownloadAnswer.java +++ b/core/src/org/apache/cloudstack/agent/directdownload/DirectDownloadAnswer.java @@ -25,11 +25,13 @@ public class DirectDownloadAnswer extends Answer { private Long templateSize; private String installPath; + private boolean retryOnOtherHosts; - public DirectDownloadAnswer(final boolean result, final String msg) { + public DirectDownloadAnswer(final boolean result, final String msg, final boolean retry) { super(null); this.result = result; this.details = msg; + this.retryOnOtherHosts = retry; } public DirectDownloadAnswer(final boolean result, final Long size, final String installPath) { @@ -46,4 +48,8 @@ public class DirectDownloadAnswer extends Answer { public String getInstallPath() { return installPath; } + + public boolean isRetryOnOtherHosts() { + return retryOnOtherHosts; + } } \ No newline at end of file diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 49c121b34cc..f8272c2bcdc 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1322,7 +1322,7 @@ public class KVMStorageProcessor implements StorageProcessor { public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) { final PrimaryDataStoreTO pool = cmd.getDestPool(); if (!pool.getPoolType().equals(StoragePoolType.NetworkFilesystem)) { - return new DirectDownloadAnswer(false, "Unsupported pool type " + pool.getPoolType().toString()); + return new DirectDownloadAnswer(false, "Unsupported pool type " + pool.getPoolType().toString(), true); } KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); DirectTemplateDownloader downloader; @@ -1330,35 +1330,26 @@ public class KVMStorageProcessor implements StorageProcessor { try { downloader = getDirectTemplateDownloaderFromCommand(cmd, destPool); } catch (IllegalArgumentException e) { - return new DirectDownloadAnswer(false, "Unable to create direct downloader: " + e.getMessage()); + return new DirectDownloadAnswer(false, "Unable to create direct downloader: " + e.getMessage(), true); } - int retries = 3; - boolean ok = false; - do { - try { - retries--; - s_logger.info("Trying to download template, retries left: " + retries); - if (!downloader.downloadTemplate()) { - s_logger.warn("Couldn't download template"); - continue; - } - if (!downloader.validateChecksum()) { - s_logger.warn("Couldn't validate template checksum"); - continue; - } - if (!downloader.extractAndInstallDownloadedTemplate()) { - s_logger.warn("Couldn't extract and install template"); - continue; - } - ok = true; - } catch (CloudRuntimeException e) { - s_logger.warn("Error downloading template " + cmd.getTemplateId() + " due to: " + e.getMessage() + " retries left: " + retries); + try { + s_logger.info("Trying to download template"); + if (!downloader.downloadTemplate()) { + s_logger.warn("Couldn't download template"); + return new DirectDownloadAnswer(false, "Unable to download template", true); } - } while (!ok && retries > 0); - - if (!ok) { - return new DirectDownloadAnswer(false, "Unable to download template " + cmd.getTemplateId()); + if (!downloader.validateChecksum()) { + s_logger.warn("Couldn't validate template checksum"); + return new DirectDownloadAnswer(false, "Checksum validation failed", false); + } + if (!downloader.extractAndInstallDownloadedTemplate()) { + s_logger.warn("Couldn't extract and install template"); + return new DirectDownloadAnswer(false, "Extraction and installation failed", false); + } + } catch (CloudRuntimeException e) { + s_logger.warn("Error downloading template " + cmd.getTemplateId() + " due to: " + e.getMessage()); + return new DirectDownloadAnswer(false, "Unable to download template: " + e.getMessage(), true); } DirectTemplateInformation info = downloader.getTemplateInformation(); diff --git a/server/src/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index 83d0fcc528e..fa337720277 100644 --- a/server/src/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -255,10 +255,15 @@ public class DirectDownloadManagerImpl extends ManagerBase implements DirectDown int hostIndex = 0; Answer answer = null; Long hostToSendDownloadCmd = hostsToRetry[hostIndex]; - while (!downloaded && retry > 0) { + boolean continueRetrying = true; + while (!downloaded && retry > 0 && continueRetrying) { s_logger.debug("Sending Direct download command to host " + hostToSendDownloadCmd); answer = agentManager.easySend(hostToSendDownloadCmd, cmd); - downloaded = answer != null && answer.getResult(); + if (answer != null) { + DirectDownloadAnswer ans = (DirectDownloadAnswer)answer; + downloaded = answer.getResult(); + continueRetrying = ans.isRetryOnOtherHosts(); + } hostToSendDownloadCmd = hostsToRetry[(hostIndex + 1) % hostsToRetry.length]; retry --; } diff --git a/utils/src/com/cloud/utils/UriUtils.java b/utils/src/com/cloud/utils/UriUtils.java index 998a0872a49..7bd4234e310 100644 --- a/utils/src/com/cloud/utils/UriUtils.java +++ b/utils/src/com/cloud/utils/UriUtils.java @@ -439,9 +439,16 @@ public class UriUtils { HttpClient httpClient = getHttpClient(); GetMethod getMethod = new GetMethod(metalinkUrl); List urls = new ArrayList<>(); + int status; try { - if (httpClient.executeMethod(getMethod) == HttpStatus.SC_OK) { - InputStream is = getMethod.getResponseBodyAsStream(); + status = httpClient.executeMethod(getMethod); + } catch (IOException e) { + s_logger.error("Error retrieving urls form metalink: " + metalinkUrl); + return null; + } + try { + InputStream is = getMethod.getResponseBodyAsStream(); + if (status == HttpStatus.SC_OK) { Map> metalinkUrlsMap = getMultipleValuesFromXML(is, new String[] {"url"}); if (metalinkUrlsMap.containsKey("url")) { List metalinkUrls = metalinkUrlsMap.get("url");