diff --git a/cmd/main.go b/cmd/main.go index 510a92176..1565c15cd 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -579,27 +579,7 @@ func main() { if slices.Contains(mainConfig.EnabledControllers, "failover-reservations-controller") { setupLog.Info("enabling controller", "controller", "failover-reservations-controller") failoverConfig := conf.GetConfigOrDie[failover.FailoverConfig]() - - // Apply defaults for unset values - defaults := failover.DefaultConfig() - if failoverConfig.DatasourceName == "" { - failoverConfig.DatasourceName = defaults.DatasourceName - } - if failoverConfig.SchedulerURL == "" { - failoverConfig.SchedulerURL = defaults.SchedulerURL - } - if failoverConfig.ReconcileInterval == 0 { - failoverConfig.ReconcileInterval = defaults.ReconcileInterval - } - if failoverConfig.Creator == "" { - failoverConfig.Creator = defaults.Creator - } - if failoverConfig.FlavorFailoverRequirements == nil { - failoverConfig.FlavorFailoverRequirements = defaults.FlavorFailoverRequirements - } - if failoverConfig.RevalidationInterval == 0 { - failoverConfig.RevalidationInterval = defaults.RevalidationInterval - } + failoverConfig.ApplyDefaults() // DatasourceName is still required - check after applying defaults if failoverConfig.DatasourceName == "" { diff --git a/helm/bundles/cortex-nova/values.yaml b/helm/bundles/cortex-nova/values.yaml index 4683188c0..1b2084d40 100644 --- a/helm/bundles/cortex-nova/values.yaml +++ b/helm/bundles/cortex-nova/values.yaml @@ -175,22 +175,23 @@ cortex-scheduling-controllers: # Maps flavor name patterns (glob) to required failover count # Example: {"hana_*": 2, "m1.xlarge": 1} flavorFailoverRequirements: - "*": 2 + "*": 1 # How often to check for missing failover reservations (periodic bulk reconciliation) - # 35s = 35000000000 nanoseconds - reconcileInterval: 35000000000 - # Used when maxVMsToProcess limits processing, allows faster catch-up - # 100ms = 100000000 nanoseconds - shortReconcileInterval: 100000000 - # Tag for failover reservations (for identification and cleanup) - creator: cortex-failover-controller - # Limits VMs processed per cycle. Set to 0 to process all VMs. - maxVMsToProcess: 25 + reconcileInterval: 15m + # Used when maxVMsToProcess limits processing, allows faster catch-up and for the first reconcile + shortReconcileInterval: 1m + # Number of max VMs to process in one periodic reconciliation loop + maxVMsToProcess: 5 + # Minimum successful reservations to use short interval + minSuccessForShortInterval: 1 + # Maximum failures allowed to still use short interval + maxFailuresForShortInterval: 99 # If true, uses hypervisor CRD as source of truth for VM location instead of postgres trustHypervisorLocation: true # How often to re-validate acknowledged failover reservations - # 30m = 1800000000000 nanoseconds - revalidationInterval: 1800000000000 + revalidationInterval: 30m + # Prevents creating multiple new reservations on the same hypervisor per cycle + limitOneNewReservationPerHypervisor: false cortex-knowledge-controllers: <<: *cortex diff --git a/internal/scheduling/reservations/failover/config.go b/internal/scheduling/reservations/failover/config.go index f5e01ba09..79dc94480 100644 --- a/internal/scheduling/reservations/failover/config.go +++ b/internal/scheduling/reservations/failover/config.go @@ -3,7 +3,11 @@ package failover -import "time" +import ( + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // FailoverConfig defines the configuration for failover reservation management. type FailoverConfig struct { @@ -13,7 +17,8 @@ type FailoverConfig struct { FlavorFailoverRequirements map[string]int `json:"flavorFailoverRequirements"` // ReconcileInterval is how often to check for missing failover reservations. - ReconcileInterval time.Duration `json:"reconcileInterval"` + // Supports Go duration strings like "30s", "1m", "15m". + ReconcileInterval metav1.Duration `json:"reconcileInterval"` // Creator tag for failover reservations (for identification and cleanup). Creator string `json:"creator"` @@ -27,14 +32,23 @@ type FailoverConfig struct { SchedulerURL string `json:"schedulerURL"` // MaxVMsToProcess limits the number of VMs to process per reconciliation cycle. - // Set to 0 or negative to process all VMs (default behavior). + // Set to negative to process all VMs (default behavior). // Useful for debugging and testing with large VM counts. MaxVMsToProcess int `json:"maxVMsToProcess"` // ShortReconcileInterval is used when MaxVMsToProcess limits processing. // This allows faster catch-up when there are more VMs to process. // Set to 0 to use ReconcileInterval (default behavior). - ShortReconcileInterval time.Duration `json:"shortReconcileInterval"` + // Supports Go duration strings like "100ms", "1s", "1m". + ShortReconcileInterval metav1.Duration `json:"shortReconcileInterval"` + + // MinSuccessForShortInterval is the minimum number of successful reservations (created + reused) + // required to use ShortReconcileInterval. Default: 1. Use 0 to require no minimum. + MinSuccessForShortInterval *int `json:"minSuccessForShortInterval"` + + // MaxFailuresForShortInterval is the maximum number of failures allowed to still use + // ShortReconcileInterval. Default: 99. Use 0 to allow no failures. + MaxFailuresForShortInterval *int `json:"maxFailuresForShortInterval"` // TrustHypervisorLocation when true, uses the hypervisor CRD as the source of truth // for VM location instead of postgres (OSEXTSRVATTRHost). This is useful when there @@ -49,19 +63,80 @@ type FailoverConfig struct { // After a reservation is acknowledged, it will be re-validated after this interval // to ensure the reservation host is still valid for all allocated VMs. // Default: 30 minutes - RevalidationInterval time.Duration `json:"revalidationInterval"` + // Supports Go duration strings like "15m", "30m", "1h". + RevalidationInterval metav1.Duration `json:"revalidationInterval"` + + // LimitOneNewReservationPerHypervisor when true, prevents creating multiple new + // reservations on the same hypervisor within a single reconcile cycle. + // This helps spread reservations across hypervisors. + // Default: true + LimitOneNewReservationPerHypervisor bool `json:"limitOneNewReservationPerHypervisor"` + + // VMSelectionRotationInterval controls how often the VM selection offset rotates + // when MaxVMsToProcess limits processing. Every N reconcile cycles, the offset + // rotates to process different VMs. This ensures all VMs eventually get processed. + // Default: 4 (rotate every 4th reconcile cycle). Use 0 to disable rotation. + VMSelectionRotationInterval *int `json:"vmSelectionRotationInterval"` +} + +// intPtr returns a pointer to the given int value. +func intPtr(i int) *int { + return &i +} + +// ApplyDefaults fills in any unset values with defaults. +func (c *FailoverConfig) ApplyDefaults() { + defaults := DefaultConfig() + if c.DatasourceName == "" { + c.DatasourceName = defaults.DatasourceName + } + if c.SchedulerURL == "" { + c.SchedulerURL = defaults.SchedulerURL + } + if c.ReconcileInterval.Duration == 0 { + c.ReconcileInterval = defaults.ReconcileInterval + } + if c.Creator == "" { + c.Creator = defaults.Creator + } + if c.FlavorFailoverRequirements == nil { + c.FlavorFailoverRequirements = defaults.FlavorFailoverRequirements + } + if c.RevalidationInterval.Duration == 0 { + c.RevalidationInterval = defaults.RevalidationInterval + } + if c.ShortReconcileInterval.Duration == 0 { + c.ShortReconcileInterval = defaults.ShortReconcileInterval + } + if c.MinSuccessForShortInterval == nil { + c.MinSuccessForShortInterval = defaults.MinSuccessForShortInterval + } + if c.MaxFailuresForShortInterval == nil { + c.MaxFailuresForShortInterval = defaults.MaxFailuresForShortInterval + } + if c.MaxVMsToProcess == 0 { + c.MaxVMsToProcess = defaults.MaxVMsToProcess + } + if c.VMSelectionRotationInterval == nil { + c.VMSelectionRotationInterval = defaults.VMSelectionRotationInterval + } } // DefaultConfig returns a default configuration. func DefaultConfig() FailoverConfig { return FailoverConfig{ - FlavorFailoverRequirements: map[string]int{"*": 2}, // by default all VMs get 2 failover reservations - ReconcileInterval: 30 * time.Second, - ShortReconcileInterval: 100 * time.Millisecond, - Creator: "cortex-failover-controller", - DatasourceName: "nova-servers", // we have the server and flavor data source (both store in same postgres and same secret but still) - SchedulerURL: "http://localhost:8080/scheduler/nova/external", - TrustHypervisorLocation: false, - RevalidationInterval: 30 * time.Minute, + FlavorFailoverRequirements: map[string]int{"*": 2}, // by default all VMs get 2 failover reservations + ReconcileInterval: metav1.Duration{Duration: 30 * time.Second}, + ShortReconcileInterval: metav1.Duration{Duration: 100 * time.Millisecond}, + MinSuccessForShortInterval: intPtr(1), + MaxFailuresForShortInterval: intPtr(99), + MaxVMsToProcess: 30, + Creator: "cortex-failover-controller", + DatasourceName: "nova-servers", // we have the server and flavor data source (both store in same postgres and same secret but still) + SchedulerURL: "http://localhost:8080/scheduler/nova/external", + TrustHypervisorLocation: false, + RevalidationInterval: metav1.Duration{Duration: 30 * time.Minute}, + LimitOneNewReservationPerHypervisor: true, + VMSelectionRotationInterval: intPtr(4), } } diff --git a/internal/scheduling/reservations/failover/context.go b/internal/scheduling/reservations/failover/context.go index e18d5eac5..448259692 100644 --- a/internal/scheduling/reservations/failover/context.go +++ b/internal/scheduling/reservations/failover/context.go @@ -8,8 +8,14 @@ import ( "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" "github.com/go-logr/logr" + "github.com/google/uuid" ) +// WithNewGlobalRequestID creates a new context with a failover-prefixed global request ID. +func WithNewGlobalRequestID(ctx context.Context) context.Context { + return reservations.WithGlobalRequestID(ctx, "failover-"+uuid.New().String()) +} + // LoggerFromContext returns a logger with greq and req values from the context. // This creates a child logger with the request tracking values pre-attached, // so you don't need to repeat them in every log call. diff --git a/internal/scheduling/reservations/failover/controller.go b/internal/scheduling/reservations/failover/controller.go index 949ec894a..4cc7d3c1e 100644 --- a/internal/scheduling/reservations/failover/controller.go +++ b/internal/scheduling/reservations/failover/controller.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "path/filepath" + "slices" "sort" "time" @@ -64,9 +65,7 @@ type vmFailoverNeed struct { // It validates the reservation and acknowledges it if valid, or deletes it if invalid. // After processing, it requeues for periodic re-validation. func (c *FailoverReservationController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - // Generate a random UUID for request tracking - globalReqID := uuid.New().String() - ctx = reservations.WithGlobalRequestID(ctx, globalReqID) + ctx = WithNewGlobalRequestID(ctx) logger := LoggerFromContext(ctx).WithValues("reservation", req.Name, "namespace", req.Namespace) logger.Info("reconciling failover reservation", "reservation", req.Name) @@ -90,7 +89,7 @@ func (c *FailoverReservationController) Reconcile(ctx context.Context, req ctrl. // Skip if no failover status (reservation not yet initialized by periodic controller) if res.Status.FailoverReservation == nil { logger.V(1).Info("skipping reservation without failover status") - return ctrl.Result{RequeueAfter: c.Config.RevalidationInterval}, nil + return ctrl.Result{RequeueAfter: c.Config.RevalidationInterval.Duration}, nil } // Validate and acknowledge the reservation @@ -122,7 +121,7 @@ func (c *FailoverReservationController) reconcileValidateAndAcknowledge(ctx cont return ctrl.Result{}, patchErr } - return ctrl.Result{RequeueAfter: c.Config.RevalidationInterval}, nil + return ctrl.Result{RequeueAfter: c.Config.RevalidationInterval.Duration}, nil } // Validate the reservation @@ -130,7 +129,7 @@ func (c *FailoverReservationController) reconcileValidateAndAcknowledge(ctx cont if validationErr != nil { logger.Error(validationErr, "transient error during reservation validation, will retry", "host", res.Status.Host) - return ctrl.Result{RequeueAfter: c.Config.RevalidationInterval}, nil + return ctrl.Result{RequeueAfter: c.Config.RevalidationInterval.Duration}, nil } if !valid { @@ -168,7 +167,7 @@ func (c *FailoverReservationController) reconcileValidateAndAcknowledge(ctx cont logger.V(1).Info("reservation validation passed (no new changes to acknowledge)", "host", res.Status.Host) } - return ctrl.Result{RequeueAfter: c.Config.RevalidationInterval}, nil + return ctrl.Result{RequeueAfter: c.Config.RevalidationInterval.Duration}, nil } // validateReservation validates that a reservation is still valid for all its allocated VMs. @@ -243,8 +242,7 @@ type reconcileSummary struct { func (c *FailoverReservationController) ReconcilePeriodic(ctx context.Context) (ctrl.Result, error) { startTime := time.Now() c.reconcileCount++ - globalReqID := uuid.New().String() - ctx = reservations.WithGlobalRequestID(ctx, globalReqID) + ctx = WithNewGlobalRequestID(ctx) logger := LoggerFromContext(ctx) var summary reconcileSummary @@ -322,9 +320,17 @@ func (c *FailoverReservationController) ReconcilePeriodic(ctx context.Context) ( // Log summary duration := time.Since(startTime) + requeueAfter := c.Config.ReconcileInterval.Duration + successCount := summary.totalCreated + summary.totalReused + madeProgress := successCount >= *c.Config.MinSuccessForShortInterval + lowFailures := summary.totalFailed <= *c.Config.MaxFailuresForShortInterval + if hitMaxVMsLimit && madeProgress && lowFailures && c.Config.ShortReconcileInterval.Duration > 0 { + requeueAfter = c.Config.ShortReconcileInterval.Duration + } logger.Info("periodic reconciliation completed", "reconcileCount", c.reconcileCount, "duration", duration.Round(time.Millisecond), + "requeueAfter", requeueAfter, "vmsProcessed", summary.vmsProcessed, "reservationsNeeded", summary.reservationsNeeded, "reused", summary.totalReused, @@ -333,12 +339,7 @@ func (c *FailoverReservationController) ReconcilePeriodic(ctx context.Context) ( "updated", summary.reservationsUpdated, "deleted", summary.reservationsDeleted) - if hitMaxVMsLimit && c.Config.ShortReconcileInterval > 0 { - logger.Info("requeuing with short interval due to MaxVMsToProcess limit", "shortReconcileInterval", c.Config.ShortReconcileInterval) - return ctrl.Result{RequeueAfter: c.Config.ShortReconcileInterval}, nil - } - - return ctrl.Result{RequeueAfter: c.Config.ReconcileInterval}, nil + return ctrl.Result{RequeueAfter: requeueAfter}, nil } // reconcileRemoveInvalidVMFromReservations removes VMs from reservation allocations if: @@ -418,56 +419,47 @@ func reconcileRemoveNoneligibleVMFromReservations( vmByUUID[vm.UUID] = vm } + nonEligibleVMs := CheckVMsStillEligible(vmByUUID, failoverReservations) + updatedReservations = make([]v1alpha1.Reservation, 0, len(failoverReservations)) for _, res := range failoverReservations { + vmsToRemove, needsUpdate := nonEligibleVMs[res.Name] + if !needsUpdate { + updatedReservations = append(updatedReservations, res) + continue + } + + removeSet := make(map[string]bool) + for _, vmUUID := range vmsToRemove { + removeSet[vmUUID] = true + } + allocations := getFailoverAllocations(&res) updatedAllocations := make(map[string]string) - needsUpdate := false - for vmUUID, allocatedHypervisor := range allocations { - vm, vmExists := vmByUUID[vmUUID] - if !vmExists { + if !removeSet[vmUUID] { updatedAllocations[vmUUID] = allocatedHypervisor - continue } - - tempRes := res.DeepCopy() - delete(tempRes.Status.FailoverReservation.Allocations, vmUUID) - - tempReservations := make([]v1alpha1.Reservation, 0, len(failoverReservations)) - for _, r := range failoverReservations { - if r.Name == res.Name { - tempReservations = append(tempReservations, *tempRes) - } else { - tempReservations = append(tempReservations, r) - } - } - - if !IsVMEligibleForReservation(vm, *tempRes, tempReservations) { - logger.Info("removing VM from reservation allocations because it no longer meets eligibility criteria", - "vmUUID", vmUUID, "reservation", res.Name, - "vmHypervisor", vm.CurrentHypervisor, "reservationHypervisor", res.Status.Host) - needsUpdate = true - continue - } - updatedAllocations[vmUUID] = allocatedHypervisor } - if needsUpdate { - updatedRes := res.DeepCopy() - if updatedRes.Status.FailoverReservation == nil { - updatedRes.Status.FailoverReservation = &v1alpha1.FailoverReservationStatus{} - } - updatedRes.Status.FailoverReservation.Allocations = updatedAllocations - now := metav1.Now() - updatedRes.Status.FailoverReservation.LastChanged = &now - updatedRes.Status.FailoverReservation.AcknowledgedAt = nil - updatedReservations = append(updatedReservations, *updatedRes) - reservationsToUpdate = append(reservationsToUpdate, updatedRes) - } else { - updatedReservations = append(updatedReservations, res) + logger.Info("removing non-eligible VMs from reservation", + "reservation", res.Name, + "host", res.Status.Host, + "before", len(allocations), + "after", len(updatedAllocations), + "removed", vmsToRemove) + + updatedRes := res.DeepCopy() + if updatedRes.Status.FailoverReservation == nil { + updatedRes.Status.FailoverReservation = &v1alpha1.FailoverReservationStatus{} } + updatedRes.Status.FailoverReservation.Allocations = updatedAllocations + now := metav1.Now() + updatedRes.Status.FailoverReservation.LastChanged = &now + updatedRes.Status.FailoverReservation.AcknowledgedAt = nil + updatedReservations = append(updatedReservations, *updatedRes) + reservationsToUpdate = append(reservationsToUpdate, updatedRes) } return updatedReservations, reservationsToUpdate @@ -517,7 +509,8 @@ func (c *FailoverReservationController) selectVMsToProcess( } offset := 0 - if c.reconcileCount%4 == 0 { + rotationInterval := *c.Config.VMSelectionRotationInterval + if rotationInterval > 0 && c.reconcileCount%int64(rotationInterval) == 0 { offset = int(c.reconcileCount) % len(vmsMissingFailover) } @@ -575,6 +568,7 @@ func (c *FailoverReservationController) reconcileCreateAndAssignReservations( } var totalReused, totalCreated, totalFailed int + excludeHypervisors := make(map[string]bool) for _, need := range vmsMissingFailover { vmReused := 0 @@ -592,6 +586,11 @@ func (c *FailoverReservationController) reconcileCreateAndAssignReservations( if reusedRes != nil { if err := c.patchReservationStatus(vmCtx, reusedRes); err != nil { vmLogger.Error(err, "failed to persist reused reservation", "reservationName", reusedRes.Name) + // Remove the stale reservation from the in-memory list to prevent + // other VMs from trying to reuse it in this reconcile cycle + failoverReservations = slices.DeleteFunc(failoverReservations, func(r v1alpha1.Reservation) bool { + return r.Name == reusedRes.Name + }) vmFailed++ continue } @@ -605,7 +604,7 @@ func (c *FailoverReservationController) reconcileCreateAndAssignReservations( continue } - newRes, err := c.scheduleAndBuildNewFailoverReservation(vmCtx, need.VM, allHypervisors, failoverReservations) + newRes, err := c.scheduleAndBuildNewFailoverReservation(vmCtx, need.VM, allHypervisors, failoverReservations, excludeHypervisors) if err != nil { vmLogger.V(1).Info("failed to schedule failover reservation", "error", err, "iteration", i+1, "needed", need.Count) vmFailed++ @@ -633,6 +632,7 @@ func (c *FailoverReservationController) reconcileCreateAndAssignReservations( vmCreated++ failoverReservations = append(failoverReservations, *newRes) + excludeHypervisors[newRes.Status.Host] = true } vmLogger.Info("processed VM failover reservations", @@ -728,13 +728,22 @@ func (c *FailoverReservationController) getRequiredFailoverCount(flavorName stri } // patchReservationStatus patches the status of a reservation using MergeFrom. +// If the reservation is not found in the cache (e.g., just created), it uses the +// passed-in object directly as the base for the patch. func (c *FailoverReservationController) patchReservationStatus(ctx context.Context, res *v1alpha1.Reservation) error { logger := LoggerFromContext(ctx).WithValues("reservationName", res.Name) current := &v1alpha1.Reservation{} if err := c.Get(ctx, client.ObjectKeyFromObject(res), current); err != nil { - logger.Error(err, "failed to get current reservation state") - return fmt.Errorf("failed to get current reservation state: %w", err) + if apierrors.IsNotFound(err) { + // Cache hasn't synced yet for newly created reservation, use the passed-in object + logger.V(1).Info("reservation not in cache yet, using passed-in object for status patch") + current = res.DeepCopy() + current.Status = v1alpha1.ReservationStatus{} // Clear status to create proper diff + } else { + logger.Error(err, "failed to get current reservation state") + return fmt.Errorf("failed to get current reservation state: %w", err) + } } old := current.DeepCopy() @@ -773,15 +782,20 @@ func (c *FailoverReservationController) SetupWithManager(mgr ctrl.Manager, mcl * // This can be called directly when the controller is created after the manager starts. func (c *FailoverReservationController) Start(ctx context.Context) error { log.Info("starting failover reservation controller (periodic)", - "reconcileInterval", c.Config.ReconcileInterval, - "shortReconcileInterval", c.Config.ShortReconcileInterval, + "reconcileInterval", c.Config.ReconcileInterval.Duration, + "shortReconcileInterval", c.Config.ShortReconcileInterval.Duration, + "minSuccessForShortInterval", c.Config.MinSuccessForShortInterval, + "maxFailuresForShortInterval", c.Config.MaxFailuresForShortInterval, "creator", c.Config.Creator, "datasourceName", c.Config.DatasourceName, "schedulerURL", c.Config.SchedulerURL, "flavorFailoverRequirements", c.Config.FlavorFailoverRequirements, - "maxVMsToProcess", c.Config.MaxVMsToProcess) + "maxVMsToProcess", c.Config.MaxVMsToProcess, + "trustHypervisorLocation", c.Config.TrustHypervisorLocation, + "revalidationInterval", c.Config.RevalidationInterval.Duration, + "limitOneNewReservationPerHypervisor", c.Config.LimitOneNewReservationPerHypervisor) - timer := time.NewTimer(c.Config.ReconcileInterval) + timer := time.NewTimer(c.Config.ShortReconcileInterval.Duration) defer timer.Stop() for { @@ -794,7 +808,7 @@ func (c *FailoverReservationController) Start(ctx context.Context) error { if err != nil { log.Error(err, "failover reconciliation failed") } - next := c.Config.ReconcileInterval + next := c.Config.ReconcileInterval.Duration if result.RequeueAfter > 0 { next = result.RequeueAfter } diff --git a/internal/scheduling/reservations/failover/controller_test.go b/internal/scheduling/reservations/failover/controller_test.go index 19982d405..424898b03 100644 --- a/internal/scheduling/reservations/failover/controller_test.go +++ b/internal/scheduling/reservations/failover/controller_test.go @@ -988,6 +988,9 @@ func TestSelectVMsToProcess(t *testing.T) { ctx := context.Background() controller := &FailoverReservationController{ reconcileCount: tt.reconcileCount, + Config: FailoverConfig{ + VMSelectionRotationInterval: intPtr(4), // Default rotation interval + }, } vms := createVMs(tt.vmCount) diff --git a/internal/scheduling/reservations/failover/helpers.go b/internal/scheduling/reservations/failover/helpers.go index f7efa135d..c77fc0fe7 100644 --- a/internal/scheduling/reservations/failover/helpers.go +++ b/internal/scheduling/reservations/failover/helpers.go @@ -117,9 +117,10 @@ func newFailoverReservation(ctx context.Context, vm VM, hypervisor, creator stri }, }, Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeFailover, - Resources: resources, - TargetHost: hypervisor, // Set the desired hypervisor from scheduler response + Type: v1alpha1.ReservationTypeFailover, + AvailabilityZone: vm.AvailabilityZone, + Resources: resources, + TargetHost: hypervisor, // Set the desired hypervisor from scheduler response FailoverReservation: &v1alpha1.FailoverReservationSpec{ ResourceGroup: vm.FlavorName, }, diff --git a/internal/scheduling/reservations/failover/integration_test.go b/internal/scheduling/reservations/failover/integration_test.go index b75e383e1..7fe992df5 100644 --- a/internal/scheduling/reservations/failover/integration_test.go +++ b/internal/scheduling/reservations/failover/integration_test.go @@ -229,6 +229,8 @@ func TestIntegration(t *testing.T) { newHypervisor("host5", 16, 32, 4, 8, []hv1.Instance{{ID: "vm-5", Name: "vm-5", Active: true}}, nil), newHypervisor("host6", 32, 64, 0, 0, nil, nil), newHypervisor("host7", 32, 64, 0, 0, nil, nil), + newHypervisor("host8", 32, 64, 0, 0, nil, nil), + newHypervisor("host9", 32, 64, 0, 0, nil, nil), }, VMs: []VM{ newVM("vm-1", "m1.large", "project-A", "host1", 8192, 4), @@ -680,10 +682,11 @@ func (env *IntegrationTestEnv) TriggerFailoverReconcile(flavorRequirements map[s schedulerClient := reservations.NewSchedulerClient(env.SchedulerBaseURL + "/scheduler/nova/external") config := FailoverConfig{ - ReconcileInterval: time.Minute, + ReconcileInterval: metav1.Duration{Duration: time.Minute}, Creator: "test-failover-controller", FlavorFailoverRequirements: flavorRequirements, } + config.ApplyDefaults() controller := NewFailoverReservationController( env.K8sClient, @@ -944,13 +947,14 @@ func (env *IntegrationTestEnv) simulateHostFailure(failedHosts, allHosts []strin } request := novaapi.ExternalSchedulerRequest{ - Pipeline: "nova-external-scheduler-kvm-all-filters-enabled", + Pipeline: "test-filter-pipeline", Hosts: externalHosts, Weights: weights, Spec: novaapi.NovaObject[novaapi.NovaSpec]{ Data: novaapi.NovaSpec{ - InstanceUUID: vm.UUID, - ProjectID: vm.ProjectID, + InstanceUUID: vm.UUID, + ProjectID: vm.ProjectID, + SchedulerHints: map[string]any{"_nova_check_type": []any{"evacuate"}}, Flavor: novaapi.NovaObject[novaapi.NovaFlavor]{ Data: novaapi.NovaFlavor{ Name: vm.FlavorName, @@ -974,7 +978,12 @@ func (env *IntegrationTestEnv) simulateHostFailure(failedHosts, allHosts []strin selectedHost := "" selectedReservation := "" - for _, candidateHost := range response.Hosts { + // Only consider the first 3 hosts (simulating Nova's behavior of using top hosts) + hostsToCheck := response.Hosts + if len(hostsToCheck) > 3 { + hostsToCheck = hostsToCheck[:3] + } + for _, candidateHost := range hostsToCheck { for _, res := range reservationsByHost[candidateHost] { if res.Status.FailoverReservation != nil { if _, vmUsesThis := res.Status.FailoverReservation.Allocations[vm.UUID]; vmUsesThis { @@ -1102,7 +1111,7 @@ func newIntegrationTestEnv(t *testing.T, vms []VM, hypervisors []*hv1.Hypervisor pipelines := []v1alpha1.Pipeline{ { ObjectMeta: metav1.ObjectMeta{ - Name: "nova-external-scheduler-kvm-all-filters-enabled", + Name: "test-filter-pipeline", }, Spec: v1alpha1.PipelineSpec{ Type: v1alpha1.PipelineTypeFilterWeigher, @@ -1177,6 +1186,77 @@ func newIntegrationTestEnv(t *testing.T, vms []VM, hypervisors []*hv1.Hypervisor } } +// testHTTPAPI is a simplified HTTP API for testing that delegates to the controller. +// It does NOT shuffle evacuation hosts, ensuring deterministic test results. +type testHTTPAPI struct { + delegate nova.HTTPAPIDelegate +} + +// NovaExternalScheduler handles the POST request from the Nova scheduler. +func (api *testHTTPAPI) NovaExternalScheduler(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "invalid request method", http.StatusMethodNotAllowed) + return + } + + defer r.Body.Close() + + var requestData novaapi.ExternalSchedulerRequest + if err := json.NewDecoder(r.Body).Decode(&requestData); err != nil { + http.Error(w, "failed to decode request body", http.StatusBadRequest) + return + } + + rawBytes, err := json.Marshal(requestData) + if err != nil { + http.Error(w, "failed to marshal request", http.StatusInternalServerError) + return + } + raw := runtime.RawExtension{Raw: rawBytes} + + pipelineName := requestData.Pipeline + if pipelineName == "" { + pipelineName = "test-filter-pipeline" + } + + decision := &v1alpha1.Decision{ + TypeMeta: metav1.TypeMeta{ + Kind: "Decision", + APIVersion: "cortex.cloud/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "nova-", + }, + Spec: v1alpha1.DecisionSpec{ + SchedulingDomain: v1alpha1.SchedulingDomainNova, + PipelineRef: corev1.ObjectReference{ + Name: pipelineName, + }, + ResourceID: requestData.Spec.Data.InstanceUUID, + NovaRaw: &raw, + }, + } + + ctx := r.Context() + if err := api.delegate.ProcessNewDecisionFromAPI(ctx, decision); err != nil { + http.Error(w, "failed to process scheduling decision", http.StatusInternalServerError) + return + } + + if decision.Status.Result == nil { + http.Error(w, "decision didn't produce a result", http.StatusInternalServerError) + return + } + + hosts := decision.Status.Result.OrderedHosts + response := novaapi.ExternalSchedulerResponse{Hosts: hosts} + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(response); err != nil { + http.Error(w, "failed to encode response", http.StatusInternalServerError) + return + } +} + // newIntegrationTestEnvWithTraitsFilter creates a test environment with the filter_has_requested_traits filter enabled. func newIntegrationTestEnvWithTraitsFilter(t *testing.T, vms []VM, hypervisors []*hv1.Hypervisor, reservations []*v1alpha1.Reservation) *IntegrationTestEnv { t.Helper() @@ -1221,7 +1301,7 @@ func newIntegrationTestEnvWithTraitsFilter(t *testing.T, vms []VM, hypervisors [ pipelines := []v1alpha1.Pipeline{ { ObjectMeta: metav1.ObjectMeta{ - Name: "nova-external-scheduler-kvm-all-filters-enabled", + Name: "test-filter-pipeline", }, Spec: v1alpha1.PipelineSpec{ Type: v1alpha1.PipelineTypeFilterWeigher, @@ -1297,76 +1377,6 @@ func newIntegrationTestEnvWithTraitsFilter(t *testing.T, vms []VM, hypervisors [ } } -// testHTTPAPI is a simplified HTTP API for testing that delegates to the controller. -type testHTTPAPI struct { - delegate nova.HTTPAPIDelegate -} - -// NovaExternalScheduler handles the POST request from the Nova scheduler. -func (api *testHTTPAPI) NovaExternalScheduler(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost { - http.Error(w, "invalid request method", http.StatusMethodNotAllowed) - return - } - - defer r.Body.Close() - - var requestData novaapi.ExternalSchedulerRequest - if err := json.NewDecoder(r.Body).Decode(&requestData); err != nil { - http.Error(w, "failed to decode request body", http.StatusBadRequest) - return - } - - rawBytes, err := json.Marshal(requestData) - if err != nil { - http.Error(w, "failed to marshal request", http.StatusInternalServerError) - return - } - raw := runtime.RawExtension{Raw: rawBytes} - - pipelineName := requestData.Pipeline - if pipelineName == "" { - pipelineName = "nova-external-scheduler-kvm-all-filters-enabled" - } - - decision := &v1alpha1.Decision{ - TypeMeta: metav1.TypeMeta{ - Kind: "Decision", - APIVersion: "cortex.cloud/v1alpha1", - }, - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "nova-", - }, - Spec: v1alpha1.DecisionSpec{ - SchedulingDomain: v1alpha1.SchedulingDomainNova, - PipelineRef: corev1.ObjectReference{ - Name: pipelineName, - }, - ResourceID: requestData.Spec.Data.InstanceUUID, - NovaRaw: &raw, - }, - } - - ctx := r.Context() - if err := api.delegate.ProcessNewDecisionFromAPI(ctx, decision); err != nil { - http.Error(w, "failed to process scheduling decision", http.StatusInternalServerError) - return - } - - if decision.Status.Result == nil { - http.Error(w, "decision didn't produce a result", http.StatusInternalServerError) - return - } - - hosts := decision.Status.Result.OrderedHosts - response := novaapi.ExternalSchedulerResponse{Hosts: hosts} - w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(response); err != nil { - http.Error(w, "failed to encode response", http.StatusInternalServerError) - return - } -} - // ============================================================================ // Object Creation Functions // ============================================================================ diff --git a/internal/scheduling/reservations/failover/reservation_eligibility.go b/internal/scheduling/reservations/failover/reservation_eligibility.go index d62aee424..1f6e7353f 100644 --- a/internal/scheduling/reservations/failover/reservation_eligibility.go +++ b/internal/scheduling/reservations/failover/reservation_eligibility.go @@ -4,10 +4,16 @@ package failover import ( + "slices" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ) +// TODO: The dependency graph does not capture that we can have a senario where the VM=>Host mapping is different across multipel reservations. +// We currently set vmToCurrentHypervisor based on the last vm => host mapping we see in a reservation +// the periodic reconciler should remove all vm : host mapping that do not match the current host of a vm before we end up here. + // DependencyGraph encapsulates the data structures needed for eligibility checking. // It tracks relationships between VMs, reservations, and hypervisors. type DependencyGraph struct { @@ -23,20 +29,119 @@ type DependencyGraph struct { reservationToHost map[string]string } +// IsVMEligibleForReservation checks if a VM is eligible to use a specific reservation. +// A VM is eligible if it satisfies all the following constraints: +// (1) A VM cannot reserve a slot on its own hypervisor. +// (2) A VM's N reservation slots must be placed on N distinct hypervisors. +// (3) For any reservation r, no two VMs that use r may be on the same hypervisor. +// (4) For VM v with slots R, any other VM that uses any slot must not run on v's host or slot hosts. +// (5) For VM v with slots R, no two other VMs using v's slots can be on the same hypervisor. +func IsVMEligibleForReservation(vm VM, reservation v1alpha1.Reservation, allFailoverReservations []v1alpha1.Reservation) bool { + // Check if VM is already using this reservation + resAllocations := getFailoverAllocations(&reservation) + if _, exists := resAllocations[vm.UUID]; exists { + return false + } + + // Ensure the candidate reservation is included in allFailoverReservations + reservationInList := false + for _, res := range allFailoverReservations { + if res.Name == reservation.Name && res.Namespace == reservation.Namespace { + reservationInList = true + break + } + } + if !reservationInList { + allFailoverReservations = append(append([]v1alpha1.Reservation{}, allFailoverReservations...), reservation) + } + + graph := newDependencyGraph(vm, reservation, allFailoverReservations) + return graph.isVMEligibleForReservation(reservationKey(reservation.Namespace, reservation.Name)) +} + +// CheckVMsStillEligible checks if VMs in reservations are still eligible. +// Returns a map of reservation name -> list of VM UUIDs that are no longer eligible. +func CheckVMsStillEligible( + vms map[string]VM, + failoverReservations []v1alpha1.Reservation, +) map[string][]string { + + baseGraph := newBaseDependencyGraph(failoverReservations) + result := make(map[string][]string) + + for _, res := range failoverReservations { + allocations := getFailoverAllocations(&res) + resKey := reservationKey(res.Namespace, res.Name) + + // Sort VM UUIDs for deterministic iteration order + vmUUIDs := make([]string, 0, len(allocations)) + for vmUUID := range allocations { + vmUUIDs = append(vmUUIDs, vmUUID) + } + slices.Sort(vmUUIDs) + + for _, vmUUID := range vmUUIDs { + vm, vmExists := vms[vmUUID] + if !vmExists { + continue + } + + isEligible := false + if vm.CurrentHypervisor == baseGraph.vmToCurrentHypervisor[vmUUID] { + isEligible = baseGraph.isVMEligibleForReservation(resKey) + } + + if !isEligible { + result[res.Name] = append(result[res.Name], vmUUID) + baseGraph.removeVMFromReservation(vm.UUID, resKey, res.Status.Host) + } + } + } + + return result +} + +// FindEligibleReservations finds all reservations that a VM is eligible to use. +func FindEligibleReservations( + vm VM, + failoverReservations []v1alpha1.Reservation, +) []v1alpha1.Reservation { + + baseGraph := newBaseDependencyGraph(failoverReservations) + + var eligible []v1alpha1.Reservation + for _, res := range failoverReservations { + if !doesVMFitInReservation(vm, res) { + continue + } + + resAllocations := getFailoverAllocations(&res) + if _, exists := resAllocations[vm.UUID]; exists { + continue + } + + candidateResKey := reservationKey(res.Namespace, res.Name) + candidateResHost := res.Status.Host + + baseGraph.addVMToReservation(vm.UUID, vm.CurrentHypervisor, candidateResKey, candidateResHost) + isEligible := baseGraph.isVMEligibleForReservation(candidateResKey) + baseGraph.removeVMFromReservation(vm.UUID, candidateResKey, candidateResHost) + + if isEligible { + eligible = append(eligible, res) + } + } + + return eligible +} + // reservationKey returns a unique key for a reservation (namespace/name). -// This prevents collisions between same-named reservations in different namespaces. func reservationKey(namespace, name string) string { return namespace + "/" + name } -// newDependencyGraph builds a DependencyGraph for eligibility checking. -// The VM is treated as already being part of the candidate reservation. -func newDependencyGraph( - vm VM, - candidateReservation v1alpha1.Reservation, - allFailoverReservations []v1alpha1.Reservation, -) *DependencyGraph { - +// newBaseDependencyGraph builds a base DependencyGraph from all reservations. +func newBaseDependencyGraph(allFailoverReservations []v1alpha1.Reservation) *DependencyGraph { g := &DependencyGraph{ vmToReservations: make(map[string]map[string]bool), vmToCurrentHypervisor: make(map[string]string), @@ -45,7 +150,6 @@ func newDependencyGraph( reservationToHost: make(map[string]string), } - // Process all reservations for _, res := range allFailoverReservations { resKey := reservationKey(res.Namespace, res.Name) resHost := res.Status.Host @@ -63,30 +167,22 @@ func newDependencyGraph( } } - // Process candidate reservation (may not be in allFailoverReservations) - candidateResKey := reservationKey(candidateReservation.Namespace, candidateReservation.Name) - candidateResHost := candidateReservation.Status.Host + return g +} - g.ensureResInMaps(candidateResKey) - g.reservationToHost[candidateResKey] = candidateResHost +// newDependencyGraph builds a DependencyGraph with the VM added to the candidate reservation. +func newDependencyGraph( + vm VM, + candidateReservation v1alpha1.Reservation, + allFailoverReservations []v1alpha1.Reservation, +) *DependencyGraph { - candidateAllocations := getFailoverAllocations(&candidateReservation) - for vmUUID, vmHypervisor := range candidateAllocations { - g.ensureVMInMaps(vmUUID) - g.vmToReservations[vmUUID][candidateResKey] = true - g.vmToCurrentHypervisor[vmUUID] = vmHypervisor - g.vmToReservationHosts[vmUUID][candidateResHost] = true - g.reservationToVMs[candidateResKey][vmUUID] = true - } + g := newBaseDependencyGraph(allFailoverReservations) - // Add the VM we're checking with its current hypervisor - g.ensureVMInMaps(vm.UUID) - g.vmToCurrentHypervisor[vm.UUID] = vm.CurrentHypervisor + candidateResKey := reservationKey(candidateReservation.Namespace, candidateReservation.Name) + candidateResHost := candidateReservation.Status.Host - // KEY: Treat VM as already in the candidate reservation - g.vmToReservations[vm.UUID][candidateResKey] = true - g.vmToReservationHosts[vm.UUID][candidateResHost] = true - g.reservationToVMs[candidateResKey][vm.UUID] = true + g.addVMToReservation(vm.UUID, vm.CurrentHypervisor, candidateResKey, candidateResHost) return g } @@ -106,8 +202,24 @@ func (g *DependencyGraph) ensureResInMaps(resName string) { } } +// addVMToReservation adds a VM to a reservation in the graph. +func (g *DependencyGraph) addVMToReservation(vmUUID, vmHypervisor, resKey, resHost string) { + g.ensureVMInMaps(vmUUID) + g.ensureResInMaps(resKey) + g.vmToCurrentHypervisor[vmUUID] = vmHypervisor + g.vmToReservations[vmUUID][resKey] = true + g.vmToReservationHosts[vmUUID][resHost] = true + g.reservationToVMs[resKey][vmUUID] = true +} + +// removeVMFromReservation removes a VM from a reservation in the graph. +func (g *DependencyGraph) removeVMFromReservation(vmUUID, resKey, resHost string) { + delete(g.vmToReservations[vmUUID], resKey) + delete(g.vmToReservationHosts[vmUUID], resHost) + delete(g.reservationToVMs[resKey], vmUUID) +} + // checkAllVMConstraints checks if a single VM satisfies all constraints (1-5). -// Returns true if the VM satisfies all constraints. func (g *DependencyGraph) checkAllVMConstraints(vmUUID string) bool { vmCurrentHypervisor := g.vmToCurrentHypervisor[vmUUID] vmSlotHypervisors := g.vmToReservationHosts[vmUUID] @@ -176,83 +288,27 @@ func (g *DependencyGraph) isVMEligibleForReservation(candidateResName string) bo return true } -// IsVMEligibleForReservation checks if a VM is eligible to use a specific reservation. -// A VM is eligible if it satisfies all the following constraints: -// (1) A VM cannot reserve a slot on its own hypervisor. -// (2) A VM's N reservation slots must be placed on N distinct hypervisors (no hypervisor overlap among slots). -// (3) For any reservation r, no two VMs that use r may be on the same hypervisor (directly or potentially via a reservation). -// (4) For VM v with slots R = {r1..rn}, any other VM vi that uses any rj must not run on hs(v) nor on any hs(rj). -// (5) For VM v with slots R = {r1..rn}, there exist no vm_1, vm_2 (vm_1 != v and vm_2 != v) -// -// with vm_1 uses r_j and vm_2 uses r_k and hypervisor(vm_1) = hypervisor(vm_2). -func IsVMEligibleForReservation(vm VM, reservation v1alpha1.Reservation, allFailoverReservations []v1alpha1.Reservation) bool { - // Check if VM is already using this reservation - resAllocations := getFailoverAllocations(&reservation) - if _, exists := resAllocations[vm.UUID]; exists { - return false - } - - // Ensure the candidate reservation is included in allFailoverReservations - reservationInList := false - for _, res := range allFailoverReservations { - if res.Name == reservation.Name && res.Namespace == reservation.Namespace { - reservationInList = true - break - } - } - if !reservationInList { - allFailoverReservations = append(append([]v1alpha1.Reservation{}, allFailoverReservations...), reservation) - } - - // Build dependency graph (with VM already in the candidate reservation) - graph := newDependencyGraph(vm, reservation, allFailoverReservations) - - // Check all constraints for all VMs in the reservation - return graph.isVMEligibleForReservation(reservationKey(reservation.Namespace, reservation.Name)) -} - // doesVMFitInReservation checks if a VM's resources fit within a reservation's resources. -// Returns true if all VM resources are less than or equal to the reservation's resources. func doesVMFitInReservation(vm VM, reservation v1alpha1.Reservation) bool { - // Check memory: VM's memory must be <= reservation's memory if vmMemory, ok := vm.Resources["memory"]; ok { if resMemory, ok := reservation.Spec.Resources[hv1.ResourceMemory]; ok { if vmMemory.Cmp(resMemory) > 0 { - return false // VM memory exceeds reservation memory + return false } } else { - return false // Reservation has no memory resource defined + return false } } - // Check CPU: VM's vcpus must be <= reservation's cpu - // Note: VM uses "vcpus" key, but reservations use "cpu" as the canonical key. if vmVCPUs, ok := vm.Resources["vcpus"]; ok { if resCPU, ok := reservation.Spec.Resources[hv1.ResourceCPU]; ok { if vmVCPUs.Cmp(resCPU) > 0 { - return false // VM vcpus exceeds reservation cpu + return false } } else { - return false // Reservation has no cpu resource defined + return false } } return true } - -// FindEligibleReservations finds all reservations that a VM is eligible to use. -// It checks both resource fit and eligibility constraints. -func FindEligibleReservations( - vm VM, - failoverReservations []v1alpha1.Reservation, -) []v1alpha1.Reservation { - //TODO: we create data mappings inside IsVMEligibleForReservation those should probably be done already on this level to avoid redundant work - var eligible []v1alpha1.Reservation - for _, res := range failoverReservations { - if doesVMFitInReservation(vm, res) && IsVMEligibleForReservation(vm, res, failoverReservations) { - eligible = append(eligible, res) - } - } - - return eligible -} diff --git a/internal/scheduling/reservations/failover/reservation_eligibility_test.go b/internal/scheduling/reservations/failover/reservation_eligibility_test.go index ddae548cc..841d15abe 100644 --- a/internal/scheduling/reservations/failover/reservation_eligibility_test.go +++ b/internal/scheduling/reservations/failover/reservation_eligibility_test.go @@ -1302,6 +1302,590 @@ func TestDataStructureConsistency(t *testing.T) { } } +// TestNewBaseDependencyGraph tests the newBaseDependencyGraph function. +func TestNewBaseDependencyGraph(t *testing.T) { + testCases := []struct { + name string + reservations []v1alpha1.Reservation + expectedVMCount int + expectedResCount int + expectedVMToRes map[string][]string // vmUUID -> list of reservation keys + expectedResToVMs map[string][]string // resKey -> list of vmUUIDs + expectedVMHypervisor map[string]string // vmUUID -> hypervisor + }{ + { + name: "empty reservations", + reservations: []v1alpha1.Reservation{}, + expectedVMCount: 0, + expectedResCount: 0, + expectedVMToRes: map[string][]string{}, + expectedResToVMs: map[string][]string{}, + expectedVMHypervisor: map[string]string{}, + }, + { + name: "single reservation with no VMs", + reservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host1", map[string]string{}), + }, + expectedVMCount: 0, + expectedResCount: 1, + expectedVMToRes: map[string][]string{}, + expectedResToVMs: map[string][]string{"/res-1": {}}, + expectedVMHypervisor: map[string]string{}, + }, + { + name: "single reservation with one VM", + reservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{"vm-1": "host1"}), + }, + expectedVMCount: 1, + expectedResCount: 1, + expectedVMToRes: map[string][]string{"vm-1": {"/res-1"}}, + expectedResToVMs: map[string][]string{"/res-1": {"vm-1"}}, + expectedVMHypervisor: map[string]string{"vm-1": "host1"}, + }, + { + name: "single reservation with multiple VMs", + reservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host3", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + }, + expectedVMCount: 2, + expectedResCount: 1, + expectedVMToRes: map[string][]string{"vm-1": {"/res-1"}, "vm-2": {"/res-1"}}, + expectedResToVMs: map[string][]string{"/res-1": {"vm-1", "vm-2"}}, + expectedVMHypervisor: map[string]string{"vm-1": "host1", "vm-2": "host2"}, + }, + { + name: "multiple reservations with shared VMs", + reservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host3", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host4", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + }, + expectedVMCount: 2, + expectedResCount: 2, + expectedVMToRes: map[string][]string{"vm-1": {"/res-1", "/res-2"}, "vm-2": {"/res-2"}}, + expectedResToVMs: map[string][]string{"/res-1": {"vm-1"}, "/res-2": {"vm-1", "vm-2"}}, + expectedVMHypervisor: map[string]string{"vm-1": "host1", "vm-2": "host2"}, + }, + { + name: "VM in multiple reservations tracks all hosts", + reservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host3", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host4", map[string]string{"vm-1": "host1"}), + makeReservation("res-3", "host5", map[string]string{"vm-1": "host1"}), + }, + expectedVMCount: 1, + expectedResCount: 3, + expectedVMToRes: map[string][]string{"vm-1": {"/res-1", "/res-2", "/res-3"}}, + expectedResToVMs: map[string][]string{"/res-1": {"vm-1"}, "/res-2": {"vm-1"}, "/res-3": {"vm-1"}}, + expectedVMHypervisor: map[string]string{"vm-1": "host1"}, + }, + { + name: "complex: 3 reservations with 1-3 VMs each, 4 different VMs", + reservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host5", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host6", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + makeReservation("res-3", "host7", map[string]string{"vm-2": "host2", "vm-3": "host3", "vm-4": "host4"}), + }, + expectedVMCount: 4, + expectedResCount: 3, + expectedVMToRes: map[string][]string{ + "vm-1": {"/res-1", "/res-2"}, + "vm-2": {"/res-2", "/res-3"}, + "vm-3": {"/res-3"}, + "vm-4": {"/res-3"}, + }, + expectedResToVMs: map[string][]string{ + "/res-1": {"vm-1"}, + "/res-2": {"vm-1", "vm-2"}, + "/res-3": {"vm-2", "vm-3", "vm-4"}, + }, + expectedVMHypervisor: map[string]string{ + "vm-1": "host1", + "vm-2": "host2", + "vm-3": "host3", + "vm-4": "host4", + }, + }, + { + // This tests the case where a VM has different host allocations across reservations. + // This can happen when a VM migrates - the old allocation in one reservation might + // have a stale host while another reservation has the current host. + // The graph uses the LAST seen hypervisor for each VM (order depends on map iteration). + name: "VM with different host allocations across reservations (stale data)", + reservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host5", map[string]string{"vm-1": "host1-old"}), + makeReservation("res-2", "host6", map[string]string{"vm-1": "host1-new", "vm-2": "host2"}), + }, + expectedVMCount: 2, + expectedResCount: 2, + expectedVMToRes: map[string][]string{ + "vm-1": {"/res-1", "/res-2"}, + "vm-2": {"/res-2"}, + }, + expectedResToVMs: map[string][]string{ + "/res-1": {"vm-1"}, + "/res-2": {"vm-1", "vm-2"}, + }, + // Note: The hypervisor stored depends on iteration order, but both reservations + // track the VM. + expectedVMHypervisor: map[string]string{ + // We can't predict which one wins due to map iteration order, + // so we skip this check for vm-1 in this test case + "vm-2": "host2", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + graph := newBaseDependencyGraph(tc.reservations) + + // Check VM count + if len(graph.vmToReservations) != tc.expectedVMCount { + t.Errorf("vmToReservations has %d VMs, expected %d", len(graph.vmToReservations), tc.expectedVMCount) + } + + // Check reservation count + if len(graph.reservationToVMs) != tc.expectedResCount { + t.Errorf("reservationToVMs has %d reservations, expected %d", len(graph.reservationToVMs), tc.expectedResCount) + } + + // Check VM to reservations mapping + for vmUUID, expectedResKeys := range tc.expectedVMToRes { + actualResKeys := graph.vmToReservations[vmUUID] + if len(actualResKeys) != len(expectedResKeys) { + t.Errorf("VM %s has %d reservations, expected %d", vmUUID, len(actualResKeys), len(expectedResKeys)) + } + for _, resKey := range expectedResKeys { + if !actualResKeys[resKey] { + t.Errorf("VM %s missing reservation %s", vmUUID, resKey) + } + } + } + + // Check reservation to VMs mapping + for resKey, expectedVMs := range tc.expectedResToVMs { + actualVMs := graph.reservationToVMs[resKey] + if len(actualVMs) != len(expectedVMs) { + t.Errorf("Reservation %s has %d VMs, expected %d", resKey, len(actualVMs), len(expectedVMs)) + } + for _, vmUUID := range expectedVMs { + if !actualVMs[vmUUID] { + t.Errorf("Reservation %s missing VM %s", resKey, vmUUID) + } + } + } + + // Check VM hypervisors + for vmUUID, expectedHypervisor := range tc.expectedVMHypervisor { + actualHypervisor := graph.vmToCurrentHypervisor[vmUUID] + if actualHypervisor != expectedHypervisor { + t.Errorf("VM %s has hypervisor %s, expected %s", vmUUID, actualHypervisor, expectedHypervisor) + } + } + }) + } +} + +// makeGraph creates a DependencyGraph from reservations for testing. +func makeGraph(reservations []v1alpha1.Reservation) *DependencyGraph { + return newBaseDependencyGraph(reservations) +} + +// graphsEqual compares two DependencyGraphs for equality. +// VMs with empty reservation sets are considered equivalent to not being in the graph. +func graphsEqual(t *testing.T, actual, expected *DependencyGraph) { + t.Helper() + + // Count VMs with non-empty reservations + countActiveVMs := func(g *DependencyGraph) int { + count := 0 + for _, res := range g.vmToReservations { + if len(res) > 0 { + count++ + } + } + return count + } + + actualActiveVMs := countActiveVMs(actual) + expectedActiveVMs := countActiveVMs(expected) + if actualActiveVMs != expectedActiveVMs { + t.Errorf("vmToReservations: got %d active VMs, want %d", actualActiveVMs, expectedActiveVMs) + } + + // Compare vmToReservations (only for VMs with reservations) + for vmUUID, expectedRes := range expected.vmToReservations { + if len(expectedRes) == 0 { + continue // Skip VMs with no reservations + } + actualRes := actual.vmToReservations[vmUUID] + if len(actualRes) != len(expectedRes) { + t.Errorf("vmToReservations[%s]: got %d reservations, want %d", vmUUID, len(actualRes), len(expectedRes)) + } + for resKey := range expectedRes { + if !actualRes[resKey] { + t.Errorf("vmToReservations[%s]: missing reservation %s", vmUUID, resKey) + } + } + } + + // Compare vmToCurrentHypervisor (only for VMs with reservations) + for vmUUID, expectedHV := range expected.vmToCurrentHypervisor { + if len(expected.vmToReservations[vmUUID]) == 0 { + continue // Skip VMs with no reservations + } + if actualHV := actual.vmToCurrentHypervisor[vmUUID]; actualHV != expectedHV { + t.Errorf("vmToCurrentHypervisor[%s]: got %s, want %s", vmUUID, actualHV, expectedHV) + } + } + + // Compare vmToReservationHosts (only for VMs with reservations) + for vmUUID, expectedHosts := range expected.vmToReservationHosts { + if len(expected.vmToReservations[vmUUID]) == 0 { + continue // Skip VMs with no reservations + } + actualHosts := actual.vmToReservationHosts[vmUUID] + if len(actualHosts) != len(expectedHosts) { + t.Errorf("vmToReservationHosts[%s]: got %d hosts, want %d", vmUUID, len(actualHosts), len(expectedHosts)) + } + for host := range expectedHosts { + if !actualHosts[host] { + t.Errorf("vmToReservationHosts[%s]: missing host %s", vmUUID, host) + } + } + } + + // Compare reservationToVMs + if len(actual.reservationToVMs) != len(expected.reservationToVMs) { + t.Errorf("reservationToVMs: got %d reservations, want %d", len(actual.reservationToVMs), len(expected.reservationToVMs)) + } + for resKey, expectedVMs := range expected.reservationToVMs { + actualVMs := actual.reservationToVMs[resKey] + if len(actualVMs) != len(expectedVMs) { + t.Errorf("reservationToVMs[%s]: got %d VMs, want %d", resKey, len(actualVMs), len(expectedVMs)) + } + for vmUUID := range expectedVMs { + if !actualVMs[vmUUID] { + t.Errorf("reservationToVMs[%s]: missing VM %s", resKey, vmUUID) + } + } + } +} + +// TestNewDependencyGraph tests the newDependencyGraph function. +func TestNewDependencyGraph(t *testing.T) { + testCases := []struct { + name string + vm VM + candidateRes v1alpha1.Reservation + allReservations []v1alpha1.Reservation + expectGraph *DependencyGraph + }{ + { + name: "VM added to empty reservation with no other reservations", + vm: makeVM("vm-1", "host1"), + candidateRes: makeReservation("res-1", "host2", map[string]string{}), + allReservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{}), + }, + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{"vm-1": "host1"}), + }), + }, + { + name: "VM added to reservation with existing VM", + vm: makeVM("vm-2", "host2"), + candidateRes: makeReservation("res-1", "host3", map[string]string{"vm-1": "host1"}), + allReservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host3", map[string]string{"vm-1": "host1"}), + }, + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host3", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + }), + }, + { + name: "VM added to one of multiple reservations", + vm: makeVM("vm-3", "host3"), + candidateRes: makeReservation("res-2", "host5", map[string]string{}), + allReservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host4", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host5", map[string]string{}), + }, + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host4", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host5", map[string]string{"vm-3": "host3"}), + }), + }, + { + name: "VM already in other reservations, added to new one", + vm: makeVM("vm-1", "host1"), + candidateRes: makeReservation("res-2", "host5", map[string]string{}), + allReservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host4", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host5", map[string]string{}), + }, + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host4", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host5", map[string]string{"vm-1": "host1"}), + }), + }, + { + name: "complex: 3 reservations with 4 VMs, add vm-5 to res-2", + vm: makeVM("vm-5", "host8"), + candidateRes: makeReservation("res-2", "host6", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + allReservations: []v1alpha1.Reservation{ + makeReservation("res-1", "host5", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host6", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + makeReservation("res-3", "host7", map[string]string{"vm-2": "host2", "vm-3": "host3", "vm-4": "host4"}), + }, + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host5", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host6", map[string]string{"vm-1": "host1", "vm-2": "host2", "vm-5": "host8"}), + makeReservation("res-3", "host7", map[string]string{"vm-2": "host2", "vm-3": "host3", "vm-4": "host4"}), + }), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + graph := newDependencyGraph(tc.vm, tc.candidateRes, tc.allReservations) + graphsEqual(t, graph, tc.expectGraph) + }) + } +} + +// TestAddVMToReservation tests the addVMToReservation method. +func TestAddVMToReservation(t *testing.T) { + testCases := []struct { + name string + initGraph *DependencyGraph + vmUUID string + vmHypervisor string + resKey string + resHost string + expectGraph *DependencyGraph + }{ + { + name: "add VM to empty graph", + initGraph: makeGraph(nil), + vmUUID: "vm-1", + vmHypervisor: "host1", + resKey: "/res-1", + resHost: "host2", + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{"vm-1": "host1"}), + }), + }, + { + name: "add VM to existing reservation with one VM", + initGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{"vm-1": "host1"}), + }), + vmUUID: "vm-2", + vmHypervisor: "host3", + resKey: "/res-1", + resHost: "host2", + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{"vm-1": "host1", "vm-2": "host3"}), + }), + }, + { + name: "add existing VM to second reservation", + initGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{"vm-1": "host1"}), + }), + vmUUID: "vm-1", + vmHypervisor: "host1", + resKey: "/res-2", + resHost: "host3", + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host3", map[string]string{"vm-1": "host1"}), + }), + }, + { + name: "add VM to complex graph with 3 reservations and 4 VMs", + initGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host5", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host6", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + makeReservation("res-3", "host7", map[string]string{"vm-2": "host2", "vm-3": "host3"}), + }), + vmUUID: "vm-4", + vmHypervisor: "host4", + resKey: "/res-3", + resHost: "host7", + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host5", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host6", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + makeReservation("res-3", "host7", map[string]string{"vm-2": "host2", "vm-3": "host3", "vm-4": "host4"}), + }), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.initGraph.addVMToReservation(tc.vmUUID, tc.vmHypervisor, tc.resKey, tc.resHost) + graphsEqual(t, tc.initGraph, tc.expectGraph) + }) + } +} + +// TestRemoveVMFromReservation tests the removeVMFromReservation method. +func TestRemoveVMFromReservation(t *testing.T) { + testCases := []struct { + name string + initGraph *DependencyGraph + vmUUID string + resKey string + resHost string + expectGraph *DependencyGraph + }{ + { + name: "remove VM from single reservation", + initGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{"vm-1": "host1"}), + }), + vmUUID: "vm-1", + resKey: "/res-1", + resHost: "host2", + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{}), + }), + }, + { + name: "remove VM from one of two reservations", + initGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host3", map[string]string{"vm-1": "host1"}), + }), + vmUUID: "vm-1", + resKey: "/res-1", + resHost: "host2", + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{}), + makeReservation("res-2", "host3", map[string]string{"vm-1": "host1"}), + }), + }, + { + name: "remove one VM from reservation with multiple VMs", + initGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host3", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + }), + vmUUID: "vm-1", + resKey: "/res-1", + resHost: "host3", + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host3", map[string]string{"vm-2": "host2"}), + }), + }, + { + name: "remove VM from complex graph with 3 reservations and 4 VMs", + initGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host5", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host6", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + makeReservation("res-3", "host7", map[string]string{"vm-2": "host2", "vm-3": "host3", "vm-4": "host4"}), + }), + vmUUID: "vm-2", + resKey: "/res-3", + resHost: "host7", + expectGraph: makeGraph([]v1alpha1.Reservation{ + makeReservation("res-1", "host5", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host6", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + makeReservation("res-3", "host7", map[string]string{"vm-3": "host3", "vm-4": "host4"}), + }), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.initGraph.removeVMFromReservation(tc.vmUUID, tc.resKey, tc.resHost) + graphsEqual(t, tc.initGraph, tc.expectGraph) + }) + } +} + +// TestAddRemoveVMRoundTrip tests that adding and removing a VM leaves the graph unchanged. +func TestAddRemoveVMRoundTrip(t *testing.T) { + testCases := []struct { + name string + initialRes []v1alpha1.Reservation + vmUUID string + vmHypervisor string + resKey string + resHost string + }{ + { + name: "add and remove VM from existing reservation", + initialRes: []v1alpha1.Reservation{ + makeReservation("res-1", "host2", map[string]string{"vm-1": "host1"}), + }, + vmUUID: "vm-2", + vmHypervisor: "host3", + resKey: "/res-1", + resHost: "host2", + }, + { + name: "add and remove VM from empty graph", + initialRes: []v1alpha1.Reservation{}, + vmUUID: "vm-1", + vmHypervisor: "host1", + resKey: "/res-1", + resHost: "host2", + }, + { + name: "add and remove VM from complex graph with 3 reservations and 4 VMs", + initialRes: []v1alpha1.Reservation{ + makeReservation("res-1", "host5", map[string]string{"vm-1": "host1"}), + makeReservation("res-2", "host6", map[string]string{"vm-1": "host1", "vm-2": "host2"}), + makeReservation("res-3", "host7", map[string]string{"vm-2": "host2", "vm-3": "host3", "vm-4": "host4"}), + }, + vmUUID: "vm-5", + vmHypervisor: "host8", + resKey: "/res-2", + resHost: "host6", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + graph := newBaseDependencyGraph(tc.initialRes) + + // Capture initial state + initialVMCount := len(graph.vmToReservations) + initialResVMCount := len(graph.reservationToVMs[tc.resKey]) + + // Add VM + graph.addVMToReservation(tc.vmUUID, tc.vmHypervisor, tc.resKey, tc.resHost) + + // Verify VM was added + if !graph.vmToReservations[tc.vmUUID][tc.resKey] { + t.Errorf("VM %s not added to reservation %s", tc.vmUUID, tc.resKey) + } + + // Remove VM + graph.removeVMFromReservation(tc.vmUUID, tc.resKey, tc.resHost) + + // Verify VM was removed from reservation + if graph.vmToReservations[tc.vmUUID][tc.resKey] { + t.Errorf("VM %s still in reservation %s after removal", tc.vmUUID, tc.resKey) + } + + // Verify reservation VM count is back to initial (for existing reservations) + if len(tc.initialRes) > 0 { + if len(graph.reservationToVMs[tc.resKey]) != initialResVMCount { + t.Errorf("Reservation %s has %d VMs, expected %d", tc.resKey, len(graph.reservationToVMs[tc.resKey]), initialResVMCount) + } + } + + // Note: VM entry may still exist in vmToReservations with empty map + // This is expected behavior - we don't clean up empty VM entries + _ = initialVMCount + }) + } +} + // TestFindEligibleReservations tests the FindEligibleReservations function. func TestFindEligibleReservations(t *testing.T) { testCases := []struct { diff --git a/internal/scheduling/reservations/failover/reservation_scheduling.go b/internal/scheduling/reservations/failover/reservation_scheduling.go index 41ab34a14..873e5dbca 100644 --- a/internal/scheduling/reservations/failover/reservation_scheduling.go +++ b/internal/scheduling/reservations/failover/reservation_scheduling.go @@ -204,8 +204,11 @@ func (c *FailoverReservationController) validateVMViaSchedulerEvacuation( // Build a single-host request to validate the VM can use the reservation host // Use vm.CurrentHypervisor directly instead of a separate parameter to avoid stale data + // Set _nova_check_type to "evacuate" so that filter_has_enough_capacity unlocks + // the failover reservation's resources for this VM (avoiding self-blocking). + // Use the actual VM UUID (not prefixed) so the filter can match it in the reservation's allocations. scheduleReq := reservations.ScheduleReservationRequest{ - InstanceUUID: "validate-" + vm.UUID, + InstanceUUID: vm.UUID, ProjectID: vm.ProjectID, FlavorName: vm.FlavorName, FlavorExtraSpecs: flavorExtraSpecs, @@ -215,6 +218,7 @@ func (c *FailoverReservationController) validateVMViaSchedulerEvacuation( IgnoreHosts: []string{vm.CurrentHypervisor}, Pipeline: PipelineAcknowledgeFailoverReservation, AvailabilityZone: vm.AvailabilityZone, + SchedulerHints: map[string]any{"_nova_check_type": "evacuate"}, } logger.V(1).Info("validating VM via scheduler evacuation", @@ -250,11 +254,14 @@ func (c *FailoverReservationController) validateVMViaSchedulerEvacuation( // scheduleAndBuildNewFailoverReservation schedules a failover reservation for a VM. // Returns the built reservation (in-memory only, not persisted). // The caller is responsible for persisting the reservation to the cluster. +// excludeHypervisors is a set of hypervisors to exclude from consideration (e.g., hypervisors +// that already had a new reservation created in this reconcile cycle). func (c *FailoverReservationController) scheduleAndBuildNewFailoverReservation( ctx context.Context, vm VM, allHypervisors []string, failoverReservations []v1alpha1.Reservation, + excludeHypervisors map[string]bool, ) (*v1alpha1.Reservation, error) { logger := LoggerFromContext(ctx) @@ -268,6 +275,12 @@ func (c *FailoverReservationController) scheduleAndBuildNewFailoverReservation( // Iterate through scheduler-returned hypervisors to find one that passes eligibility constraints var selectedHypervisor string for _, candidateHypervisor := range validHypervisors { + // Skip hypervisors that already had a new reservation created in this reconcile cycle + if c.Config.LimitOneNewReservationPerHypervisor && excludeHypervisors[candidateHypervisor] { + logger.V(1).Info("skipping hypervisor (already has new reservation this cycle)", + "vmUUID", vm.UUID, "hypervisor", candidateHypervisor) + continue + } // Check if the VM can create a new reservation on this hypervisor hypotheticalRes := v1alpha1.Reservation{ Status: v1alpha1.ReservationStatus{ diff --git a/internal/scheduling/reservations/scheduler_client.go b/internal/scheduling/reservations/scheduler_client.go index 618cec04a..91d62ef74 100644 --- a/internal/scheduling/reservations/scheduler_client.go +++ b/internal/scheduling/reservations/scheduler_client.go @@ -75,6 +75,9 @@ type ScheduleReservationRequest struct { // AvailabilityZone is the availability zone to schedule in. // This is used by the filter_correct_az filter to ensure hosts are in the correct AZ. AvailabilityZone string + // SchedulerHints are hints passed to the scheduler pipeline. + // Used to set _nova_check_type for evacuation intent detection. + SchedulerHints map[string]any } // ScheduleReservationResponse contains the result of scheduling a reservation. @@ -124,7 +127,7 @@ func (c *SchedulerClient) ScheduleReservation(ctx context.Context, req ScheduleR ProjectID: req.ProjectID, AvailabilityZone: req.AvailabilityZone, IgnoreHosts: ignoreHosts, - SchedulerHints: make(map[string]any), // Initialize to empty map for consistent behavior + SchedulerHints: req.getSchedulerHints(), Flavor: api.NovaObject[api.NovaFlavor]{ Data: api.NovaFlavor{ Name: req.FlavorName, @@ -147,7 +150,8 @@ func (c *SchedulerClient) ScheduleReservation(ctx context.Context, req ScheduleR "memoryMB", req.MemoryMB, "vcpus", req.VCPUs, "eligibleHostsCount", len(req.EligibleHosts), - "ignoreHosts", req.IgnoreHosts) + "ignoreHosts", req.IgnoreHosts, + "isEvacuation", req.isEvacuation()) // Marshal the request reqBody, err := json.Marshal(externalSchedulerRequest) @@ -164,8 +168,10 @@ func (c *SchedulerClient) ScheduleReservation(ctx context.Context, req ScheduleR } httpReq.Header.Set("Content-Type", "application/json") - // Send the request + // Send the request and measure duration + startTime := time.Now() response, err := c.HTTPClient.Do(httpReq) + duration := time.Since(startTime) if err != nil { logger.Error(err, "failed to send external scheduler request") return nil, fmt.Errorf("failed to send external scheduler request: %w", err) @@ -185,9 +191,27 @@ func (c *SchedulerClient) ScheduleReservation(ctx context.Context, req ScheduleR return nil, fmt.Errorf("failed to decode external scheduler response: %w", err) } - logger.V(1).Info("received external scheduler response", "hostsCount", len(externalSchedulerResponse.Hosts)) + logger.V(1).Info("received external scheduler response", + "hostsCount", len(externalSchedulerResponse.Hosts), + "duration", duration.Round(time.Millisecond)) return &ScheduleReservationResponse{ Hosts: externalSchedulerResponse.Hosts, }, nil } + +// getSchedulerHints returns the scheduler hints, or an empty map if nil. +func (req ScheduleReservationRequest) getSchedulerHints() map[string]any { + if req.SchedulerHints == nil { + return make(map[string]any) + } + return req.SchedulerHints +} + +// isEvacuation returns true if the request has the evacuation intent hint set. +func (req ScheduleReservationRequest) isEvacuation() bool { + if req.SchedulerHints == nil { + return false + } + return req.SchedulerHints["_nova_check_type"] == "evacuate" +}