fleetd #552: warn instead of aborting when the post-restart mktemp fails #560

2026-09-12T10:55:35+02:00

agent commented

2026-09-12 10:55:35 +02:00

Fixes #552.

By the time the fresh-log mktemp ran, the daemon had already been stopped, the jar swapped, and the new daemon started. An unguarded mktemp failure there still aborted the whole script, so a caller read the resulting non-zero exit as "the redeploy failed" and the likely next action was to restart an already-correctly-restarted daemon.

Fix

Extracted the mktemp into a new capture_fresh_log_region(), guarded the same way unload_launchd_if_loaded/stop_systemd_if_loaded guard their own (if ! ... ; then), but warn instead of die — there is nothing left to protect by refusing after a successful restart.
FRESH_LOG="" is declared and the trap installed before the call that may fail to fill it in, not after.
classify_amqp_connection_errors and report_shutdown_drain each gain a new state for an uncapturable log region (REDEPLOY_AMQP_CHECK_SKIPPED, REDEPLOY_DRAIN_STATE=skipped), distinct from "captured a region with nothing in it" — each says so in its own output.
The "result" section gains a matching branch so a skipped capture can never silently read as a clean bill of health.

Tests added (scripts/test-redeploy-fleetd.sh)

test_capture_fresh_log_region_control / test_capture_fresh_log_region_mktemp_failure_returns_nonzero_and_prints_nothing — the extracted function itself, stubbing mktemp on PATH (same technique PR #548 added).
test_fresh_log_capture_failure_does_not_abort_under_sete — drives the real guarded call-site shape under set -euo pipefail with mktemp stubbed to fail, proving the script does not abort.
test_no_unguarded_mktemp_assignments_after_restart — source-text shape check: no bare VAR="..." after the restart call, so the next one added is caught too.
test_capture_fresh_log_region_call_site_is_guarded — pins the real call site is guarded, since it no longer contains the literal text "mktemp" for the shape check above to see.
test_classify_amqp_connection_errors_reports_skipped_when_log_missing, test_report_shutdown_drain_skipped_when_log_missing, test_report_shutdown_drain_na_wins_over_missing_log, test_result_section_checks_amqp_skip_before_no_error_lines — the third-state handling in each of the four readers.

Every new test was mutation-proofed by hand: pristine anchor counted, mutated with line-anchored sed, confirmed red with the test's own failure message, restored, confirmed byte-identical hash, confirmed green.

Build/test run

bash scripts/test-redeploy-fleetd.sh — exit 0, ends PASS: redeploy log classifier. No Java files touched, so mvn was not run; CI's separate shell-tests job covers this suite.

Out of scope (noted, not investigated): #555 (the main-flow seam and the eight untested decisions), same file, sequenced after this.

Fixes #552. By the time the fresh-log `mktemp` ran, the daemon had already been stopped, the jar swapped, and the new daemon started. An unguarded `mktemp` failure there still aborted the whole script, so a caller read the resulting non-zero exit as "the redeploy failed" and the likely next action was to restart an already-correctly-restarted daemon. ## Fix - Extracted the `mktemp` into a new `capture_fresh_log_region()`, guarded the same way `unload_launchd_if_loaded`/`stop_systemd_if_loaded` guard their own (`if ! ... ; then`), but `warn` instead of `die` — there is nothing left to protect by refusing after a successful restart. - `FRESH_LOG=""` is declared and the `trap` installed *before* the call that may fail to fill it in, not after. - `classify_amqp_connection_errors` and `report_shutdown_drain` each gain a new state for an uncapturable log region (`REDEPLOY_AMQP_CHECK_SKIPPED`, `REDEPLOY_DRAIN_STATE=skipped`), distinct from "captured a region with nothing in it" — each says so in its own output. - The "result" section gains a matching branch so a skipped capture can never silently read as a clean bill of health. ## Tests added (scripts/test-redeploy-fleetd.sh) - `test_capture_fresh_log_region_control` / `test_capture_fresh_log_region_mktemp_failure_returns_nonzero_and_prints_nothing` — the extracted function itself, stubbing `mktemp` on PATH (same technique PR #548 added). - `test_fresh_log_capture_failure_does_not_abort_under_sete` — drives the real guarded call-site shape under `set -euo pipefail` with `mktemp` stubbed to fail, proving the script does not abort. - `test_no_unguarded_mktemp_assignments_after_restart` — source-text shape check: no bare `VAR="..."` after the restart call, so the next one added is caught too. - `test_capture_fresh_log_region_call_site_is_guarded` — pins the real call site is guarded, since it no longer contains the literal text "mktemp" for the shape check above to see. - `test_classify_amqp_connection_errors_reports_skipped_when_log_missing`, `test_report_shutdown_drain_skipped_when_log_missing`, `test_report_shutdown_drain_na_wins_over_missing_log`, `test_result_section_checks_amqp_skip_before_no_error_lines` — the third-state handling in each of the four readers. Every new test was mutation-proofed by hand: pristine anchor counted, mutated with line-anchored `sed`, confirmed red with the test's own failure message, restored, confirmed byte-identical hash, confirmed green. ## Build/test run `bash scripts/test-redeploy-fleetd.sh` — exit 0, ends `PASS: redeploy log classifier`. No Java files touched, so `mvn` was not run; CI's separate `shell-tests` job covers this suite. Out of scope (noted, not investigated): #555 (the main-flow seam and the eight untested decisions), same file, sequenced after this.

agent added 1 commit 2026-09-12 10:55:35 +02:00

fleetd #552 : warn instead of aborting when the post-restart mktemp fails

CI / shell-tests (pull_request) Successful in 7s

Details

CI / contract (pull_request) Successful in 49s

Details

CI / build (pull_request) Successful in 2m7s

Details

f188947750

By the time the fresh-log mktemp ran, the daemon had already been stopped, the
jar swapped, and the new daemon started — an unguarded mktemp failure there
aborted the whole script anyway, so a caller read the resulting non-zero exit
as "the redeploy failed" and would restart an already-correctly-restarted
daemon.

Extract the mktemp into capture_fresh_log_region, guarded the same way
unload_launchd_if_loaded/stop_systemd_if_loaded guard their own, but warn
instead of die: there is nothing left to protect by refusing after a
successful restart. The trap is now installed before the assignment it cleans
up, using an FRESH_LOG="" sentinel readers can check.

classify_amqp_connection_errors and report_shutdown_drain both gain a new
state (REDEPLOY_AMQP_CHECK_SKIPPED / REDEPLOY_DRAIN_STATE=skipped) for an
uncapturable log region, distinct from "captured a region with nothing in
it" — and the result section gains a matching branch, so a skipped capture
can never read as a clean bill of health.

ltms merged commit 7a3b2bb7ee into main

2026-09-12 10:58:10 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#560