Tighten things up - don't need a bool - fetch the ref or nil

Signed-off-by: mitchell amihod <4623+meeech@users.noreply.github.com>

experiments/experiment.go

@@ -419,14 +419,8 @@ func (ec *experimentContext) reconcileAnalysisRun(analysis v1alpha1.ExperimentAn
 			// 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
-			if experimentutil.BelongsToRollout(ec.ex) {
-				var roRef metav1.OwnerReference
-				for _, owner := range ec.ex.OwnerReferences {
-					if owner.Kind == "Rollout" {
-						roRef = owner
-					}
-				}
-
+			roRef := experimmentUtil.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)

utils/experiment/experiment.go

@@ -18,17 +18,13 @@ import (
 
 var terminateExperimentPatch = []byte(`{"spec":{"terminate":true}}`)
 
-func BelongsToRollout(experiment *v1alpha1.Experiment) bool {
-	if len(experiment.OwnerReferences) == 0 {
-		return false
-	}
-
+func GetRolloutOwnerRef(experiment *v1alpha1.Experiment) *metav1.OwnerReference {
 	for _, owner := range experiment.OwnerReferences {
 		if owner.Kind == "Rollout" {
-			return true
+			return &owner
 		}
 	}
-	return false
+	return nil
 }
 
 func HasFinished(experiment *v1alpha1.Experiment) bool {

utils/experiment/experiment_test.go

@@ -15,35 +15,62 @@ import (
 	"github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/fake"
 )
 
-func TestBelongsToRollout(t *testing.T) {
+func TestGetRolloutOwnerRef(t *testing.T) {
 	t.Run("no owner references", func(t *testing.T) {
 		e := &v1alpha1.Experiment{}
-		belongs := BelongsToRollout(e)
-		assert.False(t, belongs)
+		ownerRef := GetRolloutOwnerRef(e)
+		assert.Nil(t, ownerRef)
 	})
 
-	t.Run("non-rollout owner", func(t *testing.T) {
+	t.Run("non-rollout owner reference", func(t *testing.T) {
 		e := &v1alpha1.Experiment{
 			ObjectMeta: metav1.ObjectMeta{
-				OwnerReferences: []metav1.OwnerReference{{
-					Kind: "Deployment",
-				}},
+				OwnerReferences: []metav1.OwnerReference{
+					{
+						Kind: "Deployment",
+						Name: "deploy",
+					},
+				},
 			},
 		}
-		belongs := BelongsToRollout(e)
-		assert.False(t, belongs)
+		ownerRef := GetRolloutOwnerRef(e)
+		assert.Nil(t, ownerRef)
 	})
 
-	t.Run("rollout owner", func(t *testing.T) {
+	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: "Rollout",
-				}},
+				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,
+				},
 			},
 		}
-		belongs := BelongsToRollout(e)
-		assert.True(t, belongs)
+		ownerRef := GetRolloutOwnerRef(e)
+		assert.Equal(t, &rolloutOwner, ownerRef)
 	})
 }