fix: lazy-detect container engine so Podman is used in Aspire dotnet deploys#8598
Open
vhvb1989 wants to merge 1 commit into
Open
fix: lazy-detect container engine so Podman is used in Aspire dotnet deploys#8598vhvb1989 wants to merge 1 commit into
vhvb1989 wants to merge 1 commit into
Conversation
ContainerEngine() was returning "docker" by default when CheckInstalled() had not been called. In the Aspire .NET container app deploy path, dotnetContainerAppTarget.RequiredExternalTools() only includes dotnet.Cli (not docker.Cli), so docker.Cli.CheckInstalled() was never invoked via tools.EnsureInstalled(). This meant the containerEngine field stayed empty and our -p:ContainerEngine=podman fix from #8527 never fired. ContainerEngine() now performs lightweight detection (AZD_CONTAINER_RUNTIME env var + PATH check) on first call if CheckInstalled() hasn't run yet, without validating versions or daemon readiness. Fixes #8525 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vhvb1989
requested review from
JeffreyCA,
RickWinter,
hemarina and
tg-msft
as code owners
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Aspire .NET container deployment paths where docker.Cli.CheckInstalled() isn’t invoked (because docker.Cli isn’t listed in RequiredExternalTools()), causing dotnet publish /t:PublishContainer to default to Docker even when Podman is the only runtime available. It updates docker.Cli.ContainerEngine() to perform lightweight, lazy container runtime detection so the correct -p:ContainerEngine=podman can be passed through.
Changes:
- Add lazy runtime detection to
docker.Cli.ContainerEngine()(env var + PATH checks) to inferdockervspodmaneven whenCheckInstalled()hasn’t run. - Add unit tests covering the lazy detection behavior and ensuring
CheckInstalled()results aren’t overridden.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
cli/azd/pkg/tools/docker/docker.go |
Adds lazy container engine detection so Aspire dotnet publish paths can correctly select Podman without requiring CheckInstalled() to run first. |
cli/azd/pkg/tools/docker/docker_test.go |
Adds test coverage for the new lazy-detection behavior and caching semantics when CheckInstalled() has already run. |
Comment on lines
44
to
+48
| func (d *Cli) ContainerEngine() string { | ||
| return d.getContainerEngine() | ||
| if d.containerEngine == "" { | ||
| d.detectContainerEngine() | ||
| } | ||
| return d.containerEngine |
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
hemarina
approved these changes
Jun 11, 2026
hemarina
left a comment
hemarina
left a comment
Contributor
There was a problem hiding this comment.
Nice, well-targeted fix — the root-cause analysis is accurate (dotnetContainerAppTarget.RequiredExternalTools() omits docker.Cli, so CheckInstalled() never runs and the engine silently defaults to docker), and lazy detection is a reasonable minimal solution. Tests pass locally. Approving. A couple of minor, non-blocking notes below.
1. AZD_CONTAINER_RUNTIME is validated inconsistently (low-medium)
CheckInstalled() returns a hard error on an invalid value, but detectContainerEngine() silently ignores an invalid value and falls through to PATH detection. Because the Aspire dotnet path is exactly the one where CheckInstalled() never runs, a typo (e.g. AZD_CONTAINER_RUNTIME=podmn) would be silently swallowed here instead of surfaced to the user. Consider aligning the two paths (e.g. share a small validation helper, or at least log when an explicitly-set value is unrecognized).
2. Test hermeticity (minor)
The t.Parallel() subtests in Test_ContainerEngine_LazyDetection don't clear AZD_CONTAINER_RUNTIME. If it happens to be exported in a dev/CI shell, detectContainerEngine() returns early and the PATH-based assertions would break. A t.Setenv("AZD_CONTAINER_RUNTIME", "") guard in those subtests would make them hermetic.
Informational (no action needed):
ContainerEngine()now mutates shared state (d.containerEngine) with no lock, anddocker.Cliis a singleton. This is not a race today since services are built/deployed sequentially — flagging only as a latent concern if parallel deploys are ever introduced.- Only the public
ContainerEngine()lazily detects; internal callers (Login/Build/Push) still default todockeron empty. Doesn't affect the Aspire path (dotnet publish does its own container build), so just noting the asymmetry.
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
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.
Problem
The fix in #8527 correctly threads the container engine to
dotnet publish, but it never actually detects Podman in the Aspire .NET container app deploy path.Root Cause
dotnetContainerAppTarget.RequiredExternalTools()returns only[dotNetCli]— it does not includedocker.Cli. This meansdocker.Cli.CheckInstalled()is never called viatools.EnsureInstalled()during the Aspire deploy flow. As a result,docker.Cli.containerEnginestays""andContainerEngine()defaults to"docker"— so-p:ContainerEngine=podmanis never appended.Fix
ContainerEngine()now performs lazy detection on first call ifCheckInstalled()hasn't run yet:AZD_CONTAINER_RUNTIMEenv vardockeris in PATHpodmanif in PATHdockerif neither found (backward compatible;CheckInstalled()will error later if actually needed)This is a lightweight check — no version validation or daemon readiness — just enough to determine which engine to pass to
dotnet publish.Testing
5 new test cases for the lazy detection path:
AZD_CONTAINER_RUNTIMEenv var respectedCheckInstalled()result if already calledAll existing docker, dotnet, and project tests pass.
Fixes #8525
Related: #8527, microsoft/aspire#17593