Skip to content

Commit 4f6238e

Browse files
iplay88keysEItanya
andauthored
Add SKIP_MIGRATIONS to run migrations out-of-band (kagent-dev#2170)
## Description Adds a `SKIP_MIGRATIONS` opt-out so database migrations can run out-of-band (e.g. from a CI/CD pipeline) instead of at controller startup. - New `--skip-migrations` controller flag (settable via the `SKIP_MIGRATIONS` env var; default off — startup migrations remain the default behavior). - When set, the controller applies nothing and instead calls the new `migrations.VerifyMigrated`, refusing to start against a database that is not fully migrated. Per source: a missing tracking table, a version behind the binary's embedded max, or a dirty tracking table is a startup error with an actionable message; a database ahead of the binary boots in compatibility mode, matching `RunUp`. - `VerifyMigrated` issues only `SELECT`s — it deliberately does not open golang-migrate, which creates the tracking table on every open — so the verification works on a connection whose role has no DDL privileges. - Helm: new `database.postgres.skipMigrations` value (default `false`) rendered into the controller configmap as `SKIP_MIGRATIONS`, plus a NOTES warning when enabled reminding the operator that migrations must be applied out-of-band before install/upgrade. ## Testing Verified on a kind cluster: upgrade with `skipMigrations=true` boots and logs `database schema verified`; renaming a tracking table makes the controller exit with `tracking table "vector_schema_migrations" does not exist — the database has not been migrated; apply migrations out-of-band or unset SKIP_MIGRATIONS`; restoring the table recovers. Fresh install without migrations returns: ``` {"level":"error","ts":"2026-07-06T17:37:17Z","logger":"setup","msg":"database migration verification failed","error":"source core: tracking table \"schema_migrations\" does not exist - the database has not been migrated; apply migrations out-of-band or unset SKIP_MIGRATIONS","stacktrace":"github.com/kagent-dev/kagent/go/core/pkg/app.Start\n\t/workspace/core/pkg/app/app.go:482\nmain.main\n\t/workspace/core/cmd/controller/main.go:34\nruntime.main\n\t/usr/lib/go/src/runtime/proc.go:290"} ``` Controller starts up correctly when migrations are manually run against the db: ``` ❯ ./go/core/bin/kagent-local db migrate up applied 9 migration(s); schema is up to date ``` --------- Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io> Co-authored-by: Eitan Yarmush <eitan.yarmush@solo.io>
1 parent d3bcf72 commit 4f6238e

6 files changed

Lines changed: 222 additions & 8 deletions

File tree

go/core/pkg/app/app.go

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,10 @@ type Config struct {
137137
// that originates TLS upstream. Off by default;
138138
MCPEgressPlaintext bool
139139
Database struct {
140-
Url string
141-
UrlFile string
142-
VectorEnabled bool
140+
Url string
141+
UrlFile string
142+
VectorEnabled bool
143+
SkipMigrations bool
143144
}
144145
Substrate struct {
145146
AteAPIEndpoint string
@@ -181,6 +182,7 @@ func (cfg *Config) SetFlags(commandLine *flag.FlagSet) {
181182
commandLine.StringVar(&cfg.Database.Url, "postgres-database-url", "postgres://postgres:kagent@kagent-postgresql.kagent.svc.cluster.local:5432/postgres", "The URL of the PostgreSQL database.")
182183
commandLine.StringVar(&cfg.Database.UrlFile, "postgres-database-url-file", "", "Path to a file containing the PostgreSQL database URL. Takes precedence over --postgres-database-url.")
183184
commandLine.BoolVar(&cfg.Database.VectorEnabled, "database-vector-enabled", true, "Enable pgvector extension and memory table. Requires pgvector to be installed on the PostgreSQL server.")
185+
commandLine.BoolVar(&cfg.Database.SkipMigrations, "skip-migrations", false, "Do not run database migrations at startup; instead verify the database is already migrated and fail if it is not. Migrations must be applied out-of-band (e.g. from a pipeline or pre-upgrade hook). Settable via the SKIP_MIGRATIONS env var.")
184186

185187
commandLine.StringVar(&cfg.WatchNamespaces, "watch-namespaces", "", "The namespaces to watch for .")
186188

@@ -470,13 +472,25 @@ func Start(getExtensionConfig GetExtensionConfig, extraSources []migrations.Sour
470472

471473
// Run migrations before connecting; schema must exist before queries.
472474
// Built-in sources run first, then any downstream-registered extras.
473-
setupLog.Info("running database migrations")
475+
// With --skip-migrations (SKIP_MIGRATIONS) the server applies nothing and
476+
// instead verifies the database is already migrated, so migrations can run
477+
// out-of-band and this connection needs no DDL privileges.
474478
sources := append(migrations.BuiltinSources(cfg.Database.VectorEnabled), extraSources...)
475-
if err := migrations.RunUp(ctx, dbURL, sources); err != nil {
476-
setupLog.Error(err, "database migration failed")
477-
os.Exit(1)
479+
if cfg.Database.SkipMigrations {
480+
setupLog.Info("skipping database migrations; verifying schema is migrated")
481+
if err := migrations.VerifyMigrated(ctx, dbURL, sources); err != nil {
482+
setupLog.Error(err, "database migration verification failed")
483+
os.Exit(1)
484+
}
485+
setupLog.Info("database schema verified")
486+
} else {
487+
setupLog.Info("running database migrations")
488+
if err := migrations.RunUp(ctx, dbURL, sources); err != nil {
489+
setupLog.Error(err, "database migration failed")
490+
os.Exit(1)
491+
}
492+
setupLog.Info("database migrations complete")
478493
}
479-
setupLog.Info("database migrations complete")
480494

481495
// Connect to database
482496
db, err := database.Connect(ctx, &database.PostgresConfig{

go/core/pkg/migrations/runner.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,83 @@ func RunUp(ctx context.Context, url string, sources []Source) error {
179179
return nil
180180
}
181181

182+
// VerifyMigrated checks, without applying or reverting anything, that every
183+
// source's migrations have been applied to the database. It is the boot-time
184+
// guard for the SKIP_MIGRATIONS deployment mode, where migrations run
185+
// out-of-band (a pipeline or pre-upgrade hook) and the server must refuse to
186+
// serve a wrong-shaped schema. It issues only SELECTs — never golang-migrate,
187+
// which creates the tracking table on open — so it is safe on a connection
188+
// whose role has no DDL privileges.
189+
//
190+
// Per source: a missing tracking table or a version behind this binary's
191+
// embedded max is an error; a dirty tracking table is an error; a database
192+
// ahead of the binary is tolerated (compatibility mode), matching RunUp.
193+
func VerifyMigrated(ctx context.Context, url string, sources []Source) error {
194+
if len(sources) == 0 {
195+
return nil
196+
}
197+
if err := validateSources(sources); err != nil {
198+
return err
199+
}
200+
// Reject the same resolved-schema collisions RunUp rejects: a colliding
201+
// source set shares one tracking table, so verification would read the
202+
// same row twice and "pass" an unsafe configuration.
203+
if err := checkResolvedSchemaCollisions(ctx, url, sources); err != nil {
204+
return err
205+
}
206+
207+
db, err := sql.Open("pgx", url)
208+
if err != nil {
209+
return fmt.Errorf("open database to verify migrations: %w", err)
210+
}
211+
defer db.Close()
212+
213+
for _, src := range sources {
214+
if err := ctx.Err(); err != nil {
215+
return fmt.Errorf("migration verification cancelled before %s: %w", src.Name, err)
216+
}
217+
maxVer, err := maxEmbeddedVersion(src.FS, src.Dir)
218+
if err != nil {
219+
return fmt.Errorf("determine max embedded version for %s: %w", src.Name, err)
220+
}
221+
222+
// For Schema == "" the unqualified name resolves via the connection's
223+
// search_path — the same place RunUp put the table.
224+
table := quoteIdentifier(src.TrackingTable)
225+
if src.Schema != "" {
226+
table = quoteIdentifier(src.Schema) + "." + table
227+
}
228+
229+
var exists bool
230+
if err := db.QueryRowContext(ctx, "SELECT to_regclass($1) IS NOT NULL", table).Scan(&exists); err != nil {
231+
return fmt.Errorf("check tracking table for %s: %w", src.Name, err)
232+
}
233+
if !exists {
234+
return fmt.Errorf("source %s: tracking table %s does not exist - the database has not been migrated; apply migrations out-of-band or unset SKIP_MIGRATIONS", src.Name, table)
235+
}
236+
237+
var version int64
238+
var dirty bool
239+
err = db.QueryRowContext(ctx, "SELECT version, dirty FROM "+table+" LIMIT 1").Scan(&version, &dirty)
240+
if errors.Is(err, sql.ErrNoRows) {
241+
version, dirty = 0, false // table exists but nothing applied yet
242+
} else if err != nil {
243+
return fmt.Errorf("read tracking table for %s: %w", src.Name, err)
244+
}
245+
246+
switch {
247+
case dirty:
248+
return fmt.Errorf("source %s is dirty at version %d: a previous migration attempt failed and must be resolved before starting with SKIP_MIGRATIONS", src.Name, version)
249+
case version < int64(maxVer):
250+
return fmt.Errorf("source %s is at version %d but this binary requires version %d: apply migrations out-of-band or unset SKIP_MIGRATIONS", src.Name, version, maxVer)
251+
case version > int64(maxVer):
252+
log.Info("database schema is ahead of this binary; running in compatibility mode",
253+
"track", src.Name, "dbVersion", version, "binaryMax", maxVer)
254+
}
255+
}
256+
return nil
257+
}
258+
182259
// validateSources rejects a source set that cannot be run safely. It checks two
183260
// things.
184261
//

go/core/pkg/migrations/runner_test.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"maps"
9+
"net/url"
910
"strings"
1011
"testing"
1112
"testing/fstest"
@@ -972,3 +973,110 @@ func TestApplySource_DirtyStateRecoveryOnRestart(t *testing.T) {
972973
t.Errorf("after restart: version = %d, want 1 (dirty cleared, rolled back)", got)
973974
}
974975
}
976+
977+
// TestVerifyMigrated covers the SKIP_MIGRATIONS boot guard: refuse an
978+
// un-migrated or behind or dirty database, tolerate exact-match and ahead
979+
// (compatibility mode), and work on a connection whose role has no DDL
980+
// privileges — the deployment mode the guard exists for.
981+
func TestVerifyMigrated(t *testing.T) {
982+
ctx := context.Background()
983+
connStr := startTestDB(t)
984+
full := []Source{coreSource(goodCoreFS)} // max embedded version 2
985+
986+
t.Run("unmigrated database is refused", func(t *testing.T) {
987+
err := VerifyMigrated(ctx, connStr, full)
988+
if err == nil || !strings.Contains(err.Error(), "has not been migrated") {
989+
t.Fatalf("error = %v, want missing-tracking-table refusal", err)
990+
}
991+
})
992+
993+
t.Run("pending migrations are refused", func(t *testing.T) {
994+
if _, err := applySource(ctx, connStr, coreSource(oneCoreFS)); err != nil {
995+
t.Fatalf("apply v1: %v", err)
996+
}
997+
err := VerifyMigrated(ctx, connStr, full)
998+
if err == nil || !strings.Contains(err.Error(), "requires version 2") {
999+
t.Fatalf("error = %v, want behind-binary refusal", err)
1000+
}
1001+
})
1002+
1003+
t.Run("fully migrated database passes", func(t *testing.T) {
1004+
if _, err := applySource(ctx, connStr, coreSource(goodCoreFS)); err != nil {
1005+
t.Fatalf("apply v2: %v", err)
1006+
}
1007+
if err := VerifyMigrated(ctx, connStr, full); err != nil {
1008+
t.Fatalf("VerifyMigrated() = %v, want nil", err)
1009+
}
1010+
})
1011+
1012+
t.Run("works without DDL privileges", func(t *testing.T) {
1013+
db, err := sql.Open("pgx", connStr)
1014+
if err != nil {
1015+
t.Fatal(err)
1016+
}
1017+
defer db.Close()
1018+
for _, q := range []string{
1019+
`CREATE ROLE readonly LOGIN PASSWORD 'ro'`,
1020+
`GRANT USAGE ON SCHEMA public TO readonly`,
1021+
`GRANT SELECT ON ALL TABLES IN SCHEMA public TO readonly`,
1022+
} {
1023+
if _, err := db.ExecContext(ctx, q); err != nil {
1024+
t.Fatalf("%s: %v", q, err)
1025+
}
1026+
}
1027+
u, err := url.Parse(connStr)
1028+
if err != nil {
1029+
t.Fatal(err)
1030+
}
1031+
u.User = url.UserPassword("readonly", "ro")
1032+
if err := VerifyMigrated(ctx, u.String(), full); err != nil {
1033+
t.Fatalf("VerifyMigrated() as readonly = %v, want nil", err)
1034+
}
1035+
})
1036+
1037+
t.Run("database ahead of binary passes", func(t *testing.T) {
1038+
if err := VerifyMigrated(ctx, connStr, []Source{coreSource(oneCoreFS)}); err != nil {
1039+
t.Fatalf("VerifyMigrated() with older binary = %v, want nil (compatibility mode)", err)
1040+
}
1041+
})
1042+
1043+
t.Run("dirty tracking table is refused", func(t *testing.T) {
1044+
db, err := sql.Open("pgx", connStr)
1045+
if err != nil {
1046+
t.Fatal(err)
1047+
}
1048+
defer db.Close()
1049+
if _, err := db.ExecContext(ctx, `UPDATE schema_migrations SET dirty = true`); err != nil {
1050+
t.Fatal(err)
1051+
}
1052+
defer func() {
1053+
if _, err := db.ExecContext(ctx, `UPDATE schema_migrations SET dirty = false`); err != nil {
1054+
t.Fatal(err)
1055+
}
1056+
}()
1057+
err = VerifyMigrated(ctx, connStr, full)
1058+
if err == nil || !strings.Contains(err.Error(), "dirty") {
1059+
t.Fatalf("error = %v, want dirty refusal", err)
1060+
}
1061+
})
1062+
1063+
t.Run("resolved schema collision is refused", func(t *testing.T) {
1064+
// Schema "" resolves to public here, colliding with the explicit
1065+
// "public" source on the same tracking table — the same source set
1066+
// RunUp rejects.
1067+
collide := []Source{
1068+
coreSource(goodCoreFS),
1069+
{Name: "explicit", Schema: "public", TrackingTable: "schema_migrations", FS: goodCoreFS, Dir: "core"},
1070+
}
1071+
err := VerifyMigrated(ctx, connStr, collide)
1072+
if err == nil || !strings.Contains(err.Error(), "resolve to the same tracking table") {
1073+
t.Fatalf("error = %v, want resolved-schema collision refusal", err)
1074+
}
1075+
})
1076+
1077+
t.Run("no sources is a no-op", func(t *testing.T) {
1078+
if err := VerifyMigrated(ctx, "postgres://unused", nil); err != nil {
1079+
t.Fatalf("VerifyMigrated() with no sources = %v, want nil", err)
1080+
}
1081+
})
1082+
}

helm/kagent/templates/NOTES.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,16 @@ DOCUMENTATION:
9292
database.postgres.bundled.enabled=false
9393
{{- end }}
9494
{{- end }}
95+
{{- if .Values.database.postgres.skipMigrations }}
96+
################################################################################
97+
# NOTE: STARTUP MIGRATIONS ARE DISABLED #
98+
################################################################################
99+
database.postgres.skipMigrations is set: the controller will not run database
100+
migrations at startup. It verifies the schema is already migrated and fails
101+
to start if it is not.
102+
103+
Ensure migrations are applied out-of-band before installing or upgrading.
104+
{{- end }}
95105
{{- if .Values.substrate.enabled }}
96106
################################################################################
97107
# WARNING: SUBSTRATE IS EXPERIMENTAL, USE AT OWN RISK #

helm/kagent/templates/controller-configmap.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ data:
5555
PROXY_URL: {{ .Values.proxy.url | quote }}
5656
{{- end }}
5757
DATABASE_VECTOR_ENABLED: {{ .Values.database.postgres.vectorEnabled | quote }}
58+
SKIP_MIGRATIONS: {{ .Values.database.postgres.skipMigrations | default false | quote }}
5859
WATCH_NAMESPACES: {{ include "kagent.watchNamespaces" . | quote }}
5960
MCP_EGRESS_PLAINTEXT: {{ .Values.controller.mcpEgressPlaintext | default false | quote }}
6061
{{- if .Values.controller.a2aClientTimeout }}

helm/kagent/values.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ database:
7979
# Required to use features that depend on database vector capability. (e.g. long-term memory)
8080
# Set to true when using an external PostgreSQL that has the pgvector extension installed.
8181
vectorEnabled: false
82+
# -- Skip running database migrations at controller startup.
83+
# The controller instead verifies the database is already migrated and fails if it is not.
84+
# Migrations must be applied out-of-band (e.g. from a CI/CD pipeline) before install/upgrade.
85+
skipMigrations: false
8286
# -- Bundled PostgreSQL instance — for development and evaluation only.
8387
# Not suitable for production. Deployed when enabled is true and url/urlFile are not set.
8488
bundled:

0 commit comments

Comments
 (0)