ssvm: prevent sql constraint violation then a SVM was removed during network oiutage

Author: Daan Hoogland

engine/schema/src/main/java/com/cloud/host/HostVO.java

@@ -600,6 +600,10 @@ public Date getRemoved() {
         return removed;
     }
 
+    public void setRemoved(Date removed) {
+        this.removed = removed;
+    }
+
     @Override
     public String getVersion() {
         return version;

engine/schema/src/main/java/com/cloud/host/dao/HostDao.java

@@ -79,6 +79,8 @@ public interface HostDao extends GenericDao<HostVO, Long>, StateDao<Status, Stat
 
     HostVO findByGuid(String guid);
 
+    HostVO findByGuidIncludingRemoved(String guid);
+
     HostVO findByTypeNameAndZoneId(long zoneId, String name, Host.Type type);
 
     List<HostVO> findHypervisorHostInCluster(long clusterId);

engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java

@@ -609,6 +609,12 @@ public HostVO findByGuid(String guid) {
         return findOneBy(sc);
     }
 
+    @Override
+    public HostVO findByGuidIncludingRemoved(String guid) {
+        SearchCriteria<HostVO> sc = GuidSearch.create("guid", guid);
+        return findOneIncludingRemovedBy(sc);
+    }
+
     /*
      * Find hosts which is in Disconnected, Down, Alert and ping timeout and server is not null, set server to null
      */

server/src/main/java/com/cloud/resource/ResourceManagerImpl.java

@@ -2281,10 +2281,32 @@ private HostVO getNewHost(StartupCommand[] startupCommands) {
             return host;
         }
 
+        host = reviveRemovedHostByGuid(fullGuid);
+        if (host != null) {
+            return host;
+        }
+
         logger.debug(String.format("Could not find Host by guid %s", fullGuid));
         return null;
     }
 
+    /**
+     * A host can be marked removed (e.g. a system VM force-stopped and destroyed from the UI) while the
+     * underlying resource is still alive and later reconnects with the same guid. The `guid` column is
+     * uniquely constrained regardless of removal, so blindly inserting a new row for it fails with a
+     * SQLIntegrityConstraintViolationException. Revive the removed row instead of creating a duplicate.
+     */
+    private HostVO reviveRemovedHostByGuid(String guid) {
+        HostVO removedHost = _hostDao.findByGuidIncludingRemoved(guid);
+        if (removedHost == null || removedHost.getRemoved() == null) {
+            return null;
+        }
+        logger.warn(String.format("Host with guid %s reconnected but was previously removed (id: %s); reviving the removed host record instead of creating a duplicate",
+                guid, removedHost.getId()));
+        removedHost.setRemoved(null);
+        return removedHost;
+    }
+
     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) {

server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

@@ -64,9 +64,11 @@
 import org.mockito.junit.MockitoJUnitRunner;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Date;
 import java.util.List;
 import java.util.UUID;
 
@@ -683,4 +685,49 @@ public void testDestroyLocalStoragePoolVolumesNoDisks() {
         resourceManager.destroyLocalStoragePoolVolumes(poolId);
         verify(volumeDao, never()).updateAndRemoveVolume(any(VolumeVO.class));
     }
+
+    private HostVO invokeGetNewHost(StartupCommand[] cmds) throws Exception {
+        Method method = ResourceManagerImpl.class.getDeclaredMethod("getNewHost", StartupCommand[].class);
+        method.setAccessible(true);
+        return (HostVO) method.invoke(resourceManager, (Object) cmds);
+    }
+
+    @Test
+    public void testGetNewHostRevivesRemovedHostOnGuidCollision() throws Exception {
+        String guid = "s-14-VM-PremiumSecondaryStorageResource";
+        String guidPrefix = "s-14-VM";
+        StartupCommand cmd = Mockito.mock(StartupCommand.class);
+        when(cmd.getGuid()).thenReturn(guid);
+        when(cmd.getGuidWithoutResource()).thenReturn(guidPrefix);
+
+        Mockito.doReturn(null).when(resourceManager).findHostByGuid(guid);
+        Mockito.doReturn(null).when(resourceManager).findHostByGuidPrefix(guidPrefix);
+
+        HostVO removedHost = Mockito.mock(HostVO.class);
+        when(removedHost.getId()).thenReturn(99L);
+        when(removedHost.getRemoved()).thenReturn(new Date());
+        when(hostDao.findByGuidIncludingRemoved(guid)).thenReturn(removedHost);
+
+        HostVO result = invokeGetNewHost(new StartupCommand[]{cmd});
+
+        Assert.assertSame(removedHost, result);
+        Mockito.verify(removedHost).setRemoved(null);
+    }
+
+    @Test
+    public void testGetNewHostReturnsNullWhenNoHostAndNoRemovedMatch() throws Exception {
+        String guid = "unknown-guid";
+        String guidPrefix = "unknown";
+        StartupCommand cmd = Mockito.mock(StartupCommand.class);
+        when(cmd.getGuid()).thenReturn(guid);
+        when(cmd.getGuidWithoutResource()).thenReturn(guidPrefix);
+
+        Mockito.doReturn(null).when(resourceManager).findHostByGuid(guid);
+        Mockito.doReturn(null).when(resourceManager).findHostByGuidPrefix(guidPrefix);
+        when(hostDao.findByGuidIncludingRemoved(guid)).thenReturn(null);
+
+        HostVO result = invokeGetNewHost(new StartupCommand[]{cmd});
+
+        Assert.assertNull(result);
+    }
 }