From 1a46986600812d487f80a27dcfd0042969704bda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Fri, 12 Jun 2020 17:59:23 +0100 Subject: [PATCH] fix(backend): cleanup state checks --- internal/filters/autocomplete_test.go | 15 +++---- internal/filters/filter_silence_author.go | 4 +- internal/filters/filter_silence_id.go | 4 +- internal/filters/filter_silence_ticket.go | 4 +- internal/models/alert.go | 15 ------- internal/models/alert_test.go | 54 ----------------------- internal/models/alertgroup_test.go | 3 -- internal/models/api.go | 4 -- 8 files changed, 11 insertions(+), 92 deletions(-) delete mode 100644 internal/models/alert_test.go diff --git a/internal/filters/autocomplete_test.go b/internal/filters/autocomplete_test.go index 518bf8d93..42ffe87c0 100644 --- a/internal/filters/autocomplete_test.go +++ b/internal/filters/autocomplete_test.go @@ -1,7 +1,6 @@ package filters_test import ( - "encoding/json" "sort" "testing" @@ -52,8 +51,9 @@ var acTests = []acTest{ Alertmanager: []models.AlertmanagerInstance{ {Cluster: "cluster", Name: "am1"}, { - Cluster: "cluster", - Name: "am2", + Cluster: "cluster", + Name: "am2", + SilencedBy: []string{"1234567890"}, Silences: map[string]*models.Silence{ "1234567890": { ID: "1234567890", @@ -130,13 +130,8 @@ func TestBuildAutocomplete(t *testing.T) { sort.Strings(result) sort.Strings(acTest.Expected) - resultJSON, _ := json.Marshal(result) - expectedJSON, _ := json.Marshal(acTest.Expected) - - if string(resultJSON) != string(expectedJSON) { - if diff := cmp.Diff(expectedJSON, resultJSON); diff != "" { - t.Errorf("Wrong autocomplete data returned (-want +got):\n%s", diff) - } + if diff := cmp.Diff(acTest.Expected, result); diff != "" { + t.Errorf("Wrong autocomplete data returned (-want +got):\n%s", diff) } } } diff --git a/internal/filters/filter_silence_author.go b/internal/filters/filter_silence_author.go index 93ab5775e..edb7f1abd 100644 --- a/internal/filters/filter_silence_author.go +++ b/internal/filters/filter_silence_author.go @@ -57,8 +57,8 @@ func newSilenceAuthorFilter() FilterT { func silenceAuthorAutocomplete(name string, operators []string, alerts []models.Alert) []models.Autocomplete { tokens := map[string]models.Autocomplete{} for _, alert := range alerts { - if alert.IsSilenced() { - for _, silenceID := range alert.SilencedBy { + for _, am := range alert.Alertmanager { + for _, silenceID := range am.SilencedBy { for _, am := range alert.Alertmanager { silence, found := am.Silences[silenceID] if found { diff --git a/internal/filters/filter_silence_id.go b/internal/filters/filter_silence_id.go index e85f71f02..a0a93d2f6 100644 --- a/internal/filters/filter_silence_id.go +++ b/internal/filters/filter_silence_id.go @@ -51,8 +51,8 @@ func newsilenceIDFilter() FilterT { func silenceIDAutocomplete(name string, operators []string, alerts []models.Alert) []models.Autocomplete { tokens := map[string]models.Autocomplete{} for _, alert := range alerts { - if alert.IsSilenced() { - for _, silenceID := range alert.SilencedBy { + for _, am := range alert.Alertmanager { + for _, silenceID := range am.SilencedBy { for _, operator := range operators { token := fmt.Sprintf("%s%s%s", name, operator, silenceID) tokens[token] = makeAC(token, []string{ diff --git a/internal/filters/filter_silence_ticket.go b/internal/filters/filter_silence_ticket.go index dfd78590c..2f7911ca5 100644 --- a/internal/filters/filter_silence_ticket.go +++ b/internal/filters/filter_silence_ticket.go @@ -57,8 +57,8 @@ func newSilenceTicketFilter() FilterT { func silenceTicketIDAutocomplete(name string, operators []string, alerts []models.Alert) []models.Autocomplete { tokens := map[string]models.Autocomplete{} for _, alert := range alerts { - if alert.IsSilenced() { - for _, silenceID := range alert.SilencedBy { + for _, am := range alert.Alertmanager { + for _, silenceID := range am.SilencedBy { for _, am := range alert.Alertmanager { silence, found := am.Silences[silenceID] if found && silence.TicketID != "" { diff --git a/internal/models/alert.go b/internal/models/alert.go index 17410714a..194d7080c 100644 --- a/internal/models/alert.go +++ b/internal/models/alert.go @@ -66,18 +66,3 @@ func (a *Alert) LabelsFingerprint() string { func (a *Alert) ContentFingerprint() string { return a.contentFP } - -// IsSilenced will return true if alert should be considered silenced -func (a *Alert) IsSilenced() bool { - return (a.State == AlertStateSuppressed && len(a.SilencedBy) > 0) -} - -// IsInhibited will return true if alert should be considered silenced -func (a *Alert) IsInhibited() bool { - return (a.State == AlertStateSuppressed && len(a.InhibitedBy) > 0) -} - -// IsActive will return true if alert is not suppressed in any way -func (a *Alert) IsActive() bool { - return (a.State == AlertStateActive) -} diff --git a/internal/models/alert_test.go b/internal/models/alert_test.go deleted file mode 100644 index 110bde20b..000000000 --- a/internal/models/alert_test.go +++ /dev/null @@ -1,54 +0,0 @@ -package models_test - -import ( - "testing" - - "github.com/prymitive/karma/internal/models" -) - -type alertStateTest struct { - alert models.Alert - isSilenced bool - isInhibited bool - isActive bool -} - -var alertStateTests = []alertStateTest{ - { - alert: models.Alert{ - State: models.AlertStateActive, - }, - isActive: true, - }, - { - alert: models.Alert{ - State: models.AlertStateSuppressed, - InhibitedBy: []string{"1234"}, - }, - isInhibited: true, - }, - { - alert: models.Alert{ - State: models.AlertStateSuppressed, - SilencedBy: []string{"1234"}, - }, - isSilenced: true, - }, -} - -func TestAlertState(t *testing.T) { - for _, testCase := range alertStateTests { - if testCase.alert.IsActive() != testCase.isActive { - t.Errorf("alert.IsActive() returned %t while %t was expected for alert %v", - testCase.alert.IsActive(), testCase.isActive, testCase.alert) - } - if testCase.alert.IsInhibited() != testCase.isInhibited { - t.Errorf("alert.IsInhibited() returned %t while %t was expected for alert %v", - testCase.alert.IsInhibited(), testCase.isInhibited, testCase.alert) - } - if testCase.alert.IsSilenced() != testCase.isSilenced { - t.Errorf("alert.IsSilenced() returned %t while %t was expected for alert %v", - testCase.alert.IsSilenced(), testCase.isSilenced, testCase.alert) - } - } -} diff --git a/internal/models/alertgroup_test.go b/internal/models/alertgroup_test.go index 9922c1856..5dac84f37 100644 --- a/internal/models/alertgroup_test.go +++ b/internal/models/alertgroup_test.go @@ -111,7 +111,6 @@ var agFPTests = []agFPTest{ Alerts: models.AlertList{ models.Alert{ Labels: map[string]string{"foo1": "bar"}, - State: models.AlertStateActive, }, }, StateCount: map[string]int{"default": 0}, @@ -126,7 +125,6 @@ var agFPTests = []agFPTest{ Alerts: models.AlertList{ models.Alert{ Labels: map[string]string{"bar": "foo"}, - State: models.AlertStateActive, }, }, StateCount: map[string]int{"default": 0}, @@ -141,7 +139,6 @@ var agFPTests = []agFPTest{ Alerts: models.AlertList{ models.Alert{ Labels: map[string]string{"bar": "foo"}, - State: models.AlertStateActive, }, }, StateCount: map[string]int{"default": 0}, diff --git a/internal/models/api.go b/internal/models/api.go index 3ed26bfb6..bb0939e60 100644 --- a/internal/models/api.go +++ b/internal/models/api.go @@ -196,10 +196,6 @@ func (ag *APIAlertGroup) dedupSilences() { silencesByCluster := map[string]map[string]int{} for _, alert := range ag.Alerts { - if alert.State != AlertStateSuppressed { - // if we find any alert that's not silenced then we can break early - return - } // process each cluster only once, rather than each alertmanager instance clusters := []string{} for _, am := range alert.Alertmanager {