feat: Add support for custom llama.cpp base image and forked llama-swap repositories#396
feat: Add support for custom llama.cpp base image and forked llama-swap repositories#396mostlygeek merged 2 commits intomostlygeek:mainfrom ryan-steed-usa:flexible-container-build
Conversation
WalkthroughReplaces hardcoded base image and repository references with configurable ARGs/variables in the Docker build script and Containerfile; adds build-arg propagation, non-root user setup (UID, GID, USER_HOME, HOME), HEALTHCHECK, and ENTRYPOINT to the container image. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 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: 1
🧹 Nitpick comments (1)
docker/build-container.sh (1)
54-54: Consider passing BASE_IMAGE for complete flexibility.The Containerfile defines ARG BASE_IMAGE with a default, but the build script doesn't pass it as a build-arg. While this works due to the default value, adding
--build-arg BASE_IMAGE=${BASE_IMAGE:-ghcr.io/ggml-org/llama.cpp}would provide full control over the base image via the script, completing the flexibility improvements from this PR.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/release.yml(2 hunks).goreleaser.yaml(2 hunks)docker/build-container.sh(2 hunks)docker/llama-swap.Containerfile(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ryan-steed-usa
Repo: mostlygeek/llama-swap PR: 391
File: docker/build-container.sh:29-32
Timestamp: 2025-11-08T16:56:35.618Z
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.
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-11-08T16:56:35.618Z
Learnt from: ryan-steed-usa
Repo: mostlygeek/llama-swap PR: 391
File: docker/build-container.sh:29-32
Timestamp: 2025-11-08T16:56:35.618Z
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.shdocker/llama-swap.Containerfile
📚 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.shdocker/llama-swap.Containerfile
⏰ 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). (2)
- GitHub Check: run-tests
- GitHub Check: run-tests
🔇 Additional comments (5)
.github/workflows/release.yml (1)
21-22: LGTM! GO111MODULE configuration aligns with goreleaser.The environment variable setup correctly uses the repository variable with a safe 'auto' fallback, coordinating with the .goreleaser.yaml configuration to ensure consistent Go module handling during releases.
docker/build-container.sh (1)
23-24: LGTM! Repository parameterization implemented correctly.The LS_REPO variable is well-designed, sourcing from GITHUB_REPOSITORY with a sensible default that maintains backward compatibility.
docker/llama-swap.Containerfile (2)
1-3: LGTM! Base image parameterization enhances flexibility.The BASE_IMAGE and BASE_TAG arguments with appropriate defaults enable custom base images while maintaining backward compatibility.
7-7: LGTM! Repository parameterization supports forked releases.The LS_REPO argument correctly enables downloading llama-swap binaries from custom repositories, aligning with the PR's goal of supporting forked builds.
.goreleaser.yaml (1)
3-4: LGTM! GO111MODULE template configured correctly.The goreleaser template syntax properly checks for the environment variable and provides a sensible 'auto' default, coordinating with the workflow configuration to enable flexible Go module mode handling for custom builds.
…ap repositories - Introduce BASE_LLAMACPP_IMAGE env var to customize llama.cpp base image - Introduce LS_REPO env var to customize llama-swap source - Use GITHUB_REPOSITORY env var to automatically detect forked repos - Update container tagging to use dynamic repo paths - Pass build args for BASE_IMAGE and LS_REPO to Containerfile - Enable flexible release downloads from forked repositories
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docker/llama-swap.Containerfile (1)
31-35: Optional: quote the full curl URL for readability and robustness.Current command works, but quoting the entire URL is a bit clearer and future-proof against any odd characters:
- curl -LO https://github.com/"${LS_REPO}"/releases/download/v"${LS_VER}"/llama-swap_"${LS_VER}"_linux_amd64.tar.gz && \ + curl -LO "https://github.com/${LS_REPO}/releases/download/v${LS_VER}/llama-swap_${LS_VER}_linux_amd64.tar.gz" && \
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docker/build-container.sh(3 hunks)docker/llama-swap.Containerfile(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docker/build-container.sh
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
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
📚 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/llama-swap.Containerfile
🔇 Additional comments (1)
docker/llama-swap.Containerfile (1)
1-7: Configurable base image and repo args look correct and backward-compatible.Using
ARG BASE_IMAGE/BASE_TAGinFROMandARG LS_REPO=mostlygeek/llama-swappreserves prior defaults while allowing forks and alternate base images via build-args. No issues spotted here.
|
LGTM |
…ap repositories (mostlygeek#396) * feat: Add support for custom llama.cpp base image and forked llama-swap repositories - Introduce BASE_LLAMACPP_IMAGE env var to customize llama.cpp base image - Introduce LS_REPO env var to customize llama-swap source - Use GITHUB_REPOSITORY env var to automatically detect forked repos - Update container tagging to use dynamic repo paths - Pass build args for BASE_IMAGE and LS_REPO to Containerfile - Enable flexible release downloads from forked repositories * chore: quote entire curl options, appease coderabbitai
Uh oh!
There was an error while loading. Please reload this page.