-
Notifications
You must be signed in to change notification settings - Fork 880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Rollback change on service creation with weightless experiments #2624
Conversation
Signed-off-by: Daniel Del Rio <[email protected]>
Signed-off-by: Daniel Del Rio <[email protected]>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
@@ -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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if templateStep.Weight != nil { | |
if (templateStep.Weight != nil || templateStep.Service != nil) { |
should we put this already in?
atm the spec doesn't allow templateStep.Service
to be anything but:
- nil
- empty object
so this shouldn't be a breaking change (famous last words)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nevermind, this is not possible without a Rollout spec change.
Codecov ReportBase: 81.52% // Head: 81.52% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2624 +/- ##
=======================================
Coverage 81.52% 81.52%
=======================================
Files 131 131
Lines 19562 19564 +2
=======================================
+ Hits 15947 15949 +2
Misses 2795 2795
Partials 820 820
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
…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.
closes: #2622
possible fix for: #2608
This rolls back the change introduced in #2397 which allows experiments with no weights to still create services. We want it so that services won't be created if there is no weight set for the experiments.
Checklist:
"fix(controller): Updates such and such. Fixes #1234"
.