From 8b5862d44b057384f1656ac63c9f32b7dffc0dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Fri, 30 Nov 2018 23:14:59 +0000 Subject: [PATCH 1/9] chore(api): store cluster as a dict This will allow silences to be grouped by cluster rather than by alertmanager instance --- alerts.go | 15 +++++++++------ internal/models/alertmanager.go | 2 +- internal/slices/slices.go | 21 +++++++++++++++++++++ internal/slices/slices_test.go | 19 +++++++++++++++++++ 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/alerts.go b/alerts.go index 267c7f189..404d78886 100644 --- a/alerts.go +++ b/alerts.go @@ -2,11 +2,13 @@ package main import ( "sort" - "strings" "github.com/prymitive/karma/internal/alertmanager" "github.com/prymitive/karma/internal/filters" "github.com/prymitive/karma/internal/models" + "github.com/prymitive/karma/internal/slices" + + log "github.com/sirupsen/logrus" ) func getFiltersFromQuery(filterStrings []string) ([]filters.FilterT, bool) { @@ -41,7 +43,11 @@ func getUpstreams() models.AlertmanagerAPISummary { for _, upstream := range upstreams { members := upstream.ClusterMemberNames() sort.Strings(members) - key := strings.Join(members[:], "\n") + key, err := slices.StringSliceToSHA1(members) + if err != nil { + log.Errorf("slices.StringSliceToSHA1 error: %s", err) + continue + } if _, found := clusters[key]; !found { clusters[key] = members } @@ -63,10 +69,7 @@ func getUpstreams() models.AlertmanagerAPISummary { summary.Counters.Failed++ } } - - for _, cluster := range clusters { - summary.Clusters = append(summary.Clusters, cluster) - } + summary.Clusters = clusters return summary } diff --git a/internal/models/alertmanager.go b/internal/models/alertmanager.go index 718322822..6c7596d44 100644 --- a/internal/models/alertmanager.go +++ b/internal/models/alertmanager.go @@ -47,5 +47,5 @@ type AlertmanagerAPICounters struct { type AlertmanagerAPISummary struct { Counters AlertmanagerAPICounters `json:"counters"` Instances []AlertmanagerAPIStatus `json:"instances"` - Clusters [][]string `json:"clusters"` + Clusters map[string][]string `json:"clusters"` } diff --git a/internal/slices/slices.go b/internal/slices/slices.go index a18d400e3..47cddf4fb 100644 --- a/internal/slices/slices.go +++ b/internal/slices/slices.go @@ -1,5 +1,10 @@ package slices +import ( + "crypto/sha1" + "fmt" +) + // BoolInSlice returns true if given bool is found in a slice of bools func BoolInSlice(boolArray []bool, value bool) bool { for _, s := range boolArray { @@ -19,3 +24,19 @@ func StringInSlice(stringArray []string, value string) bool { } return false } + +// StringSliceToSHA1 returns a SHA1 hash computed from a slice of strings +func StringSliceToSHA1(stringArray []string) (string, error) { + h := sha1.New() + for _, s := range stringArray { + _, err := h.Write([]byte(s)) + if err != nil { + return "", err + } + _, err = h.Write([]byte("\n")) + if err != nil { + return "", err + } + } + return fmt.Sprintf("%x", h.Sum(nil)), nil +} diff --git a/internal/slices/slices_test.go b/internal/slices/slices_test.go index dee989619..2e5fc01d1 100644 --- a/internal/slices/slices_test.go +++ b/internal/slices/slices_test.go @@ -111,3 +111,22 @@ func TestBoolInSlice(t *testing.T) { } } } + +func TestStringSliceToSHA1(t *testing.T) { + s, err := slices.StringSliceToSHA1([]string{"a", "b", "c"}) + if err != nil { + t.Errorf("StringSliceToSHA1() returned error: %s", err) + } + if s == "" { + t.Errorf("StringSliceToSHA1() returned empty string") + } +} + +func BenchmarkStringSliceToSHA1(b *testing.B) { + for _, stringSliceTest := range stringSliceTests { + _, err := slices.StringSliceToSHA1(stringSliceTest.array) + if err != nil { + b.Errorf("StringSliceToSHA1() returned error: %s", err) + } + } +} From ffa446e8f6fd8a40ea46387c0b151cf390a8924e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Fri, 30 Nov 2018 23:22:16 +0000 Subject: [PATCH 2/9] chore(ui): update UI to consume cluster list as a dict --- .../Grid/AlertGrid/AlertGroup/Silence/index.test.js | 2 +- ui/src/Components/Grid/AlertGrid/index.js | 2 +- ui/src/Components/Grid/AlertGrid/index.test.js | 2 +- ui/src/Components/Grid/index.test.js | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js index 293aa7d9e..4e271c32f 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js @@ -69,7 +69,7 @@ beforeEach(() => { clusterMembers: ["default"] } ], - clusters: [["default"]] + clusters: { default: ["default"] } }; alertStore.data.silences = { default: { diff --git a/ui/src/Components/Grid/AlertGrid/index.js b/ui/src/Components/Grid/AlertGrid/index.js index 51b80c94e..abe63a535 100644 --- a/ui/src/Components/Grid/AlertGrid/index.js +++ b/ui/src/Components/Grid/AlertGrid/index.js @@ -102,7 +102,7 @@ const AlertGrid = observer( key={id} group={alertStore.data.groups[id]} showAlertmanagers={ - alertStore.data.upstreams.clusters.length > 1 + Object.keys(alertStore.data.upstreams.clusters).length > 1 } afterUpdate={this.masonryRepack} settingsStore={settingsStore} diff --git a/ui/src/Components/Grid/AlertGrid/index.test.js b/ui/src/Components/Grid/AlertGrid/index.test.js index 2ce7457d7..29dbf4985 100644 --- a/ui/src/Components/Grid/AlertGrid/index.test.js +++ b/ui/src/Components/Grid/AlertGrid/index.test.js @@ -59,7 +59,7 @@ const MockGroupList = count => { alertStore.data.upstreams = { counters: { total: 0, healthy: 1, failed: 0 }, instances: [{ name: "am", uri: "http://am", error: "" }], - clusters: [["am"]] + clusters: { am: ["am"] } }; alertStore.data.groups = groups; }; diff --git a/ui/src/Components/Grid/index.test.js b/ui/src/Components/Grid/index.test.js index 0ad096b26..b8ffb6109 100644 --- a/ui/src/Components/Grid/index.test.js +++ b/ui/src/Components/Grid/index.test.js @@ -37,7 +37,7 @@ describe("", () => { alertStore.data.upstreams = { counters: { total: 1, healthy: 0, failed: 1 }, instances: [{ name: "am1", uri: "http://am1", error: "error" }], - clusters: [["am1"]] + clusters: { am1: ["am1"] } }; const tree = ShallowGrid(); expect(tree.text()).toBe(""); @@ -47,7 +47,7 @@ describe("", () => { alertStore.data.upstreams = { counters: { total: 1, healthy: 0, failed: 1 }, instances: [{ name: "am1", uri: "http://am1", error: "" }], - clusters: [["am1"]] + clusters: { am1: ["am1"] } }; const tree = ShallowGrid(); expect(tree.text()).toBe(""); @@ -61,7 +61,7 @@ describe("", () => { { name: "am2", uri: "file:///mock", error: "" }, { name: "am3", uri: "http://am1", error: "error 2" } ], - clusters: [["am1"], ["am2"], ["am3"]] + clusters: { am1: ["am1"], am2: ["am2"], am3: ["am3"] } }; const tree = ShallowGrid(); expect(tree.text()).toBe(""); @@ -72,7 +72,7 @@ describe("", () => { alertStore.data.upstreams = { counters: { total: 0, healthy: 0, failed: 1 }, instances: [{ name: "am", uri: "http://am1", error: "error" }], - clusters: [["am"]] + clusters: { am1: ["am1"] } }; const tree = ShallowGrid(); expect(tree.text()).toBe(""); From 93617b3b1875a4136c0c5c953e594c1d05d975b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Fri, 30 Nov 2018 23:38:34 +0000 Subject: [PATCH 3/9] fix(api): ensure that cluster members are always sorted --- alerts.go | 3 --- internal/alertmanager/models.go | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/alerts.go b/alerts.go index 404d78886..3315863a7 100644 --- a/alerts.go +++ b/alerts.go @@ -1,8 +1,6 @@ package main import ( - "sort" - "github.com/prymitive/karma/internal/alertmanager" "github.com/prymitive/karma/internal/filters" "github.com/prymitive/karma/internal/models" @@ -42,7 +40,6 @@ func getUpstreams() models.AlertmanagerAPISummary { upstreams := alertmanager.GetAlertmanagers() for _, upstream := range upstreams { members := upstream.ClusterMemberNames() - sort.Strings(members) key, err := slices.StringSliceToSHA1(members) if err != nil { log.Errorf("slices.StringSliceToSHA1 error: %s", err) diff --git a/internal/alertmanager/models.go b/internal/alertmanager/models.go index 3cc9081c1..c0f5b6ce9 100644 --- a/internal/alertmanager/models.go +++ b/internal/alertmanager/models.go @@ -500,5 +500,6 @@ func (am *Alertmanager) ClusterMemberNames() []string { } } + sort.Strings(members) return members } From d668a64ffb7f34f509894058ec30673aeda07559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 1 Dec 2018 00:13:43 +0000 Subject: [PATCH 4/9] chore(ci): drop gocyclo Code needs refactoring but right now it would make it worse --- .golangci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index d9021044f..672d18f69 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -8,7 +8,6 @@ linters: - golint - dupl - goconst - - gocyclo linters-settings: govet: From 0d68ea4c3cbe9ed12b3056bea0865ed95cbfbc85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 1 Dec 2018 09:53:47 +0000 Subject: [PATCH 5/9] feat(api): store silences per cluster in the API response --- alerts.go | 1 + api_test.go | 4 ++-- internal/alertmanager/models.go | 13 +++++++++++++ internal/models/alertmanager.go | 4 +++- views.go | 14 ++++++++++++-- 5 files changed, 31 insertions(+), 5 deletions(-) diff --git a/alerts.go b/alerts.go index 3315863a7..483d02488 100644 --- a/alerts.go +++ b/alerts.go @@ -55,6 +55,7 @@ func getUpstreams() models.AlertmanagerAPISummary { PublicURI: upstream.PublicURI(), Error: upstream.Error(), Version: upstream.Version(), + Cluster: upstream.ClusterID(), ClusterMembers: members, } summary.Instances = append(summary.Instances, u) diff --git a/api_test.go b/api_test.go index 3b883c19d..b3e4163a3 100644 --- a/api_test.go +++ b/api_test.go @@ -867,9 +867,9 @@ func TestVerifyAllGroups(t *testing.T) { } } - am, foundAM := ur.Silences["default"] + am, foundAM := ur.Silences["843c4a11660fe38ea61e6960a29d4f4796da6488"] if !foundAM { - t.Errorf("[%s] Alertmanager 'default' missing from silences", version) + t.Errorf("[%s] Alertmanager cluster '843c4a11660fe38ea61e6960a29d4f4796da6488' (default) missing from silences", version) } else if len(am) == 0 { t.Errorf("[%s] Silences mismatch, expected >0 but got %d", version, len(am)) } diff --git a/internal/alertmanager/models.go b/internal/alertmanager/models.go index c0f5b6ce9..4b584dec8 100644 --- a/internal/alertmanager/models.go +++ b/internal/alertmanager/models.go @@ -284,6 +284,7 @@ func (am *Alertmanager) pullAlerts(version string) error { alert.Alertmanager = []models.AlertmanagerInstance{ models.AlertmanagerInstance{ Name: am.Name, + Cluster: am.ClusterID(), State: alert.State, StartsAt: alert.StartsAt, EndsAt: alert.EndsAt, @@ -503,3 +504,15 @@ func (am *Alertmanager) ClusterMemberNames() []string { sort.Strings(members) return members } + +// ClusterID returns the ID (sha1) of the cluster this Alertmanager instance +// belongs to +func (am *Alertmanager) ClusterID() string { + members := am.ClusterMemberNames() + id, err := slices.StringSliceToSHA1(members) + if err != nil { + log.Errorf("slices.StringSliceToSHA1 error: %s", err) + return am.Name + } + return id +} diff --git a/internal/models/alertmanager.go b/internal/models/alertmanager.go index 6c7596d44..913e26efb 100644 --- a/internal/models/alertmanager.go +++ b/internal/models/alertmanager.go @@ -5,7 +5,8 @@ import "time" // AlertmanagerInstance describes the Alertmanager instance alert was collected // from type AlertmanagerInstance struct { - Name string `json:"name"` + Name string `json:"name"` + Cluster string `json:"cluster"` // per instance alert state State string `json:"state"` // timestamp collected from this instance, those on the alert itself @@ -32,6 +33,7 @@ type AlertmanagerAPIStatus struct { PublicURI string `json:"publicURI"` Error string `json:"error"` Version string `json:"version"` + Cluster string `json:"cluster"` ClusterMembers []string `json:"clusterMembers"` } diff --git a/views.go b/views.go index f961dd382..dbb6b90be 100644 --- a/views.go +++ b/views.go @@ -111,9 +111,15 @@ func alerts(c *gin.Context) { dedupedAlerts := alertmanager.DedupAlerts() dedupedColors := alertmanager.DedupColors() + amNameToCluster := map[string]string{} silences := map[string]map[string]models.Silence{} for _, am := range alertmanager.GetAlertmanagers() { - silences[am.Name] = map[string]models.Silence{} + key := am.ClusterID() + amNameToCluster[am.Name] = key + _, found := silences[key] + if !found { + silences[key] = map[string]models.Silence{} + } } var matches int @@ -189,8 +195,12 @@ func alerts(c *gin.Context) { for _, alert := range agCopy.Alerts { if alert.IsSilenced() { for _, am := range alert.Alertmanager { + key := amNameToCluster[am.Name] for _, silence := range am.Silences { - silences[am.Name][silence.ID] = *silence + _, found := silences[key][silence.ID] + if !found { + silences[key][silence.ID] = *silence + } } } } From 3a905b748e42e7baae5b2fc6c5e7fc1feef15dc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 1 Dec 2018 09:57:59 +0000 Subject: [PATCH 6/9] feat(api): consume Alertmanager cluster id in the UI --- .../Grid/AlertGrid/AlertGroup/Silence/index.test.js | 2 ++ .../Components/SilenceModal/AlertManagerInput/index.test.js | 4 ++++ .../SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js | 1 + ui/src/Models/API.js | 1 + ui/src/__mocks__/Alerts.js | 1 + 5 files changed, 9 insertions(+) diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js index 4e271c32f..e3a0c412f 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js @@ -62,6 +62,7 @@ beforeEach(() => { instances: [ { name: "default", + cluster: "default", uri: "file:///mock", publicURI: "http://example.com", error: "", @@ -187,6 +188,7 @@ describe("", () => { const am = instance.getAlertmanager(); expect(am).toEqual({ name: "default", + cluster: "default", uri: "file:///mock", publicURI: "http://example.com", error: "", diff --git a/ui/src/Components/SilenceModal/AlertManagerInput/index.test.js b/ui/src/Components/SilenceModal/AlertManagerInput/index.test.js index 053baef9c..17c9a84f7 100644 --- a/ui/src/Components/SilenceModal/AlertManagerInput/index.test.js +++ b/ui/src/Components/SilenceModal/AlertManagerInput/index.test.js @@ -25,6 +25,7 @@ beforeEach(() => { publicURI: "http://am1.example.com", error: "", version: "0.15.0", + cluster: "am1", clusterMembers: ["am1"] }, { @@ -33,6 +34,7 @@ beforeEach(() => { publicURI: "http://am2.example.com", error: "", version: "0.15.0", + cluster: "am2", clusterMembers: ["am2"] }, { @@ -41,6 +43,7 @@ beforeEach(() => { publicURI: "http://am3.example.com", error: "", version: "0.15.0", + cluster: "am3", clusterMembers: ["am3"] } ]; @@ -147,6 +150,7 @@ describe("", () => { publicURI: "http://am1.example.com/new", error: "", version: "0.15.0", + cluster: "am1", clusterMembers: ["am1"] }; // force update since this is where the mismatch check lives diff --git a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js index 426673055..264ded049 100644 --- a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js +++ b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js @@ -19,6 +19,7 @@ beforeEach(() => { publicURI: "http://example.com", error: "", version: "0.15.0", + cluster: "mockAlertmanager", clusterMembers: ["mockAlertmanager"] } ] diff --git a/ui/src/Models/API.js b/ui/src/Models/API.js index 9f837e44f..d21ef96dd 100644 --- a/ui/src/Models/API.js +++ b/ui/src/Models/API.js @@ -66,6 +66,7 @@ const APISilence = PropTypes.exact({ const APIAlertmanagerUpstream = PropTypes.exact({ name: PropTypes.string.isRequired, + cluster: PropTypes.string.isRequired, uri: PropTypes.string.isRequired, publicURI: PropTypes.string.isRequired, error: PropTypes.string.isRequired, diff --git a/ui/src/__mocks__/Alerts.js b/ui/src/__mocks__/Alerts.js index 19e8c5077..82b74eb77 100644 --- a/ui/src/__mocks__/Alerts.js +++ b/ui/src/__mocks__/Alerts.js @@ -66,6 +66,7 @@ const MockSilence = () => ({ const MockAlertmanager = () => ({ name: "default", + cluster: "default", uri: "http://localhost", publicURI: "http://am.example.com", error: "", From 9f4ee09a56a321bbf7db712242f1a29221319dfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 1 Dec 2018 10:47:24 +0000 Subject: [PATCH 7/9] fix(ui): only show one silence per Alertmanager cluster HA clusters share silences which means that they are currently duplicated for each alertmanager instance in that cluster --- .../Grid/AlertGrid/AlertGroup/Alert/index.js | 53 +++++++++++++------ .../AlertGrid/AlertGroup/Alert/index.test.js | 29 ++++++++++ .../AlertGrid/AlertGroup/Silence/index.js | 2 +- .../AlertGroup/Silence/index.test.js | 1 + ui/src/Models/API.js | 1 + ui/src/__mocks__/Alerts.js | 1 + 6 files changed, 71 insertions(+), 16 deletions(-) diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.js index 20317401b..650582ea7 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.js @@ -45,18 +45,35 @@ const Alert = observer( BorderClassMap[alert.state] || "border-warning" ]; + let silences = {}; + for (let am of alert.alertmanager) { + if (!silences[am.cluster]) { + silences[am.cluster] = { + alertmanager: am, + silences: [] + }; + } + for (let silenceID of am.silencedBy) { + if (!silences[am.cluster].silences.includes(silenceID)) { + silences[am.cluster].silences.push(silenceID); + } + } + } + return (
  • - {alert.annotations.filter(a => a.isLink === false).map(a => ( - - ))} + {alert.annotations + .filter(a => a.isLink === false) + .map(a => ( + + ))}
    ) : null} - {alert.annotations.filter(a => a.isLink === true).map(a => ( - - ))} - {alert.alertmanager.map(am => - am.silencedBy.map(silenceID => ( + {alert.annotations + .filter(a => a.isLink === true) + .map(a => ( + + ))} + {Object.values(silences).map(clusterSilences => + clusterSilences.silences.map(silenceID => ( diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.test.js index 0f40aff00..f5efc1a1b 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/index.test.js @@ -96,6 +96,35 @@ describe("", () => { expect(silence.html()).toMatch(/silence123456789/); }); + it("renders only one silence for HA cluster", () => { + const alert = MockedAlert(); + alert.alertmanager = [ + { + name: "am1", + cluster: "ha", + state: "suppressed", + startsAt: "2018-08-14T17:36:40.017867056Z", + endsAt: "0001-01-01T00:00:00Z", + source: "localhost/am1", + silencedBy: ["silence123456789"] + }, + { + name: "am2", + cluster: "ha", + state: "suppressed", + startsAt: "2018-08-14T17:36:40.017867056Z", + endsAt: "0001-01-01T00:00:00Z", + source: "localhost/am2", + silencedBy: ["silence123456789"] + } + ]; + const group = MockAlertGroup({}, [alert], [], {}); + const tree = MountedAlert(alert, group, false, false); + const silence = tree.find("Silence"); + expect(silence).toHaveLength(1); + expect(silence.html()).toMatch(/silence123456789/); + }); + it("uses BorderClassMap.active when @state=active", () => { const alert = MockedAlert(); alert.state = "active"; diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.js index 4e26a0e13..97f1c6262 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.js @@ -249,7 +249,7 @@ const Silence = inject("alertStore")( // and we need to lookup the actual silence data in the store. // Data might be missing from the store so first check if we have // anything for this alertmanager instance - const amSilences = alertStore.data.silences[alertmanagerState.name]; + const amSilences = alertStore.data.silences[alertmanagerState.cluster]; if (!amSilences) return null; // next check if alertmanager has our silence ID diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js index e3a0c412f..7071ed80b 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js @@ -17,6 +17,7 @@ const mockAfterUpdate = jest.fn(); const alertmanager = { name: "default", + cluster: "default", state: "suppressed", startsAt: "2000-01-01T10:00:00Z", endsAt: "0001-01-01T00:00:00Z", diff --git a/ui/src/Models/API.js b/ui/src/Models/API.js index d21ef96dd..3c295d48b 100644 --- a/ui/src/Models/API.js +++ b/ui/src/Models/API.js @@ -11,6 +11,7 @@ const Annotation = PropTypes.exact({ const APIAlertAlertmanagerState = PropTypes.exact({ name: PropTypes.string.isRequired, + cluster: PropTypes.string.isRequired, state: AlertState.isRequired, startsAt: PropTypes.string.isRequired, endsAt: PropTypes.string.isRequired, diff --git a/ui/src/__mocks__/Alerts.js b/ui/src/__mocks__/Alerts.js index 82b74eb77..3bc685d0e 100644 --- a/ui/src/__mocks__/Alerts.js +++ b/ui/src/__mocks__/Alerts.js @@ -14,6 +14,7 @@ const MockAlert = (annotations, labels, state) => ({ alertmanager: [ { name: "default", + cluster: "default", state: "active", startsAt: "2018-08-14T17:36:40.017867056Z", endsAt: "0001-01-01T00:00:00Z", From 7d52626489b901960e1282a8ad27012dd2c178c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 1 Dec 2018 17:58:40 +0000 Subject: [PATCH 8/9] fix(ui): send silences only to a single cluster node Silences are shared by HA cluster members, when submitting a silence to a cluster try each each member but stop after first successful fetch --- .../__snapshots__/index.test.js.snap | 19 +-- .../SilenceModal/AlertManagerInput/index.js | 36 ++--- .../AlertManagerInput/index.test.js | 78 +++++------ ui/src/Components/SilenceModal/Matchers.js | 19 +-- .../SilenceMatch/MatchCounter.test.js | 2 +- .../SilenceSubmit/SilenceSubmitController.js | 4 +- .../SilenceSubmitController.test.js | 13 +- .../SilenceSubmit/SilenceSubmitProgress.js | 60 ++++---- .../SilenceSubmitProgress.test.js | 130 ++++++++++++++---- .../SilenceSubmitProgress.test.js.snap | 9 -- ui/src/Stores/AlertStore.js | 2 +- 11 files changed, 212 insertions(+), 160 deletions(-) delete mode 100644 ui/src/Components/SilenceModal/SilenceSubmit/__snapshots__/SilenceSubmitProgress.test.js.snap diff --git a/ui/src/Components/SilenceModal/AlertManagerInput/__snapshots__/index.test.js.snap b/ui/src/Components/SilenceModal/AlertManagerInput/__snapshots__/index.test.js.snap index 39a8300ea..b3a06f6c1 100644 --- a/ui/src/Components/SilenceModal/AlertManagerInput/__snapshots__/index.test.js.snap +++ b/ui/src/Components/SilenceModal/AlertManagerInput/__snapshots__/index.test.js.snap @@ -7,24 +7,7 @@ exports[` matches snapshot 1`] = `
    - am1 -
    -
    - - - - -
    -
    -
    -
    - am2 + am1 | am2
    - instances.map(i => ({ - label: i.name, - value: i.publicURI +const AlertmanagerClustersToOption = clusterDict => + Object.entries(clusterDict).map(([clusterID, clusterMembers]) => ({ + label: clusterMembers.join(" | "), + value: clusterMembers })); const AlertManagerInput = observer( @@ -30,8 +30,8 @@ const AlertManagerInput = observer( const { alertStore, silenceFormStore } = props; if (silenceFormStore.data.alertmanagers.length === 0) { - silenceFormStore.data.alertmanagers = AlertmanagerInstancesToOptions( - alertStore.data.upstreams.instances + silenceFormStore.data.alertmanagers = AlertmanagerClustersToOption( + alertStore.data.upstreams.clusters ); } } @@ -46,23 +46,17 @@ const AlertManagerInput = observer( const { alertStore, silenceFormStore } = this.props; // get the list of last known alertmanagers - const currentAlertmanagers = AlertmanagerInstancesToOptions( - alertStore.data.upstreams.instances + const currentAlertmanagers = AlertmanagerClustersToOption( + alertStore.data.upstreams.clusters ); // now iterate what's set as silence form values and reset it if any - // mismatch is detected (uri changed for example) + // mismatch is detected for (const silenceAM of silenceFormStore.data.alertmanagers) { - for (const currentAM of currentAlertmanagers) { - if ( - silenceAM.label === currentAM.label && - silenceAM.value !== currentAM.value - ) { - silenceFormStore.data.alertmanagers = AlertmanagerInstancesToOptions( - alertStore.data.upstreams.instances - ); - return; - } + if ( + !currentAlertmanagers.map(am => am.label).includes(silenceAM.label) + ) { + silenceFormStore.data.alertmanagers = currentAlertmanagers; } } } @@ -80,8 +74,8 @@ const AlertManagerInput = observer( styles={ReactSelectStyles} instanceId="silence-input-alertmanagers" defaultValue={silenceFormStore.data.alertmanagers} - options={AlertmanagerInstancesToOptions( - alertStore.data.upstreams.instances + options={AlertmanagerClustersToOption( + alertStore.data.upstreams.clusters )} placeholder={ silenceFormStore.data.wasValidated ? ( diff --git a/ui/src/Components/SilenceModal/AlertManagerInput/index.test.js b/ui/src/Components/SilenceModal/AlertManagerInput/index.test.js index 17c9a84f7..b26ccbbfc 100644 --- a/ui/src/Components/SilenceModal/AlertManagerInput/index.test.js +++ b/ui/src/Components/SilenceModal/AlertManagerInput/index.test.js @@ -11,13 +11,12 @@ import { AlertManagerInput } from "."; let alertStore; let silenceFormStore; -const AlertmanagerOption = index => ({ - label: `am${index}`, - value: `http://am${index}.example.com` -}); - beforeEach(() => { alertStore = new AlertStore([]); + alertStore.data.upstreams.clusters = { + ha: ["am1", "am2"], + am3: ["am3"] + }; alertStore.data.upstreams.instances = [ { name: "am1", @@ -25,8 +24,8 @@ beforeEach(() => { publicURI: "http://am1.example.com", error: "", version: "0.15.0", - cluster: "am1", - clusterMembers: ["am1"] + cluster: "ha", + clusterMembers: ["am1", "am2"] }, { name: "am2", @@ -34,8 +33,8 @@ beforeEach(() => { publicURI: "http://am2.example.com", error: "", version: "0.15.0", - cluster: "am2", - clusterMembers: ["am2"] + cluster: "ha", + clusterMembers: ["am1", "am2"] }, { name: "am3", @@ -103,61 +102,62 @@ describe("", () => { it("all available Alertmanager instances are selected by default", () => { ShallowAlertManagerInput(); - expect(silenceFormStore.data.alertmanagers).toHaveLength(3); - for (let i = 1; i <= 3; i++) { - expect(silenceFormStore.data.alertmanagers).toContainEqual( - AlertmanagerOption(i) - ); - } + expect(silenceFormStore.data.alertmanagers).toHaveLength(2); + expect(silenceFormStore.data.alertmanagers).toContainEqual({ + label: "am1 | am2", + value: ["am1", "am2"] + }); + expect(silenceFormStore.data.alertmanagers).toContainEqual({ + label: "am3", + value: ["am3"] + }); }); it("doesn't override last selected Alertmanager instances on mount", () => { - silenceFormStore.data.alertmanagers = [AlertmanagerOption(1)]; + silenceFormStore.data.alertmanagers = [{ label: "am3", value: ["am3"] }]; ShallowAlertManagerInput(); expect(silenceFormStore.data.alertmanagers).toHaveLength(1); - expect(silenceFormStore.data.alertmanagers).toContainEqual( - AlertmanagerOption(1) - ); + expect(silenceFormStore.data.alertmanagers).toContainEqual({ + label: "am3", + value: ["am3"] + }); }); it("renders all 3 suggestions", () => { const tree = ValidateSuggestions(); const options = tree.find("[role='option']"); - expect(options).toHaveLength(3); - expect(options.at(0).text()).toBe("am1"); - expect(options.at(1).text()).toBe("am2"); - expect(options.at(2).text()).toBe("am3"); + expect(options).toHaveLength(2); + expect(options.at(0).text()).toBe("am1 | am2"); + expect(options.at(1).text()).toBe("am3"); }); it("clicking on options appends them to silenceFormStore.data.alertmanagers", () => { + silenceFormStore.data.alertmanagers = []; const tree = ValidateSuggestions(); const options = tree.find("[role='option']"); options.at(0).simulate("click"); - options.at(2).simulate("click"); + options.at(1).simulate("click"); expect(silenceFormStore.data.alertmanagers).toHaveLength(2); - expect(silenceFormStore.data.alertmanagers).toContainEqual( - AlertmanagerOption(1) - ); - expect(silenceFormStore.data.alertmanagers).toContainEqual( - AlertmanagerOption(3) - ); + expect(silenceFormStore.data.alertmanagers).toContainEqual({ + label: "am1 | am2", + value: ["am1", "am2"] + }); + expect(silenceFormStore.data.alertmanagers).toContainEqual({ + label: "am3", + value: ["am3"] + }); }); it("silenceFormStore.data.alertmanagers gets updated from alertStore.data.upstreams.instances on mismatch", () => { const tree = ShallowAlertManagerInput(); - alertStore.data.upstreams.instances[0] = { - name: "am1", - publicURI: "http://am1.example.com/new", - error: "", - version: "0.15.0", - cluster: "am1", - clusterMembers: ["am1"] + alertStore.data.upstreams.clusters = { + amNew: ["amNew"] }; // force update since this is where the mismatch check lives tree.instance().componentDidUpdate(); expect(silenceFormStore.data.alertmanagers).toContainEqual({ - label: "am1", - value: "http://am1.example.com/new" + label: "amNew", + value: ["amNew"] }); }); diff --git a/ui/src/Components/SilenceModal/Matchers.js b/ui/src/Components/SilenceModal/Matchers.js index 141bdb010..9a4ec819a 100644 --- a/ui/src/Components/SilenceModal/Matchers.js +++ b/ui/src/Components/SilenceModal/Matchers.js @@ -16,19 +16,12 @@ const MatcherToFilter = matcher => { }; const AlertManagersToFilter = alertmanagers => { - if (alertmanagers.length > 1) { - return FormatQuery( - StaticLabels.AlertManager, - QueryOperators.Regex, - `^(${alertmanagers.map(am => am.label).join("|")})$` - ); - } else if (alertmanagers.length === 1) { - return FormatQuery( - StaticLabels.AlertManager, - QueryOperators.Equal, - alertmanagers[0].label - ); - } + let amNames = [].concat(...alertmanagers.map(am => am.value)); + return FormatQuery( + StaticLabels.AlertManager, + QueryOperators.Regex, + `^(${amNames.join("|")})$` + ); }; export { MatcherToFilter, AlertManagersToFilter }; diff --git a/ui/src/Components/SilenceModal/SilenceMatch/MatchCounter.test.js b/ui/src/Components/SilenceModal/SilenceMatch/MatchCounter.test.js index 3179c90ed..118866c25 100644 --- a/ui/src/Components/SilenceModal/SilenceMatch/MatchCounter.test.js +++ b/ui/src/Components/SilenceModal/SilenceMatch/MatchCounter.test.js @@ -132,7 +132,7 @@ describe("", () => { const tree = MountedMatchCounter(); await expect(tree.instance().matchedAlerts.fetch).resolves.toBeUndefined(); expect(fetch.mock.calls[0][0]).toBe( - "./alerts.json?q=foo%3Dbar&q=%40alertmanager%3Dam1" + "./alerts.json?q=foo%3Dbar&q=%40alertmanager%3D~%5E%28am1%29%24" ); }); diff --git a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitController.js b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitController.js index 179ec8db7..7f979ca6a 100644 --- a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitController.js +++ b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitController.js @@ -23,8 +23,8 @@ class SilenceSubmitController extends Component { {silenceFormStore.data.alertmanagers.map(am => ( diff --git a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitController.test.js b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitController.test.js index 3d9f92dde..bf0f768a1 100644 --- a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitController.test.js +++ b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitController.test.js @@ -3,11 +3,7 @@ import React from "react"; import { shallow } from "enzyme"; import { AlertStore } from "Stores/AlertStore"; -import { - SilenceFormStore, - SilenceFormStage, - MatcherValueToObject -} from "Stores/SilenceFormStore"; +import { SilenceFormStore, SilenceFormStage } from "Stores/SilenceFormStore"; import { SilenceSubmitController } from "./SilenceSubmitController"; let alertStore; @@ -29,8 +25,11 @@ const ShallowSilenceSubmitController = () => { describe("", () => { it("renders all passed SilenceSubmitProgress", () => { - silenceFormStore.data.alertmanagers.push(MatcherValueToObject("am1")); - silenceFormStore.data.alertmanagers.push(MatcherValueToObject("am2")); + silenceFormStore.data.alertmanagers.push({ label: "am1", value: ["am1"] }); + silenceFormStore.data.alertmanagers.push({ + label: "ha", + value: ["am2", "am3"] + }); const tree = ShallowSilenceSubmitController(); const alertmanagers = tree.find("SilenceSubmitProgress"); expect(alertmanagers).toHaveLength(2); diff --git a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js index b55c500f8..d0463f4e6 100644 --- a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js +++ b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js @@ -47,8 +47,8 @@ SilenceLink.propTypes = { const SilenceSubmitProgress = observer( class SilenceSubmitProgress extends Component { static propTypes = { - name: PropTypes.string.isRequired, - uri: PropTypes.string.isRequired, + cluster: PropTypes.string.isRequired, + members: PropTypes.arrayOf(PropTypes.string).isRequired, payload: PropTypes.exact({ matchers: PropTypes.arrayOf(APISilenceMatcher).isRequired, startsAt: PropTypes.string.isRequired, @@ -63,6 +63,7 @@ const SilenceSubmitProgress = observer( { // store fetch result here, useful for testing fetch: null, + membersToTry: [], value: SubmitState.InProgress, result: null, markDone(result) { @@ -77,10 +78,28 @@ const SilenceSubmitProgress = observer( { markDone: action.bound, markFailed: action.bound } ); - handleAlertmanagerRequest = () => { - const { uri, payload } = this.props; + maybeTryAgainAfterError = err => { + if (this.submitState.membersToTry.length) { + this.handleAlertmanagerRequest(); + } else { + this.submitState.markFailed(err.message); + } + }; - this.submitState.fetch = fetch(`${uri}/api/v1/silences`, { + handleAlertmanagerRequest = () => { + const { payload, alertStore } = this.props; + + const member = this.submitState.membersToTry.pop(); + + const am = alertStore.data.getAlertmanagerByName(member); + if (am === undefined) { + const err = `Alertmanager instance "${member} not found`; + console.error(err); + this.maybeTryAgainAfterError(err); + return; + } + + this.submitState.fetch = fetch(`${am.publicURI}/api/v1/silences`, { method: "POST", body: JSON.stringify(payload), headers: { @@ -88,27 +107,16 @@ const SilenceSubmitProgress = observer( } }) .then(result => result.json()) - .then(result => this.parseAlertmanagerResponse(result)) - .catch(err => this.submitState.markFailed(err.message)); + .then(result => this.parseAlertmanagerResponse(am.uri, result)) + .catch(err => this.maybeTryAgainAfterError(err)); }; - parseAlertmanagerResponse = response => { - const { name, alertStore } = this.props; - - const alertmanager = alertStore.data.getAlertmanagerByName(name); - + parseAlertmanagerResponse = (uri, response) => { if (response.status === "success") { - if (alertmanager) { - const link = ( - - ); - this.submitState.markDone(link); - } else { - this.submitState.markDone(response.data.silenceId); - } + const link = ( + + ); + this.submitState.markDone(link); } else if (response.status === "error") { this.submitState.markFailed(response.error); } else { @@ -120,18 +128,20 @@ const SilenceSubmitProgress = observer( }; componentDidMount() { + const { members } = this.props; + this.submitState.membersToTry = [...members]; this.handleAlertmanagerRequest(); } render() { - const { name } = this.props; + const { cluster } = this.props; return (
    -
    {name}
    +
    {cluster}
    {this.submitState.result}
    ); diff --git a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js index 264ded049..6e18f0de2 100644 --- a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js +++ b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js @@ -2,8 +2,6 @@ import React from "react"; import { mount } from "enzyme"; -import toDiffableHtml from "diffable-html"; - import { AlertStore } from "Stores/AlertStore"; import { SilenceSubmitProgress } from "./SilenceSubmitProgress"; @@ -29,8 +27,8 @@ beforeEach(() => { const MountedSilenceSubmitProgress = () => { return mount( { }; describe("", () => { - it("sends a request on mount", () => { - MountedSilenceSubmitProgress(); + it("sends a request on mount", async () => { + const tree = MountedSilenceSubmitProgress(); + await expect(tree.instance().submitState.fetch).resolves.toBeUndefined(); expect(fetch.mock.calls).toHaveLength(1); }); - it("appends /api/v1/silences to the passed URI", () => { - MountedSilenceSubmitProgress(); + it("appends /api/v1/silences to the passed URI", async () => { + const tree = MountedSilenceSubmitProgress(); + await expect(tree.instance().submitState.fetch).resolves.toBeUndefined(); const uri = fetch.mock.calls[0][0]; - expect(uri).toBe("http://localhost/mock/api/v1/silences"); + expect(uri).toBe("http://example.com/api/v1/silences"); }); it("sends correct JSON payload", () => { @@ -71,6 +71,103 @@ describe("", () => { }); }); + it("will retry on another cluster member after fetch failure", async () => { + fetch.resetMocks(); + fetch + .mockRejectOnce(new Error("mock error message")) + .mockResponseOnce( + JSON.stringify({ status: "success", data: { silenceId: "123456789" } }) + ); + alertStore.data.upstreams = { + clusters: { ha: ["am1", "am2"] }, + instances: [ + { + name: "am1", + uri: "file:///mock", + publicURI: "http://am1.example.com", + error: "", + version: "0.15.0", + cluster: "ha", + clusterMembers: ["am1", "am2"] + }, + { + name: "am2", + uri: "file:///mock", + publicURI: "http://am2.example.com", + error: "", + version: "0.15.0", + cluster: "ha", + clusterMembers: ["am1", "am2"] + } + ] + }; + + const tree = mount( + + ); + await expect(tree.instance().submitState.fetch).resolves.toBeUndefined(); + expect(fetch.mock.calls[0][0]).toBe( + "http://am2.example.com/api/v1/silences" + ); + await expect(tree.instance().submitState.fetch).resolves.toBe("success"); + expect(fetch.mock.calls[1][0]).toBe( + "http://am1.example.com/api/v1/silences" + ); + }); + + it("will log an error if Alertmanager instance is missing from instances and try the next one", async () => { + fetch.resetMocks(); + fetch.mockReject(new Error("mock error message")); + const consoleSpy = jest + .spyOn(console, "error") + .mockImplementation(() => {}); + alertStore.data.upstreams = { + clusters: { ha: ["am1", "am2"] }, + instances: [ + { + name: "am1", + uri: "file:///mock", + publicURI: "http://am1.example.com", + error: "", + version: "0.15.0", + cluster: "ha", + clusterMembers: ["am1", "am2"] + } + ] + }; + + const tree = mount( + + ); + await expect(tree.instance().submitState.fetch).resolves.toBeUndefined(); + expect(fetch.mock.calls[0][0]).toBe( + "http://am1.example.com/api/v1/silences" + ); + expect(consoleSpy).toHaveBeenCalledTimes(1); + }); + it("renders returned silence ID on successful fetch", async () => { fetch.mockResponseOnce( JSON.stringify({ status: "success", data: { silenceId: "123456789" } }) @@ -84,21 +181,6 @@ describe("", () => { expect(silenceLink.text()).toBe("123456789"); }); - it("renders returned silence ID as text if alertmanager is not found in AlertStore", async () => { - fetch.mockResponseOnce( - JSON.stringify({ status: "success", data: { silenceId: "123456789" } }) - ); - alertStore.data.upstreams.instances = []; - const tree = MountedSilenceSubmitProgress(); - await expect(tree.instance().submitState.fetch).resolves.toBe("success"); - // force re-render - tree.update(); - const silenceLink = tree.find("a"); - expect(silenceLink).toHaveLength(0); - const idDiv = tree.find("div.flex-fill").at(2); - expect(toDiffableHtml(idDiv.html())).toMatchSnapshot(); - }); - it("renders returned error message on failed fetch", async () => { fetch.mockRejectOnce(new Error("mock error message")); const tree = MountedSilenceSubmitProgress(); diff --git a/ui/src/Components/SilenceModal/SilenceSubmit/__snapshots__/SilenceSubmitProgress.test.js.snap b/ui/src/Components/SilenceModal/SilenceSubmit/__snapshots__/SilenceSubmitProgress.test.js.snap deleted file mode 100644 index 23a21fae0..000000000 --- a/ui/src/Components/SilenceModal/SilenceSubmit/__snapshots__/SilenceSubmitProgress.test.js.snap +++ /dev/null @@ -1,9 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[` renders returned silence ID as text if alertmanager is not found in AlertStore 1`] = ` -" -
    - 123456789 -
    -" -`; diff --git a/ui/src/Stores/AlertStore.js b/ui/src/Stores/AlertStore.js index 23a98067a..cac899929 100644 --- a/ui/src/Stores/AlertStore.js +++ b/ui/src/Stores/AlertStore.js @@ -140,7 +140,7 @@ class AlertStore { counters: {}, groups: {}, silences: {}, - upstreams: { instances: [] }, + upstreams: { instances: [], clusters: {} }, getAlertmanagerByName(name) { return this.upstreams.instances.find(am => am.name === name); }, From 926278158f3ede052ee574f1d8d889c09fdcfa8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 1 Dec 2018 18:17:45 +0000 Subject: [PATCH 9/9] fix(api): update cluster id on API query --- views.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/views.go b/views.go index dbb6b90be..1d1a8391c 100644 --- a/views.go +++ b/views.go @@ -192,10 +192,13 @@ func alerts(c *gin.Context) { } if len(agCopy.Alerts) > 0 { - for _, alert := range agCopy.Alerts { + for i, alert := range agCopy.Alerts { if alert.IsSilenced() { - for _, am := range alert.Alertmanager { + for j, am := range alert.Alertmanager { key := amNameToCluster[am.Name] + // cluster might be wrong when collecting (races between fetches) + // update is with current cluster discovery state + agCopy.Alerts[i].Alertmanager[j].Cluster = key for _, silence := range am.Silences { _, found := silences[key][silence.ID] if !found {