Minor fixes

Signed-off-by: Rizwana777 <[email protected]>

api/v1alpha1/argorollouts_types.go

@@ -57,7 +57,7 @@ type RolloutManagerSpec struct {
 	Plugins Plugins `json:"plugins,omitempty"`
 
 	// HA options for High Availability support for Rollouts.
-	HA RolloutManagerHASpec `json:"ha,omitempty"`
+	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.
@@ -77,6 +77,7 @@ 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"`

controllers/deployment.go

@@ -45,7 +45,7 @@ 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.Enabled {
+	if cr.Spec.HA != nil && cr.Spec.HA.Enabled {
 		replicas = 2
 	}
 
@@ -70,7 +70,7 @@ func generateDesiredRolloutsDeployment(cr rolloutsmanagerv1alpha1.RolloutManager
 		},
 	}
 
-	if cr.Spec.HA.Enabled {
+	if cr.Spec.HA != nil && cr.Spec.HA.Enabled {
 		desiredDeployment.Spec.Template.Spec.Affinity = &corev1.Affinity{
 			PodAntiAffinity: &corev1.PodAntiAffinity{
 				PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{
@@ -431,7 +431,7 @@ func normalizeDeployment(inputParam appsv1.Deployment, cr rolloutsmanagerv1alpha
 
 	// Default number of replicas is 1, update it to 2 if HA is enabled
 	var replicas int32 = 1
-	if cr.Spec.HA.Enabled {
+	if cr.Spec.HA != nil && cr.Spec.HA.Enabled {
 		replicas = 2
 	}
 
@@ -461,6 +461,10 @@ func normalizeDeployment(inputParam appsv1.Deployment, cr rolloutsmanagerv1alpha
 		},
 	}
 
+	if input.Spec.Replicas == nil {
+		return appsv1.Deployment{}, fmt.Errorf("missing .spec.replicas")
+	}
+
 	if len(input.Spec.Template.Spec.Containers) != 1 {
 		return appsv1.Deployment{}, fmt.Errorf("incorrect number of .spec.template.spec.containers")
 	}
@@ -509,7 +513,7 @@ func normalizeDeployment(inputParam appsv1.Deployment, cr rolloutsmanagerv1alpha
 		inputContainer.Env = make([]corev1.EnvVar, 0)
 	}
 
-	if cr.Spec.HA.Enabled {
+	if input.Spec.Template.Spec.Affinity != nil && input.Spec.Template.Spec.Affinity.PodAffinity != nil {
 		res.Spec.Template.Spec.Affinity = &corev1.Affinity{
 			PodAntiAffinity: &corev1.PodAntiAffinity{
 				PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{
@@ -648,7 +652,7 @@ func getRolloutsCommandArgs(cr rolloutsmanagerv1alpha1.RolloutManager) []string
 		args = append(args, "--namespaced")
 	}
 
-	if cr.Spec.HA.Enabled {
+	if cr.Spec.HA != nil && cr.Spec.HA.Enabled {
 		args = append(args, "--leader-elect", "true")
 	}
 

controllers/deployment_test.go

@@ -339,6 +339,15 @@ var _ = Describe("Deployment Test", func() {
 				Expect(r.Client.Get(context.Background(), client.ObjectKeyFromObject(&updatedDeplFromClient), &updatedDeplFromClient)).To(Succeed())
 				Expect(areEqual(*updatedDepl, updatedDeplFromClient, a)).To(BeTrue(), "resource on cluster should match the resource we called Update with")
 
+				// The '.spec.replicas' value is not being set directly through the CR.
+				// Instead, we're applying the default value of 2.
+				// This 'if' condition checks whether the replicas value is set to 2 when HA is enabled.
+				if a.Spec.HA != nil && a.Spec.HA.Enabled {
+					Expect(areEqual(*updatedDepl, updatedDeplFromClient, a)).To(BeTrue())
+					// Return here to skip further steps in the test for '.spec.replicas' entry
+					return
+				}
+
 				Expect(areEqual(updatedDeplFromClient, expectedDepl, a)).ToNot(BeTrue(), "resource on cluster should NOT match the original Deployment that was created by the call to reconcileRolloutsDeployment")
 
 				By("calling reconcileRolloutsDeployment again, it should revert the change back to default")
@@ -416,12 +425,28 @@ var _ = Describe("Deployment Test", func() {
 					},
 				}
 			}),
+			Entry(".spec.replicas", func(deployment *appsv1.Deployment) {
+				a.Spec = v1alpha1.RolloutManagerSpec{
+					HA: &v1alpha1.RolloutManagerHASpec{
+						Enabled: true,
+					},
+				}
+				Expect(r.Client.Update(context.Background(), &a)).To(Succeed())
+				var replicas int32
+				deployment.Spec.Replicas = &replicas
+			}),
 		)
 
 		It("should contain two replicas and the --leader-elect argument set to true, and verify that the anti-affinity rule is added by default when HA is enabled", func() {
-			a.Spec.HA.Enabled = true
+			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())
 
@@ -642,6 +667,10 @@ var _ = Describe("normalizeDeployment tests to verify that an error is returned"
 				{Name: "volume1", MountPath: "/mnt/volume1"},
 			}
 		}, "incorrect volume mounts"),
+
+		Entry("Spec.Replicas is nil", func() {
+			deployment.Spec.Replicas = nil
+		}, "missing .spec.replicas"),
 	)
 })
 
@@ -767,7 +796,7 @@ func deploymentCR(name string, namespace string, rolloutsSelectorLabel string, v
 	}
 	setRolloutsLabelsAndAnnotationsToObject(&deploymentCR.ObjectMeta, rolloutManager)
 	replicas := int32(1)
-	if rolloutManager.Spec.HA.Enabled {
+	if rolloutManager.Spec.HA != nil && rolloutManager.Spec.HA.Enabled {
 		replicas = 2
 	}
 	deploymentCR.Spec = appsv1.DeploymentSpec{

docs/crd_reference.md

@@ -152,4 +152,17 @@ 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
 ```

tests/e2e/cluster-scoped/cluster_scoped_rollouts_test.go

@@ -389,5 +389,6 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {
 			Eventually(clusterRoleView, "1m", "1s").ShouldNot((k8s.ExistByName(k8sClient)))
 			Eventually(clusterRoleEdit, "1m", "1s").ShouldNot((k8s.ExistByName(k8sClient)))
 		})
+
 	})
 })

tests/e2e/rollout_tests_all.go

@@ -794,7 +794,7 @@ func RunRolloutsTests(namespaceScopedParam bool) {
 				},
 				Spec: rolloutsmanagerv1alpha1.RolloutManagerSpec{
 					NamespaceScoped: namespaceScopedParam,
-					HA: rolloutsmanagerv1alpha1.RolloutManagerHASpec{
+					HA: &rolloutsmanagerv1alpha1.RolloutManagerHASpec{
 						Enabled: true,
 					},
 				},
@@ -809,7 +809,9 @@ func RunRolloutsTests(namespaceScopedParam bool) {
 				},
 			}
 
-			Eventually(&depl, "30s", "1s").Should(k8s.ExistByName(k8sClient))
+			// In this test we don't check whether RolloutManager has phase: Available, and we don't check if Deployment is ready.
+			// This is because our E2E tests run in a single node cluster, which prevents HA deployments from being fully scheduled.
+			Eventually(&depl, "60s", "1s").Should(k8s.ExistByName(k8sClient))
 
 			replicas := int32(2)
 			Expect(depl.Spec.Replicas).To(Equal(&replicas))