From ae7c36e4c1bed680f2e19c44c3d50472fa5901dd Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 12 Oct 2023 18:53:47 +0530 Subject: [PATCH 1/6] Add locks to ensure connection and disconnection doesn't happen at the same time for a host --- .../cloud/agent/manager/AgentManagerImpl.java | 112 +++++++++++------- 1 file changed, 67 insertions(+), 45 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 b74c11cf1384..4028f6b3ad36 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 @@ -40,6 +40,7 @@ import com.cloud.configuration.Config; import com.cloud.utils.NumbersUtil; +import com.cloud.utils.db.GlobalLock; import org.apache.cloudstack.agent.lb.IndirectAgentLB; import org.apache.cloudstack.ca.CAManager; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; @@ -801,46 +802,56 @@ public boolean stop() { protected boolean handleDisconnectWithoutInvestigation(final AgentAttache attache, final Status.Event event, final boolean transitState, final boolean removeAgent) { final long hostId = attache.getId(); - s_logger.info("Host " + hostId + " is disconnecting with event " + event); - Status nextStatus = null; - final HostVO host = _hostDao.findById(hostId); - if (host == null) { - s_logger.warn("Can't find host with " + hostId); - nextStatus = Status.Removed; - } else { - final Status currentStatus = host.getStatus(); - if (currentStatus == Status.Down || currentStatus == Status.Alert || currentStatus == Status.Removed) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Host " + hostId + " is already " + currentStatus); - } - nextStatus = currentStatus; - } else { - try { - nextStatus = currentStatus.getNextStatus(event); - } catch (final NoTransitionException e) { - final String err = "Cannot find next status for " + event + " as current status is " + currentStatus + " for agent " + hostId; - s_logger.debug(err); - throw new CloudRuntimeException(err); + boolean result = false; + GlobalLock joinLock = getHostJoinLock(hostId); + if (joinLock.lock(60)) { + try { + s_logger.info("Host " + hostId + " is disconnecting with event " + event); + Status nextStatus = null; + final HostVO host = _hostDao.findById(hostId); + if (host == null) { + s_logger.warn("Can't find host with " + hostId); + nextStatus = Status.Removed; + } else { + final Status currentStatus = host.getStatus(); + if (currentStatus == Status.Down || currentStatus == Status.Alert || currentStatus == Status.Removed) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Host " + hostId + " is already " + currentStatus); + } + nextStatus = currentStatus; + } else { + try { + nextStatus = currentStatus.getNextStatus(event); + } catch (final NoTransitionException e) { + final String err = "Cannot find next status for " + event + " as current status is " + currentStatus + " for agent " + hostId; + s_logger.debug(err); + throw new CloudRuntimeException(err); + } + + if (s_logger.isDebugEnabled()) { + s_logger.debug("The next status of agent " + hostId + "is " + nextStatus + ", current status is " + currentStatus); + } + } + caService.purgeHostCertificate(host); } if (s_logger.isDebugEnabled()) { - s_logger.debug("The next status of agent " + hostId + "is " + nextStatus + ", current status is " + currentStatus); + s_logger.debug("Deregistering link for " + hostId + " with state " + nextStatus); } - } - caService.purgeHostCertificate(host); - } - if (s_logger.isDebugEnabled()) { - s_logger.debug("Deregistering link for " + hostId + " with state " + nextStatus); - } + removeAgent(attache, nextStatus); - removeAgent(attache, nextStatus); - // update the DB - if (host != null && transitState) { - disconnectAgent(host, event, _nodeId); + // update the DB + if (host != null && transitState) { + disconnectAgent(host, event, _nodeId); + } + } finally { + joinLock.unlock(); + } + result = true; } - - return true; + joinLock.releaseRef(); + return result; } protected boolean handleDisconnectWithInvestigation(final AgentAttache attache, Status.Event event) { @@ -1120,18 +1131,26 @@ private AgentAttache handleConnectedAgent(final Link link, final StartupCommand[ final HostVO host = _resourceMgr.createHostVOForConnectedAgent(startup); if (host != null) { - ready = new ReadyCommand(host.getDataCenterId(), host.getId(), NumbersUtil.enableHumanReadableSizes); - - if (!indirectAgentLB.compareManagementServerList(host.getId(), host.getDataCenterId(), agentMSHostList, lbAlgorithm)) { - final List newMSList = indirectAgentLB.getManagementServerList(host.getId(), host.getDataCenterId(), null); - ready.setMsHostList(newMSList); - ready.setLbAlgorithm(indirectAgentLB.getLBAlgorithmName()); - ready.setLbCheckInterval(indirectAgentLB.getLBPreferredHostCheckInterval(host.getClusterId())); - s_logger.debug("Agent's management server host list is not up to date, sending list update:" + newMSList); - } + GlobalLock joinLock = getHostJoinLock(host.getId()); + if (joinLock.lock(60)) { + try { + ready = new ReadyCommand(host.getDataCenterId(), host.getId(), NumbersUtil.enableHumanReadableSizes); + + if (!indirectAgentLB.compareManagementServerList(host.getId(), host.getDataCenterId(), agentMSHostList, lbAlgorithm)) { + final List newMSList = indirectAgentLB.getManagementServerList(host.getId(), host.getDataCenterId(), null); + ready.setMsHostList(newMSList); + ready.setLbAlgorithm(indirectAgentLB.getLBAlgorithmName()); + ready.setLbCheckInterval(indirectAgentLB.getLBPreferredHostCheckInterval(host.getClusterId())); + s_logger.debug("Agent's management server host list is not up to date, sending list update:" + newMSList); + } - attache = createAttacheForConnect(host, link); - attache = notifyMonitorsOfConnection(attache, startup, false); + attache = createAttacheForConnect(host, link); + attache = notifyMonitorsOfConnection(attache, startup, false); + } finally { + joinLock.unlock(); + } + } + joinLock.releaseRef(); } } catch (final Exception e) { s_logger.debug("Failed to handle host connection: ", e); @@ -1324,7 +1343,6 @@ protected void processRequest(final Link link, final Request request) { if (cmd instanceof PingRoutingCommand) { final boolean gatewayAccessible = ((PingRoutingCommand)cmd).isGatewayAccessible(); final HostVO host = _hostDao.findById(Long.valueOf(cmdHostId)); - if (host != null) { if (!gatewayAccessible) { // alert that host lost connection to @@ -1864,4 +1882,8 @@ public void propagateChangeToAgents(Map params) { sendCommandToAgents(hostsPerZone, params); } } + + private GlobalLock getHostJoinLock(Long hostId) { + return GlobalLock.getInternLock(String.format("%s-%s", "Host-Join", hostId)); + } } From 79100ef74d0c1bc7e7bffa31538247cf5ae02d85 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 12 Oct 2023 19:10:04 +0530 Subject: [PATCH 2/6] On ping, if agent is not in Up state, request startup command --- agent/src/main/java/com/cloud/agent/Agent.java | 4 ++++ .../main/java/com/cloud/agent/api/PingAnswer.java | 13 ++++++++++++- .../com/cloud/agent/manager/AgentManagerImpl.java | 14 ++++++++++++-- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/Agent.java b/agent/src/main/java/com/cloud/agent/Agent.java index e9213ca9b8c5..e8558931cbf5 100644 --- a/agent/src/main/java/com/cloud/agent/Agent.java +++ b/agent/src/main/java/com/cloud/agent/Agent.java @@ -40,6 +40,7 @@ import javax.naming.ConfigurationException; +import com.cloud.agent.api.PingAnswer; import com.cloud.utils.NumbersUtil; import org.apache.cloudstack.agent.lb.SetupMSListAnswer; import org.apache.cloudstack.agent.lb.SetupMSListCommand; @@ -822,6 +823,9 @@ public void processResponse(final Response response, final Link link) { listener.processControlResponse(response, (AgentControlAnswer)answer); } } + } else if (answer instanceof PingAnswer && (((PingAnswer) answer).isSendStartup()) && _reconnectAllowed) { + s_logger.info("Management server requested startup command to reinitialize the agent"); + sendStartup(link); } else { setLastPingResponseTime(); } diff --git a/core/src/main/java/com/cloud/agent/api/PingAnswer.java b/core/src/main/java/com/cloud/agent/api/PingAnswer.java index 352423807390..6353b121583a 100644 --- a/core/src/main/java/com/cloud/agent/api/PingAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/PingAnswer.java @@ -22,15 +22,26 @@ public class PingAnswer extends Answer { private PingCommand _command = null; + private boolean sendStartup = false; + protected PingAnswer() { } - public PingAnswer(PingCommand cmd) { + public PingAnswer(PingCommand cmd, boolean sendStartup) { super(cmd); _command = cmd; + this.sendStartup = sendStartup; } public PingCommand getCommand() { return _command; } + + public boolean isSendStartup() { + return sendStartup; + } + + public void setSendStartup(boolean sendStartup) { + this.sendStartup = sendStartup; + } } 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 4028f6b3ad36..d2d05285bd66 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 @@ -1284,6 +1284,8 @@ protected void processRequest(final Link link, final Request request) { connectAgent(link, cmds, request); } return; + } else if (cmd instanceof StartupCommand) { + connectAgent(link, cmds, request); } final long hostId = attache.getId(); @@ -1337,12 +1339,13 @@ protected void processRequest(final Link link, final Request request) { handleCommands(attache, request.getSequence(), new Command[] {cmd}); if (cmd instanceof PingCommand) { final long cmdHostId = ((PingCommand)cmd).getHostId(); + boolean requestStartupCommand = false; + final HostVO host = _hostDao.findById(Long.valueOf(cmdHostId)); // if the router is sending a ping, verify the // gateway was pingable if (cmd instanceof PingRoutingCommand) { final boolean gatewayAccessible = ((PingRoutingCommand)cmd).isGatewayAccessible(); - final HostVO host = _hostDao.findById(Long.valueOf(cmdHostId)); if (host != null) { if (!gatewayAccessible) { // alert that host lost connection to @@ -1355,12 +1358,19 @@ protected void processRequest(final Link link, final Request request) { "Host [" + hostDesc + "] lost connection to gateway (default route) and is possibly having network connection issues."); } else { _alertMgr.clearAlert(AlertManager.AlertType.ALERT_TYPE_ROUTING, host.getDataCenterId(), host.getPodId()); + if (host.getStatus() != Status.Up) { + // Only transit state when the status is not Up to avoid unnecessary db calls + requestStartupCommand = true; + } } } else { s_logger.debug("Not processing " + PingRoutingCommand.class.getSimpleName() + " for agent id=" + cmdHostId + "; can't find the host in the DB"); } + } else if (host != null && host.getStatus() != Status.Up) { + // Only transit state when the status is not Up to avoid unnecessary db calls + requestStartupCommand = true; } - answer = new PingAnswer((PingCommand)cmd); + answer = new PingAnswer((PingCommand)cmd, requestStartupCommand); } else if (cmd instanceof ReadyAnswer) { final HostVO host = _hostDao.findById(attache.getId()); if (host == null) { From b593a2052fc2b8be14df46b10ad1a5b288de42e2 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Tue, 10 Oct 2023 11:43:35 +0530 Subject: [PATCH 3/6] Add smoketest to check host's status update on ping --- test/integration/smoke/test_host_ping.py | 112 +++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 test/integration/smoke/test_host_ping.py diff --git a/test/integration/smoke/test_host_ping.py b/test/integration/smoke/test_host_ping.py new file mode 100644 index 000000000000..0c14314f3638 --- /dev/null +++ b/test/integration/smoke/test_host_ping.py @@ -0,0 +1,112 @@ +# 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. +""" Check state transition of host from Alert to Up on Ping +""" + +# Import Local Modules +from marvin.cloudstackTestCase import * +from marvin.lib.utils import * +from marvin.lib.base import * +from marvin.lib.common import * +from nose.plugins.attrib import attr + +_multiprocess_shared_ = False + + +class TestHostHA(cloudstackTestCase): + + def setUp(self): + self.logger = logging.getLogger('TestHM') + self.stream_handler = logging.StreamHandler() + self.logger.setLevel(logging.DEBUG) + self.logger.addHandler(self.stream_handler) + self.apiclient = self.testClient.getApiClient() + self.hypervisor = self.testClient.getHypervisorInfo() + self.mgtSvrDetails = self.config.__dict__["mgtSvr"][0].__dict__ + self.dbConnection = self.testClient.getDbConnection() + self.services = self.testClient.getParsedTestDataConfig() + self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests()) + self.pod = get_pod(self.apiclient, self.zone.id) + self.cleanup = [] + + def tearDown(self): + try: + # Clean up, terminate the created templates + cleanup_resources(self.apiclient, self.cleanup) + + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + + return + + def checkHostStateInCloudstack(self, state, host_id): + try: + listHost = Host.list( + self.apiclient, + type='Routing', + zoneid=self.zone.id, + podid=self.pod.id, + id=host_id + ) + self.assertEqual( + isinstance(listHost, list), + True, + "Check if listHost returns a valid response" + ) + + self.assertEqual( + len(listHost), + 1, + "Check if listHost returns a host" + ) + self.logger.debug(" Host state is %s " % listHost[0].state) + if listHost[0].state == state: + return True, 1 + else: + return False, 1 + except Exception as e: + self.logger.debug("Got exception %s" % e) + return False, 1 + + + @attr( + tags=[ + "advanced", + "advancedns", + "smoke", + "basic"], + required_hardware="true") + def test_01_host_ping_on_alert(self): + listHost = Host.list( + self.apiclient, + type='Routing', + zoneid=self.zone.id, + podid=self.pod.id, + ) + for host in listHost: + self.logger.debug('Hypervisor = {}'.format(host.id)) + + + hostToTest = listHost[0] + sql_query = "UPDATE host SET status = 'Alert' WHERE uuid = '" + hostToTest.id + "'" + self.dbConnection.execute(sql_query) + + hostUpInCloudstack = wait_until(40, 10, self.checkHostStateInCloudstack, "Up", hostToTest.id) + + if not(hostUpInCloudstack): + raise self.fail("Host is not up %s, in cloudstack so failing test " % (hostToTest.ipaddress)) + return From 39b9cfe94390bd7c47a88e32eb41a0e651867458 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 18 Oct 2023 12:15:54 +0530 Subject: [PATCH 4/6] Address comments --- .../cloud/agent/manager/AgentManagerImpl.java | 126 ++++++++++-------- test/integration/smoke/test_host_ping.py | 52 +++----- 2 files changed, 88 insertions(+), 90 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 d2d05285bd66..c27abcad4b41 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 @@ -799,6 +799,30 @@ public boolean stop() { return true; } + protected Status getNextStatusOnDisconnection(Host host, final Status.Event event) { + final Status currentStatus = host.getStatus(); + Status nextStatus; + if (currentStatus == Status.Down || currentStatus == Status.Alert || currentStatus == Status.Removed) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Host " + host.getId() + " is already " + currentStatus); + } + nextStatus = currentStatus; + } else { + try { + nextStatus = currentStatus.getNextStatus(event); + } catch (final NoTransitionException e) { + final String err = "Cannot find next status for " + event + " as current status is " + currentStatus + " for agent " + host.getId(); + s_logger.debug(err); + throw new CloudRuntimeException(err); + } + + if (s_logger.isDebugEnabled()) { + s_logger.debug("The next status of agent " + host.getId() + "is " + nextStatus + ", current status is " + currentStatus); + } + } + return nextStatus; + } + protected boolean handleDisconnectWithoutInvestigation(final AgentAttache attache, final Status.Event event, final boolean transitState, final boolean removeAgent) { final long hostId = attache.getId(); @@ -813,25 +837,7 @@ protected boolean handleDisconnectWithoutInvestigation(final AgentAttache attach s_logger.warn("Can't find host with " + hostId); nextStatus = Status.Removed; } else { - final Status currentStatus = host.getStatus(); - if (currentStatus == Status.Down || currentStatus == Status.Alert || currentStatus == Status.Removed) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Host " + hostId + " is already " + currentStatus); - } - nextStatus = currentStatus; - } else { - try { - nextStatus = currentStatus.getNextStatus(event); - } catch (final NoTransitionException e) { - final String err = "Cannot find next status for " + event + " as current status is " + currentStatus + " for agent " + hostId; - s_logger.debug(err); - throw new CloudRuntimeException(err); - } - - if (s_logger.isDebugEnabled()) { - s_logger.debug("The next status of agent " + hostId + "is " + nextStatus + ", current status is " + currentStatus); - } - } + nextStatus = getNextStatusOnDisconnection(host, event); caService.purgeHostCertificate(host); } @@ -1112,45 +1118,50 @@ protected AgentAttache createAttacheForConnect(final HostVO host, final Link lin return attache; } - private AgentAttache handleConnectedAgent(final Link link, final StartupCommand[] startup, final Request request) { + private AgentAttache sendReadyAndGetAttache(HostVO host, ReadyCommand ready, Link link, StartupCommand[] startup) throws ConnectionException { + final List agentMSHostList = new ArrayList<>(); + String lbAlgorithm = null; + if (startup != null && startup.length > 0) { + final String agentMSHosts = startup[0].getMsHostList(); + if (StringUtils.isNotEmpty(agentMSHosts)) { + String[] msHosts = agentMSHosts.split("@"); + if (msHosts.length > 1) { + lbAlgorithm = msHosts[1]; + } + agentMSHostList.addAll(Arrays.asList(msHosts[0].split(","))); + } + } AgentAttache attache = null; - ReadyCommand ready = null; - try { - final List agentMSHostList = new ArrayList<>(); - String lbAlgorithm = null; - if (startup != null && startup.length > 0) { - final String agentMSHosts = startup[0].getMsHostList(); - if (StringUtils.isNotEmpty(agentMSHosts)) { - String[] msHosts = agentMSHosts.split("@"); - if (msHosts.length > 1) { - lbAlgorithm = msHosts[1]; - } - agentMSHostList.addAll(Arrays.asList(msHosts[0].split(","))); + GlobalLock joinLock = getHostJoinLock(host.getId()); + if (joinLock.lock(60)) { + try { + + if (!indirectAgentLB.compareManagementServerList(host.getId(), host.getDataCenterId(), agentMSHostList, lbAlgorithm)) { + final List newMSList = indirectAgentLB.getManagementServerList(host.getId(), host.getDataCenterId(), null); + ready.setMsHostList(newMSList); + ready.setLbAlgorithm(indirectAgentLB.getLBAlgorithmName()); + ready.setLbCheckInterval(indirectAgentLB.getLBPreferredHostCheckInterval(host.getClusterId())); + s_logger.debug("Agent's management server host list is not up to date, sending list update:" + newMSList); } + + attache = createAttacheForConnect(host, link); + attache = notifyMonitorsOfConnection(attache, startup, false); + } finally { + joinLock.unlock(); } + joinLock.releaseRef(); + } + return attache; + } + private AgentAttache handleConnectedAgent(final Link link, final StartupCommand[] startup, final Request request) { + AgentAttache attache = null; + ReadyCommand ready = null; + try { final HostVO host = _resourceMgr.createHostVOForConnectedAgent(startup); if (host != null) { - GlobalLock joinLock = getHostJoinLock(host.getId()); - if (joinLock.lock(60)) { - try { - ready = new ReadyCommand(host.getDataCenterId(), host.getId(), NumbersUtil.enableHumanReadableSizes); - - if (!indirectAgentLB.compareManagementServerList(host.getId(), host.getDataCenterId(), agentMSHostList, lbAlgorithm)) { - final List newMSList = indirectAgentLB.getManagementServerList(host.getId(), host.getDataCenterId(), null); - ready.setMsHostList(newMSList); - ready.setLbAlgorithm(indirectAgentLB.getLBAlgorithmName()); - ready.setLbCheckInterval(indirectAgentLB.getLBPreferredHostCheckInterval(host.getClusterId())); - s_logger.debug("Agent's management server host list is not up to date, sending list update:" + newMSList); - } - - attache = createAttacheForConnect(host, link); - attache = notifyMonitorsOfConnection(attache, startup, false); - } finally { - joinLock.unlock(); - } - } - joinLock.releaseRef(); + ready = new ReadyCommand(host.getDataCenterId(), host.getId(), NumbersUtil.enableHumanReadableSizes); + attache = sendReadyAndGetAttache(host, ready, link, startup); } } catch (final Exception e) { s_logger.debug("Failed to handle host connection: ", e); @@ -1342,10 +1353,11 @@ protected void processRequest(final Link link, final Request request) { boolean requestStartupCommand = false; final HostVO host = _hostDao.findById(Long.valueOf(cmdHostId)); + boolean gatewayAccessible = true; // if the router is sending a ping, verify the // gateway was pingable if (cmd instanceof PingRoutingCommand) { - final boolean gatewayAccessible = ((PingRoutingCommand)cmd).isGatewayAccessible(); + gatewayAccessible = ((PingRoutingCommand)cmd).isGatewayAccessible(); if (host != null) { if (!gatewayAccessible) { // alert that host lost connection to @@ -1358,16 +1370,12 @@ protected void processRequest(final Link link, final Request request) { "Host [" + hostDesc + "] lost connection to gateway (default route) and is possibly having network connection issues."); } else { _alertMgr.clearAlert(AlertManager.AlertType.ALERT_TYPE_ROUTING, host.getDataCenterId(), host.getPodId()); - if (host.getStatus() != Status.Up) { - // Only transit state when the status is not Up to avoid unnecessary db calls - requestStartupCommand = true; - } } } else { s_logger.debug("Not processing " + PingRoutingCommand.class.getSimpleName() + " for agent id=" + cmdHostId + "; can't find the host in the DB"); } - } else if (host != null && host.getStatus() != Status.Up) { - // Only transit state when the status is not Up to avoid unnecessary db calls + } + if (host!= null && host.getStatus() != Status.Up && gatewayAccessible) { requestStartupCommand = true; } answer = new PingAnswer((PingCommand)cmd, requestStartupCommand); diff --git a/test/integration/smoke/test_host_ping.py b/test/integration/smoke/test_host_ping.py index 0c14314f3638..9de77f9b7716 100644 --- a/test/integration/smoke/test_host_ping.py +++ b/test/integration/smoke/test_host_ping.py @@ -19,19 +19,18 @@ # Import Local Modules from marvin.cloudstackTestCase import * -from marvin.lib.utils import * -from marvin.lib.base import * from marvin.lib.common import * +from marvin.lib.utils import * from nose.plugins.attrib import attr _multiprocess_shared_ = False -class TestHostHA(cloudstackTestCase): +class TestHostPing(cloudstackTestCase): - def setUp(self): + def setUp(self, handler=logging.StreamHandler()): self.logger = logging.getLogger('TestHM') - self.stream_handler = logging.StreamHandler() + self.stream_handler = handler self.logger.setLevel(logging.DEBUG) self.logger.addHandler(self.stream_handler) self.apiclient = self.testClient.getApiClient() @@ -44,35 +43,28 @@ def setUp(self): self.cleanup = [] def tearDown(self): - try: - # Clean up, terminate the created templates - cleanup_resources(self.apiclient, self.cleanup) - - except Exception as e: - raise Exception("Warning: Exception during cleanup : %s" % e) - - return + super(TestHostPing, self).tearDown() def checkHostStateInCloudstack(self, state, host_id): try: listHost = Host.list( - self.apiclient, - type='Routing', - zoneid=self.zone.id, - podid=self.pod.id, - id=host_id - ) + self.apiclient, + type='Routing', + zoneid=self.zone.id, + podid=self.pod.id, + id=host_id + ) self.assertEqual( - isinstance(listHost, list), - True, - "Check if listHost returns a valid response" - ) + isinstance(listHost, list), + True, + "Check if listHost returns a valid response" + ) self.assertEqual( - len(listHost), - 1, - "Check if listHost returns a host" - ) + len(listHost), + 1, + "Check if listHost returns a host" + ) self.logger.debug(" Host state is %s " % listHost[0].state) if listHost[0].state == state: return True, 1 @@ -82,7 +74,6 @@ def checkHostStateInCloudstack(self, state, host_id): self.logger.debug("Got exception %s" % e) return False, 1 - @attr( tags=[ "advanced", @@ -100,13 +91,12 @@ def test_01_host_ping_on_alert(self): for host in listHost: self.logger.debug('Hypervisor = {}'.format(host.id)) - hostToTest = listHost[0] sql_query = "UPDATE host SET status = 'Alert' WHERE uuid = '" + hostToTest.id + "'" self.dbConnection.execute(sql_query) - hostUpInCloudstack = wait_until(40, 10, self.checkHostStateInCloudstack, "Up", hostToTest.id) + hostUpInCloudstack = wait_until(30, 8, self.checkHostStateInCloudstack, "Up", hostToTest.id) - if not(hostUpInCloudstack): + if not (hostUpInCloudstack): raise self.fail("Host is not up %s, in cloudstack so failing test " % (hostToTest.ipaddress)) return From c8dd68f16b02106b72eb7f8ad6e9b92d0eaf2d07 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Mon, 23 Oct 2023 12:49:08 +0530 Subject: [PATCH 5/6] Resolve comments --- .../com/cloud/agent/manager/AgentManagerImpl.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 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 c27abcad4b41..e1460f4a6b52 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 @@ -804,20 +804,20 @@ protected Status getNextStatusOnDisconnection(Host host, final Status.Event even Status nextStatus; if (currentStatus == Status.Down || currentStatus == Status.Alert || currentStatus == Status.Removed) { if (s_logger.isDebugEnabled()) { - s_logger.debug("Host " + host.getId() + " is already " + currentStatus); + s_logger.debug(String.format("Host %s is already %s", host.getUuid(), currentStatus)); } nextStatus = currentStatus; } else { try { nextStatus = currentStatus.getNextStatus(event); } catch (final NoTransitionException e) { - final String err = "Cannot find next status for " + event + " as current status is " + currentStatus + " for agent " + host.getId(); + final String err = String.format("Cannot find next status for %s as current status is %s for agent %s", event, currentStatus, host.getUuid()); s_logger.debug(err); throw new CloudRuntimeException(err); } if (s_logger.isDebugEnabled()) { - s_logger.debug("The next status of agent " + host.getId() + "is " + nextStatus + ", current status is " + currentStatus); + s_logger.debug(String.format("The next status of agent %s is %s, current status is %s", host.getUuid(), nextStatus, currentStatus)); } } return nextStatus; @@ -830,11 +830,11 @@ protected boolean handleDisconnectWithoutInvestigation(final AgentAttache attach GlobalLock joinLock = getHostJoinLock(hostId); if (joinLock.lock(60)) { try { - s_logger.info("Host " + hostId + " is disconnecting with event " + event); + s_logger.info(String.format("Host %d is disconnecting with event %s", hostId, event)); Status nextStatus = null; final HostVO host = _hostDao.findById(hostId); if (host == null) { - s_logger.warn("Can't find host with " + hostId); + s_logger.warn(String.format("Can't find host with %d", hostId)); nextStatus = Status.Removed; } else { nextStatus = getNextStatusOnDisconnection(host, event); @@ -842,13 +842,13 @@ protected boolean handleDisconnectWithoutInvestigation(final AgentAttache attach } if (s_logger.isDebugEnabled()) { - s_logger.debug("Deregistering link for " + hostId + " with state " + nextStatus); + s_logger.debug(String.format("Deregistering link for %d with state %s", hostId, nextStatus)); } removeAgent(attache, nextStatus); - // update the DB if (host != null && transitState) { + // update the state for host in DB as per the event disconnectAgent(host, event, _nodeId); } } finally { From 0169fc93f1aed0d67e66bd98f6bd07bdedb9c5fd Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 25 Oct 2023 12:50:41 +0530 Subject: [PATCH 6/6] Handle case when MS is unable to acquire lock on host --- .../main/java/com/cloud/agent/manager/AgentManagerImpl.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 e1460f4a6b52..26e871877a3a 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 @@ -1149,8 +1149,10 @@ private AgentAttache sendReadyAndGetAttache(HostVO host, ReadyCommand ready, Lin } finally { joinLock.unlock(); } - joinLock.releaseRef(); + } else { + throw new ConnectionException(true, "Unable to acquire lock on host " + host.getUuid()); } + joinLock.releaseRef(); return attache; }