You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by bh...@apache.org on 2015/05/03 15:16:12 UTC

git commit: updated refs/heads/master to 1840805

Repository: cloudstack
Updated Branches:
  refs/heads/master 89fa25111 -> 1840805aa


server: Introduce Unknown Status to be used in AbstractInvestigatorImpl

The PR #211 introduced changes where the abstract investigator testIpAddress()
would return other Status, which previously only returned null, Up or Down. In
this patch we introduce a new Status "Unknown" that replaces null's semantics.

The important changes #211 introduced was the debugging statements as semantically
the changes would work same as the consumers of testIpAddress() method only used
if returned values were Up or Down and in other cases (null, Alert etc) it would
simply continue to loop through the resources being investigated.

Keeping the debug logs, this commit only replaces the previously returned null
values with Status.Unknown and fixed the debug statements to reflect the same.
In case of trapped exceptions too, we return Unknown status but log the exception
we trapped.

server: add null assertions and remove dead code with testIpAddress usage

This closes #222

Signed-off-by: Rohit Yadav <ro...@shapeblue.com>
(cherry picked from commit 7a1cb28c9f548ac185dcb7c59eb2fadb7d550718)
Signed-off-by: Rohit Yadav <ro...@shapeblue.com>


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/1840805a
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/1840805a
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/1840805a

Branch: refs/heads/master
Commit: 1840805aab4f177035bd36392e3ebd7c6abc6893
Parents: 89fa251
Author: Rohit Yadav <ro...@shapeblue.com>
Authored: Fri May 1 16:03:51 2015 +0200
Committer: Rohit Yadav <ro...@shapeblue.com>
Committed: Sun May 3 15:16:04 2015 +0200

----------------------------------------------------------------------
 api/src/com/cloud/host/Status.java                 |  3 ++-
 .../src/com/cloud/ha/AbstractInvestigatorImpl.java | 17 +++++++++--------
 .../cloud/ha/ManagementIPSystemVMInvestigator.java |  9 ++++-----
 .../src/com/cloud/ha/UserVmDomRInvestigator.java   |  8 +++-----
 4 files changed, 18 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1840805a/api/src/com/cloud/host/Status.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/host/Status.java b/api/src/com/cloud/host/Status.java
index 732a066..73e6cc9 100644
--- a/api/src/com/cloud/host/Status.java
+++ b/api/src/com/cloud/host/Status.java
@@ -31,7 +31,8 @@ public enum Status {
     Alert(true, true, true),
     Removed(true, false, true),
     Error(true, false, true),
-    Rebalancing(true, false, true);
+    Rebalancing(true, false, true),
+    Unknown(false, false, false); // null
 
     private final boolean updateManagementServer;
     private final boolean checkManagementServer;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1840805a/server/src/com/cloud/ha/AbstractInvestigatorImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/ha/AbstractInvestigatorImpl.java b/server/src/com/cloud/ha/AbstractInvestigatorImpl.java
index b222386..147cecd 100644
--- a/server/src/com/cloud/ha/AbstractInvestigatorImpl.java
+++ b/server/src/com/cloud/ha/AbstractInvestigatorImpl.java
@@ -85,14 +85,15 @@ public abstract class AbstractInvestigatorImpl extends AdapterBase implements In
         return hostIds;
     }
 
+    // Method only returns Status.Up, Status.Down and Status.Unknown
     protected Status testIpAddress(Long hostId, String testHostIp) {
         try {
             Answer pingTestAnswer = _agentMgr.send(hostId, new PingTestCommand(testHostIp));
             if (pingTestAnswer == null) {
                 if (s_logger.isDebugEnabled()) {
-                    s_logger.debug("host (" + testHostIp + ") returns null answer");
+                    s_logger.debug("host (" + testHostIp + ") returns Unknown (null) answer");
                 }
-                return Status.Alert;
+                return Status.Unknown;
             }
 
             if (pingTestAnswer.getResult()) {
@@ -103,20 +104,20 @@ public abstract class AbstractInvestigatorImpl extends AdapterBase implements In
                 return Status.Up;
             } else {
                 if (s_logger.isDebugEnabled()) {
-                    s_logger.debug("host (" + testHostIp + ") cannot be pinged, returning Alert state");
+                    s_logger.debug("host (" + testHostIp + ") cannot be pinged, returning Unknown (I don't know) state");
                 }
-                return Status.Alert;
+                return Status.Unknown;
             }
         } catch (AgentUnavailableException e) {
             if (s_logger.isDebugEnabled()) {
-                s_logger.debug("host (" + testHostIp + "): " + e.getLocalizedMessage() + ", returning Disconnected state");
+                s_logger.debug("host (" + testHostIp + "): " + e.getLocalizedMessage() + ", trapped AgentUnavailableException returning Unknown state");
             }
-            return Status.Disconnected;
+            return Status.Unknown;
         } catch (OperationTimedoutException e) {
             if (s_logger.isDebugEnabled()) {
-                s_logger.debug("host (" + testHostIp + "): " + e.getLocalizedMessage() + ", returning Alert state");
+                s_logger.debug("host (" + testHostIp + "): " + e.getLocalizedMessage() + ", trapped OperationTimedoutException returning Unknown state");
             }
-            return Status.Alert;
+            return Status.Unknown;
         }
     }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1840805a/server/src/com/cloud/ha/ManagementIPSystemVMInvestigator.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/ha/ManagementIPSystemVMInvestigator.java b/server/src/com/cloud/ha/ManagementIPSystemVMInvestigator.java
index 5db5b9c..89901d9 100644
--- a/server/src/com/cloud/ha/ManagementIPSystemVMInvestigator.java
+++ b/server/src/com/cloud/ha/ManagementIPSystemVMInvestigator.java
@@ -78,10 +78,8 @@ public class ManagementIPSystemVMInvestigator extends AbstractInvestigatorImpl {
             List<Long> otherHosts = findHostByPod(vmHost.getPodId(), vm.getHostId());
             for (Long otherHost : otherHosts) {
                 Status vmState = testIpAddress(otherHost, nic.getIp4Address());
-                if (vmState == null) {
-                    // can't get information from that host, try the next one
-                    continue;
-                }
+                assert vmState != null;
+                // In case of Status.Unknown, next host will be tried
                 if (vmState == Status.Up) {
                     if (s_logger.isDebugEnabled()) {
                         s_logger.debug("successfully pinged vm's private IP (" + vm.getPrivateIpAddress() + "), returning that the VM is up");
@@ -91,7 +89,8 @@ public class ManagementIPSystemVMInvestigator extends AbstractInvestigatorImpl {
                     // We can't ping the VM directly...if we can ping the host, then report the VM down.
                     // If we can't ping the host, then we don't have enough information.
                     Status vmHostState = testIpAddress(otherHost, vmHost.getPrivateIpAddress());
-                    if ((vmHostState != null) && (vmHostState == Status.Up)) {
+                    assert vmHostState != null;
+                    if (vmHostState == Status.Up) {
                         if (s_logger.isDebugEnabled()) {
                             s_logger.debug("successfully pinged vm's host IP (" + vmHost.getPrivateIpAddress() +
                                 "), but could not ping VM, returning that the VM is down");

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1840805a/server/src/com/cloud/ha/UserVmDomRInvestigator.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/ha/UserVmDomRInvestigator.java b/server/src/com/cloud/ha/UserVmDomRInvestigator.java
index 6084e76..782e4b3 100644
--- a/server/src/com/cloud/ha/UserVmDomRInvestigator.java
+++ b/server/src/com/cloud/ha/UserVmDomRInvestigator.java
@@ -116,14 +116,12 @@ public class UserVmDomRInvestigator extends AbstractInvestigatorImpl {
         List<Long> otherHosts = findHostByPod(agent.getPodId(), agent.getId());
 
         for (Long hostId : otherHosts) {
-
             if (s_logger.isDebugEnabled()) {
                 s_logger.debug("sending ping from (" + hostId + ") to agent's host ip address (" + agent.getPrivateIpAddress() + ")");
             }
             Status hostState = testIpAddress(hostId, agent.getPrivateIpAddress());
-            if (hostState == null) {
-                continue;
-            }
+            assert hostState != null;
+            // In case of Status.Unknown, next host will be tried
             if (hostState == Status.Up) {
                 if (s_logger.isDebugEnabled()) {
                     s_logger.debug("ping from (" + hostId + ") to agent's host ip address (" + agent.getPrivateIpAddress() +
@@ -134,7 +132,7 @@ public class UserVmDomRInvestigator extends AbstractInvestigatorImpl {
                 if (s_logger.isDebugEnabled()) {
                     s_logger.debug("returning host state: " + hostState);
                 }
-                return hostState;
+                return Status.Down;
             }
         }