-
Notifications
You must be signed in to change notification settings - Fork 760
Add ContainerBuildOptions support to ResourceContainerImageBuilder for customizing dotnet publish #10074
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
Add ContainerBuildOptions support to ResourceContainerImageBuilder for customizing dotnet publish #10074
Changes from 11 commits
e59eb66
a455b56
7d99db0
0844e4d
9864489
fcbd217
9125a8f
e00837e
bbb0e73
447c25f
096bc8e
115e593
f0c0fee
cd81d67
ad2c9f3
7f3ca1e
6fff8f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| #pragma warning disable ASPIREPUBLISHERS001 | ||
|
|
||
| using Aspire.Hosting.Dcp.Process; | ||
| using Microsoft.Extensions.Logging; | ||
|
|
||
|
|
@@ -9,49 +11,117 @@ namespace Aspire.Hosting.Publishing; | |
| internal sealed class DockerContainerRuntime(ILogger<DockerContainerRuntime> logger) : IContainerRuntime | ||
| { | ||
| public string Name => "Docker"; | ||
| private async Task<int> RunDockerBuildAsync(string contextPath, string dockerfilePath, string imageName, CancellationToken cancellationToken) | ||
| private async Task<int> RunDockerBuildAsync(string contextPath, string dockerfilePath, string imageName, ContainerBuildOptions? options, CancellationToken cancellationToken) | ||
| { | ||
| var spec = new ProcessSpec("docker") | ||
| string? builderName = null; | ||
|
|
||
| // Docker requires a custom buildkit instance for the image when | ||
| // targeting the OCI format so we construct it and remove it here. | ||
| if (options?.ImageFormat == ContainerImageFormat.Oci) | ||
| { | ||
| Arguments = $"build --file {dockerfilePath} --tag {imageName} {contextPath}", | ||
| OnOutputData = output => | ||
| if (string.IsNullOrEmpty(options?.OutputPath)) | ||
| { | ||
| logger.LogInformation("docker build (stdout): {Output}", output); | ||
| }, | ||
| OnErrorData = error => | ||
| { | ||
| logger.LogInformation("docker build (stderr): {Error}", error); | ||
| }, | ||
| ThrowOnNonZeroReturnCode = false, | ||
| InheritEnv = true | ||
| }; | ||
| throw new ArgumentException("OutputPath must be provided when ImageFormat is Oci.", nameof(options)); | ||
| } | ||
|
|
||
| logger.LogInformation("Running Docker CLI with arguments: {ArgumentList}", spec.Arguments); | ||
| var (pendingProcessResult, processDisposable) = ProcessUtil.Run(spec); | ||
| builderName = $"{imageName.Replace('/', '-').Replace(':', '-')}-builder"; | ||
| var createBuilderResult = await CreateBuildkitInstanceAsync(builderName, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| await using (processDisposable) | ||
| if (createBuilderResult != 0) | ||
| { | ||
| logger.LogError("Failed to create buildkit instance {BuilderName} with exit code {ExitCode}.", builderName, createBuilderResult); | ||
| return createBuilderResult; | ||
| } | ||
| } | ||
|
|
||
| try | ||
| { | ||
| var processResult = await pendingProcessResult | ||
| .WaitAsync(cancellationToken) | ||
| .ConfigureAwait(false); | ||
| var arguments = $"buildx build --file \"{dockerfilePath}\" --tag \"{imageName}\""; | ||
|
|
||
| if (processResult.ExitCode != 0) | ||
| // Use the specific builder for OCI builds | ||
| if (!string.IsNullOrEmpty(builderName)) | ||
| { | ||
| logger.LogError("Docker build for {ImageName} failed with exit code {ExitCode}.", imageName, processResult.ExitCode); | ||
| return processResult.ExitCode; | ||
| arguments += $" --builder \"{builderName}\""; | ||
| } | ||
|
|
||
| logger.LogInformation("Docker build for {ImageName} succeeded.", imageName); | ||
| return processResult.ExitCode; | ||
| // Add platform support if specified | ||
| if (options?.TargetPlatform is not null) | ||
| { | ||
| arguments += $" --platform \"{options.TargetPlatform.Value.ToRuntimePlatformString()}\""; | ||
| } | ||
|
|
||
| // Add output format support if specified | ||
| if (options?.ImageFormat is not null || !string.IsNullOrEmpty(options?.OutputPath)) | ||
| { | ||
| var outputType = options?.ImageFormat switch | ||
| { | ||
| ContainerImageFormat.Oci => "type=oci", | ||
| ContainerImageFormat.Docker => "type=docker", | ||
| null => "type=docker", | ||
| _ => throw new ArgumentOutOfRangeException(nameof(options), options.ImageFormat, "Invalid container image format") | ||
| }; | ||
|
|
||
| if (!string.IsNullOrEmpty(options?.OutputPath)) | ||
| { | ||
| outputType += $",dest=\"{options.OutputPath}/{imageName}.tar\""; | ||
| } | ||
|
|
||
| arguments += $" --output \"{outputType}\""; | ||
| } | ||
|
|
||
| arguments += $" \"{contextPath}\""; | ||
|
|
||
| var spec = new ProcessSpec("docker") | ||
| { | ||
| Arguments = arguments, | ||
| OnOutputData = output => | ||
| { | ||
| logger.LogInformation("docker buildx (stdout): {Output}", output); | ||
| }, | ||
| OnErrorData = error => | ||
| { | ||
| logger.LogInformation("docker buildx (stderr): {Error}", error); | ||
| }, | ||
| ThrowOnNonZeroReturnCode = false, | ||
| InheritEnv = true | ||
| }; | ||
|
|
||
| logger.LogInformation("Running Docker CLI with arguments: {ArgumentList}", spec.Arguments); | ||
| var (pendingProcessResult, processDisposable) = ProcessUtil.Run(spec); | ||
|
|
||
| await using (processDisposable) | ||
| { | ||
| var processResult = await pendingProcessResult | ||
| .WaitAsync(cancellationToken) | ||
| .ConfigureAwait(false); | ||
|
|
||
| if (processResult.ExitCode != 0) | ||
| { | ||
| logger.LogError("docker buildx for {ImageName} failed with exit code {ExitCode}.", imageName, processResult.ExitCode); | ||
| return processResult.ExitCode; | ||
| } | ||
|
|
||
| logger.LogInformation("docker buildx for {ImageName} succeeded.", imageName); | ||
| return processResult.ExitCode; | ||
| } | ||
| } | ||
| finally | ||
| { | ||
| // Clean up the buildkit instance if we created one | ||
| if (!string.IsNullOrEmpty(builderName)) | ||
| { | ||
| await RemoveBuildkitInstanceAsync(builderName, cancellationToken).ConfigureAwait(false); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public async Task BuildImageAsync(string contextPath, string dockerfilePath, string imageName, CancellationToken cancellationToken) | ||
| public async Task BuildImageAsync(string contextPath, string dockerfilePath, string imageName, ContainerBuildOptions? options, CancellationToken cancellationToken) | ||
| { | ||
| var exitCode = await RunDockerBuildAsync( | ||
| contextPath, | ||
| dockerfilePath, | ||
| imageName, | ||
| options, | ||
| cancellationToken).ConfigureAwait(false); | ||
|
|
||
| if (exitCode != 0) | ||
|
|
@@ -64,14 +134,14 @@ public Task<bool> CheckIfRunningAsync(CancellationToken cancellationToken) | |
| { | ||
| var spec = new ProcessSpec("docker") | ||
| { | ||
| Arguments = "info", | ||
| Arguments = "buildx version", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did this change from
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're taking a dependency on |
||
| OnOutputData = output => | ||
| { | ||
| logger.LogInformation("docker info (stdout): {Output}", output); | ||
| logger.LogInformation("docker buildx version (stdout): {Output}", output); | ||
| }, | ||
| OnErrorData = error => | ||
| { | ||
| logger.LogInformation("docker info (stderr): {Error}", error); | ||
| logger.LogInformation("docker buildx version (stderr): {Error}", error); | ||
| }, | ||
| ThrowOnNonZeroReturnCode = false, | ||
| InheritEnv = true | ||
|
|
@@ -80,24 +150,105 @@ public Task<bool> CheckIfRunningAsync(CancellationToken cancellationToken) | |
| logger.LogInformation("Running Docker CLI with arguments: {ArgumentList}", spec.Arguments); | ||
| var (pendingProcessResult, processDisposable) = ProcessUtil.Run(spec); | ||
|
|
||
| return CheckDockerInfoAsync(pendingProcessResult, processDisposable, cancellationToken); | ||
| return CheckDockerBuildxAsync(pendingProcessResult, processDisposable, cancellationToken); | ||
|
|
||
| async Task<bool> CheckDockerInfoAsync(Task<ProcessResult> pendingResult, IAsyncDisposable processDisposable, CancellationToken ct) | ||
| async Task<bool> CheckDockerBuildxAsync(Task<ProcessResult> pendingResult, IAsyncDisposable processDisposable, CancellationToken ct) | ||
| { | ||
| await using (processDisposable) | ||
| { | ||
| var processResult = await pendingResult.WaitAsync(ct).ConfigureAwait(false); | ||
|
|
||
| if (processResult.ExitCode != 0) | ||
| { | ||
| logger.LogError("Docker info failed with exit code {ExitCode}.", processResult.ExitCode); | ||
| logger.LogError("Docker buildx version failed with exit code {ExitCode}.", processResult.ExitCode); | ||
| return false; | ||
| } | ||
|
|
||
| // Optionally, parse output for health, but exit code 0 is usually sufficient. | ||
| logger.LogInformation("Docker is running and healthy."); | ||
| logger.LogInformation("Docker buildx is available and running."); | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private async Task<int> CreateBuildkitInstanceAsync(string builderName, CancellationToken cancellationToken) | ||
| { | ||
| var arguments = $"buildx create --name \"{builderName}\" --driver docker-container"; | ||
|
|
||
| var spec = new ProcessSpec("docker") | ||
| { | ||
| Arguments = arguments, | ||
| OnOutputData = output => | ||
| { | ||
| logger.LogInformation("docker buildx create (stdout): {Output}", output); | ||
| }, | ||
| OnErrorData = error => | ||
| { | ||
| logger.LogInformation("docker buildx create (stderr): {Error}", error); | ||
| }, | ||
| ThrowOnNonZeroReturnCode = false, | ||
| InheritEnv = true | ||
| }; | ||
|
|
||
| logger.LogInformation("Creating buildkit instance with arguments: {ArgumentList}", spec.Arguments); | ||
| var (pendingProcessResult, processDisposable) = ProcessUtil.Run(spec); | ||
|
|
||
| await using (processDisposable) | ||
| { | ||
| var processResult = await pendingProcessResult | ||
| .WaitAsync(cancellationToken) | ||
| .ConfigureAwait(false); | ||
|
|
||
| if (processResult.ExitCode != 0) | ||
| { | ||
| logger.LogError("Failed to create buildkit instance {BuilderName} with exit code {ExitCode}.", builderName, processResult.ExitCode); | ||
| } | ||
| else | ||
| { | ||
| logger.LogInformation("Successfully created buildkit instance {BuilderName}.", builderName); | ||
| } | ||
|
|
||
| return processResult.ExitCode; | ||
| } | ||
| } | ||
|
|
||
| private async Task<int> RemoveBuildkitInstanceAsync(string builderName, CancellationToken cancellationToken) | ||
| { | ||
| var arguments = $"buildx rm \"{builderName}\""; | ||
|
|
||
| var spec = new ProcessSpec("docker") | ||
| { | ||
| Arguments = arguments, | ||
| OnOutputData = output => | ||
| { | ||
| logger.LogInformation("docker buildx rm (stdout): {Output}", output); | ||
| }, | ||
| OnErrorData = error => | ||
| { | ||
| logger.LogInformation("docker buildx rm (stderr): {Error}", error); | ||
| }, | ||
| ThrowOnNonZeroReturnCode = false, | ||
| InheritEnv = true | ||
| }; | ||
|
|
||
| logger.LogInformation("Removing buildkit instance with arguments: {ArgumentList}", spec.Arguments); | ||
| var (pendingProcessResult, processDisposable) = ProcessUtil.Run(spec); | ||
|
|
||
| await using (processDisposable) | ||
| { | ||
| var processResult = await pendingProcessResult | ||
| .WaitAsync(cancellationToken) | ||
| .ConfigureAwait(false); | ||
|
|
||
| if (processResult.ExitCode != 0) | ||
| { | ||
| logger.LogWarning("Failed to remove buildkit instance {BuilderName} with exit code {ExitCode}.", builderName, processResult.ExitCode); | ||
| } | ||
| else | ||
| { | ||
| logger.LogInformation("Successfully removed buildkit instance {BuilderName}.", builderName); | ||
| } | ||
|
|
||
| return processResult.ExitCode; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,13 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| #pragma warning disable ASPIREPUBLISHERS001 | ||
|
|
||
| namespace Aspire.Hosting.Publishing; | ||
|
|
||
| internal interface IContainerRuntime | ||
| { | ||
| string Name { get; } | ||
| Task<bool> CheckIfRunningAsync(CancellationToken cancellationToken); | ||
| public Task BuildImageAsync(string contextPath, string dockerfilePath, string imageName, CancellationToken cancellationToken); | ||
| public Task BuildImageAsync(string contextPath, string dockerfilePath, string imageName, ContainerBuildOptions? options, CancellationToken cancellationToken); | ||
| } |
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.
How does quotes inside of quotes work here?
--output "type=oci,dest="C:\temp\foo.tar""Uh oh!
There was an error while loading. Please reload this page.
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.
Quoted values have been working fine for me in Bash/zshell. Some further digging reveals that Powershell doesn't handle this as nicely so the best thing to do is no quotes, although that assumes an output path with no spaces. :/
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.
Why can't this be inverted? Quote the path, but don't quote around the value to
--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.
You mean like
--output type=oci,dest="C:\temp\foo.tar"? I think that's probably worse because there is no indication to the shell that the value is on cohesive string other than the whitespace separation. We're also pretty consistently quoting outer arguments in our CLI calls.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
What else could have whitespace?