Skip to content

fix(reset): don't hold the reset response on an in-flight EC2 boot - #2677

Merged
vieiralucas merged 2 commits into
mainfrom
fix-container-cli-blocking-workers
Oct 4, 2026
Merged

vieiralucas merged 2 commits into
mainfrom
fix-container-cli-blocking-workers

Conversation

@vieiralucas

@vieiralucas vieiralucas commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

Summary

python-test intermittently fails on main (and deterministically on #2666): the client.reset() right after test_ec2_get_instances gets no response within the SDK's 5s timeout.

The cause is not tokio workers blocked by sync calls. The reset handler awaits its container teardown, and EC2's remove_instances took each instance's lifecycle lock first. The RunInstances boot task holds that lock for the whole run_instance, which includes pulling the instance image (amazonlinux:2023 by default, seconds on a cold runner). So the reset response waited for the image pull.

  • Ec2Runtime::remove_instances now tries the lifecycle lock. When an instance's lifecycle task is in flight, its teardown (container and data volume) runs on a task of its own once the lock frees, and the reset returns at once.
    • Instance ids are unique per instance, so a deferred teardown can never touch a newer instance.
    • The in-flight boot also reaps itself when it finds its row gone.
  • Audit for blocking container-CLI calls on async workers:
    • EC2, RDS, ElastiCache, ECS, Lambda, CodeBuild, MSK and MQ runtime operations already use tokio::process. The sync detection (detect_container_cli, HostNetworking::detect) runs only in startup constructors. Reset's stop_all paths are async. CodeBuild's sync rm -f is a panic-path Drop guard by design.
    • The one sync process call inside an async fn was ECR's trivy --version probe in scan_layers. It now runs via spawn_blocking.

Non-code surfaces: none affected. This is internal reset behavior; docs, SDKs and AGENTS.md are unchanged.

Test plan

  • New unit test ec2::runtime::reset_tests::reset_does_not_wait_for_an_in_flight_boot: with the lifecycle lock held (a boot pulling its image), remove_instances returns within 2s, and once the lock is released the deferred teardown removes the container (driven by a fake container CLI). It fails on main.
  • New E2E ec2_instance_runtime::reset_right_after_run_instances_is_prompt: reset within 5s right after RunInstances. It is Docker-backed and runs in CI only, because Docker hangs on the local machine; it compiles locally.
  • cargo test -p fakecloud-ec2 -p fakecloud-ecr --lib passes. Workspace clippy (--all-targets -D warnings) and fmt are clean.

Summary by cubic

Fixes the reset endpoint so a reset right after RunInstances no longer waits on the instance boot, which previously held the response while pulling the image and could cause client timeouts. An EC2 instance with an in-flight lifecycle task is now torn down asynchronously once the task releases its lock; because instance ids are unique per instance, the deferred teardown can't affect a newer instance. Also moves ECR's blocking trivy --version probe into spawn_blocking to keep it off the async request handlers.

  • Adds unit and Docker-backed E2E tests; the E2E asserts the booting instance's container is both created and then removed by the deferred teardown.

Written for commit 0739bfc. Summary will update on new commits.

Review in cubic

A reset tore EC2 instances down under their lifecycle lock, which a boot
holds while it pulls the instance image, so a reset right after RunInstances
waited for the pull (seconds on a cold CI runner) and clients timed out.
An instance whose lifecycle task is in flight is now torn down on a task of
its own once that task lets go; the reset returns at once. Also keep the
blocking trivy probe off the async workers.
@vieiralucas
vieiralucas merged commit 2dd9866 into main Oct 4, 2026
159 checks passed
@vieiralucas
vieiralucas deleted the fix-container-cli-blocking-workers branch October 4, 2026 10:47
vieiralucas added a commit that referenced this pull request Oct 4, 2026
After rebasing onto the deferred reset teardown (#2677): the reset test now
also checks the IMDS sidecar is removed, the duplicate reset e2e is dropped
(ec2_instance_runtime covers it), and fake CLI scripts wait out ETXTBSY
before use so parallel tests forking can't fail their first exec.
vieiralucas added a commit that referenced this pull request Oct 4, 2026
After rebasing onto the deferred reset teardown (#2677): the reset test now
also checks the IMDS sidecar is removed, the duplicate reset e2e is dropped
(ec2_instance_runtime covers it), and fake CLI scripts wait out ETXTBSY
before use so parallel tests forking can't fail their first exec.
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