fix: append weighted destination only when weight is mentioned in exp…

…eriment template Signed-off-by: divyansh375 <[email protected]>
argoproj · May 7, 2023 · 81bf063 · 81bf063
1 parent e7e45f7
commit 81bf063
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 20 deletions.
diff --git a/experiments/experiment_test.go b/experiments/experiment_test.go
@@ -529,6 +529,7 @@ func TestServiceInheritPortsFromRS(t *testing.T) {
 	assert.NotNil(t, exCtx.templateServices["bar"])
 	assert.Equal(t, exCtx.templateServices["bar"].Name, "foo-bar")
 	assert.Equal(t, exCtx.templateServices["bar"].Spec.Ports[0].Port, int32(80))
+	assert.Equal(t, exCtx.templateServices["bar"].Spec.Ports[0].Name, "testport")
 }
 
 func TestServiceNameSet(t *testing.T) {

diff --git a/rollout/trafficrouting.go b/rollout/trafficrouting.go
@@ -354,11 +354,13 @@ func (c *rolloutContext) calculateWeightDestinationsFromExperiment() []v1alpha1.
 		}
 		for _, templateStatus := range c.currentEx.Status.TemplateStatuses {
 			templateWeight := getTemplateWeight(templateStatus.Name)
-			weightDestinations = append(weightDestinations, v1alpha1.WeightDestination{
-				ServiceName:     templateStatus.ServiceName,
-				PodTemplateHash: templateStatus.PodTemplateHash,
-				Weight:          *templateWeight,
-			})
+			if templateWeight != nil {
+				weightDestinations = append(weightDestinations, v1alpha1.WeightDestination{
+					ServiceName:     templateStatus.ServiceName,
+					PodTemplateHash: templateStatus.PodTemplateHash,
+					Weight:          *templateWeight,
+				})
+			}
 		}
 	}
 	return weightDestinations

diff --git a/rollout/trafficrouting_test.go b/rollout/trafficrouting_test.go
@@ -248,13 +248,21 @@ func TestRolloutWithExperimentStep(t *testing.T) {
 					SpecRef:  "canary",
 					Replicas: pointer.Int32Ptr(1),
 					Weight:   pointer.Int32Ptr(5),
-				}},
+				},
+					{
+						Name:     "experiment-template-without-weight",
+						SpecRef:  "stable",
+						Replicas: pointer.Int32Ptr(1),
+					}},
 			},
 		},
 	}
 	r1 := newCanaryRollout("foo", 10, nil, steps, pointer.Int32Ptr(1), intstr.FromInt(1), intstr.FromInt(0))
 	r2 := bumpVersion(r1)
-	r2.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{}
+	r1.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{
+		SMI: &v1alpha1.SMITrafficRouting{},
+	}
+	r2.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{SMI: &v1alpha1.SMITrafficRouting{}}
 	r2.Spec.Strategy.Canary.CanaryService = "canary"
 	r2.Spec.Strategy.Canary.StableService = "stable"
 
@@ -272,7 +280,12 @@ func TestRolloutWithExperimentStep(t *testing.T) {
 		Name:            "experiment-template",
 		ServiceName:     "experiment-service",
 		PodTemplateHash: rs2PodHash,
-	}}
+	},
+		{
+			Name:            "experiment-template-without-weight",
+			ServiceName:     "experiment-service-without-weight",
+			PodTemplateHash: rs2PodHash,
+		}}
 	r2.Status.Canary.CurrentExperiment = ex.Name
 
 	f.kubeobjects = append(f.kubeobjects, rs1, rs2, canarySvc, stableSvc)
@@ -292,18 +305,13 @@ func TestRolloutWithExperimentStep(t *testing.T) {
 			// make sure SetWeight was called with correct value
 			assert.Equal(t, int32(10), desiredWeight)
 			assert.Equal(t, int32(5), weightDestinations[0].Weight)
+			assert.Len(t, weightDestinations, 1)
 			assert.Equal(t, ex.Status.TemplateStatuses[0].ServiceName, weightDestinations[0].ServiceName)
 			assert.Equal(t, ex.Status.TemplateStatuses[0].PodTemplateHash, weightDestinations[0].PodTemplateHash)
 			return nil
 		})
 		f.fakeTrafficRouting.On("SetHeaderRoute", mock.Anything, mock.Anything).Return(nil)
-		f.fakeTrafficRouting.On("VerifyWeight", mock.Anything).Return(func(desiredWeight int32, weightDestinations ...v1alpha1.WeightDestination) error {
-			assert.Equal(t, int32(10), desiredWeight)
-			assert.Equal(t, int32(5), weightDestinations[0].Weight)
-			assert.Equal(t, ex.Status.TemplateStatuses[0].ServiceName, weightDestinations[0].ServiceName)
-			assert.Equal(t, ex.Status.TemplateStatuses[0].PodTemplateHash, weightDestinations[0].PodTemplateHash)
-			return nil
-		})
+		f.fakeTrafficRouting.On("VerifyWeight", mock.Anything, mock.Anything).Return(pointer.BoolPtr(true), nil)
 		f.run(getKey(r2, t))
 	})
 
@@ -318,11 +326,7 @@ func TestRolloutWithExperimentStep(t *testing.T) {
 			return nil
 		})
 		f.fakeTrafficRouting.On("SetHeaderRoute", mock.Anything, mock.Anything).Return(nil)
-		f.fakeTrafficRouting.On("VerifyWeight", mock.Anything).Return(func(desiredWeight int32, weightDestinations ...v1alpha1.WeightDestination) error {
-			assert.Equal(t, int32(10), desiredWeight)
-			assert.Len(t, weightDestinations, 0)
-			return nil
-		})
+		f.fakeTrafficRouting.On("VerifyWeight", mock.Anything, mock.Anything).Return(pointer.BoolPtr(true), nil)
 		f.run(getKey(r2, t))
 	})
 }