diff --git a/api/v1alpha1/reservation_types.go b/api/v1alpha1/reservation_types.go index 0847c27ca..3e2f186ac 100644 --- a/api/v1alpha1/reservation_types.go +++ b/api/v1alpha1/reservation_types.go @@ -36,11 +36,11 @@ const ( // its workloads to a designated spot. If this type is set, the reservation // will follow this lifecycle: // - // 1. Designation. The reservation has been created on a host. There may be - // virtual machines running on this host that need live migration away. As - // long as not all virtual machines can be migrated away from the host, the - // reservation stays in this state. For pipeline: no new virtual machine - // onto this host. + // 1. Designation. The reservation has been created on a host, but is not yet + // Active (it has no FailoverHostActive=True condition). There may be virtual + // machines running on this host that need live migration away. As long as not + // all virtual machines can be migrated away from the host, the reservation + // stays in this state. For pipeline: no new virtual machine onto this host. // // 2. Active. No more virtual machines on this host. On evacuation this // space can be used. @@ -331,22 +331,15 @@ type FailoverHostReservationStatus struct { type ReservationConditionType = string const ( - // ConditionTypeFailoverHostDesignated indicates that the reservation has - // been created on a host. There may be virtual machines running on this - // host that need live migration away. As long as not all virtual machines - // can be migrated away from the host, the reservation stays in this state. - // For pipeline: no new virtual machine onto this host. - // - // status=Unknown: It is unknown if the failover host has been designated. - // status=False: The failover host has not been designated (there was an error). - // status=True: The failover host has been designated successfully. - ConditionTypeFailoverHostDesignated ReservationConditionType = "FailoverHostDesignated" - // ConditionTypeFailoverHostActive indicates that the failover host is // active and ready to accept evacuations. No more virtual machines have // been seen on this host, so it is empty and ready to receive all vms // from another host. // + // A reservation without this condition set to True (including a freshly + // created one) represents the draining stage: it already blocks new virtual + // machines from the host, but is not yet a selectable evacuation target. + // // status=Unknown: The failover host is not yet ready to be used. // status=False: There has been some error or issue preventing the failover // host from being active. diff --git a/cmd/manager/main.go b/cmd/manager/main.go index a567fd7f4..3a3d0ca49 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -426,6 +426,8 @@ func main() { featureGates := conf.GetConfigOrDie[nova.FeatureGates]() noHostFoundCounter := crs.NewNoHostFoundCounter() placementCounter := crs.NewPlacementCounter() + failoverHostMatcherMonitor := failoverhost.NewMatcherMonitor() + metrics.Registry.MustRegister(failoverHostMatcherMonitor) // Filter-weigher pipeline controller setup. filterWeigherController := &nova.FilterWeigherPipelineController{ Monitor: filterWeigherPipelineMonitor, @@ -452,7 +454,14 @@ func main() { setupLog.Info("loaded nova API config", "evacuationShuffleK", novaAPIConfig.EvacuationShuffleK, "novaLimitHostsToRequest", novaAPIConfig.NovaLimitHostsToRequest) - nova.NewAPI(novaAPIConfig, novaScheduler, vmHandler, crRecorder, featureGates).Init(mux) + // FailoverMatcher pins an evacuation's gang target to a failover host + // reservation after the pipeline runs, as a post-hook under the scheduler + // lock (see nova.NewAPI). + failoverMatcher := &failoverhost.Matcher{ + Client: multiclusterClient, + Monitor: failoverHostMatcherMonitor, + } + nova.NewAPI(novaAPIConfig, novaScheduler, vmHandler, crRecorder, featureGates, failoverMatcher).Init(mux) // Detector pipeline controller setup. novaClient := nova.NewNovaClient() diff --git a/docs/03-reservations-and-inventory/04-failover-host-reservations.md b/docs/03-reservations-and-inventory/04-failover-host-reservations.md index 757ab26db..5ced35d17 100644 --- a/docs/03-reservations-and-inventory/04-failover-host-reservations.md +++ b/docs/03-reservations-and-inventory/04-failover-host-reservations.md @@ -460,8 +460,11 @@ stateDiagram-v2 ``` - **Designated** — the host has been chosen as a target but may still have VMs that need draining off it. No - *new* VMs are placed here. It stays in this stage until the host is empty. -- **Active** — the host is empty and ready to absorb an evacuation. + *new* VMs are placed here. It stays in this stage until the host is empty. This is the *absence* of the + `FailoverHostActive` condition rather than a condition of its own: a freshly created reservation is + Designated by default. +- **Active** — the host is empty and ready to absorb an evacuation (the `FailoverHostActive` condition is + True). Only from here on can an evacuation be routed onto it. - **Failed** — draining failed repeatedly or took too long; the host is held back from being re-chosen for a cooldown period. - **In Use** — a gang evacuation onto this host is underway (and may absorb further failures until it @@ -475,6 +478,98 @@ stateDiagram-v2 > independently rather than as a single current-state field, so the arrows show the normal path rather than the > only possible transitions. +## How failover host reservations affect scheduling + +Creating a reservation is only half the story — the scheduler has to *honor* it. Three cooperating pieces +wire failover host reservations into the Nova scheduling pipeline, split along a natural seam: a **filter** +and a **weigher** shape the candidate list *inside* the pipeline, and a thin **post-pipeline step** records +the outcome. Together they keep new workload *off* reserved hosts and steer a dead host's workload *onto* the +one reserved host it should gang-evacuate to — while deliberately *keeping* the ordinary candidates so Nova +can retry if that landing fails. + +### Selecting the target: one deterministic rule + +For an evacuation (identified by the `evacuate` intent) the dead host is read from the `source_host` +scheduler hint, and a single **deterministic rule** picks which reserved host this source should evacuate +onto. Both the filter and the weigher apply this same rule, independently, over the *same* reservation +snapshot within one pipeline run, so they always agree without coordinating: + +1. Keep the failover host reservations whose `sourceHosts` include the source host — the reservations this + host may evacuate onto. +2. If one is already **In Use** *by this same source host*, that is the selected target (the + gang-evacuation path — the whole gang lands on the same target). +3. Otherwise ignore any reservation **In Use** by a *different* source (pinning it would steal a landing + zone actively absorbing another host's gang), and any reservation that is not yet **Active** (still + draining, so not a usable landing zone yet — see the lifecycle below), then sort the rest by target + host name and take the first. +4. If nothing remains, there is no match and the evacuation simply uses the ordinary candidates. + +> [!NOTE] +> A reservation only becomes a *selectable* evacuation target once it is **Active** (its host has been +> fully drained). Before that — from the moment it is created, while it is still draining — it already +> **blocks** ordinary placements (the filter removes it for non-evacuation requests), but it is skipped by +> step 3 above. This keeps VMs off a host that is being emptied without yet routing an evacuation onto a +> host that is not actually empty. + +### The filter: block reserved hosts, keep only the selected target + +The **`filter_failover_host`** pipeline filter is purely *subtractive* — it only ever removes hosts — so its +position in the pipeline does not matter and all other filters and weighers still apply around it. It is +toggled by the `kvm.failoverHostMatching.enabled` Helm value (default `false`) and is a no-op when disabled. What it +removes depends on the request: + +- **Ordinary (non-evacuation) requests:** it removes *every* failover host, so no new VM is placed on a host + being drained or kept empty as a landing zone. +- **Evacuations:** it removes every failover host *except* the one selected by the rule above. The ordinary, + non-reserved candidates are left untouched. + +Pruning the *other* reserved hosts is the key to mutual exclusion between failover targets. Consider a VM +that could go to reserved `[h1, h2]` (both failover targets reachable from this source) plus ordinary +`[h3, h4]`, where only `h1` is selected. The filter emits `[h1, h3, h4]` — **not** `[h1, h2, h3, h4]`: `h2` +is held back so a *second*, concurrent host failure still has its own free landing zone rather than finding +it already consumed. Keeping `h3` and `h4` means that if the evacuation onto `h1` fails, Nova retries onto an +ordinary host instead of being forced onto an unmatched reserved slot. + +The filter also honors the **call-time** `ignoredReservationTypes` request option. This is what keeps the +**path finder probe** working: the probe asks the scheduler which hosts would accept a flavor, and it passes +`ignoredReservationTypes: [FailoverHostReservation]` precisely so a reserved host is *not* hidden from its own +partition. Because the filter obeys that option, the probe still discovers already-reserved hosts and the +partitioning does not fragment. + +### The weigher: lift the selected target to the front + +The **`weigher_failover_host`** weigher gives the selected target a dominating weight — far larger than any +other weigher can contribute — so the pipeline's ordinary descending-weight sort places it first. It selects +the same host the filter kept (same rule, same snapshot), so it simply lifts the one reserved host that +survived filtering above the retained ordinary candidates. For non-evacuation requests, or when no target is +selected, it has no effect. Because the weigher only *weighs*, the ordinary candidates keep their relative +order behind the target and remain available as retry fallbacks. + +### The post-pipeline step: pin the target for gang-evacuation + +A thin **Matcher** runs *after* the pipeline has produced its ordered candidates, from inside the pipeline's +global execution lock, on write runs only. It does **not** touch the candidate list — the filter and weigher +have already shaped it. Its sole job is the stateful write: it re-derives the selected reservation with the +same rule and, **only if that target is actually the top candidate**, marks it **In Use** and records the +source host on its status. + +Gating the write on the *outcome* is deliberate. If the selected target lost to other scheduling constraints +(the filter removed it, so an ordinary host ended up on top), the evacuation falls back to that ordinary host +and the Matcher writes nothing — no dangling `In Use` is left behind. In that case, and when no reservation +matches or all are taken by other sources, it increments a metric +(`cortex_failover_host_evacuations_without_reservation_total`) that drives an alert on frequent occurrences. + +Running inside the global pipeline lock is what makes **gang-evacuation** work. The *first* VM evacuated from +a dead host pins its target and writes `In Use` while holding the lock; every subsequent VM from the same +source host — serialized behind the same lock — sees that `In Use` reservation, so the selection rule keeps +choosing it (step 2) and the whole gang lands on the *same* target. The reuse is idempotent: a reservation +already `In Use` by this source needs no second write. Writing the pin *after* the pipeline (rather than +inside the filter) is what lets the write be gated on the final ordering. + +The Matcher only ever advances a reservation *to* `In Use`. Moving it back out — to `Active` once the +evacuation completes, or on cleanup — stays with the failover host lifecycle reconciler, based on the real +workload present on the host. + ## Configuration Enable the controller and configure it under the nested `failover.hosts` key: diff --git a/helm/bundles/cortex-nova/templates/alerts.yaml b/helm/bundles/cortex-nova/templates/alerts.yaml index 18322417b..e43182317 100644 --- a/helm/bundles/cortex-nova/templates/alerts.yaml +++ b/helm/bundles/cortex-nova/templates/alerts.yaml @@ -1093,5 +1093,32 @@ spec: path finder are failing (transport errors or non-2xx responses). The path finder cannot determine valid evacuation targets, so failover partitions and reservations cannot be recomputed. Check that the external scheduler is reachable and healthy. + - alert: CortexNovaFailoverHostEvacuationsWithoutReservation + # An evacuation came in whose source host matched no failover host + # reservation, so the request fell back to ordinary candidates instead of a + # pre-validated, gang-evacuable landing zone. Frequent occurrences mean the + # partitioning is not keeping failover headroom where failures actually + # happen (e.g. the source host's partition reserves nothing, or the host + # left its partition), so evacuations are re-solved under pressure. + expr: sum by (az) (increase(cortex_failover_host_evacuations_without_reservation_total{service="cortex-nova-metrics"}[15m])) > 0 + for: 15m + labels: +{{- if .Values.alerts.persesDashboardBaseURL }} + persesDashboard: "{{ .Values.alerts.persesDashboardBaseURL }}/cortex_status?var-service=cortex-nova-metrics" +{{- end }} + context: failover-hosts + service: cortex + severity: warning + support_group: workload-management + playbook: "" + annotations: + summary: "Evacuations landing without a failover host reservation in `{{ "{{" }} $labels.az {{ "}}" }}`" + description: > + Evacuation scheduling requests in `{{ "{{" }} $labels.az {{ "}}" }}` are matching no + failover host reservation for their source host, so they fall back to the ordinary + candidate set rather than a pre-validated, gang-evacuable landing zone. This means + the source hosts have no reserved failover target — check whether their partition + reserves anything (a partition of fewer than two hosts reserves nothing) and the + failover host calculation loop is healthy. {{- end }} {{- end }} diff --git a/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml b/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml index fd631a303..e4ab7fcbf 100644 --- a/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml +++ b/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml @@ -83,6 +83,19 @@ spec: # FailoverHostReservation is a new feature and is not yet counted as # reserved capacity during scheduling. - {key: ignoredReservationTypes, stringListValue: ["FailoverHostReservation"]} + - name: filter_failover_host + description: | + This step keeps new virtual machines off hosts that have a failover host + reservation. For ordinary requests it removes every such host; for + evacuations it removes all but the one failover host deterministically + selected for the source host, so another failure keeps a free landing + zone, while ordinary candidates are retained so Nova can retry on failure. + The weigher_failover_host weigher then lifts the selected host to the + front. The filter is purely subtractive, so its position does not matter. + It honors the call-time ignoredReservationTypes option so the failover + host path finder probe still discovers reserved hosts. + params: + - {key: enabled, boolValue: {{ .Values.kvm.failoverHostMatching.enabled }}} - name: filter_allowed_projects description: | This step filters hosts based on allowed projects defined in the @@ -158,6 +171,16 @@ spec: VM is allocated get a higher weight, encouraging placement on pre-reserved failover capacity. For non-evacuation requests, this weigher has no effect. + - name: weigher_failover_host + description: | + For an evacuation, this weigher lifts the failover host selected for the + request's source host to the front of the candidate list by giving it a + dominating weight, so it becomes Nova's first choice while the retained + ordinary candidates stay behind it as retry fallbacks. It selects the same + host the filter_failover_host filter kept. For non-evacuation requests it + has no effect. + params: + - {key: enabled, boolValue: {{ .Values.kvm.failoverHostMatching.enabled }}} - name: kvm_committed_resource_reservation description: | This weigher boosts hosts that have a ready CommittedResourceReservation @@ -258,6 +281,19 @@ spec: # FailoverHostReservation is a new feature and is not yet counted as # reserved capacity during scheduling. - {key: ignoredReservationTypes, stringListValue: ["FailoverHostReservation"]} + - name: filter_failover_host + description: | + This step keeps new virtual machines off hosts that have a failover host + reservation. For ordinary requests it removes every such host; for + evacuations it removes all but the one failover host deterministically + selected for the source host, so another failure keeps a free landing + zone, while ordinary candidates are retained so Nova can retry on failure. + The weigher_failover_host weigher then lifts the selected host to the + front. The filter is purely subtractive, so its position does not matter. + It honors the call-time ignoredReservationTypes option so the failover + host path finder probe still discovers reserved hosts. + params: + - {key: enabled, boolValue: {{ .Values.kvm.failoverHostMatching.enabled }}} - name: filter_allowed_projects description: | This step filters hosts based on allowed projects defined in the @@ -330,6 +366,16 @@ spec: VM is allocated get a higher weight, encouraging placement on pre-reserved failover capacity. For non-evacuation requests, this weigher has no effect. + - name: weigher_failover_host + description: | + For an evacuation, this weigher lifts the failover host selected for the + request's source host to the front of the candidate list by giving it a + dominating weight, so it becomes Nova's first choice while the retained + ordinary candidates stay behind it as retry fallbacks. It selects the same + host the filter_failover_host filter kept. For non-evacuation requests it + has no effect. + params: + - {key: enabled, boolValue: {{ .Values.kvm.failoverHostMatching.enabled }}} - name: kvm_committed_resource_reservation description: | This weigher boosts hosts that have a ready CommittedResourceReservation @@ -583,6 +629,19 @@ spec: # FailoverHostReservation is a new feature and is not yet counted as # reserved capacity during scheduling. - {key: ignoredReservationTypes, stringListValue: ["FailoverHostReservation"]} + - name: filter_failover_host + description: | + This step keeps new virtual machines off hosts that have a failover host + reservation. For ordinary requests it removes every such host; for + evacuations it removes all but the one failover host deterministically + selected for the source host, so another failure keeps a free landing + zone, while ordinary candidates are retained so Nova can retry on failure. + The weigher_failover_host weigher then lifts the selected host to the + front. The filter is purely subtractive, so its position does not matter. + It honors the call-time ignoredReservationTypes option so the failover + host path finder probe still discovers reserved hosts. + params: + - {key: enabled, boolValue: {{ .Values.kvm.failoverHostMatching.enabled }}} - name: filter_has_requested_traits description: | This step filters hosts that do not have the requested traits given by the @@ -623,6 +682,16 @@ spec: VM is allocated get a higher weight, encouraging placement on pre-reserved failover capacity. For non-evacuation requests, this weigher has no effect. + - name: weigher_failover_host + description: | + For an evacuation, this weigher lifts the failover host selected for the + request's source host to the front of the candidate list by giving it a + dominating weight, so it becomes Nova's first choice while the retained + ordinary candidates stay behind it as retry fallbacks. It selects the same + host the filter_failover_host filter kept. For non-evacuation requests it + has no effect. + params: + - {key: enabled, boolValue: {{ .Values.kvm.failoverHostMatching.enabled }}} --- apiVersion: cortex.cloud/v1alpha1 kind: Pipeline diff --git a/helm/bundles/cortex-nova/values.yaml b/helm/bundles/cortex-nova/values.yaml index 0ef1ace0b..17651e7eb 100644 --- a/helm/bundles/cortex-nova/values.yaml +++ b/helm/bundles/cortex-nova/values.yaml @@ -109,6 +109,16 @@ kvm: # Domain names that are explicitly treated as non-external-customer domains and # skipped by the filter, even if they match one of the prefixes above. ignoredDomainNames: [] + # Options for the failover host matching scheduling steps. The + # filter_failover_host filter keeps new VMs off hosts with a failover host + # reservation (and, for evacuations, prunes all but the deterministically + # selected failover host so another failure keeps a free landing zone); the + # weigher_failover_host weigher lifts that selected host to the front of the + # candidate list. Applied to every KVM pipeline that uses the steps. + failoverHostMatching: + # Disabled by default until the failover host draining controller logic is + # implemented. Set to true to enable failover-host-aware scheduling. + enabled: false # CommittedResource states that count toward the CR portion of project quota. # Shared by the quota controller (PaygUsage derivation) and the # filter_quota_enforcement pipeline step (CR deduction) — the two MUST agree, diff --git a/internal/scheduling/nova/external_scheduler_api.go b/internal/scheduling/nova/external_scheduler_api.go index 409a0f7f1..5896af198 100644 --- a/internal/scheduling/nova/external_scheduler_api.go +++ b/internal/scheduling/nova/external_scheduler_api.go @@ -56,33 +56,51 @@ type runner interface { Run(ctx context.Context, req compute.PlacementRequest, pre compute.PreHook, post compute.PostHook) (compute.PlacementResult, error) } +// FailoverHostMatcher pins an evacuation's gang target to a failover host +// reservation after the pipeline runs, marking the matching reservation InUse so +// subsequent evacuations from the same source land on the same target. It is run +// as a post-hook, under the scheduler lock. Implemented by +// reservations/failoverhost.Matcher; an interface here so the handler can be +// unit-tested and nova need not import failoverhost at the call site. +type FailoverHostMatcher interface { + // ConfirmAndMarkInUse marks the failover host reservation selected for the + // request's source host InUse, but only when it is the pipeline's top + // candidate. It does not change the candidate list. + ConfirmAndMarkInUse(ctx context.Context, request api.ExternalSchedulerRequest, orderedHosts []string) error +} + type httpAPI struct { - monitor scheduling.APIMonitor - scheduler runner - vmHandler *VirtualMachineHandler - crRecorder crs.Recorder - featureGates FeatureGates - config HTTPAPIConfig + monitor scheduling.APIMonitor + scheduler runner + vmHandler *VirtualMachineHandler + crRecorder crs.Recorder + featureGates FeatureGates + config HTTPAPIConfig + failoverMatcher FailoverHostMatcher } // NewAPI builds the Nova external-scheduler HTTP API. It calls the shared -// compute.Placement directly; VM-resource creation and committed-resource -// tracking run as post-hooks after the pipeline, under the scheduler's lock. +// compute.Placement directly; VM-resource creation, committed-resource tracking +// and failover-host pinning run as post-hooks after the pipeline, under the +// scheduler's lock. failoverMatcher may be nil, in which case no failover-host +// pinning is applied. func NewAPI( config HTTPAPIConfig, scheduler *compute.Placement, vmHandler *VirtualMachineHandler, crRecorder crs.Recorder, featureGates FeatureGates, + failoverMatcher FailoverHostMatcher, ) HTTPAPI { return &httpAPI{ - monitor: scheduling.NewSchedulerMonitor(), - scheduler: scheduler, - vmHandler: vmHandler, - crRecorder: crRecorder, - featureGates: featureGates, - config: config, + monitor: scheduling.NewSchedulerMonitor(), + scheduler: scheduler, + vmHandler: vmHandler, + crRecorder: crRecorder, + featureGates: featureGates, + config: config, + failoverMatcher: failoverMatcher, } } @@ -308,6 +326,20 @@ func (httpAPI *httpAPI) NovaExternalScheduler(w http.ResponseWriter, r *http.Req httpAPI.crRecorder.RecordNoHostFound(ctx, intent, requestData) } } + // Failover-host pinning. Mark the failover host reservation selected for + // an evacuation InUse, pinning the source->target relationship for + // gang-evacuation. The candidate shaping (removing other reserved hosts, + // lifting the selected target to the front) is already done inside the + // pipeline by the filter_failover_host filter and weigher_failover_host + // weigher; this only performs the serialized InUse write. It runs under + // the scheduler lock (write runs only) so the write is visible to + // subsequent evacuations from the same source. Read-only runs (probes, + // capacity checks) never pin. + if httpAPI.failoverMatcher != nil && !requestData.Options.ReadOnly { + if mErr := httpAPI.failoverMatcher.ConfirmAndMarkInUse(ctx, requestData, result.OrderedHosts); mErr != nil { + return fmt.Errorf("failover host matcher failed: %w", mErr) + } + } return nil } diff --git a/internal/scheduling/nova/plugins/filters/filter_failover_host.go b/internal/scheduling/nova/plugins/filters/filter_failover_host.go new file mode 100644 index 000000000..5c8c970c9 --- /dev/null +++ b/internal/scheduling/nova/plugins/filters/filter_failover_host.go @@ -0,0 +1,109 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package filters + +import ( + "context" + "slices" + + "github.com/go-logr/logr" + + api "github.com/cobaltcore-dev/cortex/api/external/nova" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" + "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/failoverhost" +) + +// FilterFailoverHostOpts configures the failover-host filter. +type FilterFailoverHostOpts struct { + // Enabled toggles the filter. When false, the filter is a no-op and all + // hosts pass. This lets the pipeline carry the step while keeping the + // behavior off until the feature is rolled out. + Enabled bool `json:"enabled"` +} + +func (FilterFailoverHostOpts) Validate() error { return nil } + +// FilterFailoverHost keeps ordinary virtual machines off hosts that have a +// failover host reservation, and for evacuations narrows the reserved hosts down +// to the single one this source host should gang-evacuate onto. +// +// For a non-evacuation request it removes every failover host, so no new virtual +// machine lands on a host being drained or kept empty as a gang-evacuation +// landing zone. +// +// For an evacuation request it removes every failover host except the one +// deterministically selected for the request's source host. Keeping only the +// selected target — and leaving the ordinary, non-reserved candidates untouched — +// means the other reserved hosts stay free for a second host failure, while Nova +// can still retry onto an ordinary candidate if the evacuation onto the selected +// target fails. The weigher_failover_host weigher then lifts the selected target +// to the front of the candidate list, applying the same selection rule. +// +// The filter is purely subtractive, so its position in the pipeline does not +// matter: other filters and weighers still run around it. +type FilterFailoverHost struct { + lib.BaseFilter[compute.PlacementRequest, FilterFailoverHostOpts] +} + +func (s *FilterFailoverHost) Run(traceLog logr.Logger, request compute.PlacementRequest) (*lib.FilterWeigherPipelineStepResult, error) { + result := s.IncludeAllHostsFromRequest(request) + if !s.Options.Enabled { + return result, nil + } + + // Honor the call-time ignored reservation types. When failover host + // reservations are ignored — as the path finder probe does (see + // reservations/failoverhost/pathfinding.go) — block nothing so reserved + // hosts stay visible to their own partition. + if slices.Contains(request.GetOptions().IgnoredReservationTypes, v1alpha1.ReservationTypeFailoverHost) { + traceLog.V(1).Info("failover host reservations ignored, not blocking any hosts") + return result, nil + } + + var reservations v1alpha1.ReservationList + if err := s.Client.List(context.Background(), &reservations); err != nil { + return nil, err + } + + // For an evacuation, keep the single failover host selected for this source so + // the gang lands there; for everything else, keep none. The selection + // (failoverhost.SelectFailoverHost) is a pure function of the reservation + // snapshot, so the weigher_failover_host weigher reaches the same target over + // the same snapshot without coordinating. A reservation is only selectable + // once it is Active (drained and empty); a still-draining one is not kept here + // — but note it is still blocked below, keeping ordinary VMs off it while it + // drains. + var keep string + if request.Intent == api.EvacuateIntent && request.SourceHost != "" { + if selected := failoverhost.SelectFailoverHost(reservations.Items, request.SourceHost); selected != nil { + keep = selected.Spec.TargetHost + } + } + + for _, reservation := range reservations.Items { + if reservation.Spec.Type != v1alpha1.ReservationTypeFailoverHost { + continue + } + // Block both the desired (TargetHost) and actual (Status.Host) host to + // cover the window while the reservation is being placed. A reservation + // blocks from creation onward (the controller deletes Revoked/Used ones), + // so its presence is enough to keep new virtual machines off the host, + // whether or not it is Active yet. + for _, host := range []string{reservation.Spec.TargetHost, reservation.Status.Host} { + if host == "" || host == keep { + continue + } + delete(result.Activations, host) // noop if host is not in the map + traceLog.V(1).Info("filtering out failover host reservation target", + "host", host, "reservation", reservation.Name) + } + } + return result, nil +} + +func init() { + Index["filter_failover_host"] = func() NovaFilter { return &FilterFailoverHost{} } +} diff --git a/internal/scheduling/nova/plugins/filters/filter_failover_host_test.go b/internal/scheduling/nova/plugins/filters/filter_failover_host_test.go new file mode 100644 index 000000000..17302c904 --- /dev/null +++ b/internal/scheduling/nova/plugins/filters/filter_failover_host_test.go @@ -0,0 +1,265 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package filters + +import ( + "testing" + + "github.com/go-logr/logr" + + api "github.com/cobaltcore-dev/cortex/api/external/nova" + "github.com/cobaltcore-dev/cortex/api/scheduling" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// failoverHostReservation builds an Active failover host reservation targeting +// targetHost (with statusHost as its actual host), able to rescue the given +// source hosts. Active is the state in which a reservation can be selected as an +// evacuation target; use notActive() to drop it back to the draining state. +func failoverHostReservation(targetHost, statusHost string, sourceHosts ...string) *v1alpha1.Reservation { + res := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "failover-host-" + targetHost}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeFailoverHost, + TargetHost: targetHost, + FailoverHostReservation: &v1alpha1.FailoverHostReservationSpec{ + SourceHosts: sourceHosts, + }, + }, + Status: v1alpha1.ReservationStatus{Host: statusHost}, + } + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ConditionTypeFailoverHostActive, + Status: metav1.ConditionTrue, + Reason: "HostDrained", + }) + return res +} + +// notActive drops a reservation out of the Active state, back to the draining +// stage (the same as a freshly created reservation): it still blocks ordinary +// VMs but cannot yet be selected for evacuation. +func notActive(res *v1alpha1.Reservation) *v1alpha1.Reservation { + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ConditionTypeFailoverHostActive, + Status: metav1.ConditionFalse, + Reason: "Draining", + }) + return res +} + +// inUseBy marks a failover host reservation as in use by the given source host. +func inUseBy(res *v1alpha1.Reservation, sourceHost string) *v1alpha1.Reservation { + res.Status.FailoverHostReservation = &v1alpha1.FailoverHostReservationStatus{SourceHost: sourceHost} + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ConditionTypeFailoverHostInUse, + Status: metav1.ConditionTrue, + Reason: "EvacuationStarted", + }) + return res +} + +func committedResourceReservationOn(targetHost string) *v1alpha1.Reservation { + return &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "cr-" + targetHost}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + TargetHost: targetHost, + }, + } +} + +// requestWithHosts builds a typed placement request over the named candidate +// hosts, with the given intent and source host (empty for non-evacuations). +func requestWithHosts(intent v1alpha1.SchedulingIntent, sourceHost string, hosts ...string) compute.PlacementRequest { + hvHosts := make([]compute.Host, len(hosts)) + for i, h := range hosts { + hvHosts[i] = compute.Host{ComputeHost: h} + } + return compute.PlacementRequest{ + Hosts: hvHosts, + Intent: intent, + SourceHost: sourceHost, + } +} + +func TestFilterFailoverHost_Run(t *testing.T) { + scheme := buildTestScheme(t) + + tests := []struct { + name string + opts FilterFailoverHostOpts + reservations []client.Object + request compute.PlacementRequest + expectedHosts []string + }{ + { + name: "disabled - all hosts pass even with reservations", + opts: FilterFailoverHostOpts{Enabled: false}, + reservations: []client.Object{failoverHostReservation("host2", "")}, + request: requestWithHosts("", "", "host1", "host2", "host3"), + expectedHosts: []string{"host1", "host2", "host3"}, + }, + { + name: "non-evacuation - blocks reserved target host", + opts: FilterFailoverHostOpts{Enabled: true}, + reservations: []client.Object{failoverHostReservation("host2", "")}, + request: requestWithHosts("", "", "host1", "host2", "host3"), + expectedHosts: []string{"host1", "host3"}, + }, + { + name: "non-evacuation - blocks both target and status host", + opts: FilterFailoverHostOpts{Enabled: true}, + reservations: []client.Object{failoverHostReservation("host3", "host4")}, + request: requestWithHosts("", "", "host1", "host2", "host3", "host4"), + expectedHosts: []string{"host1", "host2"}, + }, + { + name: "non-evacuation - ignores non-failover-host reservations", + opts: FilterFailoverHostOpts{Enabled: true}, + reservations: []client.Object{committedResourceReservationOn("host2")}, + request: requestWithHosts("", "", "host1", "host2", "host3"), + expectedHosts: []string{"host1", "host2", "host3"}, + }, + { + name: "evacuation - keeps the selected target and prunes other failover targets", + opts: FilterFailoverHostOpts{Enabled: true}, + // h1 and h2 are both reachable from source1; only the name-first (h1) + // is kept, h2 is pruned to stay free for another failure. h3/h4 ordinary. + reservations: []client.Object{ + failoverHostReservation("h1", "", "source1"), + failoverHostReservation("h2", "", "source1"), + }, + request: requestWithHosts(api.EvacuateIntent, "source1", "h1", "h2", "h3", "h4"), + expectedHosts: []string{"h1", "h3", "h4"}, + }, + { + name: "evacuation - reuses the in-use target for the same source", + opts: FilterFailoverHostOpts{Enabled: true}, + // h2 is in use by source1, so it is the selected target even though h1 + // sorts first; h1 is pruned. + reservations: []client.Object{ + failoverHostReservation("h1", "", "source1"), + inUseBy(failoverHostReservation("h2", "", "source1"), "source1"), + }, + request: requestWithHosts(api.EvacuateIntent, "source1", "h1", "h2", "h3"), + expectedHosts: []string{"h2", "h3"}, + }, + { + name: "evacuation - never keeps a target in use by another source", + opts: FilterFailoverHostOpts{Enabled: true}, + // h1 is in use by source2, so source1 selects the free h2; both reserved + // hosts except the selected h2 are pruned. + reservations: []client.Object{ + inUseBy(failoverHostReservation("h1", "", "source1", "source2"), "source2"), + failoverHostReservation("h2", "", "source1", "source2"), + }, + request: requestWithHosts(api.EvacuateIntent, "source1", "h1", "h2", "h3"), + expectedHosts: []string{"h2", "h3"}, + }, + { + name: "evacuation - no matching reservation removes all failover hosts", + opts: FilterFailoverHostOpts{Enabled: true}, + reservations: []client.Object{ + failoverHostReservation("h1", "", "otherSource"), + }, + request: requestWithHosts(api.EvacuateIntent, "source1", "h1", "h2", "h3"), + expectedHosts: []string{"h2", "h3"}, + }, + { + name: "evacuation - no source hint removes all failover hosts", + opts: FilterFailoverHostOpts{Enabled: true}, + reservations: []client.Object{failoverHostReservation("h1", "", "source1")}, + request: requestWithHosts(api.EvacuateIntent, "", "h1", "h2", "h3"), + expectedHosts: []string{"h2", "h3"}, + }, + { + name: "evacuation - a not-yet-active target is still blocked", + opts: FilterFailoverHostOpts{Enabled: true}, + // h1 is freshly created (not Active yet), so it cannot be a landing + // zone and is removed like any other reserved host, even though the + // source could otherwise evacuate onto it. + reservations: []client.Object{notActive(failoverHostReservation("h1", "", "source1"))}, + request: requestWithHosts(api.EvacuateIntent, "source1", "h1", "h2", "h3"), + expectedHosts: []string{"h2", "h3"}, + }, + { + name: "evacuation - selects the active target and prunes a not-yet-active one", + opts: FilterFailoverHostOpts{Enabled: true}, + // h1 sorts first but is not Active yet; h2 is Active, so h2 is the + // selected target and h1 is pruned as an unusable reserved host. + reservations: []client.Object{ + notActive(failoverHostReservation("h1", "", "source1")), + failoverHostReservation("h2", "", "source1"), + }, + request: requestWithHosts(api.EvacuateIntent, "source1", "h1", "h2", "h3"), + expectedHosts: []string{"h2", "h3"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + step := &FilterFailoverHost{} + step.Options = tt.opts + step.Client = fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.reservations...).Build() + + result, err := step.Run(logr.Discard(), tt.request) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if len(result.Activations) != len(tt.expectedHosts) { + t.Errorf("expected %d hosts, got %d: %v", len(tt.expectedHosts), len(result.Activations), result.Activations) + } + for _, host := range tt.expectedHosts { + if _, ok := result.Activations[host]; !ok { + t.Errorf("expected host %s to be present", host) + } + } + }) + } +} + +func TestFilterFailoverHost_CallTimeIgnoredReservationTypes(t *testing.T) { + scheme := buildTestScheme(t) + step := &FilterFailoverHost{} + step.Options = FilterFailoverHostOpts{Enabled: true} + step.Client = fake.NewClientBuilder().WithScheme(scheme). + WithObjects(failoverHostReservation("host2", "")).Build() + + request := requestWithHosts("", "", "host1", "host2", "host3") + request.Options = scheduling.Options{ + IgnoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeFailoverHost}, + } + result, err := step.Run(logr.Discard(), request) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if _, ok := result.Activations["host2"]; !ok { + t.Error("expected host2 to survive when failover host reservations are ignored at call time") + } +} + +func TestFilterFailoverHost_Index(t *testing.T) { + factory, ok := Index["filter_failover_host"] + if !ok { + t.Fatal("expected filter_failover_host to be registered in Index") + } + if _, ok := factory().(*FilterFailoverHost); !ok { + t.Fatal("expected factory to return a *FilterFailoverHost") + } +} + +func TestFilterFailoverHost_OptsValidate(t *testing.T) { + if err := (FilterFailoverHostOpts{}).Validate(); err != nil { + t.Errorf("expected nil error, got %v", err) + } + if err := (FilterFailoverHostOpts{Enabled: true}).Validate(); err != nil { + t.Errorf("expected nil error, got %v", err) + } +} diff --git a/internal/scheduling/nova/plugins/weighers/weigher_failover_host.go b/internal/scheduling/nova/plugins/weighers/weigher_failover_host.go new file mode 100644 index 000000000..405ccbd90 --- /dev/null +++ b/internal/scheduling/nova/plugins/weighers/weigher_failover_host.go @@ -0,0 +1,90 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package weighers + +import ( + "context" + + "github.com/go-logr/logr" + + api "github.com/cobaltcore-dev/cortex/api/external/nova" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" + "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/failoverhost" +) + +// failoverTargetWeight is the activation assigned to the failover host a source +// host should gang-evacuate onto. It is deliberately far larger than any other +// weigher's contribution: the pipeline squashes each step with tanh (bounding it +// to ~[-1, 1]) before summing, so a value this large saturates to ~+1 and, being +// the only host lifted at all, is guaranteed to sort to the front of the +// candidate list. That makes the selected target Nova's first choice while the +// ordinary candidates keep their relative order behind it as retry fallbacks. +const failoverTargetWeight = 1e6 + +// WeigherFailoverHostOpts configures the failover-host weigher. +type WeigherFailoverHostOpts struct { + // Enabled toggles the weigher. When false, the weigher is a no-op and has no + // effect on any host's weight. + Enabled bool `json:"enabled"` +} + +func (WeigherFailoverHostOpts) Validate() error { return nil } + +// WeigherFailoverHost lifts the failover host an evacuation should land on to the +// front of the candidate list, by giving it a dominating weight. +// +// It selects the target with the same deterministic rule as the +// filter_failover_host filter (which has already removed the other reserved +// hosts), so the two agree without coordinating: both run over the same +// reservation snapshot in one pipeline execution. The filter keeps only the +// selected target among the reserved hosts; this weigher then ensures that target +// outranks the retained ordinary candidates. +type WeigherFailoverHost struct { + lib.BaseWeigher[compute.PlacementRequest, WeigherFailoverHostOpts] +} + +func (s *WeigherFailoverHost) Run(traceLog logr.Logger, request compute.PlacementRequest) (*lib.FilterWeigherPipelineStepResult, error) { + result := s.IncludeAllHostsFromRequest(request) + if !s.Options.Enabled { + return result, nil + } + if request.Intent != api.EvacuateIntent || request.SourceHost == "" { + return result, nil + } + + var reservations v1alpha1.ReservationList + if err := s.Client.List(context.Background(), &reservations); err != nil { + return nil, err + } + + // Select the one failover host this source should gang-evacuate onto, with the + // same deterministic rule the filter_failover_host filter used over the same + // snapshot (failoverhost.SelectFailoverHost), so the filter and weigher agree + // without coordinating. + selected := failoverhost.SelectFailoverHost(reservations.Items, request.SourceHost) + if selected == nil { + return result, nil + } + target := selected.Spec.TargetHost + if _, ok := result.Activations[target]; !ok { + // The selected target conflicts with other scheduling constraints and was + // filtered out, so there is nothing to lift. + traceLog.V(1).Info("selected failover host not among candidates, not weighing", + "sourceHost", request.SourceHost, "targetHost", target) + return result, nil + } + result.Activations[target] = failoverTargetWeight + stats := s.PrepareStats(request, "failover host target [bool]") + stats.Hosts[target] = 1 + result.Statistics["failover host target"] = stats + traceLog.V(1).Info("weighting failover host target to the front", + "sourceHost", request.SourceHost, "targetHost", target) + return result, nil +} + +func init() { + Index["weigher_failover_host"] = func() NovaWeigher { return &WeigherFailoverHost{} } +} diff --git a/internal/scheduling/nova/plugins/weighers/weigher_failover_host_test.go b/internal/scheduling/nova/plugins/weighers/weigher_failover_host_test.go new file mode 100644 index 000000000..53aba9007 --- /dev/null +++ b/internal/scheduling/nova/plugins/weighers/weigher_failover_host_test.go @@ -0,0 +1,204 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package weighers + +import ( + "testing" + + "github.com/go-logr/logr" + + api "github.com/cobaltcore-dev/cortex/api/external/nova" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// failoverHostReservation builds an Active failover host reservation (the state +// in which it can be selected as an evacuation target). +func failoverHostReservation(targetHost string, sourceHosts ...string) *v1alpha1.Reservation { + res := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "failover-host-" + targetHost}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeFailoverHost, + TargetHost: targetHost, + FailoverHostReservation: &v1alpha1.FailoverHostReservationSpec{ + SourceHosts: sourceHosts, + }, + }, + } + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ConditionTypeFailoverHostActive, + Status: metav1.ConditionTrue, + Reason: "HostDrained", + }) + return res +} + +// notActive drops a reservation out of the Active state (the same as a freshly +// created, still-draining reservation). +func notActive(res *v1alpha1.Reservation) *v1alpha1.Reservation { + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ConditionTypeFailoverHostActive, + Status: metav1.ConditionFalse, + Reason: "Draining", + }) + return res +} + +func failoverInUseBy(res *v1alpha1.Reservation, sourceHost string) *v1alpha1.Reservation { + res.Status.FailoverHostReservation = &v1alpha1.FailoverHostReservationStatus{SourceHost: sourceHost} + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ConditionTypeFailoverHostInUse, + Status: metav1.ConditionTrue, + Reason: "EvacuationStarted", + }) + return res +} + +// weigherRequest builds a typed placement request over the named candidate +// hosts, with the given intent and source host (empty for non-evacuations). +func weigherRequest(intent v1alpha1.SchedulingIntent, sourceHost string, hosts ...string) compute.PlacementRequest { + hvHosts := make([]compute.Host, len(hosts)) + for i, h := range hosts { + hvHosts[i] = compute.Host{ComputeHost: h} + } + return compute.PlacementRequest{ + Hosts: hvHosts, + Intent: intent, + SourceHost: sourceHost, + } +} + +func TestWeigherFailoverHost_Run(t *testing.T) { + scheme := buildTestScheme(t) + + tests := []struct { + name string + opts WeigherFailoverHostOpts + reservations []client.Object + request compute.PlacementRequest + // wantTarget is the host expected to receive the dominating weight, or "" + // when no host should be lifted. + wantTarget string + }{ + { + name: "disabled - no effect", + opts: WeigherFailoverHostOpts{Enabled: false}, + reservations: []client.Object{failoverHostReservation("h1", "source1")}, + request: weigherRequest(api.EvacuateIntent, "source1", "h1", "h2"), + wantTarget: "", + }, + { + name: "non-evacuation - no effect", + opts: WeigherFailoverHostOpts{Enabled: true}, + reservations: []client.Object{failoverHostReservation("h1", "source1")}, + request: weigherRequest(api.LiveMigrationIntent, "source1", "h1", "h2"), + wantTarget: "", + }, + { + name: "evacuation without source hint - no effect", + opts: WeigherFailoverHostOpts{Enabled: true}, + reservations: []client.Object{failoverHostReservation("h1", "source1")}, + request: weigherRequest(api.EvacuateIntent, "", "h1", "h2"), + wantTarget: "", + }, + { + name: "evacuation - lifts the name-first matching target", + opts: WeigherFailoverHostOpts{Enabled: true}, + reservations: []client.Object{failoverHostReservation("h1", "source1"), failoverHostReservation("h2", "source1")}, + request: weigherRequest(api.EvacuateIntent, "source1", "h1", "h2", "h3"), + wantTarget: "h1", + }, + { + name: "evacuation - lifts the in-use target for the same source", + opts: WeigherFailoverHostOpts{Enabled: true}, + reservations: []client.Object{ + failoverHostReservation("h1", "source1"), + failoverInUseBy(failoverHostReservation("h2", "source1"), "source1"), + }, + request: weigherRequest(api.EvacuateIntent, "source1", "h1", "h2", "h3"), + wantTarget: "h2", + }, + { + name: "evacuation - no matching reservation - no effect", + opts: WeigherFailoverHostOpts{Enabled: true}, + reservations: []client.Object{failoverHostReservation("h1", "otherSource")}, + request: weigherRequest(api.EvacuateIntent, "source1", "h1", "h2"), + wantTarget: "", + }, + { + name: "evacuation - not-yet-active target is not lifted", + opts: WeigherFailoverHostOpts{Enabled: true}, + reservations: []client.Object{notActive(failoverHostReservation("h1", "source1"))}, + request: weigherRequest(api.EvacuateIntent, "source1", "h1", "h2"), + wantTarget: "", + }, + { + name: "evacuation - lifts the active target over a not-yet-active one", + opts: WeigherFailoverHostOpts{Enabled: true}, + reservations: []client.Object{ + notActive(failoverHostReservation("h1", "source1")), + failoverHostReservation("h2", "source1"), + }, + request: weigherRequest(api.EvacuateIntent, "source1", "h1", "h2", "h3"), + wantTarget: "h2", + }, + { + name: "evacuation - selected target not among candidates - no effect", + opts: WeigherFailoverHostOpts{Enabled: true}, + reservations: []client.Object{failoverHostReservation("h1", "source1")}, + request: weigherRequest(api.EvacuateIntent, "source1", "h2", "h3"), + wantTarget: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + step := &WeigherFailoverHost{} + step.Options = tt.opts + step.Client = fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.reservations...).Build() + + result, err := step.Run(logr.Discard(), tt.request) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + for host, weight := range result.Activations { + if host == tt.wantTarget { + if weight != failoverTargetWeight { + t.Errorf("expected target %s to get weight %v, got %v", host, failoverTargetWeight, weight) + } + } else if weight != 0 { + t.Errorf("expected host %s to have no effect (0), got %v", host, weight) + } + } + if tt.wantTarget != "" { + if _, ok := result.Activations[tt.wantTarget]; !ok { + t.Errorf("expected target %s to be present in activations", tt.wantTarget) + } + } + }) + } +} + +func TestWeigherFailoverHost_Index(t *testing.T) { + factory, ok := Index["weigher_failover_host"] + if !ok { + t.Fatal("expected weigher_failover_host to be registered in Index") + } + if _, ok := factory().(*WeigherFailoverHost); !ok { + t.Fatal("expected factory to return a *WeigherFailoverHost") + } +} + +func TestWeigherFailoverHost_OptsValidate(t *testing.T) { + if err := (WeigherFailoverHostOpts{}).Validate(); err != nil { + t.Errorf("expected nil error, got %v", err) + } + if err := (WeigherFailoverHostOpts{Enabled: true}).Validate(); err != nil { + t.Errorf("expected nil error, got %v", err) + } +} diff --git a/internal/scheduling/reservations/failoverhost/generation_test.go b/internal/scheduling/reservations/failoverhost/generation_test.go index 1d6191040..ae8fc8cdd 100644 --- a/internal/scheduling/reservations/failoverhost/generation_test.go +++ b/internal/scheduling/reservations/failoverhost/generation_test.go @@ -5,11 +5,16 @@ package failoverhost import ( "context" + "slices" "testing" + api "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" + sched "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" ) // fakeSeeder returns a fixed set of seeds, so generator tests drive the pipeline @@ -187,3 +192,295 @@ func TestGenerateAbortsOnMissingAvailabilityZone(t *testing.T) { t.Fatalf("expected Generate to abort (ok=false) on a hypervisor with no AZ") } } + +// ============================================================================ +// Whole-host guarantee scenarios +// +// These exercise the whole failover-host generation pipeline (seeder -> +// path finder -> clique builder -> planner) end to end against a *stubbed* +// scheduler whose per-(flavor, availability zone) accept-sets stand in for what +// the real Nova pipeline would answer. The accept-sets encode the compatibility +// axes the scenarios care about (traits, availability zone); that the real +// filters actually produce those answers is covered by each filter's own unit +// tests (filter_correct_az, filter_has_requested_traits, filter_has_enough_capacity). +// +// The invariant under test is the ADR 0011 whole-host guarantee: a reserved +// failover target must be able to absorb the entire workload of every host it is +// recorded as rescuing — i.e. the target must accept every flavor its source +// hosts accept. +// ============================================================================ + +const traitX = "CUSTOM_TRAIT_X" + +func TestWholeHostGuarantee(t *testing.T) { + tests := []struct { + name string + hypervisors []*hv1.Hypervisor + flavors []compute.FlavorInGroup + // acceptsByTraitAZ[trait][az] is the set of hosts the stub scheduler + // returns for a seed of the flavor with that required trait ("" for the + // no-trait flavor) in that availability zone — the compatibility the real + // pipeline would compute. Capacity fit is NOT modeled here; the path + // finder applies the real whole-host size check on top, so an undersized + // host can still be dropped despite accepting. + acceptsByTraitAZ map[string]map[string][]string + ratio float64 + minimum int + // wantReservations, when non-zero, asserts the exact reservation count. + wantReservations int + expectAny bool + forbiddenTargets []string + expectSiblings bool + }{ + { + // A single shared flavor forges a rescue link between every host, but a + // CUSTOM_TRAIT_X VM could never restart on a basic host (the scheduler + // rejects traitx there). The two special hosts form a valid trait-X + // partition and the two basics a general one; the same-flavor-set clique + // check must not merge them, or a basic host gets reserved as the + // specials' target and the whole-host guarantee breaks. + name: "trait-incompatible host is not reserved as a failover target", + hypervisors: []*hv1.Hypervisor{ + hvTraits("basic-1"), hvTraits("basic-2"), + hvTraits("special-1", traitX), hvTraits("special-2", traitX), + }, + flavors: []compute.FlavorInGroup{traitFlavor("general", ""), traitFlavor("traitx", traitX)}, + acceptsByTraitAZ: map[string]map[string][]string{ + "": {"az1": {"basic-1", "basic-2", "special-1", "special-2"}}, + traitX: {"az1": {"special-1", "special-2"}}, + }, + ratio: 0.1, minimum: 1, expectAny: true, + }, + { + // Three identical hosts accept the one flavor, so whichever becomes the + // target trivially satisfies the invariant. Guards against the check + // above being vacuously true. + name: "homogeneous partition reserves a compatible target", + hypervisors: []*hv1.Hypervisor{hvTraits("hv-1"), hvTraits("hv-2"), hvTraits("hv-3")}, + flavors: []compute.FlavorInGroup{traitFlavor("general", "")}, + acceptsByTraitAZ: map[string]map[string][]string{ + "": {"az1": {"hv-1", "hv-2", "hv-3"}}, + }, + expectAny: true, + }, + { + // Two large hosts and one small host all accept the flavor, but an + // evacuation moves a whole host's workload at once, so a large host's + // VMs could never fit on the small one. The path finder's whole-host fit + // check must drop every large->small edge, leaving "small" only ever a + // source, never a target — so it must never be reserved. + name: "undersized host is never reserved as a failover target", + hypervisors: []*hv1.Hypervisor{ + buildHV("large-1", 128, 64), buildHV("large-2", 128, 64), buildHV("small-1", 16, 8), + }, + flavors: []compute.FlavorInGroup{traitFlavor("general", "")}, + acceptsByTraitAZ: map[string]map[string][]string{ + "": {"az1": {"large-1", "large-2", "small-1"}}, + }, + expectAny: true, forbiddenTargets: []string{"small-1"}, + }, + { + // Two hosts per zone all accept the flavor, but the AZ filter only admits + // hosts of the seed's own zone, so each zone forms its own partition and + // the fleet yields exactly two reservations — a wrong cross-AZ merge would + // cap at one three-host partition's worth instead. + name: "hosts in different AZs are not merged into one partition", + hypervisors: []*hv1.Hypervisor{ + hvInZone("az1-1", "az1"), hvInZone("az1-2", "az1"), + hvInZone("az2-1", "az2"), hvInZone("az2-2", "az2"), + }, + flavors: []compute.FlavorInGroup{traitFlavor("general", "")}, + acceptsByTraitAZ: map[string]map[string][]string{ + "": { + "az1": {"az1-1", "az1-2"}, + "az2": {"az2-1", "az2-2"}, + }, + }, + expectAny: true, wantReservations: 2, + }, + { + // A ratio of 1.0 would reserve the whole partition, but the + // never-reserve-all rule caps a three-host partition at two targets. Each + // reservation must then list the other reserved host among its sources, + // the post-failover shortcut a multi-target partition relies on. + name: "multi-target partition records sibling reserved hosts as sources", + hypervisors: []*hv1.Hypervisor{hvTraits("hv-1"), hvTraits("hv-2"), hvTraits("hv-3")}, + flavors: []compute.FlavorInGroup{traitFlavor("general", "")}, + acceptsByTraitAZ: map[string]map[string][]string{ + "": {"az1": {"hv-1", "hv-2", "hv-3"}}, + }, + ratio: 1.0, minimum: 1, expectAny: true, wantReservations: 2, expectSiblings: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + objs := make([]client.Object, 0, len(tt.hypervisors)+1) + hvsByName := make(map[string]hv1.Hypervisor, len(tt.hypervisors)) + for _, hv := range tt.hypervisors { + objs = append(objs, hv) + hvsByName[hv.Name] = *hv + } + objs = append(objs, flavorGroupsKnowledge(t, true, tt.flavors...)) + + finder := azAwareStubScheduler(t, tt.acceptsByTraitAZ) + + ratio, minimum := tt.ratio, tt.minimum + if ratio == 0 { + ratio = 1.0 + } + if minimum == 0 { + minimum = 1 + } + g := &pipelineGenerator{ + Client: newTestClient(t, objs...), + seeder: &seeder{k8s: newTestClient(t, objs...)}, + pathFinder: finder, + partitionBuilder: cliquePartitionBuilder{}, + reservationPlanner: ratioReservationPlanner{ratio: ratio, minimum: minimum}, + } + desired, _, ok := g.Generate(context.Background()) + if !ok { + t.Fatal("Generate returned ok=false") + } + if tt.expectAny && len(desired) == 0 { + t.Fatal("expected at least one failover host reservation, got none") + } + if tt.wantReservations != 0 && len(desired) != tt.wantReservations { + t.Fatalf("expected %d reservations, got %d", tt.wantReservations, len(desired)) + } + for _, res := range desired { + if slices.Contains(tt.forbiddenTargets, res.Spec.TargetHost) { + t.Errorf("host %q was reserved as a failover target but must never be", res.Spec.TargetHost) + } + } + verifyWholeHostEvacuable(t, desired, tt.acceptsByTraitAZ) + if tt.expectSiblings { + verifySiblingSources(t, desired) + } + }) + } +} + +// verifyWholeHostEvacuable asserts the ADR 0011 invariant: for every reservation, +// the target host accepts every flavor each of its recorded source hosts accepts. +func verifyWholeHostEvacuable(t *testing.T, desired map[corev1.ObjectReference]v1alpha1.Reservation, acceptsByTraitAZ map[string]map[string][]string) { + t.Helper() + // Invert the accept-sets into per-host flavor sets, across all AZs. + acceptsByHost := map[string]map[string]bool{} + for flavor, byAZ := range acceptsByTraitAZ { + for _, hosts := range byAZ { + for _, h := range hosts { + if acceptsByHost[h] == nil { + acceptsByHost[h] = map[string]bool{} + } + acceptsByHost[h][flavor] = true + } + } + } + for _, res := range desired { + target := res.Spec.TargetHost + if res.Spec.FailoverHostReservation == nil { + t.Fatalf("reservation %s has no FailoverHostReservation spec", res.Name) + } + for _, source := range res.Spec.FailoverHostReservation.SourceHosts { + for flavor := range acceptsByHost[source] { + if !acceptsByHost[target][flavor] { + t.Errorf("reservation %s: target %q cannot run flavor %q that source %q accepts — %q's VMs could not evacuate onto it", + res.Name, target, flavor, source, source) + } + } + } + } +} + +// verifySiblingSources asserts that in a partition reserving more than one +// failover host, every reservation lists each other reserved host among its +// source hosts (the post-failover shortcut the planner relies on). +func verifySiblingSources(t *testing.T, desired map[corev1.ObjectReference]v1alpha1.Reservation) { + t.Helper() + targets := make([]string, 0, len(desired)) + for _, res := range desired { + targets = append(targets, res.Spec.TargetHost) + } + for _, res := range desired { + for _, sibling := range targets { + if sibling == res.Spec.TargetHost { + continue + } + if !slices.Contains(res.Spec.FailoverHostReservation.SourceHosts, sibling) { + t.Errorf("reservation for target %q does not list sibling reserved host %q as a source", + res.Spec.TargetHost, sibling) + } + } + } +} + +// azAwareStubScheduler returns a path finder wired to an in-process scheduler +// that answers each probe with the accept-set for the request's flavor *and* +// availability zone, so a scenario can model the AZ filter (a seed in az1 only +// ever accepts az1 hosts). It performs the same request-shape assertions as the +// simpler stubScheduler in pathfinding_test.go. +// +// Flavors are identified by their single required trait (the typed placement +// request carries no flavor name); the empty string keys a flavor with no +// required trait. See traitFlavor, which sets that trait. +func azAwareStubScheduler(t *testing.T, acceptsByTraitAZ map[string]map[string][]string) *evacuationPathFinder { + t.Helper() + provider := &stubPipelineProvider{pipeline: &azAwareStubPipeline{t: t, acceptsByTraitAZ: acceptsByTraitAZ}} + scheduler := sched.NewPlacement(provider, passthroughGatherer{}) + return &evacuationPathFinder{scheduler: scheduler} +} + +// azAwareStubPipeline is a fake pipeline that, like stubPipeline, validates the +// probe's request shape, but keys its candidate answers on both the flavor's +// required trait (empty when none) and the availability zone. +type azAwareStubPipeline struct { + t *testing.T + acceptsByTraitAZ map[string]map[string][]string +} + +func (s *azAwareStubPipeline) Run(req sched.PlacementRequest) (sched.PlacementResult, error) { + // A test flavor carries at most one required trait (see traitFlavor); use it + // to identify the flavor, falling back to "" for the no-trait flavor. + trait := "" + if len(req.RequiredTraits) > 0 { + trait = req.RequiredTraits[0] + } + if err := req.Options.Validate(); err != nil { + s.t.Errorf("invalid scheduling options: %v", err) + } + if !req.Options.AssumeEmptyHosts { + s.t.Errorf("request for trait %q must set AssumeEmptyHosts", trait) + } + if req.Intent != api.EvacuateIntent { + s.t.Errorf("request for trait %q must use evacuate intent, got %v", trait, req.Intent) + } + return sched.PlacementResult{OrderedHosts: s.acceptsByTraitAZ[trait][req.AvailabilityZone]}, nil +} + +// hvTraits builds a fixed-size (64Gi/32cpu) hypervisor in az1 with the given +// traits, so a scenario isolates trait (not capacity) as the compatibility axis. +func hvTraits(name string, traits ...string) *hv1.Hypervisor { + h := buildHV(name, 64, 32) + h.Status.Traits = traits + return h +} + +// hvInZone builds a fixed-size hypervisor (matching hvTraits) in the given +// availability zone, so a scenario can exercise AZ partitioning. +func hvInZone(name, az string) *hv1.Hypervisor { + h := buildHV(name, 64, 32) + h.Labels[corev1.LabelTopologyZone] = az + return h +} + +// traitFlavor builds a generic flavor requiring the given trait (or none). Its +// hypervisor type is a supported value so the seeder keeps it. +func traitFlavor(name, requiredTrait string) compute.FlavorInGroup { + extra := map[string]string{"capabilities:hypervisor_type": "ch"} + if requiredTrait != "" { + extra["trait:"+requiredTrait] = "required" + } + return compute.FlavorInGroup{Name: name, VCPUs: 1, MemoryMB: 1024, ExtraSpecs: extra} +} diff --git a/internal/scheduling/reservations/failoverhost/integration_test.go b/internal/scheduling/reservations/failoverhost/integration_test.go deleted file mode 100644 index c34dae82d..000000000 --- a/internal/scheduling/reservations/failoverhost/integration_test.go +++ /dev/null @@ -1,477 +0,0 @@ -// Copyright SAP SE -// SPDX-License-Identifier: Apache-2.0 - -package failoverhost - -import ( - "context" - "slices" - "sync" - "testing" - - "github.com/cobaltcore-dev/cortex/api/v1alpha1" - "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" - sched "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" - "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" - "github.com/cobaltcore-dev/cortex/internal/scheduling/nova" - "github.com/cobaltcore-dev/cortex/internal/scheduling/nova/plugins/filters" - "github.com/cobaltcore-dev/cortex/internal/scheduling/nova/plugins/weighers" - hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -// passthroughGatherer is a nova.CandidateGatherer that leaves the request's -// preselected hosts unchanged. The integration tests seed their own eligible -// hosts, so gathering is a no-op here. -type passthroughGatherer struct{} - -func (passthroughGatherer) MutateWithAllCandidates(context.Context, *sched.PlacementRequest) error { - return nil -} - -// ============================================================================ -// Integration Tests -// -// These exercise the whole failover-host generation pipeline (seeder -> -// path finder -> clique builder -> planner) against a *real* Nova -// filter/weigher pipeline served over HTTP, rather than a stubbed scheduler. -// The invariant under test is the ADR 0011 whole-host guarantee: a reserved -// failover target must be able to absorb the entire workload of every host it -// is recorded as rescuing. Since eligibility is static (ADR A5) that reduces -// to: the target must accept every protected flavor its source hosts accept. -// ============================================================================ - -// IntegrationTestCase defines one failover-host generation scenario. -type IntegrationTestCase struct { - Name string - Hypervisors []*hv1.Hypervisor - Flavors []compute.FlavorInGroup - - // Ratio/Minimum configure the planner. Ratio defaults to 1.0 and Minimum to - // 1 when left zero, so a scenario reserves as many targets as the partition - // allows unless it says otherwise. - Ratio float64 - Minimum int - - // ExpectReservations asserts at least one reservation is produced (most - // scenarios want this; a scenario proving nothing is reserved sets it false). - ExpectReservations bool - - // ForbiddenTargets names hosts that must never be reserved as a failover - // target, e.g. a host too small to absorb a larger peer's whole workload. - ForbiddenTargets []string - - // ExpectReservationCount, when non-zero, asserts the exact number of - // reservations produced. Used by scenarios that must reserve more than one - // target so the sibling-source wiring can be checked. - ExpectReservationCount int - - // ExpectSiblingSources asserts every reservation lists all the other - // reservations' target hosts among its source hosts — the post-failover - // shortcut a multi-target partition relies on. - ExpectSiblingSources bool -} - -func TestIntegration(t *testing.T) { - const traitX = "CUSTOM_TRAIT_X" - testCases := []IntegrationTestCase{ - // ===================================================================== - // Whole-host evacuability - // ===================================================================== - { - // The general flavor makes all four hosts mutually evacuable, so a - // naive clustering would lump the two plain "basic" hosts into the same - // partition as the two trait-X hosts and could reserve a basic host as - // their failover target. But a CUSTOM_TRAIT_X VM on a special host could - // never restart on a basic host: the real scheduler rejects the trait-X - // flavor there. The two special hosts on their own form a valid trait-X - // partition (and the two basic hosts a valid general one), so the - // coverage is genuinely achievable — the algorithm just must not merge - // the incompatible hosts together. A single shared flavor is enough to - // forge a rescue link, so this is the regression guard for the - // same-flavor-set check that keeps the clique step from merging hosts - // that do not accept exactly the same flavors. - Name: "trait-incompatible host is not reserved as a failover target", - Hypervisors: []*hv1.Hypervisor{ - hvWithTraits("basic-1"), - hvWithTraits("basic-2"), - hvWithTraits("special-1", traitX), - hvWithTraits("special-2", traitX), - }, - Flavors: []compute.FlavorInGroup{ - flavor("general", ""), - flavor("traitx", traitX), - }, - // Production defaults: floor(4 × 0.1) = 0, raised to the minimum of 1, - // so exactly one host is reserved. It is the cheapest/first by name, - // "basic-1", which cannot run traitx — so even a single reservation - // violates the whole-host guarantee. - Ratio: 0.1, - Minimum: 1, - ExpectReservations: true, - }, - // ===================================================================== - // Baseline: a homogeneous partition is safe - // ===================================================================== - { - // Three identical hosts accept the one flavor, so any of them can - // rescue any other: whichever becomes the target trivially satisfies - // the whole-host invariant. Guards against the check above being - // vacuously true. - Name: "homogeneous partition reserves a compatible target", - Hypervisors: []*hv1.Hypervisor{ - hvWithTraits("hv-1"), - hvWithTraits("hv-2"), - hvWithTraits("hv-3"), - }, - Flavors: []compute.FlavorInGroup{flavor("general", "")}, - ExpectReservations: true, - }, - // ===================================================================== - // Whole-host capacity fit - // ===================================================================== - { - // Two large hosts and one small host all accept the one general flavor, - // so under AssumeEmptyHosts the real capacity filter admits all three and - // they co-accept the seed. But an evacuation moves a whole host's - // workload at once, so a large host's VMs could never fit on the small - // one: the path finder's whole-host fit check must drop every - // large→small edge, leaving "small" only ever a source, never a - // mutual-edge target. The small host must therefore never be reserved — - // even though it happily accepts the flavor — or a large peer's workload - // would have nowhere to land. - Name: "undersized host is never reserved as a failover target", - Hypervisors: []*hv1.Hypervisor{ - buildHV("large-1", 128, 64), - buildHV("large-2", 128, 64), - buildHV("small-1", 16, 8), - }, - Flavors: []compute.FlavorInGroup{flavor("general", "")}, - ExpectReservations: true, - ForbiddenTargets: []string{"small-1"}, - }, - // ===================================================================== - // Availability-zone isolation - // ===================================================================== - { - // Two hosts in az1 and two in az2 all accept the one flavor, but the real - // AZ filter only ever admits hosts of the seed's own zone, so a seed in - // az1 co-accepts only the az1 hosts and vice versa. The two zones must - // therefore form two separate partitions: no reservation may record a - // cross-AZ source, because a VM in az1 could not be evacuated onto an az2 - // host. Each zone reserves its own target, so the fleet yields exactly - // two reservations — a wrong merge into one four-host partition would, at - // the default ratio of 1.0, cap at three instead. - Name: "hosts in different AZs are not merged into one partition", - Hypervisors: []*hv1.Hypervisor{ - hvPtrInAZ("az1-1", "az1"), - hvPtrInAZ("az1-2", "az1"), - hvPtrInAZ("az2-1", "az2"), - hvPtrInAZ("az2-2", "az2"), - }, - Flavors: []compute.FlavorInGroup{flavor("general", "")}, - ExpectReservations: true, - ExpectReservationCount: 2, - }, - // ===================================================================== - // Multi-target partition: siblings list each other as sources - // ===================================================================== - { - // A ratio of 1.0 would reserve the whole partition, but the never-reserve- - // all rule caps it at len(partition)-1, so a three-host partition reserves - // two targets. Each reservation must then list the *other* reserved host - // among its sources: once a reserved host absorbs an evacuation and - // returns to service it instantly needs a target of its own, which is why - // the planner keeps the sibling reserved hosts in each other's source - // lists. Whole-host evacuability still holds since the hosts are identical. - Name: "multi-target partition records sibling reserved hosts as sources", - Hypervisors: []*hv1.Hypervisor{ - hvWithTraits("hv-1"), - hvWithTraits("hv-2"), - hvWithTraits("hv-3"), - }, - Flavors: []compute.FlavorInGroup{flavor("general", "")}, - Ratio: 1.0, - Minimum: 1, - ExpectReservations: true, - ExpectReservationCount: 2, - ExpectSiblingSources: true, - }, - } - - for _, tc := range testCases { - t.Run(tc.Name, func(t *testing.T) { - env := newIntegrationTestEnv(t, tc.Hypervisors, tc.Flavors) - - ratio, minimum := tc.Ratio, tc.Minimum - if ratio == 0 { - ratio = 1.0 - } - if minimum == 0 { - minimum = 1 - } - - desired := env.Generate(t, ratio, minimum) - if tc.ExpectReservations && len(desired) == 0 { - t.Fatal("expected at least one failover host reservation, got none") - } - if tc.ExpectReservationCount != 0 && len(desired) != tc.ExpectReservationCount { - t.Fatalf("expected %d reservations, got %d", tc.ExpectReservationCount, len(desired)) - } - for _, res := range desired { - if slices.Contains(tc.ForbiddenTargets, res.Spec.TargetHost) { - t.Errorf("host %q was reserved as a failover target but must never be", res.Spec.TargetHost) - } - } - if tc.ExpectSiblingSources { - env.VerifySiblingSources(t, desired) - } - - env.VerifyWholeHostEvacuable(t, desired) - }) - } -} - -// ============================================================================ -// Test Environment -// ============================================================================ - -// integrationTestEnv wires a real Nova filter/weigher pipeline behind the shared -// in-process scheduler on top of a fake k8s client holding the fleet and the -// flavor-groups Knowledge CRD, so the generation pipeline can be driven end to -// end. -type integrationTestEnv struct { - k8sClient client.Client - scheduler *sched.Placement - flavors []compute.FlavorInGroup - hypervisors map[string]hv1.Hypervisor -} - -// newIntegrationTestEnv builds the fake cluster and the real scheduler pipeline. -func newIntegrationTestEnv(t *testing.T, hypervisors []*hv1.Hypervisor, flavors []compute.FlavorInGroup) *integrationTestEnv { - t.Helper() - - objects := make([]client.Object, 0, len(hypervisors)+1) - hvsByName := make(map[string]hv1.Hypervisor, len(hypervisors)) - for _, hv := range hypervisors { - objects = append(objects, hv) - hvsByName[hv.Name] = *hv - } - objects = append(objects, flavorGroupsKnowledge(t, true, flavors...)) - - k8sClient := newTestClient(t, objects...) - - controller := &nova.FilterWeigherPipelineController{ - BasePipelineController: lib.BasePipelineController[lib.FilterWeigherPipeline[sched.PlacementRequest]]{ - Client: k8sClient, - Pipelines: make(map[string]lib.FilterWeigherPipeline[sched.PlacementRequest]), - PipelineConfigs: make(map[string]v1alpha1.Pipeline), - }, - Monitor: sharedPipelineMonitor(), - } - // Mirror the production KVM pipeline for the dimensions that shape a failover - // partition: AZ, trait compatibility and whole-host capacity. - pipeline := v1alpha1.Pipeline{ - ObjectMeta: metav1.ObjectMeta{Name: evacProbePipeline}, - Spec: v1alpha1.PipelineSpec{ - Type: v1alpha1.PipelineTypeFilterWeigher, - Filters: []v1alpha1.FilterSpec{ - {Name: "filter_correct_az"}, - {Name: "filter_has_requested_traits"}, - {Name: "filter_has_enough_capacity"}, - }, - }, - } - result := lib.InitNewFilterWeigherPipeline( - context.Background(), k8sClient, pipeline.Name, - filters.Index, pipeline.Spec.Filters, - weighers.Index, pipeline.Spec.Weighers, - controller.Monitor, - ) - if len(result.FilterErrors) > 0 || len(result.WeigherErrors) > 0 { - t.Fatalf("init pipeline: filters=%v weighers=%v", result.FilterErrors, result.WeigherErrors) - } - controller.Pipelines[pipeline.Name] = result.Pipeline - controller.PipelineConfigs[pipeline.Name] = pipeline - - // KVM requests trigger candidate gathering; the test seeds its own eligible - // hosts, so use a passthrough gatherer that preserves them. - scheduler := sched.NewPlacement(controller, passthroughGatherer{}) - - return &integrationTestEnv{ - k8sClient: k8sClient, - scheduler: scheduler, - flavors: flavors, - hypervisors: hvsByName, - } -} - -// ============================================================================ -// Environment Helper Methods -// ============================================================================ - -// Generate runs the real generation pipeline (real seeder, HTTP path finder, -// clique builder and planner) and returns the desired reservations. -func (env *integrationTestEnv) Generate(t *testing.T, ratio float64, minimum int) map[corev1.ObjectReference]v1alpha1.Reservation { - t.Helper() - g := &pipelineGenerator{ - Client: env.k8sClient, - seeder: &seeder{k8s: env.k8sClient}, - pathFinder: &evacuationPathFinder{scheduler: env.scheduler}, - partitionBuilder: cliquePartitionBuilder{}, - reservationPlanner: ratioReservationPlanner{ratio: ratio, minimum: minimum}, - } - desired, _, ok := g.Generate(context.Background()) - if !ok { - t.Fatal("Generate returned ok=false") - } - return desired -} - -// AcceptsByHost returns, per host, the set of flavors the real scheduler accepts -// on it — the ground truth the reservations must respect. It reuses the path -// finder's own evacuation-intent empty-host probe so the answer matches exactly -// what the generation pipeline sees. It probes every availability zone present -// in the fleet (the AZ filter only ever admits hosts of the seed's own zone), so -// the ground truth covers multi-AZ fleets too. -func (env *integrationTestEnv) AcceptsByHost(t *testing.T) map[string]map[string]bool { - t.Helper() - zones := map[string]bool{} - for _, hv := range env.hypervisors { - zones[hv.Labels[corev1.LabelTopologyZone]] = true - } - finder := &evacuationPathFinder{scheduler: env.scheduler} - accepts := map[string]map[string]bool{} - for _, flavor := range env.flavors { - for zone := range zones { - accepting, err := finder.fetchSeedCandidates(context.Background(), Seed{ - Flavor: Flavor{ - Name: flavor.Name, - MemoryMB: flavor.MemoryMB, - VCPUs: flavor.VCPUs, - ExtraSpecs: flavor.ExtraSpecs, - }, - AvailabilityZone: zone, - }, env.hypervisors) - if err != nil { - t.Fatalf("probe accepting for %q in %q: %v", flavor.Name, zone, err) - } - for _, hv := range accepting { - if accepts[hv.Name] == nil { - accepts[hv.Name] = map[string]bool{} - } - accepts[hv.Name][flavor.Name] = true - } - } - } - return accepts -} - -// ============================================================================ -// Verification Helpers -// ============================================================================ - -// VerifyWholeHostEvacuable asserts the ADR 0011 invariant: for every reservation, -// the target host must accept every protected flavor each of its recorded source -// hosts accepts. Otherwise that source's VMs of the uncovered flavor would have -// nowhere to restart on the reserved target. -func (env *integrationTestEnv) VerifyWholeHostEvacuable(t *testing.T, desired map[corev1.ObjectReference]v1alpha1.Reservation) { - t.Helper() - accepts := env.AcceptsByHost(t) - for _, res := range desired { - target := res.Spec.TargetHost - if res.Spec.FailoverHostReservation == nil { - t.Fatalf("reservation %s has no FailoverHostReservation spec", res.Name) - } - for _, source := range res.Spec.FailoverHostReservation.SourceHosts { - for flavor := range accepts[source] { - if !accepts[target][flavor] { - t.Errorf("reservation %s: target %q is recorded as a failover host for source %q, "+ - "but cannot run flavor %q that %q accepts — the source's %q VMs could not be evacuated onto it", - res.Name, target, source, flavor, source, flavor) - } - } - } - } -} - -// VerifySiblingSources asserts that in a partition reserving more than one -// failover host, every reservation lists each *other* reserved host among its -// source hosts. The planner keeps the reserved hosts in each other's source -// lists on purpose: once a reserved host absorbs an evacuation and returns to -// service it instantly needs a failover target of its own, and leaving it a -// recorded source means that target is already in place. -func (env *integrationTestEnv) VerifySiblingSources(t *testing.T, desired map[corev1.ObjectReference]v1alpha1.Reservation) { - t.Helper() - targets := make([]string, 0, len(desired)) - for _, res := range desired { - targets = append(targets, res.Spec.TargetHost) - } - for _, res := range desired { - if res.Spec.FailoverHostReservation == nil { - t.Fatalf("reservation %s has no FailoverHostReservation spec", res.Name) - } - for _, sibling := range targets { - if sibling == res.Spec.TargetHost { - continue - } - if !slices.Contains(res.Spec.FailoverHostReservation.SourceHosts, sibling) { - t.Errorf("reservation for target %q does not list sibling reserved host %q as a source", - res.Spec.TargetHost, sibling) - } - } - } -} - -// evacProbePipeline is the name of the real Nova filter/weigher pipeline the -// probe exercises. -const evacProbePipeline = "kvm-general-purpose-load-balancing" - -// ============================================================================ -// Object Creation Functions -// ============================================================================ - -// hvWithTraits builds a fixed-size hypervisor in az1 with the given traits. The -// size is uniform across the fleet so every host is a valid gang-fit target for -// every other, isolating the trait (not capacity) as the compatibility axis. -func hvWithTraits(name string, traits ...string) *hv1.Hypervisor { - h := buildHV(name, 64, 32) - h.Status.Traits = traits - return h -} - -// hvPtrInAZ builds a fixed-size hypervisor (matching hvWithTraits) in the given -// availability zone, so a scenario can exercise the AZ filter rather than only -// az1 (buildHV, which it reuses, hardcodes az1). It is distinct from seeding -// test's hvInAZ, which returns a value and sets no capacity. -func hvPtrInAZ(name, az string) *hv1.Hypervisor { - h := buildHV(name, 64, 32) - h.Labels[corev1.LabelTopologyZone] = az - return h -} - -// flavor builds a generic flavor that requires the given trait (or none). Its -// hypervisor type is set to a supported value ("ch") only so the seeder keeps -// it; the specific type is irrelevant to what this test exercises. -func flavor(name, requiredTrait string) compute.FlavorInGroup { - extra := map[string]string{"capabilities:hypervisor_type": "ch"} - if requiredTrait != "" { - extra["trait:"+requiredTrait] = "required" - } - return compute.FlavorInGroup{Name: name, VCPUs: 1, MemoryMB: 1024, ExtraSpecs: extra} -} - -// sharedPipelineMonitor guards the one-time Prometheus collector registration -// the pipeline monitor performs, so multiple environments can be built in one -// test run without a duplicate-registration panic. -var ( - pipelineMonitorOnce sync.Once - pipelineMonitor lib.FilterWeigherPipelineMonitor -) - -func sharedPipelineMonitor() lib.FilterWeigherPipelineMonitor { - pipelineMonitorOnce.Do(func() { pipelineMonitor = lib.NewPipelineMonitor() }) - return pipelineMonitor -} diff --git a/internal/scheduling/reservations/failoverhost/matching.go b/internal/scheduling/reservations/failoverhost/matching.go new file mode 100644 index 000000000..5e0c689a9 --- /dev/null +++ b/internal/scheduling/reservations/failoverhost/matching.go @@ -0,0 +1,168 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package failoverhost + +import ( + "context" + "slices" + "sort" + + api "github.com/cobaltcore-dev/cortex/api/external/nova" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// Matcher confirms an evacuation's gang target after the scheduling pipeline has +// run and pins the source host to that target, implementing gang-evacuation. +// +// The candidate shaping — removing the other reserved hosts and lifting the +// selected failover host to the front — is done inside the pipeline by the +// filter_failover_host filter and the weigher_failover_host weigher. The Matcher +// does not touch the candidate list. Its sole job is the stateful write: once the +// pipeline has settled on a target host, it marks the matching reservation InUse +// so that subsequent evacuations from the same source host deterministically land +// on the same target. +// +// It runs from inside the pipeline controller's exclusive lock, on write runs +// only, so the InUse write is serialized against — and visible to — subsequent +// evacuation requests from the same source host. The write is committed only once +// (on the first evacuation from a given source); later requests see the +// already-InUse reservation (the filter/weigher keep selecting it) and so land on +// the same target. +// +// The Matcher only ever advances a reservation to InUse. Moving it back out +// (to Active/Used once the evacuation completes) is owned by the failover host +// lifecycle reconciler, based on the real workload present on the host. +type Matcher struct { + client.Client + // Monitor records the evacuation-without-reservation metric. May be nil. + Monitor *MatcherMonitor +} + +// ConfirmAndMarkInUse inspects the pipeline's ordered candidates for an evacuation +// request and, if the top candidate is the failover host reservation selected for +// the request's source host, marks that reservation InUse. +// +// It is a no-op for non-evacuation requests and for evacuations without a source +// host. When no failover host reservation matches the source host — or the +// selected one did not end up as the top candidate (it was filtered out by other +// constraints, so the evacuation falls back to an ordinary host) — it records the +// evacuation-without-reservation metric and writes nothing. +func (m *Matcher) ConfirmAndMarkInUse(ctx context.Context, request api.ExternalSchedulerRequest, orderedHosts []string) error { + log := logf.FromContext(ctx) + + intent, err := request.GetIntent() + if err != nil || intent != api.EvacuateIntent { + return nil //nolint:nilerr // non-evacuation requests are not matched + } + sourceHost, err := request.Spec.Data.GetSchedulerHintStr("source_host") + if err != nil || sourceHost == "" { + log.V(1).Info("no source_host hint on evacuation, nothing to confirm") + return nil //nolint:nilerr // without a source host there is nothing to match + } + + var reservations v1alpha1.ReservationList + if err := m.List(ctx, &reservations); err != nil { + return err + } + // Re-derive the selected reservation with the same deterministic rule the + // filter and weigher used over the same snapshot, so we confirm the one they + // shaped the candidates around. + selected := SelectFailoverHost(reservations.Items, sourceHost) + if selected == nil { + // Either no reservation can rescue this source, or every matching one is + // already absorbing another source host's gang. Surface it either way. + log.V(1).Info("no selectable failover host reservation for source host", "sourceHost", sourceHost) + m.Monitor.RecordEvacuationWithoutReservation(request.Spec.Data.AvailabilityZone) + return nil + } + + // Only pin when the pipeline actually settled on the selected target. If it is + // not the top candidate (filtered out by other constraints, so the evacuation + // falls back to an ordinary host), we must not leave a dangling InUse. + if len(orderedHosts) == 0 || orderedHosts[0] != selected.Spec.TargetHost { + log.V(1).Info("selected failover host is not the top candidate, not pinning", + "sourceHost", sourceHost, "targetHost", selected.Spec.TargetHost) + m.Monitor.RecordEvacuationWithoutReservation(request.Spec.Data.AvailabilityZone) + return nil + } + // Already in use by this source (gang-evacuation reuse): nothing to write. + if meta.IsStatusConditionTrue(selected.Status.Conditions, v1alpha1.ConditionTypeFailoverHostInUse) { + log.V(1).Info("failover host reservation already in use for this source, reusing", + "sourceHost", sourceHost, "reservation", selected.Name, "targetHost", selected.Spec.TargetHost) + return nil + } + if err := m.markInUse(ctx, selected, sourceHost); err != nil { + return err + } + log.V(1).Info("pinned failover host reservation for evacuation", + "sourceHost", sourceHost, "reservation", selected.Name, "targetHost", selected.Spec.TargetHost) + return nil +} + +// markInUse sets the InUse condition and records the source host on the +// reservation status. +func (m *Matcher) markInUse(ctx context.Context, res *v1alpha1.Reservation, sourceHost string) error { + old := res.DeepCopy() + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ConditionTypeFailoverHostInUse, + Status: metav1.ConditionTrue, + Reason: "EvacuationStarted", + Message: "a gang evacuation from " + sourceHost + " is underway", + }) + if res.Status.FailoverHostReservation == nil { + res.Status.FailoverHostReservation = &v1alpha1.FailoverHostReservationStatus{} + } + res.Status.FailoverHostReservation.SourceHost = sourceHost + return m.Status().Patch(ctx, res, client.MergeFrom(old)) +} + +// SelectFailoverHost picks the one failover host reservation a source host should +// gang-evacuate onto, deterministically. It is a pure function of the reservation +// snapshot, so the filter_failover_host filter, the weigher_failover_host weigher +// and this Matcher — which all run over the same snapshot within one pipeline +// execution — select the same target without coordinating. +// +// Among the reservations whose partition includes sourceHost it prefers the one +// already in use by this very source (the gang-evacuation path — the whole gang +// lands on the same target), never picks one in use by a different source (which +// would steal an active landing zone), and otherwise takes the first Active one +// by target host name. A not-yet-in-use reservation is only eligible once it is +// Active (its host drained and empty); a still-draining, not-yet-Active one is +// never selected (though the filter still blocks it from ordinary placement). +// Returns nil when nothing is selectable. +func SelectFailoverHost(reservations []v1alpha1.Reservation, sourceHost string) *v1alpha1.Reservation { + var matching []v1alpha1.Reservation + for _, res := range reservations { + if res.Spec.Type != v1alpha1.ReservationTypeFailoverHost || res.Spec.FailoverHostReservation == nil { + continue + } + if slices.Contains(res.Spec.FailoverHostReservation.SourceHosts, sourceHost) { + matching = append(matching, res) + } + } + sort.Slice(matching, func(i, j int) bool { + return matching[i].Spec.TargetHost < matching[j].Spec.TargetHost + }) + var firstActiveFree *v1alpha1.Reservation + for i := range matching { + res := &matching[i] + if !meta.IsStatusConditionTrue(res.Status.Conditions, v1alpha1.ConditionTypeFailoverHostInUse) { + if firstActiveFree == nil && meta.IsStatusConditionTrue(res.Status.Conditions, v1alpha1.ConditionTypeFailoverHostActive) { + firstActiveFree = res + } + continue + } + // In use: reuse it only if it is in use by this very source host. + if res.Status.FailoverHostReservation != nil && + res.Status.FailoverHostReservation.SourceHost == sourceHost { + return res + } + } + return firstActiveFree +} diff --git a/internal/scheduling/reservations/failoverhost/matching_monitor.go b/internal/scheduling/reservations/failoverhost/matching_monitor.go new file mode 100644 index 000000000..5ec35a99e --- /dev/null +++ b/internal/scheduling/reservations/failoverhost/matching_monitor.go @@ -0,0 +1,48 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package failoverhost + +import ( + "github.com/prometheus/client_golang/prometheus" +) + +// MatcherMonitor is the small Prometheus collector owned by the scheduling +// pipeline's failover host Matcher. It is kept separate from the failover host +// controller's Monitor so both can be registered independently: the pipeline +// controller and the failover-host-reservations-controller can be enabled at +// once without their collectors colliding on shared metric names. +type MatcherMonitor struct { + evacuationsWithoutReservation *prometheus.CounterVec +} + +// NewMatcherMonitor creates a new matcher monitor with Prometheus metrics. +func NewMatcherMonitor() *MatcherMonitor { + return &MatcherMonitor{ + evacuationsWithoutReservation: prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "cortex_failover_host_evacuations_without_reservation_total", + Help: "Total number of evacuation scheduling requests for which no failover host reservation matched the source host, by availability zone", + }, []string{"az"}), + } +} + +// RecordEvacuationWithoutReservation records an evacuation scheduling request +// whose source host matched no failover host reservation, so the request fell +// back to the ordinary candidate set. az is the request's availability zone +// (may be empty if unknown). +func (m *MatcherMonitor) RecordEvacuationWithoutReservation(az string) { + if m == nil { + return + } + m.evacuationsWithoutReservation.WithLabelValues(az).Inc() +} + +// Describe implements prometheus.Collector. +func (m *MatcherMonitor) Describe(ch chan<- *prometheus.Desc) { + m.evacuationsWithoutReservation.Describe(ch) +} + +// Collect implements prometheus.Collector. +func (m *MatcherMonitor) Collect(ch chan<- prometheus.Metric) { + m.evacuationsWithoutReservation.Collect(ch) +} diff --git a/internal/scheduling/reservations/failoverhost/matching_test.go b/internal/scheduling/reservations/failoverhost/matching_test.go new file mode 100644 index 000000000..98333b02d --- /dev/null +++ b/internal/scheduling/reservations/failoverhost/matching_test.go @@ -0,0 +1,372 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package failoverhost + +import ( + "context" + "testing" + + api "github.com/cobaltcore-dev/cortex/api/external/nova" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// matcherReservation builds an Active failover host reservation targeting +// targetHost and able to rescue the given source hosts. Active is the state in +// which a reservation can be selected as an evacuation target. +func matcherReservation(targetHost string, sourceHosts ...string) *v1alpha1.Reservation { + res := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "failover-host-" + targetHost}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeFailoverHost, + TargetHost: targetHost, + FailoverHostReservation: &v1alpha1.FailoverHostReservationSpec{ + SourceHosts: sourceHosts, + }, + }, + } + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ConditionTypeFailoverHostActive, + Status: metav1.ConditionTrue, + Reason: "HostDrained", + }) + return res +} + +// notActive drops a reservation out of the Active state (the same as a freshly +// created, still-draining reservation). +func notActive(res *v1alpha1.Reservation) *v1alpha1.Reservation { + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ConditionTypeFailoverHostActive, + Status: metav1.ConditionFalse, + Reason: "Draining", + }) + return res +} + +// inUse marks a reservation as in use by the given source host. +func inUse(res *v1alpha1.Reservation, sourceHost string) *v1alpha1.Reservation { + res.Status.FailoverHostReservation = &v1alpha1.FailoverHostReservationStatus{SourceHost: sourceHost} + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ConditionTypeFailoverHostInUse, + Status: metav1.ConditionTrue, + Reason: "EvacuationStarted", + }) + return res +} + +func evacuationRequest(sourceHost string) api.ExternalSchedulerRequest { + hints := map[string]any{"_nova_check_type": "evacuate"} + if sourceHost != "" { + hints["source_host"] = sourceHost + } + return api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{Data: api.NovaSpec{ + AvailabilityZone: "az1", + SchedulerHints: hints, + }}, + } +} + +func getReservation(t *testing.T, c client.Client, name string) v1alpha1.Reservation { + t.Helper() + var res v1alpha1.Reservation + if err := c.Get(context.Background(), types.NamespacedName{Name: name}, &res); err != nil { + t.Fatalf("failed to get reservation: %v", err) + } + return res +} + +func isInUseBy(res v1alpha1.Reservation, sourceHost string) bool { + if !meta.IsStatusConditionTrue(res.Status.Conditions, v1alpha1.ConditionTypeFailoverHostInUse) { + return false + } + return res.Status.FailoverHostReservation != nil && + res.Status.FailoverHostReservation.SourceHost == sourceHost +} + +func TestMatcher_ConfirmAndMarkInUse(t *testing.T) { + tests := []struct { + name string + reservations []*v1alpha1.Reservation + request api.ExternalSchedulerRequest + orderedHosts []string + // check runs after the call to assert reservation state in the client. + check func(t *testing.T, c client.Client) + }{ + { + name: "non-evacuation is a no-op", + reservations: []*v1alpha1.Reservation{matcherReservation("target1", "source1")}, + request: api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{Data: api.NovaSpec{ + SchedulerHints: map[string]any{"_nova_check_type": "live_migrate", "source_host": "source1"}, + }}, + }, + orderedHosts: []string{"target1", "host2"}, + check: func(t *testing.T, c client.Client) { + res := getReservation(t, c, "failover-host-target1") + if meta.IsStatusConditionTrue(res.Status.Conditions, v1alpha1.ConditionTypeFailoverHostInUse) { + t.Error("expected target1 to not be in use for a non-evacuation request") + } + }, + }, + { + name: "evacuation without source hint is a no-op", + reservations: []*v1alpha1.Reservation{matcherReservation("target1", "source1")}, + request: evacuationRequest(""), + orderedHosts: []string{"target1", "host2"}, + check: func(t *testing.T, c client.Client) { + res := getReservation(t, c, "failover-host-target1") + if meta.IsStatusConditionTrue(res.Status.Conditions, v1alpha1.ConditionTypeFailoverHostInUse) { + t.Error("expected target1 to not be in use without a source hint") + } + }, + }, + { + name: "pins the selected reservation when it is the top candidate", + reservations: []*v1alpha1.Reservation{matcherReservation("target1", "source1")}, + request: evacuationRequest("source1"), + orderedHosts: []string{"target1", "host1", "host2"}, + check: func(t *testing.T, c client.Client) { + res := getReservation(t, c, "failover-host-target1") + if !isInUseBy(res, "source1") { + t.Errorf("expected target1 to be in use by source1, got %+v", res.Status.FailoverHostReservation) + } + }, + }, + { + name: "does not pin when the selected target is not the top candidate", + reservations: []*v1alpha1.Reservation{matcherReservation("target1", "source1")}, + request: evacuationRequest("source1"), + // target1 lost to an ordinary host (e.g. filtered out by other + // constraints), so the evacuation falls back and must not pin. + orderedHosts: []string{"host1", "host2"}, + check: func(t *testing.T, c client.Client) { + res := getReservation(t, c, "failover-host-target1") + if meta.IsStatusConditionTrue(res.Status.Conditions, v1alpha1.ConditionTypeFailoverHostInUse) { + t.Error("expected target1 to NOT be pinned when it is not the top candidate") + } + }, + }, + { + name: "does not pin a not-yet-active reservation", + // target1 is the top candidate but is still draining (not Active), so + // it is not a selectable landing zone and must not be pinned. + reservations: []*v1alpha1.Reservation{notActive(matcherReservation("target1", "source1"))}, + request: evacuationRequest("source1"), + orderedHosts: []string{"target1", "host2"}, + check: func(t *testing.T, c client.Client) { + res := getReservation(t, c, "failover-host-target1") + if meta.IsStatusConditionTrue(res.Status.Conditions, v1alpha1.ConditionTypeFailoverHostInUse) { + t.Error("expected a not-yet-active target1 to NOT be pinned") + } + }, + }, + { + name: "reuses the in-use reservation for the same source, no new write", + reservations: []*v1alpha1.Reservation{ + inUse(matcherReservation("target1", "source1"), "source1"), + matcherReservation("target0", "source1"), + }, + request: evacuationRequest("source1"), + orderedHosts: []string{"target1", "host2"}, + check: func(t *testing.T, c client.Client) { + res := getReservation(t, c, "failover-host-target1") + if !isInUseBy(res, "source1") { + t.Errorf("expected target1 to stay in use by source1, got %+v", res.Status.FailoverHostReservation) + } + // target0 must remain untouched. + res0 := getReservation(t, c, "failover-host-target0") + if meta.IsStatusConditionTrue(res0.Status.Conditions, v1alpha1.ConditionTypeFailoverHostInUse) { + t.Error("expected target0 to remain free") + } + }, + }, + { + name: "does not steal a reservation in use by another source", + reservations: []*v1alpha1.Reservation{ + inUse(matcherReservation("target1", "source1", "source2"), "source1"), + matcherReservation("target2", "source1", "source2"), + }, + request: evacuationRequest("source2"), + orderedHosts: []string{"target2", "host3"}, + check: func(t *testing.T, c client.Client) { + res1 := getReservation(t, c, "failover-host-target1") + if !isInUseBy(res1, "source1") { + t.Errorf("expected target1 to stay pinned to source1, got %+v", res1.Status.FailoverHostReservation) + } + res2 := getReservation(t, c, "failover-host-target2") + if !isInUseBy(res2, "source2") { + t.Errorf("expected target2 to be pinned to source2, got %+v", res2.Status.FailoverHostReservation) + } + }, + }, + { + name: "all reservations in use by others is a no-op", + reservations: []*v1alpha1.Reservation{ + inUse(matcherReservation("target1", "source1", "source2"), "source1"), + }, + request: evacuationRequest("source2"), + orderedHosts: []string{"host3"}, + check: func(t *testing.T, c client.Client) { + res := getReservation(t, c, "failover-host-target1") + if res.Status.FailoverHostReservation.SourceHost != "source1" { + t.Errorf("expected target1 to stay pinned to source1, got %q", res.Status.FailoverHostReservation.SourceHost) + } + }, + }, + { + name: "no matching reservation is a no-op", + reservations: []*v1alpha1.Reservation{matcherReservation("target1", "otherSource")}, + request: evacuationRequest("source1"), + orderedHosts: []string{"host1", "host2"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + objs := make([]client.Object, len(tt.reservations)) + for i, r := range tt.reservations { + objs[i] = r + } + c := newTestClient(t, objs...) + m := &Matcher{Client: c, Monitor: NewMatcherMonitor()} + if err := m.ConfirmAndMarkInUse(context.Background(), tt.request, tt.orderedHosts); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tt.check != nil { + tt.check(t, c) + } + }) + } +} + +// nonFailoverReservation builds a reservation of a different type, to prove +// SelectFailoverHost ignores anything that is not a failover host reservation. +func nonFailoverReservation(targetHost string) v1alpha1.Reservation { + return v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "cr-" + targetHost}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + TargetHost: targetHost, + }, + } +} + +// TestSelectFailoverHost pins the deterministic selection rule that the filter, +// the weigher and the matcher all share. Each case lists the reservation +// snapshot and asserts which target host (if any) is selected for source1. +func TestSelectFailoverHost(t *testing.T) { + // nilSpec is a failover host reservation whose FailoverHostReservation spec is + // nil; it must be skipped rather than panic. + nilSpec := matcherReservation("nilspec", "source1") + nilSpec.Spec.FailoverHostReservation = nil + + tests := []struct { + name string + reservations []v1alpha1.Reservation + sourceHost string + want string // target host, or "" for no selection + }{ + { + name: "no reservations at all", + reservations: nil, + sourceHost: "source1", + want: "", + }, + { + name: "ignores non-failover-host reservations", + reservations: []v1alpha1.Reservation{nonFailoverReservation("t1")}, + sourceHost: "source1", + want: "", + }, + { + name: "ignores a failover host reservation with a nil spec", + reservations: []v1alpha1.Reservation{*nilSpec}, + sourceHost: "source1", + want: "", + }, + { + name: "ignores a reservation whose partition excludes the source", + reservations: []v1alpha1.Reservation{*matcherReservation("t1", "otherSource")}, + sourceHost: "source1", + want: "", + }, + { + name: "picks the single active match", + reservations: []v1alpha1.Reservation{*matcherReservation("t1", "source1")}, + sourceHost: "source1", + want: "t1", + }, + { + name: "picks the first active match by target host name", + reservations: []v1alpha1.Reservation{ + *matcherReservation("t2", "source1"), + *matcherReservation("t1", "source1"), + }, + sourceHost: "source1", + want: "t1", + }, + { + name: "a not-yet-active reservation is not selectable", + reservations: []v1alpha1.Reservation{*notActive(matcherReservation("t1", "source1"))}, + sourceHost: "source1", + want: "", + }, + { + name: "skips a not-yet-active one and picks the next active by name", + reservations: []v1alpha1.Reservation{ + *notActive(matcherReservation("t1", "source1")), + *matcherReservation("t2", "source1"), + }, + sourceHost: "source1", + want: "t2", + }, + { + name: "reuses the in-use reservation for this source over a name-earlier free one", + reservations: []v1alpha1.Reservation{ + *matcherReservation("t0", "source1"), + *inUse(matcherReservation("t1", "source1"), "source1"), + }, + sourceHost: "source1", + want: "t1", + }, + { + name: "never selects a reservation in use by another source", + reservations: []v1alpha1.Reservation{ + *inUse(matcherReservation("t1", "source1", "source2"), "source2"), + *matcherReservation("t2", "source1", "source2"), + }, + sourceHost: "source1", + want: "t2", + }, + { + name: "all matches in use by others yields no selection", + reservations: []v1alpha1.Reservation{ + *inUse(matcherReservation("t1", "source1", "source2"), "source2"), + }, + sourceHost: "source1", + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := SelectFailoverHost(tt.reservations, tt.sourceHost) + if tt.want == "" { + if got != nil { + t.Fatalf("expected no selection, got %q", got.Spec.TargetHost) + } + return + } + if got == nil { + t.Fatalf("expected %q, got no selection", tt.want) + } + if got.Spec.TargetHost != tt.want { + t.Fatalf("expected %q, got %q", tt.want, got.Spec.TargetHost) + } + }) + } +} diff --git a/internal/scheduling/reservations/failoverhost/pathfinding_test.go b/internal/scheduling/reservations/failoverhost/pathfinding_test.go index e552a0661..deb822eea 100644 --- a/internal/scheduling/reservations/failoverhost/pathfinding_test.go +++ b/internal/scheduling/reservations/failoverhost/pathfinding_test.go @@ -36,7 +36,16 @@ func stubScheduler(t *testing.T, candidatesByFlavor map[string][]string, errFlav // stubPipelineProvider serves the same stub pipeline for every pipeline name the // probe infers (always a KVM pipeline here). type stubPipelineProvider struct { - pipeline *stubPipeline + pipeline lib.FilterWeigherPipeline[compute.PlacementRequest] +} + +// passthroughGatherer is a compute.CandidateGatherer that leaves the request's +// preselected hosts unchanged. The tests seed their own eligible hosts, so +// gathering is a no-op here. +type passthroughGatherer struct{} + +func (passthroughGatherer) MutateWithAllCandidates(context.Context, *compute.PlacementRequest) error { + return nil } func (p *stubPipelineProvider) GetPipeline(string) (lib.FilterWeigherPipeline[compute.PlacementRequest], bool) {