keycloak / keycloak-benchmark

Keycloak Benchmark
https://www.keycloak.org/keycloak-benchmark/
Apache License 2.0
130 stars 72 forks source link

Consider a Deployment instead of a StatefulSet #977

Open ahus1 opened 1 month ago

ahus1 commented 1 month ago

Change Keycloak Operator for multi-site / clusterless to use a deployment as there without JGroups and the clustered caches there is no need for a StatefulSet any more.

This would simplify the deployment, and would allow for faster rolling updates.

Original discussion: https://github.com/keycloak/keycloak/discussions/11763#discussioncomment-10630505

ahus1 commented 1 month ago

FYI: @shawkins / @mhajas / @ryanemerson / @kami619 / @pruivo

pruivo commented 1 month ago

It crossed my mind last week. There is no restriction from the Infinispan side but I'm not sure how database migration is handled if multiple Keycloak nodes start concurrently (patch upgrade).

Also, the Keycloak operator would have to be modified to check the features enabled and decide to create a stateful set or a deployment. It will make the logic more complex.

ahus1 commented 1 month ago

On database migration, the is a locking mechanism based on the database, so Keycloak is fine if multiple Pods with a new version start up concurrently. They will wait until the first has finished the migration.

And yes, the Operator would need to know about this. And it would need to do something safe (enough) to switch between the two.

ryanemerson commented 1 month ago

And yes, the Operator would need to know about this. And it would need to do something safe (enough) to switch between the two.

In the case of moving away from a cluster with either the MULTI_SITE or CLUSTERLESS feature, all pods would need to be recreated anyway, so I don't think the switching logic would be too complex as we just have to gracefully scaledown the Deployment, remove it and then create a new Statefulset.

The changes being explored to improve the rolling configuration update experience could be useful here, as we could potentially state a feature's requirements (i.e. compatible with StatefulSet and/or Deployment) as part of the upgrade metadata.