diff --git a/conf/defaults.ini b/conf/defaults.ini index bcc8ca713cd..2224e9c8fcb 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -738,8 +738,8 @@ global_alert_rule = -1 #################################### Unified Alerting #################### [unified_alerting] -# Enable the Unified Alerting sub-system and interface. When enabled we'll migrate all of your alert rules and notification channels to the new system. New alert rules will be created and your notification channels will be converted into an Alertmanager configuration. Previous data is preserved to enable backwards compatibility but new data is removed. -enabled = false +# Enable the Unified Alerting sub-system and interface. When enabled we'll migrate all of your alert rules and notification channels to the new system. New alert rules will be created and your notification channels will be converted into an Alertmanager configuration. Previous data is preserved to enable backwards compatibility but new data is removed when switching. When this configuration section and flag are not defined, the state is defined at runtime. See the documentation for more details. +enabled = # Comma-separated list of organization IDs for which to disable unified alerting. Only supported if unified alerting is enabled. disabled_orgs = @@ -793,8 +793,8 @@ min_interval = 10s #################################### Alerting ############################ [alerting] -# Disable legacy alerting engine & UI features -enabled = true +# Enable the legacy alerting sub-system and interface. If Unified Alerting is already enabled and you try to go back to legacy alerting, all data that is part of Unified Alerting will be deleted. When this configuration section and flag are not defined, the state is defined at runtime. See the documentation for more details. +enabled = # Makes it possible to turn off alert execution but alerting UI is visible execute_alerts = true diff --git a/conf/sample.ini b/conf/sample.ini index eb1edccf4bc..95928af34a4 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -716,7 +716,7 @@ #################################### Unified Alerting #################### [unified_alerting] #Enable the Unified Alerting sub-system and interface. When enabled we'll migrate all of your alert rules and notification channels to the new system. New alert rules will be created and your notification channels will be converted into an Alertmanager configuration. Previous data is preserved to enable backwards compatibility but new data is removed.``` -;enabled = false +;enabled = true # Comma-separated list of organization IDs for which to disable unified alerting. Only supported if unified alerting is enabled. ;disabled_orgs = @@ -771,7 +771,7 @@ #################################### Alerting ############################ [alerting] # Disable legacy alerting engine & UI features -;enabled = true +;enabled = false # Makes it possible to turn off alert execution but alerting UI is visible ;execute_alerts = true diff --git a/e2e/panels-suite/panelEdit_base.spec.ts b/e2e/panels-suite/panelEdit_base.spec.ts index b34acb1bc3c..b99413756d0 100644 --- a/e2e/panels-suite/panelEdit_base.spec.ts +++ b/e2e/panels-suite/panelEdit_base.spec.ts @@ -9,7 +9,9 @@ e2e.scenario({ addScenarioDashBoard: false, skipScenario: false, scenario: () => { + e2e().intercept('/api/ds/query').as('query'); e2e.flows.openDashboard({ uid: 'TkZXxlNG3' }); + e2e().wait('@query'); e2e.flows.openPanelMenuItem(e2e.flows.PanelMenuItems.Edit, PANEL_UNDER_TEST); @@ -43,13 +45,12 @@ e2e.scenario({ // Can change to Alerts tab (graph panel is the default vis so the alerts tab should be rendered) e2e.components.Tab.title('Alert').should('be.visible').click(); - e2e.components.Tab.active().within((li: JQuery) => { - expect(li.text()).equals('Alert0'); // there's no alert so therefore Alert + 0 - }); - e2e.components.AlertTab.content().should('be.visible'); + e2e.components.Tab.active().should('have.text', 'Alert0'); // there's no alert so therefore Alert + 0 + e2e.components.AlertTab.content().should('not.exist'); e2e.components.QueryTab.content().should('not.exist'); e2e.components.TransformTab.content().should('not.exist'); - e2e.components.PanelAlertTabContent.content().should('not.exist'); + e2e.components.PanelAlertTabContent.content().should('exist'); + e2e.components.PanelAlertTabContent.content().should('be.visible'); e2e.components.Tab.title('Query').should('be.visible').click(); }); diff --git a/pkg/api/api.go b/pkg/api/api.go index 705cdda17c5..fcaac446ec7 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -386,7 +386,7 @@ func (hs *HTTPServer) registerRoutes() { }) apiRoute.Get("/alert-notifiers", reqEditorRole, routing.Wrap( - GetAlertNotifiers(hs.Cfg.UnifiedAlerting.Enabled)), + GetAlertNotifiers(hs.Cfg.UnifiedAlerting.IsEnabled())), ) apiRoute.Group("/alert-notifications", func(alertNotifications routing.RouteRegister) { diff --git a/pkg/api/index.go b/pkg/api/index.go index ac1e5d97f85..3872a83b7e1 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -205,9 +205,9 @@ func (hs *HTTPServer) getNavTree(c *models.ReqContext, hasEditPerm bool) ([]*dto } _, uaIsDisabledForOrg := hs.Cfg.UnifiedAlerting.DisabledOrgs[c.OrgId] - uaVisibleForOrg := hs.Cfg.UnifiedAlerting.Enabled && !uaIsDisabledForOrg + uaVisibleForOrg := hs.Cfg.UnifiedAlerting.IsEnabled() && !uaIsDisabledForOrg - if setting.AlertingEnabled || uaVisibleForOrg { + if setting.AlertingEnabled != nil && *setting.AlertingEnabled || uaVisibleForOrg { alertChildNavs := hs.buildAlertNavLinks(c, uaVisibleForOrg) navTree = append(navTree, &dtos.NavLink{ Text: "Alerting", @@ -463,9 +463,9 @@ func (hs *HTTPServer) buildCreateNavLinks(c *models.ReqContext) []*dtos.NavLink }) _, uaIsDisabledForOrg := hs.Cfg.UnifiedAlerting.DisabledOrgs[c.OrgId] - uaVisibleForOrg := hs.Cfg.UnifiedAlerting.Enabled && !uaIsDisabledForOrg + uaVisibleForOrg := hs.Cfg.UnifiedAlerting.IsEnabled() && !uaIsDisabledForOrg - if setting.AlertingEnabled || uaVisibleForOrg { + if setting.AlertingEnabled != nil && *setting.AlertingEnabled || uaVisibleForOrg { children = append(children, &dtos.NavLink{ Text: "Alert rule", SubTitle: "Create an alert rule", Id: "alert", Icon: "bell", Url: hs.Cfg.AppSubURL + "/alerting/new", diff --git a/pkg/middleware/quota_test.go b/pkg/middleware/quota_test.go index ab0cd0d768e..f58579c28b0 100644 --- a/pkg/middleware/quota_test.go +++ b/pkg/middleware/quota_test.go @@ -219,7 +219,8 @@ func TestMiddlewareQuota(t *testing.T) { }, func(cfg *setting.Cfg) { configure(cfg) - cfg.UnifiedAlerting.Enabled = true + cfg.UnifiedAlerting.Enabled = new(bool) + *cfg.UnifiedAlerting.Enabled = true cfg.Quota.Org.AlertRule = quotaUsed }) @@ -233,7 +234,8 @@ func TestMiddlewareQuota(t *testing.T) { }, func(cfg *setting.Cfg) { configure(cfg) - cfg.UnifiedAlerting.Enabled = true + cfg.UnifiedAlerting.Enabled = new(bool) + *cfg.UnifiedAlerting.Enabled = true cfg.Quota.Org.AlertRule = quotaUsed + 1 }) diff --git a/pkg/services/alerting/engine.go b/pkg/services/alerting/engine.go index 582e0fa0e4c..98aa1273684 100644 --- a/pkg/services/alerting/engine.go +++ b/pkg/services/alerting/engine.go @@ -7,6 +7,11 @@ import ( "time" "github.com/benbjohnson/clock" + "github.com/opentracing/opentracing-go" + "github.com/opentracing/opentracing-go/ext" + tlog "github.com/opentracing/opentracing-go/log" + "golang.org/x/sync/errgroup" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/usagestats" @@ -15,10 +20,6 @@ import ( "github.com/grafana/grafana/pkg/services/rendering" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tsdb/legacydata" - "github.com/opentracing/opentracing-go" - "github.com/opentracing/opentracing-go/ext" - tlog "github.com/opentracing/opentracing-go/log" - "golang.org/x/sync/errgroup" ) // AlertEngine is the background process that @@ -41,9 +42,9 @@ type AlertEngine struct { usageStatsService usagestats.Service } -// IsDisabled returns true if the alerting service is disable for this instance. +// IsDisabled returns true if the alerting service is disabled for this instance. func (e *AlertEngine) IsDisabled() bool { - return !setting.AlertingEnabled || !setting.ExecuteAlerts || e.Cfg.UnifiedAlerting.Enabled + return setting.AlertingEnabled == nil || !*setting.AlertingEnabled || !setting.ExecuteAlerts || e.Cfg.UnifiedAlerting.IsEnabled() } // ProvideAlertEngine returns a new AlertEngine. diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index 01afcb6cd52..5d7977ec01e 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -6,6 +6,8 @@ import ( "time" "github.com/benbjohnson/clock" + "golang.org/x/sync/errgroup" + "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/infra/kvstore" @@ -23,7 +25,6 @@ import ( "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" - "golang.org/x/sync/errgroup" ) const ( @@ -185,7 +186,7 @@ func (ng *AlertNG) IsDisabled() bool { if ng.Cfg == nil { return true } - return !ng.Cfg.UnifiedAlerting.Enabled + return !ng.Cfg.UnifiedAlerting.IsEnabled() } // getRuleDefaultIntervalSeconds returns the default rule interval if the interval is not set. diff --git a/pkg/services/ngalert/tests/util.go b/pkg/services/ngalert/tests/util.go index b4f36b58fb7..7d70fba042b 100644 --- a/pkg/services/ngalert/tests/util.go +++ b/pkg/services/ngalert/tests/util.go @@ -31,7 +31,8 @@ func SetupTestEnv(t *testing.T, baseInterval time.Duration) (*ngalert.AlertNG, * cfg := setting.NewCfg() cfg.AlertingBaseInterval = baseInterval // AlertNG database migrations run and the relative database tables are created only when it's enabled - cfg.UnifiedAlerting.Enabled = true + cfg.UnifiedAlerting.Enabled = new(bool) + *cfg.UnifiedAlerting.Enabled = true m := metrics.NewNGAlert(prometheus.NewRegistry()) sqlStore := sqlstore.InitTestDB(t) diff --git a/pkg/services/quota/quota.go b/pkg/services/quota/quota.go index abd0304e500..e517abde80d 100644 --- a/pkg/services/quota/quota.go +++ b/pkg/services/quota/quota.go @@ -62,7 +62,7 @@ func (qs *QuotaService) QuotaReached(c *models.ReqContext, target string) (bool, } continue } - query := models.GetGlobalQuotaByTargetQuery{Target: scope.Target, UnifiedAlertingEnabled: qs.Cfg.UnifiedAlerting.Enabled} + query := models.GetGlobalQuotaByTargetQuery{Target: scope.Target, UnifiedAlertingEnabled: qs.Cfg.UnifiedAlerting.IsEnabled()} if err := bus.DispatchCtx(c.Req.Context(), &query); err != nil { return true, err } @@ -77,7 +77,7 @@ func (qs *QuotaService) QuotaReached(c *models.ReqContext, target string) (bool, OrgId: c.OrgId, Target: scope.Target, Default: scope.DefaultLimit, - UnifiedAlertingEnabled: qs.Cfg.UnifiedAlerting.Enabled, + UnifiedAlertingEnabled: qs.Cfg.UnifiedAlerting.IsEnabled(), } if err := bus.DispatchCtx(c.Req.Context(), &query); err != nil { return true, err @@ -96,7 +96,7 @@ func (qs *QuotaService) QuotaReached(c *models.ReqContext, target string) (bool, if !c.IsSignedIn || c.UserId == 0 { continue } - query := models.GetUserQuotaByTargetQuery{UserId: c.UserId, Target: scope.Target, Default: scope.DefaultLimit, UnifiedAlertingEnabled: qs.Cfg.UnifiedAlerting.Enabled} + query := models.GetUserQuotaByTargetQuery{UserId: c.UserId, Target: scope.Target, Default: scope.DefaultLimit, UnifiedAlertingEnabled: qs.Cfg.UnifiedAlerting.IsEnabled()} if err := bus.DispatchCtx(c.Req.Context(), &query); err != nil { return true, err } diff --git a/pkg/services/sqlstore/migrations/migrations.go b/pkg/services/sqlstore/migrations/migrations.go index 65b2eb13ced..3bbeb1c463e 100644 --- a/pkg/services/sqlstore/migrations/migrations.go +++ b/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) diff --git a/pkg/services/sqlstore/migrations/migrations_test.go b/pkg/services/sqlstore/migrations/migrations_test.go index 70fe6190be4..90c7918333b 100644 --- a/pkg/services/sqlstore/migrations/migrations_test.go +++ b/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) } diff --git a/pkg/services/sqlstore/migrations/ualert/ualert.go b/pkg/services/sqlstore/migrations/ualert/ualert.go index 7941d3d3210..4114f84b480 100644 --- a/pkg/services/sqlstore/migrations/ualert/ualert.go +++ b/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 +} diff --git a/pkg/services/sqlstore/migrations/ualert/ualert_test.go b/pkg/services/sqlstore/migrations/ualert/ualert_test.go index 7d6ddbc1ef9..24a0632faea 100644 --- a/pkg/services/sqlstore/migrations/ualert/ualert_test.go +++ b/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() diff --git a/pkg/services/sqlstore/migrator/migrator.go b/pkg/services/sqlstore/migrator/migrator.go index ed36505ea71..50c4ce347fa 100644 --- a/pkg/services/sqlstore/migrator/migrator.go +++ b/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 { diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 4dd0c197461..3e0ebe77ce8 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/grafana-aws-sdk/pkg/awsds" "github.com/grafana/grafana-plugin-sdk-go/backend/gtime" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/util" @@ -153,7 +154,7 @@ var ( Quota QuotaSettings // Alerting - AlertingEnabled bool + AlertingEnabled *bool ExecuteAlerts bool AlertingRenderLimit int AlertingErrorOrTimeout string @@ -914,9 +915,6 @@ func (cfg *Cfg) Load(args CommandLineArgs) error { if err := readAlertingSettings(iniFile); err != nil { return err } - if err := cfg.ReadUnifiedAlertingSettings(iniFile); err != nil { - return err - } explore := iniFile.Section("explore") ExploreEnabled = explore.Key("enabled").MustBool(true) @@ -1377,7 +1375,11 @@ func (cfg *Cfg) readFeatureToggles(iniFile *ini.File) error { func readAlertingSettings(iniFile *ini.File) error { alerting := iniFile.Section("alerting") - AlertingEnabled = alerting.Key("enabled").MustBool(true) + enabled, err := alerting.Key("enabled").Bool() + AlertingEnabled = nil + if err == nil { + AlertingEnabled = &enabled + } ExecuteAlerts = alerting.Key("execute_alerts").MustBool(true) AlertingRenderLimit = alerting.Key("concurrent_render_limit").MustInt(5) diff --git a/pkg/setting/setting_quota.go b/pkg/setting/setting_quota.go index 406ccf2abf1..139189f7e84 100644 --- a/pkg/setting/setting_quota.go +++ b/pkg/setting/setting_quota.go @@ -68,7 +68,7 @@ func (cfg *Cfg) readQuotaSettings() { var alertOrgQuota int64 var alertGlobalQuota int64 - if cfg.UnifiedAlerting.Enabled { + if cfg.UnifiedAlerting.IsEnabled() { alertOrgQuota = quota.Key("org_alert_rule").MustInt64(100) alertGlobalQuota = quota.Key("global_alert_rule").MustInt64(-1) } diff --git a/pkg/setting/setting_test.go b/pkg/setting/setting_test.go index 42d29a7230c..17d62d57cc0 100644 --- a/pkg/setting/setting_test.go +++ b/pkg/setting/setting_test.go @@ -446,8 +446,10 @@ func TestAlertingEnabled(t *testing.T) { require.NoError(t, err) err = cfg.ReadUnifiedAlertingSettings(f) require.NoError(t, err) - assert.Equal(t, cfg.UnifiedAlerting.Enabled, false) - assert.Equal(t, AlertingEnabled, true) + assert.NotNil(t, cfg.UnifiedAlerting.Enabled) + assert.Equal(t, *cfg.UnifiedAlerting.Enabled, false) + assert.NotNil(t, AlertingEnabled) + assert.Equal(t, *AlertingEnabled, true) }, }, { @@ -461,12 +463,14 @@ func TestAlertingEnabled(t *testing.T) { require.NoError(t, err) err = cfg.ReadUnifiedAlertingSettings(f) require.NoError(t, err) - assert.Equal(t, cfg.UnifiedAlerting.Enabled, true) - assert.Equal(t, AlertingEnabled, false) + assert.NotNil(t, cfg.UnifiedAlerting.Enabled) + assert.Equal(t, *cfg.UnifiedAlerting.Enabled, true) + assert.NotNil(t, AlertingEnabled) + assert.Equal(t, *AlertingEnabled, false) }, }, { - desc: "when both alerting are enabled, it should error", + desc: "when both alerting are enabled", legacyAlertingEnabled: "true", unifiedAlertingEnabled: "true", verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { @@ -479,8 +483,8 @@ func TestAlertingEnabled(t *testing.T) { }, }, { - desc: "when legacy alerting is invalid and unified is disabled", - legacyAlertingEnabled: "invalid", + desc: "when legacy alerting is invalid (or not defined) and unified is disabled", + legacyAlertingEnabled: "", unifiedAlertingEnabled: "false", verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { err := readAlertingSettings(f) @@ -489,13 +493,15 @@ func TestAlertingEnabled(t *testing.T) { require.NoError(t, err) err = cfg.ReadUnifiedAlertingSettings(f) require.NoError(t, err) - assert.Equal(t, cfg.UnifiedAlerting.Enabled, false) - assert.Equal(t, AlertingEnabled, true) + assert.NotNil(t, cfg.UnifiedAlerting.Enabled) + assert.Equal(t, *cfg.UnifiedAlerting.Enabled, false) + assert.NotNil(t, AlertingEnabled) + assert.Equal(t, *AlertingEnabled, true) }, }, { - desc: "when legacy alerting is invalid and unified is enabled", - legacyAlertingEnabled: "invalid", + desc: "when legacy alerting is invalid (or not defined) and unified is enabled", + legacyAlertingEnabled: "", unifiedAlertingEnabled: "true", verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { err := readAlertingSettings(f) @@ -503,13 +509,17 @@ func TestAlertingEnabled(t *testing.T) { err = cfg.readFeatureToggles(f) require.NoError(t, err) err = cfg.ReadUnifiedAlertingSettings(f) - require.Error(t, err) + require.NoError(t, err) + assert.NotNil(t, cfg.UnifiedAlerting.Enabled) + assert.Equal(t, *cfg.UnifiedAlerting.Enabled, true) + assert.NotNil(t, AlertingEnabled) + assert.Equal(t, *AlertingEnabled, false) }, }, { - desc: "when legacy alerting is enabled and unified is invalid", + desc: "when legacy alerting is enabled and unified is invalid (or not defined)", legacyAlertingEnabled: "true", - unifiedAlertingEnabled: "invalid", + unifiedAlertingEnabled: "", verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { err := readAlertingSettings(f) require.NoError(t, err) @@ -517,12 +527,13 @@ func TestAlertingEnabled(t *testing.T) { require.NoError(t, err) err = cfg.ReadUnifiedAlertingSettings(f) require.NoError(t, err) - assert.Equal(t, cfg.UnifiedAlerting.Enabled, false) - assert.Equal(t, AlertingEnabled, true) + assert.Nil(t, cfg.UnifiedAlerting.Enabled) + assert.NotNil(t, AlertingEnabled) + assert.Equal(t, *AlertingEnabled, true) }, }, { - desc: "when legacy alerting is disabled and unified is invalid", + desc: "when legacy alerting is disabled and unified is invalid (or not defined)", legacyAlertingEnabled: "false", unifiedAlertingEnabled: "invalid", verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { @@ -532,12 +543,14 @@ func TestAlertingEnabled(t *testing.T) { require.NoError(t, err) err = cfg.ReadUnifiedAlertingSettings(f) require.NoError(t, err) - assert.Equal(t, cfg.UnifiedAlerting.Enabled, false) - assert.Equal(t, AlertingEnabled, false) + assert.NotNil(t, cfg.UnifiedAlerting.Enabled) + assert.Equal(t, *cfg.UnifiedAlerting.Enabled, true) + assert.NotNil(t, AlertingEnabled) + assert.Equal(t, *AlertingEnabled, false) }, }, { - desc: "when both are invalid", + desc: "when both are invalid (or not defined)", legacyAlertingEnabled: "invalid", unifiedAlertingEnabled: "invalid", verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { @@ -547,31 +560,14 @@ func TestAlertingEnabled(t *testing.T) { require.NoError(t, err) err = cfg.ReadUnifiedAlertingSettings(f) require.NoError(t, err) - assert.Equal(t, cfg.UnifiedAlerting.Enabled, false) - assert.Equal(t, AlertingEnabled, true) + assert.Nil(t, cfg.UnifiedAlerting.Enabled) + assert.Nil(t, AlertingEnabled) }, }, { - desc: "when legacy alerting is enabled and unified is disabled and feature toggle is set", - legacyAlertingEnabled: "true", - unifiedAlertingEnabled: "false", - featureToggleSet: true, - verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { - err := readAlertingSettings(f) - require.NoError(t, err) - err = cfg.readFeatureToggles(f) - require.NoError(t, err) - err = cfg.ReadUnifiedAlertingSettings(f) - require.NoError(t, err) - assert.Equal(t, cfg.UnifiedAlerting.Enabled, true) - assert.Equal(t, AlertingEnabled, false) - }, - }, - { - desc: "when legacy alerting is disabled and unified is disabled and feature toggle is set", + desc: "when both are false", legacyAlertingEnabled: "false", unifiedAlertingEnabled: "false", - featureToggleSet: true, verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { err := readAlertingSettings(f) require.NoError(t, err) @@ -579,70 +575,10 @@ func TestAlertingEnabled(t *testing.T) { require.NoError(t, err) err = cfg.ReadUnifiedAlertingSettings(f) require.NoError(t, err) - assert.Equal(t, cfg.UnifiedAlerting.Enabled, true) - assert.Equal(t, AlertingEnabled, false) - }, - }, - { - desc: "when legacy alerting is disabled and unified is invalid and feature toggle is set", - legacyAlertingEnabled: "false", - unifiedAlertingEnabled: "invalid", - featureToggleSet: true, - verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { - err := readAlertingSettings(f) - require.NoError(t, err) - err = cfg.readFeatureToggles(f) - require.NoError(t, err) - err = cfg.ReadUnifiedAlertingSettings(f) - require.NoError(t, err) - assert.Equal(t, cfg.UnifiedAlerting.Enabled, true) - assert.Equal(t, AlertingEnabled, false) - }, - }, - { - desc: "when legacy alerting is invalid and unified is disabled and feature toggle is set", - legacyAlertingEnabled: "invalid", - unifiedAlertingEnabled: "false", - featureToggleSet: true, - verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { - err := readAlertingSettings(f) - require.NoError(t, err) - err = cfg.readFeatureToggles(f) - require.NoError(t, err) - err = cfg.ReadUnifiedAlertingSettings(f) - require.NoError(t, err) - assert.Equal(t, cfg.UnifiedAlerting.Enabled, true) - assert.Equal(t, AlertingEnabled, false) - }, - }, - { - desc: "when legacy alerting is invalid and unified is enabled and feature toggle is set", - legacyAlertingEnabled: "invalid", - unifiedAlertingEnabled: "true", - featureToggleSet: true, - verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { - err := readAlertingSettings(f) - require.NoError(t, err) - err = cfg.readFeatureToggles(f) - require.NoError(t, err) - err = cfg.ReadUnifiedAlertingSettings(f) - require.Error(t, err) - }, - }, - { - desc: "when both are invalid and feature toggle is set", - legacyAlertingEnabled: "invalid", - unifiedAlertingEnabled: "invalid", - featureToggleSet: true, - verifyCfg: func(t *testing.T, cfg Cfg, f *ini.File) { - err := readAlertingSettings(f) - require.NoError(t, err) - err = cfg.readFeatureToggles(f) - require.NoError(t, err) - err = cfg.ReadUnifiedAlertingSettings(f) - require.NoError(t, err) - assert.Equal(t, cfg.UnifiedAlerting.Enabled, true) - assert.Equal(t, AlertingEnabled, false) + assert.NotNil(t, cfg.UnifiedAlerting.Enabled) + assert.Equal(t, *cfg.UnifiedAlerting.Enabled, false) + assert.NotNil(t, AlertingEnabled) + assert.Equal(t, *AlertingEnabled, false) }, }, } @@ -650,7 +586,7 @@ func TestAlertingEnabled(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { t.Cleanup(func() { - AlertingEnabled = false + AlertingEnabled = nil }) f := ini.Empty() @@ -665,13 +601,6 @@ func TestAlertingEnabled(t *testing.T) { _, err = alertingSec.NewKey("enabled", tc.legacyAlertingEnabled) require.NoError(t, err) - if tc.featureToggleSet { - alertingSec, err := f.NewSection("feature_toggles") - require.NoError(t, err) - _, err = alertingSec.NewKey("enable", "ngalert") - require.NoError(t, err) - } - tc.verifyCfg(t, *cfg, f) }) } diff --git a/pkg/setting/setting_unified_alerting.go b/pkg/setting/setting_unified_alerting.go index a415dd83742..d2234cb9bdc 100644 --- a/pkg/setting/setting_unified_alerting.go +++ b/pkg/setting/setting_unified_alerting.go @@ -7,6 +7,7 @@ import ( "time" "github.com/grafana/grafana-plugin-sdk-go/backend/gtime" + "github.com/grafana/grafana/pkg/util" "github.com/prometheus/alertmanager/cluster" @@ -63,26 +64,60 @@ type UnifiedAlertingSettings struct { EvaluationTimeout time.Duration ExecuteAlerts bool DefaultConfiguration string - Enabled bool + Enabled *bool // determines whether unified alerting is enabled. If it is nil then user did not define it and therefore its value will be determined during migration. Services should not use it directly. DisabledOrgs map[int64]struct{} } +// IsEnabled returns true if UnifiedAlertingSettings.Enabled is either nil or true. +// It hides the implementation details of the Enabled and simplifies its usage. +func (u *UnifiedAlertingSettings) IsEnabled() bool { + return u.Enabled == nil || *u.Enabled +} + +func (cfg *Cfg) readUnifiedAlertingEnabledSetting(section *ini.Section) (*bool, error) { + enabled, err := section.Key("enabled").Bool() + // the unified alerting is not enabled by default. First, check the feature flag + if err != nil { + // TODO: Remove in Grafana v9 + if cfg.FeatureToggles["ngalert"] { + cfg.Logger.Warn("ngalert feature flag is deprecated: use unified alerting enabled setting instead") + enabled = true + // feature flag overrides the legacy alerting setting. + legacyAlerting := false + AlertingEnabled = &legacyAlerting + return &enabled, nil + } + // next, check whether legacy flag is set + if AlertingEnabled != nil && !*AlertingEnabled { + enabled = true + return &enabled, nil // if legacy alerting is explicitly disabled, enable the unified alerting by default. + } + // NOTE: If the enabled flag is still not defined, the final decision is made during migration (see sqlstore.migrations.ualert.CheckUnifiedAlertingEnabledByDefault). + cfg.Logger.Info("The state of unified alerting is still not defined. The decision will be made during as we run the database migrations") + return nil, nil // the flag is not defined + } + + // If unified alerting is defined explicitly as well as legacy alerting and both are enabled, return error. + if enabled && AlertingEnabled != nil && *AlertingEnabled { + return nil, errors.New("both legacy and Grafana 8 Alerts are enabled. Disable one of them and restart") + } + // if legacy alerting is not defined but unified is determined then update the legacy with inverted value + if AlertingEnabled == nil { + legacyEnabled := !enabled + AlertingEnabled = &legacyEnabled + } + return &enabled, nil +} + // ReadUnifiedAlertingSettings reads both the `unified_alerting` and `alerting` sections of the configuration while preferring configuration the `alerting` section. // It first reads the `unified_alerting` section, then looks for non-defaults on the `alerting` section and prefers those. func (cfg *Cfg) ReadUnifiedAlertingSettings(iniFile *ini.File) error { + var err error uaCfg := UnifiedAlertingSettings{} ua := iniFile.Section("unified_alerting") - uaCfg.Enabled = ua.Key("enabled").MustBool(false) - - // TODO: Deprecate this in v8.4, if the old feature toggle ngalert is set, enable Grafana 8 Unified Alerting anyway. - if !uaCfg.Enabled && cfg.FeatureToggles["ngalert"] { - cfg.Logger.Warn("ngalert feature flag is deprecated: use unified alerting enabled setting instead") - uaCfg.Enabled = true - AlertingEnabled = false - } - - if uaCfg.Enabled && AlertingEnabled { - return errors.New("both legacy and Grafana 8 Alerts are enabled") + uaCfg.Enabled, err = cfg.readUnifiedAlertingEnabledSetting(ua) + if err != nil { + return err } uaCfg.DisabledOrgs = make(map[int64]struct{}) @@ -95,7 +130,6 @@ func (cfg *Cfg) ReadUnifiedAlertingSettings(iniFile *ini.File) error { uaCfg.DisabledOrgs[orgID] = struct{}{} } - var err error uaCfg.AdminConfigPollInterval, err = gtime.ParseDuration(valueAsString(ua, "admin_config_poll_interval", (schedulerDefaultAdminConfigPollInterval).String())) if err != nil { return err