-
Notifications
You must be signed in to change notification settings - Fork 746
Display resource endpoints for Docker Compose deploy #13216
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
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13216Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13216" |
256820a to
3c202d9
Compare
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 adds functionality to display resource endpoints after Docker Compose deployment, addressing issue #12820. The implementation uses pipeline steps to query running Docker Compose containers and reports their accessible endpoints to users.
Key changes:
- Adds endpoint discovery and display functionality using
docker compose ps --format json - Refactors Docker Compose argument building into a reusable method
- Integrates endpoint printing into the deployment pipeline workflow
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Aspire.Hosting.Docker/DockerComposeServiceResource.cs | Converts from primary constructor to explicit constructor; adds PrintEndpointsAsync method to query and display container endpoints after deployment; adds JSON deserialization classes for parsing docker compose output; registers pipeline step for endpoint printing |
| src/Aspire.Hosting.Docker/DockerComposeEnvironmentResource.cs | Extracts docker compose argument building logic into GetDockerComposeArguments helper method; changes GetEnvFilePath to static method; adds pipeline step dependency to ensure endpoint printing runs after docker-compose-up; adds --project-name argument to docker compose commands |
| if (endpointMapping.IsExternal || scheme is "http" or "https") | ||
| { | ||
| var endpoint = $"{scheme}://localhost:{publisher.PublishedPort}"; | ||
| endpoints.Add(endpoint); | ||
| } | ||
| } | ||
| } |
Copilot
AI
Nov 26, 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 logic for determining which endpoints to display could be clearer. The condition endpointMapping.IsExternal || scheme is "http" or "https" is confusing because:
externalEndpointMappingson line 371 already filters to external endpoints- When
FirstOrDefaultreturns no match (line 410),endpointMappingwill be the default struct value whereIsExternal = false - The fallback to checking
scheme is "http" or "https"suggests the intent is to show all http/https ports even without explicit mapping
Consider adding a comment to clarify this logic, or restructuring to make the intent clearer:
// Show endpoint if: it matches an external endpoint mapping, OR it's an http/https port (published ports are external by default)
var hasExplicitMapping = endpointMapping.Resource is not null;
if (hasExplicitMapping || scheme is "http" or "https")
{
var endpoint = $"{scheme}://localhost:{publisher.PublishedPort}";
endpoints.Add(endpoint);
}|
|
||
| return steps; | ||
| })); | ||
| } |
Copilot
AI
Nov 26, 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.
Missing blank line before the XML documentation comment. According to the codebase formatting conventions, there should be a blank line between the closing brace of the constructor and the XML documentation comment for the next member.
| } | |
| } |
| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) | ||
| { |
Copilot
AI
Nov 26, 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 PrintEndpointsAsync method is missing XML documentation. According to the Aspire XML documentation guidelines, internal methods should have brief <summary> tags explaining what they do. Consider adding:
/// <summary>
/// Prints the endpoints for the Docker Compose service after deployment.
/// </summary>| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) | |
| { | |
| /// <summary> | |
| /// Prints the endpoints for the Docker Compose service after deployment. | |
| /// </summary> | |
| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) |
| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) | ||
| { | ||
| var outputPath = PublishingContextUtils.GetEnvironmentOutputPath(context, environment); | ||
| var dockerComposeFilePath = Path.Combine(outputPath, "docker-compose.yaml"); | ||
|
|
||
| if (!File.Exists(dockerComposeFilePath)) | ||
| { | ||
| context.Logger.LogWarning("Docker Compose file not found at {Path}", dockerComposeFilePath); | ||
| return; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| // Use docker compose ps to get the running containers and their port mappings | ||
| var arguments = DockerComposeEnvironmentResource.GetDockerComposeArguments(context, environment); | ||
| arguments += " ps --format json"; | ||
|
|
||
| var outputLines = new List<string>(); | ||
|
|
||
| var spec = new ProcessSpec("docker") | ||
| { | ||
| Arguments = arguments, | ||
| WorkingDirectory = outputPath, | ||
| ThrowOnNonZeroReturnCode = false, | ||
| InheritEnv = true, | ||
| OnOutputData = output => | ||
| { | ||
| if (!string.IsNullOrWhiteSpace(output)) | ||
| { | ||
| outputLines.Add(output); | ||
| } | ||
| }, | ||
| OnErrorData = error => | ||
| { | ||
| if (!string.IsNullOrWhiteSpace(error)) | ||
| { | ||
| context.Logger.LogDebug("docker compose ps (stderr): {Error}", error); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| var (pendingProcessResult, processDisposable) = ProcessUtil.Run(spec); | ||
|
|
||
| await using (processDisposable) | ||
| { | ||
| var processResult = await pendingProcessResult | ||
| .WaitAsync(context.CancellationToken) | ||
| .ConfigureAwait(false); | ||
|
|
||
| if (processResult.ExitCode != 0) | ||
| { | ||
| context.Logger.LogWarning("Failed to query Docker Compose services for {ResourceName}. Exit code: {ExitCode}", TargetResource.Name, processResult.ExitCode); | ||
| return; | ||
| } | ||
|
|
||
| // Parse the JSON output to find port mappings for this service | ||
| var serviceName = TargetResource.Name.ToLowerInvariant(); | ||
| var endpoints = new HashSet<string>(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| // Get all external endpoint mappings for this resource | ||
| var externalEndpointMappings = EndpointMappings.Values.Where(m => m.IsExternal).ToList(); | ||
|
|
||
| // If there are no external endpoints configured, we're done | ||
| if (externalEndpointMappings.Count == 0) | ||
| { | ||
| context.ReportingStep.Log(LogLevel.Information, $"Successfully deployed **{TargetResource.Name}** to Docker Compose environment **{environment.Name}**. No public endpoints were configured.", enableMarkdown: true); | ||
| return; | ||
| } | ||
|
|
||
| foreach (var line in outputLines) | ||
| { | ||
| try | ||
| { | ||
| var serviceInfo = JsonSerializer.Deserialize(line, DockerComposeJsonContext.Default.DockerComposeServiceInfo); | ||
|
|
||
| if (serviceInfo is null || | ||
| !string.Equals(serviceInfo.Service, serviceName, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| if (serviceInfo.Publishers is not { Count: > 0 }) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| foreach (var publisher in serviceInfo.Publishers) | ||
| { | ||
| // Skip ports that aren't actually published (port 0 or null means not exposed) | ||
| if (publisher.PublishedPort is not > 0) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| // Try to find a matching external endpoint to get the scheme | ||
| // Match by internal port (numeric) or by exposed port | ||
| // InternalPort may be a placeholder like ${API_PORT} for projects, so also check ExposedPort | ||
| var targetPortStr = publisher.TargetPort?.ToString(CultureInfo.InvariantCulture); | ||
| var endpointMapping = externalEndpointMappings | ||
| .FirstOrDefault(m => m.InternalPort == targetPortStr || m.ExposedPort == publisher.TargetPort); | ||
|
|
||
| // If we found a matching endpoint, use its scheme; otherwise default to http for external ports | ||
| var scheme = endpointMapping.Scheme ?? "http"; | ||
|
|
||
| // Only add if we found a matching external endpoint OR if scheme is http/https | ||
| // (published ports are external by definition in docker compose) | ||
| if (endpointMapping.IsExternal || scheme is "http" or "https") | ||
| { | ||
| var endpoint = $"{scheme}://localhost:{publisher.PublishedPort}"; | ||
| endpoints.Add(endpoint); | ||
| } | ||
| } | ||
| } | ||
| catch (JsonException ex) | ||
| { | ||
| context.Logger.LogDebug(ex, "Failed to parse docker compose ps output line: {Line}", line); | ||
| } | ||
| } | ||
|
|
||
| // Display the endpoints | ||
| if (endpoints.Count > 0) | ||
| { | ||
| var endpointList = string.Join(", ", endpoints.Select(e => $"[{e}]({e})")); | ||
| context.ReportingStep.Log(LogLevel.Information, $"Successfully deployed **{TargetResource.Name}** to {endpointList}", enableMarkdown: true); | ||
| } | ||
| else | ||
| { | ||
| context.ReportingStep.Log(LogLevel.Information, $"Successfully deployed **{TargetResource.Name}** to Docker Compose environment **{environment.Name}**. No public endpoints were configured.", enableMarkdown: true); | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| context.Logger.LogWarning(ex, "Failed to retrieve endpoints for {ResourceName}", TargetResource.Name); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> |
Copilot
AI
Nov 26, 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 new endpoint printing functionality introduced in the PrintEndpointsAsync method and the pipeline step configuration lacks test coverage. Consider adding tests to verify:
- Endpoint discovery and display when containers are running
- Behavior when no external endpoints are configured
- Handling of multiple endpoints with different schemes
- Error handling when Docker Compose commands fail
The test file tests/Aspire.Hosting.Docker.Tests/DockerComposePublisherTests.cs has comprehensive test coverage for other Docker Compose functionality and would be an appropriate location for these tests.
captainsafia
left a comment
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.
@copilot Address this feedback.
| var printResourceSummary = new PipelineStep | ||
| { | ||
| Name = $"print-{resource.Name}-summary", | ||
| Action = async ctx => await PrintEndpointsAsync(ctx, composeEnvironmentResource).ConfigureAwait(false), |
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.
| Action = async ctx => await PrintEndpointsAsync(ctx, composeEnvironmentResource).ConfigureAwait(false), | |
| Action = async ctx => await PrintEndpointsAsync(ctx, _composeEnvironmentResource).ConfigureAwait(false), |
|
|
||
| var printResourceSummary = new PipelineStep | ||
| { | ||
| Name = $"print-{resource.Name}-summary", |
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.
| Name = $"print-{resource.Name}-summary", | |
| Name = $"print-{_targetResource.Name}-summary", |
| _composeEnvironmentResource = composeEnvironmentResource; | ||
|
|
||
| // Add pipeline step annotation to display endpoints after deployment | ||
| Annotations.Add(new PipelineStepAnnotation((factoryContext) => |
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.
| Annotations.Add(new PipelineStepAnnotation((factoryContext) => | |
| Annotations.Add(new PipelineStepAnnotation(_ => |
|
|
||
| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) | ||
| { | ||
| var outputPath = PublishingContextUtils.GetEnvironmentOutputPath(context, environment); |
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.
Remove this declaration and check for dockerComposeFilePath since we do it in DockerComposeEnvironmentResource.GetDockerComposeArguments now.
|
@captainsafia I've opened a new pull request, #13217, to work on those changes. Once the pull request is ready, I'll request review from you. |
…3217) * Initial plan * Address review feedback on DockerComposeServiceResource Co-authored-by: captainsafia <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: captainsafia <[email protected]>
| try | ||
| { | ||
| // Use docker compose ps to get the running containers and their port mappings | ||
| var arguments = DockerComposeEnvironmentResource.GetDockerComposeArguments(context, environment); |
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.
@copilot add a comment that shows an example of the expected format being parsed here from docker compose.
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.
@copilot add a comment that shows an example of the expected format being parsed here from docker compose.
|
@davidfowl I've opened a new pull request, #13220, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@davidfowl I've opened a new pull request, #13222, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
It's not printing the dashboard url. #:package Aspire.Hosting.Docker@13.1.0-pr.13216.g40a62e5e
#:sdk Aspire.AppHost.Sdk@13.1.0-pr.13216.g40a62e5e
var builder = DistributedApplication.CreateBuilder(args);
builder.AddDockerComposeEnvironment("env")
.WithDashboard(db => db.WithHostPort(9090));
#pragma warning disable ASPIRECSHARPAPPS001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
builder.AddCSharpApp("api3", "./api.cs")
.WithHttpEndpoint(targetPort: 5000)
.WithExternalHttpEndpoints();
builder.Build().Run(); |
| } | ||
| } | ||
|
|
||
| private async Task PrintEndpointsAsync(PipelineStepContext context, DockerComposeEnvironmentResource environment) |
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.
Doesn't this need to handle potential slow startup of services? What guaranteees that docker compose ps --format json will have the information by the time you call it?
Yep, I haven't done anything around that in this PR. Although I wonder if printing the dashboard URL might be a better strategy since it launches deterministically and can be used at the gateway to discover service URIs. |
|
If you have it enabled, we should show it. I was playing with this in my ssh docker deploy and we should make sure that it works well if containers don't start up immediately. |
Closes #12820.