From 8cb147920b8628e3be3dc0d7b8dd0b2c4c39b509 Mon Sep 17 00:00:00 2001 From: Marcus Rodan Date: Thu, 4 Feb 2021 22:06:29 +0100 Subject: [PATCH 1/3] Adds suffix for spread constraints Added unit test assertions Changed comment Signed-off-by: Marcus Rodan --- pkg/canary/deployment_controller.go | 5 +++++ pkg/canary/deployment_controller_test.go | 20 +++++++++++++++----- pkg/canary/deployment_fixture_test.go | 22 ++++++++++++++++++++++ 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go index 65afc7ca..c78a53f7 100644 --- 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 index e9019885..e516df2e 100644 --- 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 = depPrimary.Spec.Template.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution[1].PodAffinityTerm.LabelSelector.MatchExpressions[0].Values[0] + value = preferredConstraints[1].PodAffinityTerm.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] + 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.RequiredDuringSchedulingIgnoredDuringExecution[1].LabelSelector.MatchExpressions[0].Values[0] + value = requiredConstraints[1].LabelSelector.MatchExpressions[0].Values[0] + assert.False(t, strings.HasSuffix(value, "-primary")) + + topologySpreadConstraints := spec.TopologySpreadConstraints + value = topologySpreadConstraints[0].LabelSelector.MatchExpressions[0].Values[0] + assert.Equal(t, "podinfo-primary", value) + 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 index eb071eea..561d15fe 100644 --- 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"}, + }, + }, + }, + }, + }, }, }, }, From 0bebfbbb23e839ad1666a3397746388cbba34c05 Mon Sep 17 00:00:00 2001 From: Marcus Rodan Date: Mon, 8 Feb 2021 19:39:22 +0100 Subject: [PATCH 2/3] Added info to FAQ about anti-affinity Signed-off-by: Marcus Rodan --- docs/gitbook/faq.md | 72 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/docs/gitbook/faq.md b/docs/gitbook/faq.md index b252c3a8..2aa09f36 100644 --- 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,9 +316,7 @@ spec: labelSelector: matchLabels: affinity: podinfo - topologyKey: kubernetes.io/hostname -``` - + topologyKey: topology.kubernetes.io/zone ## Metrics #### How does Flagger measure the request success rate and duration? From 76b73a6319b06f7329f24a4acc6d2eab7a9c305d Mon Sep 17 00:00:00 2001 From: Marcus Rodan Date: Mon, 8 Feb 2021 19:41:16 +0100 Subject: [PATCH 3/3] Update faq.md Signed-off-by: Marcus Rodan --- docs/gitbook/faq.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/gitbook/faq.md b/docs/gitbook/faq.md index 2aa09f36..4bf4e9a3 100644 --- a/docs/gitbook/faq.md +++ b/docs/gitbook/faq.md @@ -317,6 +317,8 @@ spec: matchLabels: affinity: podinfo topologyKey: topology.kubernetes.io/zone +``` + ## Metrics #### How does Flagger measure the request success rate and duration?