feat: add label selector to serverclass

Adds field `selector` to the root of the ServerClassSpec.

This implements the same schema/logic for selecting Servers as is
used in several builtin Kubernetes APIs.

Tests have been added to show the behavior when both `selector` and
`qualifiers` are used, as well as when neither are used.

https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/label-selector/

Signed-off-by: bzub <bzubrod@gmail.com>

app/metal-controller-manager/api/v1alpha1/serverclass_filter.go

@@ -4,20 +4,38 @@
 
 package v1alpha1
 
-import "sort"
+import (
+	"fmt"
+	"sort"
 
-// FilterAcceptedServers returns a new slice of Servers that are accepted and qualify.
-//
-// Returned Servers are always sorted by name for stable results.
-func FilterAcceptedServers(servers []Server, q Qualifiers) []Server {
-	res := make([]Server, 0, len(servers))
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+)
 
-	for _, server := range servers {
-		// skip non-accepted servers
-		if !server.Spec.Accepted {
-			continue
+// AcceptedServerFilter matches Servers that have Spec.Accepted set to true.
+func AcceptedServerFilter(s Server) (bool, error) {
+	return s.Spec.Accepted, nil
+}
+
+// SelectorFilter returns a ServerFilter that matches servers against the
+// serverclass's selector field.
+func (sc *ServerClass) SelectorFilter() func(Server) (bool, error) {
+	return func(server Server) (bool, error) {
+		s, err := metav1.LabelSelectorAsSelector(&sc.Spec.Selector)
+		if err != nil {
+			return false, fmt.Errorf("failed to get selector from labelselector: %v", err)
 		}
 
+		return s.Matches(labels.Set(server.GetLabels())), nil
+	}
+}
+
+// QualifiersFilter returns a ServerFilter that matches servers against the
+// serverclass's qualifiers field.
+func (sc *ServerClass) QualifiersFilter() func(Server) (bool, error) {
+	return func(server Server) (bool, error) {
+		q := sc.Spec.Qualifiers
+
 		// check CPU qualifiers if they are present
 		if filters := q.CPU; len(filters) > 0 {
 			var match bool
@@ -30,7 +48,7 @@ func FilterAcceptedServers(servers []Server, q Qualifiers) []Server {
 			}
 
 			if !match {
-				continue
+				return false, nil
 			}
 		}
 
@@ -45,7 +63,7 @@ func FilterAcceptedServers(servers []Server, q Qualifiers) []Server {
 			}
 
 			if !match {
-				continue
+				return false, nil
 			}
 		}
 
@@ -62,14 +80,41 @@ func FilterAcceptedServers(servers []Server, q Qualifiers) []Server {
 			}
 
 			if !match {
-				continue
+				return false, nil
 			}
 		}
 
-		res = append(res, server)
+		return true, nil
+	}
+}
+
+// FilterServers returns the subset of servers that pass all provided filters.
+// In case of error the returned slice will be nil.
+func FilterServers(servers []Server, filters ...func(Server) (bool, error)) ([]Server, error) {
+	matches := make([]Server, 0, len(servers))
+
+	for _, server := range servers {
+		match := true
+
+		for _, filter := range filters {
+			var err error
+
+			match, err = filter(server)
+			if err != nil {
+				return nil, fmt.Errorf("failed to filter server: %v", err)
+			}
+
+			if !match {
+				break
+			}
+		}
+
+		if match {
+			matches = append(matches, server)
+		}
 	}
 
-	sort.Slice(res, func(i, j int) bool { return res[i].Name < res[j].Name })
+	sort.Slice(matches, func(i, j int) bool { return matches[i].Name < matches[j].Name })
 
-	return res
+	return matches, nil
 }

app/metal-controller-manager/api/v1alpha1/serverclass_filter_test.go

@@ -17,6 +17,12 @@ func TestFilterAcceptedServers(t *testing.T) {
 	t.Parallel()
 
 	atom := metalv1alpha1.Server{
+		ObjectMeta: metav1.ObjectMeta{
+			Labels: map[string]string{
+				"common-label": "true",
+				"zone":         "central",
+			},
+		},
 		Spec: metalv1alpha1.ServerSpec{
 			Accepted: true,
 			CPU: &metalv1alpha1.CPUInformation{
@@ -28,7 +34,8 @@ func TestFilterAcceptedServers(t *testing.T) {
 	ryzen := metalv1alpha1.Server{
 		ObjectMeta: metav1.ObjectMeta{
 			Labels: map[string]string{
-				"my-server-label": "true",
+				"common-label": "true",
+				"zone":         "east",
 			},
 		},
 		Spec: metalv1alpha1.ServerSpec{
@@ -45,7 +52,8 @@ func TestFilterAcceptedServers(t *testing.T) {
 	notAccepted := metalv1alpha1.Server{
 		ObjectMeta: metav1.ObjectMeta{
 			Labels: map[string]string{
-				"my-server-label": "true",
+				"common-label": "true",
+				"zone":         "west",
 			},
 		},
 		Spec: metalv1alpha1.ServerSpec{
@@ -63,9 +71,14 @@ func TestFilterAcceptedServers(t *testing.T) {
 	servers := []metalv1alpha1.Server{atom, ryzen, notAccepted}
 
 	testdata := map[string]struct {
+		s        metav1.LabelSelector
 		q        metalv1alpha1.Qualifiers
 		expected []metalv1alpha1.Server
 	}{
+		"empty selector - empty qualifier": {
+			// Matches all servers
+			expected: []metalv1alpha1.Server{atom, ryzen},
+		},
 		"Intel only": {
 			q: metalv1alpha1.Qualifiers{
 				CPU: []metalv1alpha1.CPUInformation{
@@ -90,12 +103,145 @@ func TestFilterAcceptedServers(t *testing.T) {
 			q: metalv1alpha1.Qualifiers{
 				LabelSelectors: []map[string]string{
 					{
-						"my-server-label": "true",
+						"common-label": "true",
+					},
+				},
+			},
+			expected: []metalv1alpha1.Server{atom, ryzen},
+		},
+		// This should probably only return atom. Leaving it as-is to
+		// avoid breaking changes before we remove LabelSelectors in
+		// favor of Selector.
+		"with multiple labels - single selector": {
+			q: metalv1alpha1.Qualifiers{
+				LabelSelectors: []map[string]string{
+					{
+						"common-label": "true",
+						"zone":         "central",
+					},
+				},
+			},
+			expected: []metalv1alpha1.Server{atom, ryzen},
+		},
+		"with multiple labels - multiple selectors": {
+			q: metalv1alpha1.Qualifiers{
+				LabelSelectors: []map[string]string{
+					{
+						"common-label": "true",
+					},
+					{
+						"zone": "central",
+					},
+				},
+			},
+			expected: []metalv1alpha1.Server{atom, ryzen},
+		},
+		"with same label key different label value": {
+			q: metalv1alpha1.Qualifiers{
+				LabelSelectors: []map[string]string{
+					{
+						"zone": "central",
+					},
+				},
+			},
+			expected: []metalv1alpha1.Server{atom},
+		},
+		"selector - single MatchLabels single result": {
+			s: metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"zone": "central",
+				},
+			},
+			expected: []metalv1alpha1.Server{atom},
+		},
+		"selector - single MatchLabels multiple results": {
+			s: metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"common-label": "true",
+				},
+			},
+			expected: []metalv1alpha1.Server{atom, ryzen},
+		},
+		"selector - multiple MatchLabels": {
+			s: metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"zone":         "central",
+					"common-label": "true",
+				},
+			},
+			expected: []metalv1alpha1.Server{atom},
+		},
+		"selector - MatchExpressions common label key": {
+			s: metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{
+						Key:      "common-label",
+						Operator: "Exists",
+					},
+				},
+			},
+			expected: []metalv1alpha1.Server{atom, ryzen},
+		},
+		"selector - MatchExpressions multiple values": {
+			s: metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{
+						Key:      "zone",
+						Operator: "In",
+						Values: []string{
+							"east",
+							"west",
+						},
 					},
 				},
 			},
 			expected: []metalv1alpha1.Server{ryzen},
 		},
+		"selector and qualifiers both match": {
+			s: metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"common-label": "true",
+				},
+			},
+			q: metalv1alpha1.Qualifiers{
+				SystemInformation: []metalv1alpha1.SystemInformation{
+					{
+						Manufacturer: "QEMU",
+					},
+				},
+			},
+			expected: []metalv1alpha1.Server{ryzen},
+		},
+		"selector and qualifiers with disqualifying selector": {
+			s: metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"common-label": "no-match",
+				},
+			},
+			q: metalv1alpha1.Qualifiers{
+				SystemInformation: []metalv1alpha1.SystemInformation{
+					{
+						Manufacturer: "QEMU",
+					},
+				},
+			},
+			expected: []metalv1alpha1.Server{},
+		},
+		"selector and qualifiers with disqualifying qualifier": {
+			s: metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"common-label": "true",
+				},
+			},
+			q: metalv1alpha1.Qualifiers{
+				SystemInformation: []metalv1alpha1.SystemInformation{
+					{
+						Manufacturer: "Gateway",
+					},
+				},
+			},
+			expected: []metalv1alpha1.Server{},
+		},
 		metalv1alpha1.ServerClassAny: {
 			expected: []metalv1alpha1.Server{atom, ryzen},
 		},
@@ -106,8 +252,19 @@ func TestFilterAcceptedServers(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
-			actual := metalv1alpha1.FilterAcceptedServers(servers, td.q)
-			assert.Equal(t, actual, td.expected)
+			sc := &metalv1alpha1.ServerClass{
+				Spec: metalv1alpha1.ServerClassSpec{
+					Selector:   td.s,
+					Qualifiers: td.q,
+				},
+			}
+			actual, err := metalv1alpha1.FilterServers(servers,
+				metalv1alpha1.AcceptedServerFilter,
+				sc.SelectorFilter(),
+				sc.QualifiersFilter(),
+			)
+			assert.NoError(t, err)
+			assert.Equal(t, td.expected, actual)
 		})
 	}
 }

app/metal-controller-manager/api/v1alpha1/serverclass_types.go

@@ -22,6 +22,7 @@ type Qualifiers struct {
 type ServerClassSpec struct {
 	EnvironmentRef *corev1.ObjectReference `json:"environmentRef,omitempty"`
 	Qualifiers     Qualifiers              `json:"qualifiers"`
+	Selector       metav1.LabelSelector    `json:"selector"`
 	ConfigPatches  []ConfigPatches         `json:"configPatches,omitempty"`
 }
 

app/metal-controller-manager/config/crd/bases/metal.sidero.dev_serverclasses.yaml

@@ -80,6 +80,36 @@ spec:
                     description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids'
                     type: string
                 type: object
+              selector:
+                description: Label selector for servers.
+                properties:
+                  matchExpressions:
+                    description: matchExpressions is a list of label selector requirements. The requirements are ANDed.
+                    items:
+                      description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values.
+                      properties:
+                        key:
+                          description: key is the label key that the selector applies to.
+                          type: string
+                        operator:
+                          description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist.
+                          type: string
+                        values:
+                          description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch.
+                          items:
+                            type: string
+                          type: array
+                      required:
+                      - key
+                      - operator
+                      type: object
+                    type: array
+                  matchLabels:
+                    additionalProperties:
+                      type: string
+                    description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed.
+                    type: object
+                type: object
               qualifiers:
                 properties:
                   cpu: