diff --git a/pkg/services/ngalert/remote/forked_alertmanager_test.go b/pkg/services/ngalert/remote/forked_alertmanager_test.go index 63221c45daa..e78a0561dea 100644 --- a/pkg/services/ngalert/remote/forked_alertmanager_test.go +++ b/pkg/services/ngalert/remote/forked_alertmanager_test.go @@ -3,6 +3,7 @@ package remote import ( "context" "errors" + "fmt" "testing" "time" @@ -474,6 +475,32 @@ func TestForkedAlertmanager_ModeRemotePrimary(t *testing.T) { id, err = forked.CreateSilence(ctx, testSilence) require.NoError(tt, err) require.Equal(tt, expID, id) + + // If the silence ID changes, the internal Alertmanager should attempt to expire the old silence. + newID := "new" + internal, remote, forked = genTestAlertmanagers(tt, modeRemotePrimary) + remote.EXPECT().CreateSilence(mock.Anything, mock.Anything).Return(newID, nil).Once() + internal.EXPECT().DeleteSilence(mock.Anything, mock.Anything).Return(nil).Once() + // If internal.CreateSilence() returns a new id, it should be ignored. + internal.EXPECT().CreateSilence(mock.Anything, mock.Anything).Return("random-id", nil).Once() + id, err = forked.CreateSilence(ctx, testSilence) + require.NoError(tt, err) + require.Equal(tt, newID, testSilence.ID) + require.Equal(tt, newID, id) + + // Restore original ID. + testSilence.ID = expID + + // An error attempting to delete a silence in the internal Alertmanager not be returned. + internal, remote, forked = genTestAlertmanagers(tt, modeRemotePrimary) + remote.EXPECT().CreateSilence(mock.Anything, mock.Anything).Return(newID, nil).Once() + internal.EXPECT().DeleteSilence(mock.Anything, mock.Anything).Return(fmt.Errorf("test error")).Once() + // If internal.CreateSilence() returns a new id, it should be ignored. + internal.EXPECT().CreateSilence(mock.Anything, mock.Anything).Return("random-id", nil).Once() + id, err = forked.CreateSilence(ctx, testSilence) + require.NoError(tt, err) + require.Equal(tt, newID, testSilence.ID) + require.Equal(tt, newID, id) }) t.Run("DeleteSilence", func(tt *testing.T) { diff --git a/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go b/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go index be783deef4e..1577f3a9470 100644 --- a/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go +++ b/pkg/services/ngalert/remote/remote_primary_forked_alertmanager.go @@ -2,6 +2,7 @@ package remote import ( "context" + "errors" "fmt" alertingNotify "github.com/grafana/alerting/notify" @@ -72,16 +73,30 @@ func (fam *RemotePrimaryForkedAlertmanager) GetStatus(ctx context.Context) (apim } func (fam *RemotePrimaryForkedAlertmanager) CreateSilence(ctx context.Context, silence *apimodels.PostableSilence) (string, error) { - uid, err := fam.remote.CreateSilence(ctx, silence) + originalID := silence.ID + id, err := fam.remote.CreateSilence(ctx, silence) if err != nil { return "", err } - silence.ID = uid + if originalID != "" && originalID != id { + // ID has changed, expire the old silence before creating a new one. + if err := fam.internal.DeleteSilence(ctx, originalID); err != nil { + if errors.Is(err, alertingNotify.ErrSilenceNotFound) { + // This can happen if the silence was created in the remote AM without using the Grafana UI + // in remote primary mode, or if the silence failed to be replicated in the internal AM. + fam.log.Warn("Failed to delete silence in the internal Alertmanager", "err", err, "id", originalID) + } else { + fam.log.Error("Failed to delete silence in the internal Alertmanager", "err", err, "id", originalID) + } + } + } + + silence.ID = id if _, err := fam.internal.CreateSilence(ctx, silence); err != nil { fam.log.Error("Error creating silence in the internal Alertmanager", "err", err, "silence", silence) } - return uid, nil + return id, nil } func (fam *RemotePrimaryForkedAlertmanager) DeleteSilence(ctx context.Context, id string) error {