Repository navigation
Allow pipeline steps to depend on deployment-target steps materialized during before-start - #21511
David Fowler (davidfowl) wants to merge 1 commit into
Conversation
Steps that depend on deployment-target provisioning steps (for example
provision-{name}-containerapp) failed with 'depends on unknown step'
because before-start validated the full step graph before the deployment
targets it materializes existed. Before-start now validates references for
the steps it executes; ExecuteAsync still validates the full graph.
Fixes #20301
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 21511Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 21511" |
github-actions
Bot
added
the
needs-area-label
Tests selector52 / 100 PR test projects · 4 PR jobs, from 3 changed files. Selected PR test projects (52 / 100)
Selected PR jobs (4)
How these were chosen — grouped by what changed
🔧 show 44
🧪 📦 affected project 🧪 📦 affected project Job reasons
Selection computed for commit |
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the change allowing references to provisioning steps created during before-start.
No open issues 🎉
What changed in this PR
Fixes #20301 by allowing pipeline steps to reference deployment-target provisioning steps that are created during before-start, while preserving validation of steps needed for startup.
Changes:
- Limits sequential execution’s reference validation to the target step and its transitive dependencies.
- Preserves duplicate-name checks and full-graph validation for normal pipeline execution.
- Adds core regression tests and an Azure Container App Job publishing test.
| File | Description |
|---|---|
tests/Aspire.Hosting.Tests/Pipelines/DistributedApplicationPipelineTests.cs |
Tests deferred step creation and rejection of unknown references within the executed dependency closure. |
tests/Aspire.Hosting.Azure.Tests/AzureContainerAppEnvironmentExtensionsTests.cs |
Tests publishing with a custom step that depends on Container App Job provisioning. |
src/Aspire.Hosting/Pipelines/DistributedApplicationPipeline.cs |
Separates name and reference validation and defers unrelated reference checks during sequential execution. |
Review effort: Balanced
|
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. |
|
CI green on exact head 1d79cbe (338 pass / 14 skipped). Attempt-1 failures were unrelated: Hosting-6 (windows) runner lost communication (infra); Hosting.MongoDB (ubuntu) failed in attempts 1–2 with the pre-existing 🤖 Posted by AspireAgent (AI Agent) |
Description
A pipeline step that depends on a per-resource deployment-target provisioning step (for example
provision-myjob-containerappfromPublishAsAzureContainerAppJob()) fails at startup with:Root cause
ExecuteBeforeStartHooksAsyncruns thebefore-startstep throughExecuteStepSequentiallyAsync, which calledResolveStepsAsyncand therefore validated everyDependsOn/RequiredByreference in the whole pipeline. But per-resource provisioning steps such asprovision-{name}-containerappcome from deployment targets that are only materialized whilebefore-startruns (byprepare-azure-container-apps-{env}). So a user step unrelated tobefore-start(it is required bydeploy) was rejected before the step it references could exist. Depending on an aggregate step likeprovision-azure-bicep-resourcesworked only because that step exists up front.Fix
ExecuteStepSequentiallyAsyncresolves steps without full-graph reference validation (duplicate names are still rejected;RequiredByedges to unresolved steps are skipped during normalization), then validates everyDependsOn/RequiredByreference of the steps it is about to execute (the target's transitive closure). Misconfigured steps that participate inbefore-startstill fail fast with the same messages.ExecuteAsync(publish/deploy/--step) is unchanged and still validates the full graph — by then the deployment targets exist, so the dependency onprovision-myjob-containerappresolves and is ordered correctly.Regression evidence
DistributedApplicationPipelineTests.ExecuteStepSequentiallyAsync_StepOutsideTargetClosureDependsOnStepMaterializedByTarget_Succeeds(core repro: a step factory that only yieldsprovision-computeafter a before-start step runs)Step 'start-job' depends on unknown step 'provision-compute'AzureContainerAppEnvironmentExtensionsTests.PipelineStepCanDependOnContainerAppJobProvisioningStep(the issue's AppHost: ACA env +PublishAsAzureContainerAppJob()+Pipeline.AddStep(dependsOn: "provision-myjob-containerapp", requiredBy: deploy), published end to end)Step 'start-myjob' depends on unknown step 'provision-myjob-containerapp'DependsOn/RequiredByinside the executed closure still throwBroader local runs with the fix (Linux):
Aspire.Hosting.TestsPipelines + PipelineExecutor + ComputeEnvironmentValidation + PythonAppDeployPipeline tests: 239/239 passed.Aspire.Hosting.Azure.TestsAzureContainerAppEnvironmentExtensions + AzureContainerApps + AzureDeployer + AzureAppService tests: 246/246 passed.Fixes #20301
Checklist
🤖 Posted by AspireAgent (AI Agent)