From e05b32296a435a554e8d1d37f0cb56bc1ce3c99d Mon Sep 17 00:00:00 2001 From: Alexander Olzem Date: Wed, 1 Jul 2026 13:59:15 +0200 Subject: [PATCH] fix: set Failed/Succeeded counter for single-run Orders --- .../artifactworkflow_controller_test.go | 15 +++++++++++++++ pkg/controller/order_controller.go | 17 ++++++++++------- pkg/controller/workflow_handler.go | 19 +++++++++++++++++-- test/e2e/e2e_test.go | 10 ++++++++++ 4 files changed, 52 insertions(+), 9 deletions(-) diff --git a/pkg/controller/artifactworkflow_controller_test.go b/pkg/controller/artifactworkflow_controller_test.go index 4ecabd89..fa5bcf10 100644 --- a/pkg/controller/artifactworkflow_controller_test.go +++ b/pkg/controller/artifactworkflow_controller_test.go @@ -160,6 +160,11 @@ var _ = Describe("ArtifactWorkflowController", func() { Expect(k8sClient.Get(ctx, namespacedName(aw.Namespace, aw.Name), aw)).To(Succeed()) return aw.Status.Phase }).To(Equal(arcv1alpha1.WorkflowSucceeded)) + + Eventually(func() int64 { + Expect(k8sClient.Get(ctx, namespacedName(aw.Namespace, aw.Name), aw)).To(Succeed()) + return aw.Status.Succeeded + }).Should(Equal(int64(1))) }) It("should track failed Workflow information of created ArtifactWorkflows", func() { @@ -220,6 +225,11 @@ var _ = Describe("ArtifactWorkflowController", func() { Expect(k8sClient.Get(ctx, namespacedName(aw.Namespace, aw.Name), aw)).To(Succeed()) return aw.Status.Message }).To(ContainSubstring("Step 'step1' failed")) + + Eventually(func() int64 { + Expect(k8sClient.Get(ctx, namespacedName(aw.Namespace, aw.Name), aw)).To(Succeed()) + return aw.Status.Failed + }).Should(Equal(int64(1))) }) It("should track error Workflow status of created ArtifactWorkflows", func() { @@ -253,6 +263,11 @@ var _ = Describe("ArtifactWorkflowController", func() { Expect(k8sClient.Get(ctx, namespacedName(aw.Namespace, aw.Name), aw)).To(Succeed()) return aw.Status.Phase }).To(Equal(arcv1alpha1.WorkflowError)) + + Eventually(func() int64 { + Expect(k8sClient.Get(ctx, namespacedName(aw.Namespace, aw.Name), aw)).To(Succeed()) + return aw.Status.Failed + }).Should(Equal(int64(1))) }) It("should track pending Workflow status of created ArtifactWorkflows", func() { diff --git a/pkg/controller/order_controller.go b/pkg/controller/order_controller.go index 3c1ff08c..8d4e5677 100644 --- a/pkg/controller/order_controller.go +++ b/pkg/controller/order_controller.go @@ -336,7 +336,7 @@ func (r *OrderReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl r.Recorder.Eventf(order, aw, corev1.EventTypeNormal, "Deleted", "Delete", "Deleted finished artifact workflow '%s'", sha) } - anyPhaseChanged := false + anyStatusChanged := false for sha, daw := range desiredAWs { if slices.Contains(createAWs, sha) { // If it was just created we skip the update @@ -357,18 +357,21 @@ func (r *OrderReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl return ctrlResult, errLogAndWrap(log, err, "failed to get artifact workflow") } orderAWStatus := order.Status.ArtifactWorkflows[sha] - if orderAWStatus.Phase != aw.Status.Phase || - orderAWStatus.Succeeded != aw.Status.Succeeded || - orderAWStatus.Failed != aw.Status.Failed || - !orderAWStatus.LastScheduled.Equal(aw.Status.LastScheduled) { + + phaseChanged := orderAWStatus.Phase != aw.Status.Phase + succeededChanged := orderAWStatus.Succeeded != aw.Status.Succeeded + failedChanged := orderAWStatus.Failed != aw.Status.Failed + lastScheduledChanged := !orderAWStatus.LastScheduled.Equal(aw.Status.LastScheduled) + + if phaseChanged || succeededChanged || failedChanged || lastScheduledChanged { orderAWStatus.WorkflowStatus = aw.Status.WorkflowStatus order.Status.ArtifactWorkflows[sha] = orderAWStatus - anyPhaseChanged = true + anyStatusChanged = true } } // Update status - if len(createAWs) > 0 || len(deleteAWs) > 0 || anyPhaseChanged { + if len(createAWs) > 0 || len(deleteAWs) > 0 || anyStatusChanged { log.V(1).Info("Updating order status") // Make sure ArtifactIndex is up to date for sha, daw := range desiredAWs { diff --git a/pkg/controller/workflow_handler.go b/pkg/controller/workflow_handler.go index 494357a0..82ee7b83 100644 --- a/pkg/controller/workflow_handler.go +++ b/pkg/controller/workflow_handler.go @@ -83,8 +83,23 @@ func (h *SingleWorkflowHandler) CheckArgoResources(ctx context.Context) error { return errLogAndWrap(h.log, err, "failed to get workflow") } - if updated := h.setStatusFromWorkflow(ctx, h.log, h.aw, &wf); !updated { - return nil // nothing updated + updated := h.setStatusFromWorkflow(ctx, h.log, h.aw, &wf) + + if wf.Status.Phase == wfv1alpha1.WorkflowSucceeded && h.aw.Status.Succeeded != 1 { + h.aw.Status.Succeeded = 1 + updated = true + } + + failed := wf.Status.Phase == wfv1alpha1.WorkflowError || + wf.Status.Phase == wfv1alpha1.WorkflowFailed + + if failed && h.aw.Status.Failed != 1 { + h.aw.Status.Failed = 1 + updated = true + } + + if !updated { + return nil } if err := h.Status().Update(ctx, h.aw); err != nil { diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 3a03dde7..3978e571 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -223,6 +223,16 @@ var _ = Describe("ARC", Ordered, func() { }).Should(BeTrue()) Expect(orderState(resourceName, stateSucceeded)).To(BeTrue()) + cmd := exec.Command("kubectl", "get", "-n", "default", "orders", resourceName, "-o", "go-template={{ range .status.artifactWorkflows }}{{.succeeded}}\t{{ end }}") + output, err := run(cmd) + Expect(err).NotTo(HaveOccurred()) + fields := strings.Fields(output) + Expect(fields).NotTo(BeEmpty()) + for _, field := range fields { + numSucceeded, err := strconv.Atoi(field) + Expect(err).NotTo(HaveOccurred()) + Expect(numSucceeded).To(BeNumerically(">", 0)) + } }) }