-
Notifications
You must be signed in to change notification settings - Fork 770
Add support for BeforeStart Pipeline steps #13780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…e BeforeStartEvent. This allows for ordering of BeforeStart subscriptions.
…rer and run mode provision steps.
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13780Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13780" |
| // for elements. | ||
| return builder.AddResource(resource) | ||
| .ExcludeFromManifest(); | ||
| .ExcludeFromManifest() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be helpful to note that we need to do this because the AzureEnvironmentResource needs to show up in the app model during run mode so that we can discover the pipeline step annotations on it but it needs to be hidden from the end-user.
If we continue down this route of making events into pipeline steps, we might need a better way to define a resource that is discoverable in the app model but not visible to the end user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add another comment here, and I opened Add an API for marking a resource as hidden in the dashboard (dotnet/aspire#13799) for this.
| { | ||
| var azureResources = AzureResourcePreparer.GetAzureResourcesFromAppModel(@event.Model); | ||
| // Only run in RunMode | ||
| if (!executionContext.IsRunMode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think eventually we'll get to a point where we don't run this in RunMode either and the provisioning at run-mode also happens via the provision infrastructure meta-step.
Maybe we work towards that in a follow-up PR...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - I had a similar thought when I was trying to name the steps. 😄 We already have a provisionStep named provision-azure-bicep-resources. And when I added a new step for the run-mode provision, it was conflicting, which made me think that we could think about merging these now.
Maybe we work towards that in a follow-up PR...?
For sure not as part of this PR.
| builder.Services.TryAddSingleton<AzureResourcePreparer>(); | ||
| builder.Services.TryAddSingleton<AzureProvisioner>(); | ||
|
|
||
| //#pragma warning disable ASPIREPIPELINES001 // Pipeline APIs are experimental |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What problem did you run into with this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops - this is old and needs to be deleted.
The problem I hit in the commented out code is that AddAzureProvisioning gets called multiple times (from each Azure resource). At first I was just adding the step, but that fails for duplicate steps. That caused me to look at a TryAddStep API on the pipeline, but I changed it to add the steps from the resource, which is already only added once, so the steps only get added once.
Description
There are many cases where we need to order BeforeStartEvent subscriptions. For example, if someone wants something to run before the AzureResourcePreparer, so it can add role assignments correctly, there is no way to ensure the AzureResourcePreparer's BeforeStartEvent subscription has already run.
To fix this, we reuse the Pipeline steps, which already has dependencies built in to it. Then we convert the Azure BeforeStartEvent subscriptions to pipeline steps, so they can be depended on (or required by).
Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplatebreaking-changetemplatediagnostictemplate