Merge pull request #806 from Nerja/fixedspreadconstraintssuffixes

Rewrite the primary Pod Topology Spread Constraints based on label selector
fluxcd · Feb 13, 2021 · 10b5504 · 10b5504
2 parents 5cb343d + 76b73a6
commit 10b5504
Show file tree

Hide file tree

Showing 4 changed files with 109 additions and 8 deletions.
diff --git a/docs/gitbook/faq.md b/docs/gitbook/faq.md
@@ -224,10 +224,74 @@ If you use a different convention you can specify your label with the `-selector
 
 #### Is pod affinity and anti affinity supported?
 
-For pod affinity to work you need to use a different label than the `app`, `name` or `app.kubernetes.io/name`.
+Flagger will rewrite the first value in each match expression, defined in the target deployment's pod anti-affinity and topology spread constraints, satisfying the following two requirements when creating, or updating, the primary deployment:
 
-Anti affinity example:
+* The key in the match expression must be one of the labels specified by the parameter selector-labels. The default labels are `app`,`name`,`app.kubernetes.io/name`.
+* The value must match the name of the target deployment.
 
+The rewrite done by Flagger in these cases is to suffix the value with "-primary". This rewrite can be used to spread the pods created by the canary and primary deployments across different availability zones.
+
+Example target deployment:
+```yaml
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: podinfo
+spec:
+  selector:
+    matchLabels:
+      app: podinfo
+  template:
+    metadata:
+      labels:
+        app: podinfo
+    spec:
+      affinity:
+        podAntiAffinity:
+          preferredDuringSchedulingIgnoredDuringExecution:
+          - weight: 100
+            podAffinityTerm:
+              labelSelector:
+                matchExpressions:
+                - key: app
+                  operator: In
+                  values:
+                    - podinfo
+              topologyKey: topology.kubernetes.io/zone
+```
+
+Example of generated primary deployment:
+```yaml
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: podinfo-primary
+spec:
+  selector:
+    matchLabels:
+      app: podinfo-primary
+  template:
+    metadata:
+      labels:
+        app: podinfo-primary
+    spec:
+      affinity:
+        podAntiAffinity:
+          preferredDuringSchedulingIgnoredDuringExecution:
+          - weight: 100
+            podAffinityTerm:
+              labelSelector:
+                matchExpressions:
+                - key: app
+                  operator: In
+                  values:
+                    - podinfo-primary
+              topologyKey: topology.kubernetes.io/zone
+```
+
+It is also possible to use a different label than the `app`, `name` or `app.kubernetes.io/name`.
+
+Anti affinity example(using a different label):
 ```yaml
 apiVersion: apps/v1
 kind: Deployment
@@ -252,7 +316,7 @@ spec:
               labelSelector:
                 matchLabels:
                   affinity: podinfo
-              topologyKey: kubernetes.io/hostname
+              topologyKey: topology.kubernetes.io/zone
 ```
 
 ## Metrics

diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go
@@ -456,6 +456,11 @@ func (c *DeploymentController) scale(cd *flaggerv1.Canary, replicas int32) error
 func (c *DeploymentController) getPrimaryDeploymentTemplateSpec(canaryDep *appsv1.Deployment, refs map[string]ConfigRef) corev1.PodSpec {
 	spec := c.configTracker.ApplyPrimaryConfigs(canaryDep.Spec.Template.Spec, refs)
 
+	// update TopologySpreadConstraints
+	for _, topologySpreadConstraint := range spec.TopologySpreadConstraints {
+		c.appendPrimarySuffixToValuesIfNeeded(topologySpreadConstraint.LabelSelector, canaryDep)
+	}
+
 	// update affinity
 	if affinity := spec.Affinity; affinity != nil {
 		if podAntiAffinity := affinity.PodAntiAffinity; podAntiAffinity != nil {

diff --git a/pkg/canary/deployment_controller_test.go b/pkg/canary/deployment_controller_test.go
@@ -264,7 +264,7 @@ func TestDeploymentController_Finalize(t *testing.T) {
 	}
 }
 
-func TestDeploymentController_AntiAffinity(t *testing.T) {
+func TestDeploymentController_AntiAffinityAndTopologySpreadConstraints(t *testing.T) {
 	t.Run("deployment", func(t *testing.T) {
 		dc := deploymentConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
 		mocks := newDeploymentFixture(dc)
@@ -273,14 +273,24 @@ func TestDeploymentController_AntiAffinity(t *testing.T) {
 		depPrimary, err := mocks.kubeClient.AppsV1().Deployments("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
 		require.NoError(t, err)
 
-		value := depPrimary.Spec.Template.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution[0].PodAffinityTerm.LabelSelector.MatchExpressions[0].Values[0]
+		spec := depPrimary.Spec.Template.Spec
+
+		preferredConstraints := spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution
+		value := preferredConstraints[0].PodAffinityTerm.LabelSelector.MatchExpressions[0].Values[0]
+		assert.Equal(t, "podinfo-primary", value)
+		value = preferredConstraints[1].PodAffinityTerm.LabelSelector.MatchExpressions[0].Values[0]
+		assert.False(t, strings.HasSuffix(value, "-primary"))
+
+		requiredConstraints := spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution
+		value = requiredConstraints[0].LabelSelector.MatchExpressions[0].Values[0]
 		assert.Equal(t, "podinfo-primary", value)
-		value = depPrimary.Spec.Template.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution[1].PodAffinityTerm.LabelSelector.MatchExpressions[0].Values[0]
+		value = requiredConstraints[1].LabelSelector.MatchExpressions[0].Values[0]
 		assert.False(t, strings.HasSuffix(value, "-primary"))
 
-		value = depPrimary.Spec.Template.Spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution[0].LabelSelector.MatchExpressions[0].Values[0]
+		topologySpreadConstraints := spec.TopologySpreadConstraints
+		value = topologySpreadConstraints[0].LabelSelector.MatchExpressions[0].Values[0]
 		assert.Equal(t, "podinfo-primary", value)
-		value = depPrimary.Spec.Template.Spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution[1].LabelSelector.MatchExpressions[0].Values[0]
+		value = topologySpreadConstraints[1].LabelSelector.MatchExpressions[0].Values[0]
 		assert.False(t, strings.HasSuffix(value, "-primary"))
 	})
 }
diff --git a/pkg/canary/deployment_fixture_test.go b/pkg/canary/deployment_fixture_test.go
@@ -616,6 +616,28 @@ func newDeploymentControllerTest(dc deploymentConfigs) *appsv1.Deployment {
 							},
 						},
 					},
+					TopologySpreadConstraints: []corev1.TopologySpreadConstraint{
+						{
+							LabelSelector: &metav1.LabelSelector{
+								MatchExpressions: []metav1.LabelSelectorRequirement{
+									{
+										Key:    "app",
+										Values: []string{"podinfo"},
+									},
+								},
+							},
+						},
+						{
+							LabelSelector: &metav1.LabelSelector{
+								MatchExpressions: []metav1.LabelSelectorRequirement{
+									{
+										Key:    "app",
+										Values: []string{"arbitrary-app"},
+									},
+								},
+							},
+						},
+					},
 				},
 			},
 		},