Alerting: Fix inconsistent AM raw config when applied via sync vs API #81655
Conversation
JacobsonMT
added
type/bug
area/alerting
JacobsonMT
requested review from
a team,
rwwiv,
yuri-tceretian and
grobinson-grafana
and removed request for
a team
So this failed until a configuration was applied via the API? |
Yes, and then quickly started failing again once the periodic sync happened |
More so out of curiosity, how would I replicate that on main branch? |
After save: After sync: |
AM config applied via API would use the PostableUserConfig as the AM raw config and also the hash used to decide when the AM config has changed. However, when applied via the periodic sync the PostableApiAlertingConfig would be used instead. This leads to two issues: - Inconsistent hash comparisons when modifying the AM causing redundant applies. - GetStatus assumed the raw config was PostableUserConfig causing the endpoint to return correctly after a new config is applied via API and then nothing once the periodic sync runs. Note: Technically, the upstream GrafanaAlertamanger GetStatus shouldn't be returning PostableUserConfig or PostableApiAlertingConfig, but instead GettableStatus. However, this issue required changes elsewhere and is out of scope.
JacobsonMT
force-pushed
the
jacobsonmt/fix-inconsistent-AM-raw-config
branch
from
e9e05ab
to
251c0e2
Compare
…#81655) AM config applied via API would use the PostableUserConfig as the AM raw config and also the hash used to decide when the AM config has changed. However, when applied via the periodic sync the PostableApiAlertingConfig would be used instead. This leads to two issues: - Inconsistent hash comparisons when modifying the AM causing redundant applies. - GetStatus assumed the raw config was PostableUserConfig causing the endpoint to return correctly after a new config is applied via API and then nothing once the periodic sync runs. Note: Technically, the upstream GrafanaAlertamanger GetStatus shouldn't be returning PostableUserConfig or PostableApiAlertingConfig, but instead GettableStatus. However, this issue required changes elsewhere and is out of scope.
What is this feature?
AM config applied via API would use the
PostableUserConfigas the AM raw config and also the hash used to decide when the AM config has changed. However, when applied via the periodic sync thePostableApiAlertingConfigwould be used instead.This changes makes all means of applying the AM config use
PostableApiAlertingConfig.Why do we need this feature?
This issue causes:
GetStatus(api/alertmanager/grafana/api/v2/status) assumed the raw config wasPostableUserConfigcausing the endpoint to return correctly after a new config is applied via API and then nothing once the periodic sync runs.Who is this feature for?
UA users.
Special notes for your reviewer:
Technically, the upstream
GrafanaAlertamangerGetStatusshouldn't be returningPostableUserConfigorPostableApiAlertingConfig, but insteadGettableStatus. However, this issue required changes elsewhere and is out of scope.