From 0439009d4f79837147bf96a337099ff8a95f0615 Mon Sep 17 00:00:00 2001 From: Domas Date: Mon, 14 Jun 2021 15:37:28 +0300 Subject: [PATCH] Alerting: miscellaneous alertmanager config issues, tests (#35364) --- .../alerting/unified/AmRoutes.test.tsx | 211 +++++++++++++++--- .../features/alerting/unified/AmRoutes.tsx | 19 +- .../alerting/unified/api/alertmanager.ts | 3 +- .../components/amroutes/AmRootRoute.tsx | 2 +- .../components/amroutes/AmRootRouteForm.tsx | 56 +++-- .../components/amroutes/AmSpecificRouting.tsx | 10 +- .../alerting/unified/utils/amroutes.ts | 5 +- .../features/alerting/unified/utils/redux.ts | 2 + 8 files changed, 236 insertions(+), 72 deletions(-) diff --git a/public/app/features/alerting/unified/AmRoutes.test.tsx b/public/app/features/alerting/unified/AmRoutes.test.tsx index c33309295ee..0adc7b2da03 100644 --- a/public/app/features/alerting/unified/AmRoutes.test.tsx +++ b/public/app/features/alerting/unified/AmRoutes.test.tsx @@ -3,15 +3,16 @@ import { locationService, setDataSourceSrv } from '@grafana/runtime'; import { render, waitFor } from '@testing-library/react'; import { Provider } from 'react-redux'; import { Router } from 'react-router-dom'; -import { Route } from 'app/plugins/datasource/alertmanager/types'; +import { AlertManagerCortexConfig, Route } from 'app/plugins/datasource/alertmanager/types'; import { configureStore } from 'app/store/configureStore'; import { typeAsJestMock } from 'test/helpers/typeAsJestMock'; -import { byTestId } from 'testing-library-selector'; +import { byRole, byTestId, byText } from 'testing-library-selector'; import AmRoutes from './AmRoutes'; -import { fetchAlertManagerConfig } from './api/alertmanager'; +import { fetchAlertManagerConfig, updateAlertManagerConfig } from './api/alertmanager'; import { mockDataSource, MockDataSourceSrv } from './mocks'; import { getAllDataSources } from './utils/config'; -import { DataSourceType } from './utils/datasource'; +import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; +import userEvent from '@testing-library/user-event'; Object.defineProperty(window, 'matchMedia', { writable: true, @@ -35,6 +36,7 @@ const mocks = { api: { fetchAlertManagerConfig: typeAsJestMock(fetchAlertManagerConfig), + updateAlertManagerConfig: typeAsJestMock(updateAlertManagerConfig), }, }; @@ -62,6 +64,18 @@ const ui = { rootGroupBy: byTestId('am-routes-root-group-by'), rootTimings: byTestId('am-routes-root-timings'), row: byTestId('am-routes-row'), + + rootRouteContainer: byTestId('am-root-route-container'), + + editButton: byRole('button', { name: 'Edit' }), + saveButton: byRole('button', { name: 'Save' }), + + receiverSelect: byTestId('am-receiver-select'), + groupSelect: byTestId('am-group-select'), + + groupWaitContainer: byTestId('am-group-wait'), + groupIntervalContainer: byTestId('am-group-interval'), + groupRepeatContainer: byTestId('am-repeat-interval'), }; describe('AmRoutes', () => { @@ -130,31 +144,8 @@ describe('AmRoutes', () => { routes: subroutes, }; - beforeAll(() => { - mocks.getAllDataSourcesMock.mockReturnValue(Object.values(dataSources)); - - mocks.api.fetchAlertManagerConfig.mockImplementation(() => - Promise.resolve({ - alertmanager_config: { - route: rootRoute, - receivers: [ - { - name: 'default-receiver', - }, - { - name: 'a-receiver', - }, - { - name: 'another-receiver', - }, - ], - }, - template_files: {}, - }) - ); - }); - beforeEach(() => { + mocks.getAllDataSourcesMock.mockReturnValue(Object.values(dataSources)); setDataSourceSrv(new MockDataSourceSrv(dataSources)); }); @@ -165,6 +156,24 @@ describe('AmRoutes', () => { }); it('loads and shows routes', async () => { + mocks.api.fetchAlertManagerConfig.mockResolvedValue({ + alertmanager_config: { + route: rootRoute, + receivers: [ + { + name: 'default-receiver', + }, + { + name: 'a-receiver', + }, + { + name: 'another-receiver', + }, + ], + }, + template_files: {}, + }); + await renderAmRoutes(); await waitFor(() => expect(mocks.api.fetchAlertManagerConfig).toHaveBeenCalledTimes(1)); @@ -197,4 +206,150 @@ describe('AmRoutes', () => { } }); }); + + it('can edit root route if one is already defined', async () => { + const defaultConfig: AlertManagerCortexConfig = { + alertmanager_config: { + receivers: [{ name: 'default' }, { name: 'critical' }], + route: { + receiver: 'default', + group_by: ['alertname'], + }, + templates: [], + }, + template_files: {}, + }; + const currentConfig = { current: defaultConfig }; + mocks.api.updateAlertManagerConfig.mockImplementation((amSourceName, newConfig) => { + currentConfig.current = newConfig; + return Promise.resolve(); + }); + + mocks.api.fetchAlertManagerConfig.mockImplementation(() => { + return Promise.resolve(currentConfig.current); + }); + + await renderAmRoutes(); + expect(await ui.rootReceiver.find()).toHaveTextContent('default'); + expect(ui.rootGroupBy.get()).toHaveTextContent('alertname'); + + // open root route for editing + const rootRouteContainer = await ui.rootRouteContainer.find(); + userEvent.click(ui.editButton.get(rootRouteContainer)); + + // configure receiver & group by + const receiverSelect = await ui.receiverSelect.find(); + clickSelectOption(receiverSelect, 'critical'); + + const groupSelect = ui.groupSelect.get(); + await userEvent.type(byRole('textbox').get(groupSelect), 'namespace{enter}'); + + // configure timing intervals + userEvent.click(byText('Timing options').get(rootRouteContainer)); + + await updateTiming(ui.groupWaitContainer.get(), '1', 'Minutes'); + await updateTiming(ui.groupIntervalContainer.get(), '4', 'Minutes'); + await updateTiming(ui.groupRepeatContainer.get(), '5', 'Hours'); + + //save + userEvent.click(ui.saveButton.get(rootRouteContainer)); + + // wait for it to go out of edit mode + await waitFor(() => expect(ui.editButton.query(rootRouteContainer)).not.toBeInTheDocument()); + + // check that appropriate api calls were made + expect(mocks.api.fetchAlertManagerConfig).toHaveBeenCalledTimes(3); + expect(mocks.api.updateAlertManagerConfig).toHaveBeenCalledTimes(1); + expect(mocks.api.updateAlertManagerConfig).toHaveBeenCalledWith(GRAFANA_RULES_SOURCE_NAME, { + alertmanager_config: { + receivers: [{ name: 'default' }, { name: 'critical' }], + route: { + continue: false, + group_by: ['alertname', 'namespace'], + receiver: 'critical', + routes: [], + group_interval: '4m', + group_wait: '1m', + repeat_interval: '5h', + }, + templates: [], + }, + template_files: {}, + }); + + // check that new config values are rendered + await waitFor(() => expect(ui.rootReceiver.query()).toHaveTextContent('critical')); + expect(ui.rootGroupBy.get()).toHaveTextContent('alertname, namespace'); + }); + + it('can edit root route if one is not defined yet', async () => { + mocks.api.fetchAlertManagerConfig.mockResolvedValue({ + alertmanager_config: { + receivers: [{ name: 'default' }], + }, + template_files: {}, + }); + + await renderAmRoutes(); + + // open root route for editing + const rootRouteContainer = await ui.rootRouteContainer.find(); + userEvent.click(ui.editButton.get(rootRouteContainer)); + + // configure receiver & group by + const receiverSelect = await ui.receiverSelect.find(); + clickSelectOption(receiverSelect, 'default'); + + const groupSelect = ui.groupSelect.get(); + await userEvent.type(byRole('textbox').get(groupSelect), 'severity{enter}'); + await userEvent.type(byRole('textbox').get(groupSelect), 'namespace{enter}'); + //save + userEvent.click(ui.saveButton.get(rootRouteContainer)); + + // wait for it to go out of edit mode + await waitFor(() => expect(ui.editButton.query(rootRouteContainer)).not.toBeInTheDocument()); + + // check that appropriate api calls were made + expect(mocks.api.fetchAlertManagerConfig).toHaveBeenCalledTimes(3); + expect(mocks.api.updateAlertManagerConfig).toHaveBeenCalledTimes(1); + expect(mocks.api.updateAlertManagerConfig).toHaveBeenCalledWith(GRAFANA_RULES_SOURCE_NAME, { + alertmanager_config: { + receivers: [{ name: 'default' }], + route: { + continue: false, + group_by: ['severity', 'namespace'], + receiver: 'default', + routes: [], + }, + }, + template_files: {}, + }); + }); + + it('Show error message if loading Alermanager config fails', async () => { + mocks.api.fetchAlertManagerConfig.mockRejectedValue({ + status: 500, + data: { + message: "Alertmanager has exploded. it's gone. Forget about it.", + }, + }); + await renderAmRoutes(); + await waitFor(() => expect(mocks.api.fetchAlertManagerConfig).toHaveBeenCalledTimes(1)); + expect(await byText("Alertmanager has exploded. it's gone. Forget about it.").find()).toBeInTheDocument(); + expect(ui.rootReceiver.query()).not.toBeInTheDocument(); + expect(ui.editButton.query()).not.toBeInTheDocument(); + }); }); + +const clickSelectOption = async (selectElement: HTMLElement, optionText: string): Promise => { + userEvent.click(byRole('textbox').get(selectElement)); + userEvent.click(byText(optionText).get(selectElement)); +}; + +const updateTiming = async (selectElement: HTMLElement, value: string, timeUnit: string): Promise => { + const inputs = byRole('textbox').queryAll(selectElement); + expect(inputs).toHaveLength(2); + await userEvent.type(inputs[0], value); + userEvent.click(inputs[1]); + userEvent.click(byText(timeUnit).get(selectElement)); +}; diff --git a/public/app/features/alerting/unified/AmRoutes.tsx b/public/app/features/alerting/unified/AmRoutes.tsx index d48d39d411b..47a33c4c91f 100644 --- a/public/app/features/alerting/unified/AmRoutes.tsx +++ b/public/app/features/alerting/unified/AmRoutes.tsx @@ -39,7 +39,7 @@ const AmRoutes: FC = () => { (alertManagerSourceName && amConfigs[alertManagerSourceName]) || initialAsyncRequestState; const config = result?.alertmanager_config; - const [routes, id2ExistingRoute] = useMemo(() => amRouteToFormAmRoute(config?.route), [config?.route]); + const [rootRoute, id2ExistingRoute] = useMemo(() => amRouteToFormAmRoute(config?.route), [config?.route]); const receivers = stringsToSelectableValues( (config?.receivers ?? []).map((receiver: Receiver) => receiver.name) @@ -54,14 +54,10 @@ const AmRoutes: FC = () => { }; useCleanup((state) => state.unifiedAlerting.saveAMConfig); - const { loading: saving, error: savingError, dispatched: savingDispatched } = useUnifiedAlertingSelector( - (state) => state.saveAMConfig - ); - const handleSave = (data: Partial) => { const newData = formAmRouteToAmRoute( { - ...routes, + ...rootRoute, ...data, }, id2ExistingRoute @@ -83,16 +79,11 @@ const AmRoutes: FC = () => { oldConfig: result, alertManagerSourceName: alertManagerSourceName!, successMessage: 'Saved', + refetch: true, }) ); }; - useEffect(() => { - if (savingDispatched && !saving && !savingError) { - fetchConfig(); - } - }, [fetchConfig, savingDispatched, saving, savingError]); - if (!alertManagerSourceName) { return ; } @@ -115,14 +106,14 @@ const AmRoutes: FC = () => { onEnterEditMode={enterRootRouteEditMode} onExitEditMode={exitRootRouteEditMode} receivers={receivers} - routes={routes} + routes={rootRoute} />
)} diff --git a/public/app/features/alerting/unified/api/alertmanager.ts b/public/app/features/alerting/unified/api/alertmanager.ts index 7baadcf0089..90ccaf53ae0 100644 --- a/public/app/features/alerting/unified/api/alertmanager.ts +++ b/public/app/features/alerting/unified/api/alertmanager.ts @@ -28,8 +28,7 @@ export async function fetchAlertManagerConfig(alertManagerSourceName: string): P // if no config has been uploaded to grafana, it returns error instead of latest config if ( alertManagerSourceName === GRAFANA_RULES_SOURCE_NAME && - (e.data?.message?.includes('failed to get latest configuration') || - e.data?.message?.includes('could not find an Alertmanager configuration')) + e.data?.message?.includes('could not find an Alertmanager configuration') ) { return { template_files: {}, diff --git a/public/app/features/alerting/unified/components/amroutes/AmRootRoute.tsx b/public/app/features/alerting/unified/components/amroutes/AmRootRoute.tsx index b282696f435..3cf7f681fe5 100644 --- a/public/app/features/alerting/unified/components/amroutes/AmRootRoute.tsx +++ b/public/app/features/alerting/unified/components/amroutes/AmRootRoute.tsx @@ -28,7 +28,7 @@ export const AmRootRoute: FC = ({ const styles = useStyles2(getStyles); return ( -
+
Root policy - default for all alerts diff --git a/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx b/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx index 123609841ac..0cb73e02eef 100644 --- a/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx +++ b/public/app/features/alerting/unified/components/amroutes/AmRootRouteForm.tsx @@ -36,27 +36,34 @@ export const AmRootRouteForm: FC = ({
{({ control, errors, setValue }) => ( <> - -
- ( - onChange(mapSelectValueToString(value))} + options={receivers} + /> + )} + control={control} + name="receiver" + rules={{ required: { value: true, message: 'Required.' } }} + /> + or + + Create a contact point + +
+ - + {/* @ts-ignore-check: react-hook-form made me do this */} ( @@ -86,9 +93,10 @@ export const AmRootRouteForm: FC = ({ > <>
@@ -119,9 +127,10 @@ export const AmRootRouteForm: FC = ({ <>
@@ -152,9 +161,10 @@ export const AmRootRouteForm: FC = ({ <>
diff --git a/public/app/features/alerting/unified/components/amroutes/AmSpecificRouting.tsx b/public/app/features/alerting/unified/components/amroutes/AmSpecificRouting.tsx index e6749277012..895f7ab0af1 100644 --- a/public/app/features/alerting/unified/components/amroutes/AmSpecificRouting.tsx +++ b/public/app/features/alerting/unified/components/amroutes/AmSpecificRouting.tsx @@ -3,7 +3,7 @@ import { css } from '@emotion/css'; import { GrafanaTheme2 } from '@grafana/data'; import { Button, useStyles2 } from '@grafana/ui'; import { AmRouteReceiver, FormAmRoute } from '../../types/amroutes'; -import { emptyRoute } from '../../utils/amroutes'; +import { emptyArrayFieldMatcher, emptyRoute } from '../../utils/amroutes'; import { EmptyArea } from '../EmptyArea'; import { AmRoutesTable } from './AmRoutesTable'; @@ -22,7 +22,13 @@ export const AmSpecificRouting: FC = ({ onChange, onRoot const addNewRoute = () => { setIsAddMode(true); - setActualRoutes((actualRoutes) => [...actualRoutes, emptyRoute]); + setActualRoutes((actualRoutes) => [ + ...actualRoutes, + { + ...emptyRoute, + matchers: [emptyArrayFieldMatcher], + }, + ]); }; return ( diff --git a/public/app/features/alerting/unified/utils/amroutes.ts b/public/app/features/alerting/unified/utils/amroutes.ts index f1add63b7bc..fad30fb73be 100644 --- a/public/app/features/alerting/unified/utils/amroutes.ts +++ b/public/app/features/alerting/unified/utils/amroutes.ts @@ -4,6 +4,7 @@ import { Matcher, Route } from 'app/plugins/datasource/alertmanager/types'; import { FormAmRoute } from '../types/amroutes'; import { parseInterval, timeOptions } from './time'; import { parseMatcher, stringifyMatcher } from './alertmanager'; +import { isUndefined, omitBy } from 'lodash'; const defaultValueAndType: [string, string] = ['', timeOptions[0].value]; @@ -51,8 +52,8 @@ export const emptyArrayFieldMatcher: Matcher = { export const emptyRoute: FormAmRoute = { id: '', - matchers: [emptyArrayFieldMatcher], groupBy: [], + matchers: [], routes: [], continue: false, receiver: '', @@ -133,7 +134,7 @@ export const formAmRouteToAmRoute = (formAmRoute: FormAmRoute, id2ExistingRoute: amRoute.receiver = formAmRoute.receiver; } - return amRoute; + return omitBy(amRoute, isUndefined); }; export const stringToSelectableValue = (str: string): SelectableValue => ({ diff --git a/public/app/features/alerting/unified/utils/redux.ts b/public/app/features/alerting/unified/utils/redux.ts index 7560b343a17..067f6ffa420 100644 --- a/public/app/features/alerting/unified/utils/redux.ts +++ b/public/app/features/alerting/unified/utils/redux.ts @@ -143,6 +143,8 @@ function messageFromError(e: Error | FetchError | SerializedError): string { .map((d) => d?.message) .filter((m) => !!m) .join(' '); + } else if (e.statusText) { + return e.statusText; } } return (e as Error)?.message || String(e);