Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force migration rather than halting launcher startup #2069

Merged
merged 1 commit into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion ee/agent/storage/sqlite/keyvalue_store_sqlite.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,21 @@ func (s *sqliteStore) migrate() error {

// don't prevent DB access for a missing migration, this is the result of a downgrade after previously
// running a migration
if err := m.Up(); err != nil && !errors.Is(err, migrate.ErrNoChange) && !isMissingMigrationError(err) {
if err := m.Up(); err != nil {
// Not actually errors for us -- we're in a successful state
if errors.Is(err, migrate.ErrNoChange) || isMissingMigrationError(err) {
return nil
}

// If we need to force, do that
if errDirty, ok := err.(migrate.ErrDirty); ok {
if err := m.Force(errDirty.Version); err != nil {
return fmt.Errorf("forcing migration version %d: %w", errDirty.Version, err)
}
return nil
zackattack01 marked this conversation as resolved.
Show resolved Hide resolved
}

// Some other error -- return it
return fmt.Errorf("running migrations: %w", err)
}

Expand Down
28 changes: 28 additions & 0 deletions ee/agent/storage/sqlite/keyvalue_store_sqlite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"

"github.com/golang-migrate/migrate/v4"
"github.com/golang-migrate/migrate/v4/database/sqlite"
"github.com/golang-migrate/migrate/v4/source/iofs"
"github.com/kolide/launcher/ee/agent/flags/keys"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -68,6 +69,33 @@ func TestOpenRW_DatabaseIsCorrupt(t *testing.T) {
require.NoError(t, s.Close(), "closing test store")
}

func TestOpenRW_DatabaseIsDirty(t *testing.T) {
t.Parallel()

testRootDir := t.TempDir()

// Create the database
s, err := OpenRW(context.TODO(), testRootDir, StartupSettingsStore)
require.NoError(t, err, "expected no error creating test store")

// Mark the migration as dirty
_, err = s.conn.Exec(fmt.Sprintf(`UPDATE %s SET dirty = 1;`, sqlite.DefaultMigrationsTable))
require.NoError(t, err, "marking migration as dirty")

// Close the connection
require.NoError(t, s.Close(), "expected no error closing test store")

// Open a new connection and expect that it succeeds, forcing the dirty migration
s2, err := OpenRW(context.TODO(), testRootDir, StartupSettingsStore)
require.NoError(t, err, "expected no error opening test store with dirty migration")
require.NoError(t, s2.Close(), "expected no error closing test store")

// Open and close again successfully just to be sure
s3, err := OpenRW(context.TODO(), testRootDir, StartupSettingsStore)
require.NoError(t, err, "expected no error opening test store with dirty migration")
require.NoError(t, s3.Close(), "expected no error closing test store")
}

func TestOpenRW_InvalidTable(t *testing.T) {
t.Parallel()

Expand Down
Loading