IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Alerting: Distinguish conflict violation errors (#86634)

Browse Source 
* update generator to set ID = 0 and do not set 0 if unique is needed
* return proper message when the constraint violation
This commit is contained in:
Yuri Tseretyan
2024-04-22 12:28:46 -04:00
committed by GitHub
parent de92317fc7
commit 9735a8a080
4 changed files with 42 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/services/ngalert/models/alert_rule_test.go
View File

@@ -347,7 +347,7 @@ func TestPatchPartialAlertRule(t *testing.T) {
t.Run(testCase.name, func(t *testing.T) {
var existing *AlertRule
for {
existing = AlertRuleGen()()
existing = AlertRuleGen(WithUniqueID())()
cloned := *existing
// make sure the generated rule does not match the mutated one
testCase.mutator(&cloned)

4
pkg/services/ngalert/models/testing.go
View File

@@ -71,7 +71,7 @@ func AlertRuleGen(mutators ...AlertRuleMutator) func() *AlertRule {
}
rule := &AlertRule{
ID: rand.Int63n(1500),
ID: 0,
OrgID: rand.Int63n(1500) + 1, // Prevent OrgID=0 as this does not pass alert rule validation.
Title: "TEST-ALERT-" + util.GenerateShortUID(),
Condition: "A",
@@ -110,7 +110,7 @@ func WithUniqueID() AlertRuleMutator {
usedID := make(map[int64]struct{})
return func(rule *AlertRule) {
for {
id := rand.Int63n(1500)
id := rand.Int63n(1500) + 1
if _, ok := usedID[id]; !ok {
usedID[id] = struct{}{}
rule.ID = id

15
pkg/services/ngalert/store/alert_rule.go
View File

@@ -169,7 +169,7 @@ func (st DBstore) InsertAlertRules(ctx context.Context, rules []ngmodels.AlertRu
for i := range newRules {
if _, err := sess.Insert(&newRules[i]); err != nil {
if st.SQLStore.GetDialect().IsUniqueConstraintViolation(err) {
return ngmodels.ErrAlertRuleConflict(newRules[i], ngmodels.ErrAlertRuleUniqueConstraintViolation)
return ruleConstraintViolationToErr(newRules[i], err)
}
return fmt.Errorf("failed to create new rules: %w", err)
}
@@ -215,7 +215,7 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, rules []ngmodels.UpdateR
if updated, err := sess.ID(r.Existing.ID).AllCols().Update(r.New); err != nil || updated == 0 {
if err != nil {
if st.SQLStore.GetDialect().IsUniqueConstraintViolation(err) {
return ngmodels.ErrAlertRuleConflict(r.New, ngmodels.ErrAlertRuleUniqueConstraintViolation)
return ruleConstraintViolationToErr(r.New, err)
}
return fmt.Errorf("failed to update rule [%s] %s: %w", r.New.UID, r.New.Title, err)
}
@@ -758,3 +758,14 @@ func (st DBstore) RenameReceiverInNotificationSettings(ctx context.Context, orgI
}
return len(updates), st.UpdateAlertRules(ctx, updates)
}
func ruleConstraintViolationToErr(rule ngmodels.AlertRule, err error) error {
msg := err.Error()
if strings.Contains(msg, "UQE_alert_rule_org_id_namespace_uid_title") || strings.Contains(msg, "alert_rule.org_id, alert_rule.namespace_uid, alert_rule.title") {
return ngmodels.ErrAlertRuleConflict(rule, ngmodels.ErrAlertRuleUniqueConstraintViolation)
} else if strings.Contains(msg, "UQE_alert_rule_org_id_uid") || strings.Contains(msg, "alert_rule.org_id, alert_rule.uid") {
return ngmodels.ErrAlertRuleConflict(rule, errors.New("rule UID under the same organisation should be unique"))
} else {
return ngmodels.ErrAlertRuleConflict(rule, err)
}
}

35
pkg/services/ngalert/store/alert_rule_test.go
View File

@@ -603,6 +603,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
t.Skip("skipping integration test")
}
orgID := int64(1)
sqlStore := db.InitTestDB(t)
cfg := setting.NewCfg()
cfg.UnifiedAlerting.BaseInterval = 1 * time.Second
@@ -613,7 +614,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
Cfg: cfg.UnifiedAlerting,
}
rules := models.GenerateAlertRules(5, models.AlertRuleGen(models.WithOrgID(1), withIntervalMatching(store.Cfg.BaseInterval)))
rules := models.GenerateAlertRules(5, models.AlertRuleGen(models.WithOrgID(orgID), withIntervalMatching(store.Cfg.BaseInterval)))
deref := make([]models.AlertRule, 0, len(rules))
for _, rule := range rules {
deref = append(deref, *rule)
@@ -641,14 +642,30 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
require.Truef(t, found, "Rule with key %#v was not found in database", keyWithID)
}
_, err = store.InsertAlertRules(context.Background(), []models.AlertRule{deref[0]})
require.ErrorIs(t, err, models.ErrAlertRuleUniqueConstraintViolation)
require.NotEqual(t, deref[0].UID, "")
require.NotEqual(t, deref[0].Title, "")
require.NotEqual(t, deref[0].NamespaceUID, "")
require.ErrorContains(t, err, deref[0].UID)
require.ErrorContains(t, err, deref[0].Title)
require.ErrorContains(t, err, deref[0].NamespaceUID)
t.Run("fail to insert rules with same ID", func(t *testing.T) {
_, err = store.InsertAlertRules(context.Background(), []models.AlertRule{deref[0]})
require.ErrorIs(t, err, models.ErrAlertRuleConflictBase)
})
t.Run("fail insert rules with the same title in a folder", func(t *testing.T) {
cp := models.CopyRule(&deref[0])
cp.UID = cp.UID + "-new"
_, err = store.InsertAlertRules(context.Background(), []models.AlertRule{*cp})
require.ErrorIs(t, err, models.ErrAlertRuleConflictBase)
require.ErrorIs(t, err, models.ErrAlertRuleUniqueConstraintViolation)
require.NotEqual(t, deref[0].UID, "")
require.NotEqual(t, deref[0].Title, "")
require.NotEqual(t, deref[0].NamespaceUID, "")
require.ErrorContains(t, err, deref[0].UID)
require.ErrorContains(t, err, deref[0].Title)
require.ErrorContains(t, err, deref[0].NamespaceUID)
})
t.Run("should not let insert rules with the same UID", func(t *testing.T) {
cp := models.CopyRule(&deref[0])
cp.Title = "unique-test-title"
_, err = store.InsertAlertRules(context.Background(), []models.AlertRule{*cp})
require.ErrorIs(t, err, models.ErrAlertRuleConflictBase)
require.ErrorContains(t, err, "rule UID under the same organisation should be unique")
})
}
func TestIntegrationAlertRulesNotificationSettings(t *testing.T) {