Alerting: add safeguard for migrations that might cause dataloss (#48526)

* Alerting: add safeguard for migrations that might cause dataloss

* add test for panic

* add documentation
This commit is contained in:
Jean-Philippe Quéméner 2022-05-02 10:38:42 +02:00 committed by GitHub
parent a5c570a5f6
commit 0a87ef06af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 37 additions and 0 deletions

View File

@ -9,6 +9,9 @@ app_mode = production
# instance name, defaults to HOSTNAME environment variable value or hostname if HOSTNAME var is empty
instance_name = ${HOSTNAME}
# force migration will run migrations that might cause dataloss
force_migration = false
#################################### Paths ###############################
[paths]
# Path to where grafana can store temp files, sessions, and the sqlite3 db (if that is used)

View File

@ -9,6 +9,9 @@
# instance name, defaults to HOSTNAME environment variable value or hostname if HOSTNAME var is empty
;instance_name = ${HOSTNAME}
# force migration will run migrations that might cause dataloss
;force_migration = false
#################################### Paths ####################################
[paths]
# Path to where grafana can store temp files, sessions, and the sqlite3 db (if that is used)

View File

@ -135,6 +135,10 @@ Options are `production` and `development`. Default is `production`. _Do not_ ch
Set the name of the grafana-server instance. Used in logging, internal metrics, and clustering info. Defaults to: `${HOSTNAME}`, which will be replaced with
environment variable `HOSTNAME`, if that is empty or does not exist Grafana will try to use system calls to get the machine name.
### force_migration
Force migration will run migrations that might cause data loss. Default is `false`.
<hr />
## [paths]

View File

@ -29,6 +29,7 @@ func TestAddDashAlertMigration(t *testing.T) {
name string
config *setting.Cfg
isMigrationRun bool
shouldPanic bool
expected []string // set of migration titles
}{
{
@ -47,6 +48,18 @@ func TestAddDashAlertMigration(t *testing.T) {
UnifiedAlerting: setting.UnifiedAlertingSettings{
Enabled: boolPointer(false),
},
ForceMigration: true,
},
isMigrationRun: true,
expected: []string{fmt.Sprintf(ualert.ClearMigrationEntryTitle, ualert.MigTitle), ualert.RmMigTitle},
},
{
name: "when unified alerting disabled, migration is already run and force migration is disabled, then the migration should panic",
config: &setting.Cfg{
UnifiedAlerting: setting.UnifiedAlertingSettings{
Enabled: boolPointer(false),
},
ForceMigration: false,
},
isMigrationRun: true,
expected: []string{fmt.Sprintf(ualert.ClearMigrationEntryTitle, ualert.MigTitle), ualert.RmMigTitle},
@ -75,6 +88,12 @@ func TestAddDashAlertMigration(t *testing.T) {
for _, tt := range tc {
t.Run(tt.name, func(t *testing.T) {
defer func() {
// if the code should panic, make sure it has
if r := recover(); r == nil && tt.shouldPanic {
t.Errorf("The code did not panic")
}
}()
if tt.isMigrationRun {
log := migrator.MigrationLog{
MigrationID: ualert.MigTitle,
@ -90,6 +109,7 @@ func TestAddDashAlertMigration(t *testing.T) {
}
mg := migrator.NewMigrator(x, tt.config)
ualert.AddDashAlertMigration(mg)
require.Equal(t, tt.expected, mg.GetMigrationIDs(false))
})

View File

@ -73,6 +73,10 @@ func AddDashAlertMigration(mg *migrator.Migrator) {
silences: make(map[int64][]*pb.MeshSilence),
})
case !mg.Cfg.UnifiedAlerting.IsEnabled() && migrationRun:
// Safeguard to prevent data loss when migrating from UA to LA
if !mg.Cfg.ForceMigration {
panic("New alert rules created while using unified alerting will be deleted, set force_migration=true in your grafana.ini and try again if this is okay.")
}
// Remove the migration entry that creates unified alerting data. This is so when the feature
// flag is enabled in the future the migration "move dashboard alerts to unified alerting" will be run again.
mg.AddMigration(fmt.Sprintf(clearMigrationEntryTitle, migTitle), &clearMigrationEntry{

View File

@ -386,6 +386,8 @@ type Cfg struct {
Env string
ForceMigration bool
// Analytics
CheckForGrafanaUpdates bool
CheckForPluginUpdates bool
@ -886,6 +888,7 @@ func (cfg *Cfg) Load(args CommandLineArgs) error {
Env = valueAsString(iniFile.Section(""), "app_mode", "development")
cfg.Env = Env
cfg.ForceMigration = iniFile.Section("").Key("force_migration").MustBool(false)
InstanceName = valueAsString(iniFile.Section(""), "instance_name", "unknown_instance_name")
plugins := valueAsString(iniFile.Section("paths"), "plugins", "")
cfg.PluginsPath = makeAbsolute(plugins, HomePath)