Skip to content

Commit 6a75423

Browse files
nvazquezyadvr
nvazquez
authored and
yadvr
committed
CLOUDSTACK-10231: Asserted fixes for Direct Download on KVM (#2408)
Several fixes addressed: - Dettach ISO fails when trying to detach a direct download ISO - Fix for metalink support on SSVM agents (this closes CLOUDSTACK-10238) - Reinstall VM from bypassed registered template (this closes CLOUDSTACK-10250) - Fix upload certificate error message even though operation was successful - Fix metalink download, checksum retry logic and metalink SSVM downloader
1 parent 1ad04cb commit 6a75423

18 files changed

Lines changed: 631 additions & 168 deletions

File tree

‎agent/src/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.cloud.utils.script.Script;
2323
import org.apache.cloudstack.utils.security.DigestHelper;
2424
import org.apache.commons.lang.StringUtils;
25+
import org.apache.log4j.Logger;
2526

2627
import java.io.File;
2728
import java.io.FileInputStream;
@@ -37,6 +38,8 @@ public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDown
3738
private String downloadedFilePath;
3839
private String installPath;
3940
private String checksum;
41+
private boolean redownload = false;
42+
public static final Logger s_logger = Logger.getLogger(DirectTemplateDownloaderImpl.class.getName());
4043

4144
protected DirectTemplateDownloaderImpl(final String url, final String destPoolPath, final Long templateId, final String checksum) {
4245
this.url = url;
@@ -70,6 +73,10 @@ public String getUrl() {
7073
return url;
7174
}
7275

76+
public void setUrl(String url) {
77+
this.url = url;
78+
}
79+
7380
public String getDestPoolPath() {
7481
return destPoolPath;
7582
}
@@ -86,6 +93,18 @@ public void setDownloadedFilePath(String filePath) {
8693
this.downloadedFilePath = filePath;
8794
}
8895

96+
public String getChecksum() {
97+
return checksum;
98+
}
99+
100+
public void setChecksum(String checksum) {
101+
this.checksum = checksum;
102+
}
103+
104+
public boolean isRedownload() {
105+
return redownload;
106+
}
107+
89108
/**
90109
* Return filename from url
91110
*/
@@ -155,14 +174,47 @@ public DirectTemplateInformation getTemplateInformation() {
155174
@Override
156175
public boolean validateChecksum() {
157176
if (StringUtils.isNotBlank(checksum)) {
177+
int retry = 3;
178+
boolean valid = false;
158179
try {
159-
return DigestHelper.check(checksum, new FileInputStream(downloadedFilePath));
180+
while (!valid && retry > 0) {
181+
retry--;
182+
s_logger.info("Performing checksum validation for downloaded template " + templateId + " using " + checksum + ", retries left: " + retry);
183+
valid = DigestHelper.check(checksum, new FileInputStream(downloadedFilePath));
184+
if (!valid && retry > 0) {
185+
s_logger.info("Checksum validation failded, re-downloading template");
186+
redownload = true;
187+
resetDownloadFile();
188+
downloadTemplate();
189+
}
190+
}
191+
s_logger.info("Checksum validation for template " + templateId + ": " + (valid ? "succeeded" : "failed"));
192+
return valid;
160193
} catch (IOException e) {
161-
throw new CloudRuntimeException("could not check sum for file: " + downloadedFilePath,e);
194+
throw new CloudRuntimeException("could not check sum for file: " + downloadedFilePath, e);
162195
} catch (NoSuchAlgorithmException e) {
163196
throw new CloudRuntimeException("Unknown checksum algorithm: " + checksum, e);
164197
}
165198
}
199+
s_logger.info("No checksum provided, skipping checksum validation");
166200
return true;
167201
}
202+
203+
/**
204+
* Delete and create download file
205+
*/
206+
private void resetDownloadFile() {
207+
File f = new File(getDownloadedFilePath());
208+
s_logger.info("Resetting download file: " + getDownloadedFilePath() + ", in order to re-download and persist template " + templateId + " on it");
209+
try {
210+
if (f.exists()) {
211+
f.delete();
212+
}
213+
f.createNewFile();
214+
} catch (IOException e) {
215+
s_logger.error("Error creating file to download on: " + getDownloadedFilePath() + " due to: " + e.getMessage());
216+
throw new CloudRuntimeException("Failed to create download file for direct download");
217+
}
218+
}
219+
168220
}

‎agent/src/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java‎

Lines changed: 16 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,9 @@
2222
import com.cloud.utils.exception.CloudRuntimeException;
2323
import org.apache.commons.collections.MapUtils;
2424
import org.apache.commons.httpclient.HttpClient;
25-
import org.apache.commons.httpclient.HttpMethod;
26-
import org.apache.commons.httpclient.HttpMethodRetryHandler;
2725
import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager;
28-
import org.apache.commons.httpclient.NoHttpResponseException;
26+
import org.apache.commons.httpclient.HttpStatus;
2927
import org.apache.commons.httpclient.methods.GetMethod;
30-
import org.apache.commons.httpclient.params.HttpMethodParams;
3128
import org.apache.commons.io.IOUtils;
3229
import org.apache.log4j.Logger;
3330

@@ -36,21 +33,22 @@
3633
import java.io.InputStream;
3734
import java.io.OutputStream;
3835
import java.io.IOException;
36+
import java.util.HashMap;
3937
import java.util.Map;
4038

4139
public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl {
4240

43-
private HttpClient client;
41+
protected HttpClient client;
4442
private static final MultiThreadedHttpConnectionManager s_httpClientManager = new MultiThreadedHttpConnectionManager();
45-
private static final int CHUNK_SIZE = 1024 * 1024; //1M
46-
protected HttpMethodRetryHandler myretryhandler;
4743
public static final Logger s_logger = Logger.getLogger(HttpDirectTemplateDownloader.class.getName());
4844
protected GetMethod request;
45+
protected Map<String, String> reqHeaders = new HashMap<>();
4946

5047
public HttpDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map<String, String> headers) {
5148
super(url, destPoolPath, templateId, checksum);
49+
s_httpClientManager.getParams().setConnectionTimeout(5000);
50+
s_httpClientManager.getParams().setSoTimeout(5000);
5251
client = new HttpClient(s_httpClientManager);
53-
myretryhandler = createRetryTwiceHandler();
5452
request = createRequest(url, headers);
5553
String downloadDir = getDirectDownloadTempPath(templateId);
5654
createTemporaryDirectoryAndFile(downloadDir);
@@ -64,50 +62,34 @@ protected void createTemporaryDirectoryAndFile(String downloadDir) {
6462

6563
protected GetMethod createRequest(String downloadUrl, Map<String, String> headers) {
6664
GetMethod request = new GetMethod(downloadUrl);
67-
request.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, myretryhandler);
6865
request.setFollowRedirects(true);
6966
if (MapUtils.isNotEmpty(headers)) {
7067
for (String key : headers.keySet()) {
7168
request.setRequestHeader(key, headers.get(key));
69+
reqHeaders.put(key, headers.get(key));
7270
}
7371
}
7472
return request;
7573
}
7674

77-
protected HttpMethodRetryHandler createRetryTwiceHandler() {
78-
return new HttpMethodRetryHandler() {
79-
@Override
80-
public boolean retryMethod(final HttpMethod method, final IOException exception, int executionCount) {
81-
if (executionCount >= 2) {
82-
// Do not retry if over max retry count
83-
return false;
84-
}
85-
if (exception instanceof NoHttpResponseException) {
86-
// Retry if the server dropped connection on us
87-
return true;
88-
}
89-
if (!method.isRequestSent()) {
90-
// Retry if the request has not been sent fully or
91-
// if it's OK to retry methods that have been sent
92-
return true;
93-
}
94-
// otherwise do not retry
95-
return false;
96-
}
97-
};
98-
}
99-
10075
@Override
10176
public boolean downloadTemplate() {
10277
try {
103-
client.executeMethod(request);
78+
int status = client.executeMethod(request);
79+
if (status != HttpStatus.SC_OK) {
80+
s_logger.warn("Not able to download template, status code: " + status);
81+
return false;
82+
}
83+
return performDownload();
10484
} catch (IOException e) {
10585
throw new CloudRuntimeException("Error on HTTP request: " + e.getMessage());
86+
} finally {
87+
request.releaseConnection();
10688
}
107-
return performDownload();
10889
}
10990

11091
protected boolean performDownload() {
92+
s_logger.info("Downloading template " + getTemplateId() + " from " + getUrl() + " to: " + getDownloadedFilePath());
11193
try (
11294
InputStream in = request.getResponseBodyAsStream();
11395
OutputStream out = new FileOutputStream(getDownloadedFilePath());

‎agent/src/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import org.apache.commons.io.IOUtils;
2525
import org.apache.http.HttpEntity;
2626
import org.apache.http.client.methods.CloseableHttpResponse;
27+
import org.apache.commons.collections.MapUtils;
28+
import org.apache.http.client.config.RequestConfig;
2729
import org.apache.http.client.methods.HttpGet;
2830
import org.apache.http.client.methods.HttpUriRequest;
2931
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
@@ -44,27 +46,37 @@
4446
import java.security.KeyStoreException;
4547
import java.security.NoSuchAlgorithmException;
4648
import java.security.cert.CertificateException;
49+
import java.util.Map;
4750

4851
public class HttpsDirectTemplateDownloader extends HttpDirectTemplateDownloader {
4952

5053
private CloseableHttpClient httpsClient;
5154
private HttpUriRequest req;
5255

53-
public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum) {
54-
super(url, templateId, destPoolPath, checksum, null);
56+
public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map<String, String> headers) {
57+
super(url, templateId, destPoolPath, checksum, headers);
5558
SSLContext sslcontext = null;
5659
try {
5760
sslcontext = getSSLContext();
5861
} catch (KeyStoreException | NoSuchAlgorithmException | CertificateException | IOException | KeyManagementException e) {
5962
throw new CloudRuntimeException("Failure getting SSL context for HTTPS downloader: " + e.getMessage());
6063
}
6164
SSLConnectionSocketFactory factory = new SSLConnectionSocketFactory(sslcontext, SSLConnectionSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER);
62-
httpsClient = HttpClients.custom().setSSLSocketFactory(factory).build();
63-
req = createUriRequest(url);
65+
RequestConfig config = RequestConfig.custom()
66+
.setConnectTimeout(5000)
67+
.setConnectionRequestTimeout(5000)
68+
.setSocketTimeout(5000).build();
69+
httpsClient = HttpClients.custom().setSSLSocketFactory(factory).setDefaultRequestConfig(config).build();
70+
createUriRequest(url, headers);
6471
}
6572

66-
protected HttpUriRequest createUriRequest(String downloadUrl) {
67-
return new HttpGet(downloadUrl);
73+
protected void createUriRequest(String downloadUrl, Map<String, String> headers) {
74+
req = new HttpGet(downloadUrl);
75+
if (MapUtils.isNotEmpty(headers)) {
76+
for (String headerKey: headers.keySet()) {
77+
req.setHeader(headerKey, headers.get(headerKey));
78+
}
79+
}
6880
}
6981

7082
private SSLContext getSSLContext() throws KeyStoreException, NoSuchAlgorithmException, CertificateException, IOException, KeyManagementException {
@@ -98,6 +110,7 @@ public boolean downloadTemplate() {
98110
* Consume response and persist it on getDownloadedFilePath() file
99111
*/
100112
protected boolean consumeResponse(CloseableHttpResponse response) {
113+
s_logger.info("Downloading template " + getTemplateId() + " from " + getUrl() + " to: " + getDownloadedFilePath());
101114
if (response.getStatusLine().getStatusCode() != 200) {
102115
throw new CloudRuntimeException("Error on HTTPS response");
103116
}
@@ -113,4 +126,4 @@ protected boolean consumeResponse(CloseableHttpResponse response) {
113126
return true;
114127
}
115128

116-
}
129+
}

‎agent/src/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java‎

Lines changed: 66 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,32 +18,81 @@
1818
//
1919
package com.cloud.agent.direct.download;
2020

21-
import com.cloud.utils.script.Script;
21+
import com.cloud.utils.UriUtils;
22+
import com.cloud.utils.exception.CloudRuntimeException;
23+
import org.apache.commons.collections.CollectionUtils;
24+
import org.apache.commons.lang.StringUtils;
25+
import org.apache.log4j.Logger;
2226

2327
import java.io.File;
28+
import java.util.List;
29+
import java.util.Map;
30+
import java.util.Random;
2431

25-
public class MetalinkDirectTemplateDownloader extends DirectTemplateDownloaderImpl {
32+
public class MetalinkDirectTemplateDownloader extends HttpDirectTemplateDownloader {
2633

27-
private String downloadDir;
34+
private String metalinkUrl;
35+
private List<String> metalinkUrls;
36+
private List<String> metalinkChecksums;
37+
private Random random = new Random();
38+
private static final Logger s_logger = Logger.getLogger(MetalinkDirectTemplateDownloader.class.getName());
2839

29-
public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long templateId, String checksum) {
30-
super(url, destPoolPath, templateId, checksum);
31-
String relativeDir = getDirectDownloadTempPath(templateId);
32-
downloadDir = getDestPoolPath() + File.separator + relativeDir;
33-
createFolder(downloadDir);
40+
public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long templateId, String checksum, Map<String, String> headers) {
41+
super(url, templateId, destPoolPath, checksum, headers);
42+
metalinkUrl = url;
43+
metalinkUrls = UriUtils.getMetalinkUrls(metalinkUrl);
44+
metalinkChecksums = UriUtils.getMetalinkChecksums(metalinkUrl);
45+
if (CollectionUtils.isEmpty(metalinkUrls)) {
46+
throw new CloudRuntimeException("No urls found on metalink file: " + metalinkUrl + ". Not possible to download template " + templateId);
47+
}
48+
setUrl(metalinkUrls.get(0));
49+
s_logger.info("Metalink downloader created, metalink url: " + metalinkUrl + " parsed - " +
50+
metalinkUrls.size() + " urls and " +
51+
(CollectionUtils.isNotEmpty(metalinkChecksums) ? metalinkChecksums.size() : "0") + " checksums found");
3452
}
3553

3654
@Override
3755
public boolean downloadTemplate() {
38-
String downloadCommand = "aria2c " + getUrl() + " -d " + downloadDir + " --check-integrity=true";
39-
Script.runSimpleBashScript(downloadCommand);
40-
//Remove .metalink file
41-
Script.runSimpleBashScript("rm -f " + downloadDir + File.separator + getFileNameFromUrl());
42-
String fileName = Script.runSimpleBashScript("ls " + downloadDir);
43-
if (fileName == null) {
44-
return false;
56+
if (StringUtils.isBlank(getUrl())) {
57+
throw new CloudRuntimeException("Download url has not been set, aborting");
58+
}
59+
String downloadDir = getDirectDownloadTempPath(getTemplateId());
60+
boolean downloaded = false;
61+
int i = 0;
62+
do {
63+
if (!isRedownload()) {
64+
setUrl(metalinkUrls.get(i));
65+
}
66+
s_logger.info("Trying to download template from url: " + getUrl());
67+
try {
68+
File f = new File(getDestPoolPath() + File.separator + downloadDir + File.separator + getFileNameFromUrl());
69+
if (f.exists()) {
70+
f.delete();
71+
f.createNewFile();
72+
}
73+
setDownloadedFilePath(f.getAbsolutePath());
74+
request = createRequest(getUrl(), reqHeaders);
75+
downloaded = super.downloadTemplate();
76+
if (downloaded) {
77+
s_logger.info("Successfully downloaded template from url: " + getUrl());
78+
}
79+
80+
} catch (Exception e) {
81+
s_logger.error("Error downloading template: " + getTemplateId() + " from " + getUrl() + ": " + e.getMessage());
82+
}
83+
i++;
84+
}
85+
while (!downloaded && !isRedownload() && i < metalinkUrls.size());
86+
return downloaded;
87+
}
88+
89+
@Override
90+
public boolean validateChecksum() {
91+
if (StringUtils.isBlank(getChecksum()) && CollectionUtils.isNotEmpty(metalinkChecksums)) {
92+
String chk = metalinkChecksums.get(random.nextInt(metalinkChecksums.size()));
93+
setChecksum(chk);
94+
s_logger.info("Checksum not provided but " + metalinkChecksums.size() + " found on metalink file, performing checksum using one of them: " + chk);
4595
}
46-
setDownloadedFilePath(downloadDir + File.separator + fileName);
47-
return true;
96+
return super.validateChecksum();
4897
}
4998
}

‎api/src/com/cloud/event/EventTypes.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,8 @@ public class EventTypes {
581581
public static final String EVENT_ANNOTATION_CREATE = "ANNOTATION.CREATE";
582582
public static final String EVENT_ANNOTATION_REMOVE = "ANNOTATION.REMOVE";
583583

584+
public static final String EVENT_TEMPLATE_DIRECT_DOWNLOAD_FAILURE = "TEMPLATE.DIRECT.DOWNLOAD.FAILURE";
585+
public static final String EVENT_ISO_DIRECT_DOWNLOAD_FAILURE = "ISO.DIRECT.DOWNLOAD.FAILURE";
584586

585587
static {
586588

@@ -972,6 +974,9 @@ public class EventTypes {
972974

973975
entityEventDetails.put(EVENT_ANNOTATION_CREATE, Annotation.class);
974976
entityEventDetails.put(EVENT_ANNOTATION_REMOVE, Annotation.class);
977+
978+
entityEventDetails.put(EVENT_TEMPLATE_DIRECT_DOWNLOAD_FAILURE, VirtualMachineTemplate.class);
979+
entityEventDetails.put(EVENT_ISO_DIRECT_DOWNLOAD_FAILURE, "Iso");
975980
}
976981

977982
public static String getEntityForEvent(String eventName) {

0 commit comments

Comments
 (0)