-
Notifications
You must be signed in to change notification settings - Fork 756
Fix: Skip container runtime check for Archive destinations and improve error messages #13576
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
- Updated ResourcesRequireContainerRuntimeAsync to skip runtime check for resources with Archive destination - Added runtime check to BuildImageAsync for Dockerfile resources - Updated FakeContainerRuntime test helper to support isRunning parameter - Added tests to verify the behavior (3 of 4 passing, 1 test needs refinement) Co-authored-by: captainsafia <[email protected]>
…ibility Reordered logic to check Dockerfile resources first, then Archive destination, preserving the original behavior for OCI + OutputPath combinations. Co-authored-by: captainsafia <[email protected]>
- Move container runtime check to beginning of BuildImageAsync for all resource types - Optimize ResourcesRequireContainerRuntimeAsync to call ResolveContainerBuildOptionsAsync once - Only check Destination when OutputPath is present to avoid unnecessary callback Co-authored-by: captainsafia <[email protected]>
| // Skip resources that are explicitly configured to save as archives - they don't need Docker | ||
| if (options.OutputPath is not null) | ||
| { | ||
| var buildOptionsContext = await resource.ProcessContainerBuildOptionsCallbackAsync( |
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 Don't recompute buildOptionsContext here. Add Destination to ResolvedContainerBuildOptions and update ResolveContainerBuildOptionsAsync to set it. Then check for the value directly on the ResolvedContainerBuildOptions options object here.
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.
Done in commit 82bdd4b. Added Destination property to ResolvedContainerBuildOptions, set it in ResolveContainerBuildOptionsAsync, and now checking it directly in ResourcesRequireContainerRuntimeAsync without recomputing buildOptionsContext.
- Added Destination property to ResolvedContainerBuildOptions class - Set Destination in ResolveContainerBuildOptionsAsync from context - Use Destination directly in ResourcesRequireContainerRuntimeAsync instead of calling ProcessContainerBuildOptionsCallbackAsync again - Simplifies logic and avoids redundant callback invocations Co-authored-by: captainsafia <[email protected]>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13576Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13576" |
|
|
||
| if (!containerRuntimeHealthy) | ||
| { | ||
| logger.LogError("Container runtime is not running or is unhealthy. Cannot build container image."); |
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.
| logger.LogError("Container runtime is not running or is unhealthy. Cannot build container image."); | |
| logger.LogError("Container runtime '{ContainerRuntimeName}' is not running or is unhealthy. Cannot build container image.", ContainerRuntime.Name); |
Include the container runtime name here and in the exception below.
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.
Done in commit e25852e. Updated both the log error message and exception message to include the container runtime name. Also updated test assertions to work with the new format.
- Updated error log and exception messages to include container runtime name
- Changed from "Container runtime is not running" to "Container runtime '{name}' is not running"
- Updated test assertions to match new error message format
- Improves error diagnostics by showing which runtime (docker/podman) is being checked
Co-authored-by: captainsafia <[email protected]>
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 fixes an issue where aspire deploy fails with a cryptic error message when Docker isn't running, even when container images are configured to save as archives (which don't require Docker). The changes improve error messages by including the container runtime name and optimize the runtime check logic to skip validation for Archive destinations.
Key Changes
- Added logic to skip container runtime checks for resources with
ContainerImageDestination.Archive - Improved error messages to include the specific container runtime name (e.g., "Container runtime 'docker' is not running")
- Added runtime health check to
BuildImageAsyncmethod to catch issues earlier for individual resource builds
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Aspire.Hosting/Publishing/ResourceContainerImageManager.cs | Added Destination property to ResolvedContainerBuildOptions, updated ResourcesRequireContainerRuntimeAsync to skip runtime checks for Archive destinations, improved error messages with runtime name, and added runtime check to BuildImageAsync |
| tests/Aspire.Hosting.Tests/Publishing/FakeContainerRuntime.cs | Added isRunning parameter and CheckIfRunningCallCount tracking to support testing runtime check scenarios |
| tests/Aspire.Hosting.Tests/Publishing/ResourceContainerImageManagerTests.cs | Added comprehensive test coverage for Archive/Registry/Dockerfile scenarios, mixed destinations, and updated existing test assertions to match new error message format |
| var options = await ResolveContainerBuildOptionsAsync(resource, cancellationToken).ConfigureAwait(false); | ||
|
|
||
| // Check if this resource needs a container runtime | ||
| if (await ResourcesRequireContainerRuntimeAsync([resource], cancellationToken).ConfigureAwait(false)) |
Copilot
AI
Dec 16, 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 ResolveContainerBuildOptionsAsync method is being called twice in BuildImageAsync - once at line 239 and again inside ResourcesRequireContainerRuntimeAsync at line 531. This is inefficient since the options are already resolved and available. Consider passing the already-resolved options to ResourcesRequireContainerRuntimeAsync to avoid redundant callback invocations, or move the runtime check before the options resolution to avoid the duplicate work.
|
/backport to release/13.1 |
|
Started backporting to release/13.1: https://github.com/dotnet/aspire/actions/runs/20252173091 |
Description
When using
aspire deploywith Azure Container Apps, deployment fails with cryptic error "CONTAINER1012: The local registry is not available" if Docker isn't running, even when images are configured to save as archives rather than push to registries.Changes
ResourceContainerImageManager.cs
BuildImageAsync: Moved container runtime health check to beginning of method, applying to all resource types (not just Dockerfile builds)ResolvedContainerBuildOptions: AddedDestinationproperty to track the container image destinationResolveContainerBuildOptionsAsync: Now setsDestinationproperty from the callback contextResourcesRequireContainerRuntimeAsync:ContainerImageDestination.ArchivesetDestinationdirectly fromResolvedContainerBuildOptionsto avoid redundant callback invocationsResolveContainerBuildOptionsAsynconce per resourceTest Infrastructure
FakeContainerRuntime: AddisRunningparameter andCheckIfRunningCallCounttrackingExample
Checklist
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.