Alerting: Implement ApplyConfig in the forked Alertmanager (#78684)

* Alerting: Add a sync interval for ApplyConfig in remote secondary mode

* remove out of scope code

* remove parentheses after CleanUp for consistency in test comments

* Add comment to ApplyConfig

pkg/services/ngalert/remote/forked_alertmanager_test.go

@@ -5,7 +5,9 @@ import (
 	"errors"
 	"testing"
 
+	"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/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/notifier"
 	"github.com/grafana/grafana/pkg/services/ngalert/notifier/alertmanager_mock"
 	"github.com/stretchr/testify/mock"
@@ -21,6 +23,27 @@ func TestForkedAlertmanager_ModeRemoteSecondary(t *testing.T) {
 	ctx := context.Background()
 	expErr := errors.New("test error")
 
+	t.Run("ApplyConfig", func(tt *testing.T) {
+		// ApplyConfig should be called in both Alertmanagers.
+		internal, remote, forked := genTestAlertmanagers(tt, modeRemoteSecondary)
+		internal.EXPECT().ApplyConfig(ctx, mock.Anything).Return(nil).Twice()
+		remote.EXPECT().ApplyConfig(ctx, mock.Anything).Return(nil).Twice()
+		require.NoError(tt, forked.ApplyConfig(ctx, &models.AlertConfiguration{}))
+		require.NoError(tt, forked.ApplyConfig(ctx, &models.AlertConfiguration{}))
+
+		// An error in the remote Alertmanager should not be returned.
+		internal, remote, forked = genTestAlertmanagers(tt, modeRemoteSecondary)
+		internal.EXPECT().ApplyConfig(ctx, mock.Anything).Return(nil).Once()
+		remote.EXPECT().ApplyConfig(ctx, mock.Anything).Return(expErr).Once()
+		require.NoError(tt, forked.ApplyConfig(ctx, &models.AlertConfiguration{}))
+
+		// An error in the internal Alertmanager should be returned.
+		internal, remote, forked = genTestAlertmanagers(tt, modeRemoteSecondary)
+		internal.EXPECT().ApplyConfig(ctx, mock.Anything).Return(expErr).Once()
+		remote.EXPECT().ApplyConfig(ctx, mock.Anything).Return(nil).Once()
+		require.Error(tt, forked.ApplyConfig(ctx, &models.AlertConfiguration{}), expErr)
+	})
+
 	t.Run("SaveAndApplyConfig", func(tt *testing.T) {
 		// SaveAndApplyConfig should only be called on the remote Alertmanager.
 		// State and configuration are updated on an interval.
@@ -243,16 +266,16 @@ func TestForkedAlertmanager_ModeRemoteSecondary(t *testing.T) {
 	})
 
 	t.Run("CleanUp", func(tt *testing.T) {
-		// CleanUp() should be called only in the internal Alertmanager,
+		// CleanUp should be called only in the internal Alertmanager,
 		// there's no cleanup to do in the remote one.
-		internal, _, forked := genTestAlertmanagers(tt, modeRemotePrimary)
+		internal, _, forked := genTestAlertmanagers(tt, modeRemoteSecondary)
 		internal.EXPECT().CleanUp().Once()
 		forked.CleanUp()
 	})
 
 	t.Run("StopAndWait", func(tt *testing.T) {
 		// StopAndWait should be called on both Alertmanagers.
-		internal, remote, forked := genTestAlertmanagers(tt, modeRemotePrimary)
+		internal, remote, forked := genTestAlertmanagers(tt, modeRemoteSecondary)
 		internal.EXPECT().StopAndWait().Once()
 		remote.EXPECT().StopAndWait().Once()
 		forked.StopAndWait()
@@ -475,7 +498,7 @@ func TestForkedAlertmanager_ModeRemotePrimary(t *testing.T) {
 	})
 
 	t.Run("CleanUp", func(tt *testing.T) {
-		// CleanUp() should be called only in the internal Alertmanager,
+		// CleanUp should be called only in the internal Alertmanager,
 		// there's no cleanup to do in the remote one.
 		internal, _, forked := genTestAlertmanagers(tt, modeRemotePrimary)
 		internal.EXPECT().CleanUp().Once()
@@ -515,7 +538,7 @@ func genTestAlertmanagers(t *testing.T, mode int) (*alertmanager_mock.Alertmanag
 	remote := alertmanager_mock.NewAlertmanagerMock(t)
 
 	if mode == modeRemoteSecondary {
-		return internal, remote, NewRemoteSecondaryForkedAlertmanager(internal, remote)
+		return internal, remote, NewRemoteSecondaryForkedAlertmanager(log.NewNopLogger(), internal, remote)
 	}
 	return internal, remote, NewRemotePrimaryForkedAlertmanager(internal, remote)
 }

pkg/services/ngalert/remote/remote_secondary_forked_alertmanager.go

@@ -3,25 +3,34 @@ package remote
 import (
 	"context"
 
+	"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/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/notifier"
 )
 
 type RemoteSecondaryForkedAlertmanager struct {
+	log log.Logger
+
 	internal notifier.Alertmanager
 	remote   notifier.Alertmanager
 }
 
-func NewRemoteSecondaryForkedAlertmanager(internal, remote notifier.Alertmanager) *RemoteSecondaryForkedAlertmanager {
+func NewRemoteSecondaryForkedAlertmanager(l log.Logger, internal, remote notifier.Alertmanager) *RemoteSecondaryForkedAlertmanager {
 	return &RemoteSecondaryForkedAlertmanager{
+		log:      l,
 		internal: internal,
 		remote:   remote,
 	}
 }
 
+// ApplyConfig will only log errors for the remote Alertmanager and ensure we delegate the call to the internal Alertmanager.
+// We don't care about errors in the remote Alertmanager in remote secondary mode.
 func (fam *RemoteSecondaryForkedAlertmanager) ApplyConfig(ctx context.Context, config *models.AlertConfiguration) error {
-	return nil
+	if err := fam.remote.ApplyConfig(ctx, config); err != nil {
+		fam.log.Error("Error applying config to the remote Alertmanager", "err", err)
+	}
+	return fam.internal.ApplyConfig(ctx, config)
 }
 
 // SaveAndApplyConfig is only called on the internal Alertmanager when running in remote secondary mode.