add 'local' selectors to cluster definitions in scheduler config

Author: Diaphteiros

internal/config/config_scheduler.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"fmt"
+	"maps"
 	"slices"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -24,10 +25,13 @@ type SchedulerConfig struct {
 	Strategy Strategy `json:"strategy"`
 
 	// +optional
-	Selectors          *SchedulerSelectors         `json:"selectors,omitempty"`
+	Selectors *SchedulerSelectors `json:"selectors,omitempty"`
+	// Note that CompletedSelectors.Clusters holds the global cluster selector.
+	// During Complete(), the local selector is merged with the global one (or set to the global one if nil).
+	// This means that always the local completed selector should be used, unless the task is not tied to a specific ClusterDefinition.
 	CompletedSelectors CompletedSchedulerSelectors `json:"-"`
 
-	PurposeMappings map[string]ClusterDefinition `json:"purposeMappings"`
+	PurposeMappings map[string]*ClusterDefinition `json:"purposeMappings"`
 }
 
 type SchedulerScope string
@@ -51,7 +55,9 @@ type ClusterDefinition struct {
 	// Must be equal to or greater than 0 otherwise, with 0 meaning "unlimited".
 	TenancyCount int `json:"tenancyCount,omitempty"`
 
-	Template ClusterTemplate `json:"template"`
+	Template          ClusterTemplate       `json:"template"`
+	Selector          *metav1.LabelSelector `json:"selector,omitempty"`
+	CompletedSelector labels.Selector       `json:"-"`
 }
 
 type ClusterTemplate struct {
@@ -76,7 +82,7 @@ func (c *SchedulerConfig) Default(_ *field.Path) error {
 		c.Strategy = STRATEGY_BALANCED
 	}
 	if c.PurposeMappings == nil {
-		c.PurposeMappings = map[string]ClusterDefinition{}
+		c.PurposeMappings = map[string]*ClusterDefinition{}
 	}
 	return nil
 }
@@ -118,7 +124,11 @@ func (c *SchedulerConfig) Validate(fldPath *field.Path) error {
 	for purpose, definition := range c.PurposeMappings {
 		pPath := fldPath.Key(purpose)
 		if purpose == "" {
-			errs = append(errs, field.Invalid(pPath, purpose, "purpose must not be empty"))
+			errs = append(errs, field.Invalid(fldPath, purpose, "purpose must not be empty"))
+		}
+		if definition == nil {
+			errs = append(errs, field.Required(pPath, "definition must not be nil"))
+			continue
 		}
 		if definition.TenancyCount < 0 {
 			errs = append(errs, field.Invalid(pPath.Child("tenancyCount"), definition.TenancyCount, "tenancyCount must be greater than or equal to 0"))
@@ -137,7 +147,18 @@ func (c *SchedulerConfig) Validate(fldPath *field.Path) error {
 			errs = append(errs, field.Invalid(pPath.Child("tenancyCount"), definition.TenancyCount, fmt.Sprintf("tenancyCount must be 0 if the template specifies '%s' tenancy", string(clustersv1alpha1.TENANCY_EXCLUSIVE))))
 		}
 		if cls != nil && !cls.Matches(labels.Set(definition.Template.Labels)) {
-			errs = append(errs, field.Invalid(pPath.Child("template").Child("metadata").Child("labels"), definition.Template.Labels, "labels do not match specified cluster selector"))
+			errs = append(errs, field.Invalid(pPath.Child("template").Child("metadata").Child("labels"), definition.Template.Labels, "labels do not match specified global cluster selector"))
+		}
+		var lcls labels.Selector
+		if definition.Selector != nil {
+			var err error
+			lcls, err = metav1.LabelSelectorAsSelector(definition.Selector)
+			if err != nil {
+				errs = append(errs, field.Invalid(pPath.Child("selector"), definition.Selector, err.Error()))
+			}
+		}
+		if lcls != nil && !lcls.Matches(labels.Set(definition.Template.Labels)) {
+			errs = append(errs, field.Invalid(pPath.Child("template").Child("metadata").Child("labels"), definition.Template.Labels, "labels do not match specified local cluster selector"))
 		}
 	}
 	return errs.ToAggregate()
@@ -166,5 +187,37 @@ func (c *SchedulerConfig) Complete(fldPath *field.Path) error {
 	if c.CompletedSelectors.ClusterRequests == nil {
 		c.CompletedSelectors.ClusterRequests = labels.Everything()
 	}
+
+	for purpose, definition := range c.PurposeMappings {
+		pPath := fldPath.Child("purposeMappings").Key(purpose)
+		if definition.Selector != nil {
+			var combinedSelector *metav1.LabelSelector
+			if c.Selectors.Clusters == nil {
+				combinedSelector = definition.Selector
+			} else if definition.Selector == nil {
+				combinedSelector = c.Selectors.Clusters
+			} else {
+				combinedSelector = c.Selectors.Clusters.DeepCopy()
+				if combinedSelector.MatchLabels == nil {
+					combinedSelector.MatchLabels = definition.Selector.MatchLabels
+				} else if definition.Selector.MatchLabels != nil {
+					maps.Insert(combinedSelector.MatchLabels, maps.All(definition.Selector.MatchLabels))
+				}
+				if combinedSelector.MatchExpressions == nil {
+					combinedSelector.MatchExpressions = definition.Selector.MatchExpressions
+				} else if definition.Selector.MatchExpressions != nil {
+					combinedSelector.MatchExpressions = append(combinedSelector.MatchExpressions, definition.Selector.MatchExpressions...)
+				}
+			}
+			var err error
+			definition.CompletedSelector, err = metav1.LabelSelectorAsSelector(combinedSelector)
+			if err != nil {
+				return field.Invalid(pPath.Child("selector"), combinedSelector, fmt.Sprintf("the combination of the global and local selector is invalid: %s", err.Error()))
+			}
+		} else {
+			definition.CompletedSelector = c.CompletedSelectors.Clusters
+		}
+	}
+
 	return nil
 }

internal/controllers/scheduler/controller.go

@@ -149,7 +149,7 @@ func (r *ClusterScheduler) reconcile(ctx context.Context, log logging.Logger, re
 			namespace = cDef.Template.Namespace
 		}
 		clusterList := &clustersv1alpha1.ClusterList{}
-		if err := r.OnboardingCluster.Client().List(ctx, clusterList, client.InNamespace(namespace), client.MatchingLabelsSelector{Selector: r.Config.CompletedSelectors.Clusters}); err != nil {
+		if err := r.OnboardingCluster.Client().List(ctx, clusterList, client.InNamespace(namespace), client.MatchingLabelsSelector{Selector: cDef.CompletedSelector}); err != nil {
 			rr.ReconcileError = errutils.WithReason(fmt.Errorf("error listing Clusters: %w", err), cconst.ReasonOnboardingClusterInteractionProblem)
 			return rr
 		}

internal/controllers/scheduler/controller_test.go

@@ -365,4 +365,38 @@ var _ = Describe("Scheduler", func() {
 
 	})
 
+	It("should combine cluster label selectors correctly", func() {
+		_, env := defaultTestSetup("testdata", "test-04")
+
+		// should use the existing cluster
+		req := &clustersv1alpha1.ClusterRequest{}
+		Expect(env.Client().Get(env.Ctx, ctrlutils.ObjectKey("shared", "foo"), req)).To(Succeed())
+		Expect(req.Status.Cluster).To(BeNil())
+		env.ShouldReconcile(testutils.RequestFromObject(req))
+		Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(req), req)).To(Succeed())
+		Expect(req.Status.Cluster).ToNot(BeNil())
+		Expect(req.Status.Cluster.Name).To(Equal("shared"))
+		Expect(req.Status.Cluster.Namespace).To(Equal("foo"))
+
+		// should create a new cluster
+		req2 := &clustersv1alpha1.ClusterRequest{}
+		Expect(env.Client().Get(env.Ctx, ctrlutils.ObjectKey("shared2", "foo"), req2)).To(Succeed())
+		Expect(req2.Status.Cluster).To(BeNil())
+		env.ShouldReconcile(testutils.RequestFromObject(req2))
+		Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(req2), req2)).To(Succeed())
+		Expect(req2.Status.Cluster).ToNot(BeNil())
+		Expect(req2.Status.Cluster.Name).To(Equal("shared2"))
+		Expect(req2.Status.Cluster.Namespace).To(Equal("foo"))
+
+		// should use the existing cluster
+		req3 := &clustersv1alpha1.ClusterRequest{}
+		Expect(env.Client().Get(env.Ctx, ctrlutils.ObjectKey("shared3", "foo"), req3)).To(Succeed())
+		Expect(req3.Status.Cluster).To(BeNil())
+		env.ShouldReconcile(testutils.RequestFromObject(req3))
+		Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(req3), req3)).To(Succeed())
+		Expect(req3.Status.Cluster).ToNot(BeNil())
+		Expect(req3.Status.Cluster.Name).To(Equal("shared2"))
+		Expect(req3.Status.Cluster.Namespace).To(Equal("foo"))
+	})
+
 })

internal/controllers/scheduler/testdata/test-04/cluster/cluster-0.yaml

@@ -0,0 +1,16 @@
+apiVersion: clusters.openmcp.cloud/v1alpha1
+kind: Cluster
+metadata:
+  name: shared
+  namespace: foo
+  labels:
+    foo.bar.baz/foobar: "true"
+spec:
+  profile: test-profile
+  kubernetes:
+    version: "1.32"
+  purposes:
+  - test
+  - shared
+  - shared2
+  tenancy: Shared

internal/controllers/scheduler/testdata/test-04/cluster/req-0.yaml

@@ -0,0 +1,8 @@
+apiVersion: clusters.openmcp.cloud/v1alpha1
+kind: ClusterRequest
+metadata:
+  name: shared
+  namespace: foo
+  uid: 4d6aa495-54c0-4df2-bc7b-05b103f02e69
+spec:
+  purpose: shared

internal/controllers/scheduler/testdata/test-04/cluster/req-1.yaml

@@ -0,0 +1,8 @@
+apiVersion: clusters.openmcp.cloud/v1alpha1
+kind: ClusterRequest
+metadata:
+  name: shared2
+  namespace: foo
+  uid: bf7d8a52-ea1e-4303-a0f9-bf2ef0cc21ea
+spec:
+  purpose: shared2

internal/controllers/scheduler/testdata/test-04/cluster/req-2.yaml

@@ -0,0 +1,8 @@
+apiVersion: clusters.openmcp.cloud/v1alpha1
+kind: ClusterRequest
+metadata:
+  name: shared3
+  namespace: foo
+  uid: 7642e165-196c-478f-a225-765bd97a29d8
+spec:
+  purpose: shared2

internal/controllers/scheduler/testdata/test-04/config.yaml

@@ -0,0 +1,27 @@
+scheduler:
+  scope: Cluster
+  selectors:
+    clusters:
+      matchLabels:
+        foo.bar.baz/foobar: "true"
+  purposeMappings:
+    shared:
+      template:
+        metadata:
+          labels:
+            foo.bar.baz/foobar: "true"
+        spec:
+          profile: test-profile
+          tenancy: Shared
+    shared2:
+      selector:
+        matchLabels:
+          foo.bar.baz/foobaz: "true"
+      template:
+        metadata:
+          labels:
+            foo.bar.baz/foobar: "true"
+            foo.bar.baz/foobaz: "true"
+        spec:
+          profile: test-profile
+          tenancy: Shared