Add KVM HA Client validation on HA execution flow

plugins/hypervisors/kvm/src/main/java/com/cloud/ha/KVMInvestigator.java

@@ -26,14 +26,14 @@
 import com.cloud.host.Status;
 import com.cloud.host.dao.HostDao;
 import com.cloud.hypervisor.Hypervisor;
+import com.cloud.hypervisor.kvm.resource.KvmAgentHaClient;
 import com.cloud.resource.ResourceManager;
 import com.cloud.storage.Storage.StoragePoolType;
 import com.cloud.utils.component.AdapterBase;
 import org.apache.cloudstack.ha.HAManager;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.log4j.Logger;
-import org.jetbrains.annotations.NotNull;
 
 import javax.inject.Inject;
 import java.util.List;
@@ -81,11 +81,19 @@ public Status isAgentAlive(Host agent) {
         Status agentStatus = Status.Disconnected;
         boolean hasNfs = isHostServedByNfsPool(agent);
         if (hasNfs) {
-            s_logger.debug("Agent investigation was requested on host " + agent + ", checking agent status via NFS storage.");
             agentStatus = checkAgentStatusViaNfs(agent);
+            s_logger.debug(String.format("Agent investigation was requested on host %s, agent status via NFS storage is %s.", agent, agentStatus));
         } else {
-            s_logger.debug(
-                    "Agent investigation was requested on host " + agent + ", but host has no NFS storage. Skipping investigation via NFS.");
+            s_logger.debug(String.format("Agent investigation was requested on host %s, but host has no NFS storage. Skipping investigation via NFS.", agent));
+        }
+
+        KvmAgentHaClient kvmAgentHaClient = new KvmAgentHaClient(agent.getPrivateIpAddress());
+        boolean isKvmAgentRunning = kvmAgentHaClient.isKvmHaAgentRunning();
+        if(isKvmAgentRunning) {
+            agentStatus = Status.Up;
+            s_logger.debug(String.format("Checking agent %s status; KVM HA webserver is Running as expected."));
+        } else {
+            s_logger.warn(String.format("Checking agent %s status. Failed to check host status via KVM Agent HA webserver"));
         }
 
         return agentStatus;
@@ -119,7 +127,6 @@ private boolean hasNfsPoolClusterWideForHost(Host agent) {
         return false;
     }
 
-    @NotNull
     private Status checkAgentStatusViaNfs(Host agent) {
         Status hostStatus = null;
         Status neighbourStatus = null;

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KvmAgentHaClient.java

@@ -16,13 +16,17 @@
 package com.cloud.hypervisor.kvm.resource;
 
 import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.gson.JsonParser;
+import org.apache.cloudstack.utils.redfish.RedfishException;
+import org.apache.commons.httpclient.HttpStatus;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpRequestBase;
 import org.apache.http.client.utils.URIBuilder;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.log4j.Logger;
+import org.jetbrains.annotations.Nullable;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -46,73 +50,102 @@ public class KvmAgentHaClient {
 
     private static final Logger LOGGER = Logger.getLogger(KvmAgentHaClient.class);
     private final static int WAIT_FOR_REQUEST_RETRY = 2;
+    private final static String VM_COUNT = "count";
+    private final static int ERROR_CODE = -1;
+    private final static String EXPECTED_HTTP_STATUS = "2XX";
+    private static final int MAX_REQUEST_RETRIES = 2;
+    private static final int DEFAULT_PORT = 8080;
     private String agentIpAddress;
     private int port;
-    private int requestMaxRetries = 0; //TODO
 
-    public KvmAgentHaClient(String agentIpAddress, int port) {
+    /**
+     * Instantiates a webclient that checks, via a webserver running on the KVM host, the VMs running
+     * @param agentIpAddress address of the KVM host running the webserver
+     */
+    public KvmAgentHaClient(String agentIpAddress) {
         this.agentIpAddress = agentIpAddress;
-        this.port = port;
+    }
+
+    public boolean isKvmHaAgentRunning() {
+        if (countRunningVmsOnAgent() < 0) {
+            return false;
+        }
+        return true;
     }
 
     /**
-     * TODO
-     *  Returns the System ID. Used when sending Computer System requests (e.g. ComputerSystem.Reset request).
+     *  Returns the number of VMs running on the KVM host according to libvirt.
      */
-    public String checkVmsRunningOnAgent() {
-        String url = String.format("http://%s:%d", agentIpAddress, port);
+    public int countRunningVmsOnAgent() {
+        String url = String.format("http://%s:%d", agentIpAddress, DEFAULT_PORT);
+        HttpResponse response = executeHttpRequest(url);
 
-        URIBuilder builder = null;
+        if (response == null)
+            return ERROR_CODE;
+
+        return Integer.valueOf(processHttpResponseIntoJson(response));
+    }
+
+    /**
+     * Executes a GET request for the given URL address.
+     */
+    @Nullable
+    protected HttpResponse executeHttpRequest(String url) {
         HttpGet httpReq = null;
         try {
-            builder = new URIBuilder(url);
+            URIBuilder builder = new URIBuilder(url);
             httpReq = new HttpGet(builder.build());
         } catch (URISyntaxException e) {
-            throw new CloudRuntimeException(String.format("Failed to create URI for GET request [URL: %s] due to exception.", url), e);
+            LOGGER.error(String.format("Failed to create URI for GET request [URL: %s] due to exception.", url), e);
+            return null;
         }
 
         HttpClient client = HttpClientBuilder.create().build();
-
         HttpResponse response = null;
-
         try {
             response = client.execute(httpReq);
         } catch (IOException e) {
-            if (requestMaxRetries == 0) {
-                throw new CloudRuntimeException(String.format("Failed to execute HTTP %s request [URL: %s] due to exception %s.", httpReq.getMethod(), url, e), e);
+            if (MAX_REQUEST_RETRIES == 0) {
+                LOGGER.warn(String.format("Failed to execute HTTP %s request [URL: %s] due to exception %s.", httpReq.getMethod(), url, e), e);
+                return null;
             }
             retryHttpRequest(url, httpReq, client);
         }
-
-        return processHttpResponseIntoJson(response);
+        return response;
     }
 
     /**
-     * TODO
+     * Re-executes the HTTP GET request until it gets a response or it reaches the maximum request retries (#MAX_REQUEST_RETRIES)
      */
     protected HttpResponse retryHttpRequest(String url, HttpRequestBase httpReq, HttpClient client) {
         LOGGER.warn(String.format("Failed to execute HTTP %s request [URL: %s]. Executing the request again.", httpReq.getMethod(), url));
         HttpResponse response = null;
-        for (int attempt = 1; attempt < requestMaxRetries + 1; attempt++) {
+        for (int attempt = 1; attempt < MAX_REQUEST_RETRIES + 1; attempt++) {
             try {
                 TimeUnit.SECONDS.sleep(WAIT_FOR_REQUEST_RETRY);
-                LOGGER.debug(String.format("Retry HTTP %s request [URL: %s], attempt %d/%d.", httpReq.getMethod(), url, attempt, requestMaxRetries));
+                LOGGER.debug(String.format("Retry HTTP %s request [URL: %s], attempt %d/%d.", httpReq.getMethod(), url, attempt, MAX_REQUEST_RETRIES));
                 response = client.execute(httpReq);
             } catch (IOException | InterruptedException e) {
-                if (attempt == requestMaxRetries) {
-                    throw new CloudRuntimeException(
-                            String.format("Failed to execute HTTP %s request retry attempt %d/%d [URL: %s] due to exception %s", httpReq.getMethod(), attempt, requestMaxRetries,
+                if (attempt == MAX_REQUEST_RETRIES) {
+                    LOGGER.error(
+                            String.format("Failed to execute HTTP %s request retry attempt %d/%d [URL: %s] due to exception %s", httpReq.getMethod(), attempt, MAX_REQUEST_RETRIES,
                                     url, e));
                 } else {
-                    LOGGER.warn(
-                            String.format("Failed to execute HTTP %s request retry attempt %d/%d [URL: %s] due to exception %s", httpReq.getMethod(), attempt, requestMaxRetries,
+                    LOGGER.error(
+                            String.format("Failed to execute HTTP %s request retry attempt %d/%d [URL: %s] due to exception %s", httpReq.getMethod(), attempt, MAX_REQUEST_RETRIES,
                                     url, e));
                 }
             }
         }
 
         if (response == null) {
-            throw new CloudRuntimeException(String.format("Failed to execute HTTP %s request [URL: %s].", httpReq.getMethod(), url));
+            LOGGER.error(String.format("Failed to execute HTTP %s request [URL: %s].", httpReq.getMethod(), url));
+        }
+
+        int statusCode = response.getStatusLine().getStatusCode();
+        if (statusCode < HttpStatus.SC_OK || statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) {
+            throw new RedfishException(String.format("Failed to get VMs information with a %s request to URL '%s'. The expected HTTP status code is '%s' but it got '%s'.",
+                    HttpGet.METHOD_NAME, url, EXPECTED_HTTP_STATUS, statusCode));
         }
 
         LOGGER.debug(String.format("Successfully executed HTTP %s request [URL: %s].", httpReq.getMethod(), url));
@@ -126,14 +159,19 @@ protected HttpResponse retryHttpRequest(String url, HttpRequestBase httpReq, Htt
     protected String processHttpResponseIntoJson(HttpResponse response) {
         InputStream in;
         String jsonString;
+        if (response == null) {
+            return Integer.toString(ERROR_CODE);
+        }
         try {
             in = response.getEntity().getContent();
             BufferedReader streamReader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8));
             jsonString = streamReader.readLine();
         } catch (UnsupportedOperationException | IOException e) {
-            throw new CloudRuntimeException("Failed to process system Response", e);
+            throw new CloudRuntimeException("Failed to process response", e);
         }
-        return jsonString;
+
+        String vmsCount = new JsonParser().parse(jsonString).getAsJsonObject().get(VM_COUNT).getAsString();
+        return vmsCount;
     }
 
 }

plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/kvm/ha/KVMHostActivityChecker.java

@@ -26,7 +26,9 @@
 import com.cloud.host.HostVO;
 import com.cloud.host.Status;
 import com.cloud.hypervisor.Hypervisor;
+import com.cloud.hypervisor.kvm.resource.KvmAgentHaClient;
 import com.cloud.resource.ResourceManager;
+import com.cloud.storage.Storage;
 import com.cloud.storage.StorageManager;
 import com.cloud.storage.StoragePool;
 import com.cloud.storage.Volume;
@@ -81,7 +83,29 @@ public boolean isActive(Host r, DateTime suspectTime) throws HACheckerException
 
     @Override
     public boolean isHealthy(Host r) {
-        return isAgentActive(r);
+        boolean isHealthy = false;
+        HashMap<StoragePool, List<Volume>> poolVolMap = getVolumeUuidOnHost(r);
+        isHealthy = isHealthyCheckViaNfs(r, isHealthy, poolVolMap);
+
+        KvmAgentHaClient kvmAgentHaClient = new KvmAgentHaClient(r.getPrivateIpAddress());
+        boolean isKvmAgentRunning = kvmAgentHaClient.isKvmHaAgentRunning();
+
+        if(!isHealthy && isKvmAgentRunning) {
+            isHealthy = true;
+        }
+
+        return isHealthy;
+    }
+
+    private boolean isHealthyCheckViaNfs(Host r, boolean isHealthy, HashMap<StoragePool, List<Volume>> poolVolMap) {
+        for (StoragePool pool : poolVolMap.keySet()) {
+            if(Storage.StoragePoolType.NetworkFilesystem == pool.getPoolType()
+                    || Storage.StoragePoolType.ManagedNFS == pool.getPoolType()
+                    || Storage.StoragePoolType.ManagedNFS == pool.getPoolType()) {
+                isHealthy = isAgentActive(r);
+            }
+        }
+        return isHealthy;
     }
 
     private boolean isAgentActive(Host agent) {
@@ -151,19 +175,33 @@ private boolean isVMActivtyOnHost(Host agent, DateTime suspectTime) throws HAChe
         if (agent.getHypervisorType() != Hypervisor.HypervisorType.KVM && agent.getHypervisorType() != Hypervisor.HypervisorType.LXC) {
             throw new IllegalStateException(String.format("Calling KVM investigator for non KVM Host of type [%s].", agent.getHypervisorType()));
         }
-        boolean activityStatus = true;
+        boolean activityStatus = false;
         HashMap<StoragePool, List<Volume>> poolVolMap = getVolumeUuidOnHost(agent);
         for (StoragePool pool : poolVolMap.keySet()) {
-            activityStatus = verifyActivityOfStorageOnHost(poolVolMap, pool, agent, suspectTime, activityStatus);
-            if (!activityStatus) {
-                LOG.warn(String.format("It seems that the storage pool [%s] does not have activity on %s.", pool.getId(), agent.toString()));
-                break;
+            if(Storage.StoragePoolType.NetworkFilesystem == pool.getPoolType()
+                    || Storage.StoragePoolType.ManagedNFS == pool.getPoolType()
+                    || Storage.StoragePoolType.ManagedNFS == pool.getPoolType()) {
+                activityStatus = verifyActivityOfStorageOnHost(poolVolMap, pool, agent, suspectTime, activityStatus);
+                if (!activityStatus) {
+                    LOG.warn(String.format("It seems that the storage pool [%s] does not have activity on %s.", pool.getId(), agent.toString()));
+                    break;
+                }
             }
         }
 
+        KvmAgentHaClient kvmAgentHaClient = new KvmAgentHaClient(agent.getPrivateIpAddress());
+        boolean isKvmAgentRunning = kvmAgentHaClient.isKvmHaAgentRunning();
+
+        if(!activityStatus && isKvmAgentRunning) {
+            activityStatus = true;
+        } else {
+            LOG.warn(String.format("No VM activity detected on %s. This might trigger HA Host Recovery and/or Fence.", agent.toString()));
+        }
+
         return activityStatus;
     }
 
+
     protected boolean verifyActivityOfStorageOnHost(HashMap<StoragePool, List<Volume>> poolVolMap, StoragePool pool, Host agent, DateTime suspectTime, boolean activityStatus) throws HACheckerException, IllegalStateException {
         List<Volume> volume_list = poolVolMap.get(pool);
         final CheckVMActivityOnStoragePoolCommand cmd = new CheckVMActivityOnStoragePoolCommand(agent, pool, volume_list, suspectTime);

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/KvmAgentHaClientTest.java

@@ -15,17 +15,35 @@
  */
 package com.cloud.hypervisor.kvm.resource;
 
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
+import org.apache.http.StatusLine;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.mockito.Mockito;
 
-@RunWith(MockitoJUnitRunner.class)
+//@RunWith(MockitoJUnitRunner.class)
 public class KvmAgentHaClientTest {
 
+    private static final String AGENT_ADDRESS = "kvm-agent.domain.name";
+
+    private KvmAgentHaClient kvmAgentHaClient = Mockito.spy(new KvmAgentHaClient(AGENT_ADDRESS));
+
     //TODO
-    @Test
+//    @test
     public void checkHostStatusTest() {
-        KvmAgentHaClient kvmAgentHaClient = new KvmAgentHaClient("host", 8080);
-        System.out.println(kvmAgentHaClient.checkVmsRunningOnAgent());
+        int kvmAgentResponse = kvmAgentHaClient.countRunningVmsOnAgent();
+    }
+
+//    @Test
+    public void isKvmHaAgentRunningTest() {
+        boolean isKvmAgentRunning = kvmAgentHaClient.isKvmHaAgentRunning();
     }
+
+    private CloseableHttpResponse mockResponse(int httpStatusCode) {
+        StatusLine statusLine = Mockito.mock(StatusLine.class);
+        Mockito.doReturn(httpStatusCode).when(statusLine).getStatusCode();
+        CloseableHttpResponse response = Mockito.mock(CloseableHttpResponse.class);
+        Mockito.doReturn(statusLine).when(response).getStatusLine();
+        Mockito.doReturn(response).when(kvmAgentHaClient).executeHttpRequest(Mockito.anyString());
+        return response;
+    }
+
 }