From 53945afedf18945fe9f7d13c15eac00d693ef880 Mon Sep 17 00:00:00 2001 From: Alex Moreno Date: Wed, 1 Feb 2023 13:15:03 +0100 Subject: [PATCH] Alerting: Allow alert rule pausing from API (#62326) * Add is_paused attr to the POST alert rule group endpoint * Add is_paused to alerting API POST alert rule group * Fixed tests * Add is_paused to alerting gettable endpoints * Fix integration tests * Alerting: allow to pause existing rules (#62401) * Display Pause Rule switch in Editing Rule form * add isPaused property to form interface and dto * map isPaused prop with is_paused value from DTO Also update test snapshots * Append '(Paused)' text on alert list state column when appropriate * Change Switch styles according to discussion with UX Also adding a tooltip with info what this means * Adjust styles * Fix alignment and isPaused type definition Co-authored-by: gillesdemey * Fix test * Fix test * Fix RuleList test --------- Co-authored-by: gillesdemey * wip * Fix tests and add comments to clarify AlertRuleWithOptionals * Fix one more test * Fix tests * Fix typo in comment * Fix alert rule(s) cannot be paused via API * Add integration tests for alerting api pausing flow * Remove duplicated integration test --------- Co-authored-by: Virginia Cepeda Co-authored-by: gillesdemey Co-authored-by: George Robinson --- pkg/services/ngalert/api/api_ruler.go | 3 +- .../ngalert/api/api_ruler_validation.go | 22 ++- .../ngalert/api/api_ruler_validation_test.go | 18 +++ pkg/services/ngalert/api/tooling/api.json | 6 + .../api/tooling/definitions/cortex-ruler.go | 2 + pkg/services/ngalert/api/tooling/post.json | 6 + pkg/services/ngalert/api/tooling/spec.json | 6 + pkg/services/ngalert/models/alert_rule.go | 16 ++- .../ngalert/models/alert_rule_test.go | 33 +++-- .../ngalert/provisioning/alert_rules.go | 4 +- pkg/services/ngalert/store/deltas.go | 8 +- pkg/services/ngalert/store/deltas_test.go | 36 +++-- .../api/alerting/api_alertmanager_test.go | 7 + pkg/tests/api/alerting/api_ruler_test.go | 130 ++++++++++++++++++ pkg/tests/api/alerting/testing.go | 1 + public/api-merged.json | 4 +- .../unified/RuleEditorExisting.test.tsx | 1 + .../unified/RuleEditorGrafanaRules.test.tsx | 1 + .../alerting/unified/RuleList.test.tsx | 4 +- .../components/rule-editor/AlertRuleForm.tsx | 1 + .../rule-editor/GrafanaEvaluationBehavior.tsx | 41 +++++- .../components/rules/AlertStateTag.tsx | 5 +- .../unified/components/rules/RuleState.tsx | 5 +- .../unified/components/rules/RulesTable.tsx | 6 +- .../alerting/unified/types/rule-form.ts | 1 + .../__snapshots__/rule-form.test.ts.snap | 2 + .../alerting/unified/utils/rule-form.ts | 4 +- public/app/types/unified-alerting-dto.ts | 1 + 28 files changed, 328 insertions(+), 46 deletions(-) diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index b467a3b4034..8a9632f898f 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -324,7 +324,7 @@ func (srv RulerSrv) RoutePostNameRulesConfig(c *contextmodel.ReqContext, ruleGro // updateAlertRulesInGroup calculates changes (rules to add,update,delete), verifies that the user is authorized to do the calculated changes and updates database. // All operations are performed in a single transaction -func (srv RulerSrv) updateAlertRulesInGroup(c *contextmodel.ReqContext, groupKey ngmodels.AlertRuleGroupKey, rules []*ngmodels.AlertRule) response.Response { +func (srv RulerSrv) updateAlertRulesInGroup(c *contextmodel.ReqContext, groupKey ngmodels.AlertRuleGroupKey, rules []*ngmodels.AlertRuleWithOptionals) response.Response { var finalChanges *store.GroupDelta hasAccess := accesscontrol.HasAccess(srv.ac, c) err := srv.xactManager.InTransaction(c.Req.Context(), func(tranCtx context.Context) error { @@ -482,6 +482,7 @@ func toGettableExtendedRuleNode(r ngmodels.AlertRule, namespaceID int64, provena NoDataState: apimodels.NoDataState(r.NoDataState), ExecErrState: apimodels.ExecutionErrorState(r.ExecErrState), Provenance: provenance, + IsPaused: r.IsPaused, }, } forDuration := model.Duration(r.For) diff --git a/pkg/services/ngalert/api/api_ruler_validation.go b/pkg/services/ngalert/api/api_ruler_validation.go index a8f86eee6ab..0594ae15c0e 100644 --- a/pkg/services/ngalert/api/api_ruler_validation.go +++ b/pkg/services/ngalert/api/api_ruler_validation.go @@ -149,12 +149,13 @@ func validateForInterval(ruleNode *apimodels.PostableExtendedRuleNode) (time.Dur // validateRuleGroup validates API model (definitions.PostableRuleGroupConfig) and converts it to a collection of models.AlertRule. // Returns a slice that contains all rules described by API model or error if either group specification or an alert definition is not valid. +// It also returns a map containing current existing alerts that don't contain the is_paused field in the body of the call. func validateRuleGroup( ruleGroupConfig *apimodels.PostableRuleGroupConfig, orgId int64, namespace *folder.Folder, conditionValidator func(ngmodels.Condition) error, - cfg *setting.UnifiedAlertingSettings) ([]*ngmodels.AlertRule, error) { + cfg *setting.UnifiedAlertingSettings) ([]*ngmodels.AlertRuleWithOptionals, error) { if ruleGroupConfig.Name == "" { return nil, errors.New("rule group name cannot be empty") } @@ -175,7 +176,7 @@ func validateRuleGroup( // TODO should we validate that interval is >= cfg.MinInterval? Currently, we allow to save but fix the specified interval if it is < cfg.MinInterval - result := make([]*ngmodels.AlertRule, 0, len(ruleGroupConfig.Rules)) + result := make([]*ngmodels.AlertRuleWithOptionals, 0, len(ruleGroupConfig.Rules)) uids := make(map[string]int, cap(result)) for idx := range ruleGroupConfig.Rules { rule, err := validateRuleNode(&ruleGroupConfig.Rules[idx], ruleGroupConfig.Name, interval, orgId, namespace, conditionValidator, cfg) @@ -189,8 +190,23 @@ func validateRuleGroup( } uids[rule.UID] = idx } + + var hasPause, isPaused bool + original := ruleGroupConfig.Rules[idx] + if alert := original.GrafanaManagedAlert; alert != nil { + if alert.IsPaused != nil { + isPaused = *alert.IsPaused + hasPause = true + } + } + + ruleWithOptionals := ngmodels.AlertRuleWithOptionals{} + rule.IsPaused = isPaused rule.RuleGroupIndex = idx + 1 - result = append(result, rule) + ruleWithOptionals.AlertRule = *rule + ruleWithOptionals.HasPause = hasPause + + result = append(result, &ruleWithOptionals) } return result, nil } diff --git a/pkg/services/ngalert/api/api_ruler_validation_test.go b/pkg/services/ngalert/api/api_ruler_validation_test.go index 00d76433882..9dc994e8a36 100644 --- a/pkg/services/ngalert/api/api_ruler_validation_test.go +++ b/pkg/services/ngalert/api/api_ruler_validation_test.go @@ -119,6 +119,7 @@ func TestValidateRuleGroup(t *testing.T) { require.Len(t, alerts, len(rules)) require.Equal(t, len(rules), conditionValidations) }) + t.Run("should default to default interval from config if group interval is 0", func(t *testing.T) { g := validGroup(cfg, rules...) g.Interval = 0 @@ -128,6 +129,23 @@ func TestValidateRuleGroup(t *testing.T) { require.NoError(t, err) for _, alert := range alerts { require.Equal(t, int64(cfg.DefaultRuleEvaluationInterval.Seconds()), alert.IntervalSeconds) + require.False(t, alert.HasPause) + } + }) + + t.Run("should show the payload has isPaused field", func(t *testing.T) { + for _, rule := range rules { + isPaused := true + rule.GrafanaManagedAlert.IsPaused = &isPaused + isPaused = !(isPaused) + } + g := validGroup(cfg, rules...) + alerts, err := validateRuleGroup(&g, orgId, folder, func(condition models.Condition) error { + return nil + }, cfg) + require.NoError(t, err) + for _, alert := range alerts { + require.True(t, alert.HasPause) } }) } diff --git a/pkg/services/ngalert/api/tooling/api.json b/pkg/services/ngalert/api/tooling/api.json index ec09cd406dd..eac67975b3d 100644 --- a/pkg/services/ngalert/api/tooling/api.json +++ b/pkg/services/ngalert/api/tooling/api.json @@ -1210,6 +1210,9 @@ "format": "int64", "type": "integer" }, + "is_paused": { + "type": "boolean" + }, "namespace_id": { "format": "int64", "type": "integer" @@ -2130,6 +2133,9 @@ ], "type": "string" }, + "is_paused": { + "type": "boolean" + }, "no_data_state": { "enum": [ "Alerting", diff --git a/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go b/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go index 4acab8ab44d..4a96c98a188 100644 --- a/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go +++ b/pkg/services/ngalert/api/tooling/definitions/cortex-ruler.go @@ -374,6 +374,7 @@ type PostableGrafanaRule struct { UID string `json:"uid" yaml:"uid"` NoDataState NoDataState `json:"no_data_state" yaml:"no_data_state"` ExecErrState ExecutionErrorState `json:"exec_err_state" yaml:"exec_err_state"` + IsPaused *bool `json:"is_paused" yaml:"is_paused"` } // swagger:model @@ -393,4 +394,5 @@ type GettableGrafanaRule struct { NoDataState NoDataState `json:"no_data_state" yaml:"no_data_state"` ExecErrState ExecutionErrorState `json:"exec_err_state" yaml:"exec_err_state"` Provenance models.Provenance `json:"provenance,omitempty" yaml:"provenance,omitempty"` + IsPaused bool `json:"is_paused" yaml:"is_paused"` } diff --git a/pkg/services/ngalert/api/tooling/post.json b/pkg/services/ngalert/api/tooling/post.json index deb7e4bb53b..e2d625d9e09 100644 --- a/pkg/services/ngalert/api/tooling/post.json +++ b/pkg/services/ngalert/api/tooling/post.json @@ -1210,6 +1210,9 @@ "format": "int64", "type": "integer" }, + "is_paused": { + "type": "boolean" + }, "namespace_id": { "format": "int64", "type": "integer" @@ -2130,6 +2133,9 @@ ], "type": "string" }, + "is_paused": { + "type": "boolean" + }, "no_data_state": { "enum": [ "Alerting", diff --git a/pkg/services/ngalert/api/tooling/spec.json b/pkg/services/ngalert/api/tooling/spec.json index ae560c6dcf1..972006cc4cf 100644 --- a/pkg/services/ngalert/api/tooling/spec.json +++ b/pkg/services/ngalert/api/tooling/spec.json @@ -3825,6 +3825,9 @@ "type": "integer", "format": "int64" }, + "is_paused": { + "type": "boolean" + }, "namespace_id": { "type": "integer", "format": "int64" @@ -4746,6 +4749,9 @@ "Error" ] }, + "is_paused": { + "type": "boolean" + }, "no_data_state": { "type": "string", "enum": [ diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index 5e96c82cbca..85fb5748064 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -162,6 +162,17 @@ type AlertRule struct { IsPaused bool } +// AlertRuleWithOptionals This is to avoid having to pass in additional arguments deep in the call stack. Alert rule +// object is created in an early validation step without knowledge about current alert rule fields or if they need to be +// overridden. This is done in a later step and, in that step, we did not have knowledge about if a field was optional +// nor its possible value. +type AlertRuleWithOptionals struct { + AlertRule + // This parameter is to know if an optional API field was sent and, therefore, patch it with the current field from + // DB in case it was not sent. + HasPause bool +} + // GetDashboardUID returns the DashboardUID or "". func (alertRule *AlertRule) GetDashboardUID() string { if alertRule.DashboardUID != nil { @@ -443,7 +454,7 @@ func (c Condition) IsValid() bool { // - AlertRule.Condition and AlertRule.Data // // If either of the pair is specified, neither is patched. -func PatchPartialAlertRule(existingRule *AlertRule, ruleToPatch *AlertRule) { +func PatchPartialAlertRule(existingRule *AlertRule, ruleToPatch *AlertRuleWithOptionals) { if ruleToPatch.Title == "" { ruleToPatch.Title = existingRule.Title } @@ -469,6 +480,9 @@ func PatchPartialAlertRule(existingRule *AlertRule, ruleToPatch *AlertRule) { if ruleToPatch.For == -1 { ruleToPatch.For = existingRule.For } + if !ruleToPatch.HasPause { + ruleToPatch.IsPaused = existingRule.IsPaused + } } func ValidateRuleGroupInterval(intervalSeconds, baseIntervalSeconds int64) error { diff --git a/pkg/services/ngalert/models/alert_rule_test.go b/pkg/services/ngalert/models/alert_rule_test.go index bf1bbd4007e..8f33d2dc875 100644 --- a/pkg/services/ngalert/models/alert_rule_test.go +++ b/pkg/services/ngalert/models/alert_rule_test.go @@ -166,51 +166,58 @@ func TestPatchPartialAlertRule(t *testing.T) { t.Run("patches", func(t *testing.T) { testCases := []struct { name string - mutator func(r *AlertRule) + mutator func(r *AlertRuleWithOptionals) }{ { name: "title is empty", - mutator: func(r *AlertRule) { + mutator: func(r *AlertRuleWithOptionals) { r.Title = "" }, }, { name: "condition and data are empty", - mutator: func(r *AlertRule) { + mutator: func(r *AlertRuleWithOptionals) { r.Condition = "" r.Data = nil }, }, { name: "ExecErrState is empty", - mutator: func(r *AlertRule) { + mutator: func(r *AlertRuleWithOptionals) { r.ExecErrState = "" }, }, { name: "NoDataState is empty", - mutator: func(r *AlertRule) { + mutator: func(r *AlertRuleWithOptionals) { r.NoDataState = "" }, }, { name: "For is -1", - mutator: func(r *AlertRule) { + mutator: func(r *AlertRuleWithOptionals) { r.For = -1 }, }, + { + name: "IsPaused did not come in request", + mutator: func(r *AlertRuleWithOptionals) { + r.IsPaused = true + }, + }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - var existing *AlertRule + var existing *AlertRuleWithOptionals for { - existing = AlertRuleGen(func(rule *AlertRule) { + rule := AlertRuleGen(func(rule *AlertRule) { rule.For = time.Duration(rand.Int63n(1000) + 1) })() + existing = &AlertRuleWithOptionals{AlertRule: *rule} cloned := *existing testCase.mutator(&cloned) - if !cmp.Equal(*existing, cloned, cmp.FilterPath(func(path cmp.Path) bool { + if !cmp.Equal(existing, cloned, cmp.FilterPath(func(path cmp.Path) bool { return path.String() == "Data.modelProps" }, cmp.Ignore())) { break @@ -220,7 +227,7 @@ func TestPatchPartialAlertRule(t *testing.T) { testCase.mutator(&patch) require.NotEqual(t, *existing, patch) - PatchPartialAlertRule(existing, &patch) + PatchPartialAlertRule(&existing.AlertRule, &patch) require.Equal(t, *existing, patch) }) } @@ -301,10 +308,10 @@ func TestPatchPartialAlertRule(t *testing.T) { break } } - patch := *existing - testCase.mutator(&patch) + patch := AlertRuleWithOptionals{AlertRule: *existing} + testCase.mutator(&patch.AlertRule) PatchPartialAlertRule(existing, &patch) - require.NotEqual(t, *existing, patch) + require.NotEqual(t, *existing, &patch.AlertRule) }) } }) diff --git a/pkg/services/ngalert/provisioning/alert_rules.go b/pkg/services/ngalert/provisioning/alert_rules.go index 18b2ba3f361..c7c9489444c 100644 --- a/pkg/services/ngalert/provisioning/alert_rules.go +++ b/pkg/services/ngalert/provisioning/alert_rules.go @@ -238,13 +238,13 @@ func (service *AlertRuleService) ReplaceRuleGroup(ctx context.Context, orgID int NamespaceUID: group.FolderUID, RuleGroup: group.Title, } - rules := make([]*models.AlertRule, len(group.Rules)) + rules := make([]*models.AlertRuleWithOptionals, len(group.Rules)) group = *syncGroupRuleFields(&group, orgID) for i := range group.Rules { if err := group.Rules[i].SetDashboardAndPanelFromAnnotations(); err != nil { return err } - rules = append(rules, &group.Rules[i]) + rules = append(rules, &models.AlertRuleWithOptionals{AlertRule: group.Rules[i], HasPause: true}) } delta, err := store.CalculateChanges(ctx, service.ruleStore, key, rules) if err != nil { diff --git a/pkg/services/ngalert/store/deltas.go b/pkg/services/ngalert/store/deltas.go index bfb4a8daa15..776e3ade336 100644 --- a/pkg/services/ngalert/store/deltas.go +++ b/pkg/services/ngalert/store/deltas.go @@ -38,7 +38,7 @@ type RuleReader interface { // CalculateChanges calculates the difference between rules in the group in the database and the submitted rules. If a submitted rule has UID it tries to find it in the database (in other groups). // returns a list of rules that need to be added, updated and deleted. Deleted considered rules in the database that belong to the group but do not exist in the list of submitted rules. -func CalculateChanges(ctx context.Context, ruleReader RuleReader, groupKey models.AlertRuleGroupKey, submittedRules []*models.AlertRule) (*GroupDelta, error) { +func CalculateChanges(ctx context.Context, ruleReader RuleReader, groupKey models.AlertRuleGroupKey, submittedRules []*models.AlertRuleWithOptionals) (*GroupDelta, error) { affectedGroups := make(map[models.AlertRuleGroupKey]models.RulesGroup) q := &models.ListAlertRulesQuery{ OrgID: groupKey.OrgID, @@ -93,20 +93,20 @@ func CalculateChanges(ctx context.Context, ruleReader RuleReader, groupKey model } if existing == nil { - toAdd = append(toAdd, r) + toAdd = append(toAdd, &r.AlertRule) continue } models.PatchPartialAlertRule(existing, r) - diff := existing.Diff(r, AlertRuleFieldsToIgnoreInDiff[:]...) + diff := existing.Diff(&r.AlertRule, AlertRuleFieldsToIgnoreInDiff[:]...) if len(diff) == 0 { continue } toUpdate = append(toUpdate, RuleDelta{ Existing: existing, - New: r, + New: &r.AlertRule, Diff: diff, }) continue diff --git a/pkg/services/ngalert/store/deltas_test.go b/pkg/services/ngalert/store/deltas_test.go index f802e3509e6..b47a62ce4a4 100644 --- a/pkg/services/ngalert/store/deltas_test.go +++ b/pkg/services/ngalert/store/deltas_test.go @@ -24,7 +24,11 @@ func TestCalculateChanges(t *testing.T) { fakeStore := fakes.NewRuleStore(t) groupKey := models.GenerateGroupKey(orgId) - submitted := models.GenerateAlertRules(rand.Intn(5)+1, models.AlertRuleGen(withOrgID(orgId), simulateSubmitted, withoutUID)) + rules := models.GenerateAlertRules(rand.Intn(5)+1, models.AlertRuleGen(withOrgID(orgId), simulateSubmitted, withoutUID)) + submitted := make([]*models.AlertRuleWithOptionals, 0, len(rules)) + for _, rule := range rules { + submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *rule}) + } changes, err := CalculateChanges(context.Background(), fakeStore, groupKey, submitted) require.NoError(t, err) @@ -51,7 +55,7 @@ func TestCalculateChanges(t *testing.T) { fakeStore := fakes.NewRuleStore(t) fakeStore.PutRule(context.Background(), inDatabase...) - changes, err := CalculateChanges(context.Background(), fakeStore, groupKey, make([]*models.AlertRule, 0)) + changes, err := CalculateChanges(context.Background(), fakeStore, groupKey, make([]*models.AlertRuleWithOptionals, 0)) require.NoError(t, err) require.Equal(t, groupKey, changes.GroupKey) @@ -70,7 +74,11 @@ func TestCalculateChanges(t *testing.T) { t.Run("should detect alerts that needs to be updated", func(t *testing.T) { groupKey := models.GenerateGroupKey(orgId) inDatabaseMap, inDatabase := models.GenerateUniqueAlertRules(rand.Intn(5)+1, models.AlertRuleGen(withGroupKey(groupKey))) - submittedMap, submitted := models.GenerateUniqueAlertRules(len(inDatabase), models.AlertRuleGen(simulateSubmitted, withGroupKey(groupKey), withUIDs(inDatabaseMap))) + submittedMap, rules := models.GenerateUniqueAlertRules(len(inDatabase), models.AlertRuleGen(simulateSubmitted, withGroupKey(groupKey), withUIDs(inDatabaseMap))) + submitted := make([]*models.AlertRuleWithOptionals, 0, len(rules)) + for _, rule := range rules { + submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *rule}) + } fakeStore := fakes.NewRuleStore(t) fakeStore.PutRule(context.Background(), inDatabase...) @@ -98,7 +106,7 @@ func TestCalculateChanges(t *testing.T) { groupKey := models.GenerateGroupKey(orgId) _, inDatabase := models.GenerateUniqueAlertRules(rand.Intn(5)+1, models.AlertRuleGen(withGroupKey(groupKey))) - submitted := make([]*models.AlertRule, 0, len(inDatabase)) + submitted := make([]*models.AlertRuleWithOptionals, 0, len(inDatabase)) for _, rule := range inDatabase { r := models.CopyRule(rule) @@ -107,7 +115,7 @@ func TestCalculateChanges(t *testing.T) { r.Version = int64(rand.Int31()) r.Updated = r.Updated.Add(1 * time.Minute) - submitted = append(submitted, r) + submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *r}) } fakeStore := fakes.NewRuleStore(t) @@ -171,14 +179,14 @@ func TestCalculateChanges(t *testing.T) { expected := models.AlertRuleGen(simulateSubmitted, testCase.mutator)() expected.UID = dbRule.UID submitted := *expected - changes, err := CalculateChanges(context.Background(), fakeStore, groupKey, []*models.AlertRule{&submitted}) + changes, err := CalculateChanges(context.Background(), fakeStore, groupKey, []*models.AlertRuleWithOptionals{{AlertRule: submitted}}) require.NoError(t, err) require.Len(t, changes.Update, 1) ch := changes.Update[0] require.Equal(t, ch.Existing, dbRule) - fixed := *expected + fixed := models.AlertRuleWithOptionals{AlertRule: *expected} models.PatchPartialAlertRule(dbRule, &fixed) - require.Equal(t, fixed, *ch.New) + require.Equal(t, fixed.AlertRule, *ch.New) }) } }) @@ -199,7 +207,11 @@ func TestCalculateChanges(t *testing.T) { RuleGroup: groupName, } - submittedMap, submitted := models.GenerateUniqueAlertRules(rand.Intn(len(inDatabase)-5)+5, models.AlertRuleGen(simulateSubmitted, withGroupKey(groupKey), withUIDs(inDatabaseMap))) + submittedMap, rules := models.GenerateUniqueAlertRules(rand.Intn(len(inDatabase)-5)+5, models.AlertRuleGen(simulateSubmitted, withGroupKey(groupKey), withUIDs(inDatabaseMap))) + submitted := make([]*models.AlertRuleWithOptionals, 0, len(rules)) + for _, rule := range rules { + submitted = append(submitted, &models.AlertRuleWithOptionals{AlertRule: *rule}) + } changes, err := CalculateChanges(context.Background(), fakeStore, groupKey, submitted) require.NoError(t, err) @@ -228,7 +240,7 @@ func TestCalculateChanges(t *testing.T) { submitted := models.AlertRuleGen(withOrgID(orgId), simulateSubmitted)() require.NotEqual(t, "", submitted.UID) - _, err := CalculateChanges(context.Background(), fakeStore, groupKey, []*models.AlertRule{submitted}) + _, err := CalculateChanges(context.Background(), fakeStore, groupKey, []*models.AlertRuleWithOptionals{{AlertRule: *submitted}}) require.Error(t, err) }) @@ -246,7 +258,7 @@ func TestCalculateChanges(t *testing.T) { groupKey := models.GenerateGroupKey(orgId) submitted := models.AlertRuleGen(withOrgID(orgId), simulateSubmitted, withoutUID)() - _, err := CalculateChanges(context.Background(), fakeStore, groupKey, []*models.AlertRule{submitted}) + _, err := CalculateChanges(context.Background(), fakeStore, groupKey, []*models.AlertRuleWithOptionals{{AlertRule: *submitted}}) require.ErrorIs(t, err, expectedErr) }) @@ -264,7 +276,7 @@ func TestCalculateChanges(t *testing.T) { groupKey := models.GenerateGroupKey(orgId) submitted := models.AlertRuleGen(withOrgID(orgId), simulateSubmitted)() - _, err := CalculateChanges(context.Background(), fakeStore, groupKey, []*models.AlertRule{submitted}) + _, err := CalculateChanges(context.Background(), fakeStore, groupKey, []*models.AlertRuleWithOptionals{{AlertRule: *submitted}}) require.ErrorIs(t, err, expectedErr) }) } diff --git a/pkg/tests/api/alerting/api_alertmanager_test.go b/pkg/tests/api/alerting/api_alertmanager_test.go index b77357cddeb..65ca48ec988 100644 --- a/pkg/tests/api/alerting/api_alertmanager_test.go +++ b/pkg/tests/api/alerting/api_alertmanager_test.go @@ -764,6 +764,7 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) { ], "updated": "2021-05-19T19:47:55Z", "intervalSeconds": 60, + "is_paused": false, "version": 1, "uid": "", "namespace_uid": %q, @@ -1220,6 +1221,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { ], "updated":"2021-02-21T01:10:30Z", "intervalSeconds":60, + "is_paused": false, "version":1, "uid":"uid", "namespace_uid":"nsuid", @@ -1256,6 +1258,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { ], "updated":"2021-02-21T01:10:30Z", "intervalSeconds":60, + "is_paused": false, "version":1, "uid":"uid", "namespace_uid":"nsuid", @@ -1563,6 +1566,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { ], "updated":"2021-02-21T01:10:30Z", "intervalSeconds":60, + "is_paused": false, "version":2, "uid":"uid", "namespace_uid":"nsuid", @@ -1672,6 +1676,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { ], "updated":"2021-02-21T01:10:30Z", "intervalSeconds":60, + "is_paused":false, "version":3, "uid":"uid", "namespace_uid":"nsuid", @@ -1757,6 +1762,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { ], "updated":"2021-02-21T01:10:30Z", "intervalSeconds":60, + "is_paused":false, "version":3, "uid":"uid", "namespace_uid":"nsuid", @@ -2063,6 +2069,7 @@ func TestIntegrationQuota(t *testing.T) { ], "updated":"2021-02-21T01:10:30Z", "intervalSeconds":60, + "is_paused": false, "version":2, "uid":"uid", "namespace_uid":"nsuid", diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index e37e88c03b2..d26981f5b3e 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -113,6 +113,7 @@ func TestIntegrationAlertRulePermissions(t *testing.T) { ], "updated":"2021-02-21T01:10:30Z", "intervalSeconds":60, + "is_paused":false, "version":1, "uid":"uid", "namespace_uid":"nsuid", @@ -163,6 +164,7 @@ func TestIntegrationAlertRulePermissions(t *testing.T) { ], "updated":"2021-02-21T01:10:30Z", "intervalSeconds":60, + "is_paused":false, "version":1, "uid":"uid", "namespace_uid":"nsuid", @@ -236,6 +238,7 @@ func TestIntegrationAlertRulePermissions(t *testing.T) { ], "updated":"2021-02-21T01:10:30Z", "intervalSeconds":60, + "is_paused":false, "version":1, "uid":"uid", "namespace_uid":"nsuid", @@ -510,6 +513,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { }], "updated": "2021-02-21T01:10:30Z", "intervalSeconds": 60, + "is_paused": false, "version": 1, "uid": "uid", "namespace_uid": "nsuid", @@ -543,6 +547,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { }], "updated": "2021-02-21T01:10:30Z", "intervalSeconds": 60, + "is_paused": false, "version": 1, "uid": "uid", "namespace_uid": "nsuid", @@ -588,6 +593,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { }], "updated": "2021-02-21T01:10:30Z", "intervalSeconds": 60, + "is_paused": false, "version": 1, "uid": "uid", "namespace_uid": "nsuid", @@ -935,3 +941,127 @@ func newTestingRuleConfig(t *testing.T) apimodels.PostableRuleGroupConfig { }, } } + +func TestIntegrationRulePause(t *testing.T) { + testinfra.SQLiteIntegrationTest(t) + + // Setup Grafana and its Database + dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + DisableLegacyAlerting: true, + EnableUnifiedAlerting: true, + DisableAnonymous: true, + AppModeProduction: true, + }) + grafanaListedAddr, store := testinfra.StartGrafana(t, dir, path) + + // Create a user to make authenticated requests + createUser(t, store, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleEditor), + Password: "password", + Login: "grafana", + }) + + client := newAlertingApiClient(grafanaListedAddr, "grafana", "password") + folder1Title := "folder1" + client.CreateFolder(t, util.GenerateShortUID(), folder1Title) + + t.Run("should create a paused rule if isPaused is true", func(t *testing.T) { + group := generateAlertRuleGroup(1, alertRuleGen()) + expectedIsPaused := true + group.Rules[0].GrafanaManagedAlert.IsPaused = &expectedIsPaused + + status, body := client.PostRulesGroup(t, folder1Title, &group) + require.Equalf(t, http.StatusAccepted, status, "failed to post rule group. Response: %s", body) + getGroup := client.GetRulesGroup(t, folder1Title, group.Name) + require.Equalf(t, http.StatusAccepted, status, "failed to get rule group. Response: %s", body) + require.Equal(t, expectedIsPaused, getGroup.Rules[0].GrafanaManagedAlert.IsPaused) + }) + + t.Run("should create a unpaused rule if isPaused is false", func(t *testing.T) { + group := generateAlertRuleGroup(1, alertRuleGen()) + expectedIsPaused := false + group.Rules[0].GrafanaManagedAlert.IsPaused = &expectedIsPaused + + status, body := client.PostRulesGroup(t, folder1Title, &group) + require.Equalf(t, http.StatusAccepted, status, "failed to post rule group. Response: %s", body) + getGroup := client.GetRulesGroup(t, folder1Title, group.Name) + require.Equalf(t, http.StatusAccepted, status, "failed to get rule group. Response: %s", body) + require.Equal(t, expectedIsPaused, getGroup.Rules[0].GrafanaManagedAlert.IsPaused) + }) + + t.Run("should create a unpaused rule if isPaused is not present", func(t *testing.T) { + group := generateAlertRuleGroup(1, alertRuleGen()) + group.Rules[0].GrafanaManagedAlert.IsPaused = nil + + status, body := client.PostRulesGroup(t, folder1Title, &group) + require.Equalf(t, http.StatusAccepted, status, "failed to post rule group. Response: %s", body) + getGroup := client.GetRulesGroup(t, folder1Title, group.Name) + require.Equalf(t, http.StatusAccepted, status, "failed to get rule group. Response: %s", body) + require.False(t, getGroup.Rules[0].GrafanaManagedAlert.IsPaused) + }) + + getBooleanPointer := func(b bool) *bool { return &b } + testCases := []struct { + description string + isPausedInDb bool + isPausedInBody *bool + expectedIsPausedInDb bool + }{ + { + description: "should pause rule if there is a paused rule in DB and isPaused is true", + isPausedInDb: true, + isPausedInBody: getBooleanPointer(true), + expectedIsPausedInDb: true, + }, + { + description: "should unpause rule if there is a paused rule in DB and isPaused is false", + isPausedInDb: true, + isPausedInBody: getBooleanPointer(false), + expectedIsPausedInDb: false, + }, + { + description: "should keep rule paused if there is a paused rule in DB and isPaused is not present", + isPausedInDb: true, + isPausedInBody: nil, + expectedIsPausedInDb: true, + }, + { + description: "should pause rule if there is an unpaused rule in DB and isPaused is true", + isPausedInDb: false, + isPausedInBody: getBooleanPointer(true), + expectedIsPausedInDb: true, + }, + { + description: "should unpause rule if there is an unpaused rule in DB and isPaused is false", + isPausedInDb: false, + isPausedInBody: getBooleanPointer(false), + expectedIsPausedInDb: false, + }, + { + description: "should keep rule unpaused if there is an unpaused rule in DB and isPaused is not present", + isPausedInDb: false, + isPausedInBody: nil, + expectedIsPausedInDb: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + group := generateAlertRuleGroup(1, alertRuleGen()) + group.Rules[0].GrafanaManagedAlert.IsPaused = &tc.isPausedInDb + + status, body := client.PostRulesGroup(t, folder1Title, &group) + require.Equalf(t, http.StatusAccepted, status, "failed to post rule group. Response: %s", body) + getGroup := client.GetRulesGroup(t, folder1Title, group.Name) + require.Equalf(t, http.StatusAccepted, status, "failed to get rule group. Response: %s", body) + + group = convertGettableRuleGroupToPostable(getGroup.GettableRuleGroupConfig) + group.Rules[0].GrafanaManagedAlert.IsPaused = tc.isPausedInBody + status, body = client.PostRulesGroup(t, folder1Title, &group) + require.Equalf(t, http.StatusAccepted, status, "failed to post rule group. Response: %s", body) + + getGroup = client.GetRulesGroup(t, folder1Title, group.Name) + require.Equal(t, tc.expectedIsPausedInDb, getGroup.Rules[0].GrafanaManagedAlert.IsPaused) + }) + } +} diff --git a/pkg/tests/api/alerting/testing.go b/pkg/tests/api/alerting/testing.go index 15be8bb188a..60d66627fa0 100644 --- a/pkg/tests/api/alerting/testing.go +++ b/pkg/tests/api/alerting/testing.go @@ -160,6 +160,7 @@ func convertGettableGrafanaRuleToPostable(gettable *apimodels.GettableGrafanaRul UID: gettable.UID, NoDataState: gettable.NoDataState, ExecErrState: gettable.ExecErrState, + IsPaused: &gettable.IsPaused, } } diff --git a/public/api-merged.json b/public/api-merged.json index 9148eb501d5..38154bac4b8 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -15746,6 +15746,9 @@ "Error" ] }, + "is_paused": { + "type": "boolean" + }, "no_data_state": { "type": "string", "enum": [ @@ -19312,7 +19315,6 @@ } }, "receiver": { - "description": "Receiver receiver", "type": "object", "required": [ "active", diff --git a/public/app/features/alerting/unified/RuleEditorExisting.test.tsx b/public/app/features/alerting/unified/RuleEditorExisting.test.tsx index d0dbaf8c397..9862a96baa5 100644 --- a/public/app/features/alerting/unified/RuleEditorExisting.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorExisting.test.tsx @@ -210,6 +210,7 @@ describe('RuleEditor grafana managed rules', () => { condition: 'B', data: getDefaultQueries(), exec_err_state: GrafanaAlertStateDecision.Error, + is_paused: false, no_data_state: 'NoData', title: 'my great new rule', }, diff --git a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx index f35a627b2ca..99e943fb74f 100644 --- a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx @@ -160,6 +160,7 @@ describe('RuleEditor grafana managed rules', () => { condition: 'B', data: getDefaultQueries(), exec_err_state: GrafanaAlertStateDecision.Error, + is_paused: false, no_data_state: 'NoData', title: 'my great new rule', }, diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx index 9d2aede224c..fccb1899192 100644 --- a/public/app/features/alerting/unified/RuleList.test.tsx +++ b/public/app/features/alerting/unified/RuleList.test.tsx @@ -371,8 +371,8 @@ describe('RuleList', () => { const instanceRows = byTestId('row').getAll(instancesTable); expect(instanceRows).toHaveLength(2); - expect(instanceRows![0]).toHaveTextContent('Firingfoo=barseverity=warning2021-03-18 08:47:05'); - expect(instanceRows![1]).toHaveTextContent('Firingfoo=bazseverity=error2021-03-18 08:47:05'); + expect(instanceRows![0]).toHaveTextContent('Firing foo=barseverity=warning2021-03-18 08:47:05'); + expect(instanceRows![1]).toHaveTextContent('Firing foo=bazseverity=error2021-03-18 08:47:05'); // expand details of an instance await userEvent.click(ui.ruleCollapseToggle.get(instanceRows![0])); diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx index 83b9c87c8bf..0e7a6c485a2 100644 --- a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx @@ -255,6 +255,7 @@ export const AlertRuleForm: FC = ({ existing, prefill }) => { initialFolder={defaultValues.folder} evaluateEvery={evaluateEvery} setEvaluateEvery={setEvaluateEvery} + existing={Boolean(existing)} /> ) : ( diff --git a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx index 710e584b09e..5dfae11cf77 100644 --- a/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx @@ -4,7 +4,7 @@ import { RegisterOptions, useFormContext } from 'react-hook-form'; import { GrafanaTheme2, SelectableValue } from '@grafana/data'; import { Stack } from '@grafana/experimental'; -import { Button, Field, InlineLabel, Input, InputControl, useStyles2 } from '@grafana/ui'; +import { Button, Field, InlineLabel, Input, InputControl, useStyles2, Switch, Tooltip, Icon } from '@grafana/ui'; import { RulerRuleDTO, RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; import { logInfo, LogMessages } from '../../Analytics'; @@ -253,14 +253,20 @@ export function GrafanaEvaluationBehavior({ initialFolder, evaluateEvery, setEvaluateEvery, + existing, }: { initialFolder: RuleForm | null; evaluateEvery: string; setEvaluateEvery: (value: string) => void; + existing: boolean; }) { const styles = useStyles2(getStyles); const [showErrorHandling, setShowErrorHandling] = useState(false); + const { watch, setValue } = useFormContext(); + + const isPaused = watch('isPaused'); + return ( // TODO remove "and alert condition" for recording rules @@ -271,6 +277,31 @@ export function GrafanaEvaluationBehavior({ evaluateEvery={evaluateEvery} /> + + {existing && ( + + ( + + { + setValue('isPaused', value.currentTarget.checked); + }} + value={Boolean(isPaused)} + /> + + + )} + name="isPaused" + /> + + )} ({ margin-right: ${theme.spacing(1)}; color: ${theme.colors.warning.text}; `, + infoIcon: css` + margin-left: 10px; + `, warningMessage: css` color: ${theme.colors.warning.text}; `, @@ -354,4 +388,9 @@ const getStyles = (theme: GrafanaTheme2) => ({ marginTop: css` margin-top: ${theme.spacing(1)}; `, + switchLabel: css(` + color: ${theme.colors.text.primary}, + cursor: 'pointer', + fontSize: ${theme.typography.bodySmall.fontSize}, + `), }); diff --git a/public/app/features/alerting/unified/components/rules/AlertStateTag.tsx b/public/app/features/alerting/unified/components/rules/AlertStateTag.tsx index 510da6f2f1e..8b37765405a 100644 --- a/public/app/features/alerting/unified/components/rules/AlertStateTag.tsx +++ b/public/app/features/alerting/unified/components/rules/AlertStateTag.tsx @@ -8,10 +8,11 @@ import { StateTag } from '../StateTag'; interface Props { state: PromAlertingRuleState | GrafanaAlertState | GrafanaAlertStateWithReason | AlertState; size?: 'md' | 'sm'; + isPaused?: boolean; } -export const AlertStateTag: FC = ({ state, size = 'md' }) => ( +export const AlertStateTag: FC = ({ state, isPaused = false, size = 'md' }) => ( - {alertStateToReadable(state)} + {alertStateToReadable(state)} {isPaused ? ' (Paused)' : ''} ); diff --git a/public/app/features/alerting/unified/components/rules/RuleState.tsx b/public/app/features/alerting/unified/components/rules/RuleState.tsx index c94b69b5e1a..017557fc634 100644 --- a/public/app/features/alerting/unified/components/rules/RuleState.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleState.tsx @@ -14,9 +14,10 @@ interface Props { rule: CombinedRule; isDeleting: boolean; isCreating: boolean; + isPaused?: boolean; } -export const RuleState: FC = ({ rule, isDeleting, isCreating }) => { +export const RuleState: FC = ({ rule, isDeleting, isCreating, isPaused }) => { const style = useStyles2(getStyle); const { promRule } = rule; @@ -68,7 +69,7 @@ export const RuleState: FC = ({ rule, isDeleting, isCreating }) => { } else if (promRule && isAlertingRule(promRule)) { return ( - + {forTime} ); diff --git a/public/app/features/alerting/unified/components/rules/RulesTable.tsx b/public/app/features/alerting/unified/components/rules/RulesTable.tsx index 5e1c0fddb0e..04780159320 100644 --- a/public/app/features/alerting/unified/components/rules/RulesTable.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesTable.tsx @@ -114,9 +114,13 @@ function useColumns(showSummaryColumn: boolean, showGroupColumn: boolean) { const { namespace } = rule; const { rulesSource } = namespace; const { promRule, rulerRule } = rule; + const isDeleting = !!(hasRuler(rulesSource) && rulerRulesLoaded(rulesSource) && promRule && !rulerRule); const isCreating = !!(hasRuler(rulesSource) && rulerRulesLoaded(rulesSource) && rulerRule && !promRule); - return ; + const isGrafanaManagedRule = isGrafanaRulerRule(rulerRule); + const isPaused = isGrafanaManagedRule && Boolean(rulerRule.grafana_alert.is_paused); + + return ; }, size: '165px', }, diff --git a/public/app/features/alerting/unified/types/rule-form.ts b/public/app/features/alerting/unified/types/rule-form.ts index 3a097ab778b..5a8e602b9df 100644 --- a/public/app/features/alerting/unified/types/rule-form.ts +++ b/public/app/features/alerting/unified/types/rule-form.ts @@ -29,6 +29,7 @@ export interface RuleFormValues { folder: RuleForm | null; evaluateEvery: string; evaluateFor: string; + isPaused?: boolean; // cortex / loki rules namespace: string; diff --git a/public/app/features/alerting/unified/utils/__snapshots__/rule-form.test.ts.snap b/public/app/features/alerting/unified/utils/__snapshots__/rule-form.test.ts.snap index 77658b56588..bfd1da8464e 100644 --- a/public/app/features/alerting/unified/utils/__snapshots__/rule-form.test.ts.snap +++ b/public/app/features/alerting/unified/utils/__snapshots__/rule-form.test.ts.snap @@ -12,6 +12,7 @@ exports[`formValuesToRulerGrafanaRuleDTO should correctly convert rule form valu "condition": "A", "data": [], "exec_err_state": "Error", + "is_paused": false, "no_data_state": "NoData", "title": "", }, @@ -49,6 +50,7 @@ exports[`formValuesToRulerGrafanaRuleDTO should not save both instant and range }, ], "exec_err_state": "Error", + "is_paused": false, "no_data_state": "NoData", "title": "", }, diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts index a55810ab18b..1c7ac452a7f 100644 --- a/public/app/features/alerting/unified/utils/rule-form.ts +++ b/public/app/features/alerting/unified/utils/rule-form.ts @@ -96,7 +96,7 @@ function listifyLabelsOrAnnotations(item: Labels | Annotations | undefined): Arr } export function formValuesToRulerGrafanaRuleDTO(values: RuleFormValues): PostableRuleGrafanaRuleDTO { - const { name, condition, noDataState, execErrState, evaluateFor, queries } = values; + const { name, condition, noDataState, execErrState, evaluateFor, queries, isPaused } = values; if (condition) { return { grafana_alert: { @@ -105,6 +105,7 @@ export function formValuesToRulerGrafanaRuleDTO(values: RuleFormValues): Postabl no_data_state: noDataState, exec_err_state: execErrState, data: queries.map(fixBothInstantAndRangeQuery), + is_paused: Boolean(isPaused), }, for: evaluateFor, annotations: arrayToRecord(values.annotations || []), @@ -135,6 +136,7 @@ export function rulerRuleToFormValues(ruleWithLocation: RuleWithLocation): RuleF annotations: listifyLabelsOrAnnotations(rule.annotations), labels: listifyLabelsOrAnnotations(rule.labels), folder: { title: namespace, id: ga.namespace_id }, + isPaused: ga.is_paused, }; } else { throw new Error('Unexpected type of rule for grafana rules source'); diff --git a/public/app/types/unified-alerting-dto.ts b/public/app/types/unified-alerting-dto.ts index 4d00880782e..487d8cda0a3 100644 --- a/public/app/types/unified-alerting-dto.ts +++ b/public/app/types/unified-alerting-dto.ts @@ -200,6 +200,7 @@ export interface PostableGrafanaRuleDefinition { no_data_state: GrafanaAlertStateDecision; exec_err_state: GrafanaAlertStateDecision; data: AlertQuery[]; + is_paused?: boolean; } export interface GrafanaRuleDefinition extends PostableGrafanaRuleDefinition { id?: string;