From d0765cb9ee8caa23c401ad720e1711e34c54de9a Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Wed, 8 Dec 2021 19:50:00 -0300 Subject: [PATCH] Do not exit on `serve`/`prepare` if there are unknown migrations (#3262) * Do not exit on serve/prepare if there are unknown migrations * PR review changes --- cmd/fleet/prepare.go | 8 +++---- cmd/fleet/serve.go | 8 +++---- cmd/fleetctl/debug.go | 1 - server/datastore/mysql/migrations_test.go | 14 ++++++++++++ server/datastore/mysql/mysql.go | 24 ++++++++++++++++++++- server/datastore/mysql/mysql_test.go | 26 ++++++++++++++++++++--- 6 files changed, 68 insertions(+), 13 deletions(-) diff --git a/cmd/fleet/prepare.go b/cmd/fleet/prepare.go index fde33a418d..31cd1b6f30 100644 --- a/cmd/fleet/prepare.go +++ b/cmd/fleet/prepare.go @@ -74,16 +74,16 @@ To setup Fleet infrastructure, use one of the available commands. } case fleet.UnknownMigrations: fmt.Printf("################################################################################\n"+ - "# ERROR:\n"+ + "# WARNING:\n"+ "# Your Fleet database has unrecognized migrations. This could happen when\n"+ "# running an older version of Fleet on a newer migrated database.\n"+ "#\n"+ "# Unknown migrations: tables=%v, data=%v.\n"+ - "#\n"+ - "# Upgrade Fleet server version.\n"+ "################################################################################\n", status.UnknownTable, status.UnknownData) - os.Exit(1) + if dev { + os.Exit(1) + } } if err := ds.MigrateTables(cmd.Context()); err != nil { diff --git a/cmd/fleet/serve.go b/cmd/fleet/serve.go index d416170cc5..74e206d477 100644 --- a/cmd/fleet/serve.go +++ b/cmd/fleet/serve.go @@ -180,16 +180,16 @@ the way that the Fleet server works. // OK case fleet.UnknownMigrations: fmt.Printf("################################################################################\n"+ - "# ERROR:\n"+ + "# WARNING:\n"+ "# Your Fleet database has unrecognized migrations. This could happen when\n"+ "# running an older version of Fleet on a newer migrated database.\n"+ "#\n"+ "# Unknown migrations: tables=%v, data=%v.\n"+ - "#\n"+ - "# Upgrade Fleet server version.\n"+ "################################################################################\n", migrationStatus.UnknownTable, migrationStatus.UnknownData) - os.Exit(1) + if dev { + os.Exit(1) + } case fleet.SomeMigrationsCompleted: fmt.Printf("################################################################################\n"+ "# WARNING:\n"+ diff --git a/cmd/fleetctl/debug.go b/cmd/fleetctl/debug.go index be2feeb9da..ce41a6570d 100644 --- a/cmd/fleetctl/debug.go +++ b/cmd/fleetctl/debug.go @@ -499,7 +499,6 @@ Such migrations can be applied via "fleet prepare db" before running "fleet serv case fleet.AllMigrationsCompleted: fmt.Println("Migrations up-to-date.") case fleet.UnknownMigrations: - // Shouldn't happen, because fleet serve won't be running if this is the case. fmt.Printf("Unknown migrations detected: tables=%v, data=%v.\n", migrationStatus.UnknownTable, migrationStatus.UnknownData) case fleet.SomeMigrationsCompleted: diff --git a/server/datastore/mysql/migrations_test.go b/server/datastore/mysql/migrations_test.go index c53dca1ef2..794cbf0b61 100644 --- a/server/datastore/mysql/migrations_test.go +++ b/server/datastore/mysql/migrations_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/fleetdm/fleet/v4/server/config" + "github.com/fleetdm/fleet/v4/server/datastore/mysql/migrations/tables" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -38,6 +39,19 @@ func TestMigrationStatus(t *testing.T) { assert.EqualValues(t, fleet.AllMigrationsCompleted, status.StatusCode) assert.Empty(t, status.MissingTable) assert.Empty(t, status.MissingData) + + // Insert unknown migration. + ds.writer.Exec(`INSERT INTO ` + tables.MigrationClient.TableName + ` (version_id, is_applied) VALUES (1638994765, 1)`) + status, err = ds.MigrationStatus(context.Background()) + require.NoError(t, err) + assert.EqualValues(t, fleet.UnknownMigrations, status.StatusCode) + ds.writer.Exec(`DELETE FROM ` + tables.MigrationClient.TableName + ` WHERE version_id = 1638994765`) + + status, err = ds.MigrationStatus(context.Background()) + require.NoError(t, err) + assert.EqualValues(t, fleet.AllMigrationsCompleted, status.StatusCode) + assert.Empty(t, status.MissingTable) + assert.Empty(t, status.MissingData) } func TestMigrations(t *testing.T) { diff --git a/server/datastore/mysql/mysql.go b/server/datastore/mysql/mysql.go index 6809f94692..c0ec5f81d6 100644 --- a/server/datastore/mysql/mysql.go +++ b/server/datastore/mysql/mysql.go @@ -361,12 +361,14 @@ func (d *Datastore) MigrationStatus(ctx context.Context) (*fleet.MigrationStatus missingTable, unknownTable, equalTable := compareVersions( getVersionsFromMigrations(knownTable), appliedTable, + knownUnknownTableMigrations, ) knownData := data.MigrationClient.Migrations missingData, unknownData, equalData := compareVersions( getVersionsFromMigrations(knownData), appliedData, + knownUnknownDataMigrations, ) if equalData && equalTable { @@ -393,9 +395,28 @@ func (d *Datastore) MigrationStatus(ctx context.Context) (*fleet.MigrationStatus }, nil } +var ( + knownUnknownTableMigrations = map[int64]struct{}{ + // This migration was introduced incorrectly in fleet-v4.4.0 and its + // timestamp was changed in fleet-v4.4.1. + 20210924114500: {}, + } + knownUnknownDataMigrations = map[int64]struct{}{} +) + +func unknownUnknowns(in []int64, knownUnknowns map[int64]struct{}) []int64 { + var result []int64 + for _, t := range in { + if _, ok := knownUnknowns[t]; !ok { + result = append(result, t) + } + } + return result +} + // compareVersions returns any missing or extra elements in v2 with respect to v1 // (v1 or v2 need not be ordered). -func compareVersions(v1, v2 []int64) (missing []int64, unknown []int64, equal bool) { +func compareVersions(v1, v2 []int64, knownUnknowns map[int64]struct{}) (missing []int64, unknown []int64, equal bool) { v1s := make(map[int64]struct{}) for _, m := range v1 { v1s[m] = struct{}{} @@ -414,6 +435,7 @@ func compareVersions(v1, v2 []int64) (missing []int64, unknown []int64, equal bo unknown = append(unknown, m) } } + unknown = unknownUnknowns(unknown, knownUnknowns) if len(missing) == 0 && len(unknown) == 0 { return nil, nil, true } diff --git a/server/datastore/mysql/mysql_test.go b/server/datastore/mysql/mysql_test.go index 55412fe78e..c2c32f0cf9 100644 --- a/server/datastore/mysql/mysql_test.go +++ b/server/datastore/mysql/mysql_test.go @@ -805,8 +805,9 @@ func TestCompareVersions(t *testing.T) { for _, tc := range []struct { name string - v1 []int64 - v2 []int64 + v1 []int64 + v2 []int64 + knownUnknowns map[int64]struct{} expMissing []int64 expUnknown []int64 @@ -858,6 +859,25 @@ func TestCompareVersions(t *testing.T) { expUnknown: []int64{4}, expEqual: false, }, + { + name: "known-unknown", + v1: []int64{1, 2, 3}, + v2: []int64{1, 2, 3, 4}, + knownUnknowns: map[int64]struct{}{ + 4: {}, + }, + expEqual: true, + }, + { + name: "unknowns", + v1: []int64{1, 2, 3}, + v2: []int64{1, 2, 3, 4, 5}, + expUnknown: []int64{5}, + knownUnknowns: map[int64]struct{}{ + 4: {}, + }, + expEqual: false, + }, { name: "missing-and-unknown", v1: []int64{1, 2, 3}, @@ -868,7 +888,7 @@ func TestCompareVersions(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - missing, unknown, equal := compareVersions(tc.v1, tc.v2) + missing, unknown, equal := compareVersions(tc.v1, tc.v2, tc.knownUnknowns) require.Equal(t, tc.expMissing, missing) require.Equal(t, tc.expUnknown, unknown) require.Equal(t, tc.expEqual, equal)