-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Error when conflicting Deployment due to label selector version #39228
Error when conflicting Deployment due to label selector version #39228
Conversation
@manusa: Do you have any insights on this? |
This seems kinda risky, no? |
This comment has been minimized.
This comment has been minimized.
I do have a recent experience about a complaint from a JKube user regarding something related. Starting from v1.16, we switched to use standard/recommended Kubernetes labels as opposed to those that JKube (formerly Fabric8 Maven Plugin) prescribed. When the user upgraded to the newer JKube version with the changes, their pipelines failed due to the existent Deployment selector containing different values. We did recommend the |
56cc053
to
6fffce9
Compare
This comment has been minimized.
This comment has been minimized.
6fffce9
to
070107b
Compare
@iocanel if having version in label makes impossible to update deployments shouldn't this not be avoided? |
Status for workflow
|
Adding the version to the selector has it's pros and cons. If we just remove it then users will only be able to correlate pods by name, which can be problematic as users won't be able to route traffic between version etc. Things like canary, a/b testing it will become trickier. So, we need give users the option to decide how they'll roll. |
Something additional that worth mentioning ... This behavior (having the version being part of the label) is inherited by dekorate since 2019. Meaning that its not something that we just recently change. The main reason why this is has been just discovered is the transition from I suggest we move forward with this pull request. And move to an optional opt-in strategy for the next major release. |
@gsmet @yrodiere wdyt ? seems like good first step to inform users what to do early. I suggest we in anohter PR make |
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.
@gsmet @yrodiere wdyt ? seems like good first step to inform users what to do early.
Agreed. If it's going to fail, better fail early with an actionable message.
I suggest we in anohter PR make
add-version-to-label-selectors
be false by default. That will still cause exception on existing deployments that has label but with this PR its now informative/helpful, and any future quarkus versions wil allow rolling deployments by default.
Great minds think alike: #39180 (comment)
Resolves: #39180
What the pull request does is that it checks if an existing Deployment is already installed with conflicting Deployment version and provides detailed error message with instructions on how the situation should be handled.
Also, it does the checking before applied resources (e.g. Service etc) that could leave the application in a non-working state.