From 9197bd5d3d69cb487bf0310250b426dfad58676c Mon Sep 17 00:00:00 2001 From: Roy Arnon Date: Sun, 22 Dec 2024 22:47:05 +0200 Subject: [PATCH] fix: move recorder event to after experiment reconcilation, fixes #4021 Signed-off-by: Roy Arnon --- USERS.md | 1 + experiments/controller.go | 16 +++++++++++++++- experiments/experiment.go | 9 --------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/USERS.md b/USERS.md index ed196d4234..9e1422162f 100644 --- a/USERS.md +++ b/USERS.md @@ -61,3 +61,4 @@ Organizations below are **officially** using Argo Rollouts. Please send a PR wit 1. [WeLab Bank](https://www.welab.bank/) 1. [Yotpo](https://www.yotpo.com/) 1. [VGS](https://www.vgs.io) +1. [Taboola](https://www.taboola.com) diff --git a/experiments/controller.go b/experiments/controller.go index 3aa1519e46..1ae869de4d 100644 --- a/experiments/controller.go +++ b/experiments/controller.go @@ -2,6 +2,7 @@ package experiments import ( "context" + corev1 "k8s.io/api/core/v1" "sync" "time" @@ -318,6 +319,7 @@ func (ec *Controller) syncHandler(ctx context.Context, key string) error { } func (ec *Controller) persistExperimentStatus(orig *v1alpha1.Experiment, newStatus *v1alpha1.ExperimentStatus) error { + prevStatus := orig.Status ctx := context.TODO() logCtx := logutil.WithExperiment(orig) patch, modified, err := diff.CreateTwoWayMergePatch( @@ -336,15 +338,27 @@ func (ec *Controller) persistExperimentStatus(orig *v1alpha1.Experiment, newStat return nil } logCtx.Debugf("Experiment Patch: %s", patch) - _, err = ec.argoProjClientset.ArgoprojV1alpha1().Experiments(orig.Namespace).Patch(ctx, orig.Name, patchtypes.MergePatchType, patch, metav1.PatchOptions{}) + patched, err := ec.argoProjClientset.ArgoprojV1alpha1().Experiments(orig.Namespace).Patch(ctx, orig.Name, patchtypes.MergePatchType, patch, metav1.PatchOptions{}) if err != nil { logCtx.Warningf("Error updating experiment: %v", err) return err } logCtx.Info("Patch status successfully") + ec.recordEvent(patched, prevStatus, newStatus) return nil } +func (ec *Controller) recordEvent(ex *v1alpha1.Experiment, prevStatus v1alpha1.ExperimentStatus, newStatus *v1alpha1.ExperimentStatus) { + if prevStatus.Phase != newStatus.Phase { + eventType := corev1.EventTypeNormal + switch newStatus.Phase { + case v1alpha1.AnalysisPhaseError, v1alpha1.AnalysisPhaseFailed, v1alpha1.AnalysisPhaseInconclusive: + eventType = corev1.EventTypeWarning + } + ec.recorder.Eventf(ex, record.EventOptions{EventType: eventType, EventReason: "Experiment" + string(newStatus.Phase)}, "Experiment transitioned from %s -> %s", prevStatus.Phase, newStatus.Phase) + } +} + // enqueueIfCompleted conditionally enqueues the AnalysisRun's Experiment if the run is complete func (ec *Controller) enqueueIfCompleted(obj any) { run := unstructuredutil.ObjectToAnalysisRun(obj) diff --git a/experiments/experiment.go b/experiments/experiment.go index 1b40464015..b27ed5326a 100644 --- a/experiments/experiment.go +++ b/experiments/experiment.go @@ -523,7 +523,6 @@ func (ec *experimentContext) ResolveAnalysisRunArgs(args []v1alpha1.Argument) ([ } func (ec *experimentContext) calculateStatus() *v1alpha1.ExperimentStatus { - prevStatus := ec.newStatus.DeepCopy() switch ec.newStatus.Phase { case "": ec.newStatus.Phase = v1alpha1.AnalysisPhasePending @@ -568,14 +567,6 @@ func (ec *experimentContext) calculateStatus() *v1alpha1.ExperimentStatus { } } ec.newStatus = calculateExperimentConditions(ec.ex, *ec.newStatus) - if prevStatus.Phase != ec.newStatus.Phase { - eventType := corev1.EventTypeNormal - switch ec.newStatus.Phase { - case v1alpha1.AnalysisPhaseError, v1alpha1.AnalysisPhaseFailed, v1alpha1.AnalysisPhaseInconclusive: - eventType = corev1.EventTypeWarning - } - ec.recorder.Eventf(ec.ex, record.EventOptions{EventType: eventType, EventReason: "Experiment" + string(ec.newStatus.Phase)}, "Experiment transitioned from %s -> %s", prevStatus.Phase, ec.newStatus.Phase) - } return ec.newStatus }