fix: wait for the stopped state in mode:stop before returning#69
Merged
Conversation
StopInstances only moves the instance to 'stopping'; mode:stop with reuse:stop returned immediately, so a workflow run queued right behind the stopping one saw an empty warm pool (findStoppedPoolInstance and consumers filter on instance-state-name=stopped) and cold-launched a duplicate instance that the reaper does not drain until its stopped-max-age passes. Observed in terraform-provider-namecheap CI on 2026-07-10: run A's stop-runner exited at 08:45:51 (0.2ms after StopInstances), run B's queue released at 08:45:55, and at 08:46:04 run B logged warm_start outcome:pool_empty and launched a second instance; every subsequent run that day warned 'Found 2 stopped warm-pool instances'. Block in stopInstanceById on waitUntilInstanceStopped (maxWaitTime 300s, mirroring waitForInstanceRunning) so 'mode:stop finished' implies 'instance is reusable'. A stop that cannot complete within the window now fails the step loudly instead of leaving a silent stopping-state race. Signed-off-by: yuriyryabikov <22548029+kurok@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
mode: stopwithreuse: stopreturns as soon asStopInstancesis accepted — the instance is still in thestoppingstate for the next ~30–60s. Every warm-pool lookup (findStoppedPoolInstanceand workflow-side pre-attach filters) matchesinstance-state-name=stoppedonly, so a workflow run queued tightly behind the stopping one sees an empty pool and cold-launches a duplicate instance. The duplicate then sits stopped all day (the leak reaper's defaultreaper-stopped-max-agecorrectly skips same-day instances) and every subsequent run warnsFound 2 stopped warm-pool instances (…); the reaper is not draining them.Evidence (terraform-provider-namecheap CI, 2026-07-10, UTC)
{"step":"stop_instance","instance_id":"i-060edd…"}—is stopped (warm pool)printed 0.2 ms laterstopping){"step":"warm_start","outcome":"pool_empty"}→ cold-launchesi-087854…Found 2 stopped warm-pool instancesSame two-instance pattern the previous day (
i-049890…/i-03e692…).Fix
Block
stopInstanceByIdonwaitUntilInstanceStopped(maxWaitTime: 300, mirroringwaitForInstanceRunning's conventions) so "mode: stopfinished" implies "instance is reusable" — that is the serialization boundary queued follow-up runs rely on. A stop that can't complete within the window now fails the step loudly (previously a silent race).Consumer-side counterpart (widening the pre-attach filter to
stopped,stopping+aws ec2 wait instance-stopped) ships in the terraform-provider-namecheap workflow; either fix alone closes the race, together they're belt-and-braces.Testing
stopInstanceByIdtest asserts the waiter is invoked afterStopInstanceswith the right instance id; new negative test asserts a waiter timeout propagates.npm run lintclean;dist/rebuilt vianpm ci && npm run package.Cost note
stop-runner jobs now run ~30–60s longer (hosted-runner minutes) — that's the price of the boundary being truthful; the duplicate t3.medium + EBS parked daily cost more.