From 9c7bb8c838f6d9c0d16481640a864b5594b9cf8c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 2 Dec 2019 14:23:59 +0530 Subject: [PATCH 01/30] changes for configurable timeouts for direct download Signed-off-by: Abhishek Kumar --- .../HttpDirectTemplateDownloader.java | 25 ++--- .../HttpsDirectTemplateDownloader.java | 46 ++++----- .../MetalinkDirectTemplateDownloader.java | 17 ++-- .../download/DirectDownloadManager.java | 25 ++++- .../directdownload/DirectDownloadCommand.java | 31 +++++- .../HttpDirectDownloadCommand.java | 8 +- .../HttpsDirectDownloadCommand.java | 9 +- .../MetalinkDirectDownloadCommand.java | 8 +- .../kvm/storage/KVMStorageProcessor.java | 6 +- .../download/DirectDownloadManagerImpl.java | 98 ++++++++++++------- 10 files changed, 183 insertions(+), 90 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java index 147ccabf8fc8..c131bf60d70a 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java @@ -19,22 +19,23 @@ package com.cloud.agent.direct.download; -import com.cloud.utils.exception.CloudRuntimeException; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.HashMap; +import java.util.Map; + import org.apache.commons.collections.MapUtils; import org.apache.commons.httpclient.HttpClient; -import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager; import org.apache.commons.httpclient.HttpStatus; +import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager; import org.apache.commons.httpclient.methods.GetMethod; import org.apache.commons.io.IOUtils; import org.apache.log4j.Logger; -import java.io.File; -import java.io.FileOutputStream; -import java.io.InputStream; -import java.io.OutputStream; -import java.io.IOException; -import java.util.HashMap; -import java.util.Map; +import com.cloud.utils.exception.CloudRuntimeException; public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl { @@ -44,10 +45,10 @@ public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl { protected GetMethod request; protected Map reqHeaders = new HashMap<>(); - public HttpDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers) { + public HttpDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, int connectTimeout, int soTimeout) { super(url, destPoolPath, templateId, checksum); - s_httpClientManager.getParams().setConnectionTimeout(5000); - s_httpClientManager.getParams().setSoTimeout(5000); + s_httpClientManager.getParams().setConnectionTimeout(connectTimeout); + s_httpClientManager.getParams().setSoTimeout(soTimeout); client = new HttpClient(s_httpClientManager); request = createRequest(url, headers); String downloadDir = getDirectDownloadTempPath(templateId); diff --git a/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java index 38f59837cd83..85b4274327dd 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java @@ -19,28 +19,12 @@ package com.cloud.agent.direct.download; -import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.script.Script; -import org.apache.commons.io.IOUtils; -import org.apache.http.HttpEntity; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.commons.collections.MapUtils; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; -import org.apache.http.conn.ssl.TrustSelfSignedStrategy; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.ssl.SSLContexts; - -import javax.net.ssl.SSLContext; import java.io.File; import java.io.FileInputStream; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.FileOutputStream; import java.security.KeyManagementException; import java.security.KeyStore; import java.security.KeyStoreException; @@ -48,13 +32,31 @@ import java.security.cert.CertificateException; import java.util.Map; +import javax.net.ssl.SSLContext; + +import org.apache.commons.collections.MapUtils; +import org.apache.commons.io.IOUtils; +import org.apache.http.HttpEntity; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.conn.ssl.TrustSelfSignedStrategy; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.ssl.SSLContexts; + +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.Script; + public class HttpsDirectTemplateDownloader extends HttpDirectTemplateDownloader { private CloseableHttpClient httpsClient; private HttpUriRequest req; - public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers) { - super(url, templateId, destPoolPath, checksum, headers); + public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, int connectTimeout, int soTimeout, int connectionRequestTimeout) { + super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout); SSLContext sslcontext = null; try { sslcontext = getSSLContext(); @@ -63,9 +65,9 @@ public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoo } SSLConnectionSocketFactory factory = new SSLConnectionSocketFactory(sslcontext, SSLConnectionSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER); RequestConfig config = RequestConfig.custom() - .setConnectTimeout(5000) - .setConnectionRequestTimeout(5000) - .setSocketTimeout(5000).build(); + .setConnectTimeout(connectTimeout) + .setConnectionRequestTimeout(connectionRequestTimeout) + .setSocketTimeout(soTimeout).build(); httpsClient = HttpClients.custom().setSSLSocketFactory(factory).setDefaultRequestConfig(config).build(); createUriRequest(url, headers); } diff --git a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java index 2fd8ba036111..fcc5aaf76673 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java @@ -18,17 +18,18 @@ // package com.cloud.agent.direct.download; -import com.cloud.utils.UriUtils; -import com.cloud.utils.exception.CloudRuntimeException; -import org.apache.commons.collections.CollectionUtils; -import org.apache.commons.lang.StringUtils; -import org.apache.log4j.Logger; - import java.io.File; import java.util.List; import java.util.Map; import java.util.Random; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang.StringUtils; +import org.apache.log4j.Logger; + +import com.cloud.utils.UriUtils; +import com.cloud.utils.exception.CloudRuntimeException; + public class MetalinkDirectTemplateDownloader extends HttpDirectTemplateDownloader { private String metalinkUrl; @@ -37,8 +38,8 @@ public class MetalinkDirectTemplateDownloader extends HttpDirectTemplateDownload private Random random = new Random(); private static final Logger s_logger = Logger.getLogger(MetalinkDirectTemplateDownloader.class.getName()); - public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long templateId, String checksum, Map headers) { - super(url, templateId, destPoolPath, checksum, headers); + public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long templateId, String checksum, Map headers, int connectTimeout, int soTimeout) { + super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout); metalinkUrl = url; metalinkUrls = UriUtils.getMetalinkUrls(metalinkUrl); metalinkChecksums = UriUtils.getMetalinkChecksums(metalinkUrl); diff --git a/api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManager.java b/api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManager.java index d627ffa69d18..8145c177ef39 100644 --- a/api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManager.java +++ b/api/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManager.java @@ -17,13 +17,18 @@ package org.apache.cloudstack.direct.download; -import com.cloud.utils.component.PluggableService; import org.apache.cloudstack.framework.agent.direct.download.DirectDownloadService; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; +import com.cloud.utils.component.PluggableService; + public interface DirectDownloadManager extends DirectDownloadService, PluggableService, Configurable { + static final int DEFAULT_DIRECT_DOWNLOAD_CONNECT_TIMEOUT = 5000; + static final int DEFAULT_DIRECT_DOWNLOAD_SOCKET_TIMEOUT = 5000; + static final int DEFAULT_DIRECT_DOWNLOAD_CONNECTION_REQUEST_TIMEOUT = 5000; + ConfigKey DirectDownloadCertificateUploadInterval = new ConfigKey<>("Advanced", Long.class, "direct.download.certificate.background.task.interval", "0", @@ -32,6 +37,24 @@ public interface DirectDownloadManager extends DirectDownloadService, PluggableS "Only certificates which have not been revoked from hosts are uploaded", false); + static final ConfigKey DirectDownloadConnectTimeout = new ConfigKey("Advanced", Integer.class, + "direct.download.connect.timeout", + String.valueOf(DEFAULT_DIRECT_DOWNLOAD_CONNECT_TIMEOUT), + "Connection establishment timeout in milliseconds for direct download", + true); + + static final ConfigKey DirectDownloadSocketTimeout = new ConfigKey("Advanced", Integer.class, + "direct.download.socket.timeout", + String.valueOf(DEFAULT_DIRECT_DOWNLOAD_SOCKET_TIMEOUT), + "Socket timeout (SO_TIMEOUT) in milliseconds for direct download", + true); + + static final ConfigKey DirectDownloadConnectionRequestTimeout = new ConfigKey("Advanced", Integer.class, + "direct.download.connection.request.timeout", + String.valueOf(DEFAULT_DIRECT_DOWNLOAD_CONNECTION_REQUEST_TIMEOUT), + "Requesting a connection from connection manager timeout in milliseconds for direct download", + true); + /** * Revoke direct download certificate with alias 'alias' from hosts of hypervisor type 'hypervisor' */ diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java index 7a05d6144e4f..e7cdcd21f61c 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java @@ -19,11 +19,11 @@ package org.apache.cloudstack.agent.directdownload; +import java.util.Map; + import org.apache.cloudstack.storage.command.StorageSubSystemCommand; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; -import java.util.Map; - public abstract class DirectDownloadCommand extends StorageSubSystemCommand { public enum DownloadProtocol { @@ -35,6 +35,9 @@ public enum DownloadProtocol { private PrimaryDataStoreTO destPool; private String checksum; private Map headers; + private int connectTimeout; + private int soTimeout; + private int connectionRequestTimeout; protected DirectDownloadCommand (final String url, final Long templateId, final PrimaryDataStoreTO destPool, final String checksum, final Map headers) { this.url = url; @@ -64,6 +67,30 @@ public Map getHeaders() { return headers; } + public int getConnectTimeout() { + return connectTimeout; + } + + public void setConnectTimeout(int connectTimeout) { + this.connectTimeout = connectTimeout; + } + + public int getSoTimeout() { + return soTimeout; + } + + public void setSoTimeout(int soTimeout) { + this.soTimeout = soTimeout; + } + + public int getConnectionRequestTimeout() { + return connectionRequestTimeout; + } + + public void setConnectionRequestTimeout(int connectionRequestTimeout) { + this.connectionRequestTimeout = connectionRequestTimeout; + } + @Override public void setExecuteInSequence(boolean inSeq) { } diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java index 7e32688154cb..4e1433406f37 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java @@ -18,14 +18,16 @@ */ package org.apache.cloudstack.agent.directdownload; -import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; - import java.util.Map; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; + public class HttpDirectDownloadCommand extends DirectDownloadCommand { - public HttpDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers) { + public HttpDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers, int connectTimeout, int soTimeout) { super(url, templateId, destPool, checksum, headers); + setConnectTimeout(connectTimeout); + setSoTimeout(soTimeout); } } diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java index ca926f1c7615..3de70ecbcdb8 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java @@ -19,13 +19,16 @@ package org.apache.cloudstack.agent.directdownload; -import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; - import java.util.Map; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; + public class HttpsDirectDownloadCommand extends DirectDownloadCommand { - public HttpsDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers) { + public HttpsDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers, int connectTimeout, int soTimeout, int connectionRequestTimeout) { super(url, templateId, destPool, checksum, headers); + setConnectTimeout(connectTimeout); + setSoTimeout(soTimeout); + setConnectionRequestTimeout(connectionRequestTimeout); } } \ No newline at end of file diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java index da528d966947..6438120a4f52 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java @@ -18,14 +18,16 @@ // package org.apache.cloudstack.agent.directdownload; -import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; - import java.util.Map; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; + public class MetalinkDirectDownloadCommand extends DirectDownloadCommand { - public MetalinkDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers) { + public MetalinkDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers, int connectTimeout, int soTimeout) { super(url, templateId, destPool, checksum, headers); + setConnectTimeout(connectTimeout); + setSoTimeout(soTimeout); } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 9a2fd275dd3c..979583b13992 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1678,13 +1678,13 @@ public Answer forgetObject(final ForgetObjectCmd cmd) { */ private DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDownloadCommand cmd, KVMStoragePool destPool) { if (cmd instanceof HttpDirectDownloadCommand) { - return new HttpDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders()); + return new HttpDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout()); } else if (cmd instanceof HttpsDirectDownloadCommand) { - return new HttpsDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders()); + return new HttpsDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout(), cmd.getConnectionRequestTimeout()); } else if (cmd instanceof NfsDirectDownloadCommand) { return new NfsDirectTemplateDownloader(cmd.getUrl(), destPool.getLocalPath(), cmd.getTemplateId(), cmd.getChecksum()); } else if (cmd instanceof MetalinkDirectDownloadCommand) { - return new MetalinkDirectTemplateDownloader(cmd.getUrl(), destPool.getLocalPath(), cmd.getTemplateId(), cmd.getChecksum(), cmd.getHeaders()); + return new MetalinkDirectTemplateDownloader(cmd.getUrl(), destPool.getLocalPath(), cmd.getTemplateId(), cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout()); } else { throw new IllegalArgumentException("Unsupported protocol, please provide HTTP(S), NFS or a metalink"); } diff --git a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index 2eb6d36b9bf9..8ba261bb7f40 100644 --- a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -20,30 +20,6 @@ import static com.cloud.storage.Storage.ImageFormat; -import com.cloud.agent.AgentManager; -import com.cloud.agent.api.Answer; -import com.cloud.dc.DataCenterVO; -import com.cloud.dc.dao.DataCenterDao; -import com.cloud.event.ActionEventUtils; -import com.cloud.event.EventTypes; -import com.cloud.event.EventVO; -import com.cloud.exception.AgentUnavailableException; -import com.cloud.exception.OperationTimedoutException; -import com.cloud.host.Host; -import com.cloud.host.HostVO; -import com.cloud.host.Status; -import com.cloud.host.dao.HostDao; -import com.cloud.hypervisor.Hypervisor.HypervisorType; -import com.cloud.storage.DataStoreRole; -import com.cloud.storage.VMTemplateStoragePoolVO; -import com.cloud.storage.VMTemplateStorageResourceAssoc; -import com.cloud.storage.VMTemplateVO; -import com.cloud.storage.dao.VMTemplateDao; -import com.cloud.storage.dao.VMTemplatePoolDao; -import com.cloud.utils.component.ManagerBase; -import com.cloud.utils.concurrency.NamedThreadFactory; -import com.cloud.utils.exception.CloudRuntimeException; - import java.net.URI; import java.net.URISyntaxException; import java.security.cert.Certificate; @@ -51,26 +27,26 @@ import java.security.cert.CertificateExpiredException; import java.security.cert.CertificateNotYetValidException; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Arrays; -import java.util.Collections; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; + import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.utils.security.CertificateHelper; -import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.DirectDownloadAnswer; +import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand.DownloadProtocol; import org.apache.cloudstack.agent.directdownload.HttpDirectDownloadCommand; +import org.apache.cloudstack.agent.directdownload.HttpsDirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.MetalinkDirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.NfsDirectDownloadCommand; -import org.apache.cloudstack.agent.directdownload.HttpsDirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.RevokeDirectDownloadCertificateCommand; import org.apache.cloudstack.agent.directdownload.SetupDirectDownloadCertificateCommand; import org.apache.cloudstack.context.CallContext; @@ -79,6 +55,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.poll.BackgroundPollManager; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; @@ -89,6 +66,32 @@ import org.apache.log4j.Logger; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; + +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; +import com.cloud.dc.DataCenterVO; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.event.ActionEventUtils; +import com.cloud.event.EventTypes; +import com.cloud.event.EventVO; +import com.cloud.exception.AgentUnavailableException; +import com.cloud.exception.OperationTimedoutException; +import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.host.Status; +import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.VMTemplateStorageResourceAssoc; +import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VMTemplatePoolDao; +import com.cloud.utils.component.ManagerBase; +import com.cloud.utils.concurrency.NamedThreadFactory; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.security.CertificateHelper; + import sun.security.x509.X509CertImpl; public class DirectDownloadManagerImpl extends ManagerBase implements DirectDownloadManager { @@ -119,6 +122,8 @@ public class DirectDownloadManagerImpl extends ManagerBase implements DirectDown private BackgroundPollManager backgroundPollManager; @Inject private DataCenterDao dataCenterDao; + @Inject + private ConfigurationDao configDao; protected ScheduledExecutorService executorService; @@ -320,14 +325,38 @@ private void logUsageEvent(VMTemplateVO template, long poolId) { */ private DirectDownloadCommand getDirectDownloadCommandFromProtocol(DownloadProtocol protocol, String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map httpHeaders) { + int connectTimeout = DEFAULT_DIRECT_DOWNLOAD_CONNECT_TIMEOUT; + int soTimeout = DEFAULT_DIRECT_DOWNLOAD_SOCKET_TIMEOUT; + int connectionRequestTimeout = DEFAULT_DIRECT_DOWNLOAD_CONNECTION_REQUEST_TIMEOUT; + if (DownloadProtocol.HTTP.equals(protocol) || + DownloadProtocol.HTTPS.equals(protocol) || + DownloadProtocol.METALINK.equals(protocol)) { + try { + connectTimeout = Integer.parseInt(configDao.getValue(DirectDownloadConnectTimeout.key())); + } catch (NumberFormatException nfe) { + s_logger.warn(String.format("Unable to retrieve configuration: %s value", DirectDownloadConnectTimeout.key()), nfe); + } + try { + soTimeout = Integer.parseInt(configDao.getValue(DirectDownloadSocketTimeout.key())); + } catch (NumberFormatException nfe) { + s_logger.warn(String.format("Unable to retrieve configuration: %s value", DirectDownloadSocketTimeout.key()), nfe); + } + } + if (DownloadProtocol.HTTPS.equals(protocol)) { + try { + connectionRequestTimeout = Integer.parseInt(configDao.getValue(DirectDownloadConnectionRequestTimeout.key())); + } catch (NumberFormatException nfe) { + s_logger.warn(String.format("Unable to retrieve configuration: %s value", DirectDownloadConnectionRequestTimeout.key()), nfe); + } + } if (protocol.equals(DownloadProtocol.HTTP)) { - return new HttpDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders); + return new HttpDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders, connectTimeout, soTimeout); } else if (protocol.equals(DownloadProtocol.HTTPS)) { - return new HttpsDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders); + return new HttpsDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders, connectTimeout, soTimeout, connectionRequestTimeout); } else if (protocol.equals(DownloadProtocol.NFS)) { return new NfsDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders); } else if (protocol.equals(DownloadProtocol.METALINK)) { - return new MetalinkDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders); + return new MetalinkDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders, connectTimeout, soTimeout); } else { return null; } @@ -549,7 +578,10 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[]{ - DirectDownloadCertificateUploadInterval + DirectDownloadCertificateUploadInterval, + DirectDownloadConnectTimeout, + DirectDownloadSocketTimeout, + DirectDownloadConnectionRequestTimeout }; } From a674444222f75882ebe31aa40bf7c7688cd8b21d Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 24 Dec 2019 13:01:05 +0530 Subject: [PATCH 02/30] server: refactor direct download config value retrieval Signed-off-by: Abhishek Kumar --- .../download/DirectDownloadManagerImpl.java | 27 +++---------------- 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index 8ba261bb7f40..792a5a30ee84 100644 --- a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -325,30 +325,9 @@ private void logUsageEvent(VMTemplateVO template, long poolId) { */ private DirectDownloadCommand getDirectDownloadCommandFromProtocol(DownloadProtocol protocol, String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map httpHeaders) { - int connectTimeout = DEFAULT_DIRECT_DOWNLOAD_CONNECT_TIMEOUT; - int soTimeout = DEFAULT_DIRECT_DOWNLOAD_SOCKET_TIMEOUT; - int connectionRequestTimeout = DEFAULT_DIRECT_DOWNLOAD_CONNECTION_REQUEST_TIMEOUT; - if (DownloadProtocol.HTTP.equals(protocol) || - DownloadProtocol.HTTPS.equals(protocol) || - DownloadProtocol.METALINK.equals(protocol)) { - try { - connectTimeout = Integer.parseInt(configDao.getValue(DirectDownloadConnectTimeout.key())); - } catch (NumberFormatException nfe) { - s_logger.warn(String.format("Unable to retrieve configuration: %s value", DirectDownloadConnectTimeout.key()), nfe); - } - try { - soTimeout = Integer.parseInt(configDao.getValue(DirectDownloadSocketTimeout.key())); - } catch (NumberFormatException nfe) { - s_logger.warn(String.format("Unable to retrieve configuration: %s value", DirectDownloadSocketTimeout.key()), nfe); - } - } - if (DownloadProtocol.HTTPS.equals(protocol)) { - try { - connectionRequestTimeout = Integer.parseInt(configDao.getValue(DirectDownloadConnectionRequestTimeout.key())); - } catch (NumberFormatException nfe) { - s_logger.warn(String.format("Unable to retrieve configuration: %s value", DirectDownloadConnectionRequestTimeout.key()), nfe); - } - } + int connectTimeout = DirectDownloadConnectTimeout.value(); + int soTimeout = DirectDownloadSocketTimeout.value(); + int connectionRequestTimeout = DirectDownloadConnectionRequestTimeout.value(); if (protocol.equals(DownloadProtocol.HTTP)) { return new HttpDirectDownloadCommand(url, templateId, destPool, checksum, httpHeaders, connectTimeout, soTimeout); } else if (protocol.equals(DownloadProtocol.HTTPS)) { From fab61d5e89deb791b12a48c7526d349d2322f3ff Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 24 Dec 2019 13:26:18 +0530 Subject: [PATCH 03/30] refactored direc download cmd, downloader classes Signed-off-by: Abhishek Kumar --- .../HttpDirectTemplateDownloader.java | 6 ++--- .../HttpsDirectTemplateDownloader.java | 8 +++---- .../MetalinkDirectTemplateDownloader.java | 2 +- .../directdownload/DirectDownloadCommand.java | 23 +++++++++++-------- .../HttpDirectDownloadCommand.java | 4 +--- .../HttpsDirectDownloadCommand.java | 5 +--- .../MetalinkDirectDownloadCommand.java | 4 +--- .../NfsDirectDownloadCommand.java | 6 ++--- 8 files changed, 27 insertions(+), 31 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java index c131bf60d70a..f91af40dd404 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java @@ -45,10 +45,10 @@ public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl { protected GetMethod request; protected Map reqHeaders = new HashMap<>(); - public HttpDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, int connectTimeout, int soTimeout) { + public HttpDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, Integer connectTimeout, Integer soTimeout) { super(url, destPoolPath, templateId, checksum); - s_httpClientManager.getParams().setConnectionTimeout(connectTimeout); - s_httpClientManager.getParams().setSoTimeout(soTimeout); + s_httpClientManager.getParams().setConnectionTimeout(connectTimeout == null ? 5000 : connectTimeout); + s_httpClientManager.getParams().setSoTimeout(soTimeout == null ? 5000 : soTimeout); client = new HttpClient(s_httpClientManager); request = createRequest(url, headers); String downloadDir = getDirectDownloadTempPath(templateId); diff --git a/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java index 85b4274327dd..f320846e75d9 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java @@ -55,7 +55,7 @@ public class HttpsDirectTemplateDownloader extends HttpDirectTemplateDownloader private CloseableHttpClient httpsClient; private HttpUriRequest req; - public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, int connectTimeout, int soTimeout, int connectionRequestTimeout) { + public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, Integer connectTimeout, Integer soTimeout, Integer connectionRequestTimeout) { super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout); SSLContext sslcontext = null; try { @@ -65,9 +65,9 @@ public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoo } SSLConnectionSocketFactory factory = new SSLConnectionSocketFactory(sslcontext, SSLConnectionSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER); RequestConfig config = RequestConfig.custom() - .setConnectTimeout(connectTimeout) - .setConnectionRequestTimeout(connectionRequestTimeout) - .setSocketTimeout(soTimeout).build(); + .setConnectTimeout(connectTimeout == null ? 5000 : connectTimeout) + .setConnectionRequestTimeout(connectionRequestTimeout == null ? 5000 : connectionRequestTimeout) + .setSocketTimeout(soTimeout == null ? 5000 : soTimeout).build(); httpsClient = HttpClients.custom().setSSLSocketFactory(factory).setDefaultRequestConfig(config).build(); createUriRequest(url, headers); } diff --git a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java index fcc5aaf76673..0a571015bee7 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java @@ -38,7 +38,7 @@ public class MetalinkDirectTemplateDownloader extends HttpDirectTemplateDownload private Random random = new Random(); private static final Logger s_logger = Logger.getLogger(MetalinkDirectTemplateDownloader.class.getName()); - public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long templateId, String checksum, Map headers, int connectTimeout, int soTimeout) { + public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long templateId, String checksum, Map headers, Integer connectTimeout, Integer soTimeout) { super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout); metalinkUrl = url; metalinkUrls = UriUtils.getMetalinkUrls(metalinkUrl); diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java index e7cdcd21f61c..506cd3140de8 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java @@ -35,16 +35,19 @@ public enum DownloadProtocol { private PrimaryDataStoreTO destPool; private String checksum; private Map headers; - private int connectTimeout; - private int soTimeout; - private int connectionRequestTimeout; + private Integer connectTimeout; + private Integer soTimeout; + private Integer connectionRequestTimeout; - protected DirectDownloadCommand (final String url, final Long templateId, final PrimaryDataStoreTO destPool, final String checksum, final Map headers) { + protected DirectDownloadCommand (final String url, final Long templateId, final PrimaryDataStoreTO destPool, final String checksum, final Map headers, final Integer connectTimeout, final Integer soTimeout, final Integer connectionRequestTimeout) { this.url = url; this.templateId = templateId; this.destPool = destPool; this.checksum = checksum; this.headers = headers; + this.connectTimeout = connectTimeout; + this.soTimeout = soTimeout; + this.connectionRequestTimeout = connectionRequestTimeout; } public String getUrl() { @@ -67,27 +70,27 @@ public Map getHeaders() { return headers; } - public int getConnectTimeout() { + public Integer getConnectTimeout() { return connectTimeout; } - public void setConnectTimeout(int connectTimeout) { + public void setConnectTimeout(Integer connectTimeout) { this.connectTimeout = connectTimeout; } - public int getSoTimeout() { + public Integer getSoTimeout() { return soTimeout; } - public void setSoTimeout(int soTimeout) { + public void setSoTimeout(Integer soTimeout) { this.soTimeout = soTimeout; } - public int getConnectionRequestTimeout() { + public Integer getConnectionRequestTimeout() { return connectionRequestTimeout; } - public void setConnectionRequestTimeout(int connectionRequestTimeout) { + public void setConnectionRequestTimeout(Integer connectionRequestTimeout) { this.connectionRequestTimeout = connectionRequestTimeout; } diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java index 4e1433406f37..f131b3b0a7fc 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java @@ -25,9 +25,7 @@ public class HttpDirectDownloadCommand extends DirectDownloadCommand { public HttpDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers, int connectTimeout, int soTimeout) { - super(url, templateId, destPool, checksum, headers); - setConnectTimeout(connectTimeout); - setSoTimeout(soTimeout); + super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, null); } } diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java index 3de70ecbcdb8..8c415b6bd8df 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java @@ -26,9 +26,6 @@ public class HttpsDirectDownloadCommand extends DirectDownloadCommand { public HttpsDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers, int connectTimeout, int soTimeout, int connectionRequestTimeout) { - super(url, templateId, destPool, checksum, headers); - setConnectTimeout(connectTimeout); - setSoTimeout(soTimeout); - setConnectionRequestTimeout(connectionRequestTimeout); + super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, connectionRequestTimeout); } } \ No newline at end of file diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java index 6438120a4f52..a3edcebe7ded 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java @@ -25,9 +25,7 @@ public class MetalinkDirectDownloadCommand extends DirectDownloadCommand { public MetalinkDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map headers, int connectTimeout, int soTimeout) { - super(url, templateId, destPool, checksum, headers); - setConnectTimeout(connectTimeout); - setSoTimeout(soTimeout); + super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, null); } } diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/NfsDirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/NfsDirectDownloadCommand.java index abc0137dfbd7..0bf9c4d934b9 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/NfsDirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/NfsDirectDownloadCommand.java @@ -18,14 +18,14 @@ // package org.apache.cloudstack.agent.directdownload; -import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; - import java.util.Map; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; + public class NfsDirectDownloadCommand extends DirectDownloadCommand { public NfsDirectDownloadCommand(final String url, final Long templateId, final PrimaryDataStoreTO destPool, final String checksum, final Map headers) { - super(url, templateId, destPool, checksum, headers); + super(url, templateId, destPool, checksum, headers, null, null, null); } } From 55e699240e0638ccd7aaa87d6b81a6e1d346b174 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 12 Feb 2020 17:21:02 +0530 Subject: [PATCH 04/30] server, services: allow direct download template for SSVM, CPVM Signed-off-by: Abhishek Kumar --- .../com/cloud/consoleproxy/ConsoleProxyManagerImpl.java | 8 +++++++- .../secondarystorage/SecondaryStorageManagerImpl.java | 3 +-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java b/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java index a299937650c7..368fc33876c1 100644 --- a/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java +++ b/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java @@ -100,6 +100,7 @@ import com.cloud.resource.UnableDeleteHostException; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; +import com.cloud.storage.DataStoreRole; import com.cloud.storage.Storage; import com.cloud.storage.StoragePoolStatus; import com.cloud.storage.VMTemplateStorageResourceAssoc.Status; @@ -1011,7 +1012,12 @@ public boolean isZoneReady(Map zoneHostInfoMap, long dataCen } return false; } - TemplateDataStoreVO templateHostRef = _vmTemplateStoreDao.findByTemplateZoneDownloadStatus(template.getId(), dataCenterId, Status.DOWNLOADED); + TemplateDataStoreVO templateHostRef = null; + if (template.isDirectDownload()) { + templateHostRef = _vmTemplateStoreDao.findByTemplate(template.getId(), DataStoreRole.Image); + } else { + templateHostRef = _vmTemplateStoreDao.findByTemplateZoneDownloadStatus(template.getId(), dataCenterId, Status.DOWNLOADED); + } if (templateHostRef != null) { boolean useLocalStorage = false; diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index 8b2ed40c15db..a1a3873bf884 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -828,8 +828,7 @@ public boolean isZoneReady(Map zoneHostInfoMap, long dataCen return false; } - DataStore store = templateMgr.getImageStore(dataCenterId, template.getId()); - if (store == null) { + if (!template.isDirectDownload() && templateMgr.getImageStore(dataCenterId, template.getId()) == null) { if (s_logger.isDebugEnabled()) { s_logger.debug("No secondary storage available in zone " + dataCenterId + ", wait until it is ready to launch secondary storage vm"); } From 71197cb66af2df25c038c7d86ba89611a6c693bb Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 13 Feb 2020 11:12:18 +0530 Subject: [PATCH 05/30] list bypassed system templates Signed-off-by: Abhishek Kumar --- .../main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java index dd1f2fcf1641..2725df514c33 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java @@ -345,7 +345,7 @@ public boolean configure(String name, Map params) throws Configu readySystemTemplateSearch.and("state", readySystemTemplateSearch.entity().getState(), SearchCriteria.Op.EQ); readySystemTemplateSearch.and("templateType", readySystemTemplateSearch.entity().getTemplateType(), SearchCriteria.Op.EQ); SearchBuilder templateDownloadSearch = _templateDataStoreDao.createSearchBuilder(); - templateDownloadSearch.and("downloadState", templateDownloadSearch.entity().getDownloadState(), SearchCriteria.Op.EQ); + templateDownloadSearch.and("downloadState", templateDownloadSearch.entity().getDownloadState(), SearchCriteria.Op.IN); readySystemTemplateSearch.join("vmTemplateJoinTemplateStoreRef", templateDownloadSearch, templateDownloadSearch.entity().getTemplateId(), readySystemTemplateSearch.entity().getId(), JoinBuilder.JoinType.INNER); SearchBuilder hostHyperSearch2 = _hostDao.createSearchBuilder(); @@ -860,7 +860,7 @@ public VMTemplateVO findSystemVMReadyTemplate(long zoneId, HypervisorType hyperv sc.setParameters("state", VirtualMachineTemplate.State.Active); sc.setJoinParameters("tmplHyper", "type", Host.Type.Routing); sc.setJoinParameters("tmplHyper", "zoneId", zoneId); - sc.setJoinParameters("vmTemplateJoinTemplateStoreRef", "downloadState", VMTemplateStorageResourceAssoc.Status.DOWNLOADED); + sc.setJoinParameters("vmTemplateJoinTemplateStoreRef", "downloadState", (Object) new VMTemplateStorageResourceAssoc.Status[] {VMTemplateStorageResourceAssoc.Status.DOWNLOADED, VMTemplateStorageResourceAssoc.Status.BYPASSED}); // order by descending order of id List tmplts = listBy(sc, new Filter(VMTemplateVO.class, "id", false, null, null)); From ac48a66c32c7ce4e2528edf18e0268358ccee32e Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 13 Feb 2020 13:02:18 +0530 Subject: [PATCH 06/30] fix Signed-off-by: Abhishek Kumar --- .../src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java index 2725df514c33..6773c205aa37 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java @@ -860,7 +860,7 @@ public VMTemplateVO findSystemVMReadyTemplate(long zoneId, HypervisorType hyperv sc.setParameters("state", VirtualMachineTemplate.State.Active); sc.setJoinParameters("tmplHyper", "type", Host.Type.Routing); sc.setJoinParameters("tmplHyper", "zoneId", zoneId); - sc.setJoinParameters("vmTemplateJoinTemplateStoreRef", "downloadState", (Object) new VMTemplateStorageResourceAssoc.Status[] {VMTemplateStorageResourceAssoc.Status.DOWNLOADED, VMTemplateStorageResourceAssoc.Status.BYPASSED}); + sc.setJoinParameters("vmTemplateJoinTemplateStoreRef", "downloadState", new VMTemplateStorageResourceAssoc.Status[] {VMTemplateStorageResourceAssoc.Status.DOWNLOADED, VMTemplateStorageResourceAssoc.Status.BYPASSED}); // order by descending order of id List tmplts = listBy(sc, new Filter(VMTemplateVO.class, "id", false, null, null)); From 8a7bf4f3dc692e0ccc4904a0a3468ed38d99ca61 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 10 Jan 2020 12:08:12 -0300 Subject: [PATCH 07/30] Remove constraint for NFS storage --- .../com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 979583b13992..0539f3f20915 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1693,9 +1693,6 @@ private DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDo @Override 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(), true); - } KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); DirectTemplateDownloader downloader; From ed92071ed4b0c89018b2eaac61b696e44f07ba13 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 10 Jan 2020 13:48:23 -0300 Subject: [PATCH 08/30] Add new property on agent.properties --- agent/conf/agent.properties | 3 +++ .../kvm/resource/LibvirtComputingResource.java | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties index 7472e3922277..24592387b09e 100644 --- a/agent/conf/agent.properties +++ b/agent/conf/agent.properties @@ -115,6 +115,9 @@ domr.scripts.dir=scripts/network/domr/kvm # set the hypervisor type, values are: kvm, lxc hypervisor.type=kvm +# This parameter specifies a directory on the host local storage for temporary storing direct download templates +#direct.download.temporary.download.location=/var/lib/libvirt/images + # set the hypervisor URI. Usually there is no need for changing this # For KVM: qemu:///system # For LXC: lxc:/// diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 34439fc6e702..77616af86e83 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -475,6 +475,10 @@ public String getOvsPvlanVmPath() { return _ovsPvlanVmPath; } + public String getDirectDownloadTemporaryDownloadPath() { + return directDownloadTemporaryDownloadPath; + } + public String getResizeVolumePath() { return _resizeVolumePath; } @@ -527,6 +531,7 @@ protected enum BridgeType { protected boolean dpdkSupport = false; protected String dpdkOvsPath; + protected String directDownloadTemporaryDownloadPath; private String getEndIpFromStartIp(final String startIp, final int numIps) { final String[] tokens = startIp.split("[.]"); @@ -574,6 +579,10 @@ private Map getDeveloperProperties() throws ConfigurationExcepti } } + private String getDefaultDirectDownloadTemporaryPath() { + return "/var/lib/libvirt/images"; + } + protected String getDefaultNetworkScriptsDir() { return "scripts/vm/network/vnet"; } @@ -653,6 +662,11 @@ public boolean configure(final String name, final Map params) th } } + directDownloadTemporaryDownloadPath = (String) params.get("direct.download.temporary.download.location"); + if (org.apache.commons.lang.StringUtils.isBlank(directDownloadTemporaryDownloadPath)) { + directDownloadTemporaryDownloadPath = getDefaultDirectDownloadTemporaryPath(); + } + params.put("domr.scripts.dir", domrScriptsDir); _virtRouterResource = new VirtualRoutingResource(this); From 9e2682a7acda0d5ade06d80cb0d7f822e17a6a2b Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 10 Jan 2020 14:35:13 -0300 Subject: [PATCH 09/30] Add free disk space on the host prior template download --- .../directdownload/DirectDownloadCommand.java | 9 +++++++ .../kvm/storage/KVMStorageProcessor.java | 27 +++++++++++++++++++ .../download/DirectDownloadManagerImpl.java | 1 + 3 files changed, 37 insertions(+) diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java index 506cd3140de8..fbbc204bfd59 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java @@ -38,6 +38,7 @@ public enum DownloadProtocol { private Integer connectTimeout; private Integer soTimeout; private Integer connectionRequestTimeout; + private Long templateSize; protected DirectDownloadCommand (final String url, final Long templateId, final PrimaryDataStoreTO destPool, final String checksum, final Map headers, final Integer connectTimeout, final Integer soTimeout, final Integer connectionRequestTimeout) { this.url = url; @@ -94,6 +95,14 @@ public void setConnectionRequestTimeout(Integer connectionRequestTimeout) { this.connectionRequestTimeout = connectionRequestTimeout; } + public Long getTemplateSize() { + return templateSize; + } + + public void setTemplateSize(Long templateSize) { + this.templateSize = templateSize; + } + @Override public void setExecuteInSequence(boolean inSeq) { } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 0539f3f20915..537b41e61c71 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1703,6 +1703,11 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) } try { + s_logger.info("Checking for free space on the host for downloading the template"); + if (!isEnoughSpaceForDownloadTemplateOnTemporaryLocation(cmd.getTemplateSize())) { + String msg = "Not enough space on the defined temporary location to download the template " + cmd.getTemplateId(); + return new DirectDownloadAnswer(false, msg, true); + } s_logger.info("Trying to download template"); if (!downloader.downloadTemplate()) { s_logger.warn("Couldn't download template"); @@ -1724,4 +1729,26 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) DirectTemplateInformation info = downloader.getTemplateInformation(); return new DirectDownloadAnswer(true, info.getSize(), info.getInstallPath()); } + + /** + * Perform a free space check on the host for downloading the direct download templates + * @param templateSize template size obtained from remote server when registering the template + */ + private boolean isEnoughSpaceForDownloadTemplateOnTemporaryLocation(Long templateSize) { + if (templateSize == null || templateSize == 0L) { + s_logger.info("The server did not provide the template size, assuming there is enough space to download it"); + return true; + } + String cmd = String.format("df --output=avail %s | tail -1", resource.getDirectDownloadTemporaryDownloadPath()); + String result = Script.runSimpleBashScript(cmd); + Long availableBytes; + try { + availableBytes = Long.parseLong(result); + } catch (NumberFormatException e) { + String msg = "Could not parse the output " + result + " as a number, therefore not able to check for free space"; + s_logger.error(msg, e); + return false; + } + return availableBytes >= templateSize; + } } diff --git a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index 792a5a30ee84..d9ee0febf61d 100644 --- a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -252,6 +252,7 @@ public void downloadTemplate(long templateId, long poolId, long hostId) { DownloadProtocol protocol = getProtocolFromUrl(url); DirectDownloadCommand cmd = getDirectDownloadCommandFromProtocol(protocol, url, templateId, to, checksum, headers); + cmd.setTemplateSize(template.getSize()); Answer answer = sendDirectDownloadCommand(cmd, template, poolId, host); From 7329c8c5ac7d28d532f286547ee6e756b8c1101d Mon Sep 17 00:00:00 2001 From: nvazquez Date: Mon, 13 Jan 2020 14:21:19 -0300 Subject: [PATCH 10/30] Add unit tests for the free space check --- .../kvm/storage/KVMStorageProcessor.java | 2 +- .../kvm/storage/KVMStorageProcessorTest.java | 44 ++++++++++++++++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 537b41e61c71..00ec351d55c2 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1734,7 +1734,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) * Perform a free space check on the host for downloading the direct download templates * @param templateSize template size obtained from remote server when registering the template */ - private boolean isEnoughSpaceForDownloadTemplateOnTemporaryLocation(Long templateSize) { + protected boolean isEnoughSpaceForDownloadTemplateOnTemporaryLocation(Long templateSize) { if (templateSize == null || templateSize == 0L) { s_logger.info("The server did not provide the template size, assuming there is enough space to download it"); return true; diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java index 63d46bc87e9c..36d957038a27 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java @@ -21,13 +21,22 @@ import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import javax.naming.ConfigurationException; +import com.cloud.utils.script.Script; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; import org.mockito.InjectMocks; +import org.mockito.Matchers; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; -import org.mockito.Spy; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +@PrepareForTest({ Script.class }) +@RunWith(PowerMockRunner.class) public class KVMStorageProcessorTest { @Mock @@ -35,26 +44,47 @@ public class KVMStorageProcessorTest { @Mock LibvirtComputingResource resource; - private static final Long TEMPLATE_ID = 202l; - private static final String EXPECTED_DIRECT_DOWNLOAD_DIR = "template/2/202"; - - @Spy @InjectMocks private KVMStorageProcessor storageProcessor; + private static final String directDownloadTemporaryPath = "/var/lib/libvirt/images/dd"; + private static final long templateSize = 80000L; + @Before public void setUp() throws ConfigurationException { MockitoAnnotations.initMocks(this); storageProcessor = new KVMStorageProcessor(storagePoolManager, resource); + PowerMockito.mockStatic(Script.class); + Mockito.when(resource.getDirectDownloadTemporaryDownloadPath()).thenReturn(directDownloadTemporaryPath); } @Test - public void testCloneVolumeFromBaseTemplate() throws Exception { + public void testIsEnoughSpaceForDownloadTemplateOnTemporaryLocationAssumeEnoughSpaceWhenNotProvided() { + boolean result = storageProcessor.isEnoughSpaceForDownloadTemplateOnTemporaryLocation(null); + Assert.assertTrue(result); + } + @Test + public void testIsEnoughSpaceForDownloadTemplateOnTemporaryLocationNotEnoughSpace() { + String output = String.valueOf(templateSize - 30000L); + Mockito.when(Script.runSimpleBashScript(Matchers.anyString())).thenReturn(output); + boolean result = storageProcessor.isEnoughSpaceForDownloadTemplateOnTemporaryLocation(templateSize); + Assert.assertFalse(result); } @Test - public void testCopyVolumeFromImageCacheToPrimary() throws Exception { + public void testIsEnoughSpaceForDownloadTemplateOnTemporaryLocationEnoughSpace() { + String output = String.valueOf(templateSize + 30000L); + Mockito.when(Script.runSimpleBashScript(Matchers.anyString())).thenReturn(output); + boolean result = storageProcessor.isEnoughSpaceForDownloadTemplateOnTemporaryLocation(templateSize); + Assert.assertTrue(result); + } + @Test + public void testIsEnoughSpaceForDownloadTemplateOnTemporaryLocationNotExistingLocation() { + String output = String.format("df: ‘%s’: No such file or directory", directDownloadTemporaryPath); + Mockito.when(Script.runSimpleBashScript(Matchers.anyString())).thenReturn(output); + boolean result = storageProcessor.isEnoughSpaceForDownloadTemplateOnTemporaryLocation(templateSize); + Assert.assertFalse(result); } } From 029c817701011f5a184d71dcde614bb72329ba45 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 14 Jan 2020 12:04:24 -0300 Subject: [PATCH 11/30] Fix free space check - retrieve avaiable size in bytes --- .../hypervisor/kvm/storage/KVMStorageProcessor.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 00ec351d55c2..d02257de0303 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1732,20 +1732,20 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) /** * Perform a free space check on the host for downloading the direct download templates - * @param templateSize template size obtained from remote server when registering the template + * @param templateSize template size obtained from remote server when registering the template (in bytes) */ protected boolean isEnoughSpaceForDownloadTemplateOnTemporaryLocation(Long templateSize) { if (templateSize == null || templateSize == 0L) { s_logger.info("The server did not provide the template size, assuming there is enough space to download it"); return true; } - String cmd = String.format("df --output=avail %s | tail -1", resource.getDirectDownloadTemporaryDownloadPath()); - String result = Script.runSimpleBashScript(cmd); + String cmd = String.format("df --output=avail %s -B 1 | tail -1", resource.getDirectDownloadTemporaryDownloadPath()); + String resultInBytes = Script.runSimpleBashScript(cmd); Long availableBytes; try { - availableBytes = Long.parseLong(result); + availableBytes = Long.parseLong(resultInBytes); } catch (NumberFormatException e) { - String msg = "Could not parse the output " + result + " as a number, therefore not able to check for free space"; + String msg = "Could not parse the output " + resultInBytes + " as a number, therefore not able to check for free space"; s_logger.error(msg, e); return false; } From 114f05406a3a8061c4cf00e10d8078eeb6bc1417 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 14 Jan 2020 12:20:10 -0300 Subject: [PATCH 12/30] Update default location for direct download --- agent/conf/agent.properties | 2 +- .../cloud/hypervisor/kvm/resource/LibvirtComputingResource.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties index 24592387b09e..525c19f8a592 100644 --- a/agent/conf/agent.properties +++ b/agent/conf/agent.properties @@ -116,7 +116,7 @@ domr.scripts.dir=scripts/network/domr/kvm hypervisor.type=kvm # This parameter specifies a directory on the host local storage for temporary storing direct download templates -#direct.download.temporary.download.location=/var/lib/libvirt/images +#direct.download.temporary.download.location=/var/lib/libvirt/images/direct-download # set the hypervisor URI. Usually there is no need for changing this # For KVM: qemu:///system diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 77616af86e83..c885d54fedb9 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -580,7 +580,7 @@ private Map getDeveloperProperties() throws ConfigurationExcepti } private String getDefaultDirectDownloadTemporaryPath() { - return "/var/lib/libvirt/images"; + return "/var/lib/libvirt/images/direct-download"; } protected String getDefaultNetworkScriptsDir() { From ae8059d8f6b2cd195cd2624310f8375976640261 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 15 Jan 2020 13:22:03 -0300 Subject: [PATCH 13/30] Improve the method to retrieve hosts to retry on depending on the destination pool type and scope --- .../download/DirectDownloadManagerImpl.java | 69 +++++++++++-------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index d9ee0febf61d..2ca3595e5d88 100644 --- a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -20,6 +20,32 @@ import static com.cloud.storage.Storage.ImageFormat; +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; +import com.cloud.dc.DataCenterVO; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.event.ActionEventUtils; +import com.cloud.event.EventTypes; +import com.cloud.event.EventVO; +import com.cloud.exception.AgentUnavailableException; +import com.cloud.exception.OperationTimedoutException; +import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.host.Status; +import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.ScopeType; +import com.cloud.storage.Storage; +import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.VMTemplateStorageResourceAssoc; +import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VMTemplatePoolDao; +import com.cloud.utils.component.ManagerBase; +import com.cloud.utils.concurrency.NamedThreadFactory; +import com.cloud.utils.exception.CloudRuntimeException; + import java.net.URI; import java.net.URISyntaxException; import java.security.cert.Certificate; @@ -27,11 +53,11 @@ import java.security.cert.CertificateExpiredException; import java.security.cert.CertificateNotYetValidException; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; + import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -67,29 +93,6 @@ import org.joda.time.DateTime; import org.joda.time.DateTimeZone; -import com.cloud.agent.AgentManager; -import com.cloud.agent.api.Answer; -import com.cloud.dc.DataCenterVO; -import com.cloud.dc.dao.DataCenterDao; -import com.cloud.event.ActionEventUtils; -import com.cloud.event.EventTypes; -import com.cloud.event.EventVO; -import com.cloud.exception.AgentUnavailableException; -import com.cloud.exception.OperationTimedoutException; -import com.cloud.host.Host; -import com.cloud.host.HostVO; -import com.cloud.host.Status; -import com.cloud.host.dao.HostDao; -import com.cloud.hypervisor.Hypervisor.HypervisorType; -import com.cloud.storage.DataStoreRole; -import com.cloud.storage.VMTemplateStoragePoolVO; -import com.cloud.storage.VMTemplateStorageResourceAssoc; -import com.cloud.storage.VMTemplateVO; -import com.cloud.storage.dao.VMTemplateDao; -import com.cloud.storage.dao.VMTemplatePoolDao; -import com.cloud.utils.component.ManagerBase; -import com.cloud.utils.concurrency.NamedThreadFactory; -import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.security.CertificateHelper; import sun.security.x509.X509CertImpl; @@ -202,7 +205,7 @@ protected List getRunningHostIdsInTheSameCluster(Long clusterId, long data */ protected Long[] createHostIdsList(List hostIds, long hostId) { if (CollectionUtils.isEmpty(hostIds)) { - return Arrays.asList(hostId).toArray(new Long[1]); + return Collections.singletonList(hostId).toArray(new Long[1]); } Long[] ids = new Long[hostIds.size() + 1]; ids[0] = hostId; @@ -215,11 +218,15 @@ protected Long[] createHostIdsList(List hostIds, long hostId) { } /** - * Get hosts to retry download having hostId as the first element + * Get alternative hosts to retry downloading a template. The planner have previously selected a host and a storage pool + * @return array of host ids which can access the storage pool */ - protected Long[] getHostsToRetryOn(Long clusterId, long dataCenterId, HypervisorType hypervisorType, long hostId) { - List hostIds = getRunningHostIdsInTheSameCluster(clusterId, dataCenterId, hypervisorType, hostId); - return createHostIdsList(hostIds, hostId); + protected Long[] getHostsToRetryOn(Host host, StoragePoolVO storagePool) { + List clusterHostIds = new ArrayList<>(); + if (storagePool.getPoolType() != Storage.StoragePoolType.Filesystem || storagePool.getScope() != ScopeType.HOST) { + clusterHostIds = getRunningHostIdsInTheSameCluster(host.getClusterId(), host.getDataCenterId(), host.getHypervisorType(), host.getId()); + } + return createHostIdsList(clusterHostIds, host.getId()); } @Override @@ -285,7 +292,9 @@ public void downloadTemplate(long templateId, long poolId, long hostId) { private Answer sendDirectDownloadCommand(DirectDownloadCommand cmd, VMTemplateVO template, long poolId, HostVO host) { boolean downloaded = false; int retry = 3; - Long[] hostsToRetry = getHostsToRetryOn(host.getClusterId(), host.getDataCenterId(), host.getHypervisorType(), host.getId()); + + StoragePoolVO storagePoolVO = primaryDataStoreDao.findById(poolId); + Long[] hostsToRetry = getHostsToRetryOn(host, storagePoolVO); int hostIndex = 0; Answer answer = null; Long hostToSendDownloadCmd = hostsToRetry[hostIndex]; From 3b7b682f85c831891d084fef3c5514d22584d513 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 15 Jan 2020 14:00:03 -0300 Subject: [PATCH 14/30] Verify location for temporary download exists before checking free space --- .../kvm/storage/KVMStorageProcessor.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index d02257de0303..15600675ab9b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1703,11 +1703,22 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) } try { + s_logger.info("Verifying temporary location for downloading the template exists on the host"); + String temporaryDownloadPath = resource.getDirectDownloadTemporaryDownloadPath(); + if (!isLocationAccessible(temporaryDownloadPath)) { + String msg = "The temporary location path for downloading templates does not exist: " + + temporaryDownloadPath + " on this host"; + s_logger.error(msg); + return new DirectDownloadAnswer(false, msg, true); + } + s_logger.info("Checking for free space on the host for downloading the template"); if (!isEnoughSpaceForDownloadTemplateOnTemporaryLocation(cmd.getTemplateSize())) { String msg = "Not enough space on the defined temporary location to download the template " + cmd.getTemplateId(); + s_logger.error(msg); return new DirectDownloadAnswer(false, msg, true); } + s_logger.info("Trying to download template"); if (!downloader.downloadTemplate()) { s_logger.warn("Couldn't download template"); @@ -1730,6 +1741,14 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(true, info.getSize(), info.getInstallPath()); } + /** + * True if location exists + */ + private boolean isLocationAccessible(String temporaryDownloadPath) { + File dir = new File(temporaryDownloadPath); + return dir.exists(); + } + /** * Perform a free space check on the host for downloading the direct download templates * @param templateSize template size obtained from remote server when registering the template (in bytes) From 56a5da010b60c19de5e6ed2d9050d2219728010c Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 17 Jan 2020 09:45:39 -0300 Subject: [PATCH 15/30] In progress - refactor and extension --- .../download/DirectTemplateDownloader.java | 6 +- .../DirectTemplateDownloaderImpl.java | 12 ++-- .../HttpDirectTemplateDownloader.java | 33 ++++++----- .../HttpsDirectTemplateDownloader.java | 48 ++++++++-------- .../MetalinkDirectTemplateDownloader.java | 26 +++++---- .../download/NfsDirectTemplateDownloader.java | 9 +-- .../DirectTemplateDownloaderImplTest.java | 8 --- .../kvm/storage/IscsiAdmStorageAdaptor.java | 5 ++ .../kvm/storage/KVMStoragePoolManager.java | 5 ++ .../kvm/storage/KVMStorageProcessor.java | 47 +++++++++------- .../kvm/storage/LibvirtStorageAdaptor.java | 56 ++++++++++++++++++- .../kvm/storage/ManagedNfsStorageAdaptor.java | 5 ++ .../kvm/storage/StorageAdaptor.java | 7 +++ 13 files changed, 178 insertions(+), 89 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java index a88b4526e9d9..db32e31c232a 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java @@ -19,6 +19,8 @@ package com.cloud.agent.direct.download; +import com.cloud.utils.Pair; + public interface DirectTemplateDownloader { class DirectTemplateInformation { @@ -47,9 +49,9 @@ public String getChecksum() { /** * Perform template download to pool specified on downloader creation - * @return true if successful, false if not + * @return (true if successful, false if not, download file path) */ - boolean downloadTemplate(); + Pair downloadTemplate(); /** * Perform extraction (if necessary) and installation of previously downloaded template diff --git a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java index 419ab7d1bbde..28e6a8cf2ef1 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java @@ -39,13 +39,17 @@ public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDown private String installPath; private String checksum; private boolean redownload = false; + private String temporaryDownloadPath; + public static final Logger s_logger = Logger.getLogger(DirectTemplateDownloaderImpl.class.getName()); - protected DirectTemplateDownloaderImpl(final String url, final String destPoolPath, final Long templateId, final String checksum) { + protected DirectTemplateDownloaderImpl(final String url, final String destPoolPath, final Long templateId, + final String checksum, final String temporaryDownloadPath) { this.url = url; this.destPoolPath = destPoolPath; this.templateId = templateId; this.checksum = checksum; + this.temporaryDownloadPath = temporaryDownloadPath; } private static String directDownloadDir = "template"; @@ -53,10 +57,10 @@ protected DirectTemplateDownloaderImpl(final String url, final String destPoolPa /** * Return direct download temporary path to download template */ - protected static String getDirectDownloadTempPath(Long templateId) { + protected String getDirectDownloadTempPath(Long templateId) { String templateIdAsString = String.valueOf(templateId); - return directDownloadDir + File.separator + templateIdAsString.substring(0,1) + - File.separator + templateIdAsString; + return this.temporaryDownloadPath + File.separator + directDownloadDir + File.separator + + templateIdAsString.substring(0,1) + File.separator + templateIdAsString; } /** diff --git a/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java index f91af40dd404..fc2360344048 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java @@ -27,6 +27,8 @@ import java.util.HashMap; import java.util.Map; +import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.commons.collections.MapUtils; import org.apache.commons.httpclient.HttpClient; import org.apache.commons.httpclient.HttpStatus; @@ -35,8 +37,6 @@ import org.apache.commons.io.IOUtils; import org.apache.log4j.Logger; -import com.cloud.utils.exception.CloudRuntimeException; - public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl { protected HttpClient client; @@ -45,20 +45,25 @@ public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl { protected GetMethod request; protected Map reqHeaders = new HashMap<>(); - public HttpDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, Integer connectTimeout, Integer soTimeout) { - super(url, destPoolPath, templateId, checksum); + public HttpDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, + Map headers, Integer connectTimeout, Integer soTimeout, String downloadPath) { + super(url, destPoolPath, templateId, checksum, downloadPath); s_httpClientManager.getParams().setConnectionTimeout(connectTimeout == null ? 5000 : connectTimeout); s_httpClientManager.getParams().setSoTimeout(soTimeout == null ? 5000 : soTimeout); client = new HttpClient(s_httpClientManager); request = createRequest(url, headers); String downloadDir = getDirectDownloadTempPath(templateId); - createTemporaryDirectoryAndFile(downloadDir); + File tempFile = createTemporaryDirectoryAndFile(downloadDir); + setDownloadedFilePath(tempFile.getAbsolutePath()); } - protected void createTemporaryDirectoryAndFile(String downloadDir) { - createFolder(getDestPoolPath() + File.separator + downloadDir); - File f = new File(getDestPoolPath() + File.separator + downloadDir + File.separator + getFileNameFromUrl()); - setDownloadedFilePath(f.getAbsolutePath()); + /** + * Create download directory (if it does not exist) and set the download file + * @return + */ + protected File createTemporaryDirectoryAndFile(String downloadDir) { + createFolder(downloadDir); + return new File(downloadDir + File.separator + getFileNameFromUrl()); } protected GetMethod createRequest(String downloadUrl, Map headers) { @@ -74,12 +79,12 @@ protected GetMethod createRequest(String downloadUrl, Map header } @Override - public boolean downloadTemplate() { + public Pair downloadTemplate() { try { int status = client.executeMethod(request); if (status != HttpStatus.SC_OK) { s_logger.warn("Not able to download template, status code: " + status); - return false; + return new Pair<>(false, null); } return performDownload(); } catch (IOException e) { @@ -89,7 +94,7 @@ public boolean downloadTemplate() { } } - protected boolean performDownload() { + protected Pair performDownload() { s_logger.info("Downloading template " + getTemplateId() + " from " + getUrl() + " to: " + getDownloadedFilePath()); try ( InputStream in = request.getResponseBodyAsStream(); @@ -98,8 +103,8 @@ protected boolean performDownload() { IOUtils.copy(in, out); } catch (IOException e) { s_logger.error("Error downloading template " + getTemplateId() + " due to: " + e.getMessage()); - return false; + return new Pair<>(false, null); } - return true; + return new Pair<>(true, getDownloadedFilePath()); } } \ No newline at end of file diff --git a/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java index f320846e75d9..d788310f68e7 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java @@ -19,6 +19,23 @@ package com.cloud.agent.direct.download; +import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.Script; +import org.apache.commons.io.IOUtils; +import org.apache.http.HttpEntity; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.commons.collections.MapUtils; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.conn.ssl.TrustSelfSignedStrategy; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.ssl.SSLContexts; + +import javax.net.ssl.SSLContext; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; @@ -32,31 +49,14 @@ import java.security.cert.CertificateException; import java.util.Map; -import javax.net.ssl.SSLContext; - -import org.apache.commons.collections.MapUtils; -import org.apache.commons.io.IOUtils; -import org.apache.http.HttpEntity; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; -import org.apache.http.conn.ssl.TrustSelfSignedStrategy; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.ssl.SSLContexts; - -import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.script.Script; - public class HttpsDirectTemplateDownloader extends HttpDirectTemplateDownloader { private CloseableHttpClient httpsClient; private HttpUriRequest req; - public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, Integer connectTimeout, Integer soTimeout, Integer connectionRequestTimeout) { - super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout); + public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, + Integer connectTimeout, Integer soTimeout, Integer connectionRequestTimeout, String temporaryDownloadPath) { + super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout, temporaryDownloadPath); SSLContext sslcontext = null; try { sslcontext = getSSLContext(); @@ -98,7 +98,7 @@ private SSLContext getSSLContext() throws KeyStoreException, NoSuchAlgorithmExce } @Override - public boolean downloadTemplate() { + public Pair downloadTemplate() { CloseableHttpResponse response; try { response = httpsClient.execute(req); @@ -111,7 +111,7 @@ public boolean downloadTemplate() { /** * Consume response and persist it on getDownloadedFilePath() file */ - protected boolean consumeResponse(CloseableHttpResponse response) { + protected Pair consumeResponse(CloseableHttpResponse response) { s_logger.info("Downloading template " + getTemplateId() + " from " + getUrl() + " to: " + getDownloadedFilePath()); if (response.getStatusLine().getStatusCode() != 200) { throw new CloudRuntimeException("Error on HTTPS response"); @@ -123,9 +123,9 @@ protected boolean consumeResponse(CloseableHttpResponse response) { IOUtils.copy(in, out); } catch (Exception e) { s_logger.error("Error parsing response for template " + getTemplateId() + " due to: " + e.getMessage()); - return false; + return new Pair<>(false, null); } - return true; + return new Pair<>(true, getDownloadedFilePath()); } } diff --git a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java index 0a571015bee7..2f2d1d2d8611 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java @@ -18,17 +18,17 @@ // package com.cloud.agent.direct.download; -import java.io.File; -import java.util.List; -import java.util.Map; -import java.util.Random; - +import com.cloud.utils.Pair; +import com.cloud.utils.UriUtils; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; -import com.cloud.utils.UriUtils; -import com.cloud.utils.exception.CloudRuntimeException; +import java.io.File; +import java.util.List; +import java.util.Map; +import java.util.Random; public class MetalinkDirectTemplateDownloader extends HttpDirectTemplateDownloader { @@ -38,8 +38,9 @@ public class MetalinkDirectTemplateDownloader extends HttpDirectTemplateDownload private Random random = new Random(); private static final Logger s_logger = Logger.getLogger(MetalinkDirectTemplateDownloader.class.getName()); - public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long templateId, String checksum, Map headers, Integer connectTimeout, Integer soTimeout) { - super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout); + public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long templateId, String checksum, + Map headers, Integer connectTimeout, Integer soTimeout, String downloadPath) { + super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout, downloadPath); metalinkUrl = url; metalinkUrls = UriUtils.getMetalinkUrls(metalinkUrl); metalinkChecksums = UriUtils.getMetalinkChecksums(metalinkUrl); @@ -53,7 +54,7 @@ public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long te } @Override - public boolean downloadTemplate() { + public Pair downloadTemplate() { if (StringUtils.isBlank(getUrl())) { throw new CloudRuntimeException("Download url has not been set, aborting"); } @@ -73,7 +74,8 @@ public boolean downloadTemplate() { } setDownloadedFilePath(f.getAbsolutePath()); request = createRequest(getUrl(), reqHeaders); - downloaded = super.downloadTemplate(); + Pair downloadResult = super.downloadTemplate(); + downloaded = downloadResult.first(); if (downloaded) { s_logger.info("Successfully downloaded template from url: " + getUrl()); } @@ -84,7 +86,7 @@ public boolean downloadTemplate() { i++; } while (!downloaded && !isRedownload() && i < metalinkUrls.size()); - return downloaded; + return new Pair<>(downloaded, getDownloadedFilePath()); } @Override diff --git a/agent/src/main/java/com/cloud/agent/direct/download/NfsDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/NfsDirectTemplateDownloader.java index 16901afedf1b..932477031d6a 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/NfsDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/NfsDirectTemplateDownloader.java @@ -18,6 +18,7 @@ // package com.cloud.agent.direct.download; +import com.cloud.utils.Pair; import com.cloud.utils.UriUtils; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; @@ -51,13 +52,13 @@ private void parseUrl() { } } - public NfsDirectTemplateDownloader(String url, String destPool, Long templateId, String checksum) { - super(url, destPool, templateId, checksum); + public NfsDirectTemplateDownloader(String url, String destPool, Long templateId, String checksum, String downloadPath) { + super(url, destPool, templateId, checksum, downloadPath); parseUrl(); } @Override - public boolean downloadTemplate() { + public Pair downloadTemplate() { String mountSrcUuid = UUID.randomUUID().toString(); String mount = String.format(mountCommand, srcHost + ":" + srcPath, "/mnt/" + mountSrcUuid); Script.runSimpleBashScript(mount); @@ -65,6 +66,6 @@ public boolean downloadTemplate() { setDownloadedFilePath(downloadDir + File.separator + getFileNameFromUrl()); Script.runSimpleBashScript("cp /mnt/" + mountSrcUuid + srcPath + " " + getDownloadedFilePath()); Script.runSimpleBashScript("umount /mnt/" + mountSrcUuid); - return true; + return new Pair<>(true, getDownloadedFilePath()); } } diff --git a/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java b/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java index b244d02f4993..42a671301441 100644 --- a/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java +++ b/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java @@ -18,8 +18,6 @@ // package com.cloud.agent.direct.download; -import org.junit.Assert; -import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.runners.MockitoJUnitRunner; @@ -27,10 +25,4 @@ public class DirectTemplateDownloaderImplTest { private static final Long templateId = 202l; - - @Test - public void testGetDirectDownloadTempPath() { - String path = DirectTemplateDownloaderImpl.getDirectDownloadTempPath(templateId); - Assert.assertEquals("template/2/202", path); - } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java index bad0151ed8f4..90f2f7112d19 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java @@ -445,4 +445,9 @@ public boolean createFolder(String uuid, String path) { public KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, String name, PhysicalDiskFormat format, long size, KVMStoragePool destPool, int timeout) { return null; } + + @Override + public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool) { + return null; + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java index c1f73d7a088d..32df7ebdeab1 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java @@ -405,4 +405,9 @@ public KVMPhysicalDisk createDiskWithTemplateBacking(KVMPhysicalDisk template, S return adaptor.createDiskFromTemplateBacking(template, name, format, size, destPool, timeout); } + public KVMPhysicalDisk createPhysicalDiskFromDirectDownloadTemplate(String templateFilePath, KVMStoragePool destPool, int timeout) { + StorageAdaptor adaptor = getStorageAdaptor(destPool.getType()); + return adaptor.createTemplateFromDirectDownloadFile(templateFilePath, destPool); + } + } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 15600675ab9b..de73876afd84 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -36,6 +36,7 @@ import javax.naming.ConfigurationException; +import com.cloud.utils.Pair; import org.apache.cloudstack.agent.directdownload.DirectDownloadAnswer; import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.HttpDirectDownloadCommand; @@ -89,7 +90,6 @@ import com.cloud.agent.api.to.NfsTO; import com.cloud.agent.api.to.S3TO; import com.cloud.agent.direct.download.DirectTemplateDownloader; -import com.cloud.agent.direct.download.DirectTemplateDownloader.DirectTemplateInformation; import com.cloud.agent.direct.download.HttpDirectTemplateDownloader; import com.cloud.agent.direct.download.HttpsDirectTemplateDownloader; import com.cloud.agent.direct.download.MetalinkDirectTemplateDownloader; @@ -1676,15 +1676,18 @@ public Answer forgetObject(final ForgetObjectCmd cmd) { /** * Get direct template downloader from direct download command and destination pool */ - private DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDownloadCommand cmd, KVMStoragePool destPool) { + private DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDownloadCommand cmd, + KVMStoragePool destPool, + String temporaryDownloadPath) { if (cmd instanceof HttpDirectDownloadCommand) { - return new HttpDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout()); + return new HttpDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout(), temporaryDownloadPath); } else if (cmd instanceof HttpsDirectDownloadCommand) { - return new HttpsDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout(), cmd.getConnectionRequestTimeout()); + return new HttpsDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout(), cmd.getConnectionRequestTimeout(), temporaryDownloadPath); } else if (cmd instanceof NfsDirectDownloadCommand) { - return new NfsDirectTemplateDownloader(cmd.getUrl(), destPool.getLocalPath(), cmd.getTemplateId(), cmd.getChecksum()); + return new NfsDirectTemplateDownloader(cmd.getUrl(), destPool.getLocalPath(), cmd.getTemplateId(), cmd.getChecksum(), temporaryDownloadPath); } else if (cmd instanceof MetalinkDirectDownloadCommand) { - return new MetalinkDirectTemplateDownloader(cmd.getUrl(), destPool.getLocalPath(), cmd.getTemplateId(), cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout()); + return new MetalinkDirectTemplateDownloader(cmd.getUrl(), destPool.getLocalPath(), cmd.getTemplateId(), + cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout(), temporaryDownloadPath); } else { throw new IllegalArgumentException("Unsupported protocol, please provide HTTP(S), NFS or a metalink"); } @@ -1694,13 +1697,9 @@ private DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDo public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) { final PrimaryDataStoreTO pool = cmd.getDestPool(); KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); - DirectTemplateDownloader downloader; - - try { - downloader = getDirectTemplateDownloaderFromCommand(cmd, destPool); - } catch (IllegalArgumentException e) { - return new DirectDownloadAnswer(false, "Unable to create direct downloader: " + e.getMessage(), true); - } + DirectTemplateDownloader downloader = null; + String path; + long size = 0L; try { s_logger.info("Verifying temporary location for downloading the template exists on the host"); @@ -1719,26 +1718,34 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, msg, true); } + downloader = getDirectTemplateDownloaderFromCommand(cmd, destPool, temporaryDownloadPath); s_logger.info("Trying to download template"); - if (!downloader.downloadTemplate()) { + Pair result = downloader.downloadTemplate(); + if (!result.first()) { s_logger.warn("Couldn't download template"); return new DirectDownloadAnswer(false, "Unable to download template", true); } - if (!downloader.validateChecksum()) { + String tempFilePath = result.second(); + /**if (!downloader.validateChecksum()) { s_logger.warn("Couldn't validate template checksum"); return new DirectDownloadAnswer(false, "Checksum validation failed", false); - } - if (!downloader.extractAndInstallDownloadedTemplate()) { + }**/ + KVMPhysicalDisk template = storagePoolMgr.createPhysicalDiskFromDirectDownloadTemplate(tempFilePath, destPool, 100); + path = template.getPath(); + size = template.getSize(); + /**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); + } catch (IllegalArgumentException e) { + return new DirectDownloadAnswer(false, "Unable to create direct downloader: " + e.getMessage(), true); } - DirectTemplateInformation info = downloader.getTemplateInformation(); - return new DirectDownloadAnswer(true, info.getSize(), info.getInstallPath()); + //DirectTemplateInformation info = downloader.getTemplateInformation(); + return new DirectDownloadAnswer(true, size, path); } /** diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index f858a4f15772..e290ba94f173 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -122,6 +122,60 @@ public KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, S return disk; } + /** + * Checks if downloaded template is extractable + * @return true if it should be extracted, false if not + */ + private boolean isTemplateExtractable(String templatePath) { + String type = Script.runSimpleBashScript("file " + templatePath + " | awk -F' ' '{print $2}'"); + return type.equalsIgnoreCase("bzip2") || type.equalsIgnoreCase("gzip") || type.equalsIgnoreCase("zip"); + } + + /** + * Return extract command to execute given downloaded file + * @param downloadedTemplateFile + * @param templateUuid + */ + private String getExtractCommandForDownloadedFile(String downloadedTemplateFile, String templateUuid) { + if (downloadedTemplateFile.endsWith(".zip")) { + return "unzip -p " + downloadedTemplateFile + " | cat > " + templateUuid; + } else if (downloadedTemplateFile.endsWith(".bz2")) { + return "bunzip2 -c " + downloadedTemplateFile + " > " + templateUuid; + } else if (downloadedTemplateFile.endsWith(".gz")) { + return "gunzip -c " + downloadedTemplateFile + " > " + templateUuid; + } else { + throw new CloudRuntimeException("Unable to extract template " + downloadedTemplateFile); + } + } + + /** + * Extract downloaded template into installPath, remove compressed file + */ + private void extractDownloadedTemplate(String downloadedTemplateFile, KVMStoragePool destPool, String templateUuid) { + String destinationFile = destPool.getLocalPath() + File.separator + templateUuid; + String extractCommand = getExtractCommandForDownloadedFile(downloadedTemplateFile, destinationFile); + Script.runSimpleBashScript(extractCommand); + Script.runSimpleBashScript("rm -f " + downloadedTemplateFile); + } + + @Override + public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool) { + File sourceFile = new File(templateFilePath); + if (!sourceFile.exists()) { + throw new CloudRuntimeException("Direct download template file " + sourceFile + " does not exist on this host"); + } + String templateUuid = UUID.randomUUID().toString(); + if (destPool.getType() == StoragePoolType.NetworkFilesystem || destPool.getType() == StoragePoolType.Filesystem + || destPool.getType() == StoragePoolType.SharedMountPoint) { + if (isTemplateExtractable(templateFilePath)) { + extractDownloadedTemplate(templateFilePath, destPool, templateUuid); + } else { + Script.runSimpleBashScript("mv " + templateFilePath + " " + templateUuid); + } + } + return destPool.getPhysicalDisk(templateUuid); + } + public StorageVol getVolume(StoragePool pool, String volName) { StorageVol vol = null; @@ -1198,7 +1252,7 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMSt if (disk.getFormat() == PhysicalDiskFormat.TAR) { newDisk = destPool.createPhysicalDisk(name, PhysicalDiskFormat.DIR, Storage.ProvisioningType.THIN, disk.getVirtualSize()); } else { - newDisk = destPool.createPhysicalDisk(name, Storage.ProvisioningType.THIN, disk.getVirtualSize()); + newDisk = destPool.createPhysicalDisk(name, Storage.ProvisioningType.THIN, disk.getVirtualSize()); } } else { newDisk = new KVMPhysicalDisk(destPool.getSourceDir() + "/" + name, name, destPool); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java index 309308ae9c15..fe583e385cbe 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java @@ -318,6 +318,11 @@ public KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, S return null; } + @Override + public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool) { + return null; + } + @Override public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, PhysicalDiskFormat format, ProvisioningType provisioningType, long size) { return null; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java index a3c1387aa6b6..86392da5ea3d 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java @@ -81,4 +81,11 @@ public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, String name, PhysicalDiskFormat format, long size, KVMStoragePool destPool, int timeout); + + /** + * Create physical disk on Primary Storage from direct download template on the host (in temporary location) + * @param templateFilePath + * @param destPool + */ + KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool); } From fe8f72c873bc419b3a80a327753a322230b42757 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 21 Jan 2020 00:07:15 -0300 Subject: [PATCH 16/30] Refactor and fix --- .../download/DirectTemplateDownloader.java | 36 ----------- .../DirectTemplateDownloaderImpl.java | 61 ------------------- .../kvm/storage/KVMStorageProcessor.java | 22 +++---- 3 files changed, 7 insertions(+), 112 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java index db32e31c232a..32b84a34436d 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java @@ -23,48 +23,12 @@ public interface DirectTemplateDownloader { - class DirectTemplateInformation { - private String installPath; - private Long size; - private String checksum; - - public DirectTemplateInformation(String installPath, Long size, String checksum) { - this.installPath = installPath; - this.size = size; - this.checksum = checksum; - } - - public String getInstallPath() { - return installPath; - } - - public Long getSize() { - return size; - } - - public String getChecksum() { - return checksum; - } - } - /** * Perform template download to pool specified on downloader creation * @return (true if successful, false if not, download file path) */ Pair downloadTemplate(); - /** - * Perform extraction (if necessary) and installation of previously downloaded template - * @return true if successful, false if not - */ - boolean extractAndInstallDownloadedTemplate(); - - /** - * Get template information after it is properly installed on pool - * @return template information - */ - DirectTemplateInformation getTemplateInformation(); - /** * Perform checksum validation of previously downloadeed template * @return true if successful, false if not diff --git a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java index 28e6a8cf2ef1..01603b941024 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java @@ -19,7 +19,6 @@ package com.cloud.agent.direct.download; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.script.Script; import org.apache.cloudstack.utils.security.DigestHelper; import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; @@ -28,7 +27,6 @@ import java.io.FileInputStream; import java.io.IOException; import java.security.NoSuchAlgorithmException; -import java.util.UUID; public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDownloader { @@ -36,7 +34,6 @@ public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDown private String destPoolPath; private Long templateId; private String downloadedFilePath; - private String installPath; private String checksum; private boolean redownload = false; private String temporaryDownloadPath; @@ -117,64 +114,6 @@ public String getFileNameFromUrl() { return urlParts[urlParts.length - 1]; } - /** - * Checks if downloaded template is extractable - * @return true if it should be extracted, false if not - */ - private boolean isTemplateExtractable() { - String type = Script.runSimpleBashScript("file " + downloadedFilePath + " | awk -F' ' '{print $2}'"); - return type.equalsIgnoreCase("bzip2") || type.equalsIgnoreCase("gzip") || type.equalsIgnoreCase("zip"); - } - - @Override - public boolean extractAndInstallDownloadedTemplate() { - installPath = UUID.randomUUID().toString(); - if (isTemplateExtractable()) { - extractDownloadedTemplate(); - } else { - Script.runSimpleBashScript("mv " + downloadedFilePath + " " + getInstallFullPath()); - } - return true; - } - - /** - * Return install full path - */ - private String getInstallFullPath() { - return destPoolPath + File.separator + installPath; - } - - /** - * Return extract command to execute given downloaded file - */ - private String getExtractCommandForDownloadedFile() { - if (downloadedFilePath.endsWith(".zip")) { - return "unzip -p " + downloadedFilePath + " | cat > " + getInstallFullPath(); - } else if (downloadedFilePath.endsWith(".bz2")) { - return "bunzip2 -c " + downloadedFilePath + " > " + getInstallFullPath(); - } else if (downloadedFilePath.endsWith(".gz")) { - return "gunzip -c " + downloadedFilePath + " > " + getInstallFullPath(); - } else { - throw new CloudRuntimeException("Unable to extract template " + templateId + " on " + downloadedFilePath); - } - } - - /** - * Extract downloaded template into installPath, remove compressed file - */ - private void extractDownloadedTemplate() { - String extractCommand = getExtractCommandForDownloadedFile(); - Script.runSimpleBashScript(extractCommand); - Script.runSimpleBashScript("rm -f " + downloadedFilePath); - } - - @Override - public DirectTemplateInformation getTemplateInformation() { - String sizeResult = Script.runSimpleBashScript("ls -als " + getInstallFullPath() + " | awk '{print $1}'"); - long size = Long.parseLong(sizeResult); - return new DirectTemplateInformation(installPath, size, checksum); - } - @Override public boolean validateChecksum() { if (StringUtils.isNotBlank(checksum)) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index de73876afd84..6f3f3fbc3bb7 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1696,10 +1696,8 @@ private DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDo @Override public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) { final PrimaryDataStoreTO pool = cmd.getDestPool(); - KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); - DirectTemplateDownloader downloader = null; - String path; - long size = 0L; + DirectTemplateDownloader downloader; + KVMPhysicalDisk template; try { s_logger.info("Verifying temporary location for downloading the template exists on the host"); @@ -1718,6 +1716,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, msg, true); } + KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); downloader = getDirectTemplateDownloaderFromCommand(cmd, destPool, temporaryDownloadPath); s_logger.info("Trying to download template"); Pair result = downloader.downloadTemplate(); @@ -1726,17 +1725,11 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, "Unable to download template", true); } String tempFilePath = result.second(); - /**if (!downloader.validateChecksum()) { + if (!downloader.validateChecksum()) { s_logger.warn("Couldn't validate template checksum"); return new DirectDownloadAnswer(false, "Checksum validation failed", false); - }**/ - KVMPhysicalDisk template = storagePoolMgr.createPhysicalDiskFromDirectDownloadTemplate(tempFilePath, destPool, 100); - path = template.getPath(); - size = template.getSize(); - /**if (!downloader.extractAndInstallDownloadedTemplate()) { - s_logger.warn("Couldn't extract and install template"); - return new DirectDownloadAnswer(false, "Extraction and installation failed", false); - }**/ + } + template = storagePoolMgr.createPhysicalDiskFromDirectDownloadTemplate(tempFilePath, destPool, 100); } 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); @@ -1744,8 +1737,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, "Unable to create direct downloader: " + e.getMessage(), true); } - //DirectTemplateInformation info = downloader.getTemplateInformation(); - return new DirectDownloadAnswer(true, size, path); + return new DirectDownloadAnswer(true, template.getSize(), template.getPath()); } /** From 40fb1492b88da59e58496fbf8d22cdcc2d72b204 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 22 Jan 2020 02:29:24 -0300 Subject: [PATCH 17/30] Last fixes and marvin tests --- .../DirectTemplateDownloaderImpl.java | 2 +- .../MetalinkDirectTemplateDownloader.java | 3 +- .../kvm/storage/KVMStorageProcessor.java | 2 +- .../integration/smoke/test_direct_download.py | 148 ++++++++++++++++-- 4 files changed, 138 insertions(+), 17 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java index 01603b941024..9c150e92a989 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java @@ -36,7 +36,7 @@ public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDown private String downloadedFilePath; private String checksum; private boolean redownload = false; - private String temporaryDownloadPath; + protected String temporaryDownloadPath; public static final Logger s_logger = Logger.getLogger(DirectTemplateDownloaderImpl.class.getName()); diff --git a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java index 2f2d1d2d8611..3a04bd2b5a01 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java @@ -58,7 +58,6 @@ public Pair downloadTemplate() { if (StringUtils.isBlank(getUrl())) { throw new CloudRuntimeException("Download url has not been set, aborting"); } - String downloadDir = getDirectDownloadTempPath(getTemplateId()); boolean downloaded = false; int i = 0; do { @@ -67,7 +66,7 @@ public Pair downloadTemplate() { } s_logger.info("Trying to download template from url: " + getUrl()); try { - File f = new File(getDestPoolPath() + File.separator + downloadDir + File.separator + getFileNameFromUrl()); + File f = new File(getDownloadedFilePath()); if (f.exists()) { f.delete(); f.createNewFile(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 6f3f3fbc3bb7..8a4fdf0045fa 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1737,7 +1737,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, "Unable to create direct downloader: " + e.getMessage(), true); } - return new DirectDownloadAnswer(true, template.getSize(), template.getPath()); + return new DirectDownloadAnswer(true, template.getSize(), template.getName()); } /** diff --git a/test/integration/smoke/test_direct_download.py b/test/integration/smoke/test_direct_download.py index 132deb4f6989..a6c98e52286e 100644 --- a/test/integration/smoke/test_direct_download.py +++ b/test/integration/smoke/test_direct_download.py @@ -23,7 +23,8 @@ NetworkOffering, Network, Template, - VirtualMachine) + VirtualMachine, + StoragePool) from marvin.lib.common import (get_pod, get_zone) from nose.plugins.attrib import attr @@ -160,11 +161,15 @@ def setUpClass(cls): cls.services = cls.testClient.getParsedTestDataConfig() cls._cleanup = [] - cls.hypervisorNotSupported = False - if cls.hypervisor.lower() not in ['kvm', 'lxc']: - cls.hypervisorNotSupported = True + cls.hypervisorSupported = False + cls.nfsStorageFound = False + cls.localStorageFound = False + cls.sharedMountPointFound = False - if not cls.hypervisorNotSupported: + if cls.hypervisor.lower() in ['kvm', 'lxc']: + cls.hypervisorSupported = True + + if cls.hypervisorSupported: cls.services["test_templates"]["kvm"]["directdownload"] = "true" cls.template = Template.register(cls.apiclient, cls.services["test_templates"]["kvm"], zoneid=cls.zone.id, hypervisor=cls.hypervisor) @@ -192,6 +197,25 @@ def setUpClass(cls): ) cls._cleanup.append(cls.l2_network) cls._cleanup.append(cls.network_offering) + + storage_pools = StoragePool.list( + cls.apiclient, + zoneid=cls.zone.id + ) + for pool in storage_pools: + if not cls.nfsStorageFound and pool.type == "NetworkFilesystem": + cls.nfsStorageFound = True + cls.nfsPoolId = pool.id + elif not cls.localStorageFound and pool.type == "Filesystem": + cls.localStorageFound = True + cls.localPoolId = pool.id + elif not cls.sharedMountPointFound and pool.type == "SharedMountPoint": + cls.sharedMountPointFound = True + cls.sharedPoolId = pool.id + + cls.nfsKvmNotAvailable = not cls.hypervisorSupported or not cls.nfsStorageFound + cls.localStorageKvmNotAvailable = not cls.hypervisorSupported or not cls.localStorageFound + cls.sharedMountPointKvmNotAvailable = not cls.hypervisorSupported or not cls.sharedMountPointFound return @classmethod @@ -215,26 +239,124 @@ def tearDown(self): raise Exception("Warning: Exception during cleanup : %s" % e) return - @skipTestIf("hypervisorNotSupported") + def getCurrentStoragePoolTags(self, poolId): + local_pool = StoragePool.list( + self.apiclient, + id=poolId + ) + return local_pool[0].tags + + def updateStoragePoolTags(self, poolId, tags): + StoragePool.update( + self.apiclient, + id=poolId, + tags=tags + ) + + def createServiceOffering(self, name, type, tags): + services = { + "cpunumber": 1, + "cpuspeed": 512, + "memory": 256, + "displaytext": name, + "name": name, + "storagetype": type + } + return ServiceOffering.create( + self.apiclient, + services, + tags=tags + ) + + + @skipTestIf("nfsKvmNotAvailable") @attr(tags=["advanced", "basic", "eip", "advancedns", "sg"], required_hardware="false") - def test_01_deploy_vm_from_direct_download_template(self): - """Test Deploy VM from direct download template + def test_01_deploy_vm_from_direct_download_template_nfs_storage(self): + """Test Deploy VM from direct download template on NFS storage """ - # Validate the following - # 1. Register direct download template - # 2. Deploy VM from direct download template + # Create service offering for local storage using storage tags + tags = self.getCurrentStoragePoolTags(self.nfsPoolId) + test_tag = "marvin_test_nfs_storage_direct_download" + self.updateStoragePoolTags(self.nfsPoolId, test_tag) + nfs_storage_offering = self.createServiceOffering("TestNFSStorageDirectDownload", "shared", test_tag) vm = VirtualMachine.create( self.apiclient, self.services["virtual_machine"], - serviceofferingid=self.service_offering.id, + serviceofferingid=nfs_storage_offering.id, networkids=self.l2_network.id ) self.assertEqual( vm.state, "Running", - "Check VM deployed from direct download template is running" + "Check VM deployed from direct download template is running on NFS storage" ) + + # Revert storage tags for the storage pool used in this test + self.updateStoragePoolTags(self.nfsPoolId, tags) + self.cleanup.append(vm) + self.cleanup.append(nfs_storage_offering) + return + + @skipTestIf("localStorageKvmNotAvailable") + @attr(tags=["advanced", "basic", "eip", "advancedns", "sg"], required_hardware="false") + def test_02_deploy_vm_from_direct_download_template_local_storage(self): + """Test Deploy VM from direct download template on local storage + """ + + # Create service offering for local storage using storage tags + tags = self.getCurrentStoragePoolTags(self.localPoolId) + test_tag = "marvin_test_local_storage_direct_download" + self.updateStoragePoolTags(self.localPoolId, test_tag) + local_storage_offering = self.createServiceOffering("TestLocalStorageDirectDownload", "local", test_tag) + + # Deploy VM + vm = VirtualMachine.create( + self.apiclient, + self.services["virtual_machine"], + serviceofferingid=local_storage_offering.id, + networkids=self.l2_network.id, + ) + self.assertEqual( + vm.state, + "Running", + "Check VM deployed from direct download template is running on local storage" + ) + + # Revert storage tags for the storage pool used in this test + self.updateStoragePoolTags(self.localPoolId, tags) + self.cleanup.append(vm) + self.cleanup.append(local_storage_offering) + return + + @skipTestIf("sharedMountPointKvmNotAvailable") + @attr(tags=["advanced", "basic", "eip", "advancedns", "sg"], required_hardware="false") + def test_03_deploy_vm_from_direct_download_template_shared_mount_point_storage(self): + """Test Deploy VM from direct download template on shared mount point + """ + + # Create service offering for local storage using storage tags + tags = self.getCurrentStoragePoolTags(self.sharedPoolId) + test_tag = "marvin_test_shared_mount_point_storage_direct_download" + self.updateStoragePoolTags(self.sharedPoolId, test_tag) + shared_offering = self.createServiceOffering("TestSharedMountPointStorageDirectDownload", "shared", test_tag) + + # Deploy VM + vm = VirtualMachine.create( + self.apiclient, + self.services["virtual_machine"], + serviceofferingid=shared_offering.id, + networkids=self.l2_network.id, + ) + self.assertEqual( + vm.state, + "Running", + "Check VM deployed from direct download template is running on shared mount point" + ) + + # Revert storage tags for the storage pool used in this test + self.updateStoragePoolTags(self.sharedPoolId, tags) self.cleanup.append(vm) + self.cleanup.append(shared_offering) return From d31ed3e19b264f672683b5adcc98fd7064bc5bed Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 22 Jan 2020 02:50:59 -0300 Subject: [PATCH 18/30] Remove unused test file --- .../DirectTemplateDownloaderImplTest.java | 28 ------------------- 1 file changed, 28 deletions(-) delete mode 100644 agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java diff --git a/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java b/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java deleted file mode 100644 index 42a671301441..000000000000 --- a/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java +++ /dev/null @@ -1,28 +0,0 @@ -// -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. -// -package com.cloud.agent.direct.download; - -import org.junit.runner.RunWith; -import org.mockito.runners.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class DirectTemplateDownloaderImplTest { - - private static final Long templateId = 202l; -} From 1415ad8cd82e9c8444ae88bfd6e438300d079f9f Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 22 Jan 2020 07:19:37 -0300 Subject: [PATCH 19/30] Improve logging --- .../cloud/hypervisor/kvm/storage/KVMStorageProcessor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 8a4fdf0045fa..a2858540d13f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1700,7 +1700,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) KVMPhysicalDisk template; try { - s_logger.info("Verifying temporary location for downloading the template exists on the host"); + s_logger.debug("Verifying temporary location for downloading the template exists on the host"); String temporaryDownloadPath = resource.getDirectDownloadTemporaryDownloadPath(); if (!isLocationAccessible(temporaryDownloadPath)) { String msg = "The temporary location path for downloading templates does not exist: " + @@ -1709,7 +1709,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, msg, true); } - s_logger.info("Checking for free space on the host for downloading the template"); + s_logger.debug("Checking for free space on the host for downloading the template"); if (!isEnoughSpaceForDownloadTemplateOnTemporaryLocation(cmd.getTemplateSize())) { String msg = "Not enough space on the defined temporary location to download the template " + cmd.getTemplateId(); s_logger.error(msg); @@ -1718,7 +1718,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); downloader = getDirectTemplateDownloaderFromCommand(cmd, destPool, temporaryDownloadPath); - s_logger.info("Trying to download template"); + s_logger.debug("Trying to download template"); Pair result = downloader.downloadTemplate(); if (!result.first()) { s_logger.warn("Couldn't download template"); From 0223450d07eb0d365cbca3aa2fcbbaaba6122b1e Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 29 Jan 2020 01:24:53 -0300 Subject: [PATCH 20/30] Change default path for direct download --- agent/conf/agent.properties | 2 +- .../cloud/hypervisor/kvm/resource/LibvirtComputingResource.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties index 525c19f8a592..24592387b09e 100644 --- a/agent/conf/agent.properties +++ b/agent/conf/agent.properties @@ -116,7 +116,7 @@ domr.scripts.dir=scripts/network/domr/kvm hypervisor.type=kvm # This parameter specifies a directory on the host local storage for temporary storing direct download templates -#direct.download.temporary.download.location=/var/lib/libvirt/images/direct-download +#direct.download.temporary.download.location=/var/lib/libvirt/images # set the hypervisor URI. Usually there is no need for changing this # For KVM: qemu:///system diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index c885d54fedb9..77616af86e83 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -580,7 +580,7 @@ private Map getDeveloperProperties() throws ConfigurationExcepti } private String getDefaultDirectDownloadTemporaryPath() { - return "/var/lib/libvirt/images/direct-download"; + return "/var/lib/libvirt/images"; } protected String getDefaultNetworkScriptsDir() { From ec5146ace5f0ae3b5a0331cbae61e7780089eed9 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 29 Jan 2020 01:41:12 -0300 Subject: [PATCH 21/30] Fix upload certificate --- test/integration/smoke/test_direct_download.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/integration/smoke/test_direct_download.py b/test/integration/smoke/test_direct_download.py index a6c98e52286e..324fb5972cb2 100644 --- a/test/integration/smoke/test_direct_download.py +++ b/test/integration/smoke/test_direct_download.py @@ -30,6 +30,7 @@ from nose.plugins.attrib import attr from marvin.cloudstackAPI import (uploadTemplateDirectDownloadCertificate, revokeTemplateDirectDownloadCertificate) from marvin.lib.decoratorGenerators import skipTestIf +import uuid class TestUploadDirectDownloadCertificates(cloudstackTestCase): @@ -91,7 +92,7 @@ def test_01_sanity_check_on_certificates(self): cmd = uploadTemplateDirectDownloadCertificate.uploadTemplateDirectDownloadCertificateCmd() cmd.hypervisor = self.hypervisor - cmd.name = "marvin-test-verify-certs" + cmd.name = "marvin-test-verify-certs" + str(uuid.uuid1()) cmd.certificate = self.certificates["invalid"] cmd.zoneid = self.zone.id @@ -126,7 +127,7 @@ def test_02_upload_direct_download_certificates(self): cmd = uploadTemplateDirectDownloadCertificate.uploadTemplateDirectDownloadCertificateCmd() cmd.hypervisor = self.hypervisor - cmd.name = "marvin-test-verify-certs" + cmd.name = "marvin-test-verify-certs" + str(uuid.uuid1()) cmd.certificate = self.certificates["valid"] cmd.zoneid = self.zone.id From 40391eb8978808e39a5fa2916145fd22fa7f7b21 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Thu, 20 Feb 2020 09:49:09 -0300 Subject: [PATCH 22/30] Fix ISO failure after retry --- .../src/main/java/com/cloud/template/TemplateManagerImpl.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 93905fc0f3fb..769a814fd31d 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -587,6 +587,10 @@ public void prepareIsoForVmProfile(VirtualMachineProfile profile, DeployDestinat template = prepareIso(vm.getIsoId(), vm.getDataCenterId(), dest.getHost().getId(), poolId); } else { template = _tmplFactory.getTemplate(vm.getIsoId(), DataStoreRole.Primary, dest.getDataCenter().getId()); + if (template.isDirectDownload() && template.getInstallPath() == null) { + s_logger.error("Error getting ISO for direct download"); + throw new CloudRuntimeException("Error getting ISO for direct download"); + } } if (template == null){ From dedb7dc07f08681a64f0bdd9eb2029b56ddc68b1 Mon Sep 17 00:00:00 2001 From: Rakesh Date: Tue, 18 Feb 2020 09:33:30 +0100 Subject: [PATCH 23/30] server: ignore site to site vpn status check on internallbvm (#3864) When the state of the site to site vpn changes, the check is done on all the virtual routers including the internal load balancing vm as well. It is not needed to check the state for internal load balancing vm --- .../network/router/VirtualNetworkApplianceManagerImpl.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index 87933456de92..50cd8a4876f2 100644 --- a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -885,6 +885,10 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { @DB protected void updateSite2SiteVpnConnectionState(final List routers) { for (final DomainRouterVO router : routers) { + if (router.getRole() == Role.INTERNAL_LB_VM) { + continue; + } + final List conns = _s2sVpnMgr.getConnectionsForRouter(router); if (conns == null || conns.isEmpty()) { continue; From 08874e97feb0e285286a130e01944d64f50a7b5b Mon Sep 17 00:00:00 2001 From: Andrija Panic <45762285+andrijapanicsb@users.noreply.github.com> Date: Tue, 18 Feb 2020 09:40:51 +0100 Subject: [PATCH 24/30] engine/schema: remove duplicate index region (#3882) Remove duplicate index region. --- .../main/resources/META-INF/db/schema-41300to41400-cleanup.sql | 3 +++ 1 file changed, 3 insertions(+) diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41300to41400-cleanup.sql b/engine/schema/src/main/resources/META-INF/db/schema-41300to41400-cleanup.sql index fa631ef50fa5..7758f116dd7f 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41300to41400-cleanup.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41300to41400-cleanup.sql @@ -23,3 +23,6 @@ DELETE FROM `cloud`.`configuration` WHERE name = 'host.maintenance.retries'; -- Stop asking user (in the upgraded documentation) to remove a trailing slash for local KVM pool UPDATE `cloud`.`storage_pool` SET path="/var/lib/libvirt/images" WHERE path="/var/lib/libvirt/images/"; + +-- remove (one of) duplicate unique indexes from Region table +ALTER TABLE `region` DROP INDEX `id_3`; From ce118703356eb0c63e6c258f35c9998fbfa5f9fb Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 19 Feb 2020 08:36:19 +0100 Subject: [PATCH 25/30] kvm: fix exception in volume statts after storage migration (#3884) On kvm, the 'path' of volume is the file name on primary storage. we should use 'path' instead of 'uuid' in volume statistics. Fixes: #3878 --- server/src/main/java/com/cloud/server/StatsCollector.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index b2f244229e32..14a9084124f1 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -928,13 +928,7 @@ protected void runInContext() { List volumes = _volsDao.findByPoolId(pool.getId(), null); List volumeLocators = new ArrayList(); for (VolumeVO volume : volumes) { - if (volume.getFormat() == ImageFormat.QCOW2) { - if (pool.isManaged()) { - volumeLocators.add(volume.getPath()); - } else { - volumeLocators.add(volume.getUuid()); - } - } else if (volume.getFormat() == ImageFormat.VHD) { + if (volume.getFormat() == ImageFormat.QCOW2 || volume.getFormat() == ImageFormat.VHD) { volumeLocators.add(volume.getPath()); } else if (volume.getFormat() == ImageFormat.OVA) { volumeLocators.add(volume.getChainInfo()); From c08320decf785705d6b263b1ccb237bcf2aa3b2c Mon Sep 17 00:00:00 2001 From: Rakesh Date: Wed, 19 Feb 2020 08:39:52 +0100 Subject: [PATCH 26/30] server: Add new command to update security group name (#3739) By default, once we create a security group we cant change its name. In this feature, we introduce a new API command "updateSecurityGroup" which allows us to rename the security group name. Although we can't change the name of the "default" security group. --- .travis.yml | 1 + .../main/java/com/cloud/event/EventTypes.java | 1 + .../security/SecurityGroupService.java | 11 +- .../securitygroup/UpdateSecurityGroupCmd.java | 105 ++++++ .../network/security/SecurityGroupVO.java | 4 + .../security/SecurityGroupManagerImpl.java | 59 ++++ .../cloud/server/ManagementServerImpl.java | 2 + .../smoke/test_update_security_group.py | 312 ++++++++++++++++++ ui/scripts/network.js | 31 +- 9 files changed, 521 insertions(+), 5 deletions(-) create mode 100644 api/src/main/java/org/apache/cloudstack/api/command/user/securitygroup/UpdateSecurityGroupCmd.java create mode 100644 test/integration/smoke/test_update_security_group.py diff --git a/.travis.yml b/.travis.yml index 7c8179009e6c..e5ad914562fe 100644 --- a/.travis.yml +++ b/.travis.yml @@ -103,6 +103,7 @@ env: smoke/test_ssvm smoke/test_staticroles smoke/test_templates + smoke/test_update_security_group smoke/test_usage smoke/test_usage_events" diff --git a/api/src/main/java/com/cloud/event/EventTypes.java b/api/src/main/java/com/cloud/event/EventTypes.java index dc52611c82c3..33950f8f2784 100644 --- a/api/src/main/java/com/cloud/event/EventTypes.java +++ b/api/src/main/java/com/cloud/event/EventTypes.java @@ -327,6 +327,7 @@ public class EventTypes { public static final String EVENT_SECURITY_GROUP_DELETE = "SG.DELETE"; public static final String EVENT_SECURITY_GROUP_ASSIGN = "SG.ASSIGN"; public static final String EVENT_SECURITY_GROUP_REMOVE = "SG.REMOVE"; + public static final String EVENT_SECURITY_GROUP_UPDATE = "SG.UPDATE"; // Host public static final String EVENT_HOST_RECONNECT = "HOST.RECONNECT"; diff --git a/api/src/main/java/com/cloud/network/security/SecurityGroupService.java b/api/src/main/java/com/cloud/network/security/SecurityGroupService.java index d8b3346f54a4..dce7b3d41fea 100644 --- a/api/src/main/java/com/cloud/network/security/SecurityGroupService.java +++ b/api/src/main/java/com/cloud/network/security/SecurityGroupService.java @@ -18,16 +18,17 @@ import java.util.List; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.PermissionDeniedException; +import com.cloud.exception.ResourceInUseException; + import org.apache.cloudstack.api.command.user.securitygroup.AuthorizeSecurityGroupEgressCmd; import org.apache.cloudstack.api.command.user.securitygroup.AuthorizeSecurityGroupIngressCmd; import org.apache.cloudstack.api.command.user.securitygroup.CreateSecurityGroupCmd; import org.apache.cloudstack.api.command.user.securitygroup.DeleteSecurityGroupCmd; import org.apache.cloudstack.api.command.user.securitygroup.RevokeSecurityGroupEgressCmd; import org.apache.cloudstack.api.command.user.securitygroup.RevokeSecurityGroupIngressCmd; - -import com.cloud.exception.InvalidParameterValueException; -import com.cloud.exception.PermissionDeniedException; -import com.cloud.exception.ResourceInUseException; +import org.apache.cloudstack.api.command.user.securitygroup.UpdateSecurityGroupCmd; public interface SecurityGroupService { /** @@ -43,6 +44,8 @@ public interface SecurityGroupService { boolean deleteSecurityGroup(DeleteSecurityGroupCmd cmd) throws ResourceInUseException; + SecurityGroup updateSecurityGroup(UpdateSecurityGroupCmd cmd); + public List authorizeSecurityGroupIngress(AuthorizeSecurityGroupIngressCmd cmd); public List authorizeSecurityGroupEgress(AuthorizeSecurityGroupEgressCmd cmd); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/securitygroup/UpdateSecurityGroupCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/securitygroup/UpdateSecurityGroupCmd.java new file mode 100644 index 000000000000..154ae71d6e1e --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/securitygroup/UpdateSecurityGroupCmd.java @@ -0,0 +1,105 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.user.securitygroup; + +import org.apache.log4j.Logger; + +import org.apache.cloudstack.acl.RoleType; +import org.apache.cloudstack.acl.SecurityChecker.AccessType; +import org.apache.cloudstack.api.ACL; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.BaseCustomIdCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.response.SecurityGroupResponse; + +import com.cloud.network.security.SecurityGroup; +import com.cloud.user.Account; + +@APICommand(name = UpdateSecurityGroupCmd.APINAME, description = "Updates a security group", responseObject = SecurityGroupResponse.class, entityType = {SecurityGroup.class}, + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, + since = "4.14.0.0", + authorized = {RoleType.Admin}) +public class UpdateSecurityGroupCmd extends BaseCustomIdCmd { + public static final String APINAME = "updateSecurityGroup"; + public static final Logger s_logger = Logger.getLogger(UpdateSecurityGroupCmd.class.getName()); + private static final String s_name = "updatesecuritygroupresponse"; + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @ACL(accessType = AccessType.OperateEntry) + @Parameter(name = ApiConstants.ID, type = CommandType.UUID, required=true, description="The ID of the security group.", entityType=SecurityGroupResponse.class) + private Long id; + + @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "The new name of the security group.") + private String name; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public Long getId() { + return id; + } + + public String getName() { + return name; + } + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + @Override + public String getCommandName() { + return s_name; + } + + @Override + public long getEntityOwnerId() { + SecurityGroup securityGroup = _entityMgr.findById(SecurityGroup.class, getId()); + if (securityGroup != null) { + return securityGroup.getAccountId(); + } + + return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked + } + + @Override + public void execute() { + SecurityGroup result = _securityGroupService.updateSecurityGroup(this); + if (result != null) { + SecurityGroupResponse response = _responseGenerator.createSecurityGroupResponse(result); + response.setResponseName(getCommandName()); + setResponseObject(response); + } else { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update security group"); + } + } + + @Override + public void checkUuid() { + if (getCustomId() != null) { + _uuidMgr.checkUuid(getCustomId(), SecurityGroup.class); + } + } + +} diff --git a/engine/schema/src/main/java/com/cloud/network/security/SecurityGroupVO.java b/engine/schema/src/main/java/com/cloud/network/security/SecurityGroupVO.java index 4a4c83ae74a3..3b7ceb8eb64b 100644 --- a/engine/schema/src/main/java/com/cloud/network/security/SecurityGroupVO.java +++ b/engine/schema/src/main/java/com/cloud/network/security/SecurityGroupVO.java @@ -70,6 +70,10 @@ public String getName() { return name; } + public void setName(String name) { + this.name = name; + } + @Override public String getDescription() { return description; diff --git a/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl.java b/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl.java index a0c828588c00..a3a3f9b54303 100644 --- a/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl.java +++ b/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl.java @@ -44,6 +44,7 @@ import org.apache.cloudstack.api.command.user.securitygroup.DeleteSecurityGroupCmd; import org.apache.cloudstack.api.command.user.securitygroup.RevokeSecurityGroupEgressCmd; import org.apache.cloudstack.api.command.user.securitygroup.RevokeSecurityGroupIngressCmd; +import org.apache.cloudstack.api.command.user.securitygroup.UpdateSecurityGroupCmd; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; @@ -879,6 +880,10 @@ public SecurityGroupVO createSecurityGroup(CreateSecurityGroupCmd cmd) throws Pe Account caller = CallContext.current().getCallingAccount(); Account owner = _accountMgr.finalizeOwner(caller, cmd.getAccountName(), cmd.getDomainId(), cmd.getProjectId()); + if (StringUtils.isBlank(name)) { + throw new InvalidParameterValueException("Security group name cannot be empty"); + } + if (_securityGroupDao.isNameInUse(owner.getId(), owner.getDomainId(), cmd.getSecurityGroupName())) { throw new InvalidParameterValueException("Unable to create security group, a group with name " + name + " already exists."); } @@ -1117,6 +1122,60 @@ public void doInTransactionWithoutResult(TransactionStatus status) { s_logger.debug("Security group mappings are removed successfully for vm id=" + userVmId); } + @DB + @Override + @ActionEvent(eventType = EventTypes.EVENT_SECURITY_GROUP_UPDATE, eventDescription = "updating security group") + public SecurityGroup updateSecurityGroup(UpdateSecurityGroupCmd cmd) { + final Long groupId = cmd.getId(); + final String newName = cmd.getName(); + Account caller = CallContext.current().getCallingAccount(); + + SecurityGroupVO group = _securityGroupDao.findById(groupId); + if (group == null) { + throw new InvalidParameterValueException("Unable to find security group: " + groupId + "; failed to update security group."); + } + + if (newName == null) { + s_logger.debug("security group name is not changed. id=" + groupId); + return group; + } + + if (StringUtils.isBlank(newName)) { + throw new InvalidParameterValueException("Security group name cannot be empty"); + } + + // check permissions + _accountMgr.checkAccess(caller, null, true, group); + + return Transaction.execute(new TransactionCallback() { + @Override + public SecurityGroupVO doInTransaction(TransactionStatus status) { + SecurityGroupVO group = _securityGroupDao.lockRow(groupId, true); + if (group == null) { + throw new InvalidParameterValueException("Unable to find security group by id " + groupId); + } + + if (newName.equals(group.getName())) { + s_logger.debug("security group name is not changed. id=" + groupId); + return group; + } else if (newName.equalsIgnoreCase(SecurityGroupManager.DEFAULT_GROUP_NAME)) { + throw new InvalidParameterValueException("The security group name " + SecurityGroupManager.DEFAULT_GROUP_NAME + " is reserved"); + } + + if (group.getName().equalsIgnoreCase(SecurityGroupManager.DEFAULT_GROUP_NAME)) { + throw new InvalidParameterValueException("The default security group cannot be renamed"); + } + + group.setName(newName); + _securityGroupDao.update(groupId, group); + + s_logger.debug("Updated security group id=" + groupId); + + return group; + } + }); + } + @DB @Override @ActionEvent(eventType = EventTypes.EVENT_SECURITY_GROUP_DELETE, eventDescription = "deleting security group") diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index a68b3b80d0c9..ac3460c22e1f 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -425,6 +425,7 @@ import org.apache.cloudstack.api.command.user.securitygroup.ListSecurityGroupsCmd; import org.apache.cloudstack.api.command.user.securitygroup.RevokeSecurityGroupEgressCmd; import org.apache.cloudstack.api.command.user.securitygroup.RevokeSecurityGroupIngressCmd; +import org.apache.cloudstack.api.command.user.securitygroup.UpdateSecurityGroupCmd; import org.apache.cloudstack.api.command.user.snapshot.ArchiveSnapshotCmd; import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotCmd; import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotFromVMSnapshotCmd; @@ -2895,6 +2896,7 @@ public List> getCommands() { cmdList.add(ListSecurityGroupsCmd.class); cmdList.add(RevokeSecurityGroupEgressCmd.class); cmdList.add(RevokeSecurityGroupIngressCmd.class); + cmdList.add(UpdateSecurityGroupCmd.class); cmdList.add(CreateSnapshotCmd.class); cmdList.add(CreateSnapshotFromVMSnapshotCmd.class); cmdList.add(DeleteSnapshotCmd.class); diff --git a/test/integration/smoke/test_update_security_group.py b/test/integration/smoke/test_update_security_group.py new file mode 100644 index 000000000000..41e4d596907a --- /dev/null +++ b/test/integration/smoke/test_update_security_group.py @@ -0,0 +1,312 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Tests for updating security group name +""" + +# Import Local Modules +from nose.plugins.attrib import attr +from marvin.cloudstackTestCase import cloudstackTestCase, unittest +from marvin.cloudstackAPI import updateSecurityGroup, createSecurityGroup +from marvin.sshClient import SshClient +from marvin.lib.utils import (validateList, + cleanup_resources, + random_gen) +from marvin.lib.base import (PhysicalNetwork, + Account, + Host, + TrafficType, + Domain, + Network, + NetworkOffering, + VirtualMachine, + ServiceOffering, + Zone, + SecurityGroup) +from marvin.lib.common import (get_domain, + get_zone, + get_template, + list_virtual_machines, + list_routers, + list_hosts, + get_free_vlan) +from marvin.codes import (PASS, FAIL) +import logging +import random +import time + +class TestUpdateSecurityGroup(cloudstackTestCase): + @classmethod + def setUpClass(cls): + cls.testClient = super( + TestUpdateSecurityGroup, + cls).getClsTestClient() + cls.apiclient = cls.testClient.getApiClient() + cls.testdata = cls.testClient.getParsedTestDataConfig() + cls.services = cls.testClient.getParsedTestDataConfig() + + zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests()) + cls.zone = Zone(zone.__dict__) + cls.template = get_template(cls.apiclient, cls.zone.id) + cls._cleanup = [] + + if str(cls.zone.securitygroupsenabled) != "True": + sys.exit(1) + + cls.logger = logging.getLogger("TestUpdateSecurityGroup") + cls.stream_handler = logging.StreamHandler() + cls.logger.setLevel(logging.DEBUG) + cls.logger.addHandler(cls.stream_handler) + + # Get Zone, Domain and templates + cls.domain = get_domain(cls.apiclient) + testClient = super(TestUpdateSecurityGroup, cls).getClsTestClient() + cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests()) + cls.services['mode'] = cls.zone.networktype + + # Create new domain, account, network and VM + cls.user_domain = Domain.create( + cls.apiclient, + services=cls.testdata["acl"]["domain2"], + parentdomainid=cls.domain.id) + + # Create account + cls.account = Account.create( + cls.apiclient, + cls.testdata["acl"]["accountD2"], + admin=True, + domainid=cls.user_domain.id + ) + + cls._cleanup.append(cls.account) + cls._cleanup.append(cls.user_domain) + + @classmethod + def tearDownClass(self): + try: + cleanup_resources(self.apiclient, self._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.cleanup = [] + return + + def tearDown(self): + try: + cleanup_resources(self.apiclient, self.cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + @attr(tags=["advancedsg"], required_hardware="false") + def test_01_create_security_group(self): + # Validate the following: + # + # 1. Create a new security group + # 2. Update the security group with new name + # 3. List the security group with new name as the keyword + # 4. Make sure that the response is not empty + + security_group = SecurityGroup.create( + self.apiclient, + self.testdata["security_group"], + account=self.account.name, + domainid=self.account.domainid + ) + self.debug("Created security group with ID: %s" % security_group.id) + + initial_secgroup_name = security_group.name + new_secgroup_name = "testing-update-security-group" + + cmd = updateSecurityGroup.updateSecurityGroupCmd() + cmd.id = security_group.id + cmd.name = new_secgroup_name + self.apiclient.updateSecurityGroup(cmd) + + new_security_group = SecurityGroup.list( + self.apiclient, + account=self.account.name, + domainid=self.account.domainid, + keyword=new_secgroup_name + ) + self.assertNotEqual( + len(new_security_group), + 0, + "Update security group failed" + ) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_02_duplicate_security_group_name(self): + # Validate the following + # + # 1. Create a security groups with name "test" + # 2. Try to create another security group with name "test" + # 3. Creation of second security group should fail + + security_group_name = "test" + security_group = SecurityGroup.create( + self.apiclient, + {"name": security_group_name}, + account=self.account.name, + domainid=self.account.domainid + ) + self.debug("Created security group with name: %s" % security_group.name) + + security_group_list = SecurityGroup.list( + self.apiclient, + account=self.account.name, + domainid=self.account.domainid, + keyword=security_group.name + ) + self.assertNotEqual( + len(security_group_list), + 0, + "Creating security group failed" + ) + + # Need to use createSecurituGroupCmd since SecurityGroup.create + # adds random string to security group name + with self.assertRaises(Exception): + cmd = createSecurityGroup.createSecurityGroupCmd() + cmd.name = security_group.name + cmd.account = self.account.name + cmd.domainid = self.account.domainid + self.apiclient.createSecurityGroup(cmd) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_03_update_security_group_with_existing_name(self): + # Validate the following + # + # 1. Create a security groups with name "test" + # 2. Create another security group + # 3. Try to update the second security group to update its name to "test" + # 4. Update security group should fail + + # Create security group + security_group = SecurityGroup.create( + self.apiclient, + self.testdata["security_group"], + account=self.account.name, + domainid=self.account.domainid + ) + self.debug("Created security group with ID: %s" % security_group.id) + security_group_name = security_group.name + + # Make sure its created + security_group_list = SecurityGroup.list( + self.apiclient, + account=self.account.name, + domainid=self.account.domainid, + keyword=security_group_name + ) + self.assertNotEqual( + len(security_group_list), + 0, + "Creating security group failed" + ) + + # Create another security group + second_security_group = SecurityGroup.create( + self.apiclient, + self.testdata["security_group"], + account=self.account.name, + domainid=self.account.domainid + ) + self.debug("Created security group with ID: %s" % second_security_group.id) + + # Make sure its created + security_group_list = SecurityGroup.list( + self.apiclient, + account=self.account.name, + domainid=self.account.domainid, + keyword=second_security_group.name + ) + self.assertNotEqual( + len(security_group_list), + 0, + "Creating security group failed" + ) + + # Update the security group + with self.assertRaises(Exception): + cmd = updateSecurityGroup.updateSecurityGroupCmd() + cmd.id = second_security_group.id + cmd.name = security_group_name + self.apiclient.updateSecurityGroup(cmd) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_04_update_security_group_with_empty_name(self): + # Validate the following + # + # 1. Create a security group + # 2. Update the security group to an empty name + # 3. Update security group should fail + + # Create security group + security_group = SecurityGroup.create( + self.apiclient, + self.testdata["security_group"], + account=self.account.name, + domainid=self.account.domainid + ) + self.debug("Created security group with ID: %s" % security_group.id) + + # Make sure its created + security_group_list = SecurityGroup.list( + self.apiclient, + account=self.account.name, + domainid=self.account.domainid, + keyword=security_group.name + ) + self.assertNotEqual( + len(security_group_list), + 0, + "Creating security group failed" + ) + + # Update the security group + with self.assertRaises(Exception): + cmd = updateSecurityGroup.updateSecurityGroupCmd() + cmd.id = security_group.id + cmd.name = "" + self.apiclient.updateSecurityGroup(cmd) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_05_rename_security_group(self): + # Validate the following + # + # 1. Create a security group + # 2. Update the security group and change its name to "default" + # 3. Exception should be thrown as "default" name cant be used + + security_group = SecurityGroup.create( + self.apiclient, + self.testdata["security_group"], + account=self.account.name, + domainid=self.account.domainid + ) + self.debug("Created security group with ID: %s" % security_group.id) + + with self.assertRaises(Exception): + cmd = updateSecurityGroup.updateSecurityGroupCmd() + cmd.id = security_group.id + cmd.name = "default" + self.apiclient.updateSecurityGroup(cmd) diff --git a/ui/scripts/network.js b/ui/scripts/network.js index c7ca7a0b9ebc..32671f5cea1b 100644 --- a/ui/scripts/network.js +++ b/ui/scripts/network.js @@ -361,6 +361,7 @@ args.context.item.state != 'Destroyed' && args.context.item.name != 'default') { allowedActions.push('remove'); + allowedActions.push('edit'); } return allowedActions; @@ -4523,7 +4524,11 @@ title: 'label.details', fields: [{ name: { - label: 'label.name' + label: 'label.name', + isEditable: true, + validation: { + required: true + } } }, { id: { @@ -5075,6 +5080,30 @@ }, actions: { + edit: { + label: 'label.edit', + action: function(args) { + var data = { + id: args.context.securityGroups[0].id + }; + if (args.data.name != args.context.securityGroups[0].name) { + $.extend(data, { + name: args.data.name + }); + }; + $.ajax({ + url: createURL('updateSecurityGroup'), + data: data, + success: function(json) { + var item = json.updatesecuritygroupresponse.securitygroup; + args.response.success({ + data: item + }); + } + }); + } + }, + remove: { label: 'label.action.delete.security.group', messages: { From 64c3001e5c81e8924f01712de30d0abd8ba450c3 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 19 Feb 2020 08:42:20 +0100 Subject: [PATCH 27/30] kvm: Enable virtio drivers based on guest os display name (#3879) When we add new guest os, sometimes we missed the records in guest_os_hypervisor. However, the guest disk model (virtio/ide) is determined by record in the table. It causes the issue that some new guest os(eg Debian 8/9) uses e1000 instead of virtio nic, and ide disk instead of virtio disk. To fix the issue permanantly, pass the guest os name in guest_os if the record for kvm is not found in guest_os_hypervisor. Related commit:7ac9f00eeeb4cd37ec39efeba066e799b581b1a0 --- server/src/main/java/com/cloud/hypervisor/KVMGuru.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/hypervisor/KVMGuru.java b/server/src/main/java/com/cloud/hypervisor/KVMGuru.java index 2b84ff9b04df..cf29a1a55a93 100644 --- a/server/src/main/java/com/cloud/hypervisor/KVMGuru.java +++ b/server/src/main/java/com/cloud/hypervisor/KVMGuru.java @@ -132,7 +132,7 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) { guestOsMapping = _guestOsHypervisorDao.findByOsIdAndHypervisor(guestOS.getId(), getHypervisorType().toString(), host.getHypervisorVersion()); } if (guestOsMapping == null || host == null) { - to.setPlatformEmulator("Other"); + to.setPlatformEmulator(guestOS.getDisplayName() == null ? "Other" : guestOS.getDisplayName()); } else { to.setPlatformEmulator(guestOsMapping.getGuestOsName()); } From ef174a58783f8d337c07569619bab538175d4754 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 19 Feb 2020 14:13:37 +0100 Subject: [PATCH 28/30] KVM: Propagating changes on host parameters to the agents (#3491) --- .../java/com/cloud/agent/AgentManager.java | 2 + .../cloud/agent/manager/AgentManagerImpl.java | 41 +++++++++++++++++++ .../cloudstack/agent/lb/IndirectAgentLB.java | 5 ++- .../resource/LibvirtComputingResource.java | 19 +++++++++ .../LibvirtSetHostParamsCommandWrapper.java | 1 + .../ConfigurationManagerImpl.java | 28 +++++++++++++ .../agent/lb/IndirectAgentLBServiceImpl.java | 27 ++---------- .../lb/IndirectAgentLBServiceImplTest.java | 3 +- .../test/resources/createNetworkOffering.xml | 1 + 9 files changed, 100 insertions(+), 27 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java b/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java index c51970c85f76..1d8e5faf9d4a 100644 --- a/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java +++ b/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java @@ -152,4 +152,6 @@ public enum TapAgentsAction { void notifyMonitorsOfHostAboutToBeRemoved(long hostId); void notifyMonitorsOfRemovedHost(long hostId, long clusterId); + + void propagateChangeToAgents(); } diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java index 2b80ec963c10..fe6d9d514f6f 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java @@ -1790,4 +1790,45 @@ public int getTimeout() { } + protected Map> getHostsPerZone() { + List allHosts = _resourceMgr.listAllHostsInAllZonesByType(Host.Type.Routing); + if (allHosts == null) { + return null; + } + Map> hostsByZone = new HashMap>(); + for (HostVO host : allHosts) { + if (host.getHypervisorType() == HypervisorType.KVM || host.getHypervisorType() == HypervisorType.LXC) { + Long zoneId = host.getDataCenterId(); + List hostIds = hostsByZone.get(zoneId); + if (hostIds == null) { + hostIds = new ArrayList(); + } + hostIds.add(host.getId()); + hostsByZone.put(zoneId, hostIds); + } + } + return hostsByZone; + } + + private void sendCommandToAgents(Map> hostsPerZone, Map params) { + SetHostParamsCommand cmds = new SetHostParamsCommand(params); + for (Long zoneId : hostsPerZone.keySet()) { + List hostIds = hostsPerZone.get(zoneId); + for (Long hostId : hostIds) { + Answer answer = easySend(hostId, cmds); + if (answer == null || !answer.getResult()) { + s_logger.error("Error sending parameters to agent " + hostId); + } + } + } + } + + @Override + public void propagateChangeToAgents() { + s_logger.debug("Propagating changes on host parameters to the agents"); + Map> hostsPerZone = getHostsPerZone(); + Map params = new HashMap(); + params.put("router.aggregation.command.each.timeout", _configDao.getValue("router.aggregation.command.each.timeout")); + sendCommandToAgents(hostsPerZone, params); + } } diff --git a/framework/agent-lb/src/main/java/org/apache/cloudstack/agent/lb/IndirectAgentLB.java b/framework/agent-lb/src/main/java/org/apache/cloudstack/agent/lb/IndirectAgentLB.java index 627a5ee5f50c..464c489af848 100644 --- a/framework/agent-lb/src/main/java/org/apache/cloudstack/agent/lb/IndirectAgentLB.java +++ b/framework/agent-lb/src/main/java/org/apache/cloudstack/agent/lb/IndirectAgentLB.java @@ -50,4 +50,7 @@ public interface IndirectAgentLB { * @return returns interval in seconds */ Long getLBPreferredHostCheckInterval(Long clusterId); -} \ No newline at end of file + + void propagateMSListToAgents(); + +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 77616af86e83..7cb26410bd70 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -103,6 +103,7 @@ import com.cloud.agent.api.to.NfsTO; import com.cloud.agent.api.to.NicTO; import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.agent.dao.impl.PropertiesStorage; import com.cloud.agent.resource.virtualnetwork.VRScripts; import com.cloud.agent.resource.virtualnetwork.VirtualRouterDeployer; import com.cloud.agent.resource.virtualnetwork.VirtualRoutingResource; @@ -1116,6 +1117,24 @@ public boolean configure(final String name, final Map params) th return true; } + public boolean configureHostParams(final Map params) { + final File file = PropertiesUtil.findConfigFile("agent.properties"); + if (file == null) { + s_logger.error("Unable to find the file agent.properties"); + return false; + } + // Save configurations in agent.properties + PropertiesStorage storage = new PropertiesStorage(); + storage.configure("Storage", new HashMap()); + if (params.get("router.aggregation.command.each.timeout") != null) { + String value = (String)params.get("router.aggregation.command.each.timeout"); + Integer intValue = NumbersUtil.parseInt(value, 600); + storage.persist("router.aggregation.command.each.timeout", String.valueOf(intValue)); + } + + return true; + } + protected void configureDiskActivityChecks(final Map params) { _diskActivityCheckEnabled = Boolean.parseBoolean((String)params.get("vm.diskactivity.checkenabled")); if (_diskActivityCheckEnabled) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetHostParamsCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetHostParamsCommandWrapper.java index 52dd0e9e4d4c..e07f0a6b01bf 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetHostParamsCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetHostParamsCommandWrapper.java @@ -35,6 +35,7 @@ public Answer execute(final SetHostParamsCommand command, final LibvirtComputing final Map params = command.getParams(); boolean success = libvirtComputingResource.getVirtRouterResource().configureHostParams(params); + success = success && libvirtComputingResource.configureHostParams(params); if (!success) { return new Answer(command, false, "Failed to set host parameters"); diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 1f8655d40c93..be01b9f83259 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.configuration; +import com.cloud.agent.AgentManager; import com.cloud.alert.AlertManager; import com.cloud.api.ApiDBUtils; import com.cloud.api.query.dao.NetworkOfferingJoinDao; @@ -161,6 +162,8 @@ import org.apache.cloudstack.affinity.AffinityGroup; import org.apache.cloudstack.affinity.AffinityGroupService; import org.apache.cloudstack.affinity.dao.AffinityGroupDao; +import org.apache.cloudstack.agent.lb.IndirectAgentLB; +import org.apache.cloudstack.agent.lb.IndirectAgentLBServiceImpl; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.command.admin.config.UpdateCfgCmd; import org.apache.cloudstack.api.command.admin.network.CreateManagementNetworkIpRangeCmd; @@ -187,6 +190,7 @@ import org.apache.cloudstack.api.command.admin.zone.DeleteZoneCmd; import org.apache.cloudstack.api.command.admin.zone.UpdateZoneCmd; import org.apache.cloudstack.api.command.user.network.ListNetworkOfferingsCmd; +import org.apache.cloudstack.config.ApiServiceConfiguration; import org.apache.cloudstack.config.Configuration; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; @@ -198,6 +202,7 @@ import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.config.impl.ConfigurationVO; import org.apache.cloudstack.framework.messagebus.MessageBus; +import org.apache.cloudstack.framework.messagebus.MessageSubscriber; import org.apache.cloudstack.framework.messagebus.PublishScope; import org.apache.cloudstack.region.PortableIp; import org.apache.cloudstack.region.PortableIpDao; @@ -372,6 +377,10 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati ImageStoreDetailsDao _imageStoreDetailsDao; @Inject MessageBus messageBus; + @Inject + AgentManager _agentManager; + @Inject + IndirectAgentLB _indirectAgentLB; // FIXME - why don't we have interface for DataCenterLinkLocalIpAddressDao? @@ -404,6 +413,7 @@ public boolean configure(final String name, final Map params) th populateConfigValuesForValidationSet(); weightBasedParametersForValidation(); overProvisioningFactorsForValidation(); + initMessageBusListener(); return true; } @@ -461,6 +471,24 @@ private void overProvisioningFactorsForValidation() { overprovisioningFactorsForValidation.add(CapacityManager.StorageOverprovisioningFactor.key()); } + private void initMessageBusListener() { + messageBus.subscribe(EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, new MessageSubscriber() { + @Override + public void onPublishMessage(String serderAddress, String subject, Object args) { + String globalSettingUpdated = (String) args; + if (Strings.isNullOrEmpty(globalSettingUpdated)) { + return; + } + if (globalSettingUpdated.equals(ApiServiceConfiguration.ManagementServerAddresses.key()) || + globalSettingUpdated.equals(IndirectAgentLBServiceImpl.IndirectAgentLBAlgorithm.key())) { + _indirectAgentLB.propagateMSListToAgents(); + } else if (globalSettingUpdated.equals(Config.RouterAggregationCommandEachTimeout.toString())) { + _agentManager.propagateChangeToAgents(); + } + } + }); + } + @Override public boolean start() { diff --git a/server/src/main/java/org/apache/cloudstack/agent/lb/IndirectAgentLBServiceImpl.java b/server/src/main/java/org/apache/cloudstack/agent/lb/IndirectAgentLBServiceImpl.java index 0e9b32cb5dd7..8742e2dfe6f3 100644 --- a/server/src/main/java/org/apache/cloudstack/agent/lb/IndirectAgentLBServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/agent/lb/IndirectAgentLBServiceImpl.java @@ -34,13 +34,10 @@ import org.apache.cloudstack.config.ApiServiceConfiguration; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; -import org.apache.cloudstack.framework.messagebus.MessageBus; -import org.apache.cloudstack.framework.messagebus.MessageSubscriber; import org.apache.log4j.Logger; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; -import com.cloud.event.EventTypes; import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; @@ -68,8 +65,6 @@ public class IndirectAgentLBServiceImpl extends ComponentLifecycleBase implement @Inject private HostDao hostDao; @Inject - private MessageBus messageBus; - @Inject private AgentManager agentManager; ////////////////////////////////////////////////////// @@ -208,7 +203,8 @@ private org.apache.cloudstack.agent.lb.IndirectAgentLBAlgorithm getAgentMSLBAlgo /////////////// Agent MSLB Configuration /////////////////// //////////////////////////////////////////////////////////// - private void propagateMSListToAgents() { + @Override + public void propagateMSListToAgents() { LOG.debug("Propagating management server list update to agents"); final String lbAlgorithm = getLBAlgorithmName(); final Map> dcOrderedHostsMap = new HashMap<>(); @@ -227,22 +223,6 @@ private void propagateMSListToAgents() { } } - private void configureMessageBusListener() { - messageBus.subscribe(EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, new MessageSubscriber() { - @Override - public void onPublishMessage(final String senderAddress, String subject, Object args) { - final String globalSettingUpdated = (String) args; - if (Strings.isNullOrEmpty(globalSettingUpdated)) { - return; - } - if (globalSettingUpdated.equals(ApiServiceConfiguration.ManagementServerAddresses.key()) || - globalSettingUpdated.equals(IndirectAgentLBAlgorithm.key())) { - propagateMSListToAgents(); - } - } - }); - } - private void configureAlgorithmMap() { final List algorithms = new ArrayList<>(); algorithms.add(new IndirectAgentLBStaticAlgorithm()); @@ -258,7 +238,6 @@ private void configureAlgorithmMap() { public boolean configure(final String name, final Map params) throws ConfigurationException { super.configure(name, params); configureAlgorithmMap(); - configureMessageBusListener(); return true; } @@ -274,4 +253,4 @@ public ConfigKey[] getConfigKeys() { IndirectAgentLBCheckInterval }; } -} \ No newline at end of file +} diff --git a/server/src/test/java/org/apache/cloudstack/agent/lb/IndirectAgentLBServiceImplTest.java b/server/src/test/java/org/apache/cloudstack/agent/lb/IndirectAgentLBServiceImplTest.java index 79b5421f9af7..98c2af5b7764 100644 --- a/server/src/test/java/org/apache/cloudstack/agent/lb/IndirectAgentLBServiceImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/agent/lb/IndirectAgentLBServiceImplTest.java @@ -95,7 +95,6 @@ private void configureMocks() throws NoSuchFieldException, IllegalAccessExceptio id++; } addField(agentMSLB, "hostDao", hostDao); - addField(agentMSLB, "messageBus", messageBus); addField(agentMSLB, "agentManager", agentManager); when(hostDao.listAll()).thenReturn(Arrays.asList(host4, host2, host1, host3)); @@ -205,4 +204,4 @@ public void testGetHostsPerZoneNullHosts() { when(hostDao.listAll()).thenReturn(null); Assert.assertTrue(agentMSLB.getOrderedHostIdList(DC_2_ID).size() == 0); } -} \ No newline at end of file +} diff --git a/server/src/test/resources/createNetworkOffering.xml b/server/src/test/resources/createNetworkOffering.xml index 6aac8e18aa86..32596fc03a27 100644 --- a/server/src/test/resources/createNetworkOffering.xml +++ b/server/src/test/resources/createNetworkOffering.xml @@ -58,4 +58,5 @@ + From b1b7ddeb41c624e67bda1f90b62026bf08c465ab Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 19 Feb 2020 15:02:12 +0100 Subject: [PATCH 29/30] =?UTF-8?q?Multiple=20networks=20support=20for=20vms?= =?UTF-8?q?=20in=20advanced=20zone=20with=20securit=E2=80=A6=20(#3639)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../com/cloud/agent/api/RebootCommand.java | 11 + .../agent/api/SecurityGroupRulesCmd.java | 14 +- .../security/SecurityGroupManager.java | 2 + .../cloud/vm/VirtualMachineManagerImpl.java | 38 +- .../main/java/com/cloud/vm/dao/NicDao.java | 2 + .../java/com/cloud/vm/dao/NicDaoImpl.java | 9 + .../resource/LibvirtComputingResource.java | 123 +++- ...tworkRulesVmSecondaryIpCommandWrapper.java | 4 +- .../wrapper/LibvirtPlugNicCommandWrapper.java | 10 +- .../wrapper/LibvirtRebootCommandWrapper.java | 8 +- ...bvirtSecurityGroupRulesCommandWrapper.java | 9 +- .../wrapper/LibvirtStartCommandWrapper.java | 26 +- .../LibvirtUnPlugNicCommandWrapper.java | 5 +- .../LibvirtComputingResourceTest.java | 7 +- scripts/vm/network/security_group.py | 466 +++++++++++-- .../security/SecurityGroupManagerImpl.java | 42 +- .../security/SecurityGroupManagerImpl2.java | 7 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 24 +- .../component/test_multiple_nic_support.py | 629 ++++++++++++++++++ tools/marvin/marvin/lib/utils.py | 10 +- ui/scripts/instanceWizard.js | 47 +- ui/scripts/instances.js | 10 + ui/scripts/network.js | 8 + ui/scripts/ui-custom/instanceWizard.js | 17 +- 24 files changed, 1404 insertions(+), 124 deletions(-) rename {server => engine/components-api}/src/main/java/com/cloud/network/security/SecurityGroupManager.java (95%) create mode 100644 test/integration/component/test_multiple_nic_support.py diff --git a/core/src/main/java/com/cloud/agent/api/RebootCommand.java b/core/src/main/java/com/cloud/agent/api/RebootCommand.java index eecf7f635d38..74ed76283514 100644 --- a/core/src/main/java/com/cloud/agent/api/RebootCommand.java +++ b/core/src/main/java/com/cloud/agent/api/RebootCommand.java @@ -19,8 +19,11 @@ package com.cloud.agent.api; +import com.cloud.agent.api.to.VirtualMachineTO; + public class RebootCommand extends Command { String vmName; + VirtualMachineTO vm; protected boolean executeInSequence = false; protected RebootCommand() { @@ -35,6 +38,14 @@ public String getVmName() { return this.vmName; } + public void setVirtualMachine(VirtualMachineTO vm) { + this.vm = vm; + } + + public VirtualMachineTO getVirtualMachine() { + return vm; + } + @Override public boolean executeInSequence() { return this.executeInSequence; diff --git a/core/src/main/java/com/cloud/agent/api/SecurityGroupRulesCmd.java b/core/src/main/java/com/cloud/agent/api/SecurityGroupRulesCmd.java index 1d3d15439927..ea4ab96c5a7c 100644 --- a/core/src/main/java/com/cloud/agent/api/SecurityGroupRulesCmd.java +++ b/core/src/main/java/com/cloud/agent/api/SecurityGroupRulesCmd.java @@ -30,12 +30,13 @@ import org.apache.log4j.Logger; import com.cloud.agent.api.LogLevel.Log4jLevel; +import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.utils.net.NetUtils; public class SecurityGroupRulesCmd extends Command { private static final String CIDR_LENGTH_SEPARATOR = "/"; - private static final char RULE_TARGET_SEPARATOR = ','; - private static final char RULE_COMMAND_SEPARATOR = ';'; + public static final char RULE_TARGET_SEPARATOR = ','; + public static final char RULE_COMMAND_SEPARATOR = ';'; protected static final String EGRESS_RULE = "E:"; protected static final String INGRESS_RULE = "I:"; private static final Logger LOGGER = Logger.getLogger(SecurityGroupRulesCmd.class); @@ -51,6 +52,7 @@ public class SecurityGroupRulesCmd extends Command { private List ingressRuleSet; private List egressRuleSet; private final List secIps; + private VirtualMachineTO vmTO; public static class IpPortAndProto { private final String proto; @@ -252,6 +254,14 @@ public Long getVmId() { return vmId; } + public void setVmTO(VirtualMachineTO vmTO) { + this.vmTO = vmTO; + } + + public VirtualMachineTO getVmTO() { + return vmTO; + } + /** * used for logging * @return the number of Cidrs in the in and egress rule sets for this security group rules command. diff --git a/server/src/main/java/com/cloud/network/security/SecurityGroupManager.java b/engine/components-api/src/main/java/com/cloud/network/security/SecurityGroupManager.java similarity index 95% rename from server/src/main/java/com/cloud/network/security/SecurityGroupManager.java rename to engine/components-api/src/main/java/com/cloud/network/security/SecurityGroupManager.java index 16d8ba617bca..ffca4bb013b6 100644 --- a/server/src/main/java/com/cloud/network/security/SecurityGroupManager.java +++ b/engine/components-api/src/main/java/com/cloud/network/security/SecurityGroupManager.java @@ -53,4 +53,6 @@ public interface SecurityGroupManager { SecurityGroup getSecurityGroup(String name, long accountId); boolean isVmMappedToDefaultSecurityGroup(long vmId); + + void scheduleRulesetUpdateToHosts(List affectedVms, boolean updateSeqno, Long delayMs); } diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 2c4079cd3fde..8e52c38902cc 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -24,6 +24,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.Date; import java.util.HashMap; import java.util.LinkedHashMap; @@ -166,6 +167,7 @@ import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; import com.cloud.network.router.VirtualRouter; +import com.cloud.network.security.SecurityGroupManager; import com.cloud.offering.DiskOffering; import com.cloud.offering.DiskOfferingInfo; import com.cloud.offering.NetworkOffering; @@ -333,6 +335,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac private NetworkOfferingDetailsDao networkOfferingDetailsDao; @Inject private NetworkDetailsDao networkDetailsDao; + @Inject + private SecurityGroupManager _securityGroupManager; VmWorkJobHandlerProxy _jobHandlerProxy = new VmWorkJobHandlerProxy(this); @@ -3140,11 +3144,18 @@ private void orchestrateReboot(final String vmUuid, final Map affectedVms = new ArrayList(); + affectedVms.add(vm.getId()); + _securityGroupManager.scheduleRulesetUpdateToHosts(affectedVms, true, null); + } return; } s_logger.info("Unable to reboot VM " + vm + " on " + dest.getHost() + " due to " + (rebootAnswer == null ? " no reboot answer" : rebootAnswer.getDetails())); @@ -3154,6 +3165,29 @@ private void orchestrateReboot(final String vmUuid, final Map nics = _nicsDao.listByVmId(profile.getId()); + Collections.sort(nics, new Comparator() { + @Override + public int compare(NicVO nic1, NicVO nic2) { + Long nicId1 = Long.valueOf(nic1.getDeviceId()); + Long nicId2 = Long.valueOf(nic2.getDeviceId()); + return nicId1.compareTo(nicId2); + } + }); + for (final NicVO nic : nics) { + final Network network = _networkModel.getNetwork(nic.getNetworkId()); + final NicProfile nicProfile = + new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(), null, _networkModel.isSecurityGroupSupportedInNetwork(network), + _networkModel.getNetworkTag(profile.getHypervisorType(), network)); + profile.addNic(nicProfile); + } + final VirtualMachineTO to = toVmTO(profile); + return to; + } + public Command cleanup(final VirtualMachine vm, Map dpdkInterfaceMapping) { StopCommand cmd = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType()), false); cmd.setControlIp(getControlNicIpForVM(vm)); @@ -3670,7 +3704,7 @@ private boolean orchestrateRemoveNicFromVm(final VirtualMachine vm, final Nic ni //3) Remove the nic _networkMgr.removeNic(vmProfile, nic); - _nicsDao.expunge(nic.getId()); + _nicsDao.remove(nic.getId()); return true; } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/NicDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/NicDao.java index df4fb06325f9..316c2dd53d72 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/NicDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/NicDao.java @@ -50,6 +50,8 @@ public interface NicDao extends GenericDao { NicVO findDefaultNicForVM(long instanceId); + NicVO findFirstNicForVM(long instanceId); + /** * @param networkId * @param instanceId diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java index c125d80c5342..6314ca0391ba 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java @@ -69,6 +69,7 @@ protected void init() { AllFieldsSearch.and("strategy", AllFieldsSearch.entity().getReservationStrategy(), Op.EQ); AllFieldsSearch.and("reserverName",AllFieldsSearch.entity().getReserver(),Op.EQ); AllFieldsSearch.and("macAddress", AllFieldsSearch.entity().getMacAddress(), Op.EQ); + AllFieldsSearch.and("deviceid", AllFieldsSearch.entity().getDeviceId(), Op.EQ); AllFieldsSearch.done(); IpSearch = createSearchBuilder(String.class); @@ -222,6 +223,14 @@ public NicVO findDefaultNicForVM(long instanceId) { return findOneBy(sc); } + @Override + public NicVO findFirstNicForVM(long instanceId) { + SearchCriteria sc = AllFieldsSearch.create(); + sc.setParameters("instance", instanceId); + sc.setParameters("deviceid", 0); + return findOneBy(sc); + } + @Override public NicVO getControlNicForVM(long vmId){ SearchCriteria sc = AllFieldsSearch.create(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 7cb26410bd70..d0432677df69 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -107,6 +107,7 @@ import com.cloud.agent.resource.virtualnetwork.VRScripts; import com.cloud.agent.resource.virtualnetwork.VirtualRouterDeployer; import com.cloud.agent.resource.virtualnetwork.VirtualRoutingResource; +import com.cloud.agent.api.SecurityGroupRulesCmd; import com.cloud.dc.Vlan; import com.cloud.exception.InternalErrorException; import com.cloud.host.Host.Type; @@ -147,6 +148,7 @@ import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; import com.cloud.hypervisor.kvm.storage.KVMStorageProcessor; import com.cloud.network.Networks.BroadcastDomainType; +import com.cloud.network.Networks.IsolationType; import com.cloud.network.Networks.RouterPrivateIpStrategy; import com.cloud.network.Networks.TrafficType; import com.cloud.resource.RequestWrapper; @@ -3581,7 +3583,117 @@ public boolean destroyNetworkRulesForVM(final Connect conn, final String vmName) return true; } - public boolean defaultNetworkRules(final Connect conn, final String vmName, final NicTO nic, final Long vmId, final String secIpStr) { + /** + * Function to destroy the security group rules applied to the nic's + * @param conn + * @param vmName + * @param nic + * @return + * true : If success + * false : If failure + */ + public boolean destroyNetworkRulesForNic(final Connect conn, final String vmName, final NicTO nic) { + if (!_canBridgeFirewall) { + return false; + } + final List nicSecIps = nic.getNicSecIps(); + String secIpsStr; + final StringBuilder sb = new StringBuilder(); + if (nicSecIps != null) { + for (final String ip : nicSecIps) { + sb.append(ip).append(SecurityGroupRulesCmd.RULE_COMMAND_SEPARATOR); + } + secIpsStr = sb.toString(); + } else { + secIpsStr = "0" + SecurityGroupRulesCmd.RULE_COMMAND_SEPARATOR; + } + final List intfs = getInterfaces(conn, vmName); + if (intfs.size() == 0 || intfs.size() < nic.getDeviceId()) { + return false; + } + + final InterfaceDef intf = intfs.get(nic.getDeviceId()); + final String brname = intf.getBrName(); + final String vif = intf.getDevName(); + + final Script cmd = new Script(_securityGroupPath, _timeout, s_logger); + cmd.add("destroy_network_rules_for_vm"); + cmd.add("--vmname", vmName); + if (nic.getIp() != null) { + cmd.add("--vmip", nic.getIp()); + } + cmd.add("--vmmac", nic.getMac()); + cmd.add("--vif", vif); + cmd.add("--nicsecips", secIpsStr); + + final String result = cmd.execute(); + if (result != null) { + return false; + } + return true; + } + + /** + * Function to apply default network rules for a VM + * @param conn + * @param vm + * @param checkBeforeApply + * @return + */ + public boolean applyDefaultNetworkRules(final Connect conn, final VirtualMachineTO vm, final boolean checkBeforeApply) { + NicTO[] nicTOs = new NicTO[] {}; + if (vm != null && vm.getNics() != null) { + s_logger.debug("Checking default network rules for vm " + vm.getName()); + nicTOs = vm.getNics(); + } + for (NicTO nic : nicTOs) { + if (vm.getType() != VirtualMachine.Type.User) { + nic.setPxeDisable(true); + } + } + boolean isFirstNic = true; + for (final NicTO nic : nicTOs) { + if (nic.isSecurityGroupEnabled() || nic.getIsolationUri() != null && nic.getIsolationUri().getScheme().equalsIgnoreCase(IsolationType.Ec2.toString())) { + if (vm.getType() != VirtualMachine.Type.User) { + configureDefaultNetworkRulesForSystemVm(conn, vm.getName()); + break; + } + if (!applyDefaultNetworkRulesOnNic(conn, vm.getName(), vm.getId(), nic, isFirstNic, checkBeforeApply)) { + s_logger.error("Unable to apply default network rule for nic " + nic.getName() + " for VM " + vm.getName()); + return false; + } + isFirstNic = false; + } + } + return true; + } + + /** + * Function to apply default network rules for a NIC + * @param conn + * @param vmName + * @param vmId + * @param nic + * @param isFirstNic + * @param checkBeforeApply + * @return + */ + public boolean applyDefaultNetworkRulesOnNic(final Connect conn, final String vmName, final Long vmId, final NicTO nic, boolean isFirstNic, boolean checkBeforeApply) { + final List nicSecIps = nic.getNicSecIps(); + String secIpsStr; + final StringBuilder sb = new StringBuilder(); + if (nicSecIps != null) { + for (final String ip : nicSecIps) { + sb.append(ip).append(SecurityGroupRulesCmd.RULE_COMMAND_SEPARATOR); + } + secIpsStr = sb.toString(); + } else { + secIpsStr = "0" + SecurityGroupRulesCmd.RULE_COMMAND_SEPARATOR; + } + return defaultNetworkRules(conn, vmName, nic, vmId, secIpsStr, isFirstNic, checkBeforeApply); + } + + public boolean defaultNetworkRules(final Connect conn, final String vmName, final NicTO nic, final Long vmId, final String secIpStr, final boolean isFirstNic, final boolean checkBeforeApply) { if (!_canBridgeFirewall) { return false; } @@ -3609,6 +3721,12 @@ public boolean defaultNetworkRules(final Connect conn, final String vmName, fina cmd.add("--vif", vif); cmd.add("--brname", brname); cmd.add("--nicsecips", secIpStr); + if (isFirstNic) { + cmd.add("--isFirstNic"); + } + if (checkBeforeApply) { + cmd.add("--check"); + } final String result = cmd.execute(); if (result != null) { return false; @@ -3698,7 +3816,7 @@ public boolean addNetworkRules(final String vmName, final String vmId, final Str return true; } - public boolean configureNetworkRulesVMSecondaryIP(final Connect conn, final String vmName, final String secIp, final String action) { + public boolean configureNetworkRulesVMSecondaryIP(final Connect conn, final String vmName, final String vmMac, final String secIp, final String action) { if (!_canBridgeFirewall) { return false; @@ -3707,6 +3825,7 @@ public boolean configureNetworkRulesVMSecondaryIP(final Connect conn, final Stri final Script cmd = new Script(_securityGroupPath, _timeout, s_logger); cmd.add("network_rules_vmSecondaryIp"); cmd.add("--vmname", vmName); + cmd.add("--vmmac", vmMac); cmd.add("--nicsecips", secIp); cmd.add("--action=" + action); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtNetworkRulesVmSecondaryIpCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtNetworkRulesVmSecondaryIpCommandWrapper.java index 1ad5cb459cfb..07c091ee0eee 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtNetworkRulesVmSecondaryIpCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtNetworkRulesVmSecondaryIpCommandWrapper.java @@ -41,11 +41,11 @@ public Answer execute(final NetworkRulesVmSecondaryIpCommand command, final Libv final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); final Connect conn = libvirtUtilitiesHelper.getConnectionByVmName(command.getVmName()); - result = libvirtComputingResource.configureNetworkRulesVMSecondaryIP(conn, command.getVmName(), command.getVmSecIp(), command.getAction()); + result = libvirtComputingResource.configureNetworkRulesVMSecondaryIP(conn, command.getVmName(), command.getVmMac(), command.getVmSecIp(), command.getAction()); } catch (final LibvirtException e) { s_logger.debug("Could not configure VM secondary IP! => " + e.getLocalizedMessage()); } return new Answer(command, result, ""); } -} \ No newline at end of file +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPlugNicCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPlugNicCommandWrapper.java index 1ef32afccbfc..553a71a30dfe 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPlugNicCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPlugNicCommandWrapper.java @@ -29,6 +29,7 @@ import com.cloud.hypervisor.kvm.resource.VifDriver; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; +import com.cloud.vm.VirtualMachine; import org.apache.log4j.Logger; import org.libvirt.Connect; import org.libvirt.Domain; @@ -45,6 +46,7 @@ public final class LibvirtPlugNicCommandWrapper extends CommandWrapper nicSecIps = nic.getNicSecIps(); - String secIpsStr; - final StringBuilder sb = new StringBuilder(); - if (nicSecIps != null) { - for (final String ip : nicSecIps) { - sb.append(ip).append(";"); - } - secIpsStr = sb.toString(); - } else { - secIpsStr = "0;"; - } - libvirtComputingResource.defaultNetworkRules(conn, vmName, nic, vmSpec.getId(), secIpsStr); - } - } - } + libvirtComputingResource.applyDefaultNetworkRules(conn, vmSpec, false); // pass cmdline info to system vms if (vmSpec.getType() != VirtualMachine.Type.User) { String controlIp = null; - for (final NicTO nic : nics) { + for (final NicTO nic : vmSpec.getNics()) { if (nic.getType() == TrafficType.Control) { controlIp = nic.getIp(); break; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java index 57f4083c96ff..071352c5c9a0 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java @@ -55,6 +55,9 @@ public Answer execute(final UnPlugNicCommand command, final LibvirtComputingReso for (final InterfaceDef pluggedNic : pluggedNics) { if (pluggedNic.getMacAddress().equalsIgnoreCase(nic.getMac())) { + if (nic.isSecurityGroupEnabled()) { + libvirtComputingResource.destroyNetworkRulesForNic(conn, vmName, nic); + } vm.detachDevice(pluggedNic.toString()); // We don't know which "traffic type" is associated with // each interface at this point, so inform all vif drivers @@ -79,4 +82,4 @@ public Answer execute(final UnPlugNicCommand command, final LibvirtComputingReso } } } -} \ No newline at end of file +} diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index 13f8df9f2dd2..1bf27d06ef68 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -2413,7 +2413,7 @@ public void testNetworkRulesVmSecondaryIpCommand() { } catch (final LibvirtException e) { fail(e.getMessage()); } - when(libvirtComputingResource.configureNetworkRulesVMSecondaryIP(conn, command.getVmName(), command.getVmSecIp(), command.getAction())).thenReturn(true); + when(libvirtComputingResource.configureNetworkRulesVMSecondaryIP(conn, command.getVmName(), command.getVmMac(), command.getVmSecIp(), command.getAction())).thenReturn(true); final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); assertNotNull(wrapper); @@ -2427,7 +2427,7 @@ public void testNetworkRulesVmSecondaryIpCommand() { fail(e.getMessage()); } verify(libvirtComputingResource, times(1)).getLibvirtUtilitiesHelper(); - verify(libvirtComputingResource, times(1)).configureNetworkRulesVMSecondaryIP(conn, command.getVmName(), command.getVmSecIp(), command.getAction()); + verify(libvirtComputingResource, times(1)).configureNetworkRulesVMSecondaryIP(conn, command.getVmName(), command.getVmMac(), command.getVmSecIp(), command.getAction()); } @SuppressWarnings("unchecked") @@ -3021,6 +3021,8 @@ public void testSecurityGroupRulesCmdTrue() { cidrs.add("0.0.0.0/0"); final SecurityGroupRulesCmd command = new SecurityGroupRulesCmd(guestIp, guestIp6, guestMac, vmName, vmId, signature, seqNum, ingressRuleSet, egressRuleSet, secIps); + final VirtualMachineTO vm = Mockito.mock(VirtualMachineTO.class); + command.setVmTO(vm); final LibvirtUtilitiesHelper libvirtUtilitiesHelper = Mockito.mock(LibvirtUtilitiesHelper.class); final Connect conn = Mockito.mock(Connect.class); @@ -3053,6 +3055,7 @@ public void testSecurityGroupRulesCmdTrue() { when(egressRuleSet[0].getEndPort()).thenReturn(22); when(egressRuleSet[0].getAllowedCidrs()).thenReturn(cidrs); + when(libvirtComputingResource.applyDefaultNetworkRules(conn, vm, true)).thenReturn(true); when(libvirtComputingResource.addNetworkRules(command.getVmName(), Long.toString(command.getVmId()), command.getGuestIp(), command.getGuestIp6(), command.getSignature(), Long.toString(command.getSeqNum()), command.getGuestMac(), command.stringifyRules(), vif, brname, command.getSecIpsString())).thenReturn(true); diff --git a/scripts/vm/network/security_group.py b/scripts/vm/network/security_group.py index 2ca7ba82a4e7..680177e69549 100755 --- a/scripts/vm/network/security_group.py +++ b/scripts/vm/network/security_group.py @@ -145,6 +145,44 @@ def split_ips_by_family(ips): ip6s.append(ip) return ip4s, ip6s +def destroy_network_rules_for_nic(vm_name, vm_ip, vm_mac, vif, sec_ips): + try: + rules = execute("""iptables-save -t filter | awk '/ %s / { sub(/-A/, "-D", $1) ; print }'""" % vif ).split("\n") + for rule in filter(None, rules): + try: + execute("iptables " + rule) + except: + logging.debug("Ignoring failure to delete rule: " + rule) + except: + pass + + try: + dnats = execute("""iptables-save -t nat | awk '/ %s / { sub(/-A/, "-D", $1) ; print }'""" % vif ).split("\n") + for dnat in filter(None, dnats): + try: + execute("iptables -t nat " + dnat) + except: + logging.debug("Ignoring failure to delete dnat: " + dnat) + except: + pass + + ips = sec_ips.split(';') + ips.pop() + ips.append(vm_ip) + add_to_ipset(vm_name, ips, "-D") + ebtables_rules_vmip(vm_name, vm_mac, ips, "-D") + + vmchain_in = vm_name + "-in" + vmchain_out = vm_name + "-out" + vmchain_in_src = vm_name + "-in-src" + vmchain_out_dst = vm_name + "-out-dst" + try: + execute("ebtables -t nat -D " + vmchain_in_src + " -s " + vm_mac + " -j RETURN") + execute("ebtables -t nat -D " + vmchain_out_dst + " -p ARP --arp-op Reply --arp-mac-dst " + vm_mac + " -j RETURN") + execute("ebtables -t nat -D PREROUTING -i " + vif + " -j " + vmchain_in) + execute("ebtables -t nat -D POSTROUTING -o " + vif + " -j " + vmchain_out) + except: + logging.debug("Ignoring failure to delete ebtable rules for vm: " + vm_name) def get_bridge_physdev(brname): physdev = execute("bridge -o link show | awk '/master %s / && !/^[0-9]+: vnet/ {print $2}' | head -1" % brname) @@ -158,6 +196,9 @@ def destroy_network_rules_for_vm(vm_name, vif=None): vm_ipsetname=ipset_chain_name(vm_name) delete_rules_for_vm_in_bridge_firewall_chain(vm_name) + if 1 in [vm_name.startswith(c) for c in ['r-', 's-', 'v-']]: + return True + if vm_name.startswith('i-'): vmchain_default = '-'.join(vm_name.split('-')[:-1]) + "-def" @@ -198,9 +239,6 @@ def destroy_network_rules_for_vm(vm_name, vif=None): remove_rule_log_for_vm(vm_name) remove_secip_log_for_vm(vm_name) - if 1 in [vm_name.startswith(c) for c in ['r-', 's-', 'v-']]: - return True - return True @@ -228,7 +266,7 @@ def destroy_ebtables_rules(vm_name, vif): execute("ebtables -t nat " + cmd) except: logging.debug("Ignoring failure to delete ebtables rules for vm " + vm_name) - chains = [eb_vm_chain+"-in", eb_vm_chain+"-out", eb_vm_chain+"-in-ips", eb_vm_chain+"-out-ips"] + chains = [eb_vm_chain+"-in", eb_vm_chain+"-out", eb_vm_chain+"-in-ips", eb_vm_chain+"-out-ips", eb_vm_chain+"-in-src", eb_vm_chain+"-out-dst"] for chain in chains: try: execute("ebtables -t nat -F " + chain) @@ -237,14 +275,33 @@ def destroy_ebtables_rules(vm_name, vif): logging.debug("Ignoring failure to delete ebtables chain for vm " + vm_name) -def default_ebtables_rules(vm_name, vm_ip, vm_mac, vif): +def default_ebtables_rules(vm_name, vm_ip, vm_mac, vif, is_first_nic=False): eb_vm_chain=ebtables_chain_name(vm_name) vmchain_in = eb_vm_chain + "-in" vmchain_out = eb_vm_chain + "-out" vmchain_in_ips = eb_vm_chain + "-in-ips" vmchain_out_ips = eb_vm_chain + "-out-ips" + vmchain_in_src = eb_vm_chain + "-in-src" + vmchain_out_dst = eb_vm_chain + "-out-dst" - for chain in [vmchain_in, vmchain_out, vmchain_in_ips, vmchain_out_ips]: + if not is_first_nic: + try: + execute("ebtables -t nat -A PREROUTING -i " + vif + " -j " + vmchain_in) + execute("ebtables -t nat -A POSTROUTING -o " + vif + " -j " + vmchain_out) + execute("ebtables -t nat -I " + vmchain_in_src + " -s " + vm_mac + " -j RETURN") + if vm_ip: + execute("ebtables -t nat -I " + vmchain_in_ips + " -p ARP -s " + vm_mac + " --arp-mac-src " + vm_mac + " --arp-ip-src " + vm_ip + " -j RETURN") + execute("ebtables -t nat -I " + vmchain_out_dst + " -p ARP --arp-op Reply --arp-mac-dst " + vm_mac + " -j RETURN") + if vm_ip: + execute("ebtables -t nat -I " + vmchain_out_ips + " -p ARP --arp-ip-dst " + vm_ip + " -j RETURN") + except: + logging.debug("Failed to program rules for additional nic " + vif) + return False + return + + destroy_ebtables_rules(vm_name, vif) + + for chain in [vmchain_in, vmchain_out, vmchain_in_ips, vmchain_out_ips, vmchain_in_src, vmchain_out_dst]: try: execute("ebtables -t nat -N " + chain) except: @@ -256,17 +313,19 @@ def default_ebtables_rules(vm_name, vm_ip, vm_mac, vif): execute("ebtables -t nat -A POSTROUTING -o " + vif + " -j " + vmchain_out) execute("ebtables -t nat -A " + vmchain_in_ips + " -j DROP") execute("ebtables -t nat -A " + vmchain_out_ips + " -j DROP") + execute("ebtables -t nat -A " + vmchain_in_src + " -j DROP") + execute("ebtables -t nat -A " + vmchain_out_dst + " -p ARP --arp-op Reply -j DROP") + except: logging.debug("Failed to program default rules") return False try: - execute("ebtables -t nat -A " + vmchain_in + " -s ! " + vm_mac + " -j DROP") - execute("ebtables -t nat -A " + vmchain_in + " -p ARP -s ! " + vm_mac + " -j DROP") - execute("ebtables -t nat -A " + vmchain_in + " -p ARP --arp-mac-src ! " + vm_mac + " -j DROP") + execute("ebtables -t nat -A " + vmchain_in + " -j " + vmchain_in_src) + execute("ebtables -t nat -I " + vmchain_in_src + " -s " + vm_mac + " -j RETURN") + execute("ebtables -t nat -A " + vmchain_in + " -p ARP -j " + vmchain_in_ips) if vm_ip: - execute("ebtables -t nat -A " + vmchain_in + " -p ARP -j " + vmchain_in_ips) - execute("ebtables -t nat -I " + vmchain_in_ips + " -p ARP --arp-ip-src " + vm_ip + " -j RETURN") + execute("ebtables -t nat -I " + vmchain_in_ips + " -p ARP -s " + vm_mac + " --arp-mac-src " + vm_mac + " --arp-ip-src " + vm_ip + " -j RETURN") execute("ebtables -t nat -A " + vmchain_in + " -p ARP --arp-op Request -j ACCEPT") execute("ebtables -t nat -A " + vmchain_in + " -p ARP --arp-op Reply -j ACCEPT") execute("ebtables -t nat -A " + vmchain_in + " -p ARP -j DROP") @@ -275,9 +334,10 @@ def default_ebtables_rules(vm_name, vm_ip, vm_mac, vif): return False try: - execute("ebtables -t nat -A " + vmchain_out + " -p ARP --arp-op Reply --arp-mac-dst ! " + vm_mac + " -j DROP") + execute("ebtables -t nat -A " + vmchain_out + " -p ARP --arp-op Reply -j " + vmchain_out_dst) + execute("ebtables -t nat -I " + vmchain_out_dst + " -p ARP --arp-op Reply --arp-mac-dst " + vm_mac + " -j RETURN") + execute("ebtables -t nat -A " + vmchain_out + " -p ARP -j " + vmchain_out_ips ) if vm_ip: - execute("ebtables -t nat -A " + vmchain_out + " -p ARP -j " + vmchain_out_ips ) execute("ebtables -t nat -I " + vmchain_out_ips + " -p ARP --arp-ip-dst " + vm_ip + " -j RETURN") execute("ebtables -t nat -A " + vmchain_out + " -p ARP --arp-op Request -j ACCEPT") execute("ebtables -t nat -A " + vmchain_out + " -p ARP --arp-op Reply -j ACCEPT") @@ -286,6 +346,28 @@ def default_ebtables_rules(vm_name, vm_ip, vm_mac, vif): logging.debug("Failed to program default ebtables OUT rules") return False +def refactor_ebtable_rules(vm_name): + vmchain_in = vm_name + "-in" + vmchain_in_ips = vm_name + "-in-ips" + vmchain_in_src = vm_name + "-in-src" + + try: + execute("ebtables -t nat -L " + vmchain_in_src) + logging.debug("Chain '" + vmchain_in_src + "' exists, ebtables rules have newer version, skip refactoring") + return True + except: + logging.debug("Chain '" + vmchain_in_src + "' does not exist, ebtables rules have old version, start refactoring") + + vif = execute("ebtables -t nat -L PREROUTING | grep " + vmchain_in + " | awk '{print $2}'").strip() + vm_mac = execute("ebtables -t nat -L " + vmchain_in + " | grep arp-mac-src | awk '{print $5}'").strip() + vm_ips = execute("ebtables -t nat -L " + vmchain_in_ips + " | grep arp-ip-src | awk '{print $4}'").split('\n') + + destroy_ebtables_rules(vm_name, vif) + default_ebtables_rules(vm_name, None, vm_mac, vif, True) + ebtables_rules_vmip(vm_name, vm_mac, vm_ips, "-A") + + logging.debug("Refactoring ebtables rules for vm " + vm_name + " is done") + return True def default_network_rules_systemvm(vm_name, localbrname): bridges = get_bridges(vm_name) @@ -382,8 +464,9 @@ def add_to_ipset(ipsetname, ips, action): return result -def network_rules_vmSecondaryIp(vm_name, ip_secondary, action): +def network_rules_vmSecondaryIp(vm_name, vm_mac, ip_secondary, action): logging.debug("vmName = "+ vm_name) + logging.debug("vmMac = " + vm_mac) logging.debug("action = "+ action) vmchain = vm_name @@ -393,16 +476,16 @@ def network_rules_vmSecondaryIp(vm_name, ip_secondary, action): add_to_ipset(vmchain, ip4s, action) - #add ebtables rules for the secondary ips - ebtables_rules_vmip(vm_name, ip4s, action) - #add ipv6 addresses to ipv6 ipset add_to_ipset(vmchain6, ip6s, action) - return True + #add ebtables rules for the secondary ip + refactor_ebtable_rules(vm_name) + ebtables_rules_vmip(vm_name, vm_mac, [ip_secondary], action) + return True -def ebtables_rules_vmip (vmname, ips, action): +def ebtables_rules_vmip (vmname, vmmac, ips, action): eb_vm_chain=ebtables_chain_name(vmname) vmchain_inips = eb_vm_chain + "-in-ips" vmchain_outips = eb_vm_chain + "-out-ips" @@ -415,46 +498,75 @@ def ebtables_rules_vmip (vmname, ips, action): if ip == 0 or ip == "0": continue try: - execute("ebtables -t nat " + action + " " + vmchain_inips + " -p ARP --arp-ip-src " + ip + " -j RETURN") + execute("ebtables -t nat " + action + " " + vmchain_inips + " -p ARP -s " + vmmac + " --arp-mac-src " + vmmac + " --arp-ip-src " + ip + " -j RETURN") execute("ebtables -t nat " + action + " " + vmchain_outips + " -p ARP --arp-ip-dst " + ip + " -j RETURN") except: logging.debug("Failed to program ebtables rules for secondary ip %s for vm %s with action %s" % (ip, vmname, action)) +def check_default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, sec_ips, is_first_nic=False): + brfw = get_br_fw(brname) + vmchain_default = '-'.join(vm_name.split('-')[:-1]) + "-def" + try: + rules = execute("iptables-save |grep -w %s |grep -w %s |grep -w %s" % (brfw, vif, vmchain_default)) + except: + rules = None + if rules is None or rules is "": + logging.debug("iptables rules do not exist, programming default rules for %s %s" % (vm_name,vif)) + default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, sec_ips, is_first_nic) + else: + vmchain_in = vm_name + "-in" + try: + rules = execute("ebtables -t nat -L PREROUTING | grep %s |grep -w %s" % (vmchain_in, vif)) + except: + rules = None + if rules is None or rules is "": + logging.debug("ebtables rules do not exist, programming default ebtables rules for %s %s" % (vm_name,vif)) + default_ebtables_rules(vm_name, vm_ip, vm_mac, vif, is_first_nic) + ips = sec_ips.split(';') + ips.pop() + ebtables_rules_vmip(vm_name, vm_mac, ips, "-I") + return True -def default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, sec_ips): +def default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, sec_ips, is_first_nic=False): if not add_fw_framework(brname): return False vmName = vm_name brfw = get_br_fw(brname) domID = get_vm_id(vm_name) - delete_rules_for_vm_in_bridge_firewall_chain(vmName) + vmchain = iptables_chain_name(vm_name) vmchain_egress = egress_chain_name(vm_name) vmchain_default = '-'.join(vmchain.split('-')[:-1]) + "-def" ipv6_link_local = ipv6_link_local_addr(vm_mac) - destroy_ebtables_rules(vm_name, vif) - - for chain in [vmchain, vmchain_egress, vmchain_default]: - try: - execute('iptables -N ' + chain) - except: - execute('iptables -F ' + chain) - - try: - execute('ip6tables -N ' + chain) - except: - execute('ip6tables -F ' + chain) - action = "-A" vmipsetName = ipset_chain_name(vm_name) vmipsetName6 = vmipsetName + '-6' - #create ipset and add vm ips to that ip set - if not create_ipset_forvm(vmipsetName): - logging.debug("failed to create ipset for rule %s", vmipsetName) - return False + if is_first_nic: + delete_rules_for_vm_in_bridge_firewall_chain(vmName) + destroy_ebtables_rules(vmName, vif) + + for chain in [vmchain, vmchain_egress, vmchain_default]: + try: + execute('iptables -N ' + chain) + except: + execute('iptables -F ' + chain) + + try: + execute('ip6tables -N ' + chain) + except: + execute('ip6tables -F ' + chain) + + #create ipset and add vm ips to that ip set + if not create_ipset_forvm(vmipsetName): + logging.debug("failed to create ipset for rule %s", vmipsetName) + return False + + if not create_ipset_forvm(vmipsetName6, family='inet6', type='hash:net'): + logging.debug("failed to create ivp6 ipset for rule %s", vmipsetName6) + return False #add primary nic ip to ipset if not add_to_ipset(vmipsetName, [vm_ip], action ): @@ -487,31 +599,30 @@ def default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, se #allow dhcp execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-in " + vif + " -p udp --dport 67 --sport 68 -j ACCEPT") execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-out " + vif + " -p udp --dport 68 --sport 67 -j ACCEPT") + execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-in " + vif + " -p udp --sport 67 -j DROP") #don't let vm spoof its ip address if vm_ip: execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-in " + vif + " -m set ! --match-set " + vmipsetName + " src -j DROP") + execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-out " + vif + " -m set ! --match-set " + vmipsetName + " dst -j DROP") execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-in " + vif + " -m set --match-set " + vmipsetName + " src -p udp --dport 53 -j RETURN ") execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-in " + vif + " -m set --match-set " + vmipsetName + " src -p tcp --dport 53 -j RETURN ") execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-in " + vif + " -m set --match-set " + vmipsetName + " src -j " + vmchain_egress) + execute("iptables -A " + vmchain_default + " -m physdev --physdev-is-bridged --physdev-out " + vif + " -j " + vmchain) execute("iptables -A " + vmchain + " -j DROP") except: logging.debug("Failed to program default rules for vm " + vm_name) return False - default_ebtables_rules(vm_name, vm_ip, vm_mac, vif) + default_ebtables_rules(vmchain, vm_ip, vm_mac, vif, is_first_nic) #default ebtables rules for vm secondary ips - ebtables_rules_vmip(vm_name, ip4s, "-I") + ebtables_rules_vmip(vm_name, vm_mac, ip4s, "-I") - if vm_ip: + if vm_ip and is_first_nic: if not write_rule_log_for_vm(vmName, vm_id, vm_ip, domID, '_initial_', '-1'): logging.debug("Failed to log default network rules, ignoring") - if not create_ipset_forvm(vmipsetName6, family='inet6', type='hash:net'): - logging.debug(" failed to create ivp6 ipset for rule " + str(tokens)) - return False - vm_ip6_addr = [ipv6_link_local] try: ip6 = ipaddress.ip_address(vm_ip6) @@ -763,7 +874,8 @@ def get_rule_logs_for_vms(): name = name.rstrip() if 1 not in [name.startswith(c) for c in ['r-', 's-', 'v-', 'i-'] ]: continue - network_rules_for_rebooted_vm(name) + # Move actions on rebooted vm to java code + # network_rules_for_rebooted_vm(name) if name.startswith('i-'): log = get_rule_log_for_vm(name) result.append(log) @@ -992,8 +1104,10 @@ def add_network_rules(vm_name, vm_id, vm_ip, vm_ip6, signature, seqno, vmMac, ru logging.debug("Rules already programmed for vm " + vm_name) return True - if changes[0] or changes[1] or changes[2] or changes[3]: - default_network_rules(vmName, vm_id, vm_ip, vm_ip6, vmMac, vif, brname, sec_ips) + if rules == "" or rules == None: + lines = [] + else: + lines = rules.split(';')[:-1] logging.debug("programming network rules for IP: " + vm_ip + " vmname=%s", vm_name) @@ -1007,7 +1121,7 @@ def add_network_rules(vm_name, vm_id, vm_ip, vm_ip6, signature, seqno, vmMac, ru execute('ip6tables -F ' + chain) except: logging.debug("Error flushing iptables rules for " + vm_name + ". Presuming firewall rules deleted, re-initializing." ) - default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vmMac, vif, brname, sec_ips) + default_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vmMac, vif, brname, sec_ips, True) egressrule_v4 = 0 egressrule_v6 = 0 @@ -1162,6 +1276,11 @@ def get_br_fw(brname): def add_fw_framework(brname): + try: + execute("modprobe br_netfilter") + except: + logging.debug("failed to load kernel module br_netfilter") + try: execute("sysctl -w net.bridge.bridge-nf-call-arptables=1") execute("sysctl -w net.bridge.bridge-nf-call-iptables=1") @@ -1239,6 +1358,236 @@ def add_fw_framework(brname): return False return False +def verify_network_rules(vm_name, vm_id, vm_ip, vm_ip6, vm_mac, vif, brname, sec_ips): + if vm_name is None or vm_ip is None or vm_mac is None: + print("vm_name, vm_ip and vm_mac must be specifed") + sys.exit(1) + + if vm_id is None: + vm_id = vm_name.split("-")[-2] + + if brname is None: + brname = execute("virsh domiflist %s |grep -w '%s' |tr -s ' '|cut -d ' ' -f3" % (vm_name, vm_mac)).strip() + if brname is None or brname == "": + print("Cannot find bridge") + sys.exit(1) + + if vif is None: + vif = execute("virsh domiflist %s |grep -w '%s' |tr -s ' '|cut -d ' ' -f1" % (vm_name, vm_mac)).strip() + if vif is None or vif == "": + print("Cannot find vif") + sys.exit(1) + + #vm_name = "i-2-55-VM" + #vm_id = 55 + #vm_ip = "10.11.118.128" + #vm_ip6 = "fe80::1c00:b4ff:fe00:5" + #vm_mac = "1e:00:b4:00:00:05" + #vif = "vnet11" + #brname = "cloudbr0" + #sec_ips = "10.11.118.133;10.11.118.135;10.11.118.138;" # end with ";" and seperated by ";" + + vm_ips = [] + if sec_ips is not None: + vm_ips = sec_ips.split(';') + vm_ips.pop() + vm_ips.reverse() + vm_ips.append(vm_ip) + + if not verify_ipset_for_vm(vm_name, vm_id, vm_ips, vm_ip6): + sys.exit(2) + if not verify_iptables_rules_for_bridge(brname): + sys.exit(3) + if not verify_default_iptables_rules_for_vm(vm_name, vm_id, vm_ips, vm_ip6, vm_mac, vif, brname): + sys.exit(4) + if not verify_ebtables_rules_for_vm(vm_name, vm_id, vm_ips, vm_ip6, vm_mac, vif, brname): + sys.exit(5) + + sys.exit(0) + +def verify_ipset_for_vm(vm_name, vm_id, vm_ips, vm_ip6): + vmipsetName = ipset_chain_name(vm_name) + vmipsetName6 = vmipsetName + '-6' + + rules = [] + for rule in execute("ipset list %s" % vmipsetName).split('\n'): + rules.append(rule) + + # Check if all vm ips and ip6 exist + for vm_ip in vm_ips: + found = False + for rule in rules: + if rule == vm_ip: + found = True + break + if not found: + print("vm ip %s is not found" % vm_ip) + return False + + rules = [] + for rule in execute("ipset list %s" % vmipsetName6).split('\n'): + rules.append(rule) + + if vm_ip6 is not None: + found = False + for rule in rules: + if rule == vm_ip6: + found = True + break + if not found: + print("vm ipv6 %s is not found" % vm_ip6) + return False + + return True + +def verify_iptables_rules_for_bridge(brname): + brfw = get_br_fw(brname) + brfwin = brfw + "-IN" + brfwout = brfw + "-OUT" + + expected_rules = [] + expected_rules.append("-A FORWARD -o %s -m physdev --physdev-is-bridged -j %s" % (brname, brfw)) + expected_rules.append("-A FORWARD -i %s -m physdev --physdev-is-bridged -j %s" % (brname, brfw)) + expected_rules.append("-A %s -m state --state RELATED,ESTABLISHED -j ACCEPT" % (brfw)) + expected_rules.append("-A %s -m physdev --physdev-is-in --physdev-is-bridged -j %s" % (brfw, brfwin)) + expected_rules.append("-A %s -m physdev --physdev-is-out --physdev-is-bridged -j %s" % (brfw, brfwout)) + phydev = execute("brctl show | awk '/^%s[ \t]/ {print $4}'" % brname ).strip() + expected_rules.append("-A %s -m physdev --physdev-out %s --physdev-is-bridged -j ACCEPT" % (brfw, phydev)) + + rules = execute("iptables-save |grep -w %s |grep -v \"^:\"" % brfw).split('\n') + + return verify_expected_rules_exist(expected_rules, rules) + +def verify_default_iptables_rules_for_vm(vm_name, vm_id, vm_ips, vm_ip6, vm_mac, vif, brname): + brfw = get_br_fw(brname) + brfwin = brfw + "-IN" + brfwout = brfw + "-OUT" + vmchain = iptables_chain_name(vm_name) + vmchain_egress = egress_chain_name(vm_name) + vm_def = '-'.join(vm_name.split('-')[:-1]) + "-def" + + expected_rules = [] + expected_rules.append("-A %s -m physdev --physdev-in %s --physdev-is-bridged -j %s" % (brfwin, vif, vm_def)) + expected_rules.append("-A %s -m physdev --physdev-out %s --physdev-is-bridged -j %s" % (brfwout, vif, vm_def)) + expected_rules.append("-A %s -m state --state RELATED,ESTABLISHED -j ACCEPT" % (vm_def)) + expected_rules.append("-A %s -p udp -m physdev --physdev-in %s --physdev-is-bridged -m udp --sport 68 --dport 67 -j ACCEPT" % (vm_def, vif)) + expected_rules.append("-A %s -p udp -m physdev --physdev-out %s --physdev-is-bridged -m udp --sport 67 --dport 68 -j ACCEPT" % (vm_def, vif)) + expected_rules.append("-A %s -p udp -m physdev --physdev-in %s --physdev-is-bridged -m udp --sport 67 -j DROP" % (vm_def, vif)) + expected_rules.append("-A %s -m physdev --physdev-in %s --physdev-is-bridged -m set ! --match-set %s src -j DROP" % (vm_def, vif, vm_name)) + expected_rules.append("-A %s -m physdev --physdev-out %s --physdev-is-bridged -m set ! --match-set %s dst -j DROP" % (vm_def, vif, vm_name)) + expected_rules.append("-A %s -p udp -m physdev --physdev-in %s --physdev-is-bridged -m set --match-set %s src -m udp --dport 53 -j RETURN" % (vm_def, vif, vm_name)) + expected_rules.append("-A %s -p tcp -m physdev --physdev-in %s --physdev-is-bridged -m set --match-set %s src -m tcp --dport 53 -j RETURN" % (vm_def, vif, vm_name)) + expected_rules.append("-A %s -m physdev --physdev-in %s --physdev-is-bridged -m set --match-set %s src -j %s" % (vm_def, vif, vm_name, vmchain_egress)) + expected_rules.append("-A %s -m physdev --physdev-out %s --physdev-is-bridged -j %s" % (vm_def, vif, vmchain)) + + rules = execute("iptables-save |grep -E \"%s|%s\" |grep -v \"^:\"" % (vm_name, vm_def)).split('\n') + + return verify_expected_rules_in_order(expected_rules, rules) + +def verify_ebtables_rules_for_vm(vm_name, vm_id, vm_ips, vm_ip6, vm_mac, vif, brname): + vmchain_in = vm_name + "-in" + vmchain_out = vm_name + "-out" + vmchain_in_ips = vm_name + "-in-ips" + vmchain_out_ips = vm_name + "-out-ips" + vmchain_in_src = vm_name + "-in-src" + vmchain_out_dst = vm_name + "-out-dst" + + new_mac = trim_mac_address(vm_mac) + + # PREROUTING/POSTROUTING + expected_rules = [] + expected_rules.append("-A PREROUTING -i %s -j %s" % (vif, vmchain_in)) + expected_rules.append("-A POSTROUTING -o %s -j %s" % (vif, vmchain_out)) + rules = execute("ebtables-save |grep -E \"PREROUTING|POSTROUTING\" | grep %s" % vm_name).split('\n') + if not verify_expected_rules_exist(expected_rules, rules): + return False + + rules = execute("ebtables-save | grep %s" % vm_name).split('\n') + + # vmchain_in + expected_rules = [] + expected_rules.append("-A %s -j %s" % (vmchain_in, vmchain_in_src)) + expected_rules.append("-A %s -p ARP -j %s" % (vmchain_in, vmchain_in_ips)) + expected_rules.append("-A %s -p ARP --arp-op Request -j ACCEPT" % (vmchain_in)) + expected_rules.append("-A %s -p ARP --arp-op Reply -j ACCEPT" % (vmchain_in)) + expected_rules.append("-A %s -p ARP -j DROP" % (vmchain_in)) + if not verify_expected_rules_in_order(expected_rules, rules): + return False + + # vmchain_in_src + expected_rules = [] + expected_rules.append("-A %s -s %s -j RETURN" % (vmchain_in_src, new_mac)) + expected_rules.append("-A %s -j DROP" % (vmchain_in_src)) + if not verify_expected_rules_in_order(expected_rules, rules): + return False + + # vmchain_in_ips + expected_rules = [] + for vm_ip in vm_ips: + expected_rules.append("-A %s -p ARP -s %s --arp-ip-src %s --arp-mac-src %s -j RETURN" % (vmchain_in_ips, new_mac, vm_ip, new_mac)) + expected_rules.append("-A %s -j DROP" % (vmchain_in_ips)) + if not verify_expected_rules_in_order(expected_rules, rules): + return False + + # vmchain_out + expected_rules = [] + expected_rules.append("-A %s -p ARP --arp-op Reply -j %s" % (vmchain_out, vmchain_out_dst)) + expected_rules.append("-A %s -p ARP -j %s" % (vmchain_out, vmchain_out_ips)) + expected_rules.append("-A %s -p ARP --arp-op Request -j ACCEPT" % (vmchain_out)) + expected_rules.append("-A %s -p ARP --arp-op Reply -j ACCEPT" % (vmchain_out)) + expected_rules.append("-A %s -p ARP -j DROP" % (vmchain_out)) + if not verify_expected_rules_in_order(expected_rules, rules): + return False + + # vmchain_out_dst + expected_rules = [] + expected_rules.append("-A %s -p ARP --arp-op Reply --arp-mac-dst %s -j RETURN" % (vmchain_out_dst, new_mac)) + expected_rules.append("-A %s -p ARP --arp-op Reply -j DROP" % (vmchain_out_dst)) + if not verify_expected_rules_in_order(expected_rules, rules): + return False + + # vmchain_out_ips + expected_rules = [] + for vm_ip in vm_ips: + expected_rules.append("-A %s -p ARP --arp-ip-dst %s -j RETURN" % (vmchain_out_ips, vm_ip)) + expected_rules.append("-A %s -j DROP" % (vmchain_out_ips)) + if not verify_expected_rules_in_order(expected_rules, rules): + return False + + return True + +def trim_mac_address(vm_mac): + new_mac = "" + for mac in vm_mac.split(":"): + if mac.startswith("0"): + new_mac += ":" + mac[1:] + else: + new_mac += ":" + mac + return new_mac[1:] + +def verify_expected_rules_exist(expected_rules, rules): + # Check if expected rules exist + for expected_rule in expected_rules: + found = False + for rule in rules: + if rule == expected_rule: + found = True + break + if not found: + print("Rule '%s' is not found" % expected_rule) + return False + return True + +def verify_expected_rules_in_order(expected_rules, rules): + # Check if expected rules exist in order (!!!) + i = 0 + for rule in rules: + if i < len(expected_rules) and rule == expected_rules[i]: + i += 1 + if i != len(expected_rules): + print("Cannot find rule '%s'" % expected_rules[i]) + return False + return True if __name__ == '__main__': logging.basicConfig(filename="/var/log/cloudstack/agent/security_group.log", format="%(asctime)s - %(message)s", level=logging.DEBUG) @@ -1261,6 +1610,8 @@ def add_fw_framework(brname): parser.add_argument("--nicsecips", dest="nicSecIps") parser.add_argument("--action", dest="action") parser.add_argument("--privnic", dest="privnic") + parser.add_argument("--isFirstNic", action="store_true", dest="isFirstNic") + parser.add_argument("--check", action="store_true", dest="check") args = parser.parse_args() cmd = args.command logging.debug("Executing command: %s", cmd) @@ -1274,10 +1625,15 @@ def add_fw_framework(brname): if cmd == "can_bridge_firewall": can_bridge_firewall(args.privnic) - elif cmd == "default_network_rules": - default_network_rules(args.vmName, args.vmID, args.vmIP, args.vmIP6, args.vmMAC, args.vif, args.brname, args.nicSecIps) + elif cmd == "default_network_rules" and args.check: + check_default_network_rules(args.vmName, args.vmID, args.vmIP, args.vmIP6, args.vmMAC, args.vif, args.brname, args.nicSecIps, args.isFirstNic) + elif cmd == "default_network_rules" and not args.check: + default_network_rules(args.vmName, args.vmID, args.vmIP, args.vmIP6, args.vmMAC, args.vif, args.brname, args.nicSecIps, args.isFirstNic) elif cmd == "destroy_network_rules_for_vm": - destroy_network_rules_for_vm(args.vmName, args.vif) + if args.vmIP is None: + destroy_network_rules_for_vm(args.vmName, args.vif) + else: + destroy_network_rules_for_nic(args.vmName, args.vmIP, args.vmMAC, args.vif, args.nicSecIps) elif cmd == "default_network_rules_systemvm": default_network_rules_systemvm(args.vmName, args.localbrname) elif cmd == "get_rule_logs_for_vms": @@ -1285,11 +1641,13 @@ def add_fw_framework(brname): elif cmd == "add_network_rules": add_network_rules(args.vmName, args.vmID, args.vmIP, args.vmIP6, args.sig, args.seq, args.vmMAC, args.rules, args.vif, args.brname, args.nicSecIps) elif cmd == "network_rules_vmSecondaryIp": - network_rules_vmSecondaryIp(args.vmName, args.nicSecIps, args.action) + network_rules_vmSecondaryIp(args.vmName, args.vmMAC, args.nicSecIps, args.action) elif cmd == "cleanup_rules": cleanup_rules() elif cmd == "post_default_network_rules": post_default_network_rules(args.vmName, args.vmID, args.vmIP, args.vmMAC, args.vif, args.brname, args.dhcpSvr, args.hostIp, args.hostMacAddr) + elif cmd == "verify_network_rules": + verify_network_rules(args.vmName, args.vmID, args.vmIP, args.vmIP6, args.vmMAC, args.vif, args.brname, args.nicSecIps) else: logging.debug("Unknown command: " + cmd) sys.exit(1) diff --git a/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl.java b/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl.java index a3a3f9b54303..5bb7767252ad 100644 --- a/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl.java +++ b/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl.java @@ -59,6 +59,7 @@ import com.cloud.agent.api.NetworkRulesVmSecondaryIpCommand; import com.cloud.agent.api.SecurityGroupRulesCmd; import com.cloud.agent.api.SecurityGroupRulesCmd.IpPortAndProto; +import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.agent.manager.Commands; import com.cloud.api.query.dao.SecurityGroupJoinDao; import com.cloud.configuration.Config; @@ -114,6 +115,8 @@ import com.cloud.vm.VirtualMachine.Event; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.VirtualMachineManager; +import com.cloud.vm.VirtualMachineProfile; +import com.cloud.vm.VirtualMachineProfileImpl; import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.NicSecondaryIpDao; import com.cloud.vm.dao.UserVmDao; @@ -378,6 +381,7 @@ public void handleVmStarted(VMInstanceVO vm) { } @DB + @Override public void scheduleRulesetUpdateToHosts(final List affectedVms, final boolean updateSeqno, Long delayMs) { if (affectedVms.size() == 0) { return; @@ -514,8 +518,35 @@ protected SecurityGroupRulesCmd generateRulesetCmd(String vmName, String guestIp egressResult.add(ipPortAndProto); } } - return new SecurityGroupRulesCmd(guestIp, guestIp6, guestMac, vmName, vmId, signature, seqnum, ingressResult.toArray(new IpPortAndProto[ingressResult.size()]), + SecurityGroupRulesCmd cmd = new SecurityGroupRulesCmd(guestIp, guestIp6, guestMac, vmName, vmId, signature, seqnum, ingressResult.toArray(new IpPortAndProto[ingressResult.size()]), egressResult.toArray(new IpPortAndProto[egressResult.size()]), secIps); + + final VirtualMachineTO to = getVmTO(vmId); + cmd.setVmTO(to); + return cmd; + } + + protected VirtualMachineTO getVmTO(Long vmId) { + final VMInstanceVO vm = _vmDao.findById(vmId); + final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); + final List nics = _nicDao.listByVmId(profile.getId()); + Collections.sort(nics, new Comparator() { + @Override + public int compare(NicVO nic1, NicVO nic2) { + Long nicId1 = Long.valueOf(nic1.getDeviceId()); + Long nicId2 = Long.valueOf(nic2.getDeviceId()); + return nicId1.compareTo(nicId2); + } + }); + for (final NicVO nic : nics) { + final Network network = _networkModel.getNetwork(nic.getNetworkId()); + final NicProfile nicProfile = + new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(), null, _networkModel.isSecurityGroupSupportedInNetwork(network), + _networkModel.getNetworkTag(profile.getHypervisorType(), network)); + profile.addNic(nicProfile); + } + final VirtualMachineTO to = _itMgr.toVmTO(profile); + return to; } protected void handleVmStopped(VMInstanceVO vm) { @@ -1019,16 +1050,17 @@ public void doInTransactionWithoutResult(TransactionStatus status) { agentId = vm.getHostId(); if (agentId != null) { // get nic secondary ip address - String privateIp = vm.getPrivateIpAddress(); - NicVO nic = _nicDao.findByIp4AddressAndVmId(privateIp, vm.getId()); + NicVO nic = _nicDao.findFirstNicForVM(vm.getId()); List nicSecIps = null; if (nic != null) { if (nic.getSecondaryIp()) { //get secondary ips of the vm nicSecIps = _nicSecIpDao.getSecondaryIpAddressesForNic(nic.getId()); } + } else { + return; } - SecurityGroupRulesCmd cmd = generateRulesetCmd(vm.getInstanceName(), nic.getIPv6Address(), vm.getPrivateIpAddress(), vm.getPrivateMacAddress(), vm.getId(), + SecurityGroupRulesCmd cmd = generateRulesetCmd(vm.getInstanceName(), nic.getIPv4Address(), nic.getIPv6Address(), vm.getPrivateMacAddress(), vm.getId(), generateRulesetSignature(ingressRules, egressRules), seqnum, ingressRules, egressRules, nicSecIps); Commands cmds = new Commands(cmd); try { @@ -1416,7 +1448,7 @@ public boolean securityGroupRulesForVmSecIp(long nicId, String secondaryIp, bool return true; } - String vmMac = vm.getPrivateMacAddress(); + String vmMac = nic.getMacAddress(); String vmName = vm.getInstanceName(); if (vmMac == null || vmName == null) { throw new InvalidParameterValueException("vm name or vm mac can't be null"); diff --git a/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl2.java b/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl2.java index 2d0ec61d09c2..5b4b85fc70c2 100644 --- a/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl2.java +++ b/server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl2.java @@ -177,16 +177,17 @@ public void sendRulesetUpdates(SecurityGroupWork work) { Map> egressRules = generateRulesForVM(userVmId, SecurityRuleType.EgressRule); Long agentId = vm.getHostId(); if (agentId != null) { - String privateIp = vm.getPrivateIpAddress(); - NicVO nic = _nicDao.findByIp4AddressAndVmId(privateIp, vm.getId()); + NicVO nic = _nicDao.findFirstNicForVM(vm.getId()); List nicSecIps = null; if (nic != null) { if (nic.getSecondaryIp()) { nicSecIps = _nicSecIpDao.getSecondaryIpAddressesForNic(nic.getId()); } + } else { + return; } SecurityGroupRulesCmd cmd = - generateRulesetCmd(vm.getInstanceName(), vm.getPrivateIpAddress(), nic.getIPv6Address(), vm.getPrivateMacAddress(), vm.getId(), null, work.getLogsequenceNumber(), + generateRulesetCmd(vm.getInstanceName(), nic.getIPv4Address(), nic.getIPv6Address(), vm.getPrivateMacAddress(), vm.getId(), null, work.getLogsequenceNumber(), ingressRules, egressRules, nicSecIps); cmd.setMsId(_serverId); if (s_logger.isDebugEnabled()) { diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index ad2e453080ea..3f28f1283d8a 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -3225,21 +3225,23 @@ public UserVm createAdvancedSecurityGroupVirtualMachine(DataCenter zone, Service throw new InvalidParameterValueException("Security group feature is not supported for vmWare hypervisor"); } // Only one network can be specified, and it should be security group enabled - if (networkIdList.size() > 1) { + if (networkIdList.size() > 1 && template.getHypervisorType() != HypervisorType.KVM && hypervisor != HypervisorType.KVM) { throw new InvalidParameterValueException("Only support one network per VM if security group enabled"); } - NetworkVO network = _networkDao.findById(networkIdList.get(0)); + for (Long networkId : networkIdList) { + NetworkVO network = _networkDao.findById(networkId); - if (network == null) { - throw new InvalidParameterValueException("Unable to find network by id " + networkIdList.get(0).longValue()); - } + if (network == null) { + throw new InvalidParameterValueException("Unable to find network by id " + networkId); + } - if (!_networkModel.isSecurityGroupSupportedInNetwork(network)) { - throw new InvalidParameterValueException("Network is not security group enabled: " + network.getId()); - } + if (!_networkModel.isSecurityGroupSupportedInNetwork(network)) { + throw new InvalidParameterValueException("Network is not security group enabled: " + network.getId()); + } - networkList.add(network); + networkList.add(network); + } isSecurityGroupEnabledNetworkUsed = true; } else { @@ -3253,10 +3255,6 @@ public UserVm createAdvancedSecurityGroupVirtualMachine(DataCenter zone, Service boolean isSecurityGroupEnabled = _networkModel.isSecurityGroupSupportedInNetwork(network); if (isSecurityGroupEnabled) { - if (networkIdList.size() > 1) { - throw new InvalidParameterValueException("Can't create a vm with multiple networks one of" + " which is Security Group enabled"); - } - isSecurityGroupEnabledNetworkUsed = true; } diff --git a/test/integration/component/test_multiple_nic_support.py b/test/integration/component/test_multiple_nic_support.py new file mode 100644 index 000000000000..cf3a23364957 --- /dev/null +++ b/test/integration/component/test_multiple_nic_support.py @@ -0,0 +1,629 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" tests for supporting multiple NIC's in advanced zone with security groups in cloudstack 4.14.0.0 + +""" +# Import Local Modules +from nose.plugins.attrib import attr +from marvin.cloudstackTestCase import cloudstackTestCase, unittest +from marvin.sshClient import SshClient +from marvin.lib.utils import (validateList, + cleanup_resources, + get_host_credentials, + get_process_status, + execute_command_in_host, + random_gen) +from marvin.lib.base import (PhysicalNetwork, + Account, + Host, + TrafficType, + Domain, + Network, + NetworkOffering, + VirtualMachine, + ServiceOffering, + Zone, + NIC, + SecurityGroup) +from marvin.lib.common import (get_domain, + get_zone, + get_template, + list_virtual_machines, + list_routers, + list_hosts, + get_free_vlan) +from marvin.codes import (PASS, FAILED) +import logging +import random +import time + +class TestMulipleNicSupport(cloudstackTestCase): + @classmethod + def setUpClass(cls): + cls.testClient = super( + TestMulipleNicSupport, + cls).getClsTestClient() + cls.apiclient = cls.testClient.getApiClient() + cls.testdata = cls.testClient.getParsedTestDataConfig() + cls.services = cls.testClient.getParsedTestDataConfig() + zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests()) + cls.zone = Zone(zone.__dict__) + cls._cleanup = [] + + if str(cls.zone.securitygroupsenabled) != "True": + sys.exit(1) + + cls.logger = logging.getLogger("TestMulipleNicSupport") + cls.stream_handler = logging.StreamHandler() + cls.logger.setLevel(logging.DEBUG) + cls.logger.addHandler(cls.stream_handler) + + # Get Domain and templates + cls.domain = get_domain(cls.apiclient) + cls.services['mode'] = cls.zone.networktype + + cls.template = get_template(cls.apiclient, cls.zone.id, hypervisor="KVM") + if cls.template == FAILED: + sys.exit(1) + + # Create new domain, account, network and VM + cls.user_domain = Domain.create( + cls.apiclient, + services=cls.testdata["acl"]["domain2"], + parentdomainid=cls.domain.id) + + # Create account + cls.account1 = Account.create( + cls.apiclient, + cls.testdata["acl"]["accountD2"], + admin=True, + domainid=cls.user_domain.id + ) + + # Create small service offering + cls.service_offering = ServiceOffering.create( + cls.apiclient, + cls.testdata["service_offerings"]["small"] + ) + + cls._cleanup.append(cls.service_offering) + cls.services["network"]["zoneid"] = cls.zone.id + cls.network_offering = NetworkOffering.create( + cls.apiclient, + cls.services["network_offering"], + ) + # Enable Network offering + cls.network_offering.update(cls.apiclient, state='Enabled') + + cls._cleanup.append(cls.network_offering) + cls.testdata["virtual_machine"]["zoneid"] = cls.zone.id + cls.testdata["virtual_machine"]["template"] = cls.template.id + + if cls.zone.securitygroupsenabled: + # Enable networking for reaching to VM thorugh SSH + security_group = SecurityGroup.create( + cls.apiclient, + cls.testdata["security_group"], + account=cls.account1.name, + domainid=cls.account1.domainid + ) + + # Authorize Security group to SSH to VM + ingress_rule = security_group.authorize( + cls.apiclient, + cls.testdata["ingress_rule"], + account=cls.account1.name, + domainid=cls.account1.domainid + ) + + # Authorize Security group to SSH to VM + ingress_rule2 = security_group.authorize( + cls.apiclient, + cls.testdata["ingress_rule_ICMP"], + account=cls.account1.name, + domainid=cls.account1.domainid + ) + + cls.testdata["shared_network_offering_sg"]["specifyVlan"] = 'True' + cls.testdata["shared_network_offering_sg"]["specifyIpRanges"] = 'True' + cls.shared_network_offering = NetworkOffering.create( + cls.apiclient, + cls.testdata["shared_network_offering_sg"], + conservemode=False + ) + + NetworkOffering.update( + cls.shared_network_offering, + cls.apiclient, + id=cls.shared_network_offering.id, + state="enabled" + ) + + physical_network, vlan = get_free_vlan(cls.apiclient, cls.zone.id) + cls.testdata["shared_network_sg"]["physicalnetworkid"] = physical_network.id + + random_subnet_number = random.randrange(90, 99) + cls.testdata["shared_network_sg"]["name"] = "Shared-Network-SG-Test-vlan" + str(random_subnet_number) + cls.testdata["shared_network_sg"]["displaytext"] = "Shared-Network-SG-Test-vlan" + str(random_subnet_number) + cls.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number) + cls.testdata["shared_network_sg"]["startip"] = "192.168." + str(random_subnet_number) + ".240" + cls.testdata["shared_network_sg"]["endip"] = "192.168." + str(random_subnet_number) + ".250" + cls.testdata["shared_network_sg"]["gateway"] = "192.168." + str(random_subnet_number) + ".254" + cls.network1 = Network.create( + cls.apiclient, + cls.testdata["shared_network_sg"], + networkofferingid=cls.shared_network_offering.id, + zoneid=cls.zone.id, + accountid=cls.account1.name, + domainid=cls.account1.domainid + ) + + random_subnet_number = random.randrange(100, 110) + cls.testdata["shared_network_sg"]["name"] = "Shared-Network-SG-Test-vlan" + str(random_subnet_number) + cls.testdata["shared_network_sg"]["displaytext"] = "Shared-Network-SG-Test-vlan" + str(random_subnet_number) + cls.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number) + cls.testdata["shared_network_sg"]["startip"] = "192.168." + str(random_subnet_number) + ".240" + cls.testdata["shared_network_sg"]["endip"] = "192.168." + str(random_subnet_number) + ".250" + cls.testdata["shared_network_sg"]["gateway"] = "192.168." + str(random_subnet_number) + ".254" + cls.network2 = Network.create( + cls.apiclient, + cls.testdata["shared_network_sg"], + networkofferingid=cls.shared_network_offering.id, + zoneid=cls.zone.id, + accountid=cls.account1.name, + domainid=cls.account1.domainid + ) + + random_subnet_number = random.randrange(111, 120) + cls.testdata["shared_network_sg"]["name"] = "Shared-Network-SG-Test-vlan" + str(random_subnet_number) + cls.testdata["shared_network_sg"]["displaytext"] = "Shared-Network-SG-Test-vlan" + str(random_subnet_number) + cls.testdata["shared_network_sg"]["vlan"] = "vlan://" + str(random_subnet_number) + cls.testdata["shared_network_sg"]["startip"] = "192.168." + str(random_subnet_number) + ".240" + cls.testdata["shared_network_sg"]["endip"] = "192.168." + str(random_subnet_number) + ".250" + cls.testdata["shared_network_sg"]["gateway"] = "192.168." + str(random_subnet_number) + ".254" + cls.network3 = Network.create( + cls.apiclient, + cls.testdata["shared_network_sg"], + networkofferingid=cls.shared_network_offering.id, + zoneid=cls.zone.id, + accountid=cls.account1.name, + domainid=cls.account1.domainid + ) + + try: + cls.virtual_machine1 = VirtualMachine.create( + cls.apiclient, + cls.testdata["virtual_machine"], + accountid=cls.account1.name, + domainid=cls.account1.domainid, + serviceofferingid=cls.service_offering.id, + templateid=cls.template.id, + securitygroupids=[security_group.id], + networkids=cls.network1.id + ) + for nic in cls.virtual_machine1.nic: + if nic.isdefault: + cls.virtual_machine1.ssh_ip = nic.ipaddress + cls.virtual_machine1.default_network_id = nic.networkid + break + except Exception as e: + cls.fail("Exception while deploying virtual machine: %s" % e) + + try: + cls.virtual_machine2 = VirtualMachine.create( + cls.apiclient, + cls.testdata["virtual_machine"], + accountid=cls.account1.name, + domainid=cls.account1.domainid, + serviceofferingid=cls.service_offering.id, + templateid=cls.template.id, + securitygroupids=[security_group.id], + networkids=[str(cls.network1.id), str(cls.network2.id)] + ) + for nic in cls.virtual_machine2.nic: + if nic.isdefault: + cls.virtual_machine2.ssh_ip = nic.ipaddress + cls.virtual_machine2.default_network_id = nic.networkid + break + except Exception as e: + cls.fail("Exception while deploying virtual machine: %s" % e) + + cls._cleanup.append(cls.virtual_machine1) + cls._cleanup.append(cls.virtual_machine2) + cls._cleanup.append(cls.network1) + cls._cleanup.append(cls.network2) + cls._cleanup.append(cls.network3) + cls._cleanup.append(cls.shared_network_offering) + if cls.zone.securitygroupsenabled: + cls._cleanup.append(security_group) + cls._cleanup.append(cls.account1) + cls._cleanup.append(cls.user_domain) + + @classmethod + def tearDownClass(self): + try: + cleanup_resources(self.apiclient, self._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.cleanup = [] + return + + def tearDown(self): + try: + cleanup_resources(self.apiclient, self.cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def verify_network_rules(self, vm_id): + virtual_machine = VirtualMachine.list( + self.apiclient, + id=vm_id + ) + vm = virtual_machine[0] + hosts = list_hosts( + self.apiclient, + id=vm.hostid + ) + host = hosts[0] + if host.hypervisor.lower() not in "kvm": + return + host.user, host.password = get_host_credentials(self.config, host.ipaddress) + for nic in vm.nic: + secips = "" + if len(nic.secondaryip) > 0: + for secip in nic.secondaryip: + secips += secip.ipaddress + ";" + command="/usr/share/cloudstack-common/scripts/vm/network/security_group.py verify_network_rules --vmname %s --vmip %s --vmmac %s --nicsecips '%s'" % (vm.instancename, nic.ipaddress, nic.macaddress, secips) + self.logger.debug("Executing command '%s' in host %s" % (command, host.ipaddress)) + result=execute_command_in_host(host.ipaddress, 22, + host.user, + host.password, + command) + if len(result) > 0: + self.fail("The iptables/ebtables rules for nic %s on vm %s on host %s are not correct" %(nic.ipaddress, vm.instancename, host.name)) + + @attr(tags=["adeancedsg"], required_hardware="false") + def test_01_create_vm_with_multiple_nics(self): + """Create Vm with multiple NIC's + + Steps: + # 1. Create more than 1 isolated or shared network + # 2. Create a vm and select more than 1 network while deploying + # 3. Vm is deployed successfully with 1 nic from each network + # 4. All the vm's should be pingable + :return: + """ + virtual_machine = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine2.id + ) + self.assertEqual( + len(virtual_machine), 1, + "Virtual Machine create with 2 NIC's failed") + + nicIdInVm = virtual_machine[0].nic[0] + self.assertIsNotNone(nicIdInVm, "NIC 1 not found in Virtual Machine") + + nicIdInVm = virtual_machine[0].nic[1] + self.assertIsNotNone(nicIdInVm, "NIC 2 not found in Virtual Machine") + + self.verify_network_rules(self.virtual_machine2.id) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_02_add_nic_to_vm(self): + """Create VM with single NIC and then add additional NIC + + Steps: + # 1. Create a VM by selecting one default NIC + # 2. Create few more isolated or shared networks + # 3. Add extra NIC's to the vm from the newly created networks + # 4. The deployed VM should have extra nic's added in the above + # step without any fail + # 5. The IP's of the extra NIC's should be pingable + :return: + """ + self.virtual_machine1.add_nic(self.apiclient, self.network2.id) + + virtual_machine = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine1.id + ) + + nicIdInVm = virtual_machine[0].nic[1] + self.assertIsNotNone(nicIdInVm, "Second NIC not found") + + self.verify_network_rules(self.virtual_machine1.id) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_03_add_ip_to_default_nic(self): + """ Add secondary IP's to the VM + + Steps: + # 1. Create a VM with more than 1 NIC + # 2) Navigate to Instances->NIC->Edit Secondary IP's + # ->Aquire new Secondary IP" + # 3) Add as many secondary Ip as possible to the VM + # 4) Configure the secondary IP's by referring to "Configure + # the secondary IP's" in the "Action Item" section + :return: + """ + ipaddress = NIC.addIp( + self.apiclient, + id=self.virtual_machine2.nic[0].id + ) + + self.assertIsNotNone( + ipaddress, + "Unable to add secondary IP to the default NIC") + + self.verify_network_rules(self.virtual_machine2.id) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_04_add_ip_to_remaining_nics(self): + """ Add secondary IP's to remaining NIC's + + Steps: + # 1) Create a VM with more than 1 NIC + # 2)Navigate to Instances-NIC's->Edit Secondary IP's + # ->Acquire new Secondary IP + # 3) Add secondary IP to all the NIC's of the VM + # 4) Confiugre the secondary IP's by referring to "Configure the + # secondary IP's" in the "Action Item" section + :return: + """ + + self.virtual_machine1.add_nic(self.apiclient, self.network3.id) + + vms = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine1.id + ) + + self.assertIsNotNone( + vms[0].nic[2], + "Third NIC is not added successfully to the VM") + + vms1_nic1_id = vms[0].nic[1]['id'] + vms1_nic2_id = vms[0].nic[2]['id'] + + ipaddress21 = NIC.addIp( + self.apiclient, + id=vms1_nic1_id + ) + + ipaddress22 = NIC.addIp( + self.apiclient, + id=vms1_nic1_id + ) + + self.assertIsNotNone( + ipaddress21, + "Unable to add first secondary IP to the second nic") + self.assertIsNotNone( + ipaddress22, + "Unable to add second secondary IP to second NIC") + + ipaddress31 = NIC.addIp( + self.apiclient, + id=vms1_nic2_id + ) + + ipaddress32 = NIC.addIp( + self.apiclient, + id=vms1_nic2_id + ) + + self.assertIsNotNone( + ipaddress31, + "Unable to add first secondary IP to third NIC") + self.assertIsNotNone( + ipaddress32, + "Unable to add second secondary IP to third NIC") + + self.verify_network_rules(self.virtual_machine1.id) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_05_stop_start_vm_with_multiple_nic(self): + """ Stop and Start a VM with Multple NIC + + Steps: + # 1) Create a Vm with multiple NIC's + # 2) Configure secondary IP's on the VM + # 3) Try to stop/start the VM + # 4) Ping the IP's of the vm + # 5) Remove Secondary IP from one of the NIC + :return: + """ + ipaddress1 = NIC.addIp( + self.apiclient, + id=self.virtual_machine2.nic[0].id + ) + + ipaddress2 = NIC.addIp( + self.apiclient, + id=self.virtual_machine2.nic[1].id + ) + # Stop the VM with multiple NIC's + self.virtual_machine2.stop(self.apiclient) + + virtual_machine = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine2.id + ) + + self.assertEqual( + virtual_machine[0]['state'], 'Stopped', + "Could not stop the VM with multiple NIC's") + + if virtual_machine[0]['state'] == 'Stopped': + # If stopped then try to start the VM + self.virtual_machine2.start(self.apiclient) + virtual_machine = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine2.id + ) + self.assertEqual( + virtual_machine[0]['state'], 'Running', + "Could not start the VM with multiple NIC's") + + self.verify_network_rules(self.virtual_machine2.id) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_06_migrate_vm_with_multiple_nic(self): + """ Migrate a VM with Multple NIC + + Steps: + # 1) Create a Vm with multiple NIC's + # 2) Configure secondary IP's on the VM + # 3) Try to stop/start the VM + # 4) Ping the IP's of the vm + :return: + """ + # Skipping adding Secondary IP to NIC since its already + # done in the previous test cases + + virtual_machine = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine1.id + ) + old_host_id = virtual_machine[0]['hostid'] + + try: + hosts = Host.list( + self.apiclient, + virtualmachineid=self.virtual_machine1.id, + listall=True) + self.assertEqual( + validateList(hosts)[0], + PASS, + "hosts list validation failed") + + # Get a host which is not already assigned to VM + for host in hosts: + if host.id == old_host_id: + continue + else: + host_id = host.id + break + + self.virtual_machine1.migrate(self.apiclient, host_id) + except Exception as e: + self.fail("Exception occured: %s" % e) + + # List the vm again + virtual_machine = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine1.id) + + new_host_id = virtual_machine[0]['hostid'] + + self.assertNotEqual( + old_host_id, new_host_id, + "Migration of VM to new host failed" + ) + + self.verify_network_rules(self.virtual_machine1.id) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_07_remove_secondary_ip_from_nic(self): + """ Remove secondary IP from any NIC + Steps: + # 1) Navigate to Instances + # 2) Select any vm + # 3) NIC's ->Edit secondary IP's->Release IP + # 4) The secondary IP should be successfully removed + """ + virtual_machine = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine2.id) + + # Check which NIC is having secondary IP + secondary_ips = virtual_machine[0].nic[1].secondaryip + + for secondary_ip in secondary_ips: + NIC.removeIp(self.apiclient, ipaddressid=secondary_ip['id']) + + virtual_machine = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine2.id + ) + + self.assertFalse( + virtual_machine[0].nic[1].secondaryip, + 'Failed to remove secondary IP') + + self.verify_network_rules(self.virtual_machine2.id) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_08_remove_nic_from_vm(self): + """ Remove NIC from VM + Steps: + # 1) Navigate to Instances->select any vm->NIC's->NIC 2 + # ->Click on "X" button to remove the second NIC + # 2) Remove other NIC's as well from the VM + # 3) All the NIC's should be successfully removed from the VM + :return: + """ + virtual_machine = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine2.id) + + for nic in virtual_machine[0].nic: + if nic.isdefault: + continue + self.virtual_machine2.remove_nic(self.apiclient, nic.id) + + virtual_machine = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine2.id) + + self.assertEqual( + len(virtual_machine[0].nic), 1, + "Failed to remove all the nics from the virtual machine") + + self.verify_network_rules(self.virtual_machine2.id) + + @attr(tags=["advancedsg"], required_hardware="false") + def test_09_reboot_vm_with_multiple_nic(self): + """ Reboot a VM with Multple NIC + + Steps: + # 1) Create a Vm with multiple NIC's + # 2) Configure secondary IP's on the VM + # 3) Try to reboot the VM + # 4) Ping the IP's of the vm + :return: + """ + # Skipping adding Secondary IP to NIC since its already + # done in the previous test cases + + virtual_machine = VirtualMachine.list( + self.apiclient, + id=self.virtual_machine1.id + ) + try: + self.virtual_machine1.reboot(self.apiclient) + except Exception as e: + self.fail("Exception occured: %s" % e) + + self.verify_network_rules(self.virtual_machine1.id) + diff --git a/tools/marvin/marvin/lib/utils.py b/tools/marvin/marvin/lib/utils.py index 1ad457ccf5b9..f170e0dc3bf6 100644 --- a/tools/marvin/marvin/lib/utils.py +++ b/tools/marvin/marvin/lib/utils.py @@ -62,13 +62,15 @@ def _configure_timeout(hypervisor): return timeout -def _execute_ssh_command(hostip, port, username, password, ssh_command): +def _execute_ssh_command(hostip, port, username, password, ssh_command, timeout=5): #SSH to the machine ssh = SshClient(hostip, port, username, password) # Ensure the SSH login is successful while True: res = ssh.execute(ssh_command) - if "Connection refused".lower() in res[0].lower(): + if len(res) == 0: + return res + elif "Connection refused".lower() in res[0].lower(): pass elif res[0] != "Host key verification failed.": break @@ -228,6 +230,10 @@ def get_host_credentials(config, hostip): raise Exception("Unresolvable host %s error is %s" % (hostip, e)) raise KeyError("Please provide the marvin configuration file with credentials to your hosts") +def execute_command_in_host(hostip, port, username, password, command, hypervisor=None): + timeout = _configure_timeout(hypervisor) + result = _execute_ssh_command(hostip, port, username, password, command) + return result def get_process_status(hostip, port, username, password, linklocalip, command, hypervisor=None): """Double hop and returns a command execution result""" diff --git a/ui/scripts/instanceWizard.js b/ui/scripts/instanceWizard.js index a6fdfbb3b952..04cceaa9d06c 100644 --- a/ui/scripts/instanceWizard.js +++ b/ui/scripts/instanceWizard.js @@ -168,11 +168,16 @@ }); } - return $.grep(selectedNetworks, function(network) { + var total = $.grep(selectedNetworks, function(network) { return $.grep(network.service, function(service) { return service.name == 'SecurityGroup'; }).length; }).length; //return total number of selected sg networks + + if (total > 0 && selectedHypervisor == "KVM") { + return -1; // vm with multiple IPs is supported in KVM + } + return total; }, // Data providers for each wizard step @@ -1284,8 +1289,10 @@ if (selectedZoneObj.networktype == "Advanced" && selectedZoneObj.securitygroupsenabled == true) { // Advanced SG-enabled zone var array2 = []; + var array3 = []; var myNetworks = $('.multi-wizard:visible form').data('my-networks'); //widget limitation: If using an advanced security group zone, get the guest networks like this - var defaultNetworkId = $('.multi-wizard:visible form').find('input[name=defaultNetwork]:checked').val(); + var defaultNetworkId = $('.multi-wizard:visible form').data('defaultNetwork'); + //var defaultNetworkId = $('.multi-wizard:visible form').find('input[name=defaultNetwork]:checked').val(); var checkedNetworkIdArray; if (typeof(myNetworks) == "object" && myNetworks.length != null) { //myNetworks is an array of string, e.g. ["203", "202"], @@ -1302,17 +1309,43 @@ array2.push(defaultNetworkId); } - //then, add other checked networks + var myNetworkIps = $('.multi-wizard:visible form').data('my-network-ips'); if (checkedNetworkIdArray.length > 0) { for (var i = 0; i < checkedNetworkIdArray.length; i++) { - if (checkedNetworkIdArray[i] != defaultNetworkId) //exclude defaultNetworkId that has been added to array2 + if (checkedNetworkIdArray[i] == defaultNetworkId) { + array2.unshift(defaultNetworkId); + + var ipToNetwork = { + networkid: defaultNetworkId + }; + if (myNetworkIps[i] != null && myNetworkIps[i].length > 0) { + $.extend(ipToNetwork, { + ip: myNetworkIps[i] + }); + } + array3.unshift(ipToNetwork); + } else { array2.push(checkedNetworkIdArray[i]); + + var ipToNetwork = { + networkid: checkedNetworkIdArray[i] + }; + if (myNetworkIps[i] != null && myNetworkIps[i].length > 0) { + $.extend(ipToNetwork, { + ip: myNetworkIps[i] + }); + } + array3.push(ipToNetwork); + } } } - $.extend(deployVmData, { - networkids : array2.join(",") - }); + for (var k = 0; k < array3.length; k++) { + deployVmData["iptonetworklist[" + k + "].networkid"] = array3[k].networkid; + if (array3[k].ip != undefined && array3[k].ip.length > 0) { + deployVmData["iptonetworklist[" + k + "].ip"] = array3[k].ip; + } + } } } else if (step6ContainerType == 'nothing-to-select') { if ("vpc" in args.context) { //from VPC tier diff --git a/ui/scripts/instances.js b/ui/scripts/instances.js index 545f7428faa8..c6fb671e1845 100644 --- a/ui/scripts/instances.js +++ b/ui/scripts/instances.js @@ -3354,6 +3354,16 @@ label: 'label.description' } }], + viewAll: { + path: 'network.securityGroups', + attachTo: 'id', + label: 'label.security.groups', + title: function(args) { + var title = _l('label.security.groups'); + + return title; + } + }, dataProvider: function(args) { // args.response.success({data: args.context.instances[0].securitygroup}); $.ajax({ diff --git a/ui/scripts/network.js b/ui/scripts/network.js index 32671f5cea1b..77cce337f0cc 100644 --- a/ui/scripts/network.js +++ b/ui/scripts/network.js @@ -4504,6 +4504,14 @@ var data = {}; listViewDataProvider(args, data); + if (args.context != null) { + if ("securityGroups" in args.context) { + $.extend(data, { + id: args.context.securityGroups[0].id + }); + } + } + $.ajax({ url: createURL('listSecurityGroups'), data: data, diff --git a/ui/scripts/ui-custom/instanceWizard.js b/ui/scripts/ui-custom/instanceWizard.js index b732b9ccf4b4..2450ed1b4a88 100644 --- a/ui/scripts/ui-custom/instanceWizard.js +++ b/ui/scripts/ui-custom/instanceWizard.js @@ -1516,14 +1516,29 @@ if (advSGFilter == 0) { //when total number of selected sg networks is 0, then 'Select Security Group' is skipped, go to step 6 directly showStep(6); - } else { //when total number of selected sg networks > 0 + } else if (advSGFilter > 0) { //when total number of selected sg networks > 0 if ($activeStep.find('input[type=checkbox]:checked').length > 1) { //when total number of selected networks > 1 cloudStack.dialog.notice({ message: "Can't create a vm with multiple networks one of which is Security Group enabled" }); return false; } + } else if (advSGFilter == -1) { // vm with multiple IPs is supported in KVM + var $selectNetwork = $activeStep.find('input[type=checkbox]:checked'); + var myNetworkIps = []; + $selectNetwork.each(function() { + var $specifyIp = $(this).parent().find('.specify-ip input[type=text]'); + myNetworkIps.push($specifyIp.val() == -1 ? null : $specifyIp.val()); + }); + $activeStep.closest('form').data('my-network-ips', myNetworkIps); + $selectNetwork.each(function() { + if ($(this).parent().find('input[type=radio]').is(':checked')) { + $activeStep.closest('form').data('defaultNetwork', $(this).val()); + return; + } + }) } + } } From 31adf6c47979bbb335a83da96a4032fe8b47e7ad Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 21 Feb 2020 00:29:52 -0300 Subject: [PATCH 30/30] Fix metalink filename mismatch error --- .../direct/download/MetalinkDirectTemplateDownloader.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java index 3a04bd2b5a01..c8e85277913b 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java @@ -60,18 +60,19 @@ public Pair downloadTemplate() { } boolean downloaded = false; int i = 0; + String downloadDir = getDirectDownloadTempPath(getTemplateId()); do { if (!isRedownload()) { setUrl(metalinkUrls.get(i)); } s_logger.info("Trying to download template from url: " + getUrl()); try { + setDownloadedFilePath(downloadDir + File.separator + getFileNameFromUrl()); File f = new File(getDownloadedFilePath()); if (f.exists()) { f.delete(); f.createNewFile(); } - setDownloadedFilePath(f.getAbsolutePath()); request = createRequest(getUrl(), reqHeaders); Pair downloadResult = super.downloadTemplate(); downloaded = downloadResult.first();