fix(e2e): replicate gallery images to a fixed region set so concurrent writers converge - #9067
Open
ganeshkumarashok wants to merge 2 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.
ganeshkumarashok
requested review from
AbelHu,
Devinwong,
SriHarsha001,
awesomenix,
calvin197,
cameronmeissner,
djsly,
karenychen,
lilypan26,
mxj220,
pdamianov-dev,
phealy,
r2k1,
runzhen,
sulixu,
timmy-wright,
titilambert,
xuexu6666 and
zachary-bailey
as code owners
July 28, 2026 22:58
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors AgentBaker’s E2E image-replication flow to avoid per-scenario TargetRegions read/modify/write races by introducing a discovery-only “preflight” pass that computes a per-image region manifest, performs at most one replication update per selected image-version, and persists metadata for the subsequent scenario execution pass to use read-only lookups.
Changes:
- Add a discovery-only E2E preflight coordinator that batches desired/required regions per image and writes VHD metadata for the main run.
- Update Azure SIG replication logic to merge
TargetRegions, retry on concurrent update conflicts, and wait onRegionalReplicationStatus.State == Completedfor required regions. - Improve resilience around
GalleryImageNotFoundby consulting replication state during VMSS creation retries, and stabilize random VHD selection across both passes.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| e2e/vmss.go | Classifies GalleryImageNotFound separately and consults replication state before retrying VMSS creation. |
| e2e/vhd_preflight.go | Adds preflight coordinator to batch desired/required regions per image and emit metadata. |
| e2e/vhd_preflight_test.go | Unit test for region batching/selection behavior and metadata output. |
| e2e/validators.go | Makes Ubuntu 22.04 FIPS detection resilient to VHD pointer copies by comparing distro identity. |
| e2e/test_helpers.go | Splits “scenario selection + normalization” from execution; supports preflight registration-only runs. |
| e2e/scenario_rcv1p_test.go | Skips subscription feature probing in preflight; lazily resolves branch CSE package URL in mutator. |
| e2e/scenario_helpers_test.go | Writes preflight metadata after tests complete when E2E_VHD_PREFLIGHT_OUTPUT is set. |
| e2e/config/vhd.go | Adds preflight entrypoints and metadata lookup semantics; adds VHDResourceID.Version() and random index override. |
| e2e/config/vhd_metadata_test.go | Tests metadata parsing for “missing image” entries and VHDResourceID.Version(). |
| e2e/config/azure.go | Implements batched replication: merge desired regions, conflict retry, and per-region replication-status waiting. |
| e2e/config/azure_replication_test.go | Adds tests for replication-status lookup and target-region merge behavior. |
| .pipelines/scripts/e2e_run.sh | Runs discovery-only preflight pass, persists metadata, and sets a stable random VHD index for both passes. |
| CHANGELOG.md | Documents the high-level behavior shift and rationale. |
Comment on lines
+497
to
+502
| if configuredIndex := os.Getenv("E2E_RANDOM_VHD_INDEX"); configuredIndex != "" { | ||
| index, err := strconv.Atoi(configuredIndex) | ||
| if err == nil && index >= 0 && index < len(vhds) { | ||
| return vhds[index] | ||
| } | ||
| } |
…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 Azure#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 (Azure#8893) and take a path that never writes to the gallery. Also retains the regional readiness wait from Azure#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
ganeshkumarashok
force-pushed
the
ganesh/e2e-batch-gallery-replication
branch
from
July 29, 2026 21:21
53b00c8 to
446cf0d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
e2e/vmss.go:372
- The special GalleryImageNotFound handling is gated on
s.Runtime.Cluster.Model.Location != nil. If that pointer is unexpectedly nil, we fall back to the generic retry loop and lose the intended fast-fail / wait-for-replication behavior. SinceScenario.Locationis already available and is what the run is targeting, use it as a fallback (and only override it when the cluster model location is present).
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".
e2e/config/azure.go:670
- If
ensureTargetRegionsfails, scoping the failure to “did my own region make it?” currently depends on a single follow-up GET (getImageVersion). If that GET fails transiently, we immediately return the original error even if the region update actually succeeded, which can add avoidable flakiness. Consider retrying the GET once (or a couple times) with a short delay before failing.
live, getErr := a.getImageVersion(ctx, image, *version.Name)
if getErr != nil || !hasTargetRegion(live.Properties.PublishingProfile.TargetRegions, location) {
return err
e2e/config/azure.go:746
- The error message in
updateImageVersionsays "create a new images client", but this code is creating a gallery image versions client. Also prefer%wto preserve the wrapped error for callers.
client, err := armcompute.NewGalleryImageVersionsClient(image.Gallery.SubscriptionID, a.Credential, a.ArmOptions)
if err != nil {
return fmt.Errorf("create a new images client: %v", err)
}
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, plusE2EReplicationRegions,IsE2ERegion,NormalizeRegion(ARM returns both"West US 2"and"westus2").e2eRegions" instead of timing out waiting for replication that will never happen.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
Shared image versions get replicated to all six E2E regions rather than on demand. That is the price of writer agreement, and it is what makes the race disappear without locking. Ephemeral per-test images are exempt.
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.