Conversation
WalkthroughThe install script tolerates failures when removing old Docker packages, ensures a usable XDG_RUNTIME_DIR for rootless Docker, and switches per-user Docker service commands from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant S as install_docker.sh
participant A as apt-get
participant ENV as shell env
participant SC as systemctl (sudo)
U->>S: invoke install_docker.sh
rect rgba(220,235,255,0.25)
note right of S: Pre-install cleanup (tolerant)
S->>A: apt-get remove pkg1 || true
S->>A: apt-get remove pkgN || true
end
rect rgba(220,255,220,0.18)
note right of S: Ensure runtime dir for rootless Docker
S->>ENV: export XDG_RUNTIME_DIR=${XDG_RUNTIME_DIR:-"/run/user/$(id -u)"}
end
rect rgba(255,240,200,0.18)
note right of S: Per-user Docker service (rootless)
S->>SC: sudo systemctl enable docker
S->>SC: sudo systemctl restart docker
note right of SC: Replaces `systemctl --user` calls
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
helpers/install_docker.sh (1)
9-9: Don’t blanket-suppress apt errors; skip only when package is absent.
|| truehides genuine failures (e.g., dpkg lock), not just “package not installed.” Prefer gating the removal on package presence.-for pkg in docker.io docker-doc docker-compose docker-compose-v2 podman-docker containerd runc; do sudo apt-get remove -y $pkg || true; done +for pkg in docker.io docker-doc docker-compose docker-compose-v2 podman-docker containerd runc; do + if dpkg -s "$pkg" >/dev/null 2>&1; then + sudo apt-get remove -y "$pkg" + fi +done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
helpers/install_docker.sh(2 hunks)
🔇 Additional comments (1)
helpers/install_docker.sh (1)
55-60: Align messaging with behavior.Given the fix above uses
systemctl --userfor rootless mode, this message is accurate. If you keep any system‑level enable/start in the non‑root path, it becomes contradictory. Please verify final behavior is:
- root: enable/start system service
- non‑root: enable/start user service only
Summary by CodeRabbit