SEP-1871: Repin both pins to the current builds, and own pmm-server's start period - #1396
Open
yyyyyyyan wants to merge 5 commits into
Open
SEP-1871: Repin both pins to the current builds, and own pmm-server's start period#1396yyyyyyyan wants to merge 5 commits into
yyyyyyyan wants to merge 5 commits into
Conversation
…its state The side-car now obtains its own Grafana service-account token at container start. The previous pin, 29d2962, predates that work, so a bring-up on it would start a side-car with no mint step in it and verify nothing. Repinned to 95a7038, the main-line commit that added it. Verified on the published artifact rather than on the commit, in the style README.md's "Which image to pin" already prescribes for its two other load-bearing properties: helper present /home/sep/app/grafana_service_account.py state dir /home/sep/state, mode 700, owned 1001:1001 fan-out function export_grafana_token in settings-env.sh reads secrets from dir 4 SECRETS_DIR references in settings-env.sh HEALTHCHECK CMD /home/sep/app/healthcheck.sh, start period 150s The sep-state volume is what makes the token survive a container recreate. The side-car persists it mode 0600 under /home/sep/state and re-reads it on the next start, so a restart needs no Grafana admin credential; without the volume every recreate mints a further non-expiring token on the same service account and the earlier ones stay valid until an operator deletes them. The bring-up section's token paragraph described PMM as the party that mints and publishes the token. That is no longer what makes a first boot work, so it now describes the side-car's own mint, the volume that persists it, and the fact that a token arriving through SECRETS_DIR still outranks it.
…ts start period PMM_FB_TAG moves from PR-4500-882b6ba to PR-4500-73298a4, in both places the default is spelled. 882b6ba was derived from a tree still containing PMM-15280 and PMM-15331, both of which were closed unmerged on 2026-08-20, and it could not bring the pair up at all: pmm-server exited on the PMM_SEP_POSTGRES_PASSWORD that #1383 stopped seeding, and then on /srv/sep not being writable, because that build wrote /srv/sep-secrets and did not ship the directory #1382 mounts. 73298a4 fixes all three. Measured on the artifact: /srv/sep ships 2770 uid 1000 gid 0 and seeds into the mounted volume; the secrets directory holds four files and no Grafana token; both containers reach healthy; an authenticated /sep/api/apps/ round trip answers 200. Losing PMM-15331 also lost the widened start period it carried. The image ships 25s with 3 retries at 4s, so pmm-server is marked unhealthy around 37s, while a cold start here takes ~72s to first pass readyz — and because sep-sidecar depends on service_healthy, compose aborted the dependent rather than waiting. Observed on both green tags before this change. The harness now overrides the healthcheck with a 300s start period, which is the property the build used to provide; `docker compose up -d` completes in one invocation again. sep-sidecar keeps condition: service_healthy. The README's standing advice was to fall back to service_started when repinning to a build without the gate, but that trades an aborted bring-up for a side-car that wins the race and exits on a missing SECRET_KEY. Health still means the four files exist; the start period is what gives a cold start room to get there. Two doc claims the repin invalidates are corrected with it: the caveat citing PMM-15331 as the source of the start period, and the count of files removed when PMM_ENABLE_SEP is unset, which is four rather than six now that the two Grafana token names are gone.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the PMM feature-build harness to support side-car-managed Grafana tokens while preserving reliable startup ordering.
Changes:
- Repins PMM and SEP container images.
- Persists the side-car’s Grafana token in a named volume.
- Extends PMM startup health timing and updates harness documentation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
sidecar/pmm-fb/README.md |
Documents token minting, persistence, and startup behavior. |
sidecar/pmm-fb/compose.yaml |
Repins images, adds token state storage, and overrides PMM health timing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
bootstrap.sh generates three sep-mysql test-fixture passwords and nothing else —
PMM provisions every secret the pair actually needs. But compose.yaml guarded
them with ${VAR:?run ./bootstrap.sh first}, and Compose interpolates every
service at parse time regardless of which profiles are active, so the guard on
this profile-gated service made the script a prerequisite of the pmm-server +
sep-sidecar bring-up too. Without .env, `docker compose config` exited 1 on both
the default and the mysql profile.
The guard is also duplicated. mysql-entrypoint.sh defines require_secrets with
the same three names and the same "run ./bootstrap.sh first" message, and calls
it unconditionally at top level before any use of the values, so the mysql path
stays guarded whatever compose does. Passing the three through unset removes the
redundant copy rather than weakening the check: a mysql run without bootstrap
still fails loudly, at container start instead of at parse.
Verified with no .env present at all: config exits 0 on both profiles, and a
bring-up reaches sep-sidecar healthy with four secret files, a token minted at
mode 600 owned 1001:1001, and one sep service account in Grafana.
The bring-up section now leads with the two-service command and no prerequisite,
and shows bootstrap.sh only under the mysql profile.
…ecret ownership The healthcheck block copied the image's probe and timings so it could set a 300 s start period, but Docker merges the healthcheck field by field: setting start_period alone inherits test, interval, timeout and retries from the image. The copy had already drifted from the pin it was written against — interval 5s against the image's 4s, timeout 5s against its 2s — and would have gone on probing a hardcoded readyz endpoint across repins. Reduced to start_period. The README still said PMM generates and owns every SEP secret in three places, one of them added by this branch. That held while PMM published the Grafana token; the side-car now mints its own, so the claims are scoped to the four files SEP reads from disk and the token gets its own bullet.
compose.yaml's own header still opened by mandating ./bootstrap.sh and asserting PMM owns every SEP deployment secret — the two claims this branch retired everywhere else, in the file it retired them from. It now scopes bootstrap.sh to the mysql profile and names the Grafana token as the side-car's. "Which image to pin" enumerated the two properties a pin must carry. The sep-state volume adds a third: an image without the mint passes both existing checks, comes up healthy, and mounts the volume over a directory nothing writes to, with inert Grafana auth as the only symptom. That is the failure this branch is correcting, so the checklist meant to prevent it now covers the mint.
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
The side-car now obtains its own Grafana service-account token at container start (landed on
mainas part of SEP-1871). Three harness changes follow.Side-car repin.
sep-sidecarpinned…:29d29624, a main-line tag predating the mint. Repinned to…:95a70382, the main-line commit that added it — currently bothmain's tip and the newest published tag. Verified on the published artifact rather than on the commit, in the style the harness README's "Which image to pin" section already prescribes:grafana_service_account.pyin the app root700, owned1001:1001export_grafana_tokeninsettings-env.shSECRETS_DIRreferences insettings-env.shCMD healthcheck.sh, start period 150ssep-statevolume. The side-car persists the minted token mode0600under its state directory and re-reads it on the next start, so a restart needs no Grafana admin credential. The volume extends that across a container recreate. Without it every recreate mints a further token on the same service account, and since minted tokens are asked not to expire, the ones earlier containers resolved stay valid in Grafana until an operator deletes them.pmm-server repin, and owning its start period.
PMM_FB_TAGmoves fromPR-4500-882b6batoPR-4500-73298a4, in both places the default is spelled. That tag was derived from a tree still containing PMM-15280 and PMM-15331, both closed unmerged on 2026-08-20, and could not bring the pair up at all — pmm-server exited on thePMM_SEP_POSTGRES_PASSWORDthat #1383 stopped seeding, then on/srv/sepnot being writable, since that build wrote/srv/sep-secretsand did not ship the directory #1382 mounts.Losing PMM-15331 also lost the widened start period it carried. The image ships 25 s with 3 retries at 4 s, so pmm-server is marked
unhealthyaround 37 s while a cold start here takes ~72 s to first passreadyz— and becausesep-sidecardepends onservice_healthy, compose aborted the dependent rather than waiting. This was reproducible on both green feature builds.compose.yamlnow sets a 300 s start period of its own, restoring what the build used to provide, anddocker compose up -dcompletes in one invocation again. That start period is the only healthcheck field it sets: Docker merges the healthcheck field by field, so the probe and the remaining timings keep tracking whatever the pinned image ships rather than a copy that goes stale across repins.sep-sidecarkeepscondition: service_healthy. The README's standing advice was to fall back toservice_startedwhen repinning to a build without the gate; that trades an aborted bring-up for a side-car that wins the race and exits on a missingSECRET_KEY. Health still means the four secret files exist — the start period is what gives a cold start room to reach that point.The doc claims the repin invalidates are corrected alongside it: the caveat citing PMM-15331 as the source of the start period, the count of files removed when
PMM_ENABLE_SEPis unset, which is four now that the two Grafana token names are gone, and four statements that PMM generates or owns every SEP secret — true while PMM published the Grafana token, and now false by exactly that one credential. Those are scoped to the four files SEP reads from disk, and the token gets its own bullet recording that the side-car owns it while a build publishing the two canonical names still outranks the mint.compose.yaml's own header carried two of them, alongside aRun ./bootstrap.sh firstline the degating below it had just retired.The repin checklist gains the property this branch makes load-bearing. "Which image to pin" listed two things a pin must carry; the
sep-statevolume adds a third, because an image without the mint passes both existing checks, comes up healthy, and mounts the volume over a directory nothing writes to — with inert Grafana auth as the only symptom, and nothing logged. The old pin this PR replaces was exactly such an image, so the checklist meant to catch it now probes for the mint helper and the state directory. The bring-up section's token paragraph, which credited PMM with minting and publishing the token, now describes the side-car's own mint, the volume that persists it, and the fact that a token arriving throughSECRETS_DIRstill outranks it.bootstrap.shis now genuinely mysql-only. It generates threesep-mysqltest-fixture passwords and nothing else — PMM provisions every secret the pair needs — butcompose.yamlguarded them with${VAR:?run ./bootstrap.sh first}, and Compose interpolates every service at parse time regardless of active profiles. So a guard on this profile-gated service made the script a prerequisite of the two-service bring-up too: with the generated dotenv absent,docker compose configexited 1 on both profiles. The three are now passed through unset. This removes a duplicated guard rather than weakening one —mysql-entrypoint.shdefinesrequire_secretswith the same three names and the same message, and calls it unconditionally at top level before any use, so a mysql run withoutbootstrap.shstill fails loudly, at container start instead of at parse.Tested
Measured on this branch against
PR-4500-73298a4, and separately againstPR-4500-7dcd575, with no overrides beyond a host port remap (an unrelated pmm-server holds 8443 on this machine). Torn down withdown -vafter each run.docker compose configexits 0 on both profiles, anddocker compose up -dreachessep-sidecarhealthy with four secret files, a token minted mode600owned1001:1001, and onesepservice account in Grafana.docker compose up -dcompletes in one invocation, ~72 s, both containers healthy. Without the start-period override the same command aborts withdependency failed to start: container … is unhealthy.SECRET_KEY— and no Grafana token, which is the "no token configured anywhere" precondition.id=2 name=sep role=Admin tokens=1.600owned1001:1001in thesep-statevolume.sep,inventory,tasks,celery-worker,celery-beat— carry both canonical names, each equal to the persisted token, read from each process's own/procenviron./graph/login200 → session exchange → SEP bearer →/sep/api/apps/200 with four apps.celery-worker's own environment,PMM.api_keyresolves and an authenticatedGET /graph/api/org/usersreturns one org user. That is the call that previously no-opped silently.GF_SECURITY_ADMIN_*appears indocker inspectoutput.Left for QA in its own environment:
docker compose up -d --force-recreate sep-sidecar) and confirm the token survives, which is what thesep-statevolume buys over a plain restart.--profile mysqland confirm thesep-mysqlbuild picks up the samePMM_FB_TAGdefault.Note for anyone repeating this: a
docker execinto the side-car starts from the image's declared environment and never sees PID 1's exports, so probing the token that way reports it unset even when every supervised program has it. That is the design working — inheritance is also why it stays out ofdocker inspect— but it reads like a failure. Read a supervised child's/proc/<pid>/environinstead.Known limitations
PMM-15205-sep-fbbefore looking at SEP.Checklist
New/modified functions have type hints and rST docstrings(N/A — compose and docs only)New tests added for new features or bug fixes(N/A — the mint's tests live onmain)Database migrations generated if models changed(N/A for this change)