diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index 10d355679ed..5a40a0013fd 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -188,7 +188,7 @@ func (ng *AlertNG) init() error { // Configure the remote Alertmanager. // If toggles for both modes are enabled, remote primary takes precedence. - var overrides []notifier.Option + var opts []notifier.Option moaLogger := log.New("ngalert.multiorg.alertmanager") crypto := notifier.NewCrypto(ng.SecretsService, ng.store, moaLogger) remotePrimary := ng.FeatureToggles.IsEnabled(initCtx, featuremgmt.FlagAlertmanagerRemotePrimary) @@ -220,71 +220,31 @@ func (ng *AlertNG) init() error { 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) } + store := notifier.NewFileStore(cfg.OrgID, ng.KVStore) + + // This function will be used by the MOA to create new Alertmanagers. + var override func(notifier.OrgAlertmanagerFactory) notifier.OrgAlertmanagerFactory - var override notifier.Option if remotePrimary { ng.Log.Debug("Starting Grafana with remote primary mode enabled") m.Info.WithLabelValues(metrics.ModeRemotePrimary).Set(1) - // This function will be used by the MOA to create new Alertmanagers. - override = notifier.WithAlertmanagerOverride(func(factoryFn notifier.OrgAlertmanagerFactory) notifier.OrgAlertmanagerFactory { - return func(ctx context.Context, orgID int64) (notifier.Alertmanager, error) { - // Create internal Alertmanager. - internalAM, err := factoryFn(ctx, orgID) - if err != nil { - return nil, err - } - - // Create remote Alertmanager. - cfg.OrgID = orgID - cfg.PromoteConfig = true - remoteAM, err := createRemoteAlertmanager(ctx, cfg, ng.KVStore, crypto, autogenFn, m, ng.tracer) - if err != nil { - moaLogger.Error("Failed to create remote Alertmanager, falling back to using only the internal one", "err", err) - return internalAM, nil - } - - // Use both Alertmanager implementations in the forked Alertmanager. - return remote.NewRemotePrimaryForkedAlertmanager(log.New("ngalert.forked-alertmanager.remote-primary"), internalAM, remoteAM), nil - } - }) + override = remote.NewRemotePrimaryFactory(cfg, store, crypto, autogenFn, m, ng.tracer) } else { ng.Log.Debug("Starting Grafana with remote secondary mode enabled") m.Info.WithLabelValues(metrics.ModeRemoteSecondary).Set(1) - - // This function will be used by the MOA to create new Alertmanagers. - override = notifier.WithAlertmanagerOverride(func(factoryFn notifier.OrgAlertmanagerFactory) notifier.OrgAlertmanagerFactory { - return func(ctx context.Context, orgID int64) (notifier.Alertmanager, error) { - // Create internal Alertmanager. - internalAM, err := factoryFn(ctx, orgID) - if err != nil { - return nil, err - } - - // Create remote Alertmanager. - cfg.OrgID = orgID - remoteAM, err := createRemoteAlertmanager(ctx, cfg, ng.KVStore, crypto, autogenFn, m, ng.tracer) - if err != nil { - if remoteSecondaryWithRemoteState { - // We can't start the internal Alertmanager without the remote state. - return nil, fmt.Errorf("failed to create remote Alertmanager, can't start the internal Alertmanager without the remote state: %w", err) - } - moaLogger.Error("Failed to create remote Alertmanager, falling back to using only the internal one", "err", err) - return internalAM, nil - } - - // Use both Alertmanager implementations in the forked Alertmanager. - rsCfg := remote.RemoteSecondaryConfig{ - Logger: log.New("ngalert.forked-alertmanager.remote-secondary"), - OrgID: orgID, - Store: ng.store, - SyncInterval: ng.Cfg.UnifiedAlerting.RemoteAlertmanager.SyncInterval, - WithRemoteState: remoteSecondaryWithRemoteState, - } - return remote.NewRemoteSecondaryForkedAlertmanager(rsCfg, internalAM, remoteAM) - } - }) + override = remote.NewRemoteSecondaryFactory(cfg, + store, + ng.store, + ng.Cfg.UnifiedAlerting.RemoteAlertmanager.SyncInterval, + crypto, + autogenFn, + m, + ng.tracer, + remoteSecondaryWithRemoteState, + ) } - overrides = append(overrides, override) + + opts = append(opts, notifier.WithAlertmanagerOverride(override)) } notificationHistorian, err := configureNotificationHistorian( @@ -315,7 +275,7 @@ func (ng *AlertNG) init() error { ng.SecretsService, ng.FeatureToggles, notificationHistorian, - overrides..., + opts..., ) if err != nil { return err @@ -763,10 +723,6 @@ func configureNotificationHistorian( return notificationHistorian, nil } -func createRemoteAlertmanager(ctx context.Context, cfg remote.AlertmanagerConfig, kvstore kvstore.KVStore, crypto remote.Crypto, autogenFn remote.AutogenFn, m *metrics.RemoteAlertmanager, tracer tracing.Tracer) (*remote.Alertmanager, error) { - return remote.NewAlertmanager(ctx, cfg, notifier.NewFileStore(cfg.OrgID, kvstore), crypto, autogenFn, m, tracer) -} - func createRecordingWriter(settings setting.RecordingRuleSettings, httpClientProvider httpclient.Provider, datasourceService datasources.DataSourceService, pluginContextProvider *plugincontext.Provider, clock clock.Clock, m *metrics.RemoteWriter) (schedule.RecordingWriter, error) { logger := log.New("ngalert.writer") diff --git a/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go b/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go index 81b8aebb766..344486bb14d 100644 --- a/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go +++ b/pkg/services/ngalert/notifier/multiorg_alertmanager_remote_test.go @@ -51,36 +51,24 @@ func TestMultiorgAlertmanager_RemoteSecondaryMode(t *testing.T) { }) // Create the factory function for the MOA using the forked Alertmanager in remote secondary mode. + remoteAMCfg := remote.AlertmanagerConfig{ + OrgID: 1, + URL: testsrv.URL, + TenantID: tenantID, + BasicAuthPassword: password, + DefaultConfig: setting.GetAlertmanagerDefaultConfiguration(), + } secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) - override := notifier.WithAlertmanagerOverride(func(factoryFn notifier.OrgAlertmanagerFactory) notifier.OrgAlertmanagerFactory { - return func(ctx context.Context, orgID int64) (notifier.Alertmanager, error) { - // Create internal Alertmanager. - internalAM, err := factoryFn(ctx, orgID) - require.NoError(t, err) - - // Create remote Alertmanager. - externalAMCfg := remote.AlertmanagerConfig{ - OrgID: 1, - URL: testsrv.URL, - TenantID: tenantID, - BasicAuthPassword: password, - DefaultConfig: setting.GetAlertmanagerDefaultConfiguration(), - } - m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) - remoteAM, err := remote.NewAlertmanager(ctx, externalAMCfg, notifier.NewFileStore(orgID, kvStore), notifier.NewCrypto(secretsService, configStore, log.NewNopLogger()), remote.NoopAutogenFn, m, tracing.InitializeTracerForTest()) - require.NoError(t, err) - - // Use both Alertmanager implementations in the forked Alertmanager. - cfg := remote.RemoteSecondaryConfig{ - Logger: nopLogger, - OrgID: orgID, - Store: configStore, - // Note that we're setting a sync interval of 10 seconds. - SyncInterval: 10 * time.Second, - } - return remote.NewRemoteSecondaryForkedAlertmanager(cfg, internalAM, remoteAM) - } - }) + override := remote.NewRemoteSecondaryFactory(remoteAMCfg, + notifier.NewFileStore(remoteAMCfg.OrgID, kvStore), + configStore, + 10*time.Second, + notifier.NewCrypto(secretsService, configStore, log.NewNopLogger()), + remote.NoopAutogenFn, + m.GetRemoteAlertmanagerMetrics(), + tracing.InitializeTracerForTest(), + false, + ) cfg := &setting.Cfg{ DataPath: t.TempDir(), @@ -103,7 +91,7 @@ func TestMultiorgAlertmanager_RemoteSecondaryMode(t *testing.T) { secretsService, featuremgmt.WithFeatures(), nil, - override, + notifier.WithAlertmanagerOverride(override), ) require.NoError(t, err) diff --git a/pkg/services/ngalert/remote/forked_alertmanager_test.go b/pkg/services/ngalert/remote/forked_alertmanager_test.go index af8769a2e88..72ca0dae910 100644 --- a/pkg/services/ngalert/remote/forked_alertmanager_test.go +++ b/pkg/services/ngalert/remote/forked_alertmanager_test.go @@ -810,11 +810,11 @@ func genTestAlertmanagers(t *testing.T, mode int, options ...func(RemoteSecondar cfg = opt(cfg) } - forked, err := NewRemoteSecondaryForkedAlertmanager(cfg, internal, remote) + forked, err := newRemoteSecondaryForkedAlertmanager(cfg, internal, remote) require.NoError(t, err) return internal, remote, forked } - return internal, remote, NewRemotePrimaryForkedAlertmanager(log.NewNopLogger(), internal, remote) + return internal, remote, newRemotePrimaryForkedAlertmanager(log.NewNopLogger(), internal, remote) } // errConfigStore returns an error when a method is called. diff --git a/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go b/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go index 7805eaea3d7..4e124639431 100644 --- a/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go +++ b/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go @@ -8,7 +8,9 @@ import ( alertingNotify "github.com/grafana/alerting/notify" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" + "github.com/grafana/grafana/pkg/services/ngalert/metrics" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/notifier" ) @@ -20,7 +22,40 @@ type RemotePrimaryForkedAlertmanager struct { remote remoteAlertmanager } -func NewRemotePrimaryForkedAlertmanager(log log.Logger, internal notifier.Alertmanager, remote remoteAlertmanager) *RemotePrimaryForkedAlertmanager { +// NewRemotePrimaryFactory returns a function to override the default AM factory in the multi-org Alertmanager. +func NewRemotePrimaryFactory( + cfg AlertmanagerConfig, + store stateStore, + crypto Crypto, + autogenFn AutogenFn, + m *metrics.RemoteAlertmanager, + t tracing.Tracer, +) func(notifier.OrgAlertmanagerFactory) notifier.OrgAlertmanagerFactory { + return func(factoryFn notifier.OrgAlertmanagerFactory) notifier.OrgAlertmanagerFactory { + return func(ctx context.Context, orgID int64) (notifier.Alertmanager, error) { + // Create the internal Alertmanager. + internalAM, err := factoryFn(ctx, orgID) + if err != nil { + return nil, err + } + + // Create the remote Alertmanager. + cfg.OrgID = orgID + cfg.PromoteConfig = true + l := log.New("ngalert.forked-alertmanager.remote-primary") + remoteAM, err := NewAlertmanager(ctx, cfg, store, crypto, autogenFn, m, t) + if err != nil { + l.Error("Failed to create remote Alertmanager, falling back to using only the internal one", "err", err) + return internalAM, nil + } + + // Use both implementations in the forked Alertmanager. + return newRemotePrimaryForkedAlertmanager(l, internalAM, remoteAM), nil + } + } +} + +func newRemotePrimaryForkedAlertmanager(log log.Logger, internal notifier.Alertmanager, remote remoteAlertmanager) *RemotePrimaryForkedAlertmanager { return &RemotePrimaryForkedAlertmanager{ log: log, internal: internal, diff --git a/pkg/services/ngalert/remote/remote_secondary_forked_alertmanager.go b/pkg/services/ngalert/remote/remote_secondary_forked_alertmanager.go index 45a24ee1dca..94c06f5ed19 100644 --- a/pkg/services/ngalert/remote/remote_secondary_forked_alertmanager.go +++ b/pkg/services/ngalert/remote/remote_secondary_forked_alertmanager.go @@ -9,7 +9,9 @@ import ( alertingNotify "github.com/grafana/alerting/notify" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" + "github.com/grafana/grafana/pkg/services/ngalert/metrics" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/notifier" ) @@ -60,7 +62,54 @@ func (c *RemoteSecondaryConfig) Validate() error { return nil } -func NewRemoteSecondaryForkedAlertmanager(cfg RemoteSecondaryConfig, internal notifier.Alertmanager, remote remoteAlertmanager) (*RemoteSecondaryForkedAlertmanager, error) { +// NewRemoteSecondaryFactory returns a function to override the default AM factory in the multi-org Alertmanager. +func NewRemoteSecondaryFactory( + cfg AlertmanagerConfig, + stateStore stateStore, + cfgStore configStore, + syncInterval time.Duration, + crypto Crypto, + autogenFn AutogenFn, + m *metrics.RemoteAlertmanager, + t tracing.Tracer, + withRemoteState bool, +) func(notifier.OrgAlertmanagerFactory) notifier.OrgAlertmanagerFactory { + return func(factoryFn notifier.OrgAlertmanagerFactory) notifier.OrgAlertmanagerFactory { + return func(ctx context.Context, orgID int64) (notifier.Alertmanager, error) { + // Create the remote Alertmanager first so we don't need to unregister internal AM metrics if this fails. + cfg.OrgID = orgID + l := log.New("ngalert.forked-alertmanager.remote-secondary") + remoteAM, err := NewAlertmanager(ctx, cfg, stateStore, crypto, autogenFn, m, t) + if err != nil && withRemoteState { + // We can't start the internal Alertmanager without the remote state. + return nil, fmt.Errorf("failed to create remote Alertmanager, can't start the internal Alertmanager without the remote state: %w", err) + } + + // Create the internal Alertmanager. + internalAM, err := factoryFn(ctx, orgID) + if err != nil { + return nil, fmt.Errorf("failed to create internal Alertmanager: %w", err) + } + + if remoteAM == nil { + l.Error("Failed to create remote Alertmanager, falling back to using only the internal one", "err", err) + return internalAM, nil + } + + // Use both implementations in the forked Alertmanager. + rsCfg := RemoteSecondaryConfig{ + Logger: l, + OrgID: orgID, + Store: cfgStore, + SyncInterval: syncInterval, + WithRemoteState: withRemoteState, + } + return newRemoteSecondaryForkedAlertmanager(rsCfg, internalAM, remoteAM) + } + } +} + +func newRemoteSecondaryForkedAlertmanager(cfg RemoteSecondaryConfig, internal notifier.Alertmanager, remote remoteAlertmanager) (*RemoteSecondaryForkedAlertmanager, error) { if err := cfg.Validate(); err != nil { return nil, err }