diff --git a/pkg/services/ngalert/remote/forked_alertmanager_test.go b/pkg/services/ngalert/remote/forked_alertmanager_test.go index 278d38efaa0..80d40da3617 100644 --- a/pkg/services/ngalert/remote/forked_alertmanager_test.go +++ b/pkg/services/ngalert/remote/forked_alertmanager_test.go @@ -406,6 +406,26 @@ func TestForkedAlertmanager_ModeRemotePrimary(t *testing.T) { require.NoError(tt, forked.SaveAndApplyDefaultConfig(ctx)) }) + t.Run("SaveAndApplyConfig", func(tt *testing.T) { + // SaveAndApplyConfig should first be called on the remote Alertmanager + // and then on the internal one. + internal, remote, forked := genTestAlertmanagers(tt, modeRemotePrimary) + remoteCall := remote.EXPECT().SaveAndApplyConfig(ctx, mock.Anything).Return(nil).Once() + internal.EXPECT().SaveAndApplyConfig(ctx, mock.Anything).Return(nil).Once().NotBefore(remoteCall) + require.NoError(tt, forked.SaveAndApplyConfig(ctx, &apimodels.PostableUserConfig{})) + + // If there's an error in the remote Alertmanager, it should be returned. + _, remote, forked = genTestAlertmanagers(tt, modeRemotePrimary) + remote.EXPECT().SaveAndApplyConfig(ctx, mock.Anything).Return(expErr).Once() + require.ErrorIs(tt, expErr, forked.SaveAndApplyConfig(ctx, &apimodels.PostableUserConfig{})) + + // An error in the internal Alertmanager should not be returned. + internal, remote, forked = genTestAlertmanagers(tt, modeRemotePrimary) + remote.EXPECT().SaveAndApplyConfig(ctx, mock.Anything).Return(nil).Once() + internal.EXPECT().SaveAndApplyConfig(ctx, mock.Anything).Return(expErr).Once() + require.NoError(tt, forked.SaveAndApplyConfig(ctx, &apimodels.PostableUserConfig{})) + }) + t.Run("GetStatus", func(tt *testing.T) { // We care about the status of the remote Alertmanager. _, remote, forked := genTestAlertmanagers(tt, modeRemotePrimary) diff --git a/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go b/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go index 1a360e4f513..8c2068e8179 100644 --- a/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go +++ b/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go @@ -34,12 +34,23 @@ func (fam *RemotePrimaryForkedAlertmanager) ApplyConfig(ctx context.Context, con } if err := fam.internal.ApplyConfig(ctx, config); err != nil { + // An error in the internal Alertmanager shouldn't make the whole operation fail. + // We're replicating writes in the internal Alertmanager just for comparing and in case we need to roll back. fam.log.Error("Error applying config to the internal Alertmanager", "err", err) } return nil } func (fam *RemotePrimaryForkedAlertmanager) SaveAndApplyConfig(ctx context.Context, config *apimodels.PostableUserConfig) error { + if err := fam.remote.SaveAndApplyConfig(ctx, config); err != nil { + return err + } + + if err := fam.internal.SaveAndApplyConfig(ctx, config); err != nil { + // An error in the internal Alertmanager shouldn't make the whole operation fail. + // We're replicating writes in the internal Alertmanager just for comparing and in case we need to roll back. + fam.log.Error("Error applying config to the internal Alertmanager", "err", err) + } return nil } @@ -49,6 +60,8 @@ func (fam *RemotePrimaryForkedAlertmanager) SaveAndApplyDefaultConfig(ctx contex } if err := fam.internal.SaveAndApplyDefaultConfig(ctx); err != nil { + // An error in the internal Alertmanager shouldn't make the whole operation fail. + // We're replicating writes in the internal Alertmanager just for comparing and in case we need to roll back. fam.log.Error("Error applying the default configuration to the internal Alertmanager", "err", err) } return nil