Pass target platform to DCP for Dockerfile builds#16700
Pass target platform to DCP for Dockerfile builds#16700McDonaldSean wants to merge 2 commits intomicrosoft:mainfrom
Conversation
The publishing path (DockerContainerRuntime/PodmanContainerRuntime) reads TargetPlatform from ContainerBuildOptionsCallbackAnnotation and passes --platform to the runtime, but the local dev DCP path (ContainerCreator.ApplyBuildArgumentsAsync) ignored the annotation. As a result, docker build defaulted to the host architecture, so on hosts whose architecture differed from the resource's target platform (e.g. arm64 hosts with a linux/amd64 target), Dockerfile builds could fail or produce images for the wrong platform. Add a Platform property to the DCP BuildContext model and populate it from the resolved ContainerBuildOptionsCallbackContext when applying build arguments. The DCP runtime already supports a json:"platform" field on the build spec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16700Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16700" |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for passing a container target platform into the DCP build spec for local-dev Dockerfile builds, aligning DCP behavior with the publishing path so docker build doesn’t default to the host architecture.
Changes:
- Add
platformto the DCPBuildContextmodel. - Populate DCP build spec platform from the resolved
ContainerBuildOptionsCallbackContext.TargetPlatform. - Add a unit test asserting the DCP container build spec includes the platform.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs | Adds coverage verifying platform is emitted into the DCP build spec for Dockerfile resources. |
| src/Aspire.Hosting/Dcp/Model/Container.cs | Extends the DCP build model with a platform JSON field. |
| src/Aspire.Hosting/Dcp/ContainerCreator.cs | Resolves build options and sets Spec.Build.Platform for DCP-created Dockerfile builds. |
| var builder = DistributedApplication.CreateBuilder(); | ||
| builder.AddDockerfile("mycontainer", tempDockerfileContext.ContextPath, tempDockerfileContext.DockerfilePath); | ||
|
|
||
| var kubernetesService = new TestKubernetesService(); | ||
|
|
||
| using var app = builder.Build(); | ||
| var distributedAppModel = app.Services.GetRequiredService<DistributedApplicationModel>(); | ||
|
|
||
| var appExecutor = CreateAppExecutor(distributedAppModel, kubernetesService: kubernetesService); | ||
|
|
||
| await appExecutor.RunApplicationAsync(); | ||
|
|
||
| var container = Assert.Single(kubernetesService.CreatedResources.OfType<Container>()); | ||
| Assert.NotNull(container.Spec.Build); | ||
| Assert.Equal("linux/amd64", container.Spec.Build!.Platform); |
| var buildOptionsContext = await modelContainerResource.ProcessContainerBuildOptionsCallbackAsync( | ||
| serviceProvider, | ||
| NullLogger.Instance, |
| #pragma warning disable ASPIREEXTENSION001 | ||
| #pragma warning disable ASPIRECERTIFICATES001 | ||
| #pragma warning disable ASPIRECONTAINERSHELLEXECUTION001 | ||
| #pragma warning disable ASPIREPIPELINES003 |
|
@microsoft-github-policy-service agree |
|
|
||
| // Optional target platform for the build (e.g. "linux/amd64") | ||
| [JsonPropertyName("platform")] | ||
| public string? Platform { get; set; } |
There was a problem hiding this comment.
There isn't a platform field in the DCP types (https://github.com/microsoft/dcp/blob/main/api/v1/container_types.go#L140), so any proposed change would need to start there.
| Secrets = dcpBuildSecrets | ||
| }; | ||
|
|
||
| var buildOptionsContext = await modelContainerResource.ProcessContainerBuildOptionsCallbackAsync( |
There was a problem hiding this comment.
This would need to be done in a way that doesn't take a dependency on the publish types in DCP (no Aspire.Hosting.Publishing dependency).
|
|
||
| if (buildOptionsContext.TargetPlatform is { } targetPlatform) | ||
| { | ||
| dcpContainerResource.Spec.Build.Platform = targetPlatform.ToRuntimePlatformString(); |
There was a problem hiding this comment.
I'm nervous about us overriding the default run time platform to linux/amd64 regardless of the host architecture at run time when building a Dockerfile just because I've seen enough cases where the emulation layers fail. I'd want to consider supporting separate publish and runtime defaults (stick with linux/amd64 as the default publish platform, but continue to use the current machine architecture as the default at runtime.
- Plumb the per-resource ILogger through ApplyBuildArgumentsAsync instead of NullLogger so warnings/errors from build-options callbacks surface in local dev. Matches the other ProcessContainerBuildOptionsCallbackAsync call sites. - Narrow the ASPIREPIPELINES003 suppression to the specific call/usage rather than disabling it file-wide. - Make DockerfileContainerBuildSpecIncludesPlatform explicitly set TargetPlatform via WithContainerBuildOptions (using LinuxArm64) so the test validates propagation rather than relying on AddDockerfile's default of LinuxAmd64. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks — this all makes sense. Re-reading the surrounding code, I think the intended shape is:
Marking as draft until #4 lands. |
* Support target platform for container builds Add an optional Platform field to ContainerBuildContext that DCP forwards to docker/podman build via --platform. This lets Aspire (and other DCP clients) request a specific build target architecture for Dockerfile builds during local dev, instead of always defaulting to the host architecture. The change is additive and opt-in: existing workloads have Platform="" which is omitted from the build command, preserving today's behavior exactly. The Equal method and GetLifecycleKey hash both include the new field so spec changes propagate correctly: changing Platform on a persistent container will trigger a rebuild. Context: microsoft/aspire#16699 / microsoft/aspire#16700. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Disambiguate Stage/Platform in lifecycle key hash Address PR review: writing Stage and Platform as adjacent raw byte slices made the boundary ambiguous (e.g., Stage="ab",Platform="c" hashed the same as Stage="a",Platform="bc"). Encode Platform via gob (length-framed) and only when non-empty, both to disambiguate the hash and to preserve the legacy lifecycle-key value for existing workloads where Platform is unset. Add a test asserting the boundary case produces distinct keys. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Description
The publishing path (
DockerContainerRuntime/PodmanContainerRuntime) readsTargetPlatformfromContainerBuildOptionsCallbackAnnotationand passes--platformto the runtime, but the local dev DCP path (ContainerCreator.ApplyBuildArgumentsAsync) ignored the annotation. As a result,docker builddefaulted to the host architecture, so on hosts whose architecture differed from the resource's target platform (e.g. arm64 hosts with alinux/amd64target), Dockerfile builds could fail or produce images for the wrong platform.This change adds a
Platformproperty to the DCPBuildContextmodel and populates it from the resolvedContainerBuildOptionsCallbackContextwhen applying build arguments. The DCP runtime already supports ajson:"platform"field on the build spec, so the C# side just needed to send it.Fixes #16699
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: