From cc35f9ddb0ca938a22c79aa02c42552d6656881c Mon Sep 17 00:00:00 2001 From: nvazquez Date: Thu, 15 Mar 2018 20:56:36 -0300 Subject: [PATCH 1/7] CLOUDSTACK-10326: Prevent hosts fall into Maintenance when there are running VMs on it --- engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java | 5 +++++ .../schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java | 9 +++++++++ server/src/com/cloud/resource/ResourceManagerImpl.java | 3 ++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java b/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java index 69efea42df9..addc46c1af8 100755 --- a/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java +++ b/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java @@ -150,4 +150,9 @@ public interface VMInstanceDao extends GenericDao, StateDao< VMInstanceVO findVMByHostNameInZone(String hostName, long zoneId); boolean isPowerStateUpToDate(long instanceId); + + /** + * List running VMs which host and lastHost id are the same + */ + List listRunningVmsByHostAndLastHostSameId(long hostId); } diff --git a/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java b/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java index 6e97d1275a6..cb7592403fc 100755 --- a/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java +++ b/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java @@ -304,6 +304,15 @@ public class VMInstanceDaoImpl extends GenericDaoBase implem return listBy(sc); } + @Override + public List listRunningVmsByHostAndLastHostSameId(long hostId) { + SearchCriteria sc = AllFieldsSearch.create(); + sc.setParameters("host", hostId); + sc.setParameters("lastHost", hostId); + sc.setParameters("state", State.Running); + return listBy(sc); + } + @Override public List listByZoneId(long zoneId) { SearchCriteria sc = AllFieldsSearch.create(); diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java b/server/src/com/cloud/resource/ResourceManagerImpl.java index 2966d41d8bf..3797a0d2303 100755 --- a/server/src/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/com/cloud/resource/ResourceManagerImpl.java @@ -1296,7 +1296,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, if (host.getType() != Host.Type.Storage) { final List vos = _vmDao.listByHostId(hostId); final List vosMigrating = _vmDao.listVmsMigratingFromHost(hostId); - if (vos.isEmpty() && vosMigrating.isEmpty()) { + final List failedMigratedVms = _vmDao.listRunningVmsByHostAndLastHostSameId(hostId); + if (vos.isEmpty() && vosMigrating.isEmpty() && failedMigratedVms.isEmpty()) { resourceStateTransitTo(host, ResourceState.Event.InternalEnterMaintenance, _nodeId); hostInMaintenance = true; ActionEventUtils.onCompletedActionEvent(CallContext.current().getCallingUserId(), CallContext.current().getCallingAccountId(), EventVO.LEVEL_INFO, EventTypes.EVENT_MAINTENANCE_PREPARE, "completed maintenance for host " + hostId, 0); From 08a8330633c5c3f87535239de99b9f1393a18df3 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Sun, 18 Mar 2018 15:49:43 -0300 Subject: [PATCH 2/7] CLOUDSTACK-10326: Fix for infinite loop on PrepareForMaintenance --- .../src/com/cloud/vm/dao/VMInstanceDao.java | 5 +---- .../src/com/cloud/vm/dao/VMInstanceDaoImpl.java | 12 +++++++++--- .../com/cloud/resource/ResourceManagerImpl.java | 16 +++++++++++----- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java b/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java index addc46c1af8..6fda4a15c32 100755 --- a/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java +++ b/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java @@ -151,8 +151,5 @@ public interface VMInstanceDao extends GenericDao, StateDao< boolean isPowerStateUpToDate(long instanceId); - /** - * List running VMs which host and lastHost id are the same - */ - List listRunningVmsByHostAndLastHostSameId(long hostId); + List listNonMigratingVmsByHostEqualsLastHost(long hostId); } diff --git a/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java b/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java index cb7592403fc..1565f53233b 100755 --- a/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java +++ b/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java @@ -93,6 +93,7 @@ public class VMInstanceDaoImpl extends GenericDaoBase implem protected GenericSearchBuilder DistinctHostNameSearch; protected SearchBuilder HostAndStateSearch; protected SearchBuilder StartingWithNoHostSearch; + protected SearchBuilder NotMigratingSearch; @Inject ResourceTagDao _tagsDao; @@ -280,6 +281,11 @@ public class VMInstanceDaoImpl extends GenericDaoBase implem DistinctHostNameSearch.join("nicSearch", nicSearch, DistinctHostNameSearch.entity().getId(), nicSearch.entity().getInstanceId(), JoinBuilder.JoinType.INNER); DistinctHostNameSearch.done(); + NotMigratingSearch = createSearchBuilder(); + NotMigratingSearch.and("host", NotMigratingSearch.entity().getHostId(), Op.EQ); + NotMigratingSearch.and("lastHost", NotMigratingSearch.entity().getLastHostId(), Op.EQ); + NotMigratingSearch.and("state", NotMigratingSearch.entity().getState(), Op.NEQ); + NotMigratingSearch.done(); } @Override @@ -305,11 +311,11 @@ public class VMInstanceDaoImpl extends GenericDaoBase implem } @Override - public List listRunningVmsByHostAndLastHostSameId(long hostId) { - SearchCriteria sc = AllFieldsSearch.create(); + public List listNonMigratingVmsByHostEqualsLastHost(long hostId) { + SearchCriteria sc = NotMigratingSearch.create(); sc.setParameters("host", hostId); sc.setParameters("lastHost", hostId); - sc.setParameters("state", State.Running); + sc.setParameters("state", State.Migrating); return listBy(sc); } diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java b/server/src/com/cloud/resource/ResourceManagerImpl.java index 3797a0d2303..25bfc4c9c67 100755 --- a/server/src/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/com/cloud/resource/ResourceManagerImpl.java @@ -1296,11 +1296,17 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, if (host.getType() != Host.Type.Storage) { final List vos = _vmDao.listByHostId(hostId); final List vosMigrating = _vmDao.listVmsMigratingFromHost(hostId); - final List failedMigratedVms = _vmDao.listRunningVmsByHostAndLastHostSameId(hostId); - if (vos.isEmpty() && vosMigrating.isEmpty() && failedMigratedVms.isEmpty()) { - resourceStateTransitTo(host, ResourceState.Event.InternalEnterMaintenance, _nodeId); - hostInMaintenance = true; - ActionEventUtils.onCompletedActionEvent(CallContext.current().getCallingUserId(), CallContext.current().getCallingAccountId(), EventVO.LEVEL_INFO, EventTypes.EVENT_MAINTENANCE_PREPARE, "completed maintenance for host " + hostId, 0); + final List failedMigratedVms = _vmDao.listNonMigratingVmsByHostEqualsLastHost(hostId); + if (vos.isEmpty() && vosMigrating.isEmpty()) { + if (!failedMigratedVms.isEmpty()) { + s_logger.debug("Unable to migrate " + failedMigratedVms.size() + " VM(s) from host " + host.getUuid()); + resourceStateTransitTo(host, ResourceState.Event.UnableToMigrate, _nodeId); + } else { + s_logger.debug("Host " + host.getUuid() + " entering in Maintenance"); + resourceStateTransitTo(host, ResourceState.Event.InternalEnterMaintenance, _nodeId); + hostInMaintenance = true; + ActionEventUtils.onCompletedActionEvent(CallContext.current().getCallingUserId(), CallContext.current().getCallingAccountId(), EventVO.LEVEL_INFO, EventTypes.EVENT_MAINTENANCE_PREPARE, "completed maintenance for host " + hostId, 0); + } } } } catch (final NoTransitionException e) { From a22ab69bb6c1e45ce43d4c00520d4ea8656d21f2 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 12 Jun 2018 09:42:09 -0300 Subject: [PATCH 3/7] Set host into ErrorInMaintenance in case of failure trying to enter Maintenance mode --- .../cloud/resource/ResourceManagerImpl.java | 82 ++++++++++++++++--- .../cloud/servlet/ConsoleProxyServlet.java | 10 ++- 2 files changed, 79 insertions(+), 13 deletions(-) diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java b/server/src/com/cloud/resource/ResourceManagerImpl.java index 25bfc4c9c67..8eb53e62587 100755 --- a/server/src/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/com/cloud/resource/ResourceManagerImpl.java @@ -30,6 +30,7 @@ import java.util.Random; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.vm.dao.UserVmDetailsDao; import org.apache.commons.lang.ObjectUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -54,6 +55,8 @@ import org.apache.commons.collections.CollectionUtils; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; +import com.cloud.agent.api.GetVncPortCommand; +import com.cloud.agent.api.GetVncPortAnswer; import com.cloud.agent.api.GetGPUStatsAnswer; import com.cloud.agent.api.GetGPUStatsCommand; import com.cloud.agent.api.GetHostStatsAnswer; @@ -252,6 +255,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, private ConfigurationManager _configMgr; @Inject private ClusterVSMMapDao _clusterVSMMapDao; + @Inject + private UserVmDetailsDao userVmDetailsDao; private final long _nodeId = ManagementServerNode.getManagementServerId(); @@ -1287,6 +1292,68 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, } } + /** + * Add VNC details as user VM details for each VM in 'vms' (KVM hosts only) + */ + private void setKVMVncAccess(long hostId, List vms) { + for (VMInstanceVO vm : vms) { + GetVncPortAnswer vmVncPortAnswer = (GetVncPortAnswer) _agentMgr.easySend(hostId, new GetVncPortCommand(vm.getId(), vm.getInstanceName())); + if (vmVncPortAnswer != null) { + userVmDetailsDao.addDetail(vm.getId(), "kvm.vnc.address", vmVncPortAnswer.getAddress(), true); + userVmDetailsDao.addDetail(vm.getId(), "kvm.vnc.port", String.valueOf(vmVncPortAnswer.getPort()), true); + } + } + } + + /** + * Configure VNC access for host VMs which have failed migrating to another host while trying to enter Maintenance mode + */ + private void configureVncAccessForKVMHostFailedMigrations(HostVO host, List failedMigrations) { + if (host.getHypervisorType().equals(HypervisorType.KVM)) { + _agentMgr.pullAgentOutMaintenance(host.getId()); + setKVMVncAccess(host.getId(), failedMigrations); + _agentMgr.pullAgentToMaintenance(host.getId()); + } + } + + /** + * Set host into ErrorInMaintenance state, as errors occurred during VM migrations. Do the following: + * - Cancel scheduled migrations for those which have already failed + * - Configure VNC access for VMs (KVM hosts only) + */ + private boolean setHostIntoErrorInMaintenance(HostVO host, List failedMigrations) throws NoTransitionException { + 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); + return false; + } + + /** + * Safely transit host into Maintenance mode + */ + private 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; + } + + /** + * Return true if host goes into Maintenance mode, only when: + * - No Running, Migrating or Failed migrations (host_id = last_host_id) for the host + */ + private 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); + } + return false; + } + @Override public boolean checkAndMaintain(final long hostId) { boolean hostInMaintenance = false; @@ -1296,18 +1363,9 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, if (host.getType() != Host.Type.Storage) { final List vos = _vmDao.listByHostId(hostId); final List vosMigrating = _vmDao.listVmsMigratingFromHost(hostId); - final List failedMigratedVms = _vmDao.listNonMigratingVmsByHostEqualsLastHost(hostId); - if (vos.isEmpty() && vosMigrating.isEmpty()) { - if (!failedMigratedVms.isEmpty()) { - s_logger.debug("Unable to migrate " + failedMigratedVms.size() + " VM(s) from host " + host.getUuid()); - resourceStateTransitTo(host, ResourceState.Event.UnableToMigrate, _nodeId); - } else { - s_logger.debug("Host " + host.getUuid() + " entering in Maintenance"); - resourceStateTransitTo(host, ResourceState.Event.InternalEnterMaintenance, _nodeId); - hostInMaintenance = true; - ActionEventUtils.onCompletedActionEvent(CallContext.current().getCallingUserId(), CallContext.current().getCallingAccountId(), EventVO.LEVEL_INFO, EventTypes.EVENT_MAINTENANCE_PREPARE, "completed maintenance for host " + hostId, 0); - } - } + final List failedVmMigrations = _vmDao.listNonMigratingVmsByHostEqualsLastHost(hostId); + + hostInMaintenance = isHostInMaintenance(host, vos, vosMigrating, failedVmMigrations); } } catch (final NoTransitionException e) { s_logger.debug("Cannot transmit host " + host.getId() + "to Maintenance state", e); diff --git a/server/src/com/cloud/servlet/ConsoleProxyServlet.java b/server/src/com/cloud/servlet/ConsoleProxyServlet.java index cc788c7b118..8cfaa9fd69b 100644 --- a/server/src/com/cloud/servlet/ConsoleProxyServlet.java +++ b/server/src/com/cloud/servlet/ConsoleProxyServlet.java @@ -35,6 +35,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +import com.cloud.resource.ResourceState; import org.apache.commons.codec.binary.Base64; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -418,7 +419,14 @@ public class ConsoleProxyServlet extends HttpServlet { StringBuffer sb = new StringBuffer(rootUrl); String host = hostVo.getPrivateIpAddress(); - Pair portInfo = _ms.getVncPort(vm); + Pair portInfo; + if (hostVo.getResourceState().equals(ResourceState.ErrorInMaintenance)) { + UserVmDetailVO detailAddress = _userVmDetailsDao.findDetail(vm.getId(), "kvm.vnc.address"); + UserVmDetailVO detailPort = _userVmDetailsDao.findDetail(vm.getId(), "kvm.vnc.port"); + portInfo = new Pair<>(detailAddress.getValue(), Integer.valueOf(detailPort.getValue())); + } else { + portInfo = _ms.getVncPort(vm); + } if (s_logger.isDebugEnabled()) s_logger.debug("Port info " + portInfo.first()); From faf2a7760d6b321259fca4921970357cb49db32a Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 12 Jun 2018 11:56:41 -0300 Subject: [PATCH 4/7] Add unit tests --- .../cloud/resource/ResourceManagerImpl.java | 10 +- .../resource/ResourceManagerImplTest.java | 191 ++++++++++++++++++ 2 files changed, 196 insertions(+), 5 deletions(-) create mode 100644 server/test/com/cloud/resource/ResourceManagerImplTest.java diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java b/server/src/com/cloud/resource/ResourceManagerImpl.java index 8eb53e62587..ba3c157cf9f 100755 --- a/server/src/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/com/cloud/resource/ResourceManagerImpl.java @@ -1295,7 +1295,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, /** * Add VNC details as user VM details for each VM in 'vms' (KVM hosts only) */ - private void setKVMVncAccess(long hostId, List vms) { + protected void setKVMVncAccess(long hostId, List vms) { for (VMInstanceVO vm : vms) { GetVncPortAnswer vmVncPortAnswer = (GetVncPortAnswer) _agentMgr.easySend(hostId, new GetVncPortCommand(vm.getId(), vm.getInstanceName())); if (vmVncPortAnswer != null) { @@ -1308,7 +1308,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, /** * Configure VNC access for host VMs which have failed migrating to another host while trying to enter Maintenance mode */ - private void configureVncAccessForKVMHostFailedMigrations(HostVO host, List failedMigrations) { + protected void configureVncAccessForKVMHostFailedMigrations(HostVO host, List failedMigrations) { if (host.getHypervisorType().equals(HypervisorType.KVM)) { _agentMgr.pullAgentOutMaintenance(host.getId()); setKVMVncAccess(host.getId(), failedMigrations); @@ -1321,7 +1321,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, * - Cancel scheduled migrations for those which have already failed * - Configure VNC access for VMs (KVM hosts only) */ - private boolean setHostIntoErrorInMaintenance(HostVO host, List failedMigrations) throws NoTransitionException { + protected boolean setHostIntoErrorInMaintenance(HostVO host, List failedMigrations) throws NoTransitionException { s_logger.debug("Unable to migrate " + failedMigrations.size() + " VM(s) from host " + host.getUuid()); _haMgr.cancelScheduledMigrations(host); configureVncAccessForKVMHostFailedMigrations(host, failedMigrations); @@ -1332,7 +1332,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, /** * Safely transit host into Maintenance mode */ - private boolean setHostIntoMaintenance(HostVO host) throws NoTransitionException { + 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(), @@ -1345,7 +1345,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, * Return true if host goes into Maintenance mode, only when: * - No Running, Migrating or Failed migrations (host_id = last_host_id) for the host */ - private boolean isHostInMaintenance(HostVO host, List runningVms, List migratingVms, List failedMigrations) throws NoTransitionException { + 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) : diff --git a/server/test/com/cloud/resource/ResourceManagerImplTest.java b/server/test/com/cloud/resource/ResourceManagerImplTest.java new file mode 100644 index 00000000000..525ccd0c071 --- /dev/null +++ b/server/test/com/cloud/resource/ResourceManagerImplTest.java @@ -0,0 +1,191 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package com.cloud.resource; + +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.GetVncPortAnswer; +import com.cloud.agent.api.GetVncPortCommand; +import com.cloud.capacity.dao.CapacityDao; +import com.cloud.event.ActionEventUtils; +import com.cloud.ha.HighAvailabilityManager; +import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor; +import com.cloud.storage.StorageManager; +import com.cloud.utils.fsm.NoTransitionException; +import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.dao.UserVmDetailsDao; +import com.cloud.vm.dao.VMInstanceDao; +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.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@RunWith(PowerMockRunner.class) +@PrepareForTest({ActionEventUtils.class, ResourceManagerImpl.class}) +public class ResourceManagerImplTest { + + @Mock + private CapacityDao capacityDao; + @Mock + private StorageManager storageManager; + @Mock + private HighAvailabilityManager haManager; + @Mock + private UserVmDetailsDao userVmDetailsDao; + @Mock + private AgentManager agentManager; + @Mock + private HostDao hostDao; + @Mock + private VMInstanceDao vmInstanceDao; + + @Spy + @InjectMocks + private ResourceManagerImpl resourceManager = new ResourceManagerImpl(); + + @Mock + private HostVO host; + @Mock + private VMInstanceVO vm1; + @Mock + private VMInstanceVO vm2; + + @Mock + private GetVncPortAnswer getVncPortAnswerVm1; + @Mock + private GetVncPortAnswer getVncPortAnswerVm2; + @Mock + private GetVncPortCommand getVncPortCommandVm1; + @Mock + private GetVncPortCommand getVncPortCommandVm2; + + private static long hostId = 1L; + + private static long vm1Id = 1L; + private static String vm1InstanceName = "i-1-VM"; + private static long vm2Id = 2L; + private static String vm2InstanceName = "i-2-VM"; + + private static String vm1VncAddress = "10.2.2.2"; + private static int vm1VncPort = 5900; + private static String vm2VncAddress = "10.2.2.2"; + private static int vm2VncPort = 5901; + + @Before + public void setup() throws Exception { + MockitoAnnotations.initMocks(this); + when(host.getType()).thenReturn(Host.Type.Routing); + when(host.getId()).thenReturn(hostId); + when(host.getResourceState()).thenReturn(ResourceState.Enabled); + when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.VMware); + when(hostDao.findById(hostId)).thenReturn(host); + when(vm1.getId()).thenReturn(vm1Id); + when(vm2.getId()).thenReturn(vm2Id); + when(vm1.getInstanceName()).thenReturn(vm1InstanceName); + when(vm2.getInstanceName()).thenReturn(vm2InstanceName); + when(vmInstanceDao.listByHostId(hostId)).thenReturn(new ArrayList<>()); + when(vmInstanceDao.listVmsMigratingFromHost(hostId)).thenReturn(new ArrayList<>()); + when(vmInstanceDao.listNonMigratingVmsByHostEqualsLastHost(hostId)).thenReturn(new ArrayList<>()); + PowerMockito.mockStatic(ActionEventUtils.class); + BDDMockito.given(ActionEventUtils.onCompletedActionEvent(anyLong(), anyLong(), anyString(), anyString(), anyString(), anyLong())) + .willReturn(1L); + when(getVncPortAnswerVm1.getAddress()).thenReturn(vm1VncAddress); + when(getVncPortAnswerVm1.getPort()).thenReturn(vm1VncPort); + when(getVncPortAnswerVm2.getAddress()).thenReturn(vm2VncAddress); + when(getVncPortAnswerVm2.getPort()).thenReturn(vm2VncPort); + PowerMockito.whenNew(GetVncPortCommand.class).withArguments(vm1Id, vm1InstanceName).thenReturn(getVncPortCommandVm1); + PowerMockito.whenNew(GetVncPortCommand.class).withArguments(vm2Id, vm2InstanceName).thenReturn(getVncPortCommandVm2); + when(agentManager.easySend(eq(hostId), eq(getVncPortCommandVm1))).thenReturn(getVncPortAnswerVm1); + when(agentManager.easySend(eq(hostId), eq(getVncPortCommandVm2))).thenReturn(getVncPortAnswerVm2); + } + + @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); + List vms = Arrays.asList(vm1, vm2); + resourceManager.configureVncAccessForKVMHostFailedMigrations(host, vms); + verify(agentManager).pullAgentOutMaintenance(hostId); + verify(resourceManager).setKVMVncAccess(hostId, vms); + verify(agentManager, times(vms.size())).easySend(eq(hostId), any(GetVncPortCommand.class)); + verify(userVmDetailsDao).addDetail(eq(vm1Id), eq("kvm.vnc.address"), eq(vm1VncAddress), anyBoolean()); + verify(userVmDetailsDao).addDetail(eq(vm1Id), eq("kvm.vnc.port"), eq(String.valueOf(vm1VncPort)), anyBoolean()); + verify(userVmDetailsDao).addDetail(eq(vm2Id), eq("kvm.vnc.address"), eq(vm2VncAddress), anyBoolean()); + verify(userVmDetailsDao).addDetail(eq(vm2Id), eq("kvm.vnc.port"), eq(String.valueOf(vm2VncPort)), anyBoolean()); + verify(agentManager).pullAgentToMaintenance(hostId); + } +} From d4e302dcc67ca740242f09ef43ca97a72696c204 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Wed, 20 Jun 2018 21:18:38 +0530 Subject: [PATCH 5/7] smoketest: Fix test_vm_life_cycle secure migration tests (#2715) This fixes intermittently failing secure live VM migration tests on KVM. Signed-off-by: Rohit Yadav --- test/integration/smoke/test_vm_life_cycle.py | 290 ++++++++----------- 1 file changed, 119 insertions(+), 171 deletions(-) diff --git a/test/integration/smoke/test_vm_life_cycle.py b/test/integration/smoke/test_vm_life_cycle.py index a2daee85c10..e9970b6f212 100644 --- a/test/integration/smoke/test_vm_life_cycle.py +++ b/test/integration/smoke/test_vm_life_cycle.py @@ -839,7 +839,6 @@ class TestSecuredVmMigration(cloudstackTestCase): @classmethod def tearDownClass(cls): - cls.apiclient = super(TestSecuredVmMigration, cls).getClsTestClient().getApiClient() try: cleanup_resources(cls.apiclient, cls._cleanup) @@ -850,136 +849,30 @@ class TestSecuredVmMigration(cloudstackTestCase): self.apiclient = self.testClient.getApiClient() self.dbclient = self.testClient.getDbConnection() self.cleanup = [] + if self.hypervisor.lower() not in ["kvm"]: self.skipTest("Secured migration is not supported on other than KVM") + self.hosts = Host.list( + self.apiclient, + zoneid=self.zone.id, + type='Routing', + hypervisor='KVM') + + if len(self.hosts) < 2: + self.skipTest("Requires at least two hosts for performing migration related tests") + + self.secure_all_hosts() self.updateConfiguration("ca.plugin.root.auth.strictness", "false") - self.make_all_hosts_secure() def tearDown(self): - self.make_all_hosts_secure() - + self.secure_all_hosts() + self.updateConfiguration("ca.plugin.root.auth.strictness", "true") try: cleanup_resources(self.apiclient, self.cleanup) except Exception as e: raise Exception("Warning: Exception during cleanup : %s" % e) - @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false") - def test_01_secured_vm_migration(self): - """Test secured VM migration""" - - # Validate the following - # 1. Environment has enough hosts for migration - # 2. DeployVM on suitable host (with another host in the cluster) - # 3. Migrate the VM and assert migration successful - - hosts = self.get_hosts() - - secured_hosts = [] - - for host in hosts: - if host.details.secured == 'true': - secured_hosts.append(host) - - if len(secured_hosts) < 2: - self.skipTest("At least two hosts should be present in the zone for migration") - - origin_host = secured_hosts[0] - - self.vm_to_migrate = self.deploy_vm(origin_host) - - target_host = self.get_target_host(secured='true', virtualmachineid=self.vm_to_migrate.id) - - self.migrate_and_check(origin_host, target_host, proto='tls') - - @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false") - def test_02_not_secured_vm_migration(self): - """Test Non-secured VM Migration - """ - #self.skipTest() - # Validate the following - # 1. Prepare 2 hosts to run in non-secured more - # 2. DeployVM on suitable host (with another host in the cluster) - # 3. Migrate the VM and assert migration successful - hosts = self.get_hosts() - for host in hosts: - self.make_unsecure_connection(host) - - non_secured_hosts = [] - - hosts = self.get_hosts() - - for host in hosts: - if host.details.secured == 'false': - non_secured_hosts.append(host) - - if len(non_secured_hosts) < 2: - self.skipTest("At least two hosts should be present in the zone for migration") - origin_host = non_secured_hosts[0] - - self.vm_to_migrate = self.deploy_vm(origin_host) - - target_host = self.get_target_host(secured='false', virtualmachineid=self.vm_to_migrate.id) - - self.migrate_and_check(origin_host, target_host, proto='tcp') - - @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false") - def test_03_secured_to_nonsecured_vm_migration(self): - """Test destroy Virtual Machine - """ - - # Validate the following - # 1. Makes one of the hosts non-secured - # 2. Deploys a VM to a Secured host - # 3. Migrates the VM to the non-secured host and assers the migration is via TCP. - - hosts = self.get_hosts() - - non_secured_host = self.make_unsecure_connection(hosts[0]) - - secured_hosts = [] - hosts = self.get_hosts() - - for host in hosts: - if host.details.secured == 'true': - secured_hosts.append(host) - - self.vm_to_migrate = self.deploy_vm(secured_hosts[0]) - try: - self.migrate_and_check(origin_host=secured_hosts[0], destination_host=non_secured_host, proto='tcp') - except Exception: - pass - else: self.fail("Migration succeed, instead it should fail") - - @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false") - def test_04_nonsecured_to_secured_vm_migration(self): - """Test Non-secured VM Migration - """ - - # Validate the following - # 1. Makes one of the hosts non-secured - # 2. Deploys a VM to the non-secured host - # 3. Migrates the VM to the secured host and assers the migration is via TCP. - hosts = self.get_hosts() - - non_secured_host = self.make_unsecure_connection(hosts[0]) - - secured_hosts = [] - - hosts = self.get_hosts() - for host in hosts: - if host.details.secured == 'true': - secured_hosts.append(host) - - self.vm_to_migrate = self.deploy_vm(non_secured_host) - - try: - self.migrate_and_check(origin_host=non_secured_host, destination_host=secured_hosts[0], proto='tcp') - except Exception: - pass - else: - self.fail("Migration succeed, instead it should fail") - def get_target_host(self, secured, virtualmachineid): target_hosts = Host.listForMigration(self.apiclient, virtualmachineid=virtualmachineid) @@ -992,55 +885,44 @@ class TestSecuredVmMigration(cloudstackTestCase): def check_migration_protocol(self, protocol, host): resp = SshClient(host.ipaddress, port=22, user=self.hostConfig["username"],passwd=self.hostConfig["password"])\ - .execute("grep -a Live /var/log/cloudstack/agent/agent.log | tail -1") + .execute("grep -a listen_%s=1 /etc/libvirt/libvirtd.conf | tail -1" % protocol) if protocol not in resp[0]: - cloudstackTestCase.fail(self, "Migration protocol was not as expected: '" + protocol + "\n" - "Instead we got: " + resp[0]) + cloudstackTestCase.fail(self, "Libvirt listen protocol expected: '" + protocol + "\n" + "does not match actual: " + resp[0]) - def make_unsecure_connection(self, host): - SshClient(host.ipaddress, port=22, user=self.hostConfig["username"],passwd=self.hostConfig["password"])\ - .execute("rm -f /etc/cloudstack/agent/cloud*") + def migrate_and_check(self, vm, src_host, dest_host, proto='tls'): + """ + Migrates a VM from source host to destination host and checks status + """ + self.check_migration_protocol(protocol=proto, host=src_host) + vm.migrate(self.apiclient, hostid=dest_host.id) + vm_response = VirtualMachine.list(self.apiclient, id=vm.id)[0] + self.assertEqual(vm_response.hostid, dest_host.id, "Check destination host ID of migrated VM") - SshClient(host.ipaddress, port=22, user=self.hostConfig["username"],passwd=self.hostConfig["password"])\ - .execute("sed -i 's/listen_tls.*/listen_tls=0/g' /etc/libvirt/libvirtd.conf") - SshClient(host.ipaddress, port=22, user=self.hostConfig["username"],passwd=self.hostConfig["password"])\ - .execute("sed -i 's/listen_tcp.*/listen_tcp=1/g' /etc/libvirt/libvirtd.conf ") - SshClient(host.ipaddress, port=22, user=self.hostConfig["username"],passwd=self.hostConfig["password"])\ - .execute("sed -i '/.*_file.*/d' /etc/libvirt/libvirtd.conf") - SshClient(host.ipaddress, port=22, user=self.hostConfig["username"],passwd=self.hostConfig["password"])\ - .execute("service libvirtd restart") - SshClient(host.ipaddress, port=22, user=self.hostConfig["username"],passwd=self.hostConfig["password"])\ - .execute("service cloudstack-agent restart") + def unsecure_host(self, host): + SshClient(host.ipaddress, port=22, user=self.hostConfig["username"], passwd=self.hostConfig["password"])\ + .execute("rm -f /etc/cloudstack/agent/cloud* && \ + sed -i 's/listen_tls.*/listen_tls=0/g' /etc/libvirt/libvirtd.conf && \ + sed -i 's/listen_tcp.*/listen_tcp=1/g' /etc/libvirt/libvirtd.conf && \ + sed -i '/.*_file=.*/d' /etc/libvirt/libvirtd.conf && \ + service libvirtd restart && \ + service cloudstack-agent restart") - self.check_connection(host=host, secured='false') time.sleep(10) + self.check_connection(host=host, secured='false') return host - def make_all_hosts_secure(self): - hosts = Host.list( - self.apiclient, - zoneid=self.zone.id, - type='Routing' - ) - for host in hosts: + def secure_all_hosts(self): + for host in self.hosts: cmd = provisionCertificate.provisionCertificateCmd() cmd.hostid = host.id + cmd.reconnect = True self.apiclient.updateConfiguration(cmd) - for host in hosts: + for host in self.hosts: self.check_connection(secured='true', host=host) - def get_hosts(self): - - hosts = Host.list( - self.apiclient, - zoneid=self.zone.id, - type='Routing' - ) - self.assertEqual(validateList(hosts)[0], PASS, "hosts list validation failed") - return hosts - def deploy_vm(self, origin_host): return VirtualMachine.create( self.apiclient, @@ -1049,10 +931,9 @@ class TestSecuredVmMigration(cloudstackTestCase): domainid=self.account.domainid, serviceofferingid=self.small_offering.id, mode=self.services["mode"], - hostid=origin_host.id - ) + hostid=origin_host.id) - def check_connection(self, secured, host, retries=5, interval=5): + def check_connection(self, secured, host, retries=20, interval=6): while retries > -1: time.sleep(interval) @@ -1069,21 +950,88 @@ class TestSecuredVmMigration(cloudstackTestCase): else: return - raise Exception("Host communication is not as expected: " + secured + - ". Instead it's: " + host.details.secured) - - def migrate_and_check(self, origin_host, destination_host, proto): - - self.vm_to_migrate.migrate(self.apiclient, hostid=destination_host.id) - - self.check_migration_protocol(protocol=proto, host=origin_host) - - vm_response = VirtualMachine.list(self.apiclient, id=self.vm_to_migrate.id)[0] - - self.assertEqual(vm_response.hostid, destination_host.id, "Check destination hostID of migrated VM") + raise Exception("Host detail 'secured' was expected: " + secured + + ", actual is: " + host.details.secured) def updateConfiguration(self, name, value): cmd = updateConfiguration.updateConfigurationCmd() cmd.name = name cmd.value = value - self.apiclient.updateConfiguration(cmd) \ No newline at end of file + self.apiclient.updateConfiguration(cmd) + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg", "security"], required_hardware="false") + def test_01_secure_vm_migration(self): + """Test secure VM migration""" + # Validate the following + # 1. Environment has enough hosts for migration + # 2. DeployVM on suitable host (with another host in the cluster) + # 3. Migrate the VM and assert migration successful + + src_host = self.hosts[0] + vm = self.deploy_vm(src_host) + self.cleanup.append(vm) + + dest_host = self.get_target_host(secured='true', virtualmachineid=vm.id) + self.migrate_and_check(vm, src_host, dest_host) + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg", "security"], required_hardware="false") + def test_02_unsecure_vm_migration(self): + """Test Non-secured VM Migration + """ + # Validate the following + # 1. Prepare 2 hosts to run in non-secured more + # 2. DeployVM on suitable host (with another host in the cluster) + # 3. Migrate the VM and assert migration successful + + for host in self.hosts: + self.unsecure_host(host) + + src_host = self.hosts[0] + vm = self.deploy_vm(src_host) + self.cleanup.append(vm) + + dest_host = self.get_target_host(secured='false', virtualmachineid=vm.id) + self.migrate_and_check(vm, src_host, dest_host, proto='tcp') + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg", "security"], required_hardware="false") + def test_03_secured_to_nonsecured_vm_migration(self): + """Test destroy Virtual Machine + """ + # Validate the following + # 1. Makes one of the hosts non-secured + # 2. Deploys a VM to a Secured host + # 3. Migrates the VM to the non-secured host via TLS, and ensure exception + + unsecure_host = self.unsecure_host(self.hosts[0]) + secure_host = self.hosts[1] + + vm = self.deploy_vm(secure_host) + self.cleanup.append(vm) + + try: + self.migrate_and_check(vm, secure_host, unsecure_host, proto='tls') + except Exception: + pass + else: self.fail("Migration succeeded, instead it should fail") + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg", "security"], required_hardware="false") + def test_04_nonsecured_to_secured_vm_migration(self): + """Test Non-secured VM Migration + """ + # Validate the following + # 1. Makes one of the hosts non-secured + # 2. Deploys a VM to the non-secured host + # 3. Migrates the VM to the non-secured host via TCP, and ensure exception + + unsecure_host = self.unsecure_host(self.hosts[0]) + secure_host = self.hosts[1] + + vm = self.deploy_vm(unsecure_host) + self.cleanup.append(vm) + + try: + self.migrate_and_check(vm, unsecure_host, secure_host, proto='tcp') + except Exception: + pass + else: self.fail("Migration succeeded, instead it should fail") + From f02e402ebb86d9ab5b6f34c04e57460ed53b7827 Mon Sep 17 00:00:00 2001 From: dahn Date: Thu, 21 Jun 2018 07:33:43 +0200 Subject: [PATCH 6/7] kvm: send unsupported answer only when applicable (#2714) Throw specific NPE child when command is known not to be known. Add unit tests. --- .../com/cloud/resource/RequestWrapper.java | 23 ++++++++++------ .../resource/LibvirtComputingResource.java | 11 +++++++- .../wrapper/LibvirtRequestWrapper.java | 3 +++ .../LibvirtComputingResourceTest.java | 27 +++++++++++++++++++ 4 files changed, 55 insertions(+), 9 deletions(-) diff --git a/core/src/com/cloud/resource/RequestWrapper.java b/core/src/com/cloud/resource/RequestWrapper.java index 4e754d60a29..d1a3c1f18cc 100644 --- a/core/src/com/cloud/resource/RequestWrapper.java +++ b/core/src/com/cloud/resource/RequestWrapper.java @@ -29,6 +29,15 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; public abstract class RequestWrapper { + static public class CommandNotSupported extends NullPointerException { + public CommandNotSupported(String msg) { + super(msg); + } + public CommandNotSupported(String msg, Throwable cause) { + super(msg); + initCause(cause); + } + } private static final Logger s_logger = Logger.getLogger(RequestWrapper.class); @@ -52,7 +61,7 @@ public abstract class RequestWrapper { keepResourceClass = keepResourceClass2; } catch (final ClassCastException e) { - throw new NullPointerException("No key found for '" + command.getClass() + "' in the Map!"); + throw new CommandNotSupported("No key found for '" + command.getClass() + "' in the Map!"); } } return resource; @@ -69,14 +78,14 @@ public abstract class RequestWrapper { final Class commandClass2 = (Class) keepCommandClass.getSuperclass(); if (commandClass2 == null) { - throw new NullPointerException("All the COMMAND hierarchy tree has been visited but no compliant key has been found for '" + commandClass + "'."); + throw new CommandNotSupported("All the COMMAND hierarchy tree has been visited but no compliant key has been found for '" + commandClass + "'."); } commandWrapper = resourceCommands.get(commandClass2); keepCommandClass = commandClass2; } catch (final ClassCastException e) { - throw new NullPointerException("No key found for '" + keepCommandClass.getClass() + "' in the Map!"); + throw new CommandNotSupported("No key found for '" + keepCommandClass.getClass() + "' in the Map!"); } catch (final NullPointerException e) { // Will now traverse all the resource hierarchy. Returning null // is not a problem. @@ -102,7 +111,7 @@ public abstract class RequestWrapper { final Class resourceClass2 = (Class) keepResourceClass.getSuperclass(); if (resourceClass2 == null) { - throw new NullPointerException("All the SERVER-RESOURCE hierarchy tree has been visited but no compliant key has been found for '" + command.getClass() + "'."); + throw new CommandNotSupported("All the SERVER-RESOURCE hierarchy tree has been visited but no compliant key has been found for '" + command.getClass() + "'."); } final Hashtable, CommandWrapper> resourceCommands2 = retrieveResource(command, @@ -110,10 +119,8 @@ public abstract class RequestWrapper { keepResourceClass = resourceClass2; commandWrapper = retrieveCommands(command.getClass(), resourceCommands2); - } catch (final ClassCastException e) { - throw new NullPointerException("No key found for '" + command.getClass() + "' in the Map!"); - } catch (final NullPointerException e) { - throw e; + } catch (final ClassCastException | NullPointerException e) { + throw new CommandNotSupported("No key found for '" + command.getClass() + "' in the Map!", e); } } return commandWrapper; diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 424280cc66c..8a94b058655 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -47,6 +47,7 @@ import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import com.cloud.resource.RequestWrapper; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.cloudstack.utils.hypervisor.HypervisorUtils; @@ -1432,13 +1433,21 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return true; } + /** + * This finds a command wrapper to handle the command and executes it. + * If no wrapper is found an {@see UnsupportedAnswer} is sent back. + * Any other exceptions are to be caught and wrapped in an generic {@see Answer}, marked as failed. + * + * @param cmd the instance of a {@see Command} to execute. + * @return the for the {@see Command} appropriate {@see Answer} or {@see UnsupportedAnswer} + */ @Override public Answer executeRequest(final Command cmd) { final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); try { return wrapper.execute(cmd, this); - } catch (final Exception e) { + } catch (final RequestWrapper.CommandNotSupported cmde) { return Answer.createUnsupportedCommandAnswer(cmd); } } diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java index 0b413bc6e9f..436a1308316 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java @@ -72,6 +72,9 @@ public class LibvirtRequestWrapper extends RequestWrapper { commandWrapper = retryWhenAllFails(command, resourceClass, resourceCommands); } + if (commandWrapper == null) { + throw new CommandNotSupported("No way to handle " + command.getClass()); + } return commandWrapper.execute(command, serverResource); } } \ No newline at end of file diff --git a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index 795b96175ab..be191f5e9a5 100644 --- a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -38,6 +38,8 @@ import javax.xml.xpath.XPathConstants; import javax.xml.xpath.XPathExpressionException; import javax.xml.xpath.XPathFactory; +import com.cloud.agent.api.Command; +import com.cloud.agent.api.UnsupportedAnswer; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.CpuTuneDef; import org.apache.commons.lang.SystemUtils; import org.joda.time.Duration; @@ -5191,4 +5193,29 @@ public class LibvirtComputingResourceTest { Assert.assertEquals(CpuTuneDef.MIN_QUOTA, cpuTuneDef.getQuota()); Assert.assertEquals((int) (CpuTuneDef.MIN_QUOTA / pct), cpuTuneDef.getPeriod()); } + + @Test + public void testUnknownCommand() { + libvirtComputingResource = new LibvirtComputingResource(); + Command cmd = new Command() { + @Override public boolean executeInSequence() { + return false; + } + }; + Answer ans = libvirtComputingResource.executeRequest(cmd); + assertTrue(ans instanceof UnsupportedAnswer); + } + + @Test + public void testKnownCommand() { + libvirtComputingResource = new LibvirtComputingResource(); + Command cmd = new PingTestCommand() { + @Override public boolean executeInSequence() { + throw new NullPointerException("test succeeded"); + } + }; + Answer ans = libvirtComputingResource.executeRequest(cmd); + assertFalse(ans instanceof UnsupportedAnswer); + assertTrue(ans instanceof Answer); + } } From 52b02de43f8eaf4c25cdf3d2b514ca0bf39cb986 Mon Sep 17 00:00:00 2001 From: dahn Date: Thu, 21 Jun 2018 11:36:50 +0200 Subject: [PATCH 7/7] vpc: reuse private gateway ip for non redundant VPC (#2712) As rolling restart does not deallocate an IP before configuring it on a new VR, the code must allow it to be reused on a non-redundant VPCs gateway nic. In crease ping counts to reduce intermittent failures in smoketests. Signed-off-by: Rohit Yadav --- .../com/cloud/network/router/NicProfileHelperImpl.java | 8 +++++--- test/integration/smoke/test_privategw_acl.py | 2 +- test/integration/smoke/test_vpc_redundant.py | 2 +- test/integration/smoke/test_vpc_router_nics.py | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/server/src/com/cloud/network/router/NicProfileHelperImpl.java b/server/src/com/cloud/network/router/NicProfileHelperImpl.java index 09059855319..18ab4a93297 100644 --- a/server/src/com/cloud/network/router/NicProfileHelperImpl.java +++ b/server/src/com/cloud/network/router/NicProfileHelperImpl.java @@ -21,6 +21,7 @@ import java.net.URI; import javax.inject.Inject; +import com.cloud.utils.exception.CloudRuntimeException; import org.cloud.network.router.deployment.RouterDeploymentDefinition; import com.cloud.network.IpAddressManager; @@ -30,7 +31,6 @@ import com.cloud.network.Networks.AddressFormat; import com.cloud.network.Networks.BroadcastDomainType; import com.cloud.network.vpc.PrivateIpAddress; import com.cloud.network.vpc.PrivateIpVO; -import com.cloud.network.vpc.Vpc; import com.cloud.network.vpc.VpcGateway; import com.cloud.network.vpc.VpcManager; import com.cloud.network.vpc.dao.PrivateIpDao; @@ -65,9 +65,11 @@ public class NicProfileHelperImpl implements NicProfileHelper { PrivateIpVO ipVO = _privateIpDao.allocateIpAddress(privateNetwork.getDataCenterId(), privateNetwork.getId(), privateGateway.getIp4Address()); final Long vpcId = privateGateway.getVpcId(); - final Vpc activeVpc = _vpcMgr.getActiveVpc(vpcId); - if (activeVpc.isRedundant() && ipVO == null) { + if (ipVO == null) { ipVO = _privateIpDao.findByIpAndVpcId(vpcId, privateGateway.getIp4Address()); + if (ipVO == null) { + throw new CloudRuntimeException("cannot find IP address " + privateGateway.getIp4Address() + " to reuse for private gateway on vpc (id==" + vpcId + ")"); + } } Nic privateNic = null; diff --git a/test/integration/smoke/test_privategw_acl.py b/test/integration/smoke/test_privategw_acl.py index a6987e97c88..27328db3eb0 100644 --- a/test/integration/smoke/test_privategw_acl.py +++ b/test/integration/smoke/test_privategw_acl.py @@ -720,7 +720,7 @@ class TestPrivateGwACL(cloudstackTestCase): succeeded_pings = 0 minimum_vms_to_pass = 2 for vm_ip in vms_ips: - ssh_command = "ping -c 5 %s" % vm_ip + ssh_command = "ping -c 10 %s" % vm_ip # Should be able to SSH VM packet_loss = 100 diff --git a/test/integration/smoke/test_vpc_redundant.py b/test/integration/smoke/test_vpc_redundant.py index 64b1fa67771..fe6b4108fbf 100644 --- a/test/integration/smoke/test_vpc_redundant.py +++ b/test/integration/smoke/test_vpc_redundant.py @@ -692,7 +692,7 @@ class TestVPCRedundancy(cloudstackTestCase): def do_default_routes_test(self): for o in self.networks: for vmObj in o.get_vms(): - ssh_command = "ping -c 3 8.8.8.8" + ssh_command = "ping -c 10 8.8.8.8" # Should be able to SSH VM packet_loss = 100 diff --git a/test/integration/smoke/test_vpc_router_nics.py b/test/integration/smoke/test_vpc_router_nics.py index 092a70f47e4..854ab567542 100644 --- a/test/integration/smoke/test_vpc_router_nics.py +++ b/test/integration/smoke/test_vpc_router_nics.py @@ -451,7 +451,7 @@ class TestVPCNics(cloudstackTestCase): def do_default_routes_test(self): for o in self.networks: for vmObj in o.get_vms(): - ssh_command = "ping -c 5 8.8.8.8" + ssh_command = "ping -c 10 8.8.8.8" # Should be able to SSH VM packet_loss = 100