Skip to content

[WIP] SEP-1944: Fix issue with feature-build harness executor node - #1442

Closed
yyyyyyyan with Copilot wants to merge 1 commit into
mainfrom
copilot/sep-1944-fix-feature-build-harness-issue
Closed

[WIP] SEP-1944: Fix issue with feature-build harness executor node#1442
yyyyyyyan with Copilot wants to merge 1 commit into
mainfrom
copilot/sep-1944-fix-feature-build-harness-issue

Conversation

Copilot AI commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.

Work on the following bug ticket and open a PR with title starting with "SEP-1944: ", making sure pre-commit and tests pass.

The completion contract is the "Acceptance Criteria" section, or "Expected Behavior" when the ticket has no Acceptance Criteria section. Implement exactly that and nothing beyond it.

"Context", "Steps to Reproduce", "Actual Behavior" and "Environment" are diagnostic background recording how the defect was found. Use them to understand the defect, but do not treat them as a task list, and do not attempt to reproduce the bug or bring up databases, containers or other infrastructure — you cannot run this project's environment. A section describing hypotheses that were investigated and ruled out is context, not work. "Implementation Notes" are suggestions, not requirements.

The feature-build harness README says it has no executor node, but the mysql profile provides one

Context

The PMM + SEP feature-build harness (sidecar/pmm-fb/, on the pmm branch) gained a seeded MySQL target node, sep-mysql, in 📍 SEP-1817 so that task execution could be exercised locally. Its README.md still describes the pre-📍 SEP-1817 topology: it tells the reader the harness has no executor other than the Nomad client inside pmm-server, and that artifact download therefore cannot be exercised. Both statements are false once the mysql profile is up, and the same file contradicts them twice.

This matters beyond tidiness: the README tells a tester that the executor they are trying to use does not exist.

On the report this came from. Raised during the PMM Tech Preview validation pass, 📍 PMM-15381 (2026-08-28), where sep-mysql was reported as unusable as an execution host. That symptom did not reproduce — see Actual Behavior — so this ticket is scoped to the documentation defect that is reproducible, plus one latent robustness gap found while investigating. 📍 PMM-15381 also reported that a MySQL service registered directly with PMM did not appear in the picker; that is expected behaviour, since the picker lists Nomad nodes rather than PMM inventory, and is not in scope here.

Steps to Reproduce

  1. Check out the pmm branch and change into sidecar/pmm-fb/.
  2. Run ./bootstrap.sh, then docker compose --profile mysql up -d --build.
  3. Wait for sep-mysql to log Imported the employees seed dataset.
  4. Confirm sep-mysql is a working executor: GET /api/sep/hosts/ lists it, and Nomad reports it ready with a healthy raw_exec driver. No inventory sync is needed for this — see the note under Acceptance Criteria.
  5. Read the BASE_URL bullet in sidecar/pmm-fb/README.md (the last bullet of "How the pieces connect").

Expected Behavior

The README describes the executor topology the harness actually has, so a tester reading it can tell that sep-mysql is an execution host and that the artifact-download path is exercised when the mysql profile is up.

Actual Behavior

sidecar/pmm-fb/README.md:204-208 states, in the paragraph justifying BASE_URL:

This harness has no PMM Client nodes: its only executor is the Nomad client inside pmm-server [...] The consequence is that the harness does not exercise the production artifact-download path.

Both sentences are false with the mysql profile up, and the same file contradicts them twice:

README.md:85 documents that --profile mysql adds sep-mysql as "the sep-mysql task-execution target".

README.md:253-255 explains that sep-mysql runs privileged: true with cgroup: host precisely because "a containerised Nomad client needs it".

The BASE_URL value (http://sep-sidecar:9000/sep) stays correct, but for a different reason than the one given: sep-mysql is on the same compose network and resolves sep-sidecar through the same Docker DNS.

The reported symptom did not reproduce. On a clean bring-up (2026-09-01) every stage was green on first boot: the datadir initialised, the seed imported, the node registered with pmm-server, Nomad reported sep-mysql as status=ready eligible with raw_exec detected and healthy, GET /api/sep/hosts/ returned both pmm-server and sep-mysql, and pmm-admin status inside the container showed pmm_agent Connected with all exporters running. So the original failure was specific to that run. The most plausible candidate, and the one worth fixing on its own merits, is the recreate path:

A recreate rather than a first boot. On first boot, register_with_pmm runs pmm-agent setup, which blocks on a server round-trip, so pmm-server has ample time to become ready. On a recreate the datadir and agent id both persist, so the entrypoint skips init, seed and agent setup and lands directly on retry_cmd 30 2 pmm-admin list (sidecar/pmm-fb/mysql-entrypoint.sh:256) with no prior server round-trip behind it. On timeout it exits 1 and the node is simply absent. The comment above that line already notes the recreate has "no such wait behind it".

The side-car was still crash-looping. On a fresh database its API is unreachable for a couple of minutes, so nothing can list hosts. Tracked separately in 📍 SEP-1946.

Stale volume state. Rotating the bootstrap-generated credentials after first boot does not take effect, and dropping sep-mysql-data alone leaves the exporters authenticating with the old pmm password — which surfaces as a MySQL connection failure. mysql-target.md already documents this and the recovery (drop sep-mysql-pmm-config as well), so no change is needed for it.

Environment

SEP / Monitor Host

• SEP version: image percona/percona-sep:249f6738ae7a4ba495b34f6ccc5edf0cad4db5c3

• SEP settings (if relevant): feature-build harness sidecar/pmm-fb/, branch pmm at e6aa2b1

• Hostname / Address: sep-sidecar (compose network)

• OS: container, linux/amd64

DB / Execution Host

• Database type/version: Percona Server 8.4.10 (sep-mysql, mysql compose profile)

• Hostname / Address: sep-mysql (172.28.9.40, compose network)

• OS: container, linux/amd64

Client

• PMM server image: perconalab/pmm-server-fb:PR-4500-a2f83c2

• Bring-up: docker compose --profile mysql up -d --build

Acceptance Criteria

  • The BASE_URL paragraph in sidecar/pmm-fb/README.md no longer claims the harness has no PMM Client nodes, and no longer claims it cannot exercise the artifact-download path. The BASE_URL value itself is unchanged and is re-justified on the real reason: sep-mysql is on the same compose network and resolves sep-sidecar through Docker's embedded DNS.
  • The recreate path's registration gate no longer depends on pmm-server already being warm: sidecar/pmm-fb/mysql-entrypoint.sh:256 widens the pmm-admin list gate from retry_cmd 30 2 (~60s) to retry_cmd 90 2 (~180s) — roughly twice the slowest cold start observed for this stack (observed range and reasoning in Approach). The comment above the call states the event the budget is sized against, so the next reader can re-derive it. An explicit pmm-server readiness probe is deliberately not added; Approach records why.
  • The README's Bring-up section gains an executor check alongside the existing post-up checks (PMM UI, SEP API through nginx, SEP APIs directly): sep-mysql appears in GET /api/sep/hosts/, and the check states that it is independent of the inventory syncer (see the note below). This is the check that would have distinguished the 📍 PMM-15381 report from a healthy stack.
  • sidecar/pmm-fb/mysql-target.md cross-references the Bring-up note in README.md for how long the employees seed import takes and how to tell it has finished. It must not restate them independently — two copies of the same timing will drift.

Note on the executor check, qualifying the third criterion. mysql-target.md states that the node "never appears in SEP" until a sync runs, but that prerequisite governs the MySQL service as a backup source, which reaches SEP only through the PMM syncer. The executor list is a different surface: app/sep/api/routes/hosts.py sources it from the Tasks API (Nomad nodes) and consults Inventory only to replace a node name with a display name, tolerating Inventory failures by falling back to the raw node name. The reproduction above confirms this — both hosts came back with name equal to id, i.e. unenriched, with sep-mysql present. Conflating the two surfaces is the same class of mistake this ticket exists to correct, so the new check must not imply a sync is required.

Implementation Notes

These notes are suggestions to help the implementer get started — not requirements. The implementer should use their own judgement and may take a different approach if they find a better one.

Affected files

All on the pmm branch — see the branch note below.

sidecar/pmm-fb/README.md (lines 200-209) — the BASE_URL bullet: drop both false claims and re-justify the value.

sidecar/pmm-fb/README.md (lines 108-115) — the Bring-up post-up checklist: add the executor check.

sidecar/pmm-fb/mysql-entrypoint.sh (lines 248-259) — the pmm-admin list gate and its preceding comment.

sidecar/pmm-fb/mysql-target.md (lines 1-10) — the seed-import description near the top.

No test files, no migration, and no consumers: nothing under app/** references the harness, and it ships in no published image.

How the contradiction arose

Worth knowing before editing, because it locates the defect: the false paragraph is a later regression, not an oversight in 📍 SEP-1817.

• 2026-08-13, 7e8dcb1 (📍 SEP-1817) added sep-mysql and the README's --profile mysql section describing it as the task-execution target.

• 2026-08-15, 9e5fe18 (docs: rewrite the harness README for the file channel and native proxy) rewrote the README and, in the BASE_URL paragraph, restated the pre-📍 SEP-1817 topology — its own commit message records the reasoning as "the harness's only Nomad executor shares pmm-server's network namespace".

So the rewrite reintroduced a superseded description into a file that already documented the new node two days earlier. That is precisely why the same file contradicts itself twice, and it means the fix is to correct the newer paragraph, not to add anything the harness is missing. No root-cause issue link is recorded: 9e5fe18 carries no ticket key.

Branch: this work targets pmm, not main

sidecar/pmm-fb/ exists only on the pmm branch. All three files above are absent from main and tracked on origin/pmm (verified against origin/pmm at e6aa2b1, which is current — zero commits of drift from the commit the reproduction ran on). Branch from pmm and target the PR at pmm; a branch cut from main will not contain the directory at all. 📍 SEP-1683 scopes this deliberately — it keeps the compose harness on the pmm branch.

Approach

The two documentation items are direct edits.

Mechanism for the entrypoint gate: widen the existing retry_cmd budget. Do not add a readiness probe. retry_cmd is already the file's idiom and is already used this way for pmm-agent setup on the adjacent branch of the same function, so an explicit pmm-server readiness wait would introduce a second mechanism for the same job and a second thing to keep sized. The alternative is recorded here rather than left open in the AC: it is rejected on consistency, not on capability.

Sizing, and why the figure from the original report is not the one to use. The report justified this change with a "~90 second" cold start attributed to 📍 SEP-1817. That figure is real but hardware-specific: re-measured 2026-08-20 against PR-4500-7dcd575, a cold boot on an empty volume reached its first readyz pass at 34s on fast NVMe. So the observed cold-start range across the two hardware profiles measured is roughly 34s to 90s, and the current ~60s budget (retry_cmd 30 2 sleeps only between attempts: 29 x 2s) sits inside that range rather than safely above it. Two points follow:

  1. The current budget is not categorically broken — on the fast profile it has ~26s of margin — so do not implement this as "fix a budget that cannot work". It is hardening against hardware variance.
  2. pmm-admin list is gated on more than readyz: it needs pmm-managed to accept the agent's two-way channel, which lands after the server reports ready. So both numbers above are floors for this gate, not its cost, and the top of the range is the one to size against.

Hence retry_cmd 90 2 (~180s): about twice the slow end of the observed range, which keeps margin on hardware slower than either profile measured — where a recreate is most likely to trip. Record "sized at ~2x the slowest observed pmm-server cold start (~90s)" in the comment above the call so the next reader can re-derive it rather than re-measuring.

Risks

• The reported 📍 PMM-15381 symptom is not reproducible, so none of the three candidates above can be confirmed as its cause. The entrypoint change is justified on its own merits, not as a proven fix — do not close 📍 PMM-15381 on the strength of it.

• Verification of the entrypoint change requires a recreate against a warm datadir (the path that skips pmm-agent setup), not a clean up. A clean first boot exercises the branch that already works and will pass regardless of the change.

• The mysql profile builds a MySQL image and imports ~125 MB of seed data, so a full verification cycle costs several minutes per run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants