Skip to content

fix: capacity filter considers also reservations that are placed, but waiting for 2nd reconcile cycle for updating the status #880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mblos merged 1 commit into from
May 22, 2026
Merged

fix: capacity filter considers also reservations that are placed, but waiting for 2nd reconcile cycle for updating the status #880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 62 additions & 50 deletions internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,67 +107,79 @@ func (s *FilterHasEnoughCapacity) Run(traceLog *slog.Logger, request api.Externa
return nil, err
}
for _, reservation := range reservations.Items {
if !reservation.IsReady() {
continue // Only consider active reservations (Ready=True).
}

// Check if this reservation type should be ignored
// Check if this reservation type should be ignored — applies regardless of ready state.
if slices.Contains(s.Options.IgnoredReservationTypes, reservation.Spec.Type) {
traceLog.Debug("ignoring reservation type", "type", reservation.Spec.Type, "reservation", reservation.Name)
continue
}

// Handle reservation based on its type.
switch reservation.Spec.Type {
case v1alpha1.ReservationTypeCommittedResource, "": // Empty string for backward compatibility
// Skip if no CommittedResourceReservation spec or no resource group set.
if reservation.Spec.CommittedResourceReservation == nil || reservation.Spec.CommittedResourceReservation.ResourceGroup == "" {
continue // Not handled by us (no resource group set).
if !reservation.IsReady() {
if reservation.Spec.TargetHost == "" && reservation.Status.Host == "" {
continue // not placed yet, nothing to block
}
// TargetHost is set but Ready has not been synced yet: the reservation controller
// wrote the host in reconcile cycle 1 but hasn't completed cycle 2 (status sync).
// Block the full slot now to prevent a concurrent scheduling call from picking the
// same host. Unlock logic is intentionally skipped — an unconfirmed slot must not
// be unlocked for any VM or project.
traceLog.Warn("reservation has target host set but is not yet ready — blocking host to prevent double-booking",
"reservation", reservation.Name,
"targetHost", reservation.Spec.TargetHost,
"statusHost", reservation.Status.Host,
)
} else {
// Ready reservation: apply type-specific unlock logic.
switch reservation.Spec.Type {
case v1alpha1.ReservationTypeCommittedResource, "": // Empty string for backward compatibility
// Skip if no CommittedResourceReservation spec or no resource group set.
if reservation.Spec.CommittedResourceReservation == nil || reservation.Spec.CommittedResourceReservation.ResourceGroup == "" {
continue // Not handled by us (no resource group set).
}

// Check if this is a CR reservation scheduling request.
// If so, we should NOT unlock any CR reservations to prevent overbooking.
// CR capacity should only be unlocked for actual VM scheduling.
intent, err := request.GetIntent()
switch {
case err == nil && intent == api.ReserveForCommittedResourceIntent:
traceLog.Debug("keeping CR reservation locked for CR reservation scheduling",
"reservation", reservation.Name,
"intent", intent)
// Don't continue - fall through to block the resources
case !s.Options.LockReserved &&
// For committed resource reservations: unlock resources only if:
// 1. Project ID matches
// 2. ResourceGroup matches the flavor's hw_version
reservation.Spec.CommittedResourceReservation.ProjectID == request.Spec.Data.ProjectID &&
reservation.Spec.CommittedResourceReservation.ResourceGroup == request.Spec.Data.Flavor.Data.ExtraSpecs["hw_version"]:
traceLog.Info("unlocking resources reserved by matching committed resource reservation with allocation",
"reservation", reservation.Name,
"instanceUUID", request.Spec.Data.InstanceUUID,
"projectID", request.Spec.Data.ProjectID,
"resourceGroup", reservation.Spec.CommittedResourceReservation.ResourceGroup)
continue
}
// Check if this is a CR reservation scheduling request.
// If so, we should NOT unlock any CR reservations to prevent overbooking.
// CR capacity should only be unlocked for actual VM scheduling.
intent, err := request.GetIntent()
switch {
case err == nil && intent == api.ReserveForCommittedResourceIntent:
traceLog.Debug("keeping CR reservation locked for CR reservation scheduling",
"reservation", reservation.Name,
"intent", intent)
// Don't continue - fall through to block the resources
case !s.Options.LockReserved &&
// For committed resource reservations: unlock resources only if:
// 1. Project ID matches
// 2. ResourceGroup matches the flavor's hw_version
reservation.Spec.CommittedResourceReservation.ProjectID == request.Spec.Data.ProjectID &&
reservation.Spec.CommittedResourceReservation.ResourceGroup == request.Spec.Data.Flavor.Data.ExtraSpecs["hw_version"]:
traceLog.Info("unlocking resources reserved by matching committed resource reservation with allocation",
"reservation", reservation.Name,
"instanceUUID", request.Spec.Data.InstanceUUID,
"projectID", request.Spec.Data.ProjectID,
"resourceGroup", reservation.Spec.CommittedResourceReservation.ResourceGroup)
continue
}

case v1alpha1.ReservationTypeFailover:
// For failover reservations: if the requested VM is contained in the allocations map
// AND this is an evacuation request, unlock the resources.
// We only unlock during evacuations because:
// 1. Failover reservations are specifically for HA/evacuation scenarios.
// 2. During live migrations or other operations, we don't want to use failover capacity.
// Note: we cannot use failover reservations from other VMs, as that can invalidate our HA guarantees.
intent, err := request.GetIntent()
if err == nil && intent == api.EvacuateIntent {
if reservation.Status.FailoverReservation != nil {
if _, contained := reservation.Status.FailoverReservation.Allocations[request.Spec.Data.InstanceUUID]; contained {
traceLog.Info("unlocking resources reserved by failover reservation for VM in allocations (evacuation)",
"reservation", reservation.Name,
"instanceUUID", request.Spec.Data.InstanceUUID)
continue
case v1alpha1.ReservationTypeFailover:
// For failover reservations: if the requested VM is contained in the allocations map
// AND this is an evacuation request, unlock the resources.
// We only unlock during evacuations because:
// 1. Failover reservations are specifically for HA/evacuation scenarios.
// 2. During live migrations or other operations, we don't want to use failover capacity.
// Note: we cannot use failover reservations from other VMs, as that can invalidate our HA guarantees.
intent, err := request.GetIntent()
if err == nil && intent == api.EvacuateIntent {
if reservation.Status.FailoverReservation != nil {
if _, contained := reservation.Status.FailoverReservation.Allocations[request.Spec.Data.InstanceUUID]; contained {
traceLog.Info("unlocking resources reserved by failover reservation for VM in allocations (evacuation)",
"reservation", reservation.Name,
"instanceUUID", request.Spec.Data.InstanceUUID)
continue
}
}
}
traceLog.Debug("processing failover reservation", "reservation", reservation.Name)
}
traceLog.Debug("processing failover reservation", "reservation", reservation.Name)
}

// Block resources on BOTH Spec.TargetHost (desired) AND Status.Host (actual).
Expand Down
77 changes: 77 additions & 0 deletions internal/scheduling/nova/plugins/filters/filter_has_enough_capacity_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,32 @@ func newHypervisorWithBothCapacities(name, cpuCap, cpuEffCap, memCap, memEffCap
}
}

// newUnconfirmedReservation creates a CR reservation in the "cycle-1-done, cycle-2-pending" state:
// Spec.TargetHost is set but Status.Host is empty and Ready condition is not yet written.
// This models the window between the reservation controller writing the target host and
// completing the status patch in the next reconcile cycle.
func newUnconfirmedReservation(name, targetHost, projectID, flavorGroup, cpu, memory string) *v1alpha1.Reservation {
return &v1alpha1.Reservation{
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Spec: v1alpha1.ReservationSpec{
Type: v1alpha1.ReservationTypeCommittedResource,
TargetHost: targetHost,
Resources: map[hv1.ResourceName]resource.Quantity{
hv1.ResourceCPU: resource.MustParse(cpu),
hv1.ResourceMemory: resource.MustParse(memory),
},
CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{
ProjectID: projectID,
ResourceGroup: flavorGroup,
},
},
// No Status.Conditions, no Status.Host — Ready=false, cycle 2 not yet run.
Status: v1alpha1.ReservationStatus{},
}
}

// newCommittedReservation creates a reservation where TargetHost and Status.Host are the same.
func newCommittedReservation(
name, host, projectID, flavorName, flavorGroup, cpu, memory string,
Expand Down Expand Up @@ -567,6 +593,57 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) {
expectedHosts: []string{"host1", "host2", "host3"},
filteredHosts: []string{"host4"},
},
{
// Regression test: unconfirmed reservation (TargetHost set, Ready=false, Status.Host="")
// models the window between reconcile cycle 1 (TargetHost written) and cycle 2 (Ready synced).
// The filter must block the target host even though IsReady() returns false.
name: "Unconfirmed reservation (TargetHost set, not ready) blocks target host",
reservations: []*v1alpha1.Reservation{
newUnconfirmedReservation("unconfirmed-res", "host1", "project-X", "gp-1", "8", "16Gi"),
},
request: newNovaRequest("instance-123", "project-A", "m1.small", "gp-1", 4, "8Gi", false, []string{"host1", "host2", "host3", "host4"}),
opts: FilterHasEnoughCapacityOpts{LockReserved: false},
expectedHosts: []string{"host2", "host3"},
filteredHosts: []string{"host1", "host4"},
},
{
// Regression test: multiple unconfirmed reservations (cycle 1 done, not yet ready) targeting
// different hosts must each block their respective target host.
name: "Multiple unconfirmed reservations each block their target host",
reservations: []*v1alpha1.Reservation{
newUnconfirmedReservation("unconfirmed-1", "host1", "project-X", "gp-1", "8", "16Gi"),
newUnconfirmedReservation("unconfirmed-2", "host2", "project-X", "gp-1", "4", "8Gi"),
},
request: newNovaRequest("instance-123", "project-A", "m1.small", "gp-1", 4, "8Gi", false, []string{"host1", "host2", "host3", "host4"}),
opts: FilterHasEnoughCapacityOpts{LockReserved: false},
expectedHosts: []string{"host3"},
filteredHosts: []string{"host1", "host2", "host4"},
},
{
// Regression test: unconfirmed failover reservation (TargetHost set, Ready=false)
// must block the target host just like unconfirmed CR reservations.
name: "Unconfirmed failover reservation (TargetHost set, not ready) blocks target host",
reservations: []*v1alpha1.Reservation{
{
ObjectMeta: metav1.ObjectMeta{Name: "failover-unconfirmed"},
Spec: v1alpha1.ReservationSpec{
Type: v1alpha1.ReservationTypeFailover,
TargetHost: "host1",
Resources: map[hv1.ResourceName]resource.Quantity{
hv1.ResourceCPU: resource.MustParse("8"),
hv1.ResourceMemory: resource.MustParse("16Gi"),
},
FailoverReservation: &v1alpha1.FailoverReservationSpec{ResourceGroup: "gp-1"},
},
// No Status.Conditions, no Status.Host — Ready=false.
Status: v1alpha1.ReservationStatus{},
},
},
request: newNovaRequest("instance-123", "project-A", "m1.small", "gp-1", 4, "8Gi", false, []string{"host1", "host2", "host3", "host4"}),
opts: FilterHasEnoughCapacityOpts{LockReserved: false},
expectedHosts: []string{"host2", "host3"},
filteredHosts: []string{"host1", "host4"},
},
}

for _, tt := range tests {
Expand Down