-
Notifications
You must be signed in to change notification settings - Fork 20
fix: fix rollout controller not rollout when type is changed from External to RollingUpdate #31
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
Conversation
c0a4c10 to
cf7a863
Compare
94d5ea5 to
e6105fe
Compare
| verifyBindingsNotRolledOutConsistently(bindings) | ||
|
|
||
| By("Updating CRP rollout strategy type to empty") | ||
| rolloutCRP.Spec.Strategy.Type = "" |
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.
is this allowed? Don't our webhook/API definition/CEL block this?
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.
I tried in prod. This is allowed.
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.
Default value is used if empty.
… to RollingUpdate Signed-off-by: Wantong Jiang <wantjian@microsoft.com>
Signed-off-by: Wantong Jiang <wantjian@microsoft.com>
Description of your changes
When CRP rollout strategy is changed from
ExternaltoRollingUpdate, the rollout is not triggered. This PR fixes this issue.Fixes #
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Special notes for your reviewer