From a4147ef622907901db128c6770d2525f998fbc41 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 29 Jul 2025 15:41:38 +0530 Subject: [PATCH 01/11] 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 88a9fd34bd13..eec1eec2ff12 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, @@ -63,7 +64,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 90380b77e440..9538a78939c7 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"; @@ -1821,13 +1825,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, null, + vmDisplayName, diskOfferingId, dataDiskSize, null, null, hypervisorType, HTTPMethod.GET, userData, userDataId, userDataDetails, sshKeyPairs, null, null, true, null, affinityGroupIdList, customParameters, null, null, null, null, true, overrideDiskOfferingId, null, null); @@ -1835,12 +1841,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, null, + owner, vmHostName, vmDisplayName, diskOfferingId, dataDiskSize, null, 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, null, hypervisorType, HTTPMethod.GET, userData, userDataId, userDataDetails, sshKeyPairs, null, addrs, true, null, affinityGroupIdList, customParameters, null, null, null, @@ -1965,13 +1971,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 e23d5ca507d060f19b5c792e1c2b2d4d2caad8f3 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 30 Jul 2025 11:11:25 +0530 Subject: [PATCH 02/11] 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 9538a78939c7..c0d1428ae2c4 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -1974,7 +1974,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 7ddd0d612138..bea92dff2e33 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 5eff9102336eb12f6052ea4cf05a9074401df792 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 30 Jul 2025 11:52:03 +0530 Subject: [PATCH 03/11] 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 bea92dff2e33..8afc154bff5e 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 bbb7f3e1af77680bed2df6d957f15c2bf5c80f8a Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 25 Aug 2025 18:28:27 +0530 Subject: [PATCH 04/11] 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 c0d1428ae2c4..2ab544dfadb7 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -1979,12 +1979,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 001f320daaeee92f092e1f220f9b2f0d2e986cf0 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 26 Aug 2025 10:18:24 +0530 Subject: [PATCH 05/11] 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 2ab544dfadb7..af4ea79070d0 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -1980,7 +1980,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)); From 9fe70627fd2ffaf3dacf11740295a96e38c30704 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 9 Sep 2025 19:45:42 +0530 Subject: [PATCH 06/11] address comment Signed-off-by: Abhishek Kumar --- .../network/as/AutoScaleManagerImpl.java | 27 ++++++---- .../network/as/AutoScaleManagerImplTest.java | 49 +++++++++++++++++++ 2 files changed, 67 insertions(+), 9 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 af4ea79070d0..624fee2d5425 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -1971,6 +1971,23 @@ public void updateVmDetails(Map deployParams, Map 15 ? valid.substring(valid.length() - 15) : valid; + if (!hostName.isEmpty() && !Character.isLetter(hostName.charAt(0))) { + for (int i = 0; i < hostName.length(); i++) { + if (Character.isLetter(hostName.charAt(i))) { + hostName = hostName.charAt(i) + hostName.substring(i + 1); + break; + } + } + if (!Character.isLetter(hostName.charAt(0))) { + hostName = hostName.length() < 15 ? "a" + hostName : "a" + hostName.substring(1); + } + } + return hostName; + } + protected Pair getNextVmHostAndDisplayName(AutoScaleVmGroupVO asGroup, VirtualMachineTemplate template) { template.getGuestOSId(); GuestOSVO guestOSVO = guestOSDao.findById(template.getGuestOSId()); @@ -1983,15 +2000,7 @@ protected Pair getNextVmHostAndDisplayName(AutoScaleVmGroupVO as if (!isWindows || name.length() <= 15) { return new Pair<>(name, name); } - 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); + return new Pair<>(getTrimmedHostNameForWindows(name), name); } @Override 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 8afc154bff5e..dcbad7190995 100644 --- a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java @@ -2552,4 +2552,53 @@ public void getNextVmHostAndDisplayNameHandlesNullGuestOS() { Assert.assertTrue(result.first().matches(vmHostNamePattern)); Assert.assertEquals(result.first(), result.second()); } + + @Test + public void trimsValidWindowsHostName() { + AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); + String result = manager.getTrimmedHostNameForWindows("ValidVMName1234"); + Assert.assertEquals("ValidVMName1234", result); + } + + @Test + public void trimsInvalidCharactersFromHostName() { + AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); + String result = manager.getTrimmedHostNameForWindows("Invalid@Host#Name!"); + Assert.assertEquals("InvalidHostName", result); + } + + @Test + public void ensuresHostNameStartsWithLetter() { + AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); + String result = manager.getTrimmedHostNameForWindows("123456789012345"); + Assert.assertTrue(Character.isLetter(result.charAt(0))); + } + + @Test + public void prefixesWithLetterIfNoLetterExists() { + AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); + String result = manager.getTrimmedHostNameForWindows("1234567890-12345"); + Assert.assertEquals("a34567890-12345", result); + } + + @Test + public void trimsHostNameToLast15Characters() { + AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); + String result = manager.getTrimmedHostNameForWindows("ThisIsAReallyLongHost-Name_1234"); + Assert.assertEquals("ngHost-Name1234", result); + } + + @Test + public void handlesEmptyHostName() { + AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); + String result = manager.getTrimmedHostNameForWindows(""); + Assert.assertEquals("", result); + } + + @Test + public void handlesHostNameWithOnlyMostlyInvalidCharacters() { + AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); + String result = manager.getTrimmedHostNameForWindows("@#$%^&*() 1"); + Assert.assertEquals("a1", result); + } } From 64a7a1c6cc9626c34a2434e0b56b13835fc9a8e5 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 30 Sep 2025 18:30:46 +0530 Subject: [PATCH 07/11] fix Signed-off-by: Abhishek Kumar --- .../network/as/AutoScaleManagerImpl.java | 3 ++- .../network/as/AutoScaleManagerImplTest.java | 21 ++++++++++++++----- 2 files changed, 18 insertions(+), 6 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 624fee2d5425..4a171f3c64ab 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -1991,7 +1991,8 @@ protected String getTrimmedHostNameForWindows(String name) { protected Pair getNextVmHostAndDisplayName(AutoScaleVmGroupVO asGroup, VirtualMachineTemplate template) { template.getGuestOSId(); GuestOSVO guestOSVO = guestOSDao.findById(template.getGuestOSId()); - boolean isWindows = guestOSVO != null && guestOSVO.getName().toLowerCase().contains("windows"); + String osName = guestOSVO != null ? StringUtils.firstNonBlank(guestOSVO.getName(), guestOSVO.getDisplayName()) : ""; + boolean isWindows = osName.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 dcbad7190995..fbcb547d6220 100644 --- a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java @@ -2515,20 +2515,31 @@ public void getNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForLinu Assert.assertEquals(result.first(), result.second()); } - @Test - public void getNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplate() { - GuestOSVO guestOS = Mockito.mock(GuestOSVO.class); + private void runGetNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplate() { 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 getNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplateUsingGuestOsName() { + GuestOSVO guestOS = Mockito.mock(GuestOSVO.class); + when(guestOS.getName()).thenReturn("Windows Server"); + when(guestOSDao.findById(1L)).thenReturn(guestOS); + } + + @Test + public void getNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplateUsingGuestOsDisplayName() { + GuestOSVO guestOS = Mockito.mock(GuestOSVO.class); + when(guestOS.getDisplayName()).thenReturn("Windows Server"); + when(guestOSDao.findById(1L)).thenReturn(guestOS); } @Test From af7fd5d48587013e13d60291049a4410645b0829 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 30 Sep 2025 18:55:32 +0530 Subject: [PATCH 08/11] missing test change Signed-off-by: Abhishek Kumar --- .../java/com/cloud/network/as/AutoScaleManagerImplTest.java | 2 ++ 1 file changed, 2 insertions(+) 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 fbcb547d6220..9d69dcb2da59 100644 --- a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java @@ -2533,6 +2533,7 @@ public void getNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWind GuestOSVO guestOS = Mockito.mock(GuestOSVO.class); when(guestOS.getName()).thenReturn("Windows Server"); when(guestOSDao.findById(1L)).thenReturn(guestOS); + runGetNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplate(); } @Test @@ -2540,6 +2541,7 @@ public void getNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWind GuestOSVO guestOS = Mockito.mock(GuestOSVO.class); when(guestOS.getDisplayName()).thenReturn("Windows Server"); when(guestOSDao.findById(1L)).thenReturn(guestOS); + runGetNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplate(); } @Test From 0e9fd6741eb099eb80387690abf841ecd1179230 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 1 Oct 2025 12:34:08 +0530 Subject: [PATCH 09/11] avoid chances of npe if both guestos name and displayname are null Signed-off-by: Abhishek Kumar --- .../cloud/network/as/AutoScaleManagerImpl.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 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 4a171f3c64ab..c9f74280682f 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -1988,11 +1988,20 @@ protected String getTrimmedHostNameForWindows(String name) { return hostName; } - protected Pair getNextVmHostAndDisplayName(AutoScaleVmGroupVO asGroup, VirtualMachineTemplate template) { - template.getGuestOSId(); + protected boolean isWindowsOs(VirtualMachineTemplate template) { GuestOSVO guestOSVO = guestOSDao.findById(template.getGuestOSId()); - String osName = guestOSVO != null ? StringUtils.firstNonBlank(guestOSVO.getName(), guestOSVO.getDisplayName()) : ""; - boolean isWindows = osName.toLowerCase().contains("windows"); + if (guestOSVO == null) { + return false; + } + String osName = StringUtils.firstNonBlank(guestOSVO.getName(), guestOSVO.getDisplayName()); + if (StringUtils.isBlank(osName)) { + return false; + } + return osName.toLowerCase().contains("windows"); + } + + protected Pair getNextVmHostAndDisplayName(AutoScaleVmGroupVO asGroup, VirtualMachineTemplate template) { + boolean isWindows = isWindowsOs(template); 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 From 6d28bd274e2a592ec93f00381416097ddbac2d40 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 8 Oct 2025 14:52:59 +0530 Subject: [PATCH 10/11] change Signed-off-by: Abhishek Kumar --- .../com/cloud/network/as/AutoScaleManagerImpl.java | 11 ++++++++--- .../cloud/network/as/AutoScaleManagerImplTest.java | 10 +++++++--- 2 files changed, 15 insertions(+), 6 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 c9f74280682f..8d15e129c03e 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -195,6 +195,9 @@ import com.google.gson.reflect.TypeToken; public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManager, AutoScaleService, Configurable { + // Windows OS has a limit of 15 characters for hostname + // https://learn.microsoft.com/en-us/troubleshoot/windows-server/active-directory/naming-conventions-for-computer-domain-site-ou + protected static final int MAX_WINDOWS_VM_HOSTNAME_LENGTH = 15; @Inject protected DispatchChainFactory dispatchChainFactory = null; @@ -1973,7 +1976,9 @@ public void updateVmDetails(Map deployParams, Map 15 ? valid.substring(valid.length() - 15) : valid; + String hostName = valid.length() > MAX_WINDOWS_VM_HOSTNAME_LENGTH ? + valid.substring(valid.length() - MAX_WINDOWS_VM_HOSTNAME_LENGTH) : + valid; if (!hostName.isEmpty() && !Character.isLetter(hostName.charAt(0))) { for (int i = 0; i < hostName.length(); i++) { if (Character.isLetter(hostName.charAt(i))) { @@ -1982,7 +1987,7 @@ protected String getTrimmedHostNameForWindows(String name) { } } if (!Character.isLetter(hostName.charAt(0))) { - hostName = hostName.length() < 15 ? "a" + hostName : "a" + hostName.substring(1); + hostName = "a" + hostName.substring(hostName.length() < MAX_WINDOWS_VM_HOSTNAME_LENGTH ? 0 : 1); } } return hostName; @@ -2007,7 +2012,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 || name.length() <= 15) { + if (!isWindows || name.length() <= MAX_WINDOWS_VM_HOSTNAME_LENGTH) { return new Pair<>(name, name); } return new Pair<>(getTrimmedHostNameForWindows(name), name); 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 9d69dcb2da59..247b49c4fc27 100644 --- a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java @@ -2595,10 +2595,14 @@ public void prefixesWithLetterIfNoLetterExists() { } @Test - public void trimsHostNameToLast15Characters() { + public void trimsHostNameToLastMaxCharacters() { + String name = "ThisIsAReallyLongHost-Name_1234"; AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); - String result = manager.getTrimmedHostNameForWindows("ThisIsAReallyLongHost-Name_1234"); - Assert.assertEquals("ngHost-Name1234", result); + String result = manager.getTrimmedHostNameForWindows(name); + String normalized = name.replace("_", ""); + Assert.assertEquals(normalized + .substring(normalized.length() - AutoScaleManagerImpl.MAX_WINDOWS_VM_HOSTNAME_LENGTH), + result); } @Test From d2544d4bad6df587c56e7eb31100dc6ac9b9ced0 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 7 Nov 2025 10:15:05 +0100 Subject: [PATCH 11/11] as: simplify windows vm name --- .../network/as/AutoScaleManagerImpl.java | 34 +++------- .../network/as/AutoScaleManagerImplTest.java | 65 ++----------------- 2 files changed, 15 insertions(+), 84 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 8d15e129c03e..bb9769044cae 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -195,9 +195,6 @@ import com.google.gson.reflect.TypeToken; public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManager, AutoScaleService, Configurable { - // Windows OS has a limit of 15 characters for hostname - // https://learn.microsoft.com/en-us/troubleshoot/windows-server/active-directory/naming-conventions-for-computer-domain-site-ou - protected static final int MAX_WINDOWS_VM_HOSTNAME_LENGTH = 15; @Inject protected DispatchChainFactory dispatchChainFactory = null; @@ -303,6 +300,10 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage protected static final String VM_HOSTNAME_PREFIX = "autoScaleVm-"; protected static final int VM_HOSTNAME_RANDOM_SUFFIX_LENGTH = 6; + // Windows OS has a limit of 15 characters for hostname + // https://learn.microsoft.com/en-us/troubleshoot/windows-server/active-directory/naming-conventions-for-computer-domain-site-ou + protected static final String WINDOWS_VM_HOSTNAME_PREFIX = "as-WinVm-"; + private static final Long DEFAULT_HOST_ID = -1L; ExecutorService groupExecutor; @@ -1974,25 +1975,6 @@ public void updateVmDetails(Map deployParams, Map MAX_WINDOWS_VM_HOSTNAME_LENGTH ? - valid.substring(valid.length() - MAX_WINDOWS_VM_HOSTNAME_LENGTH) : - valid; - if (!hostName.isEmpty() && !Character.isLetter(hostName.charAt(0))) { - for (int i = 0; i < hostName.length(); i++) { - if (Character.isLetter(hostName.charAt(i))) { - hostName = hostName.charAt(i) + hostName.substring(i + 1); - break; - } - } - if (!Character.isLetter(hostName.charAt(0))) { - hostName = "a" + hostName.substring(hostName.length() < MAX_WINDOWS_VM_HOSTNAME_LENGTH ? 0 : 1); - } - } - return hostName; - } - protected boolean isWindowsOs(VirtualMachineTemplate template) { GuestOSVO guestOSVO = guestOSDao.findById(template.getGuestOSId()); if (guestOSVO == null) { @@ -2007,15 +1989,15 @@ protected boolean isWindowsOs(VirtualMachineTemplate template) { protected Pair getNextVmHostAndDisplayName(AutoScaleVmGroupVO asGroup, VirtualMachineTemplate template) { boolean isWindows = isWindowsOs(template); - String vmHostNameSuffix = "-" + asGroup.getNextVmSeq() + "-" + - RandomStringUtils.random(VM_HOSTNAME_RANDOM_SUFFIX_LENGTH, 0, 0, true, false, (char[])null, new SecureRandom()).toLowerCase(); + String winVmHostNameSuffix = RandomStringUtils.random(VM_HOSTNAME_RANDOM_SUFFIX_LENGTH, 0, 0, true, false, (char[])null, new SecureRandom()).toLowerCase(); + String vmHostNameSuffix = "-" + asGroup.getNextVmSeq() + "-" + winVmHostNameSuffix; // 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 || name.length() <= MAX_WINDOWS_VM_HOSTNAME_LENGTH) { + if (!isWindows) { return new Pair<>(name, name); } - return new Pair<>(getTrimmedHostNameForWindows(name), name); + return new Pair<>(WINDOWS_VM_HOSTNAME_PREFIX + winVmHostNameSuffix, name); } @Override 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 247b49c4fc27..c186083b8ce1 100644 --- a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java @@ -2520,12 +2520,14 @@ private void runGetNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameFor when(asVmGroupMock.getNextVmSeq()).thenReturn(1L); when(templateMock.getGuestOSId()).thenReturn(1L); Pair result = autoScaleManagerImplSpy.getNextVmHostAndDisplayName(asVmGroupMock, templateMock); - String vmHostNamePattern = AutoScaleManagerImpl.VM_HOSTNAME_PREFIX + vmGroupName + - "-" + asVmGroupMock.getNextVmSeq() + "-[a-z]{6}"; - Assert.assertTrue(result.second().matches(vmHostNamePattern)); + String vmHostNamePattern = AutoScaleManagerImpl.WINDOWS_VM_HOSTNAME_PREFIX + "[a-z]{6}"; + Assert.assertTrue(result.first().matches(vmHostNamePattern)); Assert.assertEquals(15, result.first().length()); - Assert.assertTrue(result.second().endsWith(result.first())); - + String vmDisplayHostNamePattern = AutoScaleManagerImpl.VM_HOSTNAME_PREFIX + vmGroupName + + "-" + asVmGroupMock.getNextVmSeq() + "-[a-z]{6}"; + Assert.assertTrue(result.second().matches(vmDisplayHostNamePattern)); + Assert.assertTrue(result.second().length() <= 63); + Assert.assertTrue(result.second().endsWith(result.first().split("-")[2])); } @Test @@ -2565,57 +2567,4 @@ public void getNextVmHostAndDisplayNameHandlesNullGuestOS() { Assert.assertTrue(result.first().matches(vmHostNamePattern)); Assert.assertEquals(result.first(), result.second()); } - - @Test - public void trimsValidWindowsHostName() { - AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); - String result = manager.getTrimmedHostNameForWindows("ValidVMName1234"); - Assert.assertEquals("ValidVMName1234", result); - } - - @Test - public void trimsInvalidCharactersFromHostName() { - AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); - String result = manager.getTrimmedHostNameForWindows("Invalid@Host#Name!"); - Assert.assertEquals("InvalidHostName", result); - } - - @Test - public void ensuresHostNameStartsWithLetter() { - AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); - String result = manager.getTrimmedHostNameForWindows("123456789012345"); - Assert.assertTrue(Character.isLetter(result.charAt(0))); - } - - @Test - public void prefixesWithLetterIfNoLetterExists() { - AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); - String result = manager.getTrimmedHostNameForWindows("1234567890-12345"); - Assert.assertEquals("a34567890-12345", result); - } - - @Test - public void trimsHostNameToLastMaxCharacters() { - String name = "ThisIsAReallyLongHost-Name_1234"; - AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); - String result = manager.getTrimmedHostNameForWindows(name); - String normalized = name.replace("_", ""); - Assert.assertEquals(normalized - .substring(normalized.length() - AutoScaleManagerImpl.MAX_WINDOWS_VM_HOSTNAME_LENGTH), - result); - } - - @Test - public void handlesEmptyHostName() { - AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); - String result = manager.getTrimmedHostNameForWindows(""); - Assert.assertEquals("", result); - } - - @Test - public void handlesHostNameWithOnlyMostlyInvalidCharacters() { - AutoScaleManagerImpl manager = new AutoScaleManagerImpl(); - String result = manager.getTrimmedHostNameForWindows("@#$%^&*() 1"); - Assert.assertEquals("a1", result); - } }