Skip to content

Commit

Permalink
Minor fixes
Browse files Browse the repository at this point in the history
Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>
  • Loading branch information
Rizwana777 committed Oct 22, 2024
1 parent 9ce0b8f commit 82ad855
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 11 deletions.
3 changes: 2 additions & 1 deletion api/v1alpha1/argorollouts_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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"`
Expand Down
6 changes: 5 additions & 1 deletion api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 9 additions & 5 deletions controllers/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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{
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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")
}

Expand Down
33 changes: 31 additions & 2 deletions controllers/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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"),
)
})

Expand Down Expand Up @@ -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{
Expand Down
13 changes: 13 additions & 0 deletions docs/crd_reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
1 change: 1 addition & 0 deletions tests/e2e/cluster-scoped/cluster_scoped_rollouts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {
Eventually(clusterRoleAdmin, "1m", "1s").ShouldNot((k8s.ExistByName(k8sClient)))
Eventually(clusterRoleView, "1m", "1s").ShouldNot((k8s.ExistByName(k8sClient)))
Eventually(clusterRoleEdit, "1m", "1s").ShouldNot((k8s.ExistByName(k8sClient)))

})
})
})
6 changes: 4 additions & 2 deletions tests/e2e/rollout_tests_all.go
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ func RunRolloutsTests(namespaceScopedParam bool) {
},
Spec: rolloutsmanagerv1alpha1.RolloutManagerSpec{
NamespaceScoped: namespaceScopedParam,
HA: rolloutsmanagerv1alpha1.RolloutManagerHASpec{
HA: &rolloutsmanagerv1alpha1.RolloutManagerHASpec{
Enabled: true,
},
},
Expand All @@ -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))
Expand Down

0 comments on commit 82ad855

Please sign in to comment.