Skip to content
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(keycloak) Enable Keycloak cluster nodes discovery by default #716

Conversation

oswaldo-montenegro-codecentric
Copy link

@oswaldo-montenegro-codecentric oswaldo-montenegro-codecentric commented Jun 16, 2023

feat(keycloak) Include out-of-the-box high availability mechanism

While trying out the helm chart with replicas > 1, we've had the issue that
all keycloak instances don't identify each other to be part of the keycloak cluster.
This caused issues after logging in with multiple replicas. The problem lies in the missing distributed caching configuration. This change allows the helm chart to fully function with multiple replicas out of the box.

More information: https://www.keycloak.org/server/caching

Signed-off-by: Oswaldo Montenegro [email protected]

@oswaldo-montenegro-codecentric oswaldo-montenegro-codecentric changed the title Include out-of-the-box high availability mechanism feat(Keycloak) Include out-of-the-box high availability mechanism Jun 16, 2023
@devRoemer
Copy link

I think that having the env vars added depending on replica number is not flexible enough.
There might be use cases where one wants to add those vars via extraEnv with different values.

For this case, there should at least be a boolean in values.yaml to disable automatically adding them (if not even the entire config of both values).

While trying out the helm chart with replicas > 1, we've had the issue that
all keycloak instances don't identify each other to be part of the keycloak cluster.

This caused issues after logging in with multiple replicas on the admin web console. The problem lies on the missing cluster node discovery configuration.

More information: https://www.keycloak.org/2019/05/keycloak-cluster-setup.html

Signed-off-by: Oswaldo Montenegro <[email protected]>
@oswaldo-montenegro-codecentric oswaldo-montenegro-codecentric changed the title feat(Keycloak) Include out-of-the-box high availability mechanism eat(keycloak) Enable Keycloak cluster nodes discovery by default Aug 4, 2023
@oswaldo-montenegro-codecentric
Copy link
Author

@devRoemer Thanks for your comment! I refactored my change in a way that is more flexible to change and adapt.

@oswaldo-montenegro-codecentric oswaldo-montenegro-codecentric changed the title eat(keycloak) Enable Keycloak cluster nodes discovery by default feat(keycloak) Enable Keycloak cluster nodes discovery by default Aug 4, 2023
@grieshaber
Copy link
Collaborator

Hi @oswaldo-montenegro-codecentric, thanks for the PR. Imho the defaults seem to be mixed right now.. with the default values, keycloak is installed with a single replica and autoscaling disabled, but your changes imply default cluster nodes discovery mechanisms.

From my pov, we should comment your changes, so that they are included in the values for the reference but disabled as a default. I'm not in the topic enough to verify, whether your changes do have negative impact in non-cluster mode.

@github-actions
Copy link

This pull request has been marked as stale because it has been open for 30 days with no activity. It will be automatically closed in 10 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Oct 16, 2023
@github-actions github-actions bot closed this Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants