Skip to content

Commit ac05040

Browse files
shwstpprweizhouapache
authored and
owsferraro
committed
server: trim autoscale Windows VM hostname (apache#11327)
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> Co-authored-by: Wei Zhou <weizhou@apache.org>
1 parent 4962b34 commit ac05040

3 files changed

Lines changed: 111 additions & 14 deletions

File tree

‎server/src/main/java/com/cloud/network/as/AutoScaleManager.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@
1616
// under the License.
1717
package com.cloud.network.as;
1818

19-
import com.cloud.user.Account;
2019
import org.apache.cloudstack.framework.config.ConfigKey;
2120

21+
import com.cloud.user.Account;
22+
2223
public interface AutoScaleManager extends AutoScaleService {
2324

2425
ConfigKey<Integer> AutoScaleStatsInterval = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Integer.class,
@@ -63,7 +64,5 @@ public interface AutoScaleManager extends AutoScaleService {
6364

6465
void removeVmFromVmGroup(Long vmId);
6566

66-
String getNextVmHostName(AutoScaleVmGroupVO asGroup);
67-
6867
void checkAutoScaleVmGroupName(String groupName);
6968
}

‎server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838

3939
import javax.inject.Inject;
4040

41-
import com.cloud.network.NetworkModel;
4241
import org.apache.cloudstack.acl.ControlledEntity;
4342
import org.apache.cloudstack.affinity.AffinityGroupVO;
4443
import org.apache.cloudstack.affinity.dao.AffinityGroupDao;
@@ -113,6 +112,7 @@
113112
import com.cloud.network.Network;
114113
import com.cloud.network.Network.Capability;
115114
import com.cloud.network.Network.IpAddresses;
115+
import com.cloud.network.NetworkModel;
116116
import com.cloud.network.as.AutoScaleCounter.AutoScaleCounterParam;
117117
import com.cloud.network.as.dao.AutoScalePolicyConditionMapDao;
118118
import com.cloud.network.as.dao.AutoScalePolicyDao;
@@ -146,7 +146,9 @@
146146
import com.cloud.server.ResourceTag;
147147
import com.cloud.service.ServiceOfferingVO;
148148
import com.cloud.service.dao.ServiceOfferingDao;
149+
import com.cloud.storage.GuestOSVO;
149150
import com.cloud.storage.dao.DiskOfferingDao;
151+
import com.cloud.storage.dao.GuestOSDao;
150152
import com.cloud.template.TemplateManager;
151153
import com.cloud.template.VirtualMachineTemplate;
152154
import com.cloud.user.Account;
@@ -280,6 +282,8 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage
280282
private NetworkOfferingDao networkOfferingDao;
281283
@Inject
282284
private VirtualMachineManager virtualMachineManager;
285+
@Inject
286+
GuestOSDao guestOSDao;
283287

284288
private static final String PARAM_ROOT_DISK_SIZE = "rootdisksize";
285289
private static final String PARAM_DISK_OFFERING_ID = "diskofferingid";
@@ -296,6 +300,10 @@ public class AutoScaleManagerImpl extends ManagerBase implements AutoScaleManage
296300
protected static final String VM_HOSTNAME_PREFIX = "autoScaleVm-";
297301
protected static final int VM_HOSTNAME_RANDOM_SUFFIX_LENGTH = 6;
298302

303+
// Windows OS has a limit of 15 characters for hostname
304+
// https://learn.microsoft.com/en-us/troubleshoot/windows-server/active-directory/naming-conventions-for-computer-domain-site-ou
305+
protected static final String WINDOWS_VM_HOSTNAME_PREFIX = "as-WinVm-";
306+
299307
private static final Long DEFAULT_HOST_ID = -1L;
300308

301309
ExecutorService groupExecutor;
@@ -1818,26 +1826,28 @@ protected UserVm createNewVM(AutoScaleVmGroupVO asGroup) {
18181826
List<Long> affinityGroupIdList = getVmAffinityGroupId(deployParams);
18191827
updateVmDetails(deployParams, customParameters);
18201828

1821-
String vmHostName = getNextVmHostName(asGroup);
1829+
Pair<String, String> vmHostAndDisplayName = getNextVmHostAndDisplayName(asGroup, template);
1830+
String vmHostName = vmHostAndDisplayName.first();
1831+
String vmDisplayName = vmHostAndDisplayName.second();
18221832
asGroup.setNextVmSeq(asGroup.getNextVmSeq() + 1);
18231833
autoScaleVmGroupDao.persist(asGroup);
18241834

18251835
if (zone.getNetworkType() == NetworkType.Basic) {
18261836
vm = userVmService.createBasicSecurityGroupVirtualMachine(zone, serviceOffering, template, null, owner, vmHostName,
1827-
vmHostName, diskOfferingId, dataDiskSize, null, null,
1837+
vmDisplayName, diskOfferingId, dataDiskSize, null, null,
18281838
hypervisorType, HTTPMethod.GET, userData, userDataId, userDataDetails, sshKeyPairs,
18291839
null, null, true, null, affinityGroupIdList, customParameters, null, null, null,
18301840
null, true, overrideDiskOfferingId, null, null);
18311841
} else {
18321842
if (networkModel.checkSecurityGroupSupportForNetwork(owner, zone, networkIds,
18331843
Collections.emptyList())) {
18341844
vm = userVmService.createAdvancedSecurityGroupVirtualMachine(zone, serviceOffering, template, networkIds, null,
1835-
owner, vmHostName,vmHostName, diskOfferingId, dataDiskSize, null, null,
1845+
owner, vmHostName, vmDisplayName, diskOfferingId, dataDiskSize, null, null,
18361846
hypervisorType, HTTPMethod.GET, userData, userDataId, userDataDetails, sshKeyPairs,
18371847
null, null, true, null, affinityGroupIdList, customParameters, null, null, null,
18381848
null, true, overrideDiskOfferingId, null, null, null);
18391849
} else {
1840-
vm = userVmService.createAdvancedVirtualMachine(zone, serviceOffering, template, networkIds, owner, vmHostName, vmHostName,
1850+
vm = userVmService.createAdvancedVirtualMachine(zone, serviceOffering, template, networkIds, owner, vmHostName, vmDisplayName,
18411851
diskOfferingId, dataDiskSize, null, null,
18421852
hypervisorType, HTTPMethod.GET, userData, userDataId, userDataDetails, sshKeyPairs,
18431853
null, addrs, true, null, affinityGroupIdList, customParameters, null, null, null,
@@ -1962,13 +1972,29 @@ public void updateVmDetails(Map<String, String> deployParams, Map<String, String
19621972
}
19631973
}
19641974

1965-
@Override
1966-
public String getNextVmHostName(AutoScaleVmGroupVO asGroup) {
1967-
String vmHostNameSuffix = "-" + asGroup.getNextVmSeq() + "-" +
1968-
RandomStringUtils.random(VM_HOSTNAME_RANDOM_SUFFIX_LENGTH, 0, 0, true, false, (char[])null, new SecureRandom()).toLowerCase();
1975+
protected boolean isWindowsOs(VirtualMachineTemplate template) {
1976+
GuestOSVO guestOSVO = guestOSDao.findById(template.getGuestOSId());
1977+
if (guestOSVO == null) {
1978+
return false;
1979+
}
1980+
String osName = StringUtils.firstNonBlank(guestOSVO.getName(), guestOSVO.getDisplayName());
1981+
if (StringUtils.isBlank(osName)) {
1982+
return false;
1983+
}
1984+
return osName.toLowerCase().contains("windows");
1985+
}
1986+
1987+
protected Pair<String, String> getNextVmHostAndDisplayName(AutoScaleVmGroupVO asGroup, VirtualMachineTemplate template) {
1988+
boolean isWindows = isWindowsOs(template);
1989+
String winVmHostNameSuffix = RandomStringUtils.random(VM_HOSTNAME_RANDOM_SUFFIX_LENGTH, 0, 0, true, false, (char[])null, new SecureRandom()).toLowerCase();
1990+
String vmHostNameSuffix = "-" + asGroup.getNextVmSeq() + "-" + winVmHostNameSuffix;
19691991
// Truncate vm group name because max length of vm name is 63
19701992
int subStringLength = Math.min(asGroup.getName().length(), 63 - VM_HOSTNAME_PREFIX.length() - vmHostNameSuffix.length());
1971-
return VM_HOSTNAME_PREFIX + asGroup.getName().substring(0, subStringLength) + vmHostNameSuffix;
1993+
String name = VM_HOSTNAME_PREFIX + asGroup.getName().substring(0, subStringLength) + vmHostNameSuffix;
1994+
if (!isWindows) {
1995+
return new Pair<>(name, name);
1996+
}
1997+
return new Pair<>(WINDOWS_VM_HOSTNAME_PREFIX + winVmHostNameSuffix, name);
19721998
}
19731999

19742000
@Override

‎server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,10 @@
135135
import com.cloud.service.ServiceOfferingVO;
136136
import com.cloud.service.dao.ServiceOfferingDao;
137137
import com.cloud.storage.DiskOfferingVO;
138+
import com.cloud.storage.GuestOSVO;
138139
import com.cloud.storage.VMTemplateVO;
139140
import com.cloud.storage.dao.DiskOfferingDao;
141+
import com.cloud.storage.dao.GuestOSDao;
140142
import com.cloud.template.VirtualMachineTemplate;
141143
import com.cloud.user.Account;
142144
import com.cloud.user.AccountManager;
@@ -259,9 +261,10 @@ public class AutoScaleManagerImplTest {
259261
LoadBalancingRulesService loadBalancingRulesService;
260262
@Mock
261263
VMInstanceDao vmInstanceDao;
262-
263264
@Mock
264265
VirtualMachineManager virtualMachineManager;
266+
@Mock
267+
GuestOSDao guestOSDao;
265268

266269
AccountVO account;
267270
UserVO user;
@@ -420,6 +423,11 @@ public void setUp() {
420423
userDataDetails.put("0", new HashMap<>() {{ put("key1", "value1"); put("key2", "value2"); }});
421424
Mockito.doReturn(userDataFinal).when(userVmMgr).finalizeUserData(any(), any(), any());
422425
Mockito.doReturn(userDataFinal).when(userDataMgr).validateUserData(eq(userDataFinal), nullable(BaseCmd.HTTPMethod.class));
426+
427+
when(templateMock.getGuestOSId()).thenReturn(100L);
428+
GuestOSVO guestOSMock = Mockito.mock(GuestOSVO.class);
429+
when(guestOSDao.findById(anyLong())).thenReturn(guestOSMock);
430+
when(guestOSMock.getName()).thenReturn("linux");
423431
}
424432

425433
@After
@@ -2495,4 +2503,68 @@ public void destroyVm() {
24952503

24962504
Mockito.verify(userVmMgr).expunge(eq(userVmMock));
24972505
}
2506+
2507+
@Test
2508+
public void getNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForLinuxTemplate() {
2509+
when(asVmGroupMock.getName()).thenReturn(vmGroupName);
2510+
when(asVmGroupMock.getNextVmSeq()).thenReturn(1L);
2511+
Pair<String, String> result = autoScaleManagerImplSpy.getNextVmHostAndDisplayName(asVmGroupMock, templateMock);
2512+
String vmHostNamePattern = AutoScaleManagerImpl.VM_HOSTNAME_PREFIX + vmGroupName +
2513+
"-" + asVmGroupMock.getNextVmSeq() + "-[a-z]{6}";
2514+
Assert.assertTrue(result.first().matches(vmHostNamePattern));
2515+
Assert.assertEquals(result.first(), result.second());
2516+
}
2517+
2518+
private void runGetNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplate() {
2519+
when(asVmGroupMock.getName()).thenReturn(vmGroupName);
2520+
when(asVmGroupMock.getNextVmSeq()).thenReturn(1L);
2521+
when(templateMock.getGuestOSId()).thenReturn(1L);
2522+
Pair<String, String> result = autoScaleManagerImplSpy.getNextVmHostAndDisplayName(asVmGroupMock, templateMock);
2523+
String vmHostNamePattern = AutoScaleManagerImpl.WINDOWS_VM_HOSTNAME_PREFIX + "[a-z]{6}";
2524+
Assert.assertTrue(result.first().matches(vmHostNamePattern));
2525+
Assert.assertEquals(15, result.first().length());
2526+
String vmDisplayHostNamePattern = AutoScaleManagerImpl.VM_HOSTNAME_PREFIX + vmGroupName +
2527+
"-" + asVmGroupMock.getNextVmSeq() + "-[a-z]{6}";
2528+
Assert.assertTrue(result.second().matches(vmDisplayHostNamePattern));
2529+
Assert.assertTrue(result.second().length() <= 63);
2530+
Assert.assertTrue(result.second().endsWith(result.first().split("-")[2]));
2531+
}
2532+
2533+
@Test
2534+
public void getNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplateUsingGuestOsName() {
2535+
GuestOSVO guestOS = Mockito.mock(GuestOSVO.class);
2536+
when(guestOS.getName()).thenReturn("Windows Server");
2537+
when(guestOSDao.findById(1L)).thenReturn(guestOS);
2538+
runGetNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplate();
2539+
}
2540+
2541+
@Test
2542+
public void getNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplateUsingGuestOsDisplayName() {
2543+
GuestOSVO guestOS = Mockito.mock(GuestOSVO.class);
2544+
when(guestOS.getDisplayName()).thenReturn("Windows Server");
2545+
when(guestOSDao.findById(1L)).thenReturn(guestOS);
2546+
runGetNextVmHostAndDisplayNameGeneratesCorrectHostAndDisplayNameForWindowsTemplate();
2547+
}
2548+
2549+
@Test
2550+
public void getNextVmHostAndDisplayNameTruncatesGroupNameWhenExceedingMaxLength() {
2551+
when(asVmGroupMock.getName()).thenReturn(vmGroupNameWithMaxLength);
2552+
when(asVmGroupMock.getNextVmSeq()).thenReturn(1L);
2553+
Pair<String, String> result = autoScaleManagerImplSpy.getNextVmHostAndDisplayName(asVmGroupMock, templateMock);
2554+
Assert.assertTrue(result.first().length() <= 63);
2555+
Assert.assertTrue(result.second().length() <= 63);
2556+
}
2557+
2558+
@Test
2559+
public void getNextVmHostAndDisplayNameHandlesNullGuestOS() {
2560+
when(asVmGroupMock.getName()).thenReturn(vmGroupName);
2561+
when(asVmGroupMock.getNextVmSeq()).thenReturn(1L);
2562+
when(templateMock.getGuestOSId()).thenReturn(1L);
2563+
when(guestOSDao.findById(1L)).thenReturn(null);
2564+
Pair<String, String> result = autoScaleManagerImplSpy.getNextVmHostAndDisplayName(asVmGroupMock, templateMock);
2565+
String vmHostNamePattern = AutoScaleManagerImpl.VM_HOSTNAME_PREFIX + vmGroupName +
2566+
"-" + asVmGroupMock.getNextVmSeq() + "-[a-z]{6}";
2567+
Assert.assertTrue(result.first().matches(vmHostNamePattern));
2568+
Assert.assertEquals(result.first(), result.second());
2569+
}
24982570
}

0 commit comments

Comments
 (0)