Skip to content

Add e2e ginkgo tests for extraCommandargs and Application Health Status #923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jgwest merged 1 commit into from
Sep 25, 2025
Merged

Add e2e ginkgo tests for extraCommandargs and Application Health Status #923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 121 additions & 27 deletions test/openshift/e2e/ginkgo/parallel/1-115_validate_controller_extra_command_args_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() {

})

It("ensuring that extra arguments can be added to application controller", func() {

It("ensures extra arguments are deduplicated, replaced, or preserved as expected in application-controller", func() {
By("creating a simple ArgoCD CR and waiting for it to become available")
ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()
defer cleanupFunc()
Expand All @@ -62,61 +61,156 @@ var _ = Describe("GitOps Operator Parallel E2E Tests", func() {
},
}
Expect(k8sClient.Create(ctx, argoCD)).To(Succeed())

Eventually(argoCD, "5m", "5s").Should(argocdFixture.BeAvailable())

By("verifying app controller becomes availables")
appControllerSS := &appsv1.StatefulSet{
ObjectMeta: metav1.ObjectMeta{
Name: "example-argocd-application-controller",
Namespace: ns.Name,
},
}

Eventually(appControllerSS).Should(k8sFixture.ExistByName())
Eventually(appControllerSS).Should(statefulsetFixture.HaveReadyReplicas(1))

By("adding a new parameter via .spec.controller.extraCommandArgs")
// Verify default values of --status-processors and --kubectl-parallelism-limit
Eventually(appControllerSS).Should(statefulsetFixture.HaveContainerCommandSubstring("--status-processors", 0))
Eventually(appControllerSS).Should(statefulsetFixture.HaveContainerCommandSubstring("20", 0))
Eventually(appControllerSS).Should(statefulsetFixture.HaveContainerCommandSubstring("--kubectl-parallelism-limit", 0))
Eventually(appControllerSS).Should(statefulsetFixture.HaveContainerCommandSubstring("10", 0))

// 1: Add new flag
By("adding a new flag via extraCommandArgs")
argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) {
ac.Spec.Controller.ExtraCommandArgs = []string{"--app-hard-resync"}
ac.Spec.Controller.ExtraCommandArgs = []string{"--app-hard-resync", "2"}
})

By("verifying new parameter is added, and the existing paramaters are still present")
Eventually(appControllerSS).Should(statefulsetFixture.HaveContainerCommandSubstring("--app-hard-resync", 0))

Expect(len(appControllerSS.Spec.Template.Spec.Containers[0].Command)).To(BeNumerically(">=", 10))

By("removing the extra command arg")
// 2: Replace existing non-repeatable flags --status-processors and --kubectl-parallelism-limit
By("replacing existing default flag with extraCommandArgs")
argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) {
ac.Spec.Controller.ExtraCommandArgs = nil
ac.Spec.Controller.ExtraCommandArgs = []string{
"--status-processors", "15",
"--kubectl-parallelism-limit", "20",
}
})

By("verifying the parameter has been removed")
By("new values should appear for --status-processors and --kubectl-parallelism-limit")
Eventually(appControllerSS).Should(statefulsetFixture.HaveContainerCommandSubstring("--status-processors", 0))
Eventually(appControllerSS).Should(statefulsetFixture.HaveContainerCommandSubstring("15", 0))
Eventually(appControllerSS).Should(statefulsetFixture.HaveContainerCommandSubstring("--kubectl-parallelism-limit", 0))
Eventually(appControllerSS).Should(statefulsetFixture.HaveContainerCommandSubstring("20", 0))
Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveContainerCommandSubstring("--app-hard-resync", 0))
Consistently(appControllerSS).ShouldNot(statefulsetFixture.HaveContainerCommandSubstring("--app-hard-resync", 0))
Expect(len(appControllerSS.Spec.Template.Spec.Containers[0].Command)).To(BeNumerically(">=", 10))

By("adding a new extra command arg that has the same name as existing parameters")
By("default values should be replaced (old default for --status-processors 20 and --kubectl-parallelism-limit 10 should not appear")
Consistently(func() bool {
Expect(k8sClient.Get(ctx, client.ObjectKey{
Name: appControllerSS.Name,
Namespace: appControllerSS.Namespace,
}, appControllerSS)).To(Succeed())

cmd := appControllerSS.Spec.Template.Spec.Containers[0].Command
Copy link
Member

@jgwest jgwest Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the top of this Consistently, you will need to call k8sClient.Get(ctx, (...), appControllerSS) to ensure you have the latest copy from K8s. Otherwise you are just comparing the same Go object over and over again (which is not useful)

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have addressed this comment, i have added k8sClient.Get(ctx, (...), appControllerSS) above, https://github.com/redhat-developer/gitops-operator/pull/923/files#diff-4eeec8e6b675c5a0710f157abb0ba91787c32219b01be28922eb7f8d1d6a138cR104

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

for i := range cmd {
if cmd[i] == "--status-processors" && i+1 < len(cmd) && cmd[i+1] == "20" {
return true
}
if cmd[i] == "--kubectl-parallelism-limit" && i+1 < len(cmd) && cmd[i+1] == "10" {
return true
}
}
return false
}).Should(BeFalse())

// 3: Add duplicate flag+value pairs, which should be ignored
By("adding duplicate flags with same values")
argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) {
ac.Spec.Controller.ExtraCommandArgs = []string{
"--status-processors",
"15",
"--kubectl-parallelism-limit",
"20",
"--status-processors", "15",
"--kubectl-parallelism-limit", "20",
"--status-processors", "15", // duplicate
"--kubectl-parallelism-limit", "20", // duplicate
"--hydrator-enabled",
}
})
// Verify --hydrator-enabled gets added
Eventually(appControllerSS).Should(statefulsetFixture.HaveContainerCommandSubstring("--hydrator-enabled", 0))

// But no duplicate --status-processors or --kubectl-parallelism-limit
Consistently(func() bool {
Expect(k8sClient.Get(ctx, client.ObjectKey{
Name: appControllerSS.Name,
Namespace: appControllerSS.Namespace,
}, appControllerSS)).To(Succeed())

cmd := appControllerSS.Spec.Template.Spec.Containers[0].Command
Copy link
Member

@jgwest jgwest Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, need to k8sClient.Get at top of func

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/redhat-developer/gitops-operator/pull/923/files#diff-4eeec8e6b675c5a0710f157abb0ba91787c32219b01be28922eb7f8d1d6a138cR137

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


statusProcessorsCount := 0
kubectlLimitCount := 0

for i := 0; i < len(cmd); i++ {
if cmd[i] == "--status-processors" {
statusProcessorsCount++
}
if cmd[i] == "--kubectl-parallelism-limit" {
kubectlLimitCount++
}
}

// TODO: These lines are currently failing: they are ported correctly from the original kuttl test, but the original kuttl test did not check them correctly (and thus either the behaviour in the operator changed, or the tests never worked)
// Fail if either flag appears more than once
return statusProcessorsCount > 1 || kubectlLimitCount > 1
}).Should(BeFalse())

// Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveContainerCommandSubstring("--status-processors 15", 0))
// 4: Add a repeatable flag multiple times with different values
By("adding a repeatable flag with multiple values")
argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) {
ac.Spec.Controller.ExtraCommandArgs = []string{
"--metrics-application-labels", "application.argoproj.io/template-version",
"--metrics-application-labels", "application.argoproj.io/chart-version",
}
})

// Consistently(appControllerSS).ShouldNot(statefulsetFixture.HaveContainerCommandSubstring("--status-processors 15", 0))
Eventually(appControllerSS).Should(statefulsetFixture.HaveContainerCommandSubstring("--metrics-application-labels", 0))

// Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveContainerCommandSubstring("--kubectl-parallelism-limit 20", 0))
By("Check that both --metrics-application-labels flags are present")
Eventually(func() bool {
Expect(k8sClient.Get(ctx, client.ObjectKey{
Name: appControllerSS.Name,
Namespace: appControllerSS.Namespace,
}, appControllerSS)).To(Succeed())

// Consistently(appControllerSS).ShouldNot(statefulsetFixture.HaveContainerCommandSubstring("--kubectl-parallelism-limit 20", 0))
cmd := appControllerSS.Spec.Template.Spec.Containers[0].Command
Copy link
Member

@jgwest jgwest Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, need to k8sClient.Get at top

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/redhat-developer/gitops-operator/pull/923/files#diff-4eeec8e6b675c5a0710f157abb0ba91787c32219b01be28922eb7f8d1d6a138cR173

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


})
metricVals := []string{}
for i := 0; i < len(cmd); i++ {
if cmd[i] == "--metrics-application-labels" && i+1 < len(cmd) {
metricVals = append(metricVals, cmd[i+1])
}
}

// Ensure both values are present
hasMetricLabelTemplate := false
hasMetricLabelChart := false
for _, v := range metricVals {
if v == "application.argoproj.io/template-version" {
hasMetricLabelTemplate = true
}
if v == "application.argoproj.io/chart-version" {
hasMetricLabelChart = true
}
}
return hasMetricLabelTemplate && hasMetricLabelChart
}).Should(BeTrue())

// 5: Remove all extra args
By("removing all extra args")
argocdFixture.Update(argoCD, func(ac *argov1beta1api.ArgoCD) {
ac.Spec.Controller.ExtraCommandArgs = nil
})

// Expect all custom flags to disappear
Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveContainerCommandSubstring("--metrics-application-labels", 0))
Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveContainerCommandSubstring("--status-processors 15", 0))
Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveContainerCommandSubstring("--kubectl-parallelism-limit 20", 0))
Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveContainerCommandSubstring("--hydrator-enabled", 0))
})
})
})
Loading