-
Notifications
You must be signed in to change notification settings - Fork 720
Support aspire deploy for Docker Compose #12455
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
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12455Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12455" |
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 pull request enhances Docker Compose publishing functionality by adding support for executing docker compose up commands and improving environment variable management. The changes move container image building from the publishing phase to dedicated pipeline build steps and add the ability to update existing .env file entries.
Key Changes
- Added
DockerComposeUpAsyncmethod to execute docker compose up commands as part of the deployment pipeline - Enhanced
EnvFile.Addto support updating existing keys whenonlyIfMissing=false - Refactored image building to occur in dedicated pipeline build steps instead of during the publish phase
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/Aspire.Hosting.Docker/EnvFile.cs | Added logic to update existing environment variable entries when onlyIfMissing=false, enabling key value updates in .env files |
| src/Aspire.Hosting.Docker/DockerComposePublishingContext.cs | Removed IResourceContainerImageBuilder dependency and image building logic; simplified parameter resolution to handle all parameters uniformly |
| src/Aspire.Hosting.Docker/DockerComposeEnvironmentResource.cs | Added DockerComposeUpAsync method for deployment execution, created pipeline build steps for container images, and moved image building from publish to dedicated build pipeline steps |
| // Handle parameter resources by resolving their actual values | ||
| if (source is ParameterResource parameter) | ||
| { | ||
| // For non-secret parameters, get the actual parameter value | ||
| defaultValue = await parameter.GetValueAsync(cancellationToken).ConfigureAwait(false); | ||
| } |
Copilot
AI
Oct 28, 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 condition at line 169 now calls GetValueAsync for all parameter resources, including secret parameters. However, the original code explicitly excluded secret parameters with the condition !parameter.Secret. Removing this check may result in calling GetValueAsync on secret parameters, which could expose sensitive values to the .env file or have other unintended consequences.
The original logic should be preserved to avoid handling secret parameters:
// Handle parameter resources by resolving their actual values
if (source is ParameterResource parameter && !parameter.Secret)
{
// For non-secret parameters, get the actual parameter value
defaultValue = await parameter.GetValueAsync(cancellationToken).ConfigureAwait(false);
}|
|
||
| // If the key already exists and we want to update it (onlyIfMissing = false), | ||
| // we need to find and replace the existing entry | ||
| // Find the existing key-value line and replace it |
Copilot
AI
Oct 28, 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 comment at line 47 says "Find the existing key-value line and replace it" which is redundant with the comment at line 45-46. The comments should be consolidated or the redundant one removed.
Consider simplifying to:
// If the key already exists and we want to update it (onlyIfMissing = false),
// find and replace the existing entry
for (var i = 0; i < _lines.Count; i++)| // Find the existing key-value line and replace it |
|
@captainsafia I've opened a new pull request, #12463, to work on those changes. Once the pull request is ready, I'll request review from you. |
f98d9d5 to
aa5968b
Compare
|
/test-scenario deployment-docker |
|
🤖 AI Agent Task Created Scenario: deployment-docker An AI agent has been assigned to execute this scenario. 📝 Issue: https://github.com/dotnet/aspire-playground/issues/82 Please navigate to the issue for more details and to track progress. |
…entResource (#12463) * Initial plan * Remove redundant IsExcludedFromPublish check Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
19ac48a to
35f2764
Compare
This pull request refactors the pipeline steps for Docker Compose environments and improves deployment and environment variable handling. The main changes include restructuring how pipeline steps are registered, introducing a dedicated deployment step for
docker compose up, refactoring image build logic, and enhancing environment variable updates in.envfiles.Pipeline and Deployment Step Refactoring
DockerComposeEnvironmentResourcenow asynchronously creates multiple steps: a publish step, a dedicateddocker-compose-updeployment step, and individual build steps for each compute resource. This improves flexibility and clarity in the deployment process.DockerComposeUpAsyncmethod is introduced, which runsdocker compose upas a separate pipeline step, reporting progress and handling errors more robustly.Container Image Build Logic
DockerComposePublishingContext; image building is now handled by dedicated pipeline steps registered inDockerComposeEnvironmentResource. This simplifies the publishing context and ensures images are built at the correct stage. [1] [2] [3] [4] [5]Environment Variable and Parameter Handling
DockerComposePublishingContextis improved: parameter resources now resolve their actual runtime values, not just default values, ensuring more accurate.envfiles.EnvFileclass is enhanced to support updating existing keys whenonlyIfMissingis false, allowing for more flexible environment variable management.These changes collectively improve the reliability, maintainability, and clarity of Docker Compose environment deployment and configuration in the Aspire hosting pipeline.
Fixes #12309
Fixes #11756
Fixes #11545