diff --git a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java index e3f67420a2ab..0487c8e1577d 100644 --- a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java +++ b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java @@ -433,6 +433,11 @@ public boolean scheduleMigration(final VMInstanceVO vm) { @Override public void scheduleRestart(VMInstanceVO vm, boolean investigate, ReasonType reasonType) { + if (VirtualMachine.State.Error.equals(vm.getState())) { + logger.info("Skipping HA restart for VM {} because it is in Error state", vm); + return; + } + if (!VmHaEnabled.valueIn(vm.getDataCenterId())) { String message = String.format("Unable to schedule restart for the VM %s (%d), VM high availability manager is disabled.", vm.getName(), vm.getId()); if (logger.isDebugEnabled()) { @@ -571,6 +576,13 @@ private void startVm(VirtualMachine vm, Map protected Long restart(final HaWorkVO work) { logger.debug("RESTART with HAWORK"); + long vmId = work.getInstanceId(); + VirtualMachine vm = _itMgr.findById(vmId); + if (vm != null && VirtualMachine.State.Error.equals(vm.getState())) { + logger.info("Skipping HA restart for VM {} because it is in Error state", vm); + return null; + } + List items = _haDao.listFutureHaWorkForVm(work.getInstanceId(), work.getId()); if (items.size() > 0) { StringBuilder str = new StringBuilder("Cancelling this work item because newer ones have been scheduled. Work Ids = ["); @@ -593,9 +605,6 @@ protected Long restart(final HaWorkVO work) { return (System.currentTimeMillis() >> 10) + _investigateRetryInterval; } - long vmId = work.getInstanceId(); - - VirtualMachine vm = _itMgr.findById(work.getInstanceId()); if (vm == null) { logger.info("Unable to find vm: " + vmId); return null; @@ -1037,6 +1046,12 @@ private void processWork(final HaWorkVO work) { final WorkType wt = work.getWorkType(); final VMInstanceVO vm = _instanceDao.findById(work.getInstanceId()); try { + if (wt == WorkType.HA && vm != null && VirtualMachine.State.Error.equals(vm.getState())) { + logger.info("Skipping HA restart for VM {} because it is in Error state", vm); + work.setStep(Step.Done); + return; + } + if (vm != null && !VmHaEnabled.valueIn(vm.getDataCenterId())) { if (logger.isDebugEnabled()) { logger.debug(String.format("VM high availability manager is disabled, rescheduling the HA work %s, for the VM %s (id) to retry later in case VM high availability manager is enabled on retry attempt", work, vm.getName(), vm.getId())); diff --git a/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java b/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java index 9274bd1ff085..802734195315 100644 --- a/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java +++ b/server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.ha; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -267,6 +268,194 @@ public void scheduleRestartHostNotSupported() { highAvailabilityManager.scheduleRestart(vm, true); } + @Test + public void scheduleRestartVMInErrorState() { + VMInstanceVO vm = createVmForProcessing(VirtualMachine.State.Error); + + for (boolean investigate : new boolean[] {true, false}) { + highAvailabilityManager.scheduleRestart(vm, investigate); + } + + Mockito.verifyNoInteractions(_haDao, _itMgr, _alertMgr); + } + + @Test + public void restartVMInErrorState() throws ReflectiveOperationException { + verifyErrorRestartWorkCompletes(true); + } + + @Test + public void restartVMInErrorStateWithHADisabled() throws ReflectiveOperationException { + verifyErrorRestartWorkCompletes(false); + } + + private void verifyErrorRestartWorkCompletes(boolean haEnabled) throws ReflectiveOperationException { + VMInstanceVO vm = createVmForProcessing(VirtualMachine.State.Error); + Mockito.when(_instanceDao.findById(1L)).thenReturn(vm); + // Neither retry path should be reached for a VM already in Error. + Mockito.lenient().when(_haDao.listRunningHaWorkForVm(1L)).thenReturn(Arrays.asList(createWorkForProcessing(WorkType.HA, VirtualMachine.State.Stopped))); + ConfigKey originalHaEnabled = HighAvailabilityManagerImpl.VmHaEnabled; + ConfigKey haEnabledConfig = Mockito.mock(ConfigKey.class); + Mockito.lenient().when(haEnabledConfig.valueIn(Mockito.anyLong())).thenReturn(haEnabled); + HighAvailabilityManagerImpl.VmHaEnabled = haEnabledConfig; + try { + for (VirtualMachine.State previousState : Arrays.asList(VirtualMachine.State.Stopped, VirtualMachine.State.Error)) { + HaWorkVO work = createWorkForProcessing(WorkType.HA, previousState); + + processWorkMethod.invoke(highAvailabilityManager, work); + + assertEquals(Step.Done, work.getStep()); + assertEquals(0, work.getTimesTried()); + Mockito.verify(_haDao).update(2L, work); + } + Mockito.verifyNoMoreInteractions(_haDao); + Mockito.verifyNoInteractions(_itMgr, _hostDao, _alertMgr, userVmManager, volumeMgr, haEnabledConfig); + } finally { + HighAvailabilityManagerImpl.VmHaEnabled = originalHaEnabled; + } + } + + private VMInstanceVO createVmForProcessing(VirtualMachine.State state) { + VMInstanceVO vm = new VMInstanceVO(1L, 1L, "vm-1", "i-1-VM", VirtualMachine.Type.User, + 1L, HypervisorType.KVM, 1L, 1L, 1L, 1L, true); + vm.setState(state); + vm.setDataCenterId(1L); + vm.setHostId(1L); + vm.setUuid("vm-1"); + return vm; + } + + private HaWorkVO createWorkForProcessing(WorkType type, VirtualMachine.State previousState) throws ReflectiveOperationException { + HaWorkVO work = new HaWorkVO(1L, VirtualMachine.Type.User, type, Step.Scheduled, + 1L, previousState, 0, 0L, null); + Field id = HaWorkVO.class.getDeclaredField("id"); + id.setAccessible(true); + id.set(work, 2L); + return work; + } + + @Test + public void restartVMNotInErrorStateWithHADisabledStillRetries() throws ReflectiveOperationException { + VMInstanceVO vm = createVmForProcessing(VirtualMachine.State.Running); + Mockito.when(_instanceDao.findById(1L)).thenReturn(vm); + ConfigKey originalHaEnabled = HighAvailabilityManagerImpl.VmHaEnabled; + ConfigKey haEnabledConfig = Mockito.mock(ConfigKey.class); + Mockito.when(haEnabledConfig.valueIn(Mockito.anyLong())).thenReturn(false); + HighAvailabilityManagerImpl.VmHaEnabled = haEnabledConfig; + try { + HaWorkVO work = createWorkForProcessing(WorkType.HA, VirtualMachine.State.Running); + + processWorkMethod.invoke(highAvailabilityManager, work); + + assertEquals(Step.Scheduled, work.getStep()); + assertEquals(1, work.getTimesTried()); + Mockito.verify(_haDao).update(2L, work); + Mockito.verifyNoInteractions(_itMgr, _hostDao, _alertMgr, userVmManager, volumeMgr); + } finally { + HighAvailabilityManagerImpl.VmHaEnabled = originalHaEnabled; + } + } + + @Test + public void errorVMStillProcessesStopAndDestroyWork() throws Exception { + VMInstanceVO vm = createVmForProcessing(VirtualMachine.State.Error); + Mockito.when(_instanceDao.findById(1L)).thenReturn(vm); + Mockito.when(_itMgr.findById(1L)).thenReturn(vm); + ConfigKey originalHaEnabled = HighAvailabilityManagerImpl.VmHaEnabled; + ConfigKey haEnabledConfig = Mockito.mock(ConfigKey.class); + Mockito.when(haEnabledConfig.valueIn(Mockito.anyLong())).thenReturn(true); + HighAvailabilityManagerImpl.VmHaEnabled = haEnabledConfig; + try { + for (WorkType type : Arrays.asList(WorkType.Stop, WorkType.CheckStop, WorkType.ForceStop, WorkType.Destroy)) { + HaWorkVO work = createWorkForProcessing(type, VirtualMachine.State.Error); + + processWorkMethod.invoke(highAvailabilityManager, work); + + assertEquals(Step.Done, work.getStep()); + assertEquals(0, work.getTimesTried()); + Mockito.verify(_haDao).update(2L, work); + } + Mockito.verify(_itMgr, Mockito.times(2)).advanceStop("vm-1", false); + Mockito.verify(_itMgr).advanceStop("vm-1", true); + Mockito.verify(_itMgr).destroy("vm-1", false); + } finally { + HighAvailabilityManagerImpl.VmHaEnabled = originalHaEnabled; + } + } + + @Test + public void restartVMNotInErrorStateContinuesProcessing() throws ReflectiveOperationException { + VMInstanceVO vm = createVmForProcessing(VirtualMachine.State.Running); + Mockito.when(_instanceDao.findById(1L)).thenReturn(vm); + Mockito.when(_itMgr.findById(1L)).thenReturn(vm); + Mockito.when(_hostDao.findById(1L)).thenReturn(hostVO); + Investigator investigator = Mockito.mock(CheckOnAgentInvestigator.class); + Mockito.when(investigator.isAgentAlive(hostVO)).thenReturn(Status.Up); + highAvailabilityManager.setInvestigators(Arrays.asList(investigator)); + ConfigKey originalHaEnabled = HighAvailabilityManagerImpl.VmHaEnabled; + ConfigKey haEnabledConfig = Mockito.mock(ConfigKey.class); + Mockito.when(haEnabledConfig.valueIn(Mockito.anyLong())).thenReturn(true); + HighAvailabilityManagerImpl.VmHaEnabled = haEnabledConfig; + try { + HaWorkVO work = createWorkForProcessing(WorkType.HA, VirtualMachine.State.Running); + work.setStep(Step.Investigating); + work.setReasonType(HighAvailabilityManager.ReasonType.HostMaintenance); + + processWorkMethod.invoke(highAvailabilityManager, work); + + assertEquals(Step.Done, work.getStep()); + Mockito.verify(investigator).isAgentAlive(hostVO); + Mockito.verify(_haDao).update(2L, work); + Mockito.verify(_itMgr).findById(1L); + Mockito.verifyNoMoreInteractions(_itMgr); + } finally { + HighAvailabilityManagerImpl.VmHaEnabled = originalHaEnabled; + } + } + + @Test + public void vmEnteringErrorAfterDispatchDoesNotRestart() throws ReflectiveOperationException { + verifyVmEnteringErrorAfterDispatchCompletes(false); + } + + @Test + public void vmEnteringErrorAfterDispatchWithRunningHaWorkDoesNotRetry() throws ReflectiveOperationException { + verifyVmEnteringErrorAfterDispatchCompletes(true); + } + + private void verifyVmEnteringErrorAfterDispatchCompletes(boolean runningHaWork) throws ReflectiveOperationException { + VMInstanceVO scheduledVm = createVmForProcessing(VirtualMachine.State.Stopped); + Mockito.when(_instanceDao.findById(1L)).thenReturn(scheduledVm); + VMInstanceVO currentVm = createVmForProcessing(VirtualMachine.State.Error); + Mockito.when(_itMgr.findById(1L)).thenReturn(currentVm); + if (runningHaWork) { + Mockito.lenient().when(_haDao.listRunningHaWorkForVm(1L)).thenReturn(Arrays.asList(createWorkForProcessing(WorkType.HA, VirtualMachine.State.Stopped))); + } + ConfigKey originalHaEnabled = HighAvailabilityManagerImpl.VmHaEnabled; + ConfigKey haEnabledConfig = Mockito.mock(ConfigKey.class); + Mockito.when(haEnabledConfig.valueIn(Mockito.anyLong())).thenReturn(true); + HighAvailabilityManagerImpl.VmHaEnabled = haEnabledConfig; + try { + for (Step step : Arrays.asList(Step.Scheduled, Step.Investigating)) { + HaWorkVO work = createWorkForProcessing(WorkType.HA, VirtualMachine.State.Stopped); + work.setStep(step); + work.setReasonType(HighAvailabilityManager.ReasonType.HostMaintenance); + + processWorkMethod.invoke(highAvailabilityManager, work); + + assertEquals(Step.Done, work.getStep()); + assertEquals(0, work.getTimesTried()); + Mockito.verify(_haDao).update(2L, work); + } + Mockito.verify(_itMgr, Mockito.times(2)).findById(1L); + Mockito.verifyNoMoreInteractions(_itMgr); + Mockito.verifyNoInteractions(_hostDao, _alertMgr, userVmManager, volumeMgr); + Mockito.verifyNoMoreInteractions(_haDao); + } finally { + HighAvailabilityManagerImpl.VmHaEnabled = originalHaEnabled; + } + } + @Test public void scheduleStop() { VMInstanceVO vm = Mockito.mock(VMInstanceVO.class);