Skip to content

WIP: server: refuse re-registration of deleted hosts by GUID - #13719

Open
wido wants to merge 4 commits into
apache:mainfrom
wido:reject-readd-of-deleted-hosts
Open

WIP: server: refuse re-registration of deleted hosts by GUID#13719
wido wants to merge 4 commits into
apache:mainfrom
wido:reject-readd-of-deleted-hosts

Conversation

@wido

@wido wido commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Description

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.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

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.
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 59.45946% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.73%. Comparing base (158fe4f) to head (5177783).

Files with missing lines Patch % Lines
.../src/main/java/com/cloud/host/dao/HostDaoImpl.java 0.00% 12 Missing ⚠️
...n/java/com/cloud/resource/ResourceManagerImpl.java 88.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13719      +/-   ##
============================================
- Coverage     19.74%   19.73%   -0.01%     
- Complexity    19960    19969       +9     
============================================
  Files          6371     6371              
  Lines        575784   575820      +36     
  Branches      70478    70482       +4     
============================================
- Hits         113665   113664       -1     
- Misses       449765   449806      +41     
+ Partials      12354    12350       -4     
Flag Coverage Δ
uitests 3.41% <ø> (ø)
unittests 21.01% <59.45%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread server/src/main/java/com/cloud/resource/ResourceManagerImpl.java Outdated
Comment thread server/src/main/java/com/cloud/resource/ResourceManagerImpl.java Outdated
@DaanHoogland DaanHoogland added this to the 4.24.0 milestone Jul 27, 2026
@github-actions

Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

…ed-hosts

# Conflicts:
#	server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
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.
Comment thread server/src/main/java/com/cloud/resource/ResourceManagerImpl.java Outdated
Comment thread server/src/main/java/com/cloud/resource/ResourceManagerImpl.java Outdated
Addresses review feedback: the explanation lives in the javadoc on
rejectReAddOfDeletedHost(), the inline comments only cluttered the code.
@wido

wido commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

@DaanHoogland comments removed in 43a3adc, thanks!

@DaanHoogland DaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deleted KVM host is automatically re-added to the cluster after reboot

2 participants