From c8c2ef1971022be2c16fe05dfdd3e943ae1b5785 Mon Sep 17 00:00:00 2001 From: Julius Clausnitzer Date: Tue, 9 Jun 2026 15:49:02 +0200 Subject: [PATCH 1/3] feat: bypass grace period for confirmed VM departure (#410) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirmed VMs (present in Status.Allocations) now bypass the grace period check when they disappear from the HV CRD. Their departure is authoritative — Spec.Allocations and Status.Allocations are cleared in the same reconcile pass triggered by the existing HV watch. Previously the grace period gate applied to all VMs regardless of confirmation state, meaning a confirmed VM's departure could be delayed up to 15 minutes. --- .../commitments/reservation_controller.go | 11 ++- .../reservation_controller_test.go | 76 +++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/internal/scheduling/reservations/commitments/reservation_controller.go b/internal/scheduling/reservations/commitments/reservation_controller.go index f75b152d2..f6c5761c3 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller.go +++ b/internal/scheduling/reservations/commitments/reservation_controller.go @@ -411,9 +411,12 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte allocationAge := now.Sub(allocation.CreationTimestamp.Time) isInGracePeriod := allocationAge < r.Conf.AllocationGracePeriod.Duration - if isInGracePeriod { - // New allocation: VM may not yet appear in the HV CRD (still spawning). - // Signal to requeue with the short grace-period interval; skip verification. + // Confirmed VMs (already in Status.Allocations) bypass the grace period: + // their departure from the HV CRD is authoritative and must be acted on immediately. + // Unconfirmed VMs still within the grace period may not yet appear in the HV CRD + // (still spawning), so defer verification and requeue with a short interval. + isConfirmed := existingStatusAllocations[vmUUID] != "" + if !isConfirmed && isInGracePeriod { result.HasAllocationsInGracePeriod = true logger.V(1).Info("allocation in grace period, deferring verification", "vm", vmUUID, @@ -421,7 +424,7 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte continue } - // Post-grace-period: use HV CRD as authoritative source. + // Post-grace-period or confirmed VM: use HV CRD as authoritative source. if hvInstanceSet[vmUUID] { newStatusAllocations[vmUUID] = expectedHost logger.V(1).Info("verified VM allocation via Hypervisor CRD", diff --git a/internal/scheduling/reservations/commitments/reservation_controller_test.go b/internal/scheduling/reservations/commitments/reservation_controller_test.go index 8c5817893..8423e4f8a 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller_test.go +++ b/internal/scheduling/reservations/commitments/reservation_controller_test.go @@ -305,6 +305,82 @@ func TestReconcileAllocations_HypervisorCRDPath(t *testing.T) { } } +// TestReconcileAllocations_ConfirmedVMDeparture verifies that a VM already confirmed in +// Status.Allocations is removed immediately when it disappears from the HV CRD, without +// waiting for the grace period to expire. +func TestReconcileAllocations_ConfirmedVMDeparture(t *testing.T) { + scheme := newCRTestScheme(t) + config := ReservationControllerConfig{AllocationGracePeriod: metav1.Duration{Duration: 15 * time.Minute}} + now := time.Now() + + // VM was written to Spec.Allocations very recently (within grace period) but was already + // confirmed (present in Status.Allocations). It has since disappeared from the HV CRD. + recentTime := metav1.NewTime(now.Add(-2 * time.Minute)) + + res := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "test-reservation"}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + TargetHost: "host-1", + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{ + ProjectID: "test-project", + ResourceName: "test-flavor", + Allocations: map[string]v1alpha1.CommittedResourceAllocation{ + "vm-confirmed": { + CreationTimestamp: recentTime, + Resources: map[hv1.ResourceName]resource.Quantity{ + "memory": resource.MustParse("4Gi"), + }, + }, + }, + }, + }, + Status: v1alpha1.ReservationStatus{ + Host: "host-1", + Conditions: []metav1.Condition{ + {Type: v1alpha1.ReservationConditionReady, Status: metav1.ConditionTrue, Reason: "ReservationActive"}, + }, + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ + // VM was previously confirmed — it is in Status.Allocations. + Allocations: map[string]string{"vm-confirmed": "host-1"}, + }, + }, + } + + // HV CRD exists but the VM is gone — simulates termination/evacuation. + hv := newTestHypervisorCRD("host-1", []hv1.Instance{}) + + k8sClient := newCRTestClient(scheme, res, hv) + controller := &CommitmentReservationController{Client: k8sClient, Scheme: scheme, Conf: config} + + ctx := WithNewGlobalRequestID(context.Background()) + result, err := controller.reconcileAllocations(ctx, res) + if err != nil { + t.Fatalf("reconcileAllocations() error = %v", err) + } + + // Must not be treated as grace-period — departure of confirmed VM is immediate. + if result.HasAllocationsInGracePeriod { + t.Error("confirmed VM departure must not trigger grace period requeue") + } + + var updated v1alpha1.Reservation + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(res), &updated); err != nil { + t.Fatalf("get updated reservation: %v", err) + } + + // Spec.Allocations must be empty — VM removed. + if n := len(updated.Spec.CommittedResourceReservation.Allocations); n != 0 { + t.Errorf("expected 0 spec allocations after departure, got %d", n) + } + // Status.Allocations must also be empty — updated in the same reconcile pass. + if updated.Status.CommittedResourceReservation == nil || + len(updated.Status.CommittedResourceReservation.Allocations) != 0 { + t.Errorf("expected 0 status allocations after departure, got %v", + updated.Status.CommittedResourceReservation.Allocations) + } +} + // newTestCRReservation creates a test CR reservation with allocations on "host-1". func newTestCRReservation(allocations map[string]metav1.Time) *v1alpha1.Reservation { const host = "host-1" From a7a5643ee9bc2ca164a85c058a8dfd759867e931 Mon Sep 17 00:00:00 2001 From: Julius Clausnitzer Date: Tue, 16 Jun 2026 16:11:42 +0200 Subject: [PATCH 2/3] fix --- .../reservations/commitments/reservation_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/scheduling/reservations/commitments/reservation_controller.go b/internal/scheduling/reservations/commitments/reservation_controller.go index f6c5761c3..cb099676f 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller.go +++ b/internal/scheduling/reservations/commitments/reservation_controller.go @@ -415,7 +415,7 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte // their departure from the HV CRD is authoritative and must be acted on immediately. // Unconfirmed VMs still within the grace period may not yet appear in the HV CRD // (still spawning), so defer verification and requeue with a short interval. - isConfirmed := existingStatusAllocations[vmUUID] != "" + _, isConfirmed := existingStatusAllocations[vmUUID] if !isConfirmed && isInGracePeriod { result.HasAllocationsInGracePeriod = true logger.V(1).Info("allocation in grace period, deferring verification", From 5ff5cd9fbdc12ddb0313c64c20f7883029f8f7f1 Mon Sep 17 00:00:00 2001 From: Julius Clausnitzer Date: Tue, 16 Jun 2026 16:15:59 +0200 Subject: [PATCH 3/3] fix --- .../reservations/commitments/reservation_controller_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/scheduling/reservations/commitments/reservation_controller_test.go b/internal/scheduling/reservations/commitments/reservation_controller_test.go index 8423e4f8a..6ef9b0c66 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller_test.go +++ b/internal/scheduling/reservations/commitments/reservation_controller_test.go @@ -376,8 +376,8 @@ func TestReconcileAllocations_ConfirmedVMDeparture(t *testing.T) { // Status.Allocations must also be empty — updated in the same reconcile pass. if updated.Status.CommittedResourceReservation == nil || len(updated.Status.CommittedResourceReservation.Allocations) != 0 { - t.Errorf("expected 0 status allocations after departure, got %v", - updated.Status.CommittedResourceReservation.Allocations) + t.Errorf("expected 0 status allocations after departure, got %#v", + updated.Status.CommittedResourceReservation) } }