Add HA support for Argo Rollouts (#93)

* Fix conflicts Signed-off-by: Rizwana777 <[email protected]> * Add HA support for argo-rollouts Signed-off-by: Rizwana777 <[email protected]> * Fix merge conflicts 2 Signed-off-by: Rizwana777 <[email protected]> * Minor fixes Signed-off-by: Rizwana777 <[email protected]> * Add logic that directly handles affinity field to code/tests Signed-off-by: Jonathan West <[email protected]> --------- Signed-off-by: Rizwana777 <[email protected]> Signed-off-by: Jonathan West <[email protected]> Co-authored-by: Jonathan West <[email protected]>
argoproj-labs · Oct 23, 2024 · fb79499 · fb79499
1 parent 06589e6
commit fb79499
Show file tree

Hide file tree

Showing 8 changed files with 301 additions and 1 deletion.
diff --git a/api/v1alpha1/argorollouts_types.go b/api/v1alpha1/argorollouts_types.go
@@ -55,6 +55,9 @@ type RolloutManagerSpec struct {
 
 	// Plugins specify the traffic and metric plugins in Argo Rollout
 	Plugins Plugins `json:"plugins,omitempty"`
+
+	// HA options for High Availability support for Rollouts.
+	HA *RolloutManagerHASpec `json:"ha,omitempty"`
 }
 
 // Plugin is used to integrate traffic management and metric plugins into the Argo Rollouts controller. For more information on these plugins, see the upstream Argo Rollouts documentation.
@@ -74,6 +77,12 @@ type Plugins struct {
 	Metric []Plugin `json:"metric,omitempty"`
 }
 
+// RolloutManagerHASpec specifies HA options for High Availability support for Rollouts.
+type RolloutManagerHASpec struct {
+	// Enabled will toggle HA support globally for RolloutManager.
+	Enabled bool `json:"enabled"`
+}
+
 // ArgoRolloutsNodePlacementSpec is used to specify NodeSelector and Tolerations for Rollouts workloads
 type RolloutsNodePlacementSpec struct {
 	// NodeSelector is a field of PodSpec, it is a map of key value pairs used for node selection

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/argoproj.io_rolloutmanagers.yaml b/config/crd/bases/argoproj.io_rolloutmanagers.yaml
@@ -228,6 +228,15 @@ spec:
                 items:
                   type: string
                 type: array
+              ha:
+                description: HA options for High Availability support for Rollouts.
+                properties:
+                  enabled:
+                    description: Enabled will toggle HA support globally for RolloutManager.
+                    type: boolean
+                required:
+                - enabled
+                type: object
               image:
                 description: Image defines Argo Rollouts controller image (optional)
                 type: string

diff --git a/controllers/default.go b/controllers/default.go
@@ -38,4 +38,8 @@ const (
 
 	// ClusterScopedArgoRolloutsNamespaces is an environment variable that can be used to configure namespaces that are allowed to host cluster-scoped Argo Rollouts
 	ClusterScopedArgoRolloutsNamespaces = "CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES"
+
+	KubernetesHostnameLabel = "kubernetes.io/hostname"
+
+	TopologyKubernetesZoneLabel = "topology.kubernetes.io/zone"
 )
diff --git a/controllers/deployment.go b/controllers/deployment.go
@@ -43,7 +43,14 @@ func generateDesiredRolloutsDeployment(cr rolloutsmanagerv1alpha1.RolloutManager
 		}
 	}
 
+	// Default number of replicas is 1, update it to 2 if HA is enabled
+	var replicas int32 = 1
+	if cr.Spec.HA != nil && cr.Spec.HA.Enabled {
+		replicas = 2
+	}
+
 	desiredDeployment.Spec = appsv1.DeploymentSpec{
+		Replicas: &replicas,
 		Selector: &metav1.LabelSelector{
 			MatchLabels: labels,
 		},
@@ -63,6 +70,32 @@ func generateDesiredRolloutsDeployment(cr rolloutsmanagerv1alpha1.RolloutManager
 		},
 	}
 
+	if cr.Spec.HA != nil && cr.Spec.HA.Enabled {
+		desiredDeployment.Spec.Template.Spec.Affinity = &corev1.Affinity{
+			PodAntiAffinity: &corev1.PodAntiAffinity{
+				PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{
+					{
+						PodAffinityTerm: corev1.PodAffinityTerm{
+							LabelSelector: &metav1.LabelSelector{
+								MatchLabels: labels,
+							},
+							TopologyKey: TopologyKubernetesZoneLabel,
+						},
+						Weight: int32(100),
+					},
+				},
+				RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{
+					{
+						LabelSelector: &metav1.LabelSelector{
+							MatchLabels: labels,
+						},
+						TopologyKey: KubernetesHostnameLabel,
+					},
+				},
+			},
+		}
+	}
+
 	if cr.Spec.NodePlacement != nil {
 		desiredDeployment.Spec.Template.Spec.NodeSelector = appendStringMap(
 			desiredDeployment.Spec.Template.Spec.NodeSelector, cr.Spec.NodePlacement.NodeSelector)
@@ -159,6 +192,9 @@ func (r *RolloutManagerReconciler) reconcileRolloutsDeployment(ctx context.Conte
 		actualDeployment.Spec.Template.Spec.Containers = desiredDeployment.Spec.Template.Spec.Containers
 		actualDeployment.Spec.Template.Spec.ServiceAccountName = desiredDeployment.Spec.Template.Spec.ServiceAccountName
 
+		actualDeployment.Spec.Replicas = desiredDeployment.Spec.Replicas
+		actualDeployment.Spec.Template.Spec.Affinity = desiredDeployment.Spec.Template.Spec.Affinity
+
 		actualDeployment.Labels = combineStringMaps(actualDeployment.Labels, desiredDeployment.Labels)
 		actualDeployment.Annotations = combineStringMaps(actualDeployment.Annotations, desiredDeployment.Annotations)
 
@@ -236,6 +272,14 @@ func identifyDeploymentDifference(x appsv1.Deployment, y appsv1.Deployment) stri
 		return "Spec.Template.Spec.Volumes"
 	}
 
+	if !reflect.DeepEqual(x.Spec.Replicas, y.Spec.Replicas) {
+		return "Spec.Replicas"
+	}
+
+	if !reflect.DeepEqual(x.Spec.Template.Spec.Affinity, y.Spec.Template.Spec.Affinity) {
+		return "Spec.Template.Spec.Affinity"
+	}
+
 	return ""
 }
 
@@ -389,7 +433,14 @@ func normalizeDeployment(inputParam appsv1.Deployment, cr rolloutsmanagerv1alpha
 		return appsv1.Deployment{}, fmt.Errorf("missing .spec.template.spec.volumes")
 	}
 
+	// Default number of replicas is 1
+	var replicas int32 = 1
+	if input.Spec.Replicas != nil {
+		replicas = *input.Spec.Replicas
+	}
+
 	res.Spec = appsv1.DeploymentSpec{
+		Replicas: &replicas,
 		Selector: &metav1.LabelSelector{
 			MatchLabels: normalizeMap(input.Spec.Selector.MatchLabels),
 		},
@@ -462,6 +513,48 @@ func normalizeDeployment(inputParam appsv1.Deployment, cr rolloutsmanagerv1alpha
 		inputContainer.Env = make([]corev1.EnvVar, 0)
 	}
 
+	if input.Spec.Template.Spec.Affinity != nil {
+
+		res.Spec.Template.Spec.Affinity = &corev1.Affinity{}
+
+		// As of this writing, we don't touch pod affinity field at at all, so just copy it as is: any changes to this field should be reverted.
+		res.Spec.Template.Spec.Affinity.PodAffinity = input.Spec.Template.Spec.Affinity.PodAffinity
+
+		// We do touch anti affinity field, so check it then copy it into res
+		if input.Spec.Template.Spec.Affinity.PodAntiAffinity != nil {
+
+			if len(input.Spec.Template.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution) != 1 {
+				return appsv1.Deployment{}, fmt.Errorf("incorrect number of anti-affinity PreferredDuringSchedulingIgnoredDuringExecution")
+			}
+
+			if len(input.Spec.Template.Spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution) != 1 {
+				return appsv1.Deployment{}, fmt.Errorf("incorrect number of anti-affinity RequiredDuringSchedulingIgnoredDuringExecution")
+			}
+
+			res.Spec.Template.Spec.Affinity.PodAntiAffinity = &corev1.PodAntiAffinity{
+				PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{
+					{
+						PodAffinityTerm: corev1.PodAffinityTerm{
+							LabelSelector: &metav1.LabelSelector{
+								MatchLabels: normalizeMap(input.Spec.Selector.MatchLabels),
+							},
+							TopologyKey: input.Spec.Template.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution[0].PodAffinityTerm.TopologyKey,
+						},
+						Weight: input.Spec.Template.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution[0].Weight,
+					},
+				},
+				RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{
+					{
+						LabelSelector: &metav1.LabelSelector{
+							MatchLabels: normalizeMap(input.Spec.Selector.MatchLabels),
+						},
+						TopologyKey: input.Spec.Template.Spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution[0].TopologyKey,
+					},
+				},
+			}
+		}
+	}
+
 	res.Spec.Template.Spec.Containers = []corev1.Container{{
 		Args:            inputContainer.Args,
 		Env:             inputContainer.Env,
@@ -575,6 +668,10 @@ func getRolloutsCommandArgs(cr rolloutsmanagerv1alpha1.RolloutManager) []string
 		args = append(args, "--namespaced")
 	}
 
+	if cr.Spec.HA != nil && cr.Spec.HA.Enabled {
+		args = append(args, "--leader-elect", "true")
+	}
+
 	extraArgs := cr.Spec.ExtraCommandArgs
 	err := isMergable(extraArgs, args)
 	if err != nil {

diff --git a/controllers/deployment_test.go b/controllers/deployment_test.go
@@ -416,10 +416,72 @@ var _ = Describe("Deployment Test", func() {
 					},
 				}
 			}),
+			Entry(".spec.replicas", func(deployment *appsv1.Deployment) {
+				var replicas int32 = 2
+				deployment.Spec.Replicas = &replicas
+			}),
+			Entry(".spec.template.spec.affinity.podantiaffinity", func(deployment *appsv1.Deployment) {
+				deployment.Spec.Template.Spec.Affinity = &corev1.Affinity{
+					PodAntiAffinity: &corev1.PodAntiAffinity{
+						RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{
+							{TopologyKey: "TopologyKey"},
+						},
+						PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{
+							{PodAffinityTerm: corev1.PodAffinityTerm{TopologyKey: "ToplogyKey"}},
+						},
+					},
+				}
+			}),
 		)
-
 	})
 
+	When("HA is enabled via RolloutManager .spec.HA", func() {
+
+		It("should have a Deployment that contains two replicas and the --leader-elect argument, and it should have anti-affinity rule is added by default when HA is enabled", func() {
+
+			a.Spec = v1alpha1.RolloutManagerSpec{
+				HA: &v1alpha1.RolloutManagerHASpec{
+					Enabled: true,
+				},
+			}
+			replicas := int32(2)
+
+			Expect(r.Client.Update(ctx, &a)).To(Succeed())
+
+			By("calling reconcileRolloutsDeployment to create the initial set of rollout resources")
+			Expect(r.reconcileRolloutsDeployment(ctx, a, *sa)).To(Succeed())
+
+			By("fetch the Deployment")
+			fetchedDeployment := &appsv1.Deployment{}
+			Expect(fetchObject(ctx, r.Client, a.Namespace, DefaultArgoRolloutsResourceName, fetchedDeployment)).To(Succeed())
+
+			expectedDeployment := deploymentCR(DefaultArgoRolloutsResourceName, a.Namespace, DefaultArgoRolloutsResourceName, []string{"plugin-bin", "tmp"}, "linux", DefaultArgoRolloutsResourceName, a)
+
+			By("verify that the fetched Deployment matches the desired one")
+			Expect(fetchedDeployment.Name).To(Equal(expectedDeployment.Name))
+			Expect(fetchedDeployment.Labels).To(Equal(expectedDeployment.Labels))
+			Expect(fetchedDeployment.Spec.Replicas).To(Equal(&replicas))
+			Expect(fetchedDeployment.Spec.Template.Spec.Containers[0].Args).To(ContainElements("--leader-elect", "true"))
+
+			By("verifying that the anti-affinity rules are set correctly")
+			affinity := fetchedDeployment.Spec.Template.Spec.Affinity
+			Expect(affinity).NotTo(BeNil())
+			Expect(affinity.PodAntiAffinity).NotTo(BeNil())
+
+			By("Verify PreferredDuringSchedulingIgnoredDuringExecution")
+			preferred := affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution
+			Expect(preferred).To(HaveLen(1))
+			Expect(preferred[0].Weight).To(Equal(int32(100)))
+			Expect(preferred[0].PodAffinityTerm.TopologyKey).To(Equal(TopologyKubernetesZoneLabel))
+			Expect(preferred[0].PodAffinityTerm.LabelSelector.MatchLabels).To(Equal(normalizeMap(fetchedDeployment.Spec.Selector.MatchLabels)))
+
+			By("Verify RequiredDuringSchedulingIgnoredDuringExecution")
+			required := affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution
+			Expect(required).To(HaveLen(1))
+			Expect(required[0].TopologyKey).To(Equal(KubernetesHostnameLabel))
+			Expect(required[0].LabelSelector.MatchLabels).To(Equal(normalizeMap(fetchedDeployment.Spec.Selector.MatchLabels)))
+		})
+	})
 })
 
 var _ = Describe("generateDesiredRolloutsDeployment tests", func() {
@@ -604,6 +666,34 @@ var _ = Describe("normalizeDeployment tests to verify that an error is returned"
 				{Name: "volume1", MountPath: "/mnt/volume1"},
 			}
 		}, "incorrect volume mounts"),
+
+		Entry("input.Spec.Template.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution has incorrect length: if pod anti affinity is set, it should have only 1 PreferredDuringSchedulingIgnoredDuringExecution", func() {
+			deployment.Spec.Template.Spec.Affinity = &corev1.Affinity{
+				PodAntiAffinity: &corev1.PodAntiAffinity{
+					PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{{
+						PodAffinityTerm: corev1.PodAffinityTerm{TopologyKey: "PodAffinityTerm1"},
+					}, {
+						PodAffinityTerm: corev1.PodAffinityTerm{TopologyKey: "PodAffinityTerm2"},
+					}},
+				},
+			}
+		}, "incorrect number of anti-affinity PreferredDuringSchedulingIgnoredDuringExecution"),
+
+		Entry("input.Spec.Template.Spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution has incorrect length: if pod anti affinity is set, it should have only 1 RequiredDuringSchedulingIgnoredDuringExecution", func() {
+			deployment.Spec.Template.Spec.Affinity = &corev1.Affinity{
+				PodAntiAffinity: &corev1.PodAntiAffinity{
+
+					PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{{
+						PodAffinityTerm: corev1.PodAffinityTerm{TopologyKey: "PodAffinityTerm1"}, // this is valid
+					}},
+
+					RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{ // too many
+						{TopologyKey: "TopologyKey1"},
+						{TopologyKey: "TopologyKey2"},
+					},
+				},
+			}
+		}, "incorrect number of anti-affinity RequiredDuringSchedulingIgnoredDuringExecution"),
 	)
 })
 
@@ -728,7 +818,12 @@ func deploymentCR(name string, namespace string, rolloutsSelectorLabel string, v
 		},
 	}
 	setRolloutsLabelsAndAnnotationsToObject(&deploymentCR.ObjectMeta, rolloutManager)
+	replicas := int32(1)
+	if rolloutManager.Spec.HA != nil && rolloutManager.Spec.HA.Enabled {
+		replicas = 2
+	}
 	deploymentCR.Spec = appsv1.DeploymentSpec{
+		Replicas: &replicas,
 		Selector: &metav1.LabelSelector{
 			MatchLabels: map[string]string{
 				DefaultRolloutsSelectorKey: rolloutsSelectorLabel,

diff --git a/docs/crd_reference.md b/docs/crd_reference.md
@@ -152,4 +152,19 @@ spec:
       - name: "argoproj-labs/sample-prometheus"
         location: https://github.com/argoproj-labs/sample-rollouts-metric-plugin/releases/download/v0.0.3/metric-plugin-linux-amd64
         sha256: a597a017a9a1394a31b3cbc33e08a071c88f0bd8
+```
+
+
+### RolloutManager example with HA enabled
+
+``` yaml
+apiVersion: argoproj.io/v1alpha1
+kind: RolloutManager
+metadata:
+  name: argo-rollout
+  labels:
+    example: with-ha
+spec:
+  ha:
+    enabled: true
 ```