From 4e7f152ac8241d8ea8ec23f027aa4da5eecf2222 Mon Sep 17 00:00:00 2001 From: Igal Tsoiref Date: Wed, 30 Nov 2022 22:57:11 +0200 Subject: [PATCH] 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) --- pkg/operator/controller/ingress/deployment.go | 43 +++++++++++++++++++ .../controller/ingress/deployment_test.go | 38 +++++++++++++++- pkg/operator/controller/names.go | 4 ++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/pkg/operator/controller/ingress/deployment.go b/pkg/operator/controller/ingress/deployment.go index e4e9bef81..9ac79367f 100644 --- 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 index 91139edb7..789420d87 100644 --- 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 index 095e78557..ef353bb5e 100644 --- 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