[ycombinator]

What is the problem this PR solves?

The integration tests in internal/pkg/server intermittently fail the race detector with:

WARNING: DATA RACE
Read at ... by goroutine N:
  zerolog.TestWriter.Write()
  testing.(*common).Logf()
  ...
  fleet.go:310 loggedRunFunc.func1()

Previous write at ... by goroutine M:
  testing.tRunner.func1()

loggedRunFunc (called by subsystem goroutines inside runSubsystems) logs when each subsystem exits using the zerolog context logger. In tests, that logger has a zerolog.TestWriter that writes to testing.T.Logf. If a subsystem goroutine logs its exit message after the test's testing.T has been torn down, the race detector fires.

How does this PR solve the problem?

startTestServer was starting srv.Run in an errgroup goroutine but never guaranteed that goroutine had fully exited before testing.T was invalidated.

The fix: derive srvCtx with its own cancel from the caller-supplied context, and register a t.Cleanup that:

1. Calls srvCancel() to signal the fleet server to stop.
2. Calls tsrv.waitExit() to block until all goroutines have exited, failing the test if the server exits with an unexpected error.

By the time testing.T is cleaned up, every fleet server goroutine — including loggedRunFunc — has already returned.

Related issues

• Observed in feat: add synchronous enrollment write strategy to prevent ghost agents #7712 CI run (build #16516).

[mergify]

This pull request does not have a backport label. Could you fix it @ycombinator? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-./d./d is the label to automatically backport to the 8./d branch. /d is the digit
• backport-active-all is the label that automatically backports to all active branches.
• backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
• backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

[Copilot]

Pull request overview

This PR addresses an intermittent -race failure in internal/pkg/server integration tests by ensuring Fleet Server goroutines fully exit before testing.T teardown, preventing late logs via zerolog.TestWriter from racing with test cleanup.

Changes:

• Derives a dedicated cancellable server context for the Fleet Server run loop in startTestServer.
• Registers a t.Cleanup to cancel the server context and wait for the server goroutine group to exit.
• Uses the derived server context when waiting for server readiness.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[github-actions]

TL;DR

The Buildkite failure is a test failure in TestStandAloneRunningSuite/TestAgentGracefulForceUnenroll, not infrastructure. The new UNENROLL assertion is too strict/timing-sensitive and intermittently misses the expected log line.

Remediation

• Relax step-2 verification in testing/e2e/stand_alone_test.go to avoid requiring a specific log line containing both fixed text and agentID; prefer asserting final observable state (agent process exits/dormant behavior) or match a broader server signal.
• Keep the current step-1 check (status --output json components empty), then validate with a deterministic postcondition that does not depend on fragile log timing.

Investigation details

Root Cause

TestAgentGracefulForceUnenroll was recently extended to assert UNENROLL by scanning Fleet Server stderr for:

• "Returning UNENROLL action for agent with invalid API key"
• and the enrolled agentID

This check is in testing/e2e/stand_alone_test.go:932-940.

The failing Buildkite run reports only this test failing while all other E2E tests pass, which is consistent with a test-specific assertion race/fragility rather than a broad service outage.

Evidence

• Build: https://buildkite.com/elastic/fleet-server/builds/16524
• Job/step: E2E Test (.buildkite/scripts/e2e_test.sh)
• Key log excerpt:
  • --- FAIL: TestStandAloneRunningSuite/TestAgentGracefulForceUnenroll (222.06s)
  • --- FAIL: TestStandAloneRunningSuite (740.98s)
  • Other tests in suite pass (including connection-failure/flakiness scenarios).

Code evidence:

• testing/e2e/stand_alone_test.go:930-940 describes and enforces log-based UNENROLL detection.
• The assertion requires both substrings in a polling loop, making it sensitive to log formatting/order/timing.

Verification

• Not run locally in this environment (workflow is read-only analysis).

Follow-up

If this test still flakes after loosening log matching, consider recording/checking the UNENROLL action via a structured API-level signal instead of stderr text parsing.

---

What is this? | From workflow: PR Buildkite Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

[Copilot]

🟢 Approval recommended

The change is small, targeted to test lifecycle management, and directly addresses the reported race by ensuring server goroutines exit before testing.T teardown.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The change is narrowly scoped to test infrastructure and correctly ensures server goroutines are stopped before testing.T teardown, addressing the described data race.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

internal/pkg/server/fleet_integration_test.go:237

• srvCtx is shadowed by the errgroup.WithContext assignment, which makes it easy to confuse the pre-errgroup cancelable context with the errgroup-derived context. Avoid the shadowing to keep the shutdown wiring obvious.

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[github-actions]

@Mergifyio backport 9.5 9.4 8.19

[mergify]

backport 9.5 9.4 8.19

✅ Backports have been created

Details

• Backport to branch 9.5 completed

No pull request needed: No commits between 9.5 and mergify/bp/9.5/pr-7714

• Backport to branch 9.4 completed

No pull request needed: No commits between 9.4 and mergify/bp/9.4/pr-7714

• Backport to branch 8.19 completed

No pull request needed: No commits between 8.19 and mergify/bp/8.19/pr-7714