-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: kubernetes keycloak deployment strategy #17304
Conversation
deploy/kubernetes/helm/che/custom-charts/che-keycloak/values.yaml
Outdated
Show resolved
Hide resolved
@@ -15,3 +15,4 @@ image: quay.io/eclipse/che-keycloak:nightly | |||
requireAdminPasswordChange: true | |||
keycloakAdminUserName: admin | |||
keycloakAdminUserPassword: admin | |||
deploymentStrategy: RollingUpdate |
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.
Allow to set the deployment strategy, the default RollingUpdate fail if multiattach is not supported by volumes
I think in most cases it won't be supported. What about setting default to recreate which mean - introduce an ability to override default but not change the behavior we already have?
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.
yes
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.
but other charts postgres, che, already have Recreate as deploy strategy
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
[test] |
[ci-build] |
[crw-ci-test] |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) ℹ️ |
@tolusha: commands to trigger CI jobs:
|
E2E tests of Eclipse Che Multiuser on OCP has been successful: |
[crw-ci-test] |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) ℹ️ |
[crw-ci-test] |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) ℹ️ |
cc @dmytro-ndp could you please take a look why the tests failed? Looks like not PR related. Thanks. |
[crw-ci-test] |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) ℹ️ |
[crw-ci-test] |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) ℹ️ |
[crw-ci-test] |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) ℹ️ |
@kesslerdev: PR needs to be updated from master branch, to fix Happy path tests build error:
|
@kesslerdev please rebase your changes. Then we can merge this PR. |
Allow to set the deployment strategy, the default RollingUpdate fail if multiattach is not supported by volumes Signed-off-by: Jean-François Monnier <kessler.dev@gmail.com>
Signed-off-by: Jean-François Monnier <kessler.dev@gmail.com> Co-authored-by: Serhii Leshchenko <sleshche@redhat.com>
Done ! |
[crw-ci-test] |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) ℹ️ |
[crw-ci-test] |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1) ℹ️ |
The only failed tests is related java ls which is a known issue. |
[ci-build] |
@kesslerdev thank you for contribution. |
What does this PR do?
Allow to set the deployment strategy, for the keycloak deployment chart
the default strategy RollingUpdate fail if multiattach is not supported by volumes