diff --git a/test/openshift/e2e/ginkgo/parallel/1-115_validate_controller_extra_command_args_test.go b/test/openshift/e2e/ginkgo/parallel/1-115_validate_controller_extra_command_args_test.go index f18769368..6c0d1d96d 100644 --- a/test/openshift/e2e/ginkgo/parallel/1-115_validate_controller_extra_command_args_test.go +++ b/test/openshift/e2e/ginkgo/parallel/1-115_validate_controller_extra_command_args_test.go @@ -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() @@ -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 + 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 + + 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 - }) + 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)) + }) }) }) diff --git a/test/openshift/e2e/ginkgo/parallel/1-120_validate_resource_health_persisted_in_application_status_test.go b/test/openshift/e2e/ginkgo/parallel/1-120_validate_resource_health_persisted_in_application_status_test.go new file mode 100644 index 000000000..282a405c8 --- /dev/null +++ b/test/openshift/e2e/ginkgo/parallel/1-120_validate_resource_health_persisted_in_application_status_test.go @@ -0,0 +1,214 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package parallel + +import ( + "context" + + argov1beta1api "github.com/argoproj-labs/argocd-operator/api/v1beta1" + argoprojv1a1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture" + argocdFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/argocd" + ssFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/statefulset" + fixtureUtils "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/utils" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("GitOps Operator Parallel E2E Test", func() { + const ( + argoCDName = "example-argocd" + appName = "guestbook" + appNamespace = "guestbook-1-120" + ) + + var ( + k8sClient client.Client + ctx context.Context + ) + + BeforeEach(func() { + fixture.EnsureParallelCleanSlate() + + k8sClient, _ = fixtureUtils.GetE2ETestKubeClient() + ctx = context.Background() + + }) + + Context("1-120_validate_resource_health_persisted_in_application_status", func() { + It("should persist resource health in Application CR status when configured", func() { + ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc() + defer cleanupFunc() + + Expect(k8sClient.Get(ctx, client.ObjectKey{ + Name: ns.Name, + Namespace: ns.Namespace, + }, ns)).To(Succeed()) + + By("Creating ArgoCD CR with controller.resource.health.persist=true, which is the default") + argoCD := &argov1beta1api.ArgoCD{ + ObjectMeta: metav1.ObjectMeta{Name: "example-argocd", Namespace: ns.Name}, + Spec: argov1beta1api.ArgoCDSpec{ + CmdParams: map[string]string{ + "controller.resource.health.persist": "true", + }, + }, + } + Expect(k8sClient.Create(ctx, argoCD)).To(Succeed()) + Eventually(argoCD, "5m", "5s").Should(argocdFixture.BeAvailable()) + + By("Waiting for Application Controller to be ready") + ss := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{ + Name: argoCDName + "-application-controller", + Namespace: ns.Name, + }} + Eventually(ss).Should(ssFixture.HaveReadyReplicas(1)) + + targetNamespace, cleanupFunc := fixture.CreateManagedNamespaceWithCleanupFunc(appNamespace, ns.Name) + defer cleanupFunc() + + By("Creating ArgoCD Application CR") + app := &argoprojv1a1.Application{ + ObjectMeta: metav1.ObjectMeta{ + Name: appName, + Namespace: ns.Name, + }, + Spec: argoprojv1a1.ApplicationSpec{ + Project: "default", + Source: &argoprojv1a1.ApplicationSource{ + RepoURL: "https://github.com/argoproj/argocd-example-apps", + Path: "guestbook", + TargetRevision: "HEAD", + }, + Destination: argoprojv1a1.ApplicationDestination{ + Server: "https://kubernetes.default.svc", + Namespace: targetNamespace.Name, + }, + SyncPolicy: &argoprojv1a1.SyncPolicy{ + Automated: &argoprojv1a1.SyncPolicyAutomated{}, + }, + }, + } + Expect(k8sClient.Create(ctx, app)).To(Succeed()) + + By("Validating that resource health is persisted in Application CR") + Eventually(func() bool { + var fetched argoprojv1a1.Application + if err := k8sClient.Get(ctx, types.NamespacedName{Name: appName, Namespace: ns.Name}, &fetched); err != nil { + GinkgoWriter.Println("failed to fetch Application:", err) + return false + } + + // Check if application has resources with health information + if len(fetched.Status.Resources) == 0 { + GinkgoWriter.Println("Application.Status.Resources is empty") + return false + } + + for _, res := range fetched.Status.Resources { + if res.Health == nil { + GinkgoWriter.Println("Resource", res.Kind, res.Name, "has nil Health") + return false + } + if res.Health.Status == "" { + GinkgoWriter.Println("Resource", res.Kind, res.Name, "has empty Health.Status") + return false + } + } + + // Validate resourceHealthSource is NOT present (it is omitted when health is persisted) + return len(fetched.Status.ResourceHealthSource) == 0 + }, "3m", "5s").Should(BeTrue()) + }) + + It("should not persist resource health and use resourceHealthSource when controller.resource.health.persist=false", func() { + ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc() + defer cleanupFunc() + + By("Creating ArgoCD CR with controller.resource.health.persist=false") + argoCD := &argov1beta1api.ArgoCD{ + ObjectMeta: metav1.ObjectMeta{Name: argoCDName, Namespace: ns.Name}, + Spec: argov1beta1api.ArgoCDSpec{ + CmdParams: map[string]string{ + "controller.resource.health.persist": "false", + }, + }, + } + Expect(k8sClient.Create(ctx, argoCD)).To(Succeed()) + Eventually(argoCD, "5m", "5s").Should(argocdFixture.BeAvailable()) + + ss := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{ + Name: argoCDName + "-application-controller", + Namespace: ns.Name, + }} + Eventually(ss).Should(ssFixture.HaveReadyReplicas(1)) + + targetNamespace, cleanupFunc := fixture.CreateManagedNamespaceWithCleanupFunc(appNamespace, ns.Name) + defer cleanupFunc() + + By("Creating ArgoCD Application CR") + app := &argoprojv1a1.Application{ + ObjectMeta: metav1.ObjectMeta{Name: appName, Namespace: ns.Name}, + Spec: argoprojv1a1.ApplicationSpec{ + Project: "default", + Source: &argoprojv1a1.ApplicationSource{ + RepoURL: "https://github.com/argoproj/argocd-example-apps", + Path: "guestbook", + TargetRevision: "HEAD", + }, + Destination: argoprojv1a1.ApplicationDestination{ + Server: "https://kubernetes.default.svc", + Namespace: targetNamespace.Name, + }, + SyncPolicy: &argoprojv1a1.SyncPolicy{ + Automated: &argoprojv1a1.SyncPolicyAutomated{}, + }, + }, + } + Expect(k8sClient.Create(ctx, app)).To(Succeed()) + + By("Validating that health is NOT persisted and resourceHealthSource is appTree") + Eventually(func() bool { + var fetched argoprojv1a1.Application + if err := k8sClient.Get(ctx, types.NamespacedName{Name: appName, Namespace: ns.Name}, &fetched); err != nil { + GinkgoWriter.Println("failed to fetch Application:", err) + return false + } + + // Expect resourceHealthSource to be set + if fetched.Status.ResourceHealthSource != "appTree" { + GinkgoWriter.Println("ResourceHealthSource is not set as expected") + return false + } + + // Ensure resources exist but Health is not populated + for _, res := range fetched.Status.Resources { + if res.Health != nil { + GinkgoWriter.Println("Expected nil Health but got:", res.Kind, res.Name, "Health:", res.Health.Status) + return false + } + } + return true + }, "3m", "5s").Should(BeTrue()) + }) + }) +})