From 66e0fbfce77c550dfdb772754ab1c5e23a83ff92 Mon Sep 17 00:00:00 2001 From: abh1sar Date: Tue, 25 Jun 2024 14:41:46 +0530 Subject: [PATCH 1/4] Restart agent when host comes out of maintenance --- .../com/cloud/agent/manager/AgentAttache.java | 5 ++-- .../cloud/resource/ResourceManagerImpl.java | 8 +++--- .../resource/ResourceManagerImplTest.java | 26 +++---------------- 3 files changed, 10 insertions(+), 29 deletions(-) diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java index b12a72136dd9..3a0500acb235 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java @@ -44,6 +44,7 @@ import com.cloud.agent.api.CheckVirtualMachineCommand; import com.cloud.agent.api.CleanupNetworkRulesCmd; import com.cloud.agent.api.Command; +import com.cloud.agent.api.DeleteStoragePoolCommand; import com.cloud.agent.api.MaintainCommand; import com.cloud.agent.api.MigrateCommand; import com.cloud.agent.api.ModifySshKeysCommand; @@ -119,8 +120,8 @@ public int compare(final Object o1, final Object o2) { StopCommand.class.toString(), CheckVirtualMachineCommand.class.toString(), PingTestCommand.class.toString(), CheckHealthCommand.class.toString(), ReadyCommand.class.toString(), ShutdownCommand.class.toString(), SetupCommand.class.toString(), CleanupNetworkRulesCmd.class.toString(), CheckNetworkCommand.class.toString(), PvlanSetupCommand.class.toString(), CheckOnHostCommand.class.toString(), - ModifyTargetsCommand.class.toString(), ModifySshKeysCommand.class.toString(), ModifyStoragePoolCommand.class.toString(), SetupMSListCommand.class.toString(), RollingMaintenanceCommand.class.toString(), - CleanupPersistentNetworkResourceCommand.class.toString()}; + ModifyTargetsCommand.class.toString(), ModifySshKeysCommand.class.toString(), DeleteStoragePoolCommand.class.toString(), ModifyStoragePoolCommand.class.toString(), + SetupMSListCommand.class.toString(), RollingMaintenanceCommand.class.toString(), CleanupPersistentNetworkResourceCommand.class.toString()}; protected final static String[] s_commandsNotAllowedInConnectingMode = new String[] { StartCommand.class.toString(), CreateCommand.class.toString() }; static { Arrays.sort(s_commandsAllowedInMaintenanceMode); diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 1909063dfe31..662bc5db70f5 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -2757,7 +2757,7 @@ private boolean doCancelMaintenance(final long hostId) { } } - handleAgentIfNotConnected(host, vms_migrating); + handleAgentConnectAndRestart(host, vms_migrating); try { resourceStateTransitTo(host, ResourceState.Event.AdminCancelMaintenance, _nodeId); @@ -2772,16 +2772,16 @@ private boolean doCancelMaintenance(final long hostId) { } /** - * Handle agent (if available) if its not connected before cancelling maintenance. * Agent must be connected before cancelling maintenance. + * Connect and restart agent (if available) before cancelling maintenance. * If the host status is not Up: * - If kvm.ssh.to.agent is true, then SSH into the host and restart the agent. * - If kvm.shh.to.agent is false, then fail cancelling maintenance */ - protected void handleAgentIfNotConnected(HostVO host, boolean vmsMigrating) { + protected void handleAgentConnectAndRestart(HostVO host, boolean vmsMigrating) { final boolean isAgentOnHost = host.getHypervisorType() == HypervisorType.KVM || host.getHypervisorType() == HypervisorType.LXC; - if (!isAgentOnHost || vmsMigrating || host.getStatus() == Status.Up) { + if (!isAgentOnHost || vmsMigrating) { return; } final boolean sshToAgent = Boolean.parseBoolean(_configDao.getValue(KvmSshToAgentEnabled.key())); diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index a7ddd16462e5..128d6ce58197 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -167,7 +167,6 @@ public void setup() throws Exception { when(host.getDetail("username")).thenReturn(hostUsername); when(host.getDetail("password")).thenReturn(hostPassword); when(configurationDao.getValue("ssh.privatekey")).thenReturn(hostPrivateKey); - when(host.getStatus()).thenReturn(Status.Up); when(host.getPrivateIpAddress()).thenReturn(hostPrivateIp); when(vm1.getId()).thenReturn(vm1Id); when(vm2.getId()).thenReturn(vm2Id); @@ -350,39 +349,20 @@ public void testConnectAndRestartAgentOnHost() { @Test public void testHandleAgentSSHEnabledNotConnectedAgent() { - when(host.getStatus()).thenReturn(Status.Disconnected); - resourceManager.handleAgentIfNotConnected(host, false); + resourceManager.handleAgentConnectAndRestart(host, false); verify(resourceManager).getHostCredentials(eq(host)); verify(resourceManager).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey)); } - @Test - public void testHandleAgentSSHEnabledConnectedAgent() { - when(host.getStatus()).thenReturn(Status.Up); - resourceManager.handleAgentIfNotConnected(host, false); - verify(resourceManager, never()).getHostCredentials(eq(host)); - verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey)); - } - @Test(expected = CloudRuntimeException.class) public void testHandleAgentSSHDisabledNotConnectedAgent() { - when(host.getStatus()).thenReturn(Status.Disconnected); when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("false"); - resourceManager.handleAgentIfNotConnected(host, false); - } - - @Test - public void testHandleAgentSSHDisabledConnectedAgent() { - when(host.getStatus()).thenReturn(Status.Up); - when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("false"); - resourceManager.handleAgentIfNotConnected(host, false); - verify(resourceManager, never()).getHostCredentials(eq(host)); - verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey)); + resourceManager.handleAgentConnectAndRestart(host, false); } @Test public void testHandleAgentVMsMigrating() { - resourceManager.handleAgentIfNotConnected(host, true); + resourceManager.handleAgentConnectAndRestart(host, true); verify(resourceManager, never()).getHostCredentials(eq(host)); verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey)); } From 85672d2af2a027e7da213b6cfd19320b3f93ed99 Mon Sep 17 00:00:00 2001 From: abh1sar Date: Wed, 26 Jun 2024 00:55:18 +0530 Subject: [PATCH 2/4] Don't send CreateStoragePoolCommand to hosts in maintenance mode --- .../com/cloud/resource/ResourceManager.java | 2 ++ .../ElastistorPrimaryDataStoreLifeCycle.java | 2 +- ...loudStackPrimaryDataStoreLifeCycleImpl.java | 2 +- ...StackPrimaryDataStoreLifeCycleImplTest.java | 2 +- ...lidFireSharedPrimaryDataStoreLifeCycle.java | 2 +- .../cloud/resource/ResourceManagerImpl.java | 18 ++++++++++++++++++ .../resource/MockResourceManagerImpl.java | 5 +++++ 7 files changed, 29 insertions(+), 4 deletions(-) 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 9308be5fb320..2931191d6d2c 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 @@ -118,6 +118,8 @@ public interface ResourceManager extends ResourceService, Configurable { public List listAllUpHosts(Host.Type type, Long clusterId, Long podId, long dcId); + public List listAllUpHostsNotInMaintenance(Host.Type type, Long clusterId, Long podId, long dcId); + public List listAllHostsInCluster(long clusterId); public List listHostsInClusterByStatus(long clusterId, Status status); diff --git a/plugins/storage/volume/cloudbyte/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java b/plugins/storage/volume/cloudbyte/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java index d6a67b447c57..870345d6abed 100644 --- a/plugins/storage/volume/cloudbyte/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java +++ b/plugins/storage/volume/cloudbyte/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java @@ -359,7 +359,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) { PrimaryDataStoreInfo primarystore = (PrimaryDataStoreInfo) store; // Check if there is host up in this cluster - List allHosts = _resourceMgr.listAllUpHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId()); + List allHosts = _resourceMgr.listAllUpHostsNotInMaintenance(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId()); if (allHosts.isEmpty()) { primaryDataStoreDao.expunge(primarystore.getId()); throw new CloudRuntimeException("No host up to associate a storage pool with in cluster " + primarystore.getClusterId()); diff --git a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java index 213e5620553f..82e3ec23648f 100644 --- a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java +++ b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java @@ -421,7 +421,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) { PrimaryDataStoreInfo primarystore = (PrimaryDataStoreInfo)store; // Check if there is host up in this cluster List allHosts = - _resourceMgr.listAllUpHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId()); + _resourceMgr.listAllUpHostsNotInMaintenance(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId()); if (allHosts.isEmpty()) { primaryDataStoreDao.expunge(primarystore.getId()); throw new CloudRuntimeException("No host up to associate a storage pool with in cluster " + primarystore.getClusterId()); diff --git a/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java b/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java index ffdc5143e8c9..30ba194926e7 100644 --- a/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java +++ b/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java @@ -146,7 +146,7 @@ public void initMocks() { when(dataStoreProvider.getName()).thenReturn("default"); ((StorageManagerImpl)storageMgr).registerHostListener("default", hostListener); - when(_resourceMgr.listAllUpHosts(eq(Host.Type.Routing), anyLong(), anyLong(), anyLong())).thenReturn(hostList); + when(_resourceMgr.listAllUpHostsNotInMaintenance(eq(Host.Type.Routing), anyLong(), anyLong(), anyLong())).thenReturn(hostList); when(agentMgr.easySend(anyLong(), Mockito.any(ModifyStoragePoolCommand.class))).thenReturn(answer); when(answer.getResult()).thenReturn(true); when(answer.getPoolInfo()).thenReturn(info); diff --git a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/SolidFireSharedPrimaryDataStoreLifeCycle.java b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/SolidFireSharedPrimaryDataStoreLifeCycle.java index 9cc746d4ee8e..e7a6c21e71f3 100644 --- a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/SolidFireSharedPrimaryDataStoreLifeCycle.java +++ b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/SolidFireSharedPrimaryDataStoreLifeCycle.java @@ -385,7 +385,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) { PrimaryDataStoreInfo primaryDataStoreInfo = (PrimaryDataStoreInfo)store; // check if there is at least one host up in this cluster - List allHosts = resourceMgr.listAllUpHosts(Host.Type.Routing, primaryDataStoreInfo.getClusterId(), + List allHosts = resourceMgr.listAllUpHostsNotInMaintenance(Host.Type.Routing, primaryDataStoreInfo.getClusterId(), primaryDataStoreInfo.getPodId(), primaryDataStoreInfo.getDataCenterId()); if (allHosts.isEmpty()) { diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 662bc5db70f5..c8d9de7da037 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -3073,6 +3073,24 @@ public List listAllUpHosts(Type type, Long clusterId, Long podId, long d return sc.list(); } + @Override + public List listAllUpHostsNotInMaintenance(Type type, Long clusterId, Long podId, long dcId) { + final QueryBuilder sc = QueryBuilder.create(HostVO.class); + if (type != null) { + sc.and(sc.entity().getType(), Op.EQ, type); + } + if (clusterId != null) { + sc.and(sc.entity().getClusterId(), Op.EQ, clusterId); + } + if (podId != null) { + sc.and(sc.entity().getPodId(), Op.EQ, podId); + } + sc.and(sc.entity().getDataCenterId(), Op.EQ, dcId); + sc.and(sc.entity().getStatus(), Op.EQ, Status.Up); + sc.and(sc.entity().getResourceState(), Op.NEQ, ResourceState.Maintenance); + return sc.list(); + } + @Override public List listAllUpAndEnabledNonHAHosts(final Type type, final Long clusterId, final Long podId, final long dcId) { final String haTag = _haMgr.getHaTag(); diff --git a/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java b/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java index 4d5b5ba584bf..9c8df0967d23 100755 --- a/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java +++ b/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java @@ -384,6 +384,11 @@ public List listAllUpHosts(Type type, Long clusterId, Long podId, long d return null; } + @Override + public List listAllUpHostsNotInMaintenance(Type type, Long clusterId, Long podId, long dcId) { + return null; + } + /* (non-Javadoc) * @see com.cloud.resource.ResourceManager#listAllHostsInCluster(long) */ From 2c031ac90bf3f212a5cc84f5180ed7b60dae2a1c Mon Sep 17 00:00:00 2001 From: abh1sar Date: Fri, 28 Jun 2024 11:10:31 +0530 Subject: [PATCH 3/4] CreateStoragePoolCommand can run when host in maintenance. Reverted the change to restart agent when host was already up and in maintenance --- .../com/cloud/resource/ResourceManager.java | 4 +-- .../com/cloud/agent/manager/AgentAttache.java | 4 ++- .../ElastistorPrimaryDataStoreLifeCycle.java | 2 +- ...oudStackPrimaryDataStoreLifeCycleImpl.java | 4 +-- ...tackPrimaryDataStoreLifeCycleImplTest.java | 2 +- ...idFireSharedPrimaryDataStoreLifeCycle.java | 2 +- .../cloud/resource/ResourceManagerImpl.java | 35 +++++++------------ .../resource/MockResourceManagerImpl.java | 11 +++--- .../resource/ResourceManagerImplTest.java | 6 ++-- 9 files changed, 32 insertions(+), 38 deletions(-) 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 2931191d6d2c..91197de6a84f 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 @@ -118,8 +118,6 @@ public interface ResourceManager extends ResourceService, Configurable { public List listAllUpHosts(Host.Type type, Long clusterId, Long podId, long dcId); - public List listAllUpHostsNotInMaintenance(Host.Type type, Long clusterId, Long podId, long dcId); - public List listAllHostsInCluster(long clusterId); public List listHostsInClusterByStatus(long clusterId, Status status); @@ -128,6 +126,8 @@ public interface ResourceManager extends ResourceService, Configurable { public List listAllUpAndEnabledHostsInOneZoneByHypervisor(HypervisorType type, long dcId); + public List listAllUpHostsInOneZoneByHypervisor(HypervisorType type, long dcId); + public List listAllUpAndEnabledHostsInOneZone(long dcId); public List listAllHostsInOneZoneByType(Host.Type type, long dcId); diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java index 3a0500acb235..2d8d6f1c48eb 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java @@ -44,6 +44,7 @@ import com.cloud.agent.api.CheckVirtualMachineCommand; import com.cloud.agent.api.CleanupNetworkRulesCmd; import com.cloud.agent.api.Command; +import com.cloud.agent.api.CreateStoragePoolCommand; import com.cloud.agent.api.DeleteStoragePoolCommand; import com.cloud.agent.api.MaintainCommand; import com.cloud.agent.api.MigrateCommand; @@ -120,7 +121,8 @@ public int compare(final Object o1, final Object o2) { StopCommand.class.toString(), CheckVirtualMachineCommand.class.toString(), PingTestCommand.class.toString(), CheckHealthCommand.class.toString(), ReadyCommand.class.toString(), ShutdownCommand.class.toString(), SetupCommand.class.toString(), CleanupNetworkRulesCmd.class.toString(), CheckNetworkCommand.class.toString(), PvlanSetupCommand.class.toString(), CheckOnHostCommand.class.toString(), - ModifyTargetsCommand.class.toString(), ModifySshKeysCommand.class.toString(), DeleteStoragePoolCommand.class.toString(), ModifyStoragePoolCommand.class.toString(), + ModifyTargetsCommand.class.toString(), ModifySshKeysCommand.class.toString(), + CreateStoragePoolCommand.class.toString(), DeleteStoragePoolCommand.class.toString(), ModifyStoragePoolCommand.class.toString(), SetupMSListCommand.class.toString(), RollingMaintenanceCommand.class.toString(), CleanupPersistentNetworkResourceCommand.class.toString()}; protected final static String[] s_commandsNotAllowedInConnectingMode = new String[] { StartCommand.class.toString(), CreateCommand.class.toString() }; static { diff --git a/plugins/storage/volume/cloudbyte/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java b/plugins/storage/volume/cloudbyte/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java index 870345d6abed..d6a67b447c57 100644 --- a/plugins/storage/volume/cloudbyte/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java +++ b/plugins/storage/volume/cloudbyte/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java @@ -359,7 +359,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) { PrimaryDataStoreInfo primarystore = (PrimaryDataStoreInfo) store; // Check if there is host up in this cluster - List allHosts = _resourceMgr.listAllUpHostsNotInMaintenance(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId()); + List allHosts = _resourceMgr.listAllUpHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId()); if (allHosts.isEmpty()) { primaryDataStoreDao.expunge(primarystore.getId()); throw new CloudRuntimeException("No host up to associate a storage pool with in cluster " + primarystore.getClusterId()); diff --git a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java index 82e3ec23648f..a5730d908026 100644 --- a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java +++ b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java @@ -421,7 +421,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) { PrimaryDataStoreInfo primarystore = (PrimaryDataStoreInfo)store; // Check if there is host up in this cluster List allHosts = - _resourceMgr.listAllUpHostsNotInMaintenance(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId()); + _resourceMgr.listAllUpHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId()); if (allHosts.isEmpty()) { primaryDataStoreDao.expunge(primarystore.getId()); throw new CloudRuntimeException("No host up to associate a storage pool with in cluster " + primarystore.getClusterId()); @@ -467,7 +467,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) { @Override public boolean attachZone(DataStore dataStore, ZoneScope scope, HypervisorType hypervisorType) { - List hosts = _resourceMgr.listAllUpAndEnabledHostsInOneZoneByHypervisor(hypervisorType, scope.getScopeId()); + List hosts = _resourceMgr.listAllUpHostsInOneZoneByHypervisor(hypervisorType, scope.getScopeId()); s_logger.debug("In createPool. Attaching the pool to each of the hosts."); List poolHosts = new ArrayList(); for (HostVO host : hosts) { diff --git a/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java b/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java index 30ba194926e7..ffdc5143e8c9 100644 --- a/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java +++ b/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java @@ -146,7 +146,7 @@ public void initMocks() { when(dataStoreProvider.getName()).thenReturn("default"); ((StorageManagerImpl)storageMgr).registerHostListener("default", hostListener); - when(_resourceMgr.listAllUpHostsNotInMaintenance(eq(Host.Type.Routing), anyLong(), anyLong(), anyLong())).thenReturn(hostList); + when(_resourceMgr.listAllUpHosts(eq(Host.Type.Routing), anyLong(), anyLong(), anyLong())).thenReturn(hostList); when(agentMgr.easySend(anyLong(), Mockito.any(ModifyStoragePoolCommand.class))).thenReturn(answer); when(answer.getResult()).thenReturn(true); when(answer.getPoolInfo()).thenReturn(info); diff --git a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/SolidFireSharedPrimaryDataStoreLifeCycle.java b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/SolidFireSharedPrimaryDataStoreLifeCycle.java index e7a6c21e71f3..9cc746d4ee8e 100644 --- a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/SolidFireSharedPrimaryDataStoreLifeCycle.java +++ b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/SolidFireSharedPrimaryDataStoreLifeCycle.java @@ -385,7 +385,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) { PrimaryDataStoreInfo primaryDataStoreInfo = (PrimaryDataStoreInfo)store; // check if there is at least one host up in this cluster - List allHosts = resourceMgr.listAllUpHostsNotInMaintenance(Host.Type.Routing, primaryDataStoreInfo.getClusterId(), + List allHosts = resourceMgr.listAllUpHosts(Host.Type.Routing, primaryDataStoreInfo.getClusterId(), primaryDataStoreInfo.getPodId(), primaryDataStoreInfo.getDataCenterId()); if (allHosts.isEmpty()) { diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index c8d9de7da037..04a5f0e8d01c 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -2757,7 +2757,7 @@ private boolean doCancelMaintenance(final long hostId) { } } - handleAgentConnectAndRestart(host, vms_migrating); + handleAgentIfNotConnected(host, vms_migrating); try { resourceStateTransitTo(host, ResourceState.Event.AdminCancelMaintenance, _nodeId); @@ -2772,16 +2772,16 @@ private boolean doCancelMaintenance(final long hostId) { } /** + * Handle agent (if available) if its not connected before cancelling maintenance. * Agent must be connected before cancelling maintenance. - * Connect and restart agent (if available) before cancelling maintenance. * If the host status is not Up: * - If kvm.ssh.to.agent is true, then SSH into the host and restart the agent. * - If kvm.shh.to.agent is false, then fail cancelling maintenance */ - protected void handleAgentConnectAndRestart(HostVO host, boolean vmsMigrating) { + protected void handleAgentIfNotConnected(HostVO host, boolean vmsMigrating) { final boolean isAgentOnHost = host.getHypervisorType() == HypervisorType.KVM || host.getHypervisorType() == HypervisorType.LXC; - if (!isAgentOnHost || vmsMigrating) { + if (!isAgentOnHost || vmsMigrating || host.getStatus() == Status.Up) { return; } final boolean sshToAgent = Boolean.parseBoolean(_configDao.getValue(KvmSshToAgentEnabled.key())); @@ -3073,24 +3073,6 @@ public List listAllUpHosts(Type type, Long clusterId, Long podId, long d return sc.list(); } - @Override - public List listAllUpHostsNotInMaintenance(Type type, Long clusterId, Long podId, long dcId) { - final QueryBuilder sc = QueryBuilder.create(HostVO.class); - if (type != null) { - sc.and(sc.entity().getType(), Op.EQ, type); - } - if (clusterId != null) { - sc.and(sc.entity().getClusterId(), Op.EQ, clusterId); - } - if (podId != null) { - sc.and(sc.entity().getPodId(), Op.EQ, podId); - } - sc.and(sc.entity().getDataCenterId(), Op.EQ, dcId); - sc.and(sc.entity().getStatus(), Op.EQ, Status.Up); - sc.and(sc.entity().getResourceState(), Op.NEQ, ResourceState.Maintenance); - return sc.list(); - } - @Override public List listAllUpAndEnabledNonHAHosts(final Type type, final Long clusterId, final Long podId, final long dcId) { final String haTag = _haMgr.getHaTag(); @@ -3273,6 +3255,15 @@ public List listAllUpAndEnabledHostsInOneZoneByHypervisor(final Hypervis return sc.list(); } + @Override + public List listAllUpHostsInOneZoneByHypervisor(final HypervisorType type, final long dcId) { + final QueryBuilder sc = QueryBuilder.create(HostVO.class); + sc.and(sc.entity().getHypervisorType(), Op.EQ, type); + sc.and(sc.entity().getDataCenterId(), Op.EQ, dcId); + sc.and(sc.entity().getStatus(), Op.EQ, Status.Up); + return sc.list(); + } + @Override public List listAllUpAndEnabledHostsInOneZone(final long dcId) { final QueryBuilder sc = QueryBuilder.create(HostVO.class); diff --git a/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java b/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java index 9c8df0967d23..538125a1dbcc 100755 --- a/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java +++ b/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java @@ -384,11 +384,6 @@ public List listAllUpHosts(Type type, Long clusterId, Long podId, long d return null; } - @Override - public List listAllUpHostsNotInMaintenance(Type type, Long clusterId, Long podId, long dcId) { - return null; - } - /* (non-Javadoc) * @see com.cloud.resource.ResourceManager#listAllHostsInCluster(long) */ @@ -593,6 +588,12 @@ public List listAllUpAndEnabledHostsInOneZoneByHypervisor(final Hypervis return null; } + @Override + public List listAllUpHostsInOneZoneByHypervisor(final HypervisorType type, final long dcId) { + // TODO Auto-generated method stub + return null; + } + @Override public List listAllUpAndEnabledHostsInOneZone(final long dcId) { // TODO Auto-generated method stub diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index 128d6ce58197..0e5c389b5ab0 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -349,7 +349,7 @@ public void testConnectAndRestartAgentOnHost() { @Test public void testHandleAgentSSHEnabledNotConnectedAgent() { - resourceManager.handleAgentConnectAndRestart(host, false); + resourceManager.handleAgentIfNotConnected(host, false); verify(resourceManager).getHostCredentials(eq(host)); verify(resourceManager).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey)); } @@ -357,12 +357,12 @@ public void testHandleAgentSSHEnabledNotConnectedAgent() { @Test(expected = CloudRuntimeException.class) public void testHandleAgentSSHDisabledNotConnectedAgent() { when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("false"); - resourceManager.handleAgentConnectAndRestart(host, false); + resourceManager.handleAgentIfNotConnected(host, false); } @Test public void testHandleAgentVMsMigrating() { - resourceManager.handleAgentConnectAndRestart(host, true); + resourceManager.handleAgentIfNotConnected(host, true); verify(resourceManager, never()).getHostCredentials(eq(host)); verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey)); } From 50e09feccb1d6e5a50fe3b1eb0741731aed4ea89 Mon Sep 17 00:00:00 2001 From: abh1sar Date: Fri, 28 Jun 2024 11:14:52 +0530 Subject: [PATCH 4/4] Reverted changes done to ResourceManagerImplTest --- .../resource/ResourceManagerImplTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index 0e5c389b5ab0..a7ddd16462e5 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -167,6 +167,7 @@ public void setup() throws Exception { when(host.getDetail("username")).thenReturn(hostUsername); when(host.getDetail("password")).thenReturn(hostPassword); when(configurationDao.getValue("ssh.privatekey")).thenReturn(hostPrivateKey); + when(host.getStatus()).thenReturn(Status.Up); when(host.getPrivateIpAddress()).thenReturn(hostPrivateIp); when(vm1.getId()).thenReturn(vm1Id); when(vm2.getId()).thenReturn(vm2Id); @@ -349,17 +350,36 @@ public void testConnectAndRestartAgentOnHost() { @Test public void testHandleAgentSSHEnabledNotConnectedAgent() { + when(host.getStatus()).thenReturn(Status.Disconnected); resourceManager.handleAgentIfNotConnected(host, false); verify(resourceManager).getHostCredentials(eq(host)); verify(resourceManager).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey)); } + @Test + public void testHandleAgentSSHEnabledConnectedAgent() { + when(host.getStatus()).thenReturn(Status.Up); + resourceManager.handleAgentIfNotConnected(host, false); + verify(resourceManager, never()).getHostCredentials(eq(host)); + verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey)); + } + @Test(expected = CloudRuntimeException.class) public void testHandleAgentSSHDisabledNotConnectedAgent() { + when(host.getStatus()).thenReturn(Status.Disconnected); when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("false"); resourceManager.handleAgentIfNotConnected(host, false); } + @Test + public void testHandleAgentSSHDisabledConnectedAgent() { + when(host.getStatus()).thenReturn(Status.Up); + when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("false"); + resourceManager.handleAgentIfNotConnected(host, false); + verify(resourceManager, never()).getHostCredentials(eq(host)); + verify(resourceManager, never()).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey)); + } + @Test public void testHandleAgentVMsMigrating() { resourceManager.handleAgentIfNotConnected(host, true);