feat: build both root and non-root container images#412
feat: build both root and non-root container images#412mostlygeek merged 1 commit intomostlygeek:mainfrom ryan-steed-usa:add-root-container-builds
Conversation
WalkthroughModified the build container script to implement a dual-build workflow. The script now iterates over both non-root and root container types, applying type-specific user context settings (UID, GID, USER_HOME) and tagging conventions, then builds and conditionally pushes both variants instead of a single container. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Script as build-container.sh
participant Docker
participant Registry
User->>Script: Execute build script
activate Script
loop For each container type (non-root, root)
Note over Script: Initialize type-specific UID, GID, USER_HOME
Note over Script: Set tags with -non-root suffix (if non-root type)
Script->>Docker: docker build with type-specific build-args
activate Docker
Docker-->>Script: Image built
deactivate Docker
alt if push flag enabled
Script->>Registry: docker push all tags for variant
activate Registry
Registry-->>Script: Push complete
deactivate Registry
end
end
deactivate Script
Script-->>User: Build process complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docker/build-container.sh (1)
64-65: Consider adding error handling for docker build failures.If the
docker buildcommand fails, the script continues to the push phase, which could lead to pushing stale or non-existent images. Consider adding a check to exit or skip the push if the build fails.Apply this diff to add error handling:
echo "Building $CONTAINER_TYPE $CONTAINER_TAG $LS_VER" docker build -f llama-swap.Containerfile --build-arg BASE_TAG=${BASE_TAG} --build-arg LS_VER=${LS_VER} --build-arg UID=${USER_UID} \ --build-arg GID=${USER_GID} --build-arg USER_HOME=${USER_HOME} -t ${CONTAINER_TAG} -t ${CONTAINER_LATEST} . + if [ $? -ne 0 ]; then + echo "Error: Failed to build $CONTAINER_TYPE container" + exit 1 + fi if [ "$PUSH_IMAGES" == "true" ]; then
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docker/build-container.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 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/build-container.sh
📚 Learning: 2025-11-08T16:56:35.626Z
Learnt from: ryan-steed-usa
Repo: mostlygeek/llama-swap PR: 391
File: docker/build-container.sh:29-32
Timestamp: 2025-11-08T16:56:35.626Z
Learning: In the mostlygeek/llama-swap repository, llama.cpp container tags from ghcr.io/ggml-org/llama.cpp use build identifiers (e.g., `b6981`) rather than semantic versions. The tag format is `server-<something>-<build-id>` for cpu and `server-<arch>-<something>-<build-id>` for other architectures, where extracting the third field with `awk -F '-' '{print $3}'` correctly retrieves the build identifier.
Applied to files:
docker/build-container.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-tests
🔇 Additional comments (1)
docker/build-container.sh (1)
48-70: Dual-build workflow implementation looks solid.The loop structure correctly iterates over both container types, sets appropriate user context (UID, GID, USER_HOME) for each variant, applies consistent tagging conventions (versioned and latest tags with "-non-root" suffix for the non-root type), and builds/pushes both variants. The build-arg passing aligns with the Containerfile's support for flexible user/group customization. Based on learnings, this approach enables the intended behavior of building root and non-root container images simultaneously.
|
@mostlygeek FYI, upstream vulkan builds appear to be broken. The last available build is old enough that the API call we run doesn't find it. I had to use this temporary fix on my branch: elif [ "$ARCH" == "vulkan" ]; then
# TEMPORARY HACK DUE TO LAPSED VULKAN BUILDS
LCPP_TAG=b7131
BASE_TAG=server-${ARCH}-${LCPP_TAG}
else |
2 participants
Restore default root container builds but add additional non-root container tags.
Summary by CodeRabbit
-non-rootsuffix for easy identification. Each variant is built with appropriate user context settings and includes per-type build logging.✏️ Tip: You can customize this high-level summary in your review settings.