From ae3fa5d0de3407a63e173cb367af3b517691039d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Thu, 20 Jun 2024 09:26:36 -0300 Subject: [PATCH 01/24] Add configuration to limit the number of rows deleted from vm_stats (#8740) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Jandre --- .../java/com/cloud/vm/dao/VmStatsDao.java | 6 ++++-- .../java/com/cloud/vm/dao/VmStatsDaoImpl.java | 21 ++++++++++++++++--- .../java/com/cloud/utils/db/GenericDao.java | 8 +++++++ .../com/cloud/utils/db/GenericDaoBase.java | 12 ++++++++++- .../java/com/cloud/server/StatsCollector.java | 12 ++++++----- .../com/cloud/server/StatsCollectorTest.java | 8 +++---- .../com/cloud/user/MockUsageEventDao.java | 5 +++++ 7 files changed, 57 insertions(+), 15 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java index 879faaf5c90..0d7aa703a8c 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java @@ -75,8 +75,10 @@ public interface VmStatsDao extends GenericDao { /** * Removes (expunges) all VM stats with {@code timestamp} less than * a given Date. - * @param limit the maximum date to keep stored. Records that exceed this limit will be removed. + * @param limitDate the maximum date to keep stored. Records that exceed this limit will be removed. + * @param limitPerQuery the maximum amount of rows to be removed in a single query. We loop if there are still rows to be removed after a given query. + * If 0 or negative, no limit is used. */ - void removeAllByTimestampLessThan(Date limit); + void removeAllByTimestampLessThan(Date limitDate, long limitPerQuery); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java index 1bef8f0626c..a98302e2136 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java @@ -21,6 +21,7 @@ import java.util.List; import javax.annotation.PostConstruct; +import org.apache.log4j.Logger; import org.springframework.stereotype.Component; import com.cloud.utils.db.Filter; @@ -33,6 +34,8 @@ import com.cloud.vm.VmStatsVO; @Component public class VmStatsDaoImpl extends GenericDaoBase implements VmStatsDao { + protected Logger logger = Logger.getLogger(getClass()); + protected SearchBuilder vmIdSearch; protected SearchBuilder vmIdTimestampGreaterThanEqualSearch; protected SearchBuilder vmIdTimestampLessThanEqualSearch; @@ -113,10 +116,22 @@ public class VmStatsDaoImpl extends GenericDaoBase implements V } @Override - public void removeAllByTimestampLessThan(Date limit) { + public void removeAllByTimestampLessThan(Date limitDate, long limitPerQuery) { SearchCriteria sc = timestampSearch.create(); - sc.setParameters("timestamp", limit); - expunge(sc); + sc.setParameters("timestamp", limitDate); + + logger.debug(String.format("Starting to remove all vm_stats rows older than [%s].", limitDate)); + + long totalRemoved = 0; + long removed; + + do { + removed = expunge(sc, limitPerQuery); + totalRemoved += removed; + logger.trace(String.format("Removed [%s] vm_stats rows on the last update and a sum of [%s] vm_stats rows older than [%s] until now.", removed, totalRemoved, limitDate)); + } while (limitPerQuery > 0 && removed >= limitPerQuery); + + logger.info(String.format("Removed a total of [%s] vm_stats rows older than [%s].", totalRemoved, limitDate)); } } diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java index 2fc02301cb7..b9199468bd1 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java @@ -229,6 +229,14 @@ public interface GenericDao { */ int expunge(final SearchCriteria sc); + /** + * Delete the entity beans specified by the search criteria with a given limit + * @param sc Search criteria + * @param limit Maximum number of rows that will be affected + * @return Number of rows deleted + */ + int expunge(SearchCriteria sc, long limit); + /** * expunge the removed rows. */ diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index 0eb45439769..3b950e1983d 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -1226,9 +1226,14 @@ public abstract class GenericDaoBase extends Compone } } - // FIXME: Does not work for joins. @Override public int expunge(final SearchCriteria sc) { + return expunge(sc, -1); + } + + // FIXME: Does not work for joins. + @Override + public int expunge(final SearchCriteria sc, long limit) { if (sc == null) { throw new CloudRuntimeException("Call to throw new expunge with null search Criteria"); } @@ -1241,6 +1246,11 @@ public abstract class GenericDaoBase extends Compone str.append(sc.getWhereClause()); } + if (limit > 0) { + str.append(" LIMIT "); + str.append(limit); + } + final String sql = str.toString(); final TransactionLegacy txn = TransactionLegacy.currentTxn(); diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 5aa6dd0a214..070508f2502 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -114,9 +114,6 @@ import com.cloud.org.Cluster; import com.cloud.resource.ResourceManager; import com.cloud.resource.ResourceState; import com.cloud.serializer.GsonHelper; -import com.cloud.server.StatsCollector.AbstractStatsCollector; -import com.cloud.server.StatsCollector.AutoScaleMonitor; -import com.cloud.server.StatsCollector.StorageCollector; import com.cloud.storage.ImageStoreDetailsUtil; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; @@ -287,6 +284,11 @@ public class StatsCollector extends ManagerBase implements ComponentMethodInterc protected static ConfigKey vmStatsCollectUserVMOnly = new ConfigKey<>("Advanced", Boolean.class, "vm.stats.user.vm.only", "false", "When set to 'false' stats for system VMs will be collected otherwise stats collection will be done only for user VMs", true); + protected static ConfigKey vmStatsRemoveBatchSize = new ConfigKey<>("Advanced", Long.class, "vm.stats.remove.batch.size", "0", "Indicates the" + + " limit applied to delete vm_stats entries while running the clean-up task. With this, ACS will run the delete query, applying the limit, as many times as necessary" + + " to delete all entries older than the value defined in vm.stats.max.retention.time. This is advised when retaining several days of records, which can lead to slowness" + + " on the delete query. Zero (0) means that no limit will be applied, therefore, the query will run once and without limit, keeping the default behavior.", true); + protected static ConfigKey vmDiskStatsRetentionEnabled = new ConfigKey<>("Advanced", Boolean.class, "vm.disk.stats.retention.enabled", "false", "When set to 'true' stats for VM disks will be stored in the database otherwise disk stats will not be stored", true); @@ -1965,7 +1967,7 @@ public class StatsCollector extends ManagerBase implements ComponentMethodInterc LOGGER.trace("Removing older VM stats records."); Date now = new Date(); Date limit = DateUtils.addMinutes(now, -maxRetentionTime); - vmStatsDao.removeAllByTimestampLessThan(limit); + vmStatsDao.removeAllByTimestampLessThan(limit, vmStatsRemoveBatchSize.value()); } /** @@ -2139,7 +2141,7 @@ public class StatsCollector extends ManagerBase implements ComponentMethodInterc @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {vmDiskStatsInterval, vmDiskStatsIntervalMin, vmNetworkStatsInterval, vmNetworkStatsIntervalMin, StatsTimeout, statsOutputUri, + return new ConfigKey[] {vmDiskStatsInterval, vmDiskStatsIntervalMin, vmNetworkStatsInterval, vmNetworkStatsIntervalMin, StatsTimeout, statsOutputUri, vmStatsRemoveBatchSize, vmStatsIncrementMetrics, vmStatsMaxRetentionTime, vmStatsCollectUserVMOnly, vmDiskStatsRetentionEnabled, vmDiskStatsMaxRetentionTime, MANAGEMENT_SERVER_STATUS_COLLECTION_INTERVAL, DATABASE_SERVER_STATUS_COLLECTION_INTERVAL, diff --git a/server/src/test/java/com/cloud/server/StatsCollectorTest.java b/server/src/test/java/com/cloud/server/StatsCollectorTest.java index 1f6a35cfbae..2b2451c66c7 100644 --- a/server/src/test/java/com/cloud/server/StatsCollectorTest.java +++ b/server/src/test/java/com/cloud/server/StatsCollectorTest.java @@ -28,7 +28,7 @@ import com.cloud.user.VmDiskStatisticsVO; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VmStats; import com.cloud.vm.VmStatsVO; -import com.cloud.vm.dao.VmStatsDao; +import com.cloud.vm.dao.VmStatsDaoImpl; import com.google.gson.Gson; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; @@ -81,7 +81,7 @@ public class StatsCollectorTest { private static final String DEFAULT_DATABASE_NAME = "cloudstack"; @Mock - VmStatsDao vmStatsDaoMock = Mockito.mock(VmStatsDao.class); + VmStatsDaoImpl vmStatsDaoMock; @Mock VmStatsEntry statsForCurrentIterationMock; @@ -304,7 +304,7 @@ public class StatsCollectorTest { statsCollector.cleanUpVirtualMachineStats(); - Mockito.verify(vmStatsDaoMock, Mockito.never()).removeAllByTimestampLessThan(Mockito.any()); + Mockito.verify(vmStatsDaoMock, Mockito.never()).removeAllByTimestampLessThan(Mockito.any(), Mockito.anyLong()); } @Test @@ -313,7 +313,7 @@ public class StatsCollectorTest { statsCollector.cleanUpVirtualMachineStats(); - Mockito.verify(vmStatsDaoMock).removeAllByTimestampLessThan(Mockito.any()); + Mockito.verify(vmStatsDaoMock).removeAllByTimestampLessThan(Mockito.any(), Mockito.anyLong()); } @Test diff --git a/server/src/test/java/com/cloud/user/MockUsageEventDao.java b/server/src/test/java/com/cloud/user/MockUsageEventDao.java index 52e1b1a02b4..4769126fdcb 100644 --- a/server/src/test/java/com/cloud/user/MockUsageEventDao.java +++ b/server/src/test/java/com/cloud/user/MockUsageEventDao.java @@ -210,6 +210,11 @@ public class MockUsageEventDao implements UsageEventDao{ return 0; } + @Override + public int expunge(SearchCriteria sc, long limit) { + return 0; + } + @Override public void expunge() { From 097359bef9ed2f7448083deda734806326e88d4f Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 21 Jun 2024 10:12:16 +0530 Subject: [PATCH 02/24] plugins/shutdown: fix triggerShutdown scheduling and response (#9276) Earlier the triggerShutdown API would immediately shutdown the MS and if it is the same MS on which API is called it would lead to error in the API call. This change adds a delay to the process so the MS would be able to send response to the API. Signed-off-by: Abhishek Kumar --- .../org/apache/cloudstack/shutdown/ShutdownManagerImpl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/shutdown/src/main/java/org/apache/cloudstack/shutdown/ShutdownManagerImpl.java b/plugins/shutdown/src/main/java/org/apache/cloudstack/shutdown/ShutdownManagerImpl.java index b8f5fb57155..da3bba54bdc 100644 --- a/plugins/shutdown/src/main/java/org/apache/cloudstack/shutdown/ShutdownManagerImpl.java +++ b/plugins/shutdown/src/main/java/org/apache/cloudstack/shutdown/ShutdownManagerImpl.java @@ -107,7 +107,10 @@ public class ShutdownManagerImpl extends ManagerBase implements ShutdownManager, this.shutdownTask = null; } this.shutdownTask = new ShutdownTask(this); - timer.scheduleAtFixedRate(shutdownTask, 0, 30L * 1000); + long period = 30L * 1000; + long delay = period / 2; + logger.debug(String.format("Scheduling shutdown task with delay: %d and period: %d", delay, period)); + timer.scheduleAtFixedRate(shutdownTask, delay, period); } @Override From 5ab23cd9c971b555cb80befe9c521dfb25f01fee Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 21 Jun 2024 10:28:18 +0530 Subject: [PATCH 03/24] Timeout config to copy the disks of remote KVM instance while importing the instance from an external host (#9213) * Added timeout config to copy the disks of remote KVM instance while importing the instance from an external host * Updated copy config units to mins * Cleanup remote converted file and local file when copy failed --- .../cloudstack/vm/UnmanagedVMsManager.java | 9 +++++++ .../agent/api/CopyRemoteVolumeCommand.java | 3 --- .../resource/LibvirtComputingResource.java | 27 +++++++++++++------ ...LibvirtCopyRemoteVolumeCommandWrapper.java | 17 ++++++------ .../ConfigurationManagerImpl.java | 2 ++ .../vm/UnmanagedVMsManagerImpl.java | 21 ++++++++------- .../java/com/cloud/utils/ssh/SshHelper.java | 8 +++++- 7 files changed, 58 insertions(+), 29 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java b/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java index 53aece94964..e3a484b35b3 100644 --- a/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java +++ b/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java @@ -30,6 +30,15 @@ public interface UnmanagedVMsManager extends VmImportService, UnmanageVMService, "If set to true, do not remove VM nics (and its MAC addresses) when unmanaging a VM, leaving them allocated but not reserved. " + "If set to false, nics are removed and MAC addresses can be reassigned", true, ConfigKey.Scope.Zone); + ConfigKey RemoteKvmInstanceDisksCopyTimeout = new ConfigKey<>(Integer.class, + "remote.kvm.instance.disks.copy.timeout", + "Advanced", + "30", + "Timeout (in mins) to prepare and copy the disks of remote KVM instance while importing the instance from an external host", + true, + ConfigKey.Scope.Global, + null); + static boolean isSupported(Hypervisor.HypervisorType hypervisorType) { return hypervisorType == VMware || hypervisorType == KVM; } diff --git a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java index 82bc4d7cb45..88e5669f3b1 100644 --- a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java +++ b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java @@ -23,14 +23,11 @@ import com.cloud.agent.api.to.StorageFilerTO; @LogLevel(LogLevel.Log4jLevel.Trace) public class CopyRemoteVolumeCommand extends Command { - String remoteIp; String username; String password; String srcFile; - String tmpPath; - StorageFilerTO storageFilerTO; public CopyRemoteVolumeCommand(String remoteIp, String username, String password) { 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 37aba35bb9c..76e4efdfd7f 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 @@ -5336,20 +5336,31 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv /* Scp volume from remote host to local directory */ - public String copyVolume(String srcIp, String username, String password, String localDir, String remoteFile, String tmpPath) { + public String copyVolume(String srcIp, String username, String password, String localDir, String remoteFile, String tmpPath, int timeoutInSecs) { + String outputFile = UUID.randomUUID().toString(); try { - String outputFile = UUID.randomUUID().toString(); StringBuilder command = new StringBuilder("qemu-img convert -O qcow2 "); command.append(remoteFile); - command.append(" "+tmpPath); + command.append(" " + tmpPath); command.append(outputFile); - s_logger.debug("Converting remoteFile: "+remoteFile); - SshHelper.sshExecute(srcIp, 22, username, null, password, command.toString()); - s_logger.debug("Copying remoteFile to: "+localDir); - SshHelper.scpFrom(srcIp, 22, username, null, password, localDir, tmpPath+outputFile); - s_logger.debug("Successfully copyied remoteFile to: "+localDir+"/"+outputFile); + s_logger.debug(String.format("Converting remote disk file: %s, output file: %s%s (timeout: %d secs)", remoteFile, tmpPath, outputFile, timeoutInSecs)); + SshHelper.sshExecute(srcIp, 22, username, null, password, command.toString(), timeoutInSecs * 1000); + s_logger.debug("Copying converted remote disk file " + outputFile + " to: " + localDir); + SshHelper.scpFrom(srcIp, 22, username, null, password, localDir, tmpPath + outputFile); + s_logger.debug("Successfully copied converted remote disk file to: " + localDir + "/" + outputFile); return outputFile; } catch (Exception e) { + try { + String deleteRemoteConvertedFileCmd = String.format("rm -f %s%s", tmpPath, outputFile); + SshHelper.sshExecute(srcIp, 22, username, null, password, deleteRemoteConvertedFileCmd); + } catch (Exception ignored) { + } + + try { + FileUtils.deleteQuietly(new File(localDir + "/" + outputFile)); + } catch (Exception ignored) { + } + throw new RuntimeException(e); } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java index e48edd8eec0..a5e1716da2e 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java @@ -46,7 +46,6 @@ public final class LibvirtCopyRemoteVolumeCommandWrapper extends CommandWrapper< @Override public Answer execute(final CopyRemoteVolumeCommand command, final LibvirtComputingResource libvirtComputingResource) { - String result = null; String srcIp = command.getRemoteIp(); String username = command.getUsername(); String password = command.getPassword(); @@ -56,23 +55,25 @@ public final class LibvirtCopyRemoteVolumeCommandWrapper extends CommandWrapper< KVMStoragePoolManager poolMgr = libvirtComputingResource.getStoragePoolMgr(); KVMStoragePool pool = poolMgr.getStoragePool(storageFilerTO.getType(), storageFilerTO.getUuid()); String dstPath = pool.getLocalPath(); + int timeoutInSecs = command.getWait(); try { if (storageFilerTO.getType() == Storage.StoragePoolType.Filesystem || storageFilerTO.getType() == Storage.StoragePoolType.NetworkFilesystem) { - String filename = libvirtComputingResource.copyVolume(srcIp, username, password, dstPath, srcFile, tmpPath); - s_logger.debug("Volume Copy Successful"); + String filename = libvirtComputingResource.copyVolume(srcIp, username, password, dstPath, srcFile, tmpPath, timeoutInSecs); + s_logger.debug("Volume " + srcFile + " copy successful, copied to file: " + filename); final KVMPhysicalDisk vol = pool.getPhysicalDisk(filename); final String path = vol.getPath(); long size = getVirtualSizeFromFile(path); - return new CopyRemoteVolumeAnswer(command, "", filename, size); + return new CopyRemoteVolumeAnswer(command, "", filename, size); } else { - return new Answer(command, false, "Unsupported Storage Pool"); + String msg = "Unsupported storage pool type: " + storageFilerTO.getType().toString() + ", only local and NFS pools are supported"; + return new Answer(command, false, msg); } - } catch (final Exception e) { - s_logger.error("Error while copying file from remote host: "+ e.getMessage()); - return new Answer(command, false, result); + s_logger.error("Error while copying volume file from remote host: " + e.getMessage(), e); + String msg = "Failed to copy volume due to: " + e.getMessage(); + return new Answer(command, false, msg); } } diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index f65dffb18cd..eb72a626036 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -135,6 +135,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.userdata.UserDataManager; import org.apache.cloudstack.utils.jsinterpreter.TagAsRuleHelper; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; +import org.apache.cloudstack.vm.UnmanagedVMsManager; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.ObjectUtils; @@ -557,6 +558,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati configValuesForValidation.add(StorageManager.STORAGE_POOL_CLIENT_TIMEOUT.key()); configValuesForValidation.add(StorageManager.STORAGE_POOL_CLIENT_MAX_CONNECTIONS.key()); configValuesForValidation.add(UserDataManager.VM_USERDATA_MAX_LENGTH_STRING); + configValuesForValidation.add(UnmanagedVMsManager.RemoteKvmInstanceDisksCopyTimeout.key()); } private void weightBasedParametersForValidation() { diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index ed4f377a896..6e7c0245ebc 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -794,13 +794,19 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { } } copyRemoteVolumeCommand.setTempPath(tmpPath); + int copyTimeout = UnmanagedVMsManager.RemoteKvmInstanceDisksCopyTimeout.value(); + if (copyTimeout <= 0) { + copyTimeout = Integer.valueOf(UnmanagedVMsManager.RemoteKvmInstanceDisksCopyTimeout.defaultValue()); + } + int copyTimeoutInSecs = copyTimeout * 60; + copyRemoteVolumeCommand.setWait(copyTimeoutInSecs); Answer answer = agentManager.easySend(dest.getHost().getId(), copyRemoteVolumeCommand); if (!(answer instanceof CopyRemoteVolumeAnswer)) { - throw new CloudRuntimeException("Error while copying volume"); + throw new CloudRuntimeException("Error while copying volume of remote instance: " + answer.getDetails()); } CopyRemoteVolumeAnswer copyRemoteVolumeAnswer = (CopyRemoteVolumeAnswer) answer; if(!copyRemoteVolumeAnswer.getResult()) { - throw new CloudRuntimeException("Error while copying volume"); + throw new CloudRuntimeException("Unable to copy volume of remote instance"); } diskProfile.setSize(copyRemoteVolumeAnswer.getSize()); DiskProfile profile = volumeManager.updateImportedVolume(type, diskOffering, vm, template, deviceId, @@ -813,7 +819,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { Volume.Type type, VirtualMachineTemplate template, Long deviceId, Long hostId, String diskPath, DiskProfile diskProfile) { List storagePools = primaryDataStoreDao.findLocalStoragePoolsByHostAndTags(hostId, null); - if(storagePools.size() < 1) { throw new CloudRuntimeException("Local Storage not found for host"); } @@ -826,7 +831,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { return new Pair<>(profile, storagePool); } - private Pair importKVMSharedDisk(VirtualMachine vm, DiskOffering diskOffering, Volume.Type type, VirtualMachineTemplate template, Long deviceId, Long poolId, String diskPath, DiskProfile diskProfile) { @@ -838,7 +842,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { return new Pair<>(profile, storagePool); } - private Pair importDisk(UnmanagedInstanceTO.Disk disk, VirtualMachine vm, Cluster cluster, DiskOffering diskOffering, Volume.Type type, String name, Long diskSize, Long minIops, Long maxIops, VirtualMachineTemplate template, Account owner, Long deviceId) { @@ -2335,7 +2338,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Import failed for Vm: %s. Suitable deployment destination not found", userVm.getInstanceName())); } - Map storage = dest.getStorageForDisks(); Volume volume = volumeDao.findById(diskProfile.getVolumeId()); StoragePool storagePool = storage.get(volume); @@ -2355,7 +2357,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { } diskProfile.setSize(checkVolumeAnswer.getSize()); - List> diskProfileStoragePoolList = new ArrayList<>(); try { long deviceId = 1L; @@ -2376,7 +2377,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { return userVm; } - private NetworkVO getDefaultNetwork(DataCenter zone, Account owner, boolean selectAny) throws InsufficientCapacityException, ResourceAllocationException { NetworkVO defaultNetwork = null; @@ -2503,6 +2503,9 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[]{UnmanageVMPreserveNic}; + return new ConfigKey[]{ + UnmanageVMPreserveNic, + RemoteKvmInstanceDisksCopyTimeout + }; } } diff --git a/utils/src/main/java/com/cloud/utils/ssh/SshHelper.java b/utils/src/main/java/com/cloud/utils/ssh/SshHelper.java index 6a2aa827856..1856c0b3838 100644 --- a/utils/src/main/java/com/cloud/utils/ssh/SshHelper.java +++ b/utils/src/main/java/com/cloud/utils/ssh/SshHelper.java @@ -38,12 +38,18 @@ import com.cloud.utils.Pair; public class SshHelper { private static final int DEFAULT_CONNECT_TIMEOUT = 180000; private static final int DEFAULT_KEX_TIMEOUT = 60000; + private static final int DEFAULT_WAIT_RESULT_TIMEOUT = 120000; private static final Logger s_logger = Logger.getLogger(SshHelper.class); public static Pair sshExecute(String host, int port, String user, File pemKeyFile, String password, String command) throws Exception { - return sshExecute(host, port, user, pemKeyFile, password, command, DEFAULT_CONNECT_TIMEOUT, DEFAULT_KEX_TIMEOUT, 120000); + return sshExecute(host, port, user, pemKeyFile, password, command, DEFAULT_CONNECT_TIMEOUT, DEFAULT_KEX_TIMEOUT, DEFAULT_WAIT_RESULT_TIMEOUT); + } + + public static Pair sshExecute(String host, int port, String user, File pemKeyFile, String password, String command, int waitResultTimeoutInMs) throws Exception { + + return sshExecute(host, port, user, pemKeyFile, password, command, DEFAULT_CONNECT_TIMEOUT, DEFAULT_KEX_TIMEOUT, waitResultTimeoutInMs); } public static void scpTo(String host, int port, String user, File pemKeyFile, String password, String remoteTargetDirectory, String localFile, String fileMode) From ded7b4dbe5a93baecad719fd6727698691456188 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 21 Jun 2024 07:16:06 +0200 Subject: [PATCH 04/24] test: fix test failure on ubuntu 24.04: "top: unknown option 'n'" (#9262) --- test/integration/smoke/test_service_offerings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/smoke/test_service_offerings.py b/test/integration/smoke/test_service_offerings.py index 62e39e195c2..c6a14a64471 100644 --- a/test/integration/smoke/test_service_offerings.py +++ b/test/integration/smoke/test_service_offerings.py @@ -1043,7 +1043,7 @@ class TestCpuCapServiceOfferings(cloudstackTestCase): #Get host CPU usage from top command before and after VM consuming 100% CPU find_pid_cmd = "ps -ax | grep '%s' | head -1 | awk '{print $1}'" % self.vm.id pid = ssh_host.execute(find_pid_cmd)[0] - cpu_usage_cmd = "top -b n 1 p %s | tail -1 | awk '{print $9}'" % pid + cpu_usage_cmd = "top -b -n 1 -p %s | tail -1 | awk '{print $9}'" % pid host_cpu_usage_before_str = ssh_host.execute(cpu_usage_cmd)[0] host_cpu_usage_before = round(float(host_cpu_usage_before_str)) From 083ac069ca47a8f96ed9306994e5fbe66352ebc0 Mon Sep 17 00:00:00 2001 From: Gabriel Pordeus Santos Date: Fri, 21 Jun 2024 05:58:46 -0300 Subject: [PATCH 05/24] fix assignvm template permission check (#8886) --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index f68b3a6237a..9f0adf9d5fd 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7223,10 +7223,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (template == null) { throw new InvalidParameterValueException(String.format("Template for VM: %s cannot be found", vm.getUuid())); } - if (!template.isPublicTemplate()) { - Account templateOwner = _accountMgr.getAccount(template.getAccountId()); - _accountMgr.checkAccess(newAccount, null, true, templateOwner); - } + _accountMgr.checkAccess(newAccount, AccessType.UseEntry, true, template); // VV 5: check the new account can create vm in the domain DomainVO domain = _domainDao.findById(cmd.getDomainId()); From 60f234c682dbd27acc2c763e73569acec57a383f Mon Sep 17 00:00:00 2001 From: dahn Date: Fri, 21 Jun 2024 11:50:49 +0200 Subject: [PATCH 06/24] remove Project Template Permissions inhibition (#9196) --- .../main/java/com/cloud/template/TemplateManagerImpl.java | 7 ++++--- ui/src/views/compute/DeployVM.vue | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 2ed42087020..14f54e22f56 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -189,6 +189,7 @@ import com.cloud.user.AccountManager; import com.cloud.user.AccountService; import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; +import com.cloud.user.User; import com.cloud.user.UserData; import com.cloud.user.dao.AccountDao; import com.cloud.uservm.UserVm; @@ -1448,6 +1449,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, // Input validation final Long id = cmd.getId(); final Account caller = CallContext.current().getCallingAccount(); + final User user = CallContext.current().getCallingUser(); List accountNames = cmd.getAccountNames(); List projectIds = cmd.getProjectIds(); Boolean isFeatured = cmd.isFeatured(); @@ -1517,9 +1519,8 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, } if (owner.getType() == Account.Type.PROJECT) { - // Currently project owned templates cannot be shared outside project but is available to all users within project by default. - throw new InvalidParameterValueException("Update template permissions is an invalid operation on template " + template.getName() + - ". Project owned templates cannot be shared outside template."); + // if it is a project owned template/iso, the user must at least have access to be allowed to share it. + _accountMgr.checkAccess(user, template); } // check configuration parameter(allow.public.user.templates) value for diff --git a/ui/src/views/compute/DeployVM.vue b/ui/src/views/compute/DeployVM.vue index 95919461644..0bd68e5e204 100644 --- a/ui/src/views/compute/DeployVM.vue +++ b/ui/src/views/compute/DeployVM.vue @@ -2276,6 +2276,7 @@ export default { } args.zoneid = _.get(this.zone, 'id') args.templatefilter = templateFilter + args.projectid = -1 args.details = 'all' args.showicon = 'true' args.id = this.templateId @@ -2298,6 +2299,7 @@ export default { } args.zoneid = _.get(this.zone, 'id') args.isoFilter = isoFilter + args.projectid = -1 args.bootable = true args.showicon = 'true' args.id = this.isoId From 0c422aca76740b40fc2006e6fe33412c00f669a7 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 21 Jun 2024 12:14:30 +0200 Subject: [PATCH 07/24] server: fix additional zones cannot be removed (#9261) Got an exception when delete a zone ``` com.cloud.utils.exception.CloudRuntimeException: The zone cannot be deleted because there are Secondary storages in this zone ``` --- .../cloudstack/storage/datastore/db/ImageStoreDaoImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java index 21c5dc76d96..84b88c215ca 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java @@ -130,7 +130,7 @@ public class ImageStoreDaoImpl extends GenericDaoBase implem } if (scope.getScopeId() != null) { SearchCriteria scc = createSearchCriteria(); - scc.addOr("scope", SearchCriteria.Op.EQ, ScopeType.ZONE); + scc.addOr("scope", SearchCriteria.Op.EQ, ScopeType.REGION); scc.addOr("dcId", SearchCriteria.Op.EQ, scope.getScopeId()); sc.addAnd("scope", SearchCriteria.Op.SC, scc); } From 674495b162b509e0c20017c69eae5348266252b1 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Fri, 21 Jun 2024 15:53:45 +0530 Subject: [PATCH 08/24] Fixup startVM on simulator (#9199) --- .../agent/manager/MockVmManagerImpl.java | 79 +++++++------------ 1 file changed, 29 insertions(+), 50 deletions(-) diff --git a/plugins/hypervisors/simulator/src/main/java/com/cloud/agent/manager/MockVmManagerImpl.java b/plugins/hypervisors/simulator/src/main/java/com/cloud/agent/manager/MockVmManagerImpl.java index 67f3e95e872..2ae8e7e8688 100644 --- a/plugins/hypervisors/simulator/src/main/java/com/cloud/agent/manager/MockVmManagerImpl.java +++ b/plugins/hypervisors/simulator/src/main/java/com/cloud/agent/manager/MockVmManagerImpl.java @@ -137,57 +137,36 @@ public class MockVmManagerImpl extends ManagerBase implements MockVmManager { } if (vm == null) { - final int vncPort = 0; - if (vncPort < 0) { - return "Unable to allocate VNC port"; - } vm = new MockVMVO(); - vm.setCpu(cpuHz); - vm.setMemory(ramSize); - vm.setPowerState(PowerState.PowerOn); - vm.setName(vmName); - vm.setVncPort(vncPort); - vm.setHostId(host.getId()); - vm.setBootargs(bootArgs); - if (vmName.startsWith("s-")) { - vm.setType("SecondaryStorageVm"); - } else if (vmName.startsWith("v-")) { - vm.setType("ConsoleProxy"); - } else if (vmName.startsWith("r-")) { - vm.setType("DomainRouter"); - } else if (vmName.startsWith("i-")) { - vm.setType("User"); - } - txn = TransactionLegacy.open(TransactionLegacy.SIMULATOR_DB); - try { - txn.start(); - vm = _mockVmDao.persist((MockVMVO)vm); - txn.commit(); - } catch (final Exception ex) { - txn.rollback(); - throw new CloudRuntimeException("unable to save vm to db " + vm.getName(), ex); - } finally { - txn.close(); - txn = TransactionLegacy.open(TransactionLegacy.CLOUD_DB); - txn.close(); - } - } else { - if (vm.getPowerState() == PowerState.PowerOff) { - vm.setPowerState(PowerState.PowerOn); - txn = TransactionLegacy.open(TransactionLegacy.SIMULATOR_DB); - try { - txn.start(); - _mockVmDao.update(vm.getId(), (MockVMVO)vm); - txn.commit(); - } catch (final Exception ex) { - txn.rollback(); - throw new CloudRuntimeException("unable to update vm " + vm.getName(), ex); - } finally { - txn.close(); - txn = TransactionLegacy.open(TransactionLegacy.CLOUD_DB); - txn.close(); - } - } + } + vm.setCpu(cpuHz); + vm.setMemory(ramSize); + vm.setPowerState(PowerState.PowerOn); + vm.setName(vmName); + vm.setVncPort(0); + vm.setHostId(host.getId()); + vm.setBootargs(bootArgs); + if (vmName.startsWith("s-")) { + vm.setType("SecondaryStorageVm"); + } else if (vmName.startsWith("v-")) { + vm.setType("ConsoleProxy"); + } else if (vmName.startsWith("r-")) { + vm.setType("DomainRouter"); + } else if (vmName.startsWith("i-")) { + vm.setType("User"); + } + txn = TransactionLegacy.open(TransactionLegacy.SIMULATOR_DB); + try { + txn.start(); + vm = _mockVmDao.persist((MockVMVO)vm); + txn.commit(); + } catch (final Exception ex) { + txn.rollback(); + throw new CloudRuntimeException("unable to save vm to db " + vm.getName(), ex); + } finally { + txn.close(); + txn = TransactionLegacy.open(TransactionLegacy.CLOUD_DB); + txn.close(); } if (vm.getPowerState() == PowerState.PowerOn && vmName.startsWith("s-")) { From 313a165e62bea8ed4b31e79ab05b8c5c07e6c091 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 21 Jun 2024 12:26:20 +0200 Subject: [PATCH 09/24] server: add global setting consoleproxy.sslEnabled (#8809) --- .../com/cloud/consoleproxy/AgentBasedConsoleProxyManager.java | 2 +- .../src/main/java/com/cloud/consoleproxy/AgentHookBase.java | 2 +- .../main/java/com/cloud/consoleproxy/ConsoleProxyManager.java | 3 +++ .../java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java | 4 ++-- .../com/cloud/consoleproxy/StaticConsoleProxyManager.java | 2 +- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/consoleproxy/AgentBasedConsoleProxyManager.java b/server/src/main/java/com/cloud/consoleproxy/AgentBasedConsoleProxyManager.java index a71c692aab1..1fe693a62de 100644 --- a/server/src/main/java/com/cloud/consoleproxy/AgentBasedConsoleProxyManager.java +++ b/server/src/main/java/com/cloud/consoleproxy/AgentBasedConsoleProxyManager.java @@ -118,7 +118,7 @@ public class AgentBasedConsoleProxyManager extends ManagerBase implements Consol _consoleProxyPort = NumbersUtil.parseInt(value, ConsoleProxyManager.DEFAULT_PROXY_VNC_PORT); } - value = configs.get("consoleproxy.sslEnabled"); + value = configs.get(ConsoleProxySslEnabled.key()); if (value != null && value.equalsIgnoreCase("true")) { _sslEnabled = true; } diff --git a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java index efc5a1b5d84..ab72e3dc43e 100644 --- a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java +++ b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java @@ -213,7 +213,7 @@ public abstract class AgentHookBase implements AgentHook { byte[] ksBits = null; String consoleProxyUrlDomain = _configDao.getValue(Config.ConsoleProxyUrlDomain.key()); - String consoleProxySslEnabled = _configDao.getValue("consoleproxy.sslEnabled"); + String consoleProxySslEnabled = _configDao.getValue(ConsoleProxyManager.ConsoleProxySslEnabled.key()); if (!StringUtils.isEmpty(consoleProxyUrlDomain) && !StringUtils.isEmpty(consoleProxySslEnabled) && consoleProxySslEnabled.equalsIgnoreCase("true")) { ksBits = _ksMgr.getKeystoreBits(ConsoleProxyManager.CERTIFICATE_NAME, ConsoleProxyManager.CERTIFICATE_NAME, storePassword); diff --git a/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManager.java b/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManager.java index 6280495fb1a..d271f66e02f 100644 --- a/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManager.java +++ b/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManager.java @@ -36,6 +36,9 @@ public interface ConsoleProxyManager extends Manager, ConsoleProxyService { String ALERT_SUBJECT = "proxy-alert"; String CERTIFICATE_NAME = "CPVMCertificate"; + ConfigKey ConsoleProxySslEnabled = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Boolean.class, "consoleproxy.sslEnabled", "false", + "Enable SSL for console proxy", false); + ConfigKey NoVncConsoleDefault = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Boolean.class, "novnc.console.default", "true", "If true, noVNC console will be default console for virtual machines", true); diff --git a/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java b/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java index c1d4a22bf77..b7248dd4a96 100644 --- a/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java +++ b/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java @@ -1120,7 +1120,7 @@ public class ConsoleProxyManagerImpl extends ManagerBase implements ConsoleProxy Map configs = configurationDao.getConfiguration("management-server", params); - String value = configs.get("consoleproxy.sslEnabled"); + String value = configs.get(ConsoleProxySslEnabled.key()); if (value != null && value.equalsIgnoreCase("true")) { sslEnabled = true; } @@ -1609,7 +1609,7 @@ public class ConsoleProxyManagerImpl extends ManagerBase implements ConsoleProxy @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] { NoVncConsoleDefault, NoVncConsoleSourceIpCheckEnabled }; + return new ConfigKey[] { ConsoleProxySslEnabled, NoVncConsoleDefault, NoVncConsoleSourceIpCheckEnabled }; } protected ConsoleProxyStatus parseJsonToConsoleProxyStatus(String json) throws JsonParseException { diff --git a/server/src/main/java/com/cloud/consoleproxy/StaticConsoleProxyManager.java b/server/src/main/java/com/cloud/consoleproxy/StaticConsoleProxyManager.java index bb2b426bf82..29a7497fc17 100644 --- a/server/src/main/java/com/cloud/consoleproxy/StaticConsoleProxyManager.java +++ b/server/src/main/java/com/cloud/consoleproxy/StaticConsoleProxyManager.java @@ -72,7 +72,7 @@ public class StaticConsoleProxyManager extends AgentBasedConsoleProxyManager imp _ip = "127.0.0.1"; } - String value = (String)params.get("consoleproxy.sslEnabled"); + String value = (String)params.get(ConsoleProxySslEnabled.key()); if (value != null && value.equalsIgnoreCase("true")) { _sslEnabled = true; } From 9055610034bbaa0001545a89b8bb68be24674438 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 21 Jun 2024 17:42:07 +0530 Subject: [PATCH 10/24] Remove duplicate network state checks before shutdown network (#8462) --- .../orchestration/NetworkOrchestrator.java | 17 +- .../NetworkOrchestratorTest.java | 272 +++++++++++------- 2 files changed, 165 insertions(+), 124 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 09500051df6..91367186b64 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -3026,30 +3026,21 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra @Override @DB public boolean shutdownNetwork(final long networkId, final ReservationContext context, final boolean cleanupElements) { - NetworkVO network = _networksDao.findById(networkId); - if (network.getState() == Network.State.Allocated) { - s_logger.debug("Network is already shutdown: " + network); - return true; - } - - if (network.getState() != Network.State.Implemented && network.getState() != Network.State.Shutdown) { - s_logger.debug("Network is not implemented: " + network); - return false; - } - + NetworkVO network = null; try { //do global lock for the network network = _networksDao.acquireInLockTable(networkId, NetworkLockTimeout.value()); if (network == null) { - s_logger.warn("Unable to acquire lock for the network " + network + " as a part of network shutdown"); + s_logger.warn("Network with id: " + networkId + " doesn't exist, or unable to acquire lock for it as a part of network shutdown"); return false; } + if (s_logger.isDebugEnabled()) { s_logger.debug("Lock is acquired for network " + network + " as a part of network shutdown"); } if (network.getState() == Network.State.Allocated) { - s_logger.debug("Network is already shutdown: " + network); + s_logger.debug(String.format("Network [%s] is in Allocated state, no need to shutdown.", network)); return true; } diff --git a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java index 9e64eff1816..45ed646240f 100644 --- a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java +++ b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.engine.orchestration; +import static org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService.NetworkLockTimeout; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -70,6 +71,7 @@ import com.cloud.network.guru.NetworkGuru; import com.cloud.network.vpc.VpcManager; import com.cloud.network.vpc.VpcVO; import com.cloud.offerings.NetworkOfferingVO; +import com.cloud.utils.db.EntityManager; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.Ip; import com.cloud.vm.DomainRouterVO; @@ -95,7 +97,7 @@ import junit.framework.TestCase; public class NetworkOrchestratorTest extends TestCase { static final Logger s_logger = Logger.getLogger(NetworkOrchestratorTest.class); - NetworkOrchestrator testOrchastrator = Mockito.spy(new NetworkOrchestrator()); + NetworkOrchestrator testOrchestrator = Mockito.spy(new NetworkOrchestrator()); private String guruName = "GuestNetworkGuru"; private String dhcpProvider = "VirtualRouter"; @@ -114,21 +116,22 @@ public class NetworkOrchestratorTest extends TestCase { @Before public void setUp() { // make class-scope mocks - testOrchastrator._nicDao = mock(NicDao.class); - testOrchastrator._networksDao = mock(NetworkDao.class); - testOrchastrator._networkModel = mock(NetworkModel.class); - testOrchastrator._nicSecondaryIpDao = mock(NicSecondaryIpDao.class); - testOrchastrator._ntwkSrvcDao = mock(NetworkServiceMapDao.class); - testOrchastrator._nicIpAliasDao = mock(NicIpAliasDao.class); - testOrchastrator._ipAddressDao = mock(IPAddressDao.class); - testOrchastrator._vlanDao = mock(VlanDao.class); - testOrchastrator._networkModel = mock(NetworkModel.class); - testOrchastrator._nicExtraDhcpOptionDao = mock(NicExtraDhcpOptionDao.class); - testOrchastrator.routerDao = mock(DomainRouterDao.class); - testOrchastrator.routerNetworkDao = mock(RouterNetworkDao.class); - testOrchastrator._vpcMgr = mock(VpcManager.class); - testOrchastrator.routerJoinDao = mock(DomainRouterJoinDao.class); - testOrchastrator._ipAddrMgr = mock(IpAddressManager.class); + testOrchestrator._nicDao = mock(NicDao.class); + testOrchestrator._networksDao = mock(NetworkDao.class); + testOrchestrator._networkModel = mock(NetworkModel.class); + testOrchestrator._nicSecondaryIpDao = mock(NicSecondaryIpDao.class); + testOrchestrator._ntwkSrvcDao = mock(NetworkServiceMapDao.class); + testOrchestrator._nicIpAliasDao = mock(NicIpAliasDao.class); + testOrchestrator._ipAddressDao = mock(IPAddressDao.class); + testOrchestrator._vlanDao = mock(VlanDao.class); + testOrchestrator._networkModel = mock(NetworkModel.class); + testOrchestrator._nicExtraDhcpOptionDao = mock(NicExtraDhcpOptionDao.class); + testOrchestrator.routerDao = mock(DomainRouterDao.class); + testOrchestrator.routerNetworkDao = mock(RouterNetworkDao.class); + testOrchestrator._vpcMgr = mock(VpcManager.class); + testOrchestrator.routerJoinDao = mock(DomainRouterJoinDao.class); + testOrchestrator._ipAddrMgr = mock(IpAddressManager.class); + testOrchestrator._entityMgr = mock(EntityManager.class); DhcpServiceProvider provider = mock(DhcpServiceProvider.class); Map capabilities = new HashMap(); @@ -137,13 +140,13 @@ public class NetworkOrchestratorTest extends TestCase { when(provider.getCapabilities()).thenReturn(services); capabilities.put(Network.Capability.DhcpAccrossMultipleSubnets, "true"); - when(testOrchastrator._ntwkSrvcDao.getProviderForServiceInNetwork(Matchers.anyLong(), Matchers.eq(Service.Dhcp))).thenReturn(dhcpProvider); - when(testOrchastrator._networkModel.getElementImplementingProvider(dhcpProvider)).thenReturn(provider); + when(testOrchestrator._ntwkSrvcDao.getProviderForServiceInNetwork(Matchers.anyLong(), Matchers.eq(Service.Dhcp))).thenReturn(dhcpProvider); + when(testOrchestrator._networkModel.getElementImplementingProvider(dhcpProvider)).thenReturn(provider); when(guru.getName()).thenReturn(guruName); List networkGurus = new ArrayList(); networkGurus.add(guru); - testOrchastrator.networkGurus = networkGurus; + testOrchestrator.networkGurus = networkGurus; when(networkOffering.getGuestType()).thenReturn(GuestType.L2); when(networkOffering.getId()).thenReturn(networkOfferingId); @@ -158,21 +161,21 @@ public class NetworkOrchestratorTest extends TestCase { // make sure that release dhcp will be called when(vm.getType()).thenReturn(Type.User); - when(testOrchastrator._networkModel.areServicesSupportedInNetwork(network.getId(), Service.Dhcp)).thenReturn(true); + when(testOrchestrator._networkModel.areServicesSupportedInNetwork(network.getId(), Service.Dhcp)).thenReturn(true); when(network.getTrafficType()).thenReturn(TrafficType.Guest); when(network.getGuestType()).thenReturn(GuestType.Shared); - when(testOrchastrator._nicDao.listByNetworkIdTypeAndGatewayAndBroadcastUri(nic.getNetworkId(), VirtualMachine.Type.User, nic.getIPv4Gateway(), nic.getBroadcastUri())) + when(testOrchestrator._nicDao.listByNetworkIdTypeAndGatewayAndBroadcastUri(nic.getNetworkId(), VirtualMachine.Type.User, nic.getIPv4Gateway(), nic.getBroadcastUri())) .thenReturn(new ArrayList()); when(network.getGuruName()).thenReturn(guruName); - when(testOrchastrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); + when(testOrchestrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); - testOrchastrator.removeNic(vm, nic); + testOrchestrator.removeNic(vm, nic); verify(nic, times(1)).setState(Nic.State.Deallocating); - verify(testOrchastrator._networkModel, times(2)).getElementImplementingProvider(dhcpProvider); - verify(testOrchastrator._ntwkSrvcDao, times(2)).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); - verify(testOrchastrator._networksDao, times(2)).findById(nic.getNetworkId()); + verify(testOrchestrator._networkModel, times(2)).getElementImplementingProvider(dhcpProvider); + verify(testOrchestrator._ntwkSrvcDao, times(2)).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); + verify(testOrchestrator._networksDao, times(2)).findById(nic.getNetworkId()); } @Test public void testDontRemoveDhcpServiceFromDomainRouter() { @@ -185,14 +188,14 @@ public class NetworkOrchestratorTest extends TestCase { when(vm.getType()).thenReturn(Type.DomainRouter); when(network.getGuruName()).thenReturn(guruName); - when(testOrchastrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); + when(testOrchestrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); - testOrchastrator.removeNic(vm, nic); + testOrchestrator.removeNic(vm, nic); verify(nic, times(1)).setState(Nic.State.Deallocating); - verify(testOrchastrator._networkModel, never()).getElementImplementingProvider(dhcpProvider); - verify(testOrchastrator._ntwkSrvcDao, never()).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); - verify(testOrchastrator._networksDao, times(1)).findById(nic.getNetworkId()); + verify(testOrchestrator._networkModel, never()).getElementImplementingProvider(dhcpProvider); + verify(testOrchestrator._ntwkSrvcDao, never()).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); + verify(testOrchestrator._networksDao, times(1)).findById(nic.getNetworkId()); } @Test public void testDontRemoveDhcpServiceWhenNotProvided() { @@ -203,45 +206,45 @@ public class NetworkOrchestratorTest extends TestCase { // make sure that release dhcp will *not* be called when(vm.getType()).thenReturn(Type.User); - when(testOrchastrator._networkModel.areServicesSupportedInNetwork(network.getId(), Service.Dhcp)).thenReturn(false); + when(testOrchestrator._networkModel.areServicesSupportedInNetwork(network.getId(), Service.Dhcp)).thenReturn(false); when(network.getGuruName()).thenReturn(guruName); - when(testOrchastrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); + when(testOrchestrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); - testOrchastrator.removeNic(vm, nic); + testOrchestrator.removeNic(vm, nic); verify(nic, times(1)).setState(Nic.State.Deallocating); - verify(testOrchastrator._networkModel, never()).getElementImplementingProvider(dhcpProvider); - verify(testOrchastrator._ntwkSrvcDao, never()).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); - verify(testOrchastrator._networksDao, times(1)).findById(nic.getNetworkId()); + verify(testOrchestrator._networkModel, never()).getElementImplementingProvider(dhcpProvider); + verify(testOrchestrator._ntwkSrvcDao, never()).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); + verify(testOrchestrator._networksDao, times(1)).findById(nic.getNetworkId()); } @Test public void testCheckL2OfferingServicesEmptyServices() { - when(testOrchastrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(new ArrayList<>()); - when(testOrchastrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(false); - testOrchastrator.checkL2OfferingServices(networkOffering); + when(testOrchestrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(new ArrayList<>()); + when(testOrchestrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(false); + testOrchestrator.checkL2OfferingServices(networkOffering); } @Test public void testCheckL2OfferingServicesUserDataOnly() { - when(testOrchastrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.UserData)); - when(testOrchastrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(true); - testOrchastrator.checkL2OfferingServices(networkOffering); + when(testOrchestrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.UserData)); + when(testOrchestrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(true); + testOrchestrator.checkL2OfferingServices(networkOffering); } @Test(expected = InvalidParameterValueException.class) public void testCheckL2OfferingServicesMultipleServicesIncludingUserData() { - when(testOrchastrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.UserData, Service.Dhcp)); - when(testOrchastrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(true); - testOrchastrator.checkL2OfferingServices(networkOffering); + when(testOrchestrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.UserData, Service.Dhcp)); + when(testOrchestrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(true); + testOrchestrator.checkL2OfferingServices(networkOffering); } @Test(expected = InvalidParameterValueException.class) public void testCheckL2OfferingServicesMultipleServicesNotIncludingUserData() { - when(testOrchastrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.Dns, Service.Dhcp)); - when(testOrchastrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(false); - testOrchastrator.checkL2OfferingServices(networkOffering); + when(testOrchestrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.Dns, Service.Dhcp)); + when(testOrchestrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(false); + testOrchestrator.checkL2OfferingServices(networkOffering); } @Test @@ -253,7 +256,7 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, "192.168.100.1", "255.255.255.0", "00-88-14-4D-4C-FB", requestedNicProfile, null, "192.168.100.150"); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert("192.168.100.150", "192.168.100.1", "255.255.255.0", nicProfile, 1, 1); } @@ -267,7 +270,7 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, "192.168.100.1", "255.255.255.0", "00-88-14-4D-4C-FB", requestedNicProfile, "00-88-14-4D-4C-FB", "192.168.100.150"); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert("192.168.100.150", "192.168.100.1", "255.255.255.0", nicProfile, 1, 0); } @@ -294,7 +297,7 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, "192.168.100.1", "255.255.255.0", "00-88-14-4D-4C-FB", requestedNicProfile, null, requestedIpv4Address); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert(null, null, null, nicProfile, 0, 0); } @@ -321,7 +324,7 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, ipv4Gateway, "255.255.255.0", "00-88-14-4D-4C-FB", requestedNicProfile, "00-88-14-4D-4C-FB", "192.168.100.150"); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert(null, null, null, nicProfile, 1, 0); } @@ -347,7 +350,7 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, "192.168.100.1", ipv4Netmask, "00-88-14-4D-4C-FB", requestedNicProfile, "00-88-14-4D-4C-FB", "192.168.100.150"); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert(null, null, null, nicProfile, 1, 0); } @@ -359,9 +362,9 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, "192.168.100.1", "255.255.255.0", "00-88-14-4D-4C-FB", requestedNicProfile, "00-88-14-4D-4C-FB", "192.168.100.150"); - when(testOrchastrator._vlanDao.findByNetworkIdAndIpv4(Mockito.anyLong(), Mockito.anyString())).thenReturn(null); + when(testOrchestrator._vlanDao.findByNetworkIdAndIpv4(Mockito.anyLong(), Mockito.anyString())).thenReturn(null); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert(null, null, null, nicProfile, 0, 0); } @@ -377,21 +380,21 @@ public class NetworkOrchestratorTest extends TestCase { when(ipVoSpy.getState()).thenReturn(state); if (ipVoIsNull) { - when(testOrchastrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); + when(testOrchestrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); } else { - when(testOrchastrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); + when(testOrchestrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); } VlanVO vlanSpy = Mockito.spy(new VlanVO(Vlan.VlanType.DirectAttached, "vlanTag", vlanGateway, vlanNetmask, 0l, "192.168.100.100 - 192.168.100.200", 0l, new Long(0l), "ip6Gateway", "ip6Cidr", "ip6Range")); Mockito.doReturn(0l).when(vlanSpy).getId(); - when(testOrchastrator._vlanDao.findByNetworkIdAndIpv4(Mockito.anyLong(), Mockito.anyString())).thenReturn(vlanSpy); - when(testOrchastrator._ipAddressDao.acquireInLockTable(Mockito.anyLong())).thenReturn(ipVoSpy); - when(testOrchastrator._ipAddressDao.update(Mockito.anyLong(), Mockito.any(IPAddressVO.class))).thenReturn(true); - when(testOrchastrator._ipAddressDao.releaseFromLockTable(Mockito.anyLong())).thenReturn(true); + when(testOrchestrator._vlanDao.findByNetworkIdAndIpv4(Mockito.anyLong(), Mockito.anyString())).thenReturn(vlanSpy); + when(testOrchestrator._ipAddressDao.acquireInLockTable(Mockito.anyLong())).thenReturn(ipVoSpy); + when(testOrchestrator._ipAddressDao.update(Mockito.anyLong(), Mockito.any(IPAddressVO.class))).thenReturn(true); + when(testOrchestrator._ipAddressDao.releaseFromLockTable(Mockito.anyLong())).thenReturn(true); try { - when(testOrchastrator._networkModel.getNextAvailableMacAddressInNetwork(Mockito.anyLong())).thenReturn(macAddress); + when(testOrchestrator._networkModel.getNextAvailableMacAddressInNetwork(Mockito.anyLong())).thenReturn(macAddress); } catch (InsufficientAddressCapacityException e) { e.printStackTrace(); } @@ -399,9 +402,9 @@ public class NetworkOrchestratorTest extends TestCase { private void verifyAndAssert(String requestedIpv4Address, String ipv4Gateway, String ipv4Netmask, NicProfile nicProfile, int acquireLockAndCheckIfIpv4IsFreeTimes, int nextMacAddressTimes) { - verify(testOrchastrator, times(acquireLockAndCheckIfIpv4IsFreeTimes)).acquireLockAndCheckIfIpv4IsFree(Mockito.any(Network.class), Mockito.anyString()); + verify(testOrchestrator, times(acquireLockAndCheckIfIpv4IsFreeTimes)).acquireLockAndCheckIfIpv4IsFree(Mockito.any(Network.class), Mockito.anyString()); try { - verify(testOrchastrator._networkModel, times(nextMacAddressTimes)).getNextAvailableMacAddressInNetwork(Mockito.anyLong()); + verify(testOrchestrator._networkModel, times(nextMacAddressTimes)).getNextAvailableMacAddressInNetwork(Mockito.anyLong()); } catch (InsufficientAddressCapacityException e) { e.printStackTrace(); } @@ -443,27 +446,27 @@ public class NetworkOrchestratorTest extends TestCase { ipVoSpy.setState(state); ipVoSpy.setState(state); if (isIPAddressVONull) { - when(testOrchastrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(null); + when(testOrchestrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(null); } else { - when(testOrchastrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); + when(testOrchestrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); } - when(testOrchastrator._ipAddressDao.acquireInLockTable(Mockito.anyLong())).thenReturn(ipVoSpy); - when(testOrchastrator._ipAddressDao.releaseFromLockTable(Mockito.anyLong())).thenReturn(true); - when(testOrchastrator._ipAddressDao.update(Mockito.anyLong(), Mockito.any(IPAddressVO.class))).thenReturn(true); + when(testOrchestrator._ipAddressDao.acquireInLockTable(Mockito.anyLong())).thenReturn(ipVoSpy); + when(testOrchestrator._ipAddressDao.releaseFromLockTable(Mockito.anyLong())).thenReturn(true); + when(testOrchestrator._ipAddressDao.update(Mockito.anyLong(), Mockito.any(IPAddressVO.class))).thenReturn(true); - testOrchastrator.acquireLockAndCheckIfIpv4IsFree(network, "192.168.100.150"); + testOrchestrator.acquireLockAndCheckIfIpv4IsFree(network, "192.168.100.150"); - verify(testOrchastrator._ipAddressDao, Mockito.times(findByIpTimes)).findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString()); - verify(testOrchastrator._ipAddressDao, Mockito.times(acquireLockTimes)).acquireInLockTable(Mockito.anyLong()); - verify(testOrchastrator._ipAddressDao, Mockito.times(releaseFromLockTimes)).releaseFromLockTable(Mockito.anyLong()); - verify(testOrchastrator._ipAddressDao, Mockito.times(updateTimes)).update(Mockito.anyLong(), Mockito.any(IPAddressVO.class)); - verify(testOrchastrator, Mockito.times(validateTimes)).validateLockedRequestedIp(Mockito.any(IPAddressVO.class), Mockito.any(IPAddressVO.class)); + verify(testOrchestrator._ipAddressDao, Mockito.times(findByIpTimes)).findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString()); + verify(testOrchestrator._ipAddressDao, Mockito.times(acquireLockTimes)).acquireInLockTable(Mockito.anyLong()); + verify(testOrchestrator._ipAddressDao, Mockito.times(releaseFromLockTimes)).releaseFromLockTable(Mockito.anyLong()); + verify(testOrchestrator._ipAddressDao, Mockito.times(updateTimes)).update(Mockito.anyLong(), Mockito.any(IPAddressVO.class)); + verify(testOrchestrator, Mockito.times(validateTimes)).validateLockedRequestedIp(Mockito.any(IPAddressVO.class), Mockito.any(IPAddressVO.class)); } @Test(expected = InvalidParameterValueException.class) public void validateLockedRequestedIpTestNullLockedIp() { IPAddressVO ipVoSpy = Mockito.spy(new IPAddressVO(new Ip("192.168.100.100"), 0l, 0l, 0l, true)); - testOrchastrator.validateLockedRequestedIp(ipVoSpy, null); + testOrchestrator.validateLockedRequestedIp(ipVoSpy, null); } @Test @@ -478,7 +481,7 @@ public class NetworkOrchestratorTest extends TestCase { IPAddressVO lockedIp = ipVoSpy; lockedIp.setState(states[i]); try { - testOrchastrator.validateLockedRequestedIp(ipVoSpy, lockedIp); + testOrchestrator.validateLockedRequestedIp(ipVoSpy, lockedIp); } catch (InvalidParameterValueException e) { expectedException = true; } @@ -491,7 +494,7 @@ public class NetworkOrchestratorTest extends TestCase { IPAddressVO ipVoSpy = Mockito.spy(new IPAddressVO(new Ip("192.168.100.100"), 0l, 0l, 0l, true)); IPAddressVO lockedIp = ipVoSpy; lockedIp.setState(State.Free); - testOrchastrator.validateLockedRequestedIp(ipVoSpy, lockedIp); + testOrchestrator.validateLockedRequestedIp(ipVoSpy, lockedIp); } @Test @@ -502,16 +505,16 @@ public class NetworkOrchestratorTest extends TestCase { when(vm.getType()).thenReturn(Type.User); when(network.getGuruName()).thenReturn(guruName); - when(testOrchastrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); + when(testOrchestrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); Long nicId = 1L; when(nic.getId()).thenReturn(nicId); when(vm.getParameter(VirtualMachineProfile.Param.PreserveNics)).thenReturn(true); - testOrchastrator.removeNic(vm, nic); + testOrchestrator.removeNic(vm, nic); verify(nic, never()).setState(Nic.State.Deallocating); - verify(testOrchastrator._nicDao, never()).remove(nicId); + verify(testOrchestrator._nicDao, never()).remove(nicId); } public void encodeVlanIdIntoBroadcastUriTestVxlan() { @@ -570,7 +573,7 @@ public class NetworkOrchestratorTest extends TestCase { @Test(expected = InvalidParameterValueException.class) public void encodeVlanIdIntoBroadcastUriTestNullNetwork() { - URI resultUri = testOrchastrator.encodeVlanIdIntoBroadcastUri("vxlan://123", null); + URI resultUri = testOrchestrator.encodeVlanIdIntoBroadcastUri("vxlan://123", null); } private void encodeVlanIdIntoBroadcastUriPrepareAndTest(String vlanId, String isolationMethod, String expectedIsolation, String expectedUri) { @@ -579,7 +582,7 @@ public class NetworkOrchestratorTest extends TestCase { isolationMethods.add(isolationMethod); physicalNetwork.setIsolationMethods(isolationMethods); - URI resultUri = testOrchastrator.encodeVlanIdIntoBroadcastUri(vlanId, physicalNetwork); + URI resultUri = testOrchestrator.encodeVlanIdIntoBroadcastUri(vlanId, physicalNetwork); Assert.assertEquals(expectedIsolation, resultUri.getScheme()); Assert.assertEquals(expectedUri, resultUri.toString()); @@ -597,17 +600,17 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(network.getDns2()).thenReturn(ip4Dns[1]); Mockito.when(network.getIp6Dns1()).thenReturn(ip6Dns[0]); Mockito.when(network.getIp6Dns2()).thenReturn(ip6Dns[1]); - Mockito.when(testOrchastrator._networkModel.getNetworkRate(networkId, vmId)).thenReturn(networkRate); + Mockito.when(testOrchestrator._networkModel.getNetworkRate(networkId, vmId)).thenReturn(networkRate); NicVO nicVO = Mockito.mock(NicVO.class); Mockito.when(nicVO.isDefaultNic()).thenReturn(isDefaultNic); - Mockito.when(testOrchastrator._nicDao.findById(nicId)).thenReturn(nicVO); - Mockito.when(testOrchastrator._nicDao.update(nicId, nicVO)).thenReturn(true); - Mockito.when(testOrchastrator._networkModel.isSecurityGroupSupportedInNetwork(network)).thenReturn(false); - Mockito.when(testOrchastrator._networkModel.getNetworkTag(hypervisorType, network)).thenReturn(null); - Mockito.when(testOrchastrator._ntwkSrvcDao.getDistinctProviders(networkId)).thenReturn(new ArrayList<>()); - testOrchastrator.networkElements = new ArrayList<>(); - Mockito.when(testOrchastrator._nicExtraDhcpOptionDao.listByNicId(nicId)).thenReturn(new ArrayList<>()); - Mockito.when(testOrchastrator._ntwkSrvcDao.areServicesSupportedInNetwork(networkId, Service.Dhcp)).thenReturn(false); + Mockito.when(testOrchestrator._nicDao.findById(nicId)).thenReturn(nicVO); + Mockito.when(testOrchestrator._nicDao.update(nicId, nicVO)).thenReturn(true); + Mockito.when(testOrchestrator._networkModel.isSecurityGroupSupportedInNetwork(network)).thenReturn(false); + Mockito.when(testOrchestrator._networkModel.getNetworkTag(hypervisorType, network)).thenReturn(null); + Mockito.when(testOrchestrator._ntwkSrvcDao.getDistinctProviders(networkId)).thenReturn(new ArrayList<>()); + testOrchestrator.networkElements = new ArrayList<>(); + Mockito.when(testOrchestrator._nicExtraDhcpOptionDao.listByNicId(nicId)).thenReturn(new ArrayList<>()); + Mockito.when(testOrchestrator._ntwkSrvcDao.areServicesSupportedInNetwork(networkId, Service.Dhcp)).thenReturn(false); VirtualMachineProfile virtualMachineProfile = Mockito.mock(VirtualMachineProfile.class); Mockito.when(virtualMachineProfile.getType()).thenReturn(vmType); Mockito.when(virtualMachineProfile.getId()).thenReturn(vmId); @@ -636,7 +639,7 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(vpcVO.getIp4Dns1()).thenReturn(null); Mockito.when(vpcVO.getIp6Dns1()).thenReturn(null); } - Mockito.when(testOrchastrator._vpcMgr.getActiveVpc(vpcId)).thenReturn(vpcVO); + Mockito.when(testOrchestrator._vpcMgr.getActiveVpc(vpcId)).thenReturn(vpcVO); } else { Mockito.when(routerVO.getVpcId()).thenReturn(null); Long routerNetworkId = 2L; @@ -650,13 +653,13 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(routerNetworkVO.getDns1()).thenReturn(null); Mockito.when(routerNetworkVO.getIp6Dns1()).thenReturn(null); } - Mockito.when(testOrchastrator.routerNetworkDao.getRouterNetworks(vmId)).thenReturn(List.of(routerNetworkId)); - Mockito.when(testOrchastrator._networksDao.findById(routerNetworkId)).thenReturn(routerNetworkVO); + Mockito.when(testOrchestrator.routerNetworkDao.getRouterNetworks(vmId)).thenReturn(List.of(routerNetworkId)); + Mockito.when(testOrchestrator._networksDao.findById(routerNetworkId)).thenReturn(routerNetworkVO); } - Mockito.when(testOrchastrator.routerDao.findById(vmId)).thenReturn(routerVO); + Mockito.when(testOrchestrator.routerDao.findById(vmId)).thenReturn(routerVO); NicProfile profile = null; try { - profile = testOrchastrator.prepareNic(virtualMachineProfile, deployDestination, reservationContext, nicId, network); + profile = testOrchestrator.prepareNic(virtualMachineProfile, deployDestination, reservationContext, nicId, network); } catch (InsufficientCapacityException | ResourceUnavailableException e) { Assert.fail(String.format("Failure with exception %s", e.getMessage())); } @@ -725,7 +728,7 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced); Mockito.when(network.getGateway()).thenReturn(networkGateway); Mockito.when(network.getCidr()).thenReturn(networkCidr); - Pair pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress); + Pair pair = testOrchestrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress); Assert.assertNotNull(pair); Assert.assertEquals(networkGateway, pair.first()); Assert.assertEquals(networkNetmask, pair.second()); @@ -745,9 +748,9 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(vlan.getVlanNetmask()).thenReturn(defaultNetworkNetmask); Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic); Mockito.when(ipAddressVO.getVlanId()).thenReturn(1L); - Mockito.when(testOrchastrator._vlanDao.findById(1L)).thenReturn(vlan); - Mockito.when(testOrchastrator._ipAddressDao.findByIp(ipAddress)).thenReturn(ipAddressVO); - Pair pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress); + Mockito.when(testOrchestrator._vlanDao.findById(1L)).thenReturn(vlan); + Mockito.when(testOrchestrator._ipAddressDao.findByIp(ipAddress)).thenReturn(ipAddressVO); + Pair pair = testOrchestrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress); Assert.assertNotNull(pair); Assert.assertEquals(defaultNetworkGateway, pair.first()); Assert.assertEquals(defaultNetworkNetmask, pair.second()); @@ -759,7 +762,7 @@ public class NetworkOrchestratorTest extends TestCase { DataCenter dataCenter = Mockito.mock(DataCenter.class); Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class); Mockito.when(network.getGuestType()).thenReturn(GuestType.L2); - Assert.assertNull(testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses)); + Assert.assertNull(testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses)); } @Test @@ -771,8 +774,8 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced); String ipAddress = "10.1.10.10"; Mockito.when(ipAddresses.getIp4Address()).thenReturn(ipAddress); - Mockito.when(testOrchastrator._ipAddrMgr.acquireGuestIpAddress(network, ipAddress)).thenReturn(ipAddress); - String guestIp = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + Mockito.when(testOrchestrator._ipAddrMgr.acquireGuestIpAddress(network, ipAddress)).thenReturn(ipAddress); + String guestIp = testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); Assert.assertEquals(ipAddress, guestIp); } @@ -793,8 +796,8 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(ipAddressVO.getState()).thenReturn(State.Free); Mockito.when(network.getId()).thenReturn(networkId); Mockito.when(dataCenter.getId()).thenReturn(dataCenterId); - Mockito.when(testOrchastrator._ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(networkId, dataCenterId, State.Free)).thenReturn(ipAddressVO); - String ipAddress = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + Mockito.when(testOrchestrator._ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(networkId, dataCenterId, State.Free)).thenReturn(ipAddressVO); + String ipAddress = testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); Assert.assertEquals(freeIp, ipAddress); } @@ -816,8 +819,8 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(network.getId()).thenReturn(networkId); Mockito.when(dataCenter.getId()).thenReturn(dataCenterId); Mockito.when(ipAddresses.getIp4Address()).thenReturn(requestedIp); - Mockito.when(testOrchastrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); - String ipAddress = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + Mockito.when(testOrchestrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); + String ipAddress = testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); Assert.assertEquals(requestedIp, ipAddress); } @@ -839,7 +842,54 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(network.getId()).thenReturn(networkId); Mockito.when(dataCenter.getId()).thenReturn(dataCenterId); Mockito.when(ipAddresses.getIp4Address()).thenReturn(requestedIp); - Mockito.when(testOrchastrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); - testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + Mockito.when(testOrchestrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); + testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + } + + @Test + public void testShutdownNetworkAcquireLockFailed() { + ReservationContext reservationContext = Mockito.mock(ReservationContext.class); + NetworkVO network = mock(NetworkVO.class); + long networkId = 1; + when(testOrchestrator._networksDao.acquireInLockTable(Mockito.anyLong(), Mockito.anyInt())).thenReturn(null); + + boolean shutdownNetworkStatus = testOrchestrator.shutdownNetwork(networkId, reservationContext, false); + Assert.assertFalse(shutdownNetworkStatus); + + verify(testOrchestrator._networksDao, times(1)).acquireInLockTable(networkId, NetworkLockTimeout.value()); + } + + @Test + public void testShutdownNetworkInAllocatedState() { + ReservationContext reservationContext = Mockito.mock(ReservationContext.class); + NetworkVO network = mock(NetworkVO.class); + long networkId = 1; + when(testOrchestrator._networksDao.acquireInLockTable(Mockito.anyLong(), Mockito.anyInt())).thenReturn(network); + when(network.getId()).thenReturn(networkId); + when(network.getState()).thenReturn(Network.State.Allocated); + + boolean shutdownNetworkStatus = testOrchestrator.shutdownNetwork(networkId, reservationContext, false); + Assert.assertTrue(shutdownNetworkStatus); + + verify(network, times(1)).getState(); + verify(testOrchestrator._networksDao, times(1)).acquireInLockTable(networkId, NetworkLockTimeout.value()); + verify(testOrchestrator._networksDao, times(1)).releaseFromLockTable(networkId); + } + + @Test + public void testShutdownNetworkInImplementingState() { + ReservationContext reservationContext = Mockito.mock(ReservationContext.class); + NetworkVO network = mock(NetworkVO.class); + long networkId = 1; + when(testOrchestrator._networksDao.acquireInLockTable(Mockito.anyLong(), Mockito.anyInt())).thenReturn(network); + when(network.getId()).thenReturn(networkId); + when(network.getState()).thenReturn(Network.State.Implementing); + + boolean shutdownNetworkStatus = testOrchestrator.shutdownNetwork(networkId, reservationContext, false); + Assert.assertFalse(shutdownNetworkStatus); + + verify(network, times(3)).getState(); + verify(testOrchestrator._networksDao, times(1)).acquireInLockTable(networkId, NetworkLockTimeout.value()); + verify(testOrchestrator._networksDao, times(1)).releaseFromLockTable(networkId); } } From 59e9ab9efee82705d569fdcbc673ae788b90cd17 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Sat, 22 Jun 2024 11:17:11 +0530 Subject: [PATCH 11/24] Fix volume response for service offering with disk offering (#9273) --- .../java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java index 1060fd840b5..29f66b981bd 100644 --- a/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/VolumeJoinDaoImpl.java @@ -296,9 +296,10 @@ public class VolumeJoinDaoImpl extends GenericDaoBaseWithTagInformation Date: Sun, 23 Jun 2024 16:06:45 +0200 Subject: [PATCH 12/24] set isSystem for SVM IPs (#9281) * set isSystem for SSVM IPs * Revert "set isSystem for SSVM IPs" This reverts commit 4ba71b3d6b4ea377f6778d166ff9da7d41e5d007. * set isSystem flag for SSVMs --- .../src/main/java/com/cloud/network/guru/PublicNetworkGuru.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/network/guru/PublicNetworkGuru.java b/server/src/main/java/com/cloud/network/guru/PublicNetworkGuru.java index e8374b39f53..4a426d6fb94 100644 --- a/server/src/main/java/com/cloud/network/guru/PublicNetworkGuru.java +++ b/server/src/main/java/com/cloud/network/guru/PublicNetworkGuru.java @@ -126,7 +126,7 @@ public class PublicNetworkGuru extends AdapterBase implements NetworkGuru { if (vm.getType().equals(VirtualMachine.Type.ConsoleProxy) || vm.getType().equals(VirtualMachine.Type.SecondaryStorageVm)) { forSystemVms = true; } - PublicIp ip = _ipAddrMgr.assignPublicIpAddress(dc.getId(), null, vm.getOwner(), VlanType.VirtualNetwork, null, null, false, forSystemVms); + PublicIp ip = _ipAddrMgr.assignPublicIpAddress(dc.getId(), null, vm.getOwner(), VlanType.VirtualNetwork, null, null, forSystemVms, forSystemVms); nic.setIPv4Address(ip.getAddress().toString()); nic.setIPv4Gateway(ip.getGateway()); nic.setIPv4Netmask(ip.getNetmask()); From f4612c51ec444ff597867bc105c75976d298c103 Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Sun, 23 Jun 2024 16:11:02 +0200 Subject: [PATCH 13/24] libvirtstorage: Make sure netfs storage was really mounted (#8887) --- .../kvm/storage/LibvirtStorageAdaptor.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) 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 9d62925e134..31281615bce 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 @@ -272,6 +272,16 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { } } + private void checkNetfsStoragePoolMounted(String uuid) { + String targetPath = _mountPoint + File.separator + uuid; + int mountpointResult = Script.runSimpleBashScriptForExitValue("mountpoint -q " + targetPath); + if (mountpointResult != 0) { + String errMsg = String.format("libvirt failed to mount storage pool %s at %s", uuid, targetPath); + s_logger.error(errMsg); + throw new CloudRuntimeException(errMsg); + } + } + private StoragePool createNetfsStoragePool(PoolType fsType, Connect conn, String uuid, String host, String path) throws LibvirtException { String targetPath = _mountPoint + File.separator + uuid; LibvirtStoragePoolDef spd = new LibvirtStoragePoolDef(fsType, uuid, uuid, host, path, targetPath); @@ -692,6 +702,10 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { sp.create(0); } + if (type == StoragePoolType.NetworkFilesystem) { + checkNetfsStoragePoolMounted(name); + } + return getStoragePool(name); } catch (LibvirtException e) { String error = e.toString(); @@ -756,10 +770,10 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { if (e.toString().contains("exit status 16")) { String targetPath = _mountPoint + File.separator + uuid; s_logger.error("deleteStoragePool removed pool from libvirt, but libvirt had trouble unmounting the pool. Trying umount location " + targetPath + - "again in a few seconds"); + " again in a few seconds"); String result = Script.runSimpleBashScript("sleep 5 && umount " + targetPath); if (result == null) { - s_logger.error("Succeeded in unmounting " + targetPath); + s_logger.info("Succeeded in unmounting " + targetPath); return true; } s_logger.error("Failed to unmount " + targetPath); From 6a518e29b74bffe31815400c5f42609d92ddace5 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Sun, 23 Jun 2024 22:08:13 +0530 Subject: [PATCH 14/24] Allow deletion of external managed cks nodes (#9183) * Allow deleteion of external managed cks nodes * Fix unit tests * Update plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterHelperImpl.java Co-authored-by: Abhishek Kumar --- .../cluster/KubernetesServiceHelperImpl.java | 3 +++ .../KubernetesServiceHelperImplTest.java | 20 +++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java index 1b149a97de2..9551142d24e 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImpl.java @@ -84,6 +84,9 @@ public class KubernetesServiceHelperImpl extends AdapterBase implements Kubernet KubernetesCluster kubernetesCluster = kubernetesClusterDao.findById(vmMapVO.getClusterId()); String msg = "Instance is a part of a Kubernetes cluster"; if (kubernetesCluster != null) { + if (KubernetesCluster.ClusterType.ExternalManaged.equals(kubernetesCluster.getClusterType())) { + return; + } msg += String.format(": %s", kubernetesCluster.getName()); } msg += ". Use Instance delete option from Kubernetes cluster details or scale API for " + diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java index 8749fe2b7dc..3e6688e8757 100644 --- a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesServiceHelperImplTest.java @@ -58,14 +58,30 @@ public class KubernetesServiceHelperImplTest { } @Test(expected = CloudRuntimeException.class) - public void testCheckVmCanBeDestroyedInCluster() { + public void testCheckVmCanBeDestroyedInCloudManagedCluster() { UserVm vm = Mockito.mock(UserVm.class); Mockito.when(vm.getId()).thenReturn(1L); Mockito.when(vm.getUserVmType()).thenReturn(UserVmManager.CKS_NODE); KubernetesClusterVmMapVO map = Mockito.mock(KubernetesClusterVmMapVO.class); Mockito.when(map.getClusterId()).thenReturn(1L); Mockito.when(kubernetesClusterVmMapDao.findByVmId(1L)).thenReturn(map); - Mockito.when(kubernetesClusterDao.findById(1L)).thenReturn(Mockito.mock(KubernetesClusterVO.class)); + KubernetesClusterVO kubernetesCluster = Mockito.mock(KubernetesClusterVO.class); + Mockito.when(kubernetesClusterDao.findById(1L)).thenReturn(kubernetesCluster); + Mockito.when(kubernetesCluster.getClusterType()).thenReturn(KubernetesCluster.ClusterType.CloudManaged); + kubernetesServiceHelper.checkVmCanBeDestroyed(vm); + } + + @Test + public void testCheckVmCanBeDestroyedInExternalManagedCluster() { + UserVm vm = Mockito.mock(UserVm.class); + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(vm.getUserVmType()).thenReturn(UserVmManager.CKS_NODE); + KubernetesClusterVmMapVO map = Mockito.mock(KubernetesClusterVmMapVO.class); + Mockito.when(map.getClusterId()).thenReturn(1L); + Mockito.when(kubernetesClusterVmMapDao.findByVmId(1L)).thenReturn(map); + KubernetesClusterVO kubernetesCluster = Mockito.mock(KubernetesClusterVO.class); + Mockito.when(kubernetesClusterDao.findById(1L)).thenReturn(kubernetesCluster); + Mockito.when(kubernetesCluster.getClusterType()).thenReturn(KubernetesCluster.ClusterType.ExternalManaged); kubernetesServiceHelper.checkVmCanBeDestroyed(vm); } } From c17aa0d9ada2cceaf6d3e7a17cd577cc203047a7 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Mon, 24 Jun 2024 11:34:37 +0530 Subject: [PATCH 15/24] Import Remote KVM VM logging improvements (#9284) --- .../admin/vm/ImportUnmanagedInstanceCmd.java | 8 +++---- .../cloud/agent/api/CheckVolumeAnswer.java | 1 - .../cloud/agent/api/CheckVolumeCommand.java | 1 - .../agent/api/CopyRemoteVolumeAnswer.java | 1 - .../agent/api/CopyRemoteVolumeCommand.java | 2 +- .../cloud/agent/api/GetRemoteVmsAnswer.java | 2 +- .../cloud/agent/api/GetRemoteVmsCommand.java | 2 +- .../api/GetUnmanagedInstancesAnswer.java | 2 +- .../api/GetUnmanagedInstancesCommand.java | 2 +- .../resource/LibvirtComputingResource.java | 12 +++++------ .../LibvirtGetRemoteVmsCommandWrapper.java | 21 ++++++++++--------- ...rtGetUnmanagedInstancesCommandWrapper.java | 2 +- .../vm/UnmanagedVMsManagerImpl.java | 6 +++--- 13 files changed, 30 insertions(+), 32 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java index d632c786a16..3d8b23318dd 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java @@ -203,8 +203,8 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd { for (Map entry : (Collection>)nicNetworkList.values()) { String nic = entry.get(VmDetailConstants.NIC); String networkUuid = entry.get(VmDetailConstants.NETWORK); - if (LOGGER.isTraceEnabled()) { - LOGGER.trace(String.format("nic, '%s', goes on net, '%s'", nic, networkUuid)); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug(String.format("nic, '%s', goes on net, '%s'", nic, networkUuid)); } if (StringUtils.isAnyEmpty(nic, networkUuid) || _entityMgr.findByUuid(Network.class, networkUuid) == null) { throw new InvalidParameterValueException(String.format("Network ID: %s for NIC ID: %s is invalid", networkUuid, nic)); @@ -221,8 +221,8 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd { for (Map entry : (Collection>)nicIpAddressList.values()) { String nic = entry.get(VmDetailConstants.NIC); String ipAddress = StringUtils.defaultIfEmpty(entry.get(VmDetailConstants.IP4_ADDRESS), null); - if (LOGGER.isTraceEnabled()) { - LOGGER.trace(String.format("nic, '%s', gets ip, '%s'", nic, ipAddress)); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug(String.format("nic, '%s', gets ip, '%s'", nic, ipAddress)); } if (StringUtils.isEmpty(nic)) { throw new InvalidParameterValueException(String.format("NIC ID: '%s' is invalid for IP address mapping", nic)); diff --git a/core/src/main/java/com/cloud/agent/api/CheckVolumeAnswer.java b/core/src/main/java/com/cloud/agent/api/CheckVolumeAnswer.java index dd136d8642f..5a32ab59a7a 100644 --- a/core/src/main/java/com/cloud/agent/api/CheckVolumeAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/CheckVolumeAnswer.java @@ -17,7 +17,6 @@ package com.cloud.agent.api; -@LogLevel(LogLevel.Log4jLevel.Trace) public class CheckVolumeAnswer extends Answer { private long size; diff --git a/core/src/main/java/com/cloud/agent/api/CheckVolumeCommand.java b/core/src/main/java/com/cloud/agent/api/CheckVolumeCommand.java index b4036bebf3a..bd44b35c895 100644 --- a/core/src/main/java/com/cloud/agent/api/CheckVolumeCommand.java +++ b/core/src/main/java/com/cloud/agent/api/CheckVolumeCommand.java @@ -21,7 +21,6 @@ package com.cloud.agent.api; import com.cloud.agent.api.to.StorageFilerTO; -@LogLevel(LogLevel.Log4jLevel.Trace) public class CheckVolumeCommand extends Command { String srcFile; diff --git a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeAnswer.java b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeAnswer.java index f6d7cab4596..e79005be71b 100644 --- a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeAnswer.java @@ -17,7 +17,6 @@ package com.cloud.agent.api; -@LogLevel(LogLevel.Log4jLevel.Trace) public class CopyRemoteVolumeAnswer extends Answer { private String remoteIp; diff --git a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java index 88e5669f3b1..798336b0e72 100644 --- a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java +++ b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java @@ -21,10 +21,10 @@ package com.cloud.agent.api; import com.cloud.agent.api.to.StorageFilerTO; -@LogLevel(LogLevel.Log4jLevel.Trace) public class CopyRemoteVolumeCommand extends Command { String remoteIp; String username; + @LogLevel(LogLevel.Log4jLevel.Off) String password; String srcFile; String tmpPath; diff --git a/core/src/main/java/com/cloud/agent/api/GetRemoteVmsAnswer.java b/core/src/main/java/com/cloud/agent/api/GetRemoteVmsAnswer.java index 8cd072f1da1..c4e590591d0 100644 --- a/core/src/main/java/com/cloud/agent/api/GetRemoteVmsAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/GetRemoteVmsAnswer.java @@ -22,10 +22,10 @@ import org.apache.cloudstack.vm.UnmanagedInstanceTO; import java.util.HashMap; import java.util.List; -@LogLevel(LogLevel.Log4jLevel.Trace) public class GetRemoteVmsAnswer extends Answer { private String remoteIp; + @LogLevel(LogLevel.Log4jLevel.Trace) private HashMap unmanagedInstances; List vmNames; diff --git a/core/src/main/java/com/cloud/agent/api/GetRemoteVmsCommand.java b/core/src/main/java/com/cloud/agent/api/GetRemoteVmsCommand.java index 5c71d12dbd0..5b6b9bdd360 100644 --- a/core/src/main/java/com/cloud/agent/api/GetRemoteVmsCommand.java +++ b/core/src/main/java/com/cloud/agent/api/GetRemoteVmsCommand.java @@ -19,11 +19,11 @@ package com.cloud.agent.api; -@LogLevel(LogLevel.Log4jLevel.Trace) public class GetRemoteVmsCommand extends Command { String remoteIp; String username; + @LogLevel(LogLevel.Log4jLevel.Off) String password; public GetRemoteVmsCommand(String remoteIp, String username, String password) { diff --git a/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesAnswer.java b/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesAnswer.java index 771d472be2a..950930ec614 100644 --- a/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesAnswer.java @@ -21,10 +21,10 @@ import java.util.HashMap; import org.apache.cloudstack.vm.UnmanagedInstanceTO; -@LogLevel(LogLevel.Log4jLevel.Trace) public class GetUnmanagedInstancesAnswer extends Answer { private String instanceName; + @LogLevel(LogLevel.Log4jLevel.Trace) private HashMap unmanagedInstances; GetUnmanagedInstancesAnswer() { diff --git a/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesCommand.java b/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesCommand.java index 2cd80aebea1..c0b8987e152 100644 --- a/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesCommand.java +++ b/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesCommand.java @@ -28,10 +28,10 @@ import org.apache.commons.collections.CollectionUtils; * All managed instances will be filtered while trying to find unmanaged instances. */ -@LogLevel(LogLevel.Log4jLevel.Trace) public class GetUnmanagedInstancesCommand extends Command { String instanceName; + @LogLevel(LogLevel.Log4jLevel.Trace) List managedInstancesNames; public GetUnmanagedInstancesCommand() { 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 76e4efdfd7f..86ece3c8c66 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 @@ -3771,14 +3771,14 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } public List getAllVmNames(final Connect conn) { - final ArrayList la = new ArrayList(); + final ArrayList domainNames = new ArrayList(); try { final String names[] = conn.listDefinedDomains(); for (int i = 0; i < names.length; i++) { - la.add(names[i]); + domainNames.add(names[i]); } } catch (final LibvirtException e) { - s_logger.warn("Failed to list Defined domains", e); + s_logger.warn("Failed to list defined domains", e); } int[] ids = null; @@ -3786,14 +3786,14 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv ids = conn.listDomains(); } catch (final LibvirtException e) { s_logger.warn("Failed to list domains", e); - return la; + return domainNames; } Domain dm = null; for (int i = 0; i < ids.length; i++) { try { dm = conn.domainLookupByID(ids[i]); - la.add(dm.getName()); + domainNames.add(dm.getName()); } catch (final LibvirtException e) { s_logger.warn("Unable to get vms", e); } finally { @@ -3807,7 +3807,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } } - return la; + return domainNames; } private HashMap getHostVmStateReport() { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetRemoteVmsCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetRemoteVmsCommandWrapper.java index 69802bb845f..942a68b8074 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetRemoteVmsCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetRemoteVmsCommandWrapper.java @@ -50,36 +50,37 @@ public final class LibvirtGetRemoteVmsCommandWrapper extends CommandWrapper unmanagedInstances = new HashMap<>(); try { Connect conn = LibvirtConnection.getConnection(hypervisorURI); final List allVmNames = libvirtComputingResource.getAllVmNames(conn); + s_logger.info(String.format("Found %d VMs on the remote host %s", allVmNames.size(), remoteIp)); for (String name : allVmNames) { final Domain domain = libvirtComputingResource.getDomain(conn, name); - final DomainInfo.DomainState ps = domain.getInfo().state; final VirtualMachine.PowerState state = libvirtComputingResource.convertToPowerState(ps); - s_logger.debug("VM " + domain.getName() + " - powerstate: " + ps + ", state: " + state.toString()); + s_logger.debug(String.format("Remote VM %s - powerstate: %s, state: %s", domain.getName(), ps.toString(), state.toString())); if (state == VirtualMachine.PowerState.PowerOff) { try { UnmanagedInstanceTO instance = getUnmanagedInstance(libvirtComputingResource, domain, conn); unmanagedInstances.put(instance.getName(), instance); } catch (Exception e) { - s_logger.error("Couldn't fetch VM " + domain.getName() + " details, due to: " + e.getMessage(), e); + s_logger.error("Couldn't fetch remote VM " + domain.getName() + " details, due to: " + e.getMessage(), e); } } domain.free(); } - s_logger.debug("Found " + unmanagedInstances.size() + " stopped VMs on host " + command.getRemoteIp()); + s_logger.debug("Found " + unmanagedInstances.size() + " stopped VMs on remote host " + remoteIp); return new GetRemoteVmsAnswer(command, "", unmanagedInstances); } catch (final LibvirtException e) { - s_logger.error("Failed to list stopped VMs on remote host " + command.getRemoteIp() + ", due to: " + e.getMessage(), e); + s_logger.error("Failed to list stopped VMs on remote host " + remoteIp + ", due to: " + e.getMessage(), e); if (e.getMessage().toLowerCase().contains("connection refused")) { - return new Answer(command, false, "Unable to connect to remote host " + command.getRemoteIp() + ", please check the libvirtd tcp connectivity and retry"); + return new Answer(command, false, "Unable to connect to remote host " + remoteIp + ", please check the libvirtd tcp connectivity and retry"); } - return new Answer(command, false, "Unable to list stopped VMs on remote host " + command.getRemoteIp() + ", due to: " + e.getMessage()); + return new Answer(command, false, "Unable to list stopped VMs on remote host " + remoteIp + ", due to: " + e.getMessage()); } } @@ -105,8 +106,8 @@ public final class LibvirtGetRemoteVmsCommandWrapper extends CommandWrapper unmanagedInstances = new HashMap<>(); try { diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 6e7c0245ebc..2ff3bf2ca21 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -800,6 +800,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { } int copyTimeoutInSecs = copyTimeout * 60; copyRemoteVolumeCommand.setWait(copyTimeoutInSecs); + LOGGER.error(String.format("Initiating copy remote volume %s from %s, timeout %d secs", path, remoteUrl, copyTimeoutInSecs)); Answer answer = agentManager.easySend(dest.getHost().getId(), copyRemoteVolumeCommand); if (!(answer instanceof CopyRemoteVolumeAnswer)) { throw new CloudRuntimeException("Error while copying volume of remote instance: " + answer.getDetails()); @@ -2434,7 +2435,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { return defaultNetwork; } - //generate unit test public ListResponse listVmsForImport(ListVmsForImportCmd cmd) { final Account caller = CallContext.current().getCallingAccount(); if (caller.getType() != Account.Type.ADMIN) { @@ -2474,8 +2474,8 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { private HashMap getRemoteVmsOnKVMHost(long zoneId, String remoteHostUrl, String username, String password) { //ToDo: add option to list one Vm by name List hosts = resourceManager.listAllUpAndEnabledHostsInOneZoneByHypervisor(Hypervisor.HypervisorType.KVM, zoneId); - if(hosts.size() < 1) { - throw new CloudRuntimeException("No hosts available for VM import"); + if (hosts.size() < 1) { + throw new CloudRuntimeException("No hosts available to list VMs on remote host " + remoteHostUrl); } HostVO host = hosts.get(0); GetRemoteVmsCommand getRemoteVmsCommand = new GetRemoteVmsCommand(remoteHostUrl, username, password); From f792684b9c40e6a1212176a83a9453ed4c3a5ca1 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 24 Jun 2024 03:16:21 -0400 Subject: [PATCH 16/24] Support migration of VM imported from a remote host (#9259) --- .../motion/StorageSystemDataMotionStrategy.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java index a93f624aa53..71e8c0afc8f 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java @@ -24,6 +24,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Random; import java.util.Set; import java.util.UUID; @@ -140,6 +141,9 @@ import java.util.HashSet; import java.util.stream.Collectors; import org.apache.commons.collections.CollectionUtils; +import static org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.KVM_VM_IMPORT_DEFAULT_TEMPLATE_NAME; +import static org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.VM_IMPORT_DEFAULT_TEMPLATE_NAME; + public class StorageSystemDataMotionStrategy implements DataMotionStrategy { private static final Logger LOGGER = Logger.getLogger(StorageSystemDataMotionStrategy.class); private static final Random RANDOM = new Random(System.nanoTime()); @@ -1932,7 +1936,10 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { continue; } - if (srcVolumeInfo.getTemplateId() != null) { + VMTemplateVO vmTemplate = _vmTemplateDao.findById(vmInstance.getTemplateId()); + if (srcVolumeInfo.getTemplateId() != null && + Objects.nonNull(vmTemplate) && + !Arrays.asList(KVM_VM_IMPORT_DEFAULT_TEMPLATE_NAME, VM_IMPORT_DEFAULT_TEMPLATE_NAME).contains(vmTemplate.getName())) { LOGGER.debug(String.format("Copying template [%s] of volume [%s] from source storage pool [%s] to target storage pool [%s].", srcVolumeInfo.getTemplateId(), srcVolumeInfo.getId(), sourceStoragePool.getId(), destStoragePool.getId())); copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, sourceStoragePool, destDataStore, destStoragePool, destHost); } else { @@ -2151,7 +2158,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { if (srcVolumeInfo.getHypervisorType() == HypervisorType.KVM && srcVolumeInfo.getTemplateId() != null && srcVolumeInfo.getPoolId() != null) { VMTemplateVO template = _vmTemplateDao.findById(srcVolumeInfo.getTemplateId()); - if (template.getFormat() != null && template.getFormat() != Storage.ImageFormat.ISO) { + if (Objects.nonNull(template) && template.getFormat() != null && template.getFormat() != Storage.ImageFormat.ISO) { VMTemplateStoragePoolVO ref = templatePoolDao.findByPoolTemplate(srcVolumeInfo.getPoolId(), srcVolumeInfo.getTemplateId(), null); return ref != null ? ref.getInstallPath() : null; } From f944d4c61d12c9c3b14c2de4445efc406d42b50e Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 24 Jun 2024 09:30:40 +0200 Subject: [PATCH 17/24] debian: add cpu-checker to debian/control (#9263) --- debian/control | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/control b/debian/control index 9fec540975e..c968f337bd5 100644 --- a/debian/control +++ b/debian/control @@ -24,7 +24,7 @@ Description: CloudStack server library Package: cloudstack-agent Architecture: all -Depends: ${python:Depends}, ${python3:Depends}, openjdk-11-jre-headless | java11-runtime-headless | java11-runtime | openjdk-11-jre-headless | zulu-11, cloudstack-common (= ${source:Version}), lsb-base (>= 9), openssh-client, qemu-kvm (>= 2.5) | qemu-system-x86 (>= 5.2), libvirt-bin (>= 1.3) | libvirt-daemon-system (>= 3.0), iproute2, ebtables, vlan, ipset, python3-libvirt, ethtool, iptables, cryptsetup, rng-tools, lsb-release, ufw, apparmor +Depends: ${python:Depends}, ${python3:Depends}, openjdk-11-jre-headless | java11-runtime-headless | java11-runtime | openjdk-11-jre-headless | zulu-11, cloudstack-common (= ${source:Version}), lsb-base (>= 9), openssh-client, qemu-kvm (>= 2.5) | qemu-system-x86 (>= 5.2), libvirt-bin (>= 1.3) | libvirt-daemon-system (>= 3.0), iproute2, ebtables, vlan, ipset, python3-libvirt, ethtool, iptables, cryptsetup, rng-tools, lsb-release, ufw, apparmor, cpu-checker Recommends: init-system-helpers Conflicts: cloud-agent, cloud-agent-libs, cloud-agent-deps, cloud-agent-scripts Description: CloudStack agent From 3e30283500f7bc26f75b056eb51264b5db7c2ede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Mon, 24 Jun 2024 04:32:48 -0300 Subject: [PATCH 18/24] Fix migration from local storage to NFS in KVM (#8909) * fix migration from local to nfs * remove unused imports * remove dead code --- .../StorageSystemDataMotionStrategy.java | 52 ++------------ ...NonManagedStorageSystemDataMotionTest.java | 8 +-- .../StorageSystemDataMotionStrategyTest.java | 71 ------------------- 3 files changed, 8 insertions(+), 123 deletions(-) diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java index 71e8c0afc8f..fa52933cebe 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java @@ -70,7 +70,6 @@ import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; -import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; @@ -82,7 +81,6 @@ import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo; import com.cloud.agent.api.ModifyTargetsAnswer; import com.cloud.agent.api.ModifyTargetsCommand; import com.cloud.agent.api.PrepareForMigrationCommand; -import com.cloud.agent.api.storage.CheckStorageAvailabilityCommand; import com.cloud.agent.api.storage.CopyVolumeAnswer; import com.cloud.agent.api.storage.CopyVolumeCommand; import com.cloud.agent.api.storage.MigrateVolumeAnswer; @@ -1909,7 +1907,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { throw new CloudRuntimeException("Invalid hypervisor type (only KVM supported for this operation at the time being)"); } - verifyLiveMigrationForKVM(volumeDataStoreMap, destHost); + verifyLiveMigrationForKVM(volumeDataStoreMap); VMInstanceVO vmInstance = _vmDao.findById(vmTO.getId()); vmTO.setState(vmInstance.getState()); @@ -1983,8 +1981,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { MigrateCommand.MigrateDiskInfo migrateDiskInfo; - boolean isNonManagedNfsToNfsOrSharedMountPointToNfs = supportStoragePoolType(sourceStoragePool.getPoolType()) && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination; - if (isNonManagedNfsToNfsOrSharedMountPointToNfs) { + boolean isNonManagedToNfs = supportStoragePoolType(sourceStoragePool.getPoolType(), StoragePoolType.Filesystem) && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination; + if (isNonManagedToNfs) { migrateDiskInfo = new MigrateCommand.MigrateDiskInfo(srcVolumeInfo.getPath(), MigrateCommand.MigrateDiskInfo.DiskType.FILE, MigrateCommand.MigrateDiskInfo.DriverType.QCOW2, @@ -2363,9 +2361,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { * At a high level: The source storage cannot be managed and * the destination storages can be all managed or all not managed, not mixed. */ - protected void verifyLiveMigrationForKVM(Map volumeDataStoreMap, Host destHost) { + protected void verifyLiveMigrationForKVM(Map volumeDataStoreMap) { Boolean storageTypeConsistency = null; - Map sourcePools = new HashMap<>(); for (Map.Entry entry : volumeDataStoreMap.entrySet()) { VolumeInfo volumeInfo = entry.getKey(); @@ -2392,47 +2389,6 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { } else if (storageTypeConsistency != destStoragePoolVO.isManaged()) { throw new CloudRuntimeException("Destination storage pools must be either all managed or all not managed"); } - - addSourcePoolToPoolsMap(sourcePools, srcStoragePoolVO, destStoragePoolVO); - } - verifyDestinationStorage(sourcePools, destHost); - } - - /** - * Adds source storage pool to the migration map if the destination pool is not managed and it is NFS. - */ - protected void addSourcePoolToPoolsMap(Map sourcePools, StoragePoolVO srcStoragePoolVO, StoragePoolVO destStoragePoolVO) { - if (destStoragePoolVO.isManaged() || !StoragePoolType.NetworkFilesystem.equals(destStoragePoolVO.getPoolType())) { - LOGGER.trace(String.format("Skipping adding source pool [%s] to map due to destination pool [%s] is managed or not NFS.", srcStoragePoolVO, destStoragePoolVO)); - return; - } - - String sourceStoragePoolUuid = srcStoragePoolVO.getUuid(); - if (!sourcePools.containsKey(sourceStoragePoolUuid)) { - sourcePools.put(sourceStoragePoolUuid, srcStoragePoolVO.getPoolType()); - } - } - - /** - * Perform storage validation on destination host for KVM live storage migrations. - * Validate that volume source storage pools are mounted on the destination host prior the migration - * @throws CloudRuntimeException if any source storage pool is not mounted on the destination host - */ - private void verifyDestinationStorage(Map sourcePools, Host destHost) { - if (MapUtils.isNotEmpty(sourcePools)) { - LOGGER.debug("Verifying source pools are already available on destination host " + destHost.getUuid()); - CheckStorageAvailabilityCommand cmd = new CheckStorageAvailabilityCommand(sourcePools); - try { - Answer answer = agentManager.send(destHost.getId(), cmd); - if (answer == null || !answer.getResult()) { - throw new CloudRuntimeException("Storage verification failed on host " - + destHost.getUuid() +": " + answer.getDetails()); - } - } catch (AgentUnavailableException | OperationTimedoutException e) { - e.printStackTrace(); - throw new CloudRuntimeException("Cannot perform storage verification on host " + destHost.getUuid() + - "due to: " + e.getMessage()); - } } } diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java index 8f1ada87458..c8a77a42252 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java @@ -476,19 +476,19 @@ public class KvmNonManagedStorageSystemDataMotionTest { @Test public void testVerifyLiveMigrationMapForKVM() { - kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2); + kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap); } @Test(expected = CloudRuntimeException.class) public void testVerifyLiveMigrationMapForKVMNotExistingSource() { when(primaryDataStoreDao.findById(POOL_1_ID)).thenReturn(null); - kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2); + kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap); } @Test(expected = CloudRuntimeException.class) public void testVerifyLiveMigrationMapForKVMNotExistingDest() { when(primaryDataStoreDao.findById(POOL_2_ID)).thenReturn(null); - kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2); + kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap); } @Test(expected = CloudRuntimeException.class) @@ -497,7 +497,7 @@ public class KvmNonManagedStorageSystemDataMotionTest { when(pool1.getId()).thenReturn(POOL_1_ID); when(pool2.getId()).thenReturn(POOL_2_ID); lenient().when(pool2.isManaged()).thenReturn(false); - kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2); + kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap); } @Test diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java index ea1a221c8c0..e619b40fae0 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java @@ -23,7 +23,6 @@ import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; import static org.mockito.MockitoAnnotations.initMocks; import java.util.HashMap; @@ -48,7 +47,6 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; -import org.mockito.verification.VerificationMode; import com.cloud.agent.api.MigrateCommand; import com.cloud.host.HostVO; @@ -62,7 +60,6 @@ import com.cloud.storage.VolumeVO; import java.util.AbstractMap; import java.util.Arrays; import java.util.HashSet; -import java.util.LinkedList; import java.util.List; import java.util.Set; @@ -372,72 +369,4 @@ public class StorageSystemDataMotionStrategyTest { assertFalse(strategy.isStoragePoolTypeInList(StoragePoolType.SharedMountPoint, listTypes)); } - - @Test - public void validateAddSourcePoolToPoolsMapDestinationPoolIsManaged() { - Mockito.doReturn(true).when(destinationStoragePoolVoMock).isManaged(); - strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - - Mockito.verify(destinationStoragePoolVoMock).isManaged(); - Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - } - - @Test - public void validateAddSourcePoolToPoolsMapDestinationPoolIsNotNFS() { - List storagePoolTypes = new LinkedList<>(Arrays.asList(StoragePoolType.values())); - storagePoolTypes.remove(StoragePoolType.NetworkFilesystem); - - Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged(); - storagePoolTypes.forEach(poolType -> { - Mockito.doReturn(poolType).when(destinationStoragePoolVoMock).getPoolType(); - strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - }); - - VerificationMode times = Mockito.times(storagePoolTypes.size()); - Mockito.verify(destinationStoragePoolVoMock, times).isManaged(); - Mockito.verify(destinationStoragePoolVoMock, times).getPoolType(); - Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - } - - @Test - public void validateAddSourcePoolToPoolsMapMapContainsKey() { - Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged(); - Mockito.doReturn(StoragePoolType.NetworkFilesystem).when(destinationStoragePoolVoMock).getPoolType(); - Mockito.doReturn("").when(sourceStoragePoolVoMock).getUuid(); - Mockito.doReturn(true).when(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString()); - strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - - Mockito.verify(destinationStoragePoolVoMock, never()).getScope(); - Mockito.verify(destinationStoragePoolVoMock).isManaged(); - Mockito.verify(destinationStoragePoolVoMock).getPoolType(); - Mockito.verify(sourceStoragePoolVoMock).getUuid(); - Mockito.verify(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString()); - Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - } - - @Test - public void validateAddSourcePoolToPoolsMapMapDoesNotContainsKey() { - List storagePoolTypes = new LinkedList<>(Arrays.asList(StoragePoolType.values())); - - Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged(); - Mockito.doReturn(StoragePoolType.NetworkFilesystem).when(destinationStoragePoolVoMock).getPoolType(); - Mockito.doReturn("").when(sourceStoragePoolVoMock).getUuid(); - Mockito.doReturn(false).when(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString()); - Mockito.doReturn(null).when(mapStringStoragePoolTypeMock).put(Mockito.anyString(), Mockito.any()); - - storagePoolTypes.forEach(poolType -> { - Mockito.doReturn(poolType).when(sourceStoragePoolVoMock).getPoolType(); - strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - }); - - VerificationMode times = Mockito.times(storagePoolTypes.size()); - Mockito.verify(destinationStoragePoolVoMock, never()).getScope(); - Mockito.verify(destinationStoragePoolVoMock, times).isManaged(); - Mockito.verify(destinationStoragePoolVoMock, times).getPoolType(); - Mockito.verify(sourceStoragePoolVoMock, times).getUuid(); - Mockito.verify(mapStringStoragePoolTypeMock, times).containsKey(Mockito.anyString()); - Mockito.verify(sourceStoragePoolVoMock, times).getPoolType(); - Mockito.verify(mapStringStoragePoolTypeMock, times).put(Mockito.anyString(), Mockito.any()); - Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - } } From 8b07b66f145e14bd69c2ab5d617844fd5b867c64 Mon Sep 17 00:00:00 2001 From: slavkap <51903378+slavkap@users.noreply.github.com> Date: Mon, 24 Jun 2024 10:39:21 +0300 Subject: [PATCH 19/24] Fix volume snapshot of encrypted NFS/StorPool volume (#8873) * Fix volume snapshot of encrypted NFS/StorPool volume * remove comments * removed invoking the real qemu convert command * fix UnsatisfiedLink error in unit tests * addressed comments extracted method --- .../kvm/storage/KVMStorageProcessor.java | 191 ++++++++++-------- .../kvm/storage/KVMStorageProcessorTest.java | 64 +++--- .../StorPoolBackupSnapshotCommandWrapper.java | 63 ++++-- 3 files changed, 190 insertions(+), 128 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 fc86ddb0c1c..616e41a6004 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 @@ -18,73 +18,6 @@ */ package com.cloud.hypervisor.kvm.storage; -import static com.cloud.utils.NumbersUtil.toHumanReadableSize; -import static com.cloud.utils.storage.S3.S3Utils.putFile; - -import java.io.File; -import java.io.FileNotFoundException; -import java.io.FileOutputStream; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Paths; -import java.text.DateFormat; -import java.text.SimpleDateFormat; -import java.util.Arrays; -import java.util.Date; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.UUID; -import java.util.stream.Collectors; - -import javax.naming.ConfigurationException; - -import org.apache.cloudstack.agent.directdownload.DirectDownloadAnswer; -import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand; -import org.apache.cloudstack.direct.download.DirectDownloadHelper; -import org.apache.cloudstack.direct.download.DirectTemplateDownloader; -import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; -import org.apache.cloudstack.storage.command.AttachAnswer; -import org.apache.cloudstack.storage.command.AttachCommand; -import org.apache.cloudstack.storage.command.CheckDataStoreStoragePolicyComplainceCommand; -import org.apache.cloudstack.storage.command.CopyCmdAnswer; -import org.apache.cloudstack.storage.command.CopyCommand; -import org.apache.cloudstack.storage.command.CreateObjectAnswer; -import org.apache.cloudstack.storage.command.CreateObjectCommand; -import org.apache.cloudstack.storage.command.DeleteCommand; -import org.apache.cloudstack.storage.command.DettachAnswer; -import org.apache.cloudstack.storage.command.DettachCommand; -import org.apache.cloudstack.storage.command.ForgetObjectCmd; -import org.apache.cloudstack.storage.command.IntroduceObjectCmd; -import org.apache.cloudstack.storage.command.ResignatureAnswer; -import org.apache.cloudstack.storage.command.ResignatureCommand; -import org.apache.cloudstack.storage.command.SnapshotAndCopyAnswer; -import org.apache.cloudstack.storage.command.SnapshotAndCopyCommand; -import org.apache.cloudstack.storage.command.SyncVolumePathCommand; -import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; -import org.apache.cloudstack.storage.to.SnapshotObjectTO; -import org.apache.cloudstack.storage.to.TemplateObjectTO; -import org.apache.cloudstack.storage.to.VolumeObjectTO; -import org.apache.cloudstack.utils.qemu.QemuImg; -import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; -import org.apache.cloudstack.utils.qemu.QemuImgException; -import org.apache.cloudstack.utils.qemu.QemuImgFile; -import org.apache.cloudstack.utils.qemu.QemuObject; -import org.apache.commons.collections.MapUtils; -import org.apache.commons.io.FileUtils; -import org.apache.commons.lang3.BooleanUtils; -import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.builder.ToStringBuilder; -import org.apache.commons.lang3.builder.ToStringStyle; -import org.apache.log4j.Logger; -import org.libvirt.Connect; -import org.libvirt.Domain; -import org.libvirt.DomainInfo; -import org.libvirt.DomainSnapshot; -import org.libvirt.LibvirtException; - import com.ceph.rados.IoCTX; import com.ceph.rados.Rados; import com.ceph.rados.exceptions.ErrorCode; @@ -133,6 +66,75 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; import com.cloud.utils.storage.S3.S3Utils; import com.cloud.vm.VmDetailConstants; +import org.apache.cloudstack.agent.directdownload.DirectDownloadAnswer; +import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand; +import org.apache.cloudstack.direct.download.DirectDownloadHelper; +import org.apache.cloudstack.direct.download.DirectTemplateDownloader; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.storage.command.AttachAnswer; +import org.apache.cloudstack.storage.command.AttachCommand; +import org.apache.cloudstack.storage.command.CheckDataStoreStoragePolicyComplainceCommand; +import org.apache.cloudstack.storage.command.CopyCmdAnswer; +import org.apache.cloudstack.storage.command.CopyCommand; +import org.apache.cloudstack.storage.command.CreateObjectAnswer; +import org.apache.cloudstack.storage.command.CreateObjectCommand; +import org.apache.cloudstack.storage.command.DeleteCommand; +import org.apache.cloudstack.storage.command.DettachAnswer; +import org.apache.cloudstack.storage.command.DettachCommand; +import org.apache.cloudstack.storage.command.ForgetObjectCmd; +import org.apache.cloudstack.storage.command.IntroduceObjectCmd; +import org.apache.cloudstack.storage.command.ResignatureAnswer; +import org.apache.cloudstack.storage.command.ResignatureCommand; +import org.apache.cloudstack.storage.command.SnapshotAndCopyAnswer; +import org.apache.cloudstack.storage.command.SnapshotAndCopyCommand; +import org.apache.cloudstack.storage.command.SyncVolumePathCommand; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.storage.to.SnapshotObjectTO; +import org.apache.cloudstack.storage.to.TemplateObjectTO; +import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.cloudstack.utils.cryptsetup.KeyFile; +import org.apache.cloudstack.utils.qemu.QemuImageOptions; +import org.apache.cloudstack.utils.qemu.QemuImg; +import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; +import org.apache.cloudstack.utils.qemu.QemuImgException; +import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.apache.cloudstack.utils.qemu.QemuObject; +import org.apache.cloudstack.utils.qemu.QemuObject.EncryptFormat; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.builder.ToStringBuilder; +import org.apache.commons.lang3.builder.ToStringStyle; +import org.apache.log4j.Logger; +import org.libvirt.Connect; +import org.libvirt.Domain; +import org.libvirt.DomainInfo; +import org.libvirt.DomainSnapshot; +import org.libvirt.LibvirtException; + +import javax.naming.ConfigurationException; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.text.DateFormat; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Date; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import java.util.stream.Collectors; + +import static com.cloud.utils.NumbersUtil.toHumanReadableSize; +import static com.cloud.utils.storage.S3.S3Utils.putFile; public class KVMStorageProcessor implements StorageProcessor { private static final Logger s_logger = Logger.getLogger(KVMStorageProcessor.class); @@ -1744,7 +1746,7 @@ public class KVMStorageProcessor implements StorageProcessor { snapshotPath = getSnapshotPathInPrimaryStorage(primaryPool.getLocalPath(), snapshotName); String diskLabel = takeVolumeSnapshot(resource.getDisks(conn, vmName), snapshotName, diskPath, vm); - String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, diskPath, snapshotPath, volume, cmd.getWait()); + String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, disk, snapshotPath, volume, cmd.getWait()); mergeSnapshotIntoBaseFile(vm, diskLabel, diskPath, snapshotName, volume, conn); @@ -1813,7 +1815,7 @@ public class KVMStorageProcessor implements StorageProcessor { } } else { snapshotPath = getSnapshotPathInPrimaryStorage(primaryPool.getLocalPath(), snapshotName); - String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, diskPath, snapshotPath, volume, cmd.getWait()); + String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, disk, snapshotPath, volume, cmd.getWait()); validateConvertResult(convertResult, snapshotPath); } } @@ -1936,26 +1938,43 @@ public class KVMStorageProcessor implements StorageProcessor { * @param snapshotPath Path to convert the base file; * @return null if the conversion occurs successfully or an error message that must be handled. */ - protected String convertBaseFileToSnapshotFileInPrimaryStorageDir(KVMStoragePool primaryPool, String baseFile, String snapshotPath, VolumeObjectTO volume, int wait) { - try { - s_logger.debug(String.format("Trying to convert volume [%s] (%s) to snapshot [%s].", volume, baseFile, snapshotPath)); + protected String convertBaseFileToSnapshotFileInPrimaryStorageDir(KVMStoragePool primaryPool, + KVMPhysicalDisk baseFile, String snapshotPath, VolumeObjectTO volume, int wait) { + try (KeyFile srcKey = new KeyFile(volume.getPassphrase())) { + s_logger.debug( + String.format("Trying to convert volume [%s] (%s) to snapshot [%s].", volume, baseFile, snapshotPath)); primaryPool.createFolder(TemplateConstants.DEFAULT_SNAPSHOT_ROOT_DIR); - - QemuImgFile srcFile = new QemuImgFile(baseFile); - srcFile.setFormat(PhysicalDiskFormat.QCOW2); - - QemuImgFile destFile = new QemuImgFile(snapshotPath); - destFile.setFormat(PhysicalDiskFormat.QCOW2); - - QemuImg q = new QemuImg(wait); - q.convert(srcFile, destFile); - - s_logger.debug(String.format("Converted volume [%s] (from path \"%s\") to snapshot [%s].", volume, baseFile, snapshotPath)); - return null; - } catch (QemuImgException | LibvirtException ex) { - return String.format("Failed to convert %s snapshot of volume [%s] to [%s] due to [%s].", volume, baseFile, snapshotPath, ex.getMessage()); + convertTheBaseFileToSnapshot(baseFile, snapshotPath, wait, srcKey); + } catch (QemuImgException | LibvirtException | IOException ex) { + return String.format("Failed to convert %s snapshot of volume [%s] to [%s] due to [%s].", volume, baseFile, + snapshotPath, ex.getMessage()); } + + s_logger.debug(String.format("Converted volume [%s] (from path \"%s\") to snapshot [%s].", volume, baseFile, + snapshotPath)); + return null; + } + + private void convertTheBaseFileToSnapshot(KVMPhysicalDisk baseFile, String snapshotPath, int wait, KeyFile srcKey) + throws LibvirtException, QemuImgException { + List qemuObjects = new ArrayList<>(); + Map options = new HashMap<>(); + QemuImageOptions qemuImageOpts = new QemuImageOptions(baseFile.getPath()); + if (srcKey.isSet()) { + String srcKeyName = "sec0"; + qemuObjects.add(QemuObject.prepareSecretForQemuImg(baseFile.getFormat(), EncryptFormat.LUKS, + srcKey.toString(), srcKeyName, options)); + qemuImageOpts = new QemuImageOptions(baseFile.getFormat(), baseFile.getPath(), srcKeyName); + } + QemuImgFile srcFile = new QemuImgFile(baseFile.getPath()); + srcFile.setFormat(PhysicalDiskFormat.QCOW2); + + QemuImgFile destFile = new QemuImgFile(snapshotPath); + destFile.setFormat(PhysicalDiskFormat.QCOW2); + + QemuImg q = new QemuImg(wait); + q.convert(srcFile, destFile, options, qemuObjects, qemuImageOpts, null, 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 4f3ed6bda5b..0159deda347 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 @@ -26,19 +26,10 @@ import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtUtilitiesHelper; import com.cloud.storage.template.TemplateConstants; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; -import javax.naming.ConfigurationException; - import com.cloud.utils.script.Script; -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Set; import org.apache.cloudstack.storage.to.SnapshotObjectTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.cloudstack.utils.qemu.QemuImageOptions; import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; @@ -59,6 +50,17 @@ import org.mockito.MockitoAnnotations; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; +import javax.naming.ConfigurationException; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + @RunWith(MockitoJUnitRunner.class) public class KVMStorageProcessorTest { @@ -259,40 +261,48 @@ public class KVMStorageProcessorTest { } @Test - public void convertBaseFileToSnapshotFileInPrimaryStorageDirTestFailToConvertWithQemuImgExceptionReturnErrorMessage() throws Exception { - String baseFile = "baseFile"; - String snapshotPath = "snapshotPath"; + public void convertBaseFileToSnapshotFileInPrimaryStorageDirTestFailToConvertWithQemuImgExceptionReturnErrorMessage() throws QemuImgException { + KVMPhysicalDisk baseFile = Mockito.mock(KVMPhysicalDisk.class); String errorMessage = "error"; - String expectedResult = String.format("Failed to convert %s snapshot of volume [%s] to [%s] due to [%s].", volumeObjectToMock, baseFile, snapshotPath, errorMessage); + KVMStoragePool primaryPoolMock = Mockito.mock(KVMStoragePool.class); + KVMPhysicalDisk baseFileMock = Mockito.mock(KVMPhysicalDisk.class); + VolumeObjectTO volumeMock = Mockito.mock(VolumeObjectTO.class); + QemuImgFile srcFileMock = Mockito.mock(QemuImgFile.class); + QemuImgFile destFileMock = Mockito.mock(QemuImgFile.class); + QemuImg qemuImgMock = Mockito.mock(QemuImg.class); - Mockito.doReturn(true).when(kvmStoragePoolMock).createFolder(Mockito.anyString()); - try (MockedConstruction ignored = Mockito.mockConstruction(QemuImg.class, (mock,context) -> { - Mockito.doThrow(new QemuImgException(errorMessage)).when(mock).convert(Mockito.any(QemuImgFile.class), Mockito.any(QemuImgFile.class)); - })) { - String result = storageProcessorSpy.convertBaseFileToSnapshotFileInPrimaryStorageDir(kvmStoragePoolMock, baseFile, snapshotPath, volumeObjectToMock, 1); - Assert.assertEquals(expectedResult, result); + Mockito.when(baseFileMock.getPath()).thenReturn("/path/to/baseFile"); + Mockito.when(primaryPoolMock.createFolder(Mockito.anyString())).thenReturn(true); + try (MockedConstruction