Fallback to Podman if Docker is not installed #22
Conversation
WalkthroughThe init logic now falls back to Podman when Docker is missing, stores the chosen runtime in a Changes
Sequence DiagramsequenceDiagram
autonumber
participant Init as Initialization
participant CheckDocker as Check `docker`
participant CheckPodman as Check `podman`
participant Config as Config (set `dockerCmd`)
participant Runtime as Runtime (engine/compose)
Init->>CheckDocker: look for `docker` in PATH
alt docker found
CheckDocker-->>Config: set `dockerCmd = "docker"`
else docker not found
Init->>CheckPodman: look for `podman` in PATH
alt podman found
CheckPodman-->>Config: set `dockerCmd = "podman"`
Note right of Init: log notice about Podman
else neither found
Init->>Init: fatal error (no container runtime)
end
end
Config->>Runtime: use `dockerCmd` for engine & compose checks
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/internal/docker.go (1)
84-96: Critical: docker-compose fallback breaks Podman supportThe docker-compose script fallback at Line 91 will override
dockerCmdeven when it was set to "podman", breaking the Podman fallback feature entirely. The logic needs to be restructured to handle this properly.Based on the PR description mentioning "Podman must be configured in Docker compatibility mode", it seems the compose plugin is expected. Consider one of these approaches:
Option 1: Only allow docker-compose fallback when using Docker
// Check for the ``compose`` plugin as our first choice _, composeErr := RunBasicCmd(dockerCmd, []string{"compose", "version"}) if composeErr != nil { - fmt.Println("[+] The `compose` plugin is not installed, so we'll try the deprecated `docker-compose` script") - composeScriptExists := CheckPath("docker-compose") - if composeScriptExists { - fmt.Println("[+] The `docker-compose` script is installed, so we'll use that instead") - dockerCmd = "docker-compose" - } else { - fmt.Println("[+] The `docker-compose` script is also not installed or not in the PATH") - log.Fatalln("Docker Compose is not installed, so please install it and try again: https://docs.docker.com/compose/install/") + if dockerCmd == "docker" { + fmt.Println("[+] The `compose` plugin is not installed, so we'll try the deprecated `docker-compose` script") + composeScriptExists := CheckPath("docker-compose") + if composeScriptExists { + fmt.Println("[+] The `docker-compose` script is installed, so we'll use that instead") + dockerCmd = "docker-compose" + } else { + fmt.Println("[+] The `docker-compose` script is also not installed or not in the PATH") + log.Fatalln("Docker Compose is not installed, so please install it and try again: https://docs.docker.com/compose/install/") + } + } else { + // Podman requires the compose plugin + log.Fatalln("Podman requires the compose plugin. Please install podman-compose or configure Podman with Docker compatibility: https://podman.io/") } }Option 2: Check for podman-compose when using Podman
This would require checking if podman-compose exists as a separate tool when dockerCmd is "podman".
🧹 Nitpick comments (1)
cmd/internal/utils_test.go (1)
16-20: Consider renaming variable for clarityThe test logic correctly validates that either docker or podman is available in PATH. The variable name
dockerFoundis slightly misleading since it might actually be finding podman. Consider renaming toruntimeFoundorcontainerRuntimeFoundfor better clarity.- dockerFound := CheckPath("docker") - if !dockerFound { - dockerFound = CheckPath("podman") + runtimeFound := CheckPath("docker") + if !runtimeFound { + runtimeFound = CheckPath("podman") } - assert.True(t, dockerFound, "Expected `CheckPath()` to find `docker` or `podman` in `$PATH`") + assert.True(t, runtimeFound, "Expected `CheckPath()` to find `docker` or `podman` in `$PATH`")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/internal/docker.go(3 hunks)cmd/internal/utils.go(2 hunks)cmd/internal/utils_test.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
cmd/internal/docker.go (1)
cmd/internal/utils.go (2)
CheckPath(152-155)RunBasicCmd(159-163)
cmd/internal/utils_test.go (1)
cmd/internal/utils.go (1)
CheckPath(152-155)
🔇 Additional comments (8)
cmd/internal/utils.go (2)
18-19: LGTM: Import reorganizationThe import block reorganization is a formatting-only change with no semantic impact.
168-169: LGTM: Podman support in RunCmdThe extension to handle both "docker" and "podman" is correct and necessary for the Podman fallback feature. Both runtimes use the compose subcommand in the same way.
cmd/internal/utils_test.go (1)
6-7: LGTM: Import reorganizationThe import block reorganization is a formatting-only change with no semantic impact.
cmd/internal/docker.go (5)
10-12: LGTM: Import reorganizationThe import block reorganization is a formatting-only change with no semantic impact.
24-24: LGTM: Documentation improvementGood addition documenting the Podman fallback behavior for the
dockerCmdvariable.
79-79: LGTM: Correct usage of dockerCmd variableGood use of the
dockerCmdvariable instead of hardcoded "docker", enabling Podman support.
334-358: Verify Podman compatibility with Docker client libraryThe
GetRunning()function also uses the Docker client library. Same verification and documentation considerations apply as mentioned forFetchLogs().
289-331: Confirm Podman Docker compatibility setup is documentedThe code correctly uses
client.FromEnvandclient.WithAPIVersionNegotiation(), which are compatible with Podman's Docker API layer when properly configured. However, Podman's Docker-API compatibility layer requirespodman system serviceto be enabled and is not 100% feature-complete.This code will work with Podman, but users need explicit setup instructions. Add documentation or an error message clarifying the Podman configuration requirement—particularly that the Docker-compatible socket must be accessible (either via
/var/run/docker.sockorDOCKER_HOSTenvironment variable).
|
Thanks fr the submission! I reviewed the changes and made some adjustments to log messages and the README for a release. I'll merge this for v0.1.9. |
|
Thanks @chrismaddalena! |
This PR enables bloodhound-cli to use Podman as a backup if Docker is not available. The main driver is recent license changes to Docker Desktop that restrict its use within commercial environments. Podman must be configured in Docker compatibility mode for it to be a full drop-in replacement for bloodhound-cli.
Summary by CodeRabbit