Skip to content

Commit cb6049a

Browse files
engine: cover the stop paths that decide whether an address is freed
These are the branches that decide whether a still-running instance keeps its addresses, and they had no tests. `VirtualMachineManagerImplTest`: | test | checks | |-----------------------------------------------|-----------------------------------------------| | missingReportUsesUnforcedStopAndKeepsResources | a missing report stops unforced, and a failed stop does not release resources | | powerOffKeepsForcedStop | a PowerOff report still stops forced | | ensure...skipsExternal | External instances are left to their extension | | ensure...noHostIdDoesNothing | nothing is sent when there is no host to send to | | ensure...fallsBackToLastHostId | the last known host is used when `host_id` is cleared | | ensure...agentUnavailableIsSwallowed | an unreachable host does not break the expunge | `VirtualMachinePowerStateSyncImplTest` gains the AlertManager mock the alert needs. Signed-off-by: Brad House <bhouse@nexthop.ai>
1 parent 2b38e60 commit cb6049a

2 files changed

Lines changed: 85 additions & 0 deletions

File tree

‎engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static org.junit.Assert.assertThrows;
2525
import static org.junit.Assert.assertTrue;
2626
import static org.mockito.ArgumentMatchers.any;
27+
import static org.mockito.ArgumentMatchers.anyBoolean;
2728
import static org.mockito.ArgumentMatchers.anyList;
2829
import static org.mockito.ArgumentMatchers.anyLong;
2930
import static org.mockito.ArgumentMatchers.anyString;
@@ -2049,4 +2050,85 @@ public void testUnmanageSuccessKvm() throws Exception {
20492050
}
20502051
}
20512052

2053+
@Test
2054+
public void testEnsureInstanceIsStoppedOnLastKnownHost_skipsExternal() throws Exception {
2055+
when(vmInstanceMock.getHypervisorType()).thenReturn(HypervisorType.External);
2056+
virtualMachineManagerImpl.ensureInstanceIsStoppedOnLastKnownHost(vmInstanceMock);
2057+
verify(agentManagerMock, never()).send(anyLong(), any(StopCommand.class));
2058+
}
2059+
2060+
@Test
2061+
public void testEnsureInstanceIsStoppedOnLastKnownHost_noHostIdDoesNothing() throws Exception {
2062+
when(vmInstanceMock.getHypervisorType()).thenReturn(HypervisorType.KVM);
2063+
when(vmInstanceMock.getHostId()).thenReturn(null);
2064+
when(vmInstanceMock.getLastHostId()).thenReturn(null);
2065+
virtualMachineManagerImpl.ensureInstanceIsStoppedOnLastKnownHost(vmInstanceMock);
2066+
verify(agentManagerMock, never()).send(anyLong(), any(StopCommand.class));
2067+
}
2068+
2069+
@Test
2070+
public void testEnsureInstanceIsStoppedOnLastKnownHost_fallsBackToLastHostId() throws Exception {
2071+
StopCommand stopCommand = mock(StopCommand.class);
2072+
when(vmInstanceMock.getHypervisorType()).thenReturn(HypervisorType.KVM);
2073+
when(vmInstanceMock.getHostId()).thenReturn(null);
2074+
when(vmInstanceMock.getLastHostId()).thenReturn(7L);
2075+
doReturn(stopCommand).when(virtualMachineManagerImpl).buildStopCommand(any(), any(), eq(false));
2076+
com.cloud.agent.api.Answer answer = mock(com.cloud.agent.api.Answer.class);
2077+
when(answer.getResult()).thenReturn(true);
2078+
when(agentManagerMock.send(eq(7L), any(StopCommand.class))).thenReturn(answer);
2079+
2080+
virtualMachineManagerImpl.ensureInstanceIsStoppedOnLastKnownHost(vmInstanceMock);
2081+
2082+
verify(agentManagerMock, times(1)).send(eq(7L), any(StopCommand.class));
2083+
}
2084+
2085+
@Test
2086+
public void testEnsureInstanceIsStoppedOnLastKnownHost_agentUnavailableIsSwallowed() throws Exception {
2087+
StopCommand stopCommand = mock(StopCommand.class);
2088+
when(vmInstanceMock.getHypervisorType()).thenReturn(HypervisorType.KVM);
2089+
when(vmInstanceMock.getHostId()).thenReturn(7L);
2090+
doReturn(stopCommand).when(virtualMachineManagerImpl).buildStopCommand(any(), any(), eq(false));
2091+
when(agentManagerMock.send(anyLong(), any(StopCommand.class)))
2092+
.thenThrow(new AgentUnavailableException("down", 7L));
2093+
2094+
// must not propagate, expunge continues
2095+
virtualMachineManagerImpl.ensureInstanceIsStoppedOnLastKnownHost(vmInstanceMock);
2096+
2097+
verify(agentManagerMock, times(1)).send(anyLong(), any(StopCommand.class));
2098+
}
2099+
2100+
/**
2101+
* A missing report is weak evidence, so the stop must not be forced: sendStop() answers success for an
2102+
* unreachable host when forced, which would free the addresses of an instance that is still running.
2103+
*/
2104+
@Test
2105+
public void testHandlePowerOffReport_missingReportUsesUnforcedStopAndKeepsResources() {
2106+
when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Migrating);
2107+
when(vmInstanceMock.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerReportMissing);
2108+
doReturn(mock(VirtualMachineGuru.class)).when(virtualMachineManagerImpl).getVmGuru(any());
2109+
doReturn(new Pair<>(false, "host did not answer")).when(virtualMachineManagerImpl)
2110+
.sendStop(any(), any(), eq(false), eq(true));
2111+
2112+
virtualMachineManagerImpl.handlePowerOffReportWithNoPendingJobsOnVM(vmInstanceMock);
2113+
2114+
verify(virtualMachineManagerImpl, times(1)).sendStop(any(), any(), eq(false), eq(true));
2115+
verify(virtualMachineManagerImpl, never()).releaseVmResources(any(), anyBoolean());
2116+
}
2117+
2118+
/**
2119+
* A PowerOff report is the host stating the instance is down, so that path keeps its forced stop.
2120+
*/
2121+
@Test
2122+
public void testHandlePowerOffReport_powerOffKeepsForcedStop() {
2123+
when(vmInstanceMock.getState()).thenReturn(VirtualMachine.State.Migrating);
2124+
when(vmInstanceMock.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOff);
2125+
doReturn(mock(VirtualMachineGuru.class)).when(virtualMachineManagerImpl).getVmGuru(any());
2126+
doReturn(new Pair<>(false, "host did not answer")).when(virtualMachineManagerImpl)
2127+
.sendStop(any(), any(), eq(true), eq(true));
2128+
2129+
virtualMachineManagerImpl.handlePowerOffReportWithNoPendingJobsOnVM(vmInstanceMock);
2130+
2131+
verify(virtualMachineManagerImpl, times(1)).sendStop(any(), any(), eq(true), eq(true));
2132+
verify(virtualMachineManagerImpl, never()).releaseVmResources(any(), anyBoolean());
2133+
}
20522134
}

‎engine/orchestration/src/test/java/com/cloud/vm/VirtualMachinePowerStateSyncImplTest.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.mockito.junit.MockitoJUnitRunner;
3737

3838
import com.cloud.agent.api.HostVmStateReportEntry;
39+
import com.cloud.alert.AlertManager;
3940
import com.cloud.configuration.ManagementServiceConfiguration;
4041
import com.cloud.host.HostVO;
4142
import com.cloud.host.dao.HostDao;
@@ -51,6 +52,8 @@ public class VirtualMachinePowerStateSyncImplTest {
5152
HostDao hostDao;
5253
@Mock
5354
ManagementServiceConfiguration mgmtServiceConf;
55+
@Mock
56+
AlertManager alertManager;
5457

5558
@InjectMocks
5659
VirtualMachinePowerStateSyncImpl virtualMachinePowerStateSync = new VirtualMachinePowerStateSyncImpl();

0 commit comments

Comments
 (0)