Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions engine/schema/src/main/java/com/cloud/host/HostVO.java
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,10 @@ public Date getRemoved() {
return removed;
}

public void setRemoved(Date removed) {
this.removed = removed;
}

@Override
public String getVersion() {
return version;
Expand Down
2 changes: 2 additions & 0 deletions engine/schema/src/main/java/com/cloud/host/dao/HostDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
22 changes: 22 additions & 0 deletions server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -2281,10 +2281,32 @@
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.
*/
Comment on lines +2293 to +2298
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()));

Check warning on line 2305 in server/src/main/java/com/cloud/resource/ResourceManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Invoke method(s) only conditionally.

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaFUoXlV8oM5eTecOgvA&open=AaFUoXlV8oM5eTecOgvA&pullRequest=14438
removedHost.setRemoved(null);
return removedHost;
Comment on lines +2300 to +2307
}

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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -683,4 +685,49 @@
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);

Check warning on line 699 in server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "mock".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaFUoXrf8oM5eTecOgvB&open=AaFUoXrf8oM5eTecOgvB&pullRequest=14438
when(cmd.getGuid()).thenReturn(guid);
when(cmd.getGuidWithoutResource()).thenReturn(guidPrefix);

Mockito.doReturn(null).when(resourceManager).findHostByGuid(guid);

Check warning on line 703 in server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "doReturn".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaFUoXrf8oM5eTecOgvC&open=AaFUoXrf8oM5eTecOgvC&pullRequest=14438
Mockito.doReturn(null).when(resourceManager).findHostByGuidPrefix(guidPrefix);

Check warning on line 704 in server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "doReturn".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaFUoXrf8oM5eTecOgvD&open=AaFUoXrf8oM5eTecOgvD&pullRequest=14438

HostVO removedHost = Mockito.mock(HostVO.class);

Check warning on line 706 in server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "mock".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaFUoXrf8oM5eTecOgvE&open=AaFUoXrf8oM5eTecOgvE&pullRequest=14438
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);

Check warning on line 714 in server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "verify".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaFUoXrf8oM5eTecOgvF&open=AaFUoXrf8oM5eTecOgvF&pullRequest=14438
}

@Test
public void testGetNewHostReturnsNullWhenNoHostAndNoRemovedMatch() throws Exception {
String guid = "unknown-guid";
String guidPrefix = "unknown";
StartupCommand cmd = Mockito.mock(StartupCommand.class);

Check warning on line 721 in server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "mock".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaFUoXrf8oM5eTecOgvG&open=AaFUoXrf8oM5eTecOgvG&pullRequest=14438
when(cmd.getGuid()).thenReturn(guid);
when(cmd.getGuidWithoutResource()).thenReturn(guidPrefix);

Mockito.doReturn(null).when(resourceManager).findHostByGuid(guid);

Check warning on line 725 in server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "doReturn".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaFUoXrf8oM5eTecOgvH&open=AaFUoXrf8oM5eTecOgvH&pullRequest=14438
Mockito.doReturn(null).when(resourceManager).findHostByGuidPrefix(guidPrefix);

Check warning on line 726 in server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "doReturn".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaFUoXrf8oM5eTecOgvI&open=AaFUoXrf8oM5eTecOgvI&pullRequest=14438
when(hostDao.findByGuidIncludingRemoved(guid)).thenReturn(null);

HostVO result = invokeGetNewHost(new StartupCommand[]{cmd});

Assert.assertNull(result);
}
Comment on lines +688 to +732
}
Loading