diff --git a/Makefile b/Makefile index 447a1b26..fce0408e 100644 --- a/Makefile +++ b/Makefile @@ -80,7 +80,7 @@ api-e2e-cloud: ## Run provisioning CLI API E2E tests against VOLCANO_API_URL and VOLCANO_API_E2E=1 go test ./tests/e2e/api -run '^TestAPIE2E(Smoke|Cloud)' -count=1 -timeout 240m localmode-e2e: ## Run destructive local-mode Docker smoke tests - VOLCANO_LOCALMODE_E2E=1 go test ./tests/e2e/localmode -run 'TestLocalModeE2E(Smoke|StopCleanSandboxes|DurableMessages|DurableCallbacks)' -count=1 -timeout 35m + VOLCANO_LOCALMODE_E2E=1 go test ./tests/e2e/localmode -run 'TestLocalModeE2E(Smoke|StopCleanSandboxes|DurableMessages|DurableCallbacks|DatabaseMigrations)' -count=1 -timeout 35m lint: ## Run golangci-lint (includes gofmt, goimports, and go vet) go tool golangci-lint run ./... diff --git a/docs/databases.md b/docs/databases.md index 3dc92402..099f48e5 100644 --- a/docs/databases.md +++ b/docs/databases.md @@ -28,18 +28,97 @@ it using its connection string. | Delete | `volcano databases delete ` | | List regions and PostgreSQL versions | `volcano cloud databases regions`, `volcano databases versions` | | Change size (cloud) | `volcano cloud databases resize --type ` | -| Apply local migrations | `volcano migrations deploy --all -d ` | -| Apply cloud migrations | `volcano cloud databases migration up --all -d ` | +| Compare migration files with the history | `volcano databases migration status -d ` | +| Apply pending migrations | `volcano databases migration apply -d ` | +| Record files that already ran | `volcano databases migration baseline -d --all` | +| List or show migration runs | `volcano databases migration runs list -d ` | +| Run migration files without tracking | `volcano databases migration up --all -d ` | -Local migrations run from `volcano/migrations` against a local database. Cloud -migrations use the cloud database command and connect through the managed -database proxy; the local `migrations deploy` command does not have a cloud -variant. [`volcano deploy --with-migrations`](deploy.md) runs the same -migrations as part of a whole-project deploy. +Every migration command works in both trees: prefix it with `cloud` for a cloud +project. Local development also keeps `volcano migrations deploy --all -d `, +the untracked `up`. -Each file runs as a single query, and the database proxy refuses a query with -more than one statement, so put one SQL statement in each file. Files run in -name order. +## Migrations + +Migrations are SQL files in `volcano/migrations`. With one database, put them in +`volcano/migrations` itself. With several, put each database's files in +`volcano/migrations//`, and put nothing at the top level. Names that +start with `_` or `.` are skipped, and files run in byte order of their names. +Tracked runs take only names that end in lowercase `.sql`. +Git deploy reads the directory the same way, so a layout the CLI refuses is one +a push would refuse too. + +### Tracked runs + +`migration apply` sends a database's files to a tracked migration run. Volcano +compares them with the database's migration history and applies only the files +the history does not record, then records them. A file runs once, however many +times you apply. + +```bash +volcano cloud databases migration apply -d app +``` + +```text +Starting a migration run of 3 file(s) from volcano/migrations on database 'app' +Run 7f3a1c2e-5d4b-4a91-8c6f-2b1e9d0a4c73 queued + running: rehearse + running: apply +✓ Migration run 7f3a1c2e-5d4b-4a91-8c6f-2b1e9d0a4c73 succeeded: 1 applied, 2 already applied +``` + +Before it changes the database, a cloud run rehearses the pending files on a +short-lived branch, and takes a backup where the plan includes backups. A file +that fails in the rehearsal fails the run without touching the database. Local +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 and runs in one transaction. It cannot +`COMMIT`, `ROLLBACK`, or use savepoints. A statement that cannot run in a +transaction, such as `CREATE INDEX CONCURRENTLY`, goes alone in a file whose +first line is `-- volcano:no-transaction`. That file runs outside a +transaction, so nothing rolls it back, and a retried run runs it again from its +start: write it to be safe to repeat, such as with `IF NOT EXISTS`. A run refuses a file whose content changed after +it was applied (`checksum_mismatch`): put the recorded content back and make the +change in a new file. One database runs one migration run at a time; a second +`apply` while one is queued or running names the run to wait for. + +`migration apply --no-wait` returns once the run is queued. `migration runs list` +shows a database's 50 most recent runs, including the ones Git deploys start, +and `migration runs get ` shows one run's files and why it failed. + +### Adopt a database migrated without tracking + +A database migrated with `migration up`, `migrations deploy`, or a +`--with-migrations` deploy from a CLI before tracked runs already reflects its +files, but its history records none of them. Record them first, or the first +run applies them again: + +```bash +volcano cloud databases migration status -d app # every file is pending +volcano cloud databases migration baseline -d app --all +volcano cloud databases migration apply -d app # applies only new files +``` + +`baseline` records files as applied without running them. Use `-f `, +repeatable, to record only some of them. It asks for confirmation; pass `--yes` +to skip it. + +### Availability + +Tracked runs need the `databases.migrations` capability, which the CLI reads +from the API's `GET /capabilities` before it starts one. Where the API does not +offer it, +`migration apply`, `migration baseline`, and `deploy --with-migrations` refuse +before sending anything and name the untracked command instead. `migration +status` and `migration runs` keep working, so you can still read what earlier +runs did. Local development always offers tracked runs. + +### Run files without tracking + +`migration up` connects through the managed database proxy and runs each file +as a single query, whether or not it ran before. The proxy refuses a query with +more than one statement, so put one SQL statement in each file. ## Regions and versions @@ -241,9 +320,10 @@ volcano databases create app --type volcano-db-s --region us-east-1 # Show the connection string volcano databases get app --show-connection-string -# Apply migration files from volcano/migrations to the "app" database -volcano migrations deploy --all -d app +# Apply pending migration files from volcano/migrations to the local "app" database +volcano databases migration apply -d app -# Apply migration files to the managed cloud database named "app" -volcano cloud databases migration up --all -d app +# Apply them to the cloud database named "app", and see what is recorded +volcano cloud databases migration apply -d app +volcano cloud databases migration status -d app ``` diff --git a/docs/deploy.md b/docs/deploy.md index 30cc5e48..62c07010 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -40,7 +40,7 @@ The steps run in this order. A step with nothing to deploy is skipped. | Step | Reads | Same as | |---|---|---| | variables | the env file, or the variable sections of the manifests | `variables deploy` | -| migrations | `volcano/migrations/*.sql`, only with `--with-migrations` | `migrations deploy --all` (cloud: `databases migration up --all`) | +| migrations | `volcano/migrations`, only with `--with-migrations` | `databases migration apply -d ` | | functions | the standard functions in `volcano/functions` | `functions deploy --all` | | durable | the functions the first manifest declares `kind: durable` | `durable deploy --all` | | frontends | the frontends listed in `.volcano/source-layout.json` | `frontends deploy` | @@ -87,8 +87,8 @@ its status is `provisioning`. | Flag | Description | |---|---| -| `--with-migrations` | Apply every migration in `volcano/migrations`. | -| `-d, --database` | Database to migrate. Local mode defaults to `app`. In the cloud, the default is the database the manifests' `databases:` sections declare, if they declare exactly one. | +| `--with-migrations` | Apply `volcano/migrations` in tracked migration runs. | +| `-d, --database` | Database to migrate. With a directory per database in `volcano/migrations`, the default is every database that has one. Otherwise local mode defaults to `app`, and the cloud defaults to the database the manifests' `databases:` sections declare, if they declare exactly one. | | `--env-file` | Env file to deploy. Default: `volcano/volcano.env` or `./volcano.env`. | | `--no-env-file` | Deploy no env file, even when `volcano/volcano.env` exists. Use it when a manifest's `variables` section holds the project's variables. | | `-c, --config` | Manifest to apply. Repeat it to apply several, in order. Passing any `-c` replaces default lookup: `./volcano-config.yaml` first, then `volcano/volcano-config.yaml` only when the root file is absent. | @@ -144,11 +144,35 @@ each manifest that has them. The config step applies the full manifests later. ## Migrations -Migrations run only with `--with-migrations`, and without tracking: every file -runs again on each run. Write migrations that are safe to re-run, for example -with `create table if not exists`, and put one SQL statement in each file. When -a later step fails, run the command again without `--with-migrations` and -`-d`, since the migrations already ran. +Migrations run only with `--with-migrations`. The step starts a tracked +migration run for each database, the same run +[`databases migration apply`](databases.md#tracked-runs) starts, and waits for +it to end, even with `--no-wait`. The database's migration history records each +file it applies, so a file runs once: running the deploy again after a later +step failed applies nothing new. + +```text +Starting a migration run of 3 file(s) from volcano/migrations on database 'app' +Run 7f3a1c2e-5d4b-4a91-8c6f-2b1e9d0a4c73 queued + running: apply +✓ Migration run 7f3a1c2e-5d4b-4a91-8c6f-2b1e9d0a4c73 succeeded: 1 applied, 2 already applied +``` + +A run that fails stops the deploy before the functions step. With one database, +put its files in `volcano/migrations`; with several, put each database's files +in `volcano/migrations//`, and the step migrates each of them. + +A database that an earlier CLI migrated with `--with-migrations` already +reflects its files, but its history records none of them. Record them once +before the next deploy, or the run applies them again: + +```bash +volcano cloud databases migration baseline -d app --all +``` + +Tracked runs need the `databases.migrations` capability. Where the API does not +offer it, `--with-migrations` refuses before anything changes and names +`databases migration up --all`, which runs every file again without tracking. ## Frontends diff --git a/internal/api/capabilities.go b/internal/api/capabilities.go new file mode 100644 index 00000000..8b59f927 --- /dev/null +++ b/internal/api/capabilities.go @@ -0,0 +1,28 @@ +package api + +import ( + "context" + + "github.com/Kong/volcano-cli/internal/apiclient" +) + +// CapabilityAvailable reports whether the API lists capability id as +// 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) { + resp, err := c.client.ListCapabilitiesWithResponse(ctx) + if err != nil { + return false, err + } + list, err := apiResult[apiclient.CapabilityList](resp.StatusCode(), resp.Body, resp.JSON200) + if err != nil { + return false, err + } + for _, capability := range list.Data { + if capability.Id == id { + return capability.Status == apiclient.CapabilityStatusAvailable, nil + } + } + return false, nil +} diff --git a/internal/api/database_migration_runs.go b/internal/api/database_migration_runs.go new file mode 100644 index 00000000..78f4c813 --- /dev/null +++ b/internal/api/database_migration_runs.go @@ -0,0 +1,121 @@ +package api + +import ( + "context" + "net/http" + "strings" + + "github.com/google/uuid" + + "github.com/Kong/volcano-cli/internal/apiclient" +) + +// MigrationRunInProgressCode is the 409 the API answers when the database +// already has a queued or running migration run. +const MigrationRunInProgressCode = "migration_run_in_progress" + +// MigrationRunInProgressError is a migration request refused because the +// database already has a queued or running run, which RunID names. +type MigrationRunInProgressError struct { + Err *Error + RunID uuid.UUID +} + +func (e *MigrationRunInProgressError) Error() string { return e.Err.Error() } + +func (e *MigrationRunInProgressError) Unwrap() error { return e.Err } + +// GetDatabaseMigrations returns the files a database's migration history +// records and the run that is queued or running, if any. +func (c *Client) GetDatabaseMigrations(ctx context.Context, projectID uuid.UUID, databaseName string) (*apiclient.DatabaseMigrations, error) { + resp, err := c.client.GetDatabaseMigrationsWithResponse(ctx, projectID, strings.TrimSpace(databaseName)) + if err != nil { + return nil, err + } + return apiResult(resp.StatusCode(), resp.Body, resp.JSON200, resp.JSON404, resp.JSON409, resp.JSON503) +} + +// CreateDatabaseMigrationRun starts a server-side migration run of files on +// databaseName. The server compares them with the database's history and +// applies the pending ones; nothing here connects to the database. +// idempotencyKey makes a retried request return the run the first one started, +// which the API answers with 200 instead of 202. +func (c *Client) CreateDatabaseMigrationRun( + ctx context.Context, projectID uuid.UUID, databaseName, idempotencyKey string, files []apiclient.DatabaseMigrationFile, +) (*apiclient.DatabaseMigrationRun, error) { + params := &apiclient.CreateDatabaseMigrationRunParams{} + if key := strings.TrimSpace(idempotencyKey); key != "" { + params.IdempotencyKey = &key + } + resp, err := c.client.CreateDatabaseMigrationRunWithResponse(ctx, projectID, strings.TrimSpace(databaseName), params, + apiclient.CreateDatabaseMigrationRunJSONRequestBody{Files: files}) + if err != nil { + return nil, err + } + switch { + case resp.JSON202 != nil: + return resp.JSON202, nil + case resp.JSON200 != nil: + return resp.JSON200, nil + case resp.JSON409 != nil: + return nil, migrationConflictError(resp.JSON409) + } + // A run the parser did not accept quotes database errors, which can quote + // data, so only an error body's own fields reach the error. + return nil, apiErrorFromGeneratedErrors(resp.StatusCode(), errorFieldsOnly(resp.Body), resp.JSON400, resp.JSON404, resp.JSON413) +} + +// BaselineDatabaseMigrations records files in databaseName's migration +// history as applied, without running them. The server skips files it already +// records with the same checksum, and refuses one recorded with another. +func (c *Client) BaselineDatabaseMigrations( + ctx context.Context, projectID uuid.UUID, databaseName string, files []apiclient.DatabaseMigrationFile, +) (*apiclient.DatabaseMigrationBaseline, error) { + resp, err := c.client.BaselineDatabaseMigrationsWithResponse(ctx, projectID, strings.TrimSpace(databaseName), + apiclient.BaselineDatabaseMigrationsJSONRequestBody{Files: files}) + if err != nil { + return nil, err + } + if resp.JSON200 != nil { + return resp.JSON200, nil + } + if resp.JSON409 != nil { + return nil, migrationConflictError(resp.JSON409) + } + return nil, apiErrorFromGeneratedErrors(resp.StatusCode(), resp.Body, resp.JSON400, resp.JSON404, resp.JSON413, resp.JSON503) +} + +// migrationConflictError is the error for a migration 409, naming the run in +// progress when that is the reason. +func migrationConflictError(conflict *apiclient.DatabaseMigrationConflictError) error { + apiErr := &Error{StatusCode: http.StatusConflict, Message: strings.TrimSpace(conflict.Error), fromAPI: true} + if conflict.Code != nil { + apiErr.Code = *conflict.Code + } + if apiErr.Code == MigrationRunInProgressCode && conflict.RunId != nil { + return &MigrationRunInProgressError{Err: apiErr, RunID: *conflict.RunId} + } + return apiErr +} + +// GetDatabaseMigrationRun returns one migration run with each file's outcome. +func (c *Client) GetDatabaseMigrationRun( + ctx context.Context, projectID uuid.UUID, databaseName string, runID uuid.UUID, +) (*apiclient.DatabaseMigrationRun, error) { + resp, err := c.client.GetDatabaseMigrationRunWithResponse(ctx, projectID, strings.TrimSpace(databaseName), runID) + if err != nil { + return nil, err + } + return apiResult(resp.StatusCode(), errorFieldsOnly(resp.Body), resp.JSON200, resp.JSON404) +} + +// ListDatabaseMigrationRuns returns a database's 50 most recent migration +// runs, newest first, without their files. +func (c *Client) ListDatabaseMigrationRuns(ctx context.Context, projectID uuid.UUID, databaseName string) (*apiclient.DatabaseMigrationRunList, error) { + resp, err := c.client.ListDatabaseMigrationRunsWithResponse(ctx, projectID, strings.TrimSpace(databaseName)) + if err != nil { + return nil, err + } + // A run's error can quote data, as in GetDatabaseMigrationRun. + return apiResult(resp.StatusCode(), errorFieldsOnly(resp.Body), resp.JSON200, resp.JSON404) +} diff --git a/internal/api/database_migration_runs_test.go b/internal/api/database_migration_runs_test.go new file mode 100644 index 00000000..b01cbb08 --- /dev/null +++ b/internal/api/database_migration_runs_test.go @@ -0,0 +1,72 @@ +package api + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Kong/volcano-cli/internal/apiclient" +) + +func newMigrationsTestClient(t *testing.T, handler http.HandlerFunc) *Client { + t.Helper() + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + client, err := NewClient(server.URL, "token", WithHTTPClient(server.Client())) + require.NoError(t, err) + return client +} + +// A capability counts only when the list names it available: any other +// status, or none, is unavailable. +func TestCapabilityAvailable(t *testing.T) { + client := newMigrationsTestClient(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"data":[{"id":"a","status":"available"},{"id":"b","status":"unavailable"},{"id":"c","status":"paused"}]}`)) + }) + for id, want := range map[string]bool{"a": true, "b": false, "c": false, "missing": false} { + available, err := client.CapabilityAvailable(context.Background(), id) + require.NoError(t, err) + assert.Equal(t, want, available, id) + } +} + +func TestMigrationRunConflictKeepsTheRunItNames(t *testing.T) { + runID := uuid.MustParse("7f3a1c2e-5d4b-4a91-8c6f-2b1e9d0a4c73") + var key string + client := newMigrationsTestClient(t, func(w http.ResponseWriter, r *http.Request) { + key = r.Header.Get("Idempotency-Key") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusConflict) + _, _ = w.Write([]byte(`{"error":"a migration run is already queued or running","code":"migration_run_in_progress","run_id":"` + runID.String() + `"}`)) + }) + + _, err := client.CreateDatabaseMigrationRun(context.Background(), uuid.New(), "app", "volcano-cli-1", + []apiclient.DatabaseMigrationFile{{Name: "001_a.sql", Sql: "select 1;"}}) + inProgress, ok := errors.AsType[*MigrationRunInProgressError](err) + require.True(t, ok, err) + assert.Equal(t, runID, inProgress.RunID) + assert.Equal(t, http.StatusConflict, Status(err)) + assert.Equal(t, MigrationRunInProgressCode, Code(err)) + assert.Equal(t, "volcano-cli-1", key) +} + +// An Idempotency-Key replay answers 200 with the run the key started. +func TestMigrationRunReplayReturnsTheRun(t *testing.T) { + client := newMigrationsTestClient(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":"7f3a1c2e-5d4b-4a91-8c6f-2b1e9d0a4c73","database":{"type":"database","id":"0b9c3f5e-2a41-4d7e-9f0a-6c8e1d2b3a45","name":"app"},` + + `"status":"running","phase":"apply","source":"api","warnings":[],"created_at":"2026-10-29T10:00:00Z","updated_at":"2026-10-29T10:00:00Z"}`)) + }) + + run, err := client.CreateDatabaseMigrationRun(context.Background(), uuid.New(), "app", "volcano-cli-1", + []apiclient.DatabaseMigrationFile{{Name: "001_a.sql", Sql: "select 1;"}}) + require.NoError(t, err) + assert.Equal(t, apiclient.DatabaseMigrationRunStatusRunning, run.Status) +} diff --git a/internal/cmd/databases/migration/apply.go b/internal/cmd/databases/migration/apply.go new file mode 100644 index 00000000..6e073f24 --- /dev/null +++ b/internal/cmd/databases/migration/apply.go @@ -0,0 +1,215 @@ +package migration + +import ( + "context" + "fmt" + "io" + "slices" + + "github.com/spf13/cobra" + + "github.com/Kong/volcano-cli/internal/apiclient" + clidatabase "github.com/Kong/volcano-cli/internal/database" + "github.com/Kong/volcano-cli/internal/output" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" + "github.com/Kong/volcano-cli/internal/theme" +) + +func newApply(deps cliruntime.Deps) *cobra.Command { + var database string + var noWait, jsonOutput bool + cmd := &cobra.Command{ + Use: "apply", + Short: "Apply pending migrations in a tracked run", + Long: fmt.Sprintf(`Send every migration file for a database to a tracked migration run, and wait +for it to end. Volcano applies only the files the database's migration history +does not record, each in its own transaction, and records them, so a file runs +once. It rehearses the pending files on a short-lived branch of the database +and takes a backup before it applies them, where the plan has branches and +backups. + +A file may hold several statements. It cannot COMMIT, ROLLBACK, or use +savepoints. A statement that cannot run in a transaction, such as CREATE INDEX +CONCURRENTLY, goes alone in a file whose first line is +'-- volcano:no-transaction'. That file runs outside a transaction, and a +retried run runs it again from its start, so write it to be safe to repeat. + +A database migrated without tracking, with '%s' or an earlier +--with-migrations deploy, already reflects its files. Record them with '%s' +before the first run, or the run applies them again. + +Tracked runs need the %q capability. Where the API does not offer it, the +command refuses before sending anything. + +Usage: + %s + %s`, + cliruntime.CommandPath(deps, "databases migration up"), + cliruntime.CommandPath(deps, "databases migration baseline"), + clidatabase.MigrationsCapability, + cliruntime.CommandPath(deps, "databases migration apply -d mydb"), + cliruntime.CommandPath(deps, "databases migration apply -d mydb --no-wait --json")), + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + name, err := requireDatabase(database) + if err != nil { + return err + } + return runApply(cmd.Context(), deps, name, !noWait, jsonOutput, cmd.OutOrStdout()) + }, + } + databaseFlag(cmd, &database) + cmd.Flags().BoolVar(&noWait, "no-wait", false, "Return once the run is queued") + cmd.Flags().BoolVar(&jsonOutput, "json", false, "Emit the run as machine-readable JSON") + return cmd +} + +func runApply(ctx context.Context, deps cliruntime.Deps, database string, wait, jsonOutput bool, out io.Writer) error { + migrations, err := clidatabase.ReadMigrations(clidatabase.MigrationsDir) + if err != nil { + return err + } + files := migrations.For(database) + if len(files) == 0 { + if jsonOutput { + return fmt.Errorf("no migration files for database %q in %s", database, migrations.Location(database)) + } + fmt.Fprintf(out, "No migration files for database '%s' in %s; nothing to apply\n", database, migrations.Location(database)) + return nil + } + if err := clidatabase.CheckTrackedNames(files); err != nil { + return err + } + c := commands{deps: deps, database: database} + if err := c.explain(clidatabase.NewService(deps).RequireMigrationRuns(ctx)); err != nil { + return err + } + progress := out + if jsonOutput { + progress = io.Discard + } + run, err := Apply(ctx, deps, ApplyOptions{ + Database: database, + Files: files, + Location: migrations.Location(database), + Wait: wait, + Out: progress, + }) + if jsonOutput && run != nil { + if writeErr := writeJSON(out, run); writeErr != nil { + return writeErr + } + } + return err +} + +// ApplyOptions configures Apply. +type ApplyOptions struct { + Database string + Files []clidatabase.MigrationFile + // Location names where the files came from, for the progress lines. + Location string + // Wait waits for the run to end; otherwise Apply returns once it is + // queued. + Wait bool + Out io.Writer +} + +// Apply starts a tracked migration run of opts.Files on opts.Database and, +// with opts.Wait, waits for it to end. It returns the run as it last read it, +// and an error when the run failed or could not be followed to its end. The +// caller checks that the API offers tracked runs first. +func Apply(ctx context.Context, deps cliruntime.Deps, opts ApplyOptions) (*apiclient.DatabaseMigrationRun, error) { + c := commands{deps: deps, database: opts.Database} + on := theme.On(opts.Out) + service := clidatabase.NewService(deps) + fmt.Fprintf(opts.Out, "Starting a migration run of %d file(s) from %s on database '%s'\n", len(opts.Files), opts.Location, opts.Database) + run, err := service.StartMigrationRun(ctx, opts.Database, opts.Files) + if err != nil { + return nil, c.explain(err) + } + fmt.Fprintf(opts.Out, "Run %s %s\n", run.Id, theme.Status(string(run.Status), on)) + if !opts.Wait { + fmt.Fprintf(opts.Out, "%s%s\n", theme.Dim("Follow it with: ", on), theme.Command(c.runsGet(run.Id.String()), on)) + return run, nil + } + + ended, err := service.WaitMigrationRun(ctx, opts.Database, run, func(latest *apiclient.DatabaseMigrationRun) { + fmt.Fprintf(opts.Out, " %s: %s\n", theme.Status(string(latest.Status), on), latest.Phase) + }) + if err != nil { + return ended, fmt.Errorf("stopped waiting for migration run %s on database %q, which carries on: %w\nFollow it with '%s'", + run.Id, opts.Database, err, c.runsGet(run.Id.String())) + } + printRunEnd(opts.Out, ended) + if ended.Status == apiclient.DatabaseMigrationRunStatusFailed { + return ended, c.runFailure(ended) + } + return ended, nil +} + +// printRunEnd prints how a run ended: the files it applied when it succeeded, +// and every file it did not settle as already applied when it failed. +func printRunEnd(w io.Writer, run *apiclient.DatabaseMigrationRun) { + var files []apiclient.DatabaseMigrationRunFile + if run.Files != nil { + files = *run.Files + } + counts := map[apiclient.DatabaseMigrationRunFileOutcome]int{} + for _, file := range files { + counts[file.Outcome]++ + } + var shown []apiclient.DatabaseMigrationRunFile + for _, file := range files { + if file.Outcome != apiclient.DatabaseMigrationRunFileOutcomeAlreadyApplied { + shown = append(shown, file) + } + } + if run.Status == apiclient.DatabaseMigrationRunStatusSucceeded { + output.Success(w, "Migration run %s succeeded: %d applied, %d already applied", run.Id, + counts[apiclient.DatabaseMigrationRunFileOutcomeApplied], counts[apiclient.DatabaseMigrationRunFileOutcomeAlreadyApplied]) + } else { + output.Errorf(w, "Migration run %s failed in %s: %d applied, %d already applied", run.Id, run.Phase, + counts[apiclient.DatabaseMigrationRunFileOutcomeApplied], counts[apiclient.DatabaseMigrationRunFileOutcomeAlreadyApplied]) + } + if run.BackupName != nil && *run.BackupName != "" { + if run.Phase == apiclient.DatabaseMigrationRunPhaseApply { + fmt.Fprintf(w, "Backup taken first: %s\n", *run.BackupName) + } else { + fmt.Fprintf(w, "Backup %s may not exist: the run ended while taking it\n", *run.BackupName) + } + } + output.MigrationRunFiles(w, shown) + output.MigrationRunWarnings(w, run.Warnings) +} + +// runFailure says why a run failed and what to do about it. The database's +// own message stays on the failed file, which printRunEnd showed: it can quote +// the file or its data, so it is kept out of the error. The run's message +// names the file, line, and SQLSTATE only. +func (c commands) runFailure(run *apiclient.DatabaseMigrationRun) error { + code, message := "unknown", "the run reported no reason" + if run.Error != nil { + code, message = string(run.Error.Code), theme.StripControl(run.Error.Message) + } + err := fmt.Errorf("migration run %s failed in %s (%s): %s", run.Id, run.Phase, code, message) + switch code { + case string(apiclient.MigrationFailed): + if run.Files != nil && !anyOutcome(*run.Files, apiclient.DatabaseMigrationRunFileOutcomeAlreadyApplied, apiclient.DatabaseMigrationRunFileOutcomeApplied) { + return fmt.Errorf("%w\nIf database %q already reflects these files because they ran without tracking, record them with '%s' instead", + err, c.database, c.baseline()) + } + case string(apiclient.ChecksumMismatch): + return fmt.Errorf("%w\nPut the file's recorded content back and make the change in a new file. '%s' shows which file changed", + err, c.path("status")) + case string(apiclient.HistoryMissing): + return fmt.Errorf("%w\nRecord the files that were already applied with '%s'", err, c.path("baseline -f ")) + } + return err +} + +func anyOutcome(files []apiclient.DatabaseMigrationRunFile, outcomes ...apiclient.DatabaseMigrationRunFileOutcome) bool { + return slices.ContainsFunc(files, func(file apiclient.DatabaseMigrationRunFile) bool { + return slices.Contains(outcomes, file.Outcome) + }) +} diff --git a/internal/cmd/databases/migration/baseline.go b/internal/cmd/databases/migration/baseline.go new file mode 100644 index 00000000..a3a3a6c6 --- /dev/null +++ b/internal/cmd/databases/migration/baseline.go @@ -0,0 +1,149 @@ +package migration + +import ( + "errors" + "fmt" + "strings" + + "github.com/spf13/cobra" + + "github.com/Kong/volcano-cli/internal/api" + "github.com/Kong/volcano-cli/internal/confirm" + clidatabase "github.com/Kong/volcano-cli/internal/database" + "github.com/Kong/volcano-cli/internal/output" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +func newBaseline(deps cliruntime.Deps) *cobra.Command { + var database string + var names []string + var all, yes, jsonOutput bool + cmd := &cobra.Command{ + Use: "baseline", + Short: "Record migrations as applied without running them", + Long: fmt.Sprintf(`Record migration files in a database's migration history as applied, without +running them, so later runs skip them. + +Use it once for a database whose schema already reflects its files, such as +one migrated with '%s' or an earlier --with-migrations deploy, before +its first tracked run. After a history was lost (history_missing), record +exactly the files that were already applied. A file recorded this way never +runs, so record only files the database already reflects. + +Files the history already holds with the same content are skipped, so a +repeat records nothing. A file recorded with different content is refused +(checksum_mismatch), and nothing is recorded. A baseline does not wait for +another change to the database: while one is in progress it is refused with +database_busy, and you can try again. + +By default this command prompts for confirmation. Use --yes to skip the prompt. + +Usage: + %s + %s`, + cliruntime.CommandPath(deps, "databases migration up"), + cliruntime.CommandPath(deps, "databases migration baseline -d mydb --all"), + cliruntime.CommandPath(deps, "databases migration baseline -d mydb -f 001_create_users -f 002_add_posts")), + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + name, err := requireDatabase(database) + if err != nil { + return err + } + c := commands{deps: deps, database: name} + migrations, err := clidatabase.ReadMigrations(clidatabase.MigrationsDir) + if err != nil { + return err + } + files, err := selectBaselineFiles(migrations.For(name), migrations.Location(name), names, all) + if err != nil { + return err + } + if err := clidatabase.CheckTrackedNames(files); err != nil { + return err + } + if err := clidatabase.NewService(deps).RequireMigrationRuns(cmd.Context()); err != nil { + return baselineUnavailable(err) + } + if !yes { + listed := make([]string, len(files)) + for i, file := range files { + listed[i] = " - " + file.Name() + } + // With --json, stdout carries only the JSON document. + prompt := cmd.OutOrStdout() + if jsonOutput { + prompt = cmd.ErrOrStderr() + } + confirmed, err := confirm.Action(cmd.InOrStdin(), prompt, + fmt.Sprintf("A baseline records these files in the migration history of database '%s' as applied, "+ + "without running them. A file recorded this way never runs.\n%s", name, strings.Join(listed, "\n")), + fmt.Sprintf("Record %d file(s) as applied?", len(files))) + if err != nil { + return err + } + if !confirmed { + return nil + } + } + baseline, err := clidatabase.NewService(deps).BaselineMigrations(cmd.Context(), name, files) + if err != nil { + if errors.Is(err, clidatabase.ErrMigrationRunsUnavailable) { + return baselineUnavailable(err) + } + return c.explain(err) + } + if jsonOutput { + return writeJSON(cmd.OutOrStdout(), baseline) + } + output.DatabaseMigrationBaseline(cmd.OutOrStdout(), baseline, name) + return nil + }, + } + databaseFlag(cmd, &database) + cmd.Flags().BoolVarP(&all, "all", "a", false, "Record every migration file for the database") + cmd.Flags().StringArrayVarP(&names, "file", "f", nil, "Record a migration by name or path; repeat for several") + cmd.Flags().BoolVarP(&yes, "yes", "y", false, "Skip confirmation prompt") + cmd.Flags().BoolVar(&jsonOutput, "json", false, "Emit machine-readable JSON") + cmd.MarkFlagsMutuallyExclusive("all", "file") + cmd.MarkFlagsOneRequired("all", "file") + return cmd +} + +// baselineUnavailable words an API without tracked runs for a baseline, +// which has no untracked counterpart to suggest. +func baselineUnavailable(err error) error { + if !errors.Is(err, clidatabase.ErrMigrationRunsUnavailable) { + return err + } + return api.WithCause(fmt.Errorf("%w (capability %q), so there is no migration history to record a baseline in", + clidatabase.ErrMigrationRunsUnavailable, clidatabase.MigrationsCapability), err) +} + +// selectBaselineFiles picks the files a baseline records: all of them, or +// the ones named, each of which has to exist. +func selectBaselineFiles(files []clidatabase.MigrationFile, location string, names []string, all bool) ([]clidatabase.MigrationFile, error) { + if len(files) == 0 { + return nil, fmt.Errorf("no migration files found in %s/", location) + } + if all { + return files, nil + } + var selected []clidatabase.MigrationFile + seen := map[string]bool{} + for _, name := range names { + name = strings.TrimSpace(name) + if name == "" { + return nil, errors.New("--file needs a migration name") + } + file, ok := findMigrationFile(files, name) + if !ok { + return nil, fmt.Errorf("migration %q not found in %s/\navailable migrations: %s", name, location, formatMigrationNames(files)) + } + if !seen[file.Path] { + seen[file.Path] = true + selected = append(selected, file) + } + } + return selected, nil +} diff --git a/internal/cmd/databases/migration/commands.go b/internal/cmd/databases/migration/commands.go new file mode 100644 index 00000000..b10d8694 --- /dev/null +++ b/internal/cmd/databases/migration/commands.go @@ -0,0 +1,83 @@ +package migration + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "strings" + + "github.com/spf13/cobra" + + "github.com/Kong/volcano-cli/internal/api" + clidatabase "github.com/Kong/volcano-cli/internal/database" + "github.com/Kong/volcano-cli/internal/output" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +// commands names the migration commands for one database, in the tree the +// command runs in. +type commands struct { + deps cliruntime.Deps + database string +} + +func (c commands) path(args string) string { + return cliruntime.CommandPath(c.deps, "databases migration "+args+" -d "+cliruntime.ShellQuote(c.database)) +} + +func (c commands) apply() string { return c.path("apply") } +func (c commands) up() string { return c.path("up --all") } +func (c commands) baseline() string { return c.path("baseline --all") } + +func (c commands) runsGet(runID string) string { + return cliruntime.CommandPath(c.deps, "databases migration runs get "+runID+" -d "+cliruntime.ShellQuote(c.database)) +} + +func (c commands) output() output.MigrationCommands { + return output.MigrationCommands{Apply: c.apply(), Baseline: c.baseline(), RunsGet: c.runsGet} +} + +// explain says what to do about a refusal a migration command can meet, +// keeping the refusal so the exit code still reports it. +func (c commands) explain(err error) error { + if err == nil { + return nil + } + if errors.Is(err, clidatabase.ErrMigrationRunsUnavailable) { + return api.WithCause(fmt.Errorf("%w (capability %q). "+ + "To run the files without tracking, which runs every file again, use '%s'", + clidatabase.ErrMigrationRunsUnavailable, clidatabase.MigrationsCapability, c.up()), err) + } + if inProgress, ok := errors.AsType[*api.MigrationRunInProgressError](err); ok { + return api.WithCause(fmt.Errorf("%w\nWait for it to end with '%s', then run this command again", + err, c.runsGet(inProgress.RunID.String())), err) + } + if api.Code(err) == "history_missing" { + return api.WithCause(fmt.Errorf("%w\nRecord the files that were already applied with '%s'", + err, c.path("baseline -f ")), err) + } + return err +} + +// databaseFlag adds the required --database flag. +func databaseFlag(cmd *cobra.Command, database *string) { + cmd.Flags().StringVarP(database, "database", "d", "", "Database name (required)") + if err := cmd.MarkFlagRequired("database"); err != nil { + panic(err) + } +} + +func requireDatabase(database string) (string, error) { + database = strings.TrimSpace(database) + if database == "" { + return "", errors.New("specify the database with --database/-d") + } + return database, nil +} + +func writeJSON(w io.Writer, value any) error { + encoder := json.NewEncoder(w) + encoder.SetIndent("", " ") + return encoder.Encode(value) +} diff --git a/internal/cmd/databases/migration/fake_test.go b/internal/cmd/databases/migration/fake_test.go new file mode 100644 index 00000000..07911855 --- /dev/null +++ b/internal/cmd/databases/migration/fake_test.go @@ -0,0 +1,194 @@ +package migration + +import ( + "encoding/json" + "maps" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +const ( + migrationRunID = "7f3a1c2e-5d4b-4a91-8c6f-2b1e9d0a4c73" + migrationAppDir = "/projects/" + migrationProjectID + "/databases/app" +) + +// fakeMigrationAPI serves the capability list and the migration routes of the +// database named app, and records the requests that change something. +type fakeMigrationAPI struct { + t *testing.T + // unavailable leaves databases.migrations out of the capability list. + unavailable bool + history map[string]any + // runs are the run bodies GET returns, one per read; the last one repeats. + runs []map[string]any + // startStatus and startBody answer POST migration-runs. + startStatus int + startBody map[string]any + // baselineStatus and baselineBody answer POST migrations/baseline. + baselineStatus int + baselineBody map[string]any + + mu sync.Mutex + requests []string + sent []sentFiles + keys []string + runReads int + capReads int + listedRuns map[string]any +} + +type sentFiles struct { + Files []struct { + Name string `json:"name"` + SQL string `json:"sql"` + } `json:"files"` +} + +func newFakeMigrationAPI(t *testing.T) *fakeMigrationAPI { + t.Helper() + return &fakeMigrationAPI{ + t: t, + history: map[string]any{"history": []any{}}, + startStatus: http.StatusAccepted, + startBody: migrationRun("queued", "plan", nil), + baselineStatus: http.StatusOK, + baselineBody: map[string]any{"recorded": []string{}}, + listedRuns: map[string]any{"data": []any{}}, + } +} + +func (f *fakeMigrationAPI) start() *httptest.Server { + server := httptest.NewServer(http.HandlerFunc(f.serve)) + f.t.Cleanup(server.Close) + return server +} + +func (f *fakeMigrationAPI) deps(server *httptest.Server) cliruntime.Deps { + return cliruntime.Deps{ + HTTPClient: server.Client(), + APIBaseURL: server.URL, + NewTicker: func(time.Duration) cliruntime.Ticker { return tickNow{} }, + } +} + +func (f *fakeMigrationAPI) recorded() []string { + f.mu.Lock() + defer f.mu.Unlock() + return append([]string(nil), f.requests...) +} + +func (f *fakeMigrationAPI) serve(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + switch { + case r.Method == http.MethodGet && r.URL.Path == "/capabilities": + f.capReads++ + status := "available" + if f.unavailable { + status = "unavailable" + } + f.writeJSON(w, http.StatusOK, map[string]any{"data": []any{ + map[string]any{"id": "sandboxes", "status": "available"}, + map[string]any{"id": "databases.migrations", "status": status}, + }}) + case r.Method == http.MethodGet && r.URL.Path == migrationAppDir+"/migrations": + f.writeJSON(w, http.StatusOK, f.history) + case r.Method == http.MethodPost && r.URL.Path == migrationAppDir+"/migrations/baseline": + f.requests = append(f.requests, "baseline") + f.decodeFiles(r) + f.writeJSON(w, f.baselineStatus, f.baselineBody) + case r.Method == http.MethodPost && r.URL.Path == migrationAppDir+"/migration-runs": + f.requests = append(f.requests, "start") + f.keys = append(f.keys, r.Header.Get("Idempotency-Key")) + f.decodeFiles(r) + f.writeJSON(w, f.startStatus, f.startBody) + case r.Method == http.MethodGet && r.URL.Path == migrationAppDir+"/migration-runs": + f.writeJSON(w, http.StatusOK, f.listedRuns) + case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, migrationAppDir+"/migration-runs/"): + if len(f.runs) == 0 { + f.writeJSON(w, http.StatusNotFound, map[string]any{"error": "migration run not found"}) + return + } + run := f.runs[min(f.runReads, len(f.runs)-1)] + f.runReads++ + f.writeJSON(w, http.StatusOK, run) + default: + f.t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + http.NotFound(w, r) + } +} + +func (f *fakeMigrationAPI) decodeFiles(r *http.Request) { + var body sentFiles + require.NoError(f.t, json.NewDecoder(r.Body).Decode(&body)) + f.sent = append(f.sent, body) +} + +func (f *fakeMigrationAPI) writeJSON(w http.ResponseWriter, status int, value any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + assert.NoError(f.t, json.NewEncoder(w).Encode(value)) +} + +// sentNames lists the file names of each request that carried files. +func (f *fakeMigrationAPI) sentNames() [][]string { + f.mu.Lock() + defer f.mu.Unlock() + var out [][]string + for _, body := range f.sent { + var names []string + for _, file := range body.Files { + names = append(names, file.Name) + } + out = append(out, names) + } + return out +} + +// migrationRun is a run body with files, each given as "name:outcome". +func migrationRun(status, phase string, files []string, extra ...map[string]any) map[string]any { + run := map[string]any{ + "id": migrationRunID, + "database": map[string]any{"type": "database", "id": "0b9c3f5e-2a41-4d7e-9f0a-6c8e1d2b3a45", "name": "app"}, + "status": status, + "phase": phase, + "source": "api", + "warnings": []any{}, + "created_at": "2026-10-29T10:00:00Z", + "updated_at": "2026-10-29T10:00:00Z", + } + if files != nil { + var list []any + for _, file := range files { + name, outcome, _ := strings.Cut(file, ":") + list = append(list, map[string]any{"name": name, "outcome": outcome}) + } + run["files"] = list + } + for _, fields := range extra { + maps.Copy(run, fields) + } + return run +} + +// tickNow is a ticker that is always ready, so a wait polls again at once. +type tickNow struct{} + +func (tickNow) C() <-chan time.Time { + ch := make(chan time.Time, 1) + ch <- time.Now() + return ch +} + +func (tickNow) Reset(time.Duration) {} + +func (tickNow) Stop() {} diff --git a/internal/cmd/databases/migration/helpers_test.go b/internal/cmd/databases/migration/helpers_test.go index c2f86fa0..108229b3 100644 --- a/internal/cmd/databases/migration/helpers_test.go +++ b/internal/cmd/databases/migration/helpers_test.go @@ -1,33 +1,12 @@ package migration import ( - "os" "path/filepath" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func TestScanMigrationsSortsTopLevelSQLFiles(t *testing.T) { - dir := filepath.Join(t.TempDir(), "volcano", "migrations") - require.NoError(t, os.MkdirAll(filepath.Join(dir, "nested"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "002_second.sql"), []byte("-- second"), 0o644)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "001_first.SQL"), []byte("-- first"), 0o644)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "nested", "003_nested.sql"), []byte("-- nested"), 0o644)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "README.md"), []byte("ignore"), 0o644)) - - files, err := scanMigrations(dir) - require.NoError(t, err) - assert.Equal(t, []string{"001_first.SQL", "002_second.sql"}, migrationBaseNames(files)) -} - -func TestScanMigrationsMissingDirectoryReturnsEmpty(t *testing.T) { - files, err := scanMigrations(filepath.Join(t.TempDir(), "missing")) - require.NoError(t, err) - assert.Empty(t, files) -} - func TestNormalizeTargetMigration(t *testing.T) { for _, tc := range []struct { target string @@ -41,11 +20,3 @@ func TestNormalizeTargetMigration(t *testing.T) { assert.Equal(t, tc.want, normalizeTargetMigration(tc.target)) } } - -func migrationBaseNames(files []string) []string { - values := make([]string, 0, len(files)) - for _, file := range files { - values = append(values, filepath.Base(file)) - } - return values -} diff --git a/internal/cmd/databases/migration/migration.go b/internal/cmd/databases/migration/migration.go index 95d5be8f..99c33a30 100644 --- a/internal/cmd/databases/migration/migration.go +++ b/internal/cmd/databases/migration/migration.go @@ -5,9 +5,7 @@ import ( "errors" "fmt" "io" - "os" "path/filepath" - "sort" "strings" "github.com/spf13/cobra" @@ -37,8 +35,9 @@ func New(deps cliruntime.Deps) *cobra.Command { func NewLocal(deps cliruntime.Deps) *cobra.Command { cmd := &cobra.Command{ Use: "migrations", - Short: "Manage local database migrations", - Long: "Apply SQL migrations from the volcano/migrations directory to a local database.", + Short: "Run local database migrations without tracking", + Long: "Run SQL migrations from the volcano/migrations directory on a local database, without tracking.\n" + + "To apply each file once, use '" + cliruntime.CommandPath(deps, "databases migration apply -d app") + "'.", } cmd.AddCommand(newDeploy(deps, executeSQLMigration)) return cmd @@ -48,14 +47,29 @@ func newWithExecutor(deps cliruntime.Deps, executor sqlExecutor) *cobra.Command cmd := &cobra.Command{ Use: "migration", Short: "Manage database migrations", - Long: "Apply SQL migrations from the volcano/migrations directory.", + Long: `Apply SQL migrations from the volcano/migrations directory. + +apply sends a database's files to a tracked migration run: Volcano records +each file it applies in the database's migration history, so a file runs once. +status compares the files with that history, baseline records files that +already ran without running them, and runs lists the runs. up runs files +directly, without tracking, so every file runs again on each call. + +With one database, put the files in volcano/migrations. With several, put each +database's files in volcano/migrations//. Names starting with _ or . +are skipped.`, } + cmd.AddCommand(newStatus(deps)) + cmd.AddCommand(newApply(deps)) + cmd.AddCommand(newBaseline(deps)) + cmd.AddCommand(newRuns(deps)) cmd.AddCommand(newUp(deps, executor)) return cmd } func newUp(deps cliruntime.Deps, executor sqlExecutor) *cobra.Command { - return newApply(deps, executor, "up", fmt.Sprintf(`Apply SQL migration files from volcano/migrations/ in alphabetical order. + return newUntracked(deps, executor, "up", fmt.Sprintf(`Run SQL migration files from volcano/migrations/ in name order, without +tracking. Usage: %s @@ -64,18 +78,22 @@ Usage: %s The --database flag is required. The CLI connects directly to your database via -pgproxy and executes the SQL files. +pgproxy and executes the SQL files, one statement per file. -WARNING: Migrations are executed without tracking. You are responsible for not -re-running non-idempotent migrations.`, +WARNING: Migrations are executed without tracking. Every file runs again on each +call, whether or not the database's migration history records it, so you are +responsible for not re-running non-idempotent migrations. To apply each file +once, use '%s'.`, cliruntime.CommandPath(deps, "databases migration up --all -d mydb"), cliruntime.CommandPath(deps, "databases migration up -a -d mydb"), cliruntime.CommandPath(deps, "databases migration up -d mydb -f 001_create_users"), - cliruntime.CommandPath(deps, "databases migration up --database mydb -f 001_create_users.sql"))) + cliruntime.CommandPath(deps, "databases migration up --database mydb -f 001_create_users.sql"), + cliruntime.CommandPath(deps, "databases migration apply -d mydb"))) } func newDeploy(deps cliruntime.Deps, executor sqlExecutor) *cobra.Command { - return newApply(deps, executor, "deploy", fmt.Sprintf(`Apply SQL migration files from volcano/migrations/ in alphabetical order. + return newUntracked(deps, executor, "deploy", fmt.Sprintf(`Run SQL migration files from volcano/migrations/ in name order, without +tracking. Usage: %s @@ -84,23 +102,26 @@ Usage: %s The --database flag is required. The CLI connects directly to your database and -executes the SQL files. +executes the SQL files, one statement per file. -WARNING: Migrations are executed without tracking. You are responsible for not -re-running non-idempotent migrations.`, +WARNING: Migrations are executed without tracking. Every file runs again on each +call, whether or not the database's migration history records it, so you are +responsible for not re-running non-idempotent migrations. To apply each file +once, use '%s'.`, cliruntime.CommandPath(deps, "migrations deploy --all -d mydb"), cliruntime.CommandPath(deps, "migrations deploy -a -d mydb"), cliruntime.CommandPath(deps, "migrations deploy -d mydb -f 001_create_users"), - cliruntime.CommandPath(deps, "migrations deploy --database mydb -f 001_create_users.sql"))) + cliruntime.CommandPath(deps, "migrations deploy --database mydb -f 001_create_users.sql"), + cliruntime.CommandPath(deps, "databases migration apply -d mydb"))) } -func newApply(deps cliruntime.Deps, executor sqlExecutor, use, long string) *cobra.Command { +func newUntracked(deps cliruntime.Deps, executor sqlExecutor, use, long string) *cobra.Command { var database string var file string var all bool cmd := &cobra.Command{ Use: use, - Short: "Apply migrations", + Short: "Run migration files without tracking", Long: long, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { @@ -123,12 +144,6 @@ func newApply(deps cliruntime.Deps, executor sqlExecutor, use, long string) *cob return cmd } -// ApplyAll applies every migration in volcano/migrations to database, as -// "migrations deploy --all" does. -func ApplyAll(ctx context.Context, deps cliruntime.Deps, database string, out io.Writer) error { - return runUp(ctx, upOptions{deps: deps, database: strings.TrimSpace(database), all: true, out: out}) -} - func runUp(ctx context.Context, opts upOptions) error { if opts.executor == nil { opts.executor = executeSQLMigration @@ -143,25 +158,26 @@ func runUp(ctx context.Context, opts upOptions) error { return errors.New("specify either --all to apply all migrations or --file/-f to apply a specific migration") } - migrationsDir := filepath.Join("volcano", "migrations") if opts.all { - fmt.Fprintf(opts.out, "\nScanning %s...\n", migrationsDir) + fmt.Fprintf(opts.out, "\nScanning %s...\n", clidatabase.MigrationsDir) } - allMigrations, err := scanMigrations(migrationsDir) + migrations, err := clidatabase.ReadMigrations(clidatabase.MigrationsDir) if err != nil { return fmt.Errorf("failed to scan migrations: %w", err) } + allMigrations := migrations.For(opts.database) if len(allMigrations) == 0 { + location := migrations.Location(opts.database) if opts.file != "" { - return errors.New("no migration files found in volcano/migrations/\nnote: migrations must be placed in the volcano/migrations/ directory") + return fmt.Errorf("no migration files found in %s/\nnote: migrations must be placed in the volcano/migrations/ directory, or in volcano/migrations//", location) } - fmt.Fprintln(opts.out, "No migration files found in volcano/migrations/") + fmt.Fprintf(opts.out, "No migration files found in %s/\n", location) return nil } - migrationFiles, err := selectMigrationFiles(allMigrations, opts.file, opts.all) + migrationFiles, err := selectMigrationFiles(allMigrations, migrations.Location(opts.database), opts.file, opts.all) if err != nil { return err } @@ -169,10 +185,10 @@ func runUp(ctx context.Context, opts upOptions) error { if opts.all { fmt.Fprintf(opts.out, "Found %d file(s) (sorted alphabetically):\n", len(migrationFiles)) for _, file := range migrationFiles { - fmt.Fprintf(opts.out, " - %s\n", filepath.Base(file)) + fmt.Fprintf(opts.out, " - %s\n", file.Name()) } } else { - fmt.Fprintf(opts.out, "\nExecuting migration: %s\n", filepath.Base(migrationFiles[0])) + fmt.Fprintf(opts.out, "\nExecuting migration: %s\n", migrationFiles[0].Name()) } database, err := clidatabase.NewService(opts.deps).Get(ctx, opts.database) @@ -192,20 +208,12 @@ func runUp(ctx context.Context, opts upOptions) error { fmt.Fprintln(opts.out, " Make sure these haven't been run before!") fmt.Fprintln(opts.out) - for _, sqlFile := range migrationFiles { - fmt.Fprintf(opts.out, "Applying %s... ", filepath.Base(sqlFile)) - - content, err := os.ReadFile(sqlFile) - if err != nil { + for _, file := range migrationFiles { + fmt.Fprintf(opts.out, "Applying %s... ", file.Name()) + if err := opts.executor(ctx, connectionString, file.SQL); err != nil { fmt.Fprintln(opts.out, "error") - return fmt.Errorf("failed to read migration %s: %w", filepath.Base(sqlFile), err) + return fmt.Errorf("migration %s failed: %w", file.Name(), err) } - - if err := opts.executor(ctx, connectionString, string(content)); err != nil { - fmt.Fprintln(opts.out, "error") - return fmt.Errorf("migration %s failed: %w", filepath.Base(sqlFile), err) - } - fmt.Fprintln(opts.out, "ok") } @@ -214,43 +222,28 @@ func runUp(ctx context.Context, opts upOptions) error { return nil } -func selectMigrationFiles(allMigrations []string, target string, all bool) ([]string, error) { +func selectMigrationFiles(allMigrations []clidatabase.MigrationFile, location, target string, all bool) ([]clidatabase.MigrationFile, error) { if all { return allMigrations, nil } + if file, ok := findMigrationFile(allMigrations, target); ok { + return []clidatabase.MigrationFile{file}, nil + } + return nil, fmt.Errorf("migration %q not found in %s/\navailable migrations: %s", target, location, formatMigrationNames(allMigrations)) +} +// findMigrationFile matches target, a file name with or without .sql or a +// path ending in one, against files. +func findMigrationFile(files []clidatabase.MigrationFile, target string) (clidatabase.MigrationFile, bool) { targetName := normalizeTargetMigration(target) - for _, file := range allMigrations { - baseName := filepath.Base(file) + for _, file := range files { + baseName := file.Name() nameWithoutExt := strings.TrimSuffix(baseName, filepath.Ext(baseName)) if nameWithoutExt == targetName || baseName == targetName { - return []string{file}, nil - } - } - - return nil, fmt.Errorf("migration %q not found in volcano/migrations/\navailable migrations: %s", target, formatMigrationNames(allMigrations)) -} - -func scanMigrations(migrationsDir string) ([]string, error) { - entries, err := os.ReadDir(migrationsDir) - if errors.Is(err, os.ErrNotExist) { - return nil, nil - } else if err != nil { - return nil, err - } - - var files []string - for _, entry := range entries { - if entry.IsDir() { - continue - } - if strings.HasSuffix(strings.ToLower(entry.Name()), ".sql") { - files = append(files, filepath.Join(migrationsDir, entry.Name())) + return file, true } } - - sort.Strings(files) - return files, nil + return clidatabase.MigrationFile{}, false } func normalizeTargetMigration(target string) string { @@ -261,10 +254,10 @@ func normalizeTargetMigration(target string) string { return name } -func formatMigrationNames(files []string) string { +func formatMigrationNames(files []clidatabase.MigrationFile) string { names := make([]string, len(files)) for i, file := range files { - names[i] = filepath.Base(file) + names[i] = file.Name() } return strings.Join(names, ", ") } diff --git a/internal/cmd/databases/migration/migration_test.go b/internal/cmd/databases/migration/migration_test.go index 08c54d2f..cad01f18 100644 --- a/internal/cmd/databases/migration/migration_test.go +++ b/internal/cmd/databases/migration/migration_test.go @@ -31,11 +31,11 @@ type migrationExecution struct { func TestMigrationsUpAllExecutesSortedFiles(t *testing.T) { setMigrationCommandTestHome(t) t.Chdir(t.TempDir()) - require.NoError(t, os.MkdirAll(filepath.Join("volcano", "migrations", "nested"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join("volcano", "migrations", "_drafts"), 0o755)) require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "002_second.sql"), []byte("CREATE TABLE second;"), 0o644)) require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "001_first.sql"), []byte("CREATE TABLE first;"), 0o644)) require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "003_third.sql"), []byte("CREATE TABLE third;"), 0o644)) - require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "nested", "004_nested.sql"), []byte("CREATE TABLE nested;"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "_drafts", "004_nested.sql"), []byte("CREATE TABLE nested;"), 0o644)) require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "notes.txt"), []byte("ignore me"), 0o644)) saveMigrationCommandTestConfig(t) @@ -153,16 +153,42 @@ func TestMigrationsUpValidatesFlags(t *testing.T) { } } -// ApplyAll takes the database from a caller rather than the flag, and treats -// it the way the command treats --database. -func TestApplyAllRequiresADatabase(t *testing.T) { +// With a directory per database, up runs the directory named for the +// database, as Git deploy and tracked runs read it. +func TestMigrationsUpRunsTheDatabasesDirectory(t *testing.T) { setMigrationCommandTestHome(t) t.Chdir(t.TempDir()) + writeMigrationFile(t, "app/001_app.sql", "CREATE TABLE app_table;") + writeMigrationFile(t, "analytics/001_events.sql", "CREATE TABLE events;") + saveMigrationCommandTestConfig(t) - var out bytes.Buffer - err := ApplyAll(context.Background(), cliruntime.Deps{}, " ", &out) - require.EqualError(t, err, "specify the database with --database/-d") - assert.Empty(t, out.String()) + connectionString := "postgres://example/app" + server := newMigrationDatabaseServer(t, http.StatusOK, migrationDatabasePayload("app", "active", &connectionString)) + defer server.Close() + + var executions []migrationExecution + executor := func(_ context.Context, conn, sql string) error { + executions = append(executions, migrationExecution{connectionString: conn, sql: sql}) + return nil + } + + out, err := executeMigrationCommand(t, newWithExecutor(cliruntime.Deps{HTTPClient: server.Client(), APIBaseURL: server.URL}, executor), "up", "--all", "-d", "app") + require.NoError(t, err) + assert.Equal(t, []string{"CREATE TABLE app_table;"}, migrationExecutionSQL(executions)) + assert.Contains(t, out, "Applying 001_app.sql... ok") + assert.NotContains(t, out, "001_events.sql") +} + +// Git deploy refuses top-level files beside database directories, so up does +// too rather than run half of what a push would refuse. +func TestMigrationsUpRefusesAMixedLayout(t *testing.T) { + setMigrationCommandTestHome(t) + t.Chdir(t.TempDir()) + writeMigrationFile(t, "001_top.sql", "CREATE TABLE top;") + writeMigrationFile(t, "app/002_app.sql", "CREATE TABLE app_table;") + + _, err := executeMigrationCommand(t, newWithExecutor(cliruntime.Deps{}, nil), "up", "--all", "-d", "app") + require.ErrorContains(t, err, "volcano/migrations mixes top-level files, such as 001_top.sql, with database directories, such as app/") } func TestMigrationsUpHandlesNoMigrationFiles(t *testing.T) { @@ -262,6 +288,13 @@ func TestMigrationsUpFailsFastOnSQLError(t *testing.T) { assert.NotContains(t, out, "Applying 002_later.sql") } +func writeMigrationFile(t *testing.T, name, content string) { + t.Helper() + path := filepath.Join("volcano", "migrations", filepath.FromSlash(name)) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) +} + func executeMigrationCommand(t *testing.T, cmd *cobra.Command, args ...string) (string, error) { t.Helper() var out bytes.Buffer diff --git a/internal/cmd/databases/migration/runs_get.go b/internal/cmd/databases/migration/runs_get.go new file mode 100644 index 00000000..c851eee8 --- /dev/null +++ b/internal/cmd/databases/migration/runs_get.go @@ -0,0 +1,51 @@ +package migration + +import ( + "fmt" + "strings" + + "github.com/google/uuid" + "github.com/spf13/cobra" + + clidatabase "github.com/Kong/volcano-cli/internal/database" + "github.com/Kong/volcano-cli/internal/output" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +func newRunsGet(deps cliruntime.Deps) *cobra.Command { + var database string + var jsonOutput bool + cmd := &cobra.Command{ + Use: "get ", + Short: "Show a migration run", + Long: `Show one migration run: its status and phase, what became of each file, and +why it failed when it did. A failed file carries the database's own error, +which can quote values from the file or its data. + +Usage: + ` + cliruntime.CommandPath(deps, "databases migration runs get 7f3a1c2e-5d4b-4a91-8c6f-2b1e9d0a4c73 -d mydb"), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + name, err := requireDatabase(database) + if err != nil { + return err + } + runID, err := uuid.Parse(strings.TrimSpace(args[0])) + if err != nil { + return fmt.Errorf("run ID %q is not a UUID", args[0]) + } + run, err := clidatabase.NewService(deps).MigrationRun(cmd.Context(), name, runID) + if err != nil { + return err + } + if jsonOutput { + return writeJSON(cmd.OutOrStdout(), run) + } + output.DatabaseMigrationRun(cmd.OutOrStdout(), run) + return nil + }, + } + databaseFlag(cmd, &database) + cmd.Flags().BoolVar(&jsonOutput, "json", false, "Emit machine-readable JSON") + return cmd +} diff --git a/internal/cmd/databases/migration/runs_list.go b/internal/cmd/databases/migration/runs_list.go new file mode 100644 index 00000000..6316c476 --- /dev/null +++ b/internal/cmd/databases/migration/runs_list.go @@ -0,0 +1,55 @@ +package migration + +import ( + "github.com/spf13/cobra" + + clidatabase "github.com/Kong/volcano-cli/internal/database" + "github.com/Kong/volcano-cli/internal/output" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +func newRuns(deps cliruntime.Deps) *cobra.Command { + cmd := &cobra.Command{ + Use: "runs", + Short: "Inspect tracked migration runs", + Long: `Inspect a database's tracked migration runs: the ones 'migration apply' and +--with-migrations deploys start, and the ones Git deploys start. Runs stay +readable whether or not tracked runs are available.`, + } + cmd.AddCommand(newRunsList(deps)) + cmd.AddCommand(newRunsGet(deps)) + return cmd +} + +func newRunsList(deps cliruntime.Deps) *cobra.Command { + var database string + var jsonOutput bool + cmd := &cobra.Command{ + Use: "list", + Short: "List a database's recent migration runs", + Long: `List a database's 50 most recent migration runs, newest first. Get a run for +its files. + +Usage: + ` + cliruntime.CommandPath(deps, "databases migration runs list -d mydb"), + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + name, err := requireDatabase(database) + if err != nil { + return err + } + runs, err := clidatabase.NewService(deps).MigrationRuns(cmd.Context(), name) + if err != nil { + return err + } + if jsonOutput { + return writeJSON(cmd.OutOrStdout(), runs) + } + output.DatabaseMigrationRuns(cmd.OutOrStdout(), runs, name) + return nil + }, + } + databaseFlag(cmd, &database) + cmd.Flags().BoolVar(&jsonOutput, "json", false, "Emit machine-readable JSON") + return cmd +} diff --git a/internal/cmd/databases/migration/status.go b/internal/cmd/databases/migration/status.go new file mode 100644 index 00000000..638cd1b7 --- /dev/null +++ b/internal/cmd/databases/migration/status.go @@ -0,0 +1,58 @@ +package migration + +import ( + "github.com/spf13/cobra" + + clidatabase "github.com/Kong/volcano-cli/internal/database" + "github.com/Kong/volcano-cli/internal/output" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +func newStatus(deps cliruntime.Deps) *cobra.Command { + var database string + var jsonOutput bool + cmd := &cobra.Command{ + Use: "status", + Short: "Compare migration files with a database's history", + Long: `Show where each migration file for a database stands against the database's +migration history: + + applied a tracked run applied it + baselined a baseline recorded it without running it + pending the history does not record it; the next run applies it + changed its content differs from what the history recorded; a run + refuses it until the recorded content is back + missing the history records it, but it is not in volcano/migrations + +Reading the history opens a connection to the database, which wakes it if it +is suspended. It works whether or not tracked migration runs are available. + +Usage: + ` + cliruntime.CommandPath(deps, "databases migration status -d mydb"), + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + name, err := requireDatabase(database) + if err != nil { + return err + } + c := commands{deps: deps, database: name} + migrations, err := clidatabase.ReadMigrations(clidatabase.MigrationsDir) + if err != nil { + return err + } + history, err := clidatabase.NewService(deps).MigrationHistory(cmd.Context(), name) + if err != nil { + return c.explain(err) + } + status := clidatabase.CompareMigrations(name, history, migrations.For(name)) + if jsonOutput { + return writeJSON(cmd.OutOrStdout(), status) + } + output.DatabaseMigrationStatus(cmd.OutOrStdout(), status, migrations.Location(name), c.output()) + return nil + }, + } + databaseFlag(cmd, &database) + cmd.Flags().BoolVar(&jsonOutput, "json", false, "Emit machine-readable JSON") + return cmd +} diff --git a/internal/cmd/databases/migration/tracked_test.go b/internal/cmd/databases/migration/tracked_test.go new file mode 100644 index 00000000..ca0310ba --- /dev/null +++ b/internal/cmd/databases/migration/tracked_test.go @@ -0,0 +1,468 @@ +package migration + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Kong/volcano-cli/internal/api" +) + +func setTrackedMigrationTest(t *testing.T) { + t.Helper() + setMigrationCommandTestHome(t) + t.Chdir(t.TempDir()) + saveMigrationCommandTestConfig(t) +} + +func TestApplySendsEveryFileAndWaitsForTheRun(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "002_add_pinned.sql", "ALTER TABLE notes ADD COLUMN pinned boolean;") + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);\nCREATE INDEX ON notes (id);") + fake := newFakeMigrationAPI(t) + fake.runs = []map[string]any{ + migrationRun("running", "rehearse", nil), + migrationRun("running", "apply", nil), + migrationRun("succeeded", "apply", []string{"001_create_notes.sql:already_applied", "002_add_pinned.sql:applied"}, + map[string]any{ + "backup_name": "before-migration-" + migrationRunID, + "warnings": []any{map[string]any{"code": "out_of_order", "file": "002_add_pinned.sql", "message": "002_add_pinned.sql sorts before 003"}}, + }), + } + server := fake.start() + + out, err := executeMigrationCommand(t, New(fake.deps(server)), "apply", "-d", "app") + require.NoError(t, err, out) + + assert.Equal(t, []string{"start"}, fake.recorded()) + assert.Equal(t, [][]string{{"001_create_notes.sql", "002_add_pinned.sql"}}, fake.sentNames()) + assert.Equal(t, "CREATE TABLE notes (id int);\nCREATE INDEX ON notes (id);", fake.sent[0].Files[0].SQL) + require.Len(t, fake.keys, 1) + assert.True(t, strings.HasPrefix(fake.keys[0], "volcano-cli-"), fake.keys[0]) + for _, want := range []string{ + "Starting a migration run of 2 file(s) from volcano/migrations on database 'app'", + "Run " + migrationRunID + " queued", + "running: rehearse", + "running: apply", + "succeeded: apply", + "Migration run " + migrationRunID + " succeeded: 1 applied, 1 already applied", + "Backup taken first: before-migration-" + migrationRunID, + "002_add_pinned.sql", + "Warning: out_of_order: 002_add_pinned.sql sorts before 003", + } { + assert.Contains(t, out, want) + } + assert.NotContains(t, out, "001_create_notes.sql ", "a file that was already applied is counted, not listed") +} + +func TestApplySendsTheDatabasesDirectoryWithItsName(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "app/001_create_notes.sql", "CREATE TABLE notes (id int);") + writeMigrationFile(t, "analytics/001_create_events.sql", "CREATE TABLE events (id int);") + fake := newFakeMigrationAPI(t) + fake.startBody = migrationRun("succeeded", "plan", []string{"001_create_notes.sql:already_applied"}) + server := fake.start() + + out, err := executeMigrationCommand(t, New(fake.deps(server)), "apply", "-d", "app") + require.NoError(t, err, out) + + assert.Equal(t, [][]string{{"app/001_create_notes.sql"}}, fake.sentNames()) + assert.Contains(t, out, "from volcano/migrations/app on database 'app'") + assert.Contains(t, out, "succeeded: 0 applied, 1 already applied") +} + +// Where the API does not offer tracked runs, apply says so and names the +// untracked command instead of sending anything. +func TestApplyRefusesWhereTrackedRunsAreUnavailable(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);") + fake := newFakeMigrationAPI(t) + fake.unavailable = true + server := fake.start() + + _, err := executeMigrationCommand(t, New(fake.deps(server)), "apply", "-d", "app") + require.EqualError(t, err, `tracked migration runs aren't available on this Volcano API (capability "databases.migrations"). `+ + `To run the files without tracking, which runs every file again, use 'volcano databases migration up --all -d app'`) + assert.Empty(t, fake.recorded()) +} + +// The capability list can change between the read and the start; the start's +// own refusal reads the same and keeps its 404 for the exit code. +func TestApplyWordsAFeatureUnavailableRefusal(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);") + fake := newFakeMigrationAPI(t) + fake.startStatus = http.StatusNotFound + fake.startBody = map[string]any{"error": "migration runs are not available", "code": "feature_unavailable"} + server := fake.start() + + _, err := executeMigrationCommand(t, New(fake.deps(server)), "apply", "-d", "app") + require.ErrorContains(t, err, "tracked migration runs aren't available on this Volcano API") + assert.Equal(t, http.StatusNotFound, api.Status(err)) +} + +func TestApplyNamesTheRunInProgress(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);") + fake := newFakeMigrationAPI(t) + fake.startStatus = http.StatusConflict + fake.startBody = map[string]any{"error": "a migration run is already queued or running", "code": "migration_run_in_progress", "run_id": migrationRunID} + server := fake.start() + + _, err := executeMigrationCommand(t, New(fake.deps(server)), "apply", "-d", "app") + require.ErrorContains(t, err, "a migration run is already queued or running") + require.ErrorContains(t, err, "Wait for it to end with 'volcano databases migration runs get "+migrationRunID+" -d app', then run this command again") + assert.Equal(t, http.StatusConflict, api.Status(err)) +} + +// A failed run lists what became of the files it did not settle, keeps the +// database's own message on the file rather than in the error, and suggests a +// baseline when nothing was ever recorded. +func TestApplyReportsAFailedRun(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);") + writeMigrationFile(t, "002_add_pinned.sql", "ALTER TABLE notes ADD COLUMN pinned boolean;") + failed := migrationRun("failed", "rehearse", []string{"001_create_notes.sql:failed", "002_add_pinned.sql:not_run"}, map[string]any{ + "error": map[string]any{"code": "migration_failed", "message": "migration 001_create_notes.sql failed at line 1 (SQLSTATE 42P07); it failed in rehearsal, so production was not changed"}, + }) + files := failed["files"].([]any) + files[0].(map[string]any)["error"] = map[string]any{"message": `relation "notes" already exists`, "sqlstate": "42P07", "line": 1} + fake := newFakeMigrationAPI(t) + fake.runs = []map[string]any{failed} + server := fake.start() + + out, err := executeMigrationCommand(t, New(fake.deps(server)), "apply", "-d", "app") + require.Error(t, err) + assert.Contains(t, out, "Migration run "+migrationRunID+" failed in rehearse: 0 applied, 0 already applied") + assert.Contains(t, out, `relation "notes" already exists (SQLSTATE 42P07, line 1)`) + assert.Contains(t, out, "not run") + assert.Equal(t, "migration run "+migrationRunID+" failed in rehearse (migration_failed): "+ + "migration 001_create_notes.sql failed at line 1 (SQLSTATE 42P07); it failed in rehearsal, so production was not changed\n"+ + `If database "app" already reflects these files because they ran without tracking, `+ + "record them with 'volcano databases migration baseline --all -d app' instead", err.Error()) + assert.NotContains(t, err.Error(), "already exists", "the database's message stays out of the error") +} + +func TestApplyNoWaitReturnsTheQueuedRun(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);") + fake := newFakeMigrationAPI(t) + server := fake.start() + + out, err := executeMigrationCommand(t, New(fake.deps(server)), "apply", "-d", "app", "--no-wait", "--json") + require.NoError(t, err, out) + + var run map[string]any + require.NoError(t, json.Unmarshal([]byte(out), &run), out) + assert.Equal(t, "queued", run["status"]) + assert.Zero(t, fake.runReads) +} + +func TestApplyWithoutFilesSendsNothing(t *testing.T) { + setTrackedMigrationTest(t) + fake := newFakeMigrationAPI(t) + server := fake.start() + + out, err := executeMigrationCommand(t, New(fake.deps(server)), "apply", "-d", "app") + require.NoError(t, err) + assert.Contains(t, out, "No migration files for database 'app' in volcano/migrations; nothing to apply") + assert.Zero(t, fake.capReads) + assert.Empty(t, fake.recorded()) +} + +func TestStatusComparesTheFilesWithTheHistory(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "\uFEFFCREATE TABLE notes (id int);\r\n") + writeMigrationFile(t, "002_add_pinned.sql", "ALTER TABLE notes ADD COLUMN pinned boolean; -- edited") + writeMigrationFile(t, "003_add_tags.sql", "CREATE TABLE tags (id int);") + fake := newFakeMigrationAPI(t) + fake.history = map[string]any{ + "history": []any{ + historyEntry("000_old.sql", "sha256:00", "applied"), + historyEntry("001_create_notes.sql", checksumOf("CREATE TABLE notes (id int);\n"), "applied"), + historyEntry("002_add_pinned.sql", checksumOf("ALTER TABLE notes ADD COLUMN pinned boolean;"), "baselined"), + }, + "active_run_id": migrationRunID, + } + server := fake.start() + + out, err := executeMigrationCommand(t, New(fake.deps(server)), "status", "-d", "app") + require.NoError(t, err, out) + for _, want := range []string{ + "000_old.sql", "missing", + "001_create_notes.sql", "applied", + "002_add_pinned.sql", "changed", + "003_add_tags.sql", "pending", + "1 applied, 1 pending, 1 changed, 1 missing", + "Run " + migrationRunID + " is queued or running. Follow it with: volcano databases migration runs get " + migrationRunID + " -d app", + "A run refuses an applied file whose content changed", + "Apply the pending files with: volcano databases migration apply -d app", + } { + assert.Contains(t, out, want) + } + assert.NotContains(t, out, "If they already ran without tracking") + assert.Empty(t, fake.recorded()) + + out, err = executeMigrationCommand(t, New(fake.deps(server)), "status", "-d", "app", "--json") + require.NoError(t, err, out) + var status struct { + Database string `json:"database"` + ActiveRunID string `json:"active_run_id"` + Files []struct { + Name string `json:"name"` + State string `json:"state"` + } `json:"files"` + } + require.NoError(t, json.Unmarshal([]byte(out), &status), out) + assert.Equal(t, "app", status.Database) + assert.Equal(t, migrationRunID, status.ActiveRunID) + var states []string + for _, file := range status.Files { + states = append(states, file.Name+"="+file.State) + } + assert.Equal(t, []string{"000_old.sql=missing", "001_create_notes.sql=applied", "002_add_pinned.sql=changed", "003_add_tags.sql=pending"}, states) +} + +// An empty history with files to apply is the case of a database migrated +// without tracking, so status offers the baseline too. +func TestStatusOffersABaselineForAnUntrackedDatabase(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);") + fake := newFakeMigrationAPI(t) + server := fake.start() + + out, err := executeMigrationCommand(t, New(fake.deps(server)), "status", "-d", "app") + require.NoError(t, err, out) + assert.Contains(t, out, "If they already ran without tracking, record them instead with: volcano databases migration baseline --all -d app") +} + +func TestStatusWithoutFilesShowsTheHistory(t *testing.T) { + setTrackedMigrationTest(t) + fake := newFakeMigrationAPI(t) + fake.history = map[string]any{"history": []any{historyEntry("001_create_notes.sql", "sha256:00", "applied")}} + server := fake.start() + + out, err := executeMigrationCommand(t, New(fake.deps(server)), "status", "-d", "app") + require.NoError(t, err, out) + assert.Contains(t, out, "No migration files for database 'app' in volcano/migrations; showing its history only.") + assert.Contains(t, out, "001_create_notes.sql") + assert.NotContains(t, out, "missing") +} + +func TestBaselineRecordsTheNamedFilesAfterConfirming(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);") + writeMigrationFile(t, "002_add_pinned.sql", "ALTER TABLE notes ADD COLUMN pinned boolean;") + writeMigrationFile(t, "003_add_tags.sql", "CREATE TABLE tags (id int);") + fake := newFakeMigrationAPI(t) + fake.baselineBody = map[string]any{"recorded": []string{"001_create_notes.sql", "002_add_pinned.sql"}} + server := fake.start() + + cmd := New(fake.deps(server)) + cmd.SetIn(bytes.NewBufferString("yes\n")) + out, err := executeMigrationCommand(t, cmd, "baseline", "-d", "app", "-f", "001_create_notes", "-f", "volcano/migrations/002_add_pinned.sql") + require.NoError(t, err, out) + + assert.Equal(t, [][]string{{"001_create_notes.sql", "002_add_pinned.sql"}}, fake.sentNames()) + assert.Contains(t, out, "A baseline records these files in the migration history of database 'app' as applied, without running them.") + assert.Contains(t, out, "Record 2 file(s) as applied?") + assert.Contains(t, out, "Recorded 2 file(s) as applied on database 'app', without running them") +} + +func TestBaselineDeclinedRecordsNothing(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);") + fake := newFakeMigrationAPI(t) + server := fake.start() + + cmd := New(fake.deps(server)) + cmd.SetIn(bytes.NewBufferString("no\n")) + _, err := executeMigrationCommand(t, cmd, "baseline", "-d", "app", "--all") + require.NoError(t, err) + assert.Empty(t, fake.recorded()) +} + +func TestBaselineRefusesWhatItCannotRecord(t *testing.T) { + for _, tc := range []struct { + name string + args []string + setup func(*fakeMigrationAPI) + want string + status int + }{ + { + name: "needs all or file", + args: []string{"baseline", "-d", "app", "--yes"}, + want: "at least one of the flags in the group [all file] is required", + }, + { + name: "unknown file", + args: []string{"baseline", "-d", "app", "-f", "009_missing", "--yes"}, + want: "migration \"009_missing\" not found in volcano/migrations/\navailable migrations: 001_create_notes.sql", + }, + { + name: "unavailable", + args: []string{"baseline", "-d", "app", "--all", "--yes"}, + setup: func(f *fakeMigrationAPI) { f.unavailable = true }, + want: `tracked migration runs aren't available on this Volcano API (capability "databases.migrations"), ` + + "so there is no migration history to record a baseline in", + }, + { + name: "checksum mismatch", + args: []string{"baseline", "-d", "app", "--all", "--yes"}, + setup: func(f *fakeMigrationAPI) { + f.baselineStatus = http.StatusConflict + f.baselineBody = map[string]any{"error": "001_create_notes.sql is recorded with a different checksum", "code": "checksum_mismatch"} + }, + want: "001_create_notes.sql is recorded with a different checksum", + status: http.StatusConflict, + }, + } { + t.Run(tc.name, func(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);") + fake := newFakeMigrationAPI(t) + if tc.setup != nil { + tc.setup(fake) + } + server := fake.start() + + _, err := executeMigrationCommand(t, New(fake.deps(server)), tc.args...) + require.ErrorContains(t, err, tc.want) + if tc.status != 0 { + assert.Equal(t, tc.status, api.Status(err)) + } else { + assert.Empty(t, fake.recorded()) + } + }) + } +} + +func TestRunsListAndGet(t *testing.T) { + setTrackedMigrationTest(t) + fake := newFakeMigrationAPI(t) + failed := migrationRun("failed", "apply", []string{"001_create_notes.sql:applied", "002_add_pinned.sql:failed", "003_add_tags.sql:not_run"}, map[string]any{ + "source": "git_deploy", + "commit_sha": "4f2a9c1e8b7d6a5f4e3d2c1b0a9f8e7d6c5b4a39", + "error": map[string]any{"code": "migration_failed", "message": "migration 002_add_pinned.sql failed at line 1 (SQLSTATE 42701)"}, + }) + files := failed["files"].([]any) + files[1].(map[string]any)["error"] = map[string]any{"message": "column \"pinned\" already exists\x1b[31m", "sqlstate": "42701"} + fake.runs = []map[string]any{failed} + listed := migrationRun("failed", "apply", nil, map[string]any{"error": failed["error"]}) + fake.listedRuns = map[string]any{"data": []any{listed}} + server := fake.start() + + out, err := executeMigrationCommand(t, New(fake.deps(server)), "runs", "list", "-d", "app") + require.NoError(t, err, out) + assert.Contains(t, out, migrationRunID) + assert.Contains(t, out, "migration_failed") + assert.Contains(t, out, "Showing 1 migration run(s) of database 'app'") + + out, err = executeMigrationCommand(t, New(fake.deps(server)), "runs", "get", migrationRunID, "-d", "app") + require.NoError(t, err, out) + for _, want := range []string{ + "ID: " + migrationRunID, + "Status: failed", + "Phase: apply", + "Source: git_deploy 4f2a9c1e8b7d", + "Error: migration_failed: migration 002_add_pinned.sql failed at line 1 (SQLSTATE 42701)", + `column "pinned" already exists[31m (SQLSTATE 42701)`, + "not run", + } { + assert.Contains(t, out, want) + } + assert.NotContains(t, out, "\x1b", "the database's text cannot reach the terminal as an escape sequence") + + _, err = executeMigrationCommand(t, New(fake.deps(server)), "runs", "get", "not-a-uuid", "-d", "app") + require.EqualError(t, err, `run ID "not-a-uuid" is not a UUID`) +} + +// A run that ended while taking its backup may name one that was never +// created, so the CLI does not claim it was taken. +func TestApplyDoesNotClaimABackupItMayNotHaveTaken(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);") + backup := "before-migration-" + migrationRunID + failed := migrationRun("failed", "backup", []string{"001_create_notes.sql:not_run"}, map[string]any{ + "backup_name": backup, + "error": map[string]any{"code": "backup_failed", "message": "the backup did not finish"}, + }) + fake := newFakeMigrationAPI(t) + fake.runs = []map[string]any{failed} + server := fake.start() + + out, err := executeMigrationCommand(t, New(fake.deps(server)), "apply", "-d", "app") + require.Error(t, err) + assert.Contains(t, out, "Backup "+backup+" may not exist: the run ended while taking it") + assert.NotContains(t, out, "Backup taken first") + + out, err = executeMigrationCommand(t, New(fake.deps(server)), "runs", "get", migrationRunID, "-d", "app") + require.NoError(t, err, out) + assert.Contains(t, out, "Backup: "+backup+" (may not exist: the run ended while taking it)") +} + +// The API takes only a lowercase .sql name, so the tracked commands refuse +// another casing before sending anything and say what to rename. +func TestTrackedCommandsRefuseAnUppercaseExtension(t *testing.T) { + for _, args := range [][]string{ + {"apply", "-d", "app"}, + {"baseline", "-d", "app", "--all", "--yes"}, + } { + t.Run(args[0], func(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.SQL", "CREATE TABLE notes (id int);") + fake := newFakeMigrationAPI(t) + server := fake.start() + + _, err := executeMigrationCommand(t, New(fake.deps(server)), args...) + require.EqualError(t, err, "volcano/migrations/001_create_notes.SQL: tracked migration runs take only file names "+ + "ending in lowercase .sql; rename it to 001_create_notes.sql") + assert.Empty(t, fake.recorded()) + assert.Zero(t, fake.capReads) + }) + } +} + +// With --json, the confirmation goes to stderr so stdout holds only the +// JSON document. +func TestBaselineJSONKeepsThePromptOffStdout(t *testing.T) { + setTrackedMigrationTest(t) + writeMigrationFile(t, "001_create_notes.sql", "CREATE TABLE notes (id int);") + fake := newFakeMigrationAPI(t) + fake.baselineBody = map[string]any{"recorded": []string{"001_create_notes.sql"}} + server := fake.start() + + cmd := New(fake.deps(server)) + var stdout, stderr bytes.Buffer + cmd.SetIn(bytes.NewBufferString("yes\n")) + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{"baseline", "-d", "app", "--all", "--json"}) + require.NoError(t, cmd.Execute(), stderr.String()) + + var baseline struct { + Recorded []string `json:"recorded"` + } + require.NoError(t, json.Unmarshal(stdout.Bytes(), &baseline), stdout.String()) + assert.Equal(t, []string{"001_create_notes.sql"}, baseline.Recorded) + assert.Contains(t, stderr.String(), "Record 1 file(s) as applied?") +} + +// checksumOf is the checksum the history records for text that is already +// normalized. +func checksumOf(text string) string { + sum := sha256.Sum256([]byte(text)) + return "sha256:" + hex.EncodeToString(sum[:]) +} + +func historyEntry(name, checksum, status string) map[string]any { + return map[string]any{"name": name, "checksum": checksum, "status": status, "applied_at": "2026-10-29T09:00:00Z"} +} diff --git a/internal/cmd/deploy/deploy.go b/internal/cmd/deploy/deploy.go index 7902d39c..fa92ad55 100644 --- a/internal/cmd/deploy/deploy.go +++ b/internal/cmd/deploy/deploy.go @@ -6,12 +6,12 @@ import ( "context" "fmt" "io" - "slices" "strings" "time" "github.com/spf13/cobra" + clidatabase "github.com/Kong/volcano-cli/internal/database" "github.com/Kong/volcano-cli/internal/output" cliruntime "github.com/Kong/volcano-cli/internal/runtime" "github.com/Kong/volcano-cli/internal/theme" @@ -43,7 +43,7 @@ func NewLocal(deps cliruntime.Deps) *cobra.Command { func newCommand(deps cliruntime.Deps, short string) *cobra.Command { opts := options{} - databaseHelp := "Database to apply migrations to (default: the one the manifests declare)" + databaseHelp := "Database to apply migrations to (default: each one with a directory in volcano/migrations, else the one the manifests declare)" deployExample := "deploy --env-file volcano/cloud.env" migrateExample := "deploy --env-file volcano/cloud.env --with-migrations -d app" manifestsExample := "deploy --env-file volcano/staging.env -c volcano/volcano-config.yaml -c volcano/auth.yaml" @@ -53,7 +53,7 @@ volcano/volcano.env holds the local server's values. Name the cloud project's env file with --env-file; one holding the local server's keys or a localhost VOLCANO_API_URL is refused.` if deps.LocalMode { - databaseHelp = "Database to apply migrations to (default: " + defaultLocalDatabase + ")" + databaseHelp = "Database to apply migrations to (default: each one with a directory in volcano/migrations, else " + defaultLocalDatabase + ")" deployExample = "deploy" migrateExample = "deploy --with-migrations" manifestsExample = "deploy -c volcano/volcano-config.yaml -c volcano/auth.local.yaml" @@ -66,7 +66,7 @@ VOLCANO_API_URL is refused.` needs: 1. variables volcano/volcano.env, or the variable sections of the manifests - 2. migrations volcano/migrations/*.sql, only with --with-migrations + 2. migrations volcano/migrations, only with --with-migrations 3. functions the standard functions in volcano/functions 4. durable the functions the first manifest declares durable 5. frontends the frontends listed in .volcano/source-layout.json @@ -89,14 +89,24 @@ Variables come from the env file or from a manifest's variables section, never both: applying that section deletes every variable it does not list. Pass --no-env-file when a manifest holds them.%s -Migrations run without tracking, so every file runs again on each ---with-migrations run. Write them to be safe to re-run. +--with-migrations applies the migration files in tracked runs, so each +database records the files it applied and a file runs once. The deploy waits +for the runs even with --no-wait. With a directory per database in +volcano/migrations, it migrates every database that has one, unless -d names +one. Record the files of a database migrated before without tracking first: + + %s + +Tracked runs need the %q capability; where the API does +not offer it, the deploy refuses before anything changes. Usage: %s %s %s`, envNote, + cliruntime.CommandPath(deps, "databases migration baseline -d app --all"), + clidatabase.MigrationsCapability, cliruntime.CommandPath(deps, deployExample), cliruntime.CommandPath(deps, migrateExample), cliruntime.CommandPath(deps, manifestsExample)), @@ -111,7 +121,7 @@ Usage: return runDeploy(cmd.Context(), run) }, } - cmd.Flags().BoolVar(&opts.withMigrations, "with-migrations", false, "Apply every migration in volcano/migrations") + cmd.Flags().BoolVar(&opts.withMigrations, "with-migrations", false, "Apply volcano/migrations in tracked migration runs") cmd.Flags().StringVarP(&opts.database, "database", "d", "", databaseHelp) cmd.Flags().StringVar(&opts.envFile, "env-file", "", "Env file to deploy (default: volcano/volcano.env or ./volcano.env)") cmd.Flags().BoolVar(&opts.noEnvFile, "no-env-file", false, "Deploy no env file, even when volcano/volcano.env exists") @@ -172,9 +182,6 @@ func resumeHint(done []string, rest []step) string { fmt.Fprintf(&b, "Already deployed: %s.\n", strings.Join(done, ", ")) } b.WriteString("Fix the cause and run the command again") - if slices.Contains(done, stepMigrations) { - b.WriteString(" without --with-migrations and -d, since the migrations already ran") - } var manual []string for _, s := range rest { if s.skipped { diff --git a/internal/cmd/deploy/deploy_test.go b/internal/cmd/deploy/deploy_test.go index 3d6d926c..06e8c3e0 100644 --- a/internal/cmd/deploy/deploy_test.go +++ b/internal/cmd/deploy/deploy_test.go @@ -363,6 +363,25 @@ databases: args: []string{"--with-migrations"}, want: []string{"the manifests declare 2 databases (app, analytics)"}, }, + { + name: "migrations in a mixed layout", + setup: func(t *testing.T) { + t.Helper() + writeDeployProjectFile(t, "volcano/migrations/001_init.sql", "select 1;") + writeDeployProjectFile(t, "volcano/migrations/app/002_more.sql", "select 2;") + }, + args: []string{"--with-migrations", "-d", "app"}, + want: []string{"volcano/migrations mixes top-level files, such as 001_init.sql, with database directories, such as app/"}, + }, + { + name: "migration with an uppercase extension", + setup: func(t *testing.T) { + t.Helper() + writeDeployProjectFile(t, "volcano/migrations/001_init.SQL", "select 1;") + }, + args: []string{"--with-migrations", "-d", "app"}, + want: []string{"volcano/migrations/001_init.SQL: tracked migration runs take only file names ending in lowercase .sql; rename it to 001_init.sql"}, + }, { name: "database without migrations", args: []string{"-d", "app"}, @@ -571,15 +590,16 @@ func TestDeployWithoutFunctionsIgnoresTheDefaultManifest(t *testing.T) { assert.Equal(t, []string{"config"}, fake.recorded()) } -// After the migrations ran, the hint says to leave them out of the next run. -func TestDeployResumeHintLeavesOutMigrationsThatRan(t *testing.T) { +// A tracked run skips the files that already ran, so running the whole +// command again is safe after the migrations step. +func TestDeployResumeHintAfterMigrations(t *testing.T) { hint := resumeHint([]string{stepVariables, stepMigrations}, []step{ {name: stepFunctions, manual: []string{"volcano functions deploy --all"}}, {name: stepDurable, skipped: true, manual: []string{"volcano durable deploy --all"}}, }) assert.Equal(t, "Already deployed: variables, migrations.\n"+ - "Fix the cause and run the command again without --with-migrations and -d, since the migrations already ran, or finish by hand:\n"+ + "Fix the cause and run the command again, or finish by hand:\n"+ " volcano functions deploy --all", hint) } @@ -1007,16 +1027,111 @@ func TestLocalDeployMigratesTheAppDatabase(t *testing.T) { writeDeployProjectFile(t, "volcano/functions/hello.js", `exports.handler = async () => ({});`) fake := newFakeVolcano(t) server := fake.start() + deps := fake.localDeps(server) + deps.NewTicker = func(time.Duration) cliruntime.Ticker { return tickNow{} } - out, err := executeDeployCommand(t, NewLocal(fake.localDeps(server)), "--with-migrations") - require.Error(t, err) + out, err := executeDeployCommand(t, NewLocal(deps), "--with-migrations") + require.NoError(t, err, out) assert.Contains(t, out, "2. migrations volcano/migrations -> database app") + assert.Contains(t, out, "Starting a migration run of 1 file(s) from volcano/migrations on database 'app'") + assert.Contains(t, out, "succeeded: 1 applied, 0 already applied") + assert.Equal(t, []string{"migrations app", "function hello"}, fake.recorded()) + assert.Equal(t, []string{"001_init.sql"}, fake.migrationFiles["app"]) +} + +// With a directory per database, --with-migrations migrates every database +// that has one, each with its own directory's files. +func TestDeployMigratesEveryDatabaseDirectory(t *testing.T) { + setDeployCommandTestHome(t) + writeDeployProjectFile(t, "volcano/migrations/main_db/001_notes.sql", "create table notes (id int);") + writeDeployProjectFile(t, "volcano/migrations/analytics/001_events.sql", "create table events (id int);") + fake := newFakeVolcano(t) + server := fake.start() + deps := fake.cloudDeps(server) + deps.NewTicker = func(time.Duration) cliruntime.Ticker { return tickNow{} } + + out, err := executeDeployCommand(t, New(deps), "--with-migrations", "--no-wait") + require.NoError(t, err, out) + + assert.Contains(t, out, "2. migrations volcano/migrations/analytics -> database analytics, volcano/migrations/main_db -> database main_db") + assert.Equal(t, []string{"migrations analytics", "migrations main_db"}, fake.recorded()) + assert.Equal(t, []string{"main_db/001_notes.sql"}, fake.migrationFiles["main_db"]) + assert.Equal(t, []string{"analytics/001_events.sql"}, fake.migrationFiles["analytics"]) +} + +// Where the API takes no tracked runs, the deploy refuses before it changes +// anything and names the untracked commands instead. +func TestDeployRefusesMigrationsWithoutTrackedRuns(t *testing.T) { + setDeployCommandTestHome(t) + writeFullProject(t) + writeDeployProjectFile(t, "volcano/migrations/001_init.sql", "create table notes (id int);") + fake := newFakeVolcano(t) + fake.migrationsUnavailable = true + server := fake.start() + + out, err := executeDeployCommand(t, New(fake.cloudDeps(server)), "--with-migrations", "-d", "app") + require.Error(t, err) + + assert.Equal(t, "--with-migrations applies migrations in tracked runs, which aren't available on this Volcano API "+ + `(capability "databases.migrations"). Nothing was deployed.`+"\n"+ + "To run the files without tracking, which runs every file again, use:\n"+ + " volcano cloud databases migration up --all -d app\n"+ + "then deploy without --with-migrations", err.Error()) + assert.NotContains(t, out, "Deploying to") assert.Empty(t, fake.recorded()) - require.ErrorContains(t, err, `migrations: database "app" is not active (status: provisioning)`) - assert.ErrorContains(t, err, `Fix the cause and run the command again, or finish by hand: - volcano migrations deploy --all -d app - volcano functions deploy --all`) +} + +// A project without migration files has nothing for --with-migrations to +// apply, so the step is skipped and nothing asks whether runs are available. +func TestDeployWithMigrationsSkipsAProjectWithoutThem(t *testing.T) { + setDeployCommandTestHome(t) + writeDeployProjectFile(t, "volcano/volcano.env", "API_URL=https://api.example.test\n") + fake := newFakeVolcano(t) + server := fake.start() + + out, err := executeDeployCommand(t, New(fake.cloudDeps(server)), "--with-migrations", "-d", "app") + require.NoError(t, err, out) + + assert.Contains(t, out, "2. migrations skipped: no migration files in volcano/migrations") + assert.NotContains(t, fake.reads, "capabilities") +} + +// In a per-database layout, -d naming a database without a directory skips +// the step and names the directory it looked in. +func TestDeployWithMigrationsNamesTheDirectoryItFoundEmpty(t *testing.T) { + setDeployCommandTestHome(t) + writeDeployProjectFile(t, "volcano/volcano.env", "API_URL=https://api.example.test\n") + writeDeployProjectFile(t, "volcano/migrations/main_db/001_notes.sql", "create table notes (id int);") + fake := newFakeVolcano(t) + server := fake.start() + + out, err := executeDeployCommand(t, New(fake.cloudDeps(server)), "--with-migrations", "-d", "analytics") + require.NoError(t, err, out) + + assert.Contains(t, out, "2. migrations skipped: no migration files in volcano/migrations/analytics") + assert.Empty(t, fake.migrationFiles) +} + +func TestDeployStopsAtAFailedMigrationRun(t *testing.T) { + setDeployCommandTestHome(t) + writeFullProject(t) + writeDeployProjectFile(t, "volcano/migrations/001_init.sql", "create table notes (id int);") + fake := newFakeVolcano(t) + fake.failMigrations = map[string]bool{"app": true} + server := fake.start() + deps := fake.cloudDeps(server) + deps.NewTicker = func(time.Duration) cliruntime.Ticker { return tickNow{} } + + out, err := executeDeployCommand(t, New(deps), "--with-migrations", "-d", "app") + require.Error(t, err) + + assert.Contains(t, out, "[2/6] migrations") + require.ErrorContains(t, err, "failed in apply (migration_failed): a migration failed") + require.ErrorContains(t, err, "Already deployed: variables.\n"+ + "Fix the cause and run the command again, or finish by hand:\n"+ + " volcano cloud databases migration apply -d app\n") + assert.Equal(t, []string{"variable API_URL", "migrations app"}, fake.recorded()) } func TestFrontendSelectionComesFromTheFirstManifest(t *testing.T) { diff --git a/internal/cmd/deploy/helpers_test.go b/internal/cmd/deploy/helpers_test.go index cb2381b6..f3d8bd12 100644 --- a/internal/cmd/deploy/helpers_test.go +++ b/internal/cmd/deploy/helpers_test.go @@ -128,6 +128,13 @@ type fakeVolcano struct { reads []string // durableReads counts the reads of each durable function. durableReads map[string]int + // migrationsUnavailable leaves databases.migrations out of the capability + // list. + migrationsUnavailable bool + // failMigrations names databases whose migration run fails. + failMigrations map[string]bool + // migrationFiles lists the files each migration run carried, by database. + migrationFiles map[string][]string // functionReads counts the reads of each function. functionReads map[string]int } @@ -135,15 +142,16 @@ type fakeVolcano struct { func newFakeVolcano(t *testing.T) *fakeVolcano { t.Helper() return &fakeVolcano{ - t: t, - sourceMode: "platform", - frontends: map[string]map[string][]string{}, - archives: map[string][]string{}, - deployments: map[string]uuid.UUID{}, - successors: map[string]uuid.UUID{}, - resources: map[string]uuid.UUID{}, - durableReads: map[string]int{}, - functionReads: map[string]int{}, + t: t, + sourceMode: "platform", + frontends: map[string]map[string][]string{}, + archives: map[string][]string{}, + deployments: map[string]uuid.UUID{}, + successors: map[string]uuid.UUID{}, + resources: map[string]uuid.UUID{}, + durableReads: map[string]int{}, + migrationFiles: map[string][]string{}, + functionReads: map[string]int{}, } } @@ -298,6 +306,33 @@ func (f *fakeVolcano) serve(w http.ResponseWriter, r *http.Request) { return } http.NotFound(w, r) + case r.Method == http.MethodGet && path == "/capabilities": + f.reads = append(f.reads, "capabilities") + status := "available" + if f.migrationsUnavailable { + status = "unavailable" + } + f.writeJSON(w, http.StatusOK, map[string]any{"data": []any{map[string]any{"id": "databases.migrations", "status": status}}}) + case r.Method == http.MethodPost && strings.HasPrefix(path, project+"/databases/") && strings.HasSuffix(path, "/migration-runs"): + database := strings.TrimSuffix(strings.TrimPrefix(path, project+"/databases/"), "/migration-runs") + var body struct { + Files []struct { + Name string `json:"name"` + } `json:"files"` + } + require.NoError(f.t, json.NewDecoder(r.Body).Decode(&body)) + f.calls = append(f.calls, "migrations "+database) + for _, file := range body.Files { + f.migrationFiles[database] = append(f.migrationFiles[database], file.Name) + } + f.writeJSON(w, http.StatusAccepted, f.migrationRun(database, "queued")) + case r.Method == http.MethodGet && strings.Contains(path, "/migration-runs/"): + database, _, _ := strings.Cut(strings.TrimPrefix(path, project+"/databases/"), "/") + status := "succeeded" + if f.failMigrations[database] { + status = "failed" + } + f.writeJSON(w, http.StatusOK, f.migrationRun(database, status)) case r.Method == http.MethodGet && strings.HasPrefix(path, project+"/functions/"): id := strings.TrimPrefix(path, project+"/functions/") for name, resource := range f.resources { @@ -332,6 +367,33 @@ func (f *fakeVolcano) serve(w http.ResponseWriter, r *http.Request) { } } +// migrationRun is the run a database's migration run reads as. +func (f *fakeVolcano) migrationRun(database, status string) map[string]any { + run := map[string]any{ + "id": uuid.NewSHA1(uuid.NameSpaceURL, []byte(database)).String(), + "database": map[string]any{"type": "database", "id": uuid.NewString(), "name": database}, + "status": status, + "phase": "apply", + "source": "api", + "warnings": []any{}, + "created_at": "2026-10-01T00:00:00Z", + "updated_at": "2026-10-01T00:00:00Z", + } + var files []any + for _, name := range f.migrationFiles[database] { + outcome := "applied" + if status == "failed" { + outcome = "failed" + } + files = append(files, map[string]any{"name": name, "outcome": outcome}) + } + run["files"] = files + if status == "failed" { + run["error"] = map[string]any{"code": "migration_failed", "message": "a migration failed"} + } + return run +} + // deployed returns the resource payload for a deploy of name, with a new // current deployment. func (f *fakeVolcano) deployed(name string) map[string]any { diff --git a/internal/cmd/deploy/plan.go b/internal/cmd/deploy/plan.go index d493bec0..7237778e 100644 --- a/internal/cmd/deploy/plan.go +++ b/internal/cmd/deploy/plan.go @@ -13,6 +13,7 @@ import ( "slices" "strings" + "github.com/Kong/volcano-cli/internal/api" "github.com/Kong/volcano-cli/internal/apiclient" configcmd "github.com/Kong/volcano-cli/internal/cmd/config" migrationcmd "github.com/Kong/volcano-cli/internal/cmd/databases/migration" @@ -20,6 +21,7 @@ import ( frontendscmd "github.com/Kong/volcano-cli/internal/cmd/frontends" functionscmd "github.com/Kong/volcano-cli/internal/cmd/functions" variablescmd "github.com/Kong/volcano-cli/internal/cmd/variables" + clidatabase "github.com/Kong/volcano-cli/internal/database" "github.com/Kong/volcano-cli/internal/envfile" clifrontend "github.com/Kong/volcano-cli/internal/frontend" "github.com/Kong/volcano-cli/internal/output" @@ -32,7 +34,7 @@ import ( const ( functionsDir = "volcano/functions" - migrationsDir = "volcano/migrations" + migrationsDir = clidatabase.MigrationsDir defaultLocalDatabase = "app" ) @@ -106,7 +108,7 @@ func prepare(ctx context.Context, opts options) (*plan, error) { if err != nil { return nil, err } - database, err := migrationDatabase(opts, manifests) + migrations, err := migrationTargets(opts, manifests) if err != nil { return nil, err } @@ -116,6 +118,11 @@ func prepare(ctx context.Context, opts options) (*plan, error) { if err != nil { return nil, err } + if hasMigrationFiles(migrations) { + if err := requireMigrationRuns(ctx, opts.deps, migrations); err != nil { + return nil, err + } + } deploysCode := hasFunctions || len(durable) > 0 || len(frontends) > 0 if !opts.deps.LocalMode && deploysCode { mode, err := projects.SourceMode(ctx) @@ -138,7 +145,7 @@ func prepare(ctx context.Context, opts options) (*plan, error) { target: target, steps: []step{ b.variables(envFile), - b.migrations(opts.withMigrations, database), + b.migrations(opts.withMigrations, migrations), b.functions(hasFunctions, primaryPath, len(durable) > 0), b.durable(durable, primaryPath), frontendsStep, @@ -478,16 +485,53 @@ func frontendPath(fe clifrontend.LayoutFrontend) string { return filepath.FromSlash(fe.Dir()) } -// migrationDatabase picks the database --with-migrations applies to. Local -// projects have one named app; a cloud project's comes from -d, or from the -// manifests when they declare exactly one. -func migrationDatabase(opts options, manifests []manifestFile) (string, error) { +// migrationTarget is a database --with-migrations applies files to. +type migrationTarget struct { + database string + location string + files []clidatabase.MigrationFile +} + +// migrationTargets picks the databases --with-migrations applies to and their +// files. -d names one. Otherwise, with a directory per database, every +// directory's database; with the files in volcano/migrations itself, a local +// project's database named app, or the one database the manifests declare. +func migrationTargets(opts options, manifests []manifestFile) ([]migrationTarget, error) { if !opts.withMigrations && opts.database != "" { - return "", fmt.Errorf("--database only picks where migrations go; add --with-migrations to apply them to %q", opts.database) + return nil, fmt.Errorf("--database only picks where migrations go; add --with-migrations to apply them to %q", opts.database) + } + if !opts.withMigrations { + return nil, nil + } + migrations, err := clidatabase.ReadMigrations(migrationsDir) + if err != nil { + return nil, err } - if !opts.withMigrations || opts.database != "" { - return opts.database, nil + var databases []string + switch { + case opts.database != "": + databases = []string{opts.database} + case migrations.Nested(): + databases = migrations.Databases() + default: + database, err := defaultMigrationDatabase(opts, manifests) + if err != nil { + return nil, err + } + databases = []string{database} } + targets := make([]migrationTarget, len(databases)) + for i, database := range databases { + files := migrations.For(database) + if err := clidatabase.CheckTrackedNames(files); err != nil { + return nil, err + } + targets[i] = migrationTarget{database: database, location: migrations.Location(database), files: files} + } + return targets, nil +} + +func defaultMigrationDatabase(opts options, manifests []manifestFile) (string, error) { if opts.deps.LocalMode { return defaultLocalDatabase, nil } @@ -512,6 +556,30 @@ func migrationDatabase(opts options, manifests []manifestFile) (string, error) { } } +// requireMigrationRuns refuses the whole deploy, before anything changes, +// where the API takes no tracked migration runs, and names the commands that +// run the files without tracking instead. +func requireMigrationRuns(ctx context.Context, deps cliruntime.Deps, targets []migrationTarget) error { + err := clidatabase.NewService(deps).RequireMigrationRuns(ctx) + if !errors.Is(err, clidatabase.ErrMigrationRunsUnavailable) { + return err + } + var untracked []string + for _, target := range targets { + if len(target.files) > 0 { + untracked = append(untracked, cliruntime.CommandPath(deps, "databases migration up --all -d "+cliruntime.ShellQuote(target.database))) + } + } + return api.WithCause(fmt.Errorf("--with-migrations applies migrations in tracked runs, which aren't available on this Volcano API "+ + "(capability %q). Nothing was deployed.\n"+ + "To run the files without tracking, which runs every file again, use:\n %s\nthen deploy without --with-migrations", + clidatabase.MigrationsCapability, strings.Join(untracked, "\n ")), err) +} + +func hasMigrationFiles(targets []migrationTarget) bool { + return slices.ContainsFunc(targets, func(t migrationTarget) bool { return len(t.files) > 0 }) +} + func isDir(path string) (bool, error) { info, err := os.Stat(path) if errors.Is(err, fs.ErrNotExist) { @@ -583,21 +651,44 @@ func (b planBuilder) variables(envFile string) step { return s } -func (b planBuilder) migrations(enabled bool, database string) step { +func (b planBuilder) migrations(enabled bool, targets []migrationTarget) step { s := step{name: stepMigrations} if !enabled { s.skipped = true s.detail = "pass --with-migrations to apply " + migrationsDir return s } - s.detail = migrationsDir + " -> database " + database - if b.deps.LocalMode { - s.manual = []string{b.command("migrations deploy --all -d " + database)} - } else { - s.manual = []string{b.command("databases migration up --all -d " + database)} + var details []string + var apply []migrationTarget + for _, target := range targets { + if len(target.files) > 0 { + details = append(details, target.location+" -> database "+target.database) + apply = append(apply, target) + s.manual = append(s.manual, b.command("databases migration apply -d "+cliruntime.ShellQuote(target.database))) + } + } + if len(apply) == 0 { + s.skipped = true + s.detail = "no migration files in " + migrationsDir + if len(targets) == 1 { + s.detail = "no migration files in " + targets[0].location + } + return s } + s.detail = strings.Join(details, ", ") s.run = func(ctx context.Context, r *runner) error { - return migrationcmd.ApplyAll(ctx, r.opts.deps, database, r.opts.out) + for _, target := range apply { + if _, err := migrationcmd.Apply(ctx, r.opts.deps, migrationcmd.ApplyOptions{ + Database: target.database, + Files: target.files, + Location: target.location, + Wait: true, + Out: r.opts.out, + }); err != nil { + return err + } + } + return nil } return s } diff --git a/internal/cmd/local/reset.go b/internal/cmd/local/reset.go index 2d6d9c54..8ba3f4fd 100644 --- a/internal/cmd/local/reset.go +++ b/internal/cmd/local/reset.go @@ -35,7 +35,7 @@ This command: 4. Keeps containers running After reset, redeploy your project migrations with: - volcano migrations deploy --all -d app`, + volcano databases migration apply -d app`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { return runReset(cmd.Context(), resetOptions{ diff --git a/internal/cmd/local/reset_test.go b/internal/cmd/local/reset_test.go index 71ffb62b..82e18b7d 100644 --- a/internal/cmd/local/reset_test.go +++ b/internal/cmd/local/reset_test.go @@ -43,7 +43,7 @@ func TestLocalResetCommandInvokesServerEntrypoint(t *testing.T) { require.NoError(t, err) assert.Contains(t, out, "Local reset complete.") - assert.Contains(t, out, "volcano migrations deploy --all -d app") + assert.Contains(t, out, "volcano databases migration apply -d app") assert.True(t, slices.ContainsFunc(commands, func(command []string) bool { return slices.Equal(command, []string{"exec", "volcano-server", "/app/volcano-hosting", "local", "reset", "--yes", "--format", "text"}) })) diff --git a/internal/cmd/root/root_test.go b/internal/cmd/root/root_test.go index bb9cd428..128123f4 100644 --- a/internal/cmd/root/root_test.go +++ b/internal/cmd/root/root_test.go @@ -203,6 +203,19 @@ func TestDatabasesHelpIncludesMigration(t *testing.T) { assert.Contains(t, out, "migration") } +func TestDatabaseMigrationHelpNamesTheTrackedCommands(t *testing.T) { + for _, args := range [][]string{{"databases", "migration", "--help"}, {"cloud", "databases", "migration", "--help"}} { + out, err := executeRootCommand(t, args...) + require.NoError(t, err) + for _, command := range []string{"status", "apply", "baseline", "runs", "up"} { + assert.Contains(t, out, "\n "+command+" ", args) + } + } + out, err := executeRootCommand(t, "cloud", "databases", "migration", "apply", "--help") + require.NoError(t, err) + assert.Contains(t, out, "volcano cloud databases migration apply -d mydb") +} + func TestDatabaseMigrationsUpCommandPath(t *testing.T) { t.Chdir(t.TempDir()) @@ -339,7 +352,8 @@ func TestDeployHelpNamesTheTreeItRunsFrom(t *testing.T) { out, err := executeRootCommand(t, "deploy", "--help") require.NoError(t, err) assert.Contains(t, out, "volcano deploy --with-migrations\n") - assert.Contains(t, out, "Database to apply migrations to (default: app)") + assert.Contains(t, out, "Database to apply migrations to (default: each one with a directory in volcano/migrations, else app)") + assert.Contains(t, out, "volcano databases migration baseline -d app --all") assert.NotContains(t, out, "volcano cloud deploy") assert.NotContains(t, out, "cloud.env") @@ -347,7 +361,8 @@ func TestDeployHelpNamesTheTreeItRunsFrom(t *testing.T) { require.NoError(t, err) assert.Contains(t, out, "volcano cloud deploy --env-file volcano/cloud.env --with-migrations -d app") assert.Contains(t, out, "volcano/volcano.env holds the local server's values") - assert.Contains(t, out, "default: the one the manifests declare") + assert.Contains(t, out, "else the one the manifests declare") + assert.Contains(t, out, "volcano cloud databases migration baseline -d app --all") } func TestCloudFunctionPaginationUsesCloudPath(t *testing.T) { diff --git a/internal/database/database.go b/internal/database/database.go index ea4a9463..03dbef0b 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -12,12 +12,13 @@ import ( // Service performs authenticated Volcano database workflows. type Service struct { + deps cliruntime.Deps sessions clisession.Factory } // NewService returns a database service. func NewService(deps cliruntime.Deps) Service { - return Service{sessions: clisession.NewFactory(deps)} + return Service{deps: deps, sessions: clisession.NewFactory(deps)} } // Create creates a database in the current project. diff --git a/internal/database/migration.go b/internal/database/migration.go new file mode 100644 index 00000000..b5230cb9 --- /dev/null +++ b/internal/database/migration.go @@ -0,0 +1,206 @@ +package database + +import ( + "context" + "errors" + "fmt" + "net/http" + "time" + + "github.com/google/uuid" + + "github.com/Kong/volcano-cli/internal/api" + "github.com/Kong/volcano-cli/internal/apiclient" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +// MigrationsCapability is the capability GET /capabilities lists as available +// where migration runs and baselines are. +const MigrationsCapability = "databases.migrations" + +// ErrMigrationRunsUnavailable reports an API that takes no migration runs or +// baselines: one where Volcano has not turned them on, or a release older than +// them. Reading the history and runs works either way. +var ErrMigrationRunsUnavailable = errors.New("tracked migration runs aren't available on this Volcano API") + +const codeFeatureUnavailable = "feature_unavailable" + +const ( + migrationRunPollInterval = 2 * time.Second + // migrationRunWaitTimeout outlasts a run's one-hour limit and the retry + // backoff of its last phase, so a wait ends with the run's own outcome. + migrationRunWaitTimeout = 90 * time.Minute + // migrationRunPollFailures is how many reads in a row may fail before a + // wait gives up; the run carries on without the CLI. + migrationRunPollFailures = 5 +) + +// MigrationRunDone reports whether a run has ended. +func MigrationRunDone(run *apiclient.DatabaseMigrationRun) bool { + return run.Status == apiclient.DatabaseMigrationRunStatusSucceeded || run.Status == apiclient.DatabaseMigrationRunStatusFailed +} + +// RequireMigrationRuns returns ErrMigrationRunsUnavailable unless the API +// lists MigrationsCapability as available. Callers check it before they change +// anything, so an unavailable API refuses the whole command. +func (s Service) RequireMigrationRuns(ctx context.Context) error { + authenticated, err := s.sessions.Authenticated() + if err != nil { + return err + } + available, err := authenticated.API.CapabilityAvailable(ctx, MigrationsCapability) + if api.RouteMissing(err) { + // A release older than GET /capabilities is older than migration runs. + return ErrMigrationRunsUnavailable + } + if err != nil { + return fmt.Errorf("failed to read which capabilities the API offers: %w", err) + } + if !available { + return ErrMigrationRunsUnavailable + } + return nil +} + +// MigrationHistory returns the files a database's migration history records, +// and the run that is queued or running, if any. +func (s Service) MigrationHistory(ctx context.Context, databaseName string) (*apiclient.DatabaseMigrations, error) { + authenticated, err := s.sessions.CurrentProject() + if err != nil { + return nil, err + } + history, err := authenticated.API.GetDatabaseMigrations(ctx, authenticated.ProjectID, databaseName) + if err != nil { + return nil, fmt.Errorf("failed to read the migration history of database %q: %w", databaseName, err) + } + return history, nil +} + +// BaselineMigrations records files in a database's migration history as +// applied without running them. +func (s Service) BaselineMigrations(ctx context.Context, databaseName string, files []MigrationFile) (*apiclient.DatabaseMigrationBaseline, error) { + authenticated, err := s.sessions.CurrentProject() + if err != nil { + return nil, err + } + baseline, err := authenticated.API.BaselineDatabaseMigrations(ctx, authenticated.ProjectID, databaseName, requestFiles(files)) + if err != nil { + return nil, unavailable(fmt.Errorf("failed to record a baseline on database %q: %w", databaseName, err)) + } + return baseline, nil +} + +// StartMigrationRun queues a run of files on a database. The API applies only +// the files the history does not record, so sending every file is safe. +func (s Service) StartMigrationRun(ctx context.Context, databaseName string, files []MigrationFile) (*apiclient.DatabaseMigrationRun, error) { + authenticated, err := s.sessions.CurrentProject() + if err != nil { + return nil, err + } + // A fresh key per command makes a retried request return the run it + // started, while running the command again starts a new run. + key := "volcano-cli-" + uuid.NewString() + run, err := authenticated.API.CreateDatabaseMigrationRun(ctx, authenticated.ProjectID, databaseName, key, requestFiles(files)) + if err != nil { + return nil, unavailable(fmt.Errorf("failed to start a migration run on database %q: %w", databaseName, err)) + } + return run, nil +} + +// MigrationRuns returns a database's 50 most recent migration runs. +func (s Service) MigrationRuns(ctx context.Context, databaseName string) (*apiclient.DatabaseMigrationRunList, error) { + authenticated, err := s.sessions.CurrentProject() + if err != nil { + return nil, err + } + runs, err := authenticated.API.ListDatabaseMigrationRuns(ctx, authenticated.ProjectID, databaseName) + if err != nil { + return nil, fmt.Errorf("failed to list the migration runs of database %q: %w", databaseName, err) + } + return runs, nil +} + +// MigrationRun returns one migration run of a database. +func (s Service) MigrationRun(ctx context.Context, databaseName string, runID uuid.UUID) (*apiclient.DatabaseMigrationRun, error) { + authenticated, err := s.sessions.CurrentProject() + if err != nil { + return nil, err + } + run, err := authenticated.API.GetDatabaseMigrationRun(ctx, authenticated.ProjectID, databaseName, runID) + if err != nil { + return nil, fmt.Errorf("failed to get migration run %s of database %q: %w", runID, databaseName, err) + } + return run, nil +} + +// ErrMigrationRunWaitTimedOut reports a run that had not ended when the CLI +// stopped waiting for it. The run carries on. +var ErrMigrationRunWaitTimedOut = errors.New("the migration run had not ended") + +// WaitMigrationRun polls run until it ends and returns it as it ended. +// changed sees the run each time its status or phase changes. +func (s Service) WaitMigrationRun(ctx context.Context, databaseName string, run *apiclient.DatabaseMigrationRun, changed func(*apiclient.DatabaseMigrationRun)) (*apiclient.DatabaseMigrationRun, error) { + if MigrationRunDone(run) { + return run, nil + } + authenticated, err := s.sessions.CurrentProject() + if err != nil { + return nil, err + } + timeout := cliruntime.NewTimer(s.deps, migrationRunWaitTimeout) + defer timeout.Stop() + ticker := cliruntime.NewTicker(s.deps, migrationRunPollInterval) + defer ticker.Stop() + + failures := 0 + for { + select { + case <-ctx.Done(): + return run, ctx.Err() + case <-timeout.C(): + return run, fmt.Errorf("%w after %s", ErrMigrationRunWaitTimedOut, migrationRunWaitTimeout) + case <-ticker.C(): + } + latest, err := authenticated.API.GetDatabaseMigrationRun(ctx, authenticated.ProjectID, databaseName, run.Id) + if err != nil { + failures++ + if transient(err) && failures < migrationRunPollFailures { + continue + } + return run, fmt.Errorf("failed to get migration run %s of database %q: %w", run.Id, databaseName, err) + } + failures = 0 + if latest.Status != run.Status || latest.Phase != run.Phase { + changed(latest) + } + run = latest + if MigrationRunDone(run) { + return run, nil + } + } +} + +// transient reports a read worth trying again: the server failed, or it could +// not be reached. +func transient(err error) bool { + status := api.Status(err) + return status == 0 || status >= http.StatusInternalServerError +} + +// unavailable replaces a refusal that says migration runs are not available +// with ErrMigrationRunsUnavailable, keeping the refusal so the exit code still +// reports it. +func unavailable(err error) error { + if api.RouteMissing(err) || api.Code(err) == codeFeatureUnavailable { + return api.WithCause(ErrMigrationRunsUnavailable, err) + } + return err +} + +func requestFiles(files []MigrationFile) []apiclient.DatabaseMigrationFile { + out := make([]apiclient.DatabaseMigrationFile, len(files)) + for i, file := range files { + out[i] = apiclient.DatabaseMigrationFile{Name: file.Path, Sql: file.SQL} + } + return out +} diff --git a/internal/database/migrationfiles.go b/internal/database/migrationfiles.go new file mode 100644 index 00000000..1a5492df --- /dev/null +++ b/internal/database/migrationfiles.go @@ -0,0 +1,192 @@ +package database + +import ( + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "io/fs" + "maps" + "os" + "path" + "path/filepath" + "slices" + "strings" +) + +// MigrationsDir is where a project keeps its SQL migration files. +const MigrationsDir = "volcano/migrations" + +// MigrationFile is one migration file. Path is its path under MigrationsDir +// with / separators: a file name, or /. +type MigrationFile struct { + Path string + SQL string +} + +// Name is the file name the database's migration history records. +func (f MigrationFile) Name() string { + return path.Base(f.Path) +} + +// Checksum is the checksum the migration history records for the file: its +// SHA-256 with a leading byte order mark removed and CRLF line endings +// converted to LF, so checkouts on different platforms agree. +func (f MigrationFile) Checksum() string { + normalized := strings.ReplaceAll(strings.TrimPrefix(f.SQL, "\uFEFF"), "\r\n", "\n") + sum := sha256.Sum256([]byte(normalized)) + return "sha256:" + hex.EncodeToString(sum[:]) +} + +// CheckTrackedNames refuses a file whose name a tracked run would refuse: the +// API takes only names ending in a lowercase .sql, while the directory is +// read case-insensitively so that untracked runs keep taking .SQL files. +func CheckTrackedNames(files []MigrationFile) error { + for _, file := range files { + name := file.Name() + if !strings.HasSuffix(name, ".sql") { + return fmt.Errorf("%s/%s: tracked migration runs take only file names ending in lowercase .sql; rename it to %s", + MigrationsDir, file.Path, strings.TrimSuffix(name, path.Ext(name))+".sql") + } + } + return nil +} + +// Migrations are a project's migration files. Either every file sits in +// MigrationsDir itself, for a project with one database, or every file sits +// one directory down, in a directory named for its database. Git deploy reads +// the directory the same way, so a layout one refuses the other refuses too. +type Migrations struct { + files []MigrationFile + nested bool +} + +// ReadMigrations reads the .sql files under dir, skipping entries whose names +// start with "." or "_" and everything beneath them. A missing dir holds no +// migrations. The limits on how many files a run takes and how large they may +// be are the API's to enforce. +func ReadMigrations(dir string) (Migrations, error) { + info, err := os.Stat(dir) + if errors.Is(err, fs.ErrNotExist) { + return Migrations{}, nil + } + if err != nil { + return Migrations{}, err + } + if !info.IsDir() { + return Migrations{}, fmt.Errorf("%s is not a directory", dir) + } + root, err := os.OpenRoot(dir) + if err != nil { + return Migrations{}, err + } + defer func() { _ = root.Close() }() + + var files []MigrationFile + err = fs.WalkDir(root.FS(), ".", func(rel string, d fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if rel == "." { + return nil + } + shown := path.Join(filepath.ToSlash(dir), rel) + if strings.HasPrefix(d.Name(), ".") || strings.HasPrefix(d.Name(), "_") { + if d.IsDir() { + return fs.SkipDir + } + return nil + } + // 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 { + return fmt.Errorf("%s is a symlink, which Git deploy does not follow; replace it with the file or directory it points to", shown) + } + if d.IsDir() { + return nil + } + if !strings.EqualFold(path.Ext(d.Name()), ".sql") { + return nil + } + if !d.Type().IsRegular() { + return fmt.Errorf("%s is not a regular file", shown) + } + if strings.Count(rel, "/") > 1 { + return fmt.Errorf("%s is nested too deeply; place migrations in %s or one directory below it", shown, MigrationsDir) + } + sql, err := root.ReadFile(rel) + if err != nil { + return err + } + files = append(files, MigrationFile{Path: rel, SQL: string(sql)}) + return nil + }) + if err != nil { + return Migrations{}, err + } + slices.SortFunc(files, func(a, b MigrationFile) int { return strings.Compare(a.Path, b.Path) }) + + var flat, nested *MigrationFile + for i := range files { + if strings.Contains(files[i].Path, "/") { + if nested == nil { + nested = &files[i] + } + } else if flat == nil { + flat = &files[i] + } + } + if flat != nil && nested != nil { + return Migrations{}, fmt.Errorf("%s mixes top-level files, such as %s, with database directories, such as %s/; "+ + "put every file in %s//, or start the name of a directory to skip with _", + MigrationsDir, flat.Path, path.Dir(nested.Path), MigrationsDir) + } + return Migrations{files: files, nested: nested != nil}, nil +} + +// Empty reports whether there are no migration files. +func (m Migrations) Empty() bool { + return len(m.files) == 0 +} + +// Nested reports whether the files sit in per-database directories. +func (m Migrations) Nested() bool { + return m.nested +} + +// Databases returns the databases the directories name, in name order. It is +// empty when the files sit in MigrationsDir itself. +func (m Migrations) Databases() []string { + if !m.nested { + return nil + } + names := map[string]bool{} + for _, file := range m.files { + names[path.Dir(file.Path)] = true + } + return slices.Sorted(maps.Keys(names)) +} + +// For returns the files that belong to database, in the order they run: every +// file when they sit in MigrationsDir itself, or the files in the directory +// named for database. +func (m Migrations) For(database string) []MigrationFile { + if !m.nested { + return slices.Clone(m.files) + } + var files []MigrationFile + for _, file := range m.files { + if path.Dir(file.Path) == database { + files = append(files, file) + } + } + return files +} + +// Location names where database's files live, for messages. +func (m Migrations) Location(database string) string { + if m.nested { + return MigrationsDir + "/" + database + } + return MigrationsDir +} diff --git a/internal/database/migrationfiles_test.go b/internal/database/migrationfiles_test.go new file mode 100644 index 00000000..df6f61e1 --- /dev/null +++ b/internal/database/migrationfiles_test.go @@ -0,0 +1,126 @@ +package database + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeMigrations(t *testing.T, files map[string]string) string { + t.Helper() + dir := filepath.Join(t.TempDir(), "volcano", "migrations") + require.NoError(t, os.MkdirAll(dir, 0o755)) + for name, content := range files { + path := filepath.Join(dir, filepath.FromSlash(name)) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) + } + return dir +} + +func paths(files []MigrationFile) []string { + out := make([]string, len(files)) + for i, file := range files { + out[i] = file.Path + } + return out +} + +func TestReadMigrationsFlatLayoutBelongsToAnyDatabase(t *testing.T) { + dir := writeMigrations(t, map[string]string{ + "002_second.sql": "select 2;", + "001_first.SQL": "select 1;", + "README.md": "not a migration", + ".hidden.sql": "skipped", + "_draft.sql": "skipped", + "_archive/9_x.sql": "skipped", + ".git/objects/x.sql": "skipped", + }) + + migrations, err := ReadMigrations(dir) + require.NoError(t, err) + assert.False(t, migrations.Nested()) + assert.Empty(t, migrations.Databases()) + assert.Equal(t, []string{"001_first.SQL", "002_second.sql"}, paths(migrations.For("app"))) + assert.Equal(t, []string{"001_first.SQL", "002_second.sql"}, paths(migrations.For("other"))) + assert.Equal(t, MigrationsDir, migrations.Location("app")) +} + +func TestReadMigrationsNestedLayoutBelongsToEachDirectory(t *testing.T) { + dir := writeMigrations(t, map[string]string{ + "main_db/002_b.sql": "select 2;", + "main_db/001_a.sql": "select 1;", + "analytics/001_e.sql": "select 3;", + "_shared/x.sql": "skipped", + }) + + migrations, err := ReadMigrations(dir) + require.NoError(t, err) + assert.True(t, migrations.Nested()) + assert.Equal(t, []string{"analytics", "main_db"}, migrations.Databases()) + files := migrations.For("main_db") + assert.Equal(t, []string{"main_db/001_a.sql", "main_db/002_b.sql"}, paths(files)) + assert.Equal(t, "001_a.sql", files[0].Name()) + assert.Empty(t, migrations.For("app")) + assert.Equal(t, MigrationsDir+"/main_db", migrations.Location("main_db")) +} + +func TestReadMigrationsRefusesWhatGitDeployRefuses(t *testing.T) { + for _, tc := range []struct { + name string + files map[string]string + want string + }{ + { + name: "mixed layout", + files: map[string]string{"001_top.sql": "select 1;", "app/001_a.sql": "select 1;"}, + want: "volcano/migrations mixes top-level files, such as 001_top.sql, with database directories, such as app/", + }, + { + name: "nested too deeply", + files: map[string]string{"app/v1/001_a.sql": "select 1;"}, + want: "app/v1/001_a.sql is nested too deeply; place migrations in volcano/migrations or one directory below it", + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := ReadMigrations(writeMigrations(t, tc.files)) + require.ErrorContains(t, err, tc.want) + }) + } +} + +func TestReadMigrationsRefusesASymlink(t *testing.T) { + dir := writeMigrations(t, map[string]string{"001_a.sql": "select 1;"}) + require.NoError(t, os.Symlink(filepath.Join(dir, "001_a.sql"), filepath.Join(dir, "002_link.sql"))) + + _, err := ReadMigrations(dir) + require.ErrorContains(t, err, "002_link.sql is a symlink, which Git deploy does not follow") +} + +func TestReadMigrationsWithoutTheDirectoryIsEmpty(t *testing.T) { + migrations, err := ReadMigrations(filepath.Join(t.TempDir(), "missing")) + require.NoError(t, err) + assert.True(t, migrations.Empty()) + assert.Empty(t, migrations.For("app")) +} + +// The checksum is the API's: a byte order mark and CRLF line endings do not +// change it, so checkouts on different platforms agree. +func TestMigrationChecksumIgnoresBOMAndCRLF(t *testing.T) { + // The digest of "test", as the API documents it. + assert.Equal(t, "sha256:9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", MigrationFile{SQL: "test"}.Checksum()) + assert.Equal(t, MigrationFile{SQL: "test"}.Checksum(), MigrationFile{SQL: "\uFEFFtest"}.Checksum()) + + plain := MigrationFile{SQL: "CREATE TABLE notes (id int);\nSELECT 1;\n"} + assert.Equal(t, plain.Checksum(), MigrationFile{SQL: "\uFEFFCREATE TABLE notes (id int);\r\nSELECT 1;\r\n"}.Checksum()) + assert.NotEqual(t, plain.Checksum(), MigrationFile{SQL: "CREATE TABLE notes (id int);\nSELECT 2;\n"}.Checksum()) +} + +func TestCheckTrackedNames(t *testing.T) { + require.NoError(t, CheckTrackedNames([]MigrationFile{{Path: "001_init.sql"}, {Path: "app/002_more.sql"}})) + require.EqualError(t, CheckTrackedNames([]MigrationFile{{Path: "app/002_more.Sql"}}), + "volcano/migrations/app/002_more.Sql: tracked migration runs take only file names ending in lowercase .sql; rename it to 002_more.sql") +} diff --git a/internal/database/migrationstatus.go b/internal/database/migrationstatus.go new file mode 100644 index 00000000..2b7caf81 --- /dev/null +++ b/internal/database/migrationstatus.go @@ -0,0 +1,107 @@ +package database + +import ( + "slices" + "strings" + "time" + + "github.com/google/uuid" + + "github.com/Kong/volcano-cli/internal/apiclient" +) + +// MigrationState is where one migration file stands against a database's +// migration history. +type MigrationState string + +const ( + // MigrationApplied is a file a run applied. + MigrationApplied MigrationState = "applied" + // MigrationBaselined is a file recorded as applied without running. + MigrationBaselined MigrationState = "baselined" + // MigrationPending is a file the history does not record, which the next + // run applies. + MigrationPending MigrationState = "pending" + // MigrationChanged is a recorded file whose content differs from what the + // history recorded. A run refuses it with checksum_mismatch. + MigrationChanged MigrationState = "changed" + // MigrationMissing is a recorded file that is not among the project's + // files. A run reports it with a missing_file warning. + MigrationMissing MigrationState = "missing" +) + +// MigrationFileStatus is one file and where it stands. +type MigrationFileStatus struct { + Name string `json:"name"` + State MigrationState `json:"state"` + AppliedAt *time.Time `json:"applied_at,omitempty"` + RunID *uuid.UUID `json:"run_id,omitempty"` +} + +// MigrationStatus compares a database's migration history with the project's +// files for it. +type MigrationStatus struct { + Database string `json:"database"` + ActiveRunID *uuid.UUID `json:"active_run_id,omitempty"` + Files []MigrationFileStatus `json:"files"` + // Compared is false when the project had no files for the database, so + // Files lists the history alone. + Compared bool `json:"-"` +} + +// Count returns how many files are in state. +func (s MigrationStatus) Count(state MigrationState) int { + count := 0 + for _, file := range s.Files { + if file.State == state { + count++ + } + } + return count +} + +// CompareMigrations sets each of files, and each file the history records, +// against the history. With no files, it lists the history as recorded. +func CompareMigrations(databaseName string, history *apiclient.DatabaseMigrations, files []MigrationFile) MigrationStatus { + status := MigrationStatus{Database: databaseName, ActiveRunID: history.ActiveRunId, Files: []MigrationFileStatus{}, Compared: len(files) > 0} + recorded := make(map[string]apiclient.DatabaseMigrationHistoryEntry, len(history.History)) + for _, entry := range history.History { + recorded[entry.Name] = entry + } + local := make(map[string]bool, len(files)) + for _, file := range files { + name := file.Name() + local[name] = true + entry, ok := recorded[name] + if !ok { + status.Files = append(status.Files, MigrationFileStatus{Name: name, State: MigrationPending}) + continue + } + fileStatus := recordedStatus(entry) + if entry.Checksum != file.Checksum() { + fileStatus.State = MigrationChanged + } + status.Files = append(status.Files, fileStatus) + } + for _, entry := range history.History { + if local[entry.Name] { + continue + } + fileStatus := recordedStatus(entry) + if status.Compared { + fileStatus.State = MigrationMissing + } + status.Files = append(status.Files, fileStatus) + } + slices.SortStableFunc(status.Files, func(a, b MigrationFileStatus) int { return strings.Compare(a.Name, b.Name) }) + return status +} + +func recordedStatus(entry apiclient.DatabaseMigrationHistoryEntry) MigrationFileStatus { + state := MigrationApplied + if entry.Status == apiclient.DatabaseMigrationHistoryEntryStatusBaselined { + state = MigrationBaselined + } + appliedAt := entry.AppliedAt + return MigrationFileStatus{Name: entry.Name, State: state, AppliedAt: &appliedAt, RunID: entry.RunId} +} diff --git a/internal/localmode/reset.go b/internal/localmode/reset.go index 254b26d4..f32c9f15 100644 --- a/internal/localmode/reset.go +++ b/internal/localmode/reset.go @@ -44,7 +44,7 @@ func (s Service) Reset(ctx context.Context, w io.Writer) error { return fmt.Errorf("failed to save dev state after reset: %w", err) } - fmt.Fprintln(w, "Redeploy migrations with: volcano migrations deploy --all -d app") + fmt.Fprintln(w, "Redeploy migrations with: volcano databases migration apply -d app") return nil } diff --git a/internal/localmode/reset_test.go b/internal/localmode/reset_test.go index 4f757f4a..07579b5e 100644 --- a/internal/localmode/reset_test.go +++ b/internal/localmode/reset_test.go @@ -42,7 +42,7 @@ func TestResetInvokesServerEntrypointAndRefreshesDevState(t *testing.T) { assert.Contains(t, out.String(), "Local reset complete.") assert.Contains(t, out.String(), "Default database: app") assert.Contains(t, out.String(), "Dropped databases: [project_a]") - assert.Contains(t, out.String(), "volcano migrations deploy --all -d app") + assert.Contains(t, out.String(), "volcano databases migration apply -d app") assert.True(t, runner.called("docker", localResetCommandArgs()...)) statePath, err := DevStatePath() diff --git a/internal/output/database_migration.go b/internal/output/database_migration.go new file mode 100644 index 00000000..3dd6fc18 --- /dev/null +++ b/internal/output/database_migration.go @@ -0,0 +1,222 @@ +package output + +import ( + "fmt" + "io" + "strings" + + "github.com/Kong/volcano-cli/internal/apiclient" + clidatabase "github.com/Kong/volcano-cli/internal/database" + "github.com/Kong/volcano-cli/internal/theme" +) + +// MigrationCommands are the commands the migration renderers suggest, already +// completed with the database. +type MigrationCommands struct { + Apply string + Baseline string + // RunsGet returns the command that shows a run. + RunsGet func(runID string) string +} + +// DatabaseMigrationStatus renders where each migration file stands against a +// database's migration history. location names where the files live. +func DatabaseMigrationStatus(w io.Writer, status clidatabase.MigrationStatus, location string, commands MigrationCommands) { + on := theme.On(w) + if !status.Compared { + fmt.Fprintf(w, "No migration files for database '%s' in %s; showing its history only.\n\n", status.Database, location) + } + if len(status.Files) == 0 { + fmt.Fprintf(w, "Database '%s' has no migration history\n", status.Database) + } else { + tableHead(w, on, false, 72, "%-44s %-10s %-15s", "Name", "State", "Recorded") + for _, file := range status.Files { + recorded := "-" + if file.AppliedAt != nil { + recorded = FormatTimeAgo(*file.AppliedAt) + } + fmt.Fprintf(w, "%-44s %s %-15s\n", + Truncate(theme.StripControl(file.Name), 44), migrationStateCell(file.State, on), recorded) + } + var counts []string + for _, state := range []clidatabase.MigrationState{ + clidatabase.MigrationApplied, clidatabase.MigrationBaselined, clidatabase.MigrationPending, + clidatabase.MigrationChanged, clidatabase.MigrationMissing, + } { + if count := status.Count(state); count > 0 { + counts = append(counts, fmt.Sprintf("%d %s", count, state)) + } + } + summary(w, on, "%s", strings.Join(counts, ", ")) + } + + if status.ActiveRunID != nil { + fmt.Fprintf(w, "\nRun %s is queued or running. Follow it with: %s\n", + status.ActiveRunID, theme.Command(commands.RunsGet(status.ActiveRunID.String()), on)) + } + if status.Count(clidatabase.MigrationChanged) > 0 { + fmt.Fprintf(w, "\n%s\n", theme.Dim("A run refuses an applied file whose content changed (checksum_mismatch). "+ + "Put its recorded content back and make the change in a new file.", on)) + } + if status.Count(clidatabase.MigrationPending) > 0 { + fmt.Fprintf(w, "\n%s%s\n", theme.Dim("Apply the pending files with: ", on), theme.Command(commands.Apply, on)) + if status.Count(clidatabase.MigrationApplied)+status.Count(clidatabase.MigrationBaselined) == 0 { + fmt.Fprintf(w, "%s%s\n", + theme.Dim("If they already ran without tracking, record them instead with: ", on), + theme.Command(commands.Baseline, on)) + } + } +} + +func migrationStateCell(state clidatabase.MigrationState, on bool) string { + text := fmt.Sprintf("%-10s", state) + switch state { + case clidatabase.MigrationApplied, clidatabase.MigrationBaselined: + return theme.Success(text, on) + case clidatabase.MigrationPending: + return theme.Warn(text, on) + case clidatabase.MigrationChanged: + return theme.Fail(text, on) + default: + return theme.Dim(text, on) + } +} + +// DatabaseMigrationRun renders one migration run with each file's outcome. +// A failed file's message is the database's own and can quote the file or +// its data, so it is printed, never sent anywhere. +func DatabaseMigrationRun(w io.Writer, run *apiclient.DatabaseMigrationRun) { + on := theme.On(w) + kv(w, on, "ID", "%s", run.Id.String()) + kv(w, on, "Database", "%s", run.Database.Name) + kv(w, on, "Status", "%s", theme.Status(string(run.Status), on)) + kv(w, on, "Phase", "%s", run.Phase) + kv(w, on, "Source", "%s", migrationRunSource(run)) + if run.BackupName != nil && *run.BackupName != "" { + backup := *run.BackupName + if run.Phase == apiclient.DatabaseMigrationRunPhaseBackup { + if run.Status == apiclient.DatabaseMigrationRunStatusFailed { + backup += " (may not exist: the run ended while taking it)" + } else { + backup += " (being taken)" + } + } + kv(w, on, "Backup", "%s", backup) + } + kv(w, on, "Created", "%s", FormatTimestamp(run.CreatedAt)) + if run.CompletedAt != nil { + kv(w, on, "Finished", "%s", FormatTimestamp(*run.CompletedAt)) + } + if run.Error != nil { + kv(w, on, "Error", "%s: %s", run.Error.Code, theme.StripControl(run.Error.Message)) + } + if run.Files != nil { + MigrationRunFiles(w, *run.Files) + } + MigrationRunWarnings(w, run.Warnings) +} + +// MigrationRunFiles renders what became of each file of a run. +func MigrationRunFiles(w io.Writer, files []apiclient.DatabaseMigrationRunFile) { + on := theme.On(w) + if len(files) == 0 { + return + } + tableHead(w, on, true, 64, "%-44s %-15s", "File", "Outcome") + for _, file := range files { + fmt.Fprintf(w, "%-44s %s\n", Truncate(theme.StripControl(file.Name), 44), migrationOutcomeCell(file.Outcome, on)) + if file.Error != nil { + fmt.Fprintf(w, " %s\n", theme.Fail(migrationFileError(file.Error), on)) + } + } +} + +func migrationOutcomeCell(outcome apiclient.DatabaseMigrationRunFileOutcome, on bool) string { + text := strings.ReplaceAll(string(outcome), "_", " ") + switch outcome { + case apiclient.DatabaseMigrationRunFileOutcomeApplied: + return theme.Success(text, on) + case apiclient.DatabaseMigrationRunFileOutcomeFailed, apiclient.DatabaseMigrationRunFileOutcomeUnknown: + return theme.Fail(text, on) + case apiclient.DatabaseMigrationRunFileOutcomePending: + return theme.Warn(text, on) + default: + return theme.Dim(text, on) + } +} + +func migrationFileError(fileErr *apiclient.DatabaseMigrationFileError) string { + var where []string + if fileErr.Sqlstate != nil && *fileErr.Sqlstate != "" { + where = append(where, "SQLSTATE "+*fileErr.Sqlstate) + } + if fileErr.Line != nil { + where = append(where, fmt.Sprintf("line %d", *fileErr.Line)) + } + message := theme.StripControl(fileErr.Message) + if len(where) == 0 { + return message + } + return fmt.Sprintf("%s (%s)", message, strings.Join(where, ", ")) +} + +// MigrationRunWarnings renders the conditions a run reported that did not +// stop it. +func MigrationRunWarnings(w io.Writer, warnings []apiclient.DatabaseMigrationWarning) { + if len(warnings) == 0 { + return + } + fmt.Fprintln(w) + for _, warning := range warnings { + Warning(w, "%s: %s", warning.Code, theme.StripControl(warning.Message)) + } +} + +// DatabaseMigrationRuns renders a database's recent migration runs. +func DatabaseMigrationRuns(w io.Writer, list *apiclient.DatabaseMigrationRunList, databaseName string) { + if list == nil { + list = &apiclient.DatabaseMigrationRunList{} + } + on := theme.On(w) + if len(list.Data) == 0 { + fmt.Fprintf(w, "No migration runs on database '%s'\n", databaseName) + return + } + tableHead(w, on, false, 100, "%-36s %-10s %-9s %-11s %-20s %-15s", "ID", "Status", "Phase", "Source", "Error", "Created") + for _, run := range list.Data { + errorCode := "-" + if run.Error != nil { + errorCode = string(run.Error.Code) + } + fmt.Fprintf(w, "%-36s %s %-9s %-11s %-20s %-15s\n", + run.Id.String(), + statusCell(string(run.Status), 10, on), + run.Phase, + migrationRunSource(&run), + Truncate(errorCode, 20), + FormatTimeAgo(run.CreatedAt), + ) + } + summary(w, on, "Showing %d migration run(s) of database '%s'", len(list.Data), databaseName) +} + +func migrationRunSource(run *apiclient.DatabaseMigrationRun) string { + source := string(run.Source) + if run.CommitSha != nil && *run.CommitSha != "" { + sha := *run.CommitSha + source += " " + sha[:min(len(sha), 12)] + } + return source +} + +// DatabaseMigrationBaseline renders the files a baseline recorded. +func DatabaseMigrationBaseline(w io.Writer, baseline *apiclient.DatabaseMigrationBaseline, databaseName string) { + if len(baseline.Recorded) == 0 { + Success(w, "Nothing to record: the migration history of database '%s' already holds every file", databaseName) + return + } + Success(w, "Recorded %d file(s) as applied on database '%s', without running them", len(baseline.Recorded), databaseName) + for _, name := range baseline.Recorded { + fmt.Fprintf(w, " - %s\n", theme.StripControl(name)) + } +} diff --git a/internal/projectinit/starters/base/volcano/README.md b/internal/projectinit/starters/base/volcano/README.md index 2937e177..dc140b19 100644 --- a/internal/projectinit/starters/base/volcano/README.md +++ b/internal/projectinit/starters/base/volcano/README.md @@ -10,8 +10,8 @@ Local development: `volcano deploy` deploys whatever the project has: the variables in volcano/volcano.env, the migrations in volcano/migrations, the functions in volcano/functions, the frontends listed in .volcano/source-layout.json, and -volcano/volcano-config.yaml. Migrations run again on every --with-migrations -run, so write them to be safe to repeat. +volcano/volcano-config.yaml. Each migration runs once: the database records +the files it applied, and later --with-migrations runs apply only new ones. Cloud deployment: diff --git a/internal/projectinit/starters/base/volcano/migrations/README.md b/internal/projectinit/starters/base/volcano/migrations/README.md index 99f499ef..262f4bd5 100644 --- a/internal/projectinit/starters/base/volcano/migrations/README.md +++ b/internal/projectinit/starters/base/volcano/migrations/README.md @@ -1,11 +1,12 @@ # Migrations -Place SQL migrations in this directory, one statement per file. Files run in -name order, every time, so write them to be safe to repeat. Run them locally -with: +Place SQL migrations in this directory. Files run in name order, each once: +Volcano records the files it applies in the database's migration history and +skips them on later runs. To change the schema again, add a new file rather +than editing one that ran. Run them locally with: volcano deploy --with-migrations or on their own: - volcano migrations deploy --all -d app + volcano databases migration apply -d app diff --git a/internal/projectinit/starters/nextjs-notes/web/README.md b/internal/projectinit/starters/nextjs-notes/web/README.md index daac79b4..64924f48 100644 --- a/internal/projectinit/starters/nextjs-notes/web/README.md +++ b/internal/projectinit/starters/nextjs-notes/web/README.md @@ -22,8 +22,9 @@ Run commands from the project root. 3. Open the `web` URL the deploy prints `volcano deploy` saves the variables, applies the migrations, deploys the -function and the frontend, then applies the project configuration. Migrations -run again on every `--with-migrations` run; these are safe to repeat. +function and the frontend, then applies the project configuration. Each +migration runs once: the database records the files it applied, and later +`--with-migrations` runs apply only new ones. To edit with hot reload, run `npm install` and `npm run dev`, then open [http://localhost:3000](http://localhost:3000). @@ -32,7 +33,7 @@ The same steps one at a time: ```bash volcano variables deploy -volcano migrations deploy --all -d app +volcano databases migration apply -d app volcano functions deploy --all volcano frontends deploy --name web --path . --app-root web --variable-scope scoped \ --variable NEXT_PUBLIC_VOLCANO_API_URL --variable NEXT_PUBLIC_VOLCANO_ANON_KEY \ diff --git a/tests/e2e/api/deploy_test.go b/tests/e2e/api/deploy_test.go index 3e75350e..26b6fb08 100644 --- a/tests/e2e/api/deploy_test.go +++ b/tests/e2e/api/deploy_test.go @@ -56,8 +56,9 @@ func TestAPIE2ECloudDeploy(t *testing.T) { "2. migrations volcano/migrations -> database "+database, "4. durable skipped: volcano/volcano-config.yaml declares no durable functions", "5. frontends web (.volcano/source-layout.json)", - "Applying 001_create_notes.sql... ok", - "Applying 015_notes_updated_at_trigger_create.sql... ok", + "Migration run ", + "succeeded: 15 applied, 0 already applied", + "015_notes_updated_at_trigger_create.sql", "✓ notes-summary", "✓ web", "Deployed in") @@ -127,7 +128,7 @@ frontends: "✓ echo", "✓ order-pipeline", "Deployed in") - second.requireNotContains(t, "Applying 001_create_notes.sql", "Deploy them with") + second.requireNotContains(t, "Starting a migration run", "Deploy them with") t.Cleanup(func() { _ = env.runCloudCLI(t, "durable", "delete", "order-pipeline", "--yes") _ = env.runCloudCLI(t, "functions", "delete", "echo", "--yes") diff --git a/tests/e2e/api/migrations_test.go b/tests/e2e/api/migrations_test.go index 0a4616e1..3cd2e521 100644 --- a/tests/e2e/api/migrations_test.go +++ b/tests/e2e/api/migrations_test.go @@ -1,6 +1,14 @@ package api -import "testing" +import ( + "os" + "path/filepath" + "regexp" + "testing" + "time" +) + +var apiE2EMigrationRunPattern = regexp.MustCompile(`Run ([0-9a-f-]{36}) queued`) func TestAPIE2ESmokeMigrations(t *testing.T) { env := setupAPIE2E(t, "smoke-migrations") @@ -10,3 +18,77 @@ func TestAPIE2ESmokeMigrations(t *testing.T) { migrationResult := env.runCloudCLI(t, "databases", "migration", "up", "--all", "-d", "missing_database") migrationResult.requireFailure(t, "missing_database") } + +// TestAPIE2ECloudDatabaseMigrations drives tracked migration runs against a +// real database: a run that rehearses on a branch before it applies, a run +// with nothing pending, a baseline, a --with-migrations deploy, and a file +// that fails in the rehearsal. The refusals that need no database live in the +// hosting repo's CLI suite. +func TestAPIE2ECloudDatabaseMigrations(t *testing.T) { + env := setupAPIE2E(t, "cloud-database-migrations") + + env.loginAndUse(t) + database := "cli_e2e_" + apiE2ESuffix(t) + env.runCloudCLI(t, "databases", "create", database, "--region", apiE2EDefaultRegion, "--pg-version", apiE2EDefaultPGVersion).requireSuccess(t, database) + t.Cleanup(func() { + _ = env.runCloudCLI(t, "databases", "delete", database, "--yes") + }) + env.waitForDatabaseActive(t, database, 30*time.Minute) + + migrations := filepath.Join(env.projectDir, "volcano", "migrations") + // A tracked run takes several statements in one file, which an untracked + // one refuses. + writeAPIE2EFile(t, filepath.Join(migrations, "001_create_items.sql"), + "CREATE TABLE items (id bigint PRIMARY KEY);\nCREATE INDEX items_created_idx ON items (id);\n") + env.runCloudCLI(t, "databases", "migration", "status", "-d", database).requireSuccess(t, + "001_create_items.sql", "pending", "If they already ran without tracking, record them instead with") + + // A new user is on HOBBY: the run rehearses on a branch but takes no backup. + applied := env.runCloudCLI(t, "databases", "migration", "apply", "-d", database) + applied.requireSuccess(t, "succeeded: 1 applied, 0 already applied", "001_create_items.sql", "backup_skipped") + applied.requireNotContains(t, "rehearsal_skipped") + match := apiE2EMigrationRunPattern.FindStringSubmatch(applied.output) + if match == nil { + t.Fatalf("apply did not print its run ID:\n%s", applied.output) + } + runID := match[1] + + env.runCloudCLI(t, "databases", "migration", "apply", "-d", database). + requireSuccess(t, "succeeded: 0 applied, 1 already applied") + + // A file the database already reflects is recorded, not run. + writeAPIE2EFile(t, filepath.Join(migrations, "002_create_orders.sql"), "CREATE TABLE orders (id bigint PRIMARY KEY);\n") + env.runCloudCLI(t, "databases", "migration", "baseline", "-d", database, "-f", "002_create_orders", "--yes"). + requireSuccess(t, "Recorded 1 file(s) as applied on database '"+database+"', without running them", "002_create_orders.sql") + env.runCloudCLI(t, "databases", "migration", "baseline", "-d", database, "--all", "--yes"). + requireSuccess(t, "Nothing to record") + env.runCloudCLI(t, "databases", "migration", "status", "-d", database). + requireSuccess(t, "001_create_items.sql", "applied", "002_create_orders.sql", "baselined") + + // --with-migrations applies in a tracked run too, so only the new file + // runs. With nothing else in the directory, every other step is skipped. + writeAPIE2EFile(t, filepath.Join(migrations, "003_add_items_name.sql"), "ALTER TABLE items ADD COLUMN name text;\n") + env.runCloudCLI(t, "deploy", "--with-migrations", "-d", database).requireSuccess(t, + "2. migrations volcano/migrations -> database "+database, + "succeeded: 1 applied, 2 already applied", + "003_add_items_name.sql", + "Deployed in") + + // A file that fails stops the run in the rehearsal, before the database. + broken := filepath.Join(migrations, "004_broken.sql") + writeAPIE2EFile(t, broken, "ALTER TABLE missing_table ADD COLUMN name text;\n") + env.runCloudCLI(t, "databases", "migration", "apply", "-d", database).requireFailure(t, + "failed in rehearse (migration_failed)", + `relation "missing_table" does not exist`, + "production was not changed") + if err := os.Remove(broken); err != nil { + t.Fatalf("remove %s: %v", broken, err) + } + env.runCloudCLI(t, "databases", "migration", "status", "-d", database). + requireSuccess(t, "003_add_items_name.sql", "applied") + + env.runCloudCLI(t, "databases", "migration", "runs", "list", "-d", database). + requireSuccess(t, runID, "succeeded", "failed", "migration_failed", "Showing 4 migration run(s)") + env.runCloudCLI(t, "databases", "migration", "runs", "get", runID, "-d", database). + requireSuccess(t, "Status: succeeded", "Phase: apply", "001_create_items.sql", "applied") +} diff --git a/tests/e2e/localmode/database_migrations_test.go b/tests/e2e/localmode/database_migrations_test.go new file mode 100644 index 00000000..7d3d07dd --- /dev/null +++ b/tests/e2e/localmode/database_migrations_test.go @@ -0,0 +1,87 @@ +package localmode + +import ( + "os" + "path/filepath" + "regexp" + "testing" +) + +var localModeE2EMigrationRunPattern = regexp.MustCompile(`Run ([0-9a-f-]{36}) queued`) + +// TestLocalModeE2EDatabaseMigrations is the cloud tracked-run round trip on +// the local stack's app database. Local mode has no branches or backups, so a +// run applies straight to the database, warns that it skipped both, and a +// failing file fails in the apply phase. It has its own stack, so a failure +// here does not stop the smoke's later checks. +func TestLocalModeE2EDatabaseMigrations(t *testing.T) { + if os.Getenv("VOLCANO_LOCALMODE_E2E") != "1" { + t.Skip("set VOLCANO_LOCALMODE_E2E=1 to run disposable Docker acceptance") + } + requireDocker(t) + binary := buildLocalModeE2EBinary(t) + env, dir := localModeE2EEnv(t), localModeE2EProjectDir(t) + cleanup := func() { + _, _ = runVolcanoLocalModeE2EAllowFailure(t, binary, env, dir, "stop", "--clean") + } + cleanup() + t.Cleanup(cleanup) + requireContains(t, runVolcanoLocalModeE2E(t, binary, env, dir, "start"), + "Volcano is ready for local development.") + + migrations := filepath.Join("volcano", "migrations") + writeLocalModeE2EFile(t, dir, filepath.Join(migrations, "001_create_items.sql"), + "CREATE TABLE items (id bigint PRIMARY KEY);\nCREATE INDEX items_created_idx ON items (id);\n") + status := runVolcanoLocalModeE2E(t, binary, env, dir, "databases", "migration", "status", "-d", "app") + requireContains(t, status, "001_create_items.sql") + requireContains(t, status, "pending") + requireContains(t, status, "volcano databases migration apply -d app") + + applied := runVolcanoLocalModeE2E(t, binary, env, dir, "databases", "migration", "apply", "-d", "app") + requireContains(t, applied, "succeeded: 1 applied, 0 already applied") + requireContains(t, applied, "rehearsal_skipped") + requireContains(t, applied, "backup_skipped") + match := localModeE2EMigrationRunPattern.FindStringSubmatch(applied) + if match == nil { + t.Fatalf("apply did not print its run ID:\n%s", applied) + } + runID := match[1] + + requireContains(t, runVolcanoLocalModeE2E(t, binary, env, dir, "databases", "migration", "apply", "-d", "app"), + "succeeded: 0 applied, 1 already applied") + + writeLocalModeE2EFile(t, dir, filepath.Join(migrations, "002_create_orders.sql"), "CREATE TABLE orders (id bigint PRIMARY KEY);\n") + requireContains(t, runVolcanoLocalModeE2E(t, binary, env, dir, + "databases", "migration", "baseline", "-d", "app", "-f", "002_create_orders", "--yes"), + "Recorded 1 file(s) as applied on database 'app', without running them") + status = runVolcanoLocalModeE2E(t, binary, env, dir, "databases", "migration", "status", "-d", "app") + requireContains(t, status, "applied") + requireContains(t, status, "baselined") + + writeLocalModeE2EFile(t, dir, filepath.Join(migrations, "003_add_items_name.sql"), "ALTER TABLE items ADD COLUMN name text;\n") + deployed := runVolcanoLocalModeE2EDeploy(t, binary, env, dir, "deploy", "--with-migrations") + requireContains(t, deployed, "2. migrations volcano/migrations -> database app") + requireContains(t, deployed, "succeeded: 1 applied, 2 already applied") + requireContains(t, deployed, "003_add_items_name.sql") + requireContains(t, deployed, "Deployed in") + + broken := filepath.Join(migrations, "004_broken.sql") + writeLocalModeE2EFile(t, dir, broken, "ALTER TABLE missing_table ADD COLUMN name text;\n") + failed, err := runVolcanoLocalModeE2EAllowFailure(t, binary, env, dir, "databases", "migration", "apply", "-d", "app") + if err == nil { + t.Fatalf("a run with a failing file has to fail\n%s", failed) + } + requireContains(t, failed, "failed in apply (migration_failed)") + requireContains(t, failed, `relation "missing_table" does not exist`) + removeLocalModeE2EFile(t, dir, broken) + + runs := runVolcanoLocalModeE2E(t, binary, env, dir, "databases", "migration", "runs", "list", "-d", "app") + requireContains(t, runs, runID) + requireContains(t, runs, "migration_failed") + requireContains(t, runs, "Showing 4 migration run(s)") + run := runVolcanoLocalModeE2E(t, binary, env, dir, "databases", "migration", "runs", "get", runID, "-d", "app") + requireContains(t, run, "Status: succeeded") + requireContains(t, run, "Phase: apply") + requireContains(t, run, "001_create_items.sql") + requireContains(t, run, "rehearsal_skipped") +} diff --git a/tests/e2e/localmode/deploy_test.go b/tests/e2e/localmode/deploy_test.go index 52f9b4c1..c7ca12e0 100644 --- a/tests/e2e/localmode/deploy_test.go +++ b/tests/e2e/localmode/deploy_test.go @@ -109,7 +109,8 @@ func requireLocalModeDeploysWholeProject(t *testing.T, binary string, env []stri requireContains(t, output, "4. durable deploy-pipeline") requireContains(t, output, "5. frontends deploy-web (.volcano/source-layout.json)") requireContains(t, output, "6. config volcano/volcano-config.yaml") - requireContains(t, output, "Applying 001_create_deploy_smoke.sql... ok") + requireContains(t, output, "Starting a migration run of 1 file(s) from volcano/migrations on database 'app'") + requireContains(t, output, "succeeded: 1 applied, 0 already applied") requireContains(t, output, "Skipping 1 durable function(s) declared in volcano/volcano-config.yaml: deploy-pipeline") requireNotContains(t, output, "Deploy them with") requireContains(t, output, "✓ deploy-greeting") @@ -141,7 +142,7 @@ func requireLocalModeDeploysWholeProject(t *testing.T, binary string, env []stri writeLocalModeE2EDeployEnv(t, dir, "second-run") output = runVolcanoLocalModeE2EDeploy(t, binary, env, dir, "deploy") requireContains(t, output, "2. migrations skipped: pass --with-migrations to apply volcano/migrations") - requireNotContains(t, output, "Applying 001_create_deploy_smoke.sql") + requireNotContains(t, output, "Starting a migration run") requireContains(t, output, "Deployed in") awaitLocalModeE2EFrontendServes(t, siteURL, "deploy says second-run") awaitLocalModeE2EFrontendServes(t, siteURL+"/api/hello", "route says second-run /hello") @@ -228,8 +229,8 @@ func requireLocalModeDeploysStarter(t *testing.T, binary string, env []string) { "Run: volcano deploy --with-migrations") output := runVolcanoLocalModeE2EDeploy(t, binary, env, dir, "deploy", "--with-migrations") - requireContains(t, output, "Applying 001_create_notes.sql... ok") - requireContains(t, output, "Applying 015_notes_updated_at_trigger_create.sql... ok") + requireContains(t, output, "succeeded: 15 applied, 0 already applied") + requireContains(t, output, "015_notes_updated_at_trigger_create.sql") requireContains(t, output, "✓ notes-summary") requireContains(t, output, "✓ web") requireContains(t, output, "Deployed in") diff --git a/tests/e2e/localmode/localmode_test.go b/tests/e2e/localmode/localmode_test.go index c6194fc3..69a91fe4 100644 --- a/tests/e2e/localmode/localmode_test.go +++ b/tests/e2e/localmode/localmode_test.go @@ -155,7 +155,7 @@ func TestLocalModeE2ESmoke(t *testing.T) { resetOutput := runVolcanoLocalModeE2E(t, volcanoBin, env, projectDir, "reset", "--yes") requireContains(t, resetOutput, "Local reset complete.") - requireContains(t, resetOutput, "volcano migrations deploy --all -d app") + requireContains(t, resetOutput, "volcano databases migration apply -d app") statusAfterReset := runVolcanoLocalModeE2E(t, volcanoBin, env, projectDir, "status") requireContains(t, statusAfterReset, "Volcano Local Development Status")