DaanHoogland commented on code in PR #13719:
URL: https://github.com/apache/cloudstack/pull/13719#discussion_r3862473192
##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -1066,7 +1067,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().
Review Comment:
Can you move this comment to a javadoc or just plain remove it? (probably an
AI remnance right?)
I Think it is cluttering the code and will not be read other than to confuse
coders.
##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -3217,9 +3220,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.
Review Comment:
same here, let’s keep the code as clean as possible
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
