diff --git a/.gitignore b/.gitignore index ffa0ea0..d0ee133 100644 --- a/.gitignore +++ b/.gitignore @@ -42,4 +42,5 @@ __pycache__ *.tgz .devcontainer -bin \ No newline at end of file +bin +.claude diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..e097a92 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,18 @@ +# Nebula + +## Comments + +This codebase comments the *why*, not the *what*. Match that — and match its density too, +don't exceed it. + +- Never restate the code. `// increment the counter` above `n++` is noise. +- Don't repeat a rationale a nearby doc comment already gives. Name the symbol that holds + it (`see Offering.GPUCount`) and move on. +- Doc comments: the contract, plus the one constraint a caller would otherwise get wrong. + 2-5 lines. Go longer only for a real invariant — a subtle failure mode, a decision kept + on purpose, a gotcha in an external API. +- Inline comments: only where a reader would otherwise draw the wrong conclusion. Not one + per branch, not one per field assignment. +- One comment on the tricky invariant beats five on the obvious steps. + +If a comment is longer than the code it explains, it is usually the wrong comment. diff --git a/api/v1alpha1/nodeclaim_types.go b/api/v1alpha1/nodeclaim_types.go index 51d493f..0f36a84 100644 --- a/api/v1alpha1/nodeclaim_types.go +++ b/api/v1alpha1/nodeclaim_types.go @@ -138,6 +138,81 @@ type NodeClaimStatus struct { // Endpoint is the reachable address (e.g. SSH host:port) once ready. // +optional Endpoint string `json:"endpoint,omitempty"` + + // PriceUSDPerHour is what this instance costs per hour in USD, as a decimal string + // ("7.9000"), resolved from the provider's catalog against the served Pod's shape. + // Status, not spec: it is a derived result nobody can declare up front. A string + // because it is written to be READ — a print column can only echo a field, never + // scale a fixed-point integer back into currency. + // + // Empty means UNPRICED, not free: the provider implements no provider.Pricer, or its + // catalog has no row for this candidate. Consumers must skip such a claim rather than + // count it as $0. + // + // Written once and never refreshed, so a catalog edit cannot retroactively reprice a + // running instance and rewrite the cost history it has already reported. + // +optional + PriceUSDPerHour string `json:"priceUSDPerHour,omitempty"` + + // EstimatedCostUSD is what this instance has cost SO FAR, as a decimal string ("412.94000000") + // — PriceUSDPerHour integrated over the time it has held an instance. A string for the same + // reason the rate is one: it exists to be read off a print column. + // + // More decimals than the rate, and they are not decoration: each checkpoint re-reads this field + // to measure the next window, so digits dropped here are money dropped, and a cheap enough claim + // would round back to where it started every minute and never accrue at all. Round it for + // display; never round it back into this field. + // + // ESTIMATED, and the name says so on purpose: it is our own arithmetic over a list price + // (see PriceUSDPerHour), not a figure any provider has confirmed. Nothing here has been + // invoiced. Reconcile against the provider's billing export before anyone is charged. + // + // Within those limits it is the AUTHORITATIVE total, not the Prometheus counter: it is + // written exactly once per window and survives a restart. It is also only LIVE cost — it + // dies with the claim, so it is not a history. The metric is what outlives an instance. + // +optional + EstimatedCostUSD string `json:"estimatedCostUSD,omitempty"` + + // LastAccruedAt is how far cost accrual has counted: an ANCHOR for the next measurement, + // not a note about the last one. "Accrued" in the accounting sense — cost incurred but not + // yet invoiced, which is all EstimatedCostUSD ever holds. + // + // It is the reason a restart loses nothing: the next window is rate x (now - LastAccruedAt), + // so time that passed while Nebula was down is still counted on recovery instead of vanishing + // with the in-memory total. + // + // The invariant that makes it safe: this NEVER moves past cost that has been durably + // recorded, because it advances only in the same patch that writes EstimatedCostUSD. A + // failed write therefore loses nothing — the same window is counted next time. + // + // Unset means "not counting yet". Never treat it as the epoch, which would charge decades + // on the first tick. + // +optional + LastAccruedAt *metav1.Time `json:"lastAccruedAt,omitempty"` + + // CostLabels attributes this claim's spend to whoever asked for it: the values the served + // Pod carried for the label keys the operator configured (--cost-labels). Keyed by the POD + // label key, verbatim — so this map reads like the Pod it came from. The metric emits under a + // derived name Prometheus accepts ("example.com/org-id" here is example_com_org_id there), so + // do not expect the two to match by string. + // + // Status rather than spec because it is observed from the Pod, and it sits with the rest of + // the billing record for a reason: it is stamped in the SAME patch that opens the accrual + // anchor, so no window can ever be charged before its attribution is known. + // + // Written once, on first observation, and never refreshed — relabeling a Pod must not + // retroactively re-attribute spend already reported under the old values. A name the + // current --cost-labels no longer lists is ignored rather than cleaned up; one it lists but + // this map lacks reports as "none". + // + // Nil and EMPTY differ, which is why this field has no omitempty: nil means nothing has been + // observed yet (or --cost-labels is unset), while an empty map is the settled fact that the + // Pod carried none of the configured keys. With omitempty an empty map would not survive the + // write, so every reconcile would re-read the Pod and a label added later could still move + // the claim's attribution. + // +optional + // +kubebuilder:validation:Nullable + CostLabels map[string]string `json:"costLabels"` } // +kubebuilder:object:root=true @@ -147,6 +222,8 @@ type NodeClaimStatus struct { // +kubebuilder:printcolumn:name="Region",type=string,JSONPath=`.spec.region` // +kubebuilder:printcolumn:name="ACCELERATOR",type=string,JSONPath=`.spec.accelerator` // +kubebuilder:printcolumn:name="CAPACITY_TYPE",type=string,JSONPath=`.spec.capacityType` +// +kubebuilder:printcolumn:name="PRICE/HR",type=string,JSONPath=`.status.priceUSDPerHour` +// +kubebuilder:printcolumn:name="EST_COST",type=string,JSONPath=`.status.estimatedCostUSD` // +kubebuilder:printcolumn:name="Phase",type=string,JSONPath=`.status.phase` // +kubebuilder:printcolumn:name="Instance",type=string,JSONPath=`.status.instanceID` // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 632a0d5..bdd1f95 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -68,7 +68,7 @@ func (in *NodeClaim) DeepCopyInto(out *NodeClaim) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) out.Spec = in.Spec - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeClaim. @@ -140,6 +140,17 @@ func (in *NodeClaimSpec) DeepCopy() *NodeClaimSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodeClaimStatus) DeepCopyInto(out *NodeClaimStatus) { *out = *in + if in.LastAccruedAt != nil { + in, out := &in.LastAccruedAt, &out.LastAccruedAt + *out = (*in).DeepCopy() + } + if in.CostLabels != nil { + in, out := &in.CostLabels, &out.CostLabels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeClaimStatus. diff --git a/cmd/main.go b/cmd/main.go index 8cee8f0..5e4b9bc 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -49,6 +49,7 @@ import ( webhookv1 "github.com/InftyAI/Nebula/internal/webhook/v1" nebulacert "github.com/InftyAI/Nebula/pkg/cert" "github.com/InftyAI/Nebula/pkg/failover" + nebulametrics "github.com/InftyAI/Nebula/pkg/metrics" "github.com/InftyAI/Nebula/pkg/provider" awsprovider "github.com/InftyAI/Nebula/pkg/provider/aws" "github.com/InftyAI/Nebula/pkg/provider/fake" @@ -93,6 +94,7 @@ func main() { var secureMetrics bool var enableHTTP2 bool var kubeletAddr, kubeletClientCA string + var costLabels string var tlsOpts []func(*tls.Config) flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+ "Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.") @@ -119,6 +121,14 @@ func main() { "serves TLS without client verification, because which CA signs the API server's kubelet "+ "client certificate is not portable across distributions; restrict the port with a "+ "NetworkPolicy, or set this to your API server's kubelet client CA.") + flag.StringVar(&costLabels, "cost-labels", "", + "Comma-separated Pod label keys whose values attribute cost, e.g. "+ + "\"example.com/org-id,team_id\". Qualified keys are fine: the metric label is the whole key "+ + "with '/', '-' and '.' folded to '_' (example.com/org-id is queried as example_com_org_id), so "+ + "two keys that would collide there are rejected at startup. Empty (the default) reports cost "+ + "by candidate shape only. Changing this changes the identity of every cost series. Values "+ + "come from Pod labels and are NOT capped: the manager warns once if they push the cost "+ + "metric past 5000 series, but pick keys an admission policy constrains.") opts := zap.Options{ Development: true, } @@ -127,6 +137,21 @@ func main() { ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + // Before anything can record: the counter's label names are fixed when it is constructed, and + // a bad --cost-labels must stop the process rather than silently report every tenant as "none". + attribution, err := nebulametrics.ParseCostLabels(costLabels) + if err != nil { + setupLog.Error(err, "invalid --cost-labels") + os.Exit(1) + } + if err := nebulametrics.InitCost(attribution); err != nil { + setupLog.Error(err, "configuring the cost metric") + os.Exit(1) + } + if len(attribution) > 0 { + setupLog.Info("cost attribution enabled", "labels", attribution) + } + // if the enable-http2 flag is false (the default), http/2 should be disabled // due to its vulnerabilities. More specifically, disabling http/2 will // prevent from being vulnerable to the HTTP/2 Stream Cancellation and @@ -418,6 +443,16 @@ func setupControllers(mgr ctrl.Manager, blocklist *failover.Blocklist, kubeletSr return fmt.Errorf("unable to create Pod webhook: %w", err) } } + + // Cost accrual is a clock-driven loop, not a reconciler, so it is added directly. It opts into + // leader election by NOT implementing LeaderElectionRunnable, which buys two things: N replicas + // would mean N times the writes, and each would book windows into its OWN cost counter, leaving + // increase() to read a series that holds only the races that replica won. The ledger itself is + // safe either way (see CostAccrual.accrue). + if err := mgr.Add(controller.NewCostAccrual(mgr.GetClient())); err != nil { + return fmt.Errorf("unable to add the cost accrual loop: %w", err) + } + // +kubebuilder:scaffold:builder return nil } diff --git a/config/crd/bases/nebula.inftyai.com_nodeclaims.yaml b/config/crd/bases/nebula.inftyai.com_nodeclaims.yaml index dbc8b88..e039efe 100644 --- a/config/crd/bases/nebula.inftyai.com_nodeclaims.yaml +++ b/config/crd/bases/nebula.inftyai.com_nodeclaims.yaml @@ -29,6 +29,12 @@ spec: - jsonPath: .spec.capacityType name: CAPACITY_TYPE type: string + - jsonPath: .status.priceUSDPerHour + name: PRICE/HR + type: string + - jsonPath: .status.estimatedCostUSD + name: EST_COST + type: string - jsonPath: .status.phase name: Phase type: string @@ -141,16 +147,78 @@ spec: NodeClaimStatus is the durable record reconciled against the provider by the poll loop. properties: + costLabels: + additionalProperties: + type: string + description: |- + CostLabels attributes this claim's spend to whoever asked for it: the values the served + Pod carried for the label keys the operator configured (--cost-labels). Keyed by the POD + label key, verbatim — so this map reads like the Pod it came from. The metric emits under a + derived name Prometheus accepts ("example.com/org-id" here is example_com_org_id there), so + do not expect the two to match by string. + + Status rather than spec because it is observed from the Pod, and it sits with the rest of + the billing record for a reason: it is stamped in the SAME patch that opens the accrual + anchor, so no window can ever be charged before its attribution is known. + + Written once, on first observation, and never refreshed — relabeling a Pod must not + retroactively re-attribute spend already reported under the old values. A name the + current --cost-labels no longer lists is ignored rather than cleaned up; one it lists but + this map lacks reports as "none". + + Nil and EMPTY differ, which is why this field has no omitempty: nil means nothing has been + observed yet (or --cost-labels is unset), while an empty map is the settled fact that the + Pod carried none of the configured keys. With omitempty an empty map would not survive the + write, so every reconcile would re-read the Pod and a label added later could still move + the claim's attribution. + type: object endpoint: description: Endpoint is the reachable address (e.g. SSH host:port) once ready. type: string + estimatedCostUSD: + description: |- + EstimatedCostUSD is what this instance has cost SO FAR, as a decimal string ("412.94000000") + — PriceUSDPerHour integrated over the time it has held an instance. A string for the same + reason the rate is one: it exists to be read off a print column. + + More decimals than the rate, and they are not decoration: each checkpoint re-reads this field + to measure the next window, so digits dropped here are money dropped, and a cheap enough claim + would round back to where it started every minute and never accrue at all. Round it for + display; never round it back into this field. + + ESTIMATED, and the name says so on purpose: it is our own arithmetic over a list price + (see PriceUSDPerHour), not a figure any provider has confirmed. Nothing here has been + invoiced. Reconcile against the provider's billing export before anyone is charged. + + Within those limits it is the AUTHORITATIVE total, not the Prometheus counter: it is + written exactly once per window and survives a restart. It is also only LIVE cost — it + dies with the claim, so it is not a history. The metric is what outlives an instance. + type: string instanceID: description: |- InstanceID is the provider's identifier for the external instance (e.g. a RunPod pod id). This is the field that must not be lost: the terminate finalizer uses it to reclaim the instance. type: string + lastAccruedAt: + description: |- + LastAccruedAt is how far cost accrual has counted: an ANCHOR for the next measurement, + not a note about the last one. "Accrued" in the accounting sense — cost incurred but not + yet invoiced, which is all EstimatedCostUSD ever holds. + + It is the reason a restart loses nothing: the next window is rate x (now - LastAccruedAt), + so time that passed while Nebula was down is still counted on recovery instead of vanishing + with the in-memory total. + + The invariant that makes it safe: this NEVER moves past cost that has been durably + recorded, because it advances only in the same patch that writes EstimatedCostUSD. A + failed write therefore loses nothing — the same window is counted next time. + + Unset means "not counting yet". Never treat it as the epoch, which would charge decades + on the first tick. + format: date-time + type: string nodeName: description: NodeName is the virtual Node created for this instance, once it exists. @@ -158,6 +226,21 @@ spec: phase: description: Phase is the coarse lifecycle state. type: string + priceUSDPerHour: + description: |- + PriceUSDPerHour is what this instance costs per hour in USD, as a decimal string + ("7.9000"), resolved from the provider's catalog against the served Pod's shape. + Status, not spec: it is a derived result nobody can declare up front. A string + because it is written to be READ — a print column can only echo a field, never + scale a fixed-point integer back into currency. + + Empty means UNPRICED, not free: the provider implements no provider.Pricer, or its + catalog has no row for this candidate. Consumers must skip such a claim rather than + count it as $0. + + Written once and never refreshed, so a catalog edit cannot retroactively reprice a + running instance and rewrite the cost history it has already reported. + type: string type: object type: object served: true diff --git a/docs/architecture.md b/docs/architecture.md index 6fd467c..8cabea9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -568,7 +568,12 @@ Two design rules are worth knowing here, because they constrain the code above: `Handler.metricLabels` and `placementLabels` are field-for-field mirrors. - Every label is bounded by **configuration** (NodePools, provider catalogs), never by workload. Nothing derived from a Pod name, UID, namespace or unresolved user-supplied - pool label is ever a label value. + pool label is ever a label value. Cost is no exception, which is why per-instance spend + is a NodeClaim status field rather than a label on the counter. Cost *attribution* is the + single opt-in breach of that rule — `--cost-labels` promotes chosen Pod labels onto the + counter — so it ships off by default, and warns rather than capping when the series count + says an operator chose badly: a billing metric that rewrote its own labels to defend + itself would be wrong where nobody could see it. See [metrics.md](metrics.md) for the full series list, label semantics, example queries and the two known gaps. diff --git a/docs/metrics.md b/docs/metrics.md index dfeac5a..8b74572 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -19,12 +19,13 @@ placed (gate removed) nebula_placement_decisions_total instance accepted | nebula_instance_ready_duration_seconds v -Running +Running nebula_cost_usd_total <- dollars, while it bills ``` Those are the parts whose cost and failure modes are otherwise invisible: placement can -silently leave a Pod gated forever, and provisioning runs against a third party, takes -seconds to minutes, bills money, and fails for reasons the Pod status flattens away. +silently leave a Pod gated forever, provisioning runs against a third party, takes +seconds to minutes, bills money, and fails for reasons the Pod status flattens away, and +the instance it produces keeps charging whether or not anything is using it. Everything else is already covered elsewhere and deliberately not duplicated here — reconcile counts, queue depth and API latency by controller-runtime's own collectors, and Pod-population questions ("how many Pods are gated right now?") by kube-state-metrics. @@ -32,6 +33,8 @@ Pod-population questions ("how many Pods are gated right now?") by kube-state-me - [Where they are served](#where-they-are-served) - [Placement](#placement) - [Provisioning](#provisioning) +- [Cost](#cost) +- [Attribution](#attribution) - [Label semantics](#label-semantics) - [Example queries](#example-queries) - [Known gaps](#known-gaps) @@ -39,7 +42,8 @@ Pod-population questions ("how many Pods are gated right now?") by kube-state-me ## Where they are served Every collector registers into controller-runtime's registry (see `pkg/metrics`; the -`init` in each file is what registers them, so importing the package is the only wiring), +`init` in each file is what registers them, so importing the package is the only wiring — +except the cost counter, whose labels are not known until `--cost-labels` is parsed), which means they are served on the manager's existing `--metrics-bind-address` endpoint alongside the standard controller and workqueue metrics. In the default overlay that is `:8443` with authn/authz, so a scrape needs a bearer token whose subject is bound to the @@ -111,6 +115,221 @@ Fine-grained detail is deliberately *not* here: it stays where it is already ava (the Pod's `Failed` status message and the `vnode-handler` error log). These labels exist to answer "are we losing capacity, or are our credentials broken?" at a glance. +## Cost + +What the fleet has actually spent. + +| Metric | Type | What it answers | +| --- | --- | --- | +| `nebula_cost_usd_total{phase,...}` | counter | Dollars the fleet has run up, added one accrual window at a time. Also carries whatever `--cost-labels` names (see [Attribution](#attribution)). | + +```promql +# Dollars spent yesterday, by provider. +sum by (provider) (increase(nebula_cost_usd_total[1d])) + +# Recent spend, fleet-wide. Keep the range well above BOTH the accrual interval (1m) and the +# scrape interval: increase() needs two samples inside the range, so [1m] against a 60s scrape +# usually returns nothing at all. +sum(increase(nebula_cost_usd_total[5m])) + +# Burn rate in USD/hour. +sum(rate(nebula_cost_usd_total[30m])) * 3600 +``` + +**Per shape, not per instance.** One series per candidate — the labels in [Label +semantics](#label-semantics), plus `phase` — with no claim identity on it. That is what makes the +metric *billable* rather than merely observable: `increase(...[w])` is a pure function of the +window `w`, so a billing service replaying an old window re-derives the same dollars and can +upsert them idempotently. A per-claim counter would give up exactly that — differencing a +cumulative per-claim series needs a sample at each window boundary, and an instance that lived +and died between two boundaries has neither, so its spend would be unbillable. Per-claim spend +lives on the claim instead (below). + +Sharing series across claims narrows that problem rather than removing it, because `increase()` +still needs a sample from *before* the charge and a series' first sample has none. Counter series are +also **process-local**: a redeploy or a leader handoff starts the new process with none of them. So +when the accrual loop starts — which under leader election is the moment this process starts charging +— it publishes every already-anchored, priced claim's label set at `0` under all three billing +phases, a baseline for the first window any of them will book. The seeding pass is one interval ahead +of the first tick, so a scrape lands in between. + +That covers the fleet across a restart, which is where the money is. It does **not** cover a label +set appearing *after* the pass — a shape or a tenant seen for the first time mid-process — whose +first window is still booked onto a newborn series. See [Known gaps](#known-gaps). + +**Instance-level** cost — infrastructure spend, blind to the workload on top, charging each +claim's whole rate. + +**Every number here is an estimate.** Both the counter and the field are Nebula's own arithmetic +over a hand-maintained list price — cost *incurred*, which is what "accrued" means, not cost any +provider has confirmed. Nothing below has been invoiced. + +**One arithmetic, two views.** A leader-elected loop closes each claim's window every minute: +it charges `priceUSDPerHour × (now − status.lastAccruedAt)`, adds it to +`NodeClaim.status.estimatedCostUSD` (the `EST_COST` column) and re-anchors, then books the *same* +window on the counter. So the counter is the fleet's stream of charges and the field is the +per-claim rollup of them, and "what did this one instance cost" is a `kubectl get` rather than a +query: + +``` +NAME PROVIDER ACCELERATOR PRICE/HR EST_COST PHASE +nc-train-0 modal H100:8 23.7000 148.12500000 Bound +``` + +`EST_COST` carries eight decimals to the rate's four because each checkpoint measures its window +from the field it wrote last time, so digits dropped there are money dropped, not display noise: at +four decimals a claim under $0.003/hour rounded back to its previous value every minute and never +accrued at all. Round it when you show it. + +The counter is advanced only *after* the field's patch lands. A counter has no idempotency key, so +a window booked before its write was durable would be charged twice: the anchor would not have +moved, and the next tick would re-derive the same window. The persisted anchor is also why no time +is lost to a restart — the first tick back charges the whole gap, downtime included — and why the +window is measured from `status`, not from how long the loop actually slept. + +**Teardown closes the last window too.** No checkpoint can close it — the object is going away, so +there is nothing left to re-anchor — but it is still booked on the counter, gated by the same rule: +strictly after the finalizer removal is accepted. That `Update` is a compare-and-swap, so it lands +at most once and only for a claim whose ledger is current, which is the same exactly-once guarantee +the anchor gives the checkpoint path. Without it the metric would miss far more than one window: an +instance that died before its first accrual tick would be billed **nothing at all**, making short +workloads look free while long ones stayed accurate. + +`EST_COST` does *not* get that last window, so it understates a reclaimed instance's lifetime by up +to one interval while the counter has the whole of it. The lifetime figure also goes to the log +(`claim finalized`), the only record that outlives both the object and Prometheus retention. + +**Where the number comes from.** `NodeClaim.status.priceUSDPerHour`, resolved from the +provider's catalog (`pkg/provider/catalog/data/*.csv`) against the served Pod's shape and +written once, on the claim's first reconcile — pinned so a catalog edit cannot retroactively +reprice a running instance. These are hand-maintained list prices: no committed-use discounts, +no private pricing, no gap between a Spot quote and the actual charge. Reconcile against the +provider's billing export before anyone gets invoiced. + +**Which claims are charged.** Only `Bound` and `Terminating` hold an instance (see +`NodeClaimPhase`), so only they accrue; a `Terminated` claim's `EST_COST` is its frozen final total. +`Provisioning` is excluded, undercounting by about one poll interval per instance. + +An instance that ends on its own — a preemption, a crashed sandbox — reaches `Terminated` without +passing through `Terminating`, and the loop will not touch it again. Teardown books the time since +its last checkpoint anyway, under `phase="Terminated"`, capped at one accrual interval: the claim +then waits in that phase until its Pod is deleted, and charging the wait would bill idle hours at GPU +rates. So that series is a real one, accurate to ±1 interval, and it separates spend that ended by +itself from spend we ended. + +`Terminating` still bills until teardown finishes, which is what makes the `phase` label worth +having: + +```promql +# Dollars burned on instances whose workload was already gone. Rising steadily means the +# teardown backstop is stuck. +sum(increase(nebula_cost_usd_total{phase="Terminating"}[1d])) +``` + +A claim with no usable price (no `Pricer`, or no catalog row) accrues **nothing**, never `0` — a +zero would be summed and averaged as a real "this costs nothing". Fleet totals therefore +under-report by whatever cannot be priced; an empty `PRICE/HR` in `kubectl get nc` finds those +claims. + +## Attribution + +Who to charge. Off by default; `--cost-labels` turns it on: + +``` +--cost-labels=example.com/org-id,example.com/team-id +``` + +Each entry is a **Pod label key**, written exactly as the Pod carries it. The metric label is +*derived* from it: the whole key, with `/`, `-` and `.` folded to `_`. + +| `--cost-labels` entry | Pod label read | PromQL label | +| --- | --- | --- | +| `example.com/org-id` | `example.com/org-id` | `example_com_org_id` | +| `team.id` | `team.id` | `team_id` | +| `org_id` | `org_id` | `org_id` | + +So a key is configured the way Kubernetes spells it and emitted under a name Prometheus accepts, +with nothing thrown away in between — the same convention kube-state-metrics +(`label_app_kubernetes_io_name`) and Prometheus service discovery +(`__meta_kubernetes_pod_label_…`) follow. Keeping the prefix is what makes `a.com/org-id` and +`b.com/org-id` two distinct series rather than a collision, and what keeps a qualified key from +quietly shadowing a label the metric already carries. + +Rejected at startup: a key Kubernetes would not accept, one whose derived name Prometheus would not +(anything starting with a digit — a bare `2team`, or a domain like `4paradigm.com/org-id`; use an +unqualified Pod label there), the same key twice, two keys folding to the same name (`org-id` and +`org.id` still meet), and a bare key that shadows a label the metric already carries (`provider`, +`region`, `phase`, …). Every one of those fails the process at boot instead of silently reporting +every tenant as `none`. + +Two names nothing rejects but you should still avoid: **`job` and `instance`**. Prometheus attaches +its own at scrape time, and with the default `honor_labels: false` it renames yours to +`exported_job` / `exported_instance` — so `sum by (job)` would quietly report the scrape job instead +of the tenant, with no error anywhere. `job_id` or `workload` if that is the breakdown you want. + +The values are read off the served Pod once — when the claim first becomes chargeable, in the same +status patch that opens its billing window — and pinned on `NodeClaim.status.costLabels`, keyed by +the **Pod** key rather than the derived one, so the record reads like the Pod it came from: + +```yaml +status: + costLabels: + example.com/org-id: acme + example.com/team-id: ml +``` + +Three things follow from pinning them there rather than reading the Pod at accrual time: + +- **Spend cannot be re-attributed.** Relabelling a Pod does not move dollars already reported + under another tenant, exactly as a catalog edit cannot reprice a running instance. +- **The last window is attributable.** A `Terminating` claim's Pod is often already gone, and that + window is [booked at teardown](#cost) — from `status`, which is still there. +- **The breakdown is auditable per claim.** `kubectl get nodeclaim -o yaml` shows who a given + instance was charged to, which no aggregate metric can answer. + +A claim serves exactly one Pod, UID-pinned (`spec.podRef`), so its whole rate belongs to that one +workload: nothing is split, and nothing is counted twice. + +```promql +# Yesterday's bill, per tenant. +sum by (example_com_org_id) (increase(nebula_cost_usd_total[1d])) + +# One team's burn rate, in USD/hour. +sum(rate(nebula_cost_usd_total{example_com_org_id="acme",example_com_team_id="ml"}[30m])) * 3600 +``` + +**Values are tenant-controlled, which is a cardinality risk** — and the only one on this endpoint, +since every other label is bounded by configuration. A counter never releases a series, so a +workload generator emitting a fresh `org_id` per Pod leaks one series per Pod for the life of the +process. **Nothing caps this.** Past 5000 cost series the manager logs a warning once: + +``` +WARNING: the cost metric has passed its expected series budget, which usually means an +attribution label is carrying a per-Pod value. Nothing is dropped or merged, so the dollars +stay correct, but memory and every scrape grow until this process restarts. +``` + +Warning rather than enforcing is deliberate. Merging tenants into an `overflow` bucket, or dropping +the window, each corrupts a metric an external billing service reads as truth — and silently, in +the one direction its consumer cannot detect. A metric that is too *big* is an operator's problem +with an obvious fix (constrain the values at admission — a webhook or a policy engine); a metric +that quietly rewrote its own labels is nobody's problem until invoicing. + +Prometheus knows the real number, so alert on it there rather than trusting the log line to be seen: + +```promql +count(nebula_cost_usd_total) > 5000 +``` + +A Pod that carries none of the configured labels reports `none`, the same placeholder every other +absent label uses. A Pod that carries the label with an *empty* value reports `none` too: an empty +tenant id names no payer, so it is folded into the unattributed pool rather than splitting it. + +**Changing `--cost-labels` changes the identity of every cost series.** Adding a name resets each +one to zero and starts a new set, which `increase()` reads as a reset and handles, but no +historical series will carry the new label. Roll it out at a boundary you are happy to see in a +dashboard. + ## Label semantics Every series except the `{pool,reason}` and `{provider,capacity_type,region,reason}` @@ -120,9 +339,10 @@ diagnostics carries the same label set: provider region capacity_type accelerator accelerator_count ``` -That is on purpose: a placement and the provisioning attempt it led to carry **identical -label values**, so the two join in PromQL without label surgery — "placed on Spot but -never provisioned" is one query. For the same reason the two duration histograms measure +That is on purpose: a placement, the provisioning attempt it led to, and the cost of the +instance it produced carry **identical label values**, so they join in PromQL without label +surgery — "placed on Spot but never provisioned" is one query, and so is "what did our +Spot fallbacks cost us". For the same reason the two duration histograms measure adjacent legs of one journey: `placement_wait` ends exactly where `instance_ready` begins, so together they cover `kubectl apply` to `Running`. @@ -132,7 +352,8 @@ Five label values are load-bearing: (unconstrained), no capacity tier (the provider's default), no accelerator (a CPU-only Pod). An explicit token beats an empty string, which in PromQL is indistinguishable from a label that was never set and silently matches `{region=""}` selectors nobody meant to - write. + write. It is a plain word rather than an unforgeable token, which a real value can therefore + collide with — see [Known gaps](#known-gaps). - **`pool`** on the deferral counter is only ever the name of a NodePool that *exists*, or `none`. The pool a Pod asks for is a Pod label — user-controlled and unbounded — so the `no_pool` deferral files `none` rather than the unresolved string; a mislabeled workload @@ -154,9 +375,14 @@ Five label values are load-bearing: than in the label. A spike here alongside flat `capacity`/`auth` series is the shape of a network or provider outage. -Cardinality is bounded by *configuration*, not by workload: providers x regions x tiers x -accelerator pools, all of which come from NodePools and provider catalogs. Nothing -derived from a Pod name, UID or namespace is ever a label. +Cardinality is bounded by *configuration*, not by workload, on every label above: providers x +regions x tiers x accelerator pools, all of which come from NodePools and provider catalogs. That +is what makes it safe that they are in-process counters whose series live until the process exits — +including cost, which is why it carries no claim identity. + +The one exception is [attribution](#attribution), whose values come from Pod labels. It is off by +default, and warns rather than caps when on — so if you enable it, the bound is whatever your +admission policy puts on those label values. ## Example queries @@ -185,16 +411,87 @@ sum by (provider) (rate(nebula_provision_failures_total{reason="other"}[6h])) # Requests nobody is retrying their way out of: a human has to fix these. sum by (reason) (rate(nebula_placement_deferrals_total{reason=~"no_pool|invalid_request"}[1h])) + +# What the Spot fallback actually cost over the last week: the same candidate labels, so this +# joins the placement decision to the bill. +sum by (capacity_type) (increase(nebula_cost_usd_total[7d])) + +# Dollars per placement, by candidate. Rising without a price change means instances are +# being held longer per workload. +sum by (provider, accelerator) (increase(nebula_cost_usd_total[1d])) + / sum by (provider, accelerator) (increase(nebula_placement_decisions_total[1d])) + +# Current burn rate in USD/hour, by accelerator. +sum by (accelerator) (rate(nebula_cost_usd_total[30m])) * 3600 ``` ## Known gaps -Both are deliberate, and both bias toward looking *better* than reality — worth knowing -before trusting a dashboard. - -- **Counters reset on restart.** Every collector is in-process. This is ordinary Prometheus - semantics — `rate()` and `increase()` detect resets — but no cumulative history survives - a redeploy; the scrape backend owns durability. +All of these are deliberate, and all bias toward looking *better* than reality — worth +knowing before trusting a dashboard. + +- **Counters reset on restart, and lose their unscraped tail.** Every counter and histogram here is + in-process, cost included: a redeploy or a leader handoff starts it at zero. `rate()` and + `increase()` detect the reset, but the increments booked *since the last scrape* were never + observed by anyone, and for cost they cannot be re-booked — the next leader measures from + `status.lastAccruedAt`, which has already advanced past them. So an unclean exit drops up to one + scrape interval of spend from the counter, and a crash landing between a checkpoint's write and its + booking drops that window too. **`status.estimatedCostUSD` is what survives this** — cross-check + against it after a redeploy rather than treating the counter as durable. Aggregate as + `sum(increase(...))`, never `increase(sum(...))`: only the former sees the per-series reset. +- **A series' first charge is invisible to `increase()`.** A counter's first sample carries no + information — `increase()` recovers a *rise* between two samples — so dollars that arrive on a + series' first sample are in `nebula_cost_usd_total` but in no `increase()` or `rate()` query over + it. The baselines seeded when the accrual loop starts (see [Cost](#cost)) cover every claim already + billing, so a restart does not cost the fleet a window. Two cases remain, both of them worse with + `--cost-labels` on, where a series belongs to one tenant rather than a whole shape: + - a **label set first seen mid-process** — a new tenant, or a shape nothing was running on at + startup — gets no baseline at all, so its first window is lost to `increase()` for the life of + that process. Nothing re-seeds; the accrual loop seeds once. + - an **instance born and gone inside one scrape interval**, which no baseline can help: the + baseline and the charge land in the same scrape either way. + + Both bias toward *undercounting*, and a tenant whose only job is short can read as zero spend. + Cross-check against `status.estimatedCostUSD` and the `claim finalized` log, which are the durable + records. Closing this properly means a durable per-window event stream, not a counter. +- **`EST_COST` understates a reclaimed instance; the metric does not.** The field is not written on + the deletion path (the object is going away), so it misses the window still open at teardown — + which the counter *does* book (see [Cost](#cost)). The two therefore disagree by up to one + interval on any claim that has been reclaimed, the counter being the complete figure and the log + line agreeing with it. `EST_COST` also lags live spend by up to one interval while a claim is + running — a freshness limit, not an error, since a window not charged now is charged in full next + tick. +- **The last window of a self-terminated instance is capped, not measured.** A `Terminated` claim's + anchor froze when the instance died, and nothing since distinguishes "died a minute ago" from + "died while the manager was down three hours ago", so teardown charges one interval either way + (see [Cost](#cost)). The cap is the safe direction — it cannot bill idle hours — but a preemption + during an outage is undercharged by the whole outage. Nothing is booked at all if the Pod object + never goes away, since teardown is what triggers it. +- **A crash between the teardown write and the booking loses that window.** The final window is + booked in-process right after the finalizer removal lands, and unlike a checkpoint it has no + anchor to fall back on, so a process that dies in between charges it nowhere. Exactly-once in the + direction that matters — it cannot double-charge — but it can drop one partial window per + unlucky teardown. +- **Cost is a list price, and misses two windows** — `Provisioning` claims, and any claim the + provider cannot price. See [Cost](#cost); reconcile against the billing export before + invoicing anyone. +- **Attribution is only as good as the Pod's labels, and is frozen.** A Pod that was missing its + `org_id` when the claim became chargeable is booked to `none` for its whole life; labelling it + afterwards does not backfill, by design (see [Attribution](#attribution)). Enforce the labels at + admission — a webhook or a policy engine — rather than trusting the metric to notice. +- **`none` is a placeholder a real value can forge.** A Pod whose attribution label reads literally + `none` shares a series with every Pod that carries no such label, merging that tenant's spend into + the unattributed pool. The alternatives were each worse in a way that mattered more: an unforgeable + token (``) is unreadable and vanishes wherever a label value reaches HTML, and the empty + string is what PromQL uses for "label absent", so `{org_id=""}` could no longer select the + unattributed bucket on its own. Pick attribution keys whose values your admission policy + constrains. +- **Attribution cardinality is unbounded.** Enabling `--cost-labels` promotes tenant-controlled + values onto a counter that never releases a series, and nothing caps it: a bad label choice grows + manager memory and the scrape payload until a restart. The 5000-series warning is a smoke alarm, + not a limit — see [Attribution](#attribution) for why it does not enforce, and alert on + `count(nebula_cost_usd_total)`. Budget three series per distinct label set, not one: the baselines + above are seeded for all three billing phases, and a claim only ever spends under two of them. - **`instance_ready_duration` under-samples slow boots.** The start timestamp lives only in the virtual node's in-memory tracking map, so a provision still in flight when the manager restarts is re-adopted without one and is never observed. A missing sample beats diff --git a/internal/controller/cost_accrual.go b/internal/controller/cost_accrual.go new file mode 100644 index 0000000..87f6c1f --- /dev/null +++ b/internal/controller/cost_accrual.go @@ -0,0 +1,389 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "math" + "strconv" + "time" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" + + nebulav1alpha1 "github.com/InftyAI/Nebula/api/v1alpha1" + "github.com/InftyAI/Nebula/pkg/metrics" + "github.com/InftyAI/Nebula/pkg/util" +) + +// accrualInterval is how often spend is checkpointed. It is a WRITE cadence, not an accounting +// resolution: every window is measured from the persisted anchor, so a longer interval trades +// freshness of the EST_COST column for fewer writes, never accuracy. One minute is ~8.3 writes/s +// across a 500-claim fleet, about a sixth of the client's rate budget (see the QPS in cmd/main.go) +// — the point where this stops being free and starts competing with the reconcilers for it. +const accrualInterval = time.Minute + +// accrualTimeout bounds one whole tick, List plus every write. A tick that cannot finish loses +// nothing: the anchors it did not reach are still where they were, so the next tick charges the +// same windows. +// +// Now equal to accrualInterval, which is safe rather than tidy: ticks run sequentially and a +// time.Ticker drops the ticks a slow receiver missed instead of queueing them, so the worst case is +// back-to-back ticks, and re-deriving a window from its anchor cannot double-charge it. +const accrualTimeout = time.Minute + +// CostAccrual advances each claim's durable spend ledger on a ticker. +// +// A Runnable rather than a hook on the reconcile path: spend accrues with the CLOCK, not with +// events, and a Bound claim can sit for hours without a reconcile. Leader election is the +// manager's default for a plain Runnable and is load-bearing here — two replicas accruing the +// same fleet would double every dollar. +// +// It lives beside the reconciler rather than in pkg/metrics because it WRITES: the ledger is a +// status field, and instrumentation that patches API objects is no longer instrumentation. +type CostAccrual struct { + client.Client + interval time.Duration + // now is overridable so tests can drive whole windows without sleeping. + now func() time.Time +} + +var _ manager.Runnable = (*CostAccrual)(nil) + +// NewCostAccrual builds the accrual loop over the manager's client: cached reads, direct writes. +func NewCostAccrual(c client.Client) *CostAccrual { + return &CostAccrual{Client: c, interval: accrualInterval, now: time.Now} +} + +// Start accrues until ctx is cancelled. Unlike a loop that charges the time between its own +// ticks, this one is anchored per claim in status, so a tick that is late, skipped, or the first +// after a restart all charge exactly the time that really passed. The window still open at +// shutdown is not lost either — the next process charges it. +func (a *CostAccrual) Start(ctx context.Context) error { + // An idle fleet writes nothing and emits no series at all, which is indistinguishable from a + // loop that never started. This line is the only evidence either way. + log := logf.Log.WithName("cost-accrual") + log.Info("starting cost accrual", "interval", a.interval) + + a.seedBaselines(ctx) + + ticker := time.NewTicker(a.interval) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return nil + case <-ticker.C: + a.accrueAll(ctx) + } + } +} + +// seedBaselines publishes a zero-valued series for every claim that was already chargeable when +// this process took over, before the first tick can charge one. +// +// Counter series are process-local, so without this the first post-restart window of the WHOLE FLEET +// lands on a newborn series, which increase() cannot difference (see metrics.TouchSeries). Prometheus +// recovers some of that by reset detection, but only for an unchanged scrape target: a redeploy's new +// Pod carries a new instance label, and a handoff to another replica is a different target entirely. +// +// Its position in Start is both the once-per-process guard and the ordering that matters — leader +// election makes it the moment this process starts charging, and the first tick is an interval later, +// so a scrape lands between a baseline and the window it explains. A claim that becomes chargeable +// after this pass gets no baseline; see docs/metrics.md, Known gaps. +// +// Best-effort: a failed List costs one window's visibility, never the window itself, which the anchor +// still carries. +func (a *CostAccrual) seedBaselines(ctx context.Context) { + log := logf.Log.WithName("cost-accrual") + + ctx, cancel := context.WithTimeout(ctx, accrualTimeout) + defer cancel() + + var claims nebulav1alpha1.NodeClaimList + if err := a.List(ctx, &claims); err != nil { + log.Error(err, "listing nodeclaims to seed cost baselines") + return + } + seeded := 0 + for i := range claims.Items { + nc := &claims.Items[i] + // finalRate rather than billingRate: a Terminated claim still books a window when its Pod + // is finally deleted (see settleFinalCost), so it needs a baseline as much as a live one. + if _, ok := finalRate(nc); !ok || nc.Status.LastAccruedAt == nil { + continue + } + metrics.TouchSeries(claimLabels(nc), nc.Status.CostLabels, billingPhases...) + seeded++ + } + log.Info("seeded cost baselines", "claims", seeded) +} + +// accrueAll checkpoints every billing claim once. Errors are logged per claim and never +// propagated: one unwritable claim must not cost the rest of the fleet its window. +func (a *CostAccrual) accrueAll(ctx context.Context) { + log := logf.Log.WithName("cost-accrual") + + ctx, cancel := context.WithTimeout(ctx, accrualTimeout) + defer cancel() + + var claims nebulav1alpha1.NodeClaimList + if err := a.List(ctx, &claims); err != nil { + log.Error(err, "listing nodeclaims to accrue") + return + } + for i := range claims.Items { + nc := &claims.Items[i] + if err := a.accrue(ctx, nc); err != nil { + // A conflict is the ordinary case — the reconciler patched the same claim from its + // own copy. The anchor did not move, so this window is simply charged next tick. + if apierrors.IsConflict(err) || apierrors.IsNotFound(err) { + log.V(1).Info("skipping accrual this tick", "claim", nc.Name, "reason", err.Error()) + continue + } + log.Error(err, "accruing claim cost", "claim", nc.Name) + } + } +} + +// accrue persists what the claim has cost as of now and re-anchors there, in one patch. +// +// It decides WHETHER to write; costNow decides what the number is. The pair of fields being one +// patch is what makes it crash-safe: the anchor can never +// advance past money that was recorded, so an interrupted tick re-charges the same window rather +// than skipping it. +// +// Charging is also idempotent per unit time — the amount follows from the anchor, not from how +// many ticks fire — and Update carries a resourceVersion, so a writer working from a stale ledger +// is rejected outright. Together that makes a second, unelected accruer wasteful, not harmful. +func (a *CostAccrual) accrue(ctx context.Context, nc *nebulav1alpha1.NodeClaim) error { + if _, ok := billingRate(nc); !ok { + // Not billing. Note this is stricter than costNow, which still reports a Terminated + // claim's frozen total: nothing more can be charged, so writing it again is pure churn. + return nil + } + // Second precision, because that is all metav1.Time serializes. Reading the anchor back + // truncated while having charged from the untruncated instant would re-charge the dropped + // fraction every tick — a small but systematic OVER-count, the one direction this must + // never drift. + now := a.now().Truncate(time.Second) + + // An anchor in the future — a wall-clock jump, or a hand-edited field. Charging the negative + // window would rewind the ledger. Left in place rather than pulled back to now, so billing + // resumes by itself once the clock passes it, having lost only the bogus window. + if at := nc.Status.LastAccruedAt; at != nil && !now.After(at.Time) { + return nil + } + // A claim with no anchor at all — one that became billable before this build, or whose Bound + // patch did not carry the stamp — falls through with nothing added: the window before this + // point has an unknown length, and guessing it would invent spend. Opening one here is the + // whole write. + prev := costSoFar(nc) + total, _ := costNow(nc, now) + nc.Status.EstimatedCostUSD = formatCost(total) + nc.Status.LastAccruedAt = &metav1.Time{Time: now} + if err := a.Status().Update(ctx, nc); err != nil { + return err + } + // Booked only now that the window is durable — see metrics.RecordWindow. Re-reading the + // field instead of using total charges the counter the same ROUNDED dollars the ledger took, + // so the two reconcile exactly rather than drifting a fraction of a cent per tick. + metrics.RecordWindow(claimLabels(nc), string(nc.Status.Phase), nc.Status.CostLabels, costSoFar(nc)-prev) + return nil +} + +// billingRate returns the claim's hourly rate and whether it should be charged at all. +// +// Two gates, and both are omissions rather than zeros — a zero would be summed and averaged as +// a real "this costs nothing", while absent spend is honestly unknown: +// +// - Phase. Only Bound and Terminating hold an instance that exists (see NodeClaimPhase). +// Terminating still bills until teardown finishes. Provisioning is excluded and undercounts +// by roughly one poll tick, the same lag the phase itself carries. +// - Price. Empty means UNPRICED, not free (see Status.PriceUSDPerHour): no Pricer, or no +// catalog row. An unparseable value is a corrupted claim and is treated the same. +func billingRate(nc *nebulav1alpha1.NodeClaim) (float64, bool) { + switch nc.Status.Phase { + case nebulav1alpha1.NodeClaimBound, nebulav1alpha1.NodeClaimTerminating: + default: + return 0, false + } + return finalRate(nc) +} + +// billingPhases is every phase a window can be booked under: the two billingRate admits, plus +// Terminated for the last window settleFinalCost closes. seedBaselines publishes all three up front +// rather than guessing: one always stays empty, since a reclaimed instance passes through Terminating +// and a self-terminated one does not, and which of the two it will be is not knowable in advance. +var billingPhases = []string{ + string(nebulav1alpha1.NodeClaimBound), + string(nebulav1alpha1.NodeClaimTerminating), + string(nebulav1alpha1.NodeClaimTerminated), +} + +// finalRate is billingRate without the phase gate. Terminated is excluded there because a claim +// sits in that phase until its Pod is deleted and charging those idle hours would be wrong — but the +// window between the last checkpoint and the instance's death is real, and settleFinalCost is the +// only place left that can book it. Nothing may use this to decide that a claim ACCRUES; seedBaselines +// is the one other caller, and it only asks whether a window could ever be booked. +func finalRate(nc *nebulav1alpha1.NodeClaim) (float64, bool) { + rate, err := strconv.ParseFloat(nc.Status.PriceUSDPerHour, 64) + // ParseFloat accepts "NaN" and "Inf" without an error, and both slip past rate <= 0. Left + // unchecked, one such value reaches the ledger, and from there it is unrecoverable: the total + // is re-read every tick, so NaN + x stays NaN even after the price is fixed. + if err != nil || math.IsNaN(rate) || math.IsInf(rate, 0) || rate <= 0 { + return 0, false + } + return rate, true +} + +// costSoFar reads the running total. An unparseable value is treated as zero rather than as a +// reason to stop accruing: losing the history of one corrupted claim beats freezing its ledger +// and under-reporting the fleet forever. +func costSoFar(nc *nebulav1alpha1.NodeClaim) float64 { + total, err := strconv.ParseFloat(nc.Status.EstimatedCostUSD, 64) + // Non-finite is what makes the zero here load-bearing rather than tidy: a ledger already + // holding "NaN" (see finalRate) would otherwise stay poisoned for the claim's whole life, + // since every later total is derived from this read. + if err != nil || math.IsNaN(total) || math.IsInf(total, 0) || total < 0 { + return 0 + } + return total +} + +// costDecimals is how many fractional digits the LEDGER keeps — more than priceDecimals, because a +// rate is an input that is written once while a total is an accumulator that is re-read and +// re-written every minute. Rounding it at each checkpoint would quantize the effective rate onto the +// grid: the residue is discarded rather than carried, so a claim cheaper than half a grid step per +// window freezes forever (at four digits, anything under $0.003/hr — a Modal CPU-only sandbox), and +// one just above it is charged the rounded-UP step every tick. Eight digits puts that error below +// 0.01% at any rate a catalog carries, at the cost of a wider EST_COST column. +const costDecimals = 8 + +func formatCost(usd float64) string { + return strconv.FormatFloat(usd, 'f', costDecimals, 64) +} + +// stampAccrualStart opens the billing window the moment a claim first becomes chargeable, +// returning whether it mutated the claim. +// +// Load-bearing rather than an optimisation: it is what makes a crash BEFORE the first +// checkpoint lossless. The anchor is what recovery measures from, so without it a claim that +// billed for 90 seconds and then lost its manager would be charged from whenever the next tick +// happened to find it. Free, because it rides the status patch markPhase is already making. +func stampAccrualStart(nc *nebulav1alpha1.NodeClaim) bool { + if nc.Status.LastAccruedAt != nil { + return false + } + if _, ok := billingRate(nc); !ok { + return false + } + nc.Status.LastAccruedAt = &metav1.Time{Time: time.Now().Truncate(time.Second)} + return true +} + +// settleFinalCost books the window still open at teardown and logs what the instance cost over its +// whole life. +// +// Booking is safe here despite there being no anchor left to move, because the caller has already +// had its finalizer removal ACCEPTED: that Update is a compare-and-swap, so it lands at most once, +// and only for a copy of the claim whose ledger is current — a stale anchor would have conflicted +// there rather than re-charging time the loop already took. Skipping it would lose far more than +// one window: an instance that died before its first accrual tick would be charged NOTHING at all. +// +// The lifetime figure itself only goes to the log; status is deleted with the object. +func settleFinalCost(ctx context.Context, nc *nebulav1alpha1.NodeClaim) { + var final float64 + if rate, ok := finalRate(nc); ok && nc.Status.LastAccruedAt != nil { + open := time.Since(nc.Status.LastAccruedAt.Time) + // Clamped in Terminated alone: its anchor froze when the instance died and the claim may + // have sat for days since. A Terminating claim is still running, so its whole open window is + // real money — including hours recovered after a manager outage. + if nc.Status.Phase == nebulav1alpha1.NodeClaimTerminated && open > accrualInterval { + open = accrualInterval + } + if open > 0 { + final = rate * open.Hours() + } + } + // Zero for a claim that was never priced or never anchored, which RecordWindow drops rather + // than minting a series for. + metrics.RecordWindow(claimLabels(nc), string(nc.Status.Phase), nc.Status.CostLabels, final) + logf.FromContext(ctx).Info("claim finalized", "phase", nc.Status.Phase, + "totalTimeCostUSD", formatCost(costSoFar(nc)+final)) +} + +// costNow is what a claim has cost as of now: the persisted ledger plus the window still open +// since its anchor. False means the claim has no cost to report AT ALL — neither billable nor +// ever charged, which is absent spend rather than zero spend (see billingRate). +func costNow(nc *nebulav1alpha1.NodeClaim, now time.Time) (float64, bool) { + rate, billing := billingRate(nc) + total := costSoFar(nc) + if !billing && total == 0 { + return 0, false + } + if billing && nc.Status.LastAccruedAt != nil { + if open := now.Sub(nc.Status.LastAccruedAt.Time); open > 0 { + total += rate * open.Hours() + } + } + return total, true +} + +// claimLabels renders the candidate a claim was provisioned against, so its cost carries the same +// label values as the placement and provisioning attempts that produced it and the three join +// without label surgery. +func claimLabels(nc *nebulav1alpha1.NodeClaim) metrics.Labels { + accelerator, count := util.SplitAcceleratorPool(nc.Spec.Accelerator) + return metrics.Labels{ + Provider: nc.Spec.Provider, + Region: nc.Spec.Region, + CapacityType: string(nc.Spec.CapacityType), + Accelerator: accelerator, + AcceleratorCount: count, + } +} + +// stampCostLabels copies the configured attribution labels off the served Pod, returning whether +// it mutated the claim. +// +// Written once and never refreshed, for the same reason the price is: spend already reported +// under one tenant must not be re-attributed to another by an edit to a Pod label. It runs +// alongside stampAccrualStart in one patch, so attribution is durable before the first window it +// has to explain. +func stampCostLabels(nc *nebulav1alpha1.NodeClaim, pod *corev1.Pod) bool { + keys := metrics.CostLabelKeys() + if nc.Status.CostLabels != nil || pod == nil || len(keys) == 0 { + return false + } + stamped := map[string]string{} + for _, key := range keys { + if v := pod.Labels[key]; v != "" { + stamped[key] = v + } + } + // An empty map, not nil: a Pod carrying none of the configured labels is a settled fact, and + // nil would make every later reconcile look it up again. Durable only because the field has no + // omitempty — see NodeClaimStatus.CostLabels. + nc.Status.CostLabels = stamped + return true +} diff --git a/internal/controller/cost_accrual_test.go b/internal/controller/cost_accrual_test.go new file mode 100644 index 0000000..6a73f30 --- /dev/null +++ b/internal/controller/cost_accrual_test.go @@ -0,0 +1,789 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "errors" + "math" + "reflect" + "strconv" + "strings" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus/testutil" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/event" + + nebulav1alpha1 "github.com/InftyAI/Nebula/api/v1alpha1" + "github.com/InftyAI/Nebula/pkg/metrics" + "github.com/InftyAI/Nebula/pkg/provider" +) + +// billingClaim is a claim holding a priced instance, anchored ago in the past. A nil ago leaves +// the anchor unset, i.e. never checkpointed. +func billingClaim(name, price string, ago *time.Duration) *nebulav1alpha1.NodeClaim { + nc := &nebulav1alpha1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: nebulav1alpha1.NodeClaimSpec{Provider: "modal", Accelerator: "H100:1"}, + Status: nebulav1alpha1.NodeClaimStatus{ + Phase: nebulav1alpha1.NodeClaimBound, + PriceUSDPerHour: price, + }, + } + if ago != nil { + nc.Status.LastAccruedAt = &metav1.Time{Time: time.Now().Add(-*ago).Truncate(time.Second)} + } + return nc +} + +// newAccrual wires the loop over a fake client seeded with claims, with the clock pinned so a +// whole window can be charged without sleeping. +func newAccrual(t *testing.T, claims ...*nebulav1alpha1.NodeClaim) (*CostAccrual, client.Client) { + t.Helper() + metrics.CostTotal.Reset() + + objs := make([]client.Object, 0, len(claims)) + for _, nc := range claims { + objs = append(objs, nc) + } + c := fake.NewClientBuilder(). + WithScheme(testScheme(t)). + WithObjects(objs...). + WithStatusSubresource(&nebulav1alpha1.NodeClaim{}). + Build() + return NewCostAccrual(c), c +} + +// ledger reads back the persisted total and anchor. +func ledger(t *testing.T, c client.Client, name string) (float64, *metav1.Time) { + t.Helper() + var nc nebulav1alpha1.NodeClaim + if err := c.Get(context.Background(), client.ObjectKey{Name: name}, &nc); err != nil { + t.Fatalf("get claim %q: %v", name, err) + } + if nc.Status.EstimatedCostUSD == "" { + return 0, nc.Status.LastAccruedAt + } + total, err := strconv.ParseFloat(nc.Status.EstimatedCostUSD, 64) + if err != nil { + t.Fatalf("status.estimatedCostUSD %q is not a number: %v", nc.Status.EstimatedCostUSD, err) + } + return total, nc.Status.LastAccruedAt +} + +// booked is the dollars the cost counter holds across every series. +func booked(t *testing.T) float64 { + t.Helper() + if testutil.CollectAndCount(metrics.CostTotal) == 0 { + return 0 + } + return testutil.ToFloat64(metrics.CostTotal) +} + +func TestCostAccrual_ChargesFromTheAnchor(t *testing.T) { + halfHour := 30 * time.Minute + a, c := newAccrual(t, billingClaim("bound", "98.3200", &halfHour)) + + a.accrueAll(context.Background()) + + want := 98.32 * 0.5 + total, anchor := ledger(t, c, "bound") + if math.Abs(total-want) > 1e-3 { + t.Fatalf("status.estimatedCostUSD %v, want %v", total, want) + } + // The same window reaches the counter, so a consumer's increase() and the field agree. + if got := booked(t); math.Abs(got-total) > 1e-9 { + t.Fatalf("booked %v, want %v — the counter must take exactly what the ledger did", got, total) + } + if time.Since(anchor.Time) > time.Minute { + t.Fatalf("anchor was not moved forward: %v", anchor) + } +} + +// The whole point of persisting a timestamp: a window that spans a restart is charged in full on +// the first tick back, not clipped to one interval. +func TestCostAccrual_RecoversDowntime(t *testing.T) { + down := 3 * time.Hour + a, c := newAccrual(t, billingClaim("bound", "10.0000", &down)) + + a.accrueAll(context.Background()) + + if total, _ := ledger(t, c, "bound"); math.Abs(total-30) > 1e-2 { + t.Fatalf("status.estimatedCostUSD %v after a 3h gap, want 30 — the downtime was not recovered", total) + } +} + +// Successive checkpoints accumulate on the field rather than replacing it. +func TestCostAccrual_AccumulatesAcrossTicks(t *testing.T) { + hour := time.Hour + a, c := newAccrual(t, billingClaim("bound", "4.0000", &hour)) + + // First tick charges the hour; the second, driven by a clock pushed 30 minutes on, charges + // the half hour since. + a.accrueAll(context.Background()) + a.now = func() time.Time { return time.Now().Add(30 * time.Minute) } + a.accrueAll(context.Background()) + + if total, _ := ledger(t, c, "bound"); math.Abs(total-6) > 1e-2 { + t.Fatalf("status.estimatedCostUSD %v, want 6 (1h + 0.5h at $4/hr)", total) + } +} + +// Every checkpoint re-reads the ledger to measure the next window, so the digits it keeps ARE the +// accounting resolution — see costDecimals. Driven at the real cadence, a cheap claim is where that +// shows: at four decimals $0.0028/hr froze at $0.0001 and never moved again, while $0.01/hr was +// charged a rounded-up step every minute and ran 20% over. +func TestCostAccrual_ChargesCheapClaimsAtTheirRealRate(t *testing.T) { + // Modal's CPU-only components: a 20-millicore reservation, a 64MiB one, and one whole core. + for _, rate := range []string{"0.0028", "0.0100", "0.1419"} { + t.Run(rate, func(t *testing.T) { + none := time.Duration(0) + nc := billingClaim("cheap", rate, &none) + base := nc.Status.LastAccruedAt.Time + a, c := newAccrual(t, nc) + + const ticks = 240 // four hours at the one-minute write cadence + for i := 1; i <= ticks; i++ { + at := base.Add(time.Duration(i) * accrualInterval) + a.now = func() time.Time { return at } + a.accrueAll(context.Background()) + } + + hourly, err := strconv.ParseFloat(rate, 64) + if err != nil { + t.Fatalf("bad rate %q: %v", rate, err) + } + want := hourly * float64(ticks) * accrualInterval.Hours() + total, _ := ledger(t, c, "cheap") + if math.Abs(total-want)/want > 1e-4 { + t.Fatalf("status.estimatedCostUSD %v after %d ticks at $%s/hr, want %v", total, ticks, rate, want) + } + if got := booked(t); math.Abs(got-total) > 1e-9 { + t.Fatalf("booked %v, want %v — the counter must take exactly what the ledger did", got, total) + } + }) + } +} + +// A claim that became billable before the ledger existed has no anchor. Opening one must not +// invent spend for the window whose length nobody knows. +func TestCostAccrual_StampsMissingAnchorWithoutCharging(t *testing.T) { + a, c := newAccrual(t, billingClaim("bound", "98.3200", nil)) + + a.accrueAll(context.Background()) + + total, anchor := ledger(t, c, "bound") + if total != 0 { + t.Fatalf("status.estimatedCostUSD %v on an unanchored claim, want 0", total) + } + if anchor == nil { + t.Fatal("no anchor was written, so the next tick will charge nothing either") + } + if n := testutil.CollectAndCount(metrics.CostTotal); n != 0 { + t.Fatalf("collected %d series, want 0 — nothing was charged", n) + } +} + +// A claim holding no instance, or one nobody can price, accrues NOTHING rather than zero: a zero +// is summed and averaged as a real "this costs nothing". +func TestCostAccrual_SkipsNonBilling(t *testing.T) { + hour := time.Hour + unbilled := func(name, price string, phase nebulav1alpha1.NodeClaimPhase) *nebulav1alpha1.NodeClaim { + nc := billingClaim(name, price, &hour) + nc.Status.Phase = phase + return nc + } + claims := []*nebulav1alpha1.NodeClaim{ + unbilled("provisioning", "3.9500", nebulav1alpha1.NodeClaimProvisioning), + // The instance is gone; the claim lingers only as a ledger. + unbilled("terminated", "3.9500", nebulav1alpha1.NodeClaimTerminated), + unbilled("no-phase", "3.9500", ""), + // Bound but UNPRICED (no Pricer, or no catalog row) — not free. + unbilled("unpriced", "", nebulav1alpha1.NodeClaimBound), + unbilled("malformed", "cheap", nebulav1alpha1.NodeClaimBound), + unbilled("nonpositive", "0.0000", nebulav1alpha1.NodeClaimBound), + // ParseFloat reads these as valid floats and they outrank every ordering check, so + // without an explicit test the guard against them is one refactor from vanishing. + unbilled("nan", "NaN", nebulav1alpha1.NodeClaimBound), + unbilled("inf", "+Inf", nebulav1alpha1.NodeClaimBound), + } + a, c := newAccrual(t, claims...) + + a.accrueAll(context.Background()) + + for _, nc := range claims { + if total, _ := ledger(t, c, nc.Name); total != 0 { + t.Fatalf("claim %q accrued %v, want 0", nc.Name, total) + } + } + if n := testutil.CollectAndCount(metrics.CostTotal); n != 0 { + t.Fatalf("collected %d series, want 0 — no non-billing claim may accrue", n) + } +} + +// A ledger that already holds a non-finite total recovers on the next tick. Without this the claim +// would stay poisoned for the rest of its life even after the bad price was fixed, because every +// total is derived from reading the previous one back. +func TestCostAccrual_RecoversFromPoisonedLedger(t *testing.T) { + halfHour := 30 * time.Minute + nc := billingClaim("poisoned", "10.0000", &halfHour) + nc.Status.EstimatedCostUSD = "NaN" + a, c := newAccrual(t, nc) + + a.accrueAll(context.Background()) + + want := 10 * 0.5 + total, _ := ledger(t, c, "poisoned") + // Tested for finiteness FIRST: every comparison against NaN is false, so an ordinary + // tolerance check would pass on the very value this test exists to catch. + if math.IsNaN(total) || math.IsInf(total, 0) || math.Abs(total-want) > 1e-3 { + t.Fatalf("status.estimatedCostUSD %v, want %v — the poisoned total must be dropped, not carried", total, want) + } + if got := booked(t); math.IsNaN(got) || math.Abs(got-total) > 1e-9 { + t.Fatalf("booked %v, want %v", got, total) + } +} + +// An anchor in the future (a wall-clock jump, a hand-edited field) must not rewind the ledger. +func TestCostAccrual_DropsBackwardsWindow(t *testing.T) { + ahead := -time.Hour + a, c := newAccrual(t, billingClaim("bound", "4.0000", &ahead)) + + a.accrueAll(context.Background()) + + if total, _ := ledger(t, c, "bound"); total != 0 { + t.Fatalf("status.estimatedCostUSD %v on a backwards window, want 0", total) + } + if n := testutil.CollectAndCount(metrics.CostTotal); n != 0 { + t.Fatalf("collected %d series on a backwards window, want 0", n) + } +} + +// A write that fails must leave the anchor where it was, so the next tick charges the same window +// once — not twice, and not never. +func TestCostAccrual_FailedWriteChargesNothing(t *testing.T) { + hour := time.Hour + nc := billingClaim("bound", "98.3200", &hour) + c := fake.NewClientBuilder(). + WithScheme(testScheme(t)). + WithObjects(nc). + WithStatusSubresource(&nebulav1alpha1.NodeClaim{}). + WithInterceptorFuncs(interceptor.Funcs{ + SubResourceUpdate: func(context.Context, client.Client, string, client.Object, ...client.SubResourceUpdateOption) error { + return apierrors.NewConflict(schema.GroupResource{Resource: "nodeclaims"}, "bound", errors.New("stale")) + }, + }). + Build() + a := NewCostAccrual(c) + + a.accrueAll(context.Background()) + + if total, anchor := ledger(t, c, "bound"); total != 0 || anchor.Time.Before(time.Now().Add(-90*time.Minute)) { + t.Fatalf("a failed write moved the ledger: estimatedCostUSD=%v anchor=%v", total, anchor) + } + // The ordering that keeps a retry from over-charging: a counter has no idempotency key, so a + // window booked before its patch landed would be charged again next tick. + if n := testutil.CollectAndCount(metrics.CostTotal); n != 0 { + t.Fatalf("collected %d series after a failed write, want 0", n) + } +} + +// costNow is what both the checkpoint and the teardown log line read, so the window still open +// since the anchor has to be in it. +func TestCostNow(t *testing.T) { + hour := time.Hour + anchored := billingClaim("bound", "10.0000", &hour) + anchored.Status.EstimatedCostUSD = "5.0000" + + // Terminated: the instance is gone, so the persisted total is final and no window is added. + settled := billingClaim("settled", "10.0000", &hour) + settled.Status.Phase = nebulav1alpha1.NodeClaimTerminated + settled.Status.EstimatedCostUSD = "5.0000" + + cases := map[string]struct { + claim *nebulav1alpha1.NodeClaim + want float64 + report bool + }{ + "ledger plus the open window": {claim: anchored, want: 15, report: true}, + "terminated total is frozen": {claim: settled, want: 5, report: true}, + "unanchored charges nothing": {claim: billingClaim("fresh", "10.0000", nil), want: 0, report: true}, + // Never billable and never charged: absent cost, which must not be reported as zero. + "unpriced is absent, not zero": {claim: billingClaim("unpriced", "", &hour), want: 0, report: false}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got, ok := costNow(tc.claim, time.Now()) + if ok != tc.report { + t.Fatalf("costNow ok = %v, want %v", ok, tc.report) + } + if math.Abs(got-tc.want) > 1e-2 { + t.Fatalf("costNow = %v, want %v", got, tc.want) + } + }) + } +} + +// status is deliberately untouched on the way out: writing it would race the deletion. +func TestSettleFinalCost_LeavesStatusAlone(t *testing.T) { + hour := time.Hour + nc := billingClaim("bound", "10.0000", &hour) + nc.Status.EstimatedCostUSD = "5.0000" + + settleFinalCost(context.Background(), nc) + + if nc.Status.EstimatedCostUSD != "5.0000" { + t.Fatalf("settleFinalCost wrote status.estimatedCostUSD %q; it must not touch a deleting claim", + nc.Status.EstimatedCostUSD) + } +} + +// The window open at teardown reaches the counter even though no checkpoint can close it — without +// this, an instance that never survived a tick would be billed nothing at all. +func TestSettleFinalCost_BooksTheOpenWindow(t *testing.T) { + metrics.CostTotal.Reset() + + hour := time.Hour + // Never checkpointed: the whole of its life is the open window. + nc := billingClaim("bound", "10.0000", &hour) + nc.Status.Phase = nebulav1alpha1.NodeClaimTerminating + + settleFinalCost(context.Background(), nc) + + if got := booked(t); math.Abs(got-10) > 1e-2 { + t.Fatalf("booked %v at teardown, want 10 (1h at $10/hr charged by no tick)", got) + } +} + +// An instance that ended on its own reaches Terminated, which the accrual loop refuses to touch, so +// the time since its last checkpoint is booked here or nowhere. The clamp is what keeps that safe: +// the claim then sits in Terminated until its Pod is deleted, and charging that wait would bill idle +// hours at GPU rates. +func TestSettleFinalCost_BooksTheLastWindowOfADeadInstance(t *testing.T) { + // Half a window, so the first case measures the real gap rather than tripping the clamp. + halfWindow, day := accrualInterval/2, 24*time.Hour + cases := map[string]struct { + ago time.Duration + want float64 + }{ + "the window since the last checkpoint": {ago: halfWindow, want: 10 * halfWindow.Hours()}, + "never longer than one interval": {ago: day, want: 10 * accrualInterval.Hours()}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + metrics.CostTotal.Reset() + + nc := billingClaim("settled", "10.0000", &tc.ago) + nc.Status.Phase = nebulav1alpha1.NodeClaimTerminated + nc.Status.EstimatedCostUSD = "5.0000" + + settleFinalCost(context.Background(), nc) + + // billingClaim truncates the anchor to the second, so the real window is up to a + // second longer than ago. Two seconds of spend covers that and the test's own time. + if got := booked(t); math.Abs(got-tc.want) > 10*(2*time.Second).Hours() { + t.Fatalf("booked %v, want %v", got, tc.want) + } + }) + } +} + +// Unpriced is absent spend, not zero — a claim that was never chargeable must not mint a series on +// its way out either. +func TestSettleFinalCost_BooksNothingUnpriced(t *testing.T) { + metrics.CostTotal.Reset() + + hour := time.Hour + nc := billingClaim("unpriced", "", &hour) + nc.Status.Phase = nebulav1alpha1.NodeClaimTerminated + + settleFinalCost(context.Background(), nc) + + if n := testutil.CollectAndCount(metrics.CostTotal); n != 0 { + t.Fatalf("collected %d series for a claim that was never priced, want 0", n) + } +} + +// The window is booked against the candidate the claim was provisioned on, so cost joins the +// placement and provisioning series without label surgery. +func TestCostAccrual_BooksAgainstTheCandidate(t *testing.T) { + hour := time.Hour + nc := billingClaim("bound", "10.0000", &hour) + nc.Spec.Region = "us-east-1" + nc.Spec.CapacityType = nebulav1alpha1.CapacitySpot + a, _ := newAccrual(t, nc) + + a.accrueAll(context.Background()) + + // The exposition format is line-oriented, so this cannot be wrapped. + //nolint:lll + want := ` +# HELP nebula_cost_usd_total Cumulative USD billed by external instances, added window by window as it accrues. +# TYPE nebula_cost_usd_total counter +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="Spot",phase="Bound",provider="modal",region="us-east-1"} 10 +` + if err := testutil.CollectAndCompare(metrics.CostTotal, strings.NewReader(want)); err != nil { + t.Fatal(err) + } +} + +// Attribution is copied off the Pod once and then frozen: spend already reported under one tenant +// must not move to another because somebody relabelled a Pod. +// +// Configured with QUALIFIED keys, which is what a real deployment uses: the stamp must be keyed by +// the key the Pod carries, prefix and all, not by the name the metric emits under. +func TestStampCostLabels(t *testing.T) { + t.Cleanup(func() { metrics.ConfigureCostForTest(nil) }) + metrics.ConfigureCostForTest([]string{"example.com/org-id", "example.com/team-id"}) + + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "example.com/org-id": "acme", + "example.com/team-id": "ml", + // The derived metric name is not a Pod key, so it must not be read as one. + "org_id": "ignored", + "unconfigured": "ignored", + }}} + + nc := billingClaim("bound", "10.0000", nil) + if !stampCostLabels(nc, pod) { + t.Fatal("stampCostLabels reported no change on an unstamped claim") + } + want := map[string]string{"example.com/org-id": "acme", "example.com/team-id": "ml"} + if !reflect.DeepEqual(nc.Status.CostLabels, want) { + t.Fatalf("stamped %v, want %v — only configured keys are copied", nc.Status.CostLabels, want) + } + + // A relabelled Pod must not re-attribute the claim. + pod.Labels["example.com/org-id"] = "someone-else" + if stampCostLabels(nc, pod) { + t.Fatal("stampCostLabels rewrote attribution that was already settled") + } + if nc.Status.CostLabels["example.com/org-id"] != "acme" { + t.Fatalf("org-id moved to %q", nc.Status.CostLabels["example.com/org-id"]) + } + + // A Pod carrying none of them is a settled fact too, or every later reconcile re-checks it. + bare := billingClaim("bare", "10.0000", nil) + if !stampCostLabels(bare, &corev1.Pod{}) || bare.Status.CostLabels == nil { + t.Fatal("a Pod with no configured labels must still stamp an empty map") + } +} + +// With no --cost-labels there is nothing to copy, and the claim must stay untouched rather than +// carrying an empty map that reads as "we looked and found nothing". +func TestStampCostLabels_NotConfigured(t *testing.T) { + nc := billingClaim("bound", "10.0000", nil) + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"org_id": "acme"}}} + + if stampCostLabels(nc, pod) || nc.Status.CostLabels != nil { + t.Fatalf("stamped %v with no cost labels configured, want nil", nc.Status.CostLabels) + } +} + +// The window booked carries the claim's stamped attribution, which is the whole point of pinning +// it in status: the Pod is long gone by the time a Terminating window is charged. +// +// End to end on the key/name split: the claim is keyed "example.com/org-id" and the series comes out +// as org_id. +func TestCostAccrual_BooksAttribution(t *testing.T) { + t.Cleanup(func() { metrics.ConfigureCostForTest(nil) }) + metrics.ConfigureCostForTest([]string{"example.com/org-id"}) + + hour := time.Hour + nc := billingClaim("bound", "10.0000", &hour) + nc.Status.CostLabels = map[string]string{"example.com/org-id": "acme"} + a, _ := newAccrual(t, nc) + + a.accrueAll(context.Background()) + + //nolint:lll + want := ` +# HELP nebula_cost_usd_total Cumulative USD billed by external instances, added window by window as it accrues. +# TYPE nebula_cost_usd_total counter +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",example_com_org_id="acme",phase="Bound",provider="modal",region="none"} 10 +` + if err := testutil.CollectAndCompare(metrics.CostTotal, strings.NewReader(want)); err != nil { + t.Fatal(err) + } +} + +// The whole chain --cost-labels exists for, through a real reconcile: a label on the served Pod +// reaches the claim's status, and the window charged later comes out under it. Nothing is stamped +// by hand here, so the two halves are only connected if markPhase picks the attribution up and +// opens the accrual anchor in the SAME write it records the phase and price with. +func TestMarkPhase_StampsAttributionThatIsThenBooked(t *testing.T) { + t.Cleanup(func() { metrics.ConfigureCostForTest(nil) }) + metrics.ConfigureCostForTest([]string{"example.com/org-id"}) + metrics.CostTotal.Reset() + + pod := gpuPod("H100", 1, "1", "1Gi") + pod.Labels["example.com/org-id"] = "acme" + withInstanceID(pod, "sb-1") + + nc := newClaim("c1", "p1", "default", "uid-1", "fake") + nc.Spec.Accelerator = "H100:1" + nc.Spec.Region = "us-east-1" + nc.Spec.CapacityType = nebulav1alpha1.CapacityOnDemand + + // Status writes are counted, not merely observed: a window charged before the attribution + // explaining it is durable is spend nobody can bill, so the four fields must land together. + writes := 0 + s := testScheme(t) + c := fake.NewClientBuilder(). + WithScheme(s). + WithObjects(pod, nc). + WithStatusSubresource(&nebulav1alpha1.NodeClaim{}). + WithInterceptorFuncs(interceptor.Funcs{ + SubResourceUpdate: func(ctx context.Context, cl client.Client, sub string, + obj client.Object, opts ...client.SubResourceUpdateOption) error { + writes++ + return cl.SubResource(sub).Update(ctx, obj, opts...) + }, + }). + Build() + pp := &pricedProvider{fakeProvider: &fakeProvider{name: "fake"}, rate: 10} + r := &NodeClaimReconciler{Client: c, Scheme: s, Providers: func(name string) (provider.Provider, bool) { + return pp, name == pp.name + }} + + reconcileClaim(t, r, "c1") + + if writes != 1 { + t.Fatalf("%d status writes, want 1 — the attribution and the anchor must ride one patch", writes) + } + got := getClaim(t, c, "c1") + if got.Status.Phase != nebulav1alpha1.NodeClaimBound || got.Status.PriceUSDPerHour != "10.0000" { + t.Fatalf("phase %q price %q, want Bound / 10.0000", got.Status.Phase, got.Status.PriceUSDPerHour) + } + want := map[string]string{"example.com/org-id": "acme"} + if !reflect.DeepEqual(got.Status.CostLabels, want) { + t.Fatalf("status.costLabels = %v, want %v", got.Status.CostLabels, want) + } + // The anchor opens only once the claim is Bound AND priced, so its presence is also the + // evidence that stampAccrualStart ran after the two calls it reads — not before them. + if got.Status.LastAccruedAt == nil { + t.Fatal("no accrual anchor was opened; the window before the first checkpoint is lost") + } + // Everything is settled, so a second pass over the same Pod must write nothing. + reconcileClaim(t, r, "c1") + if writes != 1 { + t.Fatalf("%d status writes after a second reconcile, want 1", writes) + } + + // Rewind the anchor an hour (truncated, as the accrual's own clock is) so a whole window is + // chargeable without sleeping, then let the loop charge it. + got.Status.LastAccruedAt = &metav1.Time{Time: time.Now().Add(-time.Hour).Truncate(time.Second)} + if err := c.Status().Update(context.Background(), got); err != nil { + t.Fatalf("rewind the anchor: %v", err) + } + NewCostAccrual(c).accrueAll(context.Background()) + + // No baseline anywhere: accrueAll is driven directly, so nothing here has seeded one. That is the + // gap this claim would fall into if it had been born mid-process — see seedBaselines. + // + // The exposition format is line-oriented, so this cannot be wrapped. + //nolint:lll + wantSeries := ` +# HELP nebula_cost_usd_total Cumulative USD billed by external instances, added window by window as it accrues. +# TYPE nebula_cost_usd_total counter +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="OnDemand",example_com_org_id="acme",phase="Bound",provider="fake",region="us-east-1"} 10 +` + if err := testutil.CollectAndCompare(metrics.CostTotal, strings.NewReader(wantSeries)); err != nil { + t.Fatal(err) + } +} + +// A restart throws away every series markPhase baselined when these claims were born, and it will +// not run again for a claim that is already anchored. Seeding is what stops the whole fleet's first +// post-restart window from landing on newborn series that increase() cannot difference. +func TestSeedBaselines(t *testing.T) { + hour := time.Hour + + // Every claim gets a shape of its own, or a claim wrongly skipped would hide behind the series + // another one seeded — only "same-shape" shares deliberately. + shape := billingClaim("same-shape", "10.0000", &hour) + // Anchored and priced, so its last window is still booked at teardown: it needs a baseline as + // much as a running claim does, which is why seeding asks finalRate rather than billingRate. + terminated := billingClaim("terminated", "10.0000", &hour) + terminated.Status.Phase = nebulav1alpha1.NodeClaimTerminated + terminated.Spec.Accelerator = "A100:2" + unanchored := billingClaim("unanchored", "10.0000", nil) + unanchored.Spec.Accelerator = "L4:1" + unpriced := billingClaim("unpriced", "", &hour) + unpriced.Spec.Accelerator = "T4:1" + cpu := billingClaim("cpu", "0.1000", &hour) + cpu.Spec.Accelerator = "" + + a, _ := newAccrual(t, billingClaim("bound", "10.0000", &hour), shape, terminated, unanchored, unpriced, cpu) + a.seedBaselines(context.Background()) + + // Three shapes, three phases each. The two claims sharing H100:1 collapse onto one set of series + // — a baseline is per SERIES, not per claim — while L4 and T4 are absent entirely: no window can + // ever be booked for a claim with no anchor or no price, so a baseline would be an empty promise. + // + // The exposition format is line-oriented, so these cannot be wrapped. + //nolint:lll + want := ` +# HELP nebula_cost_usd_total Cumulative USD billed by external instances, added window by window as it accrues. +# TYPE nebula_cost_usd_total counter +nebula_cost_usd_total{accelerator="A100",accelerator_count="2",capacity_type="none",phase="Bound",provider="modal",region="none"} 0 +nebula_cost_usd_total{accelerator="A100",accelerator_count="2",capacity_type="none",phase="Terminated",provider="modal",region="none"} 0 +nebula_cost_usd_total{accelerator="A100",accelerator_count="2",capacity_type="none",phase="Terminating",provider="modal",region="none"} 0 +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",phase="Bound",provider="modal",region="none"} 0 +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",phase="Terminated",provider="modal",region="none"} 0 +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",phase="Terminating",provider="modal",region="none"} 0 +nebula_cost_usd_total{accelerator="none",accelerator_count="none",capacity_type="none",phase="Bound",provider="modal",region="none"} 0 +nebula_cost_usd_total{accelerator="none",accelerator_count="none",capacity_type="none",phase="Terminated",provider="modal",region="none"} 0 +nebula_cost_usd_total{accelerator="none",accelerator_count="none",capacity_type="none",phase="Terminating",provider="modal",region="none"} 0 +` + if err := testutil.CollectAndCompare(metrics.CostTotal, strings.NewReader(want)); err != nil { + t.Fatal(err) + } +} + +// Seeding must land BEFORE the first tick, or it baselines a series the same tick already charged +// and buys nothing. Only Start guarantees that ordering, so it is asserted through Start — with a +// claim whose whole hour is chargeable, so a tick that beat the seed would show up as a value. +func TestStart_SeedsBeforeCharging(t *testing.T) { + hour := time.Hour + a, _ := newAccrual(t, billingClaim("bound", "10.0000", &hour)) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go func() { _ = a.Start(ctx) }() + + // a.interval is a minute, so anything observed within seconds is the seed, not a tick. + deadline := time.Now().Add(5 * time.Second) + for testutil.CollectAndCount(metrics.CostTotal) < len(billingPhases) { + if time.Now().After(deadline) { + t.Fatalf("collected %d series, want %d baselines seeded on the way to the first tick", + testutil.CollectAndCount(metrics.CostTotal), len(billingPhases)) + } + time.Sleep(10 * time.Millisecond) + } + if got := testutil.ToFloat64(metrics.CostTotal.WithLabelValues( + "modal", "none", "none", "H100", "1", "Bound")); got != 0 { + t.Fatalf("the Bound baseline holds %v, want 0 — seeding may not charge anything", got) + } +} + +// The stamp is what makes a crash before the first checkpoint lossless, so it must land exactly +// when the claim becomes chargeable — and never move once set. +func TestStampAccrualStart(t *testing.T) { + hour := time.Hour + cases := map[string]struct { + claim *nebulav1alpha1.NodeClaim + want bool + }{ + "billable and unanchored": {claim: billingClaim("a", "3.9500", nil), want: true}, + "already anchored": {claim: billingClaim("b", "3.9500", &hour), want: false}, + "unpriced": {claim: billingClaim("c", "", nil), want: false}, + } + provisioning := billingClaim("d", "3.9500", nil) + provisioning.Status.Phase = nebulav1alpha1.NodeClaimProvisioning + cases["not holding an instance"] = struct { + claim *nebulav1alpha1.NodeClaim + want bool + }{claim: provisioning, want: false} + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + before := tc.claim.Status.LastAccruedAt + if got := stampAccrualStart(tc.claim); got != tc.want { + t.Fatalf("stampAccrualStart = %v, want %v", got, tc.want) + } + if !tc.want && tc.claim.Status.LastAccruedAt != before { + t.Fatal("the anchor was rewritten") + } + }) + } +} + +// A cost checkpoint must not re-enqueue the claim, but anything alongside it must. +func TestIgnoreCostAccrual(t *testing.T) { + p := ignoreCostAccrual() + hour := time.Hour + + base := billingClaim("bound", "3.9500", &hour) + costOnly := base.DeepCopy() + costOnly.Status.EstimatedCostUSD = "12.0000" + costOnly.Status.LastAccruedAt = &metav1.Time{Time: time.Now()} + costOnly.ResourceVersion = "999" + if p.Update(event.UpdateEvent{ObjectOld: base, ObjectNew: costOnly}) { + t.Fatal("a cost-only checkpoint was enqueued") + } + + alsoPhase := costOnly.DeepCopy() + alsoPhase.Status.Phase = nebulav1alpha1.NodeClaimTerminating + if !p.Update(event.UpdateEvent{ObjectOld: base, ObjectNew: alsoPhase}) { + t.Fatal("a phase change riding along with a checkpoint was dropped") + } + + withFinalizer := base.DeepCopy() + withFinalizer.Finalizers = append(withFinalizer.Finalizers, nebulav1alpha1.TerminateInstanceFinalizer) + if !p.Update(event.UpdateEvent{ObjectOld: base, ObjectNew: withFinalizer}) { + t.Fatal("a finalizer update was dropped; the reconciler requeues off that event") + } +} + +func TestCostAccrual_StartStopsOnContextCancel(t *testing.T) { + a, _ := newAccrual(t) + a.interval = time.Millisecond + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- a.Start(ctx) }() + + cancel() + select { + case err := <-done: + if err != nil { + t.Fatalf("Start returned %v, want nil", err) + } + case <-time.After(5 * time.Second): + t.Fatal("Start did not return after its context was cancelled") + } +} + +// Start must actually checkpoint on a tick. Only that something is persisted is asserted: a test +// cannot pin the scheduler to a known window. +func TestCostAccrual_StartAccrues(t *testing.T) { + hour := time.Hour + a, c := newAccrual(t, billingClaim("bound", "3.9500", &hour)) + a.interval = time.Millisecond + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go func() { _ = a.Start(ctx) }() + + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if total, _ := ledger(t, c, "bound"); total > 0 { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatal("Start persisted nothing after 5s of 1ms ticks") +} diff --git a/internal/controller/cost_labels_persistence_test.go b/internal/controller/cost_labels_persistence_test.go new file mode 100644 index 0000000..c719ce0 --- /dev/null +++ b/internal/controller/cost_labels_persistence_test.go @@ -0,0 +1,74 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "sigs.k8s.io/controller-runtime/pkg/client" + + nebulav1alpha1 "github.com/InftyAI/Nebula/api/v1alpha1" +) + +// status.costLabels distinguishes nil (nothing observed yet) from empty (observed, the Pod carried +// none of the configured keys), and stampCostLabels relies on that to write attribution exactly +// once. Only a real apiserver can prove it: the fake client the unit tests use stores deep copies +// rather than marshalling, so an empty map survives there whatever the json tag says — while an +// omitempty would drop it on the way to etcd and leave the claim looking unstamped forever. +// Requires envtest binaries; the whole suite is skipped in BeforeSuite when they are absent. +var _ = Describe("NodeClaim status.costLabels", func() { + claim := func(name string) *nebulav1alpha1.NodeClaim { + return newClaimNoFinalizer(name, "p1", "default", "uid-1", "modal") + } + readBack := func(name string) map[string]string { + var nc nebulav1alpha1.NodeClaim + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: name}, &nc)).To(Succeed()) + return nc.Status.CostLabels + } + + It("keeps an empty map distinct from an unset one", func() { + nc := claim("cost-labels-empty") + Expect(k8sClient.Create(ctx, nc)).To(Succeed()) + DeferCleanup(func() { Expect(k8sClient.Delete(ctx, nc)).To(Succeed()) }) + + // Unset: nothing observed yet. + Expect(readBack(nc.Name)).To(BeNil()) + + nc.Status.Phase = nebulav1alpha1.NodeClaimBound + nc.Status.CostLabels = map[string]string{} + Expect(k8sClient.Status().Update(ctx, nc)).To(Succeed()) + + // Stamped and empty. A nil here is the bug this spec exists for: stampCostLabels would + // re-read the Pod on every reconcile, and a label added later would move the attribution of + // a claim whose earlier windows were already booked to "none". + Expect(readBack(nc.Name)).NotTo(BeNil()) + Expect(readBack(nc.Name)).To(BeEmpty()) + }) + + It("keeps a stamped value verbatim, qualified key and all", func() { + nc := claim("cost-labels-stamped") + Expect(k8sClient.Create(ctx, nc)).To(Succeed()) + DeferCleanup(func() { Expect(k8sClient.Delete(ctx, nc)).To(Succeed()) }) + + nc.Status.Phase = nebulav1alpha1.NodeClaimBound + nc.Status.CostLabels = map[string]string{"example.com/org-id": "acme"} + Expect(k8sClient.Status().Update(ctx, nc)).To(Succeed()) + + Expect(readBack(nc.Name)).To(Equal(map[string]string{"example.com/org-id": "acme"})) + }) +}) diff --git a/internal/controller/nodeclaim_controller.go b/internal/controller/nodeclaim_controller.go index 325ca92..05dd59b 100644 --- a/internal/controller/nodeclaim_controller.go +++ b/internal/controller/nodeclaim_controller.go @@ -18,18 +18,24 @@ package controller import ( "context" + "errors" + "strconv" "time" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" nebulav1alpha1 "github.com/InftyAI/Nebula/api/v1alpha1" @@ -78,9 +84,9 @@ type NodeClaimReconciler struct { client.Client Scheme *runtime.Scheme - // Providers resolves a provider name (NodeClaim.spec.provider) to its - // backend, used ONLY on the teardown path. Defaults to the process-wide - // registry; overridable in tests. + // Providers resolves a provider name (NodeClaim.spec.provider) to its backend, for + // the teardown path (Terminate) and for pricing (the optional provider.Pricer). + // Defaults to the process-wide registry; overridable in tests. Providers func(name string) (provider.Provider, bool) } @@ -246,12 +252,16 @@ func (r *NodeClaimReconciler) findInstanceID(ctx context.Context, prov provider. } // releaseFinalizer removes the terminate finalizer, allowing the API server to -// delete the NodeClaim. +// delete the NodeClaim — and settles what the instance cost over its whole life on the +// way out, since status.estimatedCostUSD goes with the object. func (r *NodeClaimReconciler) releaseFinalizer(ctx context.Context, nc *nebulav1alpha1.NodeClaim) (ctrl.Result, error) { if controllerutil.RemoveFinalizer(nc, nebulav1alpha1.TerminateInstanceFinalizer) { if err := r.Update(ctx, nc); err != nil { return ctrl.Result{}, err } + // Strictly after the update, which is what makes the cost booking inside exactly-once: + // this branch is unreachable on a retry, since the finalizer is already gone. + settleFinalCost(ctx, nc) } return ctrl.Result{}, nil } @@ -302,12 +312,12 @@ func (r *NodeClaimReconciler) desiredPhase(nc *nebulav1alpha1.NodeClaim, pod *co } // markPhase reflects the served Pod into the claim's status: the coarse phase -// desiredPhase maps it to, plus best-effort status.InstanceID, in one idempotent -// write. An empty desiredPhase means "leave the phase as-is" (the Bound-flap hold); -// the id is still refreshed. The provider id is the one datum that must not be lost — -// the teardown backstop reclaims the instance by it even if VK (which otherwise holds -// it only in memory) has died — so it is captured as soon as the Pod carries it, -// regardless of phase. A no-op (phase unchanged and id already set) writes nothing. +// desiredPhase maps it to, plus best-effort status.InstanceID and status.PriceUSDPerHour, +// in one idempotent write. An empty desiredPhase means "leave the phase as-is" (the +// Bound-flap hold); the id is still refreshed. The provider id is the one datum that must +// not be lost — the teardown backstop reclaims the instance by it even if VK (which +// otherwise holds it only in memory) has died — so it is captured as soon as the Pod +// carries it, regardless of phase. A no-op (nothing to change) writes nothing. func (r *NodeClaimReconciler) markPhase(ctx context.Context, nc *nebulav1alpha1.NodeClaim, pod *corev1.Pod) error { changed := false if phase := r.desiredPhase(nc, pod); phase != "" && nc.Status.Phase != phase { @@ -317,6 +327,18 @@ func (r *NodeClaimReconciler) markPhase(ctx context.Context, nc *nebulav1alpha1. if recordInstanceID(nc, pod) { changed = true } + if r.recordPrice(ctx, nc, pod) { + changed = true + } + if stampCostLabels(nc, pod) { + changed = true + } + // Last, so it sees the phase and price the two calls above just set: the window opens the + // moment those make the claim chargeable — and in the same patch as the attribution above, + // so no window is ever charged before we know who to charge it to. + if stampAccrualStart(nc) { + changed = true + } if !changed { return nil } @@ -346,6 +368,67 @@ func recordInstanceID(nc *nebulav1alpha1.NodeClaim, pod *corev1.Pod) bool { return true } +// priceDecimals is how many fractional digits status.PriceUSDPerHour keeps. Four, not two: +// the cheapest catalog rows are sub-cent per hour (a T4 fraction, Modal's memory +// component), and rounding those to cents would price them at 0.00 — indistinguishable +// from the UNPRICED empty string. +const priceDecimals = 4 + +// recordPrice resolves the hourly cost of what this claim runs and records it, returning +// whether it mutated the claim. Set ONCE: a claim already carrying a price keeps it, which +// is what pins the rate against a later catalog edit (see Status.PriceUSDPerHour). +// +// Here rather than at claim creation because the status subresource strips status on Create, +// and here rather than in the optimizer because the price must describe what was actually +// launched — the Pod's own CPU/memory reservation included, which only a provider metering +// them apart from the accelerator charges for. +// +// Unpriceable is not a failure: a provider with no Pricer, or a candidate with no catalog +// row, leaves the field empty and every consumer skips the claim. Pricing NEVER fails a +// reconcile — a wrong-looking cost column must not stop a teardown ledger from doing its +// actual job. +func (r *NodeClaimReconciler) recordPrice(ctx context.Context, nc *nebulav1alpha1.NodeClaim, pod *corev1.Pod) bool { + if nc.Status.PriceUSDPerHour != "" || pod == nil { + return false + } + prov, ok := r.provider(nc.Spec.Provider) + if !ok { + return false + } + pricer, ok := prov.(provider.Pricer) + if !ok { + return false // this provider cannot price its instances + } + + log := logf.FromContext(ctx) + accelerator, count, err := util.AcceleratorRequest(pod) + if err != nil { + // A contradictory request (a GPU count with no type). Provision rejects it too, so + // there is nothing here worth more than a trace. + log.V(1).Info("skipping price: unreadable accelerator request", "error", err) + return false + } + cpuCores, memoryMiB := util.PodReservation(pod) + rate, err := pricer.PricePerHour(provider.PriceRequest{ + AcceleratorType: accelerator, + Count: count, + CapacityType: nc.Spec.CapacityType, + CPUCores: cpuCores, + MemoryMiB: memoryMiB, + }) + if err != nil { + if errors.Is(err, provider.ErrNoPrice) { + log.V(1).Info("no price for claim", "provider", nc.Spec.Provider, "accelerator", nc.Spec.Accelerator) + } else { + // A malformed request, i.e. our bug: loud, but still not fatal to the reconcile. + log.Error(err, "pricing claim", "provider", nc.Spec.Provider) + } + return false + } + nc.Status.PriceUSDPerHour = strconv.FormatFloat(rate, 'f', priceDecimals, 64) + return true +} + // wasBound reports whether an external instance has ever been confirmed to exist // for this claim — the fact the teardown backstop keys off. A Terminated claim was // necessarily Bound first, so it also counts. @@ -400,7 +483,7 @@ func (r *NodeClaimReconciler) patchStatus(ctx context.Context, nc *nebulav1alpha // requeue. func (r *NodeClaimReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&nebulav1alpha1.NodeClaim{}). + For(&nebulav1alpha1.NodeClaim{}, builder.WithPredicates(ignoreCostAccrual())). Watches(&corev1.Pod{}, handler.EnqueueRequestsFromMapFunc(r.claimsForPod)). Named("nodeclaim"). // One claim per Pod, and the teardown path is the expensive one: per claim a @@ -411,6 +494,32 @@ func (r *NodeClaimReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } +// ignoreCostAccrual drops the update events whose ONLY change is a cost checkpoint (see +// CostAccrual). Without it every claim in the fleet is re-enqueued on the accrual cadence for a +// reconcile that has nothing to do — at 500 claims, a few hundred wasted reconciles every +// interval into the workqueue that is already this controller's bottleneck at scale. +// +// Deliberately narrow: it compares the whole object with the cost fields neutralized, rather +// than filtering on generation, so the finalizer update this reconciler relies on for its own +// requeue still comes through. Anything else that changed alongside the cost is not dropped. +func ignoreCostAccrual() predicate.Predicate { + return predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + before, okOld := e.ObjectOld.(*nebulav1alpha1.NodeClaim) + after, okNew := e.ObjectNew.(*nebulav1alpha1.NodeClaim) + if !okOld || !okNew { + return true + } + rest := after.DeepCopy() + rest.Status.EstimatedCostUSD = before.Status.EstimatedCostUSD + rest.Status.LastAccruedAt = before.Status.LastAccruedAt + rest.ResourceVersion = before.ResourceVersion + rest.ManagedFields = before.ManagedFields + return !equality.Semantic.DeepEqual(before, rest) + }, + } +} + // claimsForPod maps a Pod event to the claim that serves it. The name is DERIVED rather // than searched for: util.ClaimName is what ensureClaim named the claim, so the same // namespace/name yields the same token, truncate-and-hash case included. diff --git a/internal/controller/nodeclaim_price_test.go b/internal/controller/nodeclaim_price_test.go new file mode 100644 index 0000000..a918f32 --- /dev/null +++ b/internal/controller/nodeclaim_price_test.go @@ -0,0 +1,222 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "fmt" + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "sigs.k8s.io/controller-runtime/pkg/client" + + nebulav1alpha1 "github.com/InftyAI/Nebula/api/v1alpha1" + "github.com/InftyAI/Nebula/pkg/provider" +) + +// pricedProvider is a fakeProvider that also implements provider.Pricer. A separate type +// on purpose: adding PricePerHour to fakeProvider itself would silently start pricing every +// other test's claims, and the no-Pricer path is one this must keep exercising. +type pricedProvider struct { + *fakeProvider + rate float64 + err error + got []provider.PriceRequest // every request, in order +} + +func (p *pricedProvider) PricePerHour(req provider.PriceRequest) (float64, error) { + p.got = append(p.got, req) + return p.rate, p.err +} + +// newPricedReconciler wires a reconciler whose only provider is a Pricer named "fake". +func newPricedReconciler(t *testing.T, objs []client.Object, pp *pricedProvider) (*NodeClaimReconciler, client.Client) { + t.Helper() + r, c := newClaimReconciler(t, objs) + r.Providers = func(name string) (provider.Provider, bool) { + if name == pp.name { + return pp, true + } + return nil, false + } + return r, c +} + +// gpuPod is a served Pod shaped like a real GPU workload: the accelerator label, a +// nvidia.com/gpu count, and a CPU/memory reservation. +func gpuPod(accelerator string, gpus int64, cpu, memory string) *corev1.Pod { + pod := newPod("p1", "default", "uid-1", corev1.PodRunning) + pod.Labels = map[string]string{nebulav1alpha1.AcceleratorTypeLabel: accelerator} + pod.Spec.Containers[0].Resources = corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpu), + corev1.ResourceMemory: resource.MustParse(memory), + "nvidia.com/gpu": *resource.NewQuantity(gpus, resource.DecimalSI), + }, + } + return pod +} + +func TestRecordPrice_WritesRateAndRequest(t *testing.T) { + pod := gpuPod("H100", 2, "4", "8Gi") + claim := newClaim("c1", "p1", "default", "uid-1", "fake") + claim.Spec.CapacityType = nebulav1alpha1.CapacityOnDemand + pp := &pricedProvider{fakeProvider: &fakeProvider{name: "fake"}, rate: 7.9} + r, c := newPricedReconciler(t, []client.Object{pod, claim}, pp) + + reconcileClaim(t, r, "c1") + + // Four decimals, so a sub-cent rate is not rounded into the UNPRICED-looking 0. + if got := getClaim(t, c, "c1").Status.PriceUSDPerHour; got != "7.9000" { + t.Fatalf("PriceUSDPerHour = %q, want %q", got, "7.9000") + } + if len(pp.got) == 0 { + t.Fatal("PricePerHour was never called") + } + want := provider.PriceRequest{ + AcceleratorType: "H100", Count: 2, + CapacityType: nebulav1alpha1.CapacityOnDemand, + // The Pod's own reservation, which only a provider metering CPU/memory apart from + // the accelerator charges for — but the request carries it either way. + CPUCores: 4, MemoryMiB: 8192, + } + if pp.got[0] != want { + t.Fatalf("PriceRequest = %+v, want %+v", pp.got[0], want) + } +} + +// The rate is pinned at the first write: re-pricing on every reconcile would let a catalog +// edit retroactively reprice a running instance and rewrite cost history. +func TestRecordPrice_PinnedOnce(t *testing.T) { + pod := gpuPod("H100", 1, "1", "1Gi") + claim := newClaim("c1", "p1", "default", "uid-1", "fake") + pp := &pricedProvider{fakeProvider: &fakeProvider{name: "fake"}, rate: 3.95} + r, c := newPricedReconciler(t, []client.Object{pod, claim}, pp) + + reconcileClaim(t, r, "c1") + if got := getClaim(t, c, "c1").Status.PriceUSDPerHour; got != "3.9500" { + t.Fatalf("first pass: PriceUSDPerHour = %q, want 3.9500", got) + } + + pp.rate = 99.0 // the catalog changes under a running instance + reconcileClaim(t, r, "c1") + + if got := getClaim(t, c, "c1").Status.PriceUSDPerHour; got != "3.9500" { + t.Fatalf("second pass: PriceUSDPerHour = %q, want the pinned 3.9500", got) + } + if len(pp.got) != 1 { + t.Fatalf("PricePerHour called %d times, want 1 — a priced claim must not be re-priced", len(pp.got)) + } +} + +// Unpriceable is not a failure. Every one of these leaves the field EMPTY (which readers +// treat as UNPRICED, never as free) and must not fail the reconcile — the claim's real job +// is teardown, and a missing cost column may never block it. +func TestRecordPrice_UnpriceableLeavesEmpty(t *testing.T) { + cases := map[string]struct { + prov func() provider.Provider + pod *corev1.Pod + }{ + "provider implements no Pricer": { + prov: func() provider.Provider { return &fakeProvider{name: "fake"} }, + pod: gpuPod("H100", 1, "1", "1Gi"), + }, + "no catalog row": { + prov: func() provider.Provider { + return &pricedProvider{fakeProvider: &fakeProvider{name: "fake"}, err: provider.ErrNoPrice} + }, + pod: gpuPod("H100", 1, "1", "1Gi"), + }, + "malformed request is loud but not fatal": { + prov: func() provider.Provider { + return &pricedProvider{fakeProvider: &fakeProvider{name: "fake"}, err: fmt.Errorf("bad request")} + }, + pod: gpuPod("H100", 1, "1", "1Gi"), + }, + "contradictory accelerator request": { + prov: func() provider.Provider { + return &pricedProvider{fakeProvider: &fakeProvider{name: "fake"}, rate: 1} + }, + // nvidia.com/gpu with no accelerator label: util.AcceleratorRequest rejects it. + pod: func() *corev1.Pod { + p := gpuPod("H100", 1, "1", "1Gi") + p.Labels = nil + return p + }(), + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + claim := newClaim("c1", "p1", "default", "uid-1", "fake") + r, c := newClaimReconciler(t, []client.Object{tc.pod, claim}) + prov := tc.prov() + r.Providers = func(string) (provider.Provider, bool) { return prov, true } + + reconcileClaim(t, r, "c1") // fails the test on any error + + got := getClaim(t, c, "c1") + if got.Status.PriceUSDPerHour != "" { + t.Fatalf("PriceUSDPerHour = %q, want empty (UNPRICED)", got.Status.PriceUSDPerHour) + } + // The claim's own job must still have happened. + if got.Status.Phase != nebulav1alpha1.NodeClaimBound { + t.Fatalf("phase = %q, want Bound — pricing must not disturb the ledger", got.Status.Phase) + } + }) + } +} + +// An unregistered provider cannot be priced and must not panic on the nil returned +// alongside ok=false. +func TestRecordPrice_UnknownProvider(t *testing.T) { + pod := gpuPod("H100", 1, "1", "1Gi") + claim := newClaim("c1", "p1", "default", "uid-1", "gone") + r, c := newClaimReconciler(t, []client.Object{pod, claim}) + r.Providers = func(string) (provider.Provider, bool) { return nil, false } + + reconcileClaim(t, r, "c1") + + if got := getClaim(t, c, "c1").Status.PriceUSDPerHour; got != "" { + t.Fatalf("PriceUSDPerHour = %q, want empty", got) + } +} + +// A CPU-only claim still reaches the Pricer: a provider that can run CPU-only work +// (Modal) prices it, and one that cannot answers ErrNoPrice, which is its call to make +// rather than ours to pre-empt. +func TestRecordPrice_CPUOnlyReachesPricer(t *testing.T) { + pod := newPod("p1", "default", "uid-1", corev1.PodRunning) + pod.Spec.Containers[0].Resources = corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("500m"), + corev1.ResourceMemory: resource.MustParse("512Mi"), + }, + } + claim := newClaim("c1", "p1", "default", "uid-1", "fake") + pp := &pricedProvider{fakeProvider: &fakeProvider{name: "fake"}, rate: 0.0276} + r, c := newPricedReconciler(t, []client.Object{pod, claim}, pp) + + reconcileClaim(t, r, "c1") + + if got := getClaim(t, c, "c1").Status.PriceUSDPerHour; got != "0.0276" { + t.Fatalf("PriceUSDPerHour = %q, want 0.0276", got) + } + want := provider.PriceRequest{CPUCores: 0.5, MemoryMiB: 512} + if pp.got[0] != want { + t.Fatalf("PriceRequest = %+v, want %+v", pp.got[0], want) + } +} diff --git a/pkg/metrics/attribution.go b/pkg/metrics/attribution.go new file mode 100644 index 0000000..62e8fc3 --- /dev/null +++ b/pkg/metrics/attribution.go @@ -0,0 +1,210 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "fmt" + "regexp" + "strings" + "sync" + + "k8s.io/apimachinery/pkg/util/validation" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +// costSeriesWarnThreshold is where cost cardinality stops looking like a fleet and starts looking +// like a leak. Attribution values come from Pod labels, which are TENANT-controlled, and a counter +// never releases a series, so a workload minting a fresh value per Pod grows process memory and +// every scrape payload until a restart. +// +// A budget rather than a guess: ~1KB per series in client_golang, so 5000 is a few MB of memory and +// roughly a megabyte of exposition text — far more than any fleet reaches by candidate shape alone. +const costSeriesWarnThreshold = 5000 + +// promLabelName is Prometheus's own label-name grammar, applied to the DERIVED name rather than to +// the Pod key. The legal Kubernetes keys it still rejects are the ones STARTING with a digit — a +// bare "2team", or a digit-leading domain like "4paradigm.com/org-id" — which nothing here can +// repair without inventing a prefix of its own. +var promLabelName = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`) + +// metricNameFor derives the Prometheus label name a Pod label key is emitted under: the WHOLE key, +// with '/', '-' and '.' folded to '_' — so "example.com/org-id" is queried as example_com_org_id. +// +// Keeping the domain prefix is what every Kubernetes exporter does (kube-state-metrics emits +// label_app_kubernetes_io_name, Prometheus service discovery __meta_kubernetes_pod_label_…), and +// for the reason that the prefix is part of the key's identity: two tenants' keys stay distinct, +// and the name in a dashboard is one that appears verbatim in `kubectl get pod --show-labels`. +func metricNameFor(podKey string) (string, error) { + name := strings.NewReplacer("/", "_", "-", "_", ".", "_").Replace(podKey) + if !promLabelName.MatchString(name) { + return "", fmt.Errorf("cost label %q emits as %q, which Prometheus will not accept as a label "+ + "name (it must start with a letter or underscore)", podKey, name) + } + return name, nil +} + +// ParseCostLabels turns the --cost-labels value ("example.com/org-id,team_id") into the attribution +// dimension, in the order given. Empty input means no attribution, which is the default. +// +// Each entry is a POD LABEL KEY, qualified or not; the metric label it emits under is derived from +// it, so a key is configured as the Pod carries it and queried as PromQL can express it. It fails +// rather than skipping a bad entry: silently dropping one would be discovered at invoicing time, +// when every series has already been recorded as "none". +// +// Four rejections, all at startup: a key Kubernetes would not accept, one whose derived name +// Prometheus would not, the same key twice, and two keys folding to the SAME name. That last one is +// a corner case now that the prefix is kept ("org-id" and "org.id" still meet), but it would merge +// two tenants' spend into one series, so it fails rather than being tolerated. A derived name that +// shadows a dimension the counter already carries ("provider", "phase") gets through here and fails +// at InitCost, where the registry rejects a duplicate label name; still at startup, just with a +// less pointed message. +// +// Setting this at all is what makes cost the only metric here whose CARDINALITY is not bounded by +// configuration: the values come from Pod labels. Nothing caps them — noteSeries only warns — so +// pick keys whose value set the cluster's admission policy actually constrains. +func ParseCostLabels(spec string) ([]string, error) { + fields := strings.Split(spec, ",") + keys := make([]string, 0, len(fields)) + seenKey := map[string]struct{}{} + claimedBy := map[string]string{} // derived name -> the key that got there first + for _, raw := range fields { + key := strings.TrimSpace(raw) + if key == "" { + continue + } + // IsQualifiedName carries the whole Kubernetes key grammar — the 253-character prefix, the + // 63-character name, and the alphanumeric-at-both-ends rule — so none of it is restated here. + if errs := validation.IsQualifiedName(key); len(errs) > 0 { + return nil, fmt.Errorf("cost label %q is not a valid Pod label key: %s", key, strings.Join(errs, "; ")) + } + name, err := metricNameFor(key) + if err != nil { + return nil, err + } + if _, dup := seenKey[key]; dup { + return nil, fmt.Errorf("cost label %q is listed twice", key) + } + if first, clash := claimedBy[name]; clash { + return nil, fmt.Errorf("cost labels %q and %q both emit as %q, which would merge two "+ + "tenants' spend into one series; drop one or rename it", first, key, name) + } + seenKey[key] = struct{}{} + claimedBy[name] = key + keys = append(keys, key) + } + if len(keys) == 0 { + // nil rather than an empty slice: "unconfigured" is a state callers compare against. + return nil, nil + } + return keys, nil +} + +// costLabelKeys is the configured attribution dimension, as POD LABEL KEYS in emit order. Read on +// every recording and written only by configureCost, before the manager starts. +// +// Only the keys are kept. The Prometheus names are a pure function of them (metricNameFor) and are +// needed in exactly one place, when the counter is constructed, so storing them here would be a +// second copy of a derivable fact — one that could go stale against this one. +// +// The ORDER lives here rather than in the stamped NodeClaim, which is what makes attribution safe +// against a flag change: values are looked up BY KEY at emit time, so adding, removing or +// reordering --cost-labels can never slide a team_id into the org_id column. +var costLabelKeys []string + +// CostLabelKeys is the Pod label keys attribution reads, in emit order. Exported so the controller +// that stamps the values onto a claim reads the SAME list the metric emits, rather than keeping a +// second copy that could disagree with it. +// +// POD KEYS, not the metric's label names — the two differ whenever a key is qualified. Stamping a +// claim needs the former; building the counter needs metricNames. +func CostLabelKeys() []string { + return costLabelKeys +} + +// metricNames is the Prometheus label names a set of Pod keys is emitted under, in the same order. +// +// The error metricNameFor can return is dropped: ParseCostLabels has already rejected any key that +// fails, and if a caller ever bypassed it, prometheus.Register refuses an illegal label name — so +// the invariant is enforced twice over without an unreachable error path here. +func metricNames(podKeys []string) []string { + names := make([]string, 0, len(podKeys)) + for _, key := range podKeys { + name, _ := metricNameFor(key) + names = append(names, name) + } + return names +} + +// attributionValues renders the configured dimension from a claim's stamped labels, in +// costLabelKeys order. A key the claim never carried reports "none" rather than being omitted, +// which would move the sample to a different series. +func attributionValues(stamped map[string]string) []string { + if len(costLabelKeys) == 0 { + return nil + } + values := make([]string, 0, len(costLabelKeys)) + for _, key := range costLabelKeys { + values = append(values, orNone(stamped[key])) + } + return values +} + +// costSeries holds the label combinations emitted so far, and only until the warning fires: it is a +// leak DETECTOR, not a guard, so it is dropped once it has done its one job rather than growing +// alongside the leak it reports. Guarded because there are two recorders — the accrual loop and the +// reconciler workers that settle a teardown. +var ( + costSeriesMu sync.Mutex + costSeries map[string]struct{} + costSeriesWarned bool +) + +// noteSeries warns once when the cost counter passes costSeriesWarnThreshold distinct series. +// +// It deliberately caps NOTHING. Merging tenants into an "overflow" bucket, or dropping the window, +// would each corrupt a metric an external billing service reads as truth; a metric that is too big +// is an operator's problem to fix at admission, while one that quietly rewrote its own labels is +// nobody's problem until invoicing. Skipped entirely without attribution, where every dimension is +// bounded by NodePools and provider catalogs. +func noteSeries(values []string) { + if len(costLabelKeys) == 0 { + return + } + + costSeriesMu.Lock() + defer costSeriesMu.Unlock() + + if costSeriesWarned { + return + } + if costSeries == nil { + costSeries = map[string]struct{}{} + } + // NUL, because it cannot appear in a Kubernetes label value: joining on any legal character + // would let two different combinations collide into one key. + costSeries[strings.Join(values, "\x00")] = struct{}{} + if len(costSeries) <= costSeriesWarnThreshold { + return + } + costSeriesWarned = true + logf.Log.WithName("metrics").Info("WARNING: the cost metric has passed its expected series "+ + "budget, which usually means an attribution label is carrying a per-Pod value. Nothing is "+ + "dropped or merged, so the dollars stay correct, but memory and every scrape grow until this "+ + "process restarts. Constrain these label values at admission.", + "series", len(costSeries), "threshold", costSeriesWarnThreshold, "costLabels", costLabelKeys) + costSeries = nil +} diff --git a/pkg/metrics/attribution_test.go b/pkg/metrics/attribution_test.go new file mode 100644 index 0000000..028cf54 --- /dev/null +++ b/pkg/metrics/attribution_test.go @@ -0,0 +1,238 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "reflect" + "strconv" + "strings" + "testing" + + "github.com/prometheus/client_golang/prometheus/testutil" +) + +// withAttribution swaps in a label set for one test and restores the default afterwards. Keys go +// through the real ParseCostLabels so a test cannot configure a shape the flag could not, then +// through configureCost rather than InitCost: a registry refuses the second set of label dimensions +// it ever sees for a name, and testutil collects straight from the collector anyway. +func withAttribution(t *testing.T, podKeys ...string) { + t.Helper() + + labels, err := ParseCostLabels(strings.Join(podKeys, ",")) + if err != nil { + t.Fatalf("ParseCostLabels(%v): %v", podKeys, err) + } + configureCost(labels) + t.Cleanup(func() { configureCost(nil) }) +} + +func TestParseCostLabels(t *testing.T) { + cases := map[string]struct { + spec string + want []string + // names is the Prometheus dimension those keys emit under, checked wherever it differs from + // the keys themselves — the parse and the derivation are one contract for a caller. + names []string + err bool + }{ + "empty is no attribution": {spec: ""}, + "bare keys pass through": { + spec: "org_id,team_id", + want: []string{"org_id", "team_id"}, + }, + "whitespace and blanks": { + spec: " org_id , , team_id ", + want: []string{"org_id", "team_id"}, + }, + // The point of the whole exercise: a qualified key is configured as the Pod carries it and + // emitted as PromQL can express it — prefix and all, as kube-state-metrics does it. + "qualified key keeps its prefix": { + spec: "example.com/org-id", + want: []string{"example.com/org-id"}, + names: []string{"example_com_org_id"}, + }, + "a set of qualified keys sharing one prefix": { + spec: "example.com/experiment-id,example.com/org-id,example.com/team-id,example.com/user-id", + want: []string{ + "example.com/experiment-id", "example.com/org-id", "example.com/team-id", "example.com/user-id", + }, + names: []string{ + "example_com_experiment_id", "example_com_org_id", "example_com_team_id", "example_com_user_id", + }, + }, + // Same name part under two prefixes: distinct series, which is what keeping the prefix buys. + "same name part under two domains": { + spec: "a.com/org-id,b.com/org-id", + want: []string{"a.com/org-id", "b.com/org-id"}, + names: []string{"a_com_org_id", "b_com_org_id"}, + }, + "hyphen and dot are folded": { + spec: "org-id,team.id", + want: []string{"org-id", "team.id"}, + names: []string{"org_id", "team_id"}, + }, + // Legal Pod keys no folding can rescue: a Prometheus label name cannot start with a digit, + // and the prefix being part of the name puts a digit-leading DOMAIN in the same boat. + "leading digit": {spec: "2team", err: true}, + "digit-leading domain": {spec: "4paradigm.com/org-id", err: true}, + "trailing underscore": {spec: "org_", err: true}, + "spaces inside a key": {spec: "org id", err: true}, + "empty name part": {spec: "example.com/", err: true}, + "duplicate key": {spec: "org_id,org_id", err: true}, + // Folding still lets two distinct keys meet, rarely — and a merge of two tenants' spend is + // worth refusing even so. + "distinct keys colliding on the derived name": {spec: "org-id,org.id", err: true}, + "bare key colliding with a qualified one": {spec: "com_org_id,com/org-id", err: true}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got, err := ParseCostLabels(tc.spec) + if tc.err { + if err == nil { + t.Fatalf("ParseCostLabels(%q) = %v, want an error", tc.spec, got) + } + return + } + if err != nil { + t.Fatalf("ParseCostLabels(%q): %v", tc.spec, err) + } + if !reflect.DeepEqual(got, tc.want) { + t.Fatalf("ParseCostLabels(%q) = %v, want %v", tc.spec, got, tc.want) + } + // Unset means the keys are already legal names and emit unchanged. + wantNames := tc.names + if wantNames == nil { + wantNames = tc.want + } + if names := metricNames(got); len(wantNames) > 0 && !reflect.DeepEqual(names, wantNames) { + t.Fatalf("metricNames(%q) = %v, want %v", tc.spec, names, wantNames) + } + }) + } +} + +// A DERIVED name that shadows one of the counter's own dimensions is not rejected by the parser — +// it is caught one step later, when InitCost registers a descriptor with a duplicate label name. +// Only a BARE key can reach it now that the prefix is kept: "example.com/provider" emits as +// example_com_provider and collides with nothing. +func TestInitCost_RejectsAShadowingName(t *testing.T) { + t.Cleanup(func() { configureCost(nil) }) + + labels, err := ParseCostLabels("provider") + if err != nil { + t.Fatalf("ParseCostLabels: %v", err) + } + if err := InitCost(labels); err == nil { + t.Fatal("InitCost accepted a key emitting a label name the counter already carries") + } +} + +// Values are read by POD KEY and emitted under the DERIVED name — the whole point of splitting the +// two. A Pod that carried none of the keys is reported as "none" rather than dropping the +// dimension, which would silently split the series. +func TestRecordWindow_Attribution(t *testing.T) { + withAttribution(t, "example.com/org-id", "example.com/team-id") + + l := Labels{Provider: "modal", Accelerator: "H100", AcceleratorCount: 1} + RecordWindow(l, "Bound", map[string]string{"example.com/org-id": "acme", "example.com/team-id": "ml"}, 3.95) + RecordWindow(l, "Bound", map[string]string{"example.com/org-id": "acme"}, 1.05) + RecordWindow(l, "Bound", nil, 0.5) + + // The exposition format is line-oriented, so these cannot be wrapped. + //nolint:lll + want := ` +# HELP nebula_cost_usd_total Cumulative USD billed by external instances, added window by window as it accrues. +# TYPE nebula_cost_usd_total counter +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",example_com_org_id="acme",example_com_team_id="ml",phase="Bound",provider="modal",region="none"} 3.95 +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",example_com_org_id="acme",example_com_team_id="none",phase="Bound",provider="modal",region="none"} 1.05 +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",example_com_org_id="none",example_com_team_id="none",phase="Bound",provider="modal",region="none"} 0.5 +` + if err := testutil.CollectAndCompare(CostTotal, strings.NewReader(want)); err != nil { + t.Fatal(err) + } +} + +// Values are looked up BY POD KEY, so the order they were stamped in cannot slide a team_id into +// the org_id column — the property that makes a --cost-labels change safe. +func TestRecordWindow_AttributionIsKeyedNotPositional(t *testing.T) { + withAttribution(t, "example.com/org-id", "example.com/team-id") + + l := Labels{Provider: "modal"} + RecordWindow(l, "Bound", map[string]string{"example.com/team-id": "ml", "example.com/org-id": "acme"}, 1) + RecordWindow(l, "Bound", map[string]string{"example.com/org-id": "acme", "example.com/team-id": "ml"}, 1) + + if n := testutil.CollectAndCount(CostTotal); n != 1 { + t.Fatalf("collected %d series, want 1 — the same pair rendered two ways", n) + } +} + +// Cardinality is WATCHED, never enforced: past the threshold the warning fires once and every +// window is still booked under its own real attribution. A billing metric that merged or dropped +// samples to defend itself would be wrong in a way its consumer could not detect. +func TestNoteSeries_WarnsWithoutCapping(t *testing.T) { + withAttribution(t, "org_id") + + l := Labels{Provider: "modal"} + for i := range costSeriesWarnThreshold + 10 { + RecordWindow(l, "Bound", map[string]string{"org_id": strconv.Itoa(i)}, 1) + } + + if n := testutil.CollectAndCount(CostTotal); n != costSeriesWarnThreshold+10 { + t.Fatalf("collected %d series, want %d — nothing may be merged away", n, costSeriesWarnThreshold+10) + } + if got := testutil.ToFloat64(CostTotal.WithLabelValues( + "modal", "none", "none", "none", "none", "Bound", "0")); got != 1 { + t.Fatalf("the first tenant's series holds %v, want 1 — its attribution was rewritten", got) + } + + // Having warned, the detector drops its bookkeeping rather than tracking a leak it has already + // reported, and stays quiet from then on. + costSeriesMu.Lock() + defer costSeriesMu.Unlock() + if !costSeriesWarned || costSeries != nil { + t.Fatalf("warned = %v, tracking %d keys; want warned with the tracking released", + costSeriesWarned, len(costSeries)) + } +} + +// The detector must not run at all without attribution, where every dimension is bounded by +// configuration — otherwise a long-lived fleet would eventually warn about its own shapes. +func TestNoteSeries_QuietWithoutAttribution(t *testing.T) { + configureCost(nil) + + RecordWindow(Labels{Provider: "modal"}, "Bound", map[string]string{"org_id": "acme"}, 1) + + costSeriesMu.Lock() + defer costSeriesMu.Unlock() + if costSeries != nil { + t.Fatalf("tracking %d keys with attribution off, want none", len(costSeries)) + } +} + +// With attribution off, a stamped claim's labels are ignored rather than joined onto the series: +// the counter's shape follows --cost-labels alone. +func TestRecordWindow_IgnoresStampsWhenUnconfigured(t *testing.T) { + CostTotal.Reset() + + for _, p := range []string{"modal", "aws", "runpod"} { + RecordWindow(Labels{Provider: p}, "Bound", map[string]string{"org_id": "ignored"}, 1) + } + + if n := testutil.CollectAndCount(CostTotal); n != 3 { + t.Fatalf("collected %d series, want 3 — one per provider, with no attribution dimension", n) + } +} diff --git a/pkg/metrics/cost.go b/pkg/metrics/cost.go new file mode 100644 index 0000000..c1b68d8 --- /dev/null +++ b/pkg/metrics/cost.go @@ -0,0 +1,135 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "fmt" + "math" + + "github.com/prometheus/client_golang/prometheus" + ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +// CostTotal accumulates spend one CLOSED WINDOW at a time, which is what makes it billable: +// increase(...[w]) over any window is a pure function of that window, so a consumer replaying +// an old window re-derives the same dollars and can upsert them idempotently. That holds only for a +// series scraped before its first charge, which is what TouchSeries is for — and not at all for one +// born after that pass has run. +// +// Deliberately carries no claim identity. A per-claim series would churn — one per instance ever +// created, retained until the process exits — and worse, a claim that lived and died between two +// window boundaries could not be billed from it at all: differencing a cumulative per-claim series +// needs a sample at each boundary, and a short-lived instance has neither. Aggregating first does +// not help, since a sum over a changing claim set is not monotonic. Per-claim spend lives in +// NodeClaim.status.estimatedCostUSD (the EST_COST column) instead. +var CostTotal = newCostTotal(nil) + +// Deliberately no init here: CostTotal cannot self-register the way every other metric in this +// package does, because its label names are not known until --cost-labels is parsed. See InitCost. + +func newCostTotal(podKeys []string) *prometheus.CounterVec { + // The DERIVED names, not the Pod keys the operator configured: a qualified key is not a legal + // Prometheus label name. Derived here rather than stored, so the counter's shape and what + // CostLabelKeys hands the stamper cannot disagree. + return prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "nebula_cost_usd_total", + Help: "Cumulative USD billed by external instances, added window by window as it accrues.", + }, + withExtra(append([]string{"phase"}, metricNames(podKeys)...)...), + ) +} + +// InitCost fixes the attribution dimension the cost counter carries, from --cost-labels, and +// registers it. MUST be called once from main after flags are parsed and before the manager +// starts — cost is the one metric here that is not scraped until it is. +// +// Two facts force that. A CounterVec's label names are set at CONSTRUCTION, while the flag is not +// known until main runs; and a Prometheus registry remembers a metric NAME's label dimensions for +// the life of the process even across Unregister, so registering a placeholder first would +// permanently forbid the real shape. Nothing is at risk in the gap: no window can be recorded +// before the manager starts. +// +// Not safe against concurrent recording. +func InitCost(podKeys []string) error { + configureCost(podKeys) + if err := ctrlmetrics.Registry.Register(CostTotal); err != nil { + // The derived names, since those are what the registry objected to — a shadowing key looks + // innocent until you see what it emits as. + return fmt.Errorf("registering cost counter with labels %v: %w", metricNames(podKeys), err) + } + return nil +} + +// configureCost rebuilds the counter for a label set. Split out so tests can swap the dimension +// without touching the process-wide registry, which would refuse the second shape it ever saw. +func configureCost(podKeys []string) { + CostTotal = newCostTotal(podKeys) + costLabelKeys = podKeys + + costSeriesMu.Lock() + defer costSeriesMu.Unlock() + costSeries = nil + costSeriesWarned = false +} + +// RecordWindow books the dollars one claim ran up over a single accrual window, attributed with +// the labels stamped on the claim (NodeClaimStatus.CostLabels). +// +// Call it only AFTER the window has been persisted: a counter has no idempotency key, so a +// window added twice is charged twice. Nothing is lost by waiting, because a failed write leaves +// the anchor in place and the next tick re-derives the same window. +func RecordWindow(l Labels, phase string, attribution map[string]string, usd float64) { + // A zero window is the ordinary case for a claim whose anchor was just opened, and a + // negative one would panic. Neither is worth a series. + // + // Non-finite is checked separately because it passes every comparison above: a counter cannot + // be decremented, so one NaN added here is permanent, and it spreads — any sum() spanning that + // series is NaN too, taking the whole fleet's cost query with it. Cheaper to refuse here than + // to trust every caller's own parsing (see controller.finalRate). + if usd <= 0 || math.IsNaN(usd) || math.IsInf(usd, 0) { + return + } + values := l.values(append([]string{phase}, attributionValues(attribution)...)...) + noteSeries(values) + CostTotal.WithLabelValues(values...).Add(usd) +} + +// TouchSeries publishes one claim's label set as a zero-valued series under each of phases, so the +// first charge booked there has an earlier sample to be differenced against. +// +// increase() recovers a RISE between two samples, so a series whose very first sample already holds +// money reads as no rise at all: those dollars are in the counter's absolute value but not in any +// increase()/rate() query, which is what a billing consumer runs. Sharing series across claims +// usually hides that — but attribution makes them tenant-scoped, and a tenant whose whole usage is +// one short job would be billed nothing. +// +// This does not contradict RecordWindow's refusal of zeros. A zero WINDOW is a measurement claiming +// something cost nothing; a zero COUNTER only says nothing has been charged here yet, and publishing +// it is the ordinary way to make rate() work over label values not known until runtime. +// +// Series are PROCESS-local, so the baseline has to be republished by whatever process is doing the +// charging — see controller.CostAccrual.seedBaselines, which is the one caller. A scrape still has to +// land between the baseline and the charge, so a claim that becomes chargeable after the seeding pass +// is not covered at all; see docs/metrics.md. +func TouchSeries(l Labels, attribution map[string]string, phases ...string) { + for _, phase := range phases { + values := l.values(append([]string{phase}, attributionValues(attribution)...)...) + noteSeries(values) + CostTotal.WithLabelValues(values...).Add(0) + } +} diff --git a/pkg/metrics/cost_test.go b/pkg/metrics/cost_test.go new file mode 100644 index 0000000..722e4dc --- /dev/null +++ b/pkg/metrics/cost_test.go @@ -0,0 +1,145 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "math" + "strings" + "testing" + + "github.com/prometheus/client_golang/prometheus/testutil" +) + +// Windows accumulate on one series, which is what makes increase() over any range the dollars +// charged in that range. +func TestRecordWindow_Accumulates(t *testing.T) { + CostTotal.Reset() + + l := Labels{Provider: "aws", Region: "us-east-1", CapacityType: "Spot", Accelerator: "H100", AcceleratorCount: 8} + RecordWindow(l, "Bound", nil, 8.1933) + RecordWindow(l, "Bound", nil, 8.1933) + + got := testutil.ToFloat64(CostTotal.WithLabelValues(l.values("Bound")...)) + if want := 2 * 8.1933; math.Abs(got-want) > 1e-9 { + t.Fatalf("booked %v, want %v", got, want) + } +} + +// Claims on the same candidate share a series — this is spend by shape, not per instance, so their +// windows must sum. A CPU-only claim renders both accelerator labels "none", and Terminating spend +// splits off so a stuck teardown is visible. +func TestRecordWindow_Series(t *testing.T) { + CostTotal.Reset() + + gpu := Labels{Provider: "modal", Accelerator: "H100", AcceleratorCount: 1} + RecordWindow(gpu, "Bound", nil, 3.95) + RecordWindow(gpu, "Bound", nil, 3.95) + RecordWindow(gpu, "Terminating", nil, 0.33) + RecordWindow(Labels{Provider: "modal"}, "Bound", nil, 0.0946) + + // The exposition format is line-oriented, so these cannot be wrapped. + //nolint:lll + want := ` +# HELP nebula_cost_usd_total Cumulative USD billed by external instances, added window by window as it accrues. +# TYPE nebula_cost_usd_total counter +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",phase="Bound",provider="modal",region="none"} 7.9 +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",phase="Terminating",provider="modal",region="none"} 0.33 +nebula_cost_usd_total{accelerator="none",accelerator_count="none",capacity_type="none",phase="Bound",provider="modal",region="none"} 0.0946 +` + if err := testutil.CollectAndCompare(CostTotal, strings.NewReader(want)); err != nil { + t.Fatal(err) + } +} + +// A counter panics on a negative delta, and a zero window — the ordinary case for a claim whose +// anchor was just opened — would mint an empty series that reads as a real "this costs nothing". +func TestRecordWindow_DropsNonPositive(t *testing.T) { + CostTotal.Reset() + + for _, usd := range []float64{0, -1} { + RecordWindow(Labels{Provider: "modal"}, "Bound", nil, usd) + } + if n := testutil.CollectAndCount(CostTotal); n != 0 { + t.Fatalf("collected %d series on a non-positive window, want 0", n) + } +} + +// The baseline exists so increase() has an earlier sample to difference against, which means it must +// be a series that COLLECTS at zero — not merely a child object the counter knows about. +func TestTouchSeries_PublishesAZeroBaseline(t *testing.T) { + CostTotal.Reset() + + l := Labels{Provider: "modal", Accelerator: "H100", AcceleratorCount: 1} + TouchSeries(l, nil, "Bound", "Terminating", "Terminated") + + // The exposition format is line-oriented, so these cannot be wrapped. + //nolint:lll + want := ` +# HELP nebula_cost_usd_total Cumulative USD billed by external instances, added window by window as it accrues. +# TYPE nebula_cost_usd_total counter +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",phase="Bound",provider="modal",region="none"} 0 +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",phase="Terminated",provider="modal",region="none"} 0 +nebula_cost_usd_total{accelerator="H100",accelerator_count="1",capacity_type="none",phase="Terminating",provider="modal",region="none"} 0 +` + if err := testutil.CollectAndCompare(CostTotal, strings.NewReader(want)); err != nil { + t.Fatal(err) + } +} + +// A baseline must not displace the charge that follows it, on the series it opened or on a repeat +// touch — the whole point is that the first real window still reads as a rise from zero. +func TestTouchSeries_DoesNotDisplaceTheFirstCharge(t *testing.T) { + CostTotal.Reset() + + l := Labels{Provider: "modal"} + TouchSeries(l, nil, "Bound") + RecordWindow(l, "Bound", nil, 3.95) + TouchSeries(l, nil, "Bound") + + if got := testutil.ToFloat64(CostTotal.WithLabelValues(l.values("Bound")...)); got != 3.95 { + t.Fatalf("series reads %v, want 3.95", got) + } +} + +// The baseline is what makes a tenant-scoped series billable, so it has to land on the SAME series +// the charge will — attribution and all, or it buys nothing. +func TestTouchSeries_Attribution(t *testing.T) { + withAttribution(t, "org_id") + + l := Labels{Provider: "modal"} + TouchSeries(l, map[string]string{"org_id": "acme"}, "Bound") + RecordWindow(l, "Bound", map[string]string{"org_id": "acme"}, 1) + + if n := testutil.CollectAndCount(CostTotal); n != 1 { + t.Fatalf("collected %d series, want 1 — the baseline and the charge must share one", n) + } +} + +// NaN and Inf pass every ordering comparison, so the non-positive guard does not stop them. A +// counter cannot be decremented: one of these lands permanently, and every sum() over the fleet +// that spans the series reads NaN with it. +func TestRecordWindow_DropsNonFinite(t *testing.T) { + CostTotal.Reset() + + RecordWindow(Labels{Provider: "modal"}, "Bound", nil, 1) + for _, usd := range []float64{math.NaN(), math.Inf(1), math.Inf(-1)} { + RecordWindow(Labels{Provider: "modal"}, "Bound", nil, usd) + } + if got := testutil.ToFloat64(CostTotal); got != 1 { + t.Fatalf("counter reads %v after non-finite windows, want the 1 it legitimately took", got) + } +} diff --git a/pkg/metrics/doc.go b/pkg/metrics/doc.go index 29f942e..def30f1 100644 --- a/pkg/metrics/doc.go +++ b/pkg/metrics/doc.go @@ -18,18 +18,29 @@ limitations under the License. // // Everything here registers into controller-runtime's registry, so it is served on // the manager's existing --metrics-bind-address endpoint alongside the standard -// controller/workqueue metrics. Importing this package is what registers the -// collectors (see the init in each file); no wiring is needed in main. +// controller/workqueue metrics. Importing this package is enough wiring: every metric +// self-registers (see the init in each file) and every entry point is a helper the callers +// already on those paths invoke. +// +// The cost counter is the one exception: its label names depend on --cost-labels, so main must +// call InitCost once after flags are parsed. See InitCost for why it cannot self-register. +// +// Nothing here reads or writes the API. Cost also accrues onto a NodeClaim status field, so +// the loop that advances it stays in internal/controller and calls in here with each closed +// window. // // The instrumented surface is the path a Pod takes from admission to a running -// external instance, one file per leg: +// external instance, one file per leg, plus what that instance costs while it runs: // // placement.go the Pod is gated -> a candidate is chosen -> the gate is removed // provision.go the provider is called -> the instance reports Running +// cost.go dollars accrued, one billing window at a time +// attribution.go whose dollars those are, from the operator's chosen Pod labels // // Those are the legs whose cost and failure modes are otherwise invisible: placement -// can silently leave a Pod gated forever, and provisioning runs against a third party, -// takes minutes, bills money, and fails for reasons Pod status flattens away. +// can silently leave a Pod gated forever, provisioning runs against a third party, +// takes minutes, bills money, and fails for reasons Pod status flattens away, and the +// instance it produces keeps charging whether or not anything is using it. // Everything else is covered elsewhere and not duplicated — reconcile counts, queue // depth and API latency by controller-runtime's collectors, "how many Pods are gated // right now?" by kube-state-metrics. diff --git a/pkg/metrics/labels.go b/pkg/metrics/labels.go index 0b70658..7578605 100644 --- a/pkg/metrics/labels.go +++ b/pkg/metrics/labels.go @@ -43,11 +43,13 @@ const none = "none" // recoverable as accelerator + ":" + accelerator_count. var candidateLabels = []string{"provider", "region", "capacity_type", "accelerator", "accelerator_count"} -// withExtra returns candidateLabels plus one trailing dimension (result, reason), for -// the collectors that carry an outcome. It copies, because appending to a package-level -// slice would let two collectors share and overwrite one backing array. -func withExtra(name string) []string { - return append(append([]string{}, candidateLabels...), name) +// withExtra returns candidateLabels plus the trailing dimensions a collector carries of +// its own (result, reason, claim, phase). Variadic to pair with Labels.values, which takes +// the matching values the same way — the two are positional, so they must be given in the +// same order. It copies, because appending to a package-level slice would let two +// collectors share and overwrite one backing array. +func withExtra(names ...string) []string { + return append(append([]string{}, candidateLabels...), names...) } // Labels identifies the candidate one placement or provisioning attempt was made diff --git a/pkg/metrics/provision.go b/pkg/metrics/provision.go index b2c6f32..72fa5c0 100644 --- a/pkg/metrics/provision.go +++ b/pkg/metrics/provision.go @@ -80,9 +80,10 @@ var ( // inside the call (so a capacity shortage shows up as latency HERE), Modal // builds the image inside it (so a cache miss does). // - // The top bucket tracks the largest Capabilities.ProvisionTimeout — Modal's 10 - // minutes. A lower ceiling would bury every slow build in +Inf; at 600s, +Inf - // means one thing only: a call outran its own deadline. + // The buckets run past the largest Capabilities.ProvisionTimeout (Modal's 5 + // minutes), rather than stopping at it: a ceiling AT the deadline would bury every + // slow build in +Inf, while this way the 300s bucket is where a call killed by its + // own deadline lands and anything above it is overshoot. ProvisionDuration = prometheus.NewHistogramVec(prometheus.HistogramOpts{ Name: "nebula_provision_duration_seconds", Help: "Latency of the provider's Provision call, by provider, region, capacity type, " + diff --git a/pkg/metrics/testing.go b/pkg/metrics/testing.go new file mode 100644 index 0000000..fb11c06 --- /dev/null +++ b/pkg/metrics/testing.go @@ -0,0 +1,39 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import "strings" + +// ConfigureCostForTest points the cost counter at a set of Pod label keys, for tests in other +// packages that exercise attribution. Not for production use: InitCost is the real entry point. +// +// It exists because InitCost can only ever succeed ONCE per process — a registry remembers a +// metric name's label dimensions for the life of the process — so a test binary cannot try two +// label sets through it. This skips registration; testutil collects from the collector directly. +// +// Keys go through the real ParseCostLabels, so a test cannot configure a shape the flag could not, +// and an unparseable key panics rather than silently configuring nothing. +// +// Pass nil to restore the unconfigured default, and do so in a Cleanup: the counter is a package +// variable, so a test that leaves it swapped changes what every later test measures. +func ConfigureCostForTest(podKeys []string) { + labels, err := ParseCostLabels(strings.Join(podKeys, ",")) + if err != nil { + panic("metrics: ConfigureCostForTest: " + err.Error()) + } + configureCost(labels) +} diff --git a/pkg/provider/catalog/base.go b/pkg/provider/catalog/base.go index 1b2322b..84ec8b9 100644 --- a/pkg/provider/catalog/base.go +++ b/pkg/provider/catalog/base.go @@ -18,8 +18,10 @@ package catalog import ( "context" + "fmt" "strings" + nebulav1alpha1 "github.com/InftyAI/Nebula/api/v1alpha1" "github.com/InftyAI/Nebula/pkg/provider" ) @@ -36,9 +38,9 @@ type Lookup interface { } // Base supplies the parts of provider.Provider that are identical for every -// adapter whose price/availability comes from a catalog: Name, Offerings, and a -// default identity MapAccelerator. Adapters embed it so those three methods are -// not re-implemented per provider: +// adapter whose price/availability comes from a catalog: Name, Offerings, a +// default identity MapAccelerator, and the optional provider.Pricer. Adapters +// embed it so none of those are re-implemented per provider: // // type Provider struct { // catalog.Base @@ -47,7 +49,9 @@ type Lookup interface { // // Name and Offerings are fully generic. MapAccelerator is generic only while a provider // names its accelerators like Nebula's canonical names (Modal does); one whose identifiers -// diverge overrides just that method and still reuses the rest. +// diverge overrides just that method and still reuses the rest. PricePerHour is generic +// only while a provider's catalog price is all-in; one that meters CPU/memory separately +// overrides it and adds those components. // // Lifecycle, Capabilities and ClassifyProvisionError are genuinely provider-specific and // are not provided here. @@ -133,3 +137,59 @@ func (b Base) MapAccelerator(canonical string, count int32) (providerAccelerator } return ids, len(ids) > 0 } + +// PricePerHour implements the optional provider.Pricer from the matched catalog row. That +// row is the WHOLE rate only where a provider's price is all-in (AWS sells an instance); one +// metering CPU and memory apart (Modal, whose rows quote a GPU alone) overrides this and +// adds them. +// +// Rows match as MapAccelerator matches them, plus capacity type: the one dimension +// MapAccelerator can ignore and pricing cannot (AWS p5.48xlarge is $34.412 Spot against +// $98.320 OnDemand). Among interchangeable alternates the FIRST row wins, so the price +// describes the id a launch actually tries first. +func (b Base) PricePerHour(req provider.PriceRequest) (float64, error) { + // A CPU-only request. No row can match an empty type, so this is the same ErrNoPrice the + // loop below would reach; it is stated here for a clearer message, and because it MUST + // land before the Count check, which would otherwise flag count 0 as malformed. A + // provider that really can run CPU-only work (Modal) overrides this and prices it. + if req.AcceleratorType == "" { + return 0, fmt.Errorf("catalog: %s prices accelerators only: %w", b.ProviderName, provider.ErrNoPrice) + } + // Contradictory input, not a missing price — hence a plain error, which callers do not + // swallow the way they swallow ErrNoPrice. + if req.Count <= 0 { + return 0, fmt.Errorf("catalog: %s: accelerator %q with count %d", b.ProviderName, req.AcceleratorType, req.Count) + } + // An empty tier is the candidate a pool declaring no capacityTypes emits. + // servesCapacityTier already reads that as non-Spot, so resolve it here rather than let + // row order decide — on AWS that is a threefold difference. + tier := req.CapacityType + if tier == "" { + tier = nebulav1alpha1.CapacityOnDemand + } + + for _, o := range b.Catalog.Offerings(b.ProviderName) { + if !strings.EqualFold(o.AcceleratorType, req.AcceleratorType) || !o.Available { + continue + } + if o.CapacityType != tier { + continue + } + if o.GPUCount != 0 && o.GPUCount != req.Count { + continue + } + // An unpriced placeholder row (a hand-edited ConfigMap), not a free accelerator. + if o.PricePerHour <= 0 { + continue + } + rate := o.PricePerHour + // GPUCount 0 => the row prices ONE accelerator and the count is a runtime knob, so + // the rate scales with it. A row carrying a count prices the whole offering already. + if o.GPUCount == 0 { + rate *= float64(req.Count) + } + return rate, nil + } + return 0, fmt.Errorf("catalog: %s has no available %s x%d row on %s: %w", + b.ProviderName, req.AcceleratorType, req.Count, tier, provider.ErrNoPrice) +} diff --git a/pkg/provider/catalog/catalog.go b/pkg/provider/catalog/catalog.go index 382a116..9e709fa 100644 --- a/pkg/provider/catalog/catalog.go +++ b/pkg/provider/catalog/catalog.go @@ -39,6 +39,7 @@ import ( "fmt" "io" "io/fs" + "math" "os" "path/filepath" "strconv" @@ -188,6 +189,12 @@ func parseCSV(r io.Reader) ([]provider.Offering, error) { if err != nil { return nil, fmt.Errorf("row %d: price_per_hour %q: %w", i, field(rec, colPrice), err) } + // ParseFloat takes "NaN" and "Inf" as valid floats. Refused at the boundary because a rate + // is pinned onto the claim and integrated from there — the damage is downstream, in a + // durable ledger and a counter that cannot be corrected, not in this row. + if math.IsNaN(price) || math.IsInf(price, 0) { + return nil, fmt.Errorf("row %d: price_per_hour %q is not a finite number", i, field(rec, colPrice)) + } available, err := strconv.ParseBool(field(rec, colAvailable)) if err != nil { return nil, fmt.Errorf("row %d: available %q: %w", i, field(rec, colAvailable), err) diff --git a/pkg/provider/catalog/catalog_test.go b/pkg/provider/catalog/catalog_test.go index 35b9c4e..269a5d4 100644 --- a/pkg/provider/catalog/catalog_test.go +++ b/pkg/provider/catalog/catalog_test.go @@ -17,6 +17,7 @@ limitations under the License. package catalog import ( + "errors" "os" "path/filepath" "reflect" @@ -144,6 +145,26 @@ func TestLoadFrom_OptionalColumnsAndComments(t *testing.T) { } } +// ParseFloat accepts these as numbers, so only an explicit check keeps them out. Refused at load +// rather than downstream: the rate is pinned onto a claim and integrated from there, into a durable +// ledger and a counter that cannot be corrected afterwards. +func TestLoadFrom_RejectsNonFinitePrice(t *testing.T) { + for _, price := range []string{"NaN", "+Inf", "Infinity", "-Inf"} { + t.Run(price, func(t *testing.T) { + dir := t.TempDir() + csv := "accelerator_type,capacity_type,price_per_hour,available,updated\n" + + "H100,OnDemand," + price + ",true,2026-07-25\n" + if err := os.WriteFile(filepath.Join(dir, "modal.csv"), []byte(csv), 0o644); err != nil { + t.Fatal(err) + } + + if _, err := LoadFrom(dir); err == nil { + t.Fatalf("LoadFrom accepted price_per_hour %q, want an error", price) + } + }) + } +} + func TestLoadFrom_OverrideDir(t *testing.T) { dir := t.TempDir() csv := "accelerator_type,capacity_type,price_per_hour,available,updated\n" + @@ -271,6 +292,122 @@ func TestBaseMapAccelerator_SkipsUnavailable(t *testing.T) { } } +// The two catalog shapes price differently: a GPUCount 0 row (Modal) quotes ONE +// accelerator and scales with the count, while a row carrying a count (AWS) quotes the +// whole instance and must not be scaled. +func TestBasePricePerHour_Shapes(t *testing.T) { + od, sp := nebulav1alpha1.CapacityOnDemand, nebulav1alpha1.CapacitySpot + type pr = provider.PriceRequest + rows := []provider.Offering{ + {AcceleratorType: "H100", CapacityType: od, PricePerHour: 3.95, Available: true}, + { + AcceleratorType: "L4", AcceleratorID: "g6.48xlarge", GPUCount: 8, + CapacityType: od, PricePerHour: 13.350, Available: true, + }, + { + AcceleratorType: "L4", AcceleratorID: "g6.48xlarge", GPUCount: 8, + CapacityType: sp, PricePerHour: 4.672, Available: true, + }, + { + AcceleratorType: "L4", AcceleratorID: "g6.xlarge", GPUCount: 1, + CapacityType: od, PricePerHour: 0.805, Available: true, + }, + } + base := Base{ProviderName: "p", Catalog: fakeLookup{rows: rows}} + + cases := map[string]struct { + req provider.PriceRequest + want float64 + }{ + "per-accelerator row scales with count": {pr{AcceleratorType: "H100", Count: 2, CapacityType: od}, 7.90}, + "per-instance row does not scale": {pr{AcceleratorType: "L4", Count: 8, CapacityType: od}, 13.350}, + "count selects the row": {pr{AcceleratorType: "L4", Count: 1, CapacityType: od}, 0.805}, + "capacity type selects the row": {pr{AcceleratorType: "L4", Count: 8, CapacityType: sp}, 4.672}, + // An empty tier is the pool-declares-nothing candidate, priced as OnDemand rather + // than as whichever row comes first. + "empty tier is OnDemand": {pr{AcceleratorType: "L4", Count: 8}, 13.350}, + "case insensitive": {pr{AcceleratorType: "h100", Count: 1, CapacityType: od}, 3.95}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got, err := base.PricePerHour(tc.req) + if err != nil { + t.Fatalf("PricePerHour(%+v): %v", tc.req, err) + } + if got != tc.want { + t.Fatalf("PricePerHour(%+v) = %v, want %v", tc.req, got, tc.want) + } + }) + } +} + +// Every unpriceable request must come back as ErrNoPrice so a caller can swallow it the +// way it swallows a provider with no Pricer at all — except a malformed one, which is a +// bug and must stay loud. +func TestBasePricePerHour_NoPrice(t *testing.T) { + od := nebulav1alpha1.CapacityOnDemand + base := Base{ProviderName: "p", Catalog: fakeLookup{rows: []provider.Offering{ + {AcceleratorType: "H100", CapacityType: od, PricePerHour: 3.95, Available: true}, + {AcceleratorType: "A100-80GB", CapacityType: od, PricePerHour: 2.50, Available: false}, + {AcceleratorType: "B200", CapacityType: od, PricePerHour: 0, Available: true}, + }}} + + noPrice := map[string]provider.PriceRequest{ + "cpu-only workload": {Count: 0}, + "unknown type": {AcceleratorType: "TPU-v4", Count: 1, CapacityType: od}, + "unavailable row": {AcceleratorType: "A100-80GB", Count: 1, CapacityType: od}, + "unpriced row": {AcceleratorType: "B200", Count: 1, CapacityType: od}, + "tier has no row": {AcceleratorType: "H100", Count: 1, CapacityType: nebulav1alpha1.CapacitySpot}, + "unknown tier": {AcceleratorType: "H100", Count: 1, CapacityType: "Reserved"}, + } + for name, req := range noPrice { + t.Run(name, func(t *testing.T) { + got, err := base.PricePerHour(req) + if !errors.Is(err, provider.ErrNoPrice) { + t.Fatalf("PricePerHour(%+v) err = %v, want ErrNoPrice", req, err) + } + if got != 0 { + t.Fatalf("PricePerHour(%+v) = %v, want 0 alongside the error", req, got) + } + }) + } + + // A counted accelerator with no count is contradictory input, not a missing price: + // swallowing it as ErrNoPrice would report a GPU workload as costing nothing. + _, err := base.PricePerHour(provider.PriceRequest{AcceleratorType: "H100", Count: 0, CapacityType: od}) + if err == nil || errors.Is(err, provider.ErrNoPrice) { + t.Fatalf("PricePerHour(H100 x0) err = %v, want a loud non-ErrNoPrice error", err) + } +} + +// The embedded CSVs must price through unchanged, which is what pins the per-GPU (Modal) +// against per-instance (AWS) reading of price_per_hour to the real data. +func TestBasePricePerHour_EmbeddedCatalog(t *testing.T) { + c, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + od := nebulav1alpha1.CapacityOnDemand + + modal := Base{ProviderName: "modal", Catalog: c} + got, err := modal.PricePerHour(provider.PriceRequest{AcceleratorType: "H100", Count: 2, CapacityType: od}) + if err != nil { + t.Fatalf("modal H100 x2: %v", err) + } + if got != 2*3.95 { + t.Fatalf("modal H100 x2 = %v, want %v (per-GPU rate x count)", got, 2*3.95) + } + + aws := Base{ProviderName: "aws", Catalog: c} + got, err = aws.PricePerHour(provider.PriceRequest{AcceleratorType: "H100", Count: 8, CapacityType: od}) + if err != nil { + t.Fatalf("aws H100 x8: %v", err) + } + if got != 98.320 { + t.Fatalf("aws H100 x8 = %v, want 98.320 (whole-instance rate, unscaled)", got) + } +} + func TestOfferings_CopyIsolation(t *testing.T) { c, err := Load() if err != nil { diff --git a/pkg/provider/catalog/data/modal.csv b/pkg/provider/catalog/data/modal.csv index d4aee2d..d8d36d8 100644 --- a/pkg/provider/catalog/data/modal.csv +++ b/pkg/provider/catalog/data/modal.csv @@ -29,13 +29,15 @@ # region BLANK — Modal is region-simple # updated YYYY-MM-DD the row was last verified accelerator_type,accelerator_id,gpu_count,capacity_type,price_per_hour,available,region,updated -T4,,,OnDemand,0.59,true,,2026-07-25 -L4,,,OnDemand,0.80,true,,2026-07-25 -A10G,,,OnDemand,1.10,true,,2026-07-25 -L40S,,,OnDemand,1.95,true,,2026-07-25 -A100-40GB,,,OnDemand,2.10,true,,2026-07-25 -A100-80GB,,,OnDemand,2.50,true,,2026-07-25 +T4,,,OnDemand,0.59,true,,2026-09-02 +L4,,,OnDemand,0.80,true,,2026-09-02 +A10G,,,OnDemand,1.10,true,,2026-09-02 +L40S,,,OnDemand,1.95,true,,2026-09-02 +A100-40GB,,,OnDemand,2.10,true,,2026-09-02 +A100-80GB,,,OnDemand,2.50,true,,2026-09-02 # "H100!", not "H100": Modal silently upgrades a bare H100 request onto an H200, and # the "!" suffix is its documented opt-out. -H100,H100!,,OnDemand,3.95,true,,2026-07-25 -H200,,,OnDemand,4.55,true,,2026-07-25 +H100,H100!,,OnDemand,3.95,true,,2026-09-02 +H200,,,OnDemand,4.54,true,,2026-09-02 +B200,,,OnDemand,6.25,true,,2026-09-02 +B300,,,OnDemand,7.10,true,,2026-09-02 diff --git a/pkg/provider/catalog/data/pricing.go b/pkg/provider/catalog/data/pricing.go new file mode 100644 index 0000000..ddf4459 --- /dev/null +++ b/pkg/provider/catalog/data/pricing.go @@ -0,0 +1,64 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package data holds the price catalog. The per-accelerator rows live in the CSVs +// (embedded by the parent package, so a price change is a reviewable data diff); this +// file holds the rates that are NOT per-accelerator, and so have no CSV row to sit on. +package data + +// Modal meters CPU and memory SEPARATELY from the accelerator, so a sandbox's hourly +// cost is the GPU price PLUS these. Not universal: AWS bundles both into the instance +// price (p5.48xlarge's $98.320/hr already covers its vCPU and RAM), so a provider with +// no rates here is one whose CSV price is already all-in. +// +// Modal publishes these PER SECOND, so the literal stays exactly as printed on the price +// page and the scaling to an hour is left in the expression: the number a reviewer compares +// is the number Modal wrote, and the conversion is a compile-time constant fold. Hourly at +// all because every price downstream of provider.Offering.PricePerHour is. +// +// These are the SANDBOX/NOTEBOOK rates, ~3x Modal's standard Function rates ($0.0000131 +// and $0.00000222 per second). The tier follows what we create, not what is cheapest: a +// NodeClaim becomes one Modal Sandbox (see modal.Client.CreateSandbox). Getting it wrong is +// nearly invisible on a GPU sandbox, where the accelerator dominates, and a 3x undercount +// on a CPU-only one, where these two rates are the whole bill. +// +// The GPU price is deliberately absent: it is a modal.csv row (H100 at $3.95 per GPU +// per hour), so each number keeps a single source of truth. +const ( + ModalCPUPricePerCoreHour = 0.00003942 * 60 * 60 + ModalMemoryPricePerGiBHour = 0.00000667 * 60 * 60 +) + +// mibPerGiB converts a Pod's MiB request to the GiB the memory rate is quoted in. +const mibPerGiB = 1024 + +// ModalCPUCostPerHour and ModalMemoryCostPerHour are what Modal charges for a sandbox's +// CPU and memory, to be ADDED to its accelerator price. One function per published Modal +// rate, so each stays checkable against Modal's price page on its own. +// +// Each takes the unit the adapter already carries — fractional physical cores, and MiB +// (see modal.SandboxSpec) — so no conversion happens at the call site, which is where a +// factor-of-1024 slip would hide. +// +// Reservation, not usage: a sandbox bursting above its request toward CPULimit may bill +// above these. +func ModalCPUCostPerHour(cpuCores float64) float64 { + return cpuCores * ModalCPUPricePerCoreHour +} + +func ModalMemoryCostPerHour(memoryMiB int) float64 { + return float64(memoryMiB) / mibPerGiB * ModalMemoryPricePerGiBHour +} diff --git a/pkg/provider/modal/client.go b/pkg/provider/modal/client.go index b6d4557..fd20836 100644 --- a/pkg/provider/modal/client.go +++ b/pkg/provider/modal/client.go @@ -31,6 +31,7 @@ import ( "google.golang.org/grpc/status" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/intstr" + logf "sigs.k8s.io/controller-runtime/pkg/log" nebulav1alpha1 "github.com/InftyAI/Nebula/api/v1alpha1" "github.com/InftyAI/Nebula/pkg/provider" @@ -139,6 +140,9 @@ func (c *sdkClient) CreateSandbox(ctx context.Context, spec SandboxSpec) (string if spec.Image == "" { return "", Credential{}, fmt.Errorf("modal: empty image in sandbox spec") } + log := logf.FromContext(ctx) + + buildStart := time.Now() image, err := c.imageFor(ctx, spec) if err != nil { return "", Credential{}, err @@ -149,6 +153,7 @@ func (c *sdkClient) CreateSandbox(ctx context.Context, spec SandboxSpec) (string if err != nil { return "", Credential{}, err } + imageBuild := time.Since(buildStart) probe, err := modalProbe(spec.ReadinessProbe) if err != nil { @@ -160,6 +165,7 @@ func (c *sdkClient) CreateSandbox(ctx context.Context, spec SandboxSpec) (string return "", Credential{}, err } + createStart := time.Now() sb, err := c.mc.Sandboxes.Create(ctx, app, image, &modal.SandboxCreateParams{ Command: spec.Command, // Env is the whole environment, including values resolved from this cluster's @@ -190,10 +196,18 @@ func (c *sdkClient) CreateSandbox(ctx context.Context, spec SandboxSpec) (string if err != nil { return "", Credential{}, err } + // Emitted here rather than folded into one line at the end, so a call killed by + // ProvisionTimeout still records how far it got and how long the earlier legs took. + log.V(1).Info("modal sandbox created", "sandboxID", sb.SandboxID, + "imageBuild", imageBuild.String(), "create", time.Since(createStart).String()) + + mintStart := time.Now() cred, err := c.mintCredential(ctx, sb, firstPort(spec.Ports)) if err != nil { return "", Credential{}, err } + log.V(1).Info("modal connect credential minted", "sandboxID", sb.SandboxID, + "mint", time.Since(mintStart).String()) return sb.SandboxID, cred, nil } @@ -263,6 +277,11 @@ func (c *sdkClient) registrySecret(ctx context.Context, kv map[string]string) (* // credential belongs in an access-controlled Secret, and this layer has no cluster // access, so it hands the pair up to the virtual kubelet, which writes it. // +// CreateConnectToken BLOCKS until Modal has assigned the sandbox a worker, and that wait is +// load-bearing: it is what makes a returning CreateSandbox proof of a placed GPU rather than +// of an accepted request (see Provision). Modal caps it itself, at ~3m40s, so a queue longer +// than the cap fails a Pod whose sandbox would have been placed. +// // A failure is REPORTED, never swallowed into a zero credential. Minting is one-shot with // no read-back, so a dropped error loses the credential of a sandbox that exists and is // billing — silently, since a caller handed an empty pair has nothing to log. The Pod fails diff --git a/pkg/provider/modal/modal.go b/pkg/provider/modal/modal.go index 914d1a7..af4bf17 100644 --- a/pkg/provider/modal/modal.go +++ b/pkg/provider/modal/modal.go @@ -56,6 +56,7 @@ import ( nebulav1alpha1 "github.com/InftyAI/Nebula/api/v1alpha1" "github.com/InftyAI/Nebula/pkg/provider" "github.com/InftyAI/Nebula/pkg/provider/catalog" + "github.com/InftyAI/Nebula/pkg/provider/catalog/data" "github.com/InftyAI/Nebula/pkg/util" ) @@ -66,11 +67,16 @@ import ( // different ceiling sets spec.activeDeadlineSeconds, which maps straight through. const defaultSandboxTimeout = 24 * time.Hour -// provisionTimeout raises the vnode handler's generic Provision deadline, because Provision -// here BLOCKS on the image build (see sdkClient.buildImage): a cold image is pulled into -// Modal's cache on this call, and the create and credential legs only get what the build -// leaves them. -const provisionTimeout = 10 * time.Minute +// provisionTimeout raises the vnode handler's generic Provision deadline, because Provision here +// BLOCKS twice: a cold image is pulled into Modal's cache on this call (see sdkClient.buildImage), +// and the credential mint then waits for a worker (see sdkClient.mintCredential). +// +// The two legs SHARE this budget, and the mint is the greedy one: Modal caps it at ~3m43s of its +// own accord, so it always fails before this deadline does and the build is left roughly the +// remaining minute. That is the cost of a tight ceiling here — a cold pull of a large image can +// outrun it — and it is deliberate: a call that will die at 3m43s anyway should not hold a +// pod-controller worker for ten minutes first. +const provisionTimeout = 5 * time.Minute // compile-time assertions that Provider satisfies the interfaces. LogStreamer and // Executor are the optional halves: they are what make `kubectl logs` and `kubectl exec` @@ -87,7 +93,11 @@ var ( // real implementation (Modal SDK/HTTP) and a fake (tests) are interchangeable. type Client interface { // CreateSandbox launches one sandbox from spec and returns its Modal id plus the - // connect credential minted for it. Minting is one-shot and there is no read-back, so + // connect credential minted for it. It returns only once Modal has PLACED the sandbox on + // a worker, because the mint blocks on that (see sdkClient.mintCredential) — so a queued + // GPU is a call that blocks for minutes, and a successful return means real capacity. + // + // Minting is one-shot and there is no read-back, so // a caller that drops the credential can only get another from MintConnectCredential. // A sandbox that could not be given one is unreachable, so a failed mint is an ERROR // with no id, not a zero credential — the sandbox may exist, and the claim tag is what @@ -330,6 +340,36 @@ func (p *Provider) ExpandRegions(declared []string) []string { return []string{strings.Join(regions, regionSeparator)} } +// PricePerHour overrides catalog.Base's all-in reading of the catalog, because Modal +// meters CPU and memory SEPARATELY from the accelerator: a modal.csv row prices ONE GPU +// and nothing else, so the sandbox's real rate is that plus what its reservation costs. +// (AWS needs no override — a p5.48xlarge's price already covers its vCPU and RAM.) +// +// It also prices the CPU-ONLY sandbox that Base refuses. Base can only reject an empty +// accelerator type, having nothing but per-accelerator rows to match; Modal genuinely runs +// CPU-only work, and here the two non-accelerator rates are the whole price. +// +// ErrNoPrice for a CPU-only sandbox that reserves neither CPU nor memory: Modal then +// applies its own defaults, and we do not know them. Unpriced is the honest answer — a 0 +// would be read as free. A GPU sandbox in that state still prices, understating by those +// same defaults, which is immaterial beside the accelerator. +func (p *Provider) PricePerHour(req provider.PriceRequest) (float64, error) { + metered := data.ModalCPUCostPerHour(req.CPUCores) + data.ModalMemoryCostPerHour(req.MemoryMiB) + + if req.AcceleratorType == "" { + if req.CPUCores <= 0 || req.MemoryMiB <= 0 { + return 0, fmt.Errorf("modal: cpu-only sandbox has an unpriced default cpu or memory reservation: %w", + provider.ErrNoPrice) + } + return metered, nil + } + gpu, err := p.Base.PricePerHour(req) + if err != nil { + return 0, err // ErrNoPrice or a malformed request, both already worded by Base + } + return gpu + metered, nil +} + // Capabilities implements provider.Provider. See the package doc for why each // trait is set the way it is. func (p *Provider) Capabilities() provider.Capabilities { @@ -340,18 +380,17 @@ func (p *Provider) Capabilities() provider.Capabilities { NativeTags: true, // sandbox tags carry identity PreemptionNotice: 0, // no push; poll-based detection PollInterval: 0, // OnDemand-only (never preempts) → the default cadence is fine - ProvisionTimeout: provisionTimeout, // Provision blocks on the image build + ProvisionTimeout: provisionTimeout, // Provision blocks on the build, then on placement } } // Provision implements provider.Provider. The Pod is the source of truth for // the workload; req carries only the claim identity and capacity tier. // -// A fresh sandbox is NEVER reserved. Create returns as soon as Modal accepts it: the id is -// real (listable, terminable, fully on the hook for teardown) but the GPU may be queued for -// minutes. So id and capacity are two separate facts here, unlike AWS, and reserved=false -// is what keeps the Pod at the provisioning reason instead of claiming to be initializing. -// The queued→running transition is then observed through the poll loop's List. +// A fresh sandbox comes back RESERVED, but not because Modal's create says so — it accepts a +// sandbox whose GPU is still queued. The mint is what waits (see mintCredential), so id and +// capacity arrive together, at the price of a call that blocks for as long as the queue does — +// bounded by Modal's own ~3m43s cap on the mint, not by provisionTimeout. // // The connect credential comes back on this call and only this call, since Modal mints it // once with no read-back. The caller must persist it or it is lost; see mintCredential. @@ -368,9 +407,9 @@ func (p *Provider) Provision( // Idempotency: if a sandbox already carries this claim tag, return it rather // than creating a second (guards against a retry after a partial create). // - // Unlike a fresh create, this sandbox has been OBSERVED, so its state is known: one - // the poll loop reports Running has capacity, one still queued does not. That is more - // information than a create can return, so report it rather than a flat false. + // Unlike a fresh create, nothing here waits for placement, so the OBSERVED state is the only + // evidence of capacity: one the poll loop reports Running has it, one still queued does not. + // This is therefore the one branch that can report reserved=false. // // No credential comes back, per the Provider contract: minting is one-shot with no // read-back, and a fresh token revokes nothing, so re-minting for a sandbox whose token is @@ -394,7 +433,7 @@ func (p *Provider) Provision( } return provider.ProvisionResult{ InstanceID: id, - Reserved: false, + Reserved: true, // mintCredential is a blocking call, so the GPU is reserved on return ConnectURL: cred.URL, ConnectToken: cred.Token, }, nil diff --git a/pkg/provider/modal/modal_test.go b/pkg/provider/modal/modal_test.go index 8fb3d3b..2034f2a 100644 --- a/pkg/provider/modal/modal_test.go +++ b/pkg/provider/modal/modal_test.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "io" + "math" "slices" "strings" "sync" @@ -193,11 +194,10 @@ func TestProvision_GPUPod(t *testing.T) { if id != "sb-1" { t.Fatalf("id = %q, want sb-1", id) } - // Create only means the control plane ACCEPTED the sandbox — the GPU may still be - // queued — so a fresh sandbox is never reserved. Claiming otherwise would let the - // Pod report Initializing while nothing has been allocated. - if reserved { - t.Fatal("reserved = true; a freshly created sandbox may still be queued for capacity") + // A returning CreateSandbox means the sandbox is PLACED, not merely accepted: the + // credential mint inside it blocks until Modal assigns a worker (see mintCredential). + if !reserved { + t.Fatal("reserved = false for a created sandbox; the mint only returns once it is placed") } if f.lastSpec.GPU != "H100" || f.lastSpec.GPUCount != 2 { t.Fatalf("spec GPU=%q count=%d, want H100/2", f.lastSpec.GPU, f.lastSpec.GPUCount) @@ -1751,3 +1751,69 @@ func TestSandboxSpecStringRedactsRegistryAuth(t *testing.T) { t.Errorf("String() = %q, must not contain the password", got) } } + +// Modal's rate is the GPU row PLUS the separately metered CPU and memory, and a CPU-only +// sandbox is priced from those two alone — the case catalog.Base refuses outright. +func TestPricePerHour_AddsCPUAndMemory(t *testing.T) { + p := newTestProvider(&fakeClient{}) + od := nebulav1alpha1.CapacityOnDemand + + // Modal's published per-second sandbox rates: 4 cores = $0.567648/hr, 8 GiB = $0.192096/hr. + // Transcribed again here rather than imported from data, so a slip in those constants fails + // this test instead of being multiplied through it. + const cpuAndMem = 4*0.00003942*60*60 + 8*0.00000667*60*60 + + cases := map[string]struct { + req provider.PriceRequest + want float64 + }{ + "gpu plus metered resources": { + provider.PriceRequest{AcceleratorType: "H100", Count: 2, CapacityType: od, CPUCores: 4, MemoryMiB: 8192}, + 2*3.95 + cpuAndMem, + }, + "gpu with no reservation is the row alone": { + provider.PriceRequest{AcceleratorType: "H100", Count: 1, CapacityType: od}, + 3.95, + }, + "cpu-only is the metered resources alone": { + provider.PriceRequest{CPUCores: 4, MemoryMiB: 8192}, + cpuAndMem, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got, err := p.PricePerHour(tc.req) + if err != nil { + t.Fatalf("PricePerHour(%+v): %v", tc.req, err) + } + if math.Abs(got-tc.want) > 1e-9 { + t.Fatalf("PricePerHour(%+v) = %v, want %v", tc.req, got, tc.want) + } + }) + } +} + +// A CPU-only sandbox reserving nothing runs on Modal's own defaults, which we do not know: +// unpriced is honest, 0 would read as free. An unknown accelerator still comes back as +// Base's ErrNoPrice rather than silently costing only its CPU and memory. +func TestPricePerHour_NoPrice(t *testing.T) { + p := newTestProvider(&fakeClient{}) + + for name, req := range map[string]provider.PriceRequest{ + "cpu-only reserving nothing": {}, + "unknown accelerator": { + AcceleratorType: "TPU-v4", Count: 1, + CapacityType: nebulav1alpha1.CapacityOnDemand, CPUCores: 4, MemoryMiB: 8192, + }, + } { + t.Run(name, func(t *testing.T) { + got, err := p.PricePerHour(req) + if !errors.Is(err, provider.ErrNoPrice) { + t.Fatalf("PricePerHour(%+v) err = %v, want ErrNoPrice", req, err) + } + if got != 0 { + t.Fatalf("PricePerHour(%+v) = %v, want 0 alongside the error", req, got) + } + }) + } +} diff --git a/pkg/provider/pricing.go b/pkg/provider/pricing.go new file mode 100644 index 0000000..1ccdec0 --- /dev/null +++ b/pkg/provider/pricing.go @@ -0,0 +1,75 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package provider + +import ( + "errors" + + nebulav1alpha1 "github.com/InftyAI/Nebula/api/v1alpha1" +) + +// ErrNoPrice: this Pricer has no price for this request — no matching catalog row, or one +// that carries no usable price. Kept a distinct sentinel so a caller can treat it exactly +// as it treats a provider implementing no Pricer at all (report no cost and carry on), +// while a genuinely malformed request still surfaces as a loud error. +// +// It is NOT a provision failure and must never reach ClassifyError: a candidate nobody can +// price is still perfectly launchable. +var ErrNoPrice = errors.New("provider: no price for request") + +// PriceRequest is what to price. It describes a CANDIDATE, not an instance, and carries +// no Pod: the optimizer compares candidates before anything is provisioned, and a +// Pod-shaped signature would put pricing out of reach there. +// +// No region axis, because no catalog row is region-partitioned today (every CSV leaves +// the column empty), so a price is uniform across a provider's regions. Add the axis back +// alongside the first region-varying row. +// TODO: maybe need to support region in the future. +type PriceRequest struct { + // AcceleratorType is the canonical type ("H100"), empty for a CPU-only workload. + AcceleratorType string + // Count is how many accelerators. It does two jobs, and both providers need it: it + // SELECTS the row where a provider bakes the count into the offering (AWS's + // g4dn.xlarge is T4x1, g4dn.metal is T4x8), and it MULTIPLIES the rate where the + // provider prices per accelerator instead (Modal). Which one applies is read off the + // matched row's GPUCount rather than hardcoded per provider — see Offering.GPUCount. + Count int32 + // CapacityType selects between a row's Spot and OnDemand prices, which differ + // sharply (AWS p5.48xlarge: $34.412 Spot vs $98.320 OnDemand). + CapacityType nebulav1alpha1.CapacityType + // CPUCores and MemoryMiB are the workload's RESERVATION, priced only by providers + // that meter them separately from the accelerator. Ignored by a provider whose + // instance price is all-in. + CPUCores float64 + MemoryMiB int +} + +// Pricer reports the hourly USD cost of what a Provision would create: ONE rate, with every +// resource the provider bills for folded in. +// +// No per-resource breakdown, because providers disagree on what is even billed separately — +// AWS sells an instance whose price already covers its vCPU and RAM, while Modal meters GPU, +// CPU and memory apart. Splitting the all-in case means inventing an allocation no invoice +// line matches. Add a breakdown when a provider's own billing hands us one. +// +// It is OPTIONAL, resolved by type assertion like LogStreamer and Executor: a provider that +// cannot price its instances implements nothing and its cost goes unreported, which is +// better than a confidently wrong number. Adapters embedding catalog.Base get an +// implementation for free. +type Pricer interface { + PricePerHour(PriceRequest) (float64, error) +} diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index ae6e52c..1a44466 100644 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -54,10 +54,10 @@ type Provider interface { // credential. Reserved means the provider committed real capacity, not merely accepted // the request — two different guarantees an id alone cannot express: // - // - AWS reserves: an *instant* CreateFleet is synchronous, so an id means EC2 found - // capacity and the instance is booting; a shortfall is an error. - // - Modal does not: Create returns once the control plane accepts the sandbox, and - // the GPU may stay queued for minutes on a large shape. + // - AWS reserves in its API: an *instant* CreateFleet is synchronous, so an id means EC2 + // found capacity and the instance is booting; a shortfall is an error. + // - Modal's create does not — it accepts a sandbox whose GPU may stay queued for minutes + // — so its adapter WAITS on a later call that Modal holds until a worker is assigned. // // Callers use it for honest status: an unreserved instance is not yet initializing, so // its Pod stays at the provisioning reason. It says nothing about readiness, which is diff --git a/pkg/util/accelerator.go b/pkg/util/accelerator.go index c56798c..f1ebea9 100644 --- a/pkg/util/accelerator.go +++ b/pkg/util/accelerator.go @@ -18,6 +18,8 @@ package util import ( "fmt" + "strconv" + "strings" corev1 "k8s.io/api/core/v1" @@ -79,6 +81,25 @@ func AcceleratorPool(accelerator string, count int32) string { return fmt.Sprintf("%s%s%d", accelerator, acceleratorSep, count) } +// SplitAcceleratorPool is the inverse of AcceleratorPool: it recovers the type and count +// from a "type:count" pool identity. For readers that hold only the joined form — +// NodeClaimSpec.Accelerator — and need the two apart, as metrics labels do so both +// "every H100 size" and "every 8-GPU request" stay aggregatable. +// +// ("", 0) for an empty pool (a CPU-only claim) and for anything not in the grammar, so a +// hand-edited claim degrades to "no accelerator" rather than minting a garbage label value. +func SplitAcceleratorPool(pool string) (accelerator string, count int32) { + typ, n, found := strings.Cut(pool, acceleratorSep) + if !found || typ == "" { + return "", 0 + } + parsed, err := strconv.ParseInt(n, 10, 32) + if err != nil || parsed <= 0 { + return "", 0 + } + return typ, int32(parsed) +} + // gpuCount returns the largest nvidia.com/gpu quantity requested across the // Pod's containers, preferring limits and falling back to requests (Kubernetes // treats an extended resource's request and limit as equal, but a Pod may set diff --git a/pkg/util/accelerator_test.go b/pkg/util/accelerator_test.go index f7096c2..574f3f9 100644 --- a/pkg/util/accelerator_test.go +++ b/pkg/util/accelerator_test.go @@ -99,3 +99,47 @@ func TestAcceleratorRequest_CountFromRequestsWhenNoLimit(t *testing.T) { t.Fatalf("AcceleratorRequest = (%q, %d), want (h100, 3)", accel, count) } } + +// SplitAcceleratorPool must round-trip AcceleratorPool for every real request, and +// degrade to "no accelerator" for anything outside the grammar rather than minting a +// garbage metrics label. +func TestSplitAcceleratorPool(t *testing.T) { + for _, tc := range []struct { + pool string + wantType string + wantCount int32 + }{ + {"H100:8", "H100", 8}, + {"A100-80GB:1", "A100-80GB", 1}, + {"", "", 0}, // CPU-only claim + {"H100", "", 0}, // no separator + {":8", "", 0}, // no type + {"H100:", "", 0}, // no count + {"H100:x", "", 0}, // unparseable count + {"H100:0", "", 0}, // a count of zero is not a request + {"H100:-2", "", 0}, // negative + } { + t.Run(tc.pool, func(t *testing.T) { + typ, count := SplitAcceleratorPool(tc.pool) + if typ != tc.wantType || count != tc.wantCount { + t.Fatalf("SplitAcceleratorPool(%q) = (%q, %d), want (%q, %d)", + tc.pool, typ, count, tc.wantType, tc.wantCount) + } + }) + } +} + +// The two are inverses: whatever AcceleratorPool joins, SplitAcceleratorPool recovers. +func TestAcceleratorPool_RoundTrip(t *testing.T) { + for _, tc := range []struct { + typ string + count int32 + }{{"H100", 1}, {"H100", 8}, {"A100-80GB", 4}, {"T4", 16}} { + pool := AcceleratorPool(tc.typ, tc.count) + gotType, gotCount := SplitAcceleratorPool(pool) + if gotType != tc.typ || gotCount != tc.count { + t.Fatalf("round trip of (%q, %d) through %q = (%q, %d)", + tc.typ, tc.count, pool, gotType, gotCount) + } + } +} diff --git a/pkg/util/resources.go b/pkg/util/resources.go new file mode 100644 index 0000000..2aba96e --- /dev/null +++ b/pkg/util/resources.go @@ -0,0 +1,61 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +// mibBytes is one MiB, the unit provider.PriceRequest quotes memory in. +const mibBytes = 1024 * 1024 + +// PodReservation reads the workload's CPU and memory RESERVATION in the units +// provider.PriceRequest quotes: fractional physical cores and MiB. Requests, falling +// back to limits, and 0 for either when neither is set — which a provider reads as +// "your default", so a priced 0 is a floor, not a claim that nothing was reserved. +// +// Reservation and not the limit, because a provider metering CPU/memory apart from the +// accelerator (Modal) bills what was held for the workload; a burstable Pod's ceiling is +// not what shows up on the invoice. +// +// The FIRST container only, matching the single-workload-container shape the whole +// provisioning path assumes (see modal.sandboxSpecFromPod). Returns (0, 0) for a Pod with +// no containers. +func PodReservation(pod *corev1.Pod) (cpuCores float64, memoryMiB int) { + if pod == nil || len(pod.Spec.Containers) == 0 { + return 0, 0 + } + c := &pod.Spec.Containers[0] + cpu := reservedQty(c, corev1.ResourceCPU) + mem := reservedQty(c, corev1.ResourceMemory) + // MilliValue is cores*1000; Value is bytes. + return float64(cpu.MilliValue()) / 1000.0, int(mem.Value() / mibBytes) +} + +// reservedQty returns the container's request for name, falling back to its limit, and a +// zero quantity when it declares neither. By value, so the caller never holds a pointer +// into the Pod it was read from. +func reservedQty(c *corev1.Container, name corev1.ResourceName) resource.Quantity { + if q, ok := c.Resources.Requests[name]; ok { + return q + } + if q, ok := c.Resources.Limits[name]; ok { + return q + } + return resource.Quantity{} +} diff --git a/pkg/util/resources_test.go b/pkg/util/resources_test.go new file mode 100644 index 0000000..e7909bb --- /dev/null +++ b/pkg/util/resources_test.go @@ -0,0 +1,109 @@ +/* +Copyright 2026 The InftyAI Team. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func podWith(reqs, limits corev1.ResourceList) *corev1.Pod { + return &corev1.Pod{Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Name: "main", + Resources: corev1.ResourceRequirements{Requests: reqs, Limits: limits}, + }}}} +} + +func TestPodReservation(t *testing.T) { + cases := map[string]struct { + pod *corev1.Pod + wantCPU float64 + wantMiB int + whatFor string + }{ + "requests win over limits": { + pod: podWith( + corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2"), corev1.ResourceMemory: resource.MustParse("4Gi")}, + corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("8"), corev1.ResourceMemory: resource.MustParse("16Gi")}, + ), + wantCPU: 2, wantMiB: 4096, + whatFor: "a burstable Pod is billed for what it reserved, not its ceiling", + }, + "falls back to limits": { + pod: podWith(nil, + corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("8"), corev1.ResourceMemory: resource.MustParse("16Gi")}, + ), + wantCPU: 8, wantMiB: 16384, + whatFor: "Kubernetes defaults the request to the limit", + }, + "fractional cores": { + pod: podWith(corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("250m"), + corev1.ResourceMemory: resource.MustParse("512Mi"), + }, nil), + wantCPU: 0.25, wantMiB: 512, + whatFor: "millicores are the common way to express a fraction", + }, + "decimal memory units convert to MiB": { + pod: podWith(corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1G")}, nil), + wantCPU: 0, wantMiB: 953, // 1e9 / 1048576, truncated + whatFor: "1G is not 1Gi, and the price is quoted per GiB", + }, + "nothing declared": { + pod: podWith(nil, nil), + wantCPU: 0, wantMiB: 0, + whatFor: "0 reaches the provider as its own default", + }, + "no containers": { + pod: &corev1.Pod{}, + wantCPU: 0, wantMiB: 0, + whatFor: "must not panic on a Pod nothing has filled in yet", + }, + "nil pod": { + pod: nil, + wantCPU: 0, wantMiB: 0, + whatFor: "callers hold a Pod that may be absent", + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + cpu, mib := PodReservation(tc.pod) + if cpu != tc.wantCPU || mib != tc.wantMiB { + t.Fatalf("PodReservation = (%v cores, %v MiB), want (%v, %v): %s", + cpu, mib, tc.wantCPU, tc.wantMiB, tc.whatFor) + } + }) + } +} + +// Only the first container counts, matching the single-workload-container shape the +// provisioning path assumes; a sidecar must not inflate the priced reservation. +func TestPodReservation_FirstContainerOnly(t *testing.T) { + pod := podWith(corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("1")}, nil) + pod.Spec.Containers = append(pod.Spec.Containers, corev1.Container{ + Name: "sidecar", + Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("32"), + }}, + }) + + if cpu, _ := PodReservation(pod); cpu != 1 { + t.Fatalf("PodReservation cpu = %v, want 1 (the first container's)", cpu) + } +} diff --git a/pkg/vnode/handler.go b/pkg/vnode/handler.go index 1dd57c4..f231cb9 100644 --- a/pkg/vnode/handler.go +++ b/pkg/vnode/handler.go @@ -383,9 +383,9 @@ func (h *Handler) CreatePod(ctx context.Context, pod *corev1.Pod) error { } // Only a RESERVED instance advances: capacity is committed and it is booting. An - // unreserved id (a Modal sandbox accepted but still queued for a GPU) stays at the - // Provisioning stamped above, which is exactly true — the id is real and must be - // reclaimed, but nothing is allocated yet. + // unreserved id (a Modal sandbox re-attached by its idempotency branch while still queued + // for a GPU) stays at the Provisioning stamped above, which is exactly true — the id is + // real and must be reclaimed, but nothing is allocated yet. // // store runs either way: an id means the instance exists. markStatus first, because // store deep-copies and would otherwise track a copy without the new status.