From a3b06ae0b6e0a6aec80177606ccb7de073266a47 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Thu, 13 Aug 2026 15:28:37 +0100 Subject: [PATCH 1/2] Skip the tier once providerdoesn't support Signed-off-by: kerthcet --- config/manager/kustomization.yaml | 2 +- .../controller/nodeclaim_controller_test.go | 7 +- .../pod_placement_controller_test.go | 69 ++++++++++++++++++- internal/controller/pod_placement_helpers.go | 37 ++++++++-- pkg/provider/modal/modal.go | 7 +- 5 files changed, 108 insertions(+), 14 deletions(-) diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 086bc1e..f8bcf74 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -5,4 +5,4 @@ kind: Kustomization images: - name: controller newName: inftyai/nebula-controller - newTag: latest + newTag: 0813-02 diff --git a/internal/controller/nodeclaim_controller_test.go b/internal/controller/nodeclaim_controller_test.go index 6872fac..a535688 100644 --- a/internal/controller/nodeclaim_controller_test.go +++ b/internal/controller/nodeclaim_controller_test.go @@ -46,10 +46,13 @@ type fakeProvider struct { terminated []string // instance ids passed to Terminate, in order terminateErr error // if set, Terminate fails gpus []string // accelerators MapAccelerator offers; nil = offer any + spot bool // Capabilities().SupportsSpot (placement skips Spot without it) } -func (f *fakeProvider) Name() string { return f.name } -func (f *fakeProvider) Capabilities() provider.Capabilities { return provider.Capabilities{} } +func (f *fakeProvider) Name() string { return f.name } +func (f *fakeProvider) Capabilities() provider.Capabilities { + return provider.Capabilities{SupportsSpot: f.spot} +} func (f *fakeProvider) Provision(context.Context, *corev1.Pod, provider.ProvisionRequest) (provider.ProvisionResult, error) { return provider.ProvisionResult{}, nil } diff --git a/internal/controller/pod_placement_controller_test.go b/internal/controller/pod_placement_controller_test.go index 0cd2965..4ff0313 100644 --- a/internal/controller/pod_placement_controller_test.go +++ b/internal/controller/pod_placement_controller_test.go @@ -453,8 +453,11 @@ func TestPlacement_CapacityIsOuterAxis(t *testing.T) { pod := gatedPod("p1", "default", "uid-1", "pool-a", "H100") pool := poolWith("pool-a", []nebulav1alpha1.CapacityType{nebulav1alpha1.CapacitySpot, nebulav1alpha1.CapacityOnDemand}, "runpod", provider.ProviderModal) - runpod := &fakeProvider{name: "runpod", gpus: []string{"H100"}} - modal := &fakeProvider{name: provider.ProviderModal, gpus: []string{"H100"}} + // Both advertise Spot, so the tier is genuinely reachable and the blocklist — + // not a capability gap — is what pushes the walk onward (see + // TestPlacement_SkipsSpotWhenProviderHasNoSpotTier for that path). + runpod := &fakeProvider{name: "runpod", gpus: []string{"H100"}, spot: true} + modal := &fakeProvider{name: provider.ProviderModal, gpus: []string{"H100"}, spot: true} r, c := newPlacementReconciler(t, []client.Object{pod, pool}, runpod, modal) r.Blocklist = &fakeBlocklist{blocked: []failover.Candidate{ // Both providers' Spot is exhausted (wildcard provider, Spot only). @@ -473,6 +476,63 @@ func TestPlacement_CapacityIsOuterAxis(t *testing.T) { } } +func TestPlacement_SkipsSpotWhenProviderHasNoSpotTier(t *testing.T) { + // Modal has no user-facing preemptible capacity (SupportsSpot=false). The pool + // asks for Spot first, but that candidate is unservable, so the walk falls + // through to OnDemand — and the Pod is labelled OnDemand, which is what it will + // actually be billed as. Placing it as Spot would be a silent downgrade: the + // adapter ignores CapacityType, so the user would pay OnDemand rates for a Pod + // whose annotation claims Spot. + pod := gatedPod("p1", "default", "uid-1", "pool-a", "H100") + pool := poolWith("pool-a", []nebulav1alpha1.CapacityType{nebulav1alpha1.CapacitySpot, nebulav1alpha1.CapacityOnDemand}, + provider.ProviderModal) + prov := &fakeProvider{name: provider.ProviderModal, gpus: []string{"H100"}} // spot: false + r, c := newPlacementReconciler(t, []client.Object{pod, pool}, prov) + + reconcilePod(t, r, "default", "p1") + + got := getPod(t, c, "default", "p1") + if hasGateNamed(got) { + t.Fatal("expected the Pod placed at the OnDemand tier") + } + if got.Annotations[nebulav1alpha1.CapacityTypeAnnotation] != string(nebulav1alpha1.CapacityOnDemand) { + t.Fatalf("expected the Spot candidate skipped for OnDemand, got %q", + got.Annotations[nebulav1alpha1.CapacityTypeAnnotation]) + } +} + +func TestPlacement_SpotOnlyPoolStaysGatedOnOnDemandOnlyProvider(t *testing.T) { + // Spot is the pool's ONLY tier and the sole provider cannot serve it, so there is + // no candidate at all. The Pod stays gated — visibly unplaceable — rather than + // being quietly provisioned as OnDemand against an explicit Spot-only policy. + // Nothing here can be fixed by a lapsing TTL, so there is no requeue hint: the + // unblock is a pool edit or a provider gaining a Spot tier, both of which + // generate their own event. + pod := gatedPod("p1", "default", "uid-1", "pool-a", "H100") + pool := poolWith("pool-a", []nebulav1alpha1.CapacityType{nebulav1alpha1.CapacitySpot}, provider.ProviderModal) + prov := &fakeProvider{name: provider.ProviderModal, gpus: []string{"H100"}} // spot: false + r, c := newPlacementReconciler(t, []client.Object{pod, pool}, prov) + + res, err := r.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: "default", Name: "p1"}, + }) + if err != nil { + t.Fatalf("reconcile: %v", err) + } + if res.RequeueAfter != 0 { + t.Fatalf("expected no requeue hint for a capability gap, got %v", res.RequeueAfter) + } + + got := getPod(t, c, "default", "p1") + if !hasGateNamed(got) { + t.Fatal("expected the Pod to stay gated when no provider serves the only tier") + } + var nc nebulav1alpha1.NodeClaim + if err := c.Get(context.Background(), types.NamespacedName{Name: "default-p1"}, &nc); err == nil { + t.Fatal("expected no claim for an unplaceable Pod") + } +} + func TestPlacement_AllCandidatesBlockedRequeuesForBlockExpiry(t *testing.T) { // Every (tier, provider, region) candidate is blocked (DenyAll on the provider), // but the candidates are servable — the block is a transient failover exclusion. @@ -482,7 +542,10 @@ func TestPlacement_AllCandidatesBlockedRequeuesForBlockExpiry(t *testing.T) { pod := gatedPod("p1", "default", "uid-1", "pool-a", "H100") pool := poolWith("pool-a", []nebulav1alpha1.CapacityType{nebulav1alpha1.CapacitySpot, nebulav1alpha1.CapacityOnDemand}, provider.ProviderModal) - prov := &fakeProvider{name: provider.ProviderModal, gpus: []string{"H100"}} + // spot:true so BOTH tiers are servable and every skip is the blocklist's doing — + // the point of the test is the requeue hint, which only exists for candidates a + // lapsing TTL can free. + prov := &fakeProvider{name: provider.ProviderModal, gpus: []string{"H100"}, spot: true} r, c := newPlacementReconciler(t, []client.Object{pod, pool}, prov) r.Blocklist = &fakeBlocklist{ blocked: []failover.Candidate{{Provider: provider.ProviderModal}}, // whole-provider block (auth/quota) diff --git a/internal/controller/pod_placement_helpers.go b/internal/controller/pod_placement_helpers.go index 33d5b66..73ab8da 100644 --- a/internal/controller/pod_placement_helpers.go +++ b/internal/controller/pod_placement_helpers.go @@ -33,6 +33,7 @@ import ( nebulav1alpha1 "github.com/InftyAI/Nebula/api/v1alpha1" "github.com/InftyAI/Nebula/pkg/failover" + "github.com/InftyAI/Nebula/pkg/provider" "github.com/InftyAI/Nebula/pkg/util" ) @@ -72,15 +73,15 @@ func (r *PodPlacementReconciler) poolFor(ctx context.Context, pod *corev1.Pod) ( // failure ever reached the blocklist, so this loop only walks regions, not zones. // // Returns ok=false when every (tier, provider, region) candidate is either -// unservable (provider unregistered or does not offer the accelerator) or blocked; -// the caller then leaves the Pod gated for a later retry (a pool edit, a provider -// registering, or a block expiring). Provider quirks (e.g. Modal being -// OnDemand-only) are still handled at Provision time, not here. +// unservable (provider unregistered, does not offer the accelerator, or cannot +// serve the tier — see servesCapacity) or blocked; the caller then leaves the Pod +// gated for a later retry (a pool edit, a provider registering, or a block +// expiring). // // On ok=false it also returns retryAfter: the time until the SOONEST currently- // servable candidate (one skipped ONLY because it is blocklisted) frees, or 0 when // no candidate can ever be unblocked by a lapsing TTL (every candidate is -// unregistered or does not offer the accelerator). The caller requeues on a +// unservable for a reason no TTL can lapse). The caller requeues on a // positive hint so a Pod stuck purely on failover retries the moment a block // expires, rather than idling until the periodic resync — blocklist TTL expiry // emits no event of its own. @@ -114,6 +115,11 @@ func (r *PodPlacementReconciler) selectPlacement(ctx context.Context, pod *corev "provider", ref.Name, "capacityType", tier) continue // unregistered; NodePool status surfaces this separately } + if !servesCapacity(prov, tier) { + log.V(1).Info("skipping candidate: provider does not offer the capacity tier", + "provider", ref.Name, "capacityType", tier) + continue + } // A CPU-only Pod (no accelerator) matches any provider; an accelerator // Pod only matches a provider whose catalog serves that (type, count). // MapAccelerator is consulted only for that servability check — the block @@ -165,6 +171,27 @@ func capacityTiers(pool *nebulav1alpha1.NodePool) []nebulav1alpha1.CapacityType return pool.Spec.CapacityTypes } +// servesCapacity reports whether prov can actually deliver the candidate's +// capacity tier. Only Spot is ever refused: an OnDemand-only provider (Modal) has +// no user-facing interruptible tier, so a Spot candidate there is unservable in +// exactly the sense a missing accelerator is — the request cannot be honoured, and +// pretending otherwise is worse than skipping. Placing it would stamp +// CapacityType=Spot on the Pod, hand it to an adapter that drops the field, and +// bill the user at OnDemand rates for capacity they explicitly asked to be cheap +// and interruptible, with no error, event or status to reveal the substitution. +// Skipping instead lets the pool's next tier take over ([Spot, OnDemand] still +// lands on Modal as OnDemand, now truthfully labelled), and a Spot-only pool leaves +// the Pod gated — visibly unplaceable rather than quietly overcharged. +// +// The empty tier is "the provider's default", which every provider serves by +// definition, so it passes. +func servesCapacity(prov provider.Provider, tier nebulav1alpha1.CapacityType) bool { + if tier != nebulav1alpha1.CapacitySpot { + return true + } + return prov.Capabilities().SupportsSpot +} + // regionsFor is the inner axis for one provider ref: the regions to try, in listed // order. An empty/omitted list means "the provider's configured default region", // represented as a single empty-string candidate so the walk runs once for diff --git a/pkg/provider/modal/modal.go b/pkg/provider/modal/modal.go index 64ff168..569effc 100644 --- a/pkg/provider/modal/modal.go +++ b/pkg/provider/modal/modal.go @@ -21,9 +21,10 @@ limitations under the License. // - Lifecycle is create/terminate only. A Modal Sandbox is spun up and later // terminated; there is no stop/resume, so Capabilities.SupportsStop=false. // - Modal does not expose a user-facing spot/preemptible tier, so -// SupportsSpot=false. The optimizer therefore only ever sends OnDemand -// ProvisionRequests here (the NodePool capacity-tier loop skips Spot for -// providers that don't advertise it). +// SupportsSpot=false. Placement consults that trait (servesCapacity in the +// capacity-tier loop) and skips a Spot candidate here rather than downgrading +// it silently, so only OnDemand (or default-tier) ProvisionRequests reach this +// adapter and it never has to interpret CapacityType. // - Modal Sandboxes carry native tags, so NativeTags=true and the ClaimName // is stored as a tag rather than smuggled into the instance name. // - There is no preemption push; detection is poll-based like every provider. From 69701e7e590078a52f70affcfcc79248e1ec2498 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Thu, 13 Aug 2026 15:29:14 +0100 Subject: [PATCH 2/2] revert tag to latest Signed-off-by: kerthcet --- config/manager/kustomization.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index f8bcf74..086bc1e 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -5,4 +5,4 @@ kind: Kustomization images: - name: controller newName: inftyai/nebula-controller - newTag: 0813-02 + newTag: latest