Repository navigation
Conversation
There was a problem hiding this comment.
@andreaTP Thanks for creating this PoC.
Please see my comment in the original discussion: #11763 (reply in thread). In summary, -1 from me for this change.
|
@vmuzikar this PR has been done to enable the community to experiment with the 2 options, after we receive feedback we can decide in which direction we want to move. |
|
Thanks for providing this, I will give it a try tomorrow! |
|
@andreaTP Unfortunately, I have been struggling with getting the self-built operator to run. I basically followed https://github.com/keycloak/keycloak/blob/main/operator/README.md, updated to newest kubectl, minikube etc, built the operator successfully (image is in my local registry) bun when |
|
@andreaTP Something still does not work. What I see after building from the operator/app directory is:
I manage to install the operator using: But that's probably not the right way. I also have a problem with Minikube and Podman trying to push the operator image into Minikube's registry on my Mac. There it complains about: |
|
@andreaTP Never mind, I got it to work. The reason was the |
|
@sschu good to hear that you got it to work! |
|
@andreaTP - I gave the new version a try, and it works nicely when scaling up/down. One thing I found: I had an existing deployment of Keycloak and switched the environment variable of the operator. The operator then created a stateful set in addition to the deployment, so I now had both. The most safe way would be to delete the keycloak CR first with this PoC. If this option would get into the real operator, I would expect the operator to delete the deployment before creating the stateful set and vice versa When scaling up and down from 1 to 10 pods, I still see the "no physical address" message. The good news is that it seems to appear less often with a stateful set than with a deployment. I seems to occur for 1 in 10 pods being started. I'll open a new issue for that observation and will link this PR and the discussion to it. |
|
@andreaTP I also finally managed to test the operator with stateful set and it works like a charm. I can also confirm @ahus1 observation that switching the operator configuration creates a deployment in parallel. The pods actually formed a cluster which means it would at least be easy to migrate from deployment to stateful set and vice versa without downtime. I also changed the operator to use maxsurge=1 and maxunavailable=1 to check if that helps with deployments. Which means the operator would have to adapt count changes in the custom resource definition one by one to the deployment. Thats definitely some effort considering a stateful set provides this for free. |
|
@ahus1 @sschu thanks a lot for the feedback. I can locally reproduce all of your observations, the most meaningful being: I was working under the assumption that the "Rolling Update" settings would apply to scaling as well, which is not correct. |
I wouldn't insist for no downtime migration from deployment to statefulset and back with no downtime. A minimum requirement for me would be that the old one would be deleted in the process. |
|
At this stage, the operator is still in "preview" release, so we will allow ourselves to do breaking changes. The migration path for the next version will probably be:
This process will automatically delete the old resources as they have the correct Owner References. |
|
@andreaTP - I was testing a bit today and found that the deployment has set "maxUnavailable" to "1". When doing a rolling update of a single instance, this would lead to a downtime. I'd suggest to have "maxUnavailable" at "0" when there is only a single instance. I'm noting this in this issue as I understand that you'll probably revisit this in the process of StatefulSet vs. Deployment anyway. If you'd rather have a new issue for that, please let me know. Best, |
|
Superseded by #12757 |
This PR is just a demonstration generated from the Discussion #11763
I quickly hacked around a switch at the operator level to let ppl play with Deployment vs. StatefulSet.
To test this branch:
andreatp/keycloak-operator-enable-statefulset:0.0.1(produced from this branch)kubectl apply -f operator/app/src/main/kubernetes/kubernetes.ymlfrom this branch in the correct namespacekeycloak-operatorDeploymentnamedOPERATOR_USE_STATEFULSETe.g. hereThis PR builds on top of #12437