kvm: support Host HA on Ceph RBD primary storage

engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java

@@ -33,7 +33,8 @@
  */
 public interface HighAvailabilityManager extends Manager {
 
-    List<StoragePoolType> LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint);
+    List<StoragePoolType> LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint,
+            StoragePoolType.RBD);
 
     ConfigKey<Boolean> ForceHA = new ConfigKey<>("Advanced", Boolean.class, "force.ha", "false",
         "Force High-Availability to happen even if the VM says no.", true, Cluster);

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

@@ -20,6 +20,7 @@
 import com.cloud.agent.properties.AgentPropertiesFileHandler;
 import com.cloud.ha.HighAvailabilityManager;
 import com.cloud.utils.script.Script;
+import org.apache.commons.lang3.StringUtils;
 import org.libvirt.Connect;
 import org.libvirt.LibvirtException;
 import org.libvirt.StoragePool;
@@ -54,7 +55,11 @@ public void removeStoragePool(String uuid) {
         synchronized (haStoragePools) {
             HAStoragePool pool = haStoragePools.get(uuid);
             if (pool != null) {
-                Script.runSimpleBashScript("umount " + pool.getMountDestPath());
+                if (StringUtils.isNotEmpty(pool.getMountDestPath())) {
+                    Script umount = new Script("umount", logger);
+                    umount.add(pool.getMountDestPath());
+                    umount.execute();
+                }
                 haStoragePools.remove(uuid);
             }
         }
@@ -130,7 +135,7 @@ private void checkForNotExistingLibvirtStoragePools(Set<String> removedPools, St
                 removedPools.add(uuid);
             }
 
-            logger.debug("Found NFS storage pool [{}] in libvirt, continuing.", uuid);
+            logger.debug("Found storage pool [{}] in libvirt, continuing.", uuid);
 
         } catch (LibvirtException e) {
             logger.debug("Failed to lookup libvirt storage pool [{}].", uuid, e);

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java

@@ -37,6 +37,7 @@
 
 import com.cloud.agent.api.to.DiskTO;
 import com.cloud.agent.api.to.VirtualMachineTO;
+import com.cloud.ha.HighAvailabilityManager;
 import com.cloud.hypervisor.kvm.resource.KVMHABase;
 import com.cloud.hypervisor.kvm.resource.KVMHABase.PoolType;
 import com.cloud.hypervisor.kvm.resource.KVMHAMonitor;
@@ -445,7 +446,7 @@ public boolean disconnectPhysicalDisk(StoragePoolType type, String poolUuid, Str
 
     public synchronized boolean deleteStoragePool(StoragePoolType type, String uuid) {
         StorageAdaptor adaptor = getStorageAdaptor(type);
-        if (type == StoragePoolType.NetworkFilesystem) {
+        if (HighAvailabilityManager.LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT.contains(type)) {
             _haMonitor.removeStoragePool(uuid);
         }
         boolean deleteStatus = adaptor.deleteStoragePool(uuid);;
@@ -457,7 +458,7 @@ public synchronized boolean deleteStoragePool(StoragePoolType type, String uuid)
 
     public boolean deleteStoragePool(StoragePoolType type, String uuid, Map<String, String> details) {
         StorageAdaptor adaptor = getStorageAdaptor(type);
-        if (type == StoragePoolType.NetworkFilesystem) {
+        if (HighAvailabilityManager.LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT.contains(type)) {
             _haMonitor.removeStoragePool(uuid);
         }
         boolean deleteStatus = adaptor.deleteStoragePool(uuid, details);

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java

@@ -17,10 +17,12 @@
 package com.cloud.hypervisor.kvm.storage;
 
 import java.io.File;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
 import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.joda.time.Duration;
@@ -330,29 +332,83 @@ public boolean isPoolSupportHA() {
 
     public String getHearthBeatPath() {
         if (StoragePoolType.NetworkFilesystem.equals(type)) {
-            String kvmScriptsDir = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_SCRIPTS_DIR);
-            String scriptPath = Script.findScript(kvmScriptsDir, "kvmheartbeat.sh");
-            if (scriptPath == null) {
-                throw new CloudRuntimeException("Unable to find heartbeat script 'kvmheartbeat.sh' in directory: " + kvmScriptsDir);
-            }
-            return scriptPath;
+            return findKvmHaScript("kvmheartbeat.sh");
         } else if (StoragePoolType.SharedMountPoint.equals(type)) {
-            String kvmScriptsDir = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_SCRIPTS_DIR);
-            String scriptPath = Script.findScript(kvmScriptsDir, "kvmsmpheartbeat.sh");
-            if (scriptPath == null) {
-                throw new CloudRuntimeException("Unable to find heartbeat script 'kvmsmpheartbeat.sh' in directory: " + kvmScriptsDir);
-            }
-            return scriptPath;
+            return findKvmHaScript("kvmsmpheartbeat.sh");
+        } else if (StoragePoolType.RBD.equals(type)) {
+            return findKvmHaScript("kvmheartbeat_rbd.sh");
         }
         return null;
     }
 
+    private String findKvmHaScript(String scriptName) {
+        String kvmScriptsDir = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_SCRIPTS_DIR);
+        String scriptPath = Script.findScript(kvmScriptsDir, scriptName);
+        if (scriptPath == null) {
+            throw new CloudRuntimeException(String.format("Unable to find script '%s' in directory: %s", scriptName, kvmScriptsDir));
+        }
+        return scriptPath;
+    }
+
+    /**
+     * Returns the Ceph monitors as expected by "--mon-host": the comma-separated monitors of the pool, trimmed
+     * and without empty entries, each with the pool's monitor port if one is set (IPv6 addresses are enclosed
+     * in square brackets then).
+     */
+    protected String getRbdMonitors() {
+        List<String> monitors = new ArrayList<>();
+        for (String monitor : sourceHost.split(",")) {
+            monitor = monitor.trim();
+            if (monitor.isEmpty()) {
+                continue;
+            }
+            if (sourcePort > 0) {
+                if (monitor.contains(":") && !monitor.startsWith("[")) {
+                    monitor = "[" + monitor + "]";
+                }
+                monitor = monitor + ":" + sourcePort;
+            }
+            monitors.add(monitor);
+        }
+        return String.join(",", monitors);
+    }
+
+    /**
+     * Adds the Ceph cluster connection details (monitors, pool and, if cephx is enabled, credentials)
+     * to a heartbeat/VM-activity check {@link Script} for a RBD storage pool. Mirrors the "mon_host"/"id"/"key"
+     * options that qemu itself uses to talk to RBD (see {@link KVMPhysicalDisk#RBDStringBuilder}).
+     */
+    protected void addRbdConnectionArgs(Script cmd) {
+        boolean hasUser = StringUtils.isNotBlank(authUsername);
+        boolean hasSecret = StringUtils.isNotBlank(authSecret);
+        if (hasUser != hasSecret) {
+            throw new CloudRuntimeException(String.format("The Ceph user and key must be either both set or both unset for the storage pool %s", uuid));
+        }
+        cmd.add("-s", getRbdMonitors());
+        cmd.add("-o", sourceDir);
+        if (hasUser) {
+            cmd.add("-n", authUsername);
+            cmd.add("-k", authSecret);
+        }
+    }
+
+    /**
+     * Adds the arguments identifying the storage to a heartbeat/VM-activity check {@link Script}:
+     * the Ceph connection details for a RBD pool, or the NFS server, path and mount point otherwise.
+     */
+    private void addPoolConnectionArgs(Script cmd, HAStoragePool pool) {
+        if (StoragePoolType.RBD.equals(type)) {
+            addRbdConnectionArgs(cmd);
+        } else {
+            cmd.add("-i", pool.getPoolIp());
+            cmd.add("-p", pool.getPoolMountSourcePath());
+            cmd.add("-m", pool.getMountDestPath());
+        }
+    }
 
     public String createHeartBeatCommand(HAStoragePool primaryStoragePool, String hostPrivateIp, boolean hostValidation) {
         Script cmd = new Script(primaryStoragePool.getPool().getHearthBeatPath(), HeartBeatUpdateTimeoutInMs, logger);
-        cmd.add("-i", primaryStoragePool.getPoolIp());
-        cmd.add("-p", primaryStoragePool.getPoolMountSourcePath());
-        cmd.add("-m", primaryStoragePool.getMountDestPath());
+        addPoolConnectionArgs(cmd, primaryStoragePool);
 
         if (hostValidation) {
             cmd.add("-h", hostPrivateIp);
@@ -377,9 +433,7 @@ public String getStorageNodeId() {
     public Boolean hasHeartBeat(HAStoragePool pool, HostTO host) {
         String hostIp = host.getPrivateNetwork().getIp();
         Script cmd = new Script(getHearthBeatPath(), HeartBeatCheckerTimeoutInMs, logger);
-        cmd.add("-i", pool.getPoolIp());
-        cmd.add("-p", pool.getPoolMountSourcePath());
-        cmd.add("-m", pool.getMountDestPath());
+        addPoolConnectionArgs(cmd, pool);
         cmd.add("-h", hostIp);
         cmd.add("-r");
         cmd.add("-t", String.valueOf(HeartBeatUpdateFreqInMs / 1000));
@@ -401,10 +455,12 @@ public Boolean hasHeartBeat(HAStoragePool pool, HostTO host) {
     @Override
     public Boolean hasVmActivity(HAStoragePool pool, HostTO host, Duration activityScriptTimeout, String volumeUUIDListString, String vmActivityCheckPath, long duration) {
         String hostIp = host.getPrivateNetwork().getIp();
-        Script cmd = new Script(vmActivityCheckPath, activityScriptTimeout.getStandardSeconds(), logger);
-        cmd.add("-i", pool.getPoolIp());
-        cmd.add("-p", pool.getPoolMountSourcePath());
-        cmd.add("-m", pool.getMountDestPath());
+        // RBD volumes have no shared mount point to stat(), so the RBD-specific script
+        // (using RBD watchers to detect activity) is used instead of the generic,
+        // NFS/SharedMountPoint-oriented script path passed in by the caller.
+        String scriptPath = StoragePoolType.RBD.equals(type) ? findKvmHaScript("kvmvmactivity_rbd.sh") : vmActivityCheckPath;
+        Script cmd = new Script(scriptPath, activityScriptTimeout.getStandardSeconds(), logger);
+        addPoolConnectionArgs(cmd, pool);
         cmd.add("-h", hostIp);
         cmd.add("-u", volumeUUIDListString);
         cmd.add("-t", String.valueOf(System.currentTimeMillis() / 1000));

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java

@@ -23,6 +23,8 @@
 import org.mockito.Mockito;
 
 import com.cloud.storage.Storage.StoragePoolType;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
 
 import junit.framework.TestCase;
 import org.mockito.junit.MockitoJUnitRunner;
@@ -98,4 +100,126 @@ public void testExternalSnapshot() {
         LibvirtStoragePool clvmPool = new LibvirtStoragePool(uuid, name, StoragePoolType.CLVM, adapter, storage);
         assertTrue(clvmPool.isExternalSnapshot());
     }
+
+    @Test
+    public void testIsPoolSupportHA() {
+        String uuid = "0f7a58bd-1a85-4b1f-9f91-12f3d1ecf5a5";
+        String name = "myfirstpool";
+
+        StorageAdaptor adapter = Mockito.mock(LibvirtStorageAdaptor.class);
+        StoragePool storage = Mockito.mock(StoragePool.class);
+
+        // NetworkFilesystem, SharedMountPoint and RBD all support the KVM Host-HA
+        // heartbeat/VM-activity check mechanism.
+        assertTrue(new LibvirtStoragePool(uuid, name, StoragePoolType.NetworkFilesystem, adapter, storage).isPoolSupportHA());
+        assertTrue(new LibvirtStoragePool(uuid, name, StoragePoolType.SharedMountPoint, adapter, storage).isPoolSupportHA());
+        assertTrue(new LibvirtStoragePool(uuid, name, StoragePoolType.RBD, adapter, storage).isPoolSupportHA());
+
+        // Other pool types have no HA support.
+        assertFalse(new LibvirtStoragePool(uuid, name, StoragePoolType.CLVM, adapter, storage).isPoolSupportHA());
+        assertFalse(new LibvirtStoragePool(uuid, name, StoragePoolType.Filesystem, adapter, storage).isPoolSupportHA());
+    }
+
+    private String getRbdMonitors(String sourceHost, int sourcePort) {
+        LibvirtStoragePool pool = new LibvirtStoragePool("0f7a58bd-1a85-4b1f-9f91-12f3d1ecf5a5", "myfirstpool", StoragePoolType.RBD,
+                Mockito.mock(LibvirtStorageAdaptor.class), Mockito.mock(StoragePool.class));
+        pool.setSourceHost(sourceHost);
+        pool.setSourcePort(sourcePort);
+        return pool.getRbdMonitors();
+    }
+
+    @Test
+    public void testRbdMonitorsWithoutPort() {
+        assertEquals("10.0.0.1", getRbdMonitors("10.0.0.1", 0));
+        assertEquals("10.0.0.1,10.0.0.2,10.0.0.3", getRbdMonitors("10.0.0.1,10.0.0.2,10.0.0.3", 0));
+        assertEquals("fd00::1,fd00::2", getRbdMonitors("fd00::1,fd00::2", 0));
+    }
+
+    @Test
+    public void testRbdMonitorsIpv4WithPort() {
+        assertEquals("10.0.0.1:6789", getRbdMonitors("10.0.0.1", 6789));
+        assertEquals("10.0.0.1:3300,10.0.0.2:3300,10.0.0.3:3300", getRbdMonitors("10.0.0.1,10.0.0.2,10.0.0.3", 3300));
+    }
+
+    @Test
+    public void testRbdMonitorsIpv6WithPort() {
+        assertEquals("[fd00::1]:3300", getRbdMonitors("fd00::1", 3300));
+        assertEquals("[fd00::1]:3300,[fd00::2]:3300", getRbdMonitors("fd00::1,fd00::2", 3300));
+        // already enclosed in square brackets
+        assertEquals("[fd00::1]:3300,[fd00::2]:3300", getRbdMonitors("[fd00::1],[fd00::2]", 3300));
+    }
+
+    @Test
+    public void testRbdMonitorsMixedIpv4AndIpv6WithPort() {
+        assertEquals("10.0.0.1:3300,[fd00::1]:3300,[fd00::2]:3300,mon4.example.com:3300",
+                getRbdMonitors("10.0.0.1, fd00::1,[fd00::2] ,mon4.example.com", 3300));
+    }
+
+    @Test
+    public void testRbdMonitorsMultipleIpv4AndIpv6WithoutPort() {
+        // no port: the default Ceph port is used, so the monitors are passed as they are
+        assertEquals("[fd00::1],[fd00::2],[fd00::3]", getRbdMonitors("[fd00::1],[fd00::2],[fd00::3]", 0));
+        assertEquals("10.0.0.1,10.0.0.2,fd00::1,[fd00::2]", getRbdMonitors("10.0.0.1,10.0.0.2,fd00::1,[fd00::2]", 0));
+        assertEquals("10.0.0.1,fd00::1", getRbdMonitors("10.0.0.1,fd00::1", -1));
+    }
+
+    @Test
+    public void testRbdMonitorsMultipleIpv4AndIpv6WithPort() {
+        assertEquals("10.0.0.1:6789,10.0.0.2:6789,[fd00::1]:6789,[fd00::2]:6789",
+                getRbdMonitors("10.0.0.1,10.0.0.2,fd00::1,[fd00::2]", 6789));
+        assertEquals("[fd00::1]:6789,10.0.0.1:6789,[fd00::2]:6789,10.0.0.2:6789",
+                getRbdMonitors("fd00::1,10.0.0.1,[fd00::2],10.0.0.2", 6789));
+    }
+
+    @Test
+    public void testRbdMonitorsAreTrimmedAndEmptyEntriesSkipped() {
+        assertEquals("10.0.0.1,10.0.0.2,fd00::1", getRbdMonitors(" 10.0.0.1, 10.0.0.2,,fd00::1 ,", 0));
+        assertEquals("10.0.0.1:6789,10.0.0.2:6789,[fd00::1]:6789", getRbdMonitors(" 10.0.0.1, 10.0.0.2,,fd00::1 ,", 6789));
+    }
+
+    private LibvirtStoragePool getRbdPool(String authUsername, String authSecret) {
+        LibvirtStoragePool pool = new LibvirtStoragePool("0f7a58bd-1a85-4b1f-9f91-12f3d1ecf5a5", "myfirstpool", StoragePoolType.RBD,
+                Mockito.mock(LibvirtStorageAdaptor.class), Mockito.mock(StoragePool.class));
+        pool.setSourceHost("10.0.0.1,fd00::1");
+        pool.setSourcePort(6789);
+        pool.setSourceDir("rbdpool");
+        pool.setAuthUsername(authUsername);
+        pool.setAuthSecret(authSecret);
+        return pool;
+    }
+
+    @Test
+    public void testAddRbdConnectionArgsWithCephx() {
+        Script cmd = Mockito.mock(Script.class);
+        getRbdPool("cephuser", "cephkey").addRbdConnectionArgs(cmd);
+        Mockito.verify(cmd).add("-s", "10.0.0.1:6789,[fd00::1]:6789");
+        Mockito.verify(cmd).add("-o", "rbdpool");
+        Mockito.verify(cmd).add("-n", "cephuser");
+        Mockito.verify(cmd).add("-k", "cephkey");
+        Mockito.verifyNoMoreInteractions(cmd);
+    }
+
+    @Test
+    public void testAddRbdConnectionArgsWithoutCephx() {
+        for (String[] noAuth : new String[][] {{null, null}, {"", ""}, {" ", null}}) {
+            Script cmd = Mockito.mock(Script.class);
+            getRbdPool(noAuth[0], noAuth[1]).addRbdConnectionArgs(cmd);
+            Mockito.verify(cmd).add("-s", "10.0.0.1:6789,[fd00::1]:6789");
+            Mockito.verify(cmd).add("-o", "rbdpool");
+            Mockito.verifyNoMoreInteractions(cmd);
+        }
+    }
+
+    @Test
+    public void testAddRbdConnectionArgsWithCephUserOnlyOrKeyOnly() {
+        for (String[] partial : new String[][] {{"cephuser", null}, {"cephuser", " "}, {null, "cephkey"}, {"", "cephkey"}}) {
+            Script cmd = Mockito.mock(Script.class);
+            try {
+                getRbdPool(partial[0], partial[1]).addRbdConnectionArgs(cmd);
+                fail("Expected a CloudRuntimeException for user [" + partial[0] + "] and key [" + partial[1] + "]");
+            } catch (CloudRuntimeException expected) {
+                Mockito.verifyNoInteractions(cmd);
+            }
+        }
+    }
 }