Serialize same-environment Azure Container Apps deployment#18699
Serialize same-environment Azure Container Apps deployment#18699mitchdenny wants to merge 7 commits into
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18699Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18699" |
github-actions
Bot
added
the
area-integrations
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR makes Azure Container Apps' per-environment write-lock constraint explicit in the application model. Azure serializes create/update operations within a single managed environment, so parallel model-graph deployers (e.g. the internal EV2 pipeline) currently hit ContainerAppOperationInProgress. During publish only, AzureContainerAppEnvironmentResource.PrepareDeploymentTargetsAsync now chains all compute resources targeted to an environment into a single serial DependsOn order, so any deployer honoring the resource dependency graph deploys at most one app at a time per environment.
The approach is sound: it collects same-environment targets in model order, derives the existing dependency map from Reference/WaitFor relationships, performs a topological sort (model order as a stable tie-break, with a cycle-safe fallback), and adds a synthetic DependsOn edge between consecutive pairs only when the existing graph doesn't already order them — avoiding redundant edges and cycles. Cross-environment edges are never added, and run mode is unaffected (the method returns early when not publishing).
I traced the algorithm through all six test scenarios and confirmed the edges produced match the assertions; the transitive-redundancy check correctly ignores in-progress synthetic edges because each node's only synthetic out-edge is the one being added. I also confirmed ResourceNameComparer.cs is compiled into this assembly, that Reference/WaitFor relationship directions match GetExistingDependencies, and that relationship annotations are not serialized into the bicep/manifest output (so no snapshot tests are impacted).
Changes:
- Added
AddSerialDeploymentOrderingplusGetExistingDependencies,TopologicalSort, andDependsOnTransitivelyhelpers to serialize same-environment container app deployment via syntheticDependsOnrelationships (publish-only). - Collected the environment's deployment targets during
PrepareDeploymentTargetsAsyncand invoked the new ordering pass. - Added 6 tests covering serial chaining, redundant-edge avoidance, shared-dependency serialization, per-environment independence, single-app no-op, and run-mode no-op.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppEnvironmentResource.cs | Adds the serial-deployment-ordering logic (topological sort + transitive redundancy check) that injects DependsOn edges among same-environment compute targets during publish. |
| tests/Aspire.Hosting.Azure.Tests/AzureContainerAppsTests.cs | Adds six tests validating the chaining behavior across floating apps, existing dependencies, shared dependencies, multiple environments, single-app, and run mode. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Medium
This comment has been minimized.
This comment has been minimized.
PR Testing ReportPR Information
Artifact Version Verification
Changes AnalyzedFiles Changed
Change Categories
Testability noteThe change adds Test Scenarios ExecutedScenario 1: Publish smoke — ACA env with shared dependency + floating appsObjective: Exercise the changed publish path ( AppHost: builder.AddAzureContainerAppEnvironment("env");
var shared = builder.AddContainer("shared", "myimage");
builder.AddContainer("api", "myimage").WaitFor(shared);
builder.AddContainer("worker", "myimage").WaitFor(shared);
builder.AddContainer("cache", "myimage");
builder.AddContainer("gateway", "myimage");Steps:
Observations:
Summary
Overall Result✅ PR VERIFIED (for the CLI-observable publish path) Notes
|
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
/deployment-test |
|
🚀 Deployment tests starting on PR #18699... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
Include pre-existing DependsOn relationships when building the Azure Container Apps serial deployment graph so the generated chain stays cycle-free and idempotent. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62b6966d-e65e-41be-939d-1ee0c9ebbf30
This comment has been minimized.
This comment has been minimized.
Handle mutual-reference cycles and cross-environment transitive ordering when serializing same-environment container apps. Reachability is computed over the whole model so an order routed through another environment is honored, and mutually dependent apps are grouped into a strongly connected component that is serialized as a unit rather than internally, so no DependsOn cycle is ever introduced. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62b6966d-e65e-41be-939d-1ee0c9ebbf30
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62b6966d-e65e-41be-939d-1ee0c9ebbf30
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62b6966d-e65e-41be-939d-1ee0c9ebbf30
This comment has been minimized.
This comment has been minimized.
|
/deployment-test |
|
🚀 Deployment tests starting on PR #18699... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
Container App Jobs are compute resources that write to the same managed environment as apps, so they participate in the serial deployment chain. Add a test that makes a chain member a job to lock in that behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62b6966d-e65e-41be-939d-1ee0c9ebbf30
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 8 / 100 test projects · 3 jobs, from 2 changed files. Selected test projects (8 / 100)
Selected jobs (3)
How these were chosen — grouped by what changed🔧 🧪 Job reasons
Selection computed for commit |
Description
Azure Container Apps serializes write operations per managed environment. Concurrent create/update of apps sharing the same environment fails with
ContainerAppOperationInProgress. Theaspire deploypipeline avoids this by deploying resources sequentially, but the application model itself carried no ordering edge — so model-graph-driven deployers (e.g. the internal EV2 pipeline) deploy apps in parallel and hit the error.This PR expresses that ordering in the model so any deployer that respects the resource dependency graph serializes container app deployments per environment.
Approach
During publish only,
AzureContainerAppEnvironmentResource.PrepareDeploymentTargetsAsyncchains all compute resources targeted to a managed environment into a single total serial order usingDependsOnResourceRelationshipAnnotationedges:Reference/WaitForrelationships (same-environment targets only).DependsOnedge unless the existing graph already orders them transitively — honoring user-declaredWithReference/WaitFordependencies and avoiding redundant edges or cycles.Result: a single serial chain per environment ⇒ at most one container app deploys at a time per managed environment. No cross-environment edges are added, and run mode is unaffected (the dashboard never sees these edges).
Notes
DependsOnis defined as a localinternal constbecauseKnownRelationshipTypesis internal toAspire.Hosting/Aspire.Hosting.Azureand not visible toAspire.Hosting.Azure.AppContainers.Testing
Added 6 tests in
AzureContainerAppsTests, includingContainerAppsSharingADependencyAreStillSerialized(verifies the full-serialization guarantee), redundant-edge avoidance, per-environment independence, single-app no-op, and run-mode no-op.Fixes #18682