Skip to content

Commit 4d1d10f

Browse files
authored
Merge pull request #2441 from dgageot/board/docker-agent-session-loading-sql-error-c098b121
fix: detect newer session database and show clear upgrade message
2 parents 770939b + d4a1634 commit 4d1d10f

3 files changed

Lines changed: 66 additions & 2 deletions

File tree

pkg/session/migrations.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"time"
1111

1212
"github.com/google/uuid"
13+
14+
"github.com/docker/docker-agent/pkg/version"
1315
)
1416

1517
// Migration represents a database migration
@@ -42,6 +44,11 @@ func (m *MigrationManager) InitializeMigrations(ctx context.Context) error {
4244
return fmt.Errorf("failed to create migrations table: %w", err)
4345
}
4446

47+
// Check if the database was created by a newer version of the application
48+
if err := m.checkForUnknownMigrations(ctx); err != nil {
49+
return err
50+
}
51+
4552
// Run all pending migrations
4653
err = m.RunPendingMigrations(ctx)
4754
if err != nil {
@@ -64,6 +71,30 @@ func (m *MigrationManager) createMigrationsTable(ctx context.Context) error {
6471
return err
6572
}
6673

74+
// checkForUnknownMigrations checks if the database has migrations that this binary
75+
// doesn't know about, which indicates the database was created by a newer version.
76+
// This produces a clear error instead of cryptic SQL failures from schema mismatches.
77+
func (m *MigrationManager) checkForUnknownMigrations(ctx context.Context) error {
78+
known := getAllMigrations()
79+
maxKnownID := known[len(known)-1].ID
80+
81+
var maxAppliedID int
82+
err := m.db.QueryRowContext(ctx, "SELECT COALESCE(MAX(id), 0) FROM migrations").Scan(&maxAppliedID)
83+
if err != nil {
84+
return fmt.Errorf("failed to check applied migrations: %w", err)
85+
}
86+
87+
if maxAppliedID > maxKnownID {
88+
return fmt.Errorf(
89+
"%w: you are running docker-agent %s which supports migrations up to %d, "+
90+
"but the session database has migration %d from a newer version; "+
91+
"please upgrade docker-agent to the latest version",
92+
ErrNewerDatabase, version.Version, maxKnownID, maxAppliedID)
93+
}
94+
95+
return nil
96+
}
97+
6798
// RunPendingMigrations executes all migrations that haven't been applied yet
6899
func (m *MigrationManager) RunPendingMigrations(ctx context.Context) error {
69100
migrations := getAllMigrations()

pkg/session/store.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@ import (
1919
)
2020

2121
var (
22-
ErrEmptyID = errors.New("session ID cannot be empty")
23-
ErrNotFound = errors.New("session not found")
22+
ErrEmptyID = errors.New("session ID cannot be empty")
23+
ErrNotFound = errors.New("session not found")
24+
ErrNewerDatabase = errors.New("session database was created by a newer version of docker-agent")
2425
)
2526

2627
// parseRelativeSessionRef checks if ref is a relative session reference (e.g., "-1", "-2")
@@ -367,6 +368,12 @@ func (s *InMemorySessionStore) Close() error {
367368
func NewSQLiteSessionStore(path string) (Store, error) {
368369
store, err := openAndMigrateSQLiteStore(path)
369370
if err != nil {
371+
// Don't attempt recovery for version mismatch - the user needs to upgrade,
372+
// not silently lose their data by starting fresh.
373+
if errors.Is(err, ErrNewerDatabase) {
374+
return nil, err
375+
}
376+
370377
// If migrations failed, try to recover by backing up the database and starting fresh
371378
slog.Warn("Failed to open session store, attempting recovery", "error", err)
372379

pkg/session/store_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,32 @@ func TestAgentModelOverrides_EmptyMap(t *testing.T) {
629629
assert.Empty(t, retrieved.AgentModelOverrides)
630630
}
631631

632+
func TestNewSQLiteSessionStore_RejectsNewerDatabase(t *testing.T) {
633+
tempDir := t.TempDir()
634+
dbPath := filepath.Join(tempDir, "test_newer_db.db")
635+
636+
// Create a valid store first (applies all known migrations)
637+
store, err := NewSQLiteSessionStore(dbPath)
638+
require.NoError(t, err)
639+
defer store.(*SQLiteSessionStore).Close()
640+
641+
// Inject a future migration into the database to simulate a newer version
642+
db, err := sql.Open("sqlite", dbPath)
643+
require.NoError(t, err)
644+
_, err = db.Exec(
645+
"INSERT INTO migrations (id, name, description, applied_at) VALUES (?, ?, ?, ?)",
646+
9999, "9999_future_migration", "Added by a newer version", "2099-01-01T00:00:00Z")
647+
require.NoError(t, err)
648+
db.Close()
649+
650+
// Opening the store should fail with a clear error about version mismatch
651+
_, err = NewSQLiteSessionStore(dbPath)
652+
require.Error(t, err)
653+
require.ErrorIs(t, err, ErrNewerDatabase)
654+
assert.Contains(t, err.Error(), "9999")
655+
assert.Contains(t, err.Error(), "upgrade docker-agent")
656+
}
657+
632658
func TestNewSQLiteSessionStore_MigrationFailureRecovery(t *testing.T) {
633659
tempDir := t.TempDir()
634660
dbPath := filepath.Join(tempDir, "test_migration_recovery.db")

0 commit comments

Comments
 (0)