Skip to content

STO-67: fast-fail Model Repo storage quota check - #338

Merged
jebenexer merged 1 commit into
mainfrom
benjaminbrannaka/sto-67-model-repo-storage-limit
Sep 17, 2026
Merged

jebenexer merged 1 commit into
mainfrom
benjaminbrannaka/sto-67-model-repo-storage-limit

Conversation

@jebenexer

@jebenexer jebenexer commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Resolves STO-67.
Server-side counterpart: runpod/RunPod#5929.

#339 (STO-366, pre-signed URL batching) is independent — this check runs before
session creation either way.

Problem

runpodctl already knows the total model size before uploading, but goes
straight into session creation with no check against the account's storage
quota. An over-quota upload should fail immediately with a useful message,
not after creating a model and partial upload sessions server-side.

Changes

  • api.GetModelRepoStorageUsage wraps the new modelRepoStorageUsage query.
  • checkModelRepoStorageQuota compares the total upload size against
    availableBytes and returns an actionable error when it would be exceeded.
    It fails open: a query error or unparseable response warns and proceeds,
    since a client-side pre-check must never itself break a valid upload. Skipped
    when enforcement is off (Enforced: false) or no quota is set
    (AvailableBytes: nil). createModelRepoUpload's server-side gate is still
    the real limit.
  • Wired into runAddModel after file-name/file-size validation and before
    addModelToRepo, covering both --model-path and single-file flows.

Testing

  • go vet ./..., go build ./..., go test ./..., govulncheck ./...
  • checkModelRepoStorageQuota allow/deny/boundary/fail-open, and
    modelUploadRequestedBytes' directory-vs-flag size resolution.
  • API-level tests for GetModelRepoStorageUsage, including its entry in the
    STO-357 typed-error table.
  • End-to-end test confirming addModelToRepo and createModelRepoUpload are
    never reached when the check rejects the upload.

Note

Low Risk
Client-only pre-check that fails open on API errors; upload enforcement still happens server-side, with broad test coverage and no auth or data-model changes.

Overview
Adds a client-side Model Repo storage quota pre-check on runpodctl model add so over-quota uploads fail before any server-side model or upload session is created.

The API layer gains ModelRepoStorageUsage and GetModelRepoStorageUsage, which calls the new modelRepoStorageUsage GraphQL query (optional owner; byte fields as strings). In runAddModel, upload flows compute total bytes via modelUploadRequestedBytes (--model-path sum or --file-size) and run checkModelRepoStorageQuota after upload flag validation and before addModelToRepo. When enforcement is on and availableBytes is known, uploads larger than available return an actionable error; query/parse failures fail open (stderr warning, upload continues). Server-side limits remain authoritative.

Tests cover the API client, STO-357 access-denied table entry, quota allow/deny/boundary/fail-open behavior, and an integration test that addModelToRepo / createModelRepoUpload are not called when quota rejects the upload.

Reviewed by Cursor Bugbot for commit 0b9dc54. Configure here.

@jebenexer
jebenexer force-pushed the benjaminbrannaka/sto-67-model-repo-storage-limit branch from 45f8fcf to 36a185e Compare September 10, 2026 03:06
@jebenexer

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@jebenexer

Copy link
Copy Markdown
Collaborator Author

Validated end-to-end against the real deployed dev environment (https://api.runpod.dev/graphql), now that the server-side counterpart (runpod/RunPod#5929) is merged and live on dev:

  • Confirmed the schema is deployed: ModelRepoStorageQuota table + Model.owner index exist in the dev DB, and modelRepoStorageUsage is live via introspection.
  • Baseline: real dev test account had committedBytes: 28779735, enforced: false (no quota row yet — matches real current dev state).
  • Inserted a temporary ModelRepoStorageQuota row for the test account (limit set ~1MB above existing usage), confirmed modelRepoStorageUsage reflected it correctly (enforced: true, availableBytes: 1000265).
  • runpodctl model add with a 2MB file correctly fast-failed client-side with an actionable error, before creating any model server-side (verified zero Model rows for the attempted name).
  • runpodctl model add with a 100KB file (under the remaining headroom) succeeded end-to-end against real dev infra — real model created, real file uploaded, session reached NEEDS_HASH, and modelRepoStorageUsage.committedBytes updated by exactly the uploaded size afterward.
  • Cleaned up: removed the test model via runpodctl model remove and deleted the temporary quota row; dev account is back to enforced: false.

All 12 relevant unit tests (go test ./cmd/model/... ./api/... -run "Quota|StorageUsage|ModelRepo") still pass on this branch, and it builds clean.

Note: this branch is currently 3 commits behind main (including #339, STO-366 batching, called out in this PR's description as independent) — not a blocker for this validation, but will need a rebase before merge.

Adds a client-side check that fails fast, before creating the model or
a single upload session, when a local upload would exceed the account's
Model Repo storage quota.

- api.GetModelRepoStorageUsage: wraps the server's modelRepoStorageUsage
  query (runpod/RunPod#5929).
- checkModelRepoStorageQuota: compares the total local upload size
  against availableBytes and returns a clear, actionable error when it
  would be exceeded. Fails OPEN on any query error or unparseable
  response (older server, transient network issue) -- a client-side
  pre-check must never itself become the reason an otherwise-valid
  upload fails. Skipped entirely when enforcement is disabled
  server-side.
- Wired into runAddModel right after the existing file-name/file-size
  validation, before addModelToRepo is called for either the
  --model-path (directory) or single-file (--file-name/--file-size)
  upload flows.
- Storage-usage query failures go through modelRepoHTTPError/
  modelRepoGraphQLError like the rest of the model-repo API surface, so
  access-denied and other server errors carry a stable error code (the
  STO-357 contract) rather than a bare string.

Rebased onto current main to resolve conflicts with #339 (STO-366,
pre-signed URL batching), which landed after this branch was created and
fully migrated the local upload flow from a single createModelRepoUpload
call per file to createModelRepoUploadBatch/CompleteModelRepoUploadBatch.
Kept main's batch-based upload plumbing as-is and layered this PR's own
additions on top (the getModelRepoStorageUsage var, checkModelRepoStorageQuota,
and its call site in runAddModel); dropped this branch's now-superseded
completeModelRepoUpload (singular) var, which #339 replaced with
completeModelRepoUploadAll and which nothing in the current upload flow
calls anymore.

Testing: go build/vet/test clean across the whole module (all packages,
not just cmd/model and api); govulncheck reports 0 vulnerabilities
affecting this code.
- cmd/model/modelRepoQuota_test.go covers the quota check's
  allow/deny/boundary/fail-open behavior and modelUploadRequestedBytes'
  directory-vs-flag size resolution.
- api/model_test.go covers GetModelRepoStorageUsage response parsing,
  owner-variable omission, and graphql error surfacing, and the
  model-repo access-denied table test gains an entry for it (alongside
  #339's own new CreateModelRepoUploadBatch entry, both now coexisting).
- One end-to-end test confirms addModelToRepo/createModelRepoUpload are
  never called when the fast-fail check rejects the upload.

Revalidated live against the real dev environment (api.runpod.dev) after
rebasing: over-quota model add still fast-fails client-side with zero
server-side model rows created, under-quota model add still succeeds
end-to-end through the now-batch-based upload path, and usage accounting
updates correctly afterward.
@jebenexer
jebenexer force-pushed the benjaminbrannaka/sto-67-model-repo-storage-limit branch from 36a185e to 0b9dc54 Compare September 17, 2026 15:11
@jebenexer

Copy link
Copy Markdown
Collaborator Author

Resolved the merge conflicts against current main (3 commits behind, including #339's STO-366 batching work landing after this branch was created).

Root cause: #339 fully migrated the local upload flow from a single createModelRepoUpload call per file to createModelRepoUploadBatch/CompleteModelRepoUploadBatch (even for one file), and dropped the old singular completeModelRepoUpload var this branch's own var block still referenced. The conflicts were four files where both PRs touched the same insertion points: the var (...) block wiring up the package-level function seams, the Data struct wrapping two different mutations' responses, and a table-driven access-denied test both PRs added an entry to.

How I resolved it: kept #339's batch-based upload plumbing as-is (nothing about that flow changes for this PR — the quota check runs before any upload session is created, batched or not), and layered this PR's own additions on top: the getModelRepoStorageUsage var, checkModelRepoStorageQuota, and its call site in runAddModel. Dropped this branch's now-dead singular completeModelRepoUpload var, since nothing in the current (batch-only) upload flow calls it anymore. The two struct/table-test conflicts were straightforward "both sides are additive, make them coexist" resolutions.

Validated before pushing:

  • go build/vet/test ./... clean across the entire module (not just cmd/model/api) — 0 failures.
  • govulncheck ./...: 0 vulnerabilities affecting this code.
  • Revalidated live against the real dev environment (api.runpod.dev) after rebasing: inserted a temporary quota row, confirmed over-quota model add still fast-fails client-side with zero server-side model rows created, under-quota model add still succeeds end-to-end — and this time going through the now-batch-based upload path (createModelRepoUploadBatch/completeModelRepoUploadAll), confirming the quota check correctly gates the merged code path, not just the old single-upload one. Usage accounting updated correctly afterward; cleaned up the test model and quota row.

Branch is now based on current main (2232774) and should be mergeable once CI/review pass.

@jebenexer

Copy link
Copy Markdown
Collaborator Author

bugbot run

@jebenexer

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 0b9dc54. Configure here.

@jebenexer
jebenexer merged commit 4351fca into main Sep 17, 2026
2 checks passed
@jebenexer
jebenexer deleted the benjaminbrannaka/sto-67-model-repo-storage-limit branch September 17, 2026 18:58
@promptless

promptless Bot commented Sep 17, 2026

Copy link
Copy Markdown

Promptless documentation updates

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants