vmalert: do not modify notifiers in CR

AndrewChubatiuk · AndrewChubatiuk · commit 05ef46b337c1 · 2025-10-24T16:25:56.000+03:00
diff --git a/api/operator/v1beta1/vmalert_types.go b/api/operator/v1beta1/vmalert_types.go
@@ -72,18 +72,18 @@ type VMAlertSpec struct {
 
 	// Notifier prometheus alertmanager endpoint spec. Required at least one of notifier or notifiers when there are alerting rules. e.g. http://127.0.0.1:9093
 	// If specified both notifier and notifiers, notifier will be added as last element to notifiers.
-	// only one of notifier options could be chosen: notifierConfigRef or notifiers +  notifier
+	// only one of notifier options could be chosen: notifierConfigRef or notifiers + notifier
 	// +optional
 	Notifier *VMAlertNotifierSpec `json:"notifier,omitempty"`
 
 	// Notifiers prometheus alertmanager endpoints. Required at least one of notifier or notifiers when there are alerting rules. e.g. http://127.0.0.1:9093
 	// If specified both notifier and notifiers, notifier will be added as last element to notifiers.
-	// only one of notifier options could be chosen: notifierConfigRef or notifiers +  notifier
+	// only one of notifier options could be chosen: notifierConfigRef or notifiers + notifier
 	// +optional
 	Notifiers []VMAlertNotifierSpec `json:"notifiers,omitempty"`
 
 	// NotifierConfigRef reference for secret with notifier configuration for vmalert
-	// only one of notifier options could be chosen: notifierConfigRef or notifiers +  notifier
+	// only one of notifier options could be chosen: notifierConfigRef or notifiers + notifier
 	// +optional
 	NotifierConfigRef *corev1.SecretKeySelector `json:"notifierConfigRef,omitempty"`
 
diff --git a/config/crd/overlay/crd.yaml b/config/crd/overlay/crd.yaml
@@ -19582,7 +19582,7 @@ spec:
                 description: |-
                   Notifier prometheus alertmanager endpoint spec. Required at least one of notifier or notifiers when there are alerting rules. e.g. http://127.0.0.1:9093
                   If specified both notifier and notifiers, notifier will be added as last element to notifiers.
-                  only one of notifier options could be chosen: notifierConfigRef or notifiers +  notifier
+                  only one of notifier options could be chosen: notifierConfigRef or notifiers + notifier
                 properties:
                   basicAuth:
                     description: BasicAuth allow an endpoint to authenticate over
@@ -19773,7 +19773,7 @@ spec:
               notifierConfigRef:
                 description: |-
                   NotifierConfigRef reference for secret with notifier configuration for vmalert
-                  only one of notifier options could be chosen: notifierConfigRef or notifiers +  notifier
+                  only one of notifier options could be chosen: notifierConfigRef or notifiers + notifier
                 properties:
                   key:
                     description: The key of the secret to select from.  Must be a
@@ -19799,7 +19799,7 @@ spec:
                 description: |-
                   Notifiers prometheus alertmanager endpoints. Required at least one of notifier or notifiers when there are alerting rules. e.g. http://127.0.0.1:9093
                   If specified both notifier and notifiers, notifier will be added as last element to notifiers.
-                  only one of notifier options could be chosen: notifierConfigRef or notifiers +  notifier
+                  only one of notifier options could be chosen: notifierConfigRef or notifiers + notifier
                 items:
                   description: VMAlertNotifierSpec defines the notifier url for sending
                     information about alerts
diff --git a/docs/api.md b/docs/api.md
@@ -3642,9 +3642,9 @@ Appears in: [VMAlert](#vmalert)
 | managedMetadata<a href="#vmalertspec-managedmetadata" id="vmalertspec-managedmetadata">#</a><br/>_[ManagedObjectsMetadata](#managedobjectsmetadata)_ | _(Required)_<br/>ManagedMetadata defines metadata that will be added to the all objects<br />created by operator for the given CustomResource |
 | minReadySeconds<a href="#vmalertspec-minreadyseconds" id="vmalertspec-minreadyseconds">#</a><br/>_integer_ | _(Optional)_<br/>MinReadySeconds defines a minimum number of seconds to wait before starting update next pod<br />if previous in healthy state<br />Has no effect for VLogs and VMSingle |
 | nodeSelector<a href="#vmalertspec-nodeselector" id="vmalertspec-nodeselector">#</a><br/>_object (keys:string, values:string)_ | _(Optional)_<br/>NodeSelector Define which Nodes the Pods are scheduled on. |
-| notifier<a href="#vmalertspec-notifier" id="vmalertspec-notifier">#</a><br/>_[VMAlertNotifierSpec](#vmalertnotifierspec)_ | _(Optional)_<br/>Notifier prometheus alertmanager endpoint spec. Required at least one of notifier or notifiers when there are alerting rules. e.g. http://127.0.0.1:9093<br />If specified both notifier and notifiers, notifier will be added as last element to notifiers.<br />only one of notifier options could be chosen: notifierConfigRef or notifiers +  notifier |
-| notifierConfigRef<a href="#vmalertspec-notifierconfigref" id="vmalertspec-notifierconfigref">#</a><br/>_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#secretkeyselector-v1-core)_ | _(Optional)_<br/>NotifierConfigRef reference for secret with notifier configuration for vmalert<br />only one of notifier options could be chosen: notifierConfigRef or notifiers +  notifier |
-| notifiers<a href="#vmalertspec-notifiers" id="vmalertspec-notifiers">#</a><br/>_[VMAlertNotifierSpec](#vmalertnotifierspec) array_ | _(Optional)_<br/>Notifiers prometheus alertmanager endpoints. Required at least one of notifier or notifiers when there are alerting rules. e.g. http://127.0.0.1:9093<br />If specified both notifier and notifiers, notifier will be added as last element to notifiers.<br />only one of notifier options could be chosen: notifierConfigRef or notifiers +  notifier |
+| notifier<a href="#vmalertspec-notifier" id="vmalertspec-notifier">#</a><br/>_[VMAlertNotifierSpec](#vmalertnotifierspec)_ | _(Optional)_<br/>Notifier prometheus alertmanager endpoint spec. Required at least one of notifier or notifiers when there are alerting rules. e.g. http://127.0.0.1:9093<br />If specified both notifier and notifiers, notifier will be added as last element to notifiers.<br />only one of notifier options could be chosen: notifierConfigRef or notifiers + notifier |
+| notifierConfigRef<a href="#vmalertspec-notifierconfigref" id="vmalertspec-notifierconfigref">#</a><br/>_[SecretKeySelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#secretkeyselector-v1-core)_ | _(Optional)_<br/>NotifierConfigRef reference for secret with notifier configuration for vmalert<br />only one of notifier options could be chosen: notifierConfigRef or notifiers + notifier |
+| notifiers<a href="#vmalertspec-notifiers" id="vmalertspec-notifiers">#</a><br/>_[VMAlertNotifierSpec](#vmalertnotifierspec) array_ | _(Optional)_<br/>Notifiers prometheus alertmanager endpoints. Required at least one of notifier or notifiers when there are alerting rules. e.g. http://127.0.0.1:9093<br />If specified both notifier and notifiers, notifier will be added as last element to notifiers.<br />only one of notifier options could be chosen: notifierConfigRef or notifiers + notifier |
 | paused<a href="#vmalertspec-paused" id="vmalertspec-paused">#</a><br/>_boolean_ | _(Optional)_<br/>Paused If set to true all actions on the underlying managed objects are not<br />going to be performed, except for delete actions. |
 | podDisruptionBudget<a href="#vmalertspec-poddisruptionbudget" id="vmalertspec-poddisruptionbudget">#</a><br/>_[EmbeddedPodDisruptionBudgetSpec](#embeddedpoddisruptionbudgetspec)_ | _(Optional)_<br/>PodDisruptionBudget created by operator |
 | podMetadata<a href="#vmalertspec-podmetadata" id="vmalertspec-podmetadata">#</a><br/>_[EmbeddedObjectMetadata](#embeddedobjectmetadata)_ | _(Required)_<br/>PodMetadata configures Labels and Annotations which are propagated to the VMAlert pods. |
diff --git a/internal/controller/operator/factory/vmalert/vmalert.go b/internal/controller/operator/factory/vmalert/vmalert.go
@@ -21,7 +21,6 @@ import (
 	"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/build"
 	"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/finalize"
 	"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/k8stools"
-	"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger"
 	"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/reconcile"
 )
 
@@ -85,8 +84,8 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAlert, rclient client.C
 	if cr.ParsedLastAppliedSpec != nil {
 		prevCR = cr.DeepCopy()
 		prevCR.Spec = *cr.ParsedLastAppliedSpec
-		if err := discoverNotifierIfNeeded(ctx, rclient, prevCR); err != nil {
-			logger.WithContext(ctx).Error(err, "cannot discover notifiers for prev spec")
+		if err := discoverNotifiersIfNeeded(ctx, rclient, prevCR); err != nil {
+			return fmt.Errorf("cannot discover notifiers for prev spec: %w", err)
 		}
 
 	}
@@ -102,8 +101,8 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAlert, rclient client.C
 			return fmt.Errorf("failed create service account: %w", err)
 		}
 	}
-	if err := discoverNotifierIfNeeded(ctx, rclient, cr); err != nil {
-		return fmt.Errorf("cannot discover additional notifiers: %w", err)
+	if err := discoverNotifiersIfNeeded(ctx, rclient, cr); err != nil {
+		return fmt.Errorf("cannot discover notifiers for new spec: %w", err)
 	}
 
 	ac := getAssetsCache(ctx, rclient, cr)
@@ -710,7 +709,7 @@ func buildConfigReloaderContainer(cr *vmv1beta1.VMAlert, ruleConfigMapNames []st
 	return configReloaderContainer
 }
 
-func discoverNotifierIfNeeded(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAlert) error {
+func discoverNotifiersIfNeeded(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAlert) error {
 	var additionalNotifiers []vmv1beta1.VMAlertNotifierSpec
 
 	if cr.Spec.Notifier != nil {
@@ -727,23 +726,19 @@ func discoverNotifierIfNeeded(ctx context.Context, rclient client.Client, cr *vm
 			continue
 		}
 		// discover alertmanagers
-		var ams vmv1beta1.VMAlertmanagerList
-		amListOpts, err := n.Selector.AsListOptions()
+		o, err := n.Selector.AsListOptions()
 		if err != nil {
 			return fmt.Errorf("cannot convert notifier selector as ListOptions: %w", err)
 		}
-		if err := k8stools.ListObjectsByNamespace(ctx, rclient, config.MustGetWatchNamespaces(), func(objects *vmv1beta1.VMAlertmanagerList) {
-			ams.Items = append(ams.Items, objects.Items...)
-		}, amListOpts); err != nil {
-			return fmt.Errorf("cannot list alertmanager with discovery selector: %w", err)
-		}
-
-		for _, item := range ams.Items {
-			if !item.DeletionTimestamp.IsZero() || (n.Selector.Namespace != nil && !n.Selector.Namespace.IsMatch(&item)) {
-				continue
+		if err := k8stools.ListObjectsByNamespace(ctx, rclient, config.MustGetWatchNamespaces(), func(l *vmv1beta1.VMAlertmanagerList) {
+			for _, item := range l.Items {
+				if !item.DeletionTimestamp.IsZero() || (n.Selector.Namespace != nil && !n.Selector.Namespace.IsMatch(&item)) {
+					continue
+				}
+				additionalNotifiers = append(additionalNotifiers, item.AsNotifiers()...)
 			}
-			dsc := item.AsNotifiers()
-			additionalNotifiers = append(additionalNotifiers, dsc...)
+		}, o); err != nil {
+			return fmt.Errorf("cannot list alertmanager with discovery selector: %w", err)
 		}
 	}
 	cr.Spec.Notifiers = cr.Spec.Notifiers[:cnt]
diff --git a/internal/controller/operator/factory/vmalert/vmalert_test.go b/internal/controller/operator/factory/vmalert/vmalert_test.go

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,6 @@ import (`
`21`	`21`	`"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/build"`
`22`	`22`	`"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/finalize"`
`23`	`23`	`"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/k8stools"`
`24`		`- "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/logger"`
`25`	`24`	`"github.com/VictoriaMetrics/operator/internal/controller/operator/factory/reconcile"`
`26`	`25`	`)`
`27`	`26`
`@@ -85,8 +84,8 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAlert, rclient client.C`
`85`	`84`	`if cr.ParsedLastAppliedSpec != nil {`
`86`	`85`	`prevCR = cr.DeepCopy()`
`87`	`86`	`prevCR.Spec = *cr.ParsedLastAppliedSpec`
`88`		`- if err := discoverNotifierIfNeeded(ctx, rclient, prevCR); err != nil {`
`89`		`- logger.WithContext(ctx).Error(err, "cannot discover notifiers for prev spec")`
	`87`	`+ if err := discoverNotifiersIfNeeded(ctx, rclient, prevCR); err != nil {`
	`88`	`+ return fmt.Errorf("cannot discover notifiers for prev spec: %w", err)`
`90`	`89`	`}`
`91`	`90`
`92`	`91`	`}`
`@@ -102,8 +101,8 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMAlert, rclient client.C`
`102`	`101`	`return fmt.Errorf("failed create service account: %w", err)`
`103`	`102`	`}`
`104`	`103`	`}`
`105`		`- if err := discoverNotifierIfNeeded(ctx, rclient, cr); err != nil {`
`106`		`- return fmt.Errorf("cannot discover additional notifiers: %w", err)`
	`104`	`+ if err := discoverNotifiersIfNeeded(ctx, rclient, cr); err != nil {`
	`105`	`+ return fmt.Errorf("cannot discover notifiers for new spec: %w", err)`
`107`	`106`	`}`
`108`	`107`
`109`	`108`	`ac := getAssetsCache(ctx, rclient, cr)`
`@@ -710,7 +709,7 @@ func buildConfigReloaderContainer(cr *vmv1beta1.VMAlert, ruleConfigMapNames []st`
`710`	`709`	`return configReloaderContainer`
`711`	`710`	`}`
`712`	`711`
`713`		`-func discoverNotifierIfNeeded(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAlert) error {`
	`712`	`+func discoverNotifiersIfNeeded(ctx context.Context, rclient client.Client, cr *vmv1beta1.VMAlert) error {`
`714`	`713`	`var additionalNotifiers []vmv1beta1.VMAlertNotifierSpec`
`715`	`714`
`716`	`715`	`if cr.Spec.Notifier != nil {`
`@@ -727,23 +726,19 @@ func discoverNotifierIfNeeded(ctx context.Context, rclient client.Client, cr *vm`
`727`	`726`	`continue`
`728`	`727`	`}`
`729`	`728`	`// discover alertmanagers`
`730`		`- var ams vmv1beta1.VMAlertmanagerList`
`731`		`- amListOpts, err := n.Selector.AsListOptions()`
	`729`	`+ o, err := n.Selector.AsListOptions()`
`732`	`730`	`if err != nil {`
`733`	`731`	`return fmt.Errorf("cannot convert notifier selector as ListOptions: %w", err)`
`734`	`732`	`}`
`735`		`- if err := k8stools.ListObjectsByNamespace(ctx, rclient, config.MustGetWatchNamespaces(), func(objects *vmv1beta1.VMAlertmanagerList) {`
`736`		`- ams.Items = append(ams.Items, objects.Items...)`
`737`		`- }, amListOpts); err != nil {`
`738`		`- return fmt.Errorf("cannot list alertmanager with discovery selector: %w", err)`
`739`		`- }`
`740`		`-`
`741`		`- for _, item := range ams.Items {`
`742`		`- if !item.DeletionTimestamp.IsZero() \|\| (n.Selector.Namespace != nil && !n.Selector.Namespace.IsMatch(&item)) {`
`743`		`- continue`
	`733`	`+ if err := k8stools.ListObjectsByNamespace(ctx, rclient, config.MustGetWatchNamespaces(), func(l *vmv1beta1.VMAlertmanagerList) {`
	`734`	`+ for _, item := range l.Items {`
	`735`	`+ if !item.DeletionTimestamp.IsZero() \|\| (n.Selector.Namespace != nil && !n.Selector.Namespace.IsMatch(&item)) {`
	`736`	`+ continue`
	`737`	`+ }`
	`738`	`+ additionalNotifiers = append(additionalNotifiers, item.AsNotifiers()...)`
`744`	`739`	`}`
`745`		`- dsc := item.AsNotifiers()`
`746`		`- additionalNotifiers = append(additionalNotifiers, dsc...)`
	`740`	`+ }, o); err != nil {`
	`741`	`+ return fmt.Errorf("cannot list alertmanager with discovery selector: %w", err)`
`747`	`742`	`}`
`748`	`743`	`}`
`749`	`744`	`cr.Spec.Notifiers = cr.Spec.Notifiers[:cnt]`