cron pod is now always deleted gracefully#72
cron pod is now always deleted gracefully#72henzigo wants to merge 2 commits intojg/webserver-probesfrom
Conversation
henzigo
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the Kubernetes cron Deployment and deploy flow so cron workloads are stopped via Kubernetes mechanisms (scale-down + preStop) to allow in-flight cron executions to finish before the pod is terminated, and consolidates cron resources into the main webserver kustomization output.
Changes:
- Move cron resources (Deployment + configmaps) into the
kustomize/webserverbuild output and remove the separatekustomize/cronlayer and expectedcron.yamlfixtures. - Update the cron Deployment to use
Recreate, add a long termination grace period, and replace the oldsleeppreStop with adeploy:cron:lock+deploy:cron:watchshutdown sequence. - Update deploy script to scale
deployment/cronto 0 and wait for rollout prior to running migrations.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/scenarios/production-with-cloudflare/expected/webserver.yaml | Expected manifest now includes cron configmaps + cron Deployment in the webserver output. |
| tests/scenarios/production-with-cloudflare/expected/cron.yaml | Removed expected cron-only output (cron now part of webserver output). |
| tests/scenarios/escaping-env/expected/webserver.yaml | Expected manifest now includes cron configmaps + cron Deployment in the webserver output. |
| tests/scenarios/escaping-env/expected/cron.yaml | Removed expected cron-only output (cron now part of webserver output). |
| tests/scenarios/development-single-domain/expected/webserver.yaml | Expected manifest now includes cron configmaps + cron Deployment in the webserver output. |
| tests/scenarios/development-single-domain/expected/cron.yaml | Removed expected cron-only output (cron now part of webserver output). |
| tests/scenarios/basic-production/expected/webserver.yaml | Expected manifest now includes cron configmaps + cron Deployment in the webserver output. |
| tests/scenarios/basic-production/expected/cron.yaml | Removed expected cron-only output (cron now part of webserver output). |
| kubernetes/kustomize/webserver/kustomization.yaml | Adds cron Deployment + cron configmaps to the webserver kustomization resources list. |
| kubernetes/kustomize/cron/kustomization.yaml | Removes the standalone cron kustomization. |
| kubernetes/deployments/cron.yaml | Updates cron rollout/termination behavior and adds preStop lock+watch shutdown logic. |
| deploy/parts/deploy.sh | Switches from in-pod locking/watching to scaling cron down/up around migrations. |
| UPGRADE.md | Documents the behavioral change for v5.1.0 upgrades. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| command: ["/bin/sh","-lc"] | ||
| args: | ||
| - > | ||
| ./phing warmup > /dev/null | ||
|
|
||
| # FIFO for logs | ||
| rm -f /tmp/log-pipe && mkfifo /tmp/log-pipe && chmod 666 /tmp/log-pipe | ||
|
|
||
| # crontab | ||
| crontab -u root /var/spool/cron/template | ||
|
|
||
| # run on backround | ||
| stdbuf -o0 tail -n +1 -f /tmp/log-pipe & | ||
|
|
||
| exec crond -f || exec cron -f |
There was a problem hiding this comment.
args uses a folded block scalar (>) while also including shell comment lines (e.g. # FIFO for logs). YAML folding joins the # ... line with the next command into a single shell line, causing the following command(s) to be commented out at runtime (e.g. rm -f ..., crontab ..., stdbuf ...). Use a literal block scalar (|) or remove/move the # comment lines so each command remains on its own line when executed.
| - > | ||
| ./phing warmup > /dev/null | ||
|
|
||
| # FIFO for logs | ||
| rm -f /tmp/log-pipe && mkfifo /tmp/log-pipe && chmod 666 /tmp/log-pipe | ||
|
|
||
| # crontab | ||
| crontab -u root /var/spool/cron/template | ||
|
|
||
| # run on backround | ||
| stdbuf -o0 tail -n +1 -f /tmp/log-pipe & | ||
|
|
||
| exec crond -f || exec cron -f |
There was a problem hiding this comment.
The previous single-line args used &&, so failures in ./phing warmup, FIFO setup, or crontab would stop the container and surface the error. With the new multi-line script, failures won’t stop execution unless you explicitly enable fail-fast (e.g., set -e near the top) or reintroduce && chaining for the critical steps.
| # crontab | ||
| crontab -u root /var/spool/cron/template | ||
|
|
||
| # run on backround |
There was a problem hiding this comment.
Typo in the shell comment: “backround” → “background”.
| # run on backround | |
| # run in background |
No description provided.