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 ee14e6597183..1e2544076ace 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 @@ public class HostTO { private NetworkTO storageNetwork1; private NetworkTO storageNetwork2; private String parent; + private Long sanlockHostId; protected HostTO() { } @@ -46,6 +47,14 @@ public HostTO(Host vo) { } } + 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 38249ecc3366..fd5869ccb0f0 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 @@ public static String[] toStrings(Host.Type... types) { 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_VDDK_BLOCKCOPY_SUPPORT = "host.vddk.blockcopy.support"; diff --git a/core/src/main/java/org/apache/cloudstack/storage/clvm/command/ClvmLockTransferAnswer.java b/core/src/main/java/org/apache/cloudstack/storage/clvm/command/ClvmLockTransferAnswer.java index f3c43c400b2b..cf5d0e6475d5 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/clvm/command/ClvmLockTransferAnswer.java +++ b/core/src/main/java/org/apache/cloudstack/storage/clvm/command/ClvmLockTransferAnswer.java @@ -17,6 +17,8 @@ package org.apache.cloudstack.storage.clvm.command; +import java.util.List; + import com.cloud.agent.api.Answer; /** @@ -25,7 +27,17 @@ */ public class ClvmLockTransferAnswer extends Answer { + /** Mode in which the sanlock lease of a volume is held. */ + public enum LeaseMode { + FREE, EXCLUSIVE, SHARED + } + private String currentLockHostname; + private LeaseMode leaseMode; + private Long leaseOwnerId; + private boolean leaseOwnerAlive; + private boolean leaseOwnerRenewing; + private List sharedOwnerIds; private boolean isActive; private boolean isOpen; private String lvAttributes; @@ -80,6 +92,46 @@ public void setOpen(boolean open) { isOpen = open; } + public static ClvmLockTransferAnswer leaseOwner(ClvmLockTransferCommand cmd, LeaseMode mode, Long ownerId, + boolean ownerAlive, List sharedOwnerIds) { + return leaseOwner(cmd, mode, ownerId, ownerAlive, ownerAlive, sharedOwnerIds); + } + + public static ClvmLockTransferAnswer leaseOwner(ClvmLockTransferCommand cmd, LeaseMode mode, Long ownerId, + boolean ownerAlive, boolean ownerRenewing, List sharedOwnerIds) { + ClvmLockTransferAnswer answer = new ClvmLockTransferAnswer(cmd, true, + String.format("Lease %s, owner=%s, alive=%s, shared=%s", mode, ownerId, ownerAlive, sharedOwnerIds)); + answer.leaseMode = mode; + answer.leaseOwnerId = ownerId; + answer.leaseOwnerAlive = ownerAlive; + answer.leaseOwnerRenewing = ownerRenewing; + answer.sharedOwnerIds = sharedOwnerIds; + return answer; + } + + public LeaseMode getLeaseMode() { + return leaseMode; + } + + /** sanlock host id of the exclusive holder, see {@link com.cloud.host.Host#HOST_SANLOCK_ID} */ + public Long getLeaseOwnerId() { + return leaseOwnerId; + } + + /** false when the exclusive holder stopped renewing its lockspace lease or was restarted since */ + public boolean isLeaseOwnerAlive() { + return leaseOwnerAlive; + } + + /** true while the exclusive holder still renews its lockspace lease, so its lease will not expire */ + public boolean isLeaseOwnerRenewing() { + return leaseOwnerRenewing; + } + + public List getSharedOwnerIds() { + return sharedOwnerIds; + } + public String getLvAttributes() { return lvAttributes; } diff --git a/core/src/main/java/org/apache/cloudstack/storage/clvm/command/ClvmLockTransferCommand.java b/core/src/main/java/org/apache/cloudstack/storage/clvm/command/ClvmLockTransferCommand.java index 218fb798869d..ad11894eeb2a 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/clvm/command/ClvmLockTransferCommand.java +++ b/core/src/main/java/org/apache/cloudstack/storage/clvm/command/ClvmLockTransferCommand.java @@ -46,7 +46,10 @@ public enum Operation { ACTIVATE_SHARED("-asy", "activate in shared mode"), /** Query the current lock state (lvs -o lv_attr,lv_host) */ - QUERY_LOCK_STATE("query", "query lock state"); + QUERY_LOCK_STATE("query", "query lock state"), + + /** Read the sanlock lease of the volume, which names the lock holder from any host in the lockspace */ + QUERY_LEASE_OWNER("query", "query lease owner"); private final String lvchangeFlag; private final String description; 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..bf9b1d76b8d7 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 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.CLVM, StoragePoolType.CLVM_NG); 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/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 2b9b41b6e8ac..3292bfeafd79 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 @@ private Long getClvmLockHostFromVmVolumes(Long vmId) { return null; } + ClvmPoolManager.LockLookup lockLookup = clvmPoolManager.newLockLookup(); for (VolumeVO volume : vmVolumes) { if (volume.getPoolId() == null) { continue; @@ -895,7 +896,8 @@ private Long getClvmLockHostFromVmVolumes(Long vmId) { volume.getUuid(), volume.getPath(), pool, - true + true, + lockLookup ); if (lockHostId != null) { logger.debug("Found actual CLVM lock host {} from volume {} of VM {} via LVM query", @@ -912,6 +914,7 @@ private void transferClvmLocksForVmStart(List volumes, Long destHostId return; } + ClvmPoolManager.LockLookup lockLookup = clvmPoolManager.newLockLookup(destHostId); for (VolumeVO volume : volumes) { if (volume.getPoolId() == null) { continue; @@ -927,7 +930,8 @@ private void transferClvmLocksForVmStart(List volumes, Long destHostId volume.getUuid(), volume.getPath(), pool, - true + true, + lockLookup ); 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 04e80212e8cc..8ea769917782 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 @@ -135,7 +135,7 @@ public void setUp() throws Exception { accountPSResourceCount += increment; } return null; - }).when(resourceLimitMgr).incrementResourceCount(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyBoolean(), Mockito.anyLong()); + }).when(resourceLimitMgr).incrementResourceCount(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyBoolean(), Mockito.any()); Mockito.doAnswer((Answer) invocation -> { Resource.ResourceType type = (Resource.ResourceType)invocation.getArguments()[1]; Long decrement = (Long)invocation.getArguments()[3]; @@ -143,7 +143,7 @@ public void setUp() throws Exception { accountPSResourceCount -= decrement; } return null; - }).when(resourceLimitMgr).decrementResourceCount(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyBoolean(), Mockito.anyLong()); + }).when(resourceLimitMgr).decrementResourceCount(Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyBoolean(), Mockito.any()); } private void runCheckAndUpdateVolumeAccountResourceCountTest(Long originalSize, Long newSize) { @@ -696,9 +696,9 @@ public void testTransferClvmLocksForVmStart_WithClvmVolumes() throws Exception { 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.any())).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.any())).thenReturn(currentHostId); Mockito.when(clvmPoolManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(true); @@ -768,7 +768,7 @@ public void testTransferClvmLocksForVmStart_NoLockTransferNeeded() throws Except 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.any())).thenReturn(destHostId); Mockito.when(storagePoolDao.findById(poolId)).thenReturn(clvmPool); @@ -801,7 +801,7 @@ public void testTransferClvmLocksForVmStart_EmptyVolumeList() throws Exception { method.invoke(volumeOrchestrator, new ArrayList(), 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.any()); } @Test @@ -842,7 +842,7 @@ public void testTransferClvmLocksForVmStart_SetInitialLockHost() throws Exceptio 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.any())).thenReturn(null); Mockito.when(storagePoolDao.findById(poolId)).thenReturn(clvmPool); @@ -888,7 +888,7 @@ public void testTransferClvmLocksForVmStart_MixedVolumes() throws Exception { 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.any())).thenReturn(currentHostId); Mockito.when(clvmPoolManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(true); @@ -929,7 +929,7 @@ public void testTransferClvmLocksForVmStart_TransferFails() throws Throwable { 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.any())).thenReturn(currentHostId); Mockito.when(clvmPoolManager.transferClvmVolumeLock(Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong())).thenReturn(false); diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java index 7644d4688f7e..8e28ceae6bb6 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java @@ -30,6 +30,7 @@ import com.cloud.configuration.ConfigurationManager; import com.cloud.dc.dao.DataCenterDao; import com.cloud.exception.StorageConflictException; +import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; import com.cloud.network.NetworkModel; @@ -100,6 +101,8 @@ public class DefaultHostListener implements HypervisorHostListener { ConfigurationManager configManager; @Inject NetworkDao networkDao; + @Inject + ClvmPoolManager clvmPoolManager; @Override public boolean hostAdded(long hostId) { @@ -193,11 +196,27 @@ public boolean hostConnect(long hostId, long poolId) throws StorageConflictExcep storageService.updateStorageCapabilities(poolId, false); + if (ClvmPoolManager.isClvmPoolType(pool.getPoolType())) { + alertOnSharedSanlockId(pool, host); + } + logger.info("Connection established between storage pool {} and host {}", pool, host); return createPersistentNetworkResourcesOnHost(hostId); } + /** CLVM lock holder lookups map a sanlock host id to exactly one host of the pool. */ + private void alertOnSharedSanlockId(StoragePool pool, HostVO host) { + List otherHostIds = clvmPoolManager.findHostsSharingSanlockId(pool, host.getId()); + if (otherHostIds.isEmpty()) { + return; + } + String msg = String.format("Host %s reports the same %s as hosts %s of CLVM storage pool %s. Set a unique host_id in lvmlocal.conf on each host.", + host, Host.HOST_SANLOCK_ID, otherHostIds, pool); + logger.error(msg); + alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, pool.getDataCenterId(), pool.getPodId(), msg, msg); + } + private void updateStoragePoolHostVOAndDetails(StoragePool pool, long hostId, ModifyStoragePoolAnswer mspAnswer) { StoragePoolHostVO poolHost = storagePoolHostDao.findByPoolHost(pool.getId(), hostId); if (poolHost == null) { 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..1cf9b8d6ab12 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 @@ public Boolean hasHeartBeat() { 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 = doesAnyPoolHasHeartBeat(); 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 @@ public Boolean hasHeartBeat() { 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 doesAnyPoolHasHeartBeat() { + 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 a5bea90903d0..1669f2e0e6fa 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 MOUNT_BASED_POOL_TYPES = List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.SharedMountPoint); + private final Map haStoragePools = new ConcurrentHashMap<>(); private final boolean rebootHostAndAlertManagementOnHeartbeatTimeout; @@ -60,7 +62,9 @@ public void removeStoragePool(String uuid) { 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); } } @@ -78,12 +82,16 @@ public HAStoragePool getStoragePool(String uuid) { } } + private boolean isMountBased(HAStoragePool pool) { + return MOUNT_BASED_POOL_TYPES.contains(pool.getPool().getType()); + } + protected void runHeartBeat() { synchronized (haStoragePools) { Set 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 acffa395494d..4a038034c998 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 @@ -26,6 +26,7 @@ import static com.cloud.host.Host.HOST_QEMU_IO_VERSION; import static com.cloud.host.Host.HOST_QEMU_NBD_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_RBD_DIRECT_IMPORT_SUPPORT; import static com.cloud.host.Host.HOST_VDDK_SUPPORT; @@ -229,6 +230,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.clvm.ClvmSanlockHaChecker; import com.cloud.hypervisor.kvm.storage.IscsiStorageCleanupMonitor; import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk; import com.cloud.hypervisor.kvm.storage.KVMStoragePool; @@ -4465,6 +4467,7 @@ public StartupCommand[] initialize() { 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()); @@ -6273,6 +6276,13 @@ public boolean isHostSecured() { 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/resource/wrapper/LibvirtClvmLockTransferCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtClvmLockTransferCommandWrapper.java index 18c4324addf5..01008ca9c730 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtClvmLockTransferCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtClvmLockTransferCommandWrapper.java @@ -17,7 +17,12 @@ package com.cloud.hypervisor.kvm.resource.wrapper; +import java.io.File; + import com.cloud.agent.api.Answer; +import com.cloud.hypervisor.kvm.storage.clvm.ClvmSanlockLeaseReader; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.storage.clvm.command.ClvmLockTransferAnswer.LeaseMode; import org.apache.cloudstack.storage.clvm.command.ClvmLockTransferCommand; import org.apache.cloudstack.storage.clvm.command.ClvmLockTransferAnswer; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; @@ -44,6 +49,9 @@ public Answer execute(ClvmLockTransferCommand cmd, LibvirtComputingResource serv if (operation == ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE) { return handleQueryLockState(cmd, lvPath, volumeUuid); } + if (operation == ClvmLockTransferCommand.Operation.QUERY_LEASE_OWNER) { + return handleQueryLeaseOwner(cmd, lvPath, volumeUuid); + } String lvchangeOpt; String operationDesc; @@ -90,6 +98,24 @@ public Answer execute(ClvmLockTransferCommand cmd, LibvirtComputingResource serv } } + /** Reads the sanlock lease of the LV, which names the holder from any host in the lockspace. */ + private Answer handleQueryLeaseOwner(ClvmLockTransferCommand cmd, String lvPath, String volumeUuid) { + File lv = new File(lvPath); + String vgName = lv.getParentFile().getName(); + try { + ClvmSanlockLeaseReader.Lease lease = ClvmSanlockLeaseReader.readLease(vgName, lv.getName()); + ClvmSanlockLeaseReader.OwnerLiveness owner = lease.getMode() == LeaseMode.EXCLUSIVE + ? ClvmSanlockLeaseReader.readOwnerLiveness(vgName, lease) : new ClvmSanlockLeaseReader.OwnerLiveness(false, false); + logger.info("Lease of volume {}: {}, owner={}, alive={}, renewing={}, shared={}", + volumeUuid, lease.getMode(), lease.getOwnerId(), owner.isAlive(), owner.isRenewing(), lease.getSharedOwnerIds()); + return ClvmLockTransferAnswer.leaseOwner(cmd, lease.getMode(), lease.getOwnerId(), owner.isAlive(), owner.isRenewing(), + lease.getSharedOwnerIds()); + } catch (CloudRuntimeException | NumberFormatException e) { + logger.warn("Failed to read the sanlock lease of volume {}: {}", volumeUuid, e.getMessage()); + return new ClvmLockTransferAnswer(cmd, false, "Failed to read sanlock lease: " + e.getMessage()); + } + } + /** * Query whether this host currently has the CLVM LV activated locally. * Executes: lvs -o lv_attr,lv_host,lv_active --noheadings diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDestroyCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDestroyCommandWrapper.java index b9106cec42f6..bec03af6e6d7 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDestroyCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDestroyCommandWrapper.java @@ -40,7 +40,9 @@ public Answer execute(final DestroyCommand command, final LibvirtComputingResour try { final KVMStoragePoolManager storagePoolMgr = libvirtComputingResource.getStoragePoolMgr(); final KVMStoragePool pool = storagePoolMgr.getStoragePool(vol.getPoolType(), vol.getPoolUuid()); - pool.deletePhysicalDisk(vol.getPath(), null); + if (!pool.deletePhysicalDisk(vol.getPath(), null)) { + return new Answer(command, false, "Failed to delete " + vol.getPath()); + } return new Answer(command, true, "Success"); } catch (final CloudRuntimeException e) { logger.debug("Failed to delete volume: " + e.toString()); 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 e9ec65505e40..b77fc87be770 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 @@ private KVMPhysicalDisk getPhysicalDiskWithClvmFallback(String volumeUuid, KVMSt return getPhysicalDiskViaDirectBlockDevice(volumeUuid, pool); } - private String getVgName(String sourceDir) { + static String getVgName(String sourceDir) { String vgName = sourceDir; if (vgName.startsWith("/")) { String[] parts = vgName.split("/"); @@ -962,8 +962,12 @@ private boolean cleanupCLVMVolume(String uuid, KVMStoragePool pool) { String checkResult = checkLvs.execute(); if (checkResult != null) { - logger.info("CLVM volume {} does not exist in LVM (check returned: {}), considering it as already deleted", uuid, checkResult); - return true; + if (isLvNotFound(checkResult)) { + logger.info("CLVM volume {} does not exist in LVM (check returned: {}), considering it as already deleted", uuid, checkResult); + return true; + } + logger.warn("Cannot check CLVM volume {}: {}", uuid, checkResult); + return false; } logger.info("Volume {} exists, proceeding with cleanup", uuid); @@ -990,7 +994,7 @@ private boolean cleanupCLVMVolume(String uuid, KVMStoragePool pool) { return true; } else { logger.warn("Command 'lvremove -f {}' returned error: {}", lvPath, removeResult); - if (removeResult.contains("not found") || removeResult.contains("Failed to find")) { + if (isLvNotFound(removeResult)) { logger.info("CLVM volume {} not found during cleanup, considering it as already deleted", uuid); return true; } @@ -998,10 +1002,14 @@ private boolean cleanupCLVMVolume(String uuid, KVMStoragePool pool) { } } catch (Exception ex) { logger.error("Exception during CLVM volume cleanup for {}: {}", uuid, ex.getMessage(), ex); - return true; + return false; } } + private boolean isLvNotFound(String lvmError) { + return lvmError.contains("not found") || lvmError.contains("Failed to find"); + } + private boolean shouldSecureZeroFill(KVMStoragePool pool) { Map details = pool.getDetails(); String secureZeroFillStr = (details != null) ? details.get(KVMStoragePool.CLVM_SECURE_ZERO_FILL) : null; 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 4a578ccf341d..f1b86c511860 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 @@ public boolean disconnectPhysicalDisk(StoragePoolType type, String poolUuid, Str 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.unregisterStoragePool(uuid); } boolean deleteStatus = adaptor.deleteStoragePool(uuid); @@ -457,13 +462,12 @@ public synchronized boolean deleteStoragePool(StoragePoolType type, String uuid) public boolean deleteStoragePool(StoragePoolType type, String uuid, Map details) { StorageAdaptor adaptor = getStorageAdaptor(type); - boolean deleteStatus; - if (type == StoragePoolType.NetworkFilesystem) { + if (isHaPoolRemovedOnDelete(type)) { _haMonitor.unregisterStoragePool(uuid); - deleteStatus = adaptor.deleteStoragePool(uuid); - } else { - deleteStatus = adaptor.deleteStoragePool(uuid, details); } + boolean deleteStatus = type == StoragePoolType.NetworkFilesystem + ? adaptor.deleteStoragePool(uuid) + : adaptor.deleteStoragePool(uuid, details); synchronized (_storagePools) { _storagePools.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 7a70f2497cda..da1cb72efb40 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 @@ -34,8 +34,10 @@ import com.cloud.agent.properties.AgentPropertiesFileHandler; import com.cloud.ha.HighAvailabilityManager; import com.cloud.hypervisor.kvm.resource.KVMHABase.HAStoragePool; +import com.cloud.hypervisor.kvm.storage.clvm.ClvmSanlockHaChecker; 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; @@ -375,7 +377,14 @@ public String getHearthBeatPath() { } + 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()); @@ -402,6 +411,9 @@ public String getStorageNodeId() { @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()); @@ -427,6 +439,9 @@ public Boolean hasHeartBeat(HAStoragePool pool, HostTO host) { @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/com/cloud/hypervisor/kvm/storage/clvm/ClvmSanlockHaChecker.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/clvm/ClvmSanlockHaChecker.java new file mode 100644 index 000000000000..8f1630f2abb9 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/clvm/ClvmSanlockHaChecker.java @@ -0,0 +1,164 @@ +// 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.clvm; + +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; + final Long generation; + + LeaseState(long lastCheck, long lastLive, long ioTimeout, Long generation) { + this.lastCheck = lastCheck; + this.lastLive = lastLive; + this.ioTimeout = ioTimeout; + this.generation = generation; + } + } + + /** 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); + } + + 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; + Long generation = 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); + generation = valueOrCurrent(line, "owner_generation", generation); + } + } + if (lastCheck == null || lastLive == null || ioTimeout == null) { + return null; + } + return new LeaseState(lastCheck, lastLive, ioTimeout, generation); + } + + 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/clvm/ClvmSanlockLeaseReader.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/clvm/ClvmSanlockLeaseReader.java new file mode 100644 index 000000000000..7e79f241a17f --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/clvm/ClvmSanlockLeaseReader.java @@ -0,0 +1,196 @@ +// 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.clvm; + +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.cloudstack.storage.clvm.command.ClvmLockTransferAnswer.LeaseMode; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.OutputInterpreter; +import com.cloud.utils.script.Script; + +/** + * Reads the sanlock lease of a CLVM/CLVM_NG volume from the VG's lvmlock LV. Every host in the + * lockspace can read it, so one host names the lock holder of the volume. + */ +public class ClvmSanlockLeaseReader { + + private static final Logger LOGGER = LogManager.getLogger(ClvmSanlockLeaseReader.class); + private static final long TIMEOUT_MS = 30000; + private static final long LEASE_SIZE_BYTES = 1048576; + private static final long RENEWING_IO_TIMEOUTS = 3; + private static final Pattern SHARED_HOLDER = Pattern.compile("^\\s*(\\d+)\\s+\\d+\\s+SH\\s*$"); + + /** Lease of one volume as seen on disk. */ + public static class Lease { + final LeaseMode mode; + final Long ownerId; + final Long ownerGeneration; + final List sharedOwnerIds; + + Lease(LeaseMode mode, Long ownerId, Long ownerGeneration, List sharedOwnerIds) { + this.mode = mode; + this.ownerId = ownerId; + this.ownerGeneration = ownerGeneration; + this.sharedOwnerIds = sharedOwnerIds; + } + + public LeaseMode getMode() { + return mode; + } + + public Long getOwnerId() { + return ownerId; + } + + public List getSharedOwnerIds() { + return sharedOwnerIds; + } + } + + public static Lease readLease(String vgName, String lvName) { + String[] uuidAndOffset = readLockArgs(vgName, lvName); + String dump = dumpLease(vgName, uuidAndOffset[1]); + return parseDump(dump, uuidAndOffset[0]); + } + + /** Liveness of the exclusive holder, as seen in this host's view of the lockspace. */ + public static class OwnerLiveness { + final boolean alive; + final boolean renewing; + + public OwnerLiveness(boolean alive, boolean renewing) { + this.alive = alive; + this.renewing = renewing; + } + + public boolean isAlive() { + return alive; + } + + public boolean isRenewing() { + return renewing; + } + } + + public static OwnerLiveness readOwnerLiveness(String vgName, Lease lease) { + String status = ClvmSanlockHaChecker.readHostStatus(vgName); + ClvmSanlockHaChecker.LeaseState state = status == null ? null : ClvmSanlockHaChecker.parseLeaseState(status, lease.ownerId); + return new OwnerLiveness(isOwnerAlive(state, lease), isOwnerRenewing(state, lease)); + } + + /** True while the holder renews its lockspace lease, so its locks will not expire. Unknown counts as renewing. */ + protected static boolean isOwnerRenewing(ClvmSanlockHaChecker.LeaseState state, Lease lease) { + if (state == null) { + return true; + } + if (state.generation != null && !state.generation.equals(lease.ownerGeneration)) { + return false; + } + return state.lastCheck - state.lastLive < RENEWING_IO_TIMEOUTS * state.ioTimeout; + } + + protected static boolean isOwnerAlive(ClvmSanlockHaChecker.LeaseState state, Lease lease) { + if (state == null) { + LOGGER.warn("No lockspace lease found for sanlock host id [{}], assuming it is alive.", lease.ownerId); + return true; + } + if (state.generation != null && !state.generation.equals(lease.ownerGeneration)) { + return false; + } + return ClvmSanlockHaChecker.isLeaseAlive(state); + } + + /** @return LV uuid (the sanlock resource name) and the lease offset in the lvmlock LV */ + private static String[] readLockArgs(String vgName, String lvName) { + Script cmd = new Script("lvs", TIMEOUT_MS, LOGGER); + cmd.add("--noheadings", "-o", "lv_uuid,lv_lockargs", vgName + "/" + lvName); + OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser(); + String result = cmd.execute(parser); + if (result != null) { + throw new CloudRuntimeException(String.format("lvs failed for %s/%s: %s", vgName, lvName, result)); + } + return parseLockArgs(parser.getLines()); + } + + /** Skips warning lines, e.g. "VG ... lock skipped: lock start in progress" while the lockspace is joining. */ + protected static String[] parseLockArgs(String output) { + for (String line : output == null ? new String[0] : output.split("\\r?\\n")) { + String[] parts = line.trim().split("\\s+"); + if (parts.length == 2 && parts[1].matches("^[\\d.]+:\\d+$")) { + return new String[] {parts[0], parts[1].substring(parts[1].lastIndexOf(':') + 1)}; + } + } + throw new CloudRuntimeException(String.format("No sanlock lock args in lvs output [%s]", output)); + } + + private static String dumpLease(String vgName, String offset) { + Script cmd = new Script("sanlock", TIMEOUT_MS, LOGGER); + cmd.add("direct", "dump", String.format("%s:%s:%d", lvmlockDevice(vgName), offset, LEASE_SIZE_BYTES)); + OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser(); + String result = cmd.execute(parser); + if (result != null) { + throw new CloudRuntimeException(String.format("sanlock direct dump failed for VG %s: %s", vgName, result)); + } + return parser.getLines(); + } + + protected static String lvmlockDevice(String vgName) { + return "/dev/mapper/" + vgName.replace("-", "--") + "-lvmlock"; + } + + /** + * Parses a {@code sanlock direct dump} of one lease: a row "offset lockspace resource timestamp own gen lver", + * followed by one "host_id gen SH" row per shared holder. A non-zero timestamp means held exclusively. + */ + protected static Lease parseDump(String output, String lvUuid) { + Long timestamp = null; + Long owner = null; + Long generation = null; + List sharedOwnerIds = new ArrayList<>(); + for (String line : output.split("\\r?\\n")) { + String[] tokens = line.trim().split("\\s+"); + if (timestamp == null) { + if (tokens.length >= 6 && tokens[2].equals(lvUuid)) { + timestamp = Long.parseLong(tokens[3]); + owner = Long.parseLong(tokens[4]); + generation = Long.parseLong(tokens[5]); + } + continue; + } + Matcher shared = SHARED_HOLDER.matcher(line); + if (!shared.matches()) { + break; + } + sharedOwnerIds.add(Long.parseLong(shared.group(1))); + } + if (timestamp == null) { + throw new CloudRuntimeException(String.format("No sanlock lease for LV uuid %s in dump output", lvUuid)); + } + if (timestamp != 0) { + return new Lease(LeaseMode.EXCLUSIVE, owner, generation, sharedOwnerIds); + } + return new Lease(sharedOwnerIds.isEmpty() ? LeaseMode.FREE : LeaseMode.SHARED, null, null, sharedOwnerIds); + } +} 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 96fd42d4d544..02327b2b1a24 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 @@ public class KVMHostActivityChecker extends AdapterBase implements ActivityCheck 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 @@ private Status checkHostStatusWithSameHost(Host host) { 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 @@ private Status checkHostStatusWithNeighbourHosts(Host host) { Status hostStatusFromNeighbour = Status.Unknown; boolean reportFailureIfOneStorageIsDown = HighAvailabilityManager.KvmHAFenceHostIfHeartbeatFailsOnStorage.value(); final CheckOnHostCommand cmd = new CheckOnHostCommand(host, reportFailureIfOneStorageIsDown); + addSanlockHostId(cmd.getHost(), host); List neighbors = resourceManager.listHostsInClusterByStatus(host.getClusterId(), Status.Up); for (HostVO neighbor : neighbors) { if (neighbor.getId() == host.getId() @@ -204,6 +213,9 @@ private boolean hasVMActivityOnHost(Host host, DateTime suspectTime) throws HACh protected boolean verifyActivityOfStorageOnHost(HashMap> poolVolMap, StoragePool pool, Host host, DateTime suspectTime, boolean activityStatus) throws HACheckerException, IllegalStateException { List 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 @@ protected boolean verifyActivityOfStorageOnHost(HashMap> getStoragePoolAndVolumeInfoOnHost(Host host) { List vmListOnHost = vmInstanceDao.listByHostId(host.getId()); List 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 000000000000..5f8c04f392a5 --- /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 poolsWithHeartBeats(boolean... heartBeats) { + when(host.getPrivateNetwork()).thenReturn(privateNetwork); + List 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/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index 619aab27010e..b09067674dff 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -2269,6 +2269,7 @@ public void testDestroyCommand() { when(libvirtComputingResourceMock.getStoragePoolMgr()).thenReturn(poolManager); when(poolManager.getStoragePool(vol.getPoolType(), vol.getPoolUuid())).thenReturn(primary); + when(primary.deletePhysicalDisk(vol.getPath(), null)).thenReturn(true); final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); assertNotNull(wrapper); @@ -2280,6 +2281,22 @@ public void testDestroyCommand() { verify(poolManager, times(1)).getStoragePool(vol.getPoolType(), vol.getPoolUuid()); } + @Test + public void testDestroyCommandDeleteFailed() { + final DestroyCommand command = new DestroyCommand(Mockito.mock(StoragePool.class), Mockito.mock(Volume.class), "Test"); + final KVMStoragePoolManager poolManager = Mockito.mock(KVMStoragePoolManager.class); + final KVMStoragePool primary = Mockito.mock(KVMStoragePool.class); + final VolumeTO vol = command.getVolume(); + + when(libvirtComputingResourceMock.getStoragePoolMgr()).thenReturn(poolManager); + when(poolManager.getStoragePool(vol.getPoolType(), vol.getPoolUuid())).thenReturn(primary); + when(primary.deletePhysicalDisk(vol.getPath(), null)).thenReturn(false); + + final Answer answer = LibvirtRequestWrapper.getInstance().execute(command, libvirtComputingResourceMock); + + assertFalse(answer.getResult()); + } + @SuppressWarnings("unchecked") @Test public void testDestroyCommandError() { diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ClvmStorageAdaptorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ClvmStorageAdaptorTest.java index 1736c1d3f980..121f6b1ce4d9 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ClvmStorageAdaptorTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ClvmStorageAdaptorTest.java @@ -36,6 +36,7 @@ import java.util.Map; import java.util.UUID; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -821,6 +822,38 @@ public void testCleanupCLVMVolume_VolumeNotFound() throws Exception { assert result : "Should return true when volume not found (already deleted)"; } + @Test + public void testCleanupCLVMVolume_LockedByOtherHost() throws Exception { + Mockito.when(mockPool.getLocalPath()).thenReturn("testvg"); + Mockito.when(mockPool.getUuid()).thenReturn(UUID.randomUUID().toString()); + + mockScriptConstruction = Mockito.mockConstruction(Script.class, (mock, context) -> { + boolean lvremove = "lvremove".equals(context.arguments().get(0)); + when(mock.execute()).thenReturn(lvremove ? " LV locked by other host: testvg/template-1" : null); + }); + + Method method = ClvmStorageAdaptor.class.getDeclaredMethod( + "cleanupCLVMVolume", String.class, KVMStoragePool.class); + method.setAccessible(true); + + Assert.assertFalse((boolean) method.invoke(clvmStorageAdaptor, "template-1", mockPool)); + } + + @Test + public void testCleanupCLVMVolume_CheckFailsForOtherReason() throws Exception { + Mockito.when(mockPool.getLocalPath()).thenReturn("testvg"); + Mockito.when(mockPool.getUuid()).thenReturn(UUID.randomUUID().toString()); + + mockScriptConstruction = Mockito.mockConstruction(Script.class, + (mock, context) -> when(mock.execute()).thenReturn(" Lock manager lvmlockd is not running")); + + Method method = ClvmStorageAdaptor.class.getDeclaredMethod( + "cleanupCLVMVolume", String.class, KVMStoragePool.class); + method.setAccessible(true); + + Assert.assertFalse((boolean) method.invoke(clvmStorageAdaptor, "template-1", mockPool)); + } + @Test public void testCleanupCLVMVolume_NullSourceDir() throws Exception { String volumeUuid = UUID.randomUUID().toString(); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/clvm/ClvmSanlockHaCheckerTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/clvm/ClvmSanlockHaCheckerTest.java new file mode 100644 index 000000000000..1cddff8bc888 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/clvm/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.clvm; + +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, 1L))); + assertFalse(ClvmSanlockHaChecker.isLeaseAlive(new ClvmSanlockHaChecker.LeaseState(1140, 1000, 10, 1L))); + } + + @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/com/cloud/hypervisor/kvm/storage/clvm/ClvmSanlockLeaseReaderTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/clvm/ClvmSanlockLeaseReaderTest.java new file mode 100644 index 000000000000..b935a7fd5796 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/clvm/ClvmSanlockLeaseReaderTest.java @@ -0,0 +1,124 @@ +// 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.clvm; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; +import java.util.Collections; + +import org.apache.cloudstack.storage.clvm.command.ClvmLockTransferAnswer.LeaseMode; +import org.junit.Test; + +import com.cloud.utils.exception.CloudRuntimeException; + +public class ClvmSanlockLeaseReaderTest { + + private static final String HEADER = " offset lockspace resource timestamp own gen lver\n"; + + // Captured with sanlock 3.8.4: "sanlock direct dump /dev/mapper/sharedvg-lvmlock::1048576" + private static final String EXCLUSIVE = HEADER + + "75497472 lvm_sharedvg 8Oicxc-uWrd-HEQB-L1Rr-IGZF-L08Y-IMmOoK 0001213684 0002 0001 1\n"; + + private static final String SHARED = HEADER + + "74448896 lvm_sharedvg OBe1FA-KGjN-RsXD-j93O-ovWz-uyM7-Gbro7e 0000000000 0002 0001 3\n" + + " 0001 0002 SH\n" + + " 0002 0001 SH\n"; + + private static final String RELEASED = HEADER + + "69206016 lvm_sharedvg VGLK 0000000000 0002 0001 54890\n"; + + @Test + public void testParseDumpExclusive() { + ClvmSanlockLeaseReader.Lease lease = ClvmSanlockLeaseReader.parseDump(EXCLUSIVE, "8Oicxc-uWrd-HEQB-L1Rr-IGZF-L08Y-IMmOoK"); + assertEquals(LeaseMode.EXCLUSIVE, lease.getMode()); + assertEquals(Long.valueOf(2), lease.getOwnerId()); + assertEquals(Long.valueOf(1), lease.ownerGeneration); + } + + @Test + public void testParseDumpShared() { + ClvmSanlockLeaseReader.Lease lease = ClvmSanlockLeaseReader.parseDump(SHARED, "OBe1FA-KGjN-RsXD-j93O-ovWz-uyM7-Gbro7e"); + assertEquals(LeaseMode.SHARED, lease.getMode()); + assertNull(lease.getOwnerId()); + assertEquals(Arrays.asList(1L, 2L), lease.getSharedOwnerIds()); + } + + @Test + public void testParseDumpReleasedIgnoresStaleOwner() { + ClvmSanlockLeaseReader.Lease lease = ClvmSanlockLeaseReader.parseDump(RELEASED, "VGLK"); + assertEquals(LeaseMode.FREE, lease.getMode()); + assertNull(lease.getOwnerId()); + assertEquals(Collections.emptyList(), lease.getSharedOwnerIds()); + } + + @Test(expected = CloudRuntimeException.class) + public void testParseDumpWithoutTheLease() { + ClvmSanlockLeaseReader.parseDump(EXCLUSIVE, "other-uuid"); + } + + @Test + public void testParseLockArgs() { + assertArrayEquals(new String[] {"rQXPx2-5Ecb-PdRO-oUlx-VqV9-RIYu-thyuIm", "73400320"}, + ClvmSanlockLeaseReader.parseLockArgs(" rQXPx2-5Ecb-PdRO-oUlx-VqV9-RIYu-thyuIm 1.0.0:73400320")); + } + + @Test + public void testParseLockArgsSkipsLockStartWarning() { + assertArrayEquals(new String[] {"rQXPx2-5Ecb-PdRO-oUlx-VqV9-RIYu-thyuIm", "73400320"}, + ClvmSanlockLeaseReader.parseLockArgs(" VG sharedvg2 lock skipped: lock start in progress\n" + + " rQXPx2-5Ecb-PdRO-oUlx-VqV9-RIYu-thyuIm 1.0.0:73400320\n")); + } + + @Test(expected = CloudRuntimeException.class) + public void testParseLockArgsWithoutSanlockLock() { + ClvmSanlockLeaseReader.parseLockArgs(" rQXPx2-5Ecb-PdRO-oUlx-VqV9-RIYu-thyuIm"); + } + + @Test + public void testLvmlockDeviceEscapesDashes() { + assertEquals("/dev/mapper/sharedvg-lvmlock", ClvmSanlockLeaseReader.lvmlockDevice("sharedvg")); + assertEquals("/dev/mapper/shared--vg-lvmlock", ClvmSanlockLeaseReader.lvmlockDevice("shared-vg")); + } + + @Test + public void testOwnerAliveOnlyWithSameGeneration() { + ClvmSanlockLeaseReader.Lease lease = ClvmSanlockLeaseReader.parseDump(EXCLUSIVE, "8Oicxc-uWrd-HEQB-L1Rr-IGZF-L08Y-IMmOoK"); + assertTrue(ClvmSanlockLeaseReader.isOwnerAlive(new ClvmSanlockHaChecker.LeaseState(1100, 1100, 10, 1L), lease)); + assertFalse(ClvmSanlockLeaseReader.isOwnerAlive(new ClvmSanlockHaChecker.LeaseState(1100, 1100, 10, 2L), lease)); + } + + @Test + public void testOwnerRenewingOnlyWithFreshLeaseAndSameGeneration() { + ClvmSanlockLeaseReader.Lease lease = ClvmSanlockLeaseReader.parseDump(EXCLUSIVE, "8Oicxc-uWrd-HEQB-L1Rr-IGZF-L08Y-IMmOoK"); + assertTrue(ClvmSanlockLeaseReader.isOwnerRenewing(new ClvmSanlockHaChecker.LeaseState(1020, 1000, 10, 1L), lease)); + assertFalse(ClvmSanlockLeaseReader.isOwnerRenewing(new ClvmSanlockHaChecker.LeaseState(1030, 1000, 10, 1L), lease)); + assertFalse(ClvmSanlockLeaseReader.isOwnerRenewing(new ClvmSanlockHaChecker.LeaseState(1020, 1000, 10, 2L), lease)); + assertTrue(ClvmSanlockLeaseReader.isOwnerRenewing(null, lease)); + } + + @Test + public void testOwnerNotAliveWhenLockspaceLeaseStoppedRenewing() { + ClvmSanlockLeaseReader.Lease lease = ClvmSanlockLeaseReader.parseDump(EXCLUSIVE, "8Oicxc-uWrd-HEQB-L1Rr-IGZF-L08Y-IMmOoK"); + assertFalse(ClvmSanlockLeaseReader.isOwnerAlive(new ClvmSanlockHaChecker.LeaseState(1140, 1000, 10, 1L), lease)); + } +} 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 000000000000..c4902517ae71 --- /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> 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 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 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 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 febef471e683..3bc718ff88fa 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,21 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; import java.util.stream.Collectors; import javax.inject.Inject; @@ -32,17 +46,23 @@ 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.HostDao; +import com.cloud.host.dao.HostDetailsDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.Storage; import com.cloud.storage.StoragePool; import com.cloud.storage.VolumeDetailVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.VolumeDetailsDao; +import com.cloud.utils.concurrency.NamedThreadFactory; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; +import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.storage.clvm.command.ClvmLockTransferCommand; import org.apache.cloudstack.storage.clvm.command.ClvmLockTransferAnswer; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.stereotype.Component; @@ -55,6 +75,10 @@ public class ClvmPoolManager implements Configurable { private AgentManager _agentMgr; @Inject private HostDao _hostDao; + @Inject + private HostDetailsDao _hostDetailsDao; + @Inject + private PrimaryDataStoreDao _storagePoolDao; protected Logger logger = LogManager.getLogger(getClass()); @@ -64,13 +88,121 @@ public class ClvmPoolManager implements Configurable { */ public static final String CLVM_LOCK_HOST_ID = "clvmLockHostId"; + private static final String LOCK_START_IN_PROGRESS = "lock start in progress"; + private static final List TRANSIENT_LOCK_REFUSALS = List.of("locked by other host", LOCK_START_IN_PROGRESS); + + public static final ConfigKey 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 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 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); + public static final ConfigKey ClvmLockQueryThreads = new ConfigKey<>("Advanced", Integer.class, "clvm.lock.query.threads", "5", + "Number of threads a management server uses to send CLVM lock commands to several hosts in parallel: the fallback lock holder lookup that asks every host, and the deactivation of a template LV on all hosts before it is removed. Requires a management server restart.", false); + + private static final int MAX_LEASE_READERS = 2; + + private ExecutorService lockQueryExecutor; + + /** State shared by the lock lookups of all volumes of one operation. */ + public static class LockLookup { + private final long activationDeadlineMs; + private final Long preferredHostId; + private final Set unreachableHostIds = ConcurrentHashMap.newKeySet(); + + LockLookup(long activationDeadlineMs, Long preferredHostId) { + this.activationDeadlineMs = activationDeadlineMs; + this.preferredHostId = preferredHostId; + } + + boolean isUnreachable(Long hostId) { + return unreachableHostIds.contains(hostId); + } + } + + /** Created on first use, as the config value is not available when the bean is constructed. */ + private synchronized ExecutorService getLockQueryExecutor() { + if (lockQueryExecutor == null) { + int threads = Math.max(1, ClvmLockQueryThreads.value()); + ThreadPoolExecutor executor = new ThreadPoolExecutor(threads, threads, 60L, TimeUnit.SECONDS, + new LinkedBlockingQueue<>(), new NamedThreadFactory("ClvmLockQuery")); + executor.allowCoreThreadTimeOut(true); + lockQueryExecutor = executor; + } + return lockQueryExecutor; + } + public static boolean isClvmPoolType(Storage.StoragePoolType poolType) { return Arrays.asList(Storage.StoragePoolType.CLVM, Storage.StoragePoolType.CLVM_NG).contains(poolType); } + private static String getVgName(StoragePool pool) { + String vgName = pool.getPath(); + return vgName.startsWith("/") ? vgName.substring(1) : vgName; + } + + private static String getLvPath(StoragePool pool, String lvName) { + return String.format("/dev/%s/%s", getVgName(pool), lvName); + } + + /** + * Pools of the same zone backed by the same VG share its LVs, e.g. one template LV used as backing file by volumes of all of them. + * + * @return ids of the pools sharing the VG of {@code pool}, including {@code pool} + */ + public Set findPoolIdsSharingVg(StoragePool pool) { + String vgName = getVgName(pool); + Set poolIds = _storagePoolDao.findPoolsByStorageTypeAndZone(pool.getPoolType(), pool.getDataCenterId()).stream() + .filter(p -> vgName.equals(getVgName(p))) + .map(StoragePoolVO::getId) + .collect(Collectors.toCollection(HashSet::new)); + poolIds.add(pool.getId()); + return poolIds; + } + + /** + * Deactivates an LV on every Up host of the pools sharing its VG, releasing their shared locks so it can be removed. + * Best effort: a host still using the LV refuses, and removing it then fails on the lock. + */ + public void deactivateOnAllHosts(StoragePool pool, String lvName) { + String lvPath = getLvPath(pool, lvName); + Map hosts = new LinkedHashMap<>(); + findPoolIdsSharingVg(pool).stream() + .map(_storagePoolDao::findById) + .filter(Objects::nonNull) + .flatMap(p -> findPoolHosts(p).stream()) + .filter(h -> h.getHypervisorType() == Hypervisor.HypervisorType.KVM) + .forEach(h -> hosts.putIfAbsent(h.getId(), h)); + List upHostIds = hosts.values().stream().filter(this::isUpKvmRoutingHost).map(HostVO::getId).collect(Collectors.toList()); + Map results = callOnHosts(upHostIds, lvName, hostId -> deactivateOnHost(hosts.get(hostId), lvPath, lvName)); + List notDeactivated = hosts.values().stream() + .filter(h -> !Boolean.TRUE.equals(results.get(h.getId()))) + .map(h -> String.format("%s (%s, %s)", h.getName(), h.getUuid(), h.getStatus())) + .collect(Collectors.toList()); + if (notDeactivated.isEmpty()) { + logger.info("Deactivated {} on all hosts {}", lvPath, hosts.values().stream().map(HostVO::getName).collect(Collectors.toList())); + return; + } + logger.warn("{} could not be deactivated on hosts {}. It cannot be removed while active there; deactivate it on those hosts with 'lvchange -an {}'.", + lvPath, notDeactivated, lvPath); + } + + private Boolean deactivateOnHost(HostVO host, String lvPath, String lvName) { + try { + Answer answer = _agentMgr.send(host.getId(), new ClvmLockTransferCommand(ClvmLockTransferCommand.Operation.DEACTIVATE, lvPath, lvName)); + if (answer != null && answer.getResult()) { + return true; + } + logger.warn("Failed to deactivate {} on host {}: {}", lvPath, host.getName(), answer != null ? answer.getDetails() : "null answer"); + } catch (AgentUnavailableException | OperationTimedoutException e) { + logger.warn("Could not deactivate {} on host {}: {}", lvPath, host.getName(), e.getMessage()); + } + return false; + } + /** * Gets the CLVM lock host ID for a volume, optionally querying actual LVM state. * @@ -106,17 +238,33 @@ public Long getClvmLockHostId(Long volumeId, String volumeUuid) { */ public Long getClvmLockHostId(Long volumeId, String volumeUuid, String volumePath, StoragePool pool, boolean queryActual) { + return getClvmLockHostId(volumeId, volumeUuid, volumePath, pool, queryActual, newLockLookup()); + } + + /** Same as above, sharing {@code lookup} with the other volumes of the same operation, see {@link #newLockLookup()}. */ + public Long getClvmLockHostId(Long volumeId, String volumeUuid, String volumePath, + StoragePool pool, boolean queryActual, LockLookup lookup) { 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, lookup); } return getClvmLockHostId(volumeId, volumeUuid); } + /** @return lookup state for one operation; its activation retry deadline is counted from now */ + public LockLookup newLockLookup() { + return newLockLookup(null); + } + + /** Same as above; a volume no host holds is activated on {@code preferredHostId} when possible. */ + public LockLookup newLockLookup(Long preferredHostId) { + return new LockLookup(now() + getRetryTimeoutMillis(), preferredHostId); + } + /** * 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,90 +301,110 @@ public void setClvmLockHostId(long volumeId, long hostId) { */ public Long queryCurrentLockHolder(Long volumeId, String volumeUuid, String volumePath, StoragePool pool, boolean updateDatabase) { + return queryCurrentLockHolder(volumeId, volumeUuid, volumePath, pool, updateDatabase, newLockLookup()); + } + + /** Same as above, sharing the retry deadline and the unreachable hosts in {@code lookup}. */ + public Long queryCurrentLockHolder(Long volumeId, String volumeUuid, String volumePath, + StoragePool pool, boolean updateDatabase, LockLookup lookup) { if (pool == null) { logger.error("Cannot query CLVM lock for volume {} - pool is null", volumeUuid); return null; } - String vgName = pool.getPath(); - if (vgName.startsWith("/")) { - vgName = vgName.substring(1); + String lvPath = getLvPath(pool, volumePath); + Long dbHostId = getClvmLockHostId(volumeId, volumeUuid); + // Hosts that did not answer the lock-state query for this volume + Set noAnswerHostIds = new HashSet<>(); + + Boolean dbHostActive = queryDbHostLockState(dbHostId, lvPath, volumeUuid, noAnswerHostIds, lookup); + if (Boolean.TRUE.equals(dbHostActive)) { + logger.debug("Fast path: volume {} confirmed active on DB host {}", volumeUuid, dbHostId); + return dbHostId; } - String lvPath = String.format("/dev/%s/%s", vgName, volumePath); - // Fast path: trust the DB record and verify with a single host query - Long dbHostId = getClvmLockHostId(volumeId, volumeUuid); - if (dbHostId != null) { - HostVO dbHost = _hostDao.findById(dbHostId); - if (dbHost != null && dbHost.getStatus() == Status.Up - && dbHost.getHypervisorType() == Hypervisor.HypervisorType.KVM) { - Boolean active = querySingleHostLockState(dbHostId, lvPath, volumeUuid); - if (Boolean.TRUE.equals(active)) { - logger.debug("Fast path: volume {} confirmed active on DB host {}", volumeUuid, dbHostId); - return dbHostId; + List hosts = findPoolHosts(pool); + + ClvmLockTransferAnswer lease = hosts.isEmpty() ? null : readLease(lvPath, volumeUuid, dbHostId, hosts, lookup); + if (lease != null) { + if (isLeaseFree(lease, volumeUuid)) { + return recoverInactiveVolume(volumeId, volumeUuid, lvPath, dbHostId, hosts, noAnswerHostIds, updateDatabase, lookup); + } + List holderSanlockIds = getLeaseHolderIds(lease); + if (holderSanlockIds.size() > 1) { + logger.warn("Volume {} is held in shared mode by sanlock hosts {}. Skipping exclusive lock transfer.", + volumeUuid, holderSanlockIds); + return null; + } + Long lockHostId = findHostBySanlockId(hosts, holderSanlockIds.get(0)); + if (lockHostId != null) { + if (isLeaseHolderInUse(lockHostId, dbHostId, dbHostActive, lease.isLeaseOwnerRenewing(), lvPath, volumeUuid, lookup)) { + logger.info("Volume {} is held by host {} (sanlock lease, DB had {})", volumeUuid, lockHostId, dbHostId); + return recordLockHolder(volumeId, volumeUuid, dbHostId, lockHostId, updateDatabase); } - logger.info("Fast path miss: volume {} not active on DB host {} - falling back to full fan-out", - volumeUuid, dbHostId); - } else { - logger.info("Fast path skip: DB host {} for volume {} is down/missing — falling back to full fan-out", - dbHostId, volumeUuid); + return recoverInactiveVolume(volumeId, volumeUuid, lvPath, dbHostId, hosts, noAnswerHostIds, updateDatabase, lookup); } } + return findLockHolderByFanOut(volumeId, volumeUuid, lvPath, pool, dbHostId, hosts, noAnswerHostIds, updateDatabase, lookup); + } - List hosts = null; - Long clusterId = pool.getClusterId(); - if (clusterId != null) { - hosts = _hostDao.findByClusterId(clusterId, Host.Type.Routing); - } else if (pool.getDataCenterId() > 0) { - hosts = _hostDao.findByDataCenterId(pool.getDataCenterId()); + /** + * Fast path: asks the host recorded in the DB, as an LV is only active on the host holding its lock. + * + * @return the lock state on the DB host, or null when it was not asked or did not answer + */ + private Boolean queryDbHostLockState(Long dbHostId, String lvPath, String volumeUuid, Set noAnswerHostIds, LockLookup lookup) { + if (dbHostId == null) { + return null; } - if (hosts == null || hosts.isEmpty()) { - logger.warn("No KVM routing hosts found to query CLVM lock state for volume {} (pool: {}, cluster: {}, zone: {})", - volumeUuid, pool.getName(), clusterId, pool.getDataCenterId()); + if (lookup.isUnreachable(dbHostId)) { + logger.info("Fast path skip: DB host {} for volume {} did not answer an earlier query", dbHostId, volumeUuid); return null; } + if (!isUpKvmRoutingHost(_hostDao.findById(dbHostId))) { + logger.info("Fast path skip: DB host {} for volume {} is down/missing", dbHostId, volumeUuid); + return null; + } + Boolean active = querySingleHostLockState(dbHostId, lvPath, volumeUuid, lookup); + if (active == null) { + noAnswerHostIds.add(dbHostId); + } else if (!active) { + logger.info("Fast path miss: volume {} not active on DB host {}", volumeUuid, dbHostId); + } + return active; + } - List activeHostIds = new ArrayList<>(); + /** Asks every host whether the volume is active there; used when the sanlock lease cannot name the holder. */ + private Long findLockHolderByFanOut(Long volumeId, String volumeUuid, String lvPath, StoragePool pool, Long dbHostId, + List hosts, Set noAnswerHostIds, boolean updateDatabase, LockLookup lookup) { + if (hosts.isEmpty()) { + logger.warn("No KVM routing hosts found to query CLVM lock state for volume {} (pool: {}, cluster: {}, zone: {})", + volumeUuid, pool.getName(), pool.getClusterId(), pool.getDataCenterId()); + return null; + } - for (HostVO host : hosts) { - if (host.getStatus() != Status.Up || - host.getType() != Host.Type.Routing || - host.getHypervisorType() != Hypervisor.HypervisorType.KVM) { - continue; - } - // Skip the DB host, already confirmed inactive in the fast path above - if (dbHostId != null && host.getId() == dbHostId) { - continue; - } + // The DB host was already queried in the fast path + List hostIdsToQuery = hosts.stream() + .filter(this::isUpKvmRoutingHost) + .map(HostVO::getId) + .filter(id -> !id.equals(dbHostId) && !lookup.isUnreachable(id)) + .collect(Collectors.toList()); + Map lockStates = queryHostsLockState(hostIdsToQuery, lvPath, volumeUuid, lookup); - Boolean active = querySingleHostLockState(host.getId(), lvPath, volumeUuid); + List activeHostIds = new ArrayList<>(); + for (Long hostId : hostIdsToQuery) { + Boolean active = lockStates.get(hostId); if (Boolean.TRUE.equals(active)) { - logger.debug("Volume {} is locally active on host {} (fan-out)", volumeUuid, host.getId()); - activeHostIds.add(host.getId()); + logger.debug("Volume {} is locally active on host {} (fan-out)", volumeUuid, hostId); + activeHostIds.add(hostId); + } else if (active == null) { + noAnswerHostIds.add(hostId); } } if (activeHostIds.isEmpty()) { 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); - if (targetHostId != null) { - Long recoveredHostId = tryActivateExclusivelyOnHost(volumeId, volumeUuid, lvPath, - targetHostId, updateDatabase); - if (recoveredHostId != null) { - return recoveredHostId; - } - } - - // Activation failed or no eligible host - clean up stale DB record and give up - if (updateDatabase && dbHostId != null) { - VolumeDetailVO detail = _volsDetailsDao.findDetail(volumeId, CLVM_LOCK_HOST_ID); - if (detail != null && String.valueOf(dbHostId).equals(detail.getValue())) { - _volsDetailsDao.remove(detail.getId()); - } - } - return null; + return recoverInactiveVolume(volumeId, volumeUuid, lvPath, dbHostId, hosts, noAnswerHostIds, updateDatabase, lookup); } if (activeHostIds.size() > 1) { @@ -249,26 +417,242 @@ public Long queryCurrentLockHolder(Long volumeId, String volumeUuid, String volu Long lockHostId = activeHostIds.get(0); logger.info("Volume {} is exclusively active on host {} (found via fan-out, DB had {})", volumeUuid, lockHostId, dbHostId); + return recordLockHolder(volumeId, volumeUuid, dbHostId, lockHostId, updateDatabase); + } - if (updateDatabase) { - if (dbHostId == null || !dbHostId.equals(lockHostId)) { - logger.info("Correcting database: volume {} lock host: {} -> {} (actual)", - volumeUuid, dbHostId, lockHostId); - setClvmLockHostId(volumeId, lockHostId); + private Long recordLockHolder(Long volumeId, String volumeUuid, Long dbHostId, Long lockHostId, boolean updateDatabase) { + if (updateDatabase && !lockHostId.equals(dbHostId)) { + logger.info("Correcting database: volume {} lock host: {} -> {} (actual)", + volumeUuid, dbHostId, lockHostId); + setClvmLockHostId(volumeId, lockHostId); + } + return lockHostId; + } + + /** + * Reads the sanlock lease of the volume on one host; any host in the lockspace sees every lease. + * Tries a second host if the first does not answer. + * + * @return the lease, or null when no host could read it + */ + private ClvmLockTransferAnswer readLease(String lvPath, String volumeUuid, Long dbHostId, List hosts, LockLookup lookup) { + for (Long hostId : selectLeaseReaders(dbHostId, hosts, lookup)) { + try { + Answer answer = _agentMgr.send(hostId, new ClvmLockTransferCommand( + ClvmLockTransferCommand.Operation.QUERY_LEASE_OWNER, lvPath, volumeUuid)); + if (answer instanceof ClvmLockTransferAnswer && answer.getResult() + && ((ClvmLockTransferAnswer) answer).getLeaseMode() != null) { + return (ClvmLockTransferAnswer) answer; + } + logger.debug("Host {} could not read the sanlock lease of volume {}: {}", + hostId, volumeUuid, answer != null ? answer.getDetails() : "null answer"); + } catch (AgentUnavailableException | OperationTimedoutException e) { + logger.debug("Could not query host {} for the sanlock lease of volume {}: {}", hostId, volumeUuid, e.getMessage()); + lookup.unreachableHostIds.add(hostId); } } + return null; + } - return lockHostId; + /** Up hosts that can read the lease, the preferred host and then the DB host first. */ + private List selectLeaseReaders(Long dbHostId, List hosts, LockLookup lookup) { + List hostIds = hosts.stream() + .filter(this::isUpKvmRoutingHost) + .map(HostVO::getId) + .filter(id -> !lookup.isUnreachable(id)) + .collect(Collectors.toList()); + Collections.shuffle(hostIds); + for (Long firstHostId : Arrays.asList(dbHostId, lookup.preferredHostId)) { + if (firstHostId != null && hostIds.remove(firstHostId)) { + hostIds.add(0, firstHostId); + } + } + return hostIds.subList(0, Math.min(MAX_LEASE_READERS, hostIds.size())); + } + + /** + * Whether the host named by a live lease still uses the volume. A host that restarted keeps its old lease until it + * rejoins the lockspace or the lease expires, and the LV is then not active there. When the host cannot be asked, + * a lease that is still renewed means the host is alive, otherwise it is down and its lease will expire. + */ + private boolean isLeaseHolderInUse(Long lockHostId, Long dbHostId, Boolean dbHostActive, boolean ownerRenewing, + String lvPath, String volumeUuid, LockLookup lookup) { + Boolean active; + if (lockHostId.equals(dbHostId)) { + active = dbHostActive; + } else { + active = isUpKvmRoutingHost(_hostDao.findById(lockHostId)) ? querySingleHostLockState(lockHostId, lvPath, volumeUuid, lookup) : null; + } + if (Boolean.TRUE.equals(active)) { + return true; + } + if (Boolean.FALSE.equals(active)) { + logger.info("Volume {} is not active on host {} although its sanlock lease names it, the host restarted; treating the lease as free", + volumeUuid, lockHostId); + return false; + } + if (ownerRenewing) { + logger.warn("Host {} holding volume {} cannot be reached but still renews its sanlock lease; it is alive, so its lock is kept", + lockHostId, volumeUuid); + return true; + } + logger.info("Host {} holding volume {} cannot be reached and no longer renews its sanlock lease; activating elsewhere once the lease expires", + lockHostId, volumeUuid); + return false; + } + + /** A lease that is not held, or held by a host that died or restarted since, is free to take. */ + private boolean isLeaseFree(ClvmLockTransferAnswer lease, String volumeUuid) { + if (lease.getLeaseMode() == ClvmLockTransferAnswer.LeaseMode.FREE) { + logger.debug("Volume {} is not held by any host (sanlock lease)", volumeUuid); + return true; + } + if (lease.getLeaseMode() == ClvmLockTransferAnswer.LeaseMode.EXCLUSIVE && !lease.isLeaseOwnerAlive()) { + logger.info("Volume {} is held by sanlock host {}, which is dead or restarted since; treating the lease as free", + volumeUuid, lease.getLeaseOwnerId()); + return true; + } + return false; + } + + private List getLeaseHolderIds(ClvmLockTransferAnswer lease) { + if (lease.getLeaseMode() == ClvmLockTransferAnswer.LeaseMode.EXCLUSIVE) { + return List.of(lease.getLeaseOwnerId()); + } + return lease.getSharedOwnerIds(); + } + + /** @return the only host of the pool reporting this sanlock host id, or null when none or several do */ + private Long findHostBySanlockId(List hosts, Long sanlockId) { + List hostIds = findHostIdsBySanlockId(hosts, String.valueOf(sanlockId)); + if (hostIds.size() == 1) { + return hostIds.get(0); + } + logger.warn("Sanlock host id {} matches hosts {} of the pool, expected exactly one. Querying all hosts instead.", + sanlockId, hostIds); + return null; + } + + private List findHostIdsBySanlockId(List hosts, String sanlockId) { + return hosts.stream() + .filter(h -> h.getType() == Host.Type.Routing && h.getHypervisorType() == Hypervisor.HypervisorType.KVM) + .map(HostVO::getId) + .filter(id -> sanlockId.equals(getSanlockId(id))) + .collect(Collectors.toList()); + } + + private String getSanlockId(long hostId) { + DetailVO detail = _hostDetailsDao.findDetail(hostId, Host.HOST_SANLOCK_ID); + return detail != null ? detail.getValue() : null; + } + + /** + * Checks that the host's sanlock host id identifies it among the hosts of the pool, which the lease based + * lock holder lookup relies on. + * + * @return the other hosts of the pool reporting the same sanlock host id + */ + public List findHostsSharingSanlockId(StoragePool pool, long hostId) { + String sanlockId = getSanlockId(hostId); + if (sanlockId == null) { + logger.warn("Host {} reports no {}, so CLVM lock holder lookups in pool {} query every host", + hostId, Host.HOST_SANLOCK_ID, pool.getName()); + return Collections.emptyList(); + } + List hostIds = new ArrayList<>(findHostIdsBySanlockId(findPoolHosts(pool), sanlockId)); + hostIds.remove(Long.valueOf(hostId)); + return hostIds; + } + + private boolean isUpKvmRoutingHost(HostVO host) { + return host != null + && host.getStatus() == Status.Up + && host.getType() == Host.Type.Routing + && host.getHypervisorType() == Hypervisor.HypervisorType.KVM; + } + + private List findPoolHosts(StoragePool pool) { + List hosts = null; + if (pool.getClusterId() != null) { + hosts = _hostDao.findByClusterId(pool.getClusterId(), Host.Type.Routing); + } else if (pool.getDataCenterId() > 0) { + hosts = _hostDao.findByDataCenterId(pool.getDataCenterId()); + } + return hosts != null ? hosts : Collections.emptyList(); + } + + /** + * Attempts an exclusive activation on the best available host when no host holds the volume. + * Removes the stale DB record if that fails. + */ + private Long recoverInactiveVolume(Long volumeId, String volumeUuid, String lvPath, Long dbHostId, List hosts, + Set noAnswerHostIds, boolean updateDatabase, LockLookup lookup) { + Set skipHostIds = new HashSet<>(noAnswerHostIds); + skipHostIds.addAll(lookup.unreachableHostIds); + Long targetHostId = selectActivationTargetHost(lookup.preferredHostId, dbHostId, hosts, skipHostIds); + if (targetHostId != null) { + Long recoveredHostId = tryActivateExclusivelyOnHost(volumeId, volumeUuid, lvPath, + targetHostId, updateDatabase, lookup, TRANSIENT_LOCK_REFUSALS); + if (recoveredHostId != null) { + return recoveredHostId; + } + } + + if (updateDatabase && dbHostId != null) { + VolumeDetailVO detail = _volsDetailsDao.findDetail(volumeId, CLVM_LOCK_HOST_ID); + if (detail != null && String.valueOf(dbHostId).equals(detail.getValue())) { + _volsDetailsDao.remove(detail.getId()); + } + } + return null; + } + + /** @return lock state per host that answered; hosts that did not answer are absent */ + private Map queryHostsLockState(List hostIds, String lvPath, String volumeUuid, LockLookup lookup) { + return callOnHosts(hostIds, volumeUuid, hostId -> querySingleHostLockState(hostId, lvPath, volumeUuid, lookup)); + } + + /** + * Calls the hosts in parallel, so one slow host does not delay the others. + * + * @return result per host; hosts whose call returned null are absent + */ + private Map callOnHosts(List hostIds, String volumeUuid, Function call) { + Map results = new ConcurrentHashMap<>(); + List> futures = new ArrayList<>(); + for (Long hostId : hostIds) { + futures.add(getLockQueryExecutor().submit(new ManagedContextRunnable() { + @Override + protected void runInContext() { + T result = call.apply(hostId); + if (result != null) { + results.put(hostId, result); + } + } + })); + } + for (Future future : futures) { + try { + future.get(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + break; + } catch (ExecutionException e) { + logger.warn("Host call for volume {} failed: {}", volumeUuid, e.getMessage()); + } + } + return new HashMap<>(results); } /** * Queries a single host for the CLVM LV activation state. + * A host that cannot be reached is remembered in {@code lookup}. * * @return {@code Boolean.TRUE} if the LV is active on that host, * {@code Boolean.FALSE} if reachable but inactive, * {@code null} if the host is unreachable or returned an error */ - private Boolean querySingleHostLockState(Long hostId, String lvPath, String volumeUuid) { + private Boolean querySingleHostLockState(Long hostId, String lvPath, String volumeUuid, LockLookup lookup) { try { ClvmLockTransferCommand queryCmd = new ClvmLockTransferCommand( ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE, lvPath, volumeUuid); @@ -290,6 +674,7 @@ private Boolean querySingleHostLockState(Long hostId, String lvPath, String volu return queryAnswer.isActive(); } catch (AgentUnavailableException | OperationTimedoutException e) { logger.debug("Could not query host {} for lock state: {}", hostId, e.getMessage()); + lookup.unreachableHostIds.add(hostId); return null; } } @@ -297,29 +682,36 @@ private Boolean querySingleHostLockState(Long hostId, String lvPath, String volu /** * Selects the best host on which to exclusively activate an inactive CLVM volume. * + *

Priority 0: the preferred host of the lookup, e.g. the host a VM starts on. + * *

Priority 1: the last known lock holder ({@code clvmLockHostId} from DB), if that host * is UP and KVM. * *

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 + *

Hosts that did not answer the lock-state query are skipped in both cases. + * + * @param preferredHostId host to use first, may be null + * @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 or are unreachable * @return host ID to activate on, or null if no eligible host found */ - private Long selectActivationTargetHost(Long dbHostId, List hosts) { - if (dbHostId != null) { - HostVO dbHost = _hostDao.findById(dbHostId); - if (dbHost != null && dbHost.getStatus() == Status.Up - && dbHost.getHypervisorType() == Hypervisor.HypervisorType.KVM) { + private Long selectActivationTargetHost(Long preferredHostId, Long dbHostId, List hosts, Set unreachableHostIds) { + if (preferredHostId != null && !unreachableHostIds.contains(preferredHostId) + && isUpKvmRoutingHost(_hostDao.findById(preferredHostId))) { + logger.debug("selectActivationTargetHost: using preferred host {}", preferredHostId); + return preferredHostId; + } + if (dbHostId != null && !unreachableHostIds.contains(dbHostId)) { + if (isUpKvmRoutingHost(_hostDao.findById(dbHostId))) { logger.debug("selectActivationTargetHost: preferring DB host {} (last known lock holder)", dbHostId); return dbHostId; } } if (hosts != null) { List eligible = hosts.stream() - .filter(h -> h.getStatus() == Status.Up - && h.getType() == Host.Type.Routing - && h.getHypervisorType() == Hypervisor.HypervisorType.KVM) + .filter(h -> isUpKvmRoutingHost(h) && !unreachableHostIds.contains(h.getId())) .collect(Collectors.toList()); if (!eligible.isEmpty()) { Collections.shuffle(eligible); @@ -337,6 +729,9 @@ private Long selectActivationTargetHost(Long dbHostId, List hosts) { * Sends an {@code ACTIVATE_EXCLUSIVE} command to {@code targetHostId} and optionally * persists the new lock host to the database. * + *

A temporarily refused activation (lease not expired yet, lockspace still starting) is retried + * on the same host until the deadline of {@code lookup}. Other failures are not retried. + * *

Activation failure is non-fatal: returns null so the caller can apply its own fallback. * * @param volumeId volume DB ID @@ -344,30 +739,76 @@ private Long selectActivationTargetHost(Long dbHostId, List hosts) { * @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 lookup lookup state of the operation: retry deadline and unreachable hosts * @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, LockLookup lookup, + List retryableRefusals) { + 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("Exclusively activated volume {} on host {}", volumeUuid, targetHostId); + if (updateDatabase) { + setClvmLockHostId(volumeId, targetHostId); + } + return targetHostId; } - return targetHostId; + logger.warn("Exclusive activation of volume {} on host {} failed: {}", + volumeUuid, targetHostId, + activateAnswer != null ? activateAnswer.getDetails() : "null answer"); + if (!isRetryableRefusal(activateAnswer, retryableRefusals, retryIntervalMs, lookup.activationDeadlineMs)) { + 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("Exclusive activation of volume {} on host {} threw exception: {}", + volumeUuid, targetHostId, e.getMessage()); + lookup.unreachableHostIds.add(targetHostId); + return null; + } + if (!sleep(retryIntervalMs)) { + 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()); } - return null; + } + + private boolean isRetryableRefusal(Answer activateAnswer, List retryableRefusals, long retryIntervalMs, long deadlineMs) { + return retryIntervalMs > 0 + && activateAnswer != null + && activateAnswer.getDetails() != null + && retryableRefusals.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; + } } /** @@ -407,12 +848,7 @@ public boolean transferClvmVolumeLock(String volumeUuid, Long volumeId, String v return false; } - String vgName = pool.getPath(); - if (vgName.startsWith("/")) { - vgName = vgName.substring(1); - } - - String lvPath = String.format("/dev/%s/%s", vgName, volumePath); + String lvPath = getLvPath(pool, volumePath); try { // sourceHostId is trusted as pre-validated by the caller @@ -443,23 +879,13 @@ public boolean transferClvmVolumeLock(String volumeUuid, Long volumeId, String v logger.debug("Volume {} has no active lock holder, will directly activate on destination", volumeUuid); } - ClvmLockTransferCommand activateCmd = new ClvmLockTransferCommand( - ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE, - lvPath, - volumeUuid - ); - - Answer activateAnswer = _agentMgr.send(destHostId, activateCmd); - - if (activateAnswer == null || !activateAnswer.getResult()) { - String error = activateAnswer != null ? activateAnswer.getDetails() : "null answer"; - logger.error("Failed to activate CLVM volume {} exclusively on dest host {}: {}", - volumeUuid, destHostId, error); + // A live source refuses with "locked by other host": fail fast. Only wait for the destination's lockspace to start. + if (tryActivateExclusivelyOnHost(volumeId, volumeUuid, lvPath, destHostId, true, + newLockLookup(destHostId), List.of(LOCK_START_IN_PROGRESS)) == null) { + logger.error("Failed to activate CLVM volume {} exclusively on dest host {}", volumeUuid, destHostId); return false; } - setClvmLockHostId(volumeId, destHostId); - logger.info("Successfully transferred CLVM lock for volume {} from host {} to host {}", volumeUuid, sourceHostId != null ? sourceHostId : "none", destHostId); @@ -479,7 +905,10 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[] { - CLVMSecureZeroFill + CLVMSecureZeroFill, + ClvmLockRecoveryRetryTimeout, + ClvmLockRecoveryRetryInterval, + ClvmLockQueryThreads }; } } diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 482d0b405d72..d18c7b43d7da 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -27,6 +27,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.UUID; import java.util.concurrent.ExecutionException; @@ -187,6 +188,7 @@ import com.cloud.storage.Volume; import com.cloud.storage.VolumeApiService; import com.cloud.storage.VolumeVO; +import com.cloud.storage.clvm.ClvmPoolManager; import com.cloud.storage.dao.GuestOSDao; import com.cloud.storage.dao.LaunchPermissionDao; import com.cloud.storage.dao.SnapshotDao; @@ -258,6 +260,8 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, @Inject private PrimaryDataStoreDao _poolDao; @Inject + private ClvmPoolManager clvmPoolManager; + @Inject private StoragePoolHostDao _poolHostDao; @Inject private AccountDao _accountDao; @@ -1237,12 +1241,37 @@ private Boolean templateIsUnusedInPool(VMTemplateStoragePoolVO templatePoolVO) { return false; } - if (template.getFormat() == ImageFormat.ISO || _volumeDao.isAnyVolumeActivelyUsingTemplateOnPool(template.getId(), templatePoolVO.getPoolId())) { + if (template.getFormat() == ImageFormat.ISO || isTemplateUsedByVolumes(template.getId(), templatePoolVO.getPoolId())) { return false; } return true; } + /** CLVM pools on the same VG share one template LV, used as backing file by CLVM_NG volumes of any of them. */ + private boolean isTemplateUsedByVolumes(long templateId, long poolId) { + return getPoolIdsSharingTemplate(poolId).stream() + .anyMatch(id -> _volumeDao.isAnyVolumeActivelyUsingTemplateOnPool(templateId, id)); + } + + private Set getPoolIdsSharingTemplate(long poolId) { + StoragePoolVO pool = _poolDao.findById(poolId); + if (pool == null || !ClvmPoolManager.isClvmPoolType(pool.getPoolType())) { + return Set.of(poolId); + } + return clvmPoolManager.findPoolIdsSharingVg(pool); + } + + /** @return true when another pool on the same VG still references the template LV, so only this pool's record may go */ + private boolean isTemplateLvReferencedByOtherPool(VMTemplateStoragePoolVO templatePoolVO) { + Set otherPoolIds = new HashSet<>(getPoolIdsSharingTemplate(templatePoolVO.getPoolId())); + otherPoolIds.remove(templatePoolVO.getPoolId()); + if (otherPoolIds.isEmpty()) { + return false; + } + return _tmpltPoolDao.listByTemplateId(templatePoolVO.getTemplateId(), new ArrayList<>(otherPoolIds)).stream() + .anyMatch(ref -> Objects.equals(ref.getInstallPath(), templatePoolVO.getInstallPath())); + } + @Override public List getUnusedTemplatesInPool(StoragePoolVO pool) { List unusedTemplatesInPool = new ArrayList(); @@ -1305,7 +1334,14 @@ public void evictTemplateFromStoragePool(VMTemplateStoragePoolVO templatePoolVO) logger.debug("Successfully evicted Template {} from storage pool {}", template, pool); } } + } else if (ClvmPoolManager.isClvmPoolType(pool.getPoolType()) && isTemplateLvReferencedByOtherPool(templatePoolVO)) { + if (_tmpltPoolDao.remove(templatePoolVO.getId())) { + logger.debug("Evicted Template {} from storage pool {}, keeping its LV for the other pools on the same VG", template, pool); + } } else { + if (ClvmPoolManager.isClvmPoolType(pool.getPoolType())) { + clvmPoolManager.deactivateOnAllHosts(pool, templatePoolVO.getInstallPath()); + } DestroyCommand cmd = new DestroyCommand(pool, templatePoolVO); Answer answer = _storageMgr.sendToPool(pool, cmd); diff --git a/server/src/test/java/com/cloud/storage/ClvmPoolManagerTest.java b/server/src/test/java/com/cloud/storage/ClvmPoolManagerTest.java index 1678b3760919..a6f3dd13b849 100644 --- a/server/src/test/java/com/cloud/storage/ClvmPoolManagerTest.java +++ b/server/src/test/java/com/cloud/storage/ClvmPoolManagerTest.java @@ -25,24 +25,36 @@ 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.HostDao; +import com.cloud.host.dao.HostDetailsDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.clvm.ClvmPoolManager; import com.cloud.storage.dao.VolumeDetailsDao; import org.apache.cloudstack.storage.clvm.command.ClvmLockTransferAnswer; import org.apache.cloudstack.storage.clvm.command.ClvmLockTransferCommand; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; 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.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; @@ -65,6 +77,12 @@ public class ClvmPoolManagerTest { @Mock private HostDao hostDao; + @Mock + private HostDetailsDao hostDetailsDao; + + @Mock + private PrimaryDataStoreDao storagePoolDao; + @InjectMocks private ClvmPoolManager clvmPoolManager; @@ -77,7 +95,7 @@ public class ClvmPoolManagerTest { @Before public void setUp() { - Mockito.reset(volsDetailsDao, agentMgr, hostDao); + Mockito.reset(volsDetailsDao, agentMgr, hostDao, hostDetailsDao); } @Test @@ -301,7 +319,7 @@ public void testQueryCurrentLockHolder_SuccessfulQuery() throws AgentUnavailable Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); Assert.assertEquals(HOST_ID_1, result); - verify(agentMgr, times(1)).send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class)); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); verify(hostDao, never()).findByName(any()); } @@ -480,7 +498,6 @@ public void testQueryCurrentLockHolder_UpdateDatabase_MatchingValue() throws Age // Fast path succeeded - no DB write needed, no fan-out verify(volsDetailsDao, never()).update(anyLong(), any()); verify(volsDetailsDao, never()).addDetail(anyLong(), any(), any(), Mockito.anyBoolean()); - verify(hostDao, never()).findByClusterId(anyLong(), any()); } @Test @@ -597,8 +614,8 @@ public void testQueryCurrentLockHolder_SkipsNonKVMHosts() throws AgentUnavailabl Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); Assert.assertEquals(HOST_ID_1, result); - verify(agentMgr, never()).send(eq(10L), any(ClvmLockTransferCommand.class)); - verify(agentMgr, times(1)).send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class)); + verify(agentMgr, never()).send(eq(10L), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); } @Test @@ -620,8 +637,8 @@ public void testQueryCurrentLockHolder_SkipsDownHosts() throws AgentUnavailableE Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); Assert.assertEquals(HOST_ID_1, result); - verify(agentMgr, never()).send(eq(10L), any(ClvmLockTransferCommand.class)); - verify(agentMgr, times(1)).send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class)); + verify(agentMgr, never()).send(eq(10L), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); } @Test @@ -666,10 +683,8 @@ public void testQueryCurrentLockHolder_FastPath_HitOnDbHost() throws AgentUnavai Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); Assert.assertEquals(HOST_ID_1, result); - // Only one agent call, no cluster host lookup, no fan-out - verify(agentMgr, times(1)).send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class)); - verify(hostDao, never()).findByClusterId(anyLong(), any()); - verify(hostDao, never()).findByDataCenterId(anyLong()); + // Lease unreadable: one lock-state query on the DB host, no fan-out + verify(agentMgr, times(1)).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); } /** @@ -702,8 +717,8 @@ public void testQueryCurrentLockHolder_FastPath_MissDbHost_FanOutFindsOther() th Assert.assertEquals(HOST_ID_2, result); // HOST_ID_1 queried once (fast path only), HOST_ID_2 queried once (fan-out) - verify(agentMgr, times(1)).send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class)); - verify(agentMgr, times(1)).send(eq(HOST_ID_2), any(ClvmLockTransferCommand.class)); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + verify(agentMgr, times(1)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); } /** @@ -731,8 +746,8 @@ public void testQueryCurrentLockHolder_FastPath_DbHostDown_FanOut() throws Agent Assert.assertEquals(HOST_ID_2, result); // No query to the down host at all - verify(agentMgr, never()).send(eq(HOST_ID_1), any(ClvmLockTransferCommand.class)); - verify(agentMgr, times(1)).send(eq(HOST_ID_2), any(ClvmLockTransferCommand.class)); + verify(agentMgr, never()).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + verify(agentMgr, times(1)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); } @@ -923,6 +938,596 @@ public void testQueryCurrentLockHolder_InactiveEverywhere_NoEligibleHost_Returns 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 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); + ReflectionTestUtils.setField(manager, "_hostDetailsDao", hostDetailsDao); + 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()); + ClvmPoolManager.LockLookup lookup = manager.newLockLookup(); + + Assert.assertNull(manager.getClvmLockHostId(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true, lookup)); + Assert.assertNull(manager.getClvmLockHostId(VOLUME_ID + 1, "second-volume-uuid", "second-volume-path", pool, true, lookup)); + + 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)); + ClvmPoolManager.LockLookup lookup = manager.newLockLookup(); + + Assert.assertEquals(HOST_ID_2, manager.getClvmLockHostId(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true, lookup)); + Assert.assertEquals(HOST_ID_2, manager.getClvmLockHostId(VOLUME_ID + 1, "second-volume-uuid", "second-volume-path", pool, true, lookup)); + + Assert.assertEquals(Arrays.asList(10_000L, 10_000L), manager.sleeps); + verify(agentMgr, times(4)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testFanOut_UnreachableHostQueriedOnceForAllVolumesOfOneLookup() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareRecoveryOnHost2(); + Long host3 = 3L; + HostVO downHost = hostDao.findById(HOST_ID_1); + HostVO upHost2 = createMockHost(HOST_ID_2, "host2", Status.Up, Hypervisor.HypervisorType.KVM); + HostVO upHost3 = createMockHost(host3, "host3", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Arrays.asList(downHost, upHost2, upHost3)); + when(agentMgr.send(eq(host3), any(ClvmLockTransferCommand.class))) + .thenThrow(new OperationTimedoutException(null, host3, 0, 0, false)); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(new Answer(null, true, null)); + ClvmPoolManager.LockLookup lookup = clvmPoolManager.newLockLookup(); + + Assert.assertEquals(HOST_ID_2, clvmPoolManager.getClvmLockHostId(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true, lookup)); + Assert.assertEquals(HOST_ID_2, clvmPoolManager.getClvmLockHostId(VOLUME_ID + 1, "second-volume-uuid", "second-volume-path", pool, true, lookup)); + + verify(agentMgr, times(1)).send(eq(host3), any(ClvmLockTransferCommand.class)); + verify(agentMgr, times(2)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + } + + @Test + public void testFanOut_HostsQueriedInParallel() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = mockClusterPool(); + when(volsDetailsDao.findDetail(VOLUME_ID, ClvmPoolManager.CLVM_LOCK_HOST_ID)).thenReturn(null); + 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.findByClusterId(10L, Host.Type.Routing)).thenReturn(Arrays.asList(upHost1, upHost2)); + + // Each query answers only once both queries are in flight + CountDownLatch bothInFlight = new CountDownLatch(2); + AtomicBoolean sequential = new AtomicBoolean(); + when(agentMgr.send(anyLong(), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenAnswer(invocation -> { + bothInFlight.countDown(); + if (!bothInFlight.await(5, TimeUnit.SECONDS)) { + sequential.set(true); + } + boolean active = HOST_ID_2.equals(invocation.getArgument(0)); + return new ClvmLockTransferAnswer(null, true, null, null, active, active, null); + }); + + Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertFalse("hosts were queried one after another", sequential.get()); + Assert.assertEquals(HOST_ID_2, result); + verify(agentMgr, never()).send(anyLong(), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + /** Two Up hosts in the cluster reporting sanlock ids 1 and 2; DB says HOST_ID_1 holds the lock. */ + private StoragePoolVO prepareLeaseLookup() { + StoragePoolVO pool = mockClusterPool(); + VolumeDetailVO detail = new VolumeDetailVO(); + detail.setValue(String.valueOf(HOST_ID_1)); + lenient().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); + lenient().when(hostDao.findById(HOST_ID_1)).thenReturn(host1); + lenient().when(hostDao.findById(HOST_ID_2)).thenReturn(host2); + when(hostDao.findByClusterId(10L, Host.Type.Routing)).thenReturn(Arrays.asList(host1, host2)); + stubSanlockId(HOST_ID_1, "1"); + stubSanlockId(HOST_ID_2, "2"); + stubLockState(HOST_ID_1, false); + return pool; + } + + private void stubLockState(Long hostId, boolean active) { + try { + lenient().when(agentMgr.send(eq(hostId), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenReturn(new ClvmLockTransferAnswer(null, true, null, null, active, active, null)); + } catch (AgentUnavailableException | OperationTimedoutException e) { + throw new IllegalStateException(e); + } + } + + private void stubSanlockId(Long hostId, String sanlockId) { + lenient().when(hostDetailsDao.findDetail(hostId, Host.HOST_SANLOCK_ID)).thenReturn(new DetailVO(hostId, Host.HOST_SANLOCK_ID, sanlockId)); + } + + private void stubLease(ClvmLockTransferAnswer lease) throws AgentUnavailableException, OperationTimedoutException { + when(agentMgr.send(anyLong(), operation(ClvmLockTransferCommand.Operation.QUERY_LEASE_OWNER))).thenReturn(lease); + } + + @Test + public void testLease_ExclusiveOwnerMappedToHost_NoFanOut() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareLeaseLookup(); + stubLease(ClvmLockTransferAnswer.leaseOwner(null, ClvmLockTransferAnswer.LeaseMode.EXCLUSIVE, 2L, true, Collections.emptyList())); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenReturn(new ClvmLockTransferAnswer(null, true, null, null, true, true, null)); + + Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true); + + Assert.assertEquals(HOST_ID_2, result); + verify(agentMgr, times(1)).send(anyLong(), operation(ClvmLockTransferCommand.Operation.QUERY_LEASE_OWNER)); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + verify(volsDetailsDao).update(anyLong(), any(VolumeDetailVO.class)); + } + + @Test + public void testLease_HolderRestarted_LvInactiveThere_ActivatedOnPreferredHost() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareLeaseLookup(); + stubLease(ClvmLockTransferAnswer.leaseOwner(null, ClvmLockTransferAnswer.LeaseMode.EXCLUSIVE, 2L, true, Collections.emptyList())); + 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.getClvmLockHostId(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true, + clvmPoolManager.newLockLookup(HOST_ID_2)); + + Assert.assertEquals(HOST_ID_2, result); + verify(agentMgr, times(1)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + } + + @Test + public void testLease_Free_ActivatedOnPreferredHost() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareLeaseLookup(); + stubLease(ClvmLockTransferAnswer.leaseOwner(null, ClvmLockTransferAnswer.LeaseMode.FREE, null, false, Collections.emptyList())); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(new Answer(null, true, null)); + + Long result = clvmPoolManager.getClvmLockHostId(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true, + clvmPoolManager.newLockLookup(HOST_ID_2)); + + Assert.assertEquals(HOST_ID_2, result); + verify(agentMgr, times(1)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LEASE_OWNER)); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + verify(agentMgr, never()).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + verify(agentMgr, never()).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testLease_OwnerDead_TreatedAsFree() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareLeaseLookup(); + stubLease(ClvmLockTransferAnswer.leaseOwner(null, ClvmLockTransferAnswer.LeaseMode.EXCLUSIVE, 1L, false, Collections.emptyList())); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(new Answer(null, true, null)); + + Long result = clvmPoolManager.getClvmLockHostId(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true, + clvmPoolManager.newLockLookup(HOST_ID_2)); + + Assert.assertEquals(HOST_ID_2, result); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + verify(agentMgr, never()).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + } + + @Test + public void testLease_SanlockIdOnTwoHosts_FallsBackToFanOut() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareLeaseLookup(); + stubSanlockId(HOST_ID_1, "2"); + stubLease(ClvmLockTransferAnswer.leaseOwner(null, ClvmLockTransferAnswer.LeaseMode.EXCLUSIVE, 2L, true, Collections.emptyList())); + when(agentMgr.send(eq(HOST_ID_1), 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.QUERY_LOCK_STATE))) + .thenReturn(new ClvmLockTransferAnswer(null, true, null, null, true, true, null)); + + Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertEquals(HOST_ID_2, result); + verify(agentMgr, times(1)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + } + + @Test + public void testLease_UnreadableOnTwoHosts_FallsBackToFanOut() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareLeaseLookup(); + stubLease(new ClvmLockTransferAnswer(null, false, "No sanlock lock args")); + stubLockState(HOST_ID_2, true); + + Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false); + + Assert.assertEquals(HOST_ID_2, result); + verify(agentMgr, times(2)).send(anyLong(), operation(ClvmLockTransferCommand.Operation.QUERY_LEASE_OWNER)); + verify(agentMgr, times(1)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + } + + @Test + public void testLease_NamesDbHostFoundInactive_NoSecondQuery() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareLeaseLookup(); + stubLease(ClvmLockTransferAnswer.leaseOwner(null, ClvmLockTransferAnswer.LeaseMode.EXCLUSIVE, 1L, true, Collections.emptyList())); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(new Answer(null, true, null)); + + Long result = clvmPoolManager.getClvmLockHostId(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true, + clvmPoolManager.newLockLookup(HOST_ID_2)); + + Assert.assertEquals(HOST_ID_2, result); + verify(agentMgr, times(1)).send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE)); + } + + @Test + public void testLease_HolderUnreachableNotRenewing_ActivatedOnPreferredHost() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareLeaseLookup(); + stubLease(ClvmLockTransferAnswer.leaseOwner(null, ClvmLockTransferAnswer.LeaseMode.EXCLUSIVE, 2L, true, false, Collections.emptyList())); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenThrow(new OperationTimedoutException(null, HOST_ID_2, 0, 0, false)); + when(agentMgr.send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(new Answer(null, true, null)); + + Long result = clvmPoolManager.getClvmLockHostId(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true, + clvmPoolManager.newLockLookup(HOST_ID_1)); + + Assert.assertEquals(HOST_ID_1, result); + verify(agentMgr, never()).send(anyLong(), operation(ClvmLockTransferCommand.Operation.DEACTIVATE)); + } + + @Test + public void testLease_HolderUnreachableButRenewing_KeptAsHolder() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareLeaseLookup(); + stubLease(ClvmLockTransferAnswer.leaseOwner(null, ClvmLockTransferAnswer.LeaseMode.EXCLUSIVE, 2L, true, true, Collections.emptyList())); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.QUERY_LOCK_STATE))) + .thenThrow(new OperationTimedoutException(null, HOST_ID_2, 0, 0, false)); + + Long result = clvmPoolManager.getClvmLockHostId(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true, + clvmPoolManager.newLockLookup(HOST_ID_1)); + + Assert.assertEquals(HOST_ID_2, result); + verify(agentMgr, never()).send(anyLong(), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + /** Source HOST_ID_1 is Up and deactivates; the destination is HOST_ID_2. */ + private StoragePoolVO prepareTransfer() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + when(pool.getPath()).thenReturn(VG_NAME); + HostVO source = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findById(HOST_ID_1)).thenReturn(source); + when(agentMgr.send(eq(HOST_ID_1), operation(ClvmLockTransferCommand.Operation.DEACTIVATE))) + .thenReturn(new Answer(null, true, null)); + return pool; + } + + @Test + public void testTransfer_DestinationLockStartInProgress_Retried() throws AgentUnavailableException, OperationTimedoutException { + RetryTestClvmPoolManager manager = newRetryManager(); + StoragePoolVO pool = prepareTransfer(); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(lockStartInProgress(), new Answer(null, true, null)); + + Assert.assertTrue(manager.transferClvmVolumeLock(VOLUME_UUID, VOLUME_ID, VOLUME_PATH, pool, HOST_ID_1, HOST_ID_2)); + + Assert.assertEquals(Collections.singletonList(10_000L), manager.sleeps); + verify(agentMgr, times(2)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testTransfer_LockedByOtherHost_FailsFast() throws AgentUnavailableException, OperationTimedoutException { + RetryTestClvmPoolManager manager = newRetryManager(); + StoragePoolVO pool = prepareTransfer(); + when(agentMgr.send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE))) + .thenReturn(lockedByOtherHost()); + + Assert.assertFalse(manager.transferClvmVolumeLock(VOLUME_UUID, VOLUME_ID, VOLUME_PATH, pool, HOST_ID_1, HOST_ID_2)); + + Assert.assertTrue(manager.sleeps.isEmpty()); + verify(agentMgr, times(1)).send(eq(HOST_ID_2), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testFastPathHit_NoLeaseRead() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareLeaseLookup(); + stubLockState(HOST_ID_1, true); + + Long result = clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, true); + + Assert.assertEquals(HOST_ID_1, result); + verify(agentMgr, times(1)).send(anyLong(), any(ClvmLockTransferCommand.class)); + } + + @Test + public void testLease_SharedByTwoHosts_NoTransfer() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolVO pool = prepareLeaseLookup(); + stubLease(ClvmLockTransferAnswer.leaseOwner(null, ClvmLockTransferAnswer.LeaseMode.SHARED, null, false, Arrays.asList(1L, 2L))); + + Assert.assertNull(clvmPoolManager.queryCurrentLockHolder(VOLUME_ID, VOLUME_UUID, VOLUME_PATH, pool, false)); + verify(agentMgr, never()).send(anyLong(), operation(ClvmLockTransferCommand.Operation.ACTIVATE_EXCLUSIVE)); + } + + @Test + public void testFindHostsSharingSanlockId() { + StoragePoolVO pool = prepareLeaseLookup(); + Assert.assertTrue(clvmPoolManager.findHostsSharingSanlockId(pool, HOST_ID_1).isEmpty()); + + stubSanlockId(HOST_ID_2, "1"); + Assert.assertEquals(Collections.singletonList(HOST_ID_2), clvmPoolManager.findHostsSharingSanlockId(pool, HOST_ID_1)); + } + + 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.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); @@ -933,4 +1538,48 @@ private HostVO createMockHost(Long id, String name, Status status, Hypervisor.Hy Mockito.lenient().when(host.getHypervisorType()).thenReturn(hypervisor); return host; } + + private StoragePoolVO mockVgPool(long id, String path, long clusterId) { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + lenient().when(pool.getId()).thenReturn(id); + lenient().when(pool.getPath()).thenReturn(path); + lenient().when(pool.getClusterId()).thenReturn(clusterId); + lenient().when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM_NG); + lenient().when(pool.getDataCenterId()).thenReturn(1L); + return pool; + } + + @Test + public void testFindPoolIdsSharingVg_MatchesVgNameOnly() { + StoragePoolVO pool = mockVgPool(8L, "sharedvg", 1L); + List zonePools = List.of(pool, mockVgPool(9L, "/sharedvg", 2L), mockVgPool(4L, "sharedvg2", 2L)); + when(storagePoolDao.findPoolsByStorageTypeAndZone(Storage.StoragePoolType.CLVM_NG, 1L)).thenReturn(zonePools); + + Assert.assertEquals(Set.of(8L, 9L), clvmPoolManager.findPoolIdsSharingVg(pool)); + } + + @Test + public void testDeactivateOnAllHosts_UpHostsOfPoolsSharingVg() throws Exception { + StoragePoolVO pool = mockVgPool(8L, "sharedvg", 1L); + StoragePoolVO otherPool = mockVgPool(9L, "sharedvg", 2L); + when(storagePoolDao.findPoolsByStorageTypeAndZone(Storage.StoragePoolType.CLVM_NG, 1L)).thenReturn(List.of(pool, otherPool)); + when(storagePoolDao.findById(8L)).thenReturn(pool); + when(storagePoolDao.findById(9L)).thenReturn(otherPool); + HostVO host1 = createMockHost(HOST_ID_1, "host1", Status.Up, Hypervisor.HypervisorType.KVM); + HostVO downHost = createMockHost(3L, "host3", Status.Down, Hypervisor.HypervisorType.KVM); + HostVO host2 = createMockHost(HOST_ID_2, "host2", Status.Up, Hypervisor.HypervisorType.KVM); + when(hostDao.findByClusterId(1L, Host.Type.Routing)).thenReturn(List.of(host1, downHost)); + when(hostDao.findByClusterId(2L, Host.Type.Routing)).thenReturn(List.of(host2)); + Answer ok = new Answer(null, true, "ok"); + when(agentMgr.send(anyLong(), any(ClvmLockTransferCommand.class))).thenReturn(ok); + + clvmPoolManager.deactivateOnAllHosts(pool, "template-abc"); + + ArgumentCaptor cmd = ArgumentCaptor.forClass(ClvmLockTransferCommand.class); + verify(agentMgr).send(eq(HOST_ID_1), cmd.capture()); + verify(agentMgr).send(eq(HOST_ID_2), any(ClvmLockTransferCommand.class)); + verify(agentMgr, never()).send(eq(3L), any(Command.class)); + Assert.assertEquals(ClvmLockTransferCommand.Operation.DEACTIVATE, cmd.getValue().getOperation()); + Assert.assertEquals("/dev/sharedvg/template-abc", cmd.getValue().getLvPath()); + } } diff --git a/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java b/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java index 61407a9d4c2b..9ea9b5b4a9bd 100755 --- a/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java +++ b/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java @@ -31,7 +31,9 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.UUID; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutorService; @@ -43,6 +45,7 @@ import org.junit.After; import org.junit.Assert; +import org.mockito.InOrder; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; @@ -99,6 +102,8 @@ import com.cloud.storage.StoragePoolStatus; import com.cloud.storage.TemplateProfile; import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.clvm.ClvmPoolManager; +import com.cloud.agent.api.storage.DestroyCommand; import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.VolumeVO; @@ -276,6 +281,15 @@ public class TemplateManagerImplTest extends TestCase { @Mock private UserDataVO userDataMock; + @Mock + ClvmPoolManager clvmPoolManager; + + @Mock + StorageManager storageManagerMock; + + @Mock + TemplateDataFactory templateDataFactoryMock; + public class CustomThreadPoolExecutor extends ThreadPoolExecutor { AtomicInteger ai = new AtomicInteger(0); public CustomThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit, @@ -1487,4 +1501,90 @@ public void prepareIsoForVmProfileUsesConsecutiveDeviceIdsWithoutConfigDrive() { Assert.assertEquals(Arrays.asList(3L, 4L), disks.stream().map(DiskTO::getDiskSeq).collect(java.util.stream.Collectors.toList())); }); } + + private VMTemplateStoragePoolVO mockClvmTemplateRef(long refId, long poolId, String installPath) { + VMTemplateStoragePoolVO ref = mock(VMTemplateStoragePoolVO.class); + Mockito.lenient().when(ref.getId()).thenReturn(refId); + Mockito.lenient().when(ref.getPoolId()).thenReturn(poolId); + Mockito.lenient().when(ref.getTemplateId()).thenReturn(202L); + Mockito.lenient().when(ref.getInstallPath()).thenReturn(installPath); + Mockito.lenient().when(ref.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOADED); + return ref; + } + + private PrimaryDataStore mockClvmEviction(VMTemplateStoragePoolVO ref) { + StoragePoolVO pool = mock(StoragePoolVO.class); + when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM_NG); + when(primaryDataStoreDao.findById(2L)).thenReturn(pool); + when(clvmPoolManager.findPoolIdsSharingVg(pool)).thenReturn(Set.of(2L, 3L)); + PrimaryDataStore store = mock(PrimaryDataStore.class); + Mockito.lenient().when(store.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM_NG); + when(vmTemplatePoolDao.acquireInLockTable(ref.getId())).thenReturn(ref); + when(dataStoreManager.getPrimaryDataStore(2L)).thenReturn(store); + return store; + } + + @Test + public void evictClvmTemplateKeepsLvReferencedByPoolOnSameVg() throws Exception { + VMTemplateStoragePoolVO ref = mockClvmTemplateRef(5L, 2L, "template-abc"); + VMTemplateStoragePoolVO otherRef = mockClvmTemplateRef(6L, 3L, "template-abc"); + mockClvmEviction(ref); + when(vmTemplatePoolDao.listByTemplateId(202L, List.of(3L))).thenReturn(List.of(otherRef)); + + templateManager.evictTemplateFromStoragePool(ref); + + Mockito.verify(vmTemplatePoolDao).remove(5L); + Mockito.verify(storageManagerMock, Mockito.never()).sendToPool(any(StoragePool.class), any(DestroyCommand.class)); + Mockito.verify(clvmPoolManager, Mockito.never()).deactivateOnAllHosts(any(), any()); + } + + @Test + public void evictClvmTemplateDeactivatesEverywhereBeforeRemovingLv() throws Exception { + VMTemplateStoragePoolVO ref = mockClvmTemplateRef(5L, 2L, "template-abc"); + PrimaryDataStore store = mockClvmEviction(ref); + when(vmTemplatePoolDao.listByTemplateId(202L, List.of(3L))).thenReturn(Collections.emptyList()); + com.cloud.agent.api.Answer answer = mock(com.cloud.agent.api.Answer.class); + when(answer.getResult()).thenReturn(true); + when(storageManagerMock.sendToPool(any(StoragePool.class), any(DestroyCommand.class))).thenReturn(answer); + + templateManager.evictTemplateFromStoragePool(ref); + + InOrder order = Mockito.inOrder(clvmPoolManager, storageManagerMock); + order.verify(clvmPoolManager).deactivateOnAllHosts(store, "template-abc"); + order.verify(storageManagerMock).sendToPool(any(StoragePool.class), any(DestroyCommand.class)); + Mockito.verify(vmTemplatePoolDao).remove(5L); + } + + @Test + public void evictClvmTemplateKeepsRecordWhenLvRemovalFails() throws Exception { + VMTemplateStoragePoolVO ref = mockClvmTemplateRef(5L, 2L, "template-abc"); + mockClvmEviction(ref); + when(vmTemplatePoolDao.listByTemplateId(202L, List.of(3L))).thenReturn(Collections.emptyList()); + com.cloud.agent.api.Answer answer = mock(com.cloud.agent.api.Answer.class); + when(answer.getResult()).thenReturn(false); + when(storageManagerMock.sendToPool(any(StoragePool.class), any(DestroyCommand.class))).thenReturn(answer); + + templateManager.evictTemplateFromStoragePool(ref); + + Mockito.verify(vmTemplatePoolDao, Mockito.never()).remove(5L); + } + + @Test + public void clvmTemplateUsedAsBackingOnPoolWithSameVgIsNotUnused() { + VMTemplateStoragePoolVO ref = mockClvmTemplateRef(5L, 2L, "template-abc"); + StoragePoolVO pool = mock(StoragePoolVO.class); + when(pool.getId()).thenReturn(2L); + when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.CLVM_NG); + when(primaryDataStoreDao.findById(2L)).thenReturn(pool); + when(clvmPoolManager.findPoolIdsSharingVg(pool)).thenReturn(Set.of(2L, 3L)); + VMTemplateVO template = mock(VMTemplateVO.class); + when(template.getId()).thenReturn(202L); + when(template.getTemplateType()).thenReturn(Storage.TemplateType.USER); + when(template.getFormat()).thenReturn(Storage.ImageFormat.QCOW2); + when(vmTemplateDao.findByIdIncludingRemoved(202L)).thenReturn(template); + when(vmTemplatePoolDao.listByPoolId(2L)).thenReturn(List.of(ref)); + when(volumeDao.isAnyVolumeActivelyUsingTemplateOnPool(202L, 3L)).thenReturn(true); + + Assert.assertTrue(templateManager.getUnusedTemplatesInPool(pool).isEmpty()); + } }