Merge pull request openshift#3898 from csrwng/request_serving_nodesel…

…ector_414

HOSTEDCP-1526: [release-4.14] Support additional node selectors for request serving nodes

api/v1beta1/hostedcluster_types.go

@@ -217,6 +217,10 @@ const (
 	// components should be scheduled on dedicated nodes in the management cluster.
 	DedicatedRequestServingComponentsTopology = "dedicated-request-serving-components"
 
+	// RequestServingNodeAdditionalSelectorAnnotation is used to specify an additional node selector for
+	// request serving nodes. The value is a comma-separated list of key=value pairs.
+	RequestServingNodeAdditionalSelectorAnnotation = "hypershift.openshift.io/request-serving-node-additional-selector"
+
 	// AllowGuestWebhooksServiceLabel marks a service deployed in the control plane as a valid target
 	// for validating/mutating webhooks running in the guest cluster.
 	AllowGuestWebhooksServiceLabel = "hypershift.openshift.io/allow-guest-webhooks"

hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go

@@ -1754,6 +1754,7 @@ func reconcileHostedControlPlane(hcp *hyperv1.HostedControlPlane, hcluster *hype
 		hyperv1.OLMCatalogsISRegistryOverridesAnnotation,
 		hyperv1.KubeAPIServerGOGCAnnotation,
 		hyperv1.KubeAPIServerGOMemoryLimitAnnotation,
+		hyperv1.RequestServingNodeAdditionalSelectorAnnotation,
 	}
 	for _, key := range mirroredAnnotations {
 		val, hasVal := hcluster.Annotations[key]

support/config/deployment.go

@@ -45,6 +45,8 @@ type DeploymentConfig struct {
 	DebugDeployments          sets.String
 	ResourceRequestOverrides  ResourceOverrides
 	IsolateAsRequestServing   bool
+
+	AdditionalRequestServingNodeSelector map[string]string
 }
 
 func (c *DeploymentConfig) SetContainerResourcesIfPresent(container *corev1.Container) {
@@ -296,21 +298,29 @@ func (c *DeploymentConfig) setControlPlaneIsolation(hcp *hyperv1.HostedControlPl
 	}
 
 	if c.IsolateAsRequestServing {
+		nodeSelectorRequirements := []corev1.NodeSelectorRequirement{
+			{
+				Key:      hyperv1.RequestServingComponentLabel,
+				Operator: corev1.NodeSelectorOpIn,
+				Values:   []string{"true"},
+			},
+			{
+				Key:      hyperv1.HostedClusterLabel,
+				Operator: corev1.NodeSelectorOpIn,
+				Values:   []string{clusterKey(hcp)},
+			},
+		}
+		for key, value := range c.AdditionalRequestServingNodeSelector {
+			nodeSelectorRequirements = append(nodeSelectorRequirements, corev1.NodeSelectorRequirement{
+				Key:      key,
+				Operator: corev1.NodeSelectorOpIn,
+				Values:   []string{value},
+			})
+		}
 		c.Scheduling.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution = &corev1.NodeSelector{
 			NodeSelectorTerms: []corev1.NodeSelectorTerm{
 				{
-					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{
-							Key:      hyperv1.RequestServingComponentLabel,
-							Operator: corev1.NodeSelectorOpIn,
-							Values:   []string{"true"},
-						},
-						{
-							Key:      hyperv1.HostedClusterLabel,
-							Operator: corev1.NodeSelectorOpIn,
-							Values:   []string{clusterKey(hcp)},
-						},
-					},
+					MatchExpressions: nodeSelectorRequirements,
 				},
 			},
 		}
@@ -357,6 +367,9 @@ func (c *DeploymentConfig) SetRequestServingDefaults(hcp *hyperv1.HostedControlP
 	if hcp.Annotations[hyperv1.TopologyAnnotation] == hyperv1.DedicatedRequestServingComponentsTopology {
 		c.IsolateAsRequestServing = true
 	}
+	if hcp.Annotations[hyperv1.RequestServingNodeAdditionalSelectorAnnotation] != "" {
+		c.AdditionalRequestServingNodeSelector = util.ParseNodeSelector(hcp.Annotations[hyperv1.RequestServingNodeAdditionalSelectorAnnotation])
+	}
 	c.SetDefaults(hcp, multiZoneSpreadLabels, replicas)
 	if c.AdditionalLabels == nil {
 		c.AdditionalLabels = map[string]string{}

support/util/util.go

@@ -301,3 +301,23 @@ func FirstUsableIP(cidr string) (string, error) {
 	ip[len(ipNet.IP)-1]++
 	return ip.String(), nil
 }
+
+// ParseNodeSelector parses a comma separated string of key=value pairs into a map
+func ParseNodeSelector(str string) map[string]string {
+	if len(str) == 0 {
+		return nil
+	}
+	parts := strings.Split(str, ",")
+	result := make(map[string]string)
+	for _, part := range parts {
+		kv := strings.SplitN(part, "=", 2)
+		if len(kv) != 2 {
+			continue
+		}
+		if len(kv[0]) == 0 || len(kv[1]) == 0 {
+			continue
+		}
+		result[kv[0]] = kv[1]
+	}
+	return result
+}

support/util/util_test.go

@@ -345,3 +345,63 @@ func TestFirstUsableIP(t *testing.T) {
 		})
 	}
 }
+
+func TestParseNodeSelector(t *testing.T) {
+	tests := []struct {
+		name string
+		str  string
+		want map[string]string
+	}{
+		{
+			name: "Given a valid node selector string, it should return a map of key value pairs",
+			str:  "key1=value1,key2=value2,key3=value3",
+			want: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+				"key3": "value3",
+			},
+		},
+		{
+			name: "Given a valid node selector string with empty values, it should return a map of key value pairs",
+			str:  "key1=,key2=value2,key3=",
+			want: map[string]string{
+				"key2": "value2",
+			},
+		},
+		{
+			name: "Given a valid node selector string with empty keys, it should return a map of key value pairs",
+			str:  "=value1,key2=value2,=value3",
+			want: map[string]string{
+				"key2": "value2",
+			},
+		},
+		{
+			name: "Given a valid node selector string with empty string, it should return an empty map",
+			str:  "",
+			want: nil,
+		},
+		{
+			name: "Given a valid node selector string with invalid key value pairs, it should return a map of key value pairs",
+			str:  "key1=value1,key2,key3=value3",
+			want: map[string]string{
+				"key1": "value1",
+				"key3": "value3",
+			},
+		},
+		{
+			name: "Given a valid node selector string with values that include =, it should return a map of key value pairs",
+			str:  "key1=value1=one,key2,key3=value3=three",
+			want: map[string]string{
+				"key1": "value1=one",
+				"key3": "value3=three",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			got := ParseNodeSelector(tt.str)
+			g.Expect(got).To(Equal(tt.want))
+		})
+	}
+}