operator: fix object selectors behavior

based on rules [here](https://docs.victoriametrics.com/operator/resources/vmagent/#scraping)

1. fix VMScrapeConfig reconcile: VMScrapeConfig is ignored in it's own reconcile when vmagentSpec.selectAllByDefault=false, since vmagent.IsUnmanaged() didn't include checks about VMScrapeConfig. But VMScrapeConfig could be pick up in vmagent's reconcile.
2. fix vmscrapeConfigNamespaceSelector in vmagent's reconcile, address scrapeConfigSelectors don't work as expected. VictoriaMetrics#7687
3. fix object selector when xxxNamespaceSelector matches nothing but selectAllByDefault=true. Before, it ignores the xxxNamespaceSelector that matches nothing.
4. limit the IsUnmanaged check scope in reconcile for each scrape CR.

api/operator/v1beta1/vmagent_types.go

@@ -166,7 +166,7 @@ type VMAgentSpec struct {
 	// If both nil - behaviour controlled by selectAllByDefault
 	// +optional
 	NodeScrapeNamespaceSelector *metav1.LabelSelector `json:"nodeScrapeNamespaceSelector,omitempty"`
-	// StaticScrapeSelector defines PodScrapes to be selected for target discovery.
+	// StaticScrapeSelector defines VMStaticScrape to be selected for target discovery.
 	// Works in combination with NamespaceSelector.
 	// If both nil - match everything.
 	// NamespaceSelector nil - only objects at VMAgent namespace.
@@ -636,7 +636,7 @@ func (cr VMAgent) ProbeNeedLiveness() bool {
 	return true
 }
 
-// IsUnmanaged checks if object should managed any  config objects
+// IsUnmanaged checks if object should managed any config objects
 func (cr *VMAgent) IsUnmanaged() bool {
 	// fast path
 	if cr.Spec.IngestOnlyMode {
@@ -647,9 +647,70 @@ func (cr *VMAgent) IsUnmanaged() bool {
 		cr.Spec.ServiceScrapeSelector == nil && cr.Spec.ServiceScrapeNamespaceSelector == nil &&
 		cr.Spec.PodScrapeSelector == nil && cr.Spec.PodScrapeNamespaceSelector == nil &&
 		cr.Spec.ProbeSelector == nil && cr.Spec.ProbeNamespaceSelector == nil &&
+		cr.Spec.StaticScrapeSelector == nil && cr.Spec.StaticScrapeNamespaceSelector == nil &&
+		cr.Spec.ScrapeConfigSelector == nil && cr.Spec.ScrapeConfigNamespaceSelector == nil
+}
+
+// IsNodeScrapeUnmanaged checks if vmagent should managed any VMNodeScrape objects
+func (cr *VMAgent) IsNodeScrapeUnmanaged() bool {
+	// fast path
+	if cr.Spec.IngestOnlyMode {
+		return true
+	}
+	return !cr.Spec.SelectAllByDefault &&
+		cr.Spec.NodeScrapeSelector == nil && cr.Spec.NodeScrapeNamespaceSelector == nil
+}
+
+// IsServiceScrapeUnmanaged checks if vmagent should managed any VMServiceScrape objects
+func (cr *VMAgent) IsServiceScrapeUnmanaged() bool {
+	// fast path
+	if cr.Spec.IngestOnlyMode {
+		return true
+	}
+	return !cr.Spec.SelectAllByDefault &&
+		cr.Spec.ServiceScrapeSelector == nil && cr.Spec.ServiceScrapeNamespaceSelector == nil
+}
+
+// IsUnmanaged checks if vmagent should managed any VMPodScrape objects
+func (cr *VMAgent) IsPodScrapeUnmanaged() bool {
+	// fast path
+	if cr.Spec.IngestOnlyMode {
+		return true
+	}
+	return !cr.Spec.SelectAllByDefault &&
+		cr.Spec.PodScrapeSelector == nil && cr.Spec.PodScrapeNamespaceSelector == nil
+}
+
+// IsProbeUnmanaged checks if vmagent should managed any VMProbe objects
+func (cr *VMAgent) IsProbeUnmanaged() bool {
+	// fast path
+	if cr.Spec.IngestOnlyMode {
+		return true
+	}
+	return !cr.Spec.SelectAllByDefault &&
+		cr.Spec.ProbeSelector == nil && cr.Spec.ProbeNamespaceSelector == nil
+}
+
+// IsStaticScrapeUnmanaged checks if vmagent should managed any VMStaticScrape objects
+func (cr *VMAgent) IsStaticScrapeUnmanaged() bool {
+	// fast path
+	if cr.Spec.IngestOnlyMode {
+		return true
+	}
+	return !cr.Spec.SelectAllByDefault &&
 		cr.Spec.StaticScrapeSelector == nil && cr.Spec.StaticScrapeNamespaceSelector == nil
 }
 
+// IsScrapeConfigUnmanaged checks if vmagent should managed any VMScrapeConfig objects
+func (cr *VMAgent) IsScrapeConfigUnmanaged() bool {
+	// fast path
+	if cr.Spec.IngestOnlyMode {
+		return true
+	}
+	return !cr.Spec.SelectAllByDefault &&
+		cr.Spec.ScrapeConfigSelector == nil && cr.Spec.ScrapeConfigNamespaceSelector == nil
+}
+
 // LastAppliedSpecAsPatch return last applied cluster spec as patch annotation
 func (cr *VMAgent) LastAppliedSpecAsPatch() (client.Patch, error) {
 	data, err := json.Marshal(cr.Spec)
@@ -708,7 +769,6 @@ func (cr *VMAgent) HasAnyStreamAggrRule() bool {
 
 // SetStatusTo changes update status with optional reason of fail
 func (cr *VMAgent) SetUpdateStatusTo(ctx context.Context, r client.Client, status UpdateStatus, maybeErr error) error {
-
 	return updateObjectStatus(ctx, r, &patchStatusOpts[*VMAgent, *VMAgentStatus]{
 		actualStatus: status,
 		cr:           cr,

config/crd/overlay/crd.yaml

@@ -4929,7 +4929,7 @@ spec:
                 type: array
               staticScrapeSelector:
                 description: |-
-                  StaticScrapeSelector defines PodScrapes to be selected for target discovery.
+                  StaticScrapeSelector defines VMStaticScrape to be selected for target discovery.
                   Works in combination with NamespaceSelector.
                   If both nil - match everything.
                   NamespaceSelector nil - only objects at VMAgent namespace.
@@ -20182,6 +20182,12 @@ spec:
                 description: LastSyncError contains error message for unsuccessful
                   config generation
                 type: string
+              observedGeneration:
+                description: |-
+                  ObservedGeneration defines current generation picked by operator for the
+                  reconcile
+                format: int64
+                type: integer
               status:
                 description: Status defines update status of resource
                 type: string
@@ -21261,6 +21267,12 @@ spec:
                 description: LastSyncError contains error message for unsuccessful
                   config generation
                 type: string
+              observedGeneration:
+                description: |-
+                  ObservedGeneration defines current generation picked by operator for the
+                  reconcile
+                format: int64
+                type: integer
               status:
                 description: Status defines update status of resource
                 type: string
@@ -22401,6 +22413,12 @@ spec:
                 description: LastSyncError contains error message for unsuccessful
                   config generation
                 type: string
+              observedGeneration:
+                description: |-
+                  ObservedGeneration defines current generation picked by operator for the
+                  reconcile
+                format: int64
+                type: integer
               status:
                 description: Status defines update status of resource
                 type: string
@@ -26593,6 +26611,12 @@ spec:
                 description: LastSyncError contains error message for unsuccessful
                   config generation
                 type: string
+              observedGeneration:
+                description: |-
+                  ObservedGeneration defines current generation picked by operator for the
+                  reconcile
+                format: int64
+                type: integer
               status:
                 description: Status defines update status of resource
                 type: string
@@ -27685,6 +27709,12 @@ spec:
                 description: LastSyncError contains error message for unsuccessful
                   config generation
                 type: string
+              observedGeneration:
+                description: |-
+                  ObservedGeneration defines current generation picked by operator for the
+                  reconcile
+                format: int64
+                type: integer
               status:
                 description: Status defines update status of resource
                 type: string
@@ -30367,6 +30397,12 @@ spec:
                 description: LastSyncError contains error message for unsuccessful
                   config generation
                 type: string
+              observedGeneration:
+                description: |-
+                  ObservedGeneration defines current generation picked by operator for the
+                  reconcile
+                format: int64
+                type: integer
               status:
                 description: Status defines update status of resource
                 type: string

docs/CHANGELOG.md

@@ -17,6 +17,8 @@ aliases:
 - [vmoperator](https://docs.victoriametrics.com/operator/): bump default version of VictoriaMetrics components to [1.106.1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.106.1).
 - [vmoperator](https://docs.victoriametrics.com/operator/): add new variable `VM_VMSERVICESCRAPEDEFAULT_ENFORCEENDPOINTSLICES` to use `endpointslices` instead of `endpoints` as discovery role for VMServiceScrape when generate scrape config for VMAgent.
 - [vmoperator](https://docs.victoriametrics.com/operator/): adds new flag `loggerJSONFields` to the operator logger configuration. It allows to change json encoder fields. See [this issue](https://github.com/VictoriaMetrics/operator/issues/1157) for details.
+- [vmoperator](https://docs.victoriametrics.com/operator/): fix the behaviors of `vmagentSpec.ScrapeConfigSelector` and `vmagentSpec.scrapeConfigNamespaceSelector` when `vmagentSpec.selectAllByDefault=false`. Previously, the VMScrapeConfig could be ignored.
+- [vmoperator](https://docs.victoriametrics.com/operator/): fix the behaviors of `xxxNamespaceSelector` when `vmagentSpec.selectAllByDefault=true`. See [this doc](https://docs.victoriametrics.com/operator/resources/vmagent/#scraping) for detailed rules.
 - [api](https://docs.victoriametrics.com/operator/api): adds new status field `observedGeneration`. See [this issue](https://github.com/VictoriaMetrics/operator/issues/1155) for details.
 - [api](https://docs.victoriametrics.com/operator/api): unify `updateStatus` field for CRD objects. It replaces `status`, `clusterStatus` and `singleStatus` for `VLogs`, `VMCluster` and `VMSingle` with generic `updateStatus`.
 - [alerts]: added cluster label for multicluster alerts.

docs/api.md

@@ -2696,7 +2696,7 @@ _Appears in:_
 | `statefulStorage` | StatefulStorage configures storage for StatefulSet | _[StorageSpec](#storagespec)_ | false |
 | `staticScrapeNamespaceSelector` | StaticScrapeNamespaceSelector defines Namespaces to be selected for VMStaticScrape discovery.<br />Works in combination with NamespaceSelector.<br />NamespaceSelector nil - only objects at VMAgent namespace.<br />Selector nil - only objects at NamespaceSelector namespaces.<br />If both nil - behaviour controlled by selectAllByDefault | _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#labelselector-v1-meta)_ | false |
 | `staticScrapeRelabelTemplate` | StaticScrapeRelabelTemplate defines relabel config, that will be added to each VMStaticScrape.<br />it's useful for adding specific labels to all targets | _[RelabelConfig](#relabelconfig) array_ | false |
-| `staticScrapeSelector` | StaticScrapeSelector defines PodScrapes to be selected for target discovery.<br />Works in combination with NamespaceSelector.<br />If both nil - match everything.<br />NamespaceSelector nil - only objects at VMAgent namespace.<br />Selector nil - only objects at NamespaceSelector namespaces. | _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#labelselector-v1-meta)_ | false |
+| `staticScrapeSelector` | StaticScrapeSelector defines VMStaticScrape to be selected for target discovery.<br />Works in combination with NamespaceSelector.<br />If both nil - match everything.<br />NamespaceSelector nil - only objects at VMAgent namespace.<br />Selector nil - only objects at NamespaceSelector namespaces. | _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#labelselector-v1-meta)_ | false |
 | `streamAggrConfig` | StreamAggrConfig defines global stream aggregation configuration for VMAgent | _[StreamAggrConfig](#streamaggrconfig)_ | false |
 | `terminationGracePeriodSeconds` | TerminationGracePeriodSeconds period for container graceful termination | _integer_ | false |
 | `tolerations` | Tolerations If specified, the pod's tolerations. | _[Toleration](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#toleration-v1-core) array_ | false |

internal/controller/operator/controllers.go

@@ -199,8 +199,14 @@ func isNamespaceSelectorMatches(ctx context.Context, rclient client.Client, sour
 	return false, nil
 }
 
-func isSelectorsMatchesTargetCRD(ctx context.Context, rclient client.Client, sourceCRD, targetCRD client.Object, selector, namespaceSelector *metav1.LabelSelector) (bool, error) {
-	// check namespace selector
+// isSelectorsMatchesTargetCRD checks if targetCRD matches sourceCRD by selector, namespaceSelector and selectAll.
+// see https://docs.victoriametrics.com/operator/resources/vmagent/#scraping for details
+func isSelectorsMatchesTargetCRD(ctx context.Context, rclient client.Client, sourceCRD, targetCRD client.Object, selector, namespaceSelector *metav1.LabelSelector, selectAll bool) (bool, error) {
+	// selectAll only works when NamespaceSelector and Selector both undefined
+	if selector == nil && namespaceSelector == nil {
+		return selectAll, nil
+	}
+	// check namespace selector, only return when NS not match
 	if isNsMatch, err := isNamespaceSelectorMatches(ctx, rclient, sourceCRD, targetCRD, namespaceSelector); !isNsMatch || err != nil {
 		return isNsMatch, err
 	}

internal/controller/operator/controllers_test.go

@@ -16,6 +16,7 @@ import (
 func TestIsSelectorsMatchesTargetCRD(t *testing.T) {
 	tests := []struct {
 		name              string
+		selectAll         bool
 		sourceCRD         client.Object
 		targetCRD         client.Object
 		selector          *metav1.LabelSelector
@@ -24,11 +25,12 @@ func TestIsSelectorsMatchesTargetCRD(t *testing.T) {
 		isMatch           bool
 	}{
 		{
-			name: "matches cause under same namespace",
+			name:      "match: selectors are nil, selectAll=true",
+			selectAll: true,
 			sourceCRD: &vmv1beta1.VMRule{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "rule",
-					Namespace: "default",
+					Namespace: "n1",
 					Labels: map[string]string{
 						"app": "target-app",
 					},
@@ -37,7 +39,7 @@ func TestIsSelectorsMatchesTargetCRD(t *testing.T) {
 			targetCRD: &vmv1beta1.VMAlert{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "test-vmalert",
-					Namespace: "default",
+					Namespace: "n2",
 				},
 				Spec: vmv1beta1.VMAlertSpec{
 					RuleSelector:          &metav1.LabelSelector{},
@@ -49,7 +51,8 @@ func TestIsSelectorsMatchesTargetCRD(t *testing.T) {
 			isMatch:           true,
 		},
 		{
-			name: "not match",
+			name:      "not match: selectors are nil, selectAll=false",
+			selectAll: false,
 			sourceCRD: &vmv1beta1.VMRule{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "rule",
@@ -62,7 +65,7 @@ func TestIsSelectorsMatchesTargetCRD(t *testing.T) {
 			targetCRD: &vmv1beta1.VMAlert{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "test-vmalert",
-					Namespace: "vm-stack",
+					Namespace: "default",
 				},
 				Spec: vmv1beta1.VMAlertSpec{
 					RuleSelector:          &metav1.LabelSelector{},
@@ -74,7 +77,8 @@ func TestIsSelectorsMatchesTargetCRD(t *testing.T) {
 			isMatch:           false,
 		},
 		{
-			name: "selector matches",
+			name:      "match: selector matches, selectAll=any",
+			selectAll: false,
 			sourceCRD: &vmv1beta1.VMRule{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "rule",
@@ -112,7 +116,8 @@ func TestIsSelectorsMatchesTargetCRD(t *testing.T) {
 			isMatch:           true,
 		},
 		{
-			name: "selector not match",
+			name:      "not match: selector not match, selectAll=any",
+			selectAll: true,
 			sourceCRD: &vmv1beta1.VMRule{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "rule",
@@ -163,7 +168,8 @@ func TestIsSelectorsMatchesTargetCRD(t *testing.T) {
 			isMatch:           false,
 		},
 		{
-			name: "namespaceselector matches",
+			name:      "match: namespaceselector matches, selectAll=any",
+			selectAll: false,
 			sourceCRD: &vmv1beta1.VMRule{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "rule",
@@ -204,7 +210,8 @@ func TestIsSelectorsMatchesTargetCRD(t *testing.T) {
 			isMatch: true,
 		},
 		{
-			name: "namespaceselector not matches",
+			name:      "not match: namespaceselector not matches, selectAll=any",
+			selectAll: true,
 			sourceCRD: &vmv1beta1.VMRule{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "rule",
@@ -245,7 +252,8 @@ func TestIsSelectorsMatchesTargetCRD(t *testing.T) {
 			isMatch: false,
 		},
 		{
-			name: "selector+namespaceSelector match",
+			name:      "match: selector+namespaceSelector match, selectAll=any",
+			selectAll: false,
 			sourceCRD: &vmv1beta1.VMRule{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "rule",
@@ -305,7 +313,7 @@ func TestIsSelectorsMatchesTargetCRD(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			fclient := k8stools.GetTestClientWithObjects(tt.predefinedObjects)
-			matches, err := isSelectorsMatchesTargetCRD(context.Background(), fclient, tt.sourceCRD, tt.targetCRD, tt.selector, tt.namespaceSelector)
+			matches, err := isSelectorsMatchesTargetCRD(context.Background(), fclient, tt.sourceCRD, tt.targetCRD, tt.selector, tt.namespaceSelector, tt.selectAll)
 			if err != nil {
 				t.Fatal(err)
 			}

internal/controller/operator/factory/alertmanager/config_test.go

@@ -1459,11 +1459,9 @@ func Test_UpdateDefaultAMConfig(t *testing.T) {
 						Namespace: "default",
 					},
 					Spec: vmv1beta1.VMAlertmanagerSpec{
-						ConfigSecret:            "vmalertmanager-test-am-config",
-						ConfigRawYaml:           "global: {}",
-						ConfigSelector:          &metav1.LabelSelector{},
-						ConfigNamespaceSelector: &metav1.LabelSelector{},
-						SelectAllByDefault:      true,
+						ConfigSecret:       "vmalertmanager-test-am-config",
+						ConfigRawYaml:      "global: {}",
+						SelectAllByDefault: true,
 					},
 				},
 			},

internal/controller/operator/factory/k8stools/selectors.go

@@ -38,31 +38,31 @@ func VisitObjectsForSelectorsAtNs[T any, PT interface {
 	case objectSelector != nil && nsSelector == nil:
 		// in single namespace mode, return object ns
 		namespaces = append(namespaces, objNamespace)
-	default:
+	case nsSelector != nil:
 		// perform a cluster wide request for namespaces with given filters
 		nsSelector, err := metav1.LabelSelectorAsSelector(nsSelector)
 		if err != nil {
-			return fmt.Errorf("cannot convert  selector: %w", err)
+			return fmt.Errorf("cannot convert selector: %w", err)
 		}
 		namespaces, err = SelectNamespaces(ctx, rclient, nsSelector)
 		if err != nil {
 			return fmt.Errorf("cannot select namespaces for  match: %w", err)
 		}
-	}
-	// fast path nothing selected
-	if namespaces == nil && !selectAllByDefault {
-		return nil
+		// if nsSelector is specified and no match, return directly
+		if len(namespaces) == 0 {
+			return nil
+		}
 	}
 
-	// if namespaces isn't nil, then nameSpaceSelector is defined
-	// but userSelector maybe be nil and we must set it to catch all values
+	// if userSelector is nil, we must set it to catch all values
 	if objectSelector == nil {
 		objectSelector = &metav1.LabelSelector{}
 	}
 	objLabelSelector, err := metav1.LabelSelectorAsSelector(objectSelector)
 	if err != nil {
 		return fmt.Errorf("cannot convert  to Selector: %w", err)
 	}
+	// namespaces could still be empty if nsSelector&objectSelector are nil and selectAllByDefault=true, and it's ok
 	return ListObjectsByNamespace(ctx, rclient, namespaces, cb, &client.ListOptions{LabelSelector: objLabelSelector})
 }
 

internal/controller/operator/factory/vmagent/collect_scrapes.go

@@ -17,7 +17,7 @@ func selectScrapeConfig(ctx context.Context, cr *vmv1beta1.VMAgent, rclient clie
 	var scrapeConfigsCombined []*vmv1beta1.VMScrapeConfig
 	var namespacedNames []string
 
-	if err := k8stools.VisitObjectsForSelectorsAtNs(ctx, rclient, cr.Spec.NodeScrapeNamespaceSelector, cr.Spec.ScrapeConfigSelector, cr.Namespace, cr.Spec.SelectAllByDefault,
+	if err := k8stools.VisitObjectsForSelectorsAtNs(ctx, rclient, cr.Spec.ScrapeConfigNamespaceSelector, cr.Spec.ScrapeConfigSelector, cr.Namespace, cr.Spec.SelectAllByDefault,
 		func(list *vmv1beta1.VMScrapeConfigList) {
 			for _, item := range list.Items {
 				if !item.DeletionTimestamp.IsZero() {