make sqlite migrations allow launcher downgrades

ee/agent/storage/sqlite/keyvalue_store_sqlite.go

@@ -10,6 +10,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"regexp"
 	"strings"
 
 	"github.com/golang-migrate/migrate/v4"
@@ -24,6 +25,8 @@ const (
 	StartupSettingsStore storeName = iota
 )
 
+var missingMigrationErrFormat = regexp.MustCompile(`no migration found for version \d+`)
+
 // String translates the exported int constant to the actual name of the
 // supported table in the sqlite database.
 func (s storeName) String() string {
@@ -170,7 +173,9 @@ func (s *sqliteStore) migrate(ctx context.Context) error {
 		return fmt.Errorf("creating migrate instance: %w", err)
 	}
 
-	if err := m.Up(); err != nil && !errors.Is(err, migrate.ErrNoChange) {
+	// 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) {
 		return fmt.Errorf("running migrations: %w", err)
 	}
 
@@ -317,3 +322,7 @@ ON CONFLICT (name) DO UPDATE SET value=excluded.value;`
 
 	return deletedKeys, nil
 }
+
+func isMissingMigrationError(err error) bool {
+	return missingMigrationErrFormat.MatchString(err.Error())
+}

ee/agent/storage/sqlite/keyvalue_store_sqlite_test.go

@@ -234,3 +234,36 @@ func Test_Migrations(t *testing.T) {
 	require.NoError(t, srcErr, "source error closing migration")
 	require.NoError(t, dbErr, "database error closing migration")
 }
+
+func Test_MissingMigrations(t *testing.T) {
+	t.Parallel()
+
+	tempRootDir := t.TempDir()
+
+	conn, err := validatedDbConn(context.TODO(), tempRootDir)
+	require.NoError(t, err, "setting up db connection")
+	require.NoError(t, conn.Close(), "closing test db")
+
+	d, err := iofs.New(migrations, "migrations")
+	require.NoError(t, err, "loading migration files")
+
+	m, err := migrate.NewWithSourceInstance("iofs", d, fmt.Sprintf("sqlite://%s?query", dbLocation(tempRootDir)))
+	require.NoError(t, err, "creating migrate instance")
+	require.NoError(t, m.Up(), "expected no error running all migrations")
+	currentVersion, dirty, err := m.Version()
+	require.NoError(t, err, "error looking for current version")
+	require.False(t, dirty, "did not expect dirty migration state")
+	missingMigrationVersion := int(currentVersion) + 1
+	forceVersionErr := m.Force(missingMigrationVersion)
+	require.NoError(t, forceVersionErr, "error forcing version")
+
+	srcErr, dbErr := m.Close()
+	require.NoError(t, srcErr, "source error closing migration")
+	require.NoError(t, dbErr, "database error closing migration")
+
+	// now re-open and re-attempt migrations, this will only work if we correctly ignore the missing
+	// migration file error
+	s, migrationError := OpenRW(context.TODO(), tempRootDir, StartupSettingsStore)
+	require.NoError(t, migrationError, "database error running missing migration")
+	require.NoError(t, s.Close(), "error closing sqliteStore conn")
+}

ee/agent/storage/sqlite/migrations.md

@@ -15,3 +15,6 @@ Add your SQL to these files. Adhere to best practices, including:
 1. All migrations should be able to run multiple times without error (e.g. use `CREATE TABLE IF NOT EXISTS` instead of `CREATE TABLE`)
 
 The database will automatically apply new migrations on startup.
+
+Note that because we currently package our migration files with launcher, it is possible for us to run a migration and then downgrade to a version of launcher which cannot find the corresponding migration file.
+To avoid this issue, we ignore missing migration file errors when migrating up on startup. This is enough for now, because all migrations can be run multiple times without error (see best practices note above). If these migrations become more complex, (e.g. table altercations that will break prior launcher versions, instead of completely new tables), we will need to explore another path forward here (likely hosting migrations with rollbacks outside of launcher, so that older versions can rollback newer migrations).