docs(ops): document the #3315 revision contract, and correct it after the health split - #3402
Merged
Merged
Conversation
… the health split #3315 stamped both dashboard images with their git revision and exposed it from /healthz. The stamping is in main and works; what did not survive is the documentation of it, because the health endpoint was refactored underneath it. Two stale facts, both now wrong against the code: - docs/OPERATIONS.md's "Service health contract" section showed the /livez body as {"live":…,"built":…} with no revision at all. That is the one page an operator opens to learn what these endpoints answer, and the field this issue exists to add was missing from it. - docs/ARCANE-GIT-SYNC.md showed /healthz as {"ok":true,"es":true, "revision":…}. #3317 split the endpoint into /livez (no Elasticsearch) and /readyz (503 + reason), so ok and es no longer exist. The doc described a response shape the service stopped returning a refactor ago, which is worse than saying nothing: an operator greps for it and concludes the stamp is broken. Adds the revision contract to OPERATIONS.md where the task asked for it -- the four surfaces (health body, boot log, /build.json, image label), where GIT_SHA comes from, what "unknown" means and why it is the honest default, how to turn it on, and verify-deploy.sh's three-valued exit codes. Also states why /readyz deliberately carries no revision: readiness is a property of the running container, not of a build, and the two answers diverge during a rollback. Verification (this branch, no code changed): cargo test 549 passed, 0 failed, 1 ignored cargo run --bin openapi | diff openapi.json byte-identical, no drift openapi.json 132 paths; /healthz {200,502}, /livez {200}, /readyz {200} cargo build --release ok (GIT_SHA stamped in) Live, against a release binary built with GIT_SHA=$(git rev-parse HEAD): /healthz 200 {"live":true,"built":"…","revision":"17c7e412adfb…"} /livez 200 (identical body) /readyz 503 {"ready":false,"reason":"elasticsearch is unreachable: …"} That last pair is the constraint the task named: /livez answered 200 with Elasticsearch pointed at a dead port, so it stays ES-independent and cannot restart-loop the container, while /readyz 503s with a reason. verify-deploy.sh driven end to end against that live process: 0 on a matching revision, 1 on a stale one, 0 on an abbreviated `git rev-parse --short HEAD` (a reported abbreviation, not a cry-wolf), and 2 — not 1 — when the endpoint cannot be read, so an unreadable body can never read as a stale deploy. Same for the frontend's /build.json form. Image labels exercised for real: the runtime-stage ARG/LABEL block was extracted from both Dockerfiles and built against busybox, stamped and unstamped. Stamped -> the real revision; unstamped -> "unknown" for both revision and created, never a fabricated value. scripts/tests/test_3315_image_revision.py: 40 passed, 48 subtests. The four docs gates (paths-exist, reachable, stale-paths, compose-env) pass. Refs #3315 Co-authored-by: OpenCode <opencode@users.noreply.github.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
… reporting the host The "Docs regression tests (#2576)" lane, "Scripts and Compose" and "Quality gate" are one failure, not three. All 20-odd tests in test_3283_fix.py died on the homeserver runner before reaching an assert: composable-templates: reading /tmp/arkime-composable-shadow/shadow.json: EACCES: permission denied run_script passed no SHADOW_FILE, so composable-templates.js fell back to its default -- one fixed name under os.tmpdir(). It opens that path 0700 and the file 0600 for a reason (#3343: the stash is the only record that db.pl's templates existed), so the first user to touch the directory owns it and every later run by another user is locked out. On a fresh container nothing else has been there and the suite is green; on a long-lived runner it is not, and which of those you get depends on the machine rather than the code. The suite's sibling already had this right: test_3343_fix.py passes SHADOW_FILE and inherits os.environ. This brings 3283 in line, one TemporaryDirectory per run, so the env stays the closed set it was rather than becoming os.environ. Two tests pin it, and both fail without the SHADOW_FILE line: - the default path under a private TMPDIR is planted unreadable, which is the runner's exact condition, and the run must still succeed. Planting it in the real /tmp would make the test depend on the shared state it is asserting independence from. - `shadow` then `generate` is how the stash travels in production, so that is the pairing driven here. The second run must not put back a template it never shadowed. generate alone cannot catch this: it reads the stash but never writes it. Not a chmod, and not a skip. The suite must not need to write outside its own tmpdir to run at all. Unrelated and left alone: two scripts/tests failures in test_compose_drift_watch_sweep (privileged-fallback rows) reproduce with this change stashed, and that CI row is green on this branch. Refs #3315 Co-authored-by: OpenCode <opencode@users.noreply.github.com>
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
2 participants
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
#3315's stamping is already merged and working. The work this PR does is the
part that did not survive: the documentation of the contract, which the
/healthzrefactor (PR #3382) invalidated.Found by re-verifying the issue end to end rather than re-implementing it. The
runtime code is correct and current:
revisionis on the liveness body, bothDockerfiles take
ARG GIT_SHA,build.rscarries it in, the frontend bakes/build.json, andscripts/verify-deploy.shexists. What is wrong is that bothdocs describing the contract now contradict the code.
docs/OPERATIONS.md§Service health contract{"live":…,"built":…}— norevisionfield at alldocs/ARCANE-GIT-SYNC.md§Proving which revision is deployed{"ok":true,"es":true,"revision":…}{"live":…,"built":…,"revision":…}okandeshave not existed since #3317 split the endpoint. That is the worseof the two: an operator greps a live response for a field the docs promised and
concludes the stamp is broken.
Scope
Docs + one test file. No route, handler, status code, auth tier, or
openapi.jsonbyte changed. The one additive field on the health response wasalready in
main.Also: this PR was red, and now is not
The three red checks —
Docs regression tests (#2576),Scripts and Compose,Quality gate— are one failure, not three.Docs regression tests (#2576)is a matrix row, so the other two are just the aggregate jobs reading its
result.
The row was failing on
maintoo (17 failures, run36319112911), and notbecause of anything #3315 touches:
run_scriptintests/docs/test_3283_fix.pypassed noSHADOW_FILE, socomposable-templates.jsfell back to its default — one fixed name underos.tmpdir(). The script opens that path0700and the file0600for a realreason (#3343: the stash is the only record that
db.pl's templates existed), sowhoever touches the directory first owns it and every later run by another user
is locked out. Green in a fresh container, red on a long-lived runner: the suite
was reporting the host, not the code.
The fix is one
TemporaryDirectoryper run, which is what the suite's siblingtest_3343_fix.pyalready does — so this brings 3283 in line rather thaninventing a convention. Deliberately not a chmod and not a skip: a test suite
must not need to write outside its own
tmpdirto run at all.Two tests pin it, and both fail with the
SHADOW_FILEline removed:test_the_stash_is_ours_alone_and_never_the_default_tmp_pathTMPDIRtest_two_runs_do_not_see_each_others_stashshadowthengenerate, the pairing production uses; the second must not restore what it never shadowedLeft alone on purpose: two
scripts/testsfailures intest_compose_drift_watch_sweep(privileged-fallback rows). They reproduce withthis change stashed and that CI row is green on this branch, so they are a local
environment artifact, not a regression to fold in here.
Verified
Live, against a release binary built with
GIT_SHA=$(git rev-parse HEAD):/livezanswering 200 with Elasticsearch on a dead port is the constraint theissue named, and it holds: no ES on the liveness path, so no restart loop.
verify-deploy.shdriven against that process: 0 on a matching revision,1 on a stale one, 2 — not 1 — when the endpoint cannot be read, so an
unreadable body can never read as a stale deploy.
Why docs/OPERATIONS.md specifically
The issue says to document the contract there, and it is also the right home:
it already owns the
/livez·/healthz·/readyztable and the"What to point at what" list, and it is the page an operator opens to learn what
those endpoints answer. Added
### Which revision is deployed (#3315): the foursurfaces, where
GIT_SHAcomes from, what"unknown"means and why it is thehonest default, how to turn it on, and the three-valued exit codes. Also stated
why
/readyzdeliberately carries norevision— readiness is a property ofthe running container, not of a build, and the two answers diverge during a
rollback.
Issues
Refs #3315
Closesis deliberately not used: merging this does not finish #3315, becauseGIT_SHAis still not set on the homeserver. Nothing repo-tracked writes it — thestacks'
.envfiles are root-owned and provisioned outside this repository, and/opt/stacks/apiaryis an rsync with.git/excluded, so there is no repositoryon the host for compose to ask. The first
verify-deploy.shrun will correctlyreport not stamped until an operator adds one line per stack, which is
documented rather than invented here.
Security impact
.envfiles were added.Ports, routes and auth tiers are untouched.
revisionwas already public on/livez; a git object name names no secret, and it is the same string the imagealready carries in
org.opencontainers.image.revision.Validation
The constraints the issue named, checked rather than assumed.
openapi.jsonwas regenerated, never hand-edited — and it needed noregeneration, because the health responses are annotated
body = inline(serde_json::Value), so an added struct field does not reach thedocument. The 1 ignored test is the pre-existing
report_pdf::tests::scratch_dump_multi_page_pdf("scratch: dumps a multi-page PDFto /tmp for manual inspection"). No test was weakened or deleted.
Live curl against a running release binary (
GIT_SHA=$(git rev-parse HEAD),ES deliberately pointed at a dead port):
Revision populated, not null and not empty. That third line is the constraint
that mattered:
/livezreturned 200 with Elasticsearch unreachable, so theprobe is still ES-independent and an ES blip cannot restart-loop the container,
while
/readyzstill 503s with a reason. The split is intact and unmoved.scripts/verify-deploy.shdriven end to end against that live process:PASS … matches the expected revisionFAIL deployed 17c7e412…, expected 0000…git rev-parse --short HEADPASS 17c7e412 is an abbreviation of 17c7e412adfb…— a reported abbreviation, not a cry-wolf/build.jsonImage labels exercised for real. The runtime-stage
ARG/LABELblock wasextracted from both Dockerfiles and built against
busyboxso the shippedtext was under test, not a retyped copy. Stamped → the real revision;
unstamped →
unknownfor bothrevisionandcreated— the honest default,never a fabricated value.
Both normalizers agree, which is the real risk in a two-language stamp:
write-build-info.mjsfed an UPPERCASE sha emits3dca4457f1b2c0d4e5a69788796a5b4c3d2e1f0aband"unknown"when unset,matching Rust's
normalize_revisionand the sharedrevision-corpus.jsontable. If those two ever disagree the failure is a deploymismatch that is not one.
Not validated: the two dashboard images were not built end to end
(
docker buildof the real backend Dockerfile is a full cargo release buildunder the 1.98.0 toolchain — the label mechanism was proven instead, by
extracting and building the real
ARG/LABELblock as described above). Thefrontend image was not built either;
write-build-info.mjswas run directly andits output shape verified. Nothing was run against the live homeserver, and
GIT_SHAremains unset there, so the end-to-end "stamped deploy" path isuntested by design — that is the state the docs describe.
Rollout
None — docs only, and merging does not deploy.
Turning the stamp on is separate and operator-side, unchanged by this PR: add
GIT_SHA=$(git rev-parse HEAD)to each stack's.env, then thePOST /projects/{id}/buildthat a content-change sync does not do.