fix: update containerfile user/group management commands#379
fix: update containerfile user/group management commands#379mostlygeek merged 1 commit intomostlygeek:mainfrom ryan-steed-usa:dockerfile-fix-user-group-creation
Conversation
- Replace `addgroup` with `groupadd` for system group creation - Replace `adduser` with `useradd` for system user creation - Maintain same functionality while using more standard POSIX commands
WalkthroughThe Containerfile replaces Alpine Linux user/group management commands with standard GNU equivalents: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (1)
docker/llama-swap.Containerfile (1)
18-19: Consider explicitly adding-Mflag for clarity.The migration to
useraddis correct and addresses the Ubuntu 24.04 compatibility issue. However, the removal of the--no-create-homeflag could lead to different behavior depending on the base image's/etc/login.defsconfiguration (specifically theCREATE_HOMEsetting).While
--systemtypically prevents home directory creation, explicitly adding-Mor--no-create-homewould better match the original intent and ensure consistent behavior across all base images.Apply this diff to add explicit home directory prevention:
- useradd --system --uid $UID --gid $GID \ + useradd --system -M --uid $UID --gid $GID \ --home $USER_HOME app; \Note: This is optional since line 23 explicitly creates the directories with
mkdir --parents, which will succeed regardless.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docker/llama-swap.Containerfile(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ryan-steed-usa
Repo: mostlygeek/llama-swap PR: 368
File: docker/llama-swap.Containerfile:14-20
Timestamp: 2025-10-27T16:04:23.381Z
Learning: In docker/llama-swap.Containerfile, the user/group creation logic intentionally supports adding a non-root user (non-zero UID) to the root group (GID=0) as a valid configuration. This allows flexible UID/GID customization where users can specify only a UID or both UID and GID.
📚 Learning: 2025-10-27T16:04:23.381Z
Learnt from: ryan-steed-usa
Repo: mostlygeek/llama-swap PR: 368
File: docker/llama-swap.Containerfile:14-20
Timestamp: 2025-10-27T16:04:23.381Z
Learning: In docker/llama-swap.Containerfile, the user/group creation logic intentionally supports adding a non-root user (non-zero UID) to the root group (GID=0) as a valid configuration. This allows flexible UID/GID customization where users can specify only a UID or both UID and GID.
Applied to files:
docker/llama-swap.Containerfile
🔇 Additional comments (1)
docker/llama-swap.Containerfile (1)
16-16: LGTM! Correct migration to standard POSIX command.The change from
addgrouptogroupaddcorrectly addresses Ubuntu 24.04 compatibility while maintaining the same functionality.groupaddis the standard command available across distributions.
|
Closes #380 |
This should fix the
vulkancontainer build failures. The upstream vulkan containers use Ubuntu 24.04 and the others are still on 22.04. Apparently theadduserpackage is no longer installed by default.addgroupwithgroupaddfor system group creationadduserwithuseraddfor system user creationSummary by CodeRabbit