From 9c2bf245c69c973693966dad51462d76e867ea18 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 1 Oct 2026 16:03:20 +0200 Subject: [PATCH] considder stopped VMs on autoscale --- .../as/dao/AutoScaleVmGroupVmMapDaoImpl.java | 2 +- .../dao/AutoScaleVmGroupVmMapDaoImplTest.java | 14 +++++++ .../network/as/AutoScaleManagerImpl.java | 2 +- .../network/as/AutoScaleManagerImplTest.java | 37 +++++++++++++++++++ 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/network/as/dao/AutoScaleVmGroupVmMapDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/as/dao/AutoScaleVmGroupVmMapDaoImpl.java index b2f4e578a82f..576dac14bed2 100644 --- a/engine/schema/src/main/java/com/cloud/network/as/dao/AutoScaleVmGroupVmMapDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/as/dao/AutoScaleVmGroupVmMapDaoImpl.java @@ -132,7 +132,7 @@ public int expungeByVmList(List vmIds, Long batchSize) { public int getErroredInstanceCount(long vmGroupId) { SearchCriteria sc = CountBy.create(); sc.setParameters("vmGroupId", vmGroupId); - sc.setJoinParameters("vmSearch", "states", State.Error); + sc.setJoinParameters("vmSearch", "states", State.Error, State.Stopped); final List results = customSearch(sc, null); return results.get(0); } diff --git a/engine/schema/src/test/java/com/cloud/network/as/dao/AutoScaleVmGroupVmMapDaoImplTest.java b/engine/schema/src/test/java/com/cloud/network/as/dao/AutoScaleVmGroupVmMapDaoImplTest.java index 6de8960ae747..8aa51b926b26 100644 --- a/engine/schema/src/test/java/com/cloud/network/as/dao/AutoScaleVmGroupVmMapDaoImplTest.java +++ b/engine/schema/src/test/java/com/cloud/network/as/dao/AutoScaleVmGroupVmMapDaoImplTest.java @@ -82,6 +82,20 @@ public void testCountAvailableVmsByGroup() throws Exception { Mockito.verify(searchCriteriaCountAvailableVmsByGroup).setJoinParameters("vmSearch", "states", new Object[] {VirtualMachine.State.Starting, VirtualMachine.State.Running, VirtualMachine.State.Stopping, VirtualMachine.State.Migrating}); } + @Test + public void testGetErroredInstanceCount() throws Exception { + Mockito.doReturn(Arrays.asList(3)).when(AutoScaleVmGroupVmMapDaoImplSpy).customSearch(Mockito.any(SearchCriteria.class), Mockito.eq(null)); + + long groupId = 4L; + + int result = AutoScaleVmGroupVmMapDaoImplSpy.getErroredInstanceCount(groupId); + + Assert.assertEquals(3, result); + + Mockito.verify(searchCriteriaCountAvailableVmsByGroup).setParameters("vmGroupId", groupId); + Mockito.verify(searchCriteriaCountAvailableVmsByGroup).setJoinParameters("vmSearch", "states", new Object[] {VirtualMachine.State.Error, VirtualMachine.State.Stopped}); + } + @Test public void testCountByGroup() throws Exception { Mockito.doNothing().when(searchCriteriaAutoScaleVmGroupVmMapVOMock).setParameters(Mockito.anyString(), Mockito.any()); 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 8e93c6817643..43b63dd4b084 100644 --- a/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java +++ b/server/src/main/java/com/cloud/network/as/AutoScaleManagerImpl.java @@ -2112,7 +2112,7 @@ public void doScaleUp(long groupId, Integer numVm) { String.format("Failed to assign LB rule for VM %s in AutoScale VM group %s", vm, asGroup), groupId, ApiCommandResourceType.AutoScaleVmGroup.toString(), 0); break; } - } catch (ServerApiException e) { + } catch (CloudRuntimeException e) { logger.error("Can not deploy new VM for scaling up in the group {}. Waiting for next round", asGroup); ActionEventUtils.onCompletedActionEvent(User.UID_SYSTEM, asGroup.getAccountId(), EventVO.LEVEL_ERROR, EventTypes.EVENT_AUTOSCALEVMGROUP_SCALEUP, String.format("Failed to start VM %s in AutoScale VM group %s", vm, asGroup), groupId, ApiCommandResourceType.AutoScaleVmGroup.toString(), 0); 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 4dc815cafd06..2553fc3c4264 100644 --- a/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/as/AutoScaleManagerImplTest.java @@ -1526,6 +1526,43 @@ public void testDoScaleUp() throws ResourceUnavailableException, InsufficientCap } } + /** + * Regression test for #14185: VirtualMachineManagerImpl.start() wraps a failed start into an unchecked + * CloudRuntimeException rather than the checked exceptions startNewVM converts to ServerApiException. + * Before the fix, doScaleUp's catch(ServerApiException) missed it, the VM was never destroyed, and its + * autoscale_vmgroup_vm_map row leaked forever (the VM stays in State.Stopped, invisible to both + * getErroredInstanceCount() and countAvailableVmsByGroup(), so the group scales up again next interval). + */ + @Test + public void testDoScaleUpDestroysVmWhenStartThrowsCloudRuntimeException() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { + try (MockedStatic ignored = Mockito.mockStatic(ActionEventUtils.class)) { + when(autoScaleVmGroupDao.findById(vmGroupId)).thenReturn(asVmGroupMock); + when(asVmGroupMock.getId()).thenReturn(vmGroupId); + when(asVmGroupMock.getAccountId()).thenReturn(accountId); + when(asVmGroupMock.getMaxMembers()).thenReturn(maxMembers); + when(autoScaleVmGroupVmMapDao.countAvailableVmsByGroup(vmGroupId)).thenReturn(maxMembers - 1); + when(autoScaleVmGroupVmMapDao.getErroredInstanceCount(vmGroupId)).thenReturn(0); + when(asVmGroupMock.getState()).thenReturn(AutoScaleVmGroup.State.ENABLED); + + when(autoScaleVmGroupDao.updateState(vmGroupId, AutoScaleVmGroup.State.ENABLED, AutoScaleVmGroup.State.SCALING)).thenReturn(true); + when(autoScaleVmGroupDao.updateState(vmGroupId, AutoScaleVmGroup.State.SCALING, AutoScaleVmGroup.State.ENABLED)).thenReturn(true); + Mockito.doReturn(userVmMock).when(autoScaleManagerImplSpy).createNewVM(asVmGroupMock); + when(userVmMock.getId()).thenReturn(virtualMachineId); + + Mockito.doThrow(new CloudRuntimeException(String.format("Unable to start a VM [%s] due to [Resource unavailable].", virtualMachineId))) + .when(userVmMgr).startVirtualMachine(virtualMachineId, null, new HashMap<>(), null); + Mockito.doReturn(true).when(autoScaleManagerImplSpy).destroyVm(virtualMachineId); + + autoScaleManagerImplSpy.doScaleUp(vmGroupId, 1); + + Mockito.verify(autoScaleVmGroupVmMapDao).persist(any(AutoScaleVmGroupVmMapVO.class)); + Mockito.verify(autoScaleManagerImplSpy).destroyVm(virtualMachineId); + Mockito.verify(loadBalancingRulesService, Mockito.never()).assignToLoadBalancer(anyLong(), any(), any(), eq(true)); + // the group must leave SCALING even though the start failed, so the next monitor interval can retry + Mockito.verify(autoScaleVmGroupDao).updateState(vmGroupId, AutoScaleVmGroup.State.SCALING, AutoScaleVmGroup.State.ENABLED); + } + } + @Test public void testDoScaleDown() { try (MockedStatic ignored = Mockito.mockStatic(ActionEventUtils.class)) {