Skip to content

Use /healthz HTTP probes and graceful termination settings - #28

Merged
CRThaze merged 1 commit into
comet-ml:mainfrom
gaul:healthz-probes
Jul 27, 2026
Merged

Use /healthz HTTP probes and graceful termination settings#28
CRThaze merged 1 commit into
comet-ml:mainfrom
gaul:healthz-probes

Conversation

@gaul

@gaul gaul commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

S3Proxy has served an unauthenticated GET /healthz since 3.2.0, within this chart's appVersion (3.3.0), so the probes can do better than a TCP accept check:

  • Probes: switch tcpSocket to httpGet /healthz, which distinguishes a serving proxy from one that merely holds a listen socket open (a wedged JVM passes a tcpSocket probe indefinitely). When config.tls.enabled, the probes use scheme: HTTPS; the kubelet does not verify certificates for HTTPS probes, so self-signed keystores keep working — addressing the concern in the original tcpSocket comment.
  • preStop sleep 5: endpoint removal propagates asynchronously on pod deletion; serving briefly after the deletion mark keeps late-routed connections from landing on a pod that has already begun shutting down.
  • terminationGracePeriodSeconds: 40 (new documented value): covers the preStop sleep plus the up-to-30-second in-flight request drain that S3Proxy performs on SIGTERM starting with releases after 3.3.0 (gaul/s3proxy@d637104f). On those releases a terminating pod also fails readiness while draining, which the httpGet readiness probe surfaces — pods leave Service endpoints while transfers finish instead of dropping them.

Housekeeping per CONTRIBUTING: chart version bumped to 0.4.0, the new value carries a helm-docs # -- comment, and README.md is left untouched for the release automation.

Validated locally: helm lint clean; helm template across the test-values/ scenarios (including both TLS scenarios, which are the only renders emitting scheme: HTTPS); kubeconform -strict passes on the rendered default and TLS manifests (12/12 resources).

Context: s3proxy recently gained container-focused behavior this chart can lean on — the shutdown drain above and reference manifests at examples/kubernetes, which link this chart as the Helm option. The new *_FILE credential variables were considered and are deliberately not used here: the merge-configs initContainer already keeps credentials in mounted files rather than env vars or argv.

🤖 Generated with Claude Code

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@CRThaze
CRThaze merged commit a865133 into comet-ml:main Jul 27, 2026
21 of 22 checks passed
@gaul
gaul deleted the healthz-probes branch July 27, 2026 17:38
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.

2 participants