Fix container image detection for aarch64#5898
Conversation
📝 Walkthrough""" WalkthroughThe change modifies the logic in the Docker image architecture check to better accommodate Docker versions that append variant suffixes (such as "linux/arm64/v8"). It also adds a logging statement to notify when an image architecture mismatch is detected and a reinstall is triggered. No public interfaces were changed. Changes
Assessment against linked issues
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
supervisor/docker/interface.py (1)
481-483: Add comments explaining ARM architecture variant handlingThe comments added are helpful but could be more informative about the specific issue being fixed.
Consider enhancing the comment to include more context:
- # It seems that newer Docker version return a variant for arm64 images. - # Make sure we match linux/arm64 and linux/arm64/v8. + # Newer Docker versions (e.g., 28.0.4) return a variant suffix for ARM64 images + # (e.g., "linux/arm64/v8" instead of just "linux/arm64"). + # Use startswith() to ensure we match both formats for compatibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
supervisor/docker/interface.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build armv7 supervisor
- GitHub Check: Build armhf supervisor
- GitHub Check: Build aarch64 supervisor
- GitHub Check: Run tests Python 3.13.3
🔇 Additional comments (1)
supervisor/docker/interface.py (1)
485-490: Well-placed logging for architecture mismatchGood addition of logging to indicate when an architecture mismatch is detected, providing clear information about what's happening and why reinstallation is triggered.
It seems that on newer Docker (e.g. 28.0.4) the container image built on the local system on an 64-bit Arm systems returns the architecture variant just like on 32-bit Arm (but v8 in this case). This is not expected by the current code. To make Supervisor detect the image correctly on new and old Docker versions, use substring matching.
fc03872 to
b2d2ba6
Compare
|
This really seems a BuildKit vs classic builder behavior difference, see moby/buildkit#4039 (comment). Along with the comment:
So this essentially changes Supervisor to be more forgiving if the Docker architecture get normalized (as in, stripping variant if the variant isn't returned normalized). |
Proposed change
It seems that on newer Docker (e.g. 28.0.4) the container image built on the local system on an 64-bit Arm systems returns the architecture variant just like on 32-bit Arm (but v8 in this case). This is not expected by the current code. To make Supervisor detect the image correctly on new and old Docker versions, use substring matching.
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit