LCORE-1859: Fix llama-stack startup as non-root user#1859
Open
CodeHex083 wants to merge 1 commit into
Open
Conversation
Contributor
WalkthroughThis pull request refactors how RAG kvstore seed data is mounted and initialized in Docker containers to support non-root user execution. Instead of mounting seed data directly into writable storage, it moves to read-only seed mounts with entrypoint-based copying logic, applied across both ChangesRAG Seed Mounting and Entrypoint Seeding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose-library.yaml`:
- Around line 26-28: The SELinux mount label casing is inconsistent between the
two volume entries ("./tests/e2e/rag:/opt/app-root/rag-seed:ro,Z" vs
"./scripts/lightspeed-stack-entrypoint.sh:/app-root/lightspeed-stack-entrypoint.sh:ro,z");
pick one casing and make both mounts consistent (either change the RAG seed
mount to use :ro,z or the entrypoint script to :ro,Z) unless the differing
semantics (private vs shared) are intentional—update the line containing
"./tests/e2e/rag:/opt/app-root/rag-seed:ro,Z" or the line containing
"./scripts/lightspeed-stack-entrypoint.sh:/app-root/lightspeed-stack-entrypoint.sh:ro,z"
accordingly.
In `@scripts/lightspeed-stack-entrypoint.sh`:
- Around line 16-20: When $RAG_SEED_DIR exists, check whether any .db files
actually exist before running cp to avoid the cryptic "cannot stat" error; test
the glob (e.g. set -- "$RAG_SEED_DIR"/*.db and verify [ -e "$1" ] or loop to
detect files) and if none are found, emit a clear error/warning mentioning
RAG_SEED_DIR and skip or exit instead of running cp into STORAGE_RAG_DIR; update
the block referencing RAG_SEED_DIR and STORAGE_RAG_DIR and the cp invocation
accordingly.
In `@scripts/llama-stack-entrypoint.sh`:
- Around line 18-24: The RAG seeding block in scripts/llama-stack-entrypoint.sh
is duplicated in scripts/lightspeed-stack-entrypoint.sh; extract that logic into
a shared function (e.g., seed_rag_kvstore) in a new script (e.g.,
scripts/seed-rag-kvstore.sh) and then source and call seed_rag_kvstore from both
entrypoint scripts to remove duplication; ensure the function uses the same
environment variables (RAG_SEED_DIR, STORAGE_RAG_DIR), checks that the seed
directory exists and contains .db files before copying, emits the same echo
messages, and returns non-zero on fatal errors so both llama-stack-entrypoint.sh
and lightspeed-stack-entrypoint.sh simply call seed_rag_kvstore to perform the
seeding.
- Around line 20-24: The seed-copy block that checks RAG_SEED_DIR and runs cp
may fail with a cryptic error when there are no *.db files; update the shell
snippet that uses RAG_SEED_DIR and STORAGE_RAG_DIR to validate that at least one
.db file exists before running cp, e.g., test the glob (for f in
"$RAG_SEED_DIR"/*.db; do [ -e "$f" ] && break; done) and if no files found print
a clear error/warning and skip or exit, otherwise proceed to mkdir -p
"$STORAGE_RAG_DIR" and cp the files; ensure the message mentions the missing .db
files and uses the same variable names (RAG_SEED_DIR, STORAGE_RAG_DIR) so it’s
easy to locate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9e16fb92-3b33-4064-8f0d-df37304d1c14
📒 Files selected for processing (4)
docker-compose-library.yamldocker-compose.yamlscripts/lightspeed-stack-entrypoint.shscripts/llama-stack-entrypoint.sh
📜 Review details
⏰ 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). (7)
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-05-20T08:09:36.724Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: src/client.py:104-108
Timestamp: 2026-05-20T08:09:36.724Z
Learning: In the lightspeed-stack repo, the synthesized `run.yaml` file handling in `src/client.py` (`_synthesize_library_config`) uses a fixed `/tmp` path intentionally in the PoC (PR `#1580`). The durable production requirements are tracked in spec doc R10 (docs/design/llama-stack-config-merge/llama-stack-config-merge.md): persistent known path overwritten each boot, file mode 0600 set via explicit create flag (not umask), and a `--synthesized-config-output` CLI flag for debugging. The PoC code is scheduled for removal pre-merge; the implementation JIRA "Unified llama_stack.config schema + synthesizer" inherits R10's requirements.
Applied to files:
docker-compose-library.yamldocker-compose.yamlscripts/llama-stack-entrypoint.sh
📚 Learning: 2025-08-18T10:56:55.349Z
Learnt from: matysek
Repo: lightspeed-core/lightspeed-stack PR: 292
File: pyproject.toml:0-0
Timestamp: 2025-08-18T10:56:55.349Z
Learning: The lightspeed-stack project intentionally uses a "generic image" approach, bundling many dependencies directly in the base runtime image to work for everyone, rather than using lean base images with optional dependency groups.
Applied to files:
docker-compose-library.yaml
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.
Applied to files:
docker-compose-library.yamldocker-compose.yaml
📚 Learning: 2026-05-12T15:14:34.788Z
Learnt from: syedriko
Repo: lightspeed-core/lightspeed-stack PR: 1727
File: scripts/konflux_requirements.sh:9-15
Timestamp: 2026-05-12T15:14:34.788Z
Learning: In this repo, the `.konflux/` directory is committed/tracked and is guaranteed to exist in a fresh clone. Therefore, shell scripts that write output under `.konflux/` (e.g., create files like `.konflux/<...>`) should not waste effort by calling `mkdir -p .konflux` first. Only add directory-creation logic if the script may run in an environment/repo state where `.konflux/` might not be present.
Applied to files:
scripts/lightspeed-stack-entrypoint.shscripts/llama-stack-entrypoint.sh
🔇 Additional comments (3)
docker-compose.yaml (1)
22-25: LGTM!scripts/lightspeed-stack-entrypoint.sh (1)
22-25: LGTM!docker-compose-library.yaml (1)
23-26: RAG storage permissions for UID 1001 are already set in the image
deploy/lightspeed-stack/Containerfilecreates/opt/app-root/src/.llama/storageandchown -R 1001:1001 /opt/app-root/src/.llama, and the image setsUSER 1001; this makes/opt/app-root/src/.llama/storage/ragwritable in filesystem-backed library mode.
Comment on lines
+26
to
+28
| - ./tests/e2e/rag:/opt/app-root/rag-seed:ro,Z | ||
| # Host copy so `docker compose up` picks up script changes without rebuilding | ||
| - ./scripts/lightspeed-stack-entrypoint.sh:/app-root/lightspeed-stack-entrypoint.sh:ro,z |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Inconsistent SELinux label casing between mounts.
The RAG seed mount uses :ro,Z (private label) while the entrypoint script mount uses :ro,z (shared label). For consistency and clarity, consider using the same casing unless the different labeling is intentional.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose-library.yaml` around lines 26 - 28, The SELinux mount label
casing is inconsistent between the two volume entries
("./tests/e2e/rag:/opt/app-root/rag-seed:ro,Z" vs
"./scripts/lightspeed-stack-entrypoint.sh:/app-root/lightspeed-stack-entrypoint.sh:ro,z");
pick one casing and make both mounts consistent (either change the RAG seed
mount to use :ro,z or the entrypoint script to :ro,Z) unless the differing
semantics (private vs shared) are intentional—update the line containing
"./tests/e2e/rag:/opt/app-root/rag-seed:ro,Z" or the line containing
"./scripts/lightspeed-stack-entrypoint.sh:/app-root/lightspeed-stack-entrypoint.sh:ro,z"
accordingly.
Comment on lines
+16
to
+20
| if [ -d "$RAG_SEED_DIR" ]; then | ||
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | ||
| mkdir -p "$STORAGE_RAG_DIR" | ||
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | ||
| fi |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Improve error handling for missing seed files.
If $RAG_SEED_DIR exists but contains no *.db files, the glob pattern will fail to expand and cp will exit with a cryptic error (cp: cannot stat '.../*.db': No such file or directory). Consider adding explicit validation to provide a clearer error message.
🛡️ Proposed fix to add validation
if [ -d "$RAG_SEED_DIR" ]; then
echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..."
mkdir -p "$STORAGE_RAG_DIR"
+ if ! compgen -G "$RAG_SEED_DIR/*.db" > /dev/null; then
+ echo "ERROR: No .db files found in $RAG_SEED_DIR"
+ exit 1
+ fi
cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if [ -d "$RAG_SEED_DIR" ]; then | |
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | |
| mkdir -p "$STORAGE_RAG_DIR" | |
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | |
| fi | |
| if [ -d "$RAG_SEED_DIR" ]; then | |
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | |
| mkdir -p "$STORAGE_RAG_DIR" | |
| if ! compgen -G "$RAG_SEED_DIR/*.db" > /dev/null; then | |
| echo "ERROR: No .db files found in $RAG_SEED_DIR" | |
| exit 1 | |
| fi | |
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/lightspeed-stack-entrypoint.sh` around lines 16 - 20, When
$RAG_SEED_DIR exists, check whether any .db files actually exist before running
cp to avoid the cryptic "cannot stat" error; test the glob (e.g. set --
"$RAG_SEED_DIR"/*.db and verify [ -e "$1" ] or loop to detect files) and if none
are found, emit a clear error/warning mentioning RAG_SEED_DIR and skip or exit
instead of running cp into STORAGE_RAG_DIR; update the block referencing
RAG_SEED_DIR and STORAGE_RAG_DIR and the cp invocation accordingly.
Comment on lines
+18
to
+24
| RAG_SEED_DIR="${RAG_SEED_DIR:-/opt/app-root/rag-seed}" | ||
| STORAGE_RAG_DIR="${STORAGE_RAG_DIR:-/opt/app-root/src/.llama/storage/rag}" | ||
| if [ -d "$RAG_SEED_DIR" ]; then | ||
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | ||
| mkdir -p "$STORAGE_RAG_DIR" | ||
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | ||
| fi |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider extracting duplicated seeding logic.
The RAG seeding block (lines 18-24) is identical to the logic in scripts/lightspeed-stack-entrypoint.sh (lines 14-20). Consider extracting this into a shared function or sourced script to reduce duplication and ensure consistency.
♻️ Example: Extract to a shared function
Create a shared script (e.g., scripts/seed-rag-kvstore.sh):
#!/bin/bash
# Shared function to seed RAG kvstore
seed_rag_kvstore() {
local RAG_SEED_DIR="${RAG_SEED_DIR:-/opt/app-root/rag-seed}"
local STORAGE_RAG_DIR="${STORAGE_RAG_DIR:-/opt/app-root/src/.llama/storage/rag}"
if [ -d "$RAG_SEED_DIR" ]; then
echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..."
mkdir -p "$STORAGE_RAG_DIR"
if ! compgen -G "$RAG_SEED_DIR/*.db" > /dev/null; then
echo "ERROR: No .db files found in $RAG_SEED_DIR"
exit 1
fi
cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/
fi
}Then source and call it in both entrypoint scripts:
source /app-root/seed-rag-kvstore.sh
seed_rag_kvstore🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/llama-stack-entrypoint.sh` around lines 18 - 24, The RAG seeding
block in scripts/llama-stack-entrypoint.sh is duplicated in
scripts/lightspeed-stack-entrypoint.sh; extract that logic into a shared
function (e.g., seed_rag_kvstore) in a new script (e.g.,
scripts/seed-rag-kvstore.sh) and then source and call seed_rag_kvstore from both
entrypoint scripts to remove duplication; ensure the function uses the same
environment variables (RAG_SEED_DIR, STORAGE_RAG_DIR), checks that the seed
directory exists and contains .db files before copying, emits the same echo
messages, and returns non-zero on fatal errors so both llama-stack-entrypoint.sh
and lightspeed-stack-entrypoint.sh simply call seed_rag_kvstore to perform the
seeding.
Comment on lines
+20
to
+24
| if [ -d "$RAG_SEED_DIR" ]; then | ||
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | ||
| mkdir -p "$STORAGE_RAG_DIR" | ||
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | ||
| fi |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Improve error handling for missing seed files.
If $RAG_SEED_DIR exists but contains no *.db files, the glob pattern will fail to expand and cp will exit with a cryptic error. Consider adding explicit validation for a clearer error message (same issue as in lightspeed-stack-entrypoint.sh).
🛡️ Proposed fix to add validation
if [ -d "$RAG_SEED_DIR" ]; then
echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..."
mkdir -p "$STORAGE_RAG_DIR"
+ if ! compgen -G "$RAG_SEED_DIR/*.db" > /dev/null; then
+ echo "ERROR: No .db files found in $RAG_SEED_DIR"
+ exit 1
+ fi
cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if [ -d "$RAG_SEED_DIR" ]; then | |
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | |
| mkdir -p "$STORAGE_RAG_DIR" | |
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | |
| fi | |
| if [ -d "$RAG_SEED_DIR" ]; then | |
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | |
| mkdir -p "$STORAGE_RAG_DIR" | |
| if ! compgen -G "$RAG_SEED_DIR/*.db" > /dev/null; then | |
| echo "ERROR: No .db files found in $RAG_SEED_DIR" | |
| exit 1 | |
| fi | |
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/llama-stack-entrypoint.sh` around lines 20 - 24, The seed-copy block
that checks RAG_SEED_DIR and runs cp may fail with a cryptic error when there
are no *.db files; update the shell snippet that uses RAG_SEED_DIR and
STORAGE_RAG_DIR to validate that at least one .db file exists before running cp,
e.g., test the glob (for f in "$RAG_SEED_DIR"/*.db; do [ -e "$f" ] && break;
done) and if no files found print a clear error/warning and skip or exit,
otherwise proceed to mkdir -p "$STORAGE_RAG_DIR" and cp the files; ensure the
message mentions the missing .db files and uses the same variable names
(RAG_SEED_DIR, STORAGE_RAG_DIR) so it’s easy to locate.
CodeHex083
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #124
Seed RAG kvstore is now delivered read-only and copied into the writable storage volume at startup, so the non-root container user (UID 1001) can write the registry kvstore it shares. Fixes the 'attempt to write a readonly database' crash. Covers both server and library compose modes.
Summary by CodeRabbit