diff --git a/.gitignore b/.gitignore index ce23e23261..f8e5c43938 100644 --- 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 \ No newline at end of file +junit-unit-test.xml diff --git a/experiments/controller.go b/experiments/controller.go index b3c0c56329..c1fc105ed0 100644 --- 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 index b27ed5326a..71ff7d41ce 100644 --- 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 index 9ddf771aae..34797cbdea 100644 --- 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 index 2c87bda888..6dd5981776 100644 --- 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))