Skip to content

feat(apps): per-deployment resource multiplier for upgrades#233

Merged
v0l merged 4 commits into
masterfrom
feat/app-resource-multiplier
Jul 26, 2026
Merged

feat(apps): per-deployment resource multiplier for upgrades#233
v0l merged 4 commits into
masterfrom
feat/app-resource-multiplier

Conversation

@v0l

@v0l v0l commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Managed app deployments were fixed at the catalog app's footprint and price, so the only way to offer a customer a bigger instance was to publish a second catalog entry. This adds a per-deployment resource_multiplier — the deployment's size as an integer multiple of the base app, applied to resources and price together.

What changed

Layer Change
Schema app_deployment.resource_multiplier INTEGER UNSIGNED NOT NULL DEFAULT 1 — existing deployments are base size, no backfill
Operator Container CPU/memory limits, PVC sizes and the namespace storage quota scale by the multiplier
Capacity A deployment occupies footprint × multiplier
Pricing calculate_app_upgrade_cost — prorated, mirroring VM upgrades
Billing Resize applied on payment settlement; line item repriced to app.amount × multiplier
API POST /app-deployments/{id}/upgrade-quote and /upgrade

Design notes

Increase-only is a constraint, not a policy. A PersistentVolumeClaim cannot shrink, so a downgrade would strand storage. It's enforced in three places — the API guard, the pricing engine, and again at settlement — so a replayed or out-of-order payment can never shrink a deployment.

The resize happens only when the payment settles. An abandoned upgrade leaves the deployment untouched. Quoting never mutates anything (there's an e2e assertion for exactly this).

A multiplier of 1 returns quantities untouched, so existing deployments render byte-identical pod specs and don't churn when this rolls out. Legacy rows decoding as 0 clamp to base size.

Capacity counts the multiplied footprint. Counting the base footprint would let upgraded deployments silently oversubscribe a cluster. The upgrade's additional footprint must fit on the cluster the deployment already runs on — it can't be moved, because its volumes are there.

Refactor: price_to_payment_with_type was VM-bound (it took a vm_id and looked up the VM just for user_id and a description). Split out price_to_payment_for_subscription, which needs only a subscription, a user and a description. The VM path delegates to it with identical description strings, so VM behaviour is unchanged.

⚠️ Operator prerequisite

Growing a PVC requires a StorageClass with allowVolumeExpansion: true. Without it the pod specs scale but the volumes do not, and the deployment reports a failed expansion. There is no preflight check for this yet.

Tests

New coverage: operator scaling (including the 1× / 0× / unparseable passthrough cases), capacity accounting with mixed multipliers, prorated quote math, upgrade rejection cases (downgrade, no-op, expired subscription), and settlement (applies + reprices, never shrinks, tolerates bad metadata). Plus e2e assertions on the new endpoints.

Crate Result
lnvps_api 276 ✅
lnvps_api_common 265 ✅
lnvps_db 44 ✅
lnvps_api_admin 31 ✅
lnvps_operator 27 ✅

Known gaps

  • validate_app_upgrade, create_app_upgrade_payment and the two handlers have no unit tests — they need a full RouterState, which apps.rs has no harness for. They are covered by e2e, which cargo llvm-cov won't count, so function coverage for apps.rs will show a gap.
  • resource_multiplier is read-only in the admin API by design: setting it without a payment would desync the billed price from the provisioned size. No admin override for support cases yet.

@v0l
v0l force-pushed the feat/app-resource-multiplier branch from d3ea852 to 3eaa4ac Compare July 25, 2026 18:35
Comment thread lnvps_api/src/api/apps.rs Outdated
Comment thread lnvps_api/src/subscription/app.rs Outdated
v0l added a commit that referenced this pull request Jul 25, 2026
Two review comments from v0l:

- `deployment_to_api` treated the catalog app as optional and reported a
  zero footprint when the lookup failed. `app_deployment.app_id` is a
  required foreign key, so a missing app is a broken row, not a normal
  case - and "0 CPU / 0 memory" is worse than an error because the client
  renders it as fact. It now propagates.

- An app upgrade payment always carries an AppUpgradeConfig, written by
  create_app_upgrade_payment. Missing or malformed metadata therefore
  means the payment record is corrupt *and the customer has already been
  charged*, so returning Ok() left them paid up at the old size with only
  an info log. It now errors, and distinguishes absent metadata from
  unreadable metadata so the log says which.

Note complete_payment only warns on handler errors, so this does not roll
the payment back - the difference is that the failure is now a warning
naming the payment id, instead of reading like normal operation.
v0l added 2 commits July 26, 2026 08:11
Managed app deployments were fixed at the catalog app's footprint and
price, so the only way to give a customer a bigger instance was to add a
second catalog entry. Introduce a per-deployment `resource_multiplier`:
the deployment's size as an integer multiple of the base app, applied to
resources *and* price together.

- schema: app_deployment.resource_multiplier NOT NULL DEFAULT 1, so all
  existing deployments are base size with no backfill.
- operator: container CPU/memory limits, PVC sizes and the namespace
  storage quota are scaled by it. A multiplier of 1 returns quantities
  untouched, so existing deployments render byte-identical specs and do
  not churn on rollout; 0 (legacy rows) clamps to the base size.
- capacity: a deployment now occupies footprint x multiplier. Counting
  the base footprint would let upgraded deployments oversubscribe the
  cluster.
- pricing: calculate_app_upgrade_cost mirrors the VM upgrade quote - the
  new rate for the time remaining, minus a credit for that same time
  already bought at the old rate. Expiry is unchanged, because an upgrade
  buys capacity, not time.
- billing: the resize is applied only when the upgrade payment settles,
  and the line item is repriced to app.amount x multiplier so renewals
  charge the new size. An abandoned upgrade changes nothing.
- api: POST /api/v1/app-deployments/{id}/upgrade-quote and /upgrade.

Upgrades are increase-only and capped at 16x. Increase-only is a hard
constraint, not a policy choice: a PersistentVolumeClaim cannot shrink,
so a downgrade would strand storage. It is enforced in three places (the
API guard, the pricing engine, and again at settlement) so a replayed or
out-of-order payment can never shrink a deployment. The *additional*
footprint must fit on the cluster the deployment already runs on, since
its volumes live there and it cannot be moved.

Turning a VM-shaped payment path into a product-agnostic one required
splitting price_to_payment_with_type into price_to_payment_for_subscription,
which needs only a subscription, a user and a description. The VM path
delegates to it with identical descriptions, so VM behaviour is unchanged.

Note for operators: growing a PVC requires a StorageClass with
allowVolumeExpansion: true. Without it the pod specs scale but the volumes
do not, and the deployment reports a failed expansion.
Two review comments from v0l:

- `deployment_to_api` treated the catalog app as optional and reported a
  zero footprint when the lookup failed. `app_deployment.app_id` is a
  required foreign key, so a missing app is a broken row, not a normal
  case - and "0 CPU / 0 memory" is worse than an error because the client
  renders it as fact. It now propagates.

- An app upgrade payment always carries an AppUpgradeConfig, written by
  create_app_upgrade_payment. Missing or malformed metadata therefore
  means the payment record is corrupt *and the customer has already been
  charged*, so returning Ok() left them paid up at the old size with only
  an info log. It now errors, and distinguishes absent metadata from
  unreadable metadata so the log says which.

Note complete_payment only warns on handler errors, so this does not roll
the payment back - the difference is that the failure is now a warning
naming the payment id, instead of reading like normal operation.
@v0l
v0l force-pushed the feat/app-resource-multiplier branch from efc2816 to aa82ce2 Compare July 26, 2026 07:14
v0l added 2 commits July 26, 2026 08:26
… seed

Two failures from the e2e suite on this branch.

1. The upgrade-quote endpoint returned 500 for a deployment that was never
   paid for. An upgrade is prorated against the time left on the current
   period, so a subscription with no expiry (never paid) or a past expiry
   cannot be quoted. Those conditions were only checked inside the pricing
   engine, which raises anyhow errors and therefore surfaced as 500. They
   are now checked in validate_app_upgrade, which returns 400 with an
   actionable message ("pay for it first" / "renew it before upgrading").
   The e2e assertion that caught this was deliberately written as "must
   not 5xx", and it did its job.

2. `seed_app_and_cluster` placed its cluster in `MIN(id) FROM region`
   without regard to `enabled`. Admin region DELETE only soft-deletes
   (enabled = false), so an earlier test can leave the lowest-id region
   disabled - and `GET /api/v1/apps/{id}/regions` only surfaces enabled
   regions, so the seeded cluster's region never appeared and
   test_app_deployment_ordering failed on "seeded region is deployable".
   Both app seed helpers now select the lowest *enabled* region.

The second is a pre-existing harness bug rather than part of this feature;
it is fixed here because it blocks the required e2e check.
My previous attempt swapped `MIN(id) FROM region` for
`MIN(id) FROM region WHERE enabled = 1`, which made things worse: the base
e2e dataset has no enabled region of its own (regions are created and then
soft-deleted by other tests), so the filtered query returned NULL and the
cluster insert failed with "Column 'region_id' cannot be null" - taking out
a third test.

The real defect is depending on ambient region state at all. Either way it
is a coin flip: an unfiltered MIN(id) can land on a region an earlier test
soft-deleted (enabled = false), and `GET /api/v1/apps/{id}/regions` only
surfaces enabled regions, so the seeded cluster never appeared as
deployable - the original "seeded region is deployable" failure.

Both app seed helpers now create their own enabled region, so they are
independent of what other tests leave behind.
@v0l
v0l merged commit 95d3a89 into master Jul 26, 2026
6 checks passed
@v0l
v0l deleted the feat/app-resource-multiplier branch July 26, 2026 08:00
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.

1 participant