Merge pull request #4010 from whitewindmills/deprecated-zone-rs

adopt zones in resource selector

pkg/util/selector.go

@@ -1,9 +1,12 @@
 package util
 
 import (
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/klog/v2"
+	"k8s.io/utils/strings/slices"
 
 	clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1"
 	policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
@@ -105,14 +108,31 @@ func ClusterMatches(cluster *clusterv1alpha1.Cluster, affinity policyv1alpha1.Cl
 	}
 
 	if affinity.FieldSelector != nil {
-		var matchFields labels.Selector
-		var errs []error
-		if matchFields, errs = lifted.NodeSelectorRequirementsAsSelector(affinity.FieldSelector.MatchExpressions); errs != nil {
-			return false
+		var clusterFieldsMatchExpressions []corev1.NodeSelectorRequirement
+		for i := range affinity.FieldSelector.MatchExpressions {
+			matchExpression := &affinity.FieldSelector.MatchExpressions[i]
+			if matchExpression.Key != ZoneField {
+				clusterFieldsMatchExpressions = append(clusterFieldsMatchExpressions, *matchExpression)
+				continue
+			}
+
+			// First, match zones field.
+			if !matchZones(matchExpression, cluster.Spec.Zones) {
+				return false
+			}
 		}
-		clusterFields := extractClusterFields(cluster)
-		if matchFields != nil && !matchFields.Matches(clusterFields) {
-			return false
+
+		if len(clusterFieldsMatchExpressions) > 0 {
+			// Second, match other fields.
+			var matchFields labels.Selector
+			var errs []error
+			if matchFields, errs = lifted.NodeSelectorRequirementsAsSelector(clusterFieldsMatchExpressions); errs != nil {
+				return false
+			}
+			clusterFields := extractClusterFields(cluster)
+			if matchFields != nil && !matchFields.Matches(clusterFields) {
+				return false
+			}
 		}
 	}
 
@@ -165,9 +185,41 @@ func extractClusterFields(cluster *clusterv1alpha1.Cluster) labels.Set {
 		clusterFieldsMap[RegionField] = cluster.Spec.Region
 	}
 
-	if cluster.Spec.Zone != "" {
-		clusterFieldsMap[ZoneField] = cluster.Spec.Zone
-	}
-
 	return clusterFieldsMap
 }
+
+// matchZones checks if zoneMatchExpression can match zones and returns true if it matches.
+// For unknown operators, matchZones always returns false.
+// The matching rules are as follows:
+// 1. When the operator is "In", zoneMatchExpression must contain all zones, otherwise it doesn't match.
+// 2. When the operator is "NotIn", zoneMatchExpression mustn't contain any one of zones, otherwise it doesn't match.
+// 3. When the operator is "Exists", zones mustn't be empty, otherwise it doesn't match.
+// 4. When the operator is "DoesNotExist", zones must be empty, otherwise it doesn't match.
+func matchZones(zoneMatchExpression *corev1.NodeSelectorRequirement, zones []string) bool {
+	switch zoneMatchExpression.Operator {
+	case corev1.NodeSelectorOpIn:
+		if len(zones) == 0 {
+			return false
+		}
+		for _, zone := range zones {
+			if !slices.Contains(zoneMatchExpression.Values, zone) {
+				return false
+			}
+		}
+		return true
+	case corev1.NodeSelectorOpNotIn:
+		for _, zone := range zones {
+			if slices.Contains(zoneMatchExpression.Values, zone) {
+				return false
+			}
+		}
+		return true
+	case corev1.NodeSelectorOpExists:
+		return len(zones) > 0
+	case corev1.NodeSelectorOpDoesNotExist:
+		return len(zones) == 0
+	default:
+		klog.V(5).Infof("Unsupported %q operator for zones requirement", zoneMatchExpression.Operator)
+		return false
+	}
+}

pkg/util/selector_test.go

@@ -297,7 +297,7 @@ func TestClusterMatches(t *testing.T) {
 			},
 		},
 		Spec: clusterv1alpha1.ClusterSpec{
-			Zone:     "zone1",
+			Zones:    []string{"zone1", "zone2", "zone3"},
 			Region:   "region1",
 			Provider: "provider1",
 		},
@@ -332,7 +332,7 @@ func TestClusterMatches(t *testing.T) {
 				ExcludeClusters: []string{cluster.Name},
 				FieldSelector: &policyv1alpha1.FieldSelector{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: []string{cluster.Spec.Zone}},
+						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: cluster.Spec.Zones},
 					},
 				},
 			},
@@ -373,12 +373,13 @@ func TestClusterMatches(t *testing.T) {
 			want: false,
 		},
 		{
-			name: "test cluster names and field selector(zone)",
+			name: "test cluster names and field selector(zone & region)",
 			affinity: policyv1alpha1.ClusterAffinity{
 				ClusterNames: []string{cluster.Name},
 				FieldSelector: &policyv1alpha1.FieldSelector{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: []string{cluster.Spec.Zone}},
+						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: cluster.Spec.Zones},
+						{Key: RegionField, Operator: corev1.NodeSelectorOpIn, Values: []string{cluster.Spec.Region}},
 					},
 				},
 			},
@@ -414,7 +415,7 @@ func TestClusterMatches(t *testing.T) {
 				ClusterNames: []string{cluster.Name},
 				FieldSelector: &policyv1alpha1.FieldSelector{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{Key: ZoneField, Operator: corev1.NodeSelectorOpNotIn, Values: []string{cluster.Spec.Zone}},
+						{Key: ZoneField, Operator: corev1.NodeSelectorOpNotIn, Values: cluster.Spec.Zones},
 					},
 				},
 			},
@@ -452,7 +453,7 @@ func TestClusterMatches(t *testing.T) {
 				},
 				FieldSelector: &policyv1alpha1.FieldSelector{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: []string{cluster.Spec.Zone}},
+						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: cluster.Spec.Zones},
 					},
 				},
 			},
@@ -494,7 +495,7 @@ func TestClusterMatches(t *testing.T) {
 				},
 				FieldSelector: &policyv1alpha1.FieldSelector{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{Key: ZoneField, Operator: corev1.NodeSelectorOpNotIn, Values: []string{cluster.Spec.Zone}},
+						{Key: ZoneField, Operator: corev1.NodeSelectorOpNotIn, Values: cluster.Spec.Zones},
 					},
 				},
 			},
@@ -576,7 +577,7 @@ func TestClusterMatches(t *testing.T) {
 			affinity: policyv1alpha1.ClusterAffinity{
 				FieldSelector: &policyv1alpha1.FieldSelector{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: []string{cluster.Spec.Zone}},
+						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: cluster.Spec.Zones},
 					},
 				},
 			},
@@ -588,7 +589,7 @@ func TestClusterMatches(t *testing.T) {
 				ClusterNames: []string{cluster.Name},
 				FieldSelector: &policyv1alpha1.FieldSelector{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{Key: ZoneField, Operator: corev1.NodeSelectorOpNotIn, Values: []string{cluster.Spec.Zone}},
+						{Key: ZoneField, Operator: corev1.NodeSelectorOpNotIn, Values: cluster.Spec.Zones},
 					},
 				},
 			},
@@ -690,7 +691,7 @@ func TestClusterMatches(t *testing.T) {
 			affinity: policyv1alpha1.ClusterAffinity{
 				FieldSelector: &policyv1alpha1.FieldSelector{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: []string{cluster.Spec.Zone}},
+						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: cluster.Spec.Zones},
 					},
 				},
 				LabelSelector: &metav1.LabelSelector{
@@ -705,7 +706,7 @@ func TestClusterMatches(t *testing.T) {
 			affinity: policyv1alpha1.ClusterAffinity{
 				FieldSelector: &policyv1alpha1.FieldSelector{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: []string{cluster.Spec.Zone}},
+						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: cluster.Spec.Zones},
 					},
 				},
 				LabelSelector: &metav1.LabelSelector{
@@ -720,7 +721,7 @@ func TestClusterMatches(t *testing.T) {
 			affinity: policyv1alpha1.ClusterAffinity{
 				FieldSelector: &policyv1alpha1.FieldSelector{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{Key: ZoneField, Operator: corev1.NodeSelectorOpNotIn, Values: []string{cluster.Spec.Zone}},
+						{Key: ZoneField, Operator: corev1.NodeSelectorOpNotIn, Values: cluster.Spec.Zones},
 					},
 				},
 				LabelSelector: &metav1.LabelSelector{
@@ -735,7 +736,7 @@ func TestClusterMatches(t *testing.T) {
 			affinity: policyv1alpha1.ClusterAffinity{
 				FieldSelector: &policyv1alpha1.FieldSelector{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: []string{cluster.Spec.Zone}},
+						{Key: ZoneField, Operator: corev1.NodeSelectorOpIn, Values: cluster.Spec.Zones},
 					},
 				},
 				LabelSelector: &metav1.LabelSelector{
@@ -949,19 +950,6 @@ func Test_extractClusterFields(t *testing.T) {
 				RegionField: "foo",
 			},
 		},
-		{
-			name: "zone is set",
-			args: args{
-				cluster: &clusterv1alpha1.Cluster{
-					Spec: clusterv1alpha1.ClusterSpec{
-						Zone: "foo",
-					},
-				},
-			},
-			want: labels.Set{
-				ZoneField: "foo",
-			},
-		},
 		{
 			name: "all are set",
 			args: args{
@@ -969,14 +957,12 @@ func Test_extractClusterFields(t *testing.T) {
 					Spec: clusterv1alpha1.ClusterSpec{
 						Provider: "foo",
 						Region:   "bar",
-						Zone:     "baz",
 					},
 				},
 			},
 			want: labels.Set{
 				ProviderField: "foo",
 				RegionField:   "bar",
-				ZoneField:     "baz",
 			},
 		},
 	}
@@ -988,3 +974,100 @@ func Test_extractClusterFields(t *testing.T) {
 		})
 	}
 }
+
+func Test_matchZones(t *testing.T) {
+	tests := []struct {
+		name                string
+		zoneMatchExpression *corev1.NodeSelectorRequirement
+		zones               []string
+		matched             bool
+	}{
+		{
+			name: "empty zones for In operator",
+			zoneMatchExpression: &corev1.NodeSelectorRequirement{
+				Key:      ZoneField,
+				Operator: corev1.NodeSelectorOpIn,
+				Values:   []string{"foo"},
+			},
+			zones:   nil,
+			matched: false,
+		},
+		{
+			name: "partial zones for In operator",
+			zoneMatchExpression: &corev1.NodeSelectorRequirement{
+				Key:      ZoneField,
+				Operator: corev1.NodeSelectorOpIn,
+				Values:   []string{"foo"},
+			},
+			zones:   []string{"foo", "bar"},
+			matched: false,
+		},
+		{
+			name: "all zones for In operator",
+			zoneMatchExpression: &corev1.NodeSelectorRequirement{
+				Key:      ZoneField,
+				Operator: corev1.NodeSelectorOpIn,
+				Values:   []string{"foo", "bar"},
+			},
+			zones:   []string{"foo", "bar"},
+			matched: true,
+		},
+		{
+			name: "empty zones for NotIn operator",
+			zoneMatchExpression: &corev1.NodeSelectorRequirement{
+				Key:      ZoneField,
+				Operator: corev1.NodeSelectorOpNotIn,
+				Values:   []string{"foo"},
+			},
+			zones:   nil,
+			matched: true,
+		},
+		{
+			name: "partial zones for NotIn operator",
+			zoneMatchExpression: &corev1.NodeSelectorRequirement{
+				Key:      ZoneField,
+				Operator: corev1.NodeSelectorOpNotIn,
+				Values:   []string{"foo"},
+			},
+			zones:   []string{"foo", "bar"},
+			matched: false,
+		},
+		{
+			name: "empty zones for Exists operator",
+			zoneMatchExpression: &corev1.NodeSelectorRequirement{
+				Key:      ZoneField,
+				Operator: corev1.NodeSelectorOpExists,
+				Values:   nil,
+			},
+			zones:   nil,
+			matched: false,
+		},
+		{
+			name: "empty zones for DoesNotExist operator",
+			zoneMatchExpression: &corev1.NodeSelectorRequirement{
+				Key:      ZoneField,
+				Operator: corev1.NodeSelectorOpDoesNotExist,
+				Values:   nil,
+			},
+			zones:   nil,
+			matched: true,
+		},
+		{
+			name: "unknown operator",
+			zoneMatchExpression: &corev1.NodeSelectorRequirement{
+				Key:      ZoneField,
+				Operator: corev1.NodeSelectorOpGt,
+				Values:   nil,
+			},
+			zones:   []string{"foo"},
+			matched: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := matchZones(tt.zoneMatchExpression, tt.zones); got != tt.matched {
+				t.Errorf("matchZones() got %v, but expected %v", got, tt.matched)
+			}
+		})
+	}
+}