From f3dbf171741656c4808749010cd29b85c467e4f8 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Wed, 30 Sep 2026 17:09:31 +0100 Subject: [PATCH 1/5] fix aws price Signed-off-by: kerthcet --- pkg/provider/catalog/base.go | 4 +-- pkg/provider/catalog/catalog_test.go | 4 +-- pkg/provider/catalog/data/aws.csv | 48 ++++++++++++++-------------- pkg/provider/catalog/data/pricing.go | 2 +- pkg/provider/pricing.go | 2 +- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/pkg/provider/catalog/base.go b/pkg/provider/catalog/base.go index e12591e..a8d73dd 100644 --- a/pkg/provider/catalog/base.go +++ b/pkg/provider/catalog/base.go @@ -132,8 +132,8 @@ func (b Base) MapAccelerator(canonical string, count int32) (providerAccelerator // 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 +// MapAccelerator can ignore and pricing cannot (AWS p5.48xlarge is $20.839 Spot against +// $55.040 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 diff --git a/pkg/provider/catalog/catalog_test.go b/pkg/provider/catalog/catalog_test.go index 269a5d4..b94cb3b 100644 --- a/pkg/provider/catalog/catalog_test.go +++ b/pkg/provider/catalog/catalog_test.go @@ -403,8 +403,8 @@ func TestBasePricePerHour_EmbeddedCatalog(t *testing.T) { 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) + if got != 55.040 { + t.Fatalf("aws H100 x8 = %v, want 55.040 (whole-instance rate, unscaled)", got) } } diff --git a/pkg/provider/catalog/data/aws.csv b/pkg/provider/catalog/data/aws.csv index f947d1b..8ffafd4 100644 --- a/pkg/provider/catalog/data/aws.csv +++ b/pkg/provider/catalog/data/aws.csv @@ -32,27 +32,27 @@ # region BLANK — stamped per configured region by the live probe # updated YYYY-MM-DD the row was last verified accelerator_type,accelerator_id,gpu_count,capacity_type,price_per_hour,available,region,updated -T4,g4dn.xlarge,1,OnDemand,0.526,true,,2026-07-27 -T4,g4dn.xlarge,1,Spot,0.158,true,,2026-07-27 -T4,g4dn.12xlarge,4,OnDemand,3.912,true,,2026-07-27 -T4,g4dn.12xlarge,4,Spot,1.174,true,,2026-07-27 -T4,g4dn.metal,8,OnDemand,7.824,true,,2026-07-27 -T4,g4dn.metal,8,Spot,2.347,true,,2026-07-27 -A10G,g5.xlarge,1,OnDemand,1.006,true,,2026-07-27 -A10G,g5.xlarge,1,Spot,0.352,true,,2026-07-27 -A10G,g5.12xlarge,4,OnDemand,5.672,true,,2026-07-27 -A10G,g5.12xlarge,4,Spot,1.985,true,,2026-07-27 -A10G,g5.48xlarge,8,OnDemand,16.288,true,,2026-07-27 -A10G,g5.48xlarge,8,Spot,5.700,true,,2026-07-27 -L4,g6.xlarge,1,OnDemand,0.805,true,,2026-07-27 -L4,g6.xlarge,1,Spot,0.282,true,,2026-07-27 -L4,g6.12xlarge,4,OnDemand,4.602,true,,2026-07-27 -L4,g6.12xlarge,4,Spot,1.610,true,,2026-07-27 -L4,g6.48xlarge,8,OnDemand,13.350,true,,2026-07-27 -L4,g6.48xlarge,8,Spot,4.672,true,,2026-07-27 -A100-40GB,p4d.24xlarge,8,OnDemand,32.773,true,,2026-07-29 -A100-40GB,p4d.24xlarge,8,Spot,11.470,true,,2026-07-29 -A100-80GB,p4de.24xlarge,8,OnDemand,40.966,true,,2026-07-27 -A100-80GB,p4de.24xlarge,8,Spot,14.338,true,,2026-07-27 -H100,p5.48xlarge,8,OnDemand,98.320,true,,2026-07-27 -H100,p5.48xlarge,8,Spot,34.412,true,,2026-07-27 +T4,g4dn.xlarge,1,OnDemand,0.526,true,,2026-09-30 +T4,g4dn.xlarge,1,Spot,0.274,true,,2026-09-30 +T4,g4dn.12xlarge,4,OnDemand,3.912,true,,2026-09-30 +T4,g4dn.12xlarge,4,Spot,1.561,true,,2026-09-30 +T4,g4dn.metal,8,OnDemand,7.824,true,,2026-09-30 +T4,g4dn.metal,8,Spot,5.683,true,,2026-09-30 +A10G,g5.xlarge,1,OnDemand,1.006,true,,2026-09-30 +A10G,g5.xlarge,1,Spot,0.466,true,,2026-09-30 +A10G,g5.12xlarge,4,OnDemand,5.672,true,,2026-09-30 +A10G,g5.12xlarge,4,Spot,4.576,true,,2026-09-30 +A10G,g5.48xlarge,8,OnDemand,16.288,true,,2026-09-30 +A10G,g5.48xlarge,8,Spot,8.402,true,,2026-09-30 +L4,g6.xlarge,1,OnDemand,0.805,true,,2026-09-30 +L4,g6.xlarge,1,Spot,0.601,true,,2026-09-30 +L4,g6.12xlarge,4,OnDemand,4.602,true,,2026-09-30 +L4,g6.12xlarge,4,Spot,3.300,true,,2026-09-30 +L4,g6.48xlarge,8,OnDemand,13.350,true,,2026-09-30 +L4,g6.48xlarge,8,Spot,8.292,true,,2026-09-30 +A100-40GB,p4d.24xlarge,8,OnDemand,21.958,true,,2026-09-30 +A100-40GB,p4d.24xlarge,8,Spot,17.861,true,,2026-09-30 +A100-80GB,p4de.24xlarge,8,OnDemand,27.447,true,,2026-09-30 +A100-80GB,p4de.24xlarge,8,Spot,21.514,true,,2026-09-30 +H100,p5.48xlarge,8,OnDemand,55.040,true,,2026-09-30 +H100,p5.48xlarge,8,Spot,20.839,true,,2026-09-30 diff --git a/pkg/provider/catalog/data/pricing.go b/pkg/provider/catalog/data/pricing.go index ddf4459..f844e1d 100644 --- a/pkg/provider/catalog/data/pricing.go +++ b/pkg/provider/catalog/data/pricing.go @@ -21,7 +21,7 @@ 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 +// price (p5.48xlarge's $55.040/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 diff --git a/pkg/provider/pricing.go b/pkg/provider/pricing.go index 1ccdec0..bd64286 100644 --- a/pkg/provider/pricing.go +++ b/pkg/provider/pricing.go @@ -49,7 +49,7 @@ type PriceRequest struct { // 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). + // sharply (AWS p5.48xlarge: $20.839 Spot vs $55.040 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 From b40a61e3bde525232fe71d99a2696d595d5bfb62 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Wed, 30 Sep 2026 21:35:01 +0100 Subject: [PATCH 2/5] use physical cpu for modal Signed-off-by: kerthcet --- .gitignore | 1 + pkg/provider/catalog/data/pricing.go | 3 +- pkg/provider/modal/client.go | 9 ++- pkg/provider/modal/modal.go | 112 ++++++--------------------- pkg/provider/modal/modal_test.go | 87 ++++++++++++--------- pkg/provider/pricing.go | 5 +- pkg/util/resources.go | 19 +++-- pkg/util/resources_test.go | 17 ++-- 8 files changed, 100 insertions(+), 153 deletions(-) diff --git a/.gitignore b/.gitignore index 3b5f33c..45d5271 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,4 @@ __pycache__ .devcontainer bin .claude +*.html diff --git a/pkg/provider/catalog/data/pricing.go b/pkg/provider/catalog/data/pricing.go index f844e1d..91ced0e 100644 --- a/pkg/provider/catalog/data/pricing.go +++ b/pkg/provider/catalog/data/pricing.go @@ -53,8 +53,7 @@ const mibPerGiB = 1024 // (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. +// Exact, since each sandbox is capped at its reservation (see modal.SandboxSpec.CPU). func ModalCPUCostPerHour(cpuCores float64) float64 { return cpuCores * ModalCPUPricePerCoreHour } diff --git a/pkg/provider/modal/client.go b/pkg/provider/modal/client.go index f1a5c8f..5b8fd0c 100644 --- a/pkg/provider/modal/client.go +++ b/pkg/provider/modal/client.go @@ -174,12 +174,13 @@ func (c *sdkClient) CreateSandbox(ctx context.Context, spec SandboxSpec) (string // Secrets (see provider.ProvisionRequest.Env). No Secrets field alongside it: the // SDK hydrates this map into an ephemeral server-side Modal Secret before the // create (mergeEnvIntoSecrets), so nothing named is left in the workspace. - Env: spec.Env, - GPU: gpuReservation(spec.GPU, spec.GPUCount), + Env: spec.Env, + GPU: gpuReservation(spec.GPU, spec.GPUCount), + // Limit = request, so usage can never bill above the recorded price. CPU: spec.CPU, MemoryMiB: spec.MemoryMiB, - CPULimit: spec.CPULimit, - MemoryLimitMiB: spec.MemoryLimitMiB, + CPULimit: spec.CPU, + MemoryLimitMiB: spec.MemoryMiB, EncryptedPorts: spec.Ports, // Nil leaves Modal's SchedulerPlacement unset entirely (the SDK only builds one // when Regions is non-empty), which is the unconstrained, un-multiplied case. diff --git a/pkg/provider/modal/modal.go b/pkg/provider/modal/modal.go index 9dd272b..dbc7322 100644 --- a/pkg/provider/modal/modal.go +++ b/pkg/provider/modal/modal.go @@ -55,7 +55,6 @@ import ( "time" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" nebulav1alpha1 "github.com/InftyAI/Nebula/api/v1alpha1" "github.com/InftyAI/Nebula/pkg/provider" @@ -156,23 +155,10 @@ type SandboxSpec struct { GPU string // GPUCount is how many accelerators to attach (0 for CPU-only). GPUCount int32 - // CPU is the requested cores (fractional, physical), from the Pod's first - // container resource request. Zero lets Modal apply its own default. - CPU float64 - // MemoryMiB is the requested memory in MiB, from the Pod's request. Zero lets - // Modal apply its own default. + // CPU (physical cores) and MemoryMiB come from util.PodReservation; zero is Modal's + // default. Each is sent as both request and limit (see sdkClient.CreateSandbox). + CPU float64 MemoryMiB int - // CPULimit and MemoryLimitMiB are the HARD caps, from the Pod's limits only — - // never from its requests, unlike CPU/MemoryMiB above, which fall back to limits - // when no request is given. - // - // Zero means no cap, which is also what a Pod that declares no limit means, so the - // two vocabularies line up on everything but one case: a positive limit smaller than - // Modal's unit must not truncate into that sentinel (see limitMiB). Without these a Pod's limits - // reached Modal as nothing at all: a limits-only Pod became a RESERVATION of that - // size with an unbounded ceiling — the inverse of what it asked for, and billable. - CPULimit float64 - MemoryLimitMiB int // Ports are the container ports to expose, from the Pod's containerPorts. They // declare to Modal which ports may receive traffic at all, and the connect URL // routes to the first of them (see firstPort) — one token routes to one port. @@ -231,10 +217,10 @@ type SandboxSpec struct { // a pointer, so %v would print an address, and only its presence matters. func (s SandboxSpec) String() string { return fmt.Sprintf("SandboxSpec{Image:%s Command:%v Args:%v WorkingDir:%s Env:%s GPU:%s GPUCount:%d CPU:%g "+ - "CPULimit:%g MemoryMiB:%d MemoryLimitMiB:%d Ports:%v Regions:%v Egress:%s "+ + "MemoryMiB:%d Ports:%v Regions:%v Egress:%s "+ "EgressTargets:%v Timeout:%s Tags:%v ReadinessProbe:%t RegistryAuth:%s}", s.Image, s.Command, s.Args, s.WorkingDir, provider.RedactedEnv(s.Env), s.GPU, s.GPUCount, s.CPU, - s.CPULimit, s.MemoryMiB, s.MemoryLimitMiB, s.Ports, s.Regions, s.EgressMode, + s.MemoryMiB, s.Ports, s.Regions, s.EgressMode, s.EgressTargets, s.Timeout, s.Tags, s.ReadinessProbe != nil, s.RegistryAuth) } @@ -412,7 +398,8 @@ func (p *Provider) ResolveRegions(declared, narrowTo []string) []string { // 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) + metered := data.ModalCPUCostPerHour(physicalCores(req.CPUCores)) + + data.ModalMemoryCostPerHour(req.MemoryMiB) if req.AcceleratorType == "" { if req.CPUCores <= 0 || req.MemoryMiB <= 0 { @@ -624,6 +611,7 @@ func (p *Provider) sandboxSpecFromPod(pod *corev1.Pod, req provider.ProvisionReq tags[ProbeTagKey] = probeTagValue } + vCPUs, memMiB := util.PodReservation(pod) spec := SandboxSpec{ Image: c.Image, Command: slices.Clone(c.Command), @@ -633,12 +621,10 @@ func (p *Provider) sandboxSpecFromPod(pod *corev1.Pod, req provider.ProvisionReq // everything envFrom/valueFrom referenced. pod.Spec.Containers[0].Env is NOT read // here: it holds references this adapter has no cluster access to follow. See // provider.ProvisionRequest.Env. - Env: req.Env, - CPU: cpuCores(&c), - MemoryMiB: memoryMiB(&c), - CPULimit: cpuLimitCores(&c), - MemoryLimitMiB: memoryLimitMiB(&c), - Ports: containerPorts(&c), + Env: req.Env, + CPU: physicalCores(vCPUs), + MemoryMiB: memMiB, + Ports: containerPorts(&c), // An empty request region stays an empty slice, not a one-element [""]: that // is the unconstrained case (no region declared on the pool), and it must // reach Modal as "no placement constraint" — its widest pool and its @@ -724,71 +710,21 @@ func checkRegistryAuth(a *provider.RegistryAuth) error { } } -// cpuCores reads the container's CPU request as fractional physical cores (Modal's -// unit). It prefers requests, falling back to limits, and returns 0 (→ Modal -// default) when neither is set. -func cpuCores(c *corev1.Container) float64 { return cores(resourceQty(c, corev1.ResourceCPU)) } - -// memoryMiB reads the container's memory request in MiB (Modal's unit), preferring -// requests over limits. Returns 0 (→ Modal default) when neither is set. -func memoryMiB(c *corev1.Container) int { return mib(resourceQty(c, corev1.ResourceMemory)) } - -// cpuLimitCores and memoryLimitMiB read the LIMITS, with no fallback to the request: a -// request is a floor, and reusing it as a ceiling would cap a burstable Pod that never -// asked to be capped. Zero (no limit declared) reaches Modal as "no limit", matching -// Kubernetes. The request/limit asymmetry is entirely in which lookup they use. -func cpuLimitCores(c *corev1.Container) float64 { return cores(limitQty(c, corev1.ResourceCPU)) } -func memoryLimitMiB(c *corev1.Container) int { return limitMiB(limitQty(c, corev1.ResourceMemory)) } - -// cores converts a CPU quantity to Modal's unit, fractional physical cores. MilliValue -// is cores*1000. A nil quantity (unset) is 0, which lets Modal apply its own default. -func cores(q *resource.Quantity) float64 { - if q == nil { - return 0 - } - return float64(q.MilliValue()) / 1000.0 -} +const ( + // vCPUsPerModalCore: a Kubernetes CPU is a vCPU, but Modal requests and bills physical + // cores of 2 vCPU each (modal.com/pricing). + vCPUsPerModalCore = 2 + // minModalCores is Modal's per-container minimum. + minModalCores = 0.125 +) -// mib converts a memory quantity to Modal's unit, MiB. Nil is 0, as in cores. -func mib(q *resource.Quantity) int { - if q == nil { +// physicalCores converts vCPUs to Modal physical cores, floored at minModalCores; zero +// stays zero (Modal's default). Shared with PricePerHour so price matches provisioning. +func physicalCores(vCPUs float64) float64 { + if vCPUs <= 0 { return 0 } - const miB = 1024 * 1024 - return int(q.Value() / miB) -} - -// limitMiB is mib for a LIMIT, where 0 does not mean "unset" but "no cap". A positive -// quantity below 1 MiB truncates to 0 there, so the plain conversion would hand an -// UNBOUNDED sandbox to the one Pod that asked for the tightest ceiling — the inverse -// of its declaration. Any positive limit therefore floors at 1 MiB, the smallest cap -// Modal's unit can express. Modal may then refuse it as below its own minimum, which is -// the honest answer for a limit it cannot honour, and is not silently unlimited. -func limitMiB(q *resource.Quantity) int { - if m := mib(q); m != 0 || q == nil || q.Sign() <= 0 { - return m - } - return 1 -} - -// resourceQty returns the container's request for name, falling back to its limit, -// or nil when neither is present. -func resourceQty(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 nil -} - -// limitQty returns the container's limit for name, or nil when it has none. -func limitQty(c *corev1.Container, name corev1.ResourceName) *resource.Quantity { - if q, ok := c.Resources.Limits[name]; ok { - return &q - } - return nil + return max(vCPUs/vCPUsPerModalCore, minModalCores) } // containerPorts collects the container's declared ports, which is what tells Modal diff --git a/pkg/provider/modal/modal_test.go b/pkg/provider/modal/modal_test.go index c15a5e0..8a7bbd5 100644 --- a/pkg/provider/modal/modal_test.go +++ b/pkg/provider/modal/modal_test.go @@ -320,8 +320,8 @@ func TestProvision_MapsResourcesPortsAndTimeout(t *testing.T) { if _, err := p.Provision(context.Background(), pod, provider.ProvisionRequest{ClaimName: "claim-res"}); err != nil { t.Fatalf("Provision: %v", err) } - if f.lastSpec.CPU != 2.5 { - t.Fatalf("CPU = %v, want 2.5", f.lastSpec.CPU) + if f.lastSpec.CPU != 1.25 { + t.Fatalf("CPU = %v, want 1.25 physical cores for 2500m (2.5 vCPU)", f.lastSpec.CPU) } if f.lastSpec.MemoryMiB != 4096 { t.Fatalf("MemoryMiB = %d, want 4096", f.lastSpec.MemoryMiB) @@ -334,24 +334,40 @@ func TestProvision_MapsResourcesPortsAndTimeout(t *testing.T) { } } +// Getting this wrong doubles both what the Pod gets and what it is billed. +func TestPhysicalCores(t *testing.T) { + for vCPUs, want := range map[float64]float64{ + 20: 10, + 4: 2, + 0.5: 0.25, + 0.1: minModalCores, + 0: 0, // Modal's default, never floored + -1: 0, + } { + if got := physicalCores(vCPUs); got != want { + t.Errorf("physicalCores(%v) = %v, want %v", vCPUs, got, want) + } + } +} + func TestProvision_MapsResourceLimits(t *testing.T) { cases := []struct { - name string - requests, limits corev1.ResourceList - wantCPU, wantCPULimit float64 - wantMemMiB, wantMemLimMiB int + name string + requests, limits corev1.ResourceList + wantCPU float64 + wantMemMiB int }{ { - name: "limits only: limit is the ceiling AND the request falls back to it", + name: "limits only", limits: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("2"), corev1.ResourceMemory: resource.MustParse("8Gi"), }, - wantCPU: 2, wantCPULimit: 2, - wantMemMiB: 8192, wantMemLimMiB: 8192, + wantCPU: 1, + wantMemMiB: 8192, }, { - name: "both: burstable, request below the ceiling", + name: "burstable: request raised to the limit", requests: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("500m"), corev1.ResourceMemory: resource.MustParse("1Gi"), @@ -360,27 +376,27 @@ func TestProvision_MapsResourceLimits(t *testing.T) { corev1.ResourceCPU: resource.MustParse("4"), corev1.ResourceMemory: resource.MustParse("16Gi"), }, - wantCPU: 0.5, wantCPULimit: 4, - wantMemMiB: 1024, wantMemLimMiB: 16384, + wantCPU: 2, + wantMemMiB: 16384, + }, + { + name: "requests only: capped at the request", + requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("500m"), + corev1.ResourceMemory: resource.MustParse("1Gi"), + }, + wantCPU: 0.25, + wantMemMiB: 1024, }, { - name: "neither: Modal applies its own defaults, uncapped", - wantCPU: 0, wantCPULimit: 0, - wantMemMiB: 0, wantMemLimMiB: 0, + name: "neither: Modal applies its own defaults, uncapped", + wantCPU: 0, + wantMemMiB: 0, }, { - // A ceiling below Modal's unit must not truncate into the zero that means - // "no cap" on the limit fields: it would leave the Pod asking for the - // TIGHTEST ceiling running unbounded. The matching request is a different - // question — zero there means "Modal's default", so falling back to 0 is - // correct and the asymmetry is deliberate. - name: "sub-MiB ceiling floors at 1 MiB instead of becoming uncapped", - limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1m"), - corev1.ResourceMemory: resource.MustParse("500Ki"), - }, - wantCPU: 0.001, wantCPULimit: 0.001, - wantMemMiB: 0, wantMemLimMiB: 1, + name: "cpu below Modal's minimum floors", + limits: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("1m")}, + wantCPU: minModalCores, }, } @@ -405,13 +421,9 @@ func TestProvision_MapsResourceLimits(t *testing.T) { if _, err := p.Provision(context.Background(), pod, provider.ProvisionRequest{ClaimName: "claim-lim"}); err != nil { t.Fatalf("Provision: %v", err) } - if f.lastSpec.CPU != tc.wantCPU || f.lastSpec.CPULimit != tc.wantCPULimit { - t.Fatalf("CPU/CPULimit = (%v, %v), want (%v, %v)", - f.lastSpec.CPU, f.lastSpec.CPULimit, tc.wantCPU, tc.wantCPULimit) - } - if f.lastSpec.MemoryMiB != tc.wantMemMiB || f.lastSpec.MemoryLimitMiB != tc.wantMemLimMiB { - t.Fatalf("MemoryMiB/MemoryLimitMiB = (%d, %d), want (%d, %d)", - f.lastSpec.MemoryMiB, f.lastSpec.MemoryLimitMiB, tc.wantMemMiB, tc.wantMemLimMiB) + if f.lastSpec.CPU != tc.wantCPU || f.lastSpec.MemoryMiB != tc.wantMemMiB { + t.Fatalf("CPU/MemoryMiB = (%v, %d), want (%v, %d)", + f.lastSpec.CPU, f.lastSpec.MemoryMiB, tc.wantCPU, tc.wantMemMiB) } }) } @@ -1958,10 +1970,9 @@ 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 + // 4 vCPUs (2 physical cores) + 8 GiB at Modal's sandbox rates. Transcribed, not imported + // from data, so a slip in those constants fails here. + const cpuAndMem = 2*0.00003942*60*60 + 8*0.00000667*60*60 cases := map[string]struct { req provider.PriceRequest diff --git a/pkg/provider/pricing.go b/pkg/provider/pricing.go index bd64286..b92c988 100644 --- a/pkg/provider/pricing.go +++ b/pkg/provider/pricing.go @@ -51,9 +51,8 @@ type PriceRequest struct { // CapacityType selects between a row's Spot and OnDemand prices, which differ // sharply (AWS p5.48xlarge: $20.839 Spot vs $55.040 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 (vCPUs) and MemoryMiB are the workload's size (see util.PodReservation), + // priced only by providers that meter them apart from the accelerator. CPUCores float64 MemoryMiB int } diff --git a/pkg/util/resources.go b/pkg/util/resources.go index 2aba96e..8895e95 100644 --- a/pkg/util/resources.go +++ b/pkg/util/resources.go @@ -24,14 +24,13 @@ import ( // 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. +// PodReservation returns the workload's CPU (vCPUs) and memory (MiB): limits, else +// requests, else 0 (the provider's default). The limit wins because Modal bills the greater +// of reservation and usage; it provisions this value as both request and limit, so the +// price is exact. // -// 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. +// Memory below 1 MiB truncates to 0 (Modal's default, uncapped), so its cost cannot be +// tracked correctly. Accepted: no working Pod declares one. // // 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 @@ -47,14 +46,14 @@ func PodReservation(pod *corev1.Pod) (cpuCores float64, memoryMiB int) { 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 +// reservedQty returns the container's limit for name, falling back to its request, 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 { + if q, ok := c.Resources.Limits[name]; ok { return q } - if q, ok := c.Resources.Limits[name]; ok { + if q, ok := c.Resources.Requests[name]; ok { return q } return resource.Quantity{} diff --git a/pkg/util/resources_test.go b/pkg/util/resources_test.go index e7909bb..30ea864 100644 --- a/pkg/util/resources_test.go +++ b/pkg/util/resources_test.go @@ -37,20 +37,21 @@ func TestPodReservation(t *testing.T) { wantMiB int whatFor string }{ - "requests win over limits": { + "limits win over requests": { 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", + wantCPU: 8, wantMiB: 16384, + whatFor: "a burstable Pod priced at its request is undercharged when it bursts", }, - "falls back to limits": { - pod: podWith(nil, - corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("8"), corev1.ResourceMemory: resource.MustParse("16Gi")}, + "falls back to requests": { + pod: podWith( + corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2"), corev1.ResourceMemory: resource.MustParse("4Gi")}, + nil, ), - wantCPU: 8, wantMiB: 16384, - whatFor: "Kubernetes defaults the request to the limit", + wantCPU: 2, wantMiB: 4096, + whatFor: "a request-only Pod is sized by its request", }, "fractional cores": { pod: podWith(corev1.ResourceList{ From e7ed3fa0f146683418078a47091f9374a37a43f4 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Wed, 30 Sep 2026 22:01:24 +0100 Subject: [PATCH 3/5] solve memory issue Signed-off-by: kerthcet --- pkg/util/resources.go | 12 +++--------- pkg/util/resources_test.go | 12 +++++++++++- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/pkg/util/resources.go b/pkg/util/resources.go index 8895e95..8ddfc90 100644 --- a/pkg/util/resources.go +++ b/pkg/util/resources.go @@ -27,14 +27,8 @@ const mibBytes = 1024 * 1024 // PodReservation returns the workload's CPU (vCPUs) and memory (MiB): limits, else // requests, else 0 (the provider's default). The limit wins because Modal bills the greater // of reservation and usage; it provisions this value as both request and limit, so the -// price is exact. -// -// Memory below 1 MiB truncates to 0 (Modal's default, uncapped), so its cost cannot be -// tracked correctly. Accepted: no working Pod declares one. -// -// 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. +// price is exact. Memory rounds UP to whole MiB: rounding down would turn a sub-MiB size +// into 0, i.e. unset, which Modal fills with its own unpriced default. func PodReservation(pod *corev1.Pod) (cpuCores float64, memoryMiB int) { if pod == nil || len(pod.Spec.Containers) == 0 { return 0, 0 @@ -43,7 +37,7 @@ func PodReservation(pod *corev1.Pod) (cpuCores float64, memoryMiB int) { 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) + return float64(cpu.MilliValue()) / 1000.0, int((mem.Value() + mibBytes - 1) / mibBytes) } // reservedQty returns the container's limit for name, falling back to its request, and a diff --git a/pkg/util/resources_test.go b/pkg/util/resources_test.go index 30ea864..3b85a9e 100644 --- a/pkg/util/resources_test.go +++ b/pkg/util/resources_test.go @@ -53,6 +53,16 @@ func TestPodReservation(t *testing.T) { wantCPU: 2, wantMiB: 4096, whatFor: "a request-only Pod is sized by its request", }, + "sub-MiB memory rounds up to 1 MiB": { + pod: podWith(nil, corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("500Ki")}), + wantCPU: 0, wantMiB: 1, + whatFor: "0 would leave memory unset: Modal's unpriced default", + }, + "fractional MiB rounds up": { + pod: podWith(nil, corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1536Ki")}), + wantCPU: 0, wantMiB: 2, + whatFor: "rounding down would cap the Pod below what it declared", + }, "fractional cores": { pod: podWith(corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("250m"), @@ -63,7 +73,7 @@ func TestPodReservation(t *testing.T) { }, "decimal memory units convert to MiB": { pod: podWith(corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1G")}, nil), - wantCPU: 0, wantMiB: 953, // 1e9 / 1048576, truncated + wantCPU: 0, wantMiB: 954, // 1e9 / 1048576, rounded up whatFor: "1G is not 1Gi, and the price is quoted per GiB", }, "nothing declared": { From 4bbcdaf3ef9dccc234381a644e265e15e9049064 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Wed, 30 Sep 2026 22:13:07 +0100 Subject: [PATCH 4/5] fix limiit Signed-off-by: kerthcet --- pkg/provider/catalog/data/pricing.go | 3 +- pkg/provider/modal/client.go | 9 ++-- pkg/provider/modal/modal.go | 27 +++++++----- pkg/provider/modal/modal_test.go | 47 +++++++++++---------- pkg/provider/pricing.go | 2 +- pkg/util/resources.go | 61 +++++++++++++++++++--------- pkg/util/resources_test.go | 30 +++++++++++++- 7 files changed, 119 insertions(+), 60 deletions(-) diff --git a/pkg/provider/catalog/data/pricing.go b/pkg/provider/catalog/data/pricing.go index 91ced0e..8aae9ca 100644 --- a/pkg/provider/catalog/data/pricing.go +++ b/pkg/provider/catalog/data/pricing.go @@ -53,7 +53,8 @@ const mibPerGiB = 1024 // (see modal.SandboxSpec) — so no conversion happens at the call site, which is where a // factor-of-1024 slip would hide. // -// Exact, since each sandbox is capped at its reservation (see modal.SandboxSpec.CPU). +// Given a limit these are an upper bound, since Modal bills usage above the reservation +// (see util.PodReservation). func ModalCPUCostPerHour(cpuCores float64) float64 { return cpuCores * ModalCPUPricePerCoreHour } diff --git a/pkg/provider/modal/client.go b/pkg/provider/modal/client.go index 5b8fd0c..f1a5c8f 100644 --- a/pkg/provider/modal/client.go +++ b/pkg/provider/modal/client.go @@ -174,13 +174,12 @@ func (c *sdkClient) CreateSandbox(ctx context.Context, spec SandboxSpec) (string // Secrets (see provider.ProvisionRequest.Env). No Secrets field alongside it: the // SDK hydrates this map into an ephemeral server-side Modal Secret before the // create (mergeEnvIntoSecrets), so nothing named is left in the workspace. - Env: spec.Env, - GPU: gpuReservation(spec.GPU, spec.GPUCount), - // Limit = request, so usage can never bill above the recorded price. + Env: spec.Env, + GPU: gpuReservation(spec.GPU, spec.GPUCount), CPU: spec.CPU, MemoryMiB: spec.MemoryMiB, - CPULimit: spec.CPU, - MemoryLimitMiB: spec.MemoryMiB, + CPULimit: spec.CPULimit, + MemoryLimitMiB: spec.MemoryLimitMiB, EncryptedPorts: spec.Ports, // Nil leaves Modal's SchedulerPlacement unset entirely (the SDK only builds one // when Regions is non-empty), which is the unconstrained, un-multiplied case. diff --git a/pkg/provider/modal/modal.go b/pkg/provider/modal/modal.go index dbc7322..7482f1f 100644 --- a/pkg/provider/modal/modal.go +++ b/pkg/provider/modal/modal.go @@ -155,10 +155,13 @@ type SandboxSpec struct { GPU string // GPUCount is how many accelerators to attach (0 for CPU-only). GPUCount int32 - // CPU (physical cores) and MemoryMiB come from util.PodReservation; zero is Modal's - // default. Each is sent as both request and limit (see sdkClient.CreateSandbox). - CPU float64 - MemoryMiB int + // CPU (physical cores) and MemoryMiB are the reservation, CPULimit and MemoryLimitMiB the + // hard cap, all from util.PodResources. Zero is Modal's default on a request and no cap on + // a limit. The claim is priced at the limit when one is set (see util.PodReservation). + CPU float64 + MemoryMiB int + CPULimit float64 + MemoryLimitMiB int // Ports are the container ports to expose, from the Pod's containerPorts. They // declare to Modal which ports may receive traffic at all, and the connect URL // routes to the first of them (see firstPort) — one token routes to one port. @@ -217,10 +220,10 @@ type SandboxSpec struct { // a pointer, so %v would print an address, and only its presence matters. func (s SandboxSpec) String() string { return fmt.Sprintf("SandboxSpec{Image:%s Command:%v Args:%v WorkingDir:%s Env:%s GPU:%s GPUCount:%d CPU:%g "+ - "MemoryMiB:%d Ports:%v Regions:%v Egress:%s "+ + "CPULimit:%g MemoryMiB:%d MemoryLimitMiB:%d Ports:%v Regions:%v Egress:%s "+ "EgressTargets:%v Timeout:%s Tags:%v ReadinessProbe:%t RegistryAuth:%s}", s.Image, s.Command, s.Args, s.WorkingDir, provider.RedactedEnv(s.Env), s.GPU, s.GPUCount, s.CPU, - s.MemoryMiB, s.Ports, s.Regions, s.EgressMode, + s.CPULimit, s.MemoryMiB, s.MemoryLimitMiB, s.Ports, s.Regions, s.EgressMode, s.EgressTargets, s.Timeout, s.Tags, s.ReadinessProbe != nil, s.RegistryAuth) } @@ -611,7 +614,7 @@ func (p *Provider) sandboxSpecFromPod(pod *corev1.Pod, req provider.ProvisionReq tags[ProbeTagKey] = probeTagValue } - vCPUs, memMiB := util.PodReservation(pod) + requests, limits := util.PodResources(pod) spec := SandboxSpec{ Image: c.Image, Command: slices.Clone(c.Command), @@ -621,10 +624,12 @@ func (p *Provider) sandboxSpecFromPod(pod *corev1.Pod, req provider.ProvisionReq // everything envFrom/valueFrom referenced. pod.Spec.Containers[0].Env is NOT read // here: it holds references this adapter has no cluster access to follow. See // provider.ProvisionRequest.Env. - Env: req.Env, - CPU: physicalCores(vCPUs), - MemoryMiB: memMiB, - Ports: containerPorts(&c), + Env: req.Env, + CPU: physicalCores(requests.CPU), + MemoryMiB: requests.MemoryMiB, + CPULimit: physicalCores(limits.CPU), + MemoryLimitMiB: limits.MemoryMiB, + Ports: containerPorts(&c), // An empty request region stays an empty slice, not a one-element [""]: that // is the unconstrained case (no region declared on the pool), and it must // reach Modal as "no placement constraint" — its widest pool and its diff --git a/pkg/provider/modal/modal_test.go b/pkg/provider/modal/modal_test.go index 8a7bbd5..a499f6d 100644 --- a/pkg/provider/modal/modal_test.go +++ b/pkg/provider/modal/modal_test.go @@ -352,22 +352,22 @@ func TestPhysicalCores(t *testing.T) { func TestProvision_MapsResourceLimits(t *testing.T) { cases := []struct { - name string - requests, limits corev1.ResourceList - wantCPU float64 - wantMemMiB int + name string + requests, limits corev1.ResourceList + wantCPU, wantCPULimit float64 + wantMemMiB, wantMemLimMiB int }{ { - name: "limits only", + name: "limits only: the request falls back to the limit", limits: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("2"), corev1.ResourceMemory: resource.MustParse("8Gi"), }, - wantCPU: 1, - wantMemMiB: 8192, + wantCPU: 1, wantCPULimit: 1, + wantMemMiB: 8192, wantMemLimMiB: 8192, }, { - name: "burstable: request raised to the limit", + name: "burstable: request below the ceiling", requests: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("500m"), corev1.ResourceMemory: resource.MustParse("1Gi"), @@ -376,27 +376,28 @@ func TestProvision_MapsResourceLimits(t *testing.T) { corev1.ResourceCPU: resource.MustParse("4"), corev1.ResourceMemory: resource.MustParse("16Gi"), }, - wantCPU: 2, - wantMemMiB: 16384, + wantCPU: 0.25, wantCPULimit: 2, + wantMemMiB: 1024, wantMemLimMiB: 16384, }, { - name: "requests only: capped at the request", + name: "requests only: uncapped, as in Kubernetes", requests: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("500m"), corev1.ResourceMemory: resource.MustParse("1Gi"), }, - wantCPU: 0.25, - wantMemMiB: 1024, + wantCPU: 0.25, wantCPULimit: 0, + wantMemMiB: 1024, wantMemLimMiB: 0, }, { - name: "neither: Modal applies its own defaults, uncapped", - wantCPU: 0, - wantMemMiB: 0, + name: "neither: Modal applies its own defaults, uncapped", + wantCPU: 0, wantCPULimit: 0, + wantMemMiB: 0, wantMemLimMiB: 0, }, { - name: "cpu below Modal's minimum floors", + // The limit floors with the request, or the SDK rejects limit < request. + name: "cpu below Modal's minimum floors on both sides", limits: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("1m")}, - wantCPU: minModalCores, + wantCPU: minModalCores, wantCPULimit: minModalCores, }, } @@ -421,9 +422,13 @@ func TestProvision_MapsResourceLimits(t *testing.T) { if _, err := p.Provision(context.Background(), pod, provider.ProvisionRequest{ClaimName: "claim-lim"}); err != nil { t.Fatalf("Provision: %v", err) } - if f.lastSpec.CPU != tc.wantCPU || f.lastSpec.MemoryMiB != tc.wantMemMiB { - t.Fatalf("CPU/MemoryMiB = (%v, %d), want (%v, %d)", - f.lastSpec.CPU, f.lastSpec.MemoryMiB, tc.wantCPU, tc.wantMemMiB) + if f.lastSpec.CPU != tc.wantCPU || f.lastSpec.CPULimit != tc.wantCPULimit { + t.Fatalf("CPU/CPULimit = (%v, %v), want (%v, %v)", + f.lastSpec.CPU, f.lastSpec.CPULimit, tc.wantCPU, tc.wantCPULimit) + } + if f.lastSpec.MemoryMiB != tc.wantMemMiB || f.lastSpec.MemoryLimitMiB != tc.wantMemLimMiB { + t.Fatalf("MemoryMiB/MemoryLimitMiB = (%d, %d), want (%d, %d)", + f.lastSpec.MemoryMiB, f.lastSpec.MemoryLimitMiB, tc.wantMemMiB, tc.wantMemLimMiB) } }) } diff --git a/pkg/provider/pricing.go b/pkg/provider/pricing.go index b92c988..5d1f5ee 100644 --- a/pkg/provider/pricing.go +++ b/pkg/provider/pricing.go @@ -51,7 +51,7 @@ type PriceRequest struct { // CapacityType selects between a row's Spot and OnDemand prices, which differ // sharply (AWS p5.48xlarge: $20.839 Spot vs $55.040 OnDemand). CapacityType nebulav1alpha1.CapacityType - // CPUCores (vCPUs) and MemoryMiB are the workload's size (see util.PodReservation), + // CPUCores (vCPUs) and MemoryMiB are the workload's priced size (see util.PodReservation), // priced only by providers that meter them apart from the accelerator. CPUCores float64 MemoryMiB int diff --git a/pkg/util/resources.go b/pkg/util/resources.go index 8ddfc90..84fa252 100644 --- a/pkg/util/resources.go +++ b/pkg/util/resources.go @@ -24,31 +24,52 @@ import ( // mibBytes is one MiB, the unit provider.PriceRequest quotes memory in. const mibBytes = 1024 * 1024 -// PodReservation returns the workload's CPU (vCPUs) and memory (MiB): limits, else -// requests, else 0 (the provider's default). The limit wins because Modal bills the greater -// of reservation and usage; it provisions this value as both request and limit, so the -// price is exact. Memory rounds UP to whole MiB: rounding down would turn a sub-MiB size -// into 0, i.e. unset, which Modal fills with its own unpriced default. -func PodReservation(pod *corev1.Pod) (cpuCores float64, memoryMiB int) { +// Resources is a container's CPU (vCPUs) and memory (MiB). Zero means undeclared. +type Resources struct { + CPU float64 + MemoryMiB int +} + +// PodResources returns the first container's requests and limits. A missing request falls +// back to the limit, as Kubernetes defaults it; a missing limit stays 0 (no cap). Memory +// rounds UP to whole MiB, so a sub-MiB size never becomes 0, i.e. unset. +// +// The FIRST container only, matching the single-workload-container shape the whole +// provisioning path assumes (see modal.sandboxSpecFromPod). +func PodResources(pod *corev1.Pod) (requests, limits Resources) { if pod == nil || len(pod.Spec.Containers) == 0 { - return 0, 0 + return Resources{}, Resources{} } 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 - 1) / mibBytes) + limits = Resources{ + CPU: vCPUs(c.Resources.Limits[corev1.ResourceCPU]), + MemoryMiB: ceilMiB(c.Resources.Limits[corev1.ResourceMemory]), + } + requests = limits + if q, ok := c.Resources.Requests[corev1.ResourceCPU]; ok { + requests.CPU = vCPUs(q) + } + if q, ok := c.Resources.Requests[corev1.ResourceMemory]; ok { + requests.MemoryMiB = ceilMiB(q) + } + return requests, limits } -// reservedQty returns the container's limit for name, falling back to its request, 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.Limits[name]; ok { - return q +// PodReservation is the size a Pod is priced at: its limit, else its request. Modal bills +// the greater of reservation and usage, so a limit bounds the bill and the price is an +// upper bound. A request-only Pod has no bound and may bill above its price. +func PodReservation(pod *corev1.Pod) (cpuCores float64, memoryMiB int) { + requests, limits := PodResources(pod) + cpuCores, memoryMiB = limits.CPU, limits.MemoryMiB + if cpuCores == 0 { + cpuCores = requests.CPU } - if q, ok := c.Resources.Requests[name]; ok { - return q + if memoryMiB == 0 { + memoryMiB = requests.MemoryMiB } - return resource.Quantity{} + return cpuCores, memoryMiB } + +func vCPUs(q resource.Quantity) float64 { return float64(q.MilliValue()) / 1000.0 } + +func ceilMiB(q resource.Quantity) int { return int((q.Value() + mibBytes - 1) / mibBytes) } diff --git a/pkg/util/resources_test.go b/pkg/util/resources_test.go index 3b85a9e..c56c9a9 100644 --- a/pkg/util/resources_test.go +++ b/pkg/util/resources_test.go @@ -43,7 +43,7 @@ func TestPodReservation(t *testing.T) { corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("8"), corev1.ResourceMemory: resource.MustParse("16Gi")}, ), wantCPU: 8, wantMiB: 16384, - whatFor: "a burstable Pod priced at its request is undercharged when it bursts", + whatFor: "the limit bounds what Modal can bill, so pricing it is an upper bound", }, "falls back to requests": { pod: podWith( @@ -103,6 +103,34 @@ func TestPodReservation(t *testing.T) { } } +// Provisioning reads requests and limits apart: a request falls back to the limit, but a +// limit never falls back to the request, or a request-only Pod would be capped. +func TestPodResources(t *testing.T) { + burstable := podWith( + corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("500m"), corev1.ResourceMemory: resource.MustParse("1Gi")}, + corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("4"), corev1.ResourceMemory: resource.MustParse("16Gi")}, + ) + limitsOnly := podWith(nil, corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("4")}) + requestsOnly := podWith(corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("4")}, nil) + + for name, tc := range map[string]struct { + pod *corev1.Pod + wantReq, wantLim Resources + }{ + "burstable": {burstable, Resources{0.5, 1024}, Resources{4, 16384}}, + "limits only": {limitsOnly, Resources{4, 0}, Resources{4, 0}}, + "requests only": {requestsOnly, Resources{4, 0}, Resources{}}, + "nil pod": {nil, Resources{}, Resources{}}, + } { + t.Run(name, func(t *testing.T) { + req, lim := PodResources(tc.pod) + if req != tc.wantReq || lim != tc.wantLim { + t.Fatalf("PodResources = (%+v, %+v), want (%+v, %+v)", req, lim, tc.wantReq, tc.wantLim) + } + }) + } +} + // 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) { From f97e1b7326d21d55f67e3b9f95d43dc0bfa223b5 Mon Sep 17 00:00:00 2001 From: kerthcet Date: Wed, 30 Sep 2026 22:36:06 +0100 Subject: [PATCH 5/5] fix Signed-off-by: kerthcet --- pkg/provider/catalog/data/pricing.go | 3 +-- pkg/provider/modal/modal.go | 2 +- pkg/util/resources.go | 18 ++++++------------ pkg/util/resources_test.go | 17 ++++++++--------- 4 files changed, 16 insertions(+), 24 deletions(-) diff --git a/pkg/provider/catalog/data/pricing.go b/pkg/provider/catalog/data/pricing.go index 8aae9ca..667816e 100644 --- a/pkg/provider/catalog/data/pricing.go +++ b/pkg/provider/catalog/data/pricing.go @@ -53,8 +53,7 @@ const mibPerGiB = 1024 // (see modal.SandboxSpec) — so no conversion happens at the call site, which is where a // factor-of-1024 slip would hide. // -// Given a limit these are an upper bound, since Modal bills usage above the reservation -// (see util.PodReservation). +// A floor, since Modal bills usage above the reservation (see util.PodReservation). func ModalCPUCostPerHour(cpuCores float64) float64 { return cpuCores * ModalCPUPricePerCoreHour } diff --git a/pkg/provider/modal/modal.go b/pkg/provider/modal/modal.go index 7482f1f..64540c1 100644 --- a/pkg/provider/modal/modal.go +++ b/pkg/provider/modal/modal.go @@ -157,7 +157,7 @@ type SandboxSpec struct { GPUCount int32 // CPU (physical cores) and MemoryMiB are the reservation, CPULimit and MemoryLimitMiB the // hard cap, all from util.PodResources. Zero is Modal's default on a request and no cap on - // a limit. The claim is priced at the limit when one is set (see util.PodReservation). + // a limit. The claim is priced at the request (see util.PodReservation). CPU float64 MemoryMiB int CPULimit float64 diff --git a/pkg/util/resources.go b/pkg/util/resources.go index 84fa252..2ccc1e7 100644 --- a/pkg/util/resources.go +++ b/pkg/util/resources.go @@ -55,19 +55,13 @@ func PodResources(pod *corev1.Pod) (requests, limits Resources) { return requests, limits } -// PodReservation is the size a Pod is priced at: its limit, else its request. Modal bills -// the greater of reservation and usage, so a limit bounds the bill and the price is an -// upper bound. A request-only Pod has no bound and may bill above its price. +// PodReservation is the size a Pod is priced at: its requests (see PodResources). Modal +// bills the greater of reservation and usage, so this is a floor: a Pod bursting above its +// request is billed more than its price. Never the limit, which would charge an idle Pod +// for its whole ceiling. func PodReservation(pod *corev1.Pod) (cpuCores float64, memoryMiB int) { - requests, limits := PodResources(pod) - cpuCores, memoryMiB = limits.CPU, limits.MemoryMiB - if cpuCores == 0 { - cpuCores = requests.CPU - } - if memoryMiB == 0 { - memoryMiB = requests.MemoryMiB - } - return cpuCores, memoryMiB + requests, _ := PodResources(pod) + return requests.CPU, requests.MemoryMiB } func vCPUs(q resource.Quantity) float64 { return float64(q.MilliValue()) / 1000.0 } diff --git a/pkg/util/resources_test.go b/pkg/util/resources_test.go index c56c9a9..338d976 100644 --- a/pkg/util/resources_test.go +++ b/pkg/util/resources_test.go @@ -37,21 +37,20 @@ func TestPodReservation(t *testing.T) { wantMiB int whatFor string }{ - "limits win over requests": { + "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: 8, wantMiB: 16384, - whatFor: "the limit bounds what Modal can bill, so pricing it is an upper bound", + wantCPU: 2, wantMiB: 4096, + whatFor: "an idle burstable Pod must not be charged for its whole ceiling", }, - "falls back to requests": { - pod: podWith( - corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("2"), corev1.ResourceMemory: resource.MustParse("4Gi")}, - nil, + "falls back to limits": { + pod: podWith(nil, + corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("8"), corev1.ResourceMemory: resource.MustParse("16Gi")}, ), - wantCPU: 2, wantMiB: 4096, - whatFor: "a request-only Pod is sized by its request", + wantCPU: 8, wantMiB: 16384, + whatFor: "Kubernetes defaults the request to the limit", }, "sub-MiB memory rounds up to 1 MiB": { pod: podWith(nil, corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("500Ki")}),