-
Notifications
You must be signed in to change notification settings - Fork 762
[release/13.0] Add deploy support for Docker Compose #12629
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
Conversation
* Add deploy support for Docker Compose * Fix duplicate service registration and simplify EnvFile logic (#12575) * Initial plan * Address Copilot feedback: Remove duplicate registration and simplify EnvFile logic Co-authored-by: captainsafia <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: captainsafia <[email protected]> * Refine Docker Compose publishing: env file naming, log verbosity, and status messages (#12580) * Initial plan * Address PR feedback: Update env file naming, reduce log verbosity, and enhance success message Co-authored-by: captainsafia <[email protected]> * Fix log verbosity: Move completion message to Debug level instead of task creation Co-authored-by: captainsafia <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: captainsafia <[email protected]> * Fix passing of RIDs in dotnet publish for back-compat --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: captainsafia <[email protected]>
* Initial plan * Update ContainerTargetPlatform from AllLinux to LinuxAmd64 - Replace ContainerTargetPlatform.AllLinux with LinuxAmd64 in ProjectResource.cs (2 occurrences) - Replace ContainerTargetPlatform.AllLinux with LinuxAmd64 in ContainerResourceBuilderExtensions.cs - Remove AllLinux enum value from ContainerTargetPlatform enum definition Co-authored-by: captainsafia <[email protected]> * Completed: Update ContainerTargetPlatform from AllLinux to LinuxAmd64 Co-authored-by: captainsafia <[email protected]> * Restore AllLinux enum and revert unintended template changes - Restore the AllLinux enum value in ContainerTargetPlatform - Revert all unintended changes to Aspire.ProjectTemplates localization files Co-authored-by: davidfowl <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: captainsafia <[email protected]> Co-authored-by: davidfowl <[email protected]>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12629Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12629" |
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.
Pull Request Overview
This PR refactors the Docker Compose publishing workflow to support multi-step pipelines with separate prepare, publish, and deploy stages. The key changes enable environment-specific .env files (.env.Production, .env.Staging, etc.) and introduce support for multi-platform container builds using flags on the ContainerTargetPlatform enum.
- Separates Docker Compose workflow into distinct pipeline steps (publish, prepare, docker-compose-up, docker-compose-down)
- Changes
.envfile generation to use environment-specific naming (.env.{EnvironmentName}) - Adds
[Flags]attribute toContainerTargetPlatformenum to enable multi-platform builds with a newAllLinuxcombined value - Refactors
EnvFileclass to support both key-only and key-value persistence modes
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting/Publishing/ResourceContainerImageBuilder.cs | Converts ContainerTargetPlatform enum to flags-based design; updates extension methods to handle multiple platforms; modifies MSBuild property selection based on RID count |
| src/Aspire.Hosting/CompatibilitySuppressions.xml | Adds API compatibility suppressions for enum value changes |
| src/Aspire.Hosting.Docker/EnvFile.cs | Refactors to use sorted dictionary; adds support for preserving comments and saving with/without values |
| src/Aspire.Hosting.Docker/DockerComposePublishingContext.cs | Simplifies publish step to save keys only; removes image building logic |
| src/Aspire.Hosting.Docker/DockerComposeEnvironmentResource.cs | Adds prepare, docker-compose-up, and docker-compose-down pipeline steps; implements environment-specific .env file naming |
| src/Aspire.Hosting.Docker/DockerComposePublisherLoggerExtensions.cs | Changes log level from Information to Debug |
| src/Aspire.Hosting.Docker/Aspire.Hosting.Docker.csproj | Adds process-related shared files |
| tests/Aspire.Hosting.Docker.Tests/DockerComposePublisherTests.cs | Adds tests for environment-specific .env files; updates test for image building behavior |
| tests/Aspire.Hosting.Docker.Tests/Snapshots/*.verified.env | Updates snapshot files to reflect empty values in publish step |
| { | ||
| var outputPath = PublishingContextUtils.GetEnvironmentOutputPath(context, this); | ||
| var hostEnvironment = context.Services.GetService<Microsoft.Extensions.Hosting.IHostEnvironment>(); | ||
| var environmentName = hostEnvironment?.EnvironmentName ?? Name; |
Copilot
AI
Nov 3, 2025
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.
The fallback to Name (the Docker Compose environment resource name) when EnvironmentName is unavailable is problematic. The tests expect .env.Production as the default, not .env.{resource-name}. Based on the test PrepareStep_GeneratesCorrectEnvFileWithDefaultEnvironmentName which expects .env.Production, the fallback should be \"Production\" rather than Name.
| var environmentName = hostEnvironment?.EnvironmentName ?? Name; | |
| var environmentName = hostEnvironment?.EnvironmentName ?? "Production"; |
| { | ||
| private readonly List<string> _lines = []; | ||
| private readonly HashSet<string> _keys = []; | ||
| private readonly SortedDictionary<string, EnvEntry> _entries = []; |
Copilot
AI
Nov 3, 2025
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.
Using SortedDictionary adds O(log n) overhead for insertions and lookups compared to a regular Dictionary. If the sorted order is only needed during Save(), consider using a regular Dictionary and sorting during serialization with OrderBy() to improve runtime performance for add/lookup operations.
|
Part of the aspire pipelines experience, localized to compose. Approved. |
| @@ -1,5 +1,5 @@ | |||
| # Parameter param1 | |||
| PARAM1=changed | |||
| PARAM1= | |||
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.
Are these changes intentional?
| internal static partial void EmptyModel(this ILogger logger); | ||
|
|
||
| [LoggerMessage(LogLevel.Information, "Successfully generated Compose output in '{OutputPath}'")] | ||
| [LoggerMessage(LogLevel.Debug, "Successfully generated Compose output in '{OutputPath}'")] |
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.
Without this, will people know where to look for the output?
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.
Yep, there was some duplication here because we emit the output in the completion message for the ReportingStep. I stepped down to debug since we surface Info logs by default now.
| buildSteps.RequiredBy(WellKnownPipelineSteps.Deploy) | ||
| .RequiredBy($"docker-compose-up-{Name}") |
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.
| buildSteps.RequiredBy(WellKnownPipelineSteps.Deploy) | |
| .RequiredBy($"docker-compose-up-{Name}") | |
| buildSteps.RequiredBy($"docker-compose-up-{Name}") |
Don't we only need this? Since dockerComposeUpStep.RequiredBy(WellKnownPipelineSteps.Deploy);.
|
The removal of I had to manually search for this ticket to find the change on why my code was breaking. Please update the documentation. |
|
Future breaking changes will be on aspire.dev https://aspire.dev/whats-new/aspire-13/#%EF%B8%8F-breaking-changes |
|
@davidfowl It's also not mentioned there, did check that one as-well. |
|
We suck 😄. There's a new step for docker compose that you can use: This will prompt for parameters, build images and create a final env file with values without running compose up |
|
@davidfowl Thanks for the heads-up. Sadly the ENV file is very broken due to wrong ordering. I created #13067 as follow-up. |
Customer Impact
Docker Compose environments now participate fully in the Aspire pipeline with explicit steps for
publish,prepare,docker-compose-up(required bydeploy), anddocker-compose-down.Testing
Manual.
Risk
Mediumm, because:
BuildContainerImagesoption was removed fromDockerComposeEnvironmentResourceand image building during publish was deleted in favor of pipeline-driven build steps. Existing flows relying on image auto-build during publish will now rely on the wired build steps prior to deploy. Impact is limited and aligned with pipeline design.Regression?
No, because the new steps preserve (and make explicit) the same publish capabilities that existed before.