From 806fc22662251536d721496780cf1f5726ab2d5f Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 5 Oct 2026 12:11:48 +0200 Subject: [PATCH 1/9] kvm: support Host HA on Ceph RBD primary storage Extend the existing KVM Host-HA heartbeat/VM-activity-check framework (currently limited to NetworkFilesystem and SharedMountPoint pools) to also cover RBD primary storage, based on the approach from the old PR apache/cloudstack#5862, adapted to the current HA architecture and reusing the multi-monitor Ceph support already added in #6792. - Add StoragePoolType.RBD to LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT, which is the single switch that makes pool registration, KVMHAMonitor, and the CheckOnHostCommand/CheckVMActivityOnStoragePoolCommand wrappers treat RBD pools as HA-capable. - LibvirtStoragePool: build rbd/rados connection args (--mon-host, pool, and cephx --id/--key when set) from the pool's existing sourceHost/ sourceDir/authUsername/authSecret fields for the heartbeat and VM-activity checks, mirroring the conventions KVMPhysicalDisk already uses to talk to RBD. - Add kvmheartbeat_rbd.sh and kvmvmactivity_rbd.sh: RBD has no shared mount point to write a heartbeat file to, so the heartbeat timestamp is stored as a small RADOS object per host instead, and VM activity is detected via RBD watchers (rbd status) rather than file mtimes. - Minor: fix a stale "NFS storage pool" log message in KVMHAMonitor now that this path also runs for RBD. - Add LibvirtStoragePoolTest#testIsPoolSupportHA covering the new RBD case (no HA/heartbeat tests existed previously for any pool type). --- .../com/cloud/ha/HighAvailabilityManager.java | 3 +- .../hypervisor/kvm/resource/KVMHAMonitor.java | 2 +- .../kvm/storage/LibvirtStoragePool.java | 70 ++++--- .../kvm/storage/LibvirtStoragePoolTest.java | 19 ++ scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh | 143 ++++++++++++++ .../vm/hypervisor/kvm/kvmvmactivity_rbd.sh | 184 ++++++++++++++++++ 6 files changed, 397 insertions(+), 24 deletions(-) create mode 100755 scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh create mode 100755 scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh diff --git a/engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java b/engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java index 53bfcce27038..f4f8d342eda7 100644 --- a/engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java +++ b/engine/components-api/src/main/java/com/cloud/ha/HighAvailabilityManager.java @@ -33,7 +33,8 @@ */ public interface HighAvailabilityManager extends Manager { - List LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint); + List LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint, + StoragePoolType.RBD); ConfigKey ForceHA = new ConfigKey<>("Advanced", Boolean.class, "force.ha", "false", "Force High-Availability to happen even if the VM says no.", true, Cluster); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java index 9f1b849e9727..7a7662f8b72d 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java @@ -130,7 +130,7 @@ private void checkForNotExistingLibvirtStoragePools(Set 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); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java index a8c32baa6ef3..7053dc4e4d9b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java @@ -330,29 +330,55 @@ 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 heartbeat script '%s' in directory: %s", scriptName, kvmScriptsDir)); + } + return scriptPath; + } + + /** + * 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}). + */ + private void addRbdConnectionArgs(Script cmd) { + cmd.add("-s", sourceHost); + cmd.add("-o", sourceDir); + if (authUsername != null) { + 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 +403,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 +425,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)); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java index 88d4daa2dabc..e93f8177b881 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java @@ -98,4 +98,23 @@ 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()); + } } diff --git a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh new file mode 100755 index 000000000000..a3b50717590f --- /dev/null +++ b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh @@ -0,0 +1,143 @@ +#!/bin/bash +# 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. + +# Ceph RBD flavor of kvmheartbeat.sh/kvmsmpheartbeat.sh. +# +# There is no shared POSIX mount point to write a heartbeat file to when the +# primary storage pool is Ceph RBD, so the heartbeat timestamp is instead +# stored as a small RADOS object (one object per host) in the same RBD pool. +# Any host with a working path to the Ceph cluster can write/read this object, +# which gives the same semantics as the NFS/SharedMountPoint heartbeat file. + +help() { + printf "Usage: $0 + -s ceph monitor host(s), comma separated + -o ceph/rbd pool name + -n cephx auth user (optional) + -k cephx auth key, base64 (optional, required if -n is set) + -h host + -r write/read hb log + -c cleanup + -t interval between read hb log\n" + exit 1 +} +#set -x +MonHosts= +PoolName= +CephUser= +CephKey= +HostIP= +interval= +rflag=0 +cflag=0 + +while getopts 's:o:n:k:h:t:rc' OPTION +do + case $OPTION in + s) + MonHosts="$OPTARG" + ;; + o) + PoolName="$OPTARG" + ;; + n) + CephUser="$OPTARG" + ;; + k) + CephKey="$OPTARG" + ;; + h) + HostIP="$OPTARG" + ;; + r) + rflag=1 + ;; + t) + interval="$OPTARG" + ;; + c) + cflag=1 + ;; + *) + help + ;; + esac +done + +if [ -z "$MonHosts" ] || [ -z "$PoolName" ] +then + exit 1 +fi + +RadosOpts=(--mon-host "$MonHosts") +if [ -n "$CephUser" ] +then + RadosOpts+=(--id "$CephUser" --key "$CephKey") +fi + +hbObject="KVMHA-hb-$HostIP" + +write_hbLog() { + tmpFile=$(mktemp) + date +%s > "$tmpFile" + rados -p "$PoolName" "${RadosOpts[@]}" put "$hbObject" "$tmpFile" &> /dev/null + rc=$? + rm -f "$tmpFile" + return $rc +} + +check_hbLog() { + now=$(date +%s) + hb=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$hbObject" - 2> /dev/null) + if ! [[ "$hb" =~ ^[0-9]+$ ]] + then + # Either the RADOS object doesn't exist yet (host never wrote a heartbeat) + # or the Ceph cluster can't be reached right now. Either way we can't + # confirm the host is alive, so fail safe and report it as DEAD. + return 255 + fi + diff=$(expr $now - $hb) + if [ $diff -gt $interval ] + then + return $diff + fi + return 0 +} + +if [ "$rflag" == "1" ] +then + check_hbLog + diff=$? + if [ $diff == 0 ] + then + echo "=====> ALIVE <=====" + else + echo "=====> Considering host as DEAD because last write to RADOS object [$hbObject] in pool [$PoolName] was [$diff] seconds ago, but the max interval is [$interval] <======" + fi + exit 0 +elif [ "$cflag" == "1" ] +then + /usr/bin/logger -t heartbeat "kvmheartbeat_rbd.sh will reboot system because it was unable to write the heartbeat to the Ceph RBD storage." + sync & + sleep 5 + echo b > /proc/sysrq-trigger + exit $? +else + write_hbLog + exit $? +fi diff --git a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh new file mode 100755 index 000000000000..852fff14bf9f --- /dev/null +++ b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh @@ -0,0 +1,184 @@ +#!/bin/bash +# 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. + +# Ceph RBD flavor of kvmvmactivity.sh. +# +# On NFS/SharedMountPoint storage, VM disk activity is detected via the mtime +# of the volume files on the shared mount point. RBD volumes aren't files on +# a mount point, so instead activity is detected via RBD watchers: as long as +# qemu has an RBD image open (i.e. a VM using that volume is running +# somewhere), the image will have a live watcher. The most recent +# suspect-time/watcher-state is persisted as a RADOS object (per host) +# in place of the "ac-" file used by the NFS/SMP scripts. + +help() { + printf "Usage: $0 + -s ceph monitor host(s), comma separated + -o ceph/rbd pool name + -n cephx auth user (optional) + -k cephx auth key, base64 (optional, required if -n is set) + -h host + -u volume (rbd image) uuid list + -t time on ms + -d suspect time\n" + exit 1 +} + +#set -x + +MonHosts= +PoolName= +CephUser= +CephKey= +HostIP= +UUIDList= +MSTime= +SuspectTime= + +while getopts 's:o:n:k:h:u:t:d:' OPTION +do + case $OPTION in + s) + MonHosts="$OPTARG" + ;; + o) + PoolName="$OPTARG" + ;; + n) + CephUser="$OPTARG" + ;; + k) + CephKey="$OPTARG" + ;; + h) + HostIP="$OPTARG" + ;; + u) + UUIDList="$OPTARG" + ;; + t) + MSTime="$OPTARG" + ;; + d) + SuspectTime="$OPTARG" + ;; + *) + help + ;; + esac +done + +if [ -z "$MonHosts" ] || [ -z "$PoolName" ] +then + exit 2 +fi + +if [ -z "$SuspectTime" ] +then + exit 2 +fi + +RadosOpts=(--mon-host "$MonHosts") +RbdOpts=(--mon-host "$MonHosts") +if [ -n "$CephUser" ] +then + RadosOpts+=(--id "$CephUser" --key "$CephKey") + RbdOpts+=(--id "$CephUser" --key "$CephKey") +fi + +hbObject="KVMHA-hb-$HostIP" +acObject="KVMHA-ac-$HostIP" + +# First check: heartbeat object, same as kvmheartbeat_rbd.sh +now=$(date +%s) +hb=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$hbObject" - 2> /dev/null) +if [[ "$hb" =~ ^[0-9]+$ ]] +then + diff=$(expr $now - $hb) + if [ $diff -lt 61 ] + then + echo "=====> ALIVE <=====" + exit 0 + fi +fi + +if [ -z "$UUIDList" ] +then + echo "=====> Considering host as DEAD due to empty UUIDList <======" + exit 0 +fi + +# Second check: RBD watcher based disk activity check. +# If any of the host's volumes still has a live watcher, something (most +# likely qemu on the host being checked) is actively using it right now. +latestUpdateTime=0 +for image in ${UUIDList//,/ } +do + watcherCount=$(rbd status "$PoolName/$image" "${RbdOpts[@]}" --format json 2> /dev/null | \ + python3 -c 'import json,sys +try: + print(len(json.load(sys.stdin).get("watchers", []))) +except Exception: + print(0)' 2> /dev/null) + if [ -n "$watcherCount" ] && [ "$watcherCount" -gt 0 ] 2> /dev/null + then + latestUpdateTime=$now + break + fi +done + +if [ ! -z "$(rados -p "$PoolName" "${RadosOpts[@]}" stat "$acObject" 2> /dev/null)" ] +then + acTime=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$acObject" - 2> /dev/null) +else + acTime= +fi + +tmpFile=$(mktemp) +echo "$SuspectTime:$latestUpdateTime:$MSTime" > "$tmpFile" +rados -p "$PoolName" "${RadosOpts[@]}" put "$acObject" "$tmpFile" &> /dev/null +rm -f "$tmpFile" + +if [ -z "$acTime" ]; then + if [[ $latestUpdateTime -gt $SuspectTime ]]; then + echo "=====> ALIVE <=====" + else + echo "=====> Considering host as DEAD due to RADOS object [$acObject] did not exist and condition [latestUpdateTime -gt SuspectTime] has not been satisfied. <======" + fi +else + arrTime=(${acTime//:/ }) + lastSuspectTime=${arrTime[0]} + lastUpdateTime=${arrTime[1]} + + suspectTimeDiff=$(expr $SuspectTime - $lastSuspectTime) + if [[ $suspectTimeDiff -lt 0 ]]; then + if [[ $latestUpdateTime -gt $SuspectTime ]]; then + echo "=====> ALIVE <=====" + else + echo "=====> Considering host as DEAD due to RADOS object [$acObject] exists, condition [suspectTimeDiff -lt 0] was satisfied and [latestUpdateTime -gt SuspectTime] has not been satisfied. <======" + fi + else + if [[ $latestUpdateTime -gt $lastUpdateTime ]]; then + echo "=====> ALIVE <=====" + else + echo "=====> Considering host as DEAD due to RADOS object [$acObject] exists and conditions [suspectTimeDiff -lt 0] and [latestUpdateTime -gt SuspectTime] have not been satisfied. <======" + fi + fi +fi + +exit 0 From ebc71b7a011ccfadc8f30f64654c142d392b67c3 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 8 Oct 2026 11:18:47 +0200 Subject: [PATCH 2/9] kvm: address review comments on Host HA for Ceph RBD - pass the pool's monitor port to the RBD heartbeat/activity scripts, for every monitor, instead of relying on the default port - kvmheartbeat_rbd.sh: keep the heartbeat age out of the function return status, which wraps above 255 and made a long-stale heartbeat look ALIVE - remove pools from the HA monitor on deletion for all storage pool types with HA support, without trying to umount an empty mount path - make the missing-script error message generic - add unit tests for the RBD monitor list (IPv4, IPv6, mixed, with/without port) --- .../hypervisor/kvm/resource/KVMHAMonitor.java | 5 ++- .../kvm/storage/KVMStoragePoolManager.java | 5 +-- .../kvm/storage/LibvirtStoragePool.java | 24 +++++++++++-- .../kvm/storage/LibvirtStoragePoolTest.java | 35 +++++++++++++++++++ scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh | 19 +++++----- 5 files changed, 75 insertions(+), 13 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java index 7a7662f8b72d..17b403c49650 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java +++ b/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,9 @@ 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.runSimpleBashScript("umount " + pool.getMountDestPath()); + } haStoragePools.remove(uuid); } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java index d99847fd921a..c56171577e09 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java +++ b/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 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); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java index 7053dc4e4d9b..dec006bf888b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java @@ -17,6 +17,7 @@ package com.cloud.hypervisor.kvm.storage; import java.io.File; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -343,18 +344,37 @@ 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 heartbeat script '%s' in directory: %s", scriptName, kvmScriptsDir)); + 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, + * each with the pool's monitor port if one is set (IPv6 addresses are enclosed in square brackets). + */ + protected String getRbdMonitors() { + if (sourcePort <= 0) { + return sourceHost; + } + List monitors = new ArrayList<>(); + for (String monitor : sourceHost.split(",")) { + monitor = monitor.trim(); + if (monitor.contains(":") && !monitor.startsWith("[")) { + monitor = "[" + monitor + "]"; + } + monitors.add(monitor + ":" + sourcePort); + } + 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}). */ private void addRbdConnectionArgs(Script cmd) { - cmd.add("-s", sourceHost); + cmd.add("-s", getRbdMonitors()); cmd.add("-o", sourceDir); if (authUsername != null) { cmd.add("-n", authUsername); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java index e93f8177b881..2d18a62aed98 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java @@ -117,4 +117,39 @@ public void testIsPoolSupportHA() { 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)); + } } diff --git a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh index a3b50717590f..181194482ce4 100755 --- a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh +++ b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh @@ -109,25 +109,28 @@ check_hbLog() { # Either the RADOS object doesn't exist yet (host never wrote a heartbeat) # or the Ceph cluster can't be reached right now. Either way we can't # confirm the host is alive, so fail safe and report it as DEAD. - return 255 + hbAge= + return 1 fi - diff=$(expr $now - $hb) - if [ $diff -gt $interval ] + # the age is kept in a variable, not in the return status, as a status above 255 wraps around + hbAge=$(expr $now - $hb) + if [ $hbAge -gt $interval ] then - return $diff + return 1 fi return 0 } if [ "$rflag" == "1" ] then - check_hbLog - diff=$? - if [ $diff == 0 ] + if check_hbLog then echo "=====> ALIVE <=====" + elif [ -z "$hbAge" ] + then + echo "=====> Considering host as DEAD because RADOS object [$hbObject] in pool [$PoolName] could not be read <======" else - echo "=====> Considering host as DEAD because last write to RADOS object [$hbObject] in pool [$PoolName] was [$diff] seconds ago, but the max interval is [$interval] <======" + echo "=====> Considering host as DEAD because last write to RADOS object [$hbObject] in pool [$PoolName] was [$hbAge] seconds ago, but the max interval is [$interval] <======" fi exit 0 elif [ "$cflag" == "1" ] From a36604e64460b1467444099aafbda6acb9c29df4 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 8 Oct 2026 12:17:49 +0200 Subject: [PATCH 3/9] kvm: validate input of the RBD Host HA scripts and the Ceph settings of the pool Follow-up to the review of the Host HA on Ceph RBD support: - kvmheartbeat_rbd.sh and kvmvmactivity_rbd.sh: exit with an error when the host IP, which names the RADOS objects, is missing (the self-fencing -c of the heartbeat script does not need it), or when a Ceph user is given without its key - LibvirtStoragePool: trim the Ceph monitors and skip empty entries, with or without a port; pass the Ceph user and key only if both are set and fail with a clear error if only one of them is - KVMHAMonitor: run umount without a shell when removing a pool - kvmvmactivity_rbd.sh: fix the -t help text and drop the unused field from the stored activity state - add unit tests for the monitor list (multiple IPv4 and IPv6 monitors, with and without a port, with spaces) and for the Ceph user and key handling --- .../hypervisor/kvm/resource/KVMHAMonitor.java | 4 +- .../kvm/storage/LibvirtStoragePool.java | 30 +++++--- .../kvm/storage/LibvirtStoragePoolTest.java | 70 +++++++++++++++++++ scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh | 11 +++ .../vm/hypervisor/kvm/kvmvmactivity_rbd.sh | 18 +++-- 5 files changed, 118 insertions(+), 15 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java index 17b403c49650..46499a0c4c07 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java @@ -56,7 +56,9 @@ public void removeStoragePool(String uuid) { HAStoragePool pool = haStoragePools.get(uuid); if (pool != null) { if (StringUtils.isNotEmpty(pool.getMountDestPath())) { - Script.runSimpleBashScript("umount " + pool.getMountDestPath()); + Script umount = new Script("umount", logger); + umount.add(pool.getMountDestPath()); + umount.execute(); } haStoragePools.remove(uuid); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java index dec006bf888b..6b496b0742e3 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java @@ -22,6 +22,7 @@ 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; @@ -350,20 +351,24 @@ private String findKvmHaScript(String scriptName) { } /** - * Returns the Ceph monitors as expected by "--mon-host": the comma-separated monitors of the pool, - * each with the pool's monitor port if one is set (IPv6 addresses are enclosed in square brackets). + * 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() { - if (sourcePort <= 0) { - return sourceHost; - } List monitors = new ArrayList<>(); for (String monitor : sourceHost.split(",")) { monitor = monitor.trim(); - if (monitor.contains(":") && !monitor.startsWith("[")) { - monitor = "[" + monitor + "]"; + if (monitor.isEmpty()) { + continue; + } + if (sourcePort > 0) { + if (monitor.contains(":") && !monitor.startsWith("[")) { + monitor = "[" + monitor + "]"; + } + monitor = monitor + ":" + sourcePort; } - monitors.add(monitor + ":" + sourcePort); + monitors.add(monitor); } return String.join(",", monitors); } @@ -373,10 +378,15 @@ protected String getRbdMonitors() { * 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}). */ - private void addRbdConnectionArgs(Script cmd) { + 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 (authUsername != null) { + if (hasUser) { cmd.add("-n", authUsername); cmd.add("-k", authSecret); } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java index 2d18a62aed98..6a58d1c51f2a 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java +++ b/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; @@ -152,4 +154,72 @@ 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); + } + } + } } diff --git a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh index 181194482ce4..63c0c24ac204 100755 --- a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh +++ b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh @@ -84,6 +84,17 @@ then exit 1 fi +# the host IP names the heartbeat object, so it is required except for the self-fencing (-c) +if [ "$cflag" != "1" ] && [ -z "$HostIP" ] +then + exit 1 +fi + +if [ -n "$CephUser" ] && [ -z "$CephKey" ] +then + exit 1 +fi + RadosOpts=(--mon-host "$MonHosts") if [ -n "$CephUser" ] then diff --git a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh index 852fff14bf9f..06de909b5440 100755 --- a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh +++ b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh @@ -34,7 +34,7 @@ help() { -k cephx auth key, base64 (optional, required if -n is set) -h host -u volume (rbd image) uuid list - -t time on ms + -t current time in seconds (accepted for compatibility with kvmvmactivity.sh, not used) -d suspect time\n" exit 1 } @@ -47,7 +47,6 @@ CephUser= CephKey= HostIP= UUIDList= -MSTime= SuspectTime= while getopts 's:o:n:k:h:u:t:d:' OPTION @@ -72,7 +71,7 @@ do UUIDList="$OPTARG" ;; t) - MSTime="$OPTARG" + # not used, see help ;; d) SuspectTime="$OPTARG" @@ -93,6 +92,17 @@ then exit 2 fi +# the host IP names the heartbeat and activity objects +if [ -z "$HostIP" ] +then + exit 2 +fi + +if [ -n "$CephUser" ] && [ -z "$CephKey" ] +then + exit 2 +fi + RadosOpts=(--mon-host "$MonHosts") RbdOpts=(--mon-host "$MonHosts") if [ -n "$CephUser" ] @@ -150,7 +160,7 @@ else fi tmpFile=$(mktemp) -echo "$SuspectTime:$latestUpdateTime:$MSTime" > "$tmpFile" +echo "$SuspectTime:$latestUpdateTime" > "$tmpFile" rados -p "$PoolName" "${RadosOpts[@]}" put "$acObject" "$tmpFile" &> /dev/null rm -f "$tmpFile" From daacf3e91c09a7159214f9840c7ec03e711f2fb0 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 8 Oct 2026 14:23:03 +0200 Subject: [PATCH 4/9] kvm: keep the Ceph key out of the logs and process list of the RBD Host HA checks, and harden the parsing Follow-up to the review of the Host HA on Ceph RBD support: - LibvirtStoragePool: do not add a port to Ceph monitors which have one already, and only wrap bare IPv6 addresses in square brackets; mask the Ceph key in the logged command - kvmheartbeat_rbd.sh, kvmvmactivity_rbd.sh: give the Ceph key to rados and rbd in a temporary keyfile instead of --key - kvmheartbeat_rbd.sh: require -t to be a number when checking a heartbeat - kvmvmactivity_rbd.sh: read the list of volumes without word splitting or globbing, skipping empty entries, and check if the activity object exists by the exit status of rados stat - add unit tests for monitors with a port and for the masked key --- .../kvm/storage/LibvirtStoragePool.java | 31 +++++++++++++------ .../kvm/storage/LibvirtStoragePoolTest.java | 18 ++++++++++- scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh | 14 +++++++-- .../vm/hypervisor/kvm/kvmvmactivity_rbd.sh | 18 ++++++++--- 4 files changed, 64 insertions(+), 17 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java index 6b496b0742e3..d9b81acb4e06 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java @@ -352,8 +352,7 @@ private String findKvmHaScript(String scriptName) { /** * 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). + * and without empty entries. If the pool has a monitor port, it is added to each monitor which has none yet. */ protected String getRbdMonitors() { List monitors = new ArrayList<>(); @@ -362,17 +361,28 @@ protected String getRbdMonitors() { if (monitor.isEmpty()) { continue; } - if (sourcePort > 0) { - if (monitor.contains(":") && !monitor.startsWith("[")) { - monitor = "[" + monitor + "]"; - } - monitor = monitor + ":" + sourcePort; - } - monitors.add(monitor); + monitors.add(sourcePort > 0 ? addPortToRbdMonitor(monitor) : monitor); } return String.join(",", monitors); } + private String addPortToRbdMonitor(String monitor) { + if (monitor.startsWith("[")) { + // IPv6 address in square brackets, which has a port if followed by ":" + return monitor.contains("]:") ? monitor : monitor + ":" + sourcePort; + } + int colons = StringUtils.countMatches(monitor, ":"); + if (colons == 0) { + return monitor + ":" + sourcePort; + } + if (colons == 1) { + // IPv4 address or host name, with a port + return monitor; + } + // IPv6 address without square brackets, so without a port + return "[" + monitor + "]:" + sourcePort; + } + /** * 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" @@ -388,7 +398,8 @@ protected void addRbdConnectionArgs(Script cmd) { cmd.add("-o", sourceDir); if (hasUser) { cmd.add("-n", authUsername); - cmd.add("-k", authSecret); + cmd.add("-k"); + cmd.addSensitive(authSecret); } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java index 6a58d1c51f2a..91754733715f 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java @@ -195,7 +195,8 @@ public void testAddRbdConnectionArgsWithCephx() { 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.verify(cmd).add("-k"); + Mockito.verify(cmd).addSensitive("cephkey"); Mockito.verifyNoMoreInteractions(cmd); } @@ -222,4 +223,19 @@ public void testAddRbdConnectionArgsWithCephUserOnlyOrKeyOnly() { } } } + + @Test + public void testRbdMonitorsWhichHaveAPortAlready() { + // the monitors keep their port, with or without a port of the pool + String withPorts = "10.0.0.1:6789,mon.example.com:3300,[fd00::1]:6789"; + assertEquals(withPorts, getRbdMonitors(withPorts, 0)); + assertEquals(withPorts, getRbdMonitors(withPorts, 6789)); + assertEquals(withPorts, getRbdMonitors(withPorts, 3300)); + } + + @Test + public void testRbdMonitorsWithAndWithoutAPortAreMixed() { + assertEquals("10.0.0.1:6789,10.0.0.2:3300,mon.example.com:3300,[fd00::1]:3300,[fd00::2]:6789,[fd00::3]:3300", + getRbdMonitors("10.0.0.1:6789,10.0.0.2,mon.example.com,fd00::1,[fd00::2]:6789,[fd00::3]", 3300)); + } } diff --git a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh index 63c0c24ac204..5aa678c3652c 100755 --- a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh +++ b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh @@ -95,10 +95,20 @@ then exit 1 fi +# -t is the maximum age of the heartbeat, which is compared in a check +if [ "$rflag" == "1" ] && ! [[ "$interval" =~ ^[0-9]+$ ]] +then + exit 1 +fi + RadosOpts=(--mon-host "$MonHosts") -if [ -n "$CephUser" ] +if [ -n "$CephUser" ] && [ "$cflag" != "1" ] then - RadosOpts+=(--id "$CephUser" --key "$CephKey") + # the key is given to rados in a file, to keep it out of the process list + KeyFile=$(mktemp) + trap 'rm -f "$KeyFile"' EXIT + printf '%s' "$CephKey" > "$KeyFile" + RadosOpts+=(--id "$CephUser" --keyfile "$KeyFile") fi hbObject="KVMHA-hb-$HostIP" diff --git a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh index 06de909b5440..42eda4b20b80 100755 --- a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh +++ b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh @@ -107,8 +107,12 @@ RadosOpts=(--mon-host "$MonHosts") RbdOpts=(--mon-host "$MonHosts") if [ -n "$CephUser" ] then - RadosOpts+=(--id "$CephUser" --key "$CephKey") - RbdOpts+=(--id "$CephUser" --key "$CephKey") + # the key is given to rados and rbd in a file, to keep it out of the process list + KeyFile=$(mktemp) + trap 'rm -f "$KeyFile"' EXIT + printf '%s' "$CephKey" > "$KeyFile" + RadosOpts+=(--id "$CephUser" --keyfile "$KeyFile") + RbdOpts+=(--id "$CephUser" --keyfile "$KeyFile") fi hbObject="KVMHA-hb-$HostIP" @@ -137,8 +141,14 @@ fi # If any of the host's volumes still has a live watcher, something (most # likely qemu on the host being checked) is actively using it right now. latestUpdateTime=0 -for image in ${UUIDList//,/ } +IFS=',' read -ra images <<< "$UUIDList" +for image in "${images[@]}" do + image=${image//[[:space:]]/} + if [ -z "$image" ] + then + continue + fi watcherCount=$(rbd status "$PoolName/$image" "${RbdOpts[@]}" --format json 2> /dev/null | \ python3 -c 'import json,sys try: @@ -152,7 +162,7 @@ except Exception: fi done -if [ ! -z "$(rados -p "$PoolName" "${RadosOpts[@]}" stat "$acObject" 2> /dev/null)" ] +if rados -p "$PoolName" "${RadosOpts[@]}" stat "$acObject" &> /dev/null then acTime=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$acObject" - 2> /dev/null) else From 3d932fe1f6b7c8565fe11b557e5ccbfa817e3bd0 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 9 Oct 2026 14:27:18 +0200 Subject: [PATCH 5/9] kvmvmactivity_rbd.sh: do not depend on python3, and tell failures from a DEAD host Follow-up to the review of the Host HA on Ceph RBD support: - list the watchers of an image from the plain output of rbd status instead of parsing json with python3 - fail with exit status 2, without a DEAD verdict, when the watchers or the activity state can not be read, so that it is not taken for a dead host - name in the DEAD message why the heartbeat did not show a live host - ignore an activity state which is not in the expected format - kvmheartbeat_rbd.sh, kvmvmactivity_rbd.sh: the Ceph key is used as it is, not decoded from base64, as the help text now says --- scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh | 2 +- .../vm/hypervisor/kvm/kvmvmactivity_rbd.sh | 65 +++++++++++++------ 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh index 5aa678c3652c..c2ba1ceab91b 100755 --- a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh +++ b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh @@ -29,7 +29,7 @@ help() { -s ceph monitor host(s), comma separated -o ceph/rbd pool name -n cephx auth user (optional) - -k cephx auth key, base64 (optional, required if -n is set) + -k cephx auth key (optional, required if -n is set) -h host -r write/read hb log -c cleanup diff --git a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh index 42eda4b20b80..27c7397f5b39 100755 --- a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh +++ b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh @@ -31,7 +31,7 @@ help() { -s ceph monitor host(s), comma separated -o ceph/rbd pool name -n cephx auth user (optional) - -k cephx auth key, base64 (optional, required if -n is set) + -k cephx auth key (optional, required if -n is set) -h host -u volume (rbd image) uuid list -t current time in seconds (accepted for compatibility with kvmvmactivity.sh, not used) @@ -118,22 +118,36 @@ fi hbObject="KVMHA-hb-$HostIP" acObject="KVMHA-ac-$HostIP" +# the reason why the heartbeat did not show that the host is alive, added to the messages about a DEAD host +hbNote= + +dead() { + echo "=====> Considering host as DEAD due to $1.$hbNote <======" +} + # First check: heartbeat object, same as kvmheartbeat_rbd.sh now=$(date +%s) -hb=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$hbObject" - 2> /dev/null) -if [[ "$hb" =~ ^[0-9]+$ ]] +if hb=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$hbObject" - 2> /dev/null) then - diff=$(expr $now - $hb) - if [ $diff -lt 61 ] + if [[ "$hb" =~ ^[0-9]+$ ]] then - echo "=====> ALIVE <=====" - exit 0 + diff=$(expr $now - $hb) + if [ $diff -lt 61 ] + then + echo "=====> ALIVE <=====" + exit 0 + fi + hbNote=" The heartbeat in RADOS object [$hbObject] is [$diff] seconds old." + else + hbNote=" The RADOS object [$hbObject] does not hold a heartbeat." fi +else + hbNote=" The RADOS object [$hbObject] could not be read, it does not exist or Ceph can not be reached." fi if [ -z "$UUIDList" ] then - echo "=====> Considering host as DEAD due to empty UUIDList <======" + dead "empty UUIDList" exit 0 fi @@ -149,23 +163,34 @@ do then continue fi - watcherCount=$(rbd status "$PoolName/$image" "${RbdOpts[@]}" --format json 2> /dev/null | \ - python3 -c 'import json,sys -try: - print(len(json.load(sys.stdin).get("watchers", []))) -except Exception: - print(0)' 2> /dev/null) - if [ -n "$watcherCount" ] && [ "$watcherCount" -gt 0 ] 2> /dev/null + # when the watchers can not be listed, it is unknown whether the host is alive: the script fails, which + # is not a DEAD host + if ! watchers=$(rbd status "$PoolName/$image" "${RbdOpts[@]}" 2> /dev/null) + then + echo "=====> Unable to get the watchers of the image [$PoolName/$image] <======" + exit 2 + fi + # "Watchers: none", or a line "watcher=
client. cookie=" per watcher + if grep -q '^[[:space:]]*watcher=' <<< "$watchers" then latestUpdateTime=$now break fi done +acTime= if rados -p "$PoolName" "${RadosOpts[@]}" stat "$acObject" &> /dev/null then - acTime=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$acObject" - 2> /dev/null) -else + if ! acTime=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$acObject" - 2> /dev/null) + then + echo "=====> Unable to read the RADOS object [$acObject] <======" + exit 2 + fi +fi +# a state which is not ":" (with an optional third field of older versions) +# is ignored, as if there was none +if ! [[ "$acTime" =~ ^[0-9]+:[0-9]+(:[0-9]+)?$ ]] +then acTime= fi @@ -178,7 +203,7 @@ if [ -z "$acTime" ]; then if [[ $latestUpdateTime -gt $SuspectTime ]]; then echo "=====> ALIVE <=====" else - echo "=====> Considering host as DEAD due to RADOS object [$acObject] did not exist and condition [latestUpdateTime -gt SuspectTime] has not been satisfied. <======" + dead "RADOS object [$acObject] did not exist or holds no state, and condition [latestUpdateTime -gt SuspectTime] has not been satisfied" fi else arrTime=(${acTime//:/ }) @@ -190,13 +215,13 @@ else if [[ $latestUpdateTime -gt $SuspectTime ]]; then echo "=====> ALIVE <=====" else - echo "=====> Considering host as DEAD due to RADOS object [$acObject] exists, condition [suspectTimeDiff -lt 0] was satisfied and [latestUpdateTime -gt SuspectTime] has not been satisfied. <======" + dead "RADOS object [$acObject] exists, condition [suspectTimeDiff -lt 0] was satisfied and [latestUpdateTime -gt SuspectTime] has not been satisfied" fi else if [[ $latestUpdateTime -gt $lastUpdateTime ]]; then echo "=====> ALIVE <=====" else - echo "=====> Considering host as DEAD due to RADOS object [$acObject] exists and conditions [suspectTimeDiff -lt 0] and [latestUpdateTime -gt SuspectTime] have not been satisfied. <======" + dead "RADOS object [$acObject] exists and conditions [suspectTimeDiff -lt 0] and [latestUpdateTime -gt SuspectTime] have not been satisfied" fi fi fi From 4e6ca3494e68496e8b79b09e8864da95fef525d1 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 9 Oct 2026 16:32:52 +0200 Subject: [PATCH 6/9] kvmvmactivity_rbd.sh: do not decide on an activity state which could not be saved When the new activity state can not be written to the RADOS object, the next check would use an old state. Report it as not known whether the host is alive (exit status 2), as when the watchers can not be read. --- scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh index 27c7397f5b39..dc6740c0b130 100755 --- a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh +++ b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh @@ -197,7 +197,14 @@ fi tmpFile=$(mktemp) echo "$SuspectTime:$latestUpdateTime" > "$tmpFile" rados -p "$PoolName" "${RadosOpts[@]}" put "$acObject" "$tmpFile" &> /dev/null +putResult=$? rm -f "$tmpFile" +# without the new state the next check would use an old one, so it is not known whether the host is alive +if [ $putResult -ne 0 ] +then + echo "=====> Unable to write the RADOS object [$acObject] <======" + exit 2 +fi if [ -z "$acTime" ]; then if [[ $latestUpdateTime -gt $SuspectTime ]]; then From dc7d14a97ef5e5790364f03cc558b8ec4062cdda Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Tue, 13 Oct 2026 10:42:25 +0200 Subject: [PATCH 7/9] kvm: rewrite the RBD Host HA scripts in Python, with a faster VM-activity check Follow-up to the review of the Host HA on Ceph RBD support: - kvmheartbeat_rbd.sh and kvmvmactivity_rbd.sh are now kvmheartbeat_rbd.py and kvmvmactivity_rbd.py, with the same options, output and exit status - the Ceph key is no longer an argument of the scripts: the agent gives it in the environment variable CEPH_KEY, and the scripts give it to rados and rbd in CEPH_ARGS, so it is not in the process list and no keyfile is needed; the heartbeat and the activity state are given to rados on standard input instead of temporary files - kvmvmactivity_rbd.py lists the watchers of the volumes over one connection with the Python bindings of librados and librbd (16 s instead of 97 s for 1000 volumes without a watcher), and runs rbd status for each volume if the bindings are not installed or can not be used - for a RBD pool, the volumes are given to kvmvmactivity_rbd.py in a temporary file with the new option -U, as they can be more than the 128 KiB which an argument of a command can have (about 3500 volumes), and as the whole command is written to the agent log at each check - the warnings of a failed heartbeat or VM-activity check no longer say that it may shut the host down: it is considered dead, which may trigger HA actions, such as fencing it and restarting its VMs on other hosts - add unit tests for the environment of the scripts, the Ceph user and key and the file with the volumes --- .../hypervisor/kvm/resource/KVMHAChecker.java | 2 +- .../kvm/storage/LibvirtStoragePool.java | 73 ++++- .../kvm/storage/LibvirtStoragePoolTest.java | 64 +++- scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.py | 133 +++++++++ scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh | 167 ----------- .../vm/hypervisor/kvm/kvmvmactivity_rbd.py | 282 ++++++++++++++++++ .../vm/hypervisor/kvm/kvmvmactivity_rbd.sh | 236 --------------- 7 files changed, 539 insertions(+), 418 deletions(-) create mode 100755 scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.py delete mode 100755 scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh create mode 100755 scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.py delete mode 100755 scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAChecker.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAChecker.java index 0ee59c95da39..08b5851d83f6 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAChecker.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KVMHAChecker.java @@ -53,7 +53,7 @@ public Boolean hasHeartBeat() { } if (!heartBeatCheckResult) { - logger.warn("All checks with KVMHAChecker for {} considered it as dead. It may cause a shutdown of the host.", hostAndPools); + logger.warn("All checks with KVMHAChecker for {} considered it as dead, which may trigger HA actions, such as fencing it and restarting its VMs on other hosts.", hostAndPools); } return heartBeatCheckResult; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java index d9b81acb4e06..a1534be160d6 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java @@ -17,6 +17,9 @@ package com.cloud.hypervisor.kvm.storage; import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -42,6 +45,9 @@ import com.cloud.utils.script.Script; public class LibvirtStoragePool implements KVMStoragePool { + /** The environment variable which gives the Ceph key to the RBD heartbeat and VM-activity scripts. */ + protected static final String RBD_KEY_ENVIRONMENT_VARIABLE = "CEPH_KEY"; + protected Logger logger = LogManager.getLogger(getClass()); protected String uuid; protected long capacity; @@ -336,7 +342,7 @@ public String getHearthBeatPath() { } else if (StoragePoolType.SharedMountPoint.equals(type)) { return findKvmHaScript("kvmsmpheartbeat.sh"); } else if (StoragePoolType.RBD.equals(type)) { - return findKvmHaScript("kvmheartbeat_rbd.sh"); + return findKvmHaScript("kvmheartbeat_rbd.py"); } return null; } @@ -384,8 +390,19 @@ private String addPortToRbdMonitor(String monitor) { } /** - * 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" + * Returns the environment of a heartbeat/VM-activity check script: for a RBD storage pool with cephx, the + * Ceph key, which is not an argument of the script as it would be visible in the process list. + */ + protected Map getHeartBeatScriptEnvironment() { + if (StoragePoolType.RBD.equals(type) && StringUtils.isNotBlank(authSecret)) { + return Map.of(RBD_KEY_ENVIRONMENT_VARIABLE, authSecret); + } + return null; + } + + /** + * Adds the Ceph cluster connection details (monitors, pool and, if cephx is enabled, the user; the key is in + * {@link #getHeartBeatScriptEnvironment()}) 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) { @@ -398,8 +415,6 @@ protected void addRbdConnectionArgs(Script cmd) { cmd.add("-o", sourceDir); if (hasUser) { cmd.add("-n", authUsername); - cmd.add("-k"); - cmd.addSensitive(authSecret); } } @@ -417,6 +432,33 @@ private void addPoolConnectionArgs(Script cmd, HAStoragePool pool) { } } + /** + * Adds the volumes of the host to a VM-activity check {@link Script}. For a RBD storage pool they are given in + * a file, as they can be too many for an argument of a command (the limit is 128 KiB, about 3500 volumes), + * which would also be written to the log, with all the volumes, at each check. Otherwise, or if the file can not + * be written, they are an argument. + * + * @return the file, which has to be deleted after the script ran, or null if there is none + */ + protected File addVolumeListArgs(Script cmd, String volumes) { + if (StoragePoolType.RBD.equals(type)) { + File volumeListFile = null; + try { + volumeListFile = File.createTempFile("kvmha-volumes-", ".txt"); + Files.write(volumeListFile.toPath(), volumes.getBytes(StandardCharsets.UTF_8)); + cmd.add("-U", volumeListFile.getAbsolutePath()); + return volumeListFile; + } catch (IOException e) { + logger.warn("Unable to write the list of volumes to a file, giving it to the script as an argument: {}", e.getMessage()); + if (volumeListFile != null && !volumeListFile.delete()) { + logger.debug("Unable to delete the file {}", volumeListFile); + } + } + } + cmd.add("-u", volumes); + return null; + } + public String createHeartBeatCommand(HAStoragePool primaryStoragePool, String hostPrivateIp, boolean hostValidation) { Script cmd = new Script(primaryStoragePool.getPool().getHearthBeatPath(), HeartBeatUpdateTimeoutInMs, logger); addPoolConnectionArgs(cmd, primaryStoragePool); @@ -427,7 +469,7 @@ public String createHeartBeatCommand(HAStoragePool primaryStoragePool, String ho cmd.add("-c"); } - return cmd.execute(); + return cmd.execute(new OutputInterpreter.OutputLogger(logger), getHeartBeatScriptEnvironment()); } @Override @@ -449,13 +491,13 @@ public Boolean hasHeartBeat(HAStoragePool pool, HostTO host) { cmd.add("-r"); cmd.add("-t", String.valueOf(HeartBeatUpdateFreqInMs / 1000)); OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser(); - String result = cmd.execute(parser); + String result = cmd.execute(parser, getHeartBeatScriptEnvironment()); String parsedLine = parser.getLine(); logger.debug("Checking heart beat for host IP {} with KVMHAChecker [{command=\"{}\", result: \"{}\", log: \"{}\", pool: \"{}\"}].", hostIp, cmd.toString(), result, parsedLine, pool.getPoolIp()); if (result == null && parsedLine.contains("DEAD")) { - logger.warn("Checking heart beat for host IP {} with KVMHAChecker command [{}] returned [{}]. It may cause a shutdown of the host.", hostIp, cmd.toString(), parsedLine); + logger.warn("Checking heart beat for host IP {} with KVMHAChecker command [{}] returned [{}]. The host is considered dead, which may trigger HA actions, such as fencing it and restarting its VMs on other hosts.", hostIp, cmd.toString(), parsedLine); return false; } else { logger.debug("Checking heart beat for host IP {} with KVMHAChecker command [{}] succeeded.", hostIp, cmd.toString()); @@ -469,22 +511,29 @@ public Boolean hasVmActivity(HAStoragePool pool, HostTO host, Duration activityS // 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; + String scriptPath = StoragePoolType.RBD.equals(type) ? findKvmHaScript("kvmvmactivity_rbd.py") : vmActivityCheckPath; Script cmd = new Script(scriptPath, activityScriptTimeout.getStandardSeconds(), logger); addPoolConnectionArgs(cmd, pool); cmd.add("-h", hostIp); - cmd.add("-u", volumeUUIDListString); + File volumeListFile = addVolumeListArgs(cmd, volumeUUIDListString); cmd.add("-t", String.valueOf(System.currentTimeMillis() / 1000)); cmd.add("-d", String.valueOf(duration)); OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser(); - String result = cmd.execute(parser); + String result; + try { + result = cmd.execute(parser, getHeartBeatScriptEnvironment()); + } finally { + if (volumeListFile != null && !volumeListFile.delete()) { + logger.debug("Unable to delete the file {}", volumeListFile); + } + } String parsedLine = parser.getLine(); logger.debug("Checking VM activity for host IP {} with KVMHAVMActivityChecker [{command=\"{}\", result: \"{}\", log: \"{}\", pool: \"{}\"}].", hostIp, cmd.toString(), result, parsedLine, pool.getPoolIp()); if (result == null && parsedLine.contains("DEAD")) { - logger.warn("Checking VM activity for host IP {} with KVMHAVMActivityChecker command [{}] returned [{}]. It may cause a shutdown of the host.", hostIp, cmd.toString(), parsedLine); + logger.warn("Checking VM activity for host IP {} with KVMHAVMActivityChecker command [{}] returned [{}]. The host is considered dead, which may trigger HA actions, such as fencing it and restarting its VMs on other hosts.", hostIp, cmd.toString(), parsedLine); return false; } else { logger.debug("Checking VM activity for host IP {} with KVMHAVMActivityChecker command [{}] succeeded.", hostIp, cmd.toString()); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java index 91754733715f..25c2329ae9bd 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePoolTest.java @@ -195,8 +195,7 @@ public void testAddRbdConnectionArgsWithCephx() { 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"); - Mockito.verify(cmd).addSensitive("cephkey"); + // the key is not an argument of the script, it would be visible in the process list Mockito.verifyNoMoreInteractions(cmd); } @@ -238,4 +237,65 @@ public void testRbdMonitorsWithAndWithoutAPortAreMixed() { assertEquals("10.0.0.1:6789,10.0.0.2:3300,mon.example.com:3300,[fd00::1]:3300,[fd00::2]:6789,[fd00::3]:3300", getRbdMonitors("10.0.0.1:6789,10.0.0.2,mon.example.com,fd00::1,[fd00::2]:6789,[fd00::3]", 3300)); } + + @Test + public void testHeartBeatScriptEnvironmentHasTheCephKey() { + assertEquals(java.util.Map.of("CEPH_KEY", "cephkey"), getRbdPool("cephuser", "cephkey").getHeartBeatScriptEnvironment()); + } + + @Test + public void testHeartBeatScriptEnvironmentWithoutCephx() { + assertNull(getRbdPool(null, null).getHeartBeatScriptEnvironment()); + assertNull(getRbdPool("", " ").getHeartBeatScriptEnvironment()); + } + + @Test + public void testHeartBeatScriptEnvironmentOfOtherPoolTypes() { + LibvirtStoragePool pool = new LibvirtStoragePool("0f7a58bd-1a85-4b1f-9f91-12f3d1ecf5a5", "myfirstpool", StoragePoolType.NetworkFilesystem, + Mockito.mock(LibvirtStorageAdaptor.class), Mockito.mock(StoragePool.class)); + pool.setAuthSecret("cephkey"); + assertNull(pool.getHeartBeatScriptEnvironment()); + } + + @Test + public void testVolumeListOfRbdPoolIsGivenInAFile() throws java.io.IOException { + Script cmd = Mockito.mock(Script.class); + String volumes = "uuid-1,uuid-2,uuid-3"; + java.io.File file = getRbdPool("cephuser", "cephkey").addVolumeListArgs(cmd, volumes); + try { + assertNotNull(file); + assertEquals(volumes, new String(java.nio.file.Files.readAllBytes(file.toPath()), java.nio.charset.StandardCharsets.UTF_8)); + Mockito.verify(cmd).add("-U", file.getAbsolutePath()); + Mockito.verifyNoMoreInteractions(cmd); + } finally { + if (file != null) { + file.delete(); + } + } + } + + @Test + public void testVolumeListOfRbdPoolCanBeVeryLong() throws java.io.IOException { + // more than what an argument of a command can have (128 KiB) + String volumes = String.join(",", java.util.Collections.nCopies(10000, "6d93a798-9fbe-41e2-8b69-fc6b5782f89a")); + java.io.File file = getRbdPool("cephuser", "cephkey").addVolumeListArgs(Mockito.mock(Script.class), volumes); + try { + assertNotNull(file); + assertEquals(volumes.length(), file.length()); + } finally { + if (file != null) { + file.delete(); + } + } + } + + @Test + public void testVolumeListOfOtherPoolTypesIsAnArgument() { + Script cmd = Mockito.mock(Script.class); + LibvirtStoragePool pool = new LibvirtStoragePool("0f7a58bd-1a85-4b1f-9f91-12f3d1ecf5a5", "myfirstpool", StoragePoolType.NetworkFilesystem, + Mockito.mock(LibvirtStorageAdaptor.class), Mockito.mock(StoragePool.class)); + assertNull(pool.addVolumeListArgs(cmd, "uuid-1,uuid-2")); + Mockito.verify(cmd).add("-u", "uuid-1,uuid-2"); + Mockito.verifyNoMoreInteractions(cmd); + } } diff --git a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.py b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.py new file mode 100755 index 000000000000..2192d072d108 --- /dev/null +++ b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.py @@ -0,0 +1,133 @@ +#!/usr/bin/env python3 +# 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. + +# Ceph RBD flavor of kvmheartbeat.sh/kvmsmpheartbeat.sh. +# +# There is no shared POSIX mount point to write a heartbeat file to when the +# primary storage pool is Ceph RBD, so the heartbeat timestamp is instead +# stored as a small RADOS object (one object per host) in the same RBD pool. +# Any host with a working path to the Ceph cluster can write/read this object, +# which gives the same semantics as the NFS/SharedMountPoint heartbeat file. +# +# The cephx key is not an argument, as it would be visible in the process list: +# it is read from the environment variable CEPH_KEY, and is given to rados in +# the environment variable CEPH_ARGS. + +import argparse +import os +import re +import subprocess +import sys +import time + +KEY_ENV = "CEPH_KEY" + + +class Parser(argparse.ArgumentParser): + def error(self, message): + self.print_usage(sys.stderr) + sys.exit(1) + + +def rados(args, env, data=None): + """Runs rados and returns its exit status and its output.""" + try: + result = subprocess.run(["rados"] + args, input=data, stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, env=env, universal_newlines=True) + except OSError: + return 127, "" + return result.returncode, result.stdout + + +def main(): + parser = Parser(description="Writes or checks the heartbeat of a KVM host on Ceph RBD storage", add_help=False) + parser.add_argument("-s", dest="mon_hosts", default="", help="ceph monitor host(s), comma separated") + parser.add_argument("-o", dest="pool", default="", help="ceph/rbd pool name") + parser.add_argument("-n", dest="user", default="", help="cephx auth user (optional, the key is then in " + KEY_ENV + ")") + parser.add_argument("-h", dest="host", default="", help="host") + parser.add_argument("-r", dest="check", action="store_true", help="read the heartbeat, instead of writing it") + parser.add_argument("-c", dest="cleanup", action="store_true", help="cleanup: reboot the host") + parser.add_argument("-t", dest="interval", default="", help="maximum age of the heartbeat in seconds, when it is read") + parser.add_argument("--help", action="help") + args = parser.parse_args() + + key = os.environ.pop(KEY_ENV, "") + if not args.mon_hosts or not args.pool: + sys.exit(1) + # the host IP names the heartbeat object, so it is required except for the self-fencing (-c) + if not args.cleanup and not args.host: + sys.exit(1) + if args.user and not key: + sys.exit(1) + if args.check and not re.fullmatch(r"[0-9]+", args.interval): + sys.exit(1) + + if args.cleanup: + print_and_reboot() + return + + env = dict(os.environ) + options = ["-p", args.pool, "--mon-host", args.mon_hosts] + if args.user: + options += ["--id", args.user] + env["CEPH_ARGS"] = (env.get("CEPH_ARGS", "") + " --key=" + key).strip() + + heartbeat_object = "KVMHA-hb-" + args.host + + if args.check: + sys.exit(check(options, env, heartbeat_object, args.pool, int(args.interval))) + + status, _ = rados(options + ["put", heartbeat_object, "-"], env, "%d\n" % time.time()) + sys.exit(0 if status == 0 else 1) + + +def check(options, env, heartbeat_object, pool, interval): + now = int(time.time()) + status, output = rados(options + ["get", heartbeat_object, "-"], env) + heartbeat = output.strip() + if not re.fullmatch(r"[0-9]+", heartbeat): + # Either the RADOS object doesn't exist yet (host never wrote a heartbeat) + # or the Ceph cluster can't be reached right now. Either way we can't + # confirm the host is alive, so fail safe and report it as DEAD. + print("=====> Considering host as DEAD because RADOS object [%s] in pool [%s] could not be read <======" + % (heartbeat_object, pool)) + return 0 + age = now - int(heartbeat) + if age > interval: + print("=====> Considering host as DEAD because last write to RADOS object [%s] in pool [%s] was [%d] seconds ago, " + "but the max interval is [%d] <======" % (heartbeat_object, pool, age, interval)) + else: + print("=====> ALIVE <=====") + return 0 + + +def print_and_reboot(): + subprocess.call(["/usr/bin/logger", "-t", "heartbeat", + "kvmheartbeat_rbd.py will reboot system because it was unable to write the heartbeat to the Ceph RBD storage."]) + # not waited for, the storage may be hanging + subprocess.Popen(["sync"]) + time.sleep(5) + try: + with open("/proc/sysrq-trigger", "w") as trigger: + trigger.write("b") + except OSError: + sys.exit(1) + + +if __name__ == "__main__": + main() diff --git a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh b/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh deleted file mode 100755 index c2ba1ceab91b..000000000000 --- a/scripts/vm/hypervisor/kvm/kvmheartbeat_rbd.sh +++ /dev/null @@ -1,167 +0,0 @@ -#!/bin/bash -# 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. - -# Ceph RBD flavor of kvmheartbeat.sh/kvmsmpheartbeat.sh. -# -# There is no shared POSIX mount point to write a heartbeat file to when the -# primary storage pool is Ceph RBD, so the heartbeat timestamp is instead -# stored as a small RADOS object (one object per host) in the same RBD pool. -# Any host with a working path to the Ceph cluster can write/read this object, -# which gives the same semantics as the NFS/SharedMountPoint heartbeat file. - -help() { - printf "Usage: $0 - -s ceph monitor host(s), comma separated - -o ceph/rbd pool name - -n cephx auth user (optional) - -k cephx auth key (optional, required if -n is set) - -h host - -r write/read hb log - -c cleanup - -t interval between read hb log\n" - exit 1 -} -#set -x -MonHosts= -PoolName= -CephUser= -CephKey= -HostIP= -interval= -rflag=0 -cflag=0 - -while getopts 's:o:n:k:h:t:rc' OPTION -do - case $OPTION in - s) - MonHosts="$OPTARG" - ;; - o) - PoolName="$OPTARG" - ;; - n) - CephUser="$OPTARG" - ;; - k) - CephKey="$OPTARG" - ;; - h) - HostIP="$OPTARG" - ;; - r) - rflag=1 - ;; - t) - interval="$OPTARG" - ;; - c) - cflag=1 - ;; - *) - help - ;; - esac -done - -if [ -z "$MonHosts" ] || [ -z "$PoolName" ] -then - exit 1 -fi - -# the host IP names the heartbeat object, so it is required except for the self-fencing (-c) -if [ "$cflag" != "1" ] && [ -z "$HostIP" ] -then - exit 1 -fi - -if [ -n "$CephUser" ] && [ -z "$CephKey" ] -then - exit 1 -fi - -# -t is the maximum age of the heartbeat, which is compared in a check -if [ "$rflag" == "1" ] && ! [[ "$interval" =~ ^[0-9]+$ ]] -then - exit 1 -fi - -RadosOpts=(--mon-host "$MonHosts") -if [ -n "$CephUser" ] && [ "$cflag" != "1" ] -then - # the key is given to rados in a file, to keep it out of the process list - KeyFile=$(mktemp) - trap 'rm -f "$KeyFile"' EXIT - printf '%s' "$CephKey" > "$KeyFile" - RadosOpts+=(--id "$CephUser" --keyfile "$KeyFile") -fi - -hbObject="KVMHA-hb-$HostIP" - -write_hbLog() { - tmpFile=$(mktemp) - date +%s > "$tmpFile" - rados -p "$PoolName" "${RadosOpts[@]}" put "$hbObject" "$tmpFile" &> /dev/null - rc=$? - rm -f "$tmpFile" - return $rc -} - -check_hbLog() { - now=$(date +%s) - hb=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$hbObject" - 2> /dev/null) - if ! [[ "$hb" =~ ^[0-9]+$ ]] - then - # Either the RADOS object doesn't exist yet (host never wrote a heartbeat) - # or the Ceph cluster can't be reached right now. Either way we can't - # confirm the host is alive, so fail safe and report it as DEAD. - hbAge= - return 1 - fi - # the age is kept in a variable, not in the return status, as a status above 255 wraps around - hbAge=$(expr $now - $hb) - if [ $hbAge -gt $interval ] - then - return 1 - fi - return 0 -} - -if [ "$rflag" == "1" ] -then - if check_hbLog - then - echo "=====> ALIVE <=====" - elif [ -z "$hbAge" ] - then - echo "=====> Considering host as DEAD because RADOS object [$hbObject] in pool [$PoolName] could not be read <======" - else - echo "=====> Considering host as DEAD because last write to RADOS object [$hbObject] in pool [$PoolName] was [$hbAge] seconds ago, but the max interval is [$interval] <======" - fi - exit 0 -elif [ "$cflag" == "1" ] -then - /usr/bin/logger -t heartbeat "kvmheartbeat_rbd.sh will reboot system because it was unable to write the heartbeat to the Ceph RBD storage." - sync & - sleep 5 - echo b > /proc/sysrq-trigger - exit $? -else - write_hbLog - exit $? -fi diff --git a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.py b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.py new file mode 100755 index 000000000000..d65ca265a3d9 --- /dev/null +++ b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.py @@ -0,0 +1,282 @@ +#!/usr/bin/env python3 +# 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. + +# Ceph RBD flavor of kvmvmactivity.sh. +# +# On NFS/SharedMountPoint storage, VM disk activity is detected via the mtime +# of the volume files on the shared mount point. RBD volumes aren't files on +# a mount point, so instead activity is detected via RBD watchers: as long as +# qemu has an RBD image open (i.e. a VM using that volume is running +# somewhere), the image will have a live watcher. The most recent +# suspect-time/watcher-state is persisted as a RADOS object (per host) +# in place of the "ac-" file used by the NFS/SMP scripts. +# +# The volumes are given in -u, or in a file with -U, which has no limit on the number +# of volumes: one argument of a command is limited to 128 KiB, which is about 3500 +# volumes. The file has the uuids separated by commas or by lines. +# +# The cephx key is not an argument, as it would be visible in the process list: +# it is read from the environment variable CEPH_KEY, and is given to rados and +# rbd in the environment variable CEPH_ARGS. +# +# The watchers of the volumes are listed over one connection to the Ceph cluster +# with the Python bindings of librados and librbd (python3-rados and python3-rbd), +# which is much faster than "rbd status" for each volume when a host has many +# volumes. If the bindings are not installed or can not be used, "rbd status" is +# run for each volume instead. +# +# Exit status: 0 with ALIVE or DEAD in the output, 2 if the arguments are not +# valid or if the state of the host could not be found out, which is not a DEAD host. + +import argparse +import os +import re +import subprocess +import sys +import time + +try: + import rados + import rbd +except ImportError: + rados = None + rbd = None + +KEY_ENV = "CEPH_KEY" +CONNECT_TIMEOUT = 60 + + +class Parser(argparse.ArgumentParser): + def error(self, message): + self.print_usage(sys.stderr) + sys.exit(1) + + +def ceph(tool, args, env, data=None): + """Runs rados or rbd and returns its exit status and its output.""" + try: + result = subprocess.run([tool] + args, input=data, stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, env=env, universal_newlines=True) + except OSError: + return 127, "" + return result.returncode, result.stdout + + +class WatchersError(Exception): + """The watchers of a volume could not be listed: it is not known whether the host is alive.""" + + +class BindingsWatchers: + """Lists the watchers of volumes over one connection, with the Python bindings of librados and librbd.""" + + def __init__(self, mon_hosts, pool, user, key): + if rados is None or rbd is None or not hasattr(rbd.Image, "watchers_list"): + raise RuntimeError("the Python bindings of librados and librbd can not be used") + conf = {"mon_host": mon_hosts} + if key: + conf["key"] = key + self.cluster = rados.Rados(rados_id=user if user else None, conf=conf) + self.ioctx = None + try: + self.cluster.connect(timeout=CONNECT_TIMEOUT) + self.ioctx = self.cluster.open_ioctx(pool) + except Exception: + self.close() + raise + + def has_watcher(self, image): + try: + with rbd.Image(self.ioctx, image, read_only=True) as volume: + return any(True for _ in volume.watchers_list()) + except (rados.Error, rbd.Error): + raise WatchersError(image) + + def close(self): + if self.ioctx is not None: + self.ioctx.close() + self.cluster.shutdown() + + +class CommandWatchers: + """Lists the watchers of a volume with "rbd status", which is run for each volume.""" + + def __init__(self, pool, options, env): + self.pool = pool + self.options = options + self.env = env + + def has_watcher(self, image): + status, watchers = ceph("rbd", ["status", self.pool + "/" + image] + self.options, self.env) + if status != 0: + raise WatchersError(image) + # "Watchers: none", or a line "watcher=
client. cookie=" per watcher + return re.search(r"^\s*watcher=", watchers, re.MULTILINE) is not None + + def close(self): + pass + + +def find_watcher(images, pool, mon_hosts, user, key, options, env): + """Returns True if one of the volumes has a watcher, raises WatchersError if it is not known.""" + try: + watchers = BindingsWatchers(mon_hosts, pool, user, key) + except Exception: + watchers = None + if watchers is not None: + try: + return any(watchers.has_watcher(image) for image in images) + except WatchersError: + raise + except Exception: + # not the volume, but the bindings: use the command instead + pass + finally: + watchers.close() + watchers = CommandWatchers(pool, options, env) + return any(watchers.has_watcher(image) for image in images) + + +def alive(): + print("=====> ALIVE <=====") + sys.exit(0) + + +def fail(message): + print("=====> %s <======" % message) + sys.exit(2) + + +def main(): + parser = Parser(description="Checks the VM activity of a KVM host on Ceph RBD storage", add_help=False) + parser.add_argument("-s", dest="mon_hosts", default="", help="ceph monitor host(s), comma separated") + parser.add_argument("-o", dest="pool", default="", help="ceph/rbd pool name") + parser.add_argument("-n", dest="user", default="", help="cephx auth user (optional, the key is then in " + KEY_ENV + ")") + parser.add_argument("-h", dest="host", default="", help="host") + parser.add_argument("-u", dest="images", default="", help="volume (rbd image) uuid list, separated by commas") + parser.add_argument("-U", dest="images_file", default="", + help="file with the volume (rbd image) uuid list, separated by commas or by lines, instead of -u") + parser.add_argument("-t", dest="time", default="", + help="current time in seconds (accepted for compatibility with kvmvmactivity.sh, not used)") + parser.add_argument("-d", dest="suspect_time", default="", help="suspect time") + parser.add_argument("--help", action="help") + args = parser.parse_args() + + key = os.environ.pop(KEY_ENV, "") + if not args.mon_hosts or not args.pool or not args.suspect_time: + sys.exit(2) + # the host IP names the heartbeat and activity objects + if not args.host: + sys.exit(2) + if args.user and not key: + sys.exit(2) + if not re.fullmatch(r"[0-9]+", args.suspect_time): + sys.exit(2) + suspect_time = int(args.suspect_time) + volumes = args.images + if args.images_file: + try: + with open(args.images_file) as images_file: + volumes = images_file.read() + except OSError: + sys.exit(2) + + env = dict(os.environ) + options = ["--mon-host", args.mon_hosts] + if args.user: + options += ["--id", args.user] + env["CEPH_ARGS"] = (env.get("CEPH_ARGS", "") + " --key=" + key).strip() + rados_options = ["-p", args.pool] + options + + heartbeat_object = "KVMHA-hb-" + args.host + activity_object = "KVMHA-ac-" + args.host + + # the reason why the heartbeat did not show that the host is alive, added to the messages about a DEAD host + heartbeat_note = "" + + def dead(reason): + print("=====> Considering host as DEAD due to %s.%s <======" % (reason, heartbeat_note)) + sys.exit(0) + + # First check: heartbeat object, same as kvmheartbeat_rbd.py + now = int(time.time()) + status, output = ceph("rados", rados_options + ["get", heartbeat_object, "-"], env) + heartbeat = output.strip() + if status != 0: + heartbeat_note = " The RADOS object [%s] could not be read, it does not exist or Ceph can not be reached." % heartbeat_object + elif not re.fullmatch(r"[0-9]+", heartbeat): + heartbeat_note = " The RADOS object [%s] does not hold a heartbeat." % heartbeat_object + else: + age = now - int(heartbeat) + if age < 61: + alive() + heartbeat_note = " The heartbeat in RADOS object [%s] is [%d] seconds old." % (heartbeat_object, age) + + if not volumes.strip(): + dead("empty UUIDList") + + # Second check: RBD watcher based disk activity check. + # If any of the host's volumes still has a live watcher, something (most + # likely qemu on the host being checked) is actively using it right now. + images = volumes.replace("\n", ",").split(",") + images = ["".join(image.split()) for image in images] + images = [image for image in images if image] + try: + has_watcher = find_watcher(images, args.pool, args.mon_hosts, args.user, key, options, env) + except WatchersError as error: + # when the watchers can not be listed, it is unknown whether the host is alive: the script fails, + # which is not a DEAD host + fail("Unable to get the watchers of the image [%s/%s]" % (args.pool, error)) + latest_update_time = now if has_watcher else 0 + + state = "" + status, _ = ceph("rados", rados_options + ["stat", activity_object], env) + if status == 0: + status, state = ceph("rados", rados_options + ["get", activity_object, "-"], env) + if status != 0: + fail("Unable to read the RADOS object [%s]" % activity_object) + # a state which is not ":" (with an optional third field of older versions) + # is ignored, as if there was none + match = re.fullmatch(r"([0-9]+):([0-9]+)(:[0-9]+)?", state.strip()) + + status, _ = ceph("rados", rados_options + ["put", activity_object, "-"], env, + "%d:%d\n" % (suspect_time, latest_update_time)) + # without the new state the next check would use an old one, so it is not known whether the host is alive + if status != 0: + fail("Unable to write the RADOS object [%s]" % activity_object) + + if not match: + if latest_update_time > suspect_time: + alive() + dead("RADOS object [%s] did not exist or holds no state, and condition [latestUpdateTime -gt SuspectTime] " + "has not been satisfied" % activity_object) + + last_suspect_time = int(match.group(1)) + last_update_time = int(match.group(2)) + if suspect_time - last_suspect_time < 0: + if latest_update_time > suspect_time: + alive() + dead("RADOS object [%s] exists, condition [suspectTimeDiff -lt 0] was satisfied and " + "[latestUpdateTime -gt SuspectTime] has not been satisfied" % activity_object) + if latest_update_time > last_update_time: + alive() + dead("RADOS object [%s] exists and conditions [suspectTimeDiff -lt 0] and [latestUpdateTime -gt SuspectTime] " + "have not been satisfied" % activity_object) + + +if __name__ == "__main__": + main() diff --git a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh b/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh deleted file mode 100755 index dc6740c0b130..000000000000 --- a/scripts/vm/hypervisor/kvm/kvmvmactivity_rbd.sh +++ /dev/null @@ -1,236 +0,0 @@ -#!/bin/bash -# 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. - -# Ceph RBD flavor of kvmvmactivity.sh. -# -# On NFS/SharedMountPoint storage, VM disk activity is detected via the mtime -# of the volume files on the shared mount point. RBD volumes aren't files on -# a mount point, so instead activity is detected via RBD watchers: as long as -# qemu has an RBD image open (i.e. a VM using that volume is running -# somewhere), the image will have a live watcher. The most recent -# suspect-time/watcher-state is persisted as a RADOS object (per host) -# in place of the "ac-" file used by the NFS/SMP scripts. - -help() { - printf "Usage: $0 - -s ceph monitor host(s), comma separated - -o ceph/rbd pool name - -n cephx auth user (optional) - -k cephx auth key (optional, required if -n is set) - -h host - -u volume (rbd image) uuid list - -t current time in seconds (accepted for compatibility with kvmvmactivity.sh, not used) - -d suspect time\n" - exit 1 -} - -#set -x - -MonHosts= -PoolName= -CephUser= -CephKey= -HostIP= -UUIDList= -SuspectTime= - -while getopts 's:o:n:k:h:u:t:d:' OPTION -do - case $OPTION in - s) - MonHosts="$OPTARG" - ;; - o) - PoolName="$OPTARG" - ;; - n) - CephUser="$OPTARG" - ;; - k) - CephKey="$OPTARG" - ;; - h) - HostIP="$OPTARG" - ;; - u) - UUIDList="$OPTARG" - ;; - t) - # not used, see help - ;; - d) - SuspectTime="$OPTARG" - ;; - *) - help - ;; - esac -done - -if [ -z "$MonHosts" ] || [ -z "$PoolName" ] -then - exit 2 -fi - -if [ -z "$SuspectTime" ] -then - exit 2 -fi - -# the host IP names the heartbeat and activity objects -if [ -z "$HostIP" ] -then - exit 2 -fi - -if [ -n "$CephUser" ] && [ -z "$CephKey" ] -then - exit 2 -fi - -RadosOpts=(--mon-host "$MonHosts") -RbdOpts=(--mon-host "$MonHosts") -if [ -n "$CephUser" ] -then - # the key is given to rados and rbd in a file, to keep it out of the process list - KeyFile=$(mktemp) - trap 'rm -f "$KeyFile"' EXIT - printf '%s' "$CephKey" > "$KeyFile" - RadosOpts+=(--id "$CephUser" --keyfile "$KeyFile") - RbdOpts+=(--id "$CephUser" --keyfile "$KeyFile") -fi - -hbObject="KVMHA-hb-$HostIP" -acObject="KVMHA-ac-$HostIP" - -# the reason why the heartbeat did not show that the host is alive, added to the messages about a DEAD host -hbNote= - -dead() { - echo "=====> Considering host as DEAD due to $1.$hbNote <======" -} - -# First check: heartbeat object, same as kvmheartbeat_rbd.sh -now=$(date +%s) -if hb=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$hbObject" - 2> /dev/null) -then - if [[ "$hb" =~ ^[0-9]+$ ]] - then - diff=$(expr $now - $hb) - if [ $diff -lt 61 ] - then - echo "=====> ALIVE <=====" - exit 0 - fi - hbNote=" The heartbeat in RADOS object [$hbObject] is [$diff] seconds old." - else - hbNote=" The RADOS object [$hbObject] does not hold a heartbeat." - fi -else - hbNote=" The RADOS object [$hbObject] could not be read, it does not exist or Ceph can not be reached." -fi - -if [ -z "$UUIDList" ] -then - dead "empty UUIDList" - exit 0 -fi - -# Second check: RBD watcher based disk activity check. -# If any of the host's volumes still has a live watcher, something (most -# likely qemu on the host being checked) is actively using it right now. -latestUpdateTime=0 -IFS=',' read -ra images <<< "$UUIDList" -for image in "${images[@]}" -do - image=${image//[[:space:]]/} - if [ -z "$image" ] - then - continue - fi - # when the watchers can not be listed, it is unknown whether the host is alive: the script fails, which - # is not a DEAD host - if ! watchers=$(rbd status "$PoolName/$image" "${RbdOpts[@]}" 2> /dev/null) - then - echo "=====> Unable to get the watchers of the image [$PoolName/$image] <======" - exit 2 - fi - # "Watchers: none", or a line "watcher=
client. cookie=" per watcher - if grep -q '^[[:space:]]*watcher=' <<< "$watchers" - then - latestUpdateTime=$now - break - fi -done - -acTime= -if rados -p "$PoolName" "${RadosOpts[@]}" stat "$acObject" &> /dev/null -then - if ! acTime=$(rados -p "$PoolName" "${RadosOpts[@]}" get "$acObject" - 2> /dev/null) - then - echo "=====> Unable to read the RADOS object [$acObject] <======" - exit 2 - fi -fi -# a state which is not ":" (with an optional third field of older versions) -# is ignored, as if there was none -if ! [[ "$acTime" =~ ^[0-9]+:[0-9]+(:[0-9]+)?$ ]] -then - acTime= -fi - -tmpFile=$(mktemp) -echo "$SuspectTime:$latestUpdateTime" > "$tmpFile" -rados -p "$PoolName" "${RadosOpts[@]}" put "$acObject" "$tmpFile" &> /dev/null -putResult=$? -rm -f "$tmpFile" -# without the new state the next check would use an old one, so it is not known whether the host is alive -if [ $putResult -ne 0 ] -then - echo "=====> Unable to write the RADOS object [$acObject] <======" - exit 2 -fi - -if [ -z "$acTime" ]; then - if [[ $latestUpdateTime -gt $SuspectTime ]]; then - echo "=====> ALIVE <=====" - else - dead "RADOS object [$acObject] did not exist or holds no state, and condition [latestUpdateTime -gt SuspectTime] has not been satisfied" - fi -else - arrTime=(${acTime//:/ }) - lastSuspectTime=${arrTime[0]} - lastUpdateTime=${arrTime[1]} - - suspectTimeDiff=$(expr $SuspectTime - $lastSuspectTime) - if [[ $suspectTimeDiff -lt 0 ]]; then - if [[ $latestUpdateTime -gt $SuspectTime ]]; then - echo "=====> ALIVE <=====" - else - dead "RADOS object [$acObject] exists, condition [suspectTimeDiff -lt 0] was satisfied and [latestUpdateTime -gt SuspectTime] has not been satisfied" - fi - else - if [[ $latestUpdateTime -gt $lastUpdateTime ]]; then - echo "=====> ALIVE <=====" - else - dead "RADOS object [$acObject] exists and conditions [suspectTimeDiff -lt 0] and [latestUpdateTime -gt SuspectTime] have not been satisfied" - fi - fi -fi - -exit 0 From fcf71d147d0718fb7bd0ac53efc2d371b34bcb5a Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Tue, 13 Oct 2026 11:38:23 +0200 Subject: [PATCH 8/9] kvm: choose the script which checks the VM activity by the type of the pool LibvirtCheckVMActivityOnStoragePoolCommandWrapper gave the script of the agent, kvmvmactivity.sh, to every pool, and LibvirtStoragePool replaced it with the script of RBD by itself. Choose the script in the wrapper: kvmvmactivity_rbd.py for a RBD pool, and the script of the agent for the other pools. If the script of RBD can not be found, the check fails with a message, instead of running without a script. LibvirtStoragePool uses the script which it is given. --- ...VMActivityOnStoragePoolCommandWrapper.java | 22 ++- .../kvm/storage/LibvirtStoragePool.java | 6 +- ...tivityOnStoragePoolCommandWrapperTest.java | 157 ++++++++++++++++++ 3 files changed, 179 insertions(+), 6 deletions(-) create mode 100644 plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVMActivityOnStoragePoolCommandWrapperTest.java diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVMActivityOnStoragePoolCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVMActivityOnStoragePoolCommandWrapper.java index b132f6d98ce9..745686ce33de 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVMActivityOnStoragePoolCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVMActivityOnStoragePoolCommandWrapper.java @@ -22,6 +22,8 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.CheckVMActivityOnStoragePoolCommand; import com.cloud.agent.api.to.StorageFilerTO; +import com.cloud.agent.properties.AgentProperties; +import com.cloud.agent.properties.AgentPropertiesFileHandler; import com.cloud.hypervisor.kvm.resource.KVMHABase.HAStoragePool; import com.cloud.hypervisor.kvm.storage.KVMStoragePool; import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; @@ -30,6 +32,8 @@ import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; +import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.utils.script.Script; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -50,7 +54,11 @@ public Answer execute(final CheckVMActivityOnStoragePoolCommand command, final L if (primaryPool.isPoolSupportHA()) { final HAStoragePool haPool = monitor.getStoragePool(pool.getUuid()); - final KVMHAVMActivityChecker ha = new KVMHAVMActivityChecker(haPool, command.getHost(), command.getVolumeList(), libvirtComputingResource.getVmActivityCheckPath(), command.getSuspectTimeInSeconds()); + final String vmActivityCheckPath = getVmActivityCheckPath(pool.getType(), libvirtComputingResource); + if (vmActivityCheckPath == null) { + return new Answer(command, false, "CheckVMActivityOnStoragePoolCommand: unable to find the script which checks the VM activity on a storage pool of type " + pool.getType()); + } + final KVMHAVMActivityChecker ha = new KVMHAVMActivityChecker(haPool, command.getHost(), command.getVolumeList(), vmActivityCheckPath, command.getSuspectTimeInSeconds()); final Future future = executors.submit(ha); try { final Boolean result = future.get(); @@ -67,4 +75,16 @@ public Answer execute(final CheckVMActivityOnStoragePoolCommand command, final L } return new Answer(command, false, "Unsupported Storage"); } + + /** + * The script which checks the VM activity depends on the type of the storage pool: a RBD pool has no shared mount point to look at, + * so its script checks the watchers of the volumes, instead of the files of the volumes as the script of the other pools. + */ + private String getVmActivityCheckPath(final StoragePoolType poolType, final LibvirtComputingResource libvirtComputingResource) { + if (StoragePoolType.RBD.equals(poolType)) { + final String kvmScriptsDir = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.KVM_SCRIPTS_DIR); + return Script.findScript(kvmScriptsDir, "kvmvmactivity_rbd.py"); + } + return libvirtComputingResource.getVmActivityCheckPath(); + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java index a1534be160d6..12caa7e55abf 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStoragePool.java @@ -508,11 +508,7 @@ 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(); - // 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.py") : vmActivityCheckPath; - Script cmd = new Script(scriptPath, activityScriptTimeout.getStandardSeconds(), logger); + Script cmd = new Script(vmActivityCheckPath, activityScriptTimeout.getStandardSeconds(), logger); addPoolConnectionArgs(cmd, pool); cmd.add("-h", hostIp); File volumeListFile = addVolumeListArgs(cmd, volumeUUIDListString); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVMActivityOnStoragePoolCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVMActivityOnStoragePoolCommandWrapperTest.java new file mode 100644 index 000000000000..50e7427aabd6 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVMActivityOnStoragePoolCommandWrapperTest.java @@ -0,0 +1,157 @@ +/* + * Licensed 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.wrapper; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.joda.time.Duration; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CheckVMActivityOnStoragePoolCommand; +import com.cloud.agent.api.to.HostTO; +import com.cloud.agent.api.to.StorageFilerTO; +import com.cloud.hypervisor.kvm.resource.KVMHABase; +import com.cloud.hypervisor.kvm.resource.KVMHAMonitor; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.storage.KVMStoragePool; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.utils.script.Script; + +@RunWith(MockitoJUnitRunner.class) +public class LibvirtCheckVMActivityOnStoragePoolCommandWrapperTest { + + private static final String POOL_UUID = "7684f6e5-fd91-3435-b04e-59ae0efb3914"; + private static final String RBD_SCRIPT = "/scripts/kvmvmactivity_rbd.py"; + private static final String SCRIPT = "/scripts/kvmvmactivity.sh"; + + private final LibvirtCheckVMActivityOnStoragePoolCommandWrapper wrapper = new LibvirtCheckVMActivityOnStoragePoolCommandWrapper(); + private final LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + private final KVMStoragePool pool = mock(KVMStoragePool.class); + private final CheckVMActivityOnStoragePoolCommand command = mock(CheckVMActivityOnStoragePoolCommand.class); + + private void setUp(StoragePoolType type, boolean supportsHa) { + StorageFilerTO filer = mock(StorageFilerTO.class); + when(filer.getType()).thenReturn(type); + when(filer.getUuid()).thenReturn(POOL_UUID); + when(command.getPool()).thenReturn(filer); + KVMStoragePoolManager poolManager = mock(KVMStoragePoolManager.class); + when(poolManager.getStoragePool(type, POOL_UUID)).thenReturn(pool); + when(resource.getStoragePoolMgr()).thenReturn(poolManager); + when(pool.isPoolSupportHA()).thenReturn(supportsHa); + } + + private KVMHABase.HAStoragePool addHaPool() { + KVMHABase.HAStoragePool haPool = new KVMHABase.HAStoragePool(pool, "10.0.0.1", "cloudstack", KVMHABase.PoolType.PrimaryStorage); + KVMHAMonitor monitor = mock(KVMHAMonitor.class); + when(monitor.getStoragePool(POOL_UUID)).thenReturn(haPool); + when(resource.getMonitor()).thenReturn(monitor); + return haPool; + } + + private void expectActivityCheck(String volumes, long suspectTime, boolean activity) { + when(command.getHost()).thenReturn(mock(HostTO.class)); + when(command.getVolumeList()).thenReturn(volumes); + when(command.getSuspectTimeInSeconds()).thenReturn(suspectTime); + when(pool.hasVmActivity(any(), any(), any(), anyString(), anyString(), anyLong())).thenReturn(activity); + } + + @Test + public void testRbdPoolIsCheckedWithTheScriptOfRbd() { + setUp(StoragePoolType.RBD, true); + KVMHABase.HAStoragePool haPool = addHaPool(); + expectActivityCheck("volume-1,volume-2", 123L, false); + + try (MockedStatic