[PR #355] [MERGED] fix: stabilize recurring CI flakes #374

New issue

Closed

opened 2026-10-29 14:22:45 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:22:45 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/355
Author: @yingdi-shan
Created: 10/10/2026
Status: ✅ Merged
Merged: 10/12/2026
Merged by: @yingdi-shan

Base: main ← Head: fix/ci-flakes

📝 Commits (6)

d4432fe fix(firecracker): wait for the API socket to accept connections
2d22796 test(network): retry expected-success connectivity probes
efca2b1 ci: authenticate BuildKit release lookup
3221303 test(buildkit): do not require builders on different nodes
6c01d56 fix(build): treat a reaped BuildKit daemon as stopped
b673585 fix(envd): lock /etc/passwd when adding a Dockerfile USER account

📊 Changes

8 files changed (+276 additions, -43 deletions)

View changed files

📝 .github/workflows/integration-tests.yml (+5 -3)
📝 scripts/buildkit/install-buildctl.sh (+5 -1)
📝 scripts/buildkit/test.py (+4 -2)
📝 src/api/impls/image_build/worker.rs (+4 -2)
➕ src/sandbox/envd/add_passwd_entry.sh (+38 -0)
📝 src/sandbox/envd/user.rs (+139 -4)
📝 src/sandbox/firecracker/instance.rs (+31 -9)
📝 tests/integration/fc.rs (+50 -22)

📄 Description

What

Fix six recurring CI flakes found by triaging the failed runs since mid-September. Each fix is a separate commit:

fix(firecracker): wait for the Firecracker API socket to accept connections, not just exist.
test(network): retry expected-success connectivity probes in the network integration tests.
ci: authenticate the BuildKit release lookup in install-buildctl.sh.
test(buildkit): stop requiring concurrent builders to land on different nodes.
fix(build): treat a reaped BuildKit daemon as stopped, so the shared build cache is published.
fix(envd): lock /etc/passwd when adding the generated account for a numeric Dockerfile USER.

Why

These failures recur across unrelated PRs and on main:

Flake	Seen	Root cause
`process::numeric_image_users_preserve_identity_after_start_and_resume` (`invalid default user: 'aenv-12345-23456'`)	5× (most frequent recently)	A real product race. The test image's s6 `init-adduser` runs `usermod`/`groupmod` at boot, which rewrite `/etc/passwd` via temp file + rename. AgentENV starts envd before exec'ing the image init and appended the generated account with an unlocked `>>`, so the line was intermittently lost.
`Failed to set machine configuration` / `Connection refused (os error 111)` (e.g. `fc::oci_tools_share_a_readonly_device_across_concurrent_launches`, `snapshot_attached_drive::frozen_volumes_survive_deletion_without_vm_snapshot`)	2×	`wait_for_ready` polled `socket_path.exists()`. The path appears at `bind()`, before Firecracker calls `listen()`, so the first API request could get `ECONNREFUSED`.
`fc::microvm_can_access_internet`, `fc::microvm_network_policy_controls_egress` (`Could not resolve host: www.baidu.com`, `SSL connection timeout` for `www.qq.com`, exit 124)	6×	Single-shot probes to public DNS and sites from GitHub-hosted runners.
`Prepare BuildKit test client` exits 22 (`curl: (22) ... 403`)	2×	Unauthenticated `api.github.com` calls share the per-IP rate limit on hosted runners, and `curl --retry` does not retry 403.
BuildKit `test_concurrent_builds` seed mismatch (`AssertionError: '<uuid>' != '<uuid>'`, single-node and compose)	2× (incl. this PR's first run)	`STOP_BUILDKIT` ran `kill -0` and then grepped `/proc/$pid/status`. If the buildkitd zombie was reaped in between, `grep` failed on the missing file and the script exited 1 (`grep: /proc/376/status: No such file or directory`). The worker then logged "BuildKit shutdown failed; keeping the previous cache seed", so the next builds started from an empty cache.
`Timed out waiting for concurrent builders on different nodes` (Docker Compose E2E)	4×	The scheduler's round-robin counter is advanced by every unbound gateway request, including `aenv build` session creation and status polls that 404 before the builder binds. Two concurrent builders can legitimately share a node.

N/A: found by triaging recent CI failures.

Scope and non-goals

Included: the four fixes above.
Already fixed on main and not touched here: the MinIO quay.io pull failures (now pgsty/silo) and the clippy 1.99 double_must_use failures.
Failures caused by PR-specific bugs (compile errors, Go syntax errors, unpublished agentenv-tools:0.1.2) were excluded.

Design and behavior changes

FirecrackerInstance::wait_for_ready now loops on tokio::net::UnixStream::connect until it succeeds, keeping the existing early exit if Firecracker dies and the timeout. Error messages now say "did not accept connections". The pool (pool.rs) and sandbox start paths both use this function.
assert_tcp_connect/assert_curl retry up to 3 times, 2 s apart, only when success is expected. Expected-blocked probes still run once, so a retry cannot hide a policy that fails to block.
The generated passwd account is appended by src/sandbox/envd/add_passwd_entry.sh, embedded with include_str! and run with the guest's BusyBox. It takes shadow-utils' /etc/passwd.lock link lock (the same protocol usermod/useradd use), reclaims a lock whose owner PID has exited, waits up to 5 s for a live owner, skips the append if the entry is already present, and verifies it afterwards. The script ships with the server rather than the tools drive, so it also covers sandboxes on older, immutable tools releases. Inputs are passed as positional arguments, so there is no Rust-side shell quoting.
STOP_BUILDKIT decides liveness from /proc/<pid>/status alone: the process is running only if the file exists and its state is not Z. A missing file means stopped. A process that ignores SIGTERM still makes the stop fail after the existing 30 s wait.
install-buildctl.sh sends Authorization: Bearer $GITHUB_TOKEN when the variable is set, and stays unauthenticated otherwise. curl reads the header from stdin (-H @-), so the token never appears in the process list. The integration workflow gives github.token only to a separate install step, not to make build-aenv.
BuildKit test_concurrent_builds (compose mode) now waits for the summed builder increase across nodes to reach 2 instead of requiring 2 distinct nodes. It still asserts that both workers overlap.

Compatibility and operations

Public API or generated protocol: N/A
Configuration or defaults: N/A
Snapshot manifest, artifact layout, or storage format: N/A
Upgrade and rollback: N/A, no persisted state.
Host requirements, permissions, ports, or dependencies: the integration workflow's new Install BuildKit test client step receives the job's read-only github.token.

Validation

make fmt
make clippy
make test-unit
Relevant Rust integration tests
make -C services test (required when services/ changes)
Generated clients/server regenerated with the documented make target
Documentation updated
Benchmarks or performance comparison completed

Commands and results:

make fmt                                               # pass
make clippy                                            # pass
cargo test -p agentenv --lib firecracker::instance     # 9 passed (2 new/updated readiness tests)
bash scripts/tests/verify-install-buildctl.sh          # pass, with and without GITHUB_TOKEN
cargo test -p agentenv --lib image_build               # 18 passed
cargo test -p agentenv --lib sandbox::envd::user       # 13 passed (4 new script tests; the concurrent-
  rewriter test fails with the old unlocked append: "lost aenv-4-0...")
shellcheck -s sh src/sandbox/envd/add_passwd_entry.sh  # pass
Racing real noble `usermod` in a container: old append lost 19/60 entries, locked append lost 0
STOP_BUILDKIT in alpine:3.22: clean stop, lingering zombie, already-gone PID -> exit 0;
  process ignoring SIGTERM -> exit 1
shellcheck scripts/buildkit/install-buildctl.sh        # pass
python3 -m py_compile scripts/buildkit/test.py         # pass
agentenv integration test binary (full suite, 21 tests), pinned to 4 CPUs
  with taskset to approximate hosted runners           # before: 2/12 iterations failed (numeric-user test)
                                                        # with the passwd fix: 12/12 passed

Skipped checks and reasons:

make test-unit: only the affected unit module was run.
BuildKit E2E (scripts/buildkit/test.py) and the Docker Compose E2E suite were not run locally (STOP_BUILDKIT was tested in a container); the CI E2E jobs exercise them.
make -C services test: services/ is unchanged.

Risks and reviewer notes

wait_for_ready opens one throwaway connection to Firecracker's API socket. Firecracker's API server accepts and drops idle connections, and the client already opens a connection per request.
The network-test retry adds at most ~4 s per expected-success probe, and only when a probe fails.
Holding /etc/passwd.lock for a few milliseconds means a colliding usermod in the image init fails fast with "cannot lock /etc/passwd; try again later", which is shadow's own contract between account tools, instead of AgentENV's entry being silently lost.
The relaxed BuildKit assertion no longer proves cross-node placement, which the scheduler never guaranteed. If spreading builders is a requirement, it needs a scheduler change, such as load-aware placement for builder allocations.

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests, or I explained why testing is impractical.
Logs and examples contain no credentials, tokens, or private registry information.
I did not manually edit generated code without updating its source and regenerating it.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/355 **Author:** [@yingdi-shan](https://github.com/yingdi-shan) **Created:** 10/10/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `fix/ci-flakes` --- ### 📝 Commits (6) - [`d4432fe`](https://github.com/kvcache-ai/AgentENV/commit/d4432fe2f4c86cf9fced5c40e8472922bc103adb) fix(firecracker): wait for the API socket to accept connections - [`2d22796`](https://github.com/kvcache-ai/AgentENV/commit/2d227964c0e28b5362d8ecc480ac564ac9710164) test(network): retry expected-success connectivity probes - [`efca2b1`](https://github.com/kvcache-ai/AgentENV/commit/efca2b10b5f06aef4fbcf1648a3aecc58cf9ae0f) ci: authenticate BuildKit release lookup - [`3221303`](https://github.com/kvcache-ai/AgentENV/commit/32213037240cac517e3ba5fa2ea7ee17f2fc6eb9) test(buildkit): do not require builders on different nodes - [`6c01d56`](https://github.com/kvcache-ai/AgentENV/commit/6c01d56d72a513bf65fb3ef042e01d16ad59bd9c) fix(build): treat a reaped BuildKit daemon as stopped - [`b673585`](https://github.com/kvcache-ai/AgentENV/commit/b673585325d09b923c2bd18e944e40443f1f2eca) fix(envd): lock /etc/passwd when adding a Dockerfile USER account ### 📊 Changes **8 files changed** (+276 additions, -43 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/integration-tests.yml` (+5 -3) 📝 `scripts/buildkit/install-buildctl.sh` (+5 -1) 📝 `scripts/buildkit/test.py` (+4 -2) 📝 `src/api/impls/image_build/worker.rs` (+4 -2) ➕ `src/sandbox/envd/add_passwd_entry.sh` (+38 -0) 📝 `src/sandbox/envd/user.rs` (+139 -4) 📝 `src/sandbox/firecracker/instance.rs` (+31 -9) 📝 `tests/integration/fc.rs` (+50 -22) </details> ### 📄 Description ## What Fix six recurring CI flakes found by triaging the failed runs since mid-September. Each fix is a separate commit: 1. `fix(firecracker)`: wait for the Firecracker API socket to **accept connections**, not just exist. 2. `test(network)`: retry expected-success connectivity probes in the network integration tests. 3. `ci`: authenticate the BuildKit release lookup in `install-buildctl.sh`. 4. `test(buildkit)`: stop requiring concurrent builders to land on different nodes. 5. `fix(build)`: treat a reaped BuildKit daemon as stopped, so the shared build cache is published. 6. `fix(envd)`: lock `/etc/passwd` when adding the generated account for a numeric Dockerfile `USER`. ## Why These failures recur across unrelated PRs and on `main`: | Flake | Seen | Root cause | | --- | --- | --- | | `process::numeric_image_users_preserve_identity_after_start_and_resume` (`invalid default user: 'aenv-12345-23456'`) | 5× (most frequent recently) | A real product race. The test image's s6 `init-adduser` runs `usermod`/`groupmod` at boot, which rewrite `/etc/passwd` via temp file + rename. AgentENV starts envd before exec'ing the image init and appended the generated account with an unlocked `>>`, so the line was intermittently lost. | | `Failed to set machine configuration` / `Connection refused (os error 111)` (e.g. `fc::oci_tools_share_a_readonly_device_across_concurrent_launches`, `snapshot_attached_drive::frozen_volumes_survive_deletion_without_vm_snapshot`) | 2× | `wait_for_ready` polled `socket_path.exists()`. The path appears at `bind()`, before Firecracker calls `listen()`, so the first API request could get `ECONNREFUSED`. | | `fc::microvm_can_access_internet`, `fc::microvm_network_policy_controls_egress` (`Could not resolve host: www.baidu.com`, `SSL connection timeout` for `www.qq.com`, exit 124) | 6× | Single-shot probes to public DNS and sites from GitHub-hosted runners. | | `Prepare BuildKit test client` exits 22 (`curl: (22) ... 403`) | 2× | Unauthenticated `api.github.com` calls share the per-IP rate limit on hosted runners, and `curl --retry` does not retry 403. | | BuildKit `test_concurrent_builds` seed mismatch (`AssertionError: '<uuid>' != '<uuid>'`, single-node and compose) | 2× (incl. this PR's first run) | `STOP_BUILDKIT` ran `kill -0` and then grepped `/proc/$pid/status`. If the buildkitd zombie was reaped in between, `grep` failed on the missing file and the script exited 1 (`grep: /proc/376/status: No such file or directory`). The worker then logged "BuildKit shutdown failed; keeping the previous cache seed", so the next builds started from an empty cache. | | `Timed out waiting for concurrent builders on different nodes` (Docker Compose E2E) | 4× | The scheduler's round-robin counter is advanced by *every* unbound gateway request, including `aenv build` session creation and status polls that 404 before the builder binds. Two concurrent builders can legitimately share a node. | ## Related issue N/A: found by triaging recent CI failures. ## Scope and non-goals - Included: the four fixes above. - Already fixed on `main` and not touched here: the MinIO `quay.io` pull failures (now `pgsty/silo`) and the clippy 1.99 `double_must_use` failures. - Failures caused by PR-specific bugs (compile errors, Go syntax errors, unpublished `agentenv-tools:0.1.2`) were excluded. ## Design and behavior changes - `FirecrackerInstance::wait_for_ready` now loops on `tokio::net::UnixStream::connect` until it succeeds, keeping the existing early exit if Firecracker dies and the timeout. Error messages now say "did not accept connections". The pool (`pool.rs`) and sandbox start paths both use this function. - `assert_tcp_connect`/`assert_curl` retry up to 3 times, 2 s apart, **only when success is expected**. Expected-blocked probes still run once, so a retry cannot hide a policy that fails to block. - The generated passwd account is appended by `src/sandbox/envd/add_passwd_entry.sh`, embedded with `include_str!` and run with the guest's BusyBox. It takes shadow-utils' `/etc/passwd.lock` link lock (the same protocol `usermod`/`useradd` use), reclaims a lock whose owner PID has exited, waits up to 5 s for a live owner, skips the append if the entry is already present, and verifies it afterwards. The script ships with the server rather than the tools drive, so it also covers sandboxes on older, immutable tools releases. Inputs are passed as positional arguments, so there is no Rust-side shell quoting. - `STOP_BUILDKIT` decides liveness from `/proc/<pid>/status` alone: the process is running only if the file exists and its state is not `Z`. A missing file means stopped. A process that ignores SIGTERM still makes the stop fail after the existing 30 s wait. - `install-buildctl.sh` sends `Authorization: Bearer $GITHUB_TOKEN` when the variable is set, and stays unauthenticated otherwise. curl reads the header from stdin (`-H @-`), so the token never appears in the process list. The integration workflow gives `github.token` only to a separate install step, not to `make build-aenv`. - BuildKit `test_concurrent_builds` (compose mode) now waits for the summed builder increase across nodes to reach 2 instead of requiring 2 distinct nodes. It still asserts that both workers overlap. ## Compatibility and operations - Public API or generated protocol: N/A - Configuration or defaults: N/A - Snapshot manifest, artifact layout, or storage format: N/A - Upgrade and rollback: N/A, no persisted state. - Host requirements, permissions, ports, or dependencies: the integration workflow's new `Install BuildKit test client` step receives the job's read-only `github.token`. ## Validation - [x] `make fmt` - [x] `make clippy` - [ ] `make test-unit` - [x] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) - [ ] Generated clients/server regenerated with the documented `make` target - [ ] Documentation updated - [ ] Benchmarks or performance comparison completed Commands and results: ```text make fmt # pass make clippy # pass cargo test -p agentenv --lib firecracker::instance # 9 passed (2 new/updated readiness tests) bash scripts/tests/verify-install-buildctl.sh # pass, with and without GITHUB_TOKEN cargo test -p agentenv --lib image_build # 18 passed cargo test -p agentenv --lib sandbox::envd::user # 13 passed (4 new script tests; the concurrent- rewriter test fails with the old unlocked append: "lost aenv-4-0...") shellcheck -s sh src/sandbox/envd/add_passwd_entry.sh # pass Racing real noble `usermod` in a container: old append lost 19/60 entries, locked append lost 0 STOP_BUILDKIT in alpine:3.22: clean stop, lingering zombie, already-gone PID -> exit 0; process ignoring SIGTERM -> exit 1 shellcheck scripts/buildkit/install-buildctl.sh # pass python3 -m py_compile scripts/buildkit/test.py # pass agentenv integration test binary (full suite, 21 tests), pinned to 4 CPUs with taskset to approximate hosted runners # before: 2/12 iterations failed (numeric-user test) # with the passwd fix: 12/12 passed ``` Skipped checks and reasons: - `make test-unit`: only the affected unit module was run. - BuildKit E2E (`scripts/buildkit/test.py`) and the Docker Compose E2E suite were not run locally (`STOP_BUILDKIT` was tested in a container); the CI E2E jobs exercise them. - `make -C services test`: `services/` is unchanged. ## Risks and reviewer notes - `wait_for_ready` opens one throwaway connection to Firecracker's API socket. Firecracker's API server accepts and drops idle connections, and the client already opens a connection per request. - The network-test retry adds at most ~4 s per expected-success probe, and only when a probe fails. - Holding `/etc/passwd.lock` for a few milliseconds means a colliding `usermod` in the image init fails fast with "cannot lock /etc/passwd; try again later", which is shadow's own contract between account tools, instead of AgentENV's entry being silently lost. - The relaxed BuildKit assertion no longer proves cross-node placement, which the scheduler never guaranteed. If spreading builders is a requirement, it needs a scheduler change, such as load-aware placement for builder allocations. ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 14:22:45 +00:00