add host HA support and retry transient lock refusals on activation
diff --git a/api/src/main/java/com/cloud/agent/api/to/HostTO.java b/api/src/main/java/com/cloud/agent/api/to/HostTO.java index ee14e65..1e25440 100644 --- a/api/src/main/java/com/cloud/agent/api/to/HostTO.java +++ b/api/src/main/java/com/cloud/agent/api/to/HostTO.java
@@ -25,6 +25,7 @@ private NetworkTO storageNetwork1; private NetworkTO storageNetwork2; private String parent; + private Long sanlockHostId; protected HostTO() { } @@ -46,6 +47,14 @@ } } + public Long getSanlockHostId() { + return sanlockHostId; + } + + public void setSanlockHostId(Long sanlockHostId) { + this.sanlockHostId = sanlockHostId; + } + public String getGuid() { return guid; }
diff --git a/api/src/main/java/com/cloud/host/Host.java b/api/src/main/java/com/cloud/host/Host.java index 79bec7a..d56ff86 100644 --- a/api/src/main/java/com/cloud/host/Host.java +++ b/api/src/main/java/com/cloud/host/Host.java
@@ -60,6 +60,7 @@ String HOST_RBD_VOLUME_ENCRYPTION = "host.volume.encryption.rbd"; String HOST_INSTANCE_CONVERSION = "host.instance.conversion"; String HOST_VDDK_SUPPORT = "host.vddk.support"; + String HOST_SANLOCK_ID = "host.sanlock.id"; String HOST_VDDK_LIB_DIR = "vddk.lib.dir"; String HOST_VDDK_VERSION = "host.vddk.version"; String HOST_OVFTOOL_VERSION = "host.ovftool.version";
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 53bfcce..bf9b1d7 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,7 @@ */ public interface HighAvailabilityManager extends Manager { - List<StoragePoolType> LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint); + List<StoragePoolType> LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint, StoragePoolType.CLVM, StoragePoolType.CLVM_NG); ConfigKey<Boolean> 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/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 2fad96e..3391a8b 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
@@ -881,6 +881,7 @@ return null; } + long recoveryDeadlineMs = clvmPoolManager.newLockRecoveryDeadlineMs(); for (VolumeVO volume : vmVolumes) { if (volume.getPoolId() == null) { continue; @@ -895,7 +896,8 @@ volume.getUuid(), volume.getPath(), pool, - true + true, + recoveryDeadlineMs ); if (lockHostId != null) { logger.debug("Found actual CLVM lock host {} from volume {} of VM {} via LVM query", @@ -912,6 +914,7 @@ return; } + long recoveryDeadlineMs = clvmPoolManager.newLockRecoveryDeadlineMs(); for (VolumeVO volume : volumes) { if (volume.getPoolId() == null) { continue; @@ -927,7 +930,8 @@ volume.getUuid(), volume.getPath(), pool, - true + true, + recoveryDeadlineMs ); if (currentLockHost == null) {
diff --git a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java index d61caf1..70e207a 100644 --- a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java +++ b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java
@@ -671,9 +671,9 @@ ClvmPoolManager clvmPoolManager = Mockito.mock(ClvmPoolManager.class); Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(), - Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId); + Mockito.anyString(), Mockito.any(), Mockito.eq(true), Mockito.anyLong())).thenReturn(currentHostId); Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(102L), Mockito.anyString(), - Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId); + Mockito.anyString(), Mockito.any(), Mockito.eq(true), Mockito.anyLong())).thenReturn(currentHostId); Mockito.when(clvmPoolManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(true); @@ -743,7 +743,7 @@ ClvmPoolManager clvmPoolManager = Mockito.mock(ClvmPoolManager.class); Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class), - ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true))).thenReturn(destHostId); + ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true), Mockito.anyLong())).thenReturn(destHostId); Mockito.when(storagePoolDao.findById(poolId)).thenReturn(clvmPool); @@ -776,7 +776,7 @@ method.invoke(volumeOrchestrator, new ArrayList<VolumeVO>(), destHostId, vmInstance); Mockito.verify(clvmPoolManager, Mockito.never()).getClvmLockHostId(Mockito.anyLong(), Mockito.anyString(), - Mockito.anyString(), Mockito.any(), Mockito.anyBoolean()); + Mockito.anyString(), Mockito.any(), Mockito.anyBoolean(), Mockito.anyLong()); } @Test @@ -817,7 +817,7 @@ ClvmPoolManager clvmPoolManager = Mockito.mock(ClvmPoolManager.class); Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(101L), ArgumentMatchers.nullable(String.class), - ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true))).thenReturn(null); + ArgumentMatchers.nullable(String.class), Mockito.any(), Mockito.eq(true), Mockito.anyLong())).thenReturn(null); Mockito.when(storagePoolDao.findById(poolId)).thenReturn(clvmPool); @@ -863,7 +863,7 @@ ClvmPoolManager clvmPoolManager = Mockito.mock(ClvmPoolManager.class); Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(), - Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId); + Mockito.anyString(), Mockito.any(), Mockito.eq(true), Mockito.anyLong())).thenReturn(currentHostId); Mockito.when(clvmPoolManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(true); @@ -904,7 +904,7 @@ ClvmPoolManager clvmPoolManager = Mockito.mock(ClvmPoolManager.class); Mockito.when(clvmPoolManager.getClvmLockHostId(Mockito.eq(101L), Mockito.anyString(), - Mockito.anyString(), Mockito.any(), Mockito.eq(true))).thenReturn(currentHostId); + Mockito.anyString(), Mockito.any(), Mockito.eq(true), Mockito.anyLong())).thenReturn(currentHostId); Mockito.when(clvmPoolManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(false);
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 0ee59c9..20afa31 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
@@ -44,13 +44,7 @@ storagePools.stream().map(pool -> pool.getPoolUUID()).collect(Collectors.joining(", "))); logger.debug("Checking heart beat with KVMHAChecker for {}", hostAndPools); - boolean heartBeatCheckResult = false; - for (HAStoragePool pool : storagePools) { - heartBeatCheckResult = pool.getPool().hasHeartBeat(pool, host); - if (reportIfHeartBeatFailedForOneStoragePool && !heartBeatCheckResult) { - break; - } - } + boolean heartBeatCheckResult = checkPoolsHeartBeat(); if (!heartBeatCheckResult) { logger.warn("All checks with KVMHAChecker for {} considered it as dead. It may cause a shutdown of the host.", hostAndPools); @@ -59,6 +53,22 @@ return heartBeatCheckResult; } + /* + * With reportIfHeartBeatFailedForOneStoragePool the host is dead as soon as one pool has no heart beat, + * otherwise it is dead only when none of the pools has one. + */ + private boolean checkPoolsHeartBeat() { + boolean anyPoolHasHeartBeat = false; + for (HAStoragePool pool : storagePools) { + boolean poolHasHeartBeat = pool.getPool().hasHeartBeat(pool, host); + if (reportIfHeartBeatFailedForOneStoragePool && !poolHasHeartBeat) { + return false; + } + anyPoolHasHeartBeat = anyPoolHasHeartBeat || poolHasHeartBeat; + } + return anyPoolHasHeartBeat; + } + @Override public Boolean call() throws Exception { // logger.addAppender(new org.apache.log4j.ConsoleAppender(new
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 9f1b849..a8b8a96 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
@@ -18,7 +18,7 @@ import com.cloud.agent.properties.AgentProperties; import com.cloud.agent.properties.AgentPropertiesFileHandler; -import com.cloud.ha.HighAvailabilityManager; +import com.cloud.storage.Storage.StoragePoolType; import com.cloud.utils.script.Script; import org.libvirt.Connect; import org.libvirt.LibvirtException; @@ -34,6 +34,8 @@ public class KVMHAMonitor extends KVMHABase implements Runnable { + private static final List<StoragePoolType> MOUNT_BASED_POOL_TYPES = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint); + private final Map<String, HAStoragePool> haStoragePools = new ConcurrentHashMap<>(); private final boolean rebootHostAndAlertManagementOnHeartbeatTimeout; @@ -54,7 +56,9 @@ synchronized (haStoragePools) { HAStoragePool pool = haStoragePools.get(uuid); if (pool != null) { - Script.runSimpleBashScript("umount " + pool.getMountDestPath()); + if (isMountBased(pool)) { + Script.runSimpleBashScript("umount " + pool.getMountDestPath()); + } haStoragePools.remove(uuid); } } @@ -72,12 +76,16 @@ } } + private boolean isMountBased(HAStoragePool pool) { + return MOUNT_BASED_POOL_TYPES.contains(pool.getPool().getType()); + } + protected void runHeartBeat() { synchronized (haStoragePools) { Set<String> removedPools = new HashSet<>(); for (String uuid : haStoragePools.keySet()) { HAStoragePool primaryStoragePool = haStoragePools.get(uuid); - if (HighAvailabilityManager.LIBVIRT_STORAGE_POOL_TYPES_WITH_HA_SUPPORT.contains(primaryStoragePool.getPool().getType())) { + if (isMountBased(primaryStoragePool)) { checkForNotExistingLibvirtStoragePools(removedPools, uuid); if (removedPools.contains(uuid)) { continue;
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 9009ec6..10ed8d1 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
@@ -21,6 +21,7 @@ import static com.cloud.host.Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM; import static com.cloud.host.Host.HOST_OVFTOOL_VERSION; import static com.cloud.host.Host.HOST_RBD_VOLUME_ENCRYPTION; +import static com.cloud.host.Host.HOST_SANLOCK_ID; import static com.cloud.host.Host.HOST_VDDK_LIB_DIR; import static com.cloud.host.Host.HOST_VDDK_SUPPORT; import static com.cloud.host.Host.HOST_VDDK_VERSION; @@ -217,6 +218,7 @@ import com.cloud.hypervisor.kvm.resource.rolling.maintenance.RollingMaintenanceServiceExecutor; import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtRequestWrapper; import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtUtilitiesHelper; +import com.cloud.hypervisor.kvm.storage.ClvmSanlockHaChecker; import com.cloud.hypervisor.kvm.storage.IscsiStorageCleanupMonitor; import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk; import com.cloud.hypervisor.kvm.storage.KVMStoragePool; @@ -4430,6 +4432,7 @@ cmd.setCluster(clusterId); cmd.setGatewayIpAddress(localGateway); cmd.setIqn(getIqn()); + addSanlockHostId(cmd); cmd.getHostDetails().put(HOST_VOLUME_ENCRYPTION, String.valueOf(hostSupportsVolumeEncryption())); cmd.getHostDetails().put(HOST_RBD_VOLUME_ENCRYPTION, String.valueOf(hostSupportsRbdVolumeEncryption())); cmd.setHostTags(getHostTags()); @@ -6216,6 +6219,13 @@ return true; } + private void addSanlockHostId(StartupRoutingCommand cmd) { + Long sanlockHostId = ClvmSanlockHaChecker.readLocalHostId(); + if (sanlockHostId != null) { + cmd.getHostDetails().put(HOST_SANLOCK_ID, String.valueOf(sanlockHostId)); + } + } + /** * Test host for qemu-native LUKS volume encryption (qemu-img LUKS support + cryptsetup), * reported as {@code host.volume.encryption}. RBD/librbd encryption support is a separate
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ClvmSanlockHaChecker.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ClvmSanlockHaChecker.java new file mode 100644 index 0000000..89d7e89 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ClvmSanlockHaChecker.java
@@ -0,0 +1,160 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package com.cloud.hypervisor.kvm.storage; + +import java.io.File; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import com.cloud.utils.script.OutputInterpreter; +import com.cloud.utils.script.Script; + +/** + * Reads a host's sanlock delta-lease state for a CLVM/CLVM_NG volume group; the lease is renewed + * by sanlock itself, and a host that stops renewing is reset by its watchdog. + */ +public class ClvmSanlockHaChecker { + + protected static final String LOCKSPACE_PREFIX = "lvm_"; + private static final String LVMLOCKD_SOCKET = "/run/lvm/lvmlockd.socket"; + protected static final long WATCHDOG_FIRE_TIMEOUT_SECONDS = 60; + protected static final long ID_RENEWAL_FAIL_IO_TIMEOUTS = 8; + private static final long CHECK_TIMEOUT_MS = 30000; + + private static final Logger LOGGER = LogManager.getLogger(ClvmSanlockHaChecker.class); + private static final Pattern HOST_ID_LINE = Pattern.compile("^host_id=(\\d+)$"); + private static final Pattern HOST_ENTRY = Pattern.compile("^(\\d+) timestamp \\d+\\s*$"); + + protected static class LeaseState { + final long lastCheck; + final long lastLive; + final long ioTimeout; + + LeaseState(long lastCheck, long lastLive, long ioTimeout) { + this.lastCheck = lastCheck; + this.lastLive = lastLive; + this.ioTimeout = ioTimeout; + } + } + + /** Returns this host's lvmlockd sanlock host_id, or null when lvmlockd is not running. */ + public static Long readLocalHostId() { + if (!new File(LVMLOCKD_SOCKET).exists()) { + return null; + } + Script cmd = new Script("lvmconfig", CHECK_TIMEOUT_MS, LOGGER); + cmd.add("local/host_id"); + OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser(); + if (cmd.execute(parser) != null) { + return null; + } + return parseHostId(parser.getLine()); + } + + protected static Long parseHostId(String line) { + Matcher matcher = HOST_ID_LINE.matcher(line == null ? "" : line.trim()); + if (!matcher.matches()) { + return null; + } + try { + return Long.valueOf(matcher.group(1)); + } catch (NumberFormatException e) { + LOGGER.warn("Ignoring invalid sanlock host id [{}]", line); + return null; + } + } + + /** Returns false only when the lease stopped renewing long enough that the watchdog has reset the host. */ + public static boolean isHostLeaseAlive(String vgName, Long hostId) { + if (vgName == null || hostId == null) { + LOGGER.warn("Cannot check the sanlock lease: VG name [{}] or host id [{}] is unknown, assuming the host is alive.", vgName, hostId); + return true; + } + String output = readHostStatus(vgName); + if (output == null) { + return true; + } + LeaseState state = parseLeaseState(output, hostId); + if (state == null) { + LOGGER.warn("No sanlock lease found for host id [{}] in lockspace [{}{}], assuming the host is alive.", hostId, LOCKSPACE_PREFIX, vgName); + return true; + } + return isLeaseAlive(state); + } + + private static String readHostStatus(String vgName) { + Script cmd = new Script("sanlock", CHECK_TIMEOUT_MS, LOGGER); + cmd.add("client", "host_status", "-s", LOCKSPACE_PREFIX + vgName, "-D"); + OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser(); + String result = cmd.execute(parser); + if (result != null) { + LOGGER.warn("Failed to read sanlock host status for lockspace [{}{}]: {}", LOCKSPACE_PREFIX, vgName, result); + return null; + } + return parser.getLines(); + } + + protected static LeaseState parseLeaseState(String output, long hostId) { + try { + return readLeaseState(output, hostId); + } catch (NumberFormatException e) { + LOGGER.warn("Unexpected sanlock host status for host id [{}]: {}", hostId, e.getMessage()); + return null; + } + } + + private static LeaseState readLeaseState(String output, long hostId) { + boolean inHost = false; + Long lastCheck = null; + Long lastLive = null; + Long ioTimeout = null; + for (String line : output.split("\\r?\\n")) { + Matcher entry = HOST_ENTRY.matcher(line); + if (entry.matches()) { + if (inHost) { + break; + } + inHost = Long.parseLong(entry.group(1)) == hostId; + } else if (inHost) { + lastCheck = valueOrCurrent(line, "last_check", lastCheck); + lastLive = valueOrCurrent(line, "last_live", lastLive); + ioTimeout = valueOrCurrent(line, "io_timeout", ioTimeout); + } + } + if (lastCheck == null || lastLive == null || ioTimeout == null) { + return null; + } + return new LeaseState(lastCheck, lastLive, ioTimeout); + } + + private static Long valueOrCurrent(String line, String key, Long current) { + String trimmed = line.trim(); + if (trimmed.startsWith(key + "=")) { + return Long.parseLong(trimmed.substring(key.length() + 1)); + } + return current; + } + + protected static boolean isLeaseAlive(LeaseState state) { + long hostDeadSeconds = ID_RENEWAL_FAIL_IO_TIMEOUTS * state.ioTimeout + WATCHDOG_FIRE_TIMEOUT_SECONDS; + return state.lastCheck - state.lastLive < hostDeadSeconds; + } +}
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ClvmStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ClvmStorageAdaptor.java index e9ec655..fe2f10b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ClvmStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ClvmStorageAdaptor.java
@@ -413,7 +413,7 @@ return getPhysicalDiskViaDirectBlockDevice(volumeUuid, pool); } - private String getVgName(String sourceDir) { + static String getVgName(String sourceDir) { String vgName = sourceDir; if (vgName.startsWith("/")) { String[] parts = vgName.split("/");
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 d99847f..922835f 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
@@ -43,6 +43,7 @@ import com.cloud.storage.Storage; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageLayer; +import com.cloud.storage.clvm.ClvmPoolManager; import com.cloud.storage.Volume; import com.cloud.utils.Pair; import com.cloud.utils.Ternary; @@ -443,9 +444,13 @@ return adaptor.disconnectPhysicalDisk(volPath, pool); } + private boolean isHaPoolRemovedOnDelete(StoragePoolType type) { + return type == StoragePoolType.NetworkFilesystem || ClvmPoolManager.isClvmPoolType(type); + } + public synchronized boolean deleteStoragePool(StoragePoolType type, String uuid) { StorageAdaptor adaptor = getStorageAdaptor(type); - if (type == StoragePoolType.NetworkFilesystem) { + if (isHaPoolRemovedOnDelete(type)) { _haMonitor.removeStoragePool(uuid); } boolean deleteStatus = adaptor.deleteStoragePool(uuid);; @@ -457,7 +462,7 @@ public boolean deleteStoragePool(StoragePoolType type, String uuid, Map<String, String> details) { StorageAdaptor adaptor = getStorageAdaptor(type); - if (type == StoragePoolType.NetworkFilesystem) { + if (isHaPoolRemovedOnDelete(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 a8c32ba..ed6ba1f 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
@@ -35,6 +35,7 @@ import com.cloud.hypervisor.kvm.resource.KVMHABase.HAStoragePool; import com.cloud.storage.Storage; import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.storage.clvm.ClvmPoolManager; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.OutputInterpreter; import com.cloud.utils.script.Script; @@ -348,7 +349,14 @@ } + private Boolean hasClvmHostLease(HostTO host) { + return ClvmSanlockHaChecker.isHostLeaseAlive(ClvmStorageAdaptor.getVgName(getLocalPath()), host.getSanlockHostId()); + } + public String createHeartBeatCommand(HAStoragePool primaryStoragePool, String hostPrivateIp, boolean hostValidation) { + if (ClvmPoolManager.isClvmPoolType(type)) { + return null; + } Script cmd = new Script(primaryStoragePool.getPool().getHearthBeatPath(), HeartBeatUpdateTimeoutInMs, logger); cmd.add("-i", primaryStoragePool.getPoolIp()); cmd.add("-p", primaryStoragePool.getPoolMountSourcePath()); @@ -375,6 +383,9 @@ @Override public Boolean hasHeartBeat(HAStoragePool pool, HostTO host) { + if (ClvmPoolManager.isClvmPoolType(type)) { + return hasClvmHostLease(host); + } String hostIp = host.getPrivateNetwork().getIp(); Script cmd = new Script(getHearthBeatPath(), HeartBeatCheckerTimeoutInMs, logger); cmd.add("-i", pool.getPoolIp()); @@ -400,6 +411,9 @@ @Override public Boolean hasVmActivity(HAStoragePool pool, HostTO host, Duration activityScriptTimeout, String volumeUUIDListString, String vmActivityCheckPath, long duration) { + if (ClvmPoolManager.isClvmPoolType(type)) { + return hasClvmHostLease(host); + } String hostIp = host.getPrivateNetwork().getIp(); Script cmd = new Script(vmActivityCheckPath, activityScriptTimeout.getStandardSeconds(), logger); cmd.add("-i", pool.getPoolIp());
diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/kvm/ha/KVMHostActivityChecker.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/kvm/ha/KVMHostActivityChecker.java index 96fd42d..02327b2 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/kvm/ha/KVMHostActivityChecker.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/kvm/ha/KVMHostActivityChecker.java
@@ -22,14 +22,18 @@ import com.cloud.agent.api.CheckOnHostAnswer; import com.cloud.agent.api.CheckOnHostCommand; import com.cloud.agent.api.CheckVMActivityOnStoragePoolCommand; +import com.cloud.agent.api.to.HostTO; import com.cloud.dc.dao.ClusterDao; import com.cloud.exception.StorageUnavailableException; import com.cloud.ha.HighAvailabilityManager; import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.Status; +import com.cloud.host.DetailVO; +import com.cloud.host.dao.HostDetailsDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.resource.ResourceManager; +import com.cloud.storage.clvm.ClvmPoolManager; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.Volume; @@ -44,6 +48,7 @@ import org.apache.cloudstack.ha.provider.HealthCheckerInterface; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.commons.lang.ArrayUtils; +import org.apache.commons.lang3.math.NumberUtils; import javax.inject.Inject; import java.util.ArrayList; @@ -67,6 +72,8 @@ private StorageManager storageManager; @Inject private ResourceManager resourceManager; + @Inject + private HostDetailsDao hostDetailsDao; @Override public boolean isActive(Host r, DateTime suspectTime) throws HACheckerException { @@ -126,6 +133,7 @@ Status hostStatus; boolean reportFailureIfOneStorageIsDown = HighAvailabilityManager.KvmHAFenceHostIfHeartbeatFailsOnStorage.value(); final CheckOnHostCommand cmd = new CheckOnHostCommand(host, reportFailureIfOneStorageIsDown); + addSanlockHostId(cmd.getHost(), host); try { logger.debug("Checking {} status...", host.toString()); Answer answer = agentMgr.easySend(host.getId(), cmd); @@ -153,6 +161,7 @@ Status hostStatusFromNeighbour = Status.Unknown; boolean reportFailureIfOneStorageIsDown = HighAvailabilityManager.KvmHAFenceHostIfHeartbeatFailsOnStorage.value(); final CheckOnHostCommand cmd = new CheckOnHostCommand(host, reportFailureIfOneStorageIsDown); + addSanlockHostId(cmd.getHost(), host); List<HostVO> neighbors = resourceManager.listHostsInClusterByStatus(host.getClusterId(), Status.Up); for (HostVO neighbor : neighbors) { if (neighbor.getId() == host.getId() @@ -204,6 +213,9 @@ protected boolean verifyActivityOfStorageOnHost(HashMap<StoragePool, List<Volume>> poolVolMap, StoragePool pool, Host host, DateTime suspectTime, boolean activityStatus) throws HACheckerException, IllegalStateException { List<Volume> volume_list = poolVolMap.get(pool); final CheckVMActivityOnStoragePoolCommand cmd = new CheckVMActivityOnStoragePoolCommand(host, pool, volume_list, suspectTime); + if (ClvmPoolManager.isClvmPoolType(pool.getPoolType())) { + addSanlockHostId(cmd.getHost(), host); + } logger.debug("Checking VM activity for {} on storage pool [{}].", host.toString(), pool); try { @@ -224,6 +236,13 @@ return activityStatus; } + private void addSanlockHostId(HostTO hostTO, Host host) { + DetailVO detail = hostDetailsDao.findDetail(host.getId(), Host.HOST_SANLOCK_ID); + if (detail != null && NumberUtils.isDigits(detail.getValue())) { + hostTO.setSanlockHostId(Long.valueOf(detail.getValue())); + } + } + private HashMap<StoragePool, List<Volume>> getStoragePoolAndVolumeInfoOnHost(Host host) { List<VMInstanceVO> vmListOnHost = vmInstanceDao.listByHostId(host.getId()); List<VolumeVO> volumeListOnHost = new ArrayList<>();
diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/KVMHACheckerTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/KVMHACheckerTest.java new file mode 100644 index 0000000..5f8c04f --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/KVMHACheckerTest.java
@@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package com.cloud.hypervisor.kvm.resource; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.agent.api.to.HostTO; +import com.cloud.agent.api.to.NetworkTO; +import com.cloud.hypervisor.kvm.resource.KVMHABase.HAStoragePool; +import com.cloud.hypervisor.kvm.resource.KVMHABase.PoolType; +import com.cloud.hypervisor.kvm.storage.KVMStoragePool; + +@RunWith(MockitoJUnitRunner.Silent.class) +public class KVMHACheckerTest { + + @Mock + private HostTO host; + @Mock + private NetworkTO privateNetwork; + + private List<HAStoragePool> poolsWithHeartBeats(boolean... heartBeats) { + when(host.getPrivateNetwork()).thenReturn(privateNetwork); + List<HAStoragePool> pools = new ArrayList<>(); + for (boolean heartBeat : heartBeats) { + KVMStoragePool kvmPool = mock(KVMStoragePool.class); + when(kvmPool.hasHeartBeat(any(HAStoragePool.class), any(HostTO.class))).thenReturn(heartBeat); + pools.add(new HAStoragePool(kvmPool, "host", "/path", PoolType.PrimaryStorage)); + } + return pools; + } + + private boolean checkHeartBeat(boolean reportIfFailedForOnePool, boolean... heartBeats) { + return new KVMHAChecker(poolsWithHeartBeats(heartBeats), host, reportIfFailedForOnePool).hasHeartBeat(); + } + + @Test + public void testHostIsAliveWhenAnyPoolHasHeartBeatAndOnePoolFailureIsNotReported() { + assertTrue(checkHeartBeat(false, true, false)); + assertTrue(checkHeartBeat(false, false, true)); + } + + @Test + public void testHostIsDeadWhenNoPoolHasHeartBeat() { + assertFalse(checkHeartBeat(false, false, false)); + assertFalse(checkHeartBeat(true, false, false)); + } + + @Test + public void testHostIsDeadWhenOnePoolHasNoHeartBeatAndItIsReported() { + assertFalse(checkHeartBeat(true, true, false)); + assertFalse(checkHeartBeat(true, false, true)); + } + + @Test + public void testHostIsAliveWhenAllPoolsHaveHeartBeat() { + assertTrue(checkHeartBeat(true, true, true)); + assertTrue(checkHeartBeat(false, true, true)); + } + + @Test + public void testHostIsDeadWithoutPools() { + assertFalse(checkHeartBeat(false)); + assertFalse(checkHeartBeat(true)); + } +}
diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ClvmSanlockHaCheckerTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ClvmSanlockHaCheckerTest.java new file mode 100644 index 0000000..c146595 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ClvmSanlockHaCheckerTest.java
@@ -0,0 +1,104 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + + +package com.cloud.hypervisor.kvm.storage; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +public class ClvmSanlockHaCheckerTest { + + private static final String HOST_STATUS = + "1 timestamp 437999\n" + + " last_check=438020\n" + + " last_live=438020\n" + + " last_req=0\n" + + " owner_id=1\n" + + " owner_generation=2\n" + + " timestamp=437999\n" + + " io_timeout=10\n" + + " owner_name=46a23acd-5b2c-479d-8474-583e8fced6a8.ref-trl-12\n" + + "2 timestamp 437955\n" + + " last_check=438200\n" + + " last_live=438020\n" + + " last_req=0\n" + + " owner_id=2\n" + + " owner_generation=1\n" + + " timestamp=437955\n" + + " io_timeout=10\n" + + " owner_name=31028f82-31c0-4c3d-a78a-8cc92afde498.ref-trl-12\n"; + + @Test + public void testParseLeaseStateForLiveHost() { + ClvmSanlockHaChecker.LeaseState state = ClvmSanlockHaChecker.parseLeaseState(HOST_STATUS, 1); + assertNotNull(state); + assertEquals(438020, state.lastCheck); + assertEquals(438020, state.lastLive); + assertEquals(10, state.ioTimeout); + assertTrue(ClvmSanlockHaChecker.isLeaseAlive(state)); + } + + @Test + public void testParseLeaseStateForHostWhoseLeaseStoppedRenewing() { + ClvmSanlockHaChecker.LeaseState state = ClvmSanlockHaChecker.parseLeaseState(HOST_STATUS, 2); + assertNotNull(state); + assertEquals(180, state.lastCheck - state.lastLive); + assertFalse(ClvmSanlockHaChecker.isLeaseAlive(state)); + } + + @Test + public void testLeaseStillAliveBeforeWatchdogGuarantee() { + assertTrue(ClvmSanlockHaChecker.isLeaseAlive(new ClvmSanlockHaChecker.LeaseState(1139, 1000, 10))); + assertFalse(ClvmSanlockHaChecker.isLeaseAlive(new ClvmSanlockHaChecker.LeaseState(1140, 1000, 10))); + } + + @Test + public void testParseLeaseStateForUnknownHostId() { + assertNull(ClvmSanlockHaChecker.parseLeaseState(HOST_STATUS, 3)); + assertNull(ClvmSanlockHaChecker.parseLeaseState("", 1)); + } + + @Test + public void testParseHostId() { + assertEquals(Long.valueOf(2), ClvmSanlockHaChecker.parseHostId("host_id=2")); + assertNull(ClvmSanlockHaChecker.parseHostId(" Configuration node local/host_id not found")); + assertNull(ClvmSanlockHaChecker.parseHostId(null)); + } + + @Test + public void testParseHostIdIgnoresNumberTooLarge() { + assertNull(ClvmSanlockHaChecker.parseHostId("host_id=99999999999999999999")); + } + + @Test + public void testParseLeaseStateIgnoresMalformedNumbers() { + assertNull(ClvmSanlockHaChecker.parseLeaseState("1 timestamp 10\n\tlast_check=abc\n\tlast_live=5\n\tio_timeout=10\n", 1)); + assertNull(ClvmSanlockHaChecker.parseLeaseState("99999999999999999999 timestamp 10\n\tlast_check=1\n", 1)); + } + + @Test + public void testUnknownVgOrHostIdAssumesAlive() { + assertTrue(ClvmSanlockHaChecker.isHostLeaseAlive(null, 1L)); + assertTrue(ClvmSanlockHaChecker.isHostLeaseAlive("sharedvg", null)); + } +}
diff --git a/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/kvm/ha/KVMHostActivityCheckerTest.java b/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/kvm/ha/KVMHostActivityCheckerTest.java new file mode 100644 index 0000000..c490251 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/kvm/ha/KVMHostActivityCheckerTest.java
@@ -0,0 +1,125 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.cloudstack.kvm.ha; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.HashMap; +import java.util.List; + +import org.joda.time.DateTime; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CheckOnHostCommand; +import com.cloud.agent.api.CheckVMActivityOnStoragePoolCommand; +import com.cloud.host.DetailVO; +import com.cloud.host.Host; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.host.dao.HostDetailsDao; +import com.cloud.resource.ResourceManager; +import com.cloud.dc.dao.ClusterDao; +import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.storage.StorageManager; +import com.cloud.storage.StoragePool; +import com.cloud.storage.Volume; + +@RunWith(MockitoJUnitRunner.Silent.class) +public class KVMHostActivityCheckerTest { + + @Mock + private HostDetailsDao hostDetailsDao; + @Mock + private StorageManager storageManager; + @Mock + private AgentManager agentMgr; + @Mock + private ResourceManager resourceManager; + @Mock + private ClusterDao clusterDao; + @Mock + private Host host; + @Mock + private StoragePool pool; + @Mock + private Volume volume; + @InjectMocks + private KVMHostActivityChecker checker; + + private void runActivityCheck(StoragePoolType poolType) throws Exception { + when(pool.getPoolType()).thenReturn(poolType); + when(host.getId()).thenReturn(8L); + when(volume.getPath()).thenReturn("vol"); + when(hostDetailsDao.findDetail(8L, Host.HOST_SANLOCK_ID)).thenReturn(new DetailVO(8L, Host.HOST_SANLOCK_ID, "2")); + when(storageManager.sendToPool(any(StoragePool.class), any(long[].class), any(CheckVMActivityOnStoragePoolCommand.class))).thenReturn(new Answer(null)); + HashMap<StoragePool, List<Volume>> poolVolumes = new HashMap<>(); + poolVolumes.put(pool, List.of(volume)); + checker.verifyActivityOfStorageOnHost(poolVolumes, pool, host, new DateTime(), true); + } + + @Test + public void testSanlockHostIdIsNotLookedUpForNonClvmPool() throws Exception { + runActivityCheck(StoragePoolType.NetworkFilesystem); + verify(hostDetailsDao, never()).findDetail(anyLong(), any()); + } + + @Test + public void testSanlockHostIdIsSentForClvmPool() throws Exception { + runActivityCheck(StoragePoolType.CLVM_NG); + ArgumentCaptor<CheckVMActivityOnStoragePoolCommand> captor = ArgumentCaptor.forClass(CheckVMActivityOnStoragePoolCommand.class); + verify(storageManager).sendToPool(any(StoragePool.class), any(long[].class), captor.capture()); + assertEquals(Long.valueOf(2), captor.getValue().getHost().getSanlockHostId()); + } + + @Test + public void testHostCheckSendsSanlockHostIdWhenHostReportsOne() throws Exception { + when(host.getId()).thenReturn(8L); + when(host.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(hostDetailsDao.findDetail(8L, Host.HOST_SANLOCK_ID)).thenReturn(new DetailVO(8L, Host.HOST_SANLOCK_ID, "2")); + when(agentMgr.easySend(anyLong(), any(CheckOnHostCommand.class))).thenReturn(new Answer(null, false, "down")); + checker.getHostAgentStatus(host); + ArgumentCaptor<CheckOnHostCommand> captor = ArgumentCaptor.forClass(CheckOnHostCommand.class); + verify(agentMgr).easySend(anyLong(), captor.capture()); + assertEquals(Long.valueOf(2), captor.getValue().getHost().getSanlockHostId()); + } + + @Test + public void testHostCheckSendsNoSanlockHostIdWhenHostReportsNone() throws Exception { + when(host.getId()).thenReturn(8L); + when(host.getHypervisorType()).thenReturn(HypervisorType.KVM); + when(agentMgr.easySend(anyLong(), any(CheckOnHostCommand.class))).thenReturn(new Answer(null, false, "down")); + checker.getHostAgentStatus(host); + ArgumentCaptor<CheckOnHostCommand> captor = ArgumentCaptor.forClass(CheckOnHostCommand.class); + verify(agentMgr).easySend(anyLong(), captor.capture()); + assertNull(captor.getValue().getHost().getSanlockHostId()); + } +}
diff --git a/server/src/main/java/com/cloud/storage/clvm/ClvmPoolManager.java b/server/src/main/java/com/cloud/storage/clvm/ClvmPoolManager.java index febef47..ae35560 100644 --- a/server/src/main/java/com/cloud/storage/clvm/ClvmPoolManager.java +++ b/server/src/main/java/com/cloud/storage/clvm/ClvmPoolManager.java
@@ -21,7 +21,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import javax.inject.Inject; @@ -64,6 +66,14 @@ */ public static final String CLVM_LOCK_HOST_ID = "clvmLockHostId"; + private static final List<String> TRANSIENT_LOCK_REFUSALS = List.of("locked by other host", "lock start in progress"); + + public static final ConfigKey<Integer> ClvmLockRecoveryRetryTimeout = new ConfigKey<>("Advanced", Integer.class, "clvm.lock.recovery.retry.timeout", "180", + "Maximum time in seconds, counted from the start of the CLVM lock holder lookup, to keep retrying the exclusive activation of a volume that is refused only temporarily: the previous lock holder's lease has not expired yet, or the lockspace of the volume group is still being started on the host. The lease expires 8 x io_timeout + 60 seconds after the holder stops renewing it (140 seconds with the default lvmlockd sanlock io_timeout of 10). If lvmlockd runs with a different --sanlock-timeout, set this to at least that value plus about 40 seconds. 0 disables the retry.", true); + + public static final ConfigKey<Integer> ClvmLockRecoveryRetryInterval = new ConfigKey<>("Advanced", Integer.class, "clvm.lock.recovery.retry.interval", "10", + "Time in seconds between retries of the exclusive activation of a CLVM volume that is refused only temporarily (previous lock holder's lease not expired yet, or lockspace still starting).", true); + public static final ConfigKey<Boolean> CLVMSecureZeroFill = new ConfigKey<>("Advanced", Boolean.class, "clvm.secure.zero.fill", "false", "When enabled, CLVM volumes to be zero-filled at the time of deletion to prevent data from being recovered by VMs reusing the space, as thick LVM volumes write data linearly. Note: This setting is propagated to hosts when they connect to the storage pool. Changing this setting requires disconnecting and reconnecting hosts or restarting the KVM agent for it to take effect.", false, ConfigKey.Scope.StoragePool); @@ -106,17 +116,28 @@ */ public Long getClvmLockHostId(Long volumeId, String volumeUuid, String volumePath, StoragePool pool, boolean queryActual) { + return getClvmLockHostId(volumeId, volumeUuid, volumePath, pool, queryActual, newLockRecoveryDeadlineMs()); + } + + /** Same as above, with a retry deadline shared across calls, see {@link #newLockRecoveryDeadlineMs()}. */ + public Long getClvmLockHostId(Long volumeId, String volumeUuid, String volumePath, + StoragePool pool, boolean queryActual, long activationDeadlineMs) { if (queryActual) { if (volumePath == null || pool == null) { logger.warn("Cannot query actual CLVM lock state for volume {} - missing volumePath or pool", volumeUuid); return getClvmLockHostId(volumeId, volumeUuid); } - return queryCurrentLockHolder(volumeId, volumeUuid, volumePath, pool, true); + return queryCurrentLockHolder(volumeId, volumeUuid, volumePath, pool, true, activationDeadlineMs); } return getClvmLockHostId(volumeId, volumeUuid); } + /** @return the time in milliseconds until which a refused activation is retried, counted from now */ + public long newLockRecoveryDeadlineMs() { + return now() + getRetryTimeoutMillis(); + } + /** * Safely sets or updates the CLVM_LOCK_HOST_ID detail for a volume. * If the detail already exists, it will be updated. Otherwise, it will be created. @@ -153,6 +174,12 @@ */ public Long queryCurrentLockHolder(Long volumeId, String volumeUuid, String volumePath, StoragePool pool, boolean updateDatabase) { + return queryCurrentLockHolder(volumeId, volumeUuid, volumePath, pool, updateDatabase, newLockRecoveryDeadlineMs()); + } + + /** Same as above, retrying a refused recovery activation until {@code activationDeadlineMs}. */ + public Long queryCurrentLockHolder(Long volumeId, String volumeUuid, String volumePath, + StoragePool pool, boolean updateDatabase, long activationDeadlineMs) { if (pool == null) { logger.error("Cannot query CLVM lock for volume {} - pool is null", volumeUuid); return null; @@ -164,6 +191,9 @@ } String lvPath = String.format("/dev/%s/%s", vgName, volumePath); + // Hosts that did not answer the lock-state query + Set<Long> unreachableHostIds = new HashSet<>(); + // Fast path: trust the DB record and verify with a single host query Long dbHostId = getClvmLockHostId(volumeId, volumeUuid); if (dbHostId != null) { @@ -175,6 +205,9 @@ logger.debug("Fast path: volume {} confirmed active on DB host {}", volumeUuid, dbHostId); return dbHostId; } + if (active == null) { + unreachableHostIds.add(dbHostId); + } logger.info("Fast path miss: volume {} not active on DB host {} - falling back to full fan-out", volumeUuid, dbHostId); } else { @@ -213,6 +246,8 @@ if (Boolean.TRUE.equals(active)) { logger.debug("Volume {} is locally active on host {} (fan-out)", volumeUuid, host.getId()); activeHostIds.add(host.getId()); + } else if (active == null) { + unreachableHostIds.add(host.getId()); } } @@ -220,10 +255,10 @@ logger.debug("Volume {} is not active on any reachable host — no exclusive lock held", volumeUuid); // Recovery: attempt exclusive activation on the best available host before giving up. - Long targetHostId = selectActivationTargetHost(dbHostId, hosts); + Long targetHostId = selectActivationTargetHost(dbHostId, hosts, unreachableHostIds); if (targetHostId != null) { Long recoveredHostId = tryActivateExclusivelyOnHost(volumeId, volumeUuid, lvPath, - targetHostId, updateDatabase); + targetHostId, updateDatabase, activationDeadlineMs); if (recoveredHostId != null) { return recoveredHostId; } @@ -302,12 +337,15 @@ * * <p>Priority 2: a random UP KVM routing host from the cluster/zone list (fallback). * - * @param dbHostId last known lock holder host ID from the DB (may be null) - * @param hosts routing hosts in the cluster or zone collected during fan-out + * <p>Hosts that did not answer the lock-state query are skipped in both cases. + * + * @param dbHostId last known lock holder host ID from the DB (may be null) + * @param hosts routing hosts in the cluster or zone collected during fan-out + * @param unreachableHostIds hosts that failed to answer the lock-state query * @return host ID to activate on, or null if no eligible host found */ - private Long selectActivationTargetHost(Long dbHostId, List<HostVO> hosts) { - if (dbHostId != null) { + private Long selectActivationTargetHost(Long dbHostId, List<HostVO> hosts, Set<Long> unreachableHostIds) { + if (dbHostId != null && !unreachableHostIds.contains(dbHostId)) { HostVO dbHost = _hostDao.findById(dbHostId); if (dbHost != null && dbHost.getStatus() == Status.Up && dbHost.getHypervisorType() == Hypervisor.HypervisorType.KVM) { @@ -319,7 +357,8 @@ List<HostVO> eligible = hosts.stream() .filter(h -> h.getStatus() == Status.Up && h.getType() == Host.Type.Routing - && h.getHypervisorType() == Hypervisor.HypervisorType.KVM) + && h.getHypervisorType() == Hypervisor.HypervisorType.KVM + && !unreachableHostIds.contains(h.getId())) .collect(Collectors.toList()); if (!eligible.isEmpty()) { Collections.shuffle(eligible); @@ -337,6 +376,9 @@ * Sends an {@code ACTIVATE_EXCLUSIVE} command to {@code targetHostId} and optionally * persists the new lock host to the database. * + * <p>A temporarily refused activation (lease not expired yet, lockspace still starting) is retried + * on the same host until {@code deadlineMs}. Other failures are not retried. + * * <p>Activation failure is non-fatal: returns null so the caller can apply its own fallback. * * @param volumeId volume DB ID @@ -344,30 +386,75 @@ * @param lvPath full LV device path, e.g. {@code /dev/vgname/vol-path} * @param targetHostId host to activate on * @param updateDatabase if true, persists the new lock host on success + * @param deadlineMs time in milliseconds after which a refused activation is no longer retried * @return {@code targetHostId} on success, {@code null} if the command failed or threw */ private Long tryActivateExclusivelyOnHost(Long volumeId, String volumeUuid, String lvPath, - Long targetHostId, boolean updateDatabase) { - try { - ClvmLockTransferCommand activateCmd = new ClvmLockTransferCommand( - ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE, lvPath, volumeUuid); - Answer activateAnswer = _agentMgr.send(targetHostId, activateCmd); - if (activateAnswer != null && activateAnswer.getResult()) { - logger.info("Recovery: exclusively activated volume {} on host {} (was inactive on all hosts)", - volumeUuid, targetHostId); - if (updateDatabase) { - setClvmLockHostId(volumeId, targetHostId); + Long targetHostId, boolean updateDatabase, long deadlineMs) { + long retryIntervalMs = getRetryIntervalMillis(); + while (true) { + try { + ClvmLockTransferCommand activateCmd = new ClvmLockTransferCommand( + ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE, lvPath, volumeUuid); + Answer activateAnswer = _agentMgr.send(targetHostId, activateCmd); + if (activateAnswer != null && activateAnswer.getResult()) { + logger.info("Recovery: exclusively activated volume {} on host {} (was inactive on all hosts)", + volumeUuid, targetHostId); + if (updateDatabase) { + setClvmLockHostId(volumeId, targetHostId); + } + return targetHostId; } - return targetHostId; + logger.warn("Recovery activation of volume {} on host {} failed: {}", + volumeUuid, targetHostId, + activateAnswer != null ? activateAnswer.getDetails() : "null answer"); + if (!isRetryableRefusal(activateAnswer, retryIntervalMs, deadlineMs)) { + return null; + } + logger.info("Activation of volume {} is refused temporarily, retrying on host {} in {} ms until the previous holder's lease expires or the lockspace is started", + volumeUuid, targetHostId, retryIntervalMs); + } catch (AgentUnavailableException | OperationTimedoutException e) { + logger.warn("Recovery activation of volume {} on host {} threw exception: {}", + volumeUuid, targetHostId, e.getMessage()); + return null; } - logger.warn("Recovery activation of volume {} on host {} failed: {}", - volumeUuid, targetHostId, - activateAnswer != null ? activateAnswer.getDetails() : "null answer"); - } catch (AgentUnavailableException | OperationTimedoutException e) { - logger.warn("Recovery activation of volume {} on host {} threw exception: {}", - volumeUuid, targetHostId, e.getMessage()); + if (!sleep(retryIntervalMs)) { + return null; + } } - return null; + } + + private boolean isRetryableRefusal(Answer activateAnswer, long retryIntervalMs, long deadlineMs) { + return retryIntervalMs > 0 + && activateAnswer != null + && activateAnswer.getDetails() != null + && TRANSIENT_LOCK_REFUSALS.stream().anyMatch(activateAnswer.getDetails()::contains) + && now() + retryIntervalMs <= deadlineMs; + } + + protected long getRetryTimeoutMillis() { + return ClvmLockRecoveryRetryTimeout.value() * 1000L; + } + + protected long getRetryIntervalMillis() { + return ClvmLockRecoveryRetryInterval.value() * 1000L; + } + + protected long now() { + return System.currentTimeMillis(); + } + + /** + * @return false if the thread was interrupted while waiting + */ + protected boolean sleep(long millis) { + try { + Thread.sleep(millis); + return true; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return false; + } } /** @@ -479,7 +566,9 @@ @Override public ConfigKey<?>[] getConfigKeys() { return new ConfigKey<?>[] { - CLVMSecureZeroFill + CLVMSecureZeroFill, + ClvmLockRecoveryRetryTimeout, + ClvmLockRecoveryRetryInterval }; } }
diff --git a/server/src/test/java/com/cloud/storage/ClvmPoolManagerTest.java b/server/src/test/java/com/cloud/storage/ClvmPoolManagerTest.java index 1678b37..6888e80 100644 --- a/server/src/test/java/com/cloud/storage/ClvmPoolManagerTest.java +++ b/server/src/test/java/com/cloud/storage/ClvmPoolManagerTest.java
@@ -40,9 +40,13 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.List; +import java.util.concurrent.atomic.AtomicLong; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; @@ -923,6 +927,309 @@ verify(agentMgr, never()).send(anyLong(), any(ClvmLockTransferCommand.class)); } + /** DB host is Up but times out on the query: recovery activates on the other host. */ + @Test + public void testQueryCurrentLockHolder_InactiveEverywhere_SkipsUnreachableDbHost_ActivatesOnOtherHost() + throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = mockClusterPool(); + + VolumeDetailVO detail = new VolumeDetailVO(); + detail.setValue(String.valueOf(HOST_ID_1)); + when(volsDetailsDao.findDetail(VOLUME_ID, ClvmPoolManager.CLVM_LOCK_HOST_ID)).thenReturn(detail); + + HostVO host1 = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + HostVO host2 = createMockHost(HOST_ID_2, "host2", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findById(HOST_ID_1)).thenReturn(host1); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Arrays.asList(host1, host2)); + + when(agentMgr.send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenThrow(new OperationTimedoutException(null, HOST_ID_1, 0, 0, false)); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenReturn(new ClvmLockTransferAnswer(null, true, null, null, false, false, null)); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(new Answer(null, true, null)); + + Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertEquals(HOST_ID_2, result); + verify(agentMgr, never()).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + /** No DB record, one Up host does not answer the fan-out: the fallback pick is the host that answered. */ + @Test + public void testQueryCurrentLockHolder_InactiveEverywhere_FallbackSkipsHostThatDidNotAnswer() + throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = mockClusterPool(); + when(volsDetailsDao.findDetail(VOLUME_ID, ClvmPoolManager.CLVM_LOCK_HOST_ID)).thenReturn(null); + + HostVO host1 = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + HostVO host2 = createMockHost(HOST_ID_2, "host2", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Arrays.asList(host1, host2)); + + when(agentMgr.send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenReturn(null); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenReturn(new ClvmLockTransferAnswer(null, true, null, null, false, false, null)); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(new Answer(null, true, null)); + + Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertEquals(HOST_ID_2, result); + verify(agentMgr, never()).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + /** Every Up host fails to answer: nothing is activated. */ + @Test + public void testQueryCurrentLockHolder_AllHostsUnreachable_NoActivation() + throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = mockClusterPool(); + + VolumeDetailVO detail = new VolumeDetailVO(); + detail.setValue(String.valueOf(HOST_ID_1)); + when(volsDetailsDao.findDetail(VOLUME_ID, ClvmPoolManager.CLVM_LOCK_HOST_ID)).thenReturn(detail); + + HostVO host1 = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + HostVO host2 = createMockHost(HOST_ID_2, "host2", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findById(HOST_ID_1)).thenReturn(host1); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Arrays.asList(host1, host2)); + + when(agentMgr.send(anyLong(), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenThrow(new OperationTimedoutException(null, HOST_ID_1, 0, 0, false)); + + Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + verify(agentMgr, never()).send(anyLong(), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + /** Fake clock: sleep only advances it and records the interval. */ + private static class RetryTestClvmPoolManager extends ClvmPoolManager { + private final AtomicLong clock = new AtomicLong(); + private final List<Long> sleeps = new ArrayList<>(); + private long timeoutMs = 180_000L; + + @Override + protected long now() { + return clock.get(); + } + + @Override + protected boolean sleep(long millis) { + sleeps.add(millis); + clock.addAndGet(millis); + return true; + } + + @Override + protected long getRetryTimeoutMillis() { + return timeoutMs; + } + + @Override + protected long getRetryIntervalMillis() { + return 10_000L; + } + } + + private RetryTestClvmPoolManager newRetryManager() { + RetryTestClvmPoolManager manager = new RetryTestClvmPoolManager(); + ReflectionTestUtils.setField(manager, "_volsDetailsDao", volsDetailsDao); + ReflectionTestUtils.setField(manager, "_agentMgr", agentMgr); + ReflectionTestUtils.setField(manager, "_hostDao", hostDao); + return manager; + } + + /** DB host is Down, HOST_ID_2 reports the volume inactive: recovery activates there. */ + private StoragePoolVO prepareRecoveryOnHost2() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = mockClusterPool(); + + VolumeDetailVO detail = new VolumeDetailVO(); + detail.setValue(String.valueOf(HOST_ID_1)); + when(volsDetailsDao.findDetail(VOLUME_ID, ClvmPoolManager.CLVM_LOCK_HOST_ID)).thenReturn(detail); + + HostVO downHost = createMockHost(HOST_ID_1, "host1", Status.Down, Hypervisor.HypervisorType.KVM); + HostVO upHost = createMockHost(HOST_ID_2, "host2", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findById(HOST_ID_1)).thenReturn(downHost); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Arrays.asList(downHost, upHost)); + + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenReturn(new ClvmLockTransferAnswer(null, true, null, null, false, false, null)); + return pool; + } + + private static Answer lockedByOtherHost() { + return new Answer(null, false, "lvchange -aey /dev/acsvg/test-volume-path failed: LV locked by other host: acsvg/test-volume-path"); + } + + private static Answer lockStartInProgress() { + return new Answer(null, false, "lvchange -aey /dev/acsvg/test-volume-path failed: VG acsvg lock skipped: lock start in progress Cannot access VG acsvg due to failed lock."); + } + + @Test + public void testRecoveryActivation_LockStartInProgressThenGranted_RetriesOnSameHost() throws AgentUnavailableException, OperationTimedoutException { + RetryTestClvmPoolManager manager = newRetryManager(); + StoragePoolVO pool = prepareRecoveryOnHost2(); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(lockStartInProgress(), lockedByOtherHost(), new Answer(null, true, null)); + + Long result = manager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertEquals(HOST_ID_2, result); + Assert.assertEquals(Arrays.asList(10_000L, 10_000L), manager.sleeps); + verify(agentMgr, times(3)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testRecoveryActivation_RefusedTwiceThenGranted_RetriesOnSameHost() throws AgentUnavailableException, OperationTimedoutException { + RetryTestClvmPoolManager manager = newRetryManager(); + StoragePoolVO pool = prepareRecoveryOnHost2(); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(lockedByOtherHost(), lockedByOtherHost(), new Answer(null, true, null)); + + Long result = manager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertEquals(HOST_ID_2, result); + Assert.assertEquals(Arrays.asList(10_000L, 10_000L), manager.sleeps); + verify(agentMgr, times(3)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + verify(agentMgr, never()).send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class)); + } + + @Test + public void testRecoveryActivation_RefusedUntilDeadline_GivesUp() throws AgentUnavailableException, OperationTimedoutException { + RetryTestClvmPoolManager manager = newRetryManager(); + StoragePoolVO pool = prepareRecoveryOnHost2(); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(lockedByOtherHost()); + + Long result = manager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + Assert.assertEquals(18, manager.sleeps.size()); + Assert.assertEquals(180_000L, manager.clock.get()); + verify(agentMgr, times(19)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testRecoveryActivation_DeadlineIsCountedFromLookupStart() throws AgentUnavailableException, OperationTimedoutException { + RetryTestClvmPoolManager manager = newRetryManager(); + StoragePoolVO pool = mockClusterPool(); + + VolumeDetailVO detail = new VolumeDetailVO(); + detail.setValue(String.valueOf(HOST_ID_1)); + when(volsDetailsDao.findDetail(VOLUME_ID, ClvmPoolManager.CLVM_LOCK_HOST_ID)).thenReturn(detail); + + HostVO upHost1 = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + HostVO upHost2 = createMockHost(HOST_ID_2, "host2", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findById(HOST_ID_1)).thenReturn(upHost1); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Arrays.asList(upHost1, upHost2)); + + // The query to the DB host times out after 130 seconds + when(agentMgr.send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenAnswer(invocation -> { + manager.clock.addAndGet(130_000L); + throw new OperationTimedoutException(null, HOST_ID_1, 0, 0, false); + }); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenReturn(new ClvmLockTransferAnswer(null, true, null, null, false, false, null)); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(lockedByOtherHost()); + + Long result = manager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + Assert.assertEquals(5, manager.sleeps.size()); + Assert.assertEquals(180_000L, manager.clock.get()); + verify(agentMgr, times(6)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testRecoveryActivation_OtherFailure_NotRetried() throws AgentUnavailableException, OperationTimedoutException { + RetryTestClvmPoolManager manager = newRetryManager(); + StoragePoolVO pool = prepareRecoveryOnHost2(); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(new Answer(null, false, "lvchange failed: Cannot access VG acsvg due to failed lock")); + + Long result = manager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + Assert.assertTrue(manager.sleeps.isEmpty()); + verify(agentMgr, times(1)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testRecoveryActivation_AgentException_NotRetried() throws AgentUnavailableException, OperationTimedoutException { + RetryTestClvmPoolManager manager = newRetryManager(); + StoragePoolVO pool = prepareRecoveryOnHost2(); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenThrow(new OperationTimedoutException(null, HOST_ID_2, 0, 0, false)); + + Long result = manager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + Assert.assertTrue(manager.sleeps.isEmpty()); + verify(agentMgr, times(1)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testRecoveryActivation_RetryDisabledWithZeroTimeout() throws AgentUnavailableException, OperationTimedoutException { + RetryTestClvmPoolManager manager = newRetryManager(); + manager.timeoutMs = 0L; + StoragePoolVO pool = prepareRecoveryOnHost2(); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(lockedByOtherHost()); + + Long result = manager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertNull(result); + Assert.assertTrue(manager.sleeps.isEmpty()); + verify(agentMgr, times(1)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testRecoveryActivation_SharedDeadline_SecondVolumeNotRetriedOnceBudgetIsUsed() throws AgentUnavailableException, OperationTimedoutException { + RetryTestClvmPoolManager manager = newRetryManager(); + StoragePoolVO pool = prepareRecoveryOnHost2(); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(lockedByOtherHost()); + long deadlineMs = manager.newLockRecoveryDeadlineMs(); + + Assert.assertNull(manager.getClvmLockHostId(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true, deadlineMs)); + Assert.assertNull(manager.getClvmLockHostId(VOLUME_ID + 1, "second-volume-uuid", "second-volume-path", pool, true, deadlineMs)); + + Assert.assertEquals(18, manager.sleeps.size()); + Assert.assertEquals(180_000L, manager.clock.get()); + verify(agentMgr, times(20)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testRecoveryActivation_SharedDeadline_SecondVolumeActivatedRightAfterTheFirst() throws AgentUnavailableException, OperationTimedoutException { + RetryTestClvmPoolManager manager = newRetryManager(); + StoragePoolVO pool = prepareRecoveryOnHost2(); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(lockedByOtherHost(), lockedByOtherHost(), new Answer(null, true, null), new Answer(null, true, null)); + long deadlineMs = manager.newLockRecoveryDeadlineMs(); + + Assert.assertEquals(HOST_ID_2, manager.getClvmLockHostId(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true, deadlineMs)); + Assert.assertEquals(HOST_ID_2, manager.getClvmLockHostId(VOLUME_ID + 1, "second-volume-uuid", "second-volume-path", pool, true, deadlineMs)); + + Assert.assertEquals(Arrays.asList(10_000L, 10_000L), manager.sleeps); + verify(agentMgr, times(4)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + private StoragePoolVO mockClusterPool() { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getClusterId()).thenReturn(10L); + Mockito.lenient().when(pool.getName()).thenReturn("cluster-pool"); + when(pool.getPath()).thenReturn(VG_NAME); + return pool; + } + + private static Command operation(ClvmLockTransferCommand.Operation operation) { + return Mockito.<Command>argThat(cmd -> cmd instanceof ClvmLockTransferCommand + && ((ClvmLockTransferCommand) cmd).getOperation() == operation); + } + // Helper method to create mock hosts private HostVO createMockHost(Long id, String name, Status status, Hypervisor.HypervisorType hypervisor) { HostVO host = Mockito.mock(HostVO.class);