Repository navigation
Conversation
Replace PatchHypervisorStatusWithRetry with Status().Apply(). The Nova ID lookup is inlined into Reconcile and combined with the initial condition in a single apply, avoiding two applies from the same field manager in one reconcile cycle which would cause SSA to prune the IDs. HypervisorID and ServiceID are included in every subsequent apply to retain ownership. The status config is built once at the top of Reconcile and threaded through to all sub-functions (abortOnboarding, initialOnboarding, smokeTest, completeOnboarding) which mutate it directly. An apply closure is also passed so each function calls apply() when it has determined the desired condition, rather than building and applying the config itself. The flow becomes fully declarative: compute desired state, then apply once. Seed only the condition owned by this controller (ConditionTypeOnboarding) so SSA does not claim ownership over conditions written by other controllers.
During node replacement, Nova can briefly (or persistently) hold compute records whose agent is gone. Onboarding must not adopt such a record: the stored identity goes stale the moment the replacement agent registers, leaving the new compute service disabled and the smoke test looping on 'No valid host' (nova-issues#786). SelectCurrentIdentity picks the authoritative hypervisor/service pair from the exact-hostname matches: records without a live service are skipped, the stored identity is kept among live duplicates to avoid flapping, and unresolvable duplicates are surfaced instead of guessed.
Onboarding resolved the hypervisor/service pair exactly once (while the fields were empty) and then trusted it forever. When a node is replaced, the leftover record of the dead agent can be adopted before the replacement registers, and the compute node later flips to the new service: the stored serviceId then points at an orphan, the enable step runs against the wrong service, the current service stays disabled and the smoke test loops on 'No valid host' (nova-issues#786). Every reconcile of an incompletely onboarded hypervisor now resolves the current identity and repairs the stored IDs, restarting onboarding at the enable step. A service that reports disabled during Testing or Handover routes back to the enable step as well, instead of burning scheduling rounds on a host that cannot take the test VM. Records without a live service are never adopted; an enable request that hits a vanished service re-resolves instead of erroring. Successfully onboarded hypervisors are terminal and keep their identity untouched.
Node replacement deletes the Hypervisor object and recreates it under the same name with a new UID. A reconcile that is still in flight then writes status computed for the old object, and since the API server does not enforce a UID precondition on subresource applies, the write merges into the replacement object. The apply path now checks that the observed object still exists and still carries the same UID before applying. It is a regular method that the sub-functions call directly, replacing the closure that was threaded through them as a parameter. The check reads the informer cache, so a replacement that is not yet visible can still slip through; the identity re-validation on the replacement's next reconcile repairs such a write. The unenforced-UID behavior of subresource applies is pinned by a spec so a future enforcing release gets noticed.
fwiesel
force-pushed
the
fix-786-stale-service-id
branch
from
October 30, 2026 17:20
b2f3e90 to
e1655f4
Compare
Merging this branch will increase overall coverage
Coverage detailsCompared to commit f5172c4 (run #41086905849)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the incident class documented in nova-issues#786: after node replacement, hypervisor onboarding can adopt the leftover compute record of the dead agent and store its service ID. When the replacement agent registers, Nova flips the compute node to the new service — the stored ID points at an orphan, the enable step runs against the wrong service, the current compute service stays disabled, and the smoke test loops on
No valid hostuntil someone clearsstatus.serviceIdand enables the service by hand.Stacked on #340 — merge #340 first. Until then this PR shows the onboarding SSA migration plus the fix; once #340 merges, the diff shrinks to the three fix commits.
What it does
Add Nova identity selection that refuses stale compute records—openstack.SelectCurrentIdentitypicks the authoritative hypervisor/service pair from the exact-hostname matches. Records whose service is notstate=up(dead or replaced agents) are never adopted — not even as the only match, not even when they match the stored ID. The stored identity is kept among live duplicates to avoid flapping; unresolvable duplicates are surfaced as an error instead of being guessed.Onboarding: re-validate the Nova identity and repair stale service IDs— every reconcile of a not-yet-succeeded hypervisor resolves the current identity and repairs the stored IDs when they changed, restarting onboarding at the enable step so the current compute service gets enabled. A service that reports disabled during Testing/Handover routes back to the enable step (thecompute/disable_new_servicescase) instead of burning scheduling rounds. An enable request hitting a vanished service re-resolves.Onboarding=False/Succeededstays terminal — no Nova calls, no writes.Onboarding: drop status writes for replaced Hypervisor objects— node replacement deletes and recreates the Hypervisor under the same name with a new UID. The API server does not enforce a UID precondition on subresource applies (verified and pinned by a spec, so a future enforcing release gets noticed), so the apply path checks the observed object's existence and UID client-side and drops the write when the object was replaced. The residual informer-cache race is backstopped by the identity repair from (2).Verification
go test ./...),golangci-lintclean.Succeeded, and the cross-replacement apply behavior.Not addressed here
The RCA's open question — which request first wrote the stale
6b4fccf9-…onto the new CR — remains unconfirmed; this fix is robust regardless of the answer. Follow-ups: the same UID guard for the eviction controller's status applies, the maintenance controller trusting its cachedHypervisorDisabledcondition, and the offboarding agent-pod-gate investigation.