Skip to content

feat(databases): apply migrations in tracked runs - #365

Merged
swkeever merged 5 commits into
mainfrom
skeever/database-migration-runs
Nov 13, 2026
Merged

swkeever merged 5 commits into
mainfrom
skeever/database-migration-runs

Conversation

@swkeever

@swkeever swkeever commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Adds the CLI surface for tracked database migrations (hosting#2267) and moves deploy --with-migrations onto it.

Why

  • deploy --with-migrations and databases migration up run every file again on each call, so users must write every migration to be safe to repeat or drop the flag after the first deploy. A non-idempotent file breaks the next deploy.
  • Hosting already applies migrations server-side in tracked runs: each file runs once, is rehearsed on a branch, and is backed up first. Git deploy uses them; the CLI had no way to reach them (hosting api/cli-surface.yaml lists all five routes as a gap, Kong/volcano-hosting#2421).
  • This makes the CLI path match Git deploy, so the same volcano/migrations directory behaves the same from a push or a terminal.

Merge order

Merge only once the production database_migrations flag is on: every merge here cuts a release, and --with-migrations refuses while the capability is off. Then, right after this PR, merge hosting#2374, which updates hosting's tests/e2e/cli lane for the new behavior. That lane builds CLI main, so hosting's cli-tests fails from the moment this merges until that PR lands.

localmode-e2e runs against kong/volcano:local-nightly, which is built from hosting main and includes hosting#2267, so the local stack offers databases.migrations.

Commands and flags

All of them live under volcano [cloud] databases migration, so they work in both trees.

Command What it does
status -d <db> [--json] Compares the files for <db> with the database's migration history: applied, baselined, pending, changed (checksum differs), missing (recorded, no file). Names a queued or running run. Works without the capability.
apply -d <db> [--no-wait] [--json] Sends every file for <db> to POST migration-runs with an Idempotency-Key, then follows the run to its end, printing each phase. Prints the files it applied, every file of a failed run, warnings, and the backup name.
baseline -d <db> (--all | -f <file>...) [--yes] [--json] Records files as applied without running them. Prompts first.
runs list -d <db> [--json] The 50 most recent runs, including Git deploy runs.
runs get <run-id> -d <db> [--json] One run: status, phase, source, backup, error, each file's outcome.
up -d <db> (--all | -f <file>) Unchanged untracked path. Help now says so and points to apply. Local mode keeps volcano migrations deploy as its alias.

deploy --with-migrations starts one tracked run per database and waits for it, even with --no-wait. A failed run stops the deploy at the migrations step, and the resume hint names databases migration apply -d <db>.

How client-side migrations and tracked runs relate

  • One path per command, no fallback. apply, baseline, and --with-migrations read GET /capabilities first. Without databases.migrations available they refuse before sending anything, say why, and name databases migration up --all -d <db>. They never fall back to running files client-side. status and runs don't need the capability, because the reads stay ungated.
  • up stays the explicit untracked escape hatch. It runs every file through the database proxy, one statement per file, and writes no history. Nothing calls it implicitly.
  • Adopting a database migrated client-side. Its schema reflects the files, but its history is empty, so the first tracked run would apply them again. status shows them all pending, and when nothing is recorded yet it suggests baseline --all. A run that fails with migration_failed before anything applied suggests the same. deploy --help and the docs say to baseline once before the first tracked deploy.
  • Layout. volcano/migrations is read the same way Git deploy reads it: top-level files for one database, or volcano/migrations/<database>/ per database, never both. Names starting with _ or . are skipped. Symlinks, non-regular .sql files, and deeper nesting are refused. Nested files are sent as <database>/<file>.sql, which the API accepts and strips. With the per-database layout, --with-migrations without -d migrates every database that has a directory. up now reads the same layout, so up -d app takes volcano/migrations/app/ in a per-database project, and a mixed directory is refused there too.
  • Names. Tracked runs take only names that end in lowercase .sql, so apply, baseline, and --with-migrations refuse 001_init.SQL before sending anything and name the rename. up still runs such files.
  • Checksums. status compares local files with the history using the server's normalization: a leading BOM is stripped, CRLF becomes LF, and the result is sha256:<hex>.
  • Limits. The CLI doesn't pre-check the 1000-file and 1 MiB limits. It reports the API's refusal.

Off-state and errors

  • Capability off: apply/deploy say tracked migration runs aren't available on this Volcano API (capability "databases.migrations"), name up --all, and exit non-zero. deploy adds Nothing was deployed.. A feature_unavailable answer from the route itself gets the same wording.
  • migration_run_in_progress (409) names the run and runs get <id>. Each command sends a fresh Idempotency-Key, so a retried request returns the run it started (a 200 replay is treated like the 202), while running the command again starts a new run.
  • Waiting polls every 2s for up to 90 minutes and tolerates 5 transient read failures in a row. If it stops early, it says the run carries on and names runs get <id>.
  • database_busy, history_too_large, history_missing, storage refusals: the API's message is printed and the exit code follows the status. history_missing and checksum_mismatch add what to do next.
  • A failed file's PostgreSQL message is printed under the file and never put in the returned error, so usage telemetry never sees it. The run-level error text the API composes carries only the file, line, and SQLSTATE.
  • volcano reset now suggests volcano databases migration apply -d app, because an untracked redeploy followed by --with-migrations would apply every file again.

main already vendors the Database Migrations paths and the /capabilities block (since #456), so this PR now takes main's openapi/openapi.yaml unchanged; make openapi-generate produces no diff.

Overlap with #478

#478 (stacked on #476 and #477) moves up and deploy --with-migrations onto migration runs under pg- delegated grants. To let it rebase onto this PR with little conflict:

Tests

  • Unit: file reading and layout refusals, checksums, status comparison, capability checks, apply/no-wait/JSON, polling, the run-in-progress conflict, replays, failed runs and their hints, baseline selection, confirmation, and refusals, runs list and get, deploy targets for both layouts, the deploy refusal before any step, a failed run stopping the deploy, and local deploy on app.
  • TestAPIE2ECloudDatabaseMigrations (new, standard cloud-e2e CLI suite): creates a database, then runs status (all pending, plus the baseline hint), apply (rehearsed, backup_skipped on HOBBY), apply again (nothing applied), baseline -f and baseline --all (nothing left to record), deploy --with-migrations (only the new file runs), a broken file that fails in rehearsal without touching the database, then runs list and runs get. TestAPIE2ECloudDeploy and the starter deploy now expect tracked-run output.
  • TestLocalModeE2EDatabaseMigrations (new, added to make localmode-e2e): the same round trip on the local app database, with rehearsal_skipped and backup_skipped, and the broken file failing in apply. The local smoke's deploy checks expect tracked runs, and its reset check expects the new hint.
  • hosting#2374 adds TestCLIE2ESmokeDatabaseMigrations (refusals with the gate off, and reads on a missing database) and changes the --with-migrations check in TestCLIE2ESmokeDeploy to the refusal.

Verification

At head after merging main (v0.78.0, ffe321c):

  • Conflicts resolved in docs/databases.md (kept main's regions and resize rows and sections), internal/cmd/deploy/helpers_test.go (kept both the migration-run fakes and main's function-read fakes), and openapi/openapi.yaml plus internal/apiclient/client.gen.go (took main's; make openapi-generated-check passes).
  • go build ./..., go vet ./tests/... ./cmd/..., gofmt, go mod tidy (no diff), make release-workflow-check.
  • go tool golangci-lint run --new-from-rev=origin/main --whole-files on the changed packages: 0 issues.
  • go test ./internal/cmd/databases/... ./internal/cmd/deploy/ ./internal/database/ ./internal/cmd/root/ ./internal/api/, and -run Reset in ./internal/localmode/ ./internal/cmd/local/: pass.
  • Deferred to CI: the full check job, localmode-e2e, and the hosting cli-tests run in Kong/volcano-hosting#2374 (same branch name, so it builds this branch).

Earlier, on 13784cc:

  • Cloud E2E against staging, with the gate on, through hosting's cloud-e2e-local runner at hosting main 2791f39517: go test ./tests/e2e/api -run '^TestAPIE2E(SmokeMigrations|CloudDatabaseMigrations)$'. Both passed (TestAPIE2ECloudDatabaseMigrations took 70s).
  • Local-mode E2E against an image built at hosting main 2791f39517: TestLocalModeE2EDatabaseMigrations (52s) and TestLocalModeE2ESmoke (449s) passed.
  • These are not rerun after the merge with main; the merge touches no migration code path, and the alignment commit only renames files and narrows error bodies.

Decisions to confirm

  1. --with-migrations has no client-side fallback. Following hosting's flag rules, it refuses wherever the capability is off and names up --all. So this PR merges (and releases) only once the production flag is on.
  2. No automatic baseline. A database migrated client-side gets its files applied again on its first tracked run unless someone runs baseline first. status, deploy --help, the docs, and the migration_failed hint all point to baseline --all. The CLI never baselines on its own, because recording a file that didn't run means it never runs. Local mode has no rehearsal, so there a re-run file that fails fails in apply. Each file runs in its own transaction, so the files before it stay applied and recorded.
  3. up and local migrations deploy stay. They're the escape hatch the refusals name, so they're kept and not deprecated. Their help now says they're untracked and points to apply.
  4. The per-database layout migrates every directory by default. That matches Git deploy. -d narrows it to one database.
  5. baseline prompts unless --yes is passed. A recorded file never runs, so the default asks first.
  6. up and local migrations deploy now read the same files as tracked runs and Git deploy. Before, they read only top-level .sql files, followed symlinks, and ran _- and .-prefixed files. Now they skip _/. names, read volcano/migrations/<database>/ in a per-database project, and refuse symlinks, deeper nesting, and a mixed layout. I chose this so baseline --all after up --all records exactly the files that ran. Alternative: keep up's old reading and accept that the two can disagree.
  7. hosting#2374 merges right after this. Merged before this PR, it would break hosting's cli-tests against CLI main. It drops hosting's mid-deploy failure check (--with-migrations -d missing_database failing at step 2), because that lane can't reach a migration step with the gate off. CLI unit tests still cover a step failing mid-run (TestDeployStopsAtAFailedMigrationRun).

🤖 Generated with Claude Code

Add `databases migration status`, `apply`, `baseline`, and `runs list|get`
over the hosting migration-history and migration-run API. `deploy
--with-migrations` now starts a tracked run per database instead of running
every file through the database proxy, and reads `volcano/migrations` with
the Git deploy layout: files at the top level for one database, or one
directory per database.

Tracked runs need the `databases.migrations` capability. Where the API does
not offer it, apply, baseline, and --with-migrations refuse before anything
changes and name the untracked `databases migration up --all`, which stays.
Copilot AI balanced review requested due to automatic review settings October 29, 2026 19:11
@swkeever
swkeever requested a review from a team as a code owner October 29, 2026 19:11

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

AI review: no P0-P2 findings

Reviewed 3440982. Run log

Adds tracked migration commands (status, apply, baseline, runs list|get) and moves deploy --with-migrations onto server-side tracked runs. Both require the API to report the databases.migrations capability. up, apply and deploy now read volcano/migrations with the same layout as Git deploy. The latest commit documents the -- volcano:no-transaction marker, which fixes the previous round's P2. No P0–P2 problems remain. The risk is in release timing and in behaviour changes for existing users, which a person should decide.

⚪ P3

  • internal/database/migrationfiles.go:102 Symlink refusal covers files that are not migrations
  • internal/api/capabilities.go:13 CapabilityAvailable duplicates the existing ListCapabilities call

Not checked: The AI review read the diff, the code, and the PR's text; it ran no tests or builds and read nothing outside this repository.

  • Whether the hosting API accepts <database>/<file>.sql names and computes checksums the way status assumes (leading BOM stripped, CRLF turned into LF).
  • Whether the API-written run.error.message never quotes file content or row values, since runFailure puts it in the error the command returns.
  • Whether databases.migrations is available in production and in the local server image users run, before this releases.
  • Whether the cloud and local-mode E2E suites still pass at this head; the author last ran them on 13784cc, before the merge with main.
  • Verify that production and kong/volcano:local-nightly provide tracked migrations and advertise databases.migrations before release.

Human review: needed

  • Ship only once production has databases.migrations on? Until then deploy --with-migrations, which the starters document, refuses for every user (deploy/plan.go requireMigrationRuns).
  • OK that a database migrated by an older --with-migrations re-applies every file on its first tracked deploy unless someone runs baseline --all first, with no rehearsal in local mode?
  • Should up, migrations deploy and --with-migrations refuse what they used to accept: symlinks, deeper nesting, mixed layouts, .SQL names (tracked only), and skip _/. files?
  • Is the merge order planned: hosting#2267 in production and local-nightly first, hosting#2374 right after this, since hosting's cli-tests fail against CLI main in between?
  • Is requiring an explicit baseline before existing databases’ first tracked deploy acceptable (internal/cmd/deploy/plan.go)?

The AI review found no P0-P2 problems at 3440982, so the code needs no second review for bugs. Decide the points above: approve to accept them, or request changes.

Suggested reviewer: subnetmarco (has edited docs/databases.md, docs/deploy.md and the local-mode tests; suited to judge the gating and release trade-off). Any collaborator with write access other than the author can approve.

@github-actions
github-actions Bot requested a review from subnetmarco October 29, 2026 19:18
Comment thread internal/cmd/deploy/plan.go Resolved
Comment thread internal/database/migrationfiles.go Resolved
- Refuse a migration whose name doesn't end in lowercase .sql before a
  tracked run or baseline sends it, naming the file to rename. Untracked
  `up` still runs such files.
- Send the baseline prompt to stderr with --json, so stdout stays JSON.
- Stop claiming a backup was taken when the run ended in its backup phase.
- Name the database's directory when a per-database deploy finds it empty,
  and in `up`'s "not found" error.
@swkeever

Copy link
Copy Markdown
Collaborator Author

Closing as stale: no commits since before 2026-11-10. The branch is kept; reopen if this is still needed.

@swkeever swkeever closed this Nov 12, 2026
@swkeever swkeever reopened this Nov 13, 2026
…ation-runs

# Conflicts:
#	docs/databases.md
#	internal/apiclient/client.gen.go
#	internal/cmd/deploy/helpers_test.go
#	openapi/openapi.yaml
Rename internal/api/database_migrations.go to database_migration_runs.go
and the `migration runs` commands from runs.go to runs_list.go and
runs_get.go, and name the in-progress code MigrationRunInProgressCode,
so #478 can rebase onto this branch without both PRs adding runs.go.

Run reads and a refused run start keep only an error body's own fields,
since a run's file errors can quote data.
Comment thread docs/databases.md Outdated
development has no branches or backups, so a local run applies directly and
reports `rehearsal_skipped` and `backup_skipped` warnings.

A file may hold several statements. It cannot `COMMIT`, `ROLLBACK`, use

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 P2: Transaction guarantees contradict the current migration API

The checked-in DatabaseMigrationFile.sql schema supports -- volcano:no-transaction on the first line, and requestFiles forwards it unchanged. Such files can use CREATE INDEX CONCURRENTLY, run outside a transaction, and retry from their start. These docs and apply --help incorrectly rule them out and promise transactional, once-only execution. Document the marker, its single-statement constraint, and its rollback/retry semantics in both places.

Found by gpt-6.1-sol

Tracked runs accept a file whose first line is `-- volcano:no-transaction`, which runs alone outside a transaction and again from its start on a retry. docs/databases.md and `migration apply --help` said such statements could not run at all.
}
// Git deploy does not follow links, so a migration reached through
// one would apply from here and be missing there.
if d.Type()&fs.ModeSymlink != 0 {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚪ P3: Symlink refusal covers files that are not migrations

The symlink check runs before the .sql extension check, so any symlink in volcano/migrations that isn't _- or .-prefixed fails ReadMigrations. That includes a symlinked README.md or notes file. Then up, migrations deploy, status, apply, baseline and deploy --with-migrations all stop with "is a symlink, which Git deploy does not follow", even though none of them would read that file. The stated reason (a migration reached through a link applies here but is missing in Git deploy) only covers .sql files and linked directories. Unless Git deploy also refuses non-SQL links, limit the refusal to symlinks named *.sql and symlinks that point to a directory (check with root.Stat).

Found by claude-opus-5-5

// available. The list is how a client learns what an environment offers, so a
// capability it leaves out, or reports with any status but available, is
// unavailable.
func (c *Client) CapabilityAvailable(ctx context.Context, id string) (bool, error) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚪ P3: CapabilityAvailable duplicates the existing ListCapabilities call

internal/api/lavaflow.go:23 already has Client.ListCapabilities, which calls the same ListCapabilitiesWithResponse, and sharing.CheckAvailable looks up its capability on top of it. The new CapabilityAvailable makes a second wrapper for the same route. Building it on ListCapabilities, or doing the lookup in database.Service.RequireMigrationRuns the way sharing does, keeps one place that reads /capabilities.

Found by claude-opus-5-5

@swkeever
swkeever merged commit b46711d into main Nov 13, 2026
17 of 18 checks passed
@swkeever
swkeever deleted the skeever/database-migration-runs branch November 13, 2026 15:42
@kong-volcano-app kong-volcano-app Bot mentioned this pull request Nov 13, 2026
tkkhq added a commit that referenced this pull request Nov 15, 2026
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