Skip to content

server: skip HA restart for VMs in Error state #13811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Dogface2k wants to merge 1 commit into
base: 4.20
Choose a base branch
from
+207 −3
Open

server: skip HA restart for VMs in Error state #13811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down Expand Up @@ -571,6 +576,13 @@ private void startVm(VirtualMachine vm, Map<VirtualMachineProfile.Param, Object>

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<HaWorkVO> 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 = [");
Expand All @@ -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;
Expand Down Expand Up @@ -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()));
Expand Down
189 changes: 189 additions & 0 deletions server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Boolean> originalHaEnabled = HighAvailabilityManagerImpl.VmHaEnabled;
ConfigKey<Boolean> 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<Boolean> originalHaEnabled = HighAvailabilityManagerImpl.VmHaEnabled;
ConfigKey<Boolean> 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<Boolean> originalHaEnabled = HighAvailabilityManagerImpl.VmHaEnabled;
ConfigKey<Boolean> 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<Boolean> originalHaEnabled = HighAvailabilityManagerImpl.VmHaEnabled;
ConfigKey<Boolean> 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<Boolean> originalHaEnabled = HighAvailabilityManagerImpl.VmHaEnabled;
ConfigKey<Boolean> 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);
Expand Down
Loading