Skip to content

ci(windows): keep the test data off the runner's system drive - #235

Merged
rabbitson87 merged 6 commits into
mainfrom
fix/windows-job-drive-stall
Nov 13, 2026
Merged

rabbitson87 merged 6 commits into
mainfrom
fix/windows-job-drive-stall

Conversation

@rabbitson87

Copy link
Copy Markdown
Member

What this changes

The windows CI job failed 11 of its 32 completed runs on main. Five of those had causes that #220 and #232 removed. The other six have three causes. This change fixes two of them and records the third.

Cause Runs What this change does
A. About 11 minutes after a hosted Windows runner boots, about 1.2 GB are read from its system drive in 36 to 50 s. A flushed write there takes 1.1 to 2.8 s, and a document test that writes reaches the 10 s limit of one guest call. 4 The Tests step keeps its data below RUNNER_TEMP, on the temporary drive, and prints the boot time of the runner. The stall watch of #232 is removed, because the cause is found.
B. tailscale.com v1.104.0 dereferences a nil pointer when a status with a network map is still in work as its backend closes. 1 Recorded in docs/network-provider-lifecycle.md. No code changes.
C. TestSpareAdoption compared two readings of a clock that Windows can give as equal. 1 The test now starts the adoption only when the clock has passed its reading. The assertion is the same.

The move of the test data has one side effect that review found: the temporary drive keeps no 8.3 short names, so TestExactNameRefusesARealShortAlias of internal/site skipped there. The step now runs that package first, with the default temp directory and -count=1.

No product code changes. The diff holds one workflow, three test files, and documents.

Cause B stays

The job, and a host, can still end with the panic of the Tailscale library, at the rate so far of 1 in 32 runs of the job, until Flats uses a release of the library that has commit b9d5ff3ae (2026-11-10, "ipn/ipnlocal: don't apply netmap from a replaced control client"). No such release exists on 2026-11-13: v1.104.7 is the newest. The commit removes the sequence of the CI event. A backend that closes while it handles its first network map stays possible on the development branch. I did not reproduce the panic here (480 runs of the retry tests and 720 closes near the first network map).

Evidence

  • Cause A: 18 hosted runners in four runs were measured. 17 showed the reads in the counters, 11 min 1 s to 11 min 41 s after the boot. On the 18th the counters began after the reads came. The boot times of the four runners of the CI failures are not recorded, so those events fit the clock of the reads, and no record shows the reads in them. Which process reads the drive is an inference.
  • On three runners that kept the data on C:, the run that met the reads had a guest call of 7.9 to 11.2 s, and one of the three failed two tests with the errors of the four CI events. On three runners that kept the data on D:, the longest guest call of 75 runs was 2.7 s.
  • The complete Tests step ran on 9 hosted runners with -count=1 in three runner checks. The last one is on the tree of the final commit plus a check workflow: 6 900 tests passed on each runner, 0 failed, 0 packages from the test cache, no log line of a later command with the old temp directory, and internal/privatedir ran. The first command passed all six internal/site tests on each runner.
  • TestSpareAdoption passes 30 of 30 on Windows and on Linux, and fails 3 of 3 under each of two mutations of internal/runtime/manager.go.
  • go vet ./... passes for windows/amd64, linux/amd64 and darwin/arm64. No Go test ran on macOS.

Limits

  • The step has no general guard for a later test that needs a property of the system drive. Such a test skips or fails on the temporary drive, and a skip shows only in the skip list of the log.
  • A worker process keeps its own temp path. The data directories move. What a worker writes below its own temp path was not examined.
  • The runner checks ran in a temporary workflow with -count=1. The CI run of this pull request is the first run of the step as ci.yml has it, with the test cache of the job.
  • Whether the document app can truncate its write-ahead log outside the call that commits is an open question in docs/spec/windows-support.md. The product is unchanged.

Review

Each stage had a review by a different model, in separate sessions. The author models are claude-fable-5-2 and, for the later work, claude-sonnet-5-5 (the model of the session changed once). The reviewer is claude-fable-5-1. Findings were answered one by one, corrected, and re-reviewed. The first line of each result:

Stage Round Verdict Findings and what happened
Design 1 DESIGN: REJECT 11 findings. One blocker: I had read the CI log of cause B wrongly, so the planned product change would not have removed the event. The change was withdrawn.
Design 2 DESIGN: REJECT The 11 closed. 7 new, among them a fix for cause B on the library development branch that I had missed.
Design 3 DESIGN: ACCEPT The 7 closed. 3 minor wording findings.
Design 4 to 7 DESIGN: ACCEPT each Deltas after later findings (a test that skipped, a runner whose counters began late, wording). 4, 1, 1 and 0 minor findings, all corrected.
Implementation 1 IMPLEMENTATION: ACCEPT One major condition (no runner check of the final step yet), 5 minor.
Implementation 2 IMPLEMENTATION: ACCEPT One major: the move made TestExactNameRefusesARealShortAlias skip on every runner. Corrected by running internal/site first. 1 minor.
Implementation 3 to 5 IMPLEMENTATION: ACCEPT each 2 minor, 2 minor, then no finding.
Testing 1 TESTING: ACCEPT 7 minor findings, among them a claim about the oracle of TestSpareAdoption that the test does not support. The record was corrected.
Testing 2 TESTING: ACCEPT No finding.

The review packages and receipts are on the author's machine, so they are not a source for a remote reader. This description is the accessible copy of the task report.

Release

This change holds no product code. A release from it would be the program of v0.17.0 with a new version stamp. Whether to release is the maintainer's decision, and no tag is pushed here.

About 11 minutes after a hosted Windows runner boots, its system drive is
read for about 45 s, and a flush there takes seconds. A document test that
writes then reaches the 10 s limit of one guest call. That failed the
windows job in 4 of its first 32 runs. The Tests step now keeps its data
below RUNNER_TEMP and prints the boot time of the runner.

The stall watch of #232 goes, because the cause is found.

TestSpareAdoption compared two readings of a clock that Windows can give
as equal. It now starts the adoption only when the clock has passed its
reading. The assertion is the same.

The panic of the Tailscale library that failed the job once is recorded in
docs/network-provider-lifecycle.md, with the fix that the library's
development branch has and that no release holds yet. No code changes for it.
The current-state item said that the system drive of the runner was the
cause of the four CI events. The boot times of those four runners are not
recorded, so the item now says that the events fit the clock of the reads.
It gives the count of failed tests in a run, and twelve of twelve measured
runners.

The place of the log truncation is now listed under "Open questions".

The lifecycle text dates its statement on the newest release of the
library, and the playbook names the log line with the boot time.
Fifteen of fifteen measured runners showed the reads of the system drive,
and one of them started its job about 5 min after its boot.
The temporary drive of a hosted runner keeps no short (8.3) names, so
TestExactNameRefusesARealShortAlias skipped there. The package now runs
first, with the default temp directory and with -count=1, and that test
runs on the system drive again.

The open question of the spec now says "flushes again", which is exact.
The third runner check had one runner whose job started 11 min 1 s after
its boot. The reads of the system drive came in its setup steps, and its
counters began later. The spec item now counts seventeen runners with the
reads and one without, and gives the range of the job start for both.

The spec item also names the one package that runs with the default temp
directory, for the test that needs short names.
The spec item said that the reads came on the runner whose job started
11 min 1 s after its boot. Nothing recorded them there: its counters
began later. The item now says that its counters began after the time at
which the reads come on the other runners, and recorded none.
@rabbitson87
rabbitson87 merged commit 69286e3 into main Nov 13, 2026
26 checks passed
@rabbitson87
rabbitson87 deleted the fix/windows-job-drive-stall branch November 13, 2026 06:22
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.

1 participant