From 8bbe2f7cb2243a6c863422081239a7bdf2214a65 Mon Sep 17 00:00:00 2001 From: dahn Date: Thu, 27 Apr 2023 11:36:27 +0200 Subject: [PATCH 01/10] engine/schema: use junit 4 (same as the rest of the project tests) --- engine/schema/pom.xml | 6 ----- .../upgrade/DatabaseVersionHierarchyTest.java | 22 +++++++++---------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/engine/schema/pom.xml b/engine/schema/pom.xml index 45360bfaba1..da40a524c9b 100644 --- a/engine/schema/pom.xml +++ b/engine/schema/pom.xml @@ -57,12 +57,6 @@ ini4j ${cs.ini.version} - - org.junit.jupiter - junit-jupiter - ${cs.junit.jupiter.version} - test - diff --git a/engine/schema/src/test/java/com/cloud/upgrade/DatabaseVersionHierarchyTest.java b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseVersionHierarchyTest.java index 3fb6bf5d63a..e6692bdbde1 100644 --- a/engine/schema/src/test/java/com/cloud/upgrade/DatabaseVersionHierarchyTest.java +++ b/engine/schema/src/test/java/com/cloud/upgrade/DatabaseVersionHierarchyTest.java @@ -26,15 +26,15 @@ import com.cloud.upgrade.dao.Upgrade41520to41600; import com.cloud.upgrade.dao.Upgrade41720to41800; import com.cloud.upgrade.dao.Upgrade481to490; import org.apache.cloudstack.utils.CloudStackVersion; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; +import org.junit.BeforeClass; +import org.junit.Test; import java.io.InputStream; import java.sql.Connection; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.Assert.assertEquals; -class DatabaseVersionHierarchyTest { +public class DatabaseVersionHierarchyTest { private static DatabaseVersionHierarchy hierarchy; @@ -71,8 +71,8 @@ class DatabaseVersionHierarchyTest { } - @BeforeAll - static void init() { + @BeforeClass + public static void init() { DatabaseVersionHierarchy.DatabaseVersionHierarchyBuilder builder = DatabaseVersionHierarchy.builder() .next("0.0.5", new DummyUpgrade()) .next("1.0.0.0", new DummyUpgrade()) @@ -95,23 +95,23 @@ class DatabaseVersionHierarchyTest { } @Test - void getRecentVersionMiddle() { + public void getRecentVersionMiddle() { assertEquals("2.0.0", hierarchy.getRecentVersion(CloudStackVersion.parse("2.2.2")).toString()); } @Test - void getRecentVersionEarly() { + public void getRecentVersionEarly() { assertEquals(null, hierarchy.getRecentVersion(CloudStackVersion.parse("0.0.2"))); } @Test - void getRecentVersionStart() { + public void getRecentVersionStart() { assertEquals(null, hierarchy.getRecentVersion(CloudStackVersion.parse("0.0.5"))); } @Test - void getRecentVersionJust() { + public void getRecentVersionJust() { assertEquals("0.0.5", hierarchy.getRecentVersion(CloudStackVersion.parse("0.0.9")).toString()); } @Test - void getRecentVersionExact() { + public void getRecentVersionExact() { assertEquals("0.0.5", hierarchy.getRecentVersion(CloudStackVersion.parse("1.0.0.0")).toString()); } } \ No newline at end of file From b84744d9a5be77c13a7d6fbb9ad683dca0a0f5cf Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 28 Apr 2023 16:11:33 +0530 Subject: [PATCH 02/10] server: validate ip address value on update config (#7415) Fixes #6958 Signed-off-by: Abhishek Kumar --- .../ConfigurationManagerImpl.java | 28 ++++++++ .../ConfigurationManagerImplTest.java | 65 ++++++++++++++++++- 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index c5a489ba832..9255f9275e9 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -602,6 +602,32 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati }); } + protected void validateIpAddressRelatedConfigValues(final String configName, final String value) { + if (!configName.endsWith(".ip") && !configName.endsWith(".ipaddress") && !configName.endsWith(".iprange")) { + return; + } + if (StringUtils.isEmpty(value)) { + return; + } + final ConfigKey configKey = _configDepot.get(configName); + if (configKey == null || !String.class.equals(configKey.type())) { + return; + } + boolean err = (configName.endsWith(".ip") || configName.endsWith(".ipaddress")) && !NetUtils.isValidIp4(value); + if (configName.endsWith(".iprange")) { + err = true; + if (value.contains("-")) { + String[] ips = value.split("-"); + if (ips.length == 2 && NetUtils.isValidIp4(ips[0]) && NetUtils.isValidIp4(ips[1])) { + err = false; + } + } + } + if (err) { + throw new InvalidParameterValueException("Invalid IP address value(s) specified for the config value"); + } + } + @Override public boolean start() { @@ -874,6 +900,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati catergory = config.getCategory(); } + validateIpAddressRelatedConfigValues(name, value); + if (value == null) { return _configDao.findByName(name); } diff --git a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java index e8bb8fae0b9..47dfa4b79b0 100644 --- a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java +++ b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java @@ -16,22 +16,37 @@ // under the License. package com.cloud.configuration; -import com.cloud.utils.net.NetUtils; +import java.util.List; + +import org.apache.cloudstack.framework.config.ConfigDepot; +import org.apache.cloudstack.framework.config.ConfigKey; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; import org.mockito.Mockito; import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; -import java.util.List; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.storage.StorageManager; +import com.cloud.utils.net.NetUtils; @RunWith(PowerMockRunner.class) @PrepareForTest(NetUtils.class) public class ConfigurationManagerImplTest { + @Mock + ConfigDepot configDepot; ConfigurationManagerImpl configurationManagerImplSpy = Mockito.spy(new ConfigurationManagerImpl()); + + @Before + public void setUp() throws Exception { + configurationManagerImplSpy._configDepot = configDepot; + } + @Test public void validateIfIntValueIsInRangeTestValidValueReturnNull() { String testVariable = configurationManagerImplSpy.validateIfIntValueIsInRange("String name", "3", "1-5"); @@ -191,4 +206,50 @@ public class ConfigurationManagerImplTest { String testVariable = configurationManagerImplSpy.validateRangeOther("NameTest1", "ThisShouldNotWork", "ThisShouldWork,ThisShouldAlsoWork,SoShouldThis"); Assert.assertNotNull(testVariable); } + + @Test + public void testValidateIpAddressRelatedConfigValuesUnrelated() { + configurationManagerImplSpy.validateIpAddressRelatedConfigValues(StorageManager.PreferredStoragePool.key(), "something"); + configurationManagerImplSpy.validateIpAddressRelatedConfigValues("config.ip", ""); + Mockito.when(configurationManagerImplSpy._configDepot.get("config.ip")).thenReturn(null); + configurationManagerImplSpy.validateIpAddressRelatedConfigValues("config.ip", "something"); + ConfigKey key = StorageManager.MountDisabledStoragePool; + Mockito.doReturn(key).when(configurationManagerImplSpy._configDepot).get(StorageManager.MountDisabledStoragePool.key()); + configurationManagerImplSpy.validateIpAddressRelatedConfigValues(StorageManager.MountDisabledStoragePool.key(), "false"); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateIpAddressRelatedConfigValuesInvalidIp() { + ConfigKey key = StorageManager.PreferredStoragePool; // Any ConfigKey of String type + Mockito.doReturn(key).when(configurationManagerImplSpy._configDepot).get("config.ip"); + configurationManagerImplSpy.validateIpAddressRelatedConfigValues("config.ip", "abcdefg"); + } + + @Test + public void testValidateIpAddressRelatedConfigValuesValidIp() { + ConfigKey key = StorageManager.PreferredStoragePool; // Any ConfigKey of String type + Mockito.doReturn(key).when(configurationManagerImplSpy._configDepot).get("config.ip"); + configurationManagerImplSpy.validateIpAddressRelatedConfigValues("config.ip", "192.168.1.1"); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateIpAddressRelatedConfigValuesInvalidIpRange() { + ConfigKey key = StorageManager.PreferredStoragePool; // Any ConfigKey of String type. RemoteAccessVpnManagerImpl.RemoteAccessVpnClientIpRange not accessible here + Mockito.doReturn(key).when(configurationManagerImplSpy._configDepot).get("config.iprange"); + configurationManagerImplSpy.validateIpAddressRelatedConfigValues("config.iprange", "xyz-192.168.1.20"); + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateIpAddressRelatedConfigValuesInvalidIpRange1() { + ConfigKey key = StorageManager.PreferredStoragePool; // Any ConfigKey of String type. RemoteAccessVpnManagerImpl.RemoteAccessVpnClientIpRange not accessible here + Mockito.doReturn(key).when(configurationManagerImplSpy._configDepot).get("config.iprange"); + configurationManagerImplSpy.validateIpAddressRelatedConfigValues("config.iprange", "192.168.1.20"); + } + + @Test + public void testValidateIpAddressRelatedConfigValuesValidIpRange() { + ConfigKey key = StorageManager.PreferredStoragePool; // Any ConfigKey of String type. RemoteAccessVpnManagerImpl.RemoteAccessVpnClientIpRange not accessible here + Mockito.doReturn(key).when(configurationManagerImplSpy._configDepot).get("config.iprange"); + configurationManagerImplSpy.validateIpAddressRelatedConfigValues("config.iprange", "192.168.1.1-192.168.1.100"); + } } From d147f1cc3b2c7c85a02838231c6d6a4c566056ac Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 1 May 2023 18:53:17 +0530 Subject: [PATCH 03/10] ui: fix custom offering cpuspeed during vm import (#7423) * ui: fix custom offering cpuspeed during import vm Fixes #7420 Signed-off-by: Abhishek Kumar * refactor Signed-off-by: Abhishek Kumar --------- Signed-off-by: Abhishek Kumar --- ui/src/views/tools/ImportUnmanagedInstance.vue | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ui/src/views/tools/ImportUnmanagedInstance.vue b/ui/src/views/tools/ImportUnmanagedInstance.vue index 7aaa562e20b..1e5629ba51a 100644 --- a/ui/src/views/tools/ImportUnmanagedInstance.vue +++ b/ui/src/views/tools/ImportUnmanagedInstance.vue @@ -177,6 +177,7 @@ :maxCpu="getMaxCpu()" :minMemory="getMinMemory()" :maxMemory="getMaxMemory()" + :cpuSpeed="getCPUSpeed()" @update-iops-value="updateFieldValue" @update-compute-cpunumber="updateFieldValue" @update-compute-cpuspeed="updateFieldValue" @@ -522,6 +523,15 @@ export default { } return 'serviceofferingdetails' in this.computeOffering ? this.computeOffering.serviceofferingdetails.maxmemory * 1 : Number.MAX_SAFE_INTEGER }, + getCPUSpeed () { + if (!this.computeOffering) { + return 0 + } + if (this.computeOffering.cpuspeed) { + return this.computeOffering.cpuspeed * 1 + } + return this.resource.cpuspeed * 1 || 0 + }, fetchOptions (param, name, exclude) { if (exclude && exclude.length > 0) { if (exclude.includes(name)) { From ec0f8bddf627831f027bd799c9f2e4a20ccfc372 Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Thu, 4 May 2023 14:37:58 -0600 Subject: [PATCH 04/10] Support local storage live migration for direct download templates (#7453) Co-authored-by: Marcus Sorensen --- .../com/cloud/agent/api/MigrateCommand.java | 13 ++++++ .../storage/to/TemplateObjectTO.java | 1 + ...vmNonManagedStorageDataMotionStrategy.java | 21 +++++++-- .../StorageSystemDataMotionStrategy.java | 13 ++++-- ...NonManagedStorageSystemDataMotionTest.java | 15 ++++++- .../StorageSystemDataMotionStrategyTest.java | 15 ++++++- .../wrapper/LibvirtMigrateCommandWrapper.java | 10 +++++ .../LibvirtMigrateCommandWrapperTest.java | 45 +++++++++++++++++++ 8 files changed, 124 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/MigrateCommand.java b/core/src/main/java/com/cloud/agent/api/MigrateCommand.java index a6ab45973e2..27251f4bb78 100644 --- a/core/src/main/java/com/cloud/agent/api/MigrateCommand.java +++ b/core/src/main/java/com/cloud/agent/api/MigrateCommand.java @@ -171,6 +171,7 @@ public class MigrateCommand extends Command { private final DriverType driverType; private final Source source; private final String sourceText; + private final String backingStoreText; private boolean isSourceDiskOnStorageFileSystem; public MigrateDiskInfo(final String serialNumber, final DiskType diskType, final DriverType driverType, final Source source, final String sourceText) { @@ -179,6 +180,16 @@ public class MigrateCommand extends Command { this.driverType = driverType; this.source = source; this.sourceText = sourceText; + this.backingStoreText = null; + } + + public MigrateDiskInfo(final String serialNumber, final DiskType diskType, final DriverType driverType, final Source source, final String sourceText, final String backingStoreText) { + this.serialNumber = serialNumber; + this.diskType = diskType; + this.driverType = driverType; + this.source = source; + this.sourceText = sourceText; + this.backingStoreText = backingStoreText; } public String getSerialNumber() { @@ -201,6 +212,8 @@ public class MigrateCommand extends Command { return sourceText; } + public String getBackingStoreText() { return backingStoreText; } + public boolean isSourceDiskOnStorageFileSystem() { return isSourceDiskOnStorageFileSystem; } diff --git a/core/src/main/java/org/apache/cloudstack/storage/to/TemplateObjectTO.java b/core/src/main/java/org/apache/cloudstack/storage/to/TemplateObjectTO.java index b184a74312b..a405785bf64 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/to/TemplateObjectTO.java +++ b/core/src/main/java/org/apache/cloudstack/storage/to/TemplateObjectTO.java @@ -86,6 +86,7 @@ public class TemplateObjectTO implements DataTO { this.hypervisorType = template.getHypervisorType(); this.deployAsIs = template.isDeployAsIs(); this.deployAsIsConfiguration = template.getDeployAsIsConfiguration(); + this.directDownload = template.isDirectDownload(); } @Override diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java index 3e166ee2b4c..f2ccce75690 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java @@ -149,9 +149,9 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot * Configures a {@link MigrateDiskInfo} object configured for migrating a File System volume and calls rootImageProvisioning. */ @Override - protected MigrateCommand.MigrateDiskInfo configureMigrateDiskInfo(VolumeInfo srcVolumeInfo, String destPath) { - return new MigrateCommand.MigrateDiskInfo(srcVolumeInfo.getPath(), MigrateCommand.MigrateDiskInfo.DiskType.FILE, MigrateCommand.MigrateDiskInfo.DriverType.QCOW2, - MigrateCommand.MigrateDiskInfo.Source.FILE, destPath); + protected MigrateCommand.MigrateDiskInfo configureMigrateDiskInfo(VolumeInfo srcVolumeInfo, String destPath, String backingPath) { + return new MigrateCommand.MigrateDiskInfo(srcVolumeInfo.getPath(), MigrateCommand.MigrateDiskInfo.DiskType.FILE, MigrateCommand.MigrateDiskInfo.DriverType.QCOW2, + MigrateCommand.MigrateDiskInfo.Source.FILE, destPath, backingPath); } /** @@ -163,6 +163,15 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot return new File(destStoragePool.getPath(), destVolumeInfo.getUuid()).getAbsolutePath(); } + @Override + protected String generateBackingPath(StoragePoolVO destStoragePool, VolumeInfo destVolumeInfo) { + String templateInstallPath = getVolumeBackingFile(destVolumeInfo); + if (templateInstallPath == null) { + return null; + } + return new File(destStoragePool.getPath(), templateInstallPath).getAbsolutePath(); + } + /** * Returns the template UUID with the given id. If the template ID is null, it returns null. */ @@ -201,6 +210,12 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot return; } + TemplateInfo directDownloadTemplateInfo = templateDataFactory.getReadyBypassedTemplateOnPrimaryStore(srcVolumeInfo.getTemplateId(), destDataStore.getId(), destHost.getId()); + if (directDownloadTemplateInfo != null) { + LOGGER.debug(String.format("Template %s was of direct download type and successfully staged to primary store %s", directDownloadTemplateInfo.getId(), directDownloadTemplateInfo.getDataStore().getId())); + return; + } + VMTemplateStoragePoolVO sourceVolumeTemplateStoragePoolVO = vmTemplatePoolDao.findByPoolTemplate(destStoragePool.getId(), srcVolumeInfo.getTemplateId(), null); if (sourceVolumeTemplateStoragePoolVO == null && (isStoragePoolTypeInList(destStoragePool.getPoolType(), StoragePoolType.Filesystem, StoragePoolType.SharedMountPoint))) { DataStore sourceTemplateDataStore = dataStoreManagerImpl.getRandomImageStore(srcVolumeInfo.getDataCenterId()); 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 64792a61018..1419ae36d25 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 @@ -1871,7 +1871,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { MigrateCommand.MigrateDiskInfo.Source.FILE, connectHostToVolume(destHost, destVolumeInfo.getPoolId(), volumeIdentifier)); } else { - migrateDiskInfo = configureMigrateDiskInfo(srcVolumeInfo, destPath); + String backingPath = generateBackingPath(destStoragePool, destVolumeInfo); + migrateDiskInfo = configureMigrateDiskInfo(srcVolumeInfo, destPath, backingPath); migrateDiskInfo.setSourceDiskOnStorageFileSystem(isStoragePoolTypeOfFile(sourceStoragePool)); migrateDiskInfoList.add(migrateDiskInfo); prepareDiskWithSecretConsumerDetail(vmTO, srcVolumeInfo, destVolumeInfo.getPath()); @@ -1994,14 +1995,18 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { return connectHostToVolume(destHost, destVolumeInfo.getPoolId(), destVolumeInfo.get_iScsiName()); } + protected String generateBackingPath(StoragePoolVO destStoragePool, VolumeInfo destVolumeInfo) { + return null; + } + /** * Configures a {@link MigrateDiskInfo} object with disk type of BLOCK, Driver type RAW and Source DEV */ - protected MigrateCommand.MigrateDiskInfo configureMigrateDiskInfo(VolumeInfo srcVolumeInfo, String destPath) { + protected MigrateCommand.MigrateDiskInfo configureMigrateDiskInfo(VolumeInfo srcVolumeInfo, String destPath, String backingPath) { return new MigrateCommand.MigrateDiskInfo(srcVolumeInfo.getPath(), MigrateCommand.MigrateDiskInfo.DiskType.BLOCK, MigrateCommand.MigrateDiskInfo.DriverType.RAW, - MigrateCommand.MigrateDiskInfo.Source.DEV, destPath); + MigrateCommand.MigrateDiskInfo.Source.DEV, destPath, backingPath); } /** @@ -2023,7 +2028,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { /* * Return backing file for volume (if any), only for KVM volumes */ - private String getVolumeBackingFile(VolumeInfo srcVolumeInfo) { + String getVolumeBackingFile(VolumeInfo srcVolumeInfo) { if (srcVolumeInfo.getHypervisorType() == HypervisorType.KVM && srcVolumeInfo.getTemplateId() != null && srcVolumeInfo.getPoolId() != null) { VMTemplateVO template = _vmTemplateDao.findById(srcVolumeInfo.getTemplateId()); 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 07a6a1c0c1b..8f1ada87458 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 @@ -240,7 +240,7 @@ public class KvmNonManagedStorageSystemDataMotionTest { public void configureMigrateDiskInfoTest() { VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); Mockito.doReturn("volume path").when(srcVolumeInfo).getPath(); - MigrateCommand.MigrateDiskInfo migrateDiskInfo = kvmNonManagedStorageDataMotionStrategy.configureMigrateDiskInfo(srcVolumeInfo, "destPath"); + MigrateCommand.MigrateDiskInfo migrateDiskInfo = kvmNonManagedStorageDataMotionStrategy.configureMigrateDiskInfo(srcVolumeInfo, "destPath", null); Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DiskType.FILE, migrateDiskInfo.getDiskType()); Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DriverType.QCOW2, migrateDiskInfo.getDriverType()); Assert.assertEquals(MigrateCommand.MigrateDiskInfo.Source.FILE, migrateDiskInfo.getSource()); @@ -248,6 +248,19 @@ public class KvmNonManagedStorageSystemDataMotionTest { Assert.assertEquals("volume path", migrateDiskInfo.getSerialNumber()); } + @Test + public void configureMigrateDiskInfoWithBackingTest() { + VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); + Mockito.doReturn("volume path").when(srcVolumeInfo).getPath(); + MigrateCommand.MigrateDiskInfo migrateDiskInfo = kvmNonManagedStorageDataMotionStrategy.configureMigrateDiskInfo(srcVolumeInfo, "destPath", "backingPath"); + Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DiskType.FILE, migrateDiskInfo.getDiskType()); + Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DriverType.QCOW2, migrateDiskInfo.getDriverType()); + Assert.assertEquals(MigrateCommand.MigrateDiskInfo.Source.FILE, migrateDiskInfo.getSource()); + Assert.assertEquals("destPath", migrateDiskInfo.getSourceText()); + Assert.assertEquals("volume path", migrateDiskInfo.getSerialNumber()); + Assert.assertEquals("backingPath", migrateDiskInfo.getBackingStoreText()); + } + @Test public void shouldMigrateVolumeTest() { StoragePoolVO sourceStoragePool = Mockito.spy(new StoragePoolVO()); 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 88b7bf5fc62..f49c2b16f81 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 @@ -192,7 +192,7 @@ public class StorageSystemDataMotionStrategyTest { public void configureMigrateDiskInfoTest() { VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); Mockito.doReturn("volume path").when(srcVolumeInfo).getPath(); - MigrateCommand.MigrateDiskInfo migrateDiskInfo = strategy.configureMigrateDiskInfo(srcVolumeInfo, "destPath"); + MigrateCommand.MigrateDiskInfo migrateDiskInfo = strategy.configureMigrateDiskInfo(srcVolumeInfo, "destPath", null); Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DiskType.BLOCK, migrateDiskInfo.getDiskType()); Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DriverType.RAW, migrateDiskInfo.getDriverType()); Assert.assertEquals(MigrateCommand.MigrateDiskInfo.Source.DEV, migrateDiskInfo.getSource()); @@ -200,6 +200,19 @@ public class StorageSystemDataMotionStrategyTest { Assert.assertEquals("volume path", migrateDiskInfo.getSerialNumber()); } + @Test + public void configureMigrateDiskInfoWithBackingTest() { + VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); + Mockito.doReturn("volume path").when(srcVolumeInfo).getPath(); + MigrateCommand.MigrateDiskInfo migrateDiskInfo = strategy.configureMigrateDiskInfo(srcVolumeInfo, "destPath", "backingPath"); + Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DiskType.BLOCK, migrateDiskInfo.getDiskType()); + Assert.assertEquals(MigrateCommand.MigrateDiskInfo.DriverType.RAW, migrateDiskInfo.getDriverType()); + Assert.assertEquals(MigrateCommand.MigrateDiskInfo.Source.DEV, migrateDiskInfo.getSource()); + Assert.assertEquals("destPath", migrateDiskInfo.getSourceText()); + Assert.assertEquals("volume path", migrateDiskInfo.getSerialNumber()); + Assert.assertEquals("backingPath", migrateDiskInfo.getBackingStoreText()); + } + @Test public void setVolumePathTest() { VolumeVO volume = new VolumeVO("name", 0l, 0l, 0l, 0l, 0l, "folder", "path", Storage.ProvisioningType.THIN, 0l, Volume.Type.ROOT); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java index 221285a762d..d0ab77829af 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java @@ -575,6 +575,16 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper mapMigrateStorage = new HashMap(); + final String xmlDesc = + "" + + " " + + " \n" + + " \n" + + " \n" + + " \n" + + " bf8621b3027c497d963b\n" + + " \n" + + "
\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " " + + ""; + + final String volumeFile = "3530f749-82fd-458e-9485-a357e6e541db"; + final String backingFile = "0bc745b6-f3d7-44a9-ad8e-68904b77e2ab"; + String newDiskPath = "/mnt/2d0435e1-99e0-4f1d-94c0-bee1f6f8b99e/" + volumeFile; + String newBackingStorePath = "/mnt/2d0435e1-99e0-4f1d-94c0-bee1f6f8b99e/" + backingFile; + MigrateDiskInfo diskInfo = new MigrateDiskInfo("123456", DiskType.BLOCK, DriverType.RAW, Source.FILE, newDiskPath, newBackingStorePath); + mapMigrateStorage.put("/mnt/07eb495b-5590-3877-9fb7-23c6e9a40d40/bf8621b3-027c-497d-963b-06319650f048", diskInfo); + + final String result = libvirtMigrateCmdWrapper.replaceStorage(xmlDesc, mapMigrateStorage, false); + InputStream in = IOUtils.toInputStream(result); + DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); + Document doc = docBuilder.parse(in); + assertXpath(doc, "/domain/devices/disk/backingStore/source/@file", newBackingStorePath); + assertXpath(doc, "/domain/devices/disk/source/@file", newDiskPath); + assertXpath(doc, "/domain/devices/disk/serial", "bf8621b3027c497d963b"); + + final String expectedSecretUuid = LibvirtComputingResource.generateSecretUUIDFromString(volumeFile); + assertXpath(doc, "/domain/devices/disk/encryption/secret/@uuid", expectedSecretUuid); + } + public void testReplaceStorageXmlDiskNotManagedStorage() throws ParserConfigurationException, TransformerException, SAXException, IOException { final LibvirtMigrateCommandWrapper lw = new LibvirtMigrateCommandWrapper(); String destDisk1FileName = "XXXXXXXXXXXXXX"; From cb0874f5b9e8b8bc3fc5c71be7fbebd903ba2523 Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Mon, 8 May 2023 00:24:08 -0600 Subject: [PATCH 05/10] novnc: Send console text slower to avoid overloading remote keyboard buffer (#7477) This PR slows down the console paste text function in the console ever so slightly, adding 25ms between each character. It was found that when large text is pasted, say an SSH key or something over 100 characters, it would stop somewhere in the 100-200 character range, and if the last character was capitalized it would be stuck with left shift held down. In debugging, I ran noVNC locally with websockify against a Qemu VNC and took the whole console proxy out of the equation and was still able to reproduce it. I traced the websocket packets and we were clearly firing off all of the keypresses just fine, but the ones on the end were getting dropped VNC server side. It seems we are overloading a keyboard buffer, or something along those lines when we send all of the keypress messages at once. It was also observed that we were able to send off all of the messages via websocket in just a few ms but the typing on the guest side took a few seconds, so there is some buffering and processing going on and I think we are just hitting some limit in this. This sendText function doesn't seem to exist in upstream noVNC, so just patching it here seems reasonable. I suspect we added this for feature parity with whatever noVNC replaced. In testing it is now much more reliable in sending a few paragraphs of text, and not visibly slower than before, but obviously still too slow to do anything really big with it. It isn't really the goal to be able to paste full documents. Co-authored-by: Marcus Sorensen --- systemvm/agent/noVNC/core/rfb.js | 35 ++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/systemvm/agent/noVNC/core/rfb.js b/systemvm/agent/noVNC/core/rfb.js index f92255808e3..b795aaa7525 100644 --- a/systemvm/agent/noVNC/core/rfb.js +++ b/systemvm/agent/noVNC/core/rfb.js @@ -433,21 +433,30 @@ export default class RFB extends EventTargetMixin { this._resumeAuthentication(); } - sendText(text) { - for (var i = 0; i < text.length; i++) { - const character = text.charAt(i); - var charCode = USKeyTable[character] || false; - if (charCode) { - this.sendKey(charCode, character, true); - this.sendKey(charCode, character, false); - } else { - charCode = text.charCodeAt(i) - this.sendKey(KeyTable.XK_Shift_L, "ShiftLeft", true); - this.sendKey(charCode, character, true); - this.sendKey(charCode, character, false); - this.sendKey(KeyTable.XK_Shift_L, "ShiftLeft", false); + sendText(text) { + const sleep = (time) => { + return new Promise(resolve => setTimeout(resolve, time)) + } + + const keyboardTypeText = async () => { + for (var i = 0; i < text.length; i++) { + const character = text.charAt(i); + var charCode = USKeyTable[character] || false; + if (charCode) { + this.sendKey(charCode, character, true); + this.sendKey(charCode, character, false); + } else { + charCode = text.charCodeAt(i) + this.sendKey(KeyTable.XK_Shift_L, "ShiftLeft", true); + this.sendKey(charCode, character, true); + this.sendKey(charCode, character, false); + this.sendKey(KeyTable.XK_Shift_L, "ShiftLeft", false); + } + await sleep(25) } } + + keyboardTypeText() } sendCtrlAltDel() { From bdd5363314826df2bbce55a9d091da3625367133 Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Mon, 8 May 2023 00:29:42 -0600 Subject: [PATCH 06/10] Qemu migration hook: check for source length before using element 0 (#7482) Co-authored-by: Marcus Sorensen --- agent/bindir/libvirtqemuhook.in | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/agent/bindir/libvirtqemuhook.in b/agent/bindir/libvirtqemuhook.in index cf3d36410b8..e17944d8353 100755 --- a/agent/bindir/libvirtqemuhook.in +++ b/agent/bindir/libvirtqemuhook.in @@ -61,21 +61,23 @@ def handleMigrateBegin(): try: domain = parse(sys.stdin) for interface in domain.getElementsByTagName("interface"): - source = interface.getElementsByTagName("source")[0] - bridge = source.getAttribute("bridge") - if isOldStyleBridge(bridge): - vlanId = bridge.replace("cloudVirBr", "") - phyDev = getGuestNetworkDevice() - elif isNewStyleBridge(bridge): - vlanId = re.sub(r"br(\w+)-", "", bridge) - phyDev = re.sub(r"-(\d+)$", "" , re.sub(r"^br", "" ,bridge)) - netlib = networkConfig() - if not netlib.isNetworkDev(phyDev): + sources = interface.getElementsByTagName("source") + if sources.length > 0: + source = interface.getElementsByTagName("source")[0] + bridge = source.getAttribute("bridge") + if isOldStyleBridge(bridge): + vlanId = bridge.replace("cloudVirBr", "") phyDev = getGuestNetworkDevice() - else: - continue - newBrName = "br" + phyDev + "-" + vlanId - source.setAttribute("bridge", newBrName) + elif isNewStyleBridge(bridge): + vlanId = re.sub(r"br(\w+)-", "", bridge) + phyDev = re.sub(r"-(\d+)$", "" , re.sub(r"^br", "" ,bridge)) + netlib = networkConfig() + if not netlib.isNetworkDev(phyDev): + phyDev = getGuestNetworkDevice() + else: + continue + newBrName = "br" + phyDev + "-" + vlanId + source.setAttribute("bridge", newBrName) print(domain.toxml()) except: pass From 3cb4c801fb4dfffa5e0f18ccdb1aa453db74ef19 Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Mon, 8 May 2023 00:37:41 -0600 Subject: [PATCH 07/10] server: fix null pointer on powerflex attach volume edge case (#7498) This PR fixes a null pointer edge case where a PowerFlex volume is attached to a VM. In this edge case, a VM has been created, started, stopped, and then reimaged. VM has a last host ID but the newly attached volume does not yet have a storage pool assigned, it will be assigned on the VM start. However, we assume that if the VM's host is not null, we need to try to revoke access to the volume for the host. Since there is no storage pool yet for the volume, we hit a null pointer when checking to see if the volume's pool is PowerFlex. This was affecting all storage types, I could reproduce it with local storage, since the null pointer is at the check for pool's type. Co-authored-by: Marcus Sorensen --- .../src/main/java/com/cloud/storage/VolumeApiServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index d259ad871dc..b02dac051cb 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -4260,7 +4260,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic _volsDao.update(volumeToAttach.getId(), volumeToAttach); } - if (host != null && volumeToAttachStoragePool.getPoolType() == Storage.StoragePoolType.PowerFlex) { + if (host != null && volumeToAttachStoragePool != null && volumeToAttachStoragePool.getPoolType() == Storage.StoragePoolType.PowerFlex) { // Unmap the volume on PowerFlex/ScaleIO pool for stopped VM volService.revokeAccess(volFactory.getVolume(volumeToAttach.getId()), host, dataStore); } From 8604cb53281b7b239a81a54e0466871e3d68899a Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Mon, 8 May 2023 00:39:50 -0600 Subject: [PATCH 08/10] server: Fix DirectDownload certificate check initial delay (#7494) This PR adjusts the DirectDownload certificate check initial delay. Since the time unit is in hours, I think it was a mistake to schedule the initial check to be in 60 hours after management servers start - the intention was likely 60 seconds. We had turned this feature on to run hourly, not realizing we would have to wait 2.5 days to see it first run! Co-authored-by: Marcus Sorensen --- .../cloudstack/direct/download/DirectDownloadManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index 49b87d27949..17cb96931ec 100644 --- a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -742,7 +742,7 @@ public class DirectDownloadManagerImpl extends ManagerBase implements DirectDown executorService.scheduleWithFixedDelay( new DirectDownloadCertificateUploadBackgroundTask(this, hostDao, dataCenterDao, directDownloadCertificateDao, directDownloadCertificateHostMapDao), - 60L, DirectDownloadCertificateUploadInterval.value(), TimeUnit.HOURS); + 1L, DirectDownloadCertificateUploadInterval.value(), TimeUnit.HOURS); } return true; } From ad21e863423736cc54a12f012d73d19bc71e33f8 Mon Sep 17 00:00:00 2001 From: kiranchavala Date: Mon, 8 May 2023 12:19:38 +0530 Subject: [PATCH 09/10] addAnnotation: added the various entity type supported by the api call (#7478) This PR adds documentation to the addAnnotation api call https://cloudstack.apache.org/api/apidocs-4.18/apis/addAnnotation.html Co-authored-by: Kiran Chavala --- .../api/command/admin/annotation/AddAnnotationCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/annotation/AddAnnotationCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/annotation/AddAnnotationCmd.java index 74588337ee2..c2ded921c40 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/annotation/AddAnnotationCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/annotation/AddAnnotationCmd.java @@ -41,7 +41,7 @@ public class AddAnnotationCmd extends BaseCmd { @Parameter(name = ApiConstants.ANNOTATION, type = CommandType.STRING, description = "the annotation text") private String annotation; - @Parameter(name = ApiConstants.ENTITY_TYPE, type = CommandType.STRING, description = "the entity type (only HOST is allowed atm)") + @Parameter(name = ApiConstants.ENTITY_TYPE, type = CommandType.STRING, description = "The following entity types are allowed VM, VOLUME, SNAPSHOT, VM_SNAPSHOT, INSTANCE_GROUP, SSH_KEYPAIR, USER_DATA, NETWORK, VPC, PUBLIC_IP_ADDRESS, VPN_CUSTOMER_GATEWAY, TEMPLATE, ISO, KUBERNETES_CLUSTER, SERVICE_OFFERING, DISK_OFFERING, NETWORK_OFFERING, ZONE, POD, CLUSTER, HOST, DOMAIN, PRIMARY_STORAGE, SECONDARY_STORAGE, VR, SYSTEM_VM, AUTOSCALE_VM_GROUP, MANAGEMENT_SERVER") private String entityType; @Parameter(name = ApiConstants.ENTITY_ID, type = CommandType.STRING, description = "the id of the entity to annotate") From 897dad619b88bb853d41686bc5aeaf8af5e9bc49 Mon Sep 17 00:00:00 2001 From: dahn Date: Mon, 8 May 2023 09:21:12 +0200 Subject: [PATCH 10/10] marvin: replace encodestring for encodebytes (#7027) Fixes: #6962 --- test/integration/component/test_configdrive.py | 2 +- test/integration/component/test_deploy_vm_userdata_multi_nic.py | 2 +- test/integration/component/test_deploy_vm_userdata_reg.py | 2 +- tools/marvin/marvin/cloudstackConnection.py | 2 +- tools/ngui/requester.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/component/test_configdrive.py b/test/integration/component/test_configdrive.py index b8ce4bdde5b..38e753480db 100644 --- a/test/integration/component/test_configdrive.py +++ b/test/integration/component/test_configdrive.py @@ -1025,7 +1025,7 @@ class ConfigDriveUtils: :rtype: str """ self.debug("Updating userdata for VM - %s" % vm.name) - updated_user_data = base64.encodestring(new_user_data.encode()).decode() + updated_user_data = base64.encodebytes(new_user_data.encode()).decode() with self.stopped_vm(vm): vm.update(self.api_client, userdata=updated_user_data) diff --git a/test/integration/component/test_deploy_vm_userdata_multi_nic.py b/test/integration/component/test_deploy_vm_userdata_multi_nic.py index 2f1f256399d..766c96ac119 100644 --- a/test/integration/component/test_deploy_vm_userdata_multi_nic.py +++ b/test/integration/component/test_deploy_vm_userdata_multi_nic.py @@ -126,7 +126,7 @@ class TestDeployVmWithUserDataMultiNic(cloudstackTestCase): """Test userdata update when non default nic is without userdata for deploy and update """ - self.userdata = base64.encodestring(self.userdata.encode()).decode() + self.userdata = base64.encodebytes(self.userdata.encode()).decode() network1 = Network.create( self.apiclient, diff --git a/test/integration/component/test_deploy_vm_userdata_reg.py b/test/integration/component/test_deploy_vm_userdata_reg.py index a6e3c178286..9ac0ff00eb6 100644 --- a/test/integration/component/test_deploy_vm_userdata_reg.py +++ b/test/integration/component/test_deploy_vm_userdata_reg.py @@ -99,7 +99,7 @@ class TestDeployVmWithUserData(cloudstackTestCase): # py2 didn't insert any new-lines # so we now do the encoding in the stored userdata string and remove the '\n's # to get a good easy string compare in the assert later on. - cls.userdata = base64.encodestring(cls.userdata.encode()).decode().replace('\n', '') + cls.userdata = base64.encodebytes(cls.userdata.encode()).decode().replace('\n', '') cls.user_data_2k= ''.join(random.choice(string.ascii_uppercase + string.digits) for x in range(2000)) cls.user_data_2kl = ''.join(random.choice(string.ascii_uppercase + string.digits) for x in range(1900)) diff --git a/tools/marvin/marvin/cloudstackConnection.py b/tools/marvin/marvin/cloudstackConnection.py index c5cbb18dc9e..5b438daceb7 100644 --- a/tools/marvin/marvin/cloudstackConnection.py +++ b/tools/marvin/marvin/cloudstackConnection.py @@ -147,7 +147,7 @@ class CSConnection(object): ).replace("+", "%20")] ) for r in params] ) - signature = base64.encodestring( + signature = base64.encodebytes( hmac.new(self.securityKey.encode('utf-8'), hash_str.encode('utf-8'), hashlib.sha1).digest()).strip() diff --git a/tools/ngui/requester.py b/tools/ngui/requester.py index 3f3337d3b4e..03342dfa58d 100644 --- a/tools/ngui/requester.py +++ b/tools/ngui/requester.py @@ -68,7 +68,7 @@ def make_request(command, args, logger, host, port, str.lower(urllib.quote_plus(str(r[1]))).replace("+", "%20")]) for r in request]) - sig = urllib.quote_plus(base64.encodestring(hmac.new(secretkey, hashStr, + sig = urllib.quote_plus(base64.encodebytes(hmac.new(secretkey, hashStr, hashlib.sha1).digest()).strip()) request_url += "&signature=%s" % sig request_url = "%s://%s:%s%s?%s" % (protocol, host, port, path, request_url)