diff --git a/docs/reservations/committed-resource-reservations.md b/docs/reservations/committed-resource-reservations.md index e94b922a5..6917c6438 100644 --- a/docs/reservations/committed-resource-reservations.md +++ b/docs/reservations/committed-resource-reservations.md @@ -83,19 +83,22 @@ VM allocations are tracked within reservations: flowchart LR subgraph State Res[(Reservation CRDs)] + HV[(Hypervisor CRDs)] end A[Nova Scheduler] -->|VM Create/Migrate/Resize| B[Scheduling Pipeline] B -->|update Spec.Allocations| Res - Res -->|watch| Controller - Res -->|periodic reconcile| Controller - Controller -->|update Spec/Status.Allocations| Res + Res -->|watch| C[Controller] + HV -->|watch - instance changes| C + Res -->|periodic safety-net requeue| C + C -->|update Spec/Status.Allocations| Res ``` | Component | Event | Timing | Action | |-----------|-------|--------|--------| | **Scheduling Pipeline** | VM Create, Migrate, Resize | Immediate | Add VM to `Spec.Allocations` | -| **Controller** | Reservation CRD updated | `committedResourceRequeueIntervalGracePeriod` (default: 1 min) | Verify new VMs via Nova API; update `Status.Allocations` | -| **Controller** | Periodic check | `committedResourceRequeueIntervalActive` (default: 5 min) | Verify established VMs via Hypervisor CRD; remove gone VMs from `Spec.Allocations` | +| **Controller** | Reservation CRD updated | `committedResourceRequeueIntervalGracePeriod` (default: 1 min) | Defer verification for new VMs still spawning; update `Status.Allocations` | +| **Controller** | Hypervisor CRD updated (VM appeared/disappeared) | Immediate (event-driven) | Verify allocations via Hypervisor CRD; remove gone VMs from `Spec.Allocations` | +| **Controller** | Periodic safety-net | `committedResourceRequeueIntervalActive` (default: 5 min) | Same as above; catches any missed events | **Allocation fields**: - `Spec.Allocations` — Expected VMs (written by the scheduling pipeline on placement) @@ -103,26 +106,20 @@ flowchart LR **VM allocation state diagram**: -The controller uses two sources to verify VM allocations, depending on how recently the VM was placed: -- **Nova API** — used during the grace period (`committedResourceAllocationGracePeriod`, default: 15 min) where the VM may still be starting up; provides real-time host assignment -- **Hypervisor CRD** — used for established allocations; reflects the set of instances the hypervisor operator observes on the host +The controller uses the **Hypervisor CRD** as the sole source of truth for VM allocation verification: +- **Hypervisor CRD** — used for all allocation checks; reflects the set of instances the hypervisor operator observes on the host ```mermaid stateDiagram-v2 direction LR - [*] --> SpecOnly : placement (create, migrate, resize) - SpecOnly --> Confirmed : on expected host - SpecOnly --> WrongHost : on different host - SpecOnly --> [*] : not confirmed after grace period - Confirmed --> WrongHost : not on HV CRD, found elsewhere - Confirmed --> [*] : not on HV CRD, Nova 404 - WrongHost --> Confirmed : back on expected host - WrongHost --> [*] : VM gone (404) - WrongHost --> [*] : on wrong host > grace period - state "Spec only (grace period)" as SpecOnly state "Spec + Status (on expected host)" as Confirmed - state "Spec + Status (host mismatch)" as WrongHost + + [*] --> SpecOnly : placement (create, migrate, resize) + SpecOnly --> SpecOnly : within grace period + SpecOnly --> Confirmed : found on HV CRD after grace period + SpecOnly --> [*] : not on HV CRD after grace period + Confirmed --> [*] : not on HV CRD ``` **Note**: VM allocations may not consume all resources of a reservation slot. A reservation with 128 GB may have VMs totaling only 96 GB if that fits the project's needs. Allocations may exceed reservation capacity (e.g., after VM resize). @@ -185,10 +182,12 @@ The controller watches Reservation CRDs and performs two types of reconciliation **Placement** - Finds hosts for new reservations (calls scheduler API) -**Allocation Verification** - Tracks VM lifecycle on reservations. VMs take time to appear on a host after scheduling, so new allocations are verified more frequently via the Nova API for real-time status, while established allocations are verified via the Hypervisor CRD: -- New VMs (within `committedResourceAllocationGracePeriod`, default: 15 min): checked via Nova API every `committedResourceRequeueIntervalGracePeriod` (default: 1 min) -- Established VMs: checked via Hypervisor CRD every `committedResourceRequeueIntervalActive` (default: 5 min) -- Missing VMs: removed from `Spec.Allocations` after Nova API confirms 404 +**Allocation Verification** - Tracks VM lifecycle on reservations. The controller uses the Hypervisor CRD as the sole source of truth, with two triggers: +- New VMs (within `committedResourceAllocationGracePeriod`, default: 15 min): verification deferred — VM may still be spawning; requeued every `committedResourceRequeueIntervalGracePeriod` (default: 1 min) +- Established VMs: verified reactively when the Hypervisor CRD changes (VM appeared or disappeared in `Status.Instances`), with `committedResourceRequeueIntervalActive` (default: 5 min) as a safety-net fallback +- Missing VMs: removed from `Spec.Allocations` when not found on the Hypervisor CRD after the grace period + +**Reservation migration is not supported yet.** ### Usage API diff --git a/internal/scheduling/reservations/commitments/config.go b/internal/scheduling/reservations/commitments/config.go index 17d89c8c0..937449e34 100644 --- a/internal/scheduling/reservations/commitments/config.go +++ b/internal/scheduling/reservations/commitments/config.go @@ -28,12 +28,6 @@ type Config struct { // SchedulerURL is the endpoint of the nova external scheduler SchedulerURL string `json:"schedulerURL"` - // Secret ref to SSO credentials stored in a k8s secret, if applicable. - SSOSecretRef *corev1.SecretReference `json:"ssoSecretRef"` - - // Secret ref to keystone credentials stored in a k8s secret. - KeystoneSecretRef corev1.SecretReference `json:"keystoneSecretRef"` - // Secret ref to the database credentials for querying VM state. DatabaseSecretRef *corev1.SecretReference `json:"databaseSecretRef,omitempty"` diff --git a/internal/scheduling/reservations/commitments/controller.go b/internal/scheduling/reservations/commitments/controller.go index a3aef918c..09cb00bda 100644 --- a/internal/scheduling/reservations/commitments/controller.go +++ b/internal/scheduling/reservations/commitments/controller.go @@ -12,6 +12,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -20,14 +21,12 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/predicate" - - novaservers "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" + "sigs.k8s.io/controller-runtime/pkg/reconcile" schedulerdelegationapi "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/db" "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" - schedulingnova "github.com/cobaltcore-dev/cortex/internal/scheduling/nova" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" "github.com/cobaltcore-dev/cortex/pkg/multicluster" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" @@ -46,8 +45,6 @@ type CommitmentReservationController struct { DB *db.DB // SchedulerClient for making scheduler API calls. SchedulerClient *reservations.SchedulerClient - // NovaClient for direct Nova API calls (real-time VM status). - NovaClient schedulingnova.NovaClient } // Reconcile is part of the main kubernetes reconciliation loop which aims to @@ -337,10 +334,13 @@ type reconcileAllocationsResult struct { HasAllocationsInGracePeriod bool } -// reconcileAllocations verifies all allocations in Spec against actual VM state. -// It updates Status.Allocations based on the actual host location of each VM. -// For new allocations (within grace period), it uses the Nova API for real-time status. -// For older allocations, it uses the Hypervisor CRD to check if VM is on the expected host. +// reconcileAllocations verifies all allocations in Spec against actual VM state using the +// Hypervisor CRD as the sole source of truth. +// +// For new allocations (within grace period): the VM may not yet appear in the HV CRD +// (still spawning), so we skip verification and requeue with a short interval. +// For older allocations: we check the HV CRD; VMs not found are considered leaving and +// removed from the reservation. func (r *CommitmentReservationController) reconcileAllocations(ctx context.Context, res *v1alpha1.Reservation) (*reconcileAllocationsResult, error) { logger := LoggerFromContext(ctx).WithValues("component", "controller") result := &reconcileAllocationsResult{} @@ -359,7 +359,7 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte expectedHost := res.Status.Host - // Fetch the Hypervisor CRD for the expected host (for older allocations) + // Fetch the Hypervisor CRD for the expected host. var hypervisor hv1.Hypervisor hvInstanceSet := make(map[string]bool) if expectedHost != "" { @@ -367,11 +367,11 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte if client.IgnoreNotFound(err) != nil { return nil, fmt.Errorf("failed to get hypervisor %s: %w", expectedHost, err) } - // Hypervisor not found - all older allocations will be checked via Nova API fallback + // Hypervisor not found — treat all post-grace-period VMs as stale. logger.Info("hypervisor CRD not found", "host", expectedHost) } else { - // Build set of all VM UUIDs on this hypervisor for O(1) lookup - // Include both active and inactive VMs - stopped/shelved VMs still consume the reservation slot + // Build set of all VM UUIDs on this hypervisor for O(1) lookup. + // Include both active and inactive VMs — stopped/shelved VMs still hold the slot. for _, inst := range hypervisor.Status.Instances { hvInstanceSet[inst.ID] = true } @@ -384,9 +384,9 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte res.Status.CommittedResourceReservation = &v1alpha1.CommittedResourceReservationStatus{} } - // Build new Status.Allocations map based on actual VM locations + // Build new Status.Allocations map based on HV CRD state. newStatusAllocations := make(map[string]string) - // Track allocations to remove from Spec (stale/leaving VMs) + // Track allocations to remove from Spec (stale/leaving VMs). var allocationsToRemove []string for vmUUID, allocation := range res.Spec.CommittedResourceReservation.Allocations { @@ -394,105 +394,29 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte isInGracePeriod := allocationAge < r.Conf.AllocationGracePeriod if isInGracePeriod { - // New allocation: use Nova API for real-time status + // New allocation: VM may not yet appear in the HV CRD (still spawning). + // Signal to requeue with the short grace-period interval; skip verification. result.HasAllocationsInGracePeriod = true + logger.V(1).Info("allocation in grace period, deferring verification", + "vm", vmUUID, + "allocationAge", allocationAge) + continue + } - if r.NovaClient == nil { - // No Nova client - skip verification for now, retry later - logger.V(1).Info("Nova client not available, skipping new allocation verification", - "vm", vmUUID, - "allocationAge", allocationAge) - continue - } - - server, err := r.NovaClient.Get(ctx, vmUUID) - if err != nil { - // VM not yet available in Nova (still spawning) - retry on next reconcile - logger.V(1).Info("VM not yet available in Nova API", - "vm", vmUUID, - "error", err.Error(), - "allocationAge", allocationAge) - // Keep in Spec, don't add to Status - will retry on next reconcile - continue - } - - actualHost := server.ComputeHost - switch { - case actualHost == expectedHost: - // VM is on expected host - confirmed running - newStatusAllocations[vmUUID] = actualHost - logger.V(1).Info("verified new VM allocation via Nova API", - "vm", vmUUID, - "actualHost", actualHost, - "allocationAge", allocationAge) - case actualHost != "": - // VM is on different host - migration scenario (log for now) - newStatusAllocations[vmUUID] = actualHost - logger.Info("VM on different host than expected (migration?)", - "vm", vmUUID, - "actualHost", actualHost, - "expectedHost", expectedHost, - "allocationAge", allocationAge) - default: - // VM not yet on any host - still spawning - logger.V(1).Info("VM not yet on host (spawning)", - "vm", vmUUID, - "status", server.Status, - "allocationAge", allocationAge) - // Keep in Spec, don't add to Status - will retry on next reconcile - } + // Post-grace-period: use HV CRD as authoritative source. + if hvInstanceSet[vmUUID] { + newStatusAllocations[vmUUID] = expectedHost + logger.V(1).Info("verified VM allocation via Hypervisor CRD", + "vm", vmUUID, + "host", expectedHost) } else { - // Older allocation: use Hypervisor CRD for verification - if hvInstanceSet[vmUUID] { - // VM found on expected hypervisor - confirmed running - newStatusAllocations[vmUUID] = expectedHost - logger.V(1).Info("verified VM allocation via Hypervisor CRD", - "vm", vmUUID, - "host", expectedHost) - } else { - // VM not found on expected hypervisor - check Nova API as fallback - if r.NovaClient != nil { - novaServer, novaErr := r.NovaClient.Get(ctx, vmUUID) - switch { - case novaErr == nil && novaServer.ComputeHost == expectedHost: - // VM is confirmed on the expected host via Nova - mark as running - newStatusAllocations[vmUUID] = expectedHost - logger.V(1).Info("verified VM allocation via Nova API", - "vm", vmUUID, - "host", expectedHost) - continue - case novaErr == nil && novaServer.ComputeHost != "": - // VM is on a different host past the grace period - remove from this reservation. - // The grace period is the window for VMs in transit; past that, a VM on the - // wrong host is no longer consuming this slot. - logger.Info("VM on different host past grace period, removing from reservation", - "vm", vmUUID, - "actualHost", novaServer.ComputeHost, - "expectedHost", expectedHost, - "allocationAge", allocationAge) - // fall through to removal - case novaErr == nil: - // VM has no host yet - fall through to removal - logger.V(1).Info("Nova API confirmed VM has no host", "vm", vmUUID) - default: - var notFound *novaservers.ErrServerNotFound - if !errors.As(novaErr, ¬Found) { - // Transient Nova API error - skip removal to avoid evicting live VMs - return nil, fmt.Errorf("nova API error checking VM %s: %w", vmUUID, novaErr) - } - // 404 - VM gone, fall through to removal - logger.V(1).Info("Nova API confirmed VM not found", "vm", vmUUID) - } - } - // VM not found on hypervisor and not in Nova - mark for removal (leaving VM) - allocationsToRemove = append(allocationsToRemove, vmUUID) - logger.Info("removing stale allocation (VM not found on hypervisor or Nova)", - "vm", vmUUID, - "reservation", res.Name, - "expectedHost", expectedHost, - "allocationAge", allocationAge, - "gracePeriod", r.Conf.AllocationGracePeriod) - } + allocationsToRemove = append(allocationsToRemove, vmUUID) + logger.Info("removing stale allocation (VM not found on hypervisor)", + "vm", vmUUID, + "reservation", res.Name, + "expectedHost", expectedHost, + "allocationAge", allocationAge, + "gracePeriod", r.Conf.AllocationGracePeriod) } } @@ -565,6 +489,31 @@ func (r *CommitmentReservationController) getPipelineForFlavorGroup(flavorGroupN return r.Conf.PipelineDefault } +// hypervisorToReservations maps a Hypervisor change event to the set of CR reservations +// assigned to that host. Used as the event handler for the Hypervisor CRD watch so that +// when the hypervisor operator updates Status.Instances, affected reservations are +// immediately enqueued for reconciliation. +func (r *CommitmentReservationController) hypervisorToReservations(ctx context.Context, obj client.Object) []reconcile.Request { + hvName := obj.GetName() + var reservationList v1alpha1.ReservationList + if err := r.List(ctx, &reservationList, + client.MatchingFields{indexReservationByStatusHost: hvName}, + ); err != nil { + logf.FromContext(ctx).Error(err, "failed to list reservations for hypervisor", "hypervisor", hvName) + return nil + } + requests := make([]reconcile.Request, 0, len(reservationList.Items)) + for _, res := range reservationList.Items { + if res.Spec.Type != v1alpha1.ReservationTypeCommittedResource { + continue + } + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{Name: res.Name, Namespace: res.Namespace}, + }) + } + return requests +} + // Init initializes the reconciler with required clients and DB connection. func (r *CommitmentReservationController) Init(ctx context.Context, client client.Client, conf Config) error { // Initialize database connection if DatabaseSecretRef is provided. @@ -581,19 +530,6 @@ func (r *CommitmentReservationController) Init(ctx context.Context, client clien r.SchedulerClient = reservations.NewSchedulerClient(conf.SchedulerURL) logf.FromContext(ctx).Info("scheduler client initialized for commitment reservation controller", "url", conf.SchedulerURL) - // Initialize Nova client for real-time VM status checks (optional). - // Skip if NovaClient is already set (e.g., injected for testing) or if keystone not configured. - if r.NovaClient == nil && conf.KeystoneSecretRef.Name != "" { - r.NovaClient = schedulingnova.NewNovaClient() - if err := r.NovaClient.Init(ctx, client, schedulingnova.NovaClientConfig{ - KeystoneSecretRef: conf.KeystoneSecretRef, - SSOSecretRef: conf.SSOSecretRef, - }); err != nil { - return fmt.Errorf("failed to initialize Nova client: %w", err) - } - logf.FromContext(ctx).Info("Nova client initialized for commitment reservation controller") - } - return nil } @@ -631,6 +567,10 @@ var commitmentReservationPredicate = predicate.Funcs{ }, } +// indexReservationByStatusHost is the field index key for Reservation.Status.Host. +// Used by the Hypervisor→Reservation mapper to efficiently look up reservations on a given host. +const indexReservationByStatusHost = ".status.host" + // SetupWithManager sets up the controller with the Manager. func (r *CommitmentReservationController) SetupWithManager(mgr ctrl.Manager, mcl *multicluster.Client) error { if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error { @@ -642,6 +582,22 @@ func (r *CommitmentReservationController) SetupWithManager(mgr ctrl.Manager, mcl return err } + // Index reservations by Status.Host so the HV mapper can do O(1) lookups. + if err := mgr.GetFieldIndexer().IndexField( + context.Background(), + &v1alpha1.Reservation{}, + indexReservationByStatusHost, + func(obj client.Object) []string { + res := obj.(*v1alpha1.Reservation) + if res.Status.Host == "" { + return nil + } + return []string{res.Status.Host} + }, + ); err != nil { + return fmt.Errorf("failed to index reservations by status host: %w", err) + } + // Use WatchesMulticluster to watch Reservations across all configured clusters // (home + remotes). This is required because Reservation CRDs may be stored // in remote clusters, not just the home cluster. Without this, the controller @@ -656,6 +612,19 @@ func (r *CommitmentReservationController) SetupWithManager(mgr ctrl.Manager, mcl return err } + // Watch Hypervisor CRDs reactively: when the hypervisor operator updates + // Status.Instances (VM appeared or disappeared), enqueue all reservations + // assigned to that host. This replaces periodic polling for the established-VM + // verification path — changes are detected in seconds rather than up to + // RequeueIntervalActive. RequeueIntervalActive remains as a safety-net fallback. + bldr, err = bldr.WatchesMulticluster( + &hv1.Hypervisor{}, + handler.EnqueueRequestsFromMapFunc(r.hypervisorToReservations), + ) + if err != nil { + return err + } + return bldr.Named("commitment-reservation"). WithOptions(controller.Options{ // We want to process reservations one at a time to avoid overbooking. diff --git a/internal/scheduling/reservations/commitments/controller_test.go b/internal/scheduling/reservations/commitments/controller_test.go index 68d13faf3..69434c656 100644 --- a/internal/scheduling/reservations/commitments/controller_test.go +++ b/internal/scheduling/reservations/commitments/controller_test.go @@ -145,11 +145,6 @@ func TestCommitmentReservationController_Reconcile(t *testing.T) { // Test: reconcileAllocations // ============================================================================ -// Note: Full reconcileAllocations tests require mocking NovaClient, which uses -// unexported types (nova.server, nova.migration). Tests for the Nova API path -// would need to be placed in the nova package or the types would need to be exported. -// For now, we test only the Hypervisor CRD path (when NovaClient is nil). - func TestReconcileAllocations_HypervisorCRDPath(t *testing.T) { scheme := runtime.NewScheme() if err := v1alpha1.AddToScheme(scheme); err != nil { @@ -169,6 +164,7 @@ func TestReconcileAllocations_HypervisorCRDPath(t *testing.T) { hypervisor *hv1.Hypervisor config Config expectedStatusAllocations map[string]string + expectedSpecAllocations []string // VM UUIDs expected to remain in spec; nil means no check expectedHasGracePeriodAllocs bool }{ { @@ -181,6 +177,7 @@ func TestReconcileAllocations_HypervisorCRDPath(t *testing.T) { }), config: Config{AllocationGracePeriod: 15 * time.Minute}, expectedStatusAllocations: map[string]string{"vm-1": "host-1"}, + expectedSpecAllocations: []string{"vm-1"}, expectedHasGracePeriodAllocs: false, }, { @@ -193,26 +190,29 @@ func TestReconcileAllocations_HypervisorCRDPath(t *testing.T) { }), config: Config{AllocationGracePeriod: 15 * time.Minute}, expectedStatusAllocations: map[string]string{"vm-stopped": "host-1"}, + expectedSpecAllocations: []string{"vm-stopped"}, expectedHasGracePeriodAllocs: false, }, { - name: "old allocation - VM not on hypervisor CRD (no NovaClient fallback)", + name: "old allocation - VM not on hypervisor CRD (stale, removed)", reservation: newTestCRReservation(map[string]metav1.Time{ "vm-1": oldTime, }), hypervisor: newTestHypervisorCRD("host-1", []hv1.Instance{}), // Empty config: Config{AllocationGracePeriod: 15 * time.Minute}, - expectedStatusAllocations: map[string]string{}, // Not confirmed + expectedStatusAllocations: map[string]string{}, + expectedSpecAllocations: []string{}, // Removed from spec expectedHasGracePeriodAllocs: false, }, { - name: "new allocation within grace period - no Nova client", + name: "new allocation within grace period - deferred to requeue", reservation: newTestCRReservation(map[string]metav1.Time{ "vm-1": recentTime, }), hypervisor: nil, config: Config{AllocationGracePeriod: 15 * time.Minute}, - expectedStatusAllocations: map[string]string{}, // Can't verify without Nova + expectedStatusAllocations: map[string]string{}, + expectedSpecAllocations: []string{"vm-1"}, // Kept in spec during grace period expectedHasGracePeriodAllocs: true, }, { @@ -226,6 +226,7 @@ func TestReconcileAllocations_HypervisorCRDPath(t *testing.T) { }), config: Config{AllocationGracePeriod: 15 * time.Minute}, expectedStatusAllocations: map[string]string{"vm-old": "host-1"}, // Only old one confirmed via CRD + expectedSpecAllocations: []string{"vm-new", "vm-old"}, expectedHasGracePeriodAllocs: true, }, { @@ -236,6 +237,28 @@ func TestReconcileAllocations_HypervisorCRDPath(t *testing.T) { expectedStatusAllocations: map[string]string{}, expectedHasGracePeriodAllocs: false, }, + { + name: "hypervisor CRD not found - post-grace VM removed", + reservation: newTestCRReservation(map[string]metav1.Time{ + "vm-1": oldTime, + }), + hypervisor: nil, // HV CRD does not exist (e.g. host deleted) + config: Config{AllocationGracePeriod: 15 * time.Minute}, + expectedStatusAllocations: map[string]string{}, + expectedSpecAllocations: []string{}, // Removed from spec + expectedHasGracePeriodAllocs: false, + }, + { + name: "hypervisor CRD not found - grace period VM kept", + reservation: newTestCRReservation(map[string]metav1.Time{ + "vm-1": recentTime, + }), + hypervisor: nil, // HV CRD does not exist + config: Config{AllocationGracePeriod: 15 * time.Minute}, + expectedStatusAllocations: map[string]string{}, + expectedSpecAllocations: []string{"vm-1"}, // Kept during grace period + expectedHasGracePeriodAllocs: true, + }, } for _, tt := range tests { @@ -253,10 +276,9 @@ func TestReconcileAllocations_HypervisorCRDPath(t *testing.T) { Build() controller := &CommitmentReservationController{ - Client: k8sClient, - Scheme: scheme, - Conf: tt.config, - NovaClient: nil, // No NovaClient - testing Hypervisor CRD path only + Client: k8sClient, + Scheme: scheme, + Conf: tt.config, } ctx := WithNewGlobalRequestID(context.Background()) @@ -295,6 +317,25 @@ func TestReconcileAllocations_HypervisorCRDPath(t *testing.T) { t.Errorf("VM %s: expected host %s, got %s", vmUUID, expectedHost, actualHost) } } + + // Check spec allocations if expected set is specified + if tt.expectedSpecAllocations != nil { + specAllocs := map[string]bool{} + if updated.Spec.CommittedResourceReservation != nil { + for vmUUID := range updated.Spec.CommittedResourceReservation.Allocations { + specAllocs[vmUUID] = true + } + } + if len(specAllocs) != len(tt.expectedSpecAllocations) { + t.Errorf("expected %d spec allocations, got %d: %v", + len(tt.expectedSpecAllocations), len(specAllocs), specAllocs) + } + for _, vmUUID := range tt.expectedSpecAllocations { + if !specAllocs[vmUUID] { + t.Errorf("expected VM %s in spec allocations", vmUUID) + } + } + } }) } } @@ -356,6 +397,78 @@ func newTestHypervisorCRD(name string, instances []hv1.Instance) *hv1.Hypervisor } } +// ============================================================================ +// Test: hypervisorToReservations mapper +// ============================================================================ + +// TestHypervisorToReservations tests the mapper that translates a Hypervisor change +// into reconcile requests for the CR reservations assigned to that host. +// This covers the mapper logic; the watch wiring itself (informer → mapper → enqueue) +// is controller-runtime's responsibility and is not unit-testable without envtest. +func TestHypervisorToReservations(t *testing.T) { + scheme := runtime.NewScheme() + if err := v1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add scheme: %v", err) + } + + res1 := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "res-host-1"}, + Spec: v1alpha1.ReservationSpec{Type: v1alpha1.ReservationTypeCommittedResource}, + Status: v1alpha1.ReservationStatus{Host: "host-1"}, + } + res2 := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "res-host-1b"}, + Spec: v1alpha1.ReservationSpec{Type: v1alpha1.ReservationTypeCommittedResource}, + Status: v1alpha1.ReservationStatus{Host: "host-1"}, + } + resOtherHost := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "res-host-2"}, + Spec: v1alpha1.ReservationSpec{Type: v1alpha1.ReservationTypeCommittedResource}, + Status: v1alpha1.ReservationStatus{Host: "host-2"}, + } + resNoHost := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "res-no-host"}, + Spec: v1alpha1.ReservationSpec{Type: v1alpha1.ReservationTypeCommittedResource}, + } + resFailover := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "res-failover"}, + Spec: v1alpha1.ReservationSpec{Type: v1alpha1.ReservationTypeFailover}, + Status: v1alpha1.ReservationStatus{Host: "host-1"}, + } + + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(res1, res2, resOtherHost, resNoHost, resFailover). + WithStatusSubresource(&v1alpha1.Reservation{}). + WithIndex(&v1alpha1.Reservation{}, indexReservationByStatusHost, func(obj client.Object) []string { + res := obj.(*v1alpha1.Reservation) + if res.Status.Host == "" { + return nil + } + return []string{res.Status.Host} + }). + Build() + + controller := &CommitmentReservationController{Client: k8sClient} + + hv := &hv1.Hypervisor{ObjectMeta: metav1.ObjectMeta{Name: "host-1"}} + requests := controller.hypervisorToReservations(context.Background(), hv) + + // Only CR reservations on host-1 should be enqueued; failover and other-host excluded + got := make(map[string]bool, len(requests)) + for _, req := range requests { + got[req.Name] = true + } + if len(got) != 2 { + t.Errorf("expected 2 requests, got %d: %v", len(got), got) + } + for _, name := range []string{"res-host-1", "res-host-1b"} { + if !got[name] { + t.Errorf("expected %s in requests", name) + } + } +} + // ============================================================================ // Test: reconcileInstanceReservation_Success (existing test) // ============================================================================