Wait for the old pmm-agent to exit in PMM-T2227

[claude]

Failures fixed (investigator)

• source: PMM Integration Tests run 31852528968 (scheduled, main @ 37a003d), job CLI / Integration / Generic (94930840155)
• tests:
  • cli/tests/generic.spec.ts:577 / @generic — PMM-T2227 "Verify tarball upgrade"

What failed

The only red job in the run. Its Run CLI tests step is || true, so the failure
surfaced one step later when launchable gate exited 1 on Actionable Failures | 1:

tests/generic.spec.ts:577:7 › PMM-T2227 - Verify tarball upgrade @generic
  Error: Stdout does not contain     170
  expect(received).not.toContain(expected)
  Expected substring: not "    170\n"
  Received string:        "    170\n    351\n"
    at cli/tests/generic.spec.ts:604

170 is the pid of the pmm-agent running before the tarball upgrade; 351 is the one
started after it. Both were in ps at the moment the assertion read it.

Root cause — a zero-margin race in our test, not the product

await cli.exec(`docker exec ${containerName} pkill -f pmm-agent`);
await cli.exec(`docker exec -d ${containerName} pmm-agent --debug --config-file=...`);
const newPid = await cli.exec(`docker exec ${containerName} ps -C pmm-agent -o pid=`);
...
await newPid.outNotContains(oldPid.stdout);

pkill returns once SIGTERM is delivered, not once the process is gone. The test
waits zero milliseconds for the old agent to exit, so if its teardown outlives the two
docker exec round-trips that follow, ps lists both pids and the assertion reports a
restart that did happen as one that did not.

The upgrade the test is actually verifying completed correctly — the run's own output
shows the other two assertions (Connected, new version) were never reached only
because this one failed first.

Evidence (throwaway Linode VM, main @ 37a003d)

Reproduction environment built by following .github/workflows/runner-integration-cli-tests.yml
step for step: PMM Server perconalab/pmm-server:3-dev-latest, digest
sha256:c431a6e1a488c965c3bed3c56c8a9a882fbd125b64f68269409bc08fbd078821 — byte-identical
to the image the failing run recorded to Launchable — the same latest-tarball client, and
the same --database pdpgsql=16 --database ps,ENCRYPTED_CLIENT_CONFIG=true setup.

The flake itself did not reproduce: PMM-T2227 passed 5/5 at main. What the box did
show is why the shape of the CI output can only be this race:

measurement	value
pmm-agent teardown after pkill (SIGTERM → gone)	25–85 ms (10 samples, incl. immediately after the tarball upgrade, and with pmm-server paused)
the test's own window — pkill returning → ps running	387–566 ms (10 samples, box at 3× CPU oversubscription)

So the margin here is ~6–15×, which is why an idle 6-CPU VM never trips it; CI evidently
did, at ~450 ms of unaccounted teardown.

Two controls pin down which race it is, and rule out "pmm-agent ignored SIGTERM":

• A second pmm-agent cannot start while the first is alive — it panics binding the
  agent-local JSON server (agent/agentlocal/agent_local.go:339) and dies. Verified
  directly on the box.
• kill -STOP on the old agent before pkill (so it can never process the signal)
  reproduces that instead: ps shows only the old pid, because the replacement died
  on the port. kill -CONT then lets it handle the queued SIGTERM and exit.

For CI to have seen both 170 and 351 alive, 170 must therefore have already
released its listeners and been mid-exit — a shutdown in progress, not a hang. That is
exactly what a zero-wait assertion catches, and it is not a product defect: nothing
promises pkill makes a process vanish synchronously.

Fix

Poll until the old pid is out of ps before starting the replacement. Bounded at 30s and
failing loudly (Old pmm-agent process is still running after SIGTERM!) if it never exits,
so a genuine "pmm-agent ignores SIGTERM" regression still surfaces rather than being
waited away. It also removes the second, nastier failure mode above, where the replacement
agent starts into an occupied port and dies.

Verification

On the same VM, on this branch:

• PMM-T2227: 3/3 passed (container recreated between runs).
• Full --grep "@generic|@unregister": 43 passed, 13 skipped, 1 failed. The one
  failure is PMM-T1219, exit code 127 from unzip pmm-summary.zip — unzip is not
  installed on the repro VM; it is present on the GitHub runner and that test passed in
  the failing CI run. Unrelated to this change.
• npx tsc --noEmit: no errors from generic.spec.ts (only pre-existing tsconfig.json
  TS 7.0 deprecation warnings).

Because the flake never reproduced, this change is verified by mechanism rather than by a
red-to-green flip: the assertion it removes had no timing margin by construction. Only the
next scheduled runs can confirm the flake is gone from CI.

---

Generated by Claude Code