When existing alert grows and doesn't fit the size limit anymore, it is rejected.

Signed-off-by: Peter Štibraný <[email protected]>

Author: pstibrany

pkg/alertmanager/alertmanager.go

@@ -646,26 +646,26 @@ func (a *alertsLimiter) PreStore(alert *types.Alert, existing bool) error {
 		return nil
 	}
 
-	// We allow existing alerts in with no checks, as we want to make sure that alerts already in
-	// store can be resolved.
-	if existing {
-		return nil
-	}
+	fp := alert.Fingerprint()
 
 	countLimit := a.limits.AlertmanagerMaxAlertsCount(a.tenant)
 	sizeLimit := a.limits.AlertmanagerMaxAlertsSizeBytes(a.tenant)
 
-	newSize := alertSize(alert.Alert)
+	sizeDiff := alertSize(alert.Alert)
 
 	a.mx.Lock()
 	defer a.mx.Unlock()
 
-	if countLimit > 0 && (a.count+1) > countLimit {
+	if !existing && countLimit > 0 && (a.count+1) > countLimit {
 		a.failureCounter.Inc()
 		return fmt.Errorf(errTooManyAlerts, countLimit)
 	}
 
-	if sizeLimit > 0 && (a.totalSize+newSize) > sizeLimit {
+	if existing {
+		sizeDiff -= a.sizes[fp]
+	}
+
+	if sizeLimit > 0 && (a.totalSize+sizeDiff) > sizeLimit {
 		a.failureCounter.Inc()
 		return fmt.Errorf(errAlertsTooBig, sizeLimit)
 	}

pkg/alertmanager/alertmanager_test.go

@@ -155,22 +155,32 @@ func TestAlertsLimiterWithNoLimits(t *testing.T) {
 }
 
 func TestAlertsLimiterWithCountLimit(t *testing.T) {
+	alert2WithMoreAnnotations := alert2
+	alert2WithMoreAnnotations.Annotations = model.LabelSet{"job": "test", "cluster": "prod", "new": "super-long-annotation"}
+	alert2WithMoreAnnotationsSize := alertSize(alert2WithMoreAnnotations)
+
 	ops := []callbackOp{
 		{alert: &types.Alert{Alert: alert1}, existing: false, expectedCount: 1, expectedTotalSize: alert1Size},
 		{alert: &types.Alert{Alert: alert2}, existing: false, expectedInsertError: fmt.Errorf(errTooManyAlerts, 1), expectedCount: 1, expectedTotalSize: alert1Size},
 		{alert: &types.Alert{Alert: alert1}, delete: true, expectedCount: 0, expectedTotalSize: 0},
 
 		{alert: &types.Alert{Alert: alert2}, existing: false, expectedCount: 1, expectedTotalSize: alert2Size},
+		// Update of existing alert works -- doesn't change count.
+		{alert: &types.Alert{Alert: alert2WithMoreAnnotations}, existing: true, expectedCount: 1, expectedTotalSize: alert2WithMoreAnnotationsSize},
 		{alert: &types.Alert{Alert: alert2}, delete: true, expectedCount: 0, expectedTotalSize: 0},
 	}
 
 	testLimiter(t, &mockAlertManagerLimits{maxAlertsCount: 1}, ops)
 }
 
 func TestAlertsLimiterWithSizeLimit(t *testing.T) {
+	alert2WithMoreAnnotations := alert2
+	alert2WithMoreAnnotations.Annotations = model.LabelSet{"job": "test", "cluster": "prod", "new": "super-long-annotation"}
+
 	ops := []callbackOp{
 		{alert: &types.Alert{Alert: alert1}, existing: false, expectedCount: 1, expectedTotalSize: alert1Size},
 		{alert: &types.Alert{Alert: alert2}, existing: false, expectedInsertError: fmt.Errorf(errAlertsTooBig, alert2Size), expectedCount: 1, expectedTotalSize: alert1Size},
+		{alert: &types.Alert{Alert: alert2WithMoreAnnotations}, existing: false, expectedInsertError: fmt.Errorf(errAlertsTooBig, alert2Size), expectedCount: 1, expectedTotalSize: alert1Size},
 		{alert: &types.Alert{Alert: alert1}, delete: true, expectedCount: 0, expectedTotalSize: 0},
 
 		{alert: &types.Alert{Alert: alert2}, existing: false, expectedCount: 1, expectedTotalSize: alert2Size},
@@ -183,6 +193,25 @@ func TestAlertsLimiterWithSizeLimit(t *testing.T) {
 	testLimiter(t, &mockAlertManagerLimits{maxAlertsSizeBytes: alert2Size}, ops)
 }
 
+func TestAlertsLimiterWithSizeLimitAndAnnotationUpdate(t *testing.T) {
+	alert2WithMoreAnnotations := alert2
+	alert2WithMoreAnnotations.Annotations = model.LabelSet{"job": "test", "cluster": "prod", "new": "super-long-annotation"}
+	alert2WithMoreAnnotationsSize := alertSize(alert2WithMoreAnnotations)
+
+	// Updating alert with larger annotation that goes over the size limit fails.
+	testLimiter(t, &mockAlertManagerLimits{maxAlertsSizeBytes: alert2Size}, []callbackOp{
+		{alert: &types.Alert{Alert: alert2}, existing: false, expectedCount: 1, expectedTotalSize: alert2Size},
+		{alert: &types.Alert{Alert: alert2WithMoreAnnotations}, existing: true, expectedInsertError: fmt.Errorf(errAlertsTooBig, alert2Size), expectedCount: 1, expectedTotalSize: alert2Size},
+	})
+
+	// Updating alert with larger annotations in the limit works fine.
+	testLimiter(t, &mockAlertManagerLimits{maxAlertsSizeBytes: alert2WithMoreAnnotationsSize}, []callbackOp{
+		{alert: &types.Alert{Alert: alert2}, existing: false, expectedCount: 1, expectedTotalSize: alert2Size},
+		{alert: &types.Alert{Alert: alert2WithMoreAnnotations}, existing: true, expectedCount: 1, expectedTotalSize: alert2WithMoreAnnotationsSize},
+		{alert: &types.Alert{Alert: alert2}, existing: true, expectedCount: 1, expectedTotalSize: alert2Size},
+	})
+}
+
 // testLimiter sends sequence of alerts to limiter, and checks if limiter updated reacted correctly.
 func testLimiter(t *testing.T, limits Limits, ops []callbackOp) {
 	reg := prometheus.NewPedanticRegistry()