From 6e623bcb419082edebceee2f5af6b214de355db5 Mon Sep 17 00:00:00 2001 From: Daniel Del Rio Date: Fri, 24 Feb 2023 17:40:03 +0100 Subject: [PATCH] fix: Rollback change on service creation with weightless experiments (#2624) BREAKING CHANGE There was an unintentional change in behavior related to service creation with experiments introduced in v1.4.0 this has been reverted in v1.4.1 back to the original behavior. In v1.4.0 services where always created with for inline experiments even if there was no weight set. In 1.4.1 we go back to the original behavior of requiring weight to be set in order to create a service. --- docs/features/experiment.md | 4 ++-- rollout/experiment.go | 4 +++- rollout/experiment_test.go | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/features/experiment.md b/docs/features/experiment.md index 6740895d68..b282f3b18b 100644 --- a/docs/features/experiment.md +++ b/docs/features/experiment.md @@ -6,7 +6,7 @@ The Experiment CRD allows users to have ephemeral runs of one or more ReplicaSet running ephemeral ReplicaSets, the Experiment CRD can launch AnalysisRuns alongside the ReplicaSets. Generally, those AnalysisRun is used to confirm that new ReplicaSets are running as expected. -A Service routing traffic to the Experiment ReplicaSet is also generated. +A Service routing traffic to the Experiment ReplicaSet is also generated if a weight for that experiment is set. ## Use cases of Experiments @@ -245,7 +245,7 @@ to `experiment-baseline`, leaving the remaining 90% of traffic to the old stack. !!! note When a weighted experiment step with traffic routing is used, a - Service is auto-created for each experiment template. The traffic routers use + service is auto-created for each experiment template. The traffic routers use this service to send traffic to the experiment pods. By default, the generated Service has the name of the ReplicaSet and inherits diff --git a/rollout/experiment.go b/rollout/experiment.go index a6d3bdcd43..aeac6e8049 100644 --- a/rollout/experiment.go +++ b/rollout/experiment.go @@ -64,7 +64,9 @@ func GetExperimentFromTemplate(r *v1alpha1.Rollout, stableRS, newRS *appsv1.Repl Name: templateStep.Name, Replicas: templateStep.Replicas, } - template.Service = &v1alpha1.TemplateService{} + if templateStep.Weight != nil { + template.Service = &v1alpha1.TemplateService{} + } templateRS := &appsv1.ReplicaSet{} switch templateStep.SpecRef { case v1alpha1.CanarySpecRef: diff --git a/rollout/experiment_test.go b/rollout/experiment_test.go index 1c60e9f7c7..e2e50c1f28 100644 --- a/rollout/experiment_test.go +++ b/rollout/experiment_test.go @@ -791,7 +791,7 @@ func TestRolloutCreateExperimentWithService(t *testing.T) { Replicas: pointer.Int32Ptr(1), Weight: pointer.Int32Ptr(5), }, - // Service should also be created for "canary-template" + // Service should NOT be created for "canary-template" { Name: "canary-template", SpecRef: v1alpha1.CanarySpecRef, @@ -818,5 +818,5 @@ func TestRolloutCreateExperimentWithService(t *testing.T) { assert.NotNil(t, ex.Spec.Templates[0].Service) assert.Equal(t, "canary-template", ex.Spec.Templates[1].Name) - assert.NotNil(t, ex.Spec.Templates[1].Service) + assert.Nil(t, ex.Spec.Templates[1].Service) }