[release/13.1] Fix: Skip container runtime check for Archive destinations and improve error messages #13580
+12
−218
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Removed Destination property from ResolvedContainerBuildOptions - Reverted ResourcesRequireContainerRuntimeAsync to original logic - Removed 5 tests that used ContainerImageDestination enum - Reverted FakeContainerRuntime test helper changes - Kept error message improvements with container runtime name Co-authored-by: captainsafia <[email protected]>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
[WIP] Fix skip container runtime check for Archive destinations
[release/13.1] Fix: Skip container runtime check for Archive destinations and improve error messages
Dec 16, 2025
captainsafia
merged commit bfed158
into
backport/pr-13576-to-release/13.1
2 of 3 checks passed
joperezr
pushed a commit
that referenced
this pull request
Dec 16, 2025
…image builds (#13579) * Initial plan * Implement fix for container runtime check with Archive destinations - 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]> * Fix ResourcesRequireContainerRuntimeAsync to maintain backward compatibility Reordered logic to check Dockerfile resources first, then Archive destination, preserving the original behavior for OCI + OutputPath combinations. Co-authored-by: captainsafia <[email protected]> * Address PR feedback: optimize runtime checks and move to BuildImageAsync - 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]> * Add Destination to ResolvedContainerBuildOptions to avoid recomputing - 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]> * Include container runtime name in error messages - 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]> * [release/13.1] Fix: Skip container runtime check for Archive destinations and improve error messages (#13580) * Initial plan * Remove ContainerImageDestination related parts as requested - Removed Destination property from ResolvedContainerBuildOptions - Reverted ResourcesRequireContainerRuntimeAsync to original logic - Removed 5 tests that used ContainerImageDestination enum - Reverted FakeContainerRuntime test helper changes - Kept error message improvements with container runtime name Co-authored-by: captainsafia <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: captainsafia <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: captainsafia <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Backport of #13576 to release/13.1, modified to remove
ContainerImageDestinationAPI dependencies not present in this branch.Changes
BuildImageAsync: Added runtime health check for individual resource builds to catch issues early in single-image scenariosContainerImageDestination-dependent optimizations that relied on APIs not available in release/13.1The runtime check logic reverts to the original approach: check if resources use Docker format or lack output paths, without attempting to detect Archive destinations.
Checklist
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.