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 ccd3f9cc1a2c..e6142e4688ea 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; @@ -1066,7 +1067,6 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { logger.debug("Deleting tags from database for host with UUID [{}].", host.getUuid()); _hostTagsDao.deleteTags(hostId); - host.setGuid(null); final Long clusterId = host.getClusterId(); host.setClusterId(null); _hostDao.update(host.getId(), host); @@ -3217,9 +3217,58 @@ private HostVO getNewHost(StartupCommand[] startupCommands) { } logger.debug(String.format("Could not find Host by guid %s", fullGuid)); + + 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 void validateExistingHostLocationImmutable(final HostVO host, final boolean newHost, final long dcId, final Long podId, final Long clusterId, final StartupCommand startup) { if (newHost || host == null || host.getType() != Host.Type.Routing) { 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()); + } }