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] 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); },