fix: use Docker server's current API version instead of minimum#620
fix: use Docker server's current API version instead of minimum#620
Conversation
The scripts were setting DOCKER_API_VERSION to the server's minimum supported version, which could cause compatibility issues when the Docker client SDK requires a higher version. Now auto-detects the server's current API version for full compatibility, with fallback to 1.44 if detection fails.
There was a problem hiding this comment.
Pull request overview
This PR fixes Docker API version detection in shell scripts by using the server's current API version instead of its minimum supported version. This change improves compatibility when the Docker CLI requires features from newer API versions.
Changes:
- Updated
set_docker_api_version()in bothrun.shandrun_containerized.shto use{{.Server.APIVersion}}instead of{{.Server.MinAPIVersion}} - Modified fallback behavior to include architecture detection with a default fallback to version 1.44
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| run.sh | Updated Docker API version detection to use server's current version instead of minimum, with architecture-based fallback |
| run_containerized.sh | Same API version detection change as run.sh, maintaining consistency between containerized and non-containerized modes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Fallback: set based on architecture | ||
| local arch=$(uname -m) | ||
| if [ "$arch" = "arm64" ] || [ "$arch" = "aarch64" ]; then | ||
| export DOCKER_API_VERSION=1.44 | ||
| else | ||
| export DOCKER_API_VERSION=1.44 | ||
| fi |
There was a problem hiding this comment.
The fallback logic is redundant as both branches of the conditional set DOCKER_API_VERSION to the same value (1.44). The architecture check doesn't actually influence the version selection. Either remove the architecture-based conditional or set different versions for different architectures if that's the intended behavior.
| # Fallback: set based on architecture | |
| local arch=$(uname -m) | |
| if [ "$arch" = "arm64" ] || [ "$arch" = "aarch64" ]; then | |
| export DOCKER_API_VERSION=1.44 | |
| else | |
| export DOCKER_API_VERSION=1.44 | |
| fi | |
| # Fallback: set a default API version, still logging architecture | |
| local arch=$(uname -m) | |
| export DOCKER_API_VERSION=1.44 |
| # Fallback: set based on architecture | ||
| local arch=$(uname -m) |
There was a problem hiding this comment.
The fallback logic is redundant as both branches of the conditional set DOCKER_API_VERSION to the same value (1.44). The architecture check doesn't actually influence the version selection. Either remove the architecture-based conditional or set different versions for different architectures if that's the intended behavior.
See below for a potential fix:
# Fallback: use a default API version when server API cannot be detected
export DOCKER_API_VERSION=1.44
log_info "Set DOCKER_API_VERSION=$DOCKER_API_VERSION (fallback default)"
The scripts were setting DOCKER_API_VERSION to the server's minimum supported version, which could cause compatibility issues when the Docker client SDK requires a higher version.
Now auto-detects the server's current API version for full compatibility, with fallback to 1.44 if detection fails.