feat(zookeeper): universal container process management framework (product-first pilot) - #199
Open
whg517 wants to merge 4 commits into
Open
feat(zookeeper): universal container process management framework (product-first pilot)#199whg517 wants to merge 4 commits into
whg517 wants to merge 4 commits into
Conversation
Implement the process management framework from docs/container-process-management.md as a product-first pilot in the zookeeper image (see zncdatadev#198). The framework is vendored under zookeeper/kubedoop/ for now; it will be extracted into kubedoop-base once validated across a few products, to avoid rebuilding every downstream image on each framework iteration. What this adds: - tini v0.19.0 as PID 1 (arch-aware, SHA256-pinned) for signal forwarding and zombie reaping - Universal entrypoint (bin/entrypoint.sh) that sources lib/*.sh, runs auto-discovered pre-script hooks from /kubedoop/mount/pre-script, backgrounds the CMD, and manages its lifecycle - lib/log.sh: structured logging with control-char stripping - lib/run-phase.sh: root-owned executable .sh discovery (uid 0 guard against injection from non-root writable volumes) and sequential execution - lib/signal.sh: graceful shutdown (SIGTERM -> timeout -> SIGKILL) with real exit-code propagation and post-script hooks - File ownership model: framework/mount root-owned 0755, only /kubedoop/run writable by the kubedoop user (sticky 1775) Two correctness fixes over the design's initial draft (both verified with a bash signal/exit-code test matrix): - Do not use `trap '' SIGTERM` to guard the startup race: SIG_IGN is inherited by the backgrounded child and a non-interactive child shell cannot re-trap it, so shell-based apps/hooks would silently ignore graceful shutdown. Use a real deferring handler instead (reset to SIG_DFL in the execed child). - Do not let the interrupted `wait` (128+signum) clobber the real exit code that cleanup() captured on the signal path. Deferred (per design): lib/pod-state.sh state coordination, the sidecar watchdog, and extraction into kubedoop-base. Refs: zncdatadev#198 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
5 tasks
There was a problem hiding this comment.
Pull request overview
This PR pilots the repository’s universal container process management framework in the zookeeper image (product-first rollout), adding a tini-based PID 1 setup plus a vendored /kubedoop/bin/entrypoint.sh + /kubedoop/lib/*.sh lifecycle framework. It also adds source/image-level tests for entrypoint behavior and updates docs and build tooling to match the rollout approach.
Changes:
- Add
tini+ universal entrypoint framework to the zookeeper image and fix the Log4Shell patch target path. - Vendor the framework libraries (
log.sh,run-phase.sh,signal.sh) and add new entrypoint tests (source + image wiring). - Update docs and repo tooling (.gitignore / Makefile) to support the rollout and local tooling download behavior.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
zookeeper/Dockerfile |
Installs tini, deploys vendored entrypoint framework, adjusts Log4Shell patch target, wires ENTRYPOINT. |
zookeeper/kubedoop/bin/entrypoint.sh |
New universal entrypoint orchestrating phases and lifecycle. |
zookeeper/kubedoop/lib/log.sh |
New structured logging helper used by the framework. |
zookeeper/kubedoop/lib/run-phase.sh |
New hook discovery/execution for pre/post runtime phases. |
zookeeper/kubedoop/lib/signal.sh |
New lifecycle + signal handling implementation (shutdown, exit-code preservation). |
zookeeper/tests/source-entrypoint-framework.sh |
New source-level behavioral tests for lifecycle logic. |
zookeeper/tests/image-entrypoint-framework.sh |
New image-level tests validating wiring, hooks, and signal behavior through tini. |
zookeeper/tests/README.md |
Documents how to run the new entrypoint tests. |
zookeeper/AGENTS.md |
Documents new vendored framework + test artifacts in the product. |
docs/container-process-management.md |
Updates design doc status/rollout notes and aligns lifecycle description with implementation. |
Makefile |
Fixes local yq download URL. |
.gitignore |
Adjusts ignore rules to avoid ignoring vendored framework libs; adds ignore for generated bin/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+71
to
+76
| wait "$pid" 2>/dev/null | ||
| local rc=$? | ||
| if [[ "$timed_out" -eq 1 ]]; then | ||
| wait "$pid" 2>/dev/null | ||
| rc=$? | ||
| fi |
…ess wait race - Materialize script discovery output in run_phase so find/sort failures abort the phase instead of looking like an empty directory - Re-wait in stop_process only when SIGALRM actually interrupted wait, so a child reaped at the timeout boundary keeps its real exit code - Run the entrypoint framework tests in the zookeeper CI build job - Small hardening: quote exit status, guard cd in tests, scope bin/ rule in .gitignore to the repo root
… stage Download and checksum-verify the static tini binary in a dedicated tini-downloader stage based on java-devel; the result image only receives the verified binary via COPY --from and no longer runs download tooling. Pin the version declaratively in versions.yaml (flows in as the TINI_VERSION build arg) and sync AGENTS.md and the design doc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the container process management framework from docs/container-process-management.md (#197) as a product-first pilot in the zookeeper image.
Closes #198.
What's Included
Why Product-First
The design eventually places the framework in kubedoop-base, but changing that base image rebuilds every downstream image in the PR pipeline. This PR vendors the framework into ZooKeeper first so the behavior can be proven on one product before extraction into kubedoop-base.
Tests
The image-level test validates the built image wiring: ENTRYPOINT, USER, file ownership/permissions, exit-code propagation, root-owned hooks, SIGTERM forwarding through tini and the entrypoint, and SIGKILL timeout escalation.
Note: buildx reported cache export warnings for registry cache targets, but the local image build completed successfully and loaded quay.io/zncdatadev/zookeeper:3.9.3-kubedoop0.0.0-dev.
Deferred