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

fix: Disable rolling upgrades for Caddy Kubernetes deployment #530

Closed
wants to merge 1 commit into from

Conversation

fghaas
Copy link
Contributor

@fghaas fghaas commented Nov 18, 2021

Caddy (as used by Tutor) is a single-instance stateful application, in that it uses a PersistentVolume that can only ever be mounted to one Pod. As such, it is unsuitable for rolling upgrades. In the course of an upgrade, we first need to terminate the existing Pod, then bring up its replacement (just like the other applications configured with PersistentVolumeClaims, like MySQL and MongoDB).

Streamline the caddy deployment with all others that use PVCs, and set its strategy to the Recreate type.

Reference: https://kubernetes.io/docs/tasks/run-application/run-single-instance-stateful-application/

Caddy is a single-instance stateful application, in that it uses a
PersistentVolume that can only ever be mounted to one Pod. As such, it
is unsuitable for rolling upgrades. In the course of an upgrade, we
first need to terminate the existing Pod, then bring up its
replacement (just like the other applications configured with
PersistentVolumeClaims, like MySQL and MongoDB).

Streamline the caddy deployment with all others that use PVCs, and set
its strategy to the Recreate type.

Reference: https://kubernetes.io/docs/tasks/run-application/run-single-instance-stateful-application/
@regisb
Copy link
Contributor

regisb commented Nov 22, 2021

I don't understand what issue this PR is addressing (again, please forgive my lack of k8s expertise 😓). Can you please describe more precisely which scenario we are trying to avoid here? Is this somehow related to this other conversation?

@fghaas
Copy link
Contributor Author

fghaas commented Nov 22, 2021

This is really a cosmetic issue ("explicit is better than implicit").

Right now, if I run tutor k8s start (with RUN_CADDY: true), Tutor deploys a pod from the docker.io/caddy:2.3.0 image, with its StrategyType set to RollingUpdate (this is the default).

However, that Deployment also maps a Docker Volume, /data, from a Kubernetes Persistent Volume (PV).

If I were to now set DOCKER_IMAGE_CADDY: docker.io/caddy:2 (which, at this time, maps to version 2.4.6), and then run tutor config save followed by tutor k8s start, Kubernetes would do a RollingUpdate replacement if it were not for the PV mapping. But because there is a PV mapping, that can't work, so it falls back to a Recreate strategy in which it throws away the old pod before spinning up a new one.

So, might as well specify the Recreate strategy explicitly, like we do on all the other services that use PVs.

@fghaas fghaas marked this pull request as draft November 22, 2021 16:26
@fghaas
Copy link
Contributor Author

fghaas commented Nov 22, 2021

Marking this one as a Draft. I might have a thinko here.

@fghaas
Copy link
Contributor Author

fghaas commented Nov 22, 2021

In view of #533, please disregard this PR.

So, let's please drop this one. Sorry for the noise!

@fghaas fghaas closed this Nov 22, 2021
@fghaas fghaas deleted the caddy-strategy-recreate branch January 4, 2022 15:44
@fghaas fghaas restored the caddy-strategy-recreate branch May 10, 2022 12:26
@fghaas
Copy link
Contributor Author

fghaas commented May 10, 2022

Just so we have a cross-reference: there's a somewhat related patch in #660.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants