From 2a2374c3cf4469922038724fefaf0cb0ded224a8 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Wed, 26 Jun 2019 13:38:05 +0530 Subject: [PATCH 01/19] Service layer changes for new way of tracking maintanence progress --- .../com/cloud/resource/ResourceState.java | 18 ++- .../admin/host/PrepareForMaintenanceCmd.java | 22 ++-- .../com/cloud/resource/ResourceManager.java | 5 - .../cloud/resource/ResourceManagerImpl.java | 97 ++++++++------- .../resource/ResourceManagerImplTest.java | 110 +++++------------- 5 files changed, 111 insertions(+), 141 deletions(-) diff --git a/api/src/main/java/com/cloud/resource/ResourceState.java b/api/src/main/java/com/cloud/resource/ResourceState.java index d952afa0b7dd..fbb215dd5f3c 100644 --- a/api/src/main/java/com/cloud/resource/ResourceState.java +++ b/api/src/main/java/com/cloud/resource/ResourceState.java @@ -22,7 +22,14 @@ import com.cloud.utils.fsm.StateMachine; public enum ResourceState { - Creating, Enabled, Disabled, PrepareForMaintenance, ErrorInMaintenance, Maintenance, Error; + Creating, + Enabled, + Disabled, + PrepareForMaintenance, + ErrorInMaintenance, + Maintenance, + Error, + PrepareForMaintenanceErrorsPresent; public enum Event { InternalCreated("Resource is created"), @@ -33,6 +40,7 @@ public enum Event { InternalEnterMaintenance("Resource enters maintenance"), UpdatePassword("Admin updates password of host"), UnableToMigrate("Management server migrates VM failed"), + UnableToMaintain("Managament server has exhausted all legal operations and attempts to put into maintenance has failed"), Error("An internal error happened"), DeleteHost("Admin delete a host"), @@ -99,11 +107,17 @@ public static String[] toString(ResourceState... states) { s_fsm.addTransition(ResourceState.Disabled, Event.InternalCreated, ResourceState.Disabled); s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.InternalEnterMaintenance, ResourceState.Maintenance); s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.AdminCancelMaintenance, ResourceState.Enabled); - s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.UnableToMigrate, ResourceState.ErrorInMaintenance); + s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.UnableToMigrate, ResourceState.PrepareForMaintenanceErrorsPresent); + s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.UnableToMaintain, ResourceState.ErrorInMaintenance); s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.InternalCreated, ResourceState.PrepareForMaintenance); s_fsm.addTransition(ResourceState.Maintenance, Event.AdminCancelMaintenance, ResourceState.Enabled); s_fsm.addTransition(ResourceState.Maintenance, Event.InternalCreated, ResourceState.Maintenance); s_fsm.addTransition(ResourceState.Maintenance, Event.DeleteHost, ResourceState.Disabled); + s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.InternalEnterMaintenance, ResourceState.Maintenance); + s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.AdminCancelMaintenance, ResourceState.Enabled); + s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.UnableToMigrate, ResourceState.PrepareForMaintenanceErrorsPresent); + s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.UnableToMaintain, ResourceState.ErrorInMaintenance); + s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.InternalCreated, ResourceState.PrepareForMaintenanceErrorsPresent); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.InternalCreated, ResourceState.ErrorInMaintenance); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.Disable, ResourceState.Disabled); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.DeleteHost, ResourceState.Disabled); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/host/PrepareForMaintenanceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/PrepareForMaintenanceCmd.java index e49aabc49d4b..f60812821d67 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/host/PrepareForMaintenanceCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/PrepareForMaintenanceCmd.java @@ -16,8 +16,6 @@ // under the License. package org.apache.cloudstack.api.command.admin.host; -import org.apache.log4j.Logger; - import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiCommandJobType; import org.apache.cloudstack.api.ApiConstants; @@ -27,10 +25,12 @@ import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.HostResponse; import org.apache.cloudstack.context.CallContext; +import org.apache.log4j.Logger; import com.cloud.event.EventTypes; import com.cloud.host.Host; import com.cloud.user.Account; +import com.cloud.utils.exception.CloudRuntimeException; @APICommand(name = "prepareHostForMaintenance", description = "Prepares a host for maintenance.", responseObject = HostResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) @@ -99,13 +99,17 @@ public Long getInstanceId() { @Override public void execute() { - Host result = _resourceService.maintain(this); - if (result != null) { - HostResponse response = _responseGenerator.createHostResponse(result); - response.setResponseName("host"); - this.setResponseObject(response); - } else { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to prepare host for maintenance"); + try { + Host result = _resourceService.maintain(this); + if (result != null) { + HostResponse response = _responseGenerator.createHostResponse(result); + response.setResponseName("host"); + this.setResponseObject(response); + } else { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to prepare host for maintenance"); + } + } catch (CloudRuntimeException exception) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to prepare host for maintenance due to: " + exception.getMessage()); } } } diff --git a/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java b/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java index b66f7923b4da..e7c064fbb923 100755 --- a/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java +++ b/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java @@ -47,11 +47,6 @@ */ public interface ResourceManager extends ResourceService, Configurable { - ConfigKey HostMaintenanceRetries = new ConfigKey<>("Advanced", Integer.class, - "host.maintenance.retries","20", - "Number of retries when preparing a host into Maintenance Mode is faulty before failing", - true, ConfigKey.Scope.Cluster); - ConfigKey KvmSshToAgentEnabled = new ConfigKey<>("Advanced", Boolean.class, "kvm.ssh.to.agent","true", "Number of retries when preparing a host into Maintenance Mode is faulty before failing", diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index d07a4383d015..a4264e1b3bb8 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -26,7 +26,6 @@ import java.util.List; import java.util.Map; import java.util.Random; -import java.util.concurrent.ConcurrentHashMap; import javax.inject.Inject; import javax.naming.ConfigurationException; @@ -274,8 +273,6 @@ public void setDiscoverers(final List discoverers) { private SearchBuilder _gpuAvailability; - private Map retryHostMaintenance = new ConcurrentHashMap<>(); - private void insertListener(final Integer event, final ResourceListener listener) { List lst = _lifeCycleListeners.get(event); if (lst == null) { @@ -1228,7 +1225,6 @@ private boolean doMaintain(final long hostId) { ActionEventUtils.onStartedActionEvent(CallContext.current().getCallingUserId(), CallContext.current().getCallingAccountId(), EventTypes.EVENT_MAINTENANCE_PREPARE, "starting maintenance for host " + hostId, true, 0); _agentMgr.pullAgentToMaintenance(hostId); - setHostMaintenanceRetries(host); /* TODO: move below to listener */ if (host.getType() == Host.Type.Routing) { @@ -1256,16 +1252,6 @@ private boolean doMaintain(final long hostId) { return true; } - /** - * Set retries for transiting the host into Maintenance - */ - protected void setHostMaintenanceRetries(HostVO host) { - Integer retries = HostMaintenanceRetries.valueIn(host.getClusterId()); - retryHostMaintenance.put(host.getId(), retries); - s_logger.debug(String.format("Setting the host %s (%s) retries for Maintenance mode: %s", - host.getId(), host.getName(), retries)); - } - @Override public boolean maintain(final long hostId) throws AgentUnavailableException { final Boolean result = propagateResourceEvent(hostId, ResourceState.Event.AdminAskMaintenace); @@ -1294,6 +1280,18 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) { throw new InvalidParameterValueException("There are active VMs using the host's local storage pool. Please stop all VMs on this host that use local storage."); } + if (_vmDao.findByHostInStates(hostId, State.Migrating).size() > 0) { // Incoming Migrations + throw new CloudRuntimeException("Host contains VMs migrating. Please wait for them to complete before putting to maintenance."); + } + + if (_vmDao.findByHostInStates(hostId, State.Starting).size() > 0) { + throw new CloudRuntimeException("Host contains VMs in starting state. Please wait for them to complete before putting to maintenance."); + } + + if (_vmDao.findByHostInStates(hostId, State.Error, State.Unknown, State.Shutdowned).size() > 0) { + throw new CloudRuntimeException("Host contains VMs in error/unknown/shutdown state. Please fix errors to proceed."); + } + try { processResourceEvent(ResourceListener.EVENT_PREPARE_MAINTENANCE_BEFORE, hostId); if (maintain(hostId)) { @@ -1340,7 +1338,7 @@ protected boolean setHostIntoErrorInMaintenance(HostVO host, List s_logger.debug("Unable to migrate " + failedMigrations.size() + " VM(s) from host " + host.getUuid()); _haMgr.cancelScheduledMigrations(host); configureVncAccessForKVMHostFailedMigrations(host, failedMigrations); - resourceStateTransitTo(host, ResourceState.Event.UnableToMigrate, _nodeId); + resourceStateTransitTo(host, ResourceState.Event.UnableToMaintain, _nodeId); return false; } @@ -1356,31 +1354,47 @@ protected boolean setHostIntoMaintenance(HostVO host) throws NoTransitionExcepti return true; } + protected boolean setHostIntoPrepareForMaintenanceWithErrors(HostVO host) throws NoTransitionException { + s_logger.debug("Host " + host.getUuid() + " entering in PrepareForMaintainenceWithErrors state"); + resourceStateTransitTo(host, ResourceState.Event.UnableToMigrate, _nodeId); + return true; + } + /** * Return true if host goes into Maintenance mode, only when: * - No Running, Migrating or Failed migrations (host_id = last_host_id) for the host */ - protected boolean isHostInMaintenance(HostVO host, List runningVms, List migratingVms, List failedMigrations) throws NoTransitionException { - if (CollectionUtils.isEmpty(runningVms) && CollectionUtils.isEmpty(migratingVms)) { - return CollectionUtils.isEmpty(failedMigrations) ? - setHostIntoMaintenance(host) : - setHostIntoErrorInMaintenance(host, failedMigrations); - } else if (retryHostMaintenance.containsKey(host.getId())) { - Integer retriesLeft = retryHostMaintenance.get(host.getId()); - if (retriesLeft != null) { - if (retriesLeft <= 0) { - retryHostMaintenance.remove(host.getId()); - s_logger.debug(String.format("No retries left while preparing KVM host %s (%s) for Maintenance, " + - "please investigate this connection.", - host.getId(), host.getName())); - return setHostIntoErrorInMaintenance(host, failedMigrations); - } - retriesLeft--; - retryHostMaintenance.put(host.getId(), retriesLeft); - s_logger.debug(String.format("Retries left preparing KVM host %s (%s) for Maintenance: %s", - host.getId(), host.getName(), retriesLeft)); + protected boolean attemptMaintain(HostVO host) throws NoTransitionException { + final long hostId = host.getId(); + + if (CollectionUtils.isEmpty(_vmDao.findByHostInStates(hostId, State.Migrating, State.Running, State.Starting, State.Stopping, State.Error, State.Unknown))) { + return setHostIntoMaintenance(host); + } + + final List allVmsOnHost = _vmDao.listByHostId(hostId); + final List migratingVms = _vmDao.listVmsMigratingFromHost(hostId); + final List failedMigrations = _vmDao.listNonMigratingVmsByHostEqualsLastHost(hostId); + boolean hasPendingWorkForVMs = false; + for (VMInstanceVO vmInstanceVO : allVmsOnHost) { + if (_haMgr.hasPendingHaWork(vmInstanceVO.getId())) { + hasPendingWorkForVMs = true; + break; + } + } + + if (!hasPendingWorkForVMs) { + if ((CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Running)) && CollectionUtils.isEmpty(migratingVms)) || + (CollectionUtils.isEmpty(_vmDao.findByHostInStates(hostId, State.Running)) && + CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Unknown, State.Error, State.Shutdowned)))) { + return setHostIntoErrorInMaintenance(host, failedMigrations); } } + if (hasPendingWorkForVMs && + (CollectionUtils.isNotEmpty(failedMigrations) || + CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Unknown, State.Error, State.Shutdowned))) && + (CollectionUtils.isNotEmpty(migratingVms) || CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Stopping)))) { + return setHostIntoPrepareForMaintenanceWithErrors(host); + } return false; } @@ -1392,11 +1406,7 @@ public boolean checkAndMaintain(final long hostId) { try { if (host.getType() != Host.Type.Storage) { - final List vos = _vmDao.listByHostId(hostId); - final List vosMigrating = _vmDao.listVmsMigratingFromHost(hostId); - final List failedVmMigrations = _vmDao.listNonMigratingVmsByHostEqualsLastHost(hostId); - - hostInMaintenance = isHostInMaintenance(host, vos, vosMigrating, failedVmMigrations); + hostInMaintenance = attemptMaintain(host); } } catch (final NoTransitionException e) { s_logger.debug("Cannot transmit host " + host.getId() + "to Maintenance state", e); @@ -2327,7 +2337,9 @@ private boolean doCancelMaintenance(final long hostId) { * TODO: think twice about returning true or throwing out exception, I * really prefer to exception that always exposes bugs */ - if (host.getResourceState() != ResourceState.PrepareForMaintenance && host.getResourceState() != ResourceState.Maintenance && + if (host.getResourceState() != ResourceState.PrepareForMaintenance && + host.getResourceState() != ResourceState.PrepareForMaintenanceErrorsPresent && + host.getResourceState() != ResourceState.Maintenance && host.getResourceState() != ResourceState.ErrorInMaintenance) { throw new CloudRuntimeException("Cannot perform cancelMaintenance when resource state is " + host.getResourceState() + ", hostId = " + hostId); } @@ -2349,7 +2361,6 @@ private boolean doCancelMaintenance(final long hostId) { try { resourceStateTransitTo(host, ResourceState.Event.AdminCancelMaintenance, _nodeId); _agentMgr.pullAgentOutMaintenance(hostId); - retryHostMaintenance.remove(hostId); } catch (final NoTransitionException e) { s_logger.debug("Cannot transmit host " + host.getId() + "to Enabled state", e); return false; @@ -2561,7 +2572,7 @@ public Boolean propagateResourceEvent(final long agentId, final ResourceState.Ev return null; } - s_logger.debug("Propagating resource request event:" + event.toString() + " to agent:" + agentId); + s_logger.debug("Propagating resource request event:" + event.toString() + " to agent:" + agentId); final Command[] cmds = new Command[1]; cmds[0] = new PropagateResourceEventCommand(agentId, event); @@ -2981,6 +2992,6 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {HostMaintenanceRetries}; + return new ConfigKey[] {KvmSshToAgentEnabled}; } } diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index 7d1a0fe0163e..d34a0420e192 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -17,6 +17,34 @@ package com.cloud.resource; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.BDDMockito; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.mockito.Spy; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; + import com.cloud.agent.AgentManager; import com.cloud.agent.api.GetVncPortAnswer; import com.cloud.agent.api.GetVncPortCommand; @@ -31,42 +59,12 @@ import com.cloud.storage.StorageManager; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.fsm.NoTransitionException; import com.cloud.utils.ssh.SSHCmdHelper; import com.cloud.utils.ssh.SshException; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.UserVmDetailsDao; import com.cloud.vm.dao.VMInstanceDao; import com.trilead.ssh2.Connection; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.BDDMockito; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import org.mockito.Spy; -import org.powermock.api.mockito.PowerMockito; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - -import static com.cloud.resource.ResourceState.Event.InternalEnterMaintenance; -import static com.cloud.resource.ResourceState.Event.UnableToMigrate; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyBoolean; -import static org.mockito.Matchers.anyLong; -import static org.mockito.Matchers.anyString; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; @RunWith(PowerMockRunner.class) @PrepareForTest({ActionEventUtils.class, ResourceManagerImpl.class, SSHCmdHelper.class}) @@ -169,41 +167,6 @@ public void setup() throws Exception { when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("true"); } - @Test - public void testCheckAndMaintainEnterMaintenanceMode() throws NoTransitionException { - boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); - verify(resourceManager).isHostInMaintenance(host, new ArrayList<>(), new ArrayList<>(), new ArrayList<>()); - verify(resourceManager).setHostIntoMaintenance(host); - verify(resourceManager).resourceStateTransitTo(eq(host), eq(InternalEnterMaintenance), anyLong()); - Assert.assertTrue(enterMaintenanceMode); - } - - @Test - public void testCheckAndMaintainErrorInMaintenanceRunningVms() throws NoTransitionException { - when(vmInstanceDao.listByHostId(hostId)).thenReturn(Arrays.asList(vm1, vm2)); - boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); - verify(resourceManager).isHostInMaintenance(host, Arrays.asList(vm1, vm2), new ArrayList<>(), new ArrayList<>()); - Assert.assertFalse(enterMaintenanceMode); - } - - @Test - public void testCheckAndMaintainErrorInMaintenanceMigratingVms() throws NoTransitionException { - when(vmInstanceDao.listVmsMigratingFromHost(hostId)).thenReturn(Arrays.asList(vm1, vm2)); - boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); - verify(resourceManager).isHostInMaintenance(host, new ArrayList<>(), Arrays.asList(vm1, vm2), new ArrayList<>()); - Assert.assertFalse(enterMaintenanceMode); - } - - @Test - public void testCheckAndMaintainErrorInMaintenanceFailedMigrations() throws NoTransitionException { - when(vmInstanceDao.listNonMigratingVmsByHostEqualsLastHost(hostId)).thenReturn(Arrays.asList(vm1, vm2)); - boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); - verify(resourceManager).isHostInMaintenance(host, new ArrayList<>(), new ArrayList<>(), Arrays.asList(vm1, vm2)); - verify(resourceManager).setHostIntoErrorInMaintenance(host, Arrays.asList(vm1, vm2)); - verify(resourceManager).resourceStateTransitTo(eq(host), eq(UnableToMigrate), anyLong()); - Assert.assertFalse(enterMaintenanceMode); - } - @Test public void testConfigureVncAccessForKVMHostFailedMigrations() { when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); @@ -219,23 +182,6 @@ public void testConfigureVncAccessForKVMHostFailedMigrations() { verify(agentManager).pullAgentToMaintenance(hostId); } - @Test - public void testCheckAndMaintainErrorInMaintenanceRetries() throws NoTransitionException { - resourceManager.setHostMaintenanceRetries(host); - - List failedMigrations = Arrays.asList(vm1, vm2); - when(vmInstanceDao.listByHostId(host.getId())).thenReturn(failedMigrations); - when(vmInstanceDao.listNonMigratingVmsByHostEqualsLastHost(host.getId())).thenReturn(failedMigrations); - - Integer retries = ResourceManager.HostMaintenanceRetries.valueIn(host.getClusterId()); - for (int i = 0; i <= retries; i++) { - resourceManager.checkAndMaintain(host.getId()); - } - - verify(resourceManager, times(retries + 1)).isHostInMaintenance(host, failedMigrations, new ArrayList<>(), failedMigrations); - verify(resourceManager).setHostIntoErrorInMaintenance(host, failedMigrations); - } - @Test(expected = CloudRuntimeException.class) public void testGetHostCredentialsMissingParameter() { when(host.getDetail("password")).thenReturn(null); From 766373eaa04f3efce110d53a89aa14c493eab477 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Thu, 27 Jun 2019 08:47:34 +0530 Subject: [PATCH 02/19] Fixes after offline code review --- .../com/cloud/resource/ResourceState.java | 8 ++- .../com/cloud/ha/HighAvailabilityManager.java | 1 + .../cloud/ha/HighAvailabilityManagerImpl.java | 36 +++++++++-- .../com/cloud/ha/dao/HighAvailabilityDao.java | 2 + .../cloud/ha/dao/HighAvailabilityDaoImpl.java | 17 ++++++ .../cloud/resource/ResourceManagerImpl.java | 61 +++++++++++-------- 6 files changed, 93 insertions(+), 32 deletions(-) diff --git a/api/src/main/java/com/cloud/resource/ResourceState.java b/api/src/main/java/com/cloud/resource/ResourceState.java index fbb215dd5f3c..f9dddf2c4acd 100644 --- a/api/src/main/java/com/cloud/resource/ResourceState.java +++ b/api/src/main/java/com/cloud/resource/ResourceState.java @@ -35,12 +35,13 @@ public enum Event { InternalCreated("Resource is created"), Enable("Admin enables"), Disable("Admin disables"), - AdminAskMaintenace("Admin asks to enter maintenance"), + AdminAskMaintenance("Admin asks to enter maintenance"), AdminCancelMaintenance("Admin asks to cancel maintenance"), InternalEnterMaintenance("Resource enters maintenance"), UpdatePassword("Admin updates password of host"), UnableToMigrate("Management server migrates VM failed"), - UnableToMaintain("Managament server has exhausted all legal operations and attempts to put into maintenance has failed"), + UnableToMaintain("Management server has exhausted all legal operations and attempts to put into maintenance has failed"), + ErrorsCorrected("Errors were corrected on a resource attempting to enter maintenance but encountered errors"), Error("An internal error happened"), DeleteHost("Admin delete a host"), @@ -100,7 +101,7 @@ public static String[] toString(ResourceState... states) { s_fsm.addTransition(ResourceState.Enabled, Event.Enable, ResourceState.Enabled); s_fsm.addTransition(ResourceState.Enabled, Event.InternalCreated, ResourceState.Enabled); s_fsm.addTransition(ResourceState.Enabled, Event.Disable, ResourceState.Disabled); - s_fsm.addTransition(ResourceState.Enabled, Event.AdminAskMaintenace, ResourceState.PrepareForMaintenance); + s_fsm.addTransition(ResourceState.Enabled, Event.AdminAskMaintenance, ResourceState.PrepareForMaintenance); s_fsm.addTransition(ResourceState.Enabled, Event.InternalEnterMaintenance, ResourceState.Maintenance); s_fsm.addTransition(ResourceState.Disabled, Event.Enable, ResourceState.Enabled); s_fsm.addTransition(ResourceState.Disabled, Event.Disable, ResourceState.Disabled); @@ -117,6 +118,7 @@ public static String[] toString(ResourceState... states) { s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.AdminCancelMaintenance, ResourceState.Enabled); s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.UnableToMigrate, ResourceState.PrepareForMaintenanceErrorsPresent); s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.UnableToMaintain, ResourceState.ErrorInMaintenance); + s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.ErrorsCorrected, ResourceState.PrepareForMaintenance); s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.InternalCreated, ResourceState.PrepareForMaintenanceErrorsPresent); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.InternalCreated, ResourceState.ErrorInMaintenance); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.Disable, ResourceState.Disabled); diff --git a/engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java b/engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java index ecfb6f65701f..8c63b3a67771 100644 --- a/engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java +++ b/engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java @@ -102,6 +102,7 @@ enum Step { boolean hasPendingHaWork(long vmId); + boolean hasPendingMigrationsWork(long vmId); /** * @return */ diff --git a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java index 49211f5eba37..fd6e438a568f 100644 --- a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java +++ b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java @@ -28,17 +28,19 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import org.apache.log4j.Logger; -import org.apache.log4j.NDC; import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.managed.context.ManagedContext; import org.apache.cloudstack.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.management.ManagementServerHost; +import org.apache.log4j.Logger; +import org.apache.log4j.NDC; import com.cloud.agent.AgentManager; import com.cloud.alert.AlertManager; import com.cloud.cluster.ClusterManagerListener; -import org.apache.cloudstack.management.ManagementServerHost; import com.cloud.configuration.Config; import com.cloud.dc.ClusterDetailsDao; import com.cloud.dc.DataCenterVO; @@ -101,9 +103,14 @@ * ha.retry.wait | time to wait before retrying the work item | seconds | 120 || || stop.retry.wait | time to wait * before retrying the stop | seconds | 120 || * } **/ -public class HighAvailabilityManagerImpl extends ManagerBase implements HighAvailabilityManager, ClusterManagerListener { +public class HighAvailabilityManagerImpl extends ManagerBase implements HighAvailabilityManager, ClusterManagerListener, Configurable { protected static final Logger s_logger = Logger.getLogger(HighAvailabilityManagerImpl.class); + private ConfigKey MaxRetries = new ConfigKey<>("Advanced", Integer.class, + "max.retries","5", + "Total number of attempts for trying migration of a VM.", + true, ConfigKey.Scope.Cluster); + WorkerThread[] _workers; boolean _stopped; long _timeToSleep; @@ -908,6 +915,16 @@ public boolean stop() { return true; } + @Override + public String getConfigComponentName() { + return HighAvailabilityManagerImpl.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[] {MaxRetries}; + } + protected class CleanupTask extends ManagedContextRunnable { @Override protected void runInContext() { @@ -1004,4 +1021,15 @@ public boolean hasPendingHaWork(long vmId) { List haWorks = _haDao.listPendingHaWorkForVm(vmId); return haWorks.size() > 0; } + + @Override + public boolean hasPendingMigrationsWork(long vmId) { + List haWorks = _haDao.listPendingMigrationsForVm(vmId); + for (HaWorkVO work : haWorks) { + if (work.getTimesTried() < _maxRetries) { + return true; + } + } + return false; + } } diff --git a/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDao.java b/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDao.java index 85135bb97947..e8a3e17f8052 100644 --- a/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDao.java +++ b/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDao.java @@ -83,4 +83,6 @@ public interface HighAvailabilityDao extends GenericDao { List listRunningHaWorkForVm(long vmId); List listPendingHaWorkForVm(long vmId); + + List listPendingMigrationsForVm(long vmId); } diff --git a/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDaoImpl.java b/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDaoImpl.java index 3d11eb04f537..a206d2d82d85 100644 --- a/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDaoImpl.java +++ b/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDaoImpl.java @@ -48,6 +48,7 @@ public class HighAvailabilityDaoImpl extends GenericDaoBase impl private final SearchBuilder FutureHaWorkSearch; private final SearchBuilder RunningHaWorkSearch; private final SearchBuilder PendingHaWorkSearch; + private final SearchBuilder MigratingWorkSearch; protected HighAvailabilityDaoImpl() { super(); @@ -112,6 +113,12 @@ protected HighAvailabilityDaoImpl() { PendingHaWorkSearch.and("type", PendingHaWorkSearch.entity().getType(), Op.EQ); PendingHaWorkSearch.and("step", PendingHaWorkSearch.entity().getStep(), Op.NIN); PendingHaWorkSearch.done(); + + MigratingWorkSearch = createSearchBuilder(); + MigratingWorkSearch.and("instance", MigratingWorkSearch.entity().getInstanceId(), Op.EQ); + MigratingWorkSearch.and("type", MigratingWorkSearch.entity().getType(), Op.EQ); + MigratingWorkSearch.and("step", MigratingWorkSearch.entity().getStep(), Op.NIN); + MigratingWorkSearch.done(); } @Override @@ -124,6 +131,16 @@ public List listPendingHaWorkForVm(long vmId) { return search(sc, null); } + @Override + public List listPendingMigrationsForVm(long vmId) { + SearchCriteria sc = PendingHaWorkSearch.create(); + sc.setParameters("instance", vmId); + sc.setParameters("type", WorkType.Migration); + sc.setParameters("step", Step.Done, Step.Error, Step.Cancelled); + + return search(sc, null); + } + @Override public List listRunningHaWorkForVm(long vmId) { SearchCriteria sc = RunningHaWorkSearch.create(); diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index a4264e1b3bb8..7de4d9728bfe 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1216,7 +1216,7 @@ private boolean doMaintain(final long hostId) { } try { - resourceStateTransitTo(host, ResourceState.Event.AdminAskMaintenace, _nodeId); + resourceStateTransitTo(host, ResourceState.Event.AdminAskMaintenance, _nodeId); } catch (final NoTransitionException e) { final String err = "Cannot transmit resource state of host " + host.getId() + " to " + ResourceState.Maintenance; s_logger.debug(err, e); @@ -1254,7 +1254,7 @@ private boolean doMaintain(final long hostId) { @Override public boolean maintain(final long hostId) throws AgentUnavailableException { - final Boolean result = propagateResourceEvent(hostId, ResourceState.Event.AdminAskMaintenace); + final Boolean result = propagateResourceEvent(hostId, ResourceState.Event.AdminAskMaintenance); if (result != null) { return result; } @@ -1334,10 +1334,10 @@ protected void configureVncAccessForKVMHostFailedMigrations(HostVO host, List failedMigrations) throws NoTransitionException { - s_logger.debug("Unable to migrate " + failedMigrations.size() + " VM(s) from host " + host.getUuid()); + protected boolean setHostIntoErrorInMaintenance(HostVO host, List errorVms) throws NoTransitionException { + s_logger.debug("Unable to migrate / fix errors for " + errorVms.size() + " VM(s) from host " + host.getUuid()); _haMgr.cancelScheduledMigrations(host); - configureVncAccessForKVMHostFailedMigrations(host, failedMigrations); + configureVncAccessForKVMHostFailedMigrations(host, errorVms); resourceStateTransitTo(host, ResourceState.Event.UnableToMaintain, _nodeId); return false; } @@ -1354,17 +1354,24 @@ protected boolean setHostIntoMaintenance(HostVO host) throws NoTransitionExcepti return true; } - protected boolean setHostIntoPrepareForMaintenanceWithErrors(HostVO host) throws NoTransitionException { - s_logger.debug("Host " + host.getUuid() + " entering in PrepareForMaintainenceWithErrors state"); + protected boolean setHostIntoPrepareForMaintenanceWithErrors(HostVO host, List errorVms) throws NoTransitionException { + s_logger.debug("Host " + host.getUuid() + " entering in PrepareForMaintenanceWithErrors state"); + configureVncAccessForKVMHostFailedMigrations(host, errorVms); resourceStateTransitTo(host, ResourceState.Event.UnableToMigrate, _nodeId); return true; } + protected boolean setHostIntoPrepareForMaintenanceAfterErrorsFixed(HostVO host) throws NoTransitionException { + s_logger.debug("Host " + host.getUuid() + " entering in PrepareForMaintenance state as any previous corrections have been fixed"); + resourceStateTransitTo(host, ResourceState.Event.ErrorsCorrected, _nodeId); + return true; + } + /** * Return true if host goes into Maintenance mode, only when: * - No Running, Migrating or Failed migrations (host_id = last_host_id) for the host */ - protected boolean attemptMaintain(HostVO host) throws NoTransitionException { + private boolean attemptMaintain(HostVO host) throws NoTransitionException { final long hostId = host.getId(); if (CollectionUtils.isEmpty(_vmDao.findByHostInStates(hostId, State.Migrating, State.Running, State.Starting, State.Stopping, State.Error, State.Unknown))) { @@ -1372,28 +1379,32 @@ protected boolean attemptMaintain(HostVO host) throws NoTransitionException { } final List allVmsOnHost = _vmDao.listByHostId(hostId); - final List migratingVms = _vmDao.listVmsMigratingFromHost(hostId); - final List failedMigrations = _vmDao.listNonMigratingVmsByHostEqualsLastHost(hostId); - boolean hasPendingWorkForVMs = false; + boolean hasPendingMigrationWorks = false; for (VMInstanceVO vmInstanceVO : allVmsOnHost) { - if (_haMgr.hasPendingHaWork(vmInstanceVO.getId())) { - hasPendingWorkForVMs = true; + if (_haMgr.hasPendingMigrationsWork(vmInstanceVO.getId())) { + hasPendingMigrationWorks = true; break; } } - if (!hasPendingWorkForVMs) { - if ((CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Running)) && CollectionUtils.isEmpty(migratingVms)) || - (CollectionUtils.isEmpty(_vmDao.findByHostInStates(hostId, State.Running)) && - CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Unknown, State.Error, State.Shutdowned)))) { - return setHostIntoErrorInMaintenance(host, failedMigrations); - } + final List failedMigrations = new ArrayList<>(_vmDao.listNonMigratingVmsByHostEqualsLastHost(hostId)); + final List errorVms = new ArrayList<>(_vmDao.findByHostInStates(hostId, State.Unknown, State.Error, State.Shutdowned)); + final boolean hasMigratingVms = CollectionUtils.isNotEmpty(_vmDao.listVmsMigratingFromHost(hostId)); + final boolean hasRunningVms = CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Running)); + final boolean hasFailedMigrations = CollectionUtils.isNotEmpty(failedMigrations); + final boolean hasVmsInFailureStates = CollectionUtils.isNotEmpty(errorVms); + errorVms.addAll(failedMigrations); + + if (!hasPendingMigrationWorks && (hasRunningVms || (!hasRunningVms && !hasMigratingVms && hasVmsInFailureStates))) { + return setHostIntoErrorInMaintenance(host, errorVms); + } + + if ((hasVmsInFailureStates || hasFailedMigrations) && (hasPendingMigrationWorks || hasMigratingVms || CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Stopping)))) { + return setHostIntoPrepareForMaintenanceWithErrors(host, errorVms); } - if (hasPendingWorkForVMs && - (CollectionUtils.isNotEmpty(failedMigrations) || - CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Unknown, State.Error, State.Shutdowned))) && - (CollectionUtils.isNotEmpty(migratingVms) || CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Stopping)))) { - return setHostIntoPrepareForMaintenanceWithErrors(host); + + if (host.getResourceState() == ResourceState.PrepareForMaintenanceErrorsPresent) { + return setHostIntoPrepareForMaintenanceAfterErrorsFixed(host); } return false; @@ -2444,7 +2455,7 @@ private boolean cancelMaintenance(final long hostId) { @Override public boolean executeUserRequest(final long hostId, final ResourceState.Event event) throws AgentUnavailableException { - if (event == ResourceState.Event.AdminAskMaintenace) { + if (event == ResourceState.Event.AdminAskMaintenance) { return doMaintain(hostId); } else if (event == ResourceState.Event.AdminCancelMaintenance) { return doCancelMaintenance(hostId); From b7b0f77fd5fd4be2d3f4b3d00aac1bec0278ebbb Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Thu, 4 Jul 2019 14:48:45 +0530 Subject: [PATCH 03/19] Fix marvin tests --- .../cloud/resource/ResourceManagerImpl.java | 6 +-- .../smoke/test_host_maintenance.py | 48 +++++++++++++++---- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 7de4d9728bfe..8f5abdb2a0a6 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1279,9 +1279,9 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) { if (_storageMgr.isLocalStorageActiveOnHost(host.getId())) { throw new InvalidParameterValueException("There are active VMs using the host's local storage pool. Please stop all VMs on this host that use local storage."); } - - if (_vmDao.findByHostInStates(hostId, State.Migrating).size() > 0) { // Incoming Migrations - throw new CloudRuntimeException("Host contains VMs migrating. Please wait for them to complete before putting to maintenance."); + List migratingInVMs = _vmDao.findByHostInStates(hostId, State.Migrating); + if (migratingInVMs.size() > 0) { + throw new CloudRuntimeException("Host contains incoming VMs migrating. Please wait for them to complete before putting to maintenance."); } if (_vmDao.findByHostInStates(hostId, State.Starting).size() > 0) { diff --git a/test/integration/smoke/test_host_maintenance.py b/test/integration/smoke/test_host_maintenance.py index c7cd9d3998f5..e9dbb3a786d8 100644 --- a/test/integration/smoke/test_host_maintenance.py +++ b/test/integration/smoke/test_host_maintenance.py @@ -21,7 +21,7 @@ from marvin.cloudstackTestCase import * from marvin.lib.utils import * from marvin.lib.base import * -from marvin.lib.common import (get_zone, get_pod, get_template) +from marvin.lib.common import (get_zone, get_pod, get_template, list_ssvms) from nose.plugins.attrib import attr from marvin.lib.decoratorGenerators import skipTestIf from distutils.util import strtobool @@ -106,7 +106,32 @@ def createVMs(self, hostId, number): self.cleanup.append(self.network_offering) self.cleanup.append(self.service_offering) return vms - + + def checkAllVmsRunningOnHost(self, hostId): + listVms1 = VirtualMachine.list( + self.apiclient, + hostid=hostId + ) + + if (listVms1 is not None): + self.logger.debug('Vms found to test all running = {} '.format(len(listVms1))) + for vm in listVms1: + if (vm.state != "Running"): + self.logger.debug('VirtualMachine on Host with id = {} is in {}'.format(vm.id, vm.state)) + return (False, None) + + response = list_ssvms( + self.apiclient, + hostid=hostId + ) + if isinstance(response, list): + for systemvm in response: + if systemvm.state != 'Running': + self.logger.debug("Found not running VM {}".format(systemvm.name)) + return (False, None) + + return (True, None) + def checkVmMigratingOnHost(self, hostId): vm_migrating=False listVms1 = VirtualMachine.list( @@ -118,7 +143,7 @@ def checkVmMigratingOnHost(self, hostId): self.logger.debug('Vms found = {} '.format(len(listVms1))) for vm in listVms1: if (vm.state == "Migrating"): - self.logger.debug('VirtualMachine on Hyp id = {} is in {}'.format(vm.id, vm.state)) + self.logger.debug('VirtualMachine on Host with id = {} is in {}'.format(vm.id, vm.state)) vm_migrating=True break @@ -140,7 +165,7 @@ def checkNoVmMigratingOnHost(self, hostId): break return (no_vm_migrating, None) - + def noOfVMsOnHost(self, hostId): listVms = VirtualMachine.list( self.apiclient, @@ -153,25 +178,29 @@ def noOfVMsOnHost(self, hostId): no_of_vms=no_of_vms+1 return no_of_vms - + def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id, checkVMMigration): - + # Wait for all VMs to complete any pending migrations. + if not wait_until(1, 30, self.checkAllVmsRunningOnHost, target_host_id) or not wait_until(1, 30, self.checkAllVmsRunningOnHost, other_host_id): + raise Exception("Failed to wait for all VMs to reach running state to execute test") cmd = prepareHostForMaintenance.prepareHostForMaintenanceCmd() cmd.id = target_host_id + self.logger.debug('Sending Host with id {} to prepareHostForMaintenance'.format(target_host_id)) response = self.apiclient.prepareHostForMaintenance(cmd) self.logger.debug('Host with id {} is in prepareHostForMaintenance'.format(target_host_id)) vm_migrating = wait_until(1, 10, checkVMMigration, other_host_id) - + + self.logger.debug('Canceling Host with id {} from maintain'.format(target_host_id)) cmd = cancelHostMaintenance.cancelHostMaintenanceCmd() cmd.id = target_host_id response = self.apiclient.cancelHostMaintenance(cmd) - self.logger.debug('Host with id {} is in cancelHostMaintenance'.format(target_host_id) ) + self.logger.debug('Host with id {} has been sent to cancelHostMaintenance'.format(target_host_id)) return vm_migrating - + @attr( tags=[ "advanced", @@ -257,7 +286,6 @@ def test_02_cancel_host_maintenace_with_migration_jobs(self): vm_migrating=False try: - vm_migrating = self.hostPrepareAndCancelMaintenance(listHost[0].id, listHost[1].id, self.checkVmMigratingOnHost) vm_migrating = self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id, self.checkVmMigratingOnHost) From 8a050527b6b7767c8a402aa1688410305debd75c Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Fri, 12 Jul 2019 13:03:31 +0530 Subject: [PATCH 04/19] Change state name and add documentation --- .../com/cloud/resource/ResourceState.java | 16 +++---- .../cloud/resource/ResourceManagerImpl.java | 45 ++++++++++++++----- .../resource/ResourceManagerImplTest.java | 38 ++++++++++++++++ 3 files changed, 80 insertions(+), 19 deletions(-) diff --git a/api/src/main/java/com/cloud/resource/ResourceState.java b/api/src/main/java/com/cloud/resource/ResourceState.java index f9dddf2c4acd..1d24c3b191fe 100644 --- a/api/src/main/java/com/cloud/resource/ResourceState.java +++ b/api/src/main/java/com/cloud/resource/ResourceState.java @@ -29,7 +29,7 @@ public enum ResourceState { ErrorInMaintenance, Maintenance, Error, - PrepareForMaintenanceErrorsPresent; + ErrorInPrepareForMaintenance; public enum Event { InternalCreated("Resource is created"), @@ -108,18 +108,18 @@ public static String[] toString(ResourceState... states) { s_fsm.addTransition(ResourceState.Disabled, Event.InternalCreated, ResourceState.Disabled); s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.InternalEnterMaintenance, ResourceState.Maintenance); s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.AdminCancelMaintenance, ResourceState.Enabled); - s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.UnableToMigrate, ResourceState.PrepareForMaintenanceErrorsPresent); + s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.UnableToMigrate, ResourceState.ErrorInPrepareForMaintenance); s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.UnableToMaintain, ResourceState.ErrorInMaintenance); s_fsm.addTransition(ResourceState.PrepareForMaintenance, Event.InternalCreated, ResourceState.PrepareForMaintenance); s_fsm.addTransition(ResourceState.Maintenance, Event.AdminCancelMaintenance, ResourceState.Enabled); s_fsm.addTransition(ResourceState.Maintenance, Event.InternalCreated, ResourceState.Maintenance); s_fsm.addTransition(ResourceState.Maintenance, Event.DeleteHost, ResourceState.Disabled); - s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.InternalEnterMaintenance, ResourceState.Maintenance); - s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.AdminCancelMaintenance, ResourceState.Enabled); - s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.UnableToMigrate, ResourceState.PrepareForMaintenanceErrorsPresent); - s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.UnableToMaintain, ResourceState.ErrorInMaintenance); - s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.ErrorsCorrected, ResourceState.PrepareForMaintenance); - s_fsm.addTransition(ResourceState.PrepareForMaintenanceErrorsPresent, Event.InternalCreated, ResourceState.PrepareForMaintenanceErrorsPresent); + s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.InternalEnterMaintenance, ResourceState.Maintenance); + s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.AdminCancelMaintenance, ResourceState.Enabled); + s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.UnableToMigrate, ResourceState.ErrorInPrepareForMaintenance); + s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.UnableToMaintain, ResourceState.ErrorInMaintenance); + s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.ErrorsCorrected, ResourceState.PrepareForMaintenance); + s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.InternalCreated, ResourceState.ErrorInPrepareForMaintenance); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.InternalCreated, ResourceState.ErrorInMaintenance); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.Disable, ResourceState.Disabled); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.DeleteHost, ResourceState.Disabled); diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 8f5abdb2a0a6..6d6727055962 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1368,21 +1368,37 @@ protected boolean setHostIntoPrepareForMaintenanceAfterErrorsFixed(HostVO host) } /** - * Return true if host goes into Maintenance mode, only when: - * - No Running, Migrating or Failed migrations (host_id = last_host_id) for the host + * Return true if host goes into Maintenance mode. There are various possibilities for VMs' states + * on a host. We need to track the various VM states on each run and accordingly transit to the + * appropriate state. + * + * We change states as follws - + * 1. If there are no VMs in running, migrating, starting, stopping, error, unknown states we can move + * to maintenance state. Note that there cannot be incoming migrations as the API Call prepare for + * maintenance checks incoming migrations before starting. + * 2. If there errors (like migrating VMs, error VMs, etc) we mark as ErrorInPrepareForMaintenance but + * don't stop remaining migrations/ongoing legitimate operations. + * 3. If all migration retries, legitimate operations have finished we check for VMs on the host and if + * there are still VMs in error state or in running state or failed migrations we mark the VM as + * ErrorInMaintenance state. + * 4. Lastly if there are no errors or failed migrations or running VMs but there are still pending + * legitimate operations and the host was in ErrorInPrepareForMaintenance, we push the host back + * to PrepareForMaintenance state. */ - private boolean attemptMaintain(HostVO host) throws NoTransitionException { + protected boolean attemptMaintain(HostVO host) throws NoTransitionException { final long hostId = host.getId(); - if (CollectionUtils.isEmpty(_vmDao.findByHostInStates(hostId, State.Migrating, State.Running, State.Starting, State.Stopping, State.Error, State.Unknown))) { + // Step 1: If there are no VMs in migrating, running, starting, stopping, error or unknown state we can safely move the host to maintenance. + if (CollectionUtils.isEmpty(_vmDao.findByHostInStates(host.getId(), State.Migrating, State.Running, State.Starting, State.Stopping, State.Error, State.Unknown))) { return setHostIntoMaintenance(host); } + // Step 2: Gather relevant VMs' states on the host and then based on them we can determine if final List allVmsOnHost = _vmDao.listByHostId(hostId); - boolean hasPendingMigrationWorks = false; + boolean hasPendingMigrationRetries = false; for (VMInstanceVO vmInstanceVO : allVmsOnHost) { if (_haMgr.hasPendingMigrationsWork(vmInstanceVO.getId())) { - hasPendingMigrationWorks = true; + hasPendingMigrationRetries = true; break; } } @@ -1393,17 +1409,24 @@ private boolean attemptMaintain(HostVO host) throws NoTransitionException { final boolean hasRunningVms = CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Running)); final boolean hasFailedMigrations = CollectionUtils.isNotEmpty(failedMigrations); final boolean hasVmsInFailureStates = CollectionUtils.isNotEmpty(errorVms); + final boolean hasStoppingVms = CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Stopping)); errorVms.addAll(failedMigrations); - if (!hasPendingMigrationWorks && (hasRunningVms || (!hasRunningVms && !hasMigratingVms && hasVmsInFailureStates))) { + // Step 3: If there are no pending migration retries but host still has no running VMs or ongoing migrations, + // or no VMs in failure state we move the host to ErrorInMaintenance state. + if (!hasPendingMigrationRetries && (hasRunningVms || (!hasRunningVms && !hasMigratingVms && hasVmsInFailureStates))) { return setHostIntoErrorInMaintenance(host, errorVms); } - if ((hasVmsInFailureStates || hasFailedMigrations) && (hasPendingMigrationWorks || hasMigratingVms || CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Stopping)))) { + // Step 4: IF there are pending migrations or ongoing retries left or stopping VMs and there were errors or failed + // migrations we put the host into ErrorInPrepareForMaintenance + if ((hasVmsInFailureStates || hasFailedMigrations) && (hasPendingMigrationRetries || hasMigratingVms || hasStoppingVms)) { return setHostIntoPrepareForMaintenanceWithErrors(host, errorVms); } - if (host.getResourceState() == ResourceState.PrepareForMaintenanceErrorsPresent) { + // Step 5: If there were previously errors found, but not anymore it means the operator has fixed errors and we put + // the host into PrepareForMaintenance state. + if (host.getResourceState() == ResourceState.ErrorInPrepareForMaintenance) { return setHostIntoPrepareForMaintenanceAfterErrorsFixed(host); } @@ -2349,7 +2372,7 @@ private boolean doCancelMaintenance(final long hostId) { * really prefer to exception that always exposes bugs */ if (host.getResourceState() != ResourceState.PrepareForMaintenance && - host.getResourceState() != ResourceState.PrepareForMaintenanceErrorsPresent && + host.getResourceState() != ResourceState.ErrorInPrepareForMaintenance && host.getResourceState() != ResourceState.Maintenance && host.getResourceState() != ResourceState.ErrorInMaintenance) { throw new CloudRuntimeException("Cannot perform cancelMaintenance when resource state is " + host.getResourceState() + ", hostId = " + hostId); @@ -3003,6 +3026,6 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {KvmSshToAgentEnabled}; + return new ConfigKey[0]; } } diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index d34a0420e192..b3a759f3a7e6 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -17,6 +17,8 @@ package com.cloud.resource; +import static com.cloud.resource.ResourceState.Event.InternalEnterMaintenance; +import static com.cloud.resource.ResourceState.Event.UnableToMigrate; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyLong; @@ -59,6 +61,7 @@ import com.cloud.storage.StorageManager; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.fsm.NoTransitionException; import com.cloud.utils.ssh.SSHCmdHelper; import com.cloud.utils.ssh.SshException; import com.cloud.vm.VMInstanceVO; @@ -167,6 +170,41 @@ public void setup() throws Exception { when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("true"); } + @Test + public void testCheckAndMaintainEnterMaintenanceMode() throws NoTransitionException { + boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); + verify(resourceManager).attemptMaintain(host); + verify(resourceManager).setHostIntoMaintenance(host); + verify(resourceManager).resourceStateTransitTo(eq(host), eq(InternalEnterMaintenance), anyLong()); + Assert.assertTrue(enterMaintenanceMode); + } + + @Test + public void testCheckAndMaintainErrorInMaintenanceRunningVms() throws NoTransitionException { + when(vmInstanceDao.listByHostId(hostId)).thenReturn(Arrays.asList(vm1, vm2)); + boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); + verify(resourceManager).attemptMaintain(host); + Assert.assertFalse(enterMaintenanceMode); + } + + @Test + public void testCheckAndMaintainErrorInMaintenanceMigratingVms() throws NoTransitionException { + when(vmInstanceDao.listVmsMigratingFromHost(hostId)).thenReturn(Arrays.asList(vm1, vm2)); + boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); + verify(resourceManager).attemptMaintain(host); + Assert.assertFalse(enterMaintenanceMode); + } + + @Test + public void testCheckAndMaintainErrorInMaintenanceFailedMigrations() throws NoTransitionException { + when(vmInstanceDao.listNonMigratingVmsByHostEqualsLastHost(hostId)).thenReturn(Arrays.asList(vm1, vm2)); + boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); + verify(resourceManager).attemptMaintain(host); + verify(resourceManager).setHostIntoErrorInMaintenance(host, Arrays.asList(vm1, vm2)); + verify(resourceManager).resourceStateTransitTo(eq(host), eq(UnableToMigrate), anyLong()); + Assert.assertFalse(enterMaintenanceMode); + } + @Test public void testConfigureVncAccessForKVMHostFailedMigrations() { when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); From aa43675097acab3ee6dff1e8dadbbbccfcb844c6 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Wed, 17 Jul 2019 13:56:36 +0530 Subject: [PATCH 05/19] Fix test --- .../test/java/com/cloud/resource/ResourceManagerImplTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index b3a759f3a7e6..5514fdb15bc2 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -65,6 +65,7 @@ import com.cloud.utils.ssh.SSHCmdHelper; import com.cloud.utils.ssh.SshException; import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.VirtualMachine; import com.cloud.vm.dao.UserVmDetailsDao; import com.cloud.vm.dao.VMInstanceDao; import com.trilead.ssh2.Connection; @@ -181,7 +182,7 @@ public void testCheckAndMaintainEnterMaintenanceMode() throws NoTransitionExcept @Test public void testCheckAndMaintainErrorInMaintenanceRunningVms() throws NoTransitionException { - when(vmInstanceDao.listByHostId(hostId)).thenReturn(Arrays.asList(vm1, vm2)); + when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Migrating, VirtualMachine.State.Running, VirtualMachine.State.Starting, VirtualMachine.State.Stopping, VirtualMachine.State.Error, VirtualMachine.State.Unknown)).thenReturn(Arrays.asList(vm1, vm2)); boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); verify(resourceManager).attemptMaintain(host); Assert.assertFalse(enterMaintenanceMode); From 8a84a4bac920ef23638bfaec887829496c8f54b7 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Sun, 21 Jul 2019 21:31:43 +0530 Subject: [PATCH 06/19] Fix and add more unit tests for different caseS --- .../com/cloud/resource/ResourceState.java | 4 +- .../cloud/resource/ResourceManagerImpl.java | 40 ++--- .../resource/ResourceManagerImplTest.java | 167 ++++++++++++++++-- 3 files changed, 172 insertions(+), 39 deletions(-) diff --git a/api/src/main/java/com/cloud/resource/ResourceState.java b/api/src/main/java/com/cloud/resource/ResourceState.java index 1d24c3b191fe..a296bf2ec091 100644 --- a/api/src/main/java/com/cloud/resource/ResourceState.java +++ b/api/src/main/java/com/cloud/resource/ResourceState.java @@ -25,11 +25,11 @@ public enum ResourceState { Creating, Enabled, Disabled, + ErrorInPrepareForMaintenance, PrepareForMaintenance, ErrorInMaintenance, Maintenance, - Error, - ErrorInPrepareForMaintenance; + Error; public enum Event { InternalCreated("Resource is created"), diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 6d6727055962..acf37444b081 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1329,6 +1329,18 @@ protected void configureVncAccessForKVMHostFailedMigrations(HostVO host, List return false; } - /** - * Safely transit host into Maintenance mode - */ - protected boolean setHostIntoMaintenance(HostVO host) throws NoTransitionException { - s_logger.debug("Host " + host.getUuid() + " entering in Maintenance"); - resourceStateTransitTo(host, ResourceState.Event.InternalEnterMaintenance, _nodeId); - ActionEventUtils.onCompletedActionEvent(CallContext.current().getCallingUserId(), CallContext.current().getCallingAccountId(), - EventVO.LEVEL_INFO, EventTypes.EVENT_MAINTENANCE_PREPARE, - "completed maintenance for host " + host.getId(), 0); - return true; - } - - protected boolean setHostIntoPrepareForMaintenanceWithErrors(HostVO host, List errorVms) throws NoTransitionException { + protected boolean setHostIntoErrorInPrepareForMaintenance(HostVO host, List errorVms) throws NoTransitionException { s_logger.debug("Host " + host.getUuid() + " entering in PrepareForMaintenanceWithErrors state"); configureVncAccessForKVMHostFailedMigrations(host, errorVms); resourceStateTransitTo(host, ResourceState.Event.UnableToMigrate, _nodeId); - return true; + return false; } protected boolean setHostIntoPrepareForMaintenanceAfterErrorsFixed(HostVO host) throws NoTransitionException { s_logger.debug("Host " + host.getUuid() + " entering in PrepareForMaintenance state as any previous corrections have been fixed"); resourceStateTransitTo(host, ResourceState.Event.ErrorsCorrected, _nodeId); - return true; + return false; } /** @@ -1412,16 +1412,16 @@ protected boolean attemptMaintain(HostVO host) throws NoTransitionException { final boolean hasStoppingVms = CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Stopping)); errorVms.addAll(failedMigrations); - // Step 3: If there are no pending migration retries but host still has no running VMs or ongoing migrations, - // or no VMs in failure state we move the host to ErrorInMaintenance state. - if (!hasPendingMigrationRetries && (hasRunningVms || (!hasRunningVms && !hasMigratingVms && hasVmsInFailureStates))) { + // Step 3: If there are no pending migration retries but host still has running VMs or, + // host has VMs in failure state / failed migrations we move the host to ErrorInMaintenance state. + if (!hasPendingMigrationRetries && (hasRunningVms || (!hasMigratingVms && hasVmsInFailureStates))) { return setHostIntoErrorInMaintenance(host, errorVms); } // Step 4: IF there are pending migrations or ongoing retries left or stopping VMs and there were errors or failed // migrations we put the host into ErrorInPrepareForMaintenance - if ((hasVmsInFailureStates || hasFailedMigrations) && (hasPendingMigrationRetries || hasMigratingVms || hasStoppingVms)) { - return setHostIntoPrepareForMaintenanceWithErrors(host, errorVms); + if ((hasPendingMigrationRetries || hasMigratingVms || hasStoppingVms) && (hasVmsInFailureStates || hasFailedMigrations)) { + return setHostIntoErrorInPrepareForMaintenance(host, errorVms); } // Step 5: If there were previously errors found, but not anymore it means the operator has fixed errors and we put diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index 5514fdb15bc2..b0c80b82f755 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -17,11 +17,14 @@ package com.cloud.resource; +import static com.cloud.resource.ResourceState.Event.ErrorsCorrected; import static com.cloud.resource.ResourceState.Event.InternalEnterMaintenance; +import static com.cloud.resource.ResourceState.Event.UnableToMaintain; import static com.cloud.resource.ResourceState.Event.UnableToMigrate; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyLong; +import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.never; @@ -172,38 +175,96 @@ public void setup() throws Exception { } @Test - public void testCheckAndMaintainEnterMaintenanceMode() throws NoTransitionException { + public void testCheckAndMaintainEnterMaintenanceModeNoVms() throws NoTransitionException { + // Test entering into maintenance with no VMs running on host. boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); verify(resourceManager).attemptMaintain(host); verify(resourceManager).setHostIntoMaintenance(host); + verify(resourceManager, never()).setHostIntoErrorInPrepareForMaintenance(anyObject(), anyObject()); + verify(resourceManager, never()).setHostIntoErrorInMaintenance(anyObject(), anyObject()); + verify(resourceManager, never()).setHostIntoPrepareForMaintenanceAfterErrorsFixed(anyObject()); verify(resourceManager).resourceStateTransitTo(eq(host), eq(InternalEnterMaintenance), anyLong()); + Assert.assertTrue(enterMaintenanceMode); } + @Test + public void testCheckAndMaintainProceedsWithPrepareForMaintenanceRunningVms() throws NoTransitionException { + // Test proceeding through with no events if pending migrating works / retries left. + setupRunningVMs(); + setupPendingMigrationRetries(); + verifyNoChangeInMaintenance(); + } + @Test public void testCheckAndMaintainErrorInMaintenanceRunningVms() throws NoTransitionException { - when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Migrating, VirtualMachine.State.Running, VirtualMachine.State.Starting, VirtualMachine.State.Stopping, VirtualMachine.State.Error, VirtualMachine.State.Unknown)).thenReturn(Arrays.asList(vm1, vm2)); - boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); - verify(resourceManager).attemptMaintain(host); - Assert.assertFalse(enterMaintenanceMode); + // Test entering into ErrorInMaintenance when no pending migrations etc, and due to - Running VMs + setupRunningVMs(); + setupNoPendingMigrationRetries(); + verifyErrorInMaintenanceCalls(); } @Test - public void testCheckAndMaintainErrorInMaintenanceMigratingVms() throws NoTransitionException { - when(vmInstanceDao.listVmsMigratingFromHost(hostId)).thenReturn(Arrays.asList(vm1, vm2)); - boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); - verify(resourceManager).attemptMaintain(host); - Assert.assertFalse(enterMaintenanceMode); + public void testCheckAndMaintainErrorInMaintenanceWithErrorVms() throws NoTransitionException { + // Test entering into ErrorInMaintenance when no pending migrations etc, and due to - no migrating but error VMs + setupErrorVms(); + setupNoPendingMigrationRetries(); + verifyErrorInMaintenanceCalls(); } @Test - public void testCheckAndMaintainErrorInMaintenanceFailedMigrations() throws NoTransitionException { - when(vmInstanceDao.listNonMigratingVmsByHostEqualsLastHost(hostId)).thenReturn(Arrays.asList(vm1, vm2)); - boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); - verify(resourceManager).attemptMaintain(host); - verify(resourceManager).setHostIntoErrorInMaintenance(host, Arrays.asList(vm1, vm2)); - verify(resourceManager).resourceStateTransitTo(eq(host), eq(UnableToMigrate), anyLong()); - Assert.assertFalse(enterMaintenanceMode); + public void testCheckAndMaintainErrorInPrepareForMaintenanceFailedMigrationsPendingRetries() throws NoTransitionException { + // Test entering into ErrorInPrepareForMaintenance when pending migrations retries and due to - Failed Migrations + setupFailedMigrations(); + setupPendingMigrationRetries(); + verifyErrorInPrepareForMaintenanceCalls(); + } + + @Test + public void testCheckAndMaintainErrorInPrepareForMaintenanceWithErrorVmsPendingRetries() throws NoTransitionException { + // Test entering into ErrorInMaintenance when pending migrations retries due to - no migrating but error VMs + setupErrorVms(); + setupPendingMigrationRetries(); + verifyErrorInPrepareForMaintenanceCalls(); + } + + @Test + public void testCheckAndMaintainErrorInPrepareForMaintenanceFailedMigrationsAndMigratingVms() throws NoTransitionException { + // Test entering into ErrorInPrepareForMaintenance when no pending migrations retries + // but executing migration and due to - Failed Migrations + setupFailedMigrations(); + setupNoPendingMigrationRetries(); + when(vmInstanceDao.listVmsMigratingFromHost(hostId)).thenReturn(Arrays.asList(vm2)); + verifyErrorInPrepareForMaintenanceCalls(); + } + + @Test + public void testCheckAndMaintainErrorInPrepareForMaintenanceWithErrorVmsAndMigratingVms() throws NoTransitionException { + // Test entering into ErrorInPrepareForMaintenance when no pending migrations retries + // but executing migration and due to - Error Vms + setupErrorVms(); + setupNoPendingMigrationRetries(); + when(vmInstanceDao.listVmsMigratingFromHost(hostId)).thenReturn(Arrays.asList(vm2)); + verifyErrorInPrepareForMaintenanceCalls(); + } + + @Test + public void testCheckAndMaintainErrorInPrepareForMaintenanceFailedMigrationsAndStoppingVms() throws NoTransitionException { + // Test entering into ErrorInPrepareForMaintenance when no pending migrations retries + // but stopping VMs and due to - Failed Migrations + setupFailedMigrations(); + setupNoPendingMigrationRetries(); + when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Stopping)).thenReturn(Arrays.asList(vm2)); + verifyErrorInPrepareForMaintenanceCalls(); + } + + @Test + public void testCheckAndMaintainReturnsToPrepareForMaintenanceRunningVms() throws NoTransitionException { + // Test switching back to PrepareForMaintenance + when(host.getResourceState()).thenReturn(ResourceState.ErrorInPrepareForMaintenance); + setupRunningVMs(); + setupPendingMigrationRetries(); + verifyReturnToPrepareForMaintenanceCalls(); } @Test @@ -292,4 +353,76 @@ public void testHandleAgentVMsMigrating() { verify(resourceManager, never()).getHostCredentials(eq(host)); verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword)); } + + private void setupNoPendingMigrationRetries() { + when(haManager.hasPendingMigrationsWork(vm1.getId())).thenReturn(false); + when(haManager.hasPendingMigrationsWork(vm2.getId())).thenReturn(false); + } + + private void setupRunningVMs() { + when(vmInstanceDao.listByHostId(hostId)).thenReturn(Arrays.asList(vm1, vm2)); + when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Migrating, VirtualMachine.State.Running, VirtualMachine.State.Starting, VirtualMachine.State.Stopping, VirtualMachine.State.Error, VirtualMachine.State.Unknown)).thenReturn(Arrays.asList(vm1, vm2)); + when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Running)).thenReturn(Arrays.asList(vm1, vm2)); + } + + private void setupPendingMigrationRetries() { + when(haManager.hasPendingMigrationsWork(vm1.getId())).thenReturn(true); + when(haManager.hasPendingMigrationsWork(vm2.getId())).thenReturn(false); + } + + private void setupFailedMigrations() { + when(vmInstanceDao.listByHostId(hostId)).thenReturn(Arrays.asList(vm1, vm2)); + when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Migrating, VirtualMachine.State.Running, VirtualMachine.State.Starting, VirtualMachine.State.Stopping, VirtualMachine.State.Error, VirtualMachine.State.Unknown)).thenReturn(Arrays.asList(vm1, vm2)); + when(vmInstanceDao.listNonMigratingVmsByHostEqualsLastHost(hostId)).thenReturn(Arrays.asList(vm1)); + } + + private void setupErrorVms() { + when(vmInstanceDao.listByHostId(hostId)).thenReturn(Arrays.asList(vm1, vm2)); + when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Migrating, VirtualMachine.State.Running, VirtualMachine.State.Starting, VirtualMachine.State.Stopping, VirtualMachine.State.Error, VirtualMachine.State.Unknown)).thenReturn(Arrays.asList(vm1, vm2)); + when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Unknown, VirtualMachine.State.Error, VirtualMachine.State.Shutdowned)).thenReturn(Arrays.asList(vm1)); + } + + private void verifyErrorInMaintenanceCalls() throws NoTransitionException { + boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); + verify(resourceManager).attemptMaintain(host); + verify(resourceManager).setHostIntoErrorInMaintenance(eq(host), anyObject()); + verify(resourceManager, never()).setHostIntoMaintenance(anyObject()); + verify(resourceManager, never()).setHostIntoErrorInPrepareForMaintenance(anyObject(), anyObject()); + verify(resourceManager, never()).setHostIntoPrepareForMaintenanceAfterErrorsFixed(anyObject()); + verify(resourceManager).resourceStateTransitTo(eq(host), eq(UnableToMaintain), anyLong()); + Assert.assertFalse(enterMaintenanceMode); + } + + private void verifyErrorInPrepareForMaintenanceCalls() throws NoTransitionException { + boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); + verify(resourceManager).attemptMaintain(host); + verify(resourceManager).setHostIntoErrorInPrepareForMaintenance(eq(host), anyObject()); + verify(resourceManager, never()).setHostIntoMaintenance(anyObject()); + verify(resourceManager, never()).setHostIntoErrorInMaintenance(anyObject(), anyObject()); + verify(resourceManager, never()).setHostIntoPrepareForMaintenanceAfterErrorsFixed(anyObject()); + verify(resourceManager).resourceStateTransitTo(eq(host), eq(UnableToMigrate), anyLong()); + Assert.assertFalse(enterMaintenanceMode); + } + + private void verifyReturnToPrepareForMaintenanceCalls() throws NoTransitionException { + boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); + verify(resourceManager).attemptMaintain(host); + verify(resourceManager).setHostIntoPrepareForMaintenanceAfterErrorsFixed(eq(host)); + verify(resourceManager).resourceStateTransitTo(eq(host), eq(ErrorsCorrected), anyLong()); + verify(resourceManager, never()).setHostIntoMaintenance(anyObject()); + verify(resourceManager, never()).setHostIntoErrorInPrepareForMaintenance(anyObject(), anyObject()); + verify(resourceManager, never()).setHostIntoErrorInMaintenance(anyObject(), anyObject()); + Assert.assertFalse(enterMaintenanceMode); + } + + private void verifyNoChangeInMaintenance() throws NoTransitionException { + boolean enterMaintenanceMode = resourceManager.checkAndMaintain(hostId); + verify(resourceManager).attemptMaintain(host); + verify(resourceManager, never()).setHostIntoMaintenance(anyObject()); + verify(resourceManager, never()).setHostIntoErrorInPrepareForMaintenance(anyObject(), anyObject()); + verify(resourceManager, never()).setHostIntoErrorInMaintenance(anyObject(), anyObject()); + verify(resourceManager, never()).setHostIntoPrepareForMaintenanceAfterErrorsFixed(anyObject()); + verify(resourceManager, never()).resourceStateTransitTo(anyObject(), any(), anyLong()); + Assert.assertFalse(enterMaintenanceMode); + } } From 47bb14780726b8851a46d83d6aac0e6caf284c09 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Mon, 22 Jul 2019 01:53:58 +0530 Subject: [PATCH 07/19] Fix and enhance Marvin Tests --- .../smoke/test_host_maintenance.py | 163 ++++++++++++++---- 1 file changed, 129 insertions(+), 34 deletions(-) diff --git a/test/integration/smoke/test_host_maintenance.py b/test/integration/smoke/test_host_maintenance.py index e9dbb3a786d8..c856e752db3b 100644 --- a/test/integration/smoke/test_host_maintenance.py +++ b/test/integration/smoke/test_host_maintenance.py @@ -149,22 +149,18 @@ def checkVmMigratingOnHost(self, hostId): return (vm_migrating, None) - def checkNoVmMigratingOnHost(self, hostId): - no_vm_migrating=True + def migrationsFinished(self, hostId): + migrations_finished=True listVms1 = VirtualMachine.list( self.apiclient, hostid=hostId ) if (listVms1 is not None): - self.logger.debug('Vms found = {} '.format(len(listVms1))) - for vm in listVms1: - if (vm.state == "Migrating"): - self.logger.debug('VirtualMachine on Hyp id = {} is in {}'.format(vm.id, vm.state)) - no_vm_migrating=False - break + numVms = len(listVms1) + migrations_finished = (numVms == 0) - return (no_vm_migrating, None) + return (migrations_finished, None) def noOfVMsOnHost(self, hostId): listVms = VirtualMachine.list( @@ -179,27 +175,51 @@ def noOfVMsOnHost(self, hostId): return no_of_vms - def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id, checkVMMigration): + def wait_until_host_is_in_state(self, hostid, resourcestate, interval=2, retries=100): + def check_resource_state(): + response = Host.list( + self.apiclient, + id=hostid + ) + if isinstance(response, list): + if response[0].resourcestate == resourcestate: + self.logger.debug('Host with id %s is in resource state = %s' % (hostid, resourcestate)) + return True, None + return False, None + + done, _ = wait_until(interval, retries, check_resource_state) + if not done: + raise Exception("Failed to wait for host %s to be on resource state %s" % (hostid, resourcestate)) + return True + + def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id): # Wait for all VMs to complete any pending migrations. - if not wait_until(1, 30, self.checkAllVmsRunningOnHost, target_host_id) or not wait_until(1, 30, self.checkAllVmsRunningOnHost, other_host_id): + if not wait_until(2, 60, self.checkAllVmsRunningOnHost, target_host_id) or not wait_until(2, 60, self.checkAllVmsRunningOnHost, other_host_id): raise Exception("Failed to wait for all VMs to reach running state to execute test") + + expected_vm_count_after_maintenance = self.noOfVMsOnHost(target_host_id) + self.noOfVMsOnHost(other_host_id) cmd = prepareHostForMaintenance.prepareHostForMaintenanceCmd() cmd.id = target_host_id self.logger.debug('Sending Host with id {} to prepareHostForMaintenance'.format(target_host_id)) response = self.apiclient.prepareHostForMaintenance(cmd) self.logger.debug('Host with id {} is in prepareHostForMaintenance'.format(target_host_id)) - - vm_migrating = wait_until(1, 10, checkVMMigration, other_host_id) + + migrations_finished = wait_until(2, 100, self.migrationsFinished, target_host_id) + wait_until(2, 60, self.checkAllVmsRunningOnHost, other_host_id) + other_vm_count_after_maintenance = self.noOfVMsOnHost(other_host_id) self.logger.debug('Canceling Host with id {} from maintain'.format(target_host_id)) cmd = cancelHostMaintenance.cancelHostMaintenanceCmd() cmd.id = target_host_id - response = self.apiclient.cancelHostMaintenance(cmd) + self.apiclient.cancelHostMaintenance(cmd) self.logger.debug('Host with id {} has been sent to cancelHostMaintenance'.format(target_host_id)) - - return vm_migrating + + if expected_vm_count_after_maintenance != other_vm_count_after_maintenance: + self.fail('All VMs not found on other host after maintenance. Other host VM counts expected {} but was {}'.format(expected_vm_count_after_maintenance, other_vm_count_after_maintenance)) + + return migrations_finished @attr( tags=[ @@ -225,28 +245,26 @@ def test_01_cancel_host_maintenace_with_no_migration_jobs(self): raise unittest.SkipTest("Cancel host maintenance when VMs are migrating should be tested for 2 or more hosts"); return - vm_migrating=False + migrations_finished = True try: - vm_migrating = self.hostPrepareAndCancelMaintenance(listHost[0].id, listHost[1].id, self.checkNoVmMigratingOnHost) - - vm_migrating = self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id, self.checkNoVmMigratingOnHost) + migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[0].id, listHost[1].id) + + if migrations_finished: + migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id) except Exception as e: self.logger.debug("Exception {}".format(e)) self.fail("Cancel host maintenance failed {}".format(e[0])) - if (vm_migrating == True): - raise unittest.SkipTest("VMs are migrating and the test will not be able to check the conditions the test is intended for"); + if (migrations_finished == False): + raise unittest.SkipTest("VMs are still migrating and the test will not be able to check the conditions the test is intended for"); return - - - @attr( tags=[ "advanced", @@ -283,24 +301,101 @@ def test_02_cancel_host_maintenace_with_migration_jobs(self): self.logger.debug("Creating vms = {}".format(no_vm_req)) self.vmlist = self.createVMs(listHost[0].id, no_vm_req) - vm_migrating=False + migrations_finished = True try: - vm_migrating = self.hostPrepareAndCancelMaintenance(listHost[0].id, listHost[1].id, self.checkVmMigratingOnHost) - - vm_migrating = self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id, self.checkVmMigratingOnHost) + migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[0].id, listHost[1].id) + + if migrations_finished: + migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id) except Exception as e: self.logger.debug("Exception {}".format(e)) self.fail("Cancel host maintenance failed {}".format(e[0])) - - if (vm_migrating == False): - raise unittest.SkipTest("No VM is migrating and the test will not be able to check the conditions the test is intended for"); - - + + if (migrations_finished == False): + raise unittest.SkipTest("VMs are still migrating and the test will not be able to check the conditions the test is intended for"); + + return + + @attr( + tags=[ + "advanced", + "advancedns", + "smoke", + "basic", + "eip", + "sg"], + required_hardware="true") + def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): + + listHost = Host.list( + self.apiclient, + type='Routing', + zoneid=self.zone.id, + podid=self.pod.id, + ) + for host in listHost: + self.logger.debug('2 Hypervisor = {}'.format(host.id)) + + if (len(listHost) != 2): + raise unittest.SkipTest("Cancel host maintenance when VMs are migrating can only be tested with 2 hosts"); + return + + target_host_id = listHost[0].id + other_host_id = listHost[1].id + + no_of_vms = self.noOfVMsOnHost(target_host_id) + + # Need only 2 VMs for this case. + if no_of_vms < 2: + self.logger.debug("Create VMs as there are not enough vms to check host maintenance") + no_vm_req = 2 - no_of_vms + if (no_vm_req > 0): + self.logger.debug("Creating vms = {}".format(no_vm_req)) + self.vmlist = self.createVMs(listHost[0].id, no_vm_req) + + migrations_finished = True + + ssh_client = self.get_ssh_client(listHost[1].ipaddress) + ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") + ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") + + cmd = prepareHostForMaintenance.prepareHostForMaintenanceCmd() + cmd.id = target_host_id + self.logger.debug('Sending Host with id {} to prepareHostForMaintenance'.format(target_host_id)) + response = self.apiclient.prepareHostForMaintenance(cmd) + + self.logger.debug('Attempting to put host with id {} in Maintenance'.format(target_host_id)) + + error_in_maintenance_reached = self.wait_until_host_is_in_state(target_host_id, "ErrorInMaintenance") + + self.logger.debug('Canceling Host with id {} from maintain'.format(target_host_id)) + cmd = cancelHostMaintenance.cancelHostMaintenanceCmd() + cmd.id = target_host_id + self.apiclient.cancelHostMaintenance(cmd) + + self.logger.debug('Host with id {} has been sent to cancelHostMaintenance'.format(target_host_id)) + + ssh_client.execute("iptables -I OUTPUT -j ACCEPT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'open port for migrations'") + ssh_client.execute("iptables -I OUTPUT -j ACCEPT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'open port for migrations'") + + if error_in_maintenance_reached == False: + self.fail("Error in maintenance state should have reached after ports block") + return + def get_ssh_client(self, ip, username="root", password="password", retries=10): + try: + ssh_client = SshClient(ip, 22, username, password, retries) + except Exception as e: + raise unittest.SkipTest("Unable to create ssh connection: " % e) + + self.assertIsNotNone( + ssh_client, "Failed to setup ssh connection to ip=%s" % ip) + + return ssh_client class TestHostMaintenanceAgents(cloudstackTestCase): From 2e09eed82f64289b2df4cc454bf908c3cc2f075e Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Thu, 14 Nov 2019 02:51:15 +0530 Subject: [PATCH 08/19] Fixes for corner cases --- .../cloud/agent/manager/AgentManagerImpl.java | 6 ++-- .../entity/api/db/EngineHostVO.java | 34 +++++++++++-------- .../src/main/java/com/cloud/host/HostVO.java | 28 ++++++++------- .../cloud/ha/HighAvailabilityManagerImpl.java | 19 +++++++---- .../cloud/resource/ResourceManagerImpl.java | 27 +++++++++++---- .../java/com/cloud/server/StatsCollector.java | 6 +++- .../cloud/servlet/ConsoleProxyServlet.java | 30 ++++++++++------ .../smoke/test_host_maintenance.py | 4 +-- 8 files changed, 98 insertions(+), 56 deletions(-) diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java index 60911319e334..2ee583d1db7d 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java @@ -1583,7 +1583,7 @@ protected void runInContext() { final HostVO h = sc.find(); if (h != null) { final ResourceState resourceState = h.getResourceState(); - if (resourceState == ResourceState.Disabled || resourceState == ResourceState.Maintenance || resourceState == ResourceState.ErrorInMaintenance) { + if (resourceState == ResourceState.Disabled || resourceState == ResourceState.Maintenance) { /* * Host is in non-operation state, so no investigation and direct put agent to Disconnected */ @@ -1605,7 +1605,9 @@ protected void runInContext() { } final QueryBuilder sc = QueryBuilder.create(HostVO.class); - sc.and(sc.entity().getResourceState(), Op.IN, ResourceState.PrepareForMaintenance, ResourceState.ErrorInMaintenance); + sc.and(sc.entity().getResourceState(), Op.IN, + ResourceState.PrepareForMaintenance, + ResourceState.ErrorInPrepareForMaintenance); final List hosts = sc.list(); for (final HostVO host : hosts) { diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/EngineHostVO.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/EngineHostVO.java index be1484f0bde2..c48def27c5c8 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/EngineHostVO.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/EngineHostVO.java @@ -16,16 +16,10 @@ // under the License. package org.apache.cloudstack.engine.datacenter.entity.api.db; -import com.cloud.host.Status; -import com.cloud.hypervisor.Hypervisor.HypervisorType; -import com.cloud.resource.ResourceState; -import com.cloud.storage.Storage.StoragePoolType; -import com.cloud.utils.NumbersUtil; -import com.cloud.utils.db.GenericDao; -import com.cloud.utils.db.StateMachine; -import org.apache.cloudstack.api.Identity; -import org.apache.cloudstack.engine.datacenter.entity.api.DataCenterResourceEntity.State; -import org.apache.cloudstack.engine.datacenter.entity.api.DataCenterResourceEntity.State.Event; +import java.util.Date; +import java.util.List; +import java.util.Map; +import java.util.UUID; import javax.persistence.Column; import javax.persistence.DiscriminatorColumn; @@ -42,10 +36,18 @@ import javax.persistence.Temporal; import javax.persistence.TemporalType; import javax.persistence.Transient; -import java.util.Date; -import java.util.List; -import java.util.Map; -import java.util.UUID; + +import org.apache.cloudstack.api.Identity; +import org.apache.cloudstack.engine.datacenter.entity.api.DataCenterResourceEntity.State; +import org.apache.cloudstack.engine.datacenter.entity.api.DataCenterResourceEntity.State.Event; + +import com.cloud.host.Status; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.resource.ResourceState; +import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.utils.NumbersUtil; +import com.cloud.utils.db.GenericDao; +import com.cloud.utils.db.StateMachine; @Entity @Table(name = "host") @@ -730,7 +732,9 @@ public boolean isDisabled() { @Override public boolean isInMaintenanceStates() { - return (getResourceState() == ResourceState.Maintenance || getResourceState() == ResourceState.ErrorInMaintenance || getResourceState() == ResourceState.PrepareForMaintenance); + ResourceState state = getResourceState(); + return (state == ResourceState.Maintenance || state == ResourceState.ErrorInMaintenance || + state == ResourceState.PrepareForMaintenance || state == ResourceState.ErrorInPrepareForMaintenance); } public long getUpdated() { diff --git a/engine/schema/src/main/java/com/cloud/host/HostVO.java b/engine/schema/src/main/java/com/cloud/host/HostVO.java index 7fd1e7101850..0eb32b68c774 100644 --- a/engine/schema/src/main/java/com/cloud/host/HostVO.java +++ b/engine/schema/src/main/java/com/cloud/host/HostVO.java @@ -16,12 +16,11 @@ // under the License. package com.cloud.host; -import com.cloud.agent.api.VgpuTypesInfo; -import com.cloud.hypervisor.Hypervisor.HypervisorType; -import com.cloud.resource.ResourceState; -import com.cloud.storage.Storage.StoragePoolType; -import com.cloud.utils.NumbersUtil; -import com.cloud.utils.db.GenericDao; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; import javax.persistence.Column; import javax.persistence.DiscriminatorColumn; @@ -38,11 +37,13 @@ import javax.persistence.Temporal; import javax.persistence.TemporalType; import javax.persistence.Transient; -import java.util.Date; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.UUID; + +import com.cloud.agent.api.VgpuTypesInfo; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.resource.ResourceState; +import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.utils.NumbersUtil; +import com.cloud.utils.db.GenericDao; @Entity @Table(name = "host") @@ -714,9 +715,10 @@ public void setResourceState(ResourceState state) { @Override public boolean isInMaintenanceStates() { - return (getResourceState() == ResourceState.Maintenance || getResourceState() == ResourceState.ErrorInMaintenance || getResourceState() == ResourceState.PrepareForMaintenance); + ResourceState state = getResourceState(); + return (state == ResourceState.Maintenance || state == ResourceState.ErrorInMaintenance || + state == ResourceState.PrepareForMaintenance || state == ResourceState.ErrorInPrepareForMaintenance); } - @Override public boolean isDisabled() { return (getResourceState() == ResourceState.Disabled); diff --git a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java index fd6e438a568f..ee85e1238774 100644 --- a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java +++ b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java @@ -650,9 +650,15 @@ public Long migrate(final HaWorkVO work) { _itMgr.migrateAway(vm.getUuid(), srcHostId); return null; } catch (InsufficientServerCapacityException e) { - s_logger.warn("Insufficient capacity for migrating a VM."); + s_logger.warn("Migration attempt: Insufficient capacity for migrating a VM " + + _instanceDao.findById(vmId).getUuid() + " from source host id " + srcHostId + + ". Exception: " + e.getMessage()); _resourceMgr.maintenanceFailed(srcHostId); return (System.currentTimeMillis() >> 10) + _migrateRetryInterval; + } catch (Exception e) { + s_logger.warn("Migration attempt: Unexpected exception occurred when attempting migration of " + + _instanceDao.findById(vmId) + e.getMessage()); + throw e; } } @@ -826,12 +832,13 @@ private void processWork(final HaWorkVO work) { VMInstanceVO vm = _instanceDao.findById(work.getInstanceId()); work.setUpdateTime(vm.getUpdated()); work.setPreviousState(vm.getState()); + } finally { + if (!Step.Done.equals(work.getStep()) && work.getTimesTried() >= _maxRetries) { + s_logger.warn("Giving up, retried max. times for work: " + work); + work.setStep(Step.Done); + } + _haDao.update(work.getId(), work); } - if (!Step.Done.equals(work.getStep()) && work.getTimesTried() >= _maxRetries) { - s_logger.warn("Giving up, retried max. times for work: " + work); - work.setStep(Step.Done); - } - _haDao.update(work.getId(), work); } @Override diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index acf37444b081..0d0ea010a6c2 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1209,6 +1209,13 @@ public boolean resourceStateTransitTo(final Host host, final ResourceState.Event private boolean doMaintain(final long hostId) { final HostVO host = _hostDao.findById(hostId); + s_logger.info("Maintenance: attempting maintenance of host " + host.getUuid()); + ResourceState hostState = host.getResourceState(); + if (hostState == ResourceState.PrepareForMaintenance || hostState == ResourceState.ErrorInPrepareForMaintenance || + hostState == ResourceState.Maintenance || hostState == ResourceState.ErrorInMaintenance) { + throw new CloudRuntimeException("Cannot perform maintain when resource state is " + hostState + ", hostId = " + hostId); + } + final MaintainAnswer answer = (MaintainAnswer)_agentMgr.easySend(hostId, new MaintainCommand()); if (answer == null || !answer.getResult()) { s_logger.warn("Unable to send MaintainCommand to host: " + hostId); @@ -1240,11 +1247,13 @@ private boolean doMaintain(final long hostId) { || _serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.vgpuType.toString()) != null) { // Migration is not supported for VGPU Vms so stop them. // for the last host in this cluster, stop all the VMs + s_logger.error("Maintenance: No hosts available for migrations. Scheduling shutdown instead of migrations."); _haMgr.scheduleStop(vm, hostId, WorkType.ForceStop); } else if (HypervisorType.LXC.equals(host.getHypervisorType()) && VirtualMachine.Type.User.equals(vm.getType())){ //Migration is not supported for LXC Vms. Schedule restart instead. _haMgr.scheduleRestart(vm, false); } else { + s_logger.info("Maintenance: scheduling migration of VM " + vm.getUuid() + " from host " + host.getUuid()); _haMgr.scheduleMigration(vm); } } @@ -1272,20 +1281,21 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) { throw new InvalidParameterValueException("Unable to find host with ID: " + hostId + ". Please specify a valid host ID."); } - if (_hostDao.countBy(host.getClusterId(), ResourceState.PrepareForMaintenance, ResourceState.ErrorInMaintenance) > 0) { - throw new InvalidParameterValueException("There are other servers in PrepareForMaintenance OR ErrorInMaintenance STATUS in cluster " + host.getClusterId()); + if (_hostDao.countBy(host.getClusterId(), ResourceState.PrepareForMaintenance, ResourceState.ErrorInPrepareForMaintenance) > 0) { + throw new CloudRuntimeException("There are other servers attempting migrations for maintenance. " + + "Found hosts in PrepareForMaintenance OR ErrorInPrepareForMaintenance STATUS in cluster " + host.getClusterId()); } if (_storageMgr.isLocalStorageActiveOnHost(host.getId())) { - throw new InvalidParameterValueException("There are active VMs using the host's local storage pool. Please stop all VMs on this host that use local storage."); + throw new CloudRuntimeException("There are active VMs using the host's local storage pool. Please stop all VMs on this host that use local storage."); } List migratingInVMs = _vmDao.findByHostInStates(hostId, State.Migrating); if (migratingInVMs.size() > 0) { throw new CloudRuntimeException("Host contains incoming VMs migrating. Please wait for them to complete before putting to maintenance."); } - if (_vmDao.findByHostInStates(hostId, State.Starting).size() > 0) { - throw new CloudRuntimeException("Host contains VMs in starting state. Please wait for them to complete before putting to maintenance."); + if (_vmDao.findByHostInStates(hostId, State.Starting, State.Stopping, State.Expunging).size() > 0) { + throw new CloudRuntimeException("Host contains VMs in starting/stopping/expunging state. Please wait for them to complete before putting to maintenance."); } if (_vmDao.findByHostInStates(hostId, State.Error, State.Unknown, State.Shutdowned).size() > 0) { @@ -1388,6 +1398,7 @@ protected boolean setHostIntoPrepareForMaintenanceAfterErrorsFixed(HostVO host) protected boolean attemptMaintain(HostVO host) throws NoTransitionException { final long hostId = host.getId(); + s_logger.info("Attempting maintenance for host " + host.getName()); // Step 1: If there are no VMs in migrating, running, starting, stopping, error or unknown state we can safely move the host to maintenance. if (CollectionUtils.isEmpty(_vmDao.findByHostInStates(host.getId(), State.Migrating, State.Running, State.Starting, State.Stopping, State.Error, State.Unknown))) { return setHostIntoMaintenance(host); @@ -2749,7 +2760,11 @@ public List listAllNotInMaintenanceHostsInOneZone(final Type type, final sc.and(sc.entity().getDataCenterId(), Op.EQ, dcId); } sc.and(sc.entity().getType(), Op.EQ, type); - sc.and(sc.entity().getResourceState(), Op.NIN, ResourceState.Maintenance, ResourceState.ErrorInMaintenance, ResourceState.PrepareForMaintenance, + sc.and(sc.entity().getResourceState(), Op.NIN, + ResourceState.Maintenance, + ResourceState.ErrorInMaintenance, + ResourceState.ErrorInPrepareForMaintenance, + ResourceState.PrepareForMaintenance, ResourceState.Error); return sc.list(); } diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index b2ccfe274a54..ac1ae952caca 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1556,7 +1556,11 @@ protected boolean areAllDiskStatsZero(VmDiskStatsEntry vmDiskStat) { private SearchCriteria createSearchCriteriaForHostTypeRoutingStateUpAndNotInMaintenance() { SearchCriteria sc = _hostDao.createSearchCriteria(); sc.addAnd("status", SearchCriteria.Op.EQ, Status.Up.toString()); - sc.addAnd("resourceState", SearchCriteria.Op.NIN, ResourceState.Maintenance, ResourceState.PrepareForMaintenance, ResourceState.ErrorInMaintenance); + sc.addAnd("resourceState", SearchCriteria.Op.NIN, + ResourceState.Maintenance, + ResourceState.PrepareForMaintenance, + ResourceState.ErrorInPrepareForMaintenance, + ResourceState.ErrorInMaintenance); sc.addAnd("type", SearchCriteria.Op.EQ, Host.Type.Routing.toString()); return sc; } diff --git a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java index 5a6c84f14795..ae9b5c548e56 100644 --- a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java +++ b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java @@ -35,21 +35,16 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; -import com.cloud.resource.ResourceState; +import org.apache.cloudstack.framework.security.keys.KeysManager; import org.apache.commons.codec.binary.Base64; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; import org.springframework.web.context.support.SpringBeanAutowiringSupport; -import com.cloud.vm.VmDetailConstants; -import com.google.gson.Gson; -import com.google.gson.GsonBuilder; - -import org.apache.cloudstack.framework.security.keys.KeysManager; - import com.cloud.exception.PermissionDeniedException; import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor; +import com.cloud.resource.ResourceState; import com.cloud.server.ManagementServer; import com.cloud.storage.GuestOSVO; import com.cloud.user.Account; @@ -64,7 +59,10 @@ import com.cloud.vm.UserVmDetailVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineManager; +import com.cloud.vm.VmDetailConstants; import com.cloud.vm.dao.UserVmDetailsDao; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; /** * Thumbnail access : /console?cmd=thumbnail&vm=xxx&w=xxx&h=xxx @@ -420,14 +418,24 @@ private String composeConsoleAccessUrl(String rootUrl, VirtualMachine vm, HostVO StringBuffer sb = new StringBuffer(rootUrl); String host = hostVo.getPrivateIpAddress(); - Pair portInfo; - if (hostVo.getResourceState().equals(ResourceState.ErrorInMaintenance)) { + Pair portInfo = null; + if (hostVo.getHypervisorType() == Hypervisor.HypervisorType.KVM && + (hostVo.getResourceState().equals(ResourceState.ErrorInMaintenance) || + hostVo.getResourceState().equals(ResourceState.ErrorInPrepareForMaintenance))) { UserVmDetailVO detailAddress = _userVmDetailsDao.findDetail(vm.getId(), VmDetailConstants.KVM_VNC_ADDRESS); UserVmDetailVO detailPort = _userVmDetailsDao.findDetail(vm.getId(), VmDetailConstants.KVM_VNC_PORT); - portInfo = new Pair<>(detailAddress.getValue(), Integer.valueOf(detailPort.getValue())); - } else { + if (detailAddress != null && detailPort != null) { + portInfo = new Pair<>(detailAddress.getValue(), Integer.valueOf(detailPort.getValue())); + } else { + s_logger.warn("KVM Host in ErrorInMaintenance/ErrorInPrepareForMaintenance but " + + "no VNC Address/Port was available. Falling back to default one from MS."); + } + } + + if (portInfo == null) { portInfo = _ms.getVncPort(vm); } + if (s_logger.isDebugEnabled()) s_logger.debug("Port info " + portInfo.first()); diff --git a/test/integration/smoke/test_host_maintenance.py b/test/integration/smoke/test_host_maintenance.py index c856e752db3b..9edc6bd30bcf 100644 --- a/test/integration/smoke/test_host_maintenance.py +++ b/test/integration/smoke/test_host_maintenance.py @@ -285,7 +285,7 @@ def test_02_cancel_host_maintenace_with_migration_jobs(self): for host in listHost: self.logger.debug('2 Hypervisor = {}'.format(host.id)) - if (len(listHost) != 2): + if (len(listHost) < 2): raise unittest.SkipTest("Cancel host maintenance when VMs are migrating can only be tested with 2 hosts"); return @@ -339,7 +339,7 @@ def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): for host in listHost: self.logger.debug('2 Hypervisor = {}'.format(host.id)) - if (len(listHost) != 2): + if (len(listHost) < 2): raise unittest.SkipTest("Cancel host maintenance when VMs are migrating can only be tested with 2 hosts"); return From 714239429b155f309915f837e9c2a87417519b8e Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Fri, 15 Nov 2019 00:05:20 +0530 Subject: [PATCH 09/19] More fixes and logging --- .../cloud/ha/HighAvailabilityManagerImpl.java | 19 +++--- .../cloud/ha/dao/HighAvailabilityDaoImpl.java | 6 +- .../cloud/resource/ResourceManagerImpl.java | 27 ++++++-- .../resource/ResourceManagerImplTest.java | 4 +- .../smoke/test_host_maintenance.py | 61 +++++++++++-------- 5 files changed, 72 insertions(+), 45 deletions(-) diff --git a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java index ee85e1238774..0d801a5c75cb 100644 --- a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java +++ b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java @@ -636,28 +636,30 @@ protected Long restart(final HaWorkVO work) { public Long migrate(final HaWorkVO work) { long vmId = work.getInstanceId(); - long srcHostId = work.getHostId(); + + VMInstanceVO vm = _instanceDao.findById(vmId); + if (vm == null) { + return null; + } + s_logger.info("Migration attempt: for VM " + vm.getUuid() + "from host id " + srcHostId + + ". Try number: " + work.getTimesTried() + " with max retries at " + _maxRetries); try { work.setStep(Step.Migrating); _haDao.update(work.getId(), work); - VMInstanceVO vm = _instanceDao.findById(vmId); - if (vm == null) { - return null; - } // First try starting the vm with its original planner, if it doesn't succeed send HAPlanner as its an emergency. _itMgr.migrateAway(vm.getUuid(), srcHostId); return null; } catch (InsufficientServerCapacityException e) { s_logger.warn("Migration attempt: Insufficient capacity for migrating a VM " + - _instanceDao.findById(vmId).getUuid() + " from source host id " + srcHostId + + vm.getUuid() + " from source host id " + srcHostId + ". Exception: " + e.getMessage()); _resourceMgr.maintenanceFailed(srcHostId); return (System.currentTimeMillis() >> 10) + _migrateRetryInterval; } catch (Exception e) { s_logger.warn("Migration attempt: Unexpected exception occurred when attempting migration of " + - _instanceDao.findById(vmId) + e.getMessage()); + vm.getUuid() + e.getMessage()); throw e; } } @@ -775,7 +777,8 @@ public List findTakenMigrationWork() { } private void rescheduleWork(final HaWorkVO work, final long nextTime) { - s_logger.info("Rescheduling work " + work + " to try again at " + new Date(nextTime << 10)); + s_logger.info("Rescheduling work " + work + " to try again at " + new Date(nextTime << 10) + + ". Already tried " + work.getTimesTried() + 1 + " times with max retries at " + _maxRetries); work.setTimeToTry(nextTime); work.setTimesTried(work.getTimesTried() + 1); work.setServerId(null); diff --git a/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDaoImpl.java b/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDaoImpl.java index a206d2d82d85..56e24c36ec78 100644 --- a/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDaoImpl.java +++ b/server/src/main/java/com/cloud/ha/dao/HighAvailabilityDaoImpl.java @@ -116,7 +116,7 @@ protected HighAvailabilityDaoImpl() { MigratingWorkSearch = createSearchBuilder(); MigratingWorkSearch.and("instance", MigratingWorkSearch.entity().getInstanceId(), Op.EQ); - MigratingWorkSearch.and("type", MigratingWorkSearch.entity().getType(), Op.EQ); + MigratingWorkSearch.and("workType", MigratingWorkSearch.entity().getWorkType(), Op.EQ); MigratingWorkSearch.and("step", MigratingWorkSearch.entity().getStep(), Op.NIN); MigratingWorkSearch.done(); } @@ -133,9 +133,9 @@ public List listPendingHaWorkForVm(long vmId) { @Override public List listPendingMigrationsForVm(long vmId) { - SearchCriteria sc = PendingHaWorkSearch.create(); + SearchCriteria sc = MigratingWorkSearch.create(); sc.setParameters("instance", vmId); - sc.setParameters("type", WorkType.Migration); + sc.setParameters("workType", WorkType.Migration); sc.setParameters("step", Step.Done, Step.Error, Step.Cancelled); return search(sc, null); diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 0d0ea010a6c2..75e68b492c76 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1162,6 +1162,13 @@ public Host cancelMaintenance(final CancelMaintenanceCmd cmd) { throw new InvalidParameterValueException("Host with id " + hostId.toString() + " doesn't exist"); } + if (host.getResourceState() != ResourceState.PrepareForMaintenance && + host.getResourceState() != ResourceState.ErrorInPrepareForMaintenance && + host.getResourceState() != ResourceState.Maintenance && + host.getResourceState() != ResourceState.ErrorInMaintenance) { + throw new CloudRuntimeException("Cannot perform cancelMaintenance when resource state is " + host.getResourceState() + ", hostId = " + hostId); + } + processResourceEvent(ResourceListener.EVENT_CANCEL_MAINTENANCE_BEFORE, hostId); final boolean success = cancelMaintenance(hostId); processResourceEvent(ResourceListener.EVENT_CANCEL_MAINTENANCE_AFTER, hostId); @@ -1281,6 +1288,12 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) { throw new InvalidParameterValueException("Unable to find host with ID: " + hostId + ". Please specify a valid host ID."); } + final ResourceState hostState = host.getResourceState(); + if (hostState == ResourceState.Maintenance || hostState == ResourceState.PrepareForMaintenance || + hostState == ResourceState.ErrorInMaintenance || hostState == ResourceState.ErrorInPrepareForMaintenance) { + throw new CloudRuntimeException("Host is already in state " + hostState + ". Cannot recall for maintenance until resolved."); + } + if (_hostDao.countBy(host.getClusterId(), ResourceState.PrepareForMaintenance, ResourceState.ErrorInPrepareForMaintenance) > 0) { throw new CloudRuntimeException("There are other servers attempting migrations for maintenance. " + "Found hosts in PrepareForMaintenance OR ErrorInPrepareForMaintenance STATUS in cluster " + host.getClusterId()); @@ -1400,7 +1413,8 @@ protected boolean attemptMaintain(HostVO host) throws NoTransitionException { s_logger.info("Attempting maintenance for host " + host.getName()); // Step 1: If there are no VMs in migrating, running, starting, stopping, error or unknown state we can safely move the host to maintenance. - if (CollectionUtils.isEmpty(_vmDao.findByHostInStates(host.getId(), State.Migrating, State.Running, State.Starting, State.Stopping, State.Error, State.Unknown))) { + if (CollectionUtils.isEmpty(_vmDao.findByHostInStates(host.getId(), + State.Migrating, State.Running, State.Starting, State.Stopping, State.Error, State.Unknown))) { return setHostIntoMaintenance(host); } @@ -1415,8 +1429,8 @@ protected boolean attemptMaintain(HostVO host) throws NoTransitionException { } final List failedMigrations = new ArrayList<>(_vmDao.listNonMigratingVmsByHostEqualsLastHost(hostId)); - final List errorVms = new ArrayList<>(_vmDao.findByHostInStates(hostId, State.Unknown, State.Error, State.Shutdowned)); - final boolean hasMigratingVms = CollectionUtils.isNotEmpty(_vmDao.listVmsMigratingFromHost(hostId)); + final List errorVms = new ArrayList<>(_vmDao.findByHostInStates(hostId, State.Unknown, State.Error)); + final boolean hasMigratingAwayVms = CollectionUtils.isNotEmpty(_vmDao.listVmsMigratingFromHost(hostId)); final boolean hasRunningVms = CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Running)); final boolean hasFailedMigrations = CollectionUtils.isNotEmpty(failedMigrations); final boolean hasVmsInFailureStates = CollectionUtils.isNotEmpty(errorVms); @@ -1425,13 +1439,14 @@ protected boolean attemptMaintain(HostVO host) throws NoTransitionException { // Step 3: If there are no pending migration retries but host still has running VMs or, // host has VMs in failure state / failed migrations we move the host to ErrorInMaintenance state. - if (!hasPendingMigrationRetries && (hasRunningVms || (!hasMigratingVms && hasVmsInFailureStates))) { + if ((!hasPendingMigrationRetries && !hasMigratingAwayVms && hasRunningVms) || + (!hasRunningVms && !hasMigratingAwayVms && hasVmsInFailureStates)) { return setHostIntoErrorInMaintenance(host, errorVms); } // Step 4: IF there are pending migrations or ongoing retries left or stopping VMs and there were errors or failed // migrations we put the host into ErrorInPrepareForMaintenance - if ((hasPendingMigrationRetries || hasMigratingVms || hasStoppingVms) && (hasVmsInFailureStates || hasFailedMigrations)) { + if ((hasPendingMigrationRetries || hasMigratingAwayVms || hasStoppingVms) && (hasVmsInFailureStates || hasFailedMigrations)) { return setHostIntoErrorInPrepareForMaintenance(host, errorVms); } @@ -1454,7 +1469,7 @@ public boolean checkAndMaintain(final long hostId) { hostInMaintenance = attemptMaintain(host); } } catch (final NoTransitionException e) { - s_logger.debug("Cannot transmit host " + host.getId() + "to Maintenance state", e); + s_logger.debug("Cannot transmit host " + host.getId() + " to Maintenance state", e); } return hostInMaintenance; } diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index b0c80b82f755..6faa83bc9107 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -217,6 +217,7 @@ public void testCheckAndMaintainErrorInPrepareForMaintenanceFailedMigrationsPend // Test entering into ErrorInPrepareForMaintenance when pending migrations retries and due to - Failed Migrations setupFailedMigrations(); setupPendingMigrationRetries(); + when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Running)).thenReturn(Arrays.asList(vm2)); verifyErrorInPrepareForMaintenanceCalls(); } @@ -225,6 +226,7 @@ public void testCheckAndMaintainErrorInPrepareForMaintenanceWithErrorVmsPendingR // Test entering into ErrorInMaintenance when pending migrations retries due to - no migrating but error VMs setupErrorVms(); setupPendingMigrationRetries(); + when(vmInstanceDao.listVmsMigratingFromHost(hostId)).thenReturn(Arrays.asList(vm2)); verifyErrorInPrepareForMaintenanceCalls(); } @@ -379,7 +381,7 @@ private void setupFailedMigrations() { private void setupErrorVms() { when(vmInstanceDao.listByHostId(hostId)).thenReturn(Arrays.asList(vm1, vm2)); when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Migrating, VirtualMachine.State.Running, VirtualMachine.State.Starting, VirtualMachine.State.Stopping, VirtualMachine.State.Error, VirtualMachine.State.Unknown)).thenReturn(Arrays.asList(vm1, vm2)); - when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Unknown, VirtualMachine.State.Error, VirtualMachine.State.Shutdowned)).thenReturn(Arrays.asList(vm1)); + when(vmInstanceDao.findByHostInStates(hostId, VirtualMachine.State.Unknown, VirtualMachine.State.Error)).thenReturn(Arrays.asList(vm1)); } private void verifyErrorInMaintenanceCalls() throws NoTransitionException { diff --git a/test/integration/smoke/test_host_maintenance.py b/test/integration/smoke/test_host_maintenance.py index 9edc6bd30bcf..165f6f4fc6cd 100644 --- a/test/integration/smoke/test_host_maintenance.py +++ b/test/integration/smoke/test_host_maintenance.py @@ -44,10 +44,16 @@ def setUp(self): self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests()) self.pod = get_pod(self.apiclient, self.zone.id) self.cleanup = [] + self.ssh_client = None + self.needs_unblock_iptables = False def tearDown(self): try: - # Clean up, terminate the created templates + if self.ssh_client is not None and self.needs_unblock_iptables: + self.ssh_client.execute("iptables -D OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") + self.ssh_client.execute("iptables -D OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") + + # Clean up, terminate the created templates cleanup_resources(self.apiclient, self.cleanup) except Exception as e: @@ -168,14 +174,15 @@ def noOfVMsOnHost(self, hostId): hostid=hostId ) no_of_vms=0 + self.logger.debug("Counting VMs on host " + hostId) if (listVms is not None): for vm in listVms: self.logger.debug('VirtualMachine on Hyp 1 = {}'.format(vm.id)) no_of_vms=no_of_vms+1 - + self.logger.debug("Found VMs on host " + str(no_of_vms)) return no_of_vms - def wait_until_host_is_in_state(self, hostid, resourcestate, interval=2, retries=100): + def wait_until_host_is_in_state(self, hostid, resourcestate, interval=3, retries=200): def check_resource_state(): response = Host.list( self.apiclient, @@ -183,14 +190,16 @@ def check_resource_state(): ) if isinstance(response, list): if response[0].resourcestate == resourcestate: - self.logger.debug('Host with id %s is in resource state = %s' % (hostid, resourcestate)) + self.logger.debug("Host with id " + hostid + " has reached resource state = " + resourcestate) return True, None + self.logger.debug("Waiting for host with id " + hostid + " to reach resource state = " + + resourcestate + " from state " + response[0].resourcestate) return False, None done, _ = wait_until(interval, retries, check_resource_state) if not done: - raise Exception("Failed to wait for host %s to be on resource state %s" % (hostid, resourcestate)) - return True + self.logger.error("Failed to wait for host " + hostid + " to be on resource state " + resourcestate) + return done def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id): # Wait for all VMs to complete any pending migrations. @@ -239,30 +248,30 @@ def test_01_cancel_host_maintenace_with_no_migration_jobs(self): ) for host in listHost: self.logger.debug('1 Hypervisor = {}'.format(host.id)) - - + + if (len(listHost) < 2): raise unittest.SkipTest("Cancel host maintenance when VMs are migrating should be tested for 2 or more hosts"); return migrations_finished = True - + try: migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[0].id, listHost[1].id) if migrations_finished: migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id) - + except Exception as e: self.logger.debug("Exception {}".format(e)) self.fail("Cancel host maintenance failed {}".format(e[0])) - + if (migrations_finished == False): raise unittest.SkipTest("VMs are still migrating and the test will not be able to check the conditions the test is intended for"); - - + + return @attr( @@ -275,7 +284,7 @@ def test_01_cancel_host_maintenace_with_no_migration_jobs(self): "sg"], required_hardware="true") def test_02_cancel_host_maintenace_with_migration_jobs(self): - + listHost = Host.list( self.apiclient, type='Routing', @@ -284,31 +293,31 @@ def test_02_cancel_host_maintenace_with_migration_jobs(self): ) for host in listHost: self.logger.debug('2 Hypervisor = {}'.format(host.id)) - + if (len(listHost) < 2): raise unittest.SkipTest("Cancel host maintenance when VMs are migrating can only be tested with 2 hosts"); return - + no_of_vms = self.noOfVMsOnHost(listHost[0].id) - + no_of_vms = no_of_vms + self.noOfVMsOnHost(listHost[1].id) - + if no_of_vms < 5: self.logger.debug("Create VMs as there are not enough vms to check host maintenance") no_vm_req = 5 - no_of_vms if (no_vm_req > 0): self.logger.debug("Creating vms = {}".format(no_vm_req)) self.vmlist = self.createVMs(listHost[0].id, no_vm_req) - + migrations_finished = True - + try: migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[0].id, listHost[1].id) if migrations_finished: migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id) - + except Exception as e: self.logger.debug("Exception {}".format(e)) self.fail("Cancel host maintenance failed {}".format(e[0])) @@ -358,9 +367,10 @@ def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): migrations_finished = True - ssh_client = self.get_ssh_client(listHost[1].ipaddress) - ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") - ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") + self.ssh_client = self.get_ssh_client(listHost[0].ipaddress) + self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") + self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") + self.needs_unblock_iptables = True cmd = prepareHostForMaintenance.prepareHostForMaintenanceCmd() cmd.id = target_host_id @@ -378,9 +388,6 @@ def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): self.logger.debug('Host with id {} has been sent to cancelHostMaintenance'.format(target_host_id)) - ssh_client.execute("iptables -I OUTPUT -j ACCEPT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'open port for migrations'") - ssh_client.execute("iptables -I OUTPUT -j ACCEPT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'open port for migrations'") - if error_in_maintenance_reached == False: self.fail("Error in maintenance state should have reached after ports block") From f6704fcfcda59def12298bc1c50fd9803bfad097 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Fri, 15 Nov 2019 00:39:24 +0530 Subject: [PATCH 10/19] UI fixes --- ui/scripts/metrics.js | 1 + ui/scripts/system.js | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ui/scripts/metrics.js b/ui/scripts/metrics.js index 2784eab2d8d4..da95b98e03b0 100644 --- a/ui/scripts/metrics.js +++ b/ui/scripts/metrics.js @@ -679,6 +679,7 @@ 'Down': 'off', 'Removed': 'off', 'ErrorInMaintenance': 'off', + 'ErrorInPrepareForMaintenance': 'warning', 'PrepareForMaintenance': 'warning', 'CancelMaintenance': 'warning', 'Maintenance': 'warning', diff --git a/ui/scripts/system.js b/ui/scripts/system.js index f6ef03ca9d3e..c53bbf181115 100755 --- a/ui/scripts/system.js +++ b/ui/scripts/system.js @@ -17162,7 +17162,8 @@ title: 'label.outofbandmanagement.action.issue', desc: function(args) { var host = args.context.hosts[0]; - if (host.resourcestate == 'Maintenance' || host.resourcestate == 'PrepareForMaintenance' || host.resourcestate == 'ErrorInMaintenance') { + if (host.resourcestate == 'Maintenance' || host.resourcestate == 'PrepareForMaintenance' || + host.resourcestate == 'ErrorInPrepareForMaintenance' || host.resourcestate == 'ErrorInMaintenance') { return _l('message.outofbandmanagement.action.maintenance'); } }, @@ -17776,6 +17777,7 @@ 'Down': 'off', 'Removed': 'off', 'ErrorInMaintenance': 'off', + 'ErrorInPrepareForMaintenance': 'warning', 'PrepareForMaintenance': 'warning', 'CancelMaintenance': 'warning', 'Maintenance': 'warning', @@ -21975,7 +21977,7 @@ allowedActions.push("edit"); allowedActions.push("enableMaintenanceMode"); allowedActions.push("cancelMaintenanceMode"); - } else if (jsonObj.resourcestate == "PrepareForMaintenance") { + } else if (jsonObj.resourcestate == "PrepareForMaintenance" || jsonObj.resourcestate == 'ErrorInPrepareForMaintenance') { allowedActions.push("edit"); allowedActions.push("cancelMaintenanceMode"); } else if (jsonObj.resourcestate == "Maintenance") { @@ -22029,7 +22031,7 @@ } else if (jsonObj.state == "ErrorInMaintenance") { allowedActions.push("enableMaintenanceMode"); allowedActions.push("cancelMaintenanceMode"); - } else if (jsonObj.state == "PrepareForMaintenance") { + } else if (jsonObj.state == "PrepareForMaintenance" || jsonObj.resourcestate == "ErrorInPrepareForMaintenance") { allowedActions.push("cancelMaintenanceMode"); } else if (jsonObj.state == "Maintenance") { allowedActions.push("cancelMaintenanceMode"); From 438c800cb819d7fb4b1726ea88c448c6e297b7af Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Mon, 18 Nov 2019 16:19:12 +0530 Subject: [PATCH 11/19] Some minor changes and reducing VMs on host for more contained tests --- .../cloud/ha/HighAvailabilityManagerImpl.java | 4 +-- .../cloud/resource/ResourceManagerImpl.java | 8 +++--- .../smoke/test_host_maintenance.py | 28 ++++++++++++++----- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java index 0d801a5c75cb..cfdc57193b54 100644 --- a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java +++ b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java @@ -643,7 +643,7 @@ public Long migrate(final HaWorkVO work) { return null; } s_logger.info("Migration attempt: for VM " + vm.getUuid() + "from host id " + srcHostId + - ". Try number: " + work.getTimesTried() + " with max retries at " + _maxRetries); + ". Retry count: " + work.getTimesTried() + "/" + _maxRetries + " times."); try { work.setStep(Step.Migrating); _haDao.update(work.getId(), work); @@ -778,7 +778,7 @@ public List findTakenMigrationWork() { private void rescheduleWork(final HaWorkVO work, final long nextTime) { s_logger.info("Rescheduling work " + work + " to try again at " + new Date(nextTime << 10) + - ". Already tried " + work.getTimesTried() + 1 + " times with max retries at " + _maxRetries); + ". Retry count " + work.getTimesTried() + "/" + _maxRetries + " times."); work.setTimeToTry(nextTime); work.setTimesTried(work.getTimesTried() + 1); work.setServerId(null); diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 75e68b492c76..1fcaa9ef75c1 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1290,7 +1290,7 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) { final ResourceState hostState = host.getResourceState(); if (hostState == ResourceState.Maintenance || hostState == ResourceState.PrepareForMaintenance || - hostState == ResourceState.ErrorInMaintenance || hostState == ResourceState.ErrorInPrepareForMaintenance) { + hostState == ResourceState.ErrorInPrepareForMaintenance) { throw new CloudRuntimeException("Host is already in state " + hostState + ". Cannot recall for maintenance until resolved."); } @@ -1307,11 +1307,11 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) { throw new CloudRuntimeException("Host contains incoming VMs migrating. Please wait for them to complete before putting to maintenance."); } - if (_vmDao.findByHostInStates(hostId, State.Starting, State.Stopping, State.Expunging).size() > 0) { - throw new CloudRuntimeException("Host contains VMs in starting/stopping/expunging state. Please wait for them to complete before putting to maintenance."); + if (_vmDao.findByHostInStates(hostId, State.Starting, State.Stopping).size() > 0) { + throw new CloudRuntimeException("Host contains VMs in starting/stopping state. Please wait for them to complete before putting to maintenance."); } - if (_vmDao.findByHostInStates(hostId, State.Error, State.Unknown, State.Shutdowned).size() > 0) { + if (_vmDao.findByHostInStates(hostId, State.Error, State.Unknown).size() > 0) { throw new CloudRuntimeException("Host contains VMs in error/unknown/shutdown state. Please fix errors to proceed."); } diff --git a/test/integration/smoke/test_host_maintenance.py b/test/integration/smoke/test_host_maintenance.py index 165f6f4fc6cd..978436db61ff 100644 --- a/test/integration/smoke/test_host_maintenance.py +++ b/test/integration/smoke/test_host_maintenance.py @@ -28,6 +28,7 @@ from marvin.sshClient import SshClient _multiprocess_shared_ = False +MIN_VMS_FOR_TEST = 3 class TestHostMaintenance(cloudstackTestCase): @@ -113,18 +114,27 @@ def createVMs(self, hostId, number): self.cleanup.append(self.service_offering) return vms - def checkAllVmsRunningOnHost(self, hostId): + def checkAllVmsRunningOnHost(self, data): + hostId = data["hostId"] + expectedNumVms = data["expected_vms"] if "expected_vms" in data else -1 listVms1 = VirtualMachine.list( self.apiclient, hostid=hostId ) + runningVms = 0 if (listVms1 is not None): self.logger.debug('Vms found to test all running = {} '.format(len(listVms1))) for vm in listVms1: if (vm.state != "Running"): self.logger.debug('VirtualMachine on Host with id = {} is in {}'.format(vm.id, vm.state)) return (False, None) + else: + runningVms = runningVms + 1 + if expectedNumVms != -1: + if expectedNumVms != runningVms: + return (False, None) + response = list_ssvms( self.apiclient, @@ -177,7 +187,7 @@ def noOfVMsOnHost(self, hostId): self.logger.debug("Counting VMs on host " + hostId) if (listVms is not None): for vm in listVms: - self.logger.debug('VirtualMachine on Hyp 1 = {}'.format(vm.id)) + self.logger.debug("VirtualMachine on Host " + hostId + " = " + vm.id) no_of_vms=no_of_vms+1 self.logger.debug("Found VMs on host " + str(no_of_vms)) return no_of_vms @@ -203,7 +213,8 @@ def check_resource_state(): def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id): # Wait for all VMs to complete any pending migrations. - if not wait_until(2, 60, self.checkAllVmsRunningOnHost, target_host_id) or not wait_until(2, 60, self.checkAllVmsRunningOnHost, other_host_id): + if not wait_until(3, 100, self.checkAllVmsRunningOnHost, {"hostId" : target_host_id}) or \ + not wait_until(3, 100, self.checkAllVmsRunningOnHost, {"hostId": other_host_id}): raise Exception("Failed to wait for all VMs to reach running state to execute test") expected_vm_count_after_maintenance = self.noOfVMsOnHost(target_host_id) + self.noOfVMsOnHost(other_host_id) @@ -214,8 +225,11 @@ def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id): self.logger.debug('Host with id {} is in prepareHostForMaintenance'.format(target_host_id)) - migrations_finished = wait_until(2, 100, self.migrationsFinished, target_host_id) - wait_until(2, 60, self.checkAllVmsRunningOnHost, other_host_id) + migrations_finished = wait_until(3, 200, self.migrationsFinished, target_host_id) + wait_until(3, 200, self.checkAllVmsRunningOnHost, { + "hostId": other_host_id, + "expected_vms": expected_vm_count_after_maintenance + }) other_vm_count_after_maintenance = self.noOfVMsOnHost(other_host_id) self.logger.debug('Canceling Host with id {} from maintain'.format(target_host_id)) @@ -303,9 +317,9 @@ def test_02_cancel_host_maintenace_with_migration_jobs(self): no_of_vms = no_of_vms + self.noOfVMsOnHost(listHost[1].id) - if no_of_vms < 5: + if no_of_vms < MIN_VMS_FOR_TEST: self.logger.debug("Create VMs as there are not enough vms to check host maintenance") - no_vm_req = 5 - no_of_vms + no_vm_req = MIN_VMS_FOR_TEST - no_of_vms if (no_vm_req > 0): self.logger.debug("Creating vms = {}".format(no_vm_req)) self.vmlist = self.createVMs(listHost[0].id, no_vm_req) From 7b043624213c074a3be6cee1ceff5d990c44acca Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Tue, 19 Nov 2019 13:50:42 +0530 Subject: [PATCH 12/19] Fixed ssh client auth problem causing test failure --- .../smoke/test_host_maintenance.py | 119 ++++++------------ 1 file changed, 40 insertions(+), 79 deletions(-) diff --git a/test/integration/smoke/test_host_maintenance.py b/test/integration/smoke/test_host_maintenance.py index 978436db61ff..35556520e673 100644 --- a/test/integration/smoke/test_host_maintenance.py +++ b/test/integration/smoke/test_host_maintenance.py @@ -30,8 +30,37 @@ _multiprocess_shared_ = False MIN_VMS_FOR_TEST = 3 +class TestHostMaintenanceBase(cloudstackTestCase): + def get_ssh_client(self, ip, username, password, retries=10): + """ Setup ssh client connection and return connection """ + try: + ssh_client = SshClient(ip, 22, username, password, retries) + except Exception as e: + raise unittest.SkipTest("Unable to create ssh connection: " % e) -class TestHostMaintenance(cloudstackTestCase): + self.assertIsNotNone( + ssh_client, "Failed to setup ssh connection to ip=%s" % ip) + + return ssh_client + + def wait_until_host_is_in_state(self, hostid, resourcestate, interval=3, retries=20): + def check_resource_state(): + response = Host.list( + self.apiclient, + id=hostid + ) + if isinstance(response, list): + if response[0].resourcestate == resourcestate: + self.logger.debug('Host with id %s is in resource state = %s' % (hostid, resourcestate)) + return True, None + return False, None + + done, _ = wait_until(interval, retries, check_resource_state) + if not done: + raise Exception("Failed to wait for host %s to be on resource state %s" % (hostid, resourcestate)) + return True + +class TestHostMaintenance(TestHostMaintenanceBase): def setUp(self): self.logger = logging.getLogger('TestHM') @@ -47,6 +76,8 @@ def setUp(self): self.cleanup = [] self.ssh_client = None self.needs_unblock_iptables = False + self.hostConfig = self.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][0].__dict__ + def tearDown(self): try: @@ -192,25 +223,6 @@ def noOfVMsOnHost(self, hostId): self.logger.debug("Found VMs on host " + str(no_of_vms)) return no_of_vms - def wait_until_host_is_in_state(self, hostid, resourcestate, interval=3, retries=200): - def check_resource_state(): - response = Host.list( - self.apiclient, - id=hostid - ) - if isinstance(response, list): - if response[0].resourcestate == resourcestate: - self.logger.debug("Host with id " + hostid + " has reached resource state = " + resourcestate) - return True, None - self.logger.debug("Waiting for host with id " + hostid + " to reach resource state = " + - resourcestate + " from state " + response[0].resourcestate) - return False, None - - done, _ = wait_until(interval, retries, check_resource_state) - if not done: - self.logger.error("Failed to wait for host " + hostid + " to be on resource state " + resourcestate) - return done - def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id): # Wait for all VMs to complete any pending migrations. if not wait_until(3, 100, self.checkAllVmsRunningOnHost, {"hostId" : target_host_id}) or \ @@ -265,8 +277,7 @@ def test_01_cancel_host_maintenace_with_no_migration_jobs(self): if (len(listHost) < 2): - raise unittest.SkipTest("Cancel host maintenance when VMs are migrating should be tested for 2 or more hosts"); - return + raise unittest.SkipTest("Cancel host maintenance when VMs are migrating should be tested for 2 or more hosts") migrations_finished = True @@ -282,12 +293,10 @@ def test_01_cancel_host_maintenace_with_no_migration_jobs(self): self.fail("Cancel host maintenance failed {}".format(e[0])) - if (migrations_finished == False): - raise unittest.SkipTest("VMs are still migrating and the test will not be able to check the conditions the test is intended for"); + if not migrations_finished: + raise unittest.SkipTest("VMs are still migrating and the test will not be able to check the conditions the test is intended for") - return - @attr( tags=[ "advanced", @@ -309,9 +318,7 @@ def test_02_cancel_host_maintenace_with_migration_jobs(self): self.logger.debug('2 Hypervisor = {}'.format(host.id)) if (len(listHost) < 2): - raise unittest.SkipTest("Cancel host maintenance when VMs are migrating can only be tested with 2 hosts"); - return - + raise unittest.SkipTest("Cancel host maintenance when VMs are migrating can only be tested with 2 hosts") no_of_vms = self.noOfVMsOnHost(listHost[0].id) @@ -338,9 +345,7 @@ def test_02_cancel_host_maintenace_with_migration_jobs(self): if (migrations_finished == False): - raise unittest.SkipTest("VMs are still migrating and the test will not be able to check the conditions the test is intended for"); - - return + raise unittest.SkipTest("VMs are still migrating and the test will not be able to check the conditions the test is intended for") @attr( tags=[ @@ -379,9 +384,7 @@ def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): self.logger.debug("Creating vms = {}".format(no_vm_req)) self.vmlist = self.createVMs(listHost[0].id, no_vm_req) - migrations_finished = True - - self.ssh_client = self.get_ssh_client(listHost[0].ipaddress) + self.ssh_client = self.get_ssh_client(listHost[0].ipaddress, self.hostConfig["username"], self.hostConfig["password"]) self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") self.needs_unblock_iptables = True @@ -393,7 +396,7 @@ def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): self.logger.debug('Attempting to put host with id {} in Maintenance'.format(target_host_id)) - error_in_maintenance_reached = self.wait_until_host_is_in_state(target_host_id, "ErrorInMaintenance") + error_in_maintenance_reached = self.wait_until_host_is_in_state(target_host_id, "ErrorInMaintenance", 5, 200) self.logger.debug('Canceling Host with id {} from maintain'.format(target_host_id)) cmd = cancelHostMaintenance.cancelHostMaintenanceCmd() @@ -405,20 +408,8 @@ def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): if error_in_maintenance_reached == False: self.fail("Error in maintenance state should have reached after ports block") - return - - def get_ssh_client(self, ip, username="root", password="password", retries=10): - try: - ssh_client = SshClient(ip, 22, username, password, retries) - except Exception as e: - raise unittest.SkipTest("Unable to create ssh connection: " % e) - - self.assertIsNotNone( - ssh_client, "Failed to setup ssh connection to ip=%s" % ip) - - return ssh_client -class TestHostMaintenanceAgents(cloudstackTestCase): +class TestHostMaintenanceAgents(TestHostMaintenanceBase): @classmethod def setUpClass(cls): @@ -521,23 +512,6 @@ def prepare_host_for_maintenance(self, hostid): self.apiclient.prepareHostForMaintenance(cmd) self.logger.debug('Host with id %s is in prepareHostForMaintenance' % hostid) - def wait_until_host_is_in_state(self, hostid, resourcestate, interval=3, retries=20): - def check_resource_state(): - response = Host.list( - self.apiclient, - id=hostid - ) - if isinstance(response, list): - if response[0].resourcestate == resourcestate: - self.logger.debug('Host with id %s is in resource state = %s' % (hostid, resourcestate)) - return True, None - return False, None - - done, _ = wait_until(interval, retries, check_resource_state) - if not done: - raise Exception("Failed to wait for host %s to be on resource state %s" % (hostid, resourcestate)) - return True - def wait_until_agent_is_in_state(self, hostid, state, interval=3, retries=20): def check_agent_state(): response = Host.list( @@ -627,19 +601,6 @@ def test_01_cancel_host_maintenance_ssh_enabled_agent_connected(self): self.revert_host_state_on_failure(self.host) self.fail(e) - def get_ssh_client(self, ip, username, password, retries=10): - """ Setup ssh client connection and return connection """ - - try: - ssh_client = SshClient(ip, 22, username, password, retries) - except Exception as e: - raise unittest.SkipTest("Unable to create ssh connection: " % e) - - self.assertIsNotNone( - ssh_client, "Failed to setup ssh connection to ip=%s" % ip) - - return ssh_client - @skipTestIf("hypervisorNotSupported") @attr(tags=["boris", "advancedns", "smoke", "basic", "eip", "sg"], required_hardware="true") def test_02_cancel_host_maintenance_ssh_enabled_agent_disconnected(self): From 43f592a4e7c9149baba034fd083e4dc8986f3a7b Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Mon, 25 Nov 2019 22:34:56 +0530 Subject: [PATCH 13/19] Code review changes + fixes + some more logging --- .../com/cloud/resource/ResourceState.java | 12 +++- .../com/cloud/resource/ResourceManager.java | 2 +- .../entity/api/db/EngineHostVO.java | 4 +- .../src/main/java/com/cloud/host/HostVO.java | 4 +- .../db/schema-41300to41400-cleanup.sql | 1 + .../cloud/ha/HighAvailabilityManagerImpl.java | 26 ++++--- .../cloud/resource/ResourceManagerImpl.java | 20 +++--- .../resource/MockResourceManagerImpl.java | 6 +- .../smoke/test_host_maintenance.py | 69 +++++++++---------- 9 files changed, 74 insertions(+), 70 deletions(-) diff --git a/api/src/main/java/com/cloud/resource/ResourceState.java b/api/src/main/java/com/cloud/resource/ResourceState.java index a296bf2ec091..51c008ec8cb4 100644 --- a/api/src/main/java/com/cloud/resource/ResourceState.java +++ b/api/src/main/java/com/cloud/resource/ResourceState.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.resource; +import java.util.Arrays; import java.util.List; import java.util.Set; @@ -39,7 +40,7 @@ public enum Event { AdminCancelMaintenance("Admin asks to cancel maintenance"), InternalEnterMaintenance("Resource enters maintenance"), UpdatePassword("Admin updates password of host"), - UnableToMigrate("Management server migrates VM failed"), + UnableToMigrate("Migration of VM failed, such as from scheduled HAWork"), UnableToMaintain("Management server has exhausted all legal operations and attempts to put into maintenance has failed"), ErrorsCorrected("Errors were corrected on a resource attempting to enter maintenance but encountered errors"), Error("An internal error happened"), @@ -93,6 +94,11 @@ public static String[] toString(ResourceState... states) { return strs; } + public static boolean isMaintenanceState(ResourceState state) { + return Arrays.asList(ResourceState.Maintenance, ResourceState.ErrorInMaintenance, + ResourceState.PrepareForMaintenance, ResourceState.ErrorInPrepareForMaintenance).contains(state); + } + protected static final StateMachine s_fsm = new StateMachine(); static { s_fsm.addTransition(null, Event.InternalCreated, ResourceState.Enabled); @@ -114,12 +120,14 @@ public static String[] toString(ResourceState... states) { s_fsm.addTransition(ResourceState.Maintenance, Event.AdminCancelMaintenance, ResourceState.Enabled); s_fsm.addTransition(ResourceState.Maintenance, Event.InternalCreated, ResourceState.Maintenance); s_fsm.addTransition(ResourceState.Maintenance, Event.DeleteHost, ResourceState.Disabled); + s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.InternalCreated, ResourceState.ErrorInPrepareForMaintenance); + s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.Disable, ResourceState.Disabled); + s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.DeleteHost, ResourceState.Disabled); s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.InternalEnterMaintenance, ResourceState.Maintenance); s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.AdminCancelMaintenance, ResourceState.Enabled); s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.UnableToMigrate, ResourceState.ErrorInPrepareForMaintenance); s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.UnableToMaintain, ResourceState.ErrorInMaintenance); s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.ErrorsCorrected, ResourceState.PrepareForMaintenance); - s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.InternalCreated, ResourceState.ErrorInPrepareForMaintenance); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.InternalCreated, ResourceState.ErrorInMaintenance); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.Disable, ResourceState.Disabled); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.DeleteHost, ResourceState.Disabled); diff --git a/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java b/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java index e7c064fbb923..387fa7f6415b 100755 --- a/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java +++ b/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java @@ -92,7 +92,7 @@ public interface ResourceManager extends ResourceService, Configurable { boolean umanageHost(long hostId); - boolean maintenanceFailed(long hostId); + boolean migrateAwayFailed(long hostId, long vmId); public boolean maintain(final long hostId) throws AgentUnavailableException; diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/EngineHostVO.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/EngineHostVO.java index c48def27c5c8..846b4157786d 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/EngineHostVO.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/datacenter/entity/api/db/EngineHostVO.java @@ -732,9 +732,7 @@ public boolean isDisabled() { @Override public boolean isInMaintenanceStates() { - ResourceState state = getResourceState(); - return (state == ResourceState.Maintenance || state == ResourceState.ErrorInMaintenance || - state == ResourceState.PrepareForMaintenance || state == ResourceState.ErrorInPrepareForMaintenance); + return ResourceState.isMaintenanceState(getResourceState()); } public long getUpdated() { diff --git a/engine/schema/src/main/java/com/cloud/host/HostVO.java b/engine/schema/src/main/java/com/cloud/host/HostVO.java index 0eb32b68c774..f23435945d50 100644 --- a/engine/schema/src/main/java/com/cloud/host/HostVO.java +++ b/engine/schema/src/main/java/com/cloud/host/HostVO.java @@ -715,9 +715,7 @@ public void setResourceState(ResourceState state) { @Override public boolean isInMaintenanceStates() { - ResourceState state = getResourceState(); - return (state == ResourceState.Maintenance || state == ResourceState.ErrorInMaintenance || - state == ResourceState.PrepareForMaintenance || state == ResourceState.ErrorInPrepareForMaintenance); + return ResourceState.isMaintenanceState(getResourceState()); } @Override public boolean isDisabled() { diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41300to41400-cleanup.sql b/engine/schema/src/main/resources/META-INF/db/schema-41300to41400-cleanup.sql index 57c4a611f0e7..fe84077f72ba 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41300to41400-cleanup.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41300to41400-cleanup.sql @@ -19,3 +19,4 @@ -- Schema upgrade cleanup from 4.13.0.0 to 4.14.0.0 --; +DELETE FROM `cloud`.`configuration` WHERE name = 'host.maintenance.retries'; diff --git a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java index cfdc57193b54..16a9bc3ccc47 100644 --- a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java +++ b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java @@ -321,6 +321,7 @@ public boolean scheduleMigration(final VMInstanceVO vm) { if (vm.getHostId() != null) { final HaWorkVO work = new HaWorkVO(vm.getId(), vm.getType(), WorkType.Migration, Step.Scheduled, vm.getHostId(), vm.getState(), 0, vm.getUpdated()); _haDao.persist(work); + s_logger.info("Scheduled migration work of VM " + vm.getUuid() + " from host " + vm.getHostName() + " with HAWork " + work); wakeupWorkers(); } return true; @@ -640,10 +641,11 @@ public Long migrate(final HaWorkVO work) { VMInstanceVO vm = _instanceDao.findById(vmId); if (vm == null) { + s_logger.info("Unable to find vm: " + vmId + ", skipping migrate."); return null; } s_logger.info("Migration attempt: for VM " + vm.getUuid() + "from host id " + srcHostId + - ". Retry count: " + work.getTimesTried() + "/" + _maxRetries + " times."); + ". Starting attempt: " + (1 + work.getTimesTried()) + "/" + _maxRetries + " times."); try { work.setStep(Step.Migrating); _haDao.update(work.getId(), work); @@ -655,7 +657,7 @@ public Long migrate(final HaWorkVO work) { s_logger.warn("Migration attempt: Insufficient capacity for migrating a VM " + vm.getUuid() + " from source host id " + srcHostId + ". Exception: " + e.getMessage()); - _resourceMgr.maintenanceFailed(srcHostId); + _resourceMgr.migrateAwayFailed(srcHostId, vmId); return (System.currentTimeMillis() >> 10) + _migrateRetryInterval; } catch (Exception e) { s_logger.warn("Migration attempt: Unexpected exception occurred when attempting migration of " + @@ -759,7 +761,7 @@ protected Long stopVM(final HaWorkVO work) throws ConcurrentOperationException { @Override public void cancelScheduledMigrations(final HostVO host) { WorkType type = host.getType() == HostVO.Type.Storage ? WorkType.Stop : WorkType.Migration; - + s_logger.info("Canceling all scheduled migrations from host " + host.getUuid()); _haDao.deleteMigrationWorkItems(host.getId(), type, _serverId); } @@ -777,8 +779,6 @@ public List findTakenMigrationWork() { } private void rescheduleWork(final HaWorkVO work, final long nextTime) { - s_logger.info("Rescheduling work " + work + " to try again at " + new Date(nextTime << 10) + - ". Retry count " + work.getTimesTried() + "/" + _maxRetries + " times."); work.setTimeToTry(nextTime); work.setTimesTried(work.getTimesTried() + 1); work.setServerId(null); @@ -819,7 +819,7 @@ private void processWork(final HaWorkVO work) { } if (nextTime == null) { - s_logger.info("Completed work " + work); + s_logger.info("Completed work " + work + ". Took " + (work.getTimesTried() + 1) + "/" + _maxRetries + " attempts."); work.setStep(Step.Done); } else { rescheduleWork(work, nextTime.longValue()); @@ -836,9 +836,14 @@ private void processWork(final HaWorkVO work) { work.setUpdateTime(vm.getUpdated()); work.setPreviousState(vm.getState()); } finally { - if (!Step.Done.equals(work.getStep()) && work.getTimesTried() >= _maxRetries) { - s_logger.warn("Giving up, retried max. times for work: " + work); - work.setStep(Step.Done); + if (!Step.Done.equals(work.getStep())) { + if (work.getTimesTried() >= _maxRetries) { + s_logger.warn("Giving up, retried max " + work.getTimesTried() + "/" + _maxRetries + " times for work: " + work); + work.setStep(Step.Done); + } else { + s_logger.warn("Rescheduling work " + work + " to try again at " + new Date(work.getTimeToTry() << 10) + + ". Finished attempt " + work.getTimesTried() + "/" + _maxRetries + " times."); + } } _haDao.update(work.getId(), work); } @@ -1038,6 +1043,9 @@ public boolean hasPendingMigrationsWork(long vmId) { for (HaWorkVO work : haWorks) { if (work.getTimesTried() < _maxRetries) { return true; + } else { + s_logger.warn("HAWork Job of migration type " + work + " found in database which has max " + + "retries more than " + _maxRetries + " but still not in Done, Cancelled, or Error State"); } } return false; diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 1fcaa9ef75c1..29bbdd5767bd 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -20,6 +20,7 @@ import java.net.URISyntaxException; import java.net.URLDecoder; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; @@ -1162,10 +1163,7 @@ public Host cancelMaintenance(final CancelMaintenanceCmd cmd) { throw new InvalidParameterValueException("Host with id " + hostId.toString() + " doesn't exist"); } - if (host.getResourceState() != ResourceState.PrepareForMaintenance && - host.getResourceState() != ResourceState.ErrorInPrepareForMaintenance && - host.getResourceState() != ResourceState.Maintenance && - host.getResourceState() != ResourceState.ErrorInMaintenance) { + if (!ResourceState.isMaintenanceState(host.getResourceState())) { throw new CloudRuntimeException("Cannot perform cancelMaintenance when resource state is " + host.getResourceState() + ", hostId = " + hostId); } @@ -1218,8 +1216,7 @@ private boolean doMaintain(final long hostId) { final HostVO host = _hostDao.findById(hostId); s_logger.info("Maintenance: attempting maintenance of host " + host.getUuid()); ResourceState hostState = host.getResourceState(); - if (hostState == ResourceState.PrepareForMaintenance || hostState == ResourceState.ErrorInPrepareForMaintenance || - hostState == ResourceState.Maintenance || hostState == ResourceState.ErrorInMaintenance) { + if (ResourceState.isMaintenanceState(hostState)) { throw new CloudRuntimeException("Cannot perform maintain when resource state is " + hostState + ", hostId = " + hostId); } @@ -1287,11 +1284,8 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) { s_logger.debug("Unable to find host " + hostId); throw new InvalidParameterValueException("Unable to find host with ID: " + hostId + ". Please specify a valid host ID."); } - - final ResourceState hostState = host.getResourceState(); - if (hostState == ResourceState.Maintenance || hostState == ResourceState.PrepareForMaintenance || - hostState == ResourceState.ErrorInPrepareForMaintenance) { - throw new CloudRuntimeException("Host is already in state " + hostState + ". Cannot recall for maintenance until resolved."); + if (Arrays.asList(ResourceState.Maintenance, ResourceState.PrepareForMaintenance, ResourceState.ErrorInPrepareForMaintenance).contains(host.getResourceState())) { + throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved."); } if (_hostDao.countBy(host.getClusterId(), ResourceState.PrepareForMaintenance, ResourceState.ErrorInPrepareForMaintenance) > 0) { @@ -2651,7 +2645,7 @@ public Boolean propagateResourceEvent(final long agentId, final ResourceState.Ev } @Override - public boolean maintenanceFailed(final long hostId) { + public boolean migrateAwayFailed(final long hostId, final long vmId) { final HostVO host = _hostDao.findById(hostId); if (host == null) { if (s_logger.isDebugEnabled()) { @@ -2660,6 +2654,8 @@ public boolean maintenanceFailed(final long hostId) { return false; } else { try { + s_logger.warn("Migration of VM " + _vmDao.findById(vmId) + " failed from host " + _hostDao.findById(hostId) + + ". Emitting event UnableToMigrate."); return resourceStateTransitTo(host, ResourceState.Event.UnableToMigrate, _nodeId); } catch (final NoTransitionException e) { s_logger.debug("No next resource state for host " + host.getId() + " while current state is " + host.getResourceState() + " with event " + diff --git a/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java b/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java index 82a1e923cb13..26cd820fd673 100755 --- a/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java +++ b/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java @@ -32,6 +32,7 @@ import org.apache.cloudstack.api.command.admin.host.ReconnectHostCmd; import org.apache.cloudstack.api.command.admin.host.UpdateHostCmd; import org.apache.cloudstack.api.command.admin.host.UpdateHostPasswordCmd; +import org.apache.cloudstack.framework.config.ConfigKey; import com.cloud.agent.api.StartupCommand; import com.cloud.agent.api.StartupRoutingCommand; @@ -56,7 +57,6 @@ import com.cloud.resource.ResourceState.Event; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.fsm.NoTransitionException; -import org.apache.cloudstack.framework.config.ConfigKey; public class MockResourceManagerImpl extends ManagerBase implements ResourceManager { @@ -307,10 +307,10 @@ public boolean umanageHost(final long hostId) { } /* (non-Javadoc) - * @see com.cloud.resource.ResourceManager#maintenanceFailed(long) + * @see com.cloud.resource.ResourceManager#migrateAwayFailed(long) */ @Override - public boolean maintenanceFailed(final long hostId) { + public boolean migrateAwayFailed(final long hostId, final long vmId) { // TODO Auto-generated method stub return false; } diff --git a/test/integration/smoke/test_host_maintenance.py b/test/integration/smoke/test_host_maintenance.py index 35556520e673..7ee1a369b5ee 100644 --- a/test/integration/smoke/test_host_maintenance.py +++ b/test/integration/smoke/test_host_maintenance.py @@ -60,6 +60,23 @@ def check_resource_state(): raise Exception("Failed to wait for host %s to be on resource state %s" % (hostid, resourcestate)) return True + def prepare_host_for_maintenance(self, hostid): + self.logger.debug('Sending Host with id % to prepareHostForMaintenance' % hostid) + cmd = prepareHostForMaintenance.prepareHostForMaintenanceCmd() + cmd.id = hostid + response = self.apiclient.prepareHostForMaintenance(cmd) + self.logger.debug('Host with id %s is in prepareHostForMaintenance' % hostid) + return response + + def cancel_host_maintenance(self, hostid): + self.logger.debug('Canceling Host with id %s from maintain' % (hostid)) + cmd = cancelHostMaintenance.cancelHostMaintenanceCmd() + cmd.id = hostid + res = self.apiclient.cancelHostMaintenance(cmd) + self.logger.debug('Host with id %s is cancelling maintenance' % hostid) + return res + + class TestHostMaintenance(TestHostMaintenanceBase): def setUp(self): @@ -76,6 +93,7 @@ def setUp(self): self.cleanup = [] self.ssh_client = None self.needs_unblock_iptables = False + self.hostIdToCancelMaintenance = None self.hostConfig = self.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][0].__dict__ @@ -85,7 +103,10 @@ def tearDown(self): self.ssh_client.execute("iptables -D OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") self.ssh_client.execute("iptables -D OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") - # Clean up, terminate the created templates + if self.hostIdToCancelMaintenance is not None: + self.cancel_host_maintenance(self.hostIdToCancelMaintenance) + + # Clean up, terminate the created templates cleanup_resources(self.apiclient, self.cleanup) except Exception as e: @@ -230,13 +251,10 @@ def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id): raise Exception("Failed to wait for all VMs to reach running state to execute test") expected_vm_count_after_maintenance = self.noOfVMsOnHost(target_host_id) + self.noOfVMsOnHost(other_host_id) - cmd = prepareHostForMaintenance.prepareHostForMaintenanceCmd() - cmd.id = target_host_id - self.logger.debug('Sending Host with id {} to prepareHostForMaintenance'.format(target_host_id)) - response = self.apiclient.prepareHostForMaintenance(cmd) - - self.logger.debug('Host with id {} is in prepareHostForMaintenance'.format(target_host_id)) + self.hostIdToCancelMaintenance = target_host_id + + self.prepare_host_for_maintenance(target_host_id) migrations_finished = wait_until(3, 200, self.migrationsFinished, target_host_id) wait_until(3, 200, self.checkAllVmsRunningOnHost, { "hostId": other_host_id, @@ -244,12 +262,8 @@ def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id): }) other_vm_count_after_maintenance = self.noOfVMsOnHost(other_host_id) - self.logger.debug('Canceling Host with id {} from maintain'.format(target_host_id)) - cmd = cancelHostMaintenance.cancelHostMaintenanceCmd() - cmd.id = target_host_id - self.apiclient.cancelHostMaintenance(cmd) - - self.logger.debug('Host with id {} has been sent to cancelHostMaintenance'.format(target_host_id)) + self.cancel_host_maintenance(target_host_id) + self.hostIdToCancelMaintenance = None if expected_vm_count_after_maintenance != other_vm_count_after_maintenance: self.fail('All VMs not found on other host after maintenance. Other host VM counts expected {} but was {}'.format(expected_vm_count_after_maintenance, other_vm_count_after_maintenance)) @@ -387,23 +401,16 @@ def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): self.ssh_client = self.get_ssh_client(listHost[0].ipaddress, self.hostConfig["username"], self.hostConfig["password"]) self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") - self.needs_unblock_iptables = True - cmd = prepareHostForMaintenance.prepareHostForMaintenanceCmd() - cmd.id = target_host_id - self.logger.debug('Sending Host with id {} to prepareHostForMaintenance'.format(target_host_id)) - response = self.apiclient.prepareHostForMaintenance(cmd) + self.needs_unblock_iptables = True + self.hostIdToCancelMaintenance = target_host_id - self.logger.debug('Attempting to put host with id {} in Maintenance'.format(target_host_id)) + self.prepare_host_for_maintenance(target_host_id) error_in_maintenance_reached = self.wait_until_host_is_in_state(target_host_id, "ErrorInMaintenance", 5, 200) - self.logger.debug('Canceling Host with id {} from maintain'.format(target_host_id)) - cmd = cancelHostMaintenance.cancelHostMaintenanceCmd() - cmd.id = target_host_id - self.apiclient.cancelHostMaintenance(cmd) - - self.logger.debug('Host with id {} has been sent to cancelHostMaintenance'.format(target_host_id)) + self.cancel_host_maintenance(target_host_id) + self.hostIdToCancelMaintenance = None if error_in_maintenance_reached == False: self.fail("Error in maintenance state should have reached after ports block") @@ -506,12 +513,6 @@ def set_ssh_enabled(cls, on): value = "true" if on else "false" cls.updateConfiguration('kvm.ssh.to.agent', value) - def prepare_host_for_maintenance(self, hostid): - cmd = prepareHostForMaintenance.prepareHostForMaintenanceCmd() - cmd.id = hostid - self.apiclient.prepareHostForMaintenance(cmd) - self.logger.debug('Host with id %s is in prepareHostForMaintenance' % hostid) - def wait_until_agent_is_in_state(self, hostid, state, interval=3, retries=20): def check_agent_state(): response = Host.list( @@ -529,12 +530,6 @@ def check_agent_state(): raise Exception("Failed to wait for host agent %s to be on state %s" % (hostid, state)) return True - def cancel_host_maintenance(self, hostid): - cmd = cancelHostMaintenance.cancelHostMaintenanceCmd() - cmd.id = hostid - self.apiclient.cancelHostMaintenance(cmd) - self.logger.debug('Host with id %s is cancelling maintenance' % hostid) - def get_enabled_host_connected_agent(self): hosts = Host.list( self.apiclient, From 4b380c7c3e530743b55d472bc46a2e181122437e Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Tue, 26 Nov 2019 14:22:46 +0530 Subject: [PATCH 14/19] Fix flaky tests by adding delays between host states --- .../cloud/ha/HighAvailabilityManagerImpl.java | 4 +- .../cloud/resource/ResourceManagerImpl.java | 19 ++++-- .../smoke/test_host_maintenance.py | 64 ++++++++++--------- 3 files changed, 48 insertions(+), 39 deletions(-) diff --git a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java index 16a9bc3ccc47..348a4179f3f5 100644 --- a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java +++ b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java @@ -321,7 +321,7 @@ public boolean scheduleMigration(final VMInstanceVO vm) { if (vm.getHostId() != null) { final HaWorkVO work = new HaWorkVO(vm.getId(), vm.getType(), WorkType.Migration, Step.Scheduled, vm.getHostId(), vm.getState(), 0, vm.getUpdated()); _haDao.persist(work); - s_logger.info("Scheduled migration work of VM " + vm.getUuid() + " from host " + vm.getHostName() + " with HAWork " + work); + s_logger.info("Scheduled migration work of VM " + vm.getUuid() + " from host " + _hostDao.findById(vm.getHostId()) + " with HAWork " + work); wakeupWorkers(); } return true; @@ -1041,7 +1041,7 @@ public boolean hasPendingHaWork(long vmId) { public boolean hasPendingMigrationsWork(long vmId) { List haWorks = _haDao.listPendingMigrationsForVm(vmId); for (HaWorkVO work : haWorks) { - if (work.getTimesTried() < _maxRetries) { + if (work.getTimesTried() <= _maxRetries) { return true; } else { s_logger.warn("HAWork Job of migration type " + work + " found in database which has max " + diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 29bbdd5767bd..6f3e7e62e62f 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1406,22 +1406,29 @@ protected boolean attemptMaintain(HostVO host) throws NoTransitionException { final long hostId = host.getId(); s_logger.info("Attempting maintenance for host " + host.getName()); - // Step 1: If there are no VMs in migrating, running, starting, stopping, error or unknown state we can safely move the host to maintenance. - if (CollectionUtils.isEmpty(_vmDao.findByHostInStates(host.getId(), - State.Migrating, State.Running, State.Starting, State.Stopping, State.Error, State.Unknown))) { - return setHostIntoMaintenance(host); - } - // Step 2: Gather relevant VMs' states on the host and then based on them we can determine if + // Step 0: First gather if VMs have pending HAWork for migration with retries left. final List allVmsOnHost = _vmDao.listByHostId(hostId); boolean hasPendingMigrationRetries = false; for (VMInstanceVO vmInstanceVO : allVmsOnHost) { if (_haMgr.hasPendingMigrationsWork(vmInstanceVO.getId())) { + s_logger.info("Attempting maintenance for " + host + " found pending migration for VM " + vmInstanceVO); hasPendingMigrationRetries = true; break; } } + // Step 1: If there are no VMs in migrating, running, starting, stopping, error or unknown state we can safely move the host to maintenance. + if (CollectionUtils.isEmpty(_vmDao.findByHostInStates(host.getId(), + State.Migrating, State.Running, State.Starting, State.Stopping, State.Error, State.Unknown))) { + if (hasPendingMigrationRetries) { + s_logger.error("There should not be pending retries VMs for this host as there are no running, migrating," + + "starting, stopping, error or unknown states on host " + host); + } + return setHostIntoMaintenance(host); + } + + // Step 2: Gather relevant VMs' states on the host and then based on them we can determine if final List failedMigrations = new ArrayList<>(_vmDao.listNonMigratingVmsByHostEqualsLastHost(hostId)); final List errorVms = new ArrayList<>(_vmDao.findByHostInStates(hostId, State.Unknown, State.Error)); final boolean hasMigratingAwayVms = CollectionUtils.isNotEmpty(_vmDao.listVmsMigratingFromHost(hostId)); diff --git a/test/integration/smoke/test_host_maintenance.py b/test/integration/smoke/test_host_maintenance.py index 7ee1a369b5ee..fa3bc2ade536 100644 --- a/test/integration/smoke/test_host_maintenance.py +++ b/test/integration/smoke/test_host_maintenance.py @@ -53,6 +53,10 @@ def check_resource_state(): if response[0].resourcestate == resourcestate: self.logger.debug('Host with id %s is in resource state = %s' % (hostid, resourcestate)) return True, None + else: + self.logger.debug("Waiting for host " + hostid + + " to reach state " + resourcestate + + ", with current state " + response[0].resourcestate) return False, None done, _ = wait_until(interval, retries, check_resource_state) @@ -61,19 +65,20 @@ def check_resource_state(): return True def prepare_host_for_maintenance(self, hostid): - self.logger.debug('Sending Host with id % to prepareHostForMaintenance' % hostid) + self.logger.debug("Sending Host with id %s to prepareHostForMaintenance" % hostid) cmd = prepareHostForMaintenance.prepareHostForMaintenanceCmd() cmd.id = hostid response = self.apiclient.prepareHostForMaintenance(cmd) - self.logger.debug('Host with id %s is in prepareHostForMaintenance' % hostid) + self.logger.debug("Host with id %s is in prepareHostForMaintenance" % hostid) + self.logger.debug(response) return response def cancel_host_maintenance(self, hostid): - self.logger.debug('Canceling Host with id %s from maintain' % (hostid)) + self.logger.debug("Canceling Host with id %s from maintain" % hostid) cmd = cancelHostMaintenance.cancelHostMaintenanceCmd() cmd.id = hostid res = self.apiclient.cancelHostMaintenance(cmd) - self.logger.debug('Host with id %s is cancelling maintenance' % hostid) + self.logger.debug("Host with id %s is cancelling maintenance" % hostid) return res @@ -102,9 +107,13 @@ def tearDown(self): if self.ssh_client is not None and self.needs_unblock_iptables: self.ssh_client.execute("iptables -D OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") self.ssh_client.execute("iptables -D OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") + try: + if self.hostIdToCancelMaintenance is not None: + self.cancel_host_maintenance(self.hostIdToCancelMaintenance) + except Exception as e: + self.logger.debug("Attempted host maintenance cancel but it threw exception. Skipping.") - if self.hostIdToCancelMaintenance is not None: - self.cancel_host_maintenance(self.hostIdToCancelMaintenance) + self.hostIdToCancelMaintenance = None # Clean up, terminate the created templates cleanup_resources(self.apiclient, self.cleanup) @@ -166,27 +175,18 @@ def createVMs(self, hostId, number): self.cleanup.append(self.service_offering) return vms - def checkAllVmsRunningOnHost(self, data): - hostId = data["hostId"] - expectedNumVms = data["expected_vms"] if "expected_vms" in data else -1 + def checkAllVmsRunningOnHost(self, hostId): listVms1 = VirtualMachine.list( self.apiclient, hostid=hostId ) - runningVms = 0 if (listVms1 is not None): self.logger.debug('Vms found to test all running = {} '.format(len(listVms1))) for vm in listVms1: if (vm.state != "Running"): self.logger.debug('VirtualMachine on Host with id = {} is in {}'.format(vm.id, vm.state)) return (False, None) - else: - runningVms = runningVms + 1 - if expectedNumVms != -1: - if expectedNumVms != runningVms: - return (False, None) - response = list_ssvms( self.apiclient, @@ -246,27 +246,27 @@ def noOfVMsOnHost(self, hostId): def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id): # Wait for all VMs to complete any pending migrations. - if not wait_until(3, 100, self.checkAllVmsRunningOnHost, {"hostId" : target_host_id}) or \ - not wait_until(3, 100, self.checkAllVmsRunningOnHost, {"hostId": other_host_id}): + if not wait_until(3, 100, self.checkAllVmsRunningOnHost, target_host_id) or \ + not wait_until(3, 100, self.checkAllVmsRunningOnHost, other_host_id): raise Exception("Failed to wait for all VMs to reach running state to execute test") expected_vm_count_after_maintenance = self.noOfVMsOnHost(target_host_id) + self.noOfVMsOnHost(other_host_id) + self.prepare_host_for_maintenance(target_host_id) + migrations_finished = wait_until(5, 200, self.migrationsFinished, target_host_id) + + self.wait_until_host_is_in_state(target_host_id, "Maintenance", 5, 200) self.hostIdToCancelMaintenance = target_host_id - self.prepare_host_for_maintenance(target_host_id) - migrations_finished = wait_until(3, 200, self.migrationsFinished, target_host_id) - wait_until(3, 200, self.checkAllVmsRunningOnHost, { - "hostId": other_host_id, - "expected_vms": expected_vm_count_after_maintenance - }) - other_vm_count_after_maintenance = self.noOfVMsOnHost(other_host_id) + vm_count_after_maintenance = self.noOfVMsOnHost(target_host_id) self.cancel_host_maintenance(target_host_id) - self.hostIdToCancelMaintenance = None + host_reached_enabled = self.wait_until_host_is_in_state(target_host_id, "Enabled", 5, 200) + if host_reached_enabled: + self.hostIdToCancelMaintenance = None - if expected_vm_count_after_maintenance != other_vm_count_after_maintenance: - self.fail('All VMs not found on other host after maintenance. Other host VM counts expected {} but was {}'.format(expected_vm_count_after_maintenance, other_vm_count_after_maintenance)) + if vm_count_after_maintenance != 0: + self.fail("Host to put to maintenance still has VMs running") return migrations_finished @@ -403,14 +403,16 @@ def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") self.needs_unblock_iptables = True - self.hostIdToCancelMaintenance = target_host_id + # Attempt putting host in maintenance and check if ErrorInMaintenance state is reached self.prepare_host_for_maintenance(target_host_id) - error_in_maintenance_reached = self.wait_until_host_is_in_state(target_host_id, "ErrorInMaintenance", 5, 200) + self.hostIdToCancelMaintenance = target_host_id self.cancel_host_maintenance(target_host_id) - self.hostIdToCancelMaintenance = None + host_reached_enabled = self.wait_until_host_is_in_state(target_host_id, "Enabled", 5, 200) + if host_reached_enabled: + self.hostIdToCancelMaintenance = None if error_in_maintenance_reached == False: self.fail("Error in maintenance state should have reached after ports block") From a789933f960a11d507b3ee360bb175b33361c282 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Wed, 27 Nov 2019 15:21:57 +0530 Subject: [PATCH 15/19] Added fetching only enabled hosts for tests --- .../smoke/test_host_maintenance.py | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/test/integration/smoke/test_host_maintenance.py b/test/integration/smoke/test_host_maintenance.py index fa3bc2ade536..64972e3e658d 100644 --- a/test/integration/smoke/test_host_maintenance.py +++ b/test/integration/smoke/test_host_maintenance.py @@ -250,8 +250,6 @@ def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id): not wait_until(3, 100, self.checkAllVmsRunningOnHost, other_host_id): raise Exception("Failed to wait for all VMs to reach running state to execute test") - expected_vm_count_after_maintenance = self.noOfVMsOnHost(target_host_id) + self.noOfVMsOnHost(other_host_id) - self.prepare_host_for_maintenance(target_host_id) migrations_finished = wait_until(5, 200, self.migrationsFinished, target_host_id) @@ -285,13 +283,16 @@ def test_01_cancel_host_maintenace_with_no_migration_jobs(self): type='Routing', zoneid=self.zone.id, podid=self.pod.id, + hypervisor=self.hypervisor, + resourcestate='Enabled', + state='Up' ) for host in listHost: - self.logger.debug('1 Hypervisor = {}'.format(host.id)) + self.logger.debug('Found Host = {}'.format(host.id)) if (len(listHost) < 2): - raise unittest.SkipTest("Cancel host maintenance when VMs are migrating should be tested for 2 or more hosts") + raise unittest.SkipTest("Canceling tests for host maintenance as we need 2 or more hosts up and enabled") migrations_finished = True @@ -327,12 +328,15 @@ def test_02_cancel_host_maintenace_with_migration_jobs(self): type='Routing', zoneid=self.zone.id, podid=self.pod.id, + hypervisor=self.hypervisor, + resourcestate='Enabled', + state='Up' ) for host in listHost: - self.logger.debug('2 Hypervisor = {}'.format(host.id)) + self.logger.debug('Found Host = {}'.format(host.id)) if (len(listHost) < 2): - raise unittest.SkipTest("Cancel host maintenance when VMs are migrating can only be tested with 2 hosts") + raise unittest.SkipTest("Canceling tests for host maintenance as we need 2 or more hosts up and enabled") no_of_vms = self.noOfVMsOnHost(listHost[0].id) @@ -377,13 +381,16 @@ def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): type='Routing', zoneid=self.zone.id, podid=self.pod.id, + hypervisor=self.hypervisor, + resourcestate='Enabled', + state='Up' ) + for host in listHost: - self.logger.debug('2 Hypervisor = {}'.format(host.id)) + self.logger.debug('Found Host = {}'.format(host.id)) if (len(listHost) < 2): - raise unittest.SkipTest("Cancel host maintenance when VMs are migrating can only be tested with 2 hosts"); - return + raise unittest.SkipTest("Canceling tests for host maintenance as we need 2 or more hosts up and enabled") target_host_id = listHost[0].id other_host_id = listHost[1].id @@ -543,7 +550,7 @@ def get_enabled_host_connected_agent(self): state='Up' ) if len(hosts) < 2: - raise unittest.SkipTest("Cancel host maintenance must be tested for 2 or more hosts") + raise unittest.SkipTest("Host maintenance tests must be tested for 2 or more hosts") return hosts[0] def deploy_vm_on_host(self, hostid): From 7e654b983d64c36e78f0a9a18b7f72223fb64e27 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Wed, 27 Nov 2019 22:13:17 +0530 Subject: [PATCH 16/19] Make port blocking KVM specific and refactor to handle failure --- .../smoke/test_host_maintenance.py | 106 +++++++++--------- 1 file changed, 52 insertions(+), 54 deletions(-) diff --git a/test/integration/smoke/test_host_maintenance.py b/test/integration/smoke/test_host_maintenance.py index 64972e3e658d..0e022a708f0a 100644 --- a/test/integration/smoke/test_host_maintenance.py +++ b/test/integration/smoke/test_host_maintenance.py @@ -81,6 +81,13 @@ def cancel_host_maintenance(self, hostid): self.logger.debug("Host with id %s is cancelling maintenance" % hostid) return res + def revert_host_state_on_failure(self, hostId): + cmd = updateHost.updateHostCmd() + cmd.id = hostId + cmd.allocationstate = "Enable" + response = self.apiclient.updateHost(cmd) + self.assertEqual(response.resourcestate, "Enabled") + class TestHostMaintenance(TestHostMaintenanceBase): @@ -98,7 +105,6 @@ def setUp(self): self.cleanup = [] self.ssh_client = None self.needs_unblock_iptables = False - self.hostIdToCancelMaintenance = None self.hostConfig = self.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][0].__dict__ @@ -107,13 +113,6 @@ def tearDown(self): if self.ssh_client is not None and self.needs_unblock_iptables: self.ssh_client.execute("iptables -D OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") self.ssh_client.execute("iptables -D OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") - try: - if self.hostIdToCancelMaintenance is not None: - self.cancel_host_maintenance(self.hostIdToCancelMaintenance) - except Exception as e: - self.logger.debug("Attempted host maintenance cancel but it threw exception. Skipping.") - - self.hostIdToCancelMaintenance = None # Clean up, terminate the created templates cleanup_resources(self.apiclient, self.cleanup) @@ -254,14 +253,11 @@ def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id): migrations_finished = wait_until(5, 200, self.migrationsFinished, target_host_id) self.wait_until_host_is_in_state(target_host_id, "Maintenance", 5, 200) - self.hostIdToCancelMaintenance = target_host_id vm_count_after_maintenance = self.noOfVMsOnHost(target_host_id) self.cancel_host_maintenance(target_host_id) - host_reached_enabled = self.wait_until_host_is_in_state(target_host_id, "Enabled", 5, 200) - if host_reached_enabled: - self.hostIdToCancelMaintenance = None + self.wait_until_host_is_in_state(target_host_id, "Enabled", 5, 200) if vm_count_after_maintenance != 0: self.fail("Host to put to maintenance still has VMs running") @@ -278,6 +274,13 @@ def hostPrepareAndCancelMaintenance(self, target_host_id, other_host_id): "sg"], required_hardware="true") def test_01_cancel_host_maintenace_with_no_migration_jobs(self): + """ + Tests if putting a host with no migrations (0 VMs) work back and forth + + 1) Verify if there are at least 2 hosts in enabled state. + 2) Put the host into maintenance verify success + 3) Put the other host into maintenance, verify success + """ listHost = Host.list( self.apiclient, type='Routing', @@ -294,22 +297,20 @@ def test_01_cancel_host_maintenace_with_no_migration_jobs(self): if (len(listHost) < 2): raise unittest.SkipTest("Canceling tests for host maintenance as we need 2 or more hosts up and enabled") - migrations_finished = True - try: migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[0].id, listHost[1].id) if migrations_finished: - migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id) + self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id) + else: + raise unittest.SkipTest("VMs are still migrating so reverse migration /maintenace skipped") except Exception as e: + self.revert_host_state_on_failure(listHost[0].id) + self.revert_host_state_on_failure(listHost[1].id) self.logger.debug("Exception {}".format(e)) - self.fail("Cancel host maintenance failed {}".format(e[0])) - - - if not migrations_finished: - raise unittest.SkipTest("VMs are still migrating and the test will not be able to check the conditions the test is intended for") + self.fail("Host maintenance test failed {}".format(e[0])) @attr( @@ -349,21 +350,19 @@ def test_02_cancel_host_maintenace_with_migration_jobs(self): self.logger.debug("Creating vms = {}".format(no_vm_req)) self.vmlist = self.createVMs(listHost[0].id, no_vm_req) - migrations_finished = True - try: migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[0].id, listHost[1].id) if migrations_finished: - migrations_finished = self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id) + self.hostPrepareAndCancelMaintenance(listHost[1].id, listHost[0].id) + else: + raise unittest.SkipTest("VMs are still migrating so reverse migration /maintenace skipped") except Exception as e: + self.revert_host_state_on_failure(listHost[0].id) + self.revert_host_state_on_failure(listHost[1].id) self.logger.debug("Exception {}".format(e)) - self.fail("Cancel host maintenance failed {}".format(e[0])) - - - if (migrations_finished == False): - raise unittest.SkipTest("VMs are still migrating and the test will not be able to check the conditions the test is intended for") + self.fail("Host maintenance test failed {}".format(e[0])) @attr( tags=[ @@ -375,6 +374,8 @@ def test_02_cancel_host_maintenace_with_migration_jobs(self): "sg"], required_hardware="true") def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): + if self.hypervisor.lower() != 'kvm': + raise unittest.SkipTest("Skipping migration port blocked test as it's not KVM.") listHost = Host.list( self.apiclient, @@ -405,24 +406,28 @@ def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): self.logger.debug("Creating vms = {}".format(no_vm_req)) self.vmlist = self.createVMs(listHost[0].id, no_vm_req) - self.ssh_client = self.get_ssh_client(listHost[0].ipaddress, self.hostConfig["username"], self.hostConfig["password"]) - self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") - self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") + try: + self.ssh_client = self.get_ssh_client(listHost[0].ipaddress, self.hostConfig["username"], self.hostConfig["password"]) + self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") + self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") - self.needs_unblock_iptables = True + self.needs_unblock_iptables = True - # Attempt putting host in maintenance and check if ErrorInMaintenance state is reached - self.prepare_host_for_maintenance(target_host_id) - error_in_maintenance_reached = self.wait_until_host_is_in_state(target_host_id, "ErrorInMaintenance", 5, 200) - self.hostIdToCancelMaintenance = target_host_id + # Attempt putting host in maintenance and check if ErrorInMaintenance state is reached + self.prepare_host_for_maintenance(target_host_id) + error_in_maintenance_reached = self.wait_until_host_is_in_state(target_host_id, "ErrorInMaintenance", 5, 200) - self.cancel_host_maintenance(target_host_id) - host_reached_enabled = self.wait_until_host_is_in_state(target_host_id, "Enabled", 5, 200) - if host_reached_enabled: - self.hostIdToCancelMaintenance = None + self.cancel_host_maintenance(target_host_id) + self.wait_until_host_is_in_state(target_host_id, "Enabled", 5, 200) - if error_in_maintenance_reached == False: - self.fail("Error in maintenance state should have reached after ports block") + if not error_in_maintenance_reached: + self.fail("Error in maintenance state should have reached after ports block") + + except Exception as e: + self.revert_host_state_on_failure(listHost[0].id) + self.revert_host_state_on_failure(listHost[1].id) + self.logger.debug("Exception {}".format(e)) + self.fail("Host maintenance test failed {}".format(e[0])) class TestHostMaintenanceAgents(TestHostMaintenanceBase): @@ -573,13 +578,6 @@ def assert_host_is_functional_after_cancelling_maintenance(self, hostid): ) self.cleanup.append(vm) - def revert_host_state_on_failure(self, host): - cmd = updateHost.updateHostCmd() - cmd.id = host.id - cmd.allocationstate = "Enable" - response = self.apiclient.updateHost(cmd) - self.assertEqual(response.resourcestate, "Enabled") - @skipTestIf("hypervisorNotSupported") @attr(tags=["advanced", "advancedns", "smoke", "basic", "eip", "sg"], required_hardware="true") def test_01_cancel_host_maintenance_ssh_enabled_agent_connected(self): @@ -602,7 +600,7 @@ def test_01_cancel_host_maintenance_ssh_enabled_agent_connected(self): self.wait_until_host_is_in_state(self.host.id, "Enabled") self.assert_host_is_functional_after_cancelling_maintenance(self.host.id) except Exception as e: - self.revert_host_state_on_failure(self.host) + self.revert_host_state_on_failure(self.host.id) self.fail(e) @skipTestIf("hypervisorNotSupported") @@ -638,7 +636,7 @@ def test_02_cancel_host_maintenance_ssh_enabled_agent_disconnected(self): self.assert_host_is_functional_after_cancelling_maintenance(self.host.id) except Exception as e: - self.revert_host_state_on_failure(self.host) + self.revert_host_state_on_failure(self.host.id) self.fail(e) @skipTestIf("hypervisorNotSupported") @@ -663,7 +661,7 @@ def test_03_cancel_host_maintenance_ssh_disabled_agent_connected(self): self.wait_until_host_is_in_state(self.host.id, "Enabled") self.assert_host_is_functional_after_cancelling_maintenance(self.host.id) except Exception as e: - self.revert_host_state_on_failure(self.host) + self.revert_host_state_on_failure(self.host.id) self.fail(e) @skipTestIf("hypervisorNotSupported") @@ -694,7 +692,7 @@ def test_04_cancel_host_maintenance_ssh_disabled_agent_disconnected(self): ssh_client.execute("service cloudstack-agent stop") self.wait_until_agent_is_in_state(self.host.id, "Disconnected") except Exception as e: - self.revert_host_state_on_failure(self.host) + self.revert_host_state_on_failure(self.host.id) self.fail(e) self.assertRaises(Exception, self.cancel_host_maintenance, self.host.id) @@ -709,5 +707,5 @@ def test_04_cancel_host_maintenance_ssh_disabled_agent_disconnected(self): self.wait_until_host_is_in_state(self.host.id, "Enabled") self.assert_host_is_functional_after_cancelling_maintenance(self.host.id) except Exception as e: - self.revert_host_state_on_failure(self.host) + self.revert_host_state_on_failure(self.host.id) self.fail(e) From 06c4521935f4ffe84bf449950c57b25411ffa56f Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Thu, 28 Nov 2019 00:36:28 +0530 Subject: [PATCH 17/19] Make failing migrations due to tagged host instead of port blocking --- .../smoke/test_host_maintenance.py | 59 ++++++++++--------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/test/integration/smoke/test_host_maintenance.py b/test/integration/smoke/test_host_maintenance.py index 0e022a708f0a..33536214d805 100644 --- a/test/integration/smoke/test_host_maintenance.py +++ b/test/integration/smoke/test_host_maintenance.py @@ -103,17 +103,11 @@ def setUp(self): self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests()) self.pod = get_pod(self.apiclient, self.zone.id) self.cleanup = [] - self.ssh_client = None - self.needs_unblock_iptables = False self.hostConfig = self.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][0].__dict__ def tearDown(self): try: - if self.ssh_client is not None and self.needs_unblock_iptables: - self.ssh_client.execute("iptables -D OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") - self.ssh_client.execute("iptables -D OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") - # Clean up, terminate the created templates cleanup_resources(self.apiclient, self.cleanup) @@ -122,7 +116,7 @@ def tearDown(self): return - def createVMs(self, hostId, number): + def createVMs(self, hostId, number, offering_key="tiny"): self.template = get_template( self.apiclient, @@ -137,7 +131,7 @@ def createVMs(self, hostId, number): self.service_offering = ServiceOffering.create( self.apiclient, - self.services["service_offerings"]["tiny"] + self.services["service_offerings"][offering_key] ) self.logger.debug("Using service offering %s " % self.service_offering.id) self.network_offering = NetworkOffering.create( @@ -323,7 +317,14 @@ def test_01_cancel_host_maintenace_with_no_migration_jobs(self): "sg"], required_hardware="true") def test_02_cancel_host_maintenace_with_migration_jobs(self): + """ + Tests if putting a host with migrations (3 VMs) work back and forth + 1) Verify if there are at least 2 hosts in enabled state. + 2) Deploy VMs if needed + 3) Put the host into maintenance verify success -ensure existing host has zero running VMs + 4) Put the other host into maintenance, verify success just as step 3 + """ listHost = Host.list( self.apiclient, type='Routing', @@ -373,10 +374,14 @@ def test_02_cancel_host_maintenace_with_migration_jobs(self): "eip", "sg"], required_hardware="true") - def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): - if self.hypervisor.lower() != 'kvm': - raise unittest.SkipTest("Skipping migration port blocked test as it's not KVM.") + def test_03_cancel_host_maintenace_with_migration_jobs_failure(self): + """ + Tests if putting a host with impossible migrations (2 VMs) work pushes to ErrorInMaintenance state + 1) Verify if there are at least 2 hosts in enabled state. + 2) Tag the host and deploy tagged VMs which cannot be migrated to other host without tags + 3) Put the host into maintenance verify it fails with it reaching ErrorInMaintenance + """ listHost = Host.list( self.apiclient, type='Routing', @@ -394,38 +399,38 @@ def test_03_cancel_host_maintenace_with_migration_jobs_ports_blocked(self): raise unittest.SkipTest("Canceling tests for host maintenance as we need 2 or more hosts up and enabled") target_host_id = listHost[0].id - other_host_id = listHost[1].id - - no_of_vms = self.noOfVMsOnHost(target_host_id) - - # Need only 2 VMs for this case. - if no_of_vms < 2: - self.logger.debug("Create VMs as there are not enough vms to check host maintenance") - no_vm_req = 2 - no_of_vms - if (no_vm_req > 0): - self.logger.debug("Creating vms = {}".format(no_vm_req)) - self.vmlist = self.createVMs(listHost[0].id, no_vm_req) try: - self.ssh_client = self.get_ssh_client(listHost[0].ipaddress, self.hostConfig["username"], self.hostConfig["password"]) - self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 49152:49215 -m comment --comment 'test block migrations'") - self.ssh_client.execute("iptables -I OUTPUT -j REJECT -m state --state NEW -m tcp -p tcp --dport 16509 -m comment --comment 'test block migrations'") + Host.update(self.apiclient, + id=target_host_id, + hosttags=self.services["service_offerings"]["taggedsmall"]["hosttags"]) + + no_of_vms = self.noOfVMsOnHost(target_host_id) - self.needs_unblock_iptables = True + # Need only 2 VMs for this case. + if no_of_vms < 2: + self.logger.debug("Create VMs as there are not enough vms to check host maintenance") + no_vm_req = 2 - no_of_vms + if (no_vm_req > 0): + self.logger.debug("Creating vms = {}".format(no_vm_req)) + self.vmlist = self.createVMs(listHost[0].id, no_vm_req, "taggedsmall") # Attempt putting host in maintenance and check if ErrorInMaintenance state is reached self.prepare_host_for_maintenance(target_host_id) - error_in_maintenance_reached = self.wait_until_host_is_in_state(target_host_id, "ErrorInMaintenance", 5, 200) + error_in_maintenance_reached = self.wait_until_host_is_in_state(target_host_id, "ErrorInMaintenance", 5, 300) self.cancel_host_maintenance(target_host_id) self.wait_until_host_is_in_state(target_host_id, "Enabled", 5, 200) + Host.update(self.apiclient, id=target_host_id, hosttags="") + if not error_in_maintenance_reached: self.fail("Error in maintenance state should have reached after ports block") except Exception as e: self.revert_host_state_on_failure(listHost[0].id) self.revert_host_state_on_failure(listHost[1].id) + Host.update(self.apiclient, id=target_host_id, hosttags="") self.logger.debug("Exception {}".format(e)) self.fail("Host maintenance test failed {}".format(e[0])) From ca792efa3de09e41ff73ffb8fdffc1c21cf6dfdf Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Thu, 28 Nov 2019 18:13:35 +0530 Subject: [PATCH 18/19] Added additional check for migrating VMs --- .../src/main/java/com/cloud/resource/ResourceManagerImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 6f3e7e62e62f..b1c22700cedc 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1409,6 +1409,7 @@ protected boolean attemptMaintain(HostVO host) throws NoTransitionException { // Step 0: First gather if VMs have pending HAWork for migration with retries left. final List allVmsOnHost = _vmDao.listByHostId(hostId); + final boolean hasMigratingAwayVms = CollectionUtils.isNotEmpty(_vmDao.listVmsMigratingFromHost(hostId)); boolean hasPendingMigrationRetries = false; for (VMInstanceVO vmInstanceVO : allVmsOnHost) { if (_haMgr.hasPendingMigrationsWork(vmInstanceVO.getId())) { @@ -1419,7 +1420,7 @@ protected boolean attemptMaintain(HostVO host) throws NoTransitionException { } // Step 1: If there are no VMs in migrating, running, starting, stopping, error or unknown state we can safely move the host to maintenance. - if (CollectionUtils.isEmpty(_vmDao.findByHostInStates(host.getId(), + if (!hasMigratingAwayVms && CollectionUtils.isEmpty(_vmDao.findByHostInStates(host.getId(), State.Migrating, State.Running, State.Starting, State.Stopping, State.Error, State.Unknown))) { if (hasPendingMigrationRetries) { s_logger.error("There should not be pending retries VMs for this host as there are no running, migrating," + @@ -1431,7 +1432,6 @@ protected boolean attemptMaintain(HostVO host) throws NoTransitionException { // Step 2: Gather relevant VMs' states on the host and then based on them we can determine if final List failedMigrations = new ArrayList<>(_vmDao.listNonMigratingVmsByHostEqualsLastHost(hostId)); final List errorVms = new ArrayList<>(_vmDao.findByHostInStates(hostId, State.Unknown, State.Error)); - final boolean hasMigratingAwayVms = CollectionUtils.isNotEmpty(_vmDao.listVmsMigratingFromHost(hostId)); final boolean hasRunningVms = CollectionUtils.isNotEmpty(_vmDao.findByHostInStates(hostId, State.Running)); final boolean hasFailedMigrations = CollectionUtils.isNotEmpty(failedMigrations); final boolean hasVmsInFailureStates = CollectionUtils.isNotEmpty(errorVms); From 1c90d88851ecdf5a663e4eacc993b4ee2fee510b Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Thu, 28 Nov 2019 21:50:20 +0530 Subject: [PATCH 19/19] Refactor to use single place for methods checking maintenance states --- .../main/java/com/cloud/resource/ResourceState.java | 7 ++++++- .../java/com/cloud/resource/ResourceManagerImpl.java | 10 +++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/com/cloud/resource/ResourceState.java b/api/src/main/java/com/cloud/resource/ResourceState.java index 51c008ec8cb4..9b3bafe28dbe 100644 --- a/api/src/main/java/com/cloud/resource/ResourceState.java +++ b/api/src/main/java/com/cloud/resource/ResourceState.java @@ -99,6 +99,11 @@ public static boolean isMaintenanceState(ResourceState state) { ResourceState.PrepareForMaintenance, ResourceState.ErrorInPrepareForMaintenance).contains(state); } + public static boolean canAttemptMaintenance(ResourceState state) { + return !Arrays.asList(ResourceState.Maintenance, ResourceState.PrepareForMaintenance, + ResourceState.ErrorInPrepareForMaintenance).contains(state); + } + protected static final StateMachine s_fsm = new StateMachine(); static { s_fsm.addTransition(null, Event.InternalCreated, ResourceState.Enabled); @@ -129,9 +134,9 @@ public static boolean isMaintenanceState(ResourceState state) { s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.UnableToMaintain, ResourceState.ErrorInMaintenance); s_fsm.addTransition(ResourceState.ErrorInPrepareForMaintenance, Event.ErrorsCorrected, ResourceState.PrepareForMaintenance); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.InternalCreated, ResourceState.ErrorInMaintenance); + s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.AdminAskMaintenance, ResourceState.PrepareForMaintenance); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.Disable, ResourceState.Disabled); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.DeleteHost, ResourceState.Disabled); - s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.InternalEnterMaintenance, ResourceState.Maintenance); s_fsm.addTransition(ResourceState.ErrorInMaintenance, Event.AdminCancelMaintenance, ResourceState.Enabled); s_fsm.addTransition(ResourceState.Error, Event.InternalCreated, ResourceState.Error); s_fsm.addTransition(ResourceState.Disabled, Event.DeleteHost, ResourceState.Disabled); diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index b1c22700cedc..29f7e68e08c2 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -20,7 +20,6 @@ import java.net.URISyntaxException; import java.net.URLDecoder; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; @@ -1216,7 +1215,7 @@ private boolean doMaintain(final long hostId) { final HostVO host = _hostDao.findById(hostId); s_logger.info("Maintenance: attempting maintenance of host " + host.getUuid()); ResourceState hostState = host.getResourceState(); - if (ResourceState.isMaintenanceState(hostState)) { + if (!ResourceState.canAttemptMaintenance(hostState)) { throw new CloudRuntimeException("Cannot perform maintain when resource state is " + hostState + ", hostId = " + hostId); } @@ -1284,7 +1283,7 @@ public Host maintain(final PrepareForMaintenanceCmd cmd) { s_logger.debug("Unable to find host " + hostId); throw new InvalidParameterValueException("Unable to find host with ID: " + hostId + ". Please specify a valid host ID."); } - if (Arrays.asList(ResourceState.Maintenance, ResourceState.PrepareForMaintenance, ResourceState.ErrorInPrepareForMaintenance).contains(host.getResourceState())) { + if (!ResourceState.canAttemptMaintenance(host.getResourceState())) { throw new CloudRuntimeException("Host is already in state " + host.getResourceState() + ". Cannot recall for maintenance until resolved."); } @@ -2398,10 +2397,7 @@ private boolean doCancelMaintenance(final long hostId) { * TODO: think twice about returning true or throwing out exception, I * really prefer to exception that always exposes bugs */ - if (host.getResourceState() != ResourceState.PrepareForMaintenance && - host.getResourceState() != ResourceState.ErrorInPrepareForMaintenance && - host.getResourceState() != ResourceState.Maintenance && - host.getResourceState() != ResourceState.ErrorInMaintenance) { + if (!ResourceState.isMaintenanceState(host.getResourceState())) { throw new CloudRuntimeException("Cannot perform cancelMaintenance when resource state is " + host.getResourceState() + ", hostId = " + hostId); }