kvm: fix HA checker deciding host state from the last pool only

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

@@ -40,18 +40,11 @@ public KVMHAChecker(List<HAStoragePool> pools, HostTO host, boolean reportFailur
      */
     @Override
     public Boolean checkingHeartBeat() {
-        boolean validResult = false;
-
         String hostAndPools = String.format("host IP [%s] in pools [%s]", host.getPrivateNetwork().getIp(), storagePools.stream().map(pool -> pool.getPoolUUID()).collect(Collectors.joining(", ")));
 
         logger.debug(String.format("Checking heart beat with KVMHAChecker for %s", hostAndPools));
 
-        for (HAStoragePool pool : storagePools) {
-            validResult = pool.getPool().checkingHeartBeat(pool, host);
-            if (reportFailureIfOneStorageIsDown && !validResult) {
-                break;
-            }
-        }
+        boolean validResult = checkPoolsHeartBeat();
 
         if (!validResult) {
             logger.warn(String.format("All checks with KVMHAChecker for %s considered it as dead. It may cause a shutdown of the host.", hostAndPools));
@@ -60,6 +53,22 @@ public Boolean checkingHeartBeat() {
         return validResult;
     }
 
+    /*
+     * With reportFailureIfOneStorageIsDown the host is dead as soon as one pool has no heart beat,
+     * otherwise it is dead only when none of the pools has one.
+     */
+    private boolean checkPoolsHeartBeat() {
+        boolean anyPoolHasHeartBeat = false;
+        for (HAStoragePool pool : storagePools) {
+            boolean poolHasHeartBeat = pool.getPool().checkingHeartBeat(pool, host);
+            if (reportFailureIfOneStorageIsDown && !poolHasHeartBeat) {
+                return false;
+            }
+            anyPoolHasHeartBeat = anyPoolHasHeartBeat || poolHasHeartBeat;
+        }
+        return anyPoolHasHeartBeat;
+    }
+
     @Override
     public Boolean call() throws Exception {
         // logger.addAppender(new org.apache.log4j.ConsoleAppender(new

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

@@ -0,0 +1,94 @@
+/*
+ * 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.hypervisor.kvm.resource;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import com.cloud.agent.api.to.HostTO;
+import com.cloud.agent.api.to.NetworkTO;
+import com.cloud.hypervisor.kvm.resource.KVMHABase.HAStoragePool;
+import com.cloud.hypervisor.kvm.resource.KVMHABase.PoolType;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
+
+@RunWith(MockitoJUnitRunner.Silent.class)
+public class KVMHACheckerTest {
+
+    @Mock
+    private HostTO host;
+    @Mock
+    private NetworkTO privateNetwork;
+
+    private List<HAStoragePool> poolsWithHeartBeats(boolean... heartBeats) {
+        when(host.getPrivateNetwork()).thenReturn(privateNetwork);
+        List<HAStoragePool> pools = new ArrayList<>();
+        for (boolean heartBeat : heartBeats) {
+            KVMStoragePool kvmPool = mock(KVMStoragePool.class);
+            when(kvmPool.checkingHeartBeat(any(HAStoragePool.class), any(HostTO.class))).thenReturn(heartBeat);
+            pools.add(new HAStoragePool(kvmPool, "host", "/path", PoolType.PrimaryStorage));
+        }
+        return pools;
+    }
+
+    private boolean checkHeartBeat(boolean reportIfFailedForOnePool, boolean... heartBeats) {
+        return new KVMHAChecker(poolsWithHeartBeats(heartBeats), host, reportIfFailedForOnePool).checkingHeartBeat();
+    }
+
+    @Test
+    public void testHostIsAliveWhenAnyPoolHasHeartBeatAndOnePoolFailureIsNotReported() {
+        assertTrue(checkHeartBeat(false, true, false));
+        assertTrue(checkHeartBeat(false, false, true));
+    }
+
+    @Test
+    public void testHostIsDeadWhenNoPoolHasHeartBeat() {
+        assertFalse(checkHeartBeat(false, false, false));
+        assertFalse(checkHeartBeat(true, false, false));
+    }
+
+    @Test
+    public void testHostIsDeadWhenOnePoolHasNoHeartBeatAndItIsReported() {
+        assertFalse(checkHeartBeat(true, true, false));
+        assertFalse(checkHeartBeat(true, false, true));
+    }
+
+    @Test
+    public void testHostIsAliveWhenAllPoolsHaveHeartBeat() {
+        assertTrue(checkHeartBeat(true, true, true));
+        assertTrue(checkHeartBeat(false, true, true));
+    }
+
+    @Test
+    public void testHostIsDeadWithoutPools() {
+        assertFalse(checkHeartBeat(false));
+        assertFalse(checkHeartBeat(true));
+    }
+}