fix(e2e): replicate gallery images to a fixed region set so concurrent writers converge - #9228
Open
Ganeshkumar Ashokavardhanan (ganeshkumarashok) wants to merge 3 commits into
Open
Conversation
…ions E2E was reporting "Replication took: 12m" via ensureReplication then immediately failing on VMSS create with `404 GalleryImageNotFound` for ~10 more minutes. Root cause: replicatedToCurrentRegion only checks PublishingProfile.TargetRegions (intent), and the parent LRO returns Succeeded before per-region replicas are actually serving traffic. The vmss.go retry loop (10 x 5s) was far too short to absorb the gap and surfaced the infra failure as `failed to create VMSS after 10 retries`, indistinguishable from a real test failure. Fix: - ensureReplication now polls Get with Expand=ReplicationStatus until RegionalReplicationStatus.State == Completed (20 min timeout, 15s interval). Fails fast on terminal Failed. - CreateVMSSWithRetry no longer burns fixed retries on 404 GalleryImageNotFound. It calls the replication poller, which either returns quickly when the region is Completed (fabric-eventual- consistency race -> retry once), blocks until Completed, or fails fast on Failed with the real cause attached. - Add a public WaitForImageVersionReplicatedToRegion entrypoint and unit tests for the region-name normalization in findRegionalReplicationStatus. This should eliminate the GalleryImageNotFound class of false E2E failures across Linux VHD, Windows VHD, GPU, and base AgentBaker E2E pipelines.
…t writers converge publishingProfile.targetRegions is full desired state, not an atomic append. Each E2E writer previously did GET -> append only its own region -> CreateOrUpdate, so writers computed different desired states from possibly stale reads and a concurrent write could silently drop another writer's region. The result was GalleryImageNotFound on VMSS create. PR #8823 addressed this with a sync.Mutex, which only serialises goroutines inside one process and cannot coordinate the separate E2E pipelines that race on the same image version. A lost update is only harmful when writers disagree. Replicating to a fixed set of E2E regions makes every writer - across goroutines, processes and pipelines - submit an identical desired state, so whichever write lands last is still correct and no locking or cross-process coordination is needed. Scenarios now reference the region constants instead of writing literals, so the list is the definition rather than a copy that can drift, and a scenario pinned to an unlisted region fails fast instead of timing out waiting for replication. Replication failures are only fatal when the caller's own region is missing, so contention or a quota on a region a test never uses cannot fail that test. Image versions captured at runtime for a single test stay single-region: they have exactly one writer and are deleted on cleanup. Note this only affects the standalone E2E pipelines that resolve images by tag. The VHD builder pipelines already consume published VHD metadata (#8893) and take a path that never writes to the gallery. Also retains the regional readiness wait from #8374: a version is listed in targetRegions before its regional replica actually serves traffic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 01d66448-4349-475a-975b-5c4ec42f92f3
No Windows scenario pins a location, so every Windows test runs in the default region. Replicating Windows image versions - the largest in the gallery - to the other five E2E regions cost storage for regions no Windows test can reach. Derive the region set from the image instead of using one global list. Convergence is unaffected: what matters is that every writer of a given image version computes the same desired state, not that all images share one set. The same function now backs both replication and scenario validation, so a scenario pinned to a region its image is not replicated to fails fast naming the exact list to add it to, rather than the two definitions drifting apart. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 01d66448-4349-475a-975b-5c4ec42f92f3
Ganeshkumar Ashokavardhanan (ganeshkumarashok)
requested review from
Chou Hu (AbelHu),
Devinwong,
Sri Harsha (SriHarsha001),
Abigail Liang (abigailliang-aks-sig-node),
Abdul Asfari (aboodasfari),
Nishchay (awesomenix),
Ben Brady (benjamin-brady),
Calvin S. (calvin197),
Cameron Meissner (cameronmeissner),
Sylvain Boily (djsly),
fcher,
janenotjung-hue,
Karen Chen (karenychen),
lilypan26,
Mark Ibrahim (mxj220),
Peter Damianov (pdamianov-dev),
Patrick W. Healy (phealy),
r2k1,
Runzhen (runzhen),
Shun Lyu (sinmentis),
sulixu,
Tim Wright (timmy-wright),
Thibault Cohen (titilambert),
Xu Xue (xuexu6666) and
Zachary (zachary-bailey)
as code owners
August 17, 2026 21:13
Copilot started reviewing on behalf of
Ganeshkumar Ashokavardhanan (ganeshkumarashok)
August 17, 2026 21:14
View session
Contributor
Windows Unit Test Results 3 files 12 suites 50s ⏱️ Results for commit e0720e1. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens AgentBaker’s E2E framework against cross-process Azure Compute Gallery replication races by making all writers converge on the same fixed per-OS region set, and by waiting on per-region replication readiness (not just TargetRegions) before treating an image as usable.
Changes:
- Introduces a single source of truth for E2E regions (
e2e/config/regions.go) and updates scenarios to reference region constants. - Updates SIG image replication logic to merge to a fixed desired region set, batch updates, and retry on write conflicts, while carving out non-shared “ephemeral” image versions.
- Adds replication-state polling utilities and unit tests, and extends VMSS-create retry handling for
GalleryImageNotFound.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| e2e/vmss.go | Refines retry classification and adds a GalleryImageNotFound recovery path that consults replication status before retrying. |
| e2e/test_helpers.go | Fails scenarios early if they’re configured to run in regions outside the image’s supported E2E region set; marks runtime-created images as Ephemeral. |
| e2e/scenario_test.go | Replaces hard-coded region literals with shared region constants. |
| e2e/scenario_gpu_managed_experience_test.go | Replaces hard-coded region literals with shared region constants. |
| e2e/config/vhd.go | Adds an Ephemeral flag to the image model to avoid unnecessary fan-out replication for one-off, runtime-created images. |
| e2e/config/regions.go | New: defines the canonical E2E region sets (Linux vs Windows), plus normalization/helpers used by replication + validation. |
| e2e/config/azure.go | Implements convergence-based target-region merging, conflict retries, and per-region replication completion polling; adds public wait helper for VMSS retries. |
| e2e/config/azure_replication_test.go | New: unit tests for region normalization/lookup, merge convergence property, OS-specific region sets, and ephemeral carve-out. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+370
to
+371
| toolkit.Logf(ctx, "Regional replication confirmed Completed; retrying VMSS create immediately") | ||
| continue |
Comment on lines
+363
to
+367
| if isGalleryImageMissing && s.VHD != nil && s.Runtime != nil && s.Runtime.Cluster != nil && s.Runtime.Cluster.Model != nil && s.Runtime.Cluster.Model.Location != nil { | ||
| location := *s.Runtime.Cluster.Model.Location | ||
| toolkit.Logf(ctx, "GalleryImageNotFound on VMSS create in %s; consulting regional replication status before retrying", location) | ||
| if waitErr := config.Azure.WaitForImageVersionReplicatedToRegion(ctx, s.VHD, location); waitErr != nil { | ||
| // Surface the precise infra cause instead of a generic "failed after N retries". |
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
publishingProfile.targetRegionsis full desired state, not an atomic append. Each E2E writer did:Two writers that read before either wrote compute different desired states:
[eastus][eastus, southeastasia][eastus][eastus, uaenorth]Whichever lands last wins outright, and the other region is silently dropped — surfacing later as
GalleryImageNotFoundon VMSS create.vhdbuilder/packer/replicate-captured-sig-image-version.shalready documents the same constraint: "SIG API requires specifying a complete set of replication targets".#8823 wrapped this in a
sync.Mutex. That correctly serialises goroutines within one process, but the writers that actually collide are separate E2E pipelines in separate processes, which a mutex cannot reach.Approach
A lost update is only harmful when writers disagree. So make them agree.
Every writer replicates to the same fixed set of E2E regions, so concurrent writes submit an identical desired state. Whichever lands last is still correct — no mutex, no barrier, no cross-process coordination, no ETag (the gallery image version API exposes none).
Steady state is free: once a version has every region,
missingis empty, no write is issued, and the run is a GET plus the regional readiness wait.What's here
e2e/config/regions.go— the six E2E regions as constants, split intolinuxE2ERegionsandwindowsE2ERegions, behind(*Image).E2ERegions(). That one function backs both the replication set and scenario validation, so the two cannot drift apart. IncludesNormalizeRegion(ARM returns both"West US 2"and"westus2").Image.Ephemeral— image versions captured at runtime for a single test stay single-region. They have exactly one writer and are deleted on cleanup, so there is no lost update to defend against and no reason to fan a throwaway captured OS image out to six regions.targetRegionsbefore its regional replica actually serves traffic, and that gap is the direct cause of the 404s.Scope
This only affects the standalone E2E pipelines that resolve images by tag (
e2e.yaml,e2e-gpu.yaml,e2e-gpu-azurelinux.yaml,e2e-windows.yaml,e2e-tme.yaml,e2e-rcv1p-not-opted-in.yaml).The VHD builder pipelines already set
useVhdMetadataArtifacts: trueand consume published VHD metadata (#8893); on that pathGetVHDResourceIDreturns early and E2E never writes to the gallery at all. That is the strategic fix — this PR is a safety net for the pipelines not yet on it.Related: if the
REPLICATIONSpipeline variable is extended to cover the six E2E regions, images arrive pre-replicated,missingis always empty, and this code never issues a write. Worth doing independently.Trade-off
The region set is scoped per image OS, because convergence only requires that every writer of a given image version agree — not that all images share one list.
{westus3}only. Replicating them to the other five would have been a 6x storage cost for regions no Windows test can reach.So shared Linux image versions are replicated to all six E2E regions rather than on demand. That is the price of writer agreement, and it is close to what those images reach anyway once every pipeline has run against them. Windows images and ephemeral per-test images are unaffected.
Known limitation
Two pipelines running different commits with different region lists can still disagree. That is inherent to a full-state API with no optimistic concurrency, and it self-heals on the next run.
Testing
go build ./...,go vet ./...,go test ./config(incl.-race), and the e2e test binary compiles. New unit tests cover the merge, the convergence property itself (two writers from different snapshots produce the same set), the ephemeral carve-out, region-set membership, and target-region lookup.Recreated from #9067, which was opened from a fork and could never pass
validate-pull-request-source. Same three commits, re-signed and rebased onto currentmain; the stray rootCHANGELOG.md(working notes, not a repo artifact) has been dropped.