feat(apps): per-deployment resource multiplier for upgrades#233
Merged
Conversation
v0l
force-pushed
the
feat/app-resource-multiplier
branch
from
July 25, 2026 18:35
d3ea852 to
3eaa4ac
Compare
v0l
commented
Jul 25, 2026
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.
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
force-pushed
the
feat/app-resource-multiplier
branch
from
July 26, 2026 07:14
efc2816 to
aa82ce2
Compare
… 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
app_deployment.resource_multiplier INTEGER UNSIGNED NOT NULL DEFAULT 1— existing deployments are base size, no backfillfootprint × multipliercalculate_app_upgrade_cost— prorated, mirroring VM upgradesapp.amount × multiplierPOST /app-deployments/{id}/upgrade-quoteand/upgradeDesign 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
1returns quantities untouched, so existing deployments render byte-identical pod specs and don't churn when this rolls out. Legacy rows decoding as0clamp 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_typewas VM-bound (it took avm_idand looked up the VM just foruser_idand a description). Split outprice_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.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.
lnvps_apilnvps_api_commonlnvps_dblnvps_api_adminlnvps_operatorKnown gaps
validate_app_upgrade,create_app_upgrade_paymentand the two handlers have no unit tests — they need a fullRouterState, whichapps.rshas no harness for. They are covered by e2e, whichcargo llvm-covwon't count, so function coverage forapps.rswill show a gap.resource_multiplieris 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.