From 636a3ada67425193da43f7a233dc426d616ca33c Mon Sep 17 00:00:00 2001 From: Wido den Hollander Date: Mon, 27 Jul 2026 13:08:30 +0200 Subject: [PATCH 1/3] server: refuse re-registration of deleted hosts by GUID A still-running agent whose host was deleted keeps its locally-persisted GUID and reconnects indefinitely. The management server looks up hosts by GUID excluding removed rows, so it fails to recognize the returning agent and re-registers it as a brand-new host. The existing 'add.host.on.service.restart.kvm' setting only guards this agent-side, for KVM/LXC, and only when the agent is still connected at delete time. Enforce that setting's intent on the management server: when 'add.host.on.service.restart.kvm' is false, refuse an agent whose GUID matches a previously deleted host. This also covers the case where the agent was offline when the host was deleted, and applies to all hypervisor types. - Preserve the host GUID on the soft-deleted record so the returning agent can be recognized (deleteHost no longer nulls the GUID). - Add HostDao.findByGuidIncludingRemoved / findByGuidPrefixIncludingRemoved. - getNewHost now rejects re-registration via rejectReAddOfDeletedHost. --- .../main/java/com/cloud/host/dao/HostDao.java | 12 ++++ .../java/com/cloud/host/dao/HostDaoImpl.java | 18 ++++++ .../cloud/resource/ResourceManagerImpl.java | 57 ++++++++++++++++++- 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java index d8bdabc3dcbb..9e88fa493ec2 100644 --- a/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java +++ b/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java @@ -85,6 +85,18 @@ public interface HostDao extends GenericDao, StateDao findHypervisorHostInCluster(long clusterId); diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java index 15727d9d8e66..8ed790a4b49b 100644 --- a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java @@ -118,6 +118,7 @@ public class HostDaoImpl extends GenericDaoBase implements HostDao protected SearchBuilder UnremovedIpAddressSearch; protected SearchBuilder GuidSearch; + protected SearchBuilder GuidPrefixSearch; protected SearchBuilder DcSearch; protected SearchBuilder PodSearch; protected SearchBuilder ClusterSearch; @@ -312,6 +313,10 @@ public void init() { GuidSearch.and("guid", GuidSearch.entity().getGuid(), SearchCriteria.Op.EQ); GuidSearch.done(); + GuidPrefixSearch = createSearchBuilder(); + GuidPrefixSearch.and("guid", GuidPrefixSearch.entity().getGuid(), SearchCriteria.Op.LIKE); + GuidPrefixSearch.done(); + DcSearch = createSearchBuilder(); DcSearch.and("dc", DcSearch.entity().getDataCenterId(), SearchCriteria.Op.EQ); DcSearch.and("hypervisorType", DcSearch.entity().getHypervisorType(), Op.EQ); @@ -637,6 +642,19 @@ public HostVO findByGuid(String guid) { return findOneBy(sc); } + @Override + public HostVO findByGuidIncludingRemoved(String guid) { + SearchCriteria sc = GuidSearch.create("guid", guid); + return findOneIncludingRemovedBy(sc); + } + + @Override + public HostVO findByGuidPrefixIncludingRemoved(String guidPrefix) { + SearchCriteria sc = GuidPrefixSearch.create(); + sc.setParameters("guid", guidPrefix + "%"); + return findOneIncludingRemovedBy(sc); + } + /* * Find hosts which is in Disconnected, Down, Alert and ping timeout and server is not null, set server to null */ diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 32a519c79fe9..c6694d413187 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.resource; +import static com.cloud.configuration.ConfigurationManagerImpl.ADD_HOST_ON_SERVICE_RESTART_KVM; import static com.cloud.configuration.ConfigurationManagerImpl.MIGRATE_VM_ACROSS_CLUSTERS; import static com.cloud.configuration.ConfigurationManagerImpl.SET_HOST_DOWN_TO_MAINTENANCE; import static org.apache.cloudstack.gpu.GpuService.GpuDetachOnStop; @@ -1062,7 +1063,9 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { logger.debug("Deleting tags from database for host with UUID [{}].", host.getUuid()); _hostTagsDao.deleteTags(hostId); - host.setGuid(null); + // Note: the host GUID is intentionally preserved on the (soft-)deleted record so that a + // returning agent with the same GUID can be detected and refused re-registration when + // 'add.host.on.service.restart.kvm' is false. See getNewHost()/rejectReAddOfDeletedHost(). final Long clusterId = host.getClusterId(); host.setClusterId(null); _hostDao.update(host.getId(), host); @@ -3213,9 +3216,61 @@ private HostVO getNewHost(StartupCommand[] startupCommands) { } logger.debug(String.format("Could not find Host by guid %s", fullGuid)); + + // No live host matches this GUID. Before letting the caller create a brand-new host, + // make sure this GUID does not belong to a host that was previously deleted. Otherwise a + // still-running agent whose host was deleted would silently re-register itself as a new host. + rejectReAddOfDeletedHost(fullGuid, guidPrefix); + return null; } + /** + * Refuses the (re-)registration of an agent whose GUID matches a host that was previously deleted + * (soft-removed) from CloudStack. + *

+ * This is the management-server-side, hypervisor-agnostic enforcement of the intent already expressed by + * the {@code add.host.on.service.restart.kvm} setting: when that setting is {@code false} the operator has + * indicated a deleted host must not come back. The existing enforcement (in LibvirtServerDiscoverer) only + * works for KVM/LXC and only if the agent was still connected at delete time; this guard also covers the + * case where the agent was offline when the host was deleted and later reconnects with the same GUID. + */ + protected void rejectReAddOfDeletedHost(String fullGuid, String guidPrefix) { + if (ADD_HOST_ON_SERVICE_RESTART_KVM.value()) { + return; + } + + HostVO deletedHost = findRemovedHostByGuid(fullGuid); + if (deletedHost == null && StringUtils.isNotBlank(guidPrefix)) { + deletedHost = findRemovedHostByGuidPrefix(guidPrefix); + } + + if (deletedHost != null) { + String msg = String.format( + "Refusing to (re-)register agent with GUID [%s]: a host with this GUID (id: %d, uuid: %s, name: %s) was previously deleted from CloudStack on %s. " + + "Set the global setting '%s' to true to allow a deleted host to re-register when its agent reconnects.", + fullGuid, deletedHost.getId(), deletedHost.getUuid(), deletedHost.getName(), deletedHost.getRemoved(), ADD_HOST_ON_SERVICE_RESTART_KVM.key()); + logger.warn(msg); + throw new CloudRuntimeException(msg); + } + } + + private HostVO findRemovedHostByGuid(String guid) { + if (StringUtils.isBlank(guid)) { + return null; + } + HostVO host = _hostDao.findByGuidIncludingRemoved(guid); + return host != null && host.getRemoved() != null ? host : null; + } + + private HostVO findRemovedHostByGuidPrefix(String guidPrefix) { + if (StringUtils.isBlank(guidPrefix)) { + return null; + } + HostVO host = _hostDao.findByGuidPrefixIncludingRemoved(guidPrefix); + return host != null && host.getRemoved() != null ? host : null; + } + protected HostVO createHostVO(final StartupCommand[] cmds, final ServerResource resource, final Map details, List hostTags, List storageAccessGroups, final ResourceStateAdapter.Event stateEvent) { boolean newHost = false; From 5177783b222a6e67e42ff73104aaaf7793923975 Mon Sep 17 00:00:00 2001 From: Wido den Hollander Date: Wed, 26 Aug 2026 07:48:36 +0200 Subject: [PATCH 2/3] server: add unit tests for rejectReAddOfDeletedHost Cover the management-server guard that refuses an agent whose GUID belongs to a previously deleted host: - honours 'add.host.on.service.restart.kvm': when true the guard returns early and issues no database lookup at all - rejects a match on the full GUID and, separately, on the GUID prefix - the rejection message names both the GUID and the setting to flip - a full-GUID hit short-circuits the prefix lookup - a live host (removed = null) returned by the *IncludingRemoved lookups is never rejected, so an ordinary agent reconnect keeps working - blank/null GUID and prefix are not looked up, so a blank prefix cannot turn into a wildcard query matching an unrelated host The tests mutate the static ADD_HOST_ON_SERVICE_RESTART_KVM ConfigKey, so tearDown() restores its declared default to keep the change from leaking into other tests sharing the JVM fork. --- .../resource/ResourceManagerImplTest.java | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index 403afcac8a8a..8e6b677135af 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -81,9 +81,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Date; import java.util.List; import java.util.UUID; +import static com.cloud.configuration.ConfigurationManagerImpl.ADD_HOST_ON_SERVICE_RESTART_KVM; import static com.cloud.resource.ResourceState.Event.ErrorsCorrected; import static com.cloud.resource.ResourceState.Event.InternalEnterMaintenance; import static com.cloud.resource.ResourceState.Event.UnableToMaintain; @@ -240,6 +242,9 @@ public void setup() throws Exception { @After public void tearDown() throws Exception { + // rejectReAddOfDeletedHost tests mutate this static ConfigKey; restore its declared + // default so the change cannot leak into other tests sharing this JVM fork. + overrideDefaultConfigValue(ADD_HOST_ON_SERVICE_RESTART_KVM, "_defaultValue", "true"); sshHelperMocked.close(); actionEventUtilsMocked.close(); getVncPortCommandMockedConstruction.close(); @@ -1393,4 +1398,122 @@ public void testCheckIfAllHostsInUseWithEmptyHostsInMultipleLevels() { Mockito.verify(hostDao).findByClusterId(clusterId, Host.Type.Routing); Mockito.verify(hostDao).findByPodId(podId, Host.Type.Routing); } + + private static final String DELETED_HOST_GUID = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee-LibvirtComputingResource"; + private static final String DELETED_HOST_GUID_PREFIX = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; + + private HostVO mockDeletedHost() { + HostVO deletedHost = Mockito.mock(HostVO.class); + when(deletedHost.getRemoved()).thenReturn(new Date()); + when(deletedHost.getId()).thenReturn(42L); + when(deletedHost.getUuid()).thenReturn("some-host-uuid"); + when(deletedHost.getName()).thenReturn("kvm-host-1"); + return deletedHost; + } + + /** + * When 'add.host.on.service.restart.kvm' is true the operator has opted in to letting a deleted + * host come back, so the guard must not even query the database. + */ + @Test + public void testRejectReAddOfDeletedHostDoesNothingWhenSettingEnabled() throws Exception { + overrideDefaultConfigValue(ADD_HOST_ON_SERVICE_RESTART_KVM, "_defaultValue", "true"); + + resourceManager.rejectReAddOfDeletedHost(DELETED_HOST_GUID, DELETED_HOST_GUID_PREFIX); + + verify(hostDao, never()).findByGuidIncludingRemoved(anyString()); + verify(hostDao, never()).findByGuidPrefixIncludingRemoved(anyString()); + } + + @Test + public void testRejectReAddOfDeletedHostDoesNotThrowWhenGuidIsUnknown() throws Exception { + overrideDefaultConfigValue(ADD_HOST_ON_SERVICE_RESTART_KVM, "_defaultValue", "false"); + when(hostDao.findByGuidIncludingRemoved(DELETED_HOST_GUID)).thenReturn(null); + when(hostDao.findByGuidPrefixIncludingRemoved(DELETED_HOST_GUID_PREFIX)).thenReturn(null); + + resourceManager.rejectReAddOfDeletedHost(DELETED_HOST_GUID, DELETED_HOST_GUID_PREFIX); + } + + @Test + public void testRejectReAddOfDeletedHostThrowsWhenFullGuidMatchesDeletedHost() throws Exception { + overrideDefaultConfigValue(ADD_HOST_ON_SERVICE_RESTART_KVM, "_defaultValue", "false"); + HostVO deletedHost = mockDeletedHost(); + when(hostDao.findByGuidIncludingRemoved(DELETED_HOST_GUID)).thenReturn(deletedHost); + + try { + resourceManager.rejectReAddOfDeletedHost(DELETED_HOST_GUID, DELETED_HOST_GUID_PREFIX); + Assert.fail("Expected CloudRuntimeException for an agent whose GUID belongs to a deleted host"); + } catch (CloudRuntimeException e) { + Assert.assertTrue(e.getMessage().contains(DELETED_HOST_GUID)); + Assert.assertTrue(e.getMessage().contains(ADD_HOST_ON_SERVICE_RESTART_KVM.key())); + } + + // A full-GUID hit short-circuits; the prefix lookup must not be issued. + verify(hostDao, never()).findByGuidPrefixIncludingRemoved(anyString()); + } + + @Test + public void testRejectReAddOfDeletedHostThrowsWhenGuidPrefixMatchesDeletedHost() throws Exception { + overrideDefaultConfigValue(ADD_HOST_ON_SERVICE_RESTART_KVM, "_defaultValue", "false"); + when(hostDao.findByGuidIncludingRemoved(DELETED_HOST_GUID)).thenReturn(null); + HostVO deletedHost = mockDeletedHost(); + when(hostDao.findByGuidPrefixIncludingRemoved(DELETED_HOST_GUID_PREFIX)).thenReturn(deletedHost); + + try { + resourceManager.rejectReAddOfDeletedHost(DELETED_HOST_GUID, DELETED_HOST_GUID_PREFIX); + Assert.fail("Expected CloudRuntimeException when only the GUID prefix matches a deleted host"); + } catch (CloudRuntimeException e) { + Assert.assertTrue(e.getMessage().contains(ADD_HOST_ON_SERVICE_RESTART_KVM.key())); + } + } + + /** + * A row returned by the *IncludingRemoved lookups may still be a live host. Only soft-deleted + * rows (removed != null) may be refused, otherwise a normal agent reconnect would break. + */ + @Test + public void testRejectReAddOfDeletedHostAllowsLiveHostWithSameGuid() throws Exception { + overrideDefaultConfigValue(ADD_HOST_ON_SERVICE_RESTART_KVM, "_defaultValue", "false"); + HostVO liveHost = Mockito.mock(HostVO.class); + when(liveHost.getRemoved()).thenReturn(null); + when(hostDao.findByGuidIncludingRemoved(DELETED_HOST_GUID)).thenReturn(liveHost); + when(hostDao.findByGuidPrefixIncludingRemoved(DELETED_HOST_GUID_PREFIX)).thenReturn(null); + + resourceManager.rejectReAddOfDeletedHost(DELETED_HOST_GUID, DELETED_HOST_GUID_PREFIX); + } + + @Test + public void testRejectReAddOfDeletedHostAllowsLiveHostMatchedByPrefix() throws Exception { + overrideDefaultConfigValue(ADD_HOST_ON_SERVICE_RESTART_KVM, "_defaultValue", "false"); + HostVO liveHost = Mockito.mock(HostVO.class); + when(liveHost.getRemoved()).thenReturn(null); + when(hostDao.findByGuidIncludingRemoved(DELETED_HOST_GUID)).thenReturn(null); + when(hostDao.findByGuidPrefixIncludingRemoved(DELETED_HOST_GUID_PREFIX)).thenReturn(liveHost); + + resourceManager.rejectReAddOfDeletedHost(DELETED_HOST_GUID, DELETED_HOST_GUID_PREFIX); + } + + @Test + public void testRejectReAddOfDeletedHostSkipsLookupsForBlankGuidAndPrefix() throws Exception { + overrideDefaultConfigValue(ADD_HOST_ON_SERVICE_RESTART_KVM, "_defaultValue", "false"); + + resourceManager.rejectReAddOfDeletedHost(null, null); + resourceManager.rejectReAddOfDeletedHost("", " "); + + verify(hostDao, never()).findByGuidIncludingRemoved(anyString()); + verify(hostDao, never()).findByGuidPrefixIncludingRemoved(anyString()); + } + + /** + * A blank prefix must not be turned into a wildcard lookup that could match an unrelated host. + */ + @Test + public void testRejectReAddOfDeletedHostSkipsPrefixLookupWhenPrefixBlank() throws Exception { + overrideDefaultConfigValue(ADD_HOST_ON_SERVICE_RESTART_KVM, "_defaultValue", "false"); + when(hostDao.findByGuidIncludingRemoved(DELETED_HOST_GUID)).thenReturn(null); + + resourceManager.rejectReAddOfDeletedHost(DELETED_HOST_GUID, ""); + + verify(hostDao, never()).findByGuidPrefixIncludingRemoved(anyString()); + } } From 43a3adc4a9b34fc90c7ba6f4255188218417c1c5 Mon Sep 17 00:00:00 2001 From: Wido den Hollander Date: Wed, 26 Aug 2026 17:29:09 +0200 Subject: [PATCH 3/3] server: drop verbose inline comments in host re-add guard Addresses review feedback: the explanation lives in the javadoc on rejectReAddOfDeletedHost(), the inline comments only cluttered the code. --- .../main/java/com/cloud/resource/ResourceManagerImpl.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 30518640312d..e6142e4688ea 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1067,9 +1067,6 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { logger.debug("Deleting tags from database for host with UUID [{}].", host.getUuid()); _hostTagsDao.deleteTags(hostId); - // Note: the host GUID is intentionally preserved on the (soft-)deleted record so that a - // returning agent with the same GUID can be detected and refused re-registration when - // 'add.host.on.service.restart.kvm' is false. See getNewHost()/rejectReAddOfDeletedHost(). final Long clusterId = host.getClusterId(); host.setClusterId(null); _hostDao.update(host.getId(), host); @@ -3221,9 +3218,6 @@ private HostVO getNewHost(StartupCommand[] startupCommands) { logger.debug(String.format("Could not find Host by guid %s", fullGuid)); - // No live host matches this GUID. Before letting the caller create a brand-new host, - // make sure this GUID does not belong to a host that was previously deleted. Otherwise a - // still-running agent whose host was deleted would silently re-register itself as a new host. rejectReAddOfDeletedHost(fullGuid, guidPrefix); return null;