quarkus
quarkus copied to clipboard
Remove conflicting Deployment due to label selector version
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.
@manusa: Do you have any insights on this?
This seems kinda risky, no?
:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.
Status for workflow Quarkus CI
This is the status report for running Quarkus CI on commit 38f412b9ae1a9398c3e086d5152b1a2d4e563cbb.
Failing Jobs
| Status | Name | Step | Failures | Logs | Raw logs | Build scan |
|---|---|---|---|---|---|---|
| ✖ | Initial JDK 17 Build | Build |
Failures | Logs | Raw logs | :mag: |
You can consult the Develocity build scans.
Failures
:gear: Initial JDK 17 Build #
- Failing: extensions/kubernetes/vanilla/deployment
! Skipped: docs extensions/kubernetes/kind/deployment extensions/kubernetes/minikube/deployment and 6 more
:package: extensions/kubernetes/vanilla/deployment
✖ Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.12.1:compile (default-compile) on project quarkus-kubernetes-deployment: Compilation failure
@manusa: Do you have any insights on this?
I do have a recent experience about a complaint from a JKube user regarding something related.
https://github.com/eclipse/jkube/issues/2697
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 recreate approach (where the previous Deployment is deleted and then reapplied), but this was not possible for them. See https://github.com/eclipse/jkube/issues/2697#issuecomment-1952562420
:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.
Status for workflow Quarkus CI
This is the status report for running Quarkus CI on commit 6fffce9435f43af7c26714b0eda17e018a31262f.
Failing Jobs
| Status | Name | Step | Failures | Logs | Raw logs | Build scan |
|---|---|---|---|---|---|---|
| ✖ | Initial JDK 17 Build | Build |
Failures | Logs | Raw logs | :mag: |
You can consult the Develocity build scans.
Failures
:gear: Initial JDK 17 Build #
- Failing: integration-tests/webjars-locator
:package: integration-tests/webjars-locator
✖ Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.2.1:enforce (enforce) on project quarkus-integration-test-webjars-locator:
Rule 1: io.quarkus.enforcer.RequiresMinimalDeploymentDependency failed with message:
1 minimal *-deployment dependencies are superfluous and must be removed from pom.xml:
io.quarkus:quarkus-webjars-locator-deployment:999-SNAPSHOT
@iocanel if having version in label makes impossible to update deployments shouldn't this not be avoided?
Status for workflow Quarkus CI
This is the status report for running Quarkus CI on commit 070107b3dcba53b9837d9f2703d15414dc31299d.
:white_check_mark: The latest workflow run for the pull request has completed successfully.
It should be safe to merge provided you have a look at the other checks in the summary.
You can consult the Develocity build scans.
@iocanel if having version in label makes impossible to update deployments shouldn't this not be avoided?
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. Currently, it's enabled by default. We could possibly change that but that means that it will then bite everyone once.
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 DeploymentConfig to Deployment which is the resource affected by it.
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 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.