diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index b69491b64668..f378da12af22 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1630,9 +1630,10 @@ public void orchestrateStart(final String vmUuid, final Map> getVolumesToDisconnect(VirtualMachine vm) { return volumesToDisconnect; } - protected boolean sendStop(final VirtualMachineGuru guru, final VirtualMachineProfile profile, final boolean force, final boolean checkBeforeCleanup) { + protected Pair sendStop(final VirtualMachineGuru guru, final VirtualMachineProfile profile, final boolean force, final boolean checkBeforeCleanup) { final VirtualMachine vm = profile.getVirtualMachine(); Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); StopCommand stpCmd = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType()), checkBeforeCleanup); @@ -2241,7 +2242,7 @@ protected boolean sendStop(final VirtualMachineGuru guru, final VirtualMachinePr if (!answer.getResult()) { final String details = answer.getDetails(); logger.debug("Unable to stop VM due to {}", details); - return false; + return new Pair<>(false, details); } guru.finalizeStop(profile, answer); @@ -2254,21 +2255,23 @@ protected boolean sendStop(final VirtualMachineGuru guru, final VirtualMachinePr } } } else { - logger.error("Invalid answer received in response to a StopCommand for {}", vm.getInstanceName()); - return false; + String errorMsg = String.format("Invalid answer received in response to a StopCommand for %s", vm.getInstanceName()); + logger.error(errorMsg); + return new Pair<>(false, errorMsg); } } catch (final AgentUnavailableException | OperationTimedoutException e) { - logger.warn("Unable to stop {} due to [{}].", vm.toString(), e.getMessage(), e); + String errorMsg = String.format("Unable to stop %s due to [%s].", vm.toString(), e.getMessage()); + logger.warn(errorMsg, e); if (!force) { - return false; + return new Pair<>(false, errorMsg); } } - return true; + return new Pair<>(true, null); } - protected boolean cleanup(final VirtualMachineGuru guru, final VirtualMachineProfile profile, final ItWorkVO work, final Event event, final boolean cleanUpEvenIfUnableToStop) { + protected Pair cleanup(final VirtualMachineGuru guru, final VirtualMachineProfile profile, final ItWorkVO work, final Event event, final boolean cleanUpEvenIfUnableToStop) { final VirtualMachine vm = profile.getVirtualMachine(); final State state = vm.getState(); logger.debug("Cleaning up resources for the vm {} in {} state", vm, state); @@ -2277,57 +2280,63 @@ protected boolean cleanup(final VirtualMachineGuru guru, final VirtualMachinePro if (work != null) { final Step step = work.getStep(); if (step == Step.Starting && !cleanUpEvenIfUnableToStop) { - logger.warn("Unable to cleanup vm {}; work state is incorrect: {}", vm, step); - return false; + String errorMsg = String.format("Unable to cleanup vm %s; work state is incorrect: %s", vm, step); + logger.warn(errorMsg); + return new Pair<>(false, errorMsg); } if (step == Step.Started || step == Step.Starting || step == Step.Release) { if (vm.getHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { + Pair result = sendStop(guru, profile, cleanUpEvenIfUnableToStop, false); + if (!result.first()) { logger.warn("Failed to stop vm {} in {} state as a part of cleanup process", vm, State.Starting); - return false; + return result; } } } if (step != Step.Release && step != Step.Prepare && step != Step.Started && step != Step.Starting) { logger.debug("Cleanup is not needed for vm {}; work state is incorrect: {}", vm, step); - return true; + return new Pair<>(true, null); } } else { if (vm.getHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { + Pair result = sendStop(guru, profile, cleanUpEvenIfUnableToStop, false); + if (!result.first()) { logger.warn("Failed to stop vm {} in {} state as a part of cleanup process", vm, State.Starting); - return false; + return result; } } } } else if (state == State.Stopping) { if (vm.getHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { + Pair result = sendStop(guru, profile, cleanUpEvenIfUnableToStop, false); + if (!result.first()) { logger.warn("Failed to stop vm {} in {} state as a part of cleanup process", vm, State.Stopping); - return false; + return result; } } } else if (state == State.Migrating) { if (vm.getHostId() != null || vm.getLastHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { + Pair result = sendStop(guru, profile, cleanUpEvenIfUnableToStop, false); + if (!result.first()) { logger.warn("Failed to stop vm {} in {} state as a part of cleanup process", vm, State.Migrating); - return false; + return result; } } } else if (state == State.Running) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { + Pair result = sendStop(guru, profile, cleanUpEvenIfUnableToStop, false); + if (!result.first()) { logger.warn("Failed to stop vm {} in {} state as a part of cleanup process", vm, State.Running); - return false; + return result; } } } finally { releaseVmResources(profile, cleanUpEvenIfUnableToStop); } - return true; + return new Pair<>(true, null); } protected void releaseVmResources(final VirtualMachineProfile profile, final boolean forced) { @@ -2509,7 +2518,8 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl logger.warn("Unable to transition the state but we're moving on because it's forced stop", e1); if (doCleanup) { - if (cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.StopRequested, cleanUpEvenIfUnableToStop)) { + Pair cleanupResult = cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.StopRequested, cleanUpEvenIfUnableToStop); + if (cleanupResult.first()) { try { if (work != null) { logger.debug("Updating work item to Done, id: {}", work.getId()); @@ -2524,7 +2534,8 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl } } else { logger.debug("Failed to cleanup VM: {}", vm); - throw new CloudRuntimeException("Failed to cleanup " + vm + " , current state " + vm.getState()); + String errorDetails = cleanupResult.second() != null ? " due to " + cleanupResult.second() : ""; + throw new CloudRuntimeException("Failed to cleanup " + vm + " , current state " + vm.getState() + errorDetails); } } } @@ -2582,7 +2593,8 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl } catch (final NoTransitionException e) { logger.warn("Unable to transition the state " + vm, e); } - throw new CloudRuntimeException("Unable to stop " + vm); + String errorDetails = (answer != null && answer.getDetails() != null) ? " due to " + answer.getDetails() : ""; + throw new CloudRuntimeException("Unable to stop " + vm + errorDetails); } else { logger.warn("Unable to actually stop {} but continue with release because it's a force stop", vm); vmGuru.finalizeStop(profile, answer); @@ -3261,8 +3273,9 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy } catch (final AgentUnavailableException e) { logger.error("AgentUnavailableException while cleanup on source host: {}", fromHost, e); } - cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); - throw new CloudRuntimeException("Unable to complete migration for " + vm); + Pair cleanupResult = cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); + String errorDetails = (cleanupResult.second() != null) ? " due to " + cleanupResult.second() : ""; + throw new CloudRuntimeException("Unable to complete migration for " + vm + errorDetails); } } catch (final OperationTimedoutException e) { logger.warn("Error while checking the vm {} on host {}", vm, dest.getHost(), e); @@ -3718,8 +3731,9 @@ private void orchestrateMigrateWithStorage(final String vmUuid, final long srcHo } catch (final AgentUnavailableException e) { logger.error("AgentUnavailableException while cleanup on source host: {}", srcHost, e); } - cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); - throw new CloudRuntimeException("VM not found on destination host. Unable to complete migration for " + vm); + Pair cleanupResult = cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); + String errorDetails = (cleanupResult.second() != null) ? " due to " + cleanupResult.second() : ""; + throw new CloudRuntimeException("VM not found on destination host. Unable to complete migration for " + vm + errorDetails); } } catch (final OperationTimedoutException e) { logger.error("Error while checking the vm {} is on host {}", vm, destHost, e); @@ -5003,8 +5017,9 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI } catch (final AgentUnavailableException e) { logger.error("Unable to cleanup source host [{}] due to [{}].", fromHost, e.getMessage(), e); } - cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); - throw new CloudRuntimeException("Unable to complete migration for " + vm); + Pair cleanupResult = cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); + String errorDetails = (cleanupResult.second() != null) ? " due to " + cleanupResult.second() : ""; + throw new CloudRuntimeException("Unable to complete migration for " + vm + errorDetails); } } catch (final OperationTimedoutException e) { logger.debug("Error while checking the {} on {}", vm, dstHost, e); @@ -5465,7 +5480,8 @@ private void handlePowerOffReportWithNoPendingJobsOnVM(final VMInstanceVO vm) { if (PowerState.PowerOff.equals(vm.getPowerState())) { final VirtualMachineGuru vmGuru = getVmGuru(vm); final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); - if (!sendStop(vmGuru, profile, true, true)) { + Pair result = sendStop(vmGuru, profile, true, true); + if (!result.first()) { return; } else { // Release resources on StopCommand success diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index a07870d09af2..7329b67b4be7 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -394,9 +394,9 @@ public void testSendStopWithOkAnswer() throws Exception { when(vm.getHostId()).thenReturn(1L); when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); - boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); + Pair actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); - Assert.assertTrue(actual); + Assert.assertTrue(actual.first()); } @Test @@ -409,9 +409,10 @@ public void testSendStopWithFailAnswer() throws Exception { when(vm.getHostId()).thenReturn(1L); when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); - boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); + Pair actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); - assertFalse(actual); + assertFalse(actual.first()); + Assert.assertEquals("fail", actual.second()); } @Test @@ -421,11 +422,13 @@ public void testSendStopWithNullAnswer() throws Exception { VirtualMachineProfile profile = mock(VirtualMachineProfile.class); when(profile.getVirtualMachine()).thenReturn(vm); when(vm.getHostId()).thenReturn(1L); + when(vm.getInstanceName()).thenReturn("test-vm"); when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(null); - boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); + Pair actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); - assertFalse(actual); + assertFalse(actual.first()); + Assert.assertNotNull(actual.second()); } @Test