Alerting: Make Unified Alerting enabled by default for those who do not use legacy alerting (#42200)

* update AlertingEnabled and UnifiedAlertingSettings.Enabled to be pointers
* add a pseudo migration to fix the AlertingEnabled and UnifiedAlertingSettings.Enabled if the latter is not defined
* update the default configuration file to make default value for both 'enabled' flags be undefined

Misc
* update Migrator to expose DB engine. This is needed for a ualert migration to access the database while the list of migrations is created.
* add more verbose failure when migrations do not match

Co-authored-by: gotjosh <josue@grafana.com>
Co-authored-by: Yuriy Tseretyan <yuriy.tseretyan@grafana.com>
Co-authored-by: gillesdemey <gilles.de.mey@gmail.com>

pkg/services/sqlstore/migrations/migrations.go

@@ -1,6 +1,8 @@
 package migrations
 
 import (
+	"os"
+
 	"github.com/grafana/grafana/pkg/services/sqlstore/migrations/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/sqlstore/migrations/ualert"
 	. "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
@@ -46,6 +48,11 @@ func (*OSSMigrations) AddMigration(mg *Migrator) {
 	addUserAuthTokenMigrations(mg)
 	addCacheMigration(mg)
 	addShortURLMigrations(mg)
+	// TODO Delete when unified alerting is enabled by default unconditionally (Grafana v9)
+	if err := ualert.CheckUnifiedAlertingEnabledByDefault(mg); err != nil { // this should always go before any other ualert migration
+		mg.Logger.Error("failed to determine the status of alerting engine. Enable either legacy or unified alerting explicitly and try again", "err", err)
+		os.Exit(1)
+	}
 	ualert.AddTablesMigrations(mg)
 	ualert.AddDashAlertMigration(mg)
 	addLibraryElementsMigrations(mg)

pkg/services/sqlstore/migrations/migrations_test.go

@@ -1,13 +1,16 @@
 package migrations
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+	"xorm.io/xorm"
+
 	. "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
 	"github.com/grafana/grafana/pkg/services/sqlstore/sqlutil"
 	"github.com/grafana/grafana/pkg/setting"
-	"github.com/stretchr/testify/require"
-	"xorm.io/xorm"
 )
 
 func TestMigrations(t *testing.T) {
@@ -27,7 +30,7 @@ func TestMigrations(t *testing.T) {
 	mg := NewMigrator(x, &setting.Cfg{})
 	migrations := &OSSMigrations{}
 	migrations.AddMigration(mg)
-	expectedMigrations := mg.MigrationsCount()
+	expectedMigrations := mg.GetMigrationIDs(true)
 
 	err = mg.Start()
 	require.NoError(t, err)
@@ -36,7 +39,7 @@ func TestMigrations(t *testing.T) {
 	require.NoError(t, err)
 	require.True(t, has)
 
-	require.Equal(t, expectedMigrations, result.Count)
+	checkStepsAndDatabaseMatch(t, mg, expectedMigrations)
 
 	mg = NewMigrator(x, &setting.Cfg{})
 	migrations.AddMigration(mg)
@@ -47,5 +50,44 @@ func TestMigrations(t *testing.T) {
 	has, err = x.SQL(query).Get(&result)
 	require.NoError(t, err)
 	require.True(t, has)
-	require.Equal(t, expectedMigrations, result.Count)
+	checkStepsAndDatabaseMatch(t, mg, expectedMigrations)
+}
+
+func checkStepsAndDatabaseMatch(t *testing.T, mg *Migrator, expected []string) {
+	t.Helper()
+	log, err := mg.GetMigrationLog()
+	require.NoError(t, err)
+	missing := make([]string, 0)
+	for _, id := range expected {
+		_, ok := log[id]
+		if !ok {
+			missing = append(missing, id)
+		}
+	}
+	notIntended := make([]string, 0)
+	for logId := range log {
+		found := false
+		for _, s := range expected {
+			found = s == logId
+			if found {
+				break
+			}
+		}
+		if !found {
+			notIntended = append(notIntended, logId)
+		}
+	}
+
+	if len(missing) == 0 && len(notIntended) == 0 {
+		return
+	}
+
+	var msg string
+	if len(missing) > 0 {
+		msg = fmt.Sprintf("was not executed [%v], ", strings.Join(missing, ", "))
+	}
+	if len(notIntended) > 0 {
+		msg += fmt.Sprintf("executed but should not [%v]", strings.Join(notIntended, ", "))
+	}
+	require.Failf(t, "the number of migrations does not match log in database", msg)
 }

pkg/services/sqlstore/migrations/ualert/ualert.go

@@ -57,7 +57,7 @@ func AddDashAlertMigration(mg *migrator.Migrator) {
 	_, migrationRun := logs[migTitle]
 
 	switch {
-	case mg.Cfg.UnifiedAlerting.Enabled && !migrationRun:
+	case mg.Cfg.UnifiedAlerting.IsEnabled() && !migrationRun:
 		// Remove the migration entry that removes all unified alerting data. This is so when the feature
 		// flag is removed in future the "remove unified alerting data" migration will be run again.
 		mg.AddMigration(fmt.Sprintf(clearMigrationEntryTitle, rmMigTitle), &clearMigrationEntry{
@@ -72,7 +72,7 @@ func AddDashAlertMigration(mg *migrator.Migrator) {
 			portedChannelGroupsPerOrg: make(map[int64]map[string]string),
 			silences:                  make(map[int64][]*pb.MeshSilence),
 		})
-	case !mg.Cfg.UnifiedAlerting.Enabled && migrationRun:
+	case !mg.Cfg.UnifiedAlerting.IsEnabled() && migrationRun:
 		// 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{
@@ -97,7 +97,7 @@ func RerunDashAlertMigration(mg *migrator.Migrator) {
 	cloneMigTitle := fmt.Sprintf("clone %s", migTitle)
 
 	_, migrationRun := logs[cloneMigTitle]
-	ngEnabled := mg.Cfg.UnifiedAlerting.Enabled
+	ngEnabled := mg.Cfg.UnifiedAlerting.IsEnabled()
 
 	switch {
 	case ngEnabled && !migrationRun:
@@ -117,7 +117,7 @@ func AddDashboardUIDPanelIDMigration(mg *migrator.Migrator) {
 
 	migrationID := "update dashboard_uid and panel_id from existing annotations"
 	_, migrationRun := logs[migrationID]
-	ngEnabled := mg.Cfg.UnifiedAlerting.Enabled
+	ngEnabled := mg.Cfg.UnifiedAlerting.IsEnabled()
 	undoMigrationID := "undo " + migrationID
 
 	if ngEnabled && !migrationRun {
@@ -738,3 +738,45 @@ func (u *upgradeNgAlerting) updateAlertmanagerFiles(orgId int64, migrator *migra
 func (u *upgradeNgAlerting) SQL(migrator.Dialect) string {
 	return "code migration"
 }
+
+// CheckUnifiedAlertingEnabledByDefault determines the final status of unified alerting, if it is not enabled explicitly.
+// Checks table `alert` and if it is empty, then it changes UnifiedAlerting.Enabled to true. Otherwise, it sets the flag to false.
+// After this method is executed the status of alerting should be determined, i.e. both flags will not be nil.
+// Note: this is not a real migration but a step that other migrations depend on.
+// TODO Delete when unified alerting is enabled by default unconditionally (Grafana v9)
+func CheckUnifiedAlertingEnabledByDefault(migrator *migrator.Migrator) error {
+	// if [unified_alerting][enabled] is explicitly set, we've got nothing to do here.
+	if migrator.Cfg.UnifiedAlerting.Enabled != nil {
+		return nil
+	}
+	var ualertEnabled bool
+	// this duplicates the logic in setting.ReadUnifiedAlertingSettings, and is put here just for logical completeness.
+	if setting.AlertingEnabled != nil && !*setting.AlertingEnabled {
+		ualertEnabled = true
+		migrator.Cfg.UnifiedAlerting.Enabled = &ualertEnabled
+		migrator.Logger.Debug("Unified alerting is enabled because the legacy is disabled explicitly")
+		return nil
+	}
+
+	resp := &struct {
+		Count int64
+	}{}
+	exist, err := migrator.DBEngine.IsTableExist("alert")
+	if err != nil {
+		return fmt.Errorf("failed to verify if the 'alert' table exists: %w", err)
+	}
+	if exist {
+		if _, err := migrator.DBEngine.SQL("SELECT COUNT(1) as count FROM alert").Get(resp); err != nil {
+			return fmt.Errorf("failed to read 'alert' table: %w", err)
+		}
+	}
+	// if table does not exist then we treat it as absence of legacy alerting and therefore enable unified alerting.
+
+	ualertEnabled = resp.Count == 0
+	legacyEnabled := !ualertEnabled
+	migrator.Cfg.UnifiedAlerting.Enabled = &ualertEnabled
+	setting.AlertingEnabled = &legacyEnabled
+
+	migrator.Logger.Debug(fmt.Sprintf("Found %d legacy alerts in the database. Unified alerting enabled is %v", resp.Count, ualertEnabled))
+	return nil
+}

pkg/services/sqlstore/migrations/ualert/ualert_test.go

@@ -4,7 +4,12 @@ import (
 	"fmt"
 	"testing"
 
+	"xorm.io/xorm"
+
 	"github.com/grafana/grafana/pkg/components/simplejson"
+	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
+	"github.com/grafana/grafana/pkg/services/sqlstore/sqlutil"
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 
 	"github.com/stretchr/testify/require"
@@ -73,6 +78,99 @@ func Test_validateAlertmanagerConfig(t *testing.T) {
 	}
 }
 
+func TestCheckUnifiedAlertingEnabledByDefault(t *testing.T) {
+	testDB := sqlutil.SQLite3TestDB()
+	x, err := xorm.NewEngine(testDB.DriverName, testDB.ConnStr)
+	require.NoError(t, err)
+	_, err = x.Exec("CREATE TABLE alert ( id bigint )")
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		_, err = x.Exec("DROP TABLE alert")
+		require.NoError(t, err)
+	})
+
+	tests := []struct {
+		title                   string
+		legacyAlertExists       bool
+		legacyIsDefined         bool
+		legacyValue             bool
+		expectedUnifiedAlerting bool
+	}{
+		{
+			title:                   "enable unified alerting when there are no legacy alerts",
+			legacyIsDefined:         false,
+			legacyAlertExists:       false,
+			expectedUnifiedAlerting: true,
+		},
+		{
+			title:                   "enable unified alerting when there are no legacy alerts and legacy enabled",
+			legacyIsDefined:         true,
+			legacyValue:             true,
+			legacyAlertExists:       false,
+			expectedUnifiedAlerting: true,
+		},
+		{
+			title:                   "enable unified alerting when there are no legacy alerts and legacy disabled",
+			legacyIsDefined:         true,
+			legacyValue:             false,
+			legacyAlertExists:       false,
+			expectedUnifiedAlerting: true,
+		},
+		{
+			title:                   "enable unified alerting when there are legacy alerts but legacy disabled",
+			legacyIsDefined:         true,
+			legacyValue:             false,
+			legacyAlertExists:       true,
+			expectedUnifiedAlerting: true,
+		},
+		{
+			title:                   "disable unified alerting when there are legacy alerts",
+			legacyIsDefined:         false,
+			legacyAlertExists:       true,
+			expectedUnifiedAlerting: false,
+		},
+		{
+			title:                   "disable unified alerting when there are legacy alerts and it is enabled",
+			legacyIsDefined:         true,
+			legacyValue:             true,
+			legacyAlertExists:       true,
+			expectedUnifiedAlerting: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.title, func(t *testing.T) {
+			setting.AlertingEnabled = nil
+			if test.legacyIsDefined {
+				value := test.legacyValue
+				setting.AlertingEnabled = &value
+			}
+
+			if test.legacyAlertExists {
+				_, err := x.Exec("INSERT INTO alert VALUES (1)")
+				require.NoError(t, err)
+			} else {
+				_, err := x.Exec("DELETE FROM alert")
+				require.NoError(t, err)
+			}
+
+			cfg := setting.Cfg{
+				UnifiedAlerting: setting.UnifiedAlertingSettings{
+					Enabled: nil,
+				},
+			}
+			mg := migrator.NewMigrator(x, &cfg)
+
+			err := CheckUnifiedAlertingEnabledByDefault(mg)
+			require.NoError(t, err)
+			require.NotNil(t, setting.AlertingEnabled)
+			require.NotNil(t, cfg.UnifiedAlerting.Enabled)
+			require.Equal(t, *cfg.UnifiedAlerting.Enabled, test.expectedUnifiedAlerting)
+			require.Equal(t, *setting.AlertingEnabled, !test.expectedUnifiedAlerting)
+		})
+	}
+}
+
 func configFromReceivers(t *testing.T, receivers []*PostableGrafanaReceiver) *PostableUserConfig {
 	t.Helper()
 

pkg/services/sqlstore/migrator/migrator.go

@@ -5,16 +5,17 @@ import (
 	"time"
 
 	_ "github.com/go-sql-driver/mysql"
-	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/setting"
-	"github.com/grafana/grafana/pkg/util/errutil"
 	_ "github.com/lib/pq"
 	_ "github.com/mattn/go-sqlite3"
 	"xorm.io/xorm"
+
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
 type Migrator struct {
-	x          *xorm.Engine
+	DBEngine   *xorm.Engine
 	Dialect    Dialect
 	migrations []Migration
 	Logger     log.Logger
@@ -32,10 +33,10 @@ type MigrationLog struct {
 
 func NewMigrator(engine *xorm.Engine, cfg *setting.Cfg) *Migrator {
 	mg := &Migrator{}
-	mg.x = engine
+	mg.DBEngine = engine
 	mg.Logger = log.New("migrator")
 	mg.migrations = make([]Migration, 0)
-	mg.Dialect = NewDialect(mg.x)
+	mg.Dialect = NewDialect(mg.DBEngine)
 	mg.Cfg = cfg
 	return mg
 }
@@ -49,11 +50,22 @@ func (mg *Migrator) AddMigration(id string, m Migration) {
 	mg.migrations = append(mg.migrations, m)
 }
 
+func (mg *Migrator) GetMigrationIDs(excludeNotLogged bool) []string {
+	result := make([]string, 0, len(mg.migrations))
+	for _, migration := range mg.migrations {
+		if migration.SkipMigrationLog() && excludeNotLogged {
+			continue
+		}
+		result = append(result, migration.Id())
+	}
+	return result
+}
+
 func (mg *Migrator) GetMigrationLog() (map[string]MigrationLog, error) {
 	logMap := make(map[string]MigrationLog)
 	logItems := make([]MigrationLog, 0)
 
-	exists, err := mg.x.IsTableExist(new(MigrationLog))
+	exists, err := mg.DBEngine.IsTableExist(new(MigrationLog))
 	if err != nil {
 		return nil, errutil.Wrap("failed to check table existence", err)
 	}
@@ -61,7 +73,7 @@ func (mg *Migrator) GetMigrationLog() (map[string]MigrationLog, error) {
 		return logMap, nil
 	}
 
-	if err = mg.x.Find(&logItems); err != nil {
+	if err = mg.DBEngine.Find(&logItems); err != nil {
 		return nil, err
 	}
 
@@ -132,7 +144,7 @@ func (mg *Migrator) Start() error {
 	mg.Logger.Info("migrations completed", "performed", migrationsPerformed, "skipped", migrationsSkipped, "duration", time.Since(start))
 
 	// Make sure migrations are synced
-	return mg.x.Sync2()
+	return mg.DBEngine.Sync2()
 }
 
 func (mg *Migrator) exec(m Migration, sess *xorm.Session) error {
@@ -178,7 +190,7 @@ func (mg *Migrator) exec(m Migration, sess *xorm.Session) error {
 type dbTransactionFunc func(sess *xorm.Session) error
 
 func (mg *Migrator) InTransaction(callback dbTransactionFunc) error {
-	sess := mg.x.NewSession()
+	sess := mg.DBEngine.NewSession()
 	defer sess.Close()
 
 	if err := sess.Begin(); err != nil {