fix(experiments): fire rollout event on experiment step (#4124)

* chore: ignore all debug_bin* Signed-off-by: mitchell amihod <[email protected]> * feat(experiments): Add a utility to check if an experiment belongs to a Rollout. We can then identify when an experiment is a Step in a Rollout. Signed-off-by: mitchell amihod <[email protected]> * chore: typo Signed-off-by: mitchell amihod <[email protected]> * feat(experiments): Fire k8s Event bound to the Rollout when it owns the Experiment Addresses #4009. This change will fire Analysis Run events bound to the parent Rollout object when the Experiment is a Step in the Rollout. Signed-off-by: mitchell amihod <[email protected]> * Loop through ownerReferences to find the rollout reference. If we pass belongs to rollout check, we know there is a rollout to find. Signed-off-by: mitchell amihod <[email protected]> * Tighten things up - don't need a bool - fetch the ref or nil Signed-off-by: mitchell amihod <[email protected]> --------- Signed-off-by: mitchell amihod <[email protected]>
argoproj · Feb 14, 2025 · a748f05 · a748f05
1 parent d8994c0
commit a748f05
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 10 deletions.
diff --git a/.gitignore b/.gitignore
@@ -4,7 +4,7 @@
 dist/
 *.iml
 # delve debug binaries
-__debug_bin
+__debug_bin*
 cmd/**/debug
 debug.test
 coverage.out
@@ -19,4 +19,4 @@ server/static/*
 !server/static/.gitkeep
 coverage-output-e2e/
 coverage-output-unit/
-junit-unit-test.xml
+junit-unit-test.xml
diff --git a/experiments/controller.go b/experiments/controller.go
@@ -81,7 +81,7 @@ type Controller struct {
 	resyncPeriod time.Duration
 }
 
-// ControllerConfig describes the data required to instantiate a new analysis controller
+// ControllerConfig describes the data required to instantiate a new experiments controller
 type ControllerConfig struct {
 	KubeClientSet                   kubernetes.Interface
 	ArgoProjClientset               clientset.Interface
@@ -100,7 +100,6 @@ type ControllerConfig struct {
 
 // NewController returns a new experiment controller
 func NewController(cfg ControllerConfig) *Controller {
-
 	replicaSetControl := controller.RealRSControl{
 		KubeClient: cfg.KubeClientSet,
 		Recorder:   cfg.Recorder.K8sRecorder(),

diff --git a/experiments/experiment.go b/experiments/experiment.go
@@ -72,7 +72,6 @@ func newExperimentContext(
 	resyncPeriod time.Duration,
 	enqueueExperimentAfter func(obj any, duration time.Duration),
 ) *experimentContext {
-
 	exCtx := experimentContext{
 		ex:                            experiment,
 		templateRSs:                   templateRSs,
@@ -416,6 +415,19 @@ func (ec *experimentContext) reconcileAnalysisRun(analysis v1alpha1.ExperimentAn
 				eventType = corev1.EventTypeWarning
 			}
 			ec.recorder.Eventf(ec.ex, record.EventOptions{EventType: eventType, EventReason: "AnalysisRun" + string(newStatus.Phase)}, msg)
+
+			// Handle the case where the Analysis Run belongs to an Experiment, and the Experiment is a Step in the Rollout
+			// This makes sure the rollout gets the Analysis Run events, which will then trigger any subscribed notifications
+			// #4009
+			roRef := experimentutil.GetRolloutOwnerRef(ec.ex)
+			if roRef != nil {
+				rollout, err := ec.argoProjClientset.ArgoprojV1alpha1().Rollouts(ec.ex.Namespace).Get(context.TODO(), roRef.Name, metav1.GetOptions{})
+				if err != nil {
+					ec.log.Warnf("Failed to get parent Rollout of the Experiment '%s': %v", roRef.Name, err)
+				} else {
+					ec.recorder.Eventf(rollout, record.EventOptions{EventType: corev1.EventTypeWarning, EventReason: "AnalysisRun" + string(newStatus.Phase)}, msg)
+				}
+			}
 		}
 		experimentutil.SetAnalysisRunStatus(ec.newStatus, *newStatus)
 	}()
@@ -627,7 +639,6 @@ func (ec *experimentContext) assessAnalysisRuns() (v1alpha1.AnalysisPhase, strin
 
 // newAnalysisRun generates an AnalysisRun from the experiment and template
 func (ec *experimentContext) newAnalysisRun(analysis v1alpha1.ExperimentAnalysisTemplateRef, args []v1alpha1.Argument, dryRunMetrics []v1alpha1.DryRun, measurementRetentionMetrics []v1alpha1.MeasurementRetention, analysisRunMetadata *v1alpha1.AnalysisRunMetadata) (*v1alpha1.AnalysisRun, error) {
-
 	if analysis.ClusterScope {
 		analysisTemplates, clusterAnalysisTemplates, err := ec.getAnalysisTemplatesFromClusterAnalysis(analysis)
 		if err != nil {
@@ -772,7 +783,6 @@ func (ec *experimentContext) getAnalysisTemplatesFromRefs(templateRefs *[]v1alph
 				templates = append(templates, innerTemplates...)
 			}
 		}
-
 	}
 	uniqueTemplates, uniqueClusterTemplates := analysisutil.FilterUniqueTemplates(templates, clusterTemplates)
 	return uniqueTemplates, uniqueClusterTemplates, nil

diff --git a/utils/experiment/experiment.go b/utils/experiment/experiment.go
@@ -18,6 +18,15 @@ import (
 
 var terminateExperimentPatch = []byte(`{"spec":{"terminate":true}}`)
 
+func GetRolloutOwnerRef(experiment *v1alpha1.Experiment) *metav1.OwnerReference {
+	for _, owner := range experiment.OwnerReferences {
+		if owner.Kind == "Rollout" {
+			return &owner
+		}
+	}
+	return nil
+}
+
 func HasFinished(experiment *v1alpha1.Experiment) bool {
 	return experiment.Status.Phase.Completed()
 }

diff --git a/utils/experiment/experiment_test.go b/utils/experiment/experiment_test.go
@@ -15,6 +15,65 @@ import (
 	"github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/fake"
 )
 
+func TestGetRolloutOwnerRef(t *testing.T) {
+	t.Run("no owner references", func(t *testing.T) {
+		e := &v1alpha1.Experiment{}
+		ownerRef := GetRolloutOwnerRef(e)
+		assert.Nil(t, ownerRef)
+	})
+
+	t.Run("non-rollout owner reference", func(t *testing.T) {
+		e := &v1alpha1.Experiment{
+			ObjectMeta: metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{
+					{
+						Kind: "Deployment",
+						Name: "deploy",
+					},
+				},
+			},
+		}
+		ownerRef := GetRolloutOwnerRef(e)
+		assert.Nil(t, ownerRef)
+	})
+
+	t.Run("multiple owner references with rollout", func(t *testing.T) {
+		rolloutOwner := metav1.OwnerReference{
+			Kind: "Rollout",
+			Name: "rollout",
+		}
+		e := &v1alpha1.Experiment{
+			ObjectMeta: metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{
+					{
+						Kind: "Deployment",
+						Name: "deploy",
+					},
+					rolloutOwner,
+				},
+			},
+		}
+		ownerRef := GetRolloutOwnerRef(e)
+		assert.Equal(t, &rolloutOwner, ownerRef)
+	})
+
+	t.Run("only rollout owner reference", func(t *testing.T) {
+		rolloutOwner := metav1.OwnerReference{
+			Kind: "Rollout",
+			Name: "rollout",
+		}
+		e := &v1alpha1.Experiment{
+			ObjectMeta: metav1.ObjectMeta{
+				OwnerReferences: []metav1.OwnerReference{
+					rolloutOwner,
+				},
+			},
+		}
+		ownerRef := GetRolloutOwnerRef(e)
+		assert.Equal(t, &rolloutOwner, ownerRef)
+	})
+}
+
 func TestHasFinished(t *testing.T) {
 	e := &v1alpha1.Experiment{}
 	assert.False(t, HasFinished(e))
@@ -42,7 +101,6 @@ func TestCalculateTemplateReplicasCount(t *testing.T) {
 		Status: v1alpha1.TemplateStatusFailed,
 	})
 	assert.Equal(t, int32(0), CalculateTemplateReplicasCount(e, template))
-
 }
 
 func TestPassedDurations(t *testing.T) {
@@ -70,7 +128,6 @@ func TestPassedDurations(t *testing.T) {
 	e.Status.AvailableAt = &metav1.Time{Time: now.Add(-2 * time.Second)}
 	passedDuration, _ = PassedDurations(e)
 	assert.True(t, passedDuration)
-
 }
 
 func TestGetTemplateStatusMapping(t *testing.T) {
@@ -113,7 +170,6 @@ func TestReplicaSetNameFromExperiment(t *testing.T) {
 }
 
 func TestExperimentByCreationTimestamp(t *testing.T) {
-
 	now := metav1.Now()
 	before := metav1.NewTime(metav1.Now().Add(-5 * time.Second))