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 83493ad702b7..5ce5b3af7774 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"; @@ -1810,13 +1814,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); @@ -1824,12 +1830,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, @@ -1954,13 +1960,27 @@ public void updateVmDetails(Map deployParams, Map getNextVmHostAndDisplayName(AutoScaleVmGroupVO asGroup, VirtualMachineTemplate template) { + template.getGuestOSId(); + GuestOSVO guestOSVO = guestOSDao.findById(template.getGuestOSId()); + 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 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 name = VM_HOSTNAME_PREFIX + asGroup.getName().substring(0, subStringLength) + vmHostNameSuffix; + 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); } @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 7ddd0d612138..8afc154bff5e 100644 --- a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java @@ -135,8 +135,10 @@ import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.DiskOfferingVO; +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 +261,10 @@ public class AutoScaleManagerImplTest { LoadBalancingRulesService loadBalancingRulesService; @Mock VMInstanceDao vmInstanceDao; - @Mock VirtualMachineManager virtualMachineManager; + @Mock + GuestOSDao guestOSDao; AccountVO account; UserVO user; @@ -420,6 +423,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 +2503,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()); + } }