-
Notifications
You must be signed in to change notification settings - Fork 514
fix(cli): exec remaining start wrappers so docker stop is not 10s (CLI-2192) #6203
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,9 +50,23 @@ const LEGACY_LOGFLARE_API_KEY = "api-key"; | |
| * the `unless-stopped` restart policy retries until the db is ready — | ||
| * running Logflare against an unmigrated database lets Oban die on the | ||
| * missing `public.oban_jobs` table instead. | ||
| * | ||
| * `run.sh` stays PID 1 on purpose: a plain `exec` of `beam.smp` still burned | ||
| * Docker's 10s SIGTERM grace (upstream hang). Forward TERM, wait 3s, then | ||
| * KILL. Interrupted `wait` is >128; a second `wait` recovers the BEAM's | ||
| * status unless it was already reaped (127). | ||
| */ | ||
| const LEGACY_LOGFLARE_ENTRYPOINT_SCRIPT = | ||
| "cat <<'EOF' > run.sh && sh run.sh\n./logflare eval Logflare.Release.migrate &&\n./logflare start --sname logflare\nEOF\n"; | ||
| "cat <<'EOF' > run.sh && exec sh run.sh\n" + | ||
| "./logflare eval Logflare.Release.migrate || exit $?\n" + | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| "./logflare start --sname logflare &\n" + | ||
| "BEAM_PID=$!\n" + | ||
| 'trap \'kill -TERM "$BEAM_PID" 2>/dev/null; n=0; while [ "$n" -lt 3 ] && kill -0 "$BEAM_PID" 2>/dev/null; do n=$((n+1)); sleep 1; done; kill -KILL "$BEAM_PID" 2>/dev/null\' TERM\n' + | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When analytics is enabled, this supervisor's correctness depends on real PID-1 signal delivery and the Logflare image's shutdown behavior, but the added unit test only compares the generated shell string. I checked the existing AGENTS.md reference: apps/cli/AGENTS.md:L520-L523 Useful? React with 👍 / 👎. |
||
| 'wait "$BEAM_PID"\n' + | ||
| "code=$?\n" + | ||
| 'if [ "$code" -gt 128 ]; then wait "$BEAM_PID" 2>/dev/null; code2=$?; [ "$code2" -ne 127 ] && code=$code2; fi\n' + | ||
| 'exit "$code"\n' + | ||
| "EOF\n"; | ||
|
|
||
| export interface LegacyLogflareContainerSpecInput { | ||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This intentionally changes legacy Docker behavior relative to the Go reference for Kong, Vector, Logflare, Edge Runtime, and
functions serve, butdocs/go-cli-divergences.mdcontains no entry for the new PID-1/signal-handling behavior. Add one behavioral-divergence entry covering these wrappers so support and future parity work do not mistake the changed shutdown semantics for accidental drift.AGENTS.md reference: apps/cli/AGENTS.md:L540-L544
Useful? React with 👍 / 👎.