fix: pkg/snipe reports the actual landed AZ, not the requested placement - #123
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
launchAcrossPlacements returned p.AZ (the REQUESTED placement's AZ from the loop variable) instead of result.AvailabilityZone (the ACTUAL AZ RunInstances placed the instance in, per spawnaws.LaunchResult). The two coincide when a Placement pins a specific AZ, which is why this only surfaced when Target.Placements is empty and EC2 chooses the AZ itself — the scenario Target.Placements's own doc explicitly supports. Verified via spawn/pkg/aws/client.go's newLaunchResult: LaunchResult. AvailabilityZone is read directly from the RunInstances response's Placement field, so it is genuinely the landed AZ, not an echo of the request. Added a regression test (TestSnipe_ReportsActualLandedAZWhenEC2Chooses) that fails without the fix and passes with it; updated two existing fakes whose LaunchResult didn't populate AvailabilityZone (harmless before, since the placement AZ was returned instead, but now load-bearing). Fixes #114
scttfrdmn
force-pushed
the
fix/114-snipe-actual-az
branch
from
August 19, 2026 02:29
5965ce7 to
5a5ad02
Compare
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.
Summary
pkg/snipe'sResult.AvailabilityZonenow reports the AZ the instance actually landed in (spawnaws.LaunchResult.AvailabilityZone, read from the realRunInstancesresponse), instead of the requested placement's AZ from the retry loop's variable.launchAcrossPlacements(pkg/snipe/snipe.go) returnedp.AZon success — the loop variable holding the REQUESTED placement — rather thanresult.AvailabilityZone. These coincide whenever aPlacementpins a specific AZ (the request forces the match), which is exactly why every existing test passed: they all pin aPlacement. The bug only surfaces whenTarget.Placementsis empty and EC2 chooses the AZ itself — a scenarioTarget.Placements's own doc comment explicitly describes as supported ("Empty = a single AZ-unpinned attempt per round (EC2 chooses the AZ and its default subnet)").RunInstancesresponse mapping:spawnaws.newLaunchResult(spawn'spkg/aws/client.go) setsLaunchResult.AvailabilityZonedirectly frominstance.Placement.AvailabilityZonein the API response — so it is genuinely the landed AZ, not an echo of the launch config. No live AWS call was needed to confirm this; the mapping is unambiguous in spawn's source.Closes #114.
Changes
pkg/snipe/snipe.go:launchAcrossPlacementsreturnsresult.AvailabilityZoneinstead ofp.AZ.pkg/snipe/snipe_internal_test.go:TestSnipe_ReportsActualLandedAZWhenEC2Chooses— the regression guard, with noPlacementsset and a fakeprovidethat returns a landed AZ the request never named.TestSnipe_AcquiresAfterCapacityRetries,TestSnipe_SubnetPerPlacement) to populateLaunchResult.AvailabilityZone, mirroring a real RunInstances response — previously harmless since the placement AZ was returned regardless, now load-bearing.CHANGELOG.md:[Unreleased]entry underFixed.Note
The same bug pattern exists in the legacy
pkg/watcher.Spawner.launchAcrossAZs(pkg/watcher/spawner.go:241, returns the requested AZarather thanresult.AvailabilityZone). Left untouched here — issue #114 scopes topkg/snipe, and per the[0.52.0]CHANGELOG entrywatcher.Snipe/Spawner.Snipehave no current callers in this repo (kept as-is rather than rewired). Filing this note here for visibility; happy to open a follow-up issue if wanted.Verification
git stashthe source-only change) and passes with it restored.go build ./...,go vet ./...,go test ./...clean from repo root.go build ./...,go vet ./...,go test ./...clean fromlambda/capacity-poller(untouched, confirmed still green).golangci-lint run ./pkg/snipe/...— 0 issues.Test plan