Mark initializing when reserved only

api/v1alpha1/groupversion_info.go

@@ -169,10 +169,13 @@ const (
 // whole set is here rather than the subset with in-tree readers: these are the
 // values status.reason can take, and a reader should find them in one place.
 const (
-	// PodReasonProvisioning: a provider Provision call has been issued but the
-	// instance does not yet exist — we are still allocating it (e.g. EC2
-	// RunInstances in flight). Set on CreatePod, before the first poll observes
-	// the instance.
+	// PodReasonProvisioning: capacity has not been allocated yet. Stamped by CreatePod
+	// before it calls Provision, and HELD if Provision returns an id without reserving
+	// capacity — a Modal sandbox the control plane accepted but that is still queued
+	// for a GPU. So the instance may exist (and then must be reclaimed) even under this
+	// reason; what has not happened is the allocation. Replaced by Initializing as soon
+	// as capacity is committed: at once for a provider that allocates synchronously
+	// (AWS), otherwise when the first poll observes the instance.
 	PodReasonProvisioning = "Provisioning"
 	// PodReasonInitializing: the instance EXISTS at the provider but is not yet
 	// reachable — it is booting (EC2 "pending"), running-but-not-yet-passing its
@@ -181,8 +184,10 @@ const (
 	// term. Provisioning is done; the instance is coming up. Distinct from
 	// Provisioning so a Pod stuck here points at a slow boot / failing status checks,
 	// not a stuck allocation — and so the NodeClaim controller can tell that an
-	// instance exists. The virtual kubelet stamps it only for an instance it observed
-	// in the provider's List, which is what makes it trustworthy as that evidence.
+	// instance exists. The virtual kubelet stamps it only on EVIDENCE of existence:
+	// either the provider observed the instance in its List, or Provision reported it
+	// reserved (capacity committed, not merely requested). That is what makes it
+	// trustworthy for the claim to key Bound off.
 	PodReasonInitializing = "Initializing"
 	// PodReasonRunning: the provider reports the instance running.
 	PodReasonRunning = "Running"

config/samples/deployment.yaml

@@ -38,7 +38,7 @@ spec:
         app: gpu-workload-sample
         nebula.inftyai.com/enabled: "true"
         nebula.inftyai.com/nodepool: sample
-        nebula.inftyai.com/accelerator-type: a100-80gb
+        nebula.inftyai.com/accelerator-type: a100-40gb
     spec:
       # Do NOT set nodeName or a provider nodeSelector yourself — the placement
       # controller fills the nodeSelector in when it ungates the Pod. Setting

config/samples/nodepool.yaml

@@ -17,7 +17,7 @@ spec:
     - eu-west-1
     - ca-central-1
     - sa-east-1
-  - name: modal
+  # - name: modal
   # - name: runpod
   # Outer axis: try OnDemand on every provider first, fall back to Spot.
   capacityTypes:

docs/architecture.md

@@ -476,10 +476,6 @@ status:
 When `capacityTypes` is omitted, the API defaults it to `{OnDemand, Spot}`. The
 listed order is the fallback order the placement controller walks.
 
-A pool must list between one and eight providers. The upper bound keeps the
-provider-by-region candidate expansion bounded while performance at larger
-provider counts remains unproven.
-
 ### NodeClaim (`nc`)
 
 ```yaml

docs/status.md

@@ -38,7 +38,9 @@ teardown.
 
 | Pod phase | reason | writer | instance exists? | claim phase |
 |---|---|---|---|---|
-| `Pending` | `Provisioning` | `CreatePod` | no | `Provisioning` (grace applies) |
+| `Pending` | `Provisioning` | `CreatePod`, before `Provision` | no | `Provisioning` (grace applies) |
+| `Pending` | `Provisioning` | held, after an UNRESERVED `Provision` | yes, unreserved | `Provisioning` (grace applies) |
+| `Pending` | `Initializing` | `CreatePod`, after a RESERVED `Provision` | yes | `Bound` |
 | `Pending` | `Initializing` | `applyState` ← `InstancePending` | yes | `Bound` |
 | `Running` | `Running` | `applyState` ← `InstanceRunning` | yes | `Bound` |
 | `Failed` | `ProvisionFailed` | `CreatePod` | no | `Terminated` (via `isTerminal`) |
@@ -52,20 +54,34 @@ served Pod is ABSENT: after `Bound`/`Terminating`, the claim deletes itself and
 the terminate finalizer runs; before `Bound`, it waits `placementGracePeriod`
 first.
 
-Note the two writers. `CreatePod` writes the rows where no instance exists; every
-other non-terminal row comes from `applyState`, driven by the poll loop, and is
-therefore only reachable for an instance the provider actually returned from
-`List()`.
-
-`Provisioning` is written *after* `Provision` returns, so today it is barely
-observable: by the time it lands the instance already exists, and the first poll
-tick (≤15s) replaces it with `Initializing`. That makes the two reasons hard to
-tell apart in practice even though they mean different things — "nothing exists
-yet" versus "it exists and is not yet ready". Fixing this is not simply a matter of
-writing `Provisioning` earlier: a Pod must not be tracked before its instance
-exists, because the poll loop maps a tracked Pod absent from `List()` to
-`Terminated`, which is unrecoverable (Pod phases are terminal-sticky and the claim
-reclaims on that phase).
+Note the two writers. `CreatePod` writes the rows around the `Provision` call;
+every other non-terminal row comes from `applyState`, driven by the poll loop, and
+is therefore only reachable for an instance the provider returned from `List()`.
+
+`Provisioning` is written *before* `Provision` is called. That call can run for
+minutes (AWS sweeps a region's zones on a capacity error, bounded at 2), and until
+it returns this is the only explanation the Pod carries.
+
+Emitting it early is safe; *tracking* it early is not. The poll loop maps a tracked
+Pod absent from `List()` to `Terminated`, which is unrecoverable (Pod phases are
+terminal-sticky and the claim reclaims on that phase), so a tick landing mid-
+provision would tear down the instance the call is about to return. `CreatePod`
+therefore emits without storing, and stores only once `Provision` returns.
+
+`Provision` returns `(id, reserved, error)`. `reserved` means the provider committed
+capacity, not merely accepted the request: AWS always does (`CreateFleet` with
+`FleetTypeInstant` is synchronous), a fresh Modal sandbox never does (the GPU may
+still be queued). Only a reserved instance advances to `Initializing`; an unreserved
+id holds at `Provisioning`, which is still exactly true — the id is real and must be
+reclaimed, but nothing is allocated. Either way the Pod is now tracked: `reserved`
+constrains what the status may claim, not what is owed, and says nothing about
+readiness — that is `applyState`'s job one tick later.
+
+That hold is short-lived by design. One poll tick later the same queued sandbox
+reads `Initializing`, because a provider that queues cannot tell queued from booting
+(see below). For that window the claim is `Provisioning` rather than `Bound`, so an
+orphan waits the grace period instead of being reclaimed at once — teardown still
+resolves the instance from `List()`, so nothing leaks.
 
 Important details:
 
@@ -78,9 +94,11 @@ Important details:
   "nothing exists yet" from "exists and booting", so the claim keys off
   `status.reason`, which is why the reasons are declared once as `PodReason*` in
   `api/v1alpha1`.
-- Only `Provisioning` fails to earn the guard, because nothing exists yet. If the
-  Pod is absent then, the controller waits `placementGracePeriod` (15 seconds)
-  before deleting an orphaned claim.
+- Only `Provisioning` fails to earn the guard. If the Pod is absent then, the
+  controller waits `placementGracePeriod` (15 seconds) before deleting an orphaned
+  claim. An unreserved instance reads `Provisioning` even though it exists, so it
+  gets the grace window rather than immediate reclaim — safe either way, because
+  teardown resolves the instance from `List()`, not from the reason.
 - `NodeClaimStatus.InstanceID` is recorded on a best-effort basis. The finalizer
   prefers it when present, but can still recover by matching provider instances
   by claim name through `List()`.
@@ -133,7 +151,8 @@ folded in by `List` (`StatusChecksPassed`).
   reason it could not launch. A capacity shortfall is an error
   (`ErrNoCapacity`/`ErrSpotCapacity`) that drives AZ/region/tier failover, not a
   pending instance. So an AWS instance that exists is always allocated; `pending`
-  vs `running`-without-checks are both "booting", and both are `Bound`.
+  vs `running`-without-checks are both "booting", and both are `Bound`. This is why
+  `Provision` always returns `reserved` — the Pod can go straight to `Initializing`.
 - **No `Failed` case.** Impaired status checks and `StateReason` are not consumed,
   so an instance that failed to boot currently reads as `Terminated` (looks like a
   clean teardown) or holds at `Pending`. See
@@ -191,14 +210,16 @@ only two signals and has to record a third fact itself.
   the provider what exists.
 - **Modal DOES queue**, unlike AWS: `Sandboxes.Create` returns an id immediately
   and the sandbox then waits for capacity, potentially for minutes on a large GPU
-  shape. It is `Bound` and billing throughout. See below for why that is not
-  reported distinctly.
+  shape. It is `Bound` and billing throughout. So `Provision` returns
+  `reserved=false` for a fresh sandbox and the Pod holds at `Provisioning` until the
+  first poll tick. An *adopted* sandbox has been observed, so a `running` one is
+  known to be reserved. See below for why queued is not reported distinctly.
 
 ### fake
 
 The in-memory e2e provider reports `InstanceRunning` as soon as an instance is
-created. It exists to exercise the placement and teardown paths without a real
-backend, so it has no boot or readiness phase to model.
+created, and so always reserves. It exists to exercise the placement and teardown
+paths without a real backend, so it has no boot or readiness phase to model.
 
 ---
 
@@ -207,10 +228,10 @@ backend, so it has no boot or readiness phase to model.
 Documented so the coarseness is not mistaken for a bug.
 
 **Modal: queued vs. booting.** A sandbox waiting for capacity and one actively
-starting up are both reported `Pending` / `Initializing`, and the Pod's reason
-flips from `Provisioning` to `Initializing` at the first poll tick (≤15s) whether
-or not anything changed in the sandbox. Every public signal was measured and none
-carries the boundary:
+starting up are both reported `Pending` / `Initializing`: the Pod reads
+`Provisioning` until the first poll tick (≤15s), then `Initializing` for the rest of
+both, so that one reason covers queueing and booting alike. Every public signal was
+measured and none carries the boundary:
 
 | signal | distinguishes queued from booting? |
 |---|---|

internal/controller/nodeclaim_controller.go

@@ -293,12 +293,15 @@ func (r *NodeClaimReconciler) provider(name string) (provider.Provider, bool) {
 //   - An instance EXISTS => Bound. Two Pod shapes prove existence, and the claim
 //     treats them identically because its question is existence, not readiness:
 //     Running (up and past its readiness bar) and Pending/Initializing (created and
-//     booting — vnode stamps that reason only for an instance it observed in the
-//     provider's List). A booting GPU box is just as real, and just as billable, as
-//     a serving one; if its Pod vanishes it must be reclaimed with no grace.
-//   - Otherwise => Provisioning. The instance does not exist yet (the Provision
-//     call may still be in flight), so a vanished Pod may be cache lag and the
-//     grace window applies.
+//     booting — vnode stamps that reason only on evidence of existence, either an
+//     instance observed in the provider's List or a reserved Provision). A booting
+//     GPU box is just as real, and just as billable, as a serving one; if its Pod
+//     vanishes it must be reclaimed with no grace.
+//   - Otherwise => Provisioning. No capacity has been allocated (the Provision call
+//     may still be in flight, or it returned an unreserved id — a Modal sandbox still
+//     queued for a GPU), so a vanished Pod may be cache lag and the grace window
+//     applies. An unreserved instance does exist, so this understates it for one poll
+//     tick; the grace path still reclaims by asking the provider what exists.
 func (r *NodeClaimReconciler) desiredPhase(nc *nebulav1alpha1.NodeClaim, pod *corev1.Pod) nebulav1alpha1.NodeClaimPhase {
 	switch {
 	case isTerminal(pod.Status.Phase):

internal/controller/nodeclaim_controller_test.go

@@ -50,8 +50,8 @@ type fakeProvider struct {
 
 func (f *fakeProvider) Name() string                        { return f.name }
 func (f *fakeProvider) Capabilities() provider.Capabilities { return provider.Capabilities{} }
-func (f *fakeProvider) Provision(context.Context, *corev1.Pod, provider.ProvisionRequest) (string, error) {
-	return "", nil
+func (f *fakeProvider) Provision(context.Context, *corev1.Pod, provider.ProvisionRequest) (string, bool, error) {
+	return "", false, nil
 }
 func (f *fakeProvider) Terminate(_ context.Context, id string) error {
 	f.terminated = append(f.terminated, id)

pkg/provider/aws/aws.go

@@ -382,42 +382,51 @@ func (p *Provider) Offerings(ctx context.Context) ([]provider.Offering, error) {
 // EC2 id. Terminate/Get do not need the region encoded in it — they locate the
 // instance by sweeping the swept regions (the same set List covers), since a
 // wrong-region lookup is a harmless no-op (see Terminate/Get).
-func (p *Provider) Provision(ctx context.Context, pod *corev1.Pod, req provider.ProvisionRequest) (string, error) {
+// Every id this returns is RESERVED. RunInstance launches through a CreateFleet
+// *instant* request, which is synchronous: the response carries either an instance
+// id — meaning EC2 already found capacity in some (type, AZ) cell and the instance
+// is booting on real hardware — or the reason it could not launch, which becomes an
+// error driving AZ/region/tier failover. There is no queued state for an EC2
+// instance to sit in, so the reserved return is unconditionally true on success.
+func (p *Provider) Provision(
+	ctx context.Context, pod *corev1.Pod, req provider.ProvisionRequest,
+) (string, bool, error) {
 	if pod == nil {
-		return "", errors.New("aws: nil pod")
+		return "", false, errors.New("aws: nil pod")
 	}
 	if req.ClaimName == "" {
-		return "", errors.New("aws: empty ClaimName in ProvisionRequest")
+		return "", false, errors.New("aws: empty ClaimName in ProvisionRequest")
 	}
 
 	region := req.Region
 	client, err := p.clientFor(ctx, region)
 	if err != nil {
-		return "", err
+		return "", false, err
 	}
 
 	// Idempotency: if an instance already carries this claim tag IN THIS REGION,
 	// return it rather than launching a second (guards a retry after a partial
 	// create). A claim is placed in exactly one region per attempt, so scanning the
-	// target region's client is sufficient.
+	// target region's client is sufficient. It is reserved for the same reason a
+	// fresh launch is: it only exists because some earlier instant fleet succeeded.
 	if existing, err := findByClaim(ctx, client, req.ClaimName); err != nil {
-		return "", err
+		return "", false, err
 	} else if existing != nil {
-		return existing.ID, nil
+		return existing.ID, true, nil
 	}
 
 	spec, err := p.instanceSpecFromPod(pod, req)
 	if err != nil {
-		return "", err
+		return "", false, err
 	}
 	// The Provision deadline is enforced generically by the vnode handler (from
 	// Capabilities.ProvisionTimeout), so RunInstance simply honors ctx as it fails
 	// over across zones — no adapter-local WithTimeout here.
 	id, err := client.RunInstance(ctx, spec)
 	if err != nil {
-		return "", err
+		return "", false, err
 	}
-	return id, nil
+	return id, true, nil
 }
 
 // Terminate implements provider.Provider. Idempotent by the Client contract. The