From 19e34e6237189a1ac1afa6fef1dbcbdbae157fe9 Mon Sep 17 00:00:00 2001 From: kittycatbone Date: Wed, 11 Sep 2024 15:40:10 +0300 Subject: [PATCH 1/8] DEV-46410 - Do not send a notification on error or no data state Added the following changes: * Do not send a notification on error or no data state. * Change default ExecErrState to OK * Enforce OK state to validation --- .../ngalert/api/api_ruler_validation.go | 9 ++++-- pkg/services/ngalert/models/alert_rule.go | 9 ++++-- pkg/services/ngalert/models/testing.go | 19 ++++++----- pkg/services/ngalert/state/state.go | 4 +++ pkg/services/ngalert/state/state_test.go | 12 ++++--- pkg/services/ngalert/tests/util.go | 2 +- .../api/alerting/api_alertmanager_test.go | 32 +++++++++++-------- pkg/tests/api/alerting/api_prometheus_test.go | 4 +-- .../api/alerting/api_provisioning_test.go | 2 +- pkg/tests/api/alerting/api_ruler_test.go | 9 +++--- .../alerting/test-data/hysteresis_rule.json | 4 +-- .../rule-notification-settings-1-post.json | 4 +-- .../test-data/rulegroup-1-export.json | 6 ++-- .../alerting/test-data/rulegroup-1-get.json | 6 ++-- .../alerting/test-data/rulegroup-1-post.json | 4 +-- .../test-data/rulegroup-2-export.json | 4 +-- .../alerting/test-data/rulegroup-2-get.json | 4 +-- .../alerting/test-data/rulegroup-2-post.json | 2 +- .../test-data/rulegroup-3-export.json | 6 ++-- .../alerting/test-data/rulegroup-3-get.json | 6 ++-- .../alerting/test-data/rulegroup-3-post.json | 6 ++-- 21 files changed, 91 insertions(+), 63 deletions(-) diff --git a/pkg/services/ngalert/api/api_ruler_validation.go b/pkg/services/ngalert/api/api_ruler_validation.go index 0d13fd0c2c283..e9fdb25b0ebf6 100644 --- a/pkg/services/ngalert/api/api_ruler_validation.go +++ b/pkg/services/ngalert/api/api_ruler_validation.go @@ -54,17 +54,22 @@ func validateRuleNode( } } - errorState := ngmodels.AlertingErrState + errorState := ngmodels.OkErrState // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK if ruleNode.GrafanaManagedAlert.ExecErrState == "" && canPatch { errorState = "" } if ruleNode.GrafanaManagedAlert.ExecErrState != "" { - errorState, err = ngmodels.ErrStateFromString(string(ruleNode.GrafanaManagedAlert.ExecErrState)) if err != nil { return nil, err } + errorState, err = ngmodels.ErrStateFromString(string(ruleNode.GrafanaManagedAlert.ExecErrState)) + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK state to validation + if errorState != ngmodels.OkErrState { + return nil, fmt.Errorf("%w: rule can only be with ExecErrState OK", ngmodels.ErrAlertRuleFailedValidation) + } + // LOGZ.IO GRAFANA CHANGE :: End } if len(ruleNode.GrafanaManagedAlert.Data) == 0 { diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index d6011f6de6086..c0987e8e9cd5e 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -494,10 +494,15 @@ func (alertRule *AlertRule) ValidateAlertRule(cfg setting.UnifiedAlertingSetting if alertRule.DashboardUID == nil && alertRule.PanelID != nil { return fmt.Errorf("%w: cannot have Panel ID without a Dashboard UID", ErrAlertRuleFailedValidation) } - - if _, err := ErrStateFromString(string(alertRule.ExecErrState)); err != nil { + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK state to validation + if errState, err := ErrStateFromString(string(alertRule.ExecErrState)); err != nil { return err + } else { + if errState != OkErrState { + return fmt.Errorf("%w: rule can only be with ExecErrState OK", ErrAlertRuleFailedValidation) + } } + // LOGZ.IO GRAFANA CHANGE :: End if _, err := NoDataStateFromString(string(alertRule.NoDataState)); err != nil { return err diff --git a/pkg/services/ngalert/models/testing.go b/pkg/services/ngalert/models/testing.go index 71ffd9312a965..db8784c319879 100644 --- a/pkg/services/ngalert/models/testing.go +++ b/pkg/services/ngalert/models/testing.go @@ -35,14 +35,17 @@ func AlertRuleGen(mutators ...AlertRuleMutator) func() *AlertRule { return s[rand.Intn(len(s))] } - randErrState := func() ExecutionErrorState { - s := [...]ExecutionErrorState{ - AlertingErrState, - ErrorErrState, - OkErrState, - } - return s[rand.Intn(len(s))] - } + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK state to validation + //randErrState := func() ExecutionErrorState { + // s := [...]ExecutionErrorState{ + // AlertingErrState, + // ErrorErrState, + // OkErrState, + // } + // return s[rand.Intn(len(s))] + //} + randErrState := func() ExecutionErrorState { return OkErrState } + // LOGZ.IO GRAFANA CHANGE :: End interval := (rand.Int63n(6) + 1) * 10 forInterval := time.Duration(interval*rand.Int63n(6)) * time.Second diff --git a/pkg/services/ngalert/state/state.go b/pkg/services/ngalert/state/state.go index 11c86f6b10e41..2d9ae9a0bee1e 100644 --- a/pkg/services/ngalert/state/state.go +++ b/pkg/services/ngalert/state/state.go @@ -408,6 +408,10 @@ func (a *State) NeedsSending(resendDelay time.Duration) bool { case eval.Normal: // We should send a notification if the state is Normal because it was resolved return a.Resolved + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Do not send a notification on error or no data state + case eval.NoData, eval.Error: + return false + // LOGZ.IO GRAFANA CHANGE :: end default: // We should send, and re-send notifications, each time LastSentAt is <= LastEvaluationTime + resendDelay nextSent := a.LastSentAt.Add(resendDelay) diff --git a/pkg/services/ngalert/state/state_test.go b/pkg/services/ngalert/state/state_test.go index 1684b7036c17f..5845535192333 100644 --- a/pkg/services/ngalert/state/state_test.go +++ b/pkg/services/ngalert/state/state_test.go @@ -426,8 +426,10 @@ func TestNeedsSending(t *testing.T) { }, }, { - name: "state: no-data, needs to be re-sent", - expected: true, + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Do not send a notification on error or no data state + name: "state: no-data, should not be re-sent with evaluation time ", + expected: false, + // LOGZ.IO GRAFANA CHANGE :: End resendDelay: 1 * time.Minute, testState: &State{ State: eval.NoData, @@ -446,8 +448,10 @@ func TestNeedsSending(t *testing.T) { }, }, { - name: "state: error, needs to be re-sent", - expected: true, + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Do not send a notification on error or no data state + name: "state: error, should not be re-sent with evaluation time", + expected: false, + // LOGZ.IO GRAFANA CHANGE :: End resendDelay: 1 * time.Minute, testState: &State{ State: eval.Error, diff --git a/pkg/services/ngalert/tests/util.go b/pkg/services/ngalert/tests/util.go index 6466d90c46a91..7c7d05413611a 100644 --- a/pkg/services/ngalert/tests/util.go +++ b/pkg/services/ngalert/tests/util.go @@ -133,7 +133,7 @@ func CreateTestAlertRuleWithLabels(t testing.TB, ctx context.Context, dbstore *s NamespaceUID: folderUID, RuleGroup: ruleGroup, NoDataState: models.NoData, - ExecErrState: models.AlertingErrState, + ExecErrState: models.OkErrState, // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK }, }) require.NoError(t, err) diff --git a/pkg/tests/api/alerting/api_alertmanager_test.go b/pkg/tests/api/alerting/api_alertmanager_test.go index 4ab44c164dae7..6df7a638de861 100644 --- a/pkg/tests/api/alerting/api_alertmanager_test.go +++ b/pkg/tests/api/alerting/api_alertmanager_test.go @@ -731,6 +731,7 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) { re = regexp.MustCompile(`"updated":"(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z)"`) b = re.ReplaceAll(b, []byte(`"updated":"2021-05-19T19:47:55Z"`)) + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK expectedGetRulesResponseBody := fmt.Sprintf(`{ "default": [ { @@ -776,7 +777,7 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) { "namespace_uid": %q, "rule_group": "arulegroup", "no_data_state": "NoData", - "exec_err_state": "Alerting" + "exec_err_state": "OK" } } ] @@ -1170,7 +1171,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), + ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK }, }, }, @@ -1207,6 +1208,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { // copy result to a variable with a wider scope // to be used by the next test ruleUID = generatedUIDs[0] + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK expectedGetNamespaceResponseBody = ` { "default":[ @@ -1253,7 +1255,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "namespace_uid":"nsuid", "rule_group":"arulegroup", "no_data_state":"NoData", - "exec_err_state":"Alerting" + "exec_err_state":"OK" } }, { @@ -1289,7 +1291,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "namespace_uid":"nsuid", "rule_group":"arulegroup", "no_data_state":"Alerting", - "exec_err_state":"Alerting" + "exec_err_state":"OK" } } ] @@ -1338,7 +1340,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), + ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK }, }, }, @@ -1411,7 +1413,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), + ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK }, }, { @@ -1445,7 +1447,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), + ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK }, }, }, @@ -1519,7 +1521,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), + ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK }, }, }, @@ -1549,6 +1551,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { assert.True(t, ok) assert.Equal(t, 1, len(returnedUIDs)) assert.Equal(t, ruleUID, returnedUIDs[0]) + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK assert.JSONEq(t, ` { "default":[ @@ -1597,7 +1600,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "namespace_uid":"nsuid", "rule_group":"arulegroup", "no_data_state":"Alerting", - "exec_err_state":"Alerting" + "exec_err_state":"OK" } } ] @@ -1637,7 +1640,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), + ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK }, }, }, @@ -1666,6 +1669,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { assert.True(t, ok) assert.Equal(t, 1, len(returnedUIDs)) assert.Equal(t, ruleUID, returnedUIDs[0]) + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK assert.JSONEq(t, ` { "default":[ @@ -1706,7 +1710,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "namespace_uid":"nsuid", "rule_group":"arulegroup", "no_data_state":"Alerting", - "exec_err_state":"Alerting" + "exec_err_state":"OK" } } ] @@ -1754,6 +1758,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { assert.True(t, ok) assert.Equal(t, 1, len(returnedUIDs)) assert.Equal(t, ruleUID, returnedUIDs[0]) + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK assert.JSONEq(t, ` { "default":[ @@ -1794,7 +1799,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { "namespace_uid":"nsuid", "rule_group":"arulegroup", "no_data_state":"Alerting", - "exec_err_state":"Alerting" + "exec_err_state":"OK" } } ] @@ -2209,6 +2214,7 @@ func TestIntegrationQuota(t *testing.T) { assert.True(t, ok) assert.Equal(t, 1, len(returnedUIDs)) assert.Equal(t, ruleUID, returnedUIDs[0]) + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK assert.JSONEq(t, ` { "default":[ @@ -2249,7 +2255,7 @@ func TestIntegrationQuota(t *testing.T) { "namespace_uid":"nsuid", "rule_group":"arulegroup", "no_data_state":"NoData", - "exec_err_state":"Alerting" + "exec_err_state":"OK" } } ] diff --git a/pkg/tests/api/alerting/api_prometheus_test.go b/pkg/tests/api/alerting/api_prometheus_test.go index aa9e7cfcf77fb..908867285967e 100644 --- a/pkg/tests/api/alerting/api_prometheus_test.go +++ b/pkg/tests/api/alerting/api_prometheus_test.go @@ -135,7 +135,7 @@ func TestIntegrationPrometheusRules(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), + ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK }, }, }, @@ -416,7 +416,7 @@ func TestIntegrationPrometheusRulesFilterByDashboard(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), + ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK }, }, }, diff --git a/pkg/tests/api/alerting/api_provisioning_test.go b/pkg/tests/api/alerting/api_provisioning_test.go index c0f7e6a48084a..b5bbc362d2457 100644 --- a/pkg/tests/api/alerting/api_provisioning_test.go +++ b/pkg/tests/api/alerting/api_provisioning_test.go @@ -351,7 +351,7 @@ func TestIntegrationProvisioning(t *testing.T) { t.Run("when provisioning alert rules", func(t *testing.T) { url := fmt.Sprintf("http://%s/api/v1/provisioning/alert-rules", grafanaListedAddr) - body := `{"orgID":1,"folderUID":"default","ruleGroup":"Test Group","title":"Provisioned","condition":"A","data":[{"refId":"A","queryType":"","relativeTimeRange":{"from":600,"to":0},"datasourceUid":"f558c85f-66ad-4fd1-b31d-7979e6c93db4","model":{"editorMode":"code","exemplar":false,"expr":"sum(rate(low_card[5m])) \u003e 0","format":"time_series","instant":true,"intervalMs":1000,"legendFormat":"__auto","maxDataPoints":43200,"range":false,"refId":"A"}}],"noDataState":"NoData","execErrState":"Error","for":"0s"}` + body := `{"orgID":1,"folderUID":"default","ruleGroup":"Test Group","title":"Provisioned","condition":"A","data":[{"refId":"A","queryType":"","relativeTimeRange":{"from":600,"to":0},"datasourceUid":"f558c85f-66ad-4fd1-b31d-7979e6c93db4","model":{"editorMode":"code","exemplar":false,"expr":"sum(rate(low_card[5m])) \u003e 0","format":"time_series","instant":true,"intervalMs":1000,"legendFormat":"__auto","maxDataPoints":43200,"range":false,"refId":"A"}}],"noDataState":"NoData","execErrState":"OK","for":"0s"}` // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK req := createTestRequest("POST", url, "admin", body) resp, err := http.DefaultClient.Do(req) require.NoError(t, err) diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index cf9186d8f87fd..b704f00ff2de9 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -971,7 +971,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), + ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK }, }, }, @@ -981,6 +981,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { require.Len(t, resp.Created, len(rules.Rules)) } + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK expectedAllJSON := fmt.Sprintf(` { "default": [{ @@ -1021,7 +1022,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { "namespace_uid": "nsuid", "rule_group": "anotherrulegroup", "no_data_state": "NoData", - "exec_err_state": "Alerting" + "exec_err_state": "OK" } }, { "expr": "", @@ -1054,7 +1055,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { "namespace_uid": "nsuid", "rule_group": "anotherrulegroup", "no_data_state": "Alerting", - "exec_err_state": "Alerting" + "exec_err_state": "OK" } }] }] @@ -1099,7 +1100,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { "namespace_uid": "nsuid", "rule_group": "anotherrulegroup", "no_data_state": "NoData", - "exec_err_state": "Alerting" + "exec_err_state": "OK" } }] }] diff --git a/pkg/tests/api/alerting/test-data/hysteresis_rule.json b/pkg/tests/api/alerting/test-data/hysteresis_rule.json index 53a01c9ac5e9e..feff2d2d1be23 100644 --- a/pkg/tests/api/alerting/test-data/hysteresis_rule.json +++ b/pkg/tests/api/alerting/test-data/hysteresis_rule.json @@ -7,7 +7,7 @@ "title": "Hysteresis Test", "condition": "C", "no_data_state": "NoData", - "exec_err_state": "Error", + "exec_err_state": "OK", "data": [ { "refId": "A", @@ -68,4 +68,4 @@ } } ] -} \ No newline at end of file +} diff --git a/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json b/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json index 0331fd94e27a9..0ff65c3c9cbd2 100644 --- a/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json +++ b/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json @@ -25,7 +25,7 @@ } ], "no_data_state": "NoData", - "exec_err_state": "Alerting", + "exec_err_state": "OK", "notification_settings": { "receiver": "rule-receiver", "group_by": [ @@ -55,4 +55,4 @@ "name": "rule-time-interval", "time_intervals":[{"times":[{"start_time":"10:00","end_time":"12:00"}]}] } -} \ No newline at end of file +} diff --git a/pkg/tests/api/alerting/test-data/rulegroup-1-export.json b/pkg/tests/api/alerting/test-data/rulegroup-1-export.json index b031d95ea02e0..088744d699cc4 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-1-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-1-export.json @@ -28,7 +28,7 @@ } ], "noDataState": "NoData", - "execErrState": "Alerting", + "execErrState": "OK", "for": "5m", "annotations": { "annotation": "test-annotation" @@ -59,7 +59,7 @@ } ], "noDataState": "NoData", - "execErrState": "Alerting", + "execErrState": "OK", "for": "5m", "annotations": { "annotation": "test-annotation" @@ -72,4 +72,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/pkg/tests/api/alerting/test-data/rulegroup-1-get.json b/pkg/tests/api/alerting/test-data/rulegroup-1-get.json index 27474baf8e346..51ee23b86024b 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-1-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-1-get.json @@ -40,7 +40,7 @@ "namespace_uid": "", "rule_group": "Group1", "no_data_state": "NoData", - "exec_err_state": "Alerting", + "exec_err_state": "OK", "is_paused": false } }, @@ -82,9 +82,9 @@ "namespace_uid": "", "rule_group": "Group1", "no_data_state": "NoData", - "exec_err_state": "Alerting", + "exec_err_state": "OK", "is_paused": false } } ] -} \ No newline at end of file +} diff --git a/pkg/tests/api/alerting/test-data/rulegroup-1-post.json b/pkg/tests/api/alerting/test-data/rulegroup-1-post.json index 269415be3ca76..e912122f593c2 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-1-post.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-1-post.json @@ -24,7 +24,7 @@ } ], "no_data_state": "NoData", - "exec_err_state": "Alerting" + "exec_err_state": "OK" } }, { @@ -49,7 +49,7 @@ } ], "no_data_state": "NoData", - "exec_err_state": "Alerting" + "exec_err_state": "OK" } } ] diff --git a/pkg/tests/api/alerting/test-data/rulegroup-2-export.json b/pkg/tests/api/alerting/test-data/rulegroup-2-export.json index 5f85d830260b2..043b068aad05a 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-2-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-2-export.json @@ -28,7 +28,7 @@ } ], "noDataState": "NoData", - "execErrState": "Error", + "execErrState": "OK", "for": "5m", "annotations": { "annotation": "test-annotation" @@ -41,4 +41,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/pkg/tests/api/alerting/test-data/rulegroup-2-get.json b/pkg/tests/api/alerting/test-data/rulegroup-2-get.json index dc29a8b2bd23b..63a4ed9a334b4 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-2-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-2-get.json @@ -40,9 +40,9 @@ "namespace_uid": "", "rule_group": "Group2", "no_data_state": "NoData", - "exec_err_state": "Error", + "exec_err_state": "OK", "is_paused": false } } ] -} \ No newline at end of file +} diff --git a/pkg/tests/api/alerting/test-data/rulegroup-2-post.json b/pkg/tests/api/alerting/test-data/rulegroup-2-post.json index 9258c1fa1410e..ae3f97010eed6 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-2-post.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-2-post.json @@ -24,7 +24,7 @@ } ], "no_data_state": "NoData", - "exec_err_state": "Error" + "exec_err_state": "OK" } } ] diff --git a/pkg/tests/api/alerting/test-data/rulegroup-3-export.json b/pkg/tests/api/alerting/test-data/rulegroup-3-export.json index 5a1ba9e0eac1a..0af1e0880d247 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-3-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-3-export.json @@ -28,7 +28,7 @@ } ], "noDataState": "NoData", - "execErrState": "Alerting", + "execErrState": "OK", "for": "5m", "annotations": { "annotation": "test-annotation" @@ -59,7 +59,7 @@ } ], "noDataState": "NoData", - "execErrState": "Alerting", + "execErrState": "OK", "for": "5m", "annotations": { "annotation": "test-annotation" @@ -72,4 +72,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/pkg/tests/api/alerting/test-data/rulegroup-3-get.json b/pkg/tests/api/alerting/test-data/rulegroup-3-get.json index 252dc99f13be1..fec0b9980d837 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-3-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-3-get.json @@ -40,7 +40,7 @@ "namespace_uid": "", "rule_group": "Group3", "no_data_state": "NoData", - "exec_err_state": "Alerting", + "exec_err_state": "OK", "is_paused": false } }, @@ -82,9 +82,9 @@ "namespace_uid": "", "rule_group": "Group3", "no_data_state": "NoData", - "exec_err_state": "Alerting", + "exec_err_state": "OK", "is_paused": false } } ] -} \ No newline at end of file +} diff --git a/pkg/tests/api/alerting/test-data/rulegroup-3-post.json b/pkg/tests/api/alerting/test-data/rulegroup-3-post.json index 2f528d19ad1cc..7084dfc721066 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-3-post.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-3-post.json @@ -24,7 +24,7 @@ } ], "no_data_state": "NoData", - "exec_err_state": "Alerting" + "exec_err_state": "OK" } }, { @@ -49,8 +49,8 @@ } ], "no_data_state": "NoData", - "exec_err_state": "Alerting" + "exec_err_state": "OK" } } ] -} \ No newline at end of file +} From e572a3f17b1e2135cd16ebff86e6a76870f6d402 Mon Sep 17 00:00:00 2001 From: kittycatbone Date: Wed, 11 Sep 2024 15:52:02 +0300 Subject: [PATCH 2/8] fix comment --- pkg/services/ngalert/api/api_ruler_validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/services/ngalert/api/api_ruler_validation.go b/pkg/services/ngalert/api/api_ruler_validation.go index e9fdb25b0ebf6..964b1f6f634e1 100644 --- a/pkg/services/ngalert/api/api_ruler_validation.go +++ b/pkg/services/ngalert/api/api_ruler_validation.go @@ -54,7 +54,7 @@ func validateRuleNode( } } - errorState := ngmodels.OkErrState // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + errorState := ngmodels.OkErrState // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK if ruleNode.GrafanaManagedAlert.ExecErrState == "" && canPatch { errorState = "" From 94035f24f4dfa2eab9bb879189faceff9a212c3f Mon Sep 17 00:00:00 2001 From: yasmin-tr Date: Tue, 17 Sep 2024 20:01:34 +0300 Subject: [PATCH 3/8] * switch from validation to enforcing OK value * fix tests --- .../ngalert/api/api_ruler_validation.go | 31 +++++++++---------- .../ngalert/migration/migration_test.go | 2 +- .../ngalert/migration/permissions_test.go | 2 +- .../ngalert/migration/service_test.go | 2 +- pkg/services/ngalert/models/alert_rule.go | 9 ------ pkg/services/ngalert/models/testing.go | 19 +++++------- pkg/services/ngalert/store/alert_rule.go | 1 + pkg/services/ngalert/tests/util.go | 2 +- .../api/alerting/api_alertmanager_test.go | 12 +++---- pkg/tests/api/alerting/api_prometheus_test.go | 4 +-- .../api/alerting/api_provisioning_test.go | 2 +- pkg/tests/api/alerting/api_ruler_test.go | 20 ++++++++++-- .../alerting/test-data/hysteresis_rule.json | 2 +- .../rule-notification-settings-1-post.json | 2 +- .../test-data/rulegroup-1-export.json | 4 +-- .../alerting/test-data/rulegroup-1-get.json | 4 +-- .../alerting/test-data/rulegroup-1-post.json | 4 +-- .../test-data/rulegroup-2-export.json | 2 +- .../alerting/test-data/rulegroup-2-get.json | 2 +- .../alerting/test-data/rulegroup-2-post.json | 2 +- .../test-data/rulegroup-3-export.json | 4 +-- .../alerting/test-data/rulegroup-3-get.json | 4 +-- .../alerting/test-data/rulegroup-3-post.json | 4 +-- 23 files changed, 71 insertions(+), 69 deletions(-) diff --git a/pkg/services/ngalert/api/api_ruler_validation.go b/pkg/services/ngalert/api/api_ruler_validation.go index 964b1f6f634e1..e8fabc30b6e7d 100644 --- a/pkg/services/ngalert/api/api_ruler_validation.go +++ b/pkg/services/ngalert/api/api_ruler_validation.go @@ -54,23 +54,20 @@ func validateRuleNode( } } - errorState := ngmodels.OkErrState // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK - - if ruleNode.GrafanaManagedAlert.ExecErrState == "" && canPatch { - errorState = "" - } - - if ruleNode.GrafanaManagedAlert.ExecErrState != "" { - if err != nil { - return nil, err - } - errorState, err = ngmodels.ErrStateFromString(string(ruleNode.GrafanaManagedAlert.ExecErrState)) - // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK state to validation - if errorState != ngmodels.OkErrState { - return nil, fmt.Errorf("%w: rule can only be with ExecErrState OK", ngmodels.ErrAlertRuleFailedValidation) - } - // LOGZ.IO GRAFANA CHANGE :: End - } + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value + errorState := ngmodels.OkErrState + + //if ruleNode.GrafanaManagedAlert.ExecErrState == "" && canPatch { + // errorState = "" + //} + // + //if ruleNode.GrafanaManagedAlert.ExecErrState != "" { + // if err != nil { + // return nil, err + // } + // errorState, err = ngmodels.ErrStateFromString(string(ruleNode.GrafanaManagedAlert.ExecErrState)) + //} + // LOGZ.IO GRAFANA CHANGE :: End if len(ruleNode.GrafanaManagedAlert.Data) == 0 { if canPatch { diff --git a/pkg/services/ngalert/migration/migration_test.go b/pkg/services/ngalert/migration/migration_test.go index ce887535d436e..67a5126264706 100644 --- a/pkg/services/ngalert/migration/migration_test.go +++ b/pkg/services/ngalert/migration/migration_test.go @@ -1235,7 +1235,7 @@ func TestDashAlertQueryMigration(t *testing.T) { return a.ID < b.ID }), cmpopts.IgnoreUnexported(ngModels.AlertRule{}, ngModels.AlertQuery{}), - cmpopts.IgnoreFields(ngModels.AlertRule{}, "Updated", "UID", "ID"), + cmpopts.IgnoreFields(ngModels.AlertRule{}, "Updated", "UID", "ID", "ExecErrState"), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } if tt.expectedFolder != nil { cOpt = append(cOpt, cmpopts.IgnoreFields(ngModels.AlertRule{}, "NamespaceUID")) diff --git a/pkg/services/ngalert/migration/permissions_test.go b/pkg/services/ngalert/migration/permissions_test.go index 195b0a752002e..2299ba7649c54 100644 --- a/pkg/services/ngalert/migration/permissions_test.go +++ b/pkg/services/ngalert/migration/permissions_test.go @@ -744,7 +744,7 @@ func TestDashAlertPermissionMigration(t *testing.T) { return a.Permission < b.Permission }), cmpopts.IgnoreUnexported(ngModels.AlertRule{}, ngModels.AlertQuery{}), - cmpopts.IgnoreFields(ngModels.AlertRule{}, "ID", "Updated", "UID"), + cmpopts.IgnoreFields(ngModels.AlertRule{}, "ID", "Updated", "UID", "ExecErrState"), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value cmpopts.IgnoreFields(dashboards.Dashboard{}, "ID", "Created", "Updated", "Data", "Slug"), } if !cmp.Equal(tt.expected, actual, cOpt...) { diff --git a/pkg/services/ngalert/migration/service_test.go b/pkg/services/ngalert/migration/service_test.go index 02edb31b4e477..4e03cb3b5e1c4 100644 --- a/pkg/services/ngalert/migration/service_test.go +++ b/pkg/services/ngalert/migration/service_test.go @@ -1617,7 +1617,7 @@ func compareRules(t *testing.T, x *xorm.Engine, orgId int64, expectedRules []*mo return a.Title < b.Title }), cmpopts.IgnoreUnexported(models.AlertRule{}, models.AlertQuery{}), - cmpopts.IgnoreFields(models.AlertRule{}, "Updated", "UID", "ID", "Version"), + cmpopts.IgnoreFields(models.AlertRule{}, "Updated", "UID", "ID", "Version", "ExecErrState"), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } if !cmp.Equal(expectedRules, rules, cOpt...) { t.Errorf("Unexpected Rule: %v", cmp.Diff(expectedRules, rules, cOpt...)) diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index c0987e8e9cd5e..b3ea5d5b3348c 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -494,15 +494,6 @@ func (alertRule *AlertRule) ValidateAlertRule(cfg setting.UnifiedAlertingSetting if alertRule.DashboardUID == nil && alertRule.PanelID != nil { return fmt.Errorf("%w: cannot have Panel ID without a Dashboard UID", ErrAlertRuleFailedValidation) } - // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK state to validation - if errState, err := ErrStateFromString(string(alertRule.ExecErrState)); err != nil { - return err - } else { - if errState != OkErrState { - return fmt.Errorf("%w: rule can only be with ExecErrState OK", ErrAlertRuleFailedValidation) - } - } - // LOGZ.IO GRAFANA CHANGE :: End if _, err := NoDataStateFromString(string(alertRule.NoDataState)); err != nil { return err diff --git a/pkg/services/ngalert/models/testing.go b/pkg/services/ngalert/models/testing.go index db8784c319879..71ffd9312a965 100644 --- a/pkg/services/ngalert/models/testing.go +++ b/pkg/services/ngalert/models/testing.go @@ -35,17 +35,14 @@ func AlertRuleGen(mutators ...AlertRuleMutator) func() *AlertRule { return s[rand.Intn(len(s))] } - // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK state to validation - //randErrState := func() ExecutionErrorState { - // s := [...]ExecutionErrorState{ - // AlertingErrState, - // ErrorErrState, - // OkErrState, - // } - // return s[rand.Intn(len(s))] - //} - randErrState := func() ExecutionErrorState { return OkErrState } - // LOGZ.IO GRAFANA CHANGE :: End + randErrState := func() ExecutionErrorState { + s := [...]ExecutionErrorState{ + AlertingErrState, + ErrorErrState, + OkErrState, + } + return s[rand.Intn(len(s))] + } interval := (rand.Int63n(6) + 1) * 10 forInterval := time.Duration(interval*rand.Int63n(6)) * time.Second diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index c8d95a4108df2..c36f136ae121d 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -137,6 +137,7 @@ func (st DBstore) InsertAlertRules(ctx context.Context, rules []ngmodels.AlertRu r.UID = uid } r.Version = 1 + r.ExecErrState = ngmodels.OkErrState // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value if err := st.validateAlertRule(r); err != nil { return err } diff --git a/pkg/services/ngalert/tests/util.go b/pkg/services/ngalert/tests/util.go index 7c7d05413611a..6466d90c46a91 100644 --- a/pkg/services/ngalert/tests/util.go +++ b/pkg/services/ngalert/tests/util.go @@ -133,7 +133,7 @@ func CreateTestAlertRuleWithLabels(t testing.TB, ctx context.Context, dbstore *s NamespaceUID: folderUID, RuleGroup: ruleGroup, NoDataState: models.NoData, - ExecErrState: models.OkErrState, // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + ExecErrState: models.AlertingErrState, }, }) require.NoError(t, err) diff --git a/pkg/tests/api/alerting/api_alertmanager_test.go b/pkg/tests/api/alerting/api_alertmanager_test.go index 6df7a638de861..b61367c4dff75 100644 --- a/pkg/tests/api/alerting/api_alertmanager_test.go +++ b/pkg/tests/api/alerting/api_alertmanager_test.go @@ -1171,7 +1171,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), }, }, }, @@ -1340,7 +1340,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), }, }, }, @@ -1413,7 +1413,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), }, }, { @@ -1447,7 +1447,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), }, }, }, @@ -1521,7 +1521,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), }, }, }, @@ -1640,7 +1640,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), }, }, }, diff --git a/pkg/tests/api/alerting/api_prometheus_test.go b/pkg/tests/api/alerting/api_prometheus_test.go index 908867285967e..aa9e7cfcf77fb 100644 --- a/pkg/tests/api/alerting/api_prometheus_test.go +++ b/pkg/tests/api/alerting/api_prometheus_test.go @@ -135,7 +135,7 @@ func TestIntegrationPrometheusRules(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), }, }, }, @@ -416,7 +416,7 @@ func TestIntegrationPrometheusRulesFilterByDashboard(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), }, }, }, diff --git a/pkg/tests/api/alerting/api_provisioning_test.go b/pkg/tests/api/alerting/api_provisioning_test.go index b5bbc362d2457..c0f7e6a48084a 100644 --- a/pkg/tests/api/alerting/api_provisioning_test.go +++ b/pkg/tests/api/alerting/api_provisioning_test.go @@ -351,7 +351,7 @@ func TestIntegrationProvisioning(t *testing.T) { t.Run("when provisioning alert rules", func(t *testing.T) { url := fmt.Sprintf("http://%s/api/v1/provisioning/alert-rules", grafanaListedAddr) - body := `{"orgID":1,"folderUID":"default","ruleGroup":"Test Group","title":"Provisioned","condition":"A","data":[{"refId":"A","queryType":"","relativeTimeRange":{"from":600,"to":0},"datasourceUid":"f558c85f-66ad-4fd1-b31d-7979e6c93db4","model":{"editorMode":"code","exemplar":false,"expr":"sum(rate(low_card[5m])) \u003e 0","format":"time_series","instant":true,"intervalMs":1000,"legendFormat":"__auto","maxDataPoints":43200,"range":false,"refId":"A"}}],"noDataState":"NoData","execErrState":"OK","for":"0s"}` // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + body := `{"orgID":1,"folderUID":"default","ruleGroup":"Test Group","title":"Provisioned","condition":"A","data":[{"refId":"A","queryType":"","relativeTimeRange":{"from":600,"to":0},"datasourceUid":"f558c85f-66ad-4fd1-b31d-7979e6c93db4","model":{"editorMode":"code","exemplar":false,"expr":"sum(rate(low_card[5m])) \u003e 0","format":"time_series","instant":true,"intervalMs":1000,"legendFormat":"__auto","maxDataPoints":43200,"range":false,"refId":"A"}}],"noDataState":"NoData","execErrState":"Error","for":"0s"}` req := createTestRequest("POST", url, "admin", body) resp, err := http.DefaultClient.Do(req) require.NoError(t, err) diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index b704f00ff2de9..a90e2f33d1597 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -123,6 +123,7 @@ func TestIntegrationAlertRulePermissions(t *testing.T) { "GrafanaManagedAlert.Data.Model", "GrafanaManagedAlert.NamespaceUID", "GrafanaManagedAlert.NamespaceID", + "GrafanaManagedAlert.ExecErrState", // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } // compare expected and actual and ignore the dynamic fields @@ -139,9 +140,11 @@ func TestIntegrationAlertRulePermissions(t *testing.T) { for _, rule := range allRules["folder1"][0].Rules { assert.Equal(t, "folder1", rule.GrafanaManagedAlert.NamespaceUID) + require.Equal(t, rule.GrafanaManagedAlert.ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } for _, rule := range allRules["folder2"][0].Rules { assert.Equal(t, "folder2", rule.GrafanaManagedAlert.NamespaceUID) + require.Equal(t, rule.GrafanaManagedAlert.ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } }) @@ -173,6 +176,7 @@ func TestIntegrationAlertRulePermissions(t *testing.T) { pathsToIgnore := []string{ "Groups.Rules.UID", "Groups.Folder", + "Groups.Rules.ExecErrState", // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } // compare expected and actual and ignore the dynamic fields @@ -189,6 +193,8 @@ func TestIntegrationAlertRulePermissions(t *testing.T) { require.Equal(t, "folder1", allExport.Groups[0].Folder) require.Equal(t, "folder2", allExport.Groups[1].Folder) + require.Equal(t, allExport.Groups[0].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value + require.Equal(t, allExport.Groups[1].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value }) t.Run("Export from one folder", func(t *testing.T) { @@ -424,6 +430,7 @@ func TestIntegrationAlertRuleNestedPermissions(t *testing.T) { "GrafanaManagedAlert.Data.Model", "GrafanaManagedAlert.NamespaceUID", "GrafanaManagedAlert.NamespaceID", + "GrafanaManagedAlert.ExecErrState", // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } // compare expected and actual and ignore the dynamic fields @@ -440,14 +447,17 @@ func TestIntegrationAlertRuleNestedPermissions(t *testing.T) { for _, rule := range allRules["folder1"][0].Rules { assert.Equal(t, "folder1", rule.GrafanaManagedAlert.NamespaceUID) + require.Equal(t, rule.GrafanaManagedAlert.ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } for _, rule := range allRules["folder2"][0].Rules { assert.Equal(t, "folder2", rule.GrafanaManagedAlert.NamespaceUID) + require.Equal(t, rule.GrafanaManagedAlert.ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } for _, rule := range allRules["folder1/subfolder"][0].Rules { assert.Equal(t, "subfolder", rule.GrafanaManagedAlert.NamespaceUID) + require.Equal(t, rule.GrafanaManagedAlert.ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } }) @@ -493,6 +503,7 @@ func TestIntegrationAlertRuleNestedPermissions(t *testing.T) { pathsToIgnore := []string{ "Groups.Rules.UID", "Groups.Folder", + "Groups.Rules.ExecErrState", // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } // compare expected and actual and ignore the dynamic fields @@ -510,6 +521,9 @@ func TestIntegrationAlertRuleNestedPermissions(t *testing.T) { require.Equal(t, "folder1", allExport.Groups[0].Folder) require.Equal(t, "folder2", allExport.Groups[1].Folder) require.Equal(t, "subfolder", allExport.Groups[2].Folder) + require.Equal(t, allExport.Groups[0].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value + require.Equal(t, allExport.Groups[1].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value + require.Equal(t, allExport.Groups[2].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value }) t.Run("Export from one folder", func(t *testing.T) { @@ -768,7 +782,8 @@ func TestAlertRulePostExport(t *testing.T) { pathsToIgnore := []string{ "Groups.Rules.UID", "Groups.Folder", - "Data.Model", // Model is not amended with default values + "Data.Model", // Model is not amended with default values + "Groups.Rules.ExecErrState", // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } // compare expected and actual and ignore the dynamic fields @@ -784,6 +799,7 @@ func TestAlertRulePostExport(t *testing.T) { require.Empty(t, diff) require.Equal(t, actual.Groups[0].Folder, "folder1") + require.Equal(t, actual.Groups[0].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value }) t.Run("should return 403 when no access to folder", func(t *testing.T) { @@ -971,7 +987,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) { }, }, NoDataState: apimodels.NoDataState(ngmodels.Alerting), - ExecErrState: apimodels.ExecutionErrorState(ngmodels.OkErrState), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK + ExecErrState: apimodels.ExecutionErrorState(ngmodels.AlertingErrState), }, }, }, diff --git a/pkg/tests/api/alerting/test-data/hysteresis_rule.json b/pkg/tests/api/alerting/test-data/hysteresis_rule.json index feff2d2d1be23..e296db87b8f5d 100644 --- a/pkg/tests/api/alerting/test-data/hysteresis_rule.json +++ b/pkg/tests/api/alerting/test-data/hysteresis_rule.json @@ -7,7 +7,7 @@ "title": "Hysteresis Test", "condition": "C", "no_data_state": "NoData", - "exec_err_state": "OK", + "exec_err_state": "Error", "data": [ { "refId": "A", diff --git a/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json b/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json index 0ff65c3c9cbd2..880e995d42c09 100644 --- a/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json +++ b/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json @@ -25,7 +25,7 @@ } ], "no_data_state": "NoData", - "exec_err_state": "OK", + "exec_err_state": "Alerting", "notification_settings": { "receiver": "rule-receiver", "group_by": [ diff --git a/pkg/tests/api/alerting/test-data/rulegroup-1-export.json b/pkg/tests/api/alerting/test-data/rulegroup-1-export.json index 088744d699cc4..c198ddc65e43e 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-1-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-1-export.json @@ -28,7 +28,7 @@ } ], "noDataState": "NoData", - "execErrState": "OK", + "execErrState": "Alerting", "for": "5m", "annotations": { "annotation": "test-annotation" @@ -59,7 +59,7 @@ } ], "noDataState": "NoData", - "execErrState": "OK", + "execErrState": "Alerting", "for": "5m", "annotations": { "annotation": "test-annotation" diff --git a/pkg/tests/api/alerting/test-data/rulegroup-1-get.json b/pkg/tests/api/alerting/test-data/rulegroup-1-get.json index 51ee23b86024b..7a4f3aa55830b 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-1-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-1-get.json @@ -40,7 +40,7 @@ "namespace_uid": "", "rule_group": "Group1", "no_data_state": "NoData", - "exec_err_state": "OK", + "exec_err_state": "Alerting", "is_paused": false } }, @@ -82,7 +82,7 @@ "namespace_uid": "", "rule_group": "Group1", "no_data_state": "NoData", - "exec_err_state": "OK", + "exec_err_state": "Alerting", "is_paused": false } } diff --git a/pkg/tests/api/alerting/test-data/rulegroup-1-post.json b/pkg/tests/api/alerting/test-data/rulegroup-1-post.json index e912122f593c2..269415be3ca76 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-1-post.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-1-post.json @@ -24,7 +24,7 @@ } ], "no_data_state": "NoData", - "exec_err_state": "OK" + "exec_err_state": "Alerting" } }, { @@ -49,7 +49,7 @@ } ], "no_data_state": "NoData", - "exec_err_state": "OK" + "exec_err_state": "Alerting" } } ] diff --git a/pkg/tests/api/alerting/test-data/rulegroup-2-export.json b/pkg/tests/api/alerting/test-data/rulegroup-2-export.json index 043b068aad05a..2c4ebcc2a1ba9 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-2-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-2-export.json @@ -28,7 +28,7 @@ } ], "noDataState": "NoData", - "execErrState": "OK", + "execErrState": "Error", "for": "5m", "annotations": { "annotation": "test-annotation" diff --git a/pkg/tests/api/alerting/test-data/rulegroup-2-get.json b/pkg/tests/api/alerting/test-data/rulegroup-2-get.json index 63a4ed9a334b4..8b2aa681fbb1f 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-2-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-2-get.json @@ -40,7 +40,7 @@ "namespace_uid": "", "rule_group": "Group2", "no_data_state": "NoData", - "exec_err_state": "OK", + "exec_err_state": "Error", "is_paused": false } } diff --git a/pkg/tests/api/alerting/test-data/rulegroup-2-post.json b/pkg/tests/api/alerting/test-data/rulegroup-2-post.json index ae3f97010eed6..9258c1fa1410e 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-2-post.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-2-post.json @@ -24,7 +24,7 @@ } ], "no_data_state": "NoData", - "exec_err_state": "OK" + "exec_err_state": "Error" } } ] diff --git a/pkg/tests/api/alerting/test-data/rulegroup-3-export.json b/pkg/tests/api/alerting/test-data/rulegroup-3-export.json index 0af1e0880d247..d9af83ac58d8d 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-3-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-3-export.json @@ -28,7 +28,7 @@ } ], "noDataState": "NoData", - "execErrState": "OK", + "execErrState": "Alerting", "for": "5m", "annotations": { "annotation": "test-annotation" @@ -59,7 +59,7 @@ } ], "noDataState": "NoData", - "execErrState": "OK", + "execErrState": "Alerting", "for": "5m", "annotations": { "annotation": "test-annotation" diff --git a/pkg/tests/api/alerting/test-data/rulegroup-3-get.json b/pkg/tests/api/alerting/test-data/rulegroup-3-get.json index fec0b9980d837..e80e5cc14e79e 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-3-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-3-get.json @@ -40,7 +40,7 @@ "namespace_uid": "", "rule_group": "Group3", "no_data_state": "NoData", - "exec_err_state": "OK", + "exec_err_state": "Alerting", "is_paused": false } }, @@ -82,7 +82,7 @@ "namespace_uid": "", "rule_group": "Group3", "no_data_state": "NoData", - "exec_err_state": "OK", + "exec_err_state": "Alerting", "is_paused": false } } diff --git a/pkg/tests/api/alerting/test-data/rulegroup-3-post.json b/pkg/tests/api/alerting/test-data/rulegroup-3-post.json index 7084dfc721066..d9dbda5809eeb 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-3-post.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-3-post.json @@ -24,7 +24,7 @@ } ], "no_data_state": "NoData", - "exec_err_state": "OK" + "exec_err_state": "Alerting" } }, { @@ -49,7 +49,7 @@ } ], "no_data_state": "NoData", - "exec_err_state": "OK" + "exec_err_state": "Alerting" } } ] From 8a68d15333cc7eaffc1b34e42558ab359619aff9 Mon Sep 17 00:00:00 2001 From: yasmin-tr Date: Tue, 17 Sep 2024 21:18:30 +0300 Subject: [PATCH 4/8] * enforce OK on update too --- pkg/services/ngalert/models/alert_rule.go | 4 ++++ pkg/services/ngalert/store/alert_rule.go | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index b3ea5d5b3348c..d6011f6de6086 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -495,6 +495,10 @@ func (alertRule *AlertRule) ValidateAlertRule(cfg setting.UnifiedAlertingSetting return fmt.Errorf("%w: cannot have Panel ID without a Dashboard UID", ErrAlertRuleFailedValidation) } + if _, err := ErrStateFromString(string(alertRule.ExecErrState)); err != nil { + return err + } + if _, err := NoDataStateFromString(string(alertRule.NoDataState)); err != nil { return err } diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index c36f136ae121d..53c0619692d54 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -203,7 +203,8 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, rules []ngmodels.UpdateR for _, r := range rules { var parentVersion int64 r.New.ID = r.Existing.ID - r.New.Version = r.Existing.Version // xorm will take care of increasing it (see https://xorm.io/docs/chapter-06/1.lock/) + r.New.Version = r.Existing.Version // xorm will take care of increasing it (see https://xorm.io/docs/chapter-06/1.lock/) + r.New.ExecErrState = ngmodels.OkErrState // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value if err := st.validateAlertRule(r.New); err != nil { return err } From 16506cd23cab0227cfe76d0b7e5159a9cb68cdb3 Mon Sep 17 00:00:00 2001 From: Yasmin T Date: Tue, 17 Sep 2024 21:27:12 +0300 Subject: [PATCH 5/8] revert unnecessary changes --- pkg/services/ngalert/store/alert_rule.go | 2 +- pkg/tests/api/alerting/api_ruler_test.go | 2 +- pkg/tests/api/alerting/test-data/hysteresis_rule.json | 2 +- .../alerting/test-data/rule-notification-settings-1-post.json | 2 +- pkg/tests/api/alerting/test-data/rulegroup-1-export.json | 2 +- pkg/tests/api/alerting/test-data/rulegroup-1-get.json | 2 +- pkg/tests/api/alerting/test-data/rulegroup-2-export.json | 2 +- pkg/tests/api/alerting/test-data/rulegroup-2-get.json | 2 +- pkg/tests/api/alerting/test-data/rulegroup-3-export.json | 2 +- pkg/tests/api/alerting/test-data/rulegroup-3-get.json | 2 +- pkg/tests/api/alerting/test-data/rulegroup-3-post.json | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 53c0619692d54..24a7cab874f16 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -203,7 +203,7 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, rules []ngmodels.UpdateR for _, r := range rules { var parentVersion int64 r.New.ID = r.Existing.ID - r.New.Version = r.Existing.Version // xorm will take care of increasing it (see https://xorm.io/docs/chapter-06/1.lock/) + r.New.Version = r.Existing.Version // xorm will take care of increasing it (see https://xorm.io/docs/chapter-06/1.lock/) r.New.ExecErrState = ngmodels.OkErrState // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value if err := st.validateAlertRule(r.New); err != nil { return err diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index a90e2f33d1597..b66d2bdd9f317 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -782,7 +782,7 @@ func TestAlertRulePostExport(t *testing.T) { pathsToIgnore := []string{ "Groups.Rules.UID", "Groups.Folder", - "Data.Model", // Model is not amended with default values + "Data.Model", // Model is not amended with default values "Groups.Rules.ExecErrState", // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value } diff --git a/pkg/tests/api/alerting/test-data/hysteresis_rule.json b/pkg/tests/api/alerting/test-data/hysteresis_rule.json index e296db87b8f5d..53a01c9ac5e9e 100644 --- a/pkg/tests/api/alerting/test-data/hysteresis_rule.json +++ b/pkg/tests/api/alerting/test-data/hysteresis_rule.json @@ -68,4 +68,4 @@ } } ] -} +} \ No newline at end of file diff --git a/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json b/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json index 880e995d42c09..0331fd94e27a9 100644 --- a/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json +++ b/pkg/tests/api/alerting/test-data/rule-notification-settings-1-post.json @@ -55,4 +55,4 @@ "name": "rule-time-interval", "time_intervals":[{"times":[{"start_time":"10:00","end_time":"12:00"}]}] } -} +} \ No newline at end of file diff --git a/pkg/tests/api/alerting/test-data/rulegroup-1-export.json b/pkg/tests/api/alerting/test-data/rulegroup-1-export.json index c198ddc65e43e..b031d95ea02e0 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-1-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-1-export.json @@ -72,4 +72,4 @@ ] } ] -} +} \ No newline at end of file diff --git a/pkg/tests/api/alerting/test-data/rulegroup-1-get.json b/pkg/tests/api/alerting/test-data/rulegroup-1-get.json index 7a4f3aa55830b..27474baf8e346 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-1-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-1-get.json @@ -87,4 +87,4 @@ } } ] -} +} \ No newline at end of file diff --git a/pkg/tests/api/alerting/test-data/rulegroup-2-export.json b/pkg/tests/api/alerting/test-data/rulegroup-2-export.json index 2c4ebcc2a1ba9..5f85d830260b2 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-2-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-2-export.json @@ -41,4 +41,4 @@ ] } ] -} +} \ No newline at end of file diff --git a/pkg/tests/api/alerting/test-data/rulegroup-2-get.json b/pkg/tests/api/alerting/test-data/rulegroup-2-get.json index 8b2aa681fbb1f..dc29a8b2bd23b 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-2-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-2-get.json @@ -45,4 +45,4 @@ } } ] -} +} \ No newline at end of file diff --git a/pkg/tests/api/alerting/test-data/rulegroup-3-export.json b/pkg/tests/api/alerting/test-data/rulegroup-3-export.json index d9af83ac58d8d..5a1ba9e0eac1a 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-3-export.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-3-export.json @@ -72,4 +72,4 @@ ] } ] -} +} \ No newline at end of file diff --git a/pkg/tests/api/alerting/test-data/rulegroup-3-get.json b/pkg/tests/api/alerting/test-data/rulegroup-3-get.json index e80e5cc14e79e..252dc99f13be1 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-3-get.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-3-get.json @@ -87,4 +87,4 @@ } } ] -} +} \ No newline at end of file diff --git a/pkg/tests/api/alerting/test-data/rulegroup-3-post.json b/pkg/tests/api/alerting/test-data/rulegroup-3-post.json index d9dbda5809eeb..2f528d19ad1cc 100644 --- a/pkg/tests/api/alerting/test-data/rulegroup-3-post.json +++ b/pkg/tests/api/alerting/test-data/rulegroup-3-post.json @@ -53,4 +53,4 @@ } } ] -} +} \ No newline at end of file From a7e2674c31f7b85c380a9848ae449c5bbda2d1de Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Thu, 15 Feb 2024 16:45:10 +0200 Subject: [PATCH 6/8] * fix test for evaluation fail to not send notification --- .../ngalert/schedule/schedule_unit_test.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pkg/services/ngalert/schedule/schedule_unit_test.go b/pkg/services/ngalert/schedule/schedule_unit_test.go index 0e400a51bee40..e9ab3951bda9a 100644 --- a/pkg/services/ngalert/schedule/schedule_unit_test.go +++ b/pkg/services/ngalert/schedule/schedule_unit_test.go @@ -752,13 +752,20 @@ func TestSchedule_ruleRoutine(t *testing.T) { require.NoError(t, err) }) - t.Run("it should send special alert DatasourceError", func(t *testing.T) { - sender.AssertNumberOfCalls(t, "Send", 1) - args, ok := sender.Calls()[0].Arguments[2].(definitions.PostableAlerts) - require.Truef(t, ok, fmt.Sprintf("expected argument of function was supposed to be 'definitions.PostableAlerts' but got %T", sender.Calls()[0].Arguments[2])) - assert.Len(t, args.PostableAlerts, 1) - assert.Equal(t, state.ErrorAlertName, args.PostableAlerts[0].Labels[prometheusModel.AlertNameLabel]) + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Do not send a notification on error or no data state + //t.Run("it should send special alert DatasourceError", func(t *testing.T) { + // sender.AssertNumberOfCalls(t, "Send", 1) + // args, ok := sender.Calls()[0].Arguments[2].(definitions.PostableAlerts) + // require.Truef(t, ok, fmt.Sprintf("expected argument of function was supposed to be 'definitions.PostableAlerts' but got %T", sender.Calls()[0].Arguments[2])) + // assert.Len(t, args.PostableAlerts, 1) + // assert.Equal(t, state.ErrorAlertName, args.PostableAlerts[0].Labels[prometheusModel.AlertNameLabel]) + //}) + t.Run("it should not send special alert DatasourceError", func(t *testing.T) { + sender.AssertNotCalled(t, "Send", mock.Anything, mock.Anything) + + require.NotEmpty(t, sch.stateManager.GetStatesForRuleUID(rule.OrgID, rule.UID)) }) + // LOGZ.IO GRAFANA CHANGE :: End }) t.Run("when there are alerts that should be firing", func(t *testing.T) { From 1977b914b1a8eb9fb1476e4aa8361b1ebb2c63ff Mon Sep 17 00:00:00 2001 From: yasmin-tr Date: Sun, 22 Sep 2024 15:55:35 +0300 Subject: [PATCH 7/8] * added change on provisioning request so response returns saved value * added tests to verify create+update response value is OK --- .../ngalert/provisioning/alert_rules.go | 2 ++ .../ngalert/provisioning/alert_rules_test.go | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/pkg/services/ngalert/provisioning/alert_rules.go b/pkg/services/ngalert/provisioning/alert_rules.go index bf7e93b3a1a62..d7e116ed974a2 100644 --- a/pkg/services/ngalert/provisioning/alert_rules.go +++ b/pkg/services/ngalert/provisioning/alert_rules.go @@ -157,6 +157,7 @@ func (service *AlertRuleService) CreateAlertRule(ctx context.Context, rule model } } } + rule.ExecErrState = models.OkErrState // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value err = service.xact.InTransaction(ctx, func(ctx context.Context) error { ids, err := service.ruleStore.InsertAlertRules(ctx, []models.AlertRule{ rule, @@ -411,6 +412,7 @@ func (service *AlertRuleService) UpdateAlertRule(ctx context.Context, rule model rule.Updated = time.Now() rule.ID = storedRule.ID rule.IntervalSeconds = storedRule.IntervalSeconds + rule.ExecErrState = models.OkErrState // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value err = rule.SetDashboardAndPanelFromAnnotations() if err != nil { return models.AlertRule{}, err diff --git a/pkg/services/ngalert/provisioning/alert_rules_test.go b/pkg/services/ngalert/provisioning/alert_rules_test.go index be924adef9199..df45be4c18113 100644 --- a/pkg/services/ngalert/provisioning/alert_rules_test.go +++ b/pkg/services/ngalert/provisioning/alert_rules_test.go @@ -68,6 +68,20 @@ func TestAlertRuleService(t *testing.T) { require.NoError(t, err) }) + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value + t.Run("update alert rule ExecErrState enforces OK state", func(t *testing.T) { + rule := dummyRule("test-update-errstate", orgID) + rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0) + require.NoError(t, err) + require.Equal(t, models.OkErrState, rule.ExecErrState) + + rule.ExecErrState = models.ErrorErrState + rule, err = ruleService.UpdateAlertRule(context.Background(), rule, models.ProvenanceNone) + require.NoError(t, err) + require.Equal(t, models.OkErrState, rule.ExecErrState) + }) + // LOGZ.IO GRAFANA CHANGE :: End + t.Run("group creation should propagate group title correctly", func(t *testing.T) { group := createDummyGroup("group-test-3", orgID) group.Rules[0].RuleGroup = "something different" @@ -556,6 +570,16 @@ func TestCreateAlertRule(t *testing.T) { require.NoError(t, err) }) }) + + // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value + t.Run("should create rule with OK state enforced", func(t *testing.T) { + rule := dummyRule("test-create-errstate", orgID) + rule.ExecErrState = models.AlertingErrState + rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0) + require.NoError(t, err) + require.Equal(t, models.OkErrState, rule.ExecErrState) + }) + // LOGZ.IO GRAFANA CHANGE :: End } func createAlertRuleService(t *testing.T) AlertRuleService { From b17243b7281488105f30976d968c62faabbdf1cf Mon Sep 17 00:00:00 2001 From: yasmin-tr Date: Sun, 22 Sep 2024 15:59:11 +0300 Subject: [PATCH 8/8] * test description --- pkg/services/ngalert/provisioning/alert_rules_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/services/ngalert/provisioning/alert_rules_test.go b/pkg/services/ngalert/provisioning/alert_rules_test.go index df45be4c18113..f68490ce7bf15 100644 --- a/pkg/services/ngalert/provisioning/alert_rules_test.go +++ b/pkg/services/ngalert/provisioning/alert_rules_test.go @@ -572,7 +572,7 @@ func TestCreateAlertRule(t *testing.T) { }) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value - t.Run("should create rule with OK state enforced", func(t *testing.T) { + t.Run("should create rule with ExecErrState OK state enforced", func(t *testing.T) { rule := dummyRule("test-create-errstate", orgID) rule.ExecErrState = models.AlertingErrState rule, err := ruleService.CreateAlertRule(context.Background(), rule, models.ProvenanceNone, 0)