Skip to content

Commit f3f57cc

Browse files
davidfowlCopilot
andcommitted
Address PR review feedback
- Restore soft-report behavior for compose up/down failures: use FailAsync without re-throwing, matching the original non-fatal behavior (JamesNK feedback) - Add EnsureRuntimeAvailableAsync to Podman ComposeListServicesAsync override so missing podman binary gets an actionable error - Make EnsureRuntimeAvailableAsync protected for subclass access - Rename PascalCase ContainerRuntime locals to camelCase in ResourceContainerImageManager Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ceea24f commit f3f57cc

4 files changed

Lines changed: 21 additions & 21 deletions

File tree

src/Aspire.Hosting.Docker/DockerComposeEnvironmentResource.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,7 @@ await deployTask.CompleteAsync(
243243
}
244244
catch (Exception ex)
245245
{
246-
await deployTask.CompleteAsync($"Compose deployment failed ({runtime.Name}): {ex.Message}", CompletionState.CompletedWithError, context.CancellationToken).ConfigureAwait(false);
247-
throw;
246+
await deployTask.FailAsync($"Compose deployment failed ({runtime.Name}): {ex.Message}", cancellationToken: context.CancellationToken).ConfigureAwait(false);
248247
}
249248
}
250249
}
@@ -279,8 +278,7 @@ await deployTask.CompleteAsync(
279278
}
280279
catch (Exception ex)
281280
{
282-
await deployTask.CompleteAsync($"Compose shutdown failed ({runtime.Name}): {ex.Message}", CompletionState.CompletedWithError, context.CancellationToken).ConfigureAwait(false);
283-
throw;
281+
await deployTask.FailAsync($"Compose shutdown failed ({runtime.Name}): {ex.Message}", cancellationToken: context.CancellationToken).ConfigureAwait(false);
284282
}
285283
}
286284
}

src/Aspire.Hosting/Publishing/ContainerRuntimeBase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ private static string BuildComposeArguments(ComposeOperationContext context)
536536
/// Validates that the container runtime binary is available on the system PATH.
537537
/// Fails fast with an actionable error message instead of a cryptic exit code.
538538
/// </summary>
539-
private async Task EnsureRuntimeAvailableAsync()
539+
protected async Task EnsureRuntimeAvailableAsync()
540540
{
541541
try
542542
{

src/Aspire.Hosting/Publishing/PodmanContainerRuntime.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ public PodmanContainerRuntime(ILogger<PodmanContainerRuntime> logger) : base(log
2626
/// </summary>
2727
public override async Task<IReadOnlyList<ComposeServiceInfo>?> ComposeListServicesAsync(ComposeOperationContext context, CancellationToken cancellationToken)
2828
{
29+
await EnsureRuntimeAvailableAsync().ConfigureAwait(false);
30+
2931
var arguments = $"ps --filter label=com.docker.compose.project={context.ProjectName} --format json";
3032

3133
var outputLines = new List<string>();

src/Aspire.Hosting/Publishing/ResourceContainerImageManager.cs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -206,23 +206,23 @@ private async Task<ResolvedContainerBuildOptions> ResolveContainerBuildOptionsAs
206206

207207
public async Task BuildImagesAsync(IEnumerable<IResource> resources, CancellationToken cancellationToken = default)
208208
{
209-
var ContainerRuntime = await GetContainerRuntimeAsync(cancellationToken).ConfigureAwait(false);
209+
var containerRuntime = await GetContainerRuntimeAsync(cancellationToken).ConfigureAwait(false);
210210
logger.LogInformation("Starting to build container images");
211211

212212
// Only check container runtime health if there are resources that need it
213213
if (await ResourcesRequireContainerRuntimeAsync(resources, cancellationToken).ConfigureAwait(false))
214214
{
215-
logger.LogDebug("Checking {ContainerRuntimeName} health", ContainerRuntime.Name);
215+
logger.LogDebug("Checking {ContainerRuntimeName} health", containerRuntime.Name);
216216

217-
var containerRuntimeHealthy = await ContainerRuntime.CheckIfRunningAsync(cancellationToken).ConfigureAwait(false);
217+
var containerRuntimeHealthy = await containerRuntime.CheckIfRunningAsync(cancellationToken).ConfigureAwait(false);
218218

219219
if (!containerRuntimeHealthy)
220220
{
221-
logger.LogError("Container runtime '{ContainerRuntimeName}' is not running or is unhealthy. Cannot build container images.", ContainerRuntime.Name);
222-
throw new InvalidOperationException($"Container runtime '{ContainerRuntime.Name}' is not running or is unhealthy.");
221+
logger.LogError("Container runtime '{ContainerRuntimeName}' is not running or is unhealthy. Cannot build container images.", containerRuntime.Name);
222+
throw new InvalidOperationException($"Container runtime '{containerRuntime.Name}' is not running or is unhealthy.");
223223
}
224224

225-
logger.LogDebug("{ContainerRuntimeName} is healthy", ContainerRuntime.Name);
225+
logger.LogDebug("{ContainerRuntimeName} is healthy", containerRuntime.Name);
226226
}
227227

228228
foreach (var resource in resources)
@@ -236,25 +236,25 @@ public async Task BuildImagesAsync(IEnumerable<IResource> resources, Cancellatio
236236

237237
public async Task BuildImageAsync(IResource resource, CancellationToken cancellationToken = default)
238238
{
239-
var ContainerRuntime = await GetContainerRuntimeAsync(cancellationToken).ConfigureAwait(false);
239+
var containerRuntime = await GetContainerRuntimeAsync(cancellationToken).ConfigureAwait(false);
240240
logger.LogInformation("Building container image for resource {ResourceName}", resource.Name);
241241

242242
var options = await ResolveContainerBuildOptionsAsync(resource, cancellationToken).ConfigureAwait(false);
243243

244244
// Check if this resource needs a container runtime
245245
if (await ResourcesRequireContainerRuntimeAsync([resource], cancellationToken).ConfigureAwait(false))
246246
{
247-
logger.LogDebug("Checking {ContainerRuntimeName} health", ContainerRuntime.Name);
247+
logger.LogDebug("Checking {ContainerRuntimeName} health", containerRuntime.Name);
248248

249-
var containerRuntimeHealthy = await ContainerRuntime.CheckIfRunningAsync(cancellationToken).ConfigureAwait(false);
249+
var containerRuntimeHealthy = await containerRuntime.CheckIfRunningAsync(cancellationToken).ConfigureAwait(false);
250250

251251
if (!containerRuntimeHealthy)
252252
{
253-
logger.LogError("Container runtime '{ContainerRuntimeName}' is not running or is unhealthy. Cannot build container image.", ContainerRuntime.Name);
254-
throw new InvalidOperationException($"Container runtime '{ContainerRuntime.Name}' is not running or is unhealthy.");
253+
logger.LogError("Container runtime '{ContainerRuntimeName}' is not running or is unhealthy. Cannot build container image.", containerRuntime.Name);
254+
throw new InvalidOperationException($"Container runtime '{containerRuntime.Name}' is not running or is unhealthy.");
255255
}
256256

257-
logger.LogDebug("{ContainerRuntimeName} is healthy", ContainerRuntime.Name);
257+
logger.LogDebug("{ContainerRuntimeName} is healthy", containerRuntime.Name);
258258
}
259259

260260
if (resource is ProjectResource)
@@ -421,7 +421,7 @@ private async Task<bool> ExecuteDotnetPublishAsync(IResource resource, ResolvedC
421421

422422
private async Task BuildContainerImageFromDockerfileAsync(IResource resource, DockerfileBuildAnnotation dockerfileBuildAnnotation, string imageName, ResolvedContainerBuildOptions options, CancellationToken cancellationToken)
423423
{
424-
var ContainerRuntime = await GetContainerRuntimeAsync(cancellationToken).ConfigureAwait(false);
424+
var containerRuntime = await GetContainerRuntimeAsync(cancellationToken).ConfigureAwait(false);
425425
logger.LogInformation("Building image: {ResourceName}", resource.Name);
426426

427427
// If there's a factory, generate the Dockerfile content and write it to the specified path
@@ -475,7 +475,7 @@ private async Task BuildContainerImageFromDockerfileAsync(IResource resource, Do
475475

476476
try
477477
{
478-
await ContainerRuntime.BuildImageAsync(
478+
await containerRuntime.BuildImageAsync(
479479
dockerfileBuildAnnotation.ContextPath,
480480
dockerfileBuildAnnotation.DockerfilePath,
481481
containerBuildOptions,
@@ -518,8 +518,8 @@ await ContainerRuntime.BuildImageAsync(
518518

519519
public async Task PushImageAsync(IResource resource, CancellationToken cancellationToken)
520520
{
521-
var ContainerRuntime = await GetContainerRuntimeAsync(cancellationToken).ConfigureAwait(false);
522-
await ContainerRuntime.PushImageAsync(resource, cancellationToken).ConfigureAwait(false);
521+
var containerRuntime = await GetContainerRuntimeAsync(cancellationToken).ConfigureAwait(false);
522+
await containerRuntime.PushImageAsync(resource, cancellationToken).ConfigureAwait(false);
523523
}
524524

525525
// .NET Container builds that push OCI images to a local file path do not need a runtime

0 commit comments

Comments
 (0)