Alerting: Alertmanager configuration sync loop (#88822)

* make the config sync happen on each call to ApplyConfig(), fix tests

* send autogen config

* add fake autogen function for tests

* update stale comments, tidy things up, make linter happy

* add auto-gen routes only if the feature toggle is enabled

* remove unnecessary fake autogen function

* throttle configuration syncs

* restore pkg/services/store/entity/sqlstash/sql_storage_server.go

* test sync loop in ApplyConfig, skip invalid autogen routes

* restore conf/defaults.ini

* restore conf/defaults.ini

* avoid skipping invalid auto-gen routes in SaveAndApplyConfig

* test that autogenFn is called and its errors are returned

* add debug message about the sync interval not having elapsed

* collapse two log lines into one
This commit is contained in:
Santiago
2024-06-12 10:13:34 +02:00
committed by GitHub
parent c85d10d6c3
commit 12d5251c12
5 changed files with 151 additions and 89 deletions
+14 -4
View File
@@ -28,6 +28,7 @@ import (
"github.com/grafana/grafana/pkg/services/folder"
ac "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol"
"github.com/grafana/grafana/pkg/services/ngalert/api"
"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
"github.com/grafana/grafana/pkg/services/ngalert/eval"
"github.com/grafana/grafana/pkg/services/ngalert/image"
"github.com/grafana/grafana/pkg/services/ngalert/metrics"
@@ -172,6 +173,13 @@ func (ng *AlertNG) init() error {
remotePrimary := ng.FeatureToggles.IsEnabled(initCtx, featuremgmt.FlagAlertmanagerRemotePrimary)
remoteSecondary := ng.FeatureToggles.IsEnabled(initCtx, featuremgmt.FlagAlertmanagerRemoteSecondary)
if ng.Cfg.UnifiedAlerting.RemoteAlertmanager.Enable {
autogenFn := remote.NoopAutogenFn
if ng.FeatureToggles.IsEnabled(initCtx, featuremgmt.FlagAlertingSimplifiedRouting) {
autogenFn = func(ctx context.Context, logger log.Logger, orgID int64, cfg *definitions.PostableApiAlertingConfig, skipInvalid bool) error {
return notifier.AddAutogenConfig(ctx, logger, ng.store, orgID, cfg, skipInvalid)
}
}
switch {
case remoteOnly:
ng.Log.Debug("Starting Grafana with remote only mode enabled")
@@ -190,8 +198,9 @@ func (ng *AlertNG) init() error {
TenantID: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.TenantID,
URL: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.URL,
PromoteConfig: true,
SyncInterval: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.SyncInterval,
}
remoteAM, err := createRemoteAlertmanager(cfg, ng.KVStore, ng.SecretsService.Decrypt, m)
remoteAM, err := createRemoteAlertmanager(cfg, ng.KVStore, ng.SecretsService.Decrypt, autogenFn, m)
if err != nil {
moaLogger.Error("Failed to create remote Alertmanager", "err", err)
return nil, err
@@ -259,8 +268,9 @@ func (ng *AlertNG) init() error {
OrgID: orgID,
TenantID: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.TenantID,
URL: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.URL,
SyncInterval: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.SyncInterval,
}
remoteAM, err := createRemoteAlertmanager(cfg, ng.KVStore, ng.SecretsService.Decrypt, m)
remoteAM, err := createRemoteAlertmanager(cfg, ng.KVStore, ng.SecretsService.Decrypt, autogenFn, m)
if err != nil {
moaLogger.Error("Failed to create remote Alertmanager, falling back to using only the internal one", "err", err)
return internalAM, nil
@@ -611,6 +621,6 @@ func ApplyStateHistoryFeatureToggles(cfg *setting.UnifiedAlertingStateHistorySet
}
}
func createRemoteAlertmanager(cfg remote.AlertmanagerConfig, kvstore kvstore.KVStore, decryptFn remote.DecryptFn, m *metrics.RemoteAlertmanager) (*remote.Alertmanager, error) {
return remote.NewAlertmanager(cfg, notifier.NewFileStore(cfg.OrgID, kvstore), decryptFn, m)
func createRemoteAlertmanager(cfg remote.AlertmanagerConfig, kvstore kvstore.KVStore, decryptFn remote.DecryptFn, autogenFn remote.AutogenFn, m *metrics.RemoteAlertmanager) (*remote.Alertmanager, error) {
return remote.NewAlertmanager(cfg, notifier.NewFileStore(cfg.OrgID, kvstore), decryptFn, autogenFn, m)
}
@@ -66,7 +66,7 @@ func TestMultiorgAlertmanager_RemoteSecondaryMode(t *testing.T) {
DefaultConfig: setting.GetAlertmanagerDefaultConfiguration(),
}
m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry())
remoteAM, err := remote.NewAlertmanager(externalAMCfg, notifier.NewFileStore(orgID, kvStore), secretsService.Decrypt, m)
remoteAM, err := remote.NewAlertmanager(externalAMCfg, notifier.NewFileStore(orgID, kvStore), secretsService.Decrypt, remote.NoopAutogenFn, m)
require.NoError(t, err)
// Use both Alertmanager implementations in the forked Alertmanager.
+52 -24
View File
@@ -38,10 +38,19 @@ type stateStore interface {
GetNotificationLog(ctx context.Context) (string, error)
}
// AutogenFn is a function that adds auto-generated routes to a configuration.
type AutogenFn func(ctx context.Context, logger log.Logger, orgId int64, config *apimodels.PostableApiAlertingConfig, skipInvalid bool) error
// NoopAutogenFn is used to skip auto-generating routes.
func NoopAutogenFn(_ context.Context, _ log.Logger, _ int64, _ *apimodels.PostableApiAlertingConfig, _ bool) error {
return nil
}
// DecryptFn is a function that takes in an encrypted value and returns it decrypted.
type DecryptFn func(ctx context.Context, payload []byte) ([]byte, error)
type Alertmanager struct {
autogenFn AutogenFn
decrypt DecryptFn
defaultConfig string
defaultConfigHash string
@@ -54,6 +63,9 @@ type Alertmanager struct {
tenantID string
url string
lastConfigSync time.Time
syncInterval time.Duration
amClient *remoteClient.Alertmanager
mimirClient remoteClient.MimirClient
}
@@ -68,6 +80,9 @@ type AlertmanagerConfig struct {
// PromoteConfig is a flag that determines whether the configuration should be used in the remote Alertmanager.
// The same flag is used for promoting state.
PromoteConfig bool
// SyncInterval determines how often we should attempt to synchronize configuration.
SyncInterval time.Duration
}
func (cfg *AlertmanagerConfig) Validate() error {
@@ -85,7 +100,7 @@ func (cfg *AlertmanagerConfig) Validate() error {
return nil
}
func NewAlertmanager(cfg AlertmanagerConfig, store stateStore, decryptFn DecryptFn, metrics *metrics.RemoteAlertmanager) (*Alertmanager, error) {
func NewAlertmanager(cfg AlertmanagerConfig, store stateStore, decryptFn DecryptFn, autogenFn AutogenFn, metrics *metrics.RemoteAlertmanager) (*Alertmanager, error) {
if err := cfg.Validate(); err != nil {
return nil, err
}
@@ -151,6 +166,7 @@ func NewAlertmanager(cfg AlertmanagerConfig, store stateStore, decryptFn Decrypt
return &Alertmanager{
amClient: amc,
autogenFn: autogenFn,
decrypt: decryptFn,
defaultConfig: string(rawCfg),
defaultConfigHash: fmt.Sprintf("%x", md5.Sum(rawCfg)),
@@ -160,42 +176,43 @@ func NewAlertmanager(cfg AlertmanagerConfig, store stateStore, decryptFn Decrypt
orgID: cfg.OrgID,
state: store,
sender: s,
syncInterval: cfg.SyncInterval,
tenantID: cfg.TenantID,
url: cfg.URL,
}, nil
}
// ApplyConfig is called everytime we've determined we need to apply an existing configuration to the Alertmanager,
// including the first time the Alertmanager is started. In the context of a "remote Alertmanager" it's as good of a heuristic,
// for "a function that gets called when the Alertmanager starts". As a result we do two things:
// ApplyConfig is called by the multi-org Alertmanager on startup and on every sync loop iteration (1m default).
// We do two things on startup:
// 1. Execute a readiness check to make sure the remote Alertmanager we're about to communicate with is up and ready.
// 2. Upload the configuration and state we currently hold.
// On each subsequent call to ApplyConfig we compare and upload only the configuration.
func (am *Alertmanager) ApplyConfig(ctx context.Context, config *models.AlertConfiguration) error {
if am.ready {
am.log.Debug("Alertmanager previously marked as ready, skipping readiness check and config + state update")
am.log.Debug("Alertmanager previously marked as ready, skipping readiness check and state sync")
} else {
am.log.Debug("Start readiness check for remote Alertmanager", "url", am.url)
if err := am.checkReadiness(ctx); err != nil {
return fmt.Errorf("unable to pass the readiness check: %w", err)
}
am.log.Debug("Completed readiness check for remote Alertmanager, starting state upload", "url", am.url)
if err := am.CompareAndSendState(ctx); err != nil {
return fmt.Errorf("unable to upload the state to the remote Alertmanager: %w", err)
}
am.log.Debug("Completed state upload to remote Alertmanager", "url", am.url)
}
if time.Since(am.lastConfigSync) < am.syncInterval {
am.log.Debug("Not syncing configuration to remote Alertmanager, last sync was too recent")
return nil
}
// First, execute a readiness check to make sure the remote Alertmanager is ready.
am.log.Debug("Start readiness check for remote Alertmanager", "url", am.url)
if err := am.checkReadiness(ctx); err != nil {
am.log.Error("Unable to pass the readiness check", "err", err)
return err
}
am.log.Debug("Completed readiness check for remote Alertmanager", "url", am.url)
// Send configuration and base64-encoded state if necessary.
am.log.Debug("Start configuration upload to remote Alertmanager", "url", am.url)
if err := am.CompareAndSendConfiguration(ctx, config); err != nil {
am.log.Error("Unable to upload the configuration to the remote Alertmanager", "err", err)
return fmt.Errorf("unable to upload the configuration to the remote Alertmanager: %w", err)
}
am.log.Debug("Completed configuration upload to remote Alertmanager", "url", am.url)
am.log.Debug("Start state upload to remote Alertmanager", "url", am.url)
if err := am.CompareAndSendState(ctx); err != nil {
am.log.Error("Unable to upload the state to the remote Alertmanager", "err", err)
}
am.log.Debug("Completed state upload to remote Alertmanager", "url", am.url)
return nil
}
@@ -223,7 +240,10 @@ func (am *Alertmanager) CompareAndSendConfiguration(ctx context.Context, config
return err
}
// Decrypt the configuration before comparing.
// Add auto-generated routes and decrypt before comparing.
if err := am.autogenFn(ctx, am.log, am.orgID, &c.AlertmanagerConfig, true); err != nil {
return err
}
decrypted, err := am.decryptConfiguration(ctx, c)
if err != nil {
return err
@@ -261,6 +281,7 @@ func (am *Alertmanager) sendConfiguration(ctx context.Context, decrypted *apimod
return err
}
am.metrics.LastConfigSync.SetToCurrentTime()
am.lastConfigSync = time.Now()
return nil
}
@@ -292,11 +313,15 @@ func (am *Alertmanager) SaveAndApplyConfig(ctx context.Context, cfg *apimodels.P
}
hash := fmt.Sprintf("%x", md5.Sum(rawCfg))
// Decrypt and send.
// Add auto-generated routes and decrypt before sending.
if err := am.autogenFn(ctx, am.log, am.orgID, &cfg.AlertmanagerConfig, false); err != nil {
return err
}
decrypted, err := am.decryptConfiguration(ctx, cfg)
if err != nil {
return err
}
return am.sendConfiguration(ctx, decrypted, hash, time.Now().Unix(), false)
}
@@ -307,7 +332,10 @@ func (am *Alertmanager) SaveAndApplyDefaultConfig(ctx context.Context) error {
return fmt.Errorf("unable to parse the default configuration: %w", err)
}
// Decrypt before sending.
// Add auto-generated routes and decrypt before sending.
if err := am.autogenFn(ctx, am.log, am.orgID, &c.AlertmanagerConfig, true); err != nil {
return err
}
decrypted, err := am.decryptConfiguration(ctx, c)
if err != nil {
return err
@@ -20,7 +20,9 @@ import (
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/require"
"github.com/grafana/alerting/definition"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log"
apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
"github.com/grafana/grafana/pkg/services/ngalert/metrics"
ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
@@ -51,6 +53,7 @@ const (
var (
defaultGrafanaConfig = setting.GetAlertmanagerDefaultConfiguration()
errTest = errors.New("test")
)
func TestMain(m *testing.M) {
@@ -102,7 +105,7 @@ func TestNewAlertmanager(t *testing.T) {
DefaultConfig: defaultGrafanaConfig,
}
m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry())
am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, m)
am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m)
if test.expErr != "" {
require.EqualError(tt, err, test.expErr)
return
@@ -118,16 +121,26 @@ func TestNewAlertmanager(t *testing.T) {
}
func TestApplyConfig(t *testing.T) {
// errorHandler returns an error response for the readiness check and state sync.
errorHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Add("content-type", "application/json")
w.WriteHeader(http.StatusInternalServerError)
require.NoError(t, json.NewEncoder(w).Encode(map[string]string{"status": "error"}))
})
var configSent client.UserGrafanaConfig
var lastConfigSync, lastStateSync time.Time
okHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/config") {
require.NoError(t, json.NewDecoder(r.Body).Decode(&configSent))
if r.Method == http.MethodPost {
if strings.Contains(r.URL.Path, "/config") {
require.NoError(t, json.NewDecoder(r.Body).Decode(&configSent))
lastConfigSync = time.Now()
} else {
lastStateSync = time.Now()
}
}
w.WriteHeader(http.StatusOK)
w.Header().Add("content-type", "application/json")
require.NoError(t, json.NewEncoder(w).Encode(map[string]string{"status": "success"}))
})
// Encrypt receivers to save secrets in the database.
@@ -153,6 +166,7 @@ func TestApplyConfig(t *testing.T) {
URL: server.URL,
DefaultConfig: defaultGrafanaConfig,
PromoteConfig: true,
SyncInterval: 1 * time.Hour,
}
ctx := context.Background()
@@ -163,7 +177,7 @@ func TestApplyConfig(t *testing.T) {
// An error response from the remote Alertmanager should result in the readiness check failing.
m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry())
am, err := NewAlertmanager(cfg, fstore, secretsService.Decrypt, m)
am, err := NewAlertmanager(cfg, fstore, secretsService.Decrypt, NoopAutogenFn, m)
require.NoError(t, err)
config := &ngmodels.AlertConfiguration{
@@ -183,22 +197,35 @@ func TestApplyConfig(t *testing.T) {
require.JSONEq(t, testGrafanaConfigWithSecret, string(amCfg))
require.True(t, configSent.Promoted)
// If we already got a 200 status code response, we shouldn't make the HTTP request again.
server.Config.Handler = errorHandler
// If we already got a 200 status code response and the sync interval hasn't elapsed,
// we shouldn't send the state/configuration again.
expStateSync := lastStateSync
expConfigSync := lastConfigSync
require.NoError(t, am.ApplyConfig(ctx, config))
require.True(t, am.Ready())
require.Equal(t, expStateSync, lastStateSync)
require.Equal(t, expConfigSync, lastConfigSync)
// Changing the sync interval and calling ApplyConfig again
// should result in us sending the configuration but not the state.
am.syncInterval = 0
require.NoError(t, am.ApplyConfig(ctx, config))
require.Equal(t, lastStateSync, expStateSync)
require.Greater(t, lastConfigSync, expConfigSync)
// Failing to add the auto-generated routes should result in an error.
am.autogenFn = errAutogenFn
require.ErrorIs(t, am.ApplyConfig(ctx, config), errTest)
}
func TestCompareAndSendConfiguration(t *testing.T) {
cfgWithSecret, err := notifier.Load([]byte(testGrafanaConfigWithSecret))
require.NoError(t, err)
testValue := []byte("test")
testErr := errors.New("test error")
decryptFn := func(_ context.Context, payload []byte) ([]byte, error) {
if string(payload) == string(testValue) {
return testValue, nil
}
return nil, testErr
return nil, errTest
}
var got string
@@ -222,40 +249,48 @@ func TestCompareAndSendConfiguration(t *testing.T) {
URL: server.URL,
DefaultConfig: defaultGrafanaConfig,
}
am, err := NewAlertmanager(cfg,
fstore,
decryptFn,
m,
)
require.NoError(t, err)
tests := []struct {
name string
config string
expCfg *client.UserGrafanaConfig
expErr string
name string
config string
autogenFn AutogenFn
expCfg *client.UserGrafanaConfig
expErr string
}{
{
"invalid config",
"{}",
NoopAutogenFn,
nil,
"unable to parse Alertmanager configuration: no route provided in config",
},
{
"invalid base-64 in key",
strings.Replace(testGrafanaConfigWithSecret, `"password":"test"`, `"password":"!"`, 1),
NoopAutogenFn,
nil,
"unable to decrypt the configuration: failed to decode value for key 'password': illegal base64 data at input byte 0",
},
{
"decrypt error",
testGrafanaConfigWithSecret,
NoopAutogenFn,
nil,
fmt.Sprintf("unable to decrypt the configuration: failed to decrypt value for key 'password': %s", testErr.Error()),
fmt.Sprintf("unable to decrypt the configuration: failed to decrypt value for key 'password': %s", errTest.Error()),
},
{
"error from autogen function",
strings.Replace(testGrafanaConfigWithSecret, `"password":"test"`, fmt.Sprintf("%q:%q", "password", base64.StdEncoding.EncodeToString(testValue)), 1),
errAutogenFn,
&client.UserGrafanaConfig{
GrafanaAlertmanagerConfig: cfgWithSecret,
},
errTest.Error(),
},
{
"no error",
strings.Replace(testGrafanaConfigWithSecret, `"password":"test"`, fmt.Sprintf("%q:%q", "password", base64.StdEncoding.EncodeToString(testValue)), 1),
NoopAutogenFn,
&client.UserGrafanaConfig{
GrafanaAlertmanagerConfig: cfgWithSecret,
},
@@ -265,6 +300,14 @@ func TestCompareAndSendConfiguration(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(tt *testing.T) {
am, err := NewAlertmanager(cfg,
fstore,
decryptFn,
test.autogenFn,
m,
)
require.NoError(t, err)
cfg := ngmodels.AlertConfiguration{
AlertmanagerConfiguration: test.config,
}
@@ -321,7 +364,7 @@ func TestIntegrationRemoteAlertmanagerConfiguration(t *testing.T) {
secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(db.InitTestDB(t)))
m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry())
am, err := NewAlertmanager(cfg, fstore, secretsService.Decrypt, m)
am, err := NewAlertmanager(cfg, fstore, secretsService.Decrypt, NoopAutogenFn, m)
require.NoError(t, err)
encodedFullState, err := am.getFullState(ctx)
@@ -417,6 +460,11 @@ func TestIntegrationRemoteAlertmanagerConfiguration(t *testing.T) {
require.JSONEq(t, testGrafanaConfigWithSecret, string(got))
require.Equal(t, fmt.Sprintf("%x", md5.Sum(encryptedConfig)), config.Hash)
require.False(t, config.Default)
// An error while adding auto-generated rutes should be returned.
am.autogenFn = errAutogenFn
require.ErrorIs(t, am.SaveAndApplyConfig(ctx, postableCfg), errTest)
am.autogenFn = NoopAutogenFn
}
// `SaveAndApplyDefaultConfig` should send the default Alertmanager configuration to the remote Alertmanager.
@@ -439,6 +487,11 @@ func TestIntegrationRemoteAlertmanagerConfiguration(t *testing.T) {
require.JSONEq(t, string(want), string(got))
require.Equal(t, fmt.Sprintf("%x", md5.Sum(want)), config.Hash)
require.True(t, config.Default)
// An error while adding auto-generated rutes should be returned.
am.autogenFn = errAutogenFn
require.ErrorIs(t, am.SaveAndApplyDefaultConfig(ctx), errTest)
am.autogenFn = NoopAutogenFn
}
// TODO: Now, shutdown the Alertmanager and we expect the latest configuration to be uploaded.
@@ -468,7 +521,7 @@ func TestIntegrationRemoteAlertmanagerGetStatus(t *testing.T) {
secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry())
am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, m)
am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m)
require.NoError(t, err)
// We should get the default Cloud Alertmanager configuration.
@@ -502,7 +555,7 @@ func TestIntegrationRemoteAlertmanagerSilences(t *testing.T) {
secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry())
am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, m)
am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m)
require.NoError(t, err)
// We should have no silences at first.
@@ -587,7 +640,7 @@ func TestIntegrationRemoteAlertmanagerAlerts(t *testing.T) {
secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry())
am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, m)
am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m)
require.NoError(t, err)
// Wait until the Alertmanager is ready to send alerts.
@@ -656,7 +709,7 @@ func TestIntegrationRemoteAlertmanagerReceivers(t *testing.T) {
secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry())
am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, m)
am, err := NewAlertmanager(cfg, nil, secretsService.Decrypt, NoopAutogenFn, m)
require.NoError(t, err)
// We should start with the default config.
@@ -682,6 +735,11 @@ func genAlert(active bool, labels map[string]string) amv2.PostableAlert {
}
}
// errAutogenFn is an AutogenFn that always returns an error.
func errAutogenFn(_ context.Context, _ log.Logger, _ int64, _ *definition.PostableApiAlertingConfig, _ bool) error {
return errTest
}
const defaultCloudAMConfig = `
global:
resolve_timeout: 5m