OPNET-133: Support remote worker

Adding node affinity rule to router deployement that will not allow router to run on remote worker node (node with custom role label)
openshift · Dec 2, 2022 · 4e7f152 · 4e7f152
1 parent d9d1a2b
commit 4e7f152
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 2 deletions.
diff --git a/pkg/operator/controller/ingress/deployment.go b/pkg/operator/controller/ingress/deployment.go
@@ -592,6 +592,21 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
 		nodeSelector["node-role.kubernetes.io/master"] = ""
 	default:
 		nodeSelector["node-role.kubernetes.io/worker"] = ""
+		// Disabling running on remote workers.
+		if deployment.Spec.Template.Spec.Affinity == nil {
+			deployment.Spec.Template.Spec.Affinity = &corev1.Affinity{}
+		}
+		deployment.Spec.Template.Spec.Affinity.NodeAffinity = &corev1.NodeAffinity{RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
+			NodeSelectorTerms: []corev1.NodeSelectorTerm{{
+				MatchExpressions: []corev1.NodeSelectorRequirement{
+					{
+						Key:      controller.RemoteWorkerLabel,
+						Operator: corev1.NodeSelectorOpNotIn,
+						Values:   []string{""},
+					},
+				},
+			}},
+		}}
 	}
 
 	if ci.Spec.NodePlacement != nil {
@@ -1301,6 +1316,15 @@ func hashableDeployment(deployment *appsv1.Deployment, onlyTemplate bool) *appsv
 				})
 			}
 		}
+		if affinity.NodeAffinity != nil {
+			terms := affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms
+			for _, term := range terms {
+				exprs := term.MatchExpressions
+				sort.Slice(exprs, func(i, j int) bool {
+					return cmpNodeMatchExpressions(exprs[i], exprs[j])
+				})
+			}
+		}
 	}
 	hashableDeployment.Spec.Template.Spec.Affinity = affinity
 	tolerations := make([]corev1.Toleration, len(deployment.Spec.Template.Spec.Tolerations))
@@ -1417,6 +1441,25 @@ func cmpMatchExpressions(a, b metav1.LabelSelectorRequirement) bool {
 	return false
 }
 
+// cmpNodeMatchExpressions is a helper for hashableDeployment.
+func cmpNodeMatchExpressions(a, b corev1.NodeSelectorRequirement) bool {
+	if a.Key != b.Key {
+		return a.Key < b.Key
+	}
+	if a.Operator != b.Operator {
+		return a.Operator < b.Operator
+	}
+	for i := range b.Values {
+		if i == len(a.Values) {
+			return true
+		}
+		if a.Values[i] != b.Values[i] {
+			return a.Values[i] < b.Values[i]
+		}
+	}
+	return false
+}
+
 // zeroOutDeploymentHash is a helper for hashableDeployment.
 func zeroOutDeploymentHash(labelSelector *metav1.LabelSelector) {
 	if labelSelector != nil {

diff --git a/pkg/operator/controller/ingress/deployment_test.go b/pkg/operator/controller/ingress/deployment_test.go
@@ -826,8 +826,8 @@ func TestDesiredRouterDeploymentSingleReplica(t *testing.T) {
 		t.Errorf("expected replicas to be 1, got %d", *deployment.Spec.Replicas)
 	}
 
-	if deployment.Spec.Template.Spec.Affinity != nil {
-		t.Errorf("expected no affinity, got %+v", *deployment.Spec.Template.Spec.Affinity)
+	if deployment.Spec.Template.Spec.Affinity.PodAffinity != nil {
+		t.Errorf("expected no pod affinity, got %+v", *deployment.Spec.Template.Spec.Affinity)
 	}
 
 	if deployment.Spec.Strategy.Type != "" || deployment.Spec.Strategy.RollingUpdate != nil {
@@ -1286,6 +1286,21 @@ func TestDeploymentConfigChanged(t *testing.T) {
 			},
 			expect: false,
 		},
+		{
+			description: "if the deployment template node affinity is changed",
+			mutate: func(deployment *appsv1.Deployment) {
+				deployment.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions[0].Values = []string{"xyz"}
+			},
+			expect: true,
+		},
+		{
+			description: "if the deployment template -affinity node selector expressions change ordering",
+			mutate: func(deployment *appsv1.Deployment) {
+				exprs := deployment.Spec.Template.Spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution[0].LabelSelector.MatchExpressions
+				exprs[0], exprs[1] = exprs[1], exprs[0]
+			},
+			expect: false,
+		},
 		{
 			description: "if probe values are set to default values",
 			mutate: func(deployment *appsv1.Deployment) {
@@ -1612,6 +1627,25 @@ func TestDeploymentConfigChanged(t *testing.T) {
 									},
 								},
 							},
+							NodeAffinity: &corev1.NodeAffinity{
+								RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{
+									NodeSelectorTerms: []corev1.NodeSelectorTerm{{
+										MatchExpressions: []corev1.NodeSelectorRequirement{
+											{
+												Key:      controller.RemoteWorkerLabel,
+												Operator: corev1.NodeSelectorOpNotIn,
+												Values:   []string{""},
+											},
+											// this match expression was added only for ordering change test case
+											{
+												Key:      controller.ControllerDeploymentHashLabel,
+												Operator: corev1.NodeSelectorOpIn,
+												Values:   []string{"1"},
+											},
+										},
+									}},
+								},
+							},
 						},
 						Tolerations: []corev1.Toleration{toleration, otherToleration},
 						TopologySpreadConstraints: []corev1.TopologySpreadConstraint{{

diff --git a/pkg/operator/controller/names.go b/pkg/operator/controller/names.go
@@ -40,6 +40,10 @@ const (
 	// DefaultCanaryNamespace is the default namespace for
 	// the ingress canary check resources.
 	DefaultCanaryNamespace = "openshift-ingress-canary"
+
+	// Remote worker label, used for node affinity of router deployment.
+	// Router should not run on remote worker nodes
+	RemoteWorkerLabel = "node.openshift.io/remote-worker"
 )
 
 // IngressClusterOperatorName returns the namespaced name of the ClusterOperator