From 36e636ae7a601b14fb3e4b1cbd9c76cb8a066a07 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 29 Jul 2025 15:41:38 +0530 Subject: [PATCH 1/5] server: trim autoscale Windows VM hostname Fixes #9505 Signed-off-by: Abhishek Kumar --- .../cloud/network/as/AutoScaleManager.java | 5 ++-- .../network/as/AutoScaleManagerImpl.java | 29 ++++++++++++++----- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/com/cloud/network/as/AutoScaleManager.java b/server/src/main/java/com/cloud/network/as/AutoScaleManager.java index 04d4c8d2d621..fdc4e8f70a29 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManager.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManager.java @@ -16,9 +16,10 @@ // under the License. package com.cloud.network.as; -import com.cloud.user.Account; import org.apache.cloudstack.framework.config.ConfigKey; +import com.cloud.user.Account; + public interface AutoScaleManager extends AutoScaleService { ConfigKey AutoScaleStatsInterval = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Integer.class, @@ -57,7 +58,5 @@ public interface AutoScaleManager extends AutoScaleService { void removeVmFromVmGroup(Long vmId); - String getNextVmHostName(AutoScaleVmGroupVO asGroup); - void checkAutoScaleVmGroupName(String groupName); } diff --git a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java index 463014c0b846..857ac690bda5 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -38,7 +38,6 @@ import javax.inject.Inject; -import com.cloud.network.NetworkModel; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.affinity.AffinityGroupVO; import org.apache.cloudstack.affinity.dao.AffinityGroupDao; @@ -113,6 +112,7 @@ import com.cloud.network.Network; import com.cloud.network.Network.Capability; import com.cloud.network.Network.IpAddresses; +import com.cloud.network.NetworkModel; import com.cloud.network.as.AutoScaleCounter.AutoScaleCounterParam; import com.cloud.network.as.dao.AutoScalePolicyConditionMapDao; import com.cloud.network.as.dao.AutoScalePolicyDao; @@ -146,7 +146,9 @@ import com.cloud.server.ResourceTag; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; +import com.cloud.storage.GuestOSVO; import com.cloud.storage.dao.DiskOfferingDao; +import com.cloud.storage.dao.GuestOSDao; import com.cloud.template.TemplateManager; import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; @@ -280,6 +282,8 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage private NetworkOfferingDao networkOfferingDao; @Inject private VirtualMachineManager virtualMachineManager; + @Inject + GuestOSDao guestOSDao; private static final String PARAM_ROOT_DISK_SIZE = "rootdisksize"; private static final String PARAM_DISK_OFFERING_ID = "diskofferingid"; @@ -1805,13 +1809,15 @@ protected UserVm createNewVM(AutoScaleVmGroupVO asGroup) { List affinityGroupIdList = getVmAffinityGroupId(deployParams); updateVmDetails(deployParams, customParameters); - String vmHostName = getNextVmHostName(asGroup); + Pair vmHostAndDisplayName = getNextVmHostAndDisplayName(asGroup, template); + String vmHostName = vmHostAndDisplayName.first(); + String vmDisplayName = vmHostAndDisplayName.second(); asGroup.setNextVmSeq(asGroup.getNextVmSeq() + 1); autoScaleVmGroupDao.persist(asGroup); if (zone.getNetworkType() == NetworkType.Basic) { vm = userVmService.createBasicSecurityGroupVirtualMachine(zone, serviceOffering, template, null, owner, vmHostName, - vmHostName, diskOfferingId, dataDiskSize, null, + vmDisplayName, diskOfferingId, dataDiskSize, null, hypervisorType, HTTPMethod.GET, userData, userDataId, userDataDetails, sshKeyPairs, null, null, true, null, affinityGroupIdList, customParameters, null, null, null, null, true, overrideDiskOfferingId, null, null); @@ -1819,12 +1825,12 @@ protected UserVm createNewVM(AutoScaleVmGroupVO asGroup) { if (networkModel.checkSecurityGroupSupportForNetwork(owner, zone, networkIds, Collections.emptyList())) { vm = userVmService.createAdvancedSecurityGroupVirtualMachine(zone, serviceOffering, template, networkIds, null, - owner, vmHostName,vmHostName, diskOfferingId, dataDiskSize, null, + owner, vmHostName, vmDisplayName, diskOfferingId, dataDiskSize, null, hypervisorType, HTTPMethod.GET, userData, userDataId, userDataDetails, sshKeyPairs, null, null, true, null, affinityGroupIdList, customParameters, null, null, null, null, true, overrideDiskOfferingId, null, null, null); } else { - vm = userVmService.createAdvancedVirtualMachine(zone, serviceOffering, template, networkIds, owner, vmHostName, vmHostName, + vm = userVmService.createAdvancedVirtualMachine(zone, serviceOffering, template, networkIds, owner, vmHostName, vmDisplayName, diskOfferingId, dataDiskSize, null, hypervisorType, HTTPMethod.GET, userData, userDataId, userDataDetails, sshKeyPairs, null, addrs, true, null, affinityGroupIdList, customParameters, null, null, null, @@ -1949,13 +1955,20 @@ public void updateVmDetails(Map deployParams, Map getNextVmHostAndDisplayName(AutoScaleVmGroupVO asGroup, VirtualMachineTemplate template) { + template.getGuestOSId(); + GuestOSVO guestOSVO = guestOSDao.findById(template.getGuestOSId()); + boolean isWindows = guestOSVO != null && guestOSVO.getDisplayName().toLowerCase().contains("windows"); String vmHostNameSuffix = "-" + asGroup.getNextVmSeq() + "-" + RandomStringUtils.random(VM_HOSTNAME_RANDOM_SUFFIX_LENGTH, 0, 0, true, false, (char[])null, new SecureRandom()).toLowerCase(); // Truncate vm group name because max length of vm name is 63 int subStringLength = Math.min(asGroup.getName().length(), 63 - VM_HOSTNAME_PREFIX.length() - vmHostNameSuffix.length()); - return VM_HOSTNAME_PREFIX + asGroup.getName().substring(0, subStringLength) + vmHostNameSuffix; + String displayName = VM_HOSTNAME_PREFIX + asGroup.getName().substring(0, subStringLength) + vmHostNameSuffix; + String hostName = displayName; + if (isWindows) { + hostName = displayName.substring(Math.max(0, displayName.length() - 15)); + } + return new Pair<>(hostName, displayName); } @Override From 0c34a76c1ffc846d4b4a78edc70afcc544b275a0 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 30 Jul 2025 11:11:25 +0530 Subject: [PATCH 2/5] fix Signed-off-by: Abhishek Kumar --- .../network/as/AutoScaleManagerImpl.java | 2 +- .../network/as/AutoScaleManagerImplTest.java | 61 ++++++++++++++++++- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java index 857ac690bda5..879a3a7c8dcc 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -1958,7 +1958,7 @@ public void updateVmDetails(Map deployParams, Map getNextVmHostAndDisplayName(AutoScaleVmGroupVO asGroup, VirtualMachineTemplate template) { template.getGuestOSId(); GuestOSVO guestOSVO = guestOSDao.findById(template.getGuestOSId()); - boolean isWindows = guestOSVO != null && guestOSVO.getDisplayName().toLowerCase().contains("windows"); + boolean isWindows = guestOSVO != null && guestOSVO.getName().toLowerCase().contains("windows"); String vmHostNameSuffix = "-" + asGroup.getNextVmSeq() + "-" + RandomStringUtils.random(VM_HOSTNAME_RANDOM_SUFFIX_LENGTH, 0, 0, true, false, (char[])null, new SecureRandom()).toLowerCase(); // Truncate vm group name because max length of vm name is 63 diff --git a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java index 510cd7ac1a6e..df143bf1714a 100644 --- a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java @@ -23,6 +23,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.matches; import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; @@ -135,8 +136,11 @@ import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.DiskOfferingVO; +import com.cloud.storage.GuestOS; +import com.cloud.storage.GuestOSVO; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.dao.DiskOfferingDao; +import com.cloud.storage.dao.GuestOSDao; import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; import com.cloud.user.AccountManager; @@ -259,9 +263,10 @@ public class AutoScaleManagerImplTest { LoadBalancingRulesService loadBalancingRulesService; @Mock VMInstanceDao vmInstanceDao; - @Mock VirtualMachineManager virtualMachineManager; + @Mock + GuestOSDao guestOSDao; AccountVO account; UserVO user; @@ -420,6 +425,11 @@ public void setUp() { userDataDetails.put("0", new HashMap<>() {{ put("key1", "value1"); put("key2", "value2"); }}); Mockito.doReturn(userDataFinal).when(userVmMgr).finalizeUserData(any(), any(), any()); Mockito.doReturn(userDataFinal).when(userDataMgr).validateUserData(eq(userDataFinal), nullable(BaseCmd.HTTPMethod.class)); + + when(templateMock.getGuestOSId()).thenReturn(100L); + GuestOSVO guestOSMock = Mockito.mock(GuestOSVO.class); + when(guestOSDao.findById(anyLong())).thenReturn(guestOSMock); + when(guestOSMock.getName()).thenReturn("linux"); } @After @@ -2495,4 +2505,53 @@ public void destroyVm() { Mockito.verify(userVmMgr).expunge(eq(userVmMock)); } + + @Test + public void getNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForLinuxTemplate() { + when(asVmGroupMock.getName()).thenReturn(vmGroupName); + when(asVmGroupMock.getNextVmSeq()).thenReturn(1L); + Pair result = autoScaleManagerImplSpy.getNextVmHostAndDisplayName(asVmGroupMock, templateMock); + String vmHostNamePattern = AutoScaleManagerImpl.VM_HOSTNAME_PREFIX + vmGroupName + + "-" + asVmGroupMock.getNextVmSeq() + "-[a-z]{6}"; + Assert.assertTrue(result.first().matches(vmHostNamePattern)); + Assert.assertEquals(result.first(), result.second()); + } + + @Test + public void getNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplate() { + GuestOSVO guestOS = Mockito.mock(GuestOSVO.class); + when(asVmGroupMock.getName()).thenReturn(vmGroupName); + when(asVmGroupMock.getNextVmSeq()).thenReturn(1L); + when(templateMock.getGuestOSId()).thenReturn(1L); + when(guestOS.getName()).thenReturn("Windows Server"); + when(guestOSDao.findById(1L)).thenReturn(guestOS); + Pair result = autoScaleManagerImplSpy.getNextVmHostAndDisplayName(asVmGroupMock, templateMock); + String vmHostNamePattern = AutoScaleManagerImpl.VM_HOSTNAME_PREFIX + vmGroupName + + "-" + asVmGroupMock.getNextVmSeq() + "-[a-z]{6}"; + Assert.assertTrue(result.second().matches(vmHostNamePattern)); + Assert.assertEquals(15, result.first().length()); + Assert.assertTrue(result.second().endsWith(result.first())); + } + + @Test + public void getNextVmHostAndDisplayNameTruncatesGroupNameWhenExceedingMaxLength() { + when(asVmGroupMock.getName()).thenReturn(vmGroupNameWithMaxLength); + when(asVmGroupMock.getNextVmSeq()).thenReturn(1L); + Pair result = autoScaleManagerImplSpy.getNextVmHostAndDisplayName(asVmGroupMock, templateMock); + Assert.assertTrue(result.first().length() <= 63); + Assert.assertTrue(result.second().length() <= 63); + } + + @Test + public void getNextVmHostAndDisplayNameHandlesNullGuestOS() { + when(asVmGroupMock.getName()).thenReturn(vmGroupName); + when(asVmGroupMock.getNextVmSeq()).thenReturn(1L); + when(templateMock.getGuestOSId()).thenReturn(1L); + when(guestOSDao.findById(1L)).thenReturn(null); + Pair result = autoScaleManagerImplSpy.getNextVmHostAndDisplayName(asVmGroupMock, templateMock); + String vmHostNamePattern = AutoScaleManagerImpl.VM_HOSTNAME_PREFIX + vmGroupName + + "-" + asVmGroupMock.getNextVmSeq() + "-[a-z]{6}"; + Assert.assertTrue(result.first().matches(vmHostNamePattern)); + Assert.assertEquals(result.first(), result.second()); + } } From aa2170610529d66a55f40d6dfb6bc7bd87d33c57 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 30 Jul 2025 11:52:03 +0530 Subject: [PATCH 3/5] fix imports Signed-off-by: Abhishek Kumar --- .../java/com/cloud/network/as/AutoScaleManagerImplTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java index df143bf1714a..8e57884e40f9 100644 --- a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java @@ -23,7 +23,6 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.matches; import static org.mockito.ArgumentMatchers.nullable; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; @@ -136,7 +135,6 @@ import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.DiskOfferingVO; -import com.cloud.storage.GuestOS; import com.cloud.storage.GuestOSVO; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.dao.DiskOfferingDao; From 4c9ff4cc4c6c72f851716dbeb5158e6dd4fc8f42 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 25 Aug 2025 18:28:27 +0530 Subject: [PATCH 4/5] improve Signed-off-by: Abhishek Kumar --- .../cloud/network/as/AutoScaleManagerImpl.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java index c30ca45f1d31..eb0dd20da457 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -1968,12 +1968,19 @@ protected Pair getNextVmHostAndDisplayName(AutoScaleVmGroupVO as RandomStringUtils.random(VM_HOSTNAME_RANDOM_SUFFIX_LENGTH, 0, 0, true, false, (char[])null, new SecureRandom()).toLowerCase(); // Truncate vm group name because max length of vm name is 63 int subStringLength = Math.min(asGroup.getName().length(), 63 - VM_HOSTNAME_PREFIX.length() - vmHostNameSuffix.length()); - String displayName = VM_HOSTNAME_PREFIX + asGroup.getName().substring(0, subStringLength) + vmHostNameSuffix; - String hostName = displayName; - if (isWindows) { - hostName = displayName.substring(Math.max(0, displayName.length() - 15)); + String name = VM_HOSTNAME_PREFIX + asGroup.getName().substring(0, subStringLength) + vmHostNameSuffix; + if (!isWindows) { + return new Pair<>(name, name); } - return new Pair<>(hostName, displayName); + String hostName = name.substring(Math.max(0, name.length() - 15)); + if (Character.isLetterOrDigit(hostName.charAt(0))) { + return new Pair<>(hostName, name); + } + String temp = name.substring(0, Math.max(0, name.length() - 15)).replaceAll("[^a-zA-Z0-9]", ""); + if (!temp.isEmpty()) { + return new Pair<>(temp.charAt(temp.length() - 1) + hostName.substring(1), name); + } + return new Pair<>('a' + hostName.substring(1), name); } @Override From 16240b9afaa18fe8deeae5af9b5e3a3d31cc3e9c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 26 Aug 2025 10:18:24 +0530 Subject: [PATCH 5/5] return if okay Signed-off-by: Abhishek Kumar --- .../main/java/com/cloud/network/as/AutoScaleManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java index eb0dd20da457..5ce5b3af7774 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -1969,7 +1969,7 @@ protected Pair getNextVmHostAndDisplayName(AutoScaleVmGroupVO as // Truncate vm group name because max length of vm name is 63 int subStringLength = Math.min(asGroup.getName().length(), 63 - VM_HOSTNAME_PREFIX.length() - vmHostNameSuffix.length()); String name = VM_HOSTNAME_PREFIX + asGroup.getName().substring(0, subStringLength) + vmHostNameSuffix; - if (!isWindows) { + if (!isWindows || name.length() <= 15) { return new Pair<>(name, name); } String hostName = name.substring(Math.max(0, name.length() - 15));