Repository navigation
Match failover host reservations inside nova pipeline - #1462
Open
PhilippMatthes wants to merge 8 commits into
Open
PhilippMatthes wants to merge 8 commits into
PhilippMatthes wants to merge 8 commits into
Conversation
PhilippMatthes
marked this pull request as ready for review
November 2, 2026 14:54
PhilippMatthes
requested review from
SoWieMarkus,
auhlig,
juliusclausnitzer,
mblos and
umswmayj
as code owners
November 2, 2026 14:54
umswmayj
reviewed
Nov 3, 2026
![cortex-ai-agents[bot]](https://avatars.githubusercontent.com/in/3521331?s=60&v=4){kind=small}
Route evacuation requests onto failover host reservations from inside the scheduling pipeline, pinning a source host to a single target for gang- evacuation, and block ordinary placement onto those reserved hosts. - Add the failover host Matcher (reservations/failoverhost/matcher.go) that reroutes evacuations after the pipeline, under the pipeline controller's exclusive lock, writing the InUse condition when it pins a fresh host. - Wire the Matcher into the FilterWeigherPipelineController. - Add the filter_no_failover_host pipeline filter that blocks hosts with a failover host reservation for non-evacuation requests. Named to make clear it only avoids placing new workload onto failover hosts. Disabled by default until the failover host draining controller logic lands. - Give the Matcher its own small MatcherMonitor collector so the pipeline and failover-host-reservations controllers can both register without colliding on shared metric names. Signed-off-by: Philipp Matthes <p.matthes@sap.com>
PhilippMatthes
force-pushed
the
feat/failover-host-matcher
branch
from
November 3, 2026 10:51
5c1bcf6 to
501052c
Compare
Signed-off-by: Philipp Matthes <p.matthes@sap.com>
Member
Author
|
Offline discussion outcome: there is a possible edge case of the current approach: if the fallback is triggered it will be possible for the vm to evacuate to a failover slot which is not matched (may be not InUse or InUse for a different source host). There are also some other points tbd. |
Replace the post-pipeline Matcher that reduced evacuation candidates to a single reserved host (and speculatively wrote InUse) with a filter, a weigher, and a thin outcome-based pin: - filter_failover_host (replaces filter_no_failover_host): removes all failover hosts for ordinary requests, and for evacuations keeps only the deterministically selected target, pruning the other reachable failover targets so a second failure keeps a free landing zone while ordinary candidates are retained for Nova retries. - weigher_failover_host: lifts the selected target to the front via a dominating weight, leaving the retained candidates behind it as fallbacks. - Matcher.ConfirmAndMarkInUse: no longer rewrites the candidate list; marks the reservation InUse only when its target is the pipeline's top candidate, so a target that lost to other constraints leaves no dangling InUse. Both plugins and the matcher apply the same deterministic selection rule (sort matching reservations by target host name; prefer one already InUse by this source; never steal one InUse by another source) over the same snapshot. Rename the kvm.filterNoFailoverHost.enabled Helm value to kvm.failoverHost.enabled (shared by both steps), wire the new weigher into the KVM pipelines, and rewrite the scheduling docs with the rationale. Signed-off-by: Philipp Matthes <p.matthes@sap.com>
- Rename the Helm toggle kvm.failoverHost.enabled to kvm.failoverHostMatching.enabled (names the behavior, not the resource). - Rename matcher.go/matcher_test.go to matching.go/matching_test.go. - Inline the deterministic host-selection rule directly into each call site (filter, weigher, and the post-pipeline matcher) rather than a shared helper. The plugin packages cannot import failoverhost (its in-package integration test imports them, which would cycle), so the small sort-by-name rule is duplicated inline in each, keeping every step self-contained. Signed-off-by: Philipp Matthes <p.matthes@sap.com>
Only select a failover host reservation as an evacuation target once it is Active (host fully drained and empty). A not-yet-Active reservation (freshly created, still draining) is still blocked for ordinary placements but is no longer routed an evacuation onto a host that is not actually empty. The already-InUse gang-evacuation reuse path is unchanged, and when the feature is disabled nothing changes for existing scheduling. Apply the Active gate consistently in the filter, the weigher and the post-pipeline matcher. Also remove the FailoverHostDesignated status condition: the draining stage is simply the absence of FailoverHostActive=True, the same as a freshly created reservation, so a dedicated condition is redundant. Signed-off-by: Philipp Matthes <p.matthes@sap.com>
Extract the deterministic failover-host selection rule into a single exported SelectFailoverHost in the failoverhost package, called by the filter, the weigher and the matcher, removing the three inline copies. This requires the plugin packages to import failoverhost, which previously cycled because failoverhost's in-package integration_test.go imported the real nova pipeline (filters/weighers/nova). Replace that white-box integration test with stub-scheduler scenarios in generation_test.go: the five whole-host guarantee cases (trait incompatibility, homogeneous, undersized host, AZ isolation, sibling sources) now drive the generation pipeline against a stub scheduler whose per-(flavor, AZ) accept-sets stand in for the real pipeline's answers. The real filter semantics remain covered by each filter's own unit tests. Signed-off-by: Philipp Matthes <p.matthes@sap.com>
Add a direct table test for SelectFailoverHost — the deterministic rule shared by the filter, weigher and matcher — pinning every branch: non-failover and nil-spec reservations are ignored, partition membership is required, the name-first Active reservation wins, a not-yet-Active one is skipped, an in-use reservation for this source is reused over a name-earlier free one, and one in use by another source is never stolen (bringing it to 100% coverage). Also add Validate() tests for both plugin option structs, and rename matcher_monitor.go to matching_monitor.go to match the matching.go rename. Signed-off-by: Philipp Matthes <p.matthes@sap.com>
…tcher Signed-off-by: Philipp Matthes <p.matthes@sap.com> # Conflicts: # cmd/manager/main.go # helm/bundles/cortex-nova/values.yaml # internal/scheduling/nova/filter_weigher_pipeline_controller.go # internal/scheduling/nova/filter_weigher_pipeline_controller_test.go # internal/scheduling/reservations/failoverhost/integration_test.go
Contributor
Test Coverage ReportTest Coverage 📊: 72.0% |
This branch has not been deployed
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.
Failover host reservations keep whole hosts empty as gang-evacuation landing zones, but until now the nova scheduling pipeline did not honor them: new VMs could still land on a reserved host, and evacuations were not routed onto one. This change wires both behaviors in through a filter and a weigher that shape the candidate list inside the pipeline, plus a thin post-pipeline step that records the outcome. The new position-independent
filter_failover_hoststep removes every reserved host for ordinary requests, and for evacuations removes all but the one deterministically selected for the dead host'ssource_host— pruning the other reachable failover targets so a second host failure keeps its own free landing zone, while leaving the ordinary candidates in place so Nova can retry if the evacuation onto the chosen host fails. Theweigher_failover_hoststep then gives that selected target a dominating weight so the pipeline's own sort lifts it to the front. Both apply the same deterministic rule over the same reservation snapshot (prefer the reservation already In Use by this source, never one In Use by another source, otherwise the first Active one by name), so they agree without coordinating. A reservation is only selectable as a target once it is Active — fully drained and empty; before that it still blocks ordinary placements but is never routed an evacuation onto a host that is not actually empty. Everything is toggled by thekvm.failoverHostMatching.enabledHelm value and honors the call-timeignoredReservationTypesoption so the path finder probe keeps discovering reserved hosts.The post-pipeline Matcher no longer rewrites the candidate list; it runs inside the controller's exclusive lock on write runs only and marks the selected reservation In Use strictly when that target is the pipeline's top candidate. Gating the write on the actual outcome means a target that lost to other constraints leaves no dangling In Use, and serializing it under the lock makes gang-evacuation deterministic: the first VM from a dead host pins the target, and every subsequent VM from the same source reuses it and lands on the same host. When no reservation matches, all are taken by other sources, or the selected one is not chosen, it falls back to the ordinary candidate set and emits a metric that drives a new alert on frequent occurrences. The Matcher only ever advances a reservation to In Use; moving it back out stays with the failover host lifecycle reconciler. This PR also drops the redundant FailoverHostDesignated status condition — the draining stage is simply the absence of FailoverHostActive, the same as a freshly created reservation.