LCORE-2338: LS container entrypoint + deployment artifacts for unified mode - #2319
LCORE-2338: LS container entrypoint + deployment artifacts for unified mode#2319max-svistunov wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (32)
🧰 Additional context used📓 Path-based instructions (3)**/*📄 CodeRabbit inference engine (Custom checks)
Files:
src/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
tests/unit/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (5)📚 Learning: 2026-05-20T08:09:30.641ZApplied to files:
📚 Learning: 2026-06-24T13:45:37.249ZApplied to files:
📚 Learning: 2026-07-06T15:26:18.398ZApplied to files:
📚 Learning: 2026-07-17T19:25:05.325ZApplied to files:
📚 Learning: 2026-05-12T15:14:34.788ZApplied to files:
🪛 ast-grep (0.45.1)src/llama_stack_configuration.py[warning] 1435-1435: File path is request-/variable-derived; validate and normalize to prevent path traversal. (open-filename-from-request) 🔇 Additional comments (6)
WalkthroughThe change adds automatic unified or legacy configuration selection for Llama Stack. It updates CLI dispatch, container startup, runtime mounts, configuration assets, deployment documentation, and unit tests. ChangesLlama Stack configuration modes
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The change adds unified-config auto-detection while preserving legacy startup behavior and updates the related deployment artifacts; targeted tests, end-to-end checks, and CI support the current implementation, so no actionable merge-blocking risk remains beyond normal checks. Sequence Diagram(s)sequenceDiagram
participant Entrypoint as llama-stack-entrypoint.sh
participant ConfigurationCLI as src/llama_stack_configuration.py
participant LlamaStack
Entrypoint->>ConfigurationCLI: Detect inputs and generate configuration
ConfigurationCLI-->>Entrypoint: Return generated-run.yaml or generation failure
Entrypoint->>Entrypoint: Validate generated or mounted run configuration
Entrypoint->>LlamaStack: Launch with the selected configuration
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ 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 |
69e9dff to
2f56509
Compare
2f56509 to
e74d9b1
Compare
|
@tisnik @radofuchs Could you PTAL? |
The llama_stack_configuration.py CLI previously only performed legacy enrichment: it always read the --input run.yaml and enriched it, so a container handed a unified lightspeed-stack.yaml (with no external run.yaml) could not start. The spec's server-mode trigger mechanism requires the CLI to auto-detect the configuration shape. main() now dispatches on a new has_synthesis_input() helper that mirrors the root Configuration.check_unified_vs_legacy detection on the raw YAML dict (non-empty inference.providers, non-empty vector_store.providers, or a llama_stack.config block). Unified configs are synthesized via synthesize_to_file — --input is ignored and need not exist, relative profile: paths resolve against the --config directory (R8), and the output keeps the 0600 secret-safety mode (R10). Legacy configs enrich the --input run.yaml exactly as before, so existing container layouts and CI provider-matrix runs are unaffected. Tests cover the detection matrix (all three synthesis inputs, empty provider lists, null sections, legacy path) and both CLI dispatch paths, including synthesis with a nonexistent --input and relative-profile resolution.
Make server mode work end to end from a single unified lightspeed-stack.yaml, relying on the CLI's unified-vs-legacy auto-detection: - scripts/llama-stack-entrypoint.sh: document the two startup modes in the header (synthesis from a unified config vs legacy run.yaml enrichment — the Python CLI decides), rename the intermediate file to generated-run.yaml, and fail with a clear error when generation fails and no fallback run.yaml is mounted, instead of handing llama-stack a nonexistent path. - deploy/llama-stack/test.containerfile: ship src/data/ to /opt/app-root/data so load_default_baseline() resolves next to the standalone-copied script (it reads ./data/default_run.yaml relative to its own location); chown it for the runtime user. - docker-compose.yaml: mount the src/data host copy beside the existing script host-copies, and document that the run.yaml mount is only consumed in legacy mode — one compose file serves both modes, with the mode chosen by the content of lightspeed-stack.yaml, so the CI provider-matrix workflows that pair run-*.yaml files with this compose file keep working unchanged. - deploy/llama-stack/README.md: document what the image bundles, the two startup modes, a minimal unified-only compose snippet (no run.yaml mount), and when a rebuild is needed vs covered by the host-copy mounts. .tekton/ pipelines reference no compose files and need no changes.
e74d9b1 to
3725c4b
Compare
1 participant
Description
Implements LCORE-2338: server mode works end to end from a single unified
lightspeed-stack.yaml.The ticket assumed the Python CLI already auto-detected unified vs legacy configs — it did not:
main()only performed legacy enrichment, so a container handed a unified config (no external run.yaml) could not start. This PR closes that gap and updates the deployment artifacts around it:src/llama_stack_configuration.py):main()dispatches on a newhas_synthesis_input()helper mirroring the rootConfiguration.check_unified_vs_legacydetection (non-emptyinference.providers, non-emptyvector_store.providers, or allama_stack.configblock). Unified configs are synthesized viasynthesize_to_file(--inputignored and need not exist; relativeprofile:resolves against the--configdir, R8; output keeps mode 0600, R10). Legacy configs enrich--inputexactly as before, so existing layouts and the CI provider-matrix workflows are untouched.scripts/llama-stack-entrypoint.sh): documents the two startup modes, adopts the post-renameogx stack runcommand, and fails with a clear error when generation fails and no fallback run.yaml is mounted. (Note for a follow-up:ogx stack runalready emits a FutureWarning recommendingogx run— inherited from the rename, kept for consistency with the rest of main.)deploy/llama-stack/test.containerfile): shipssrc/data/to/opt/app-root/datasoload_default_baseline()resolves next to the standalone-copied script.docker-compose.yaml): one file serves both modes — mode is chosen by the content oflightspeed-stack.yaml; therun.yamlmount is documented as legacy-only and is inert in unified mode. Adds thesrc/datahost-copy mount beside the existing script mounts.deploy/llama-stack/README.md, new): image contents, the two startup modes, a minimal unified-only compose snippet (no run.yaml mount), and rebuild guidance..tekton/pipelines reference no compose files and need no changes (flagging for @radofuchs as the Konflux owner per the ticket).Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
uv run pytest tests/unit/test_llama_stack_synthesize.py -k "main or has_synthesis" -vExpected: detection matrix + both dispatch paths pass, including synthesis with a nonexistent
--inputand relative-profile resolution.Actual: 4 passed (51 passed for the whole module; full unit suite 3176 passed).
uv run python src/llama_stack_configuration.py(viamigrate_config_dumbovertests/e2e/configs/run-ci.yaml+tests/e2e/configuration/server-mode/lightspeed-stack.yaml, enrichment inputs stripped) → rootlightspeed-stack.yaml;docker compose up -d llama-stack lightspeed-stackExpected: llama-stack log shows the synthesis path; all containers healthy;
/v1/queryanswers through LCORE → LS.Actual (re-verified after rebasing onto the OGX-rename main):
llama-stack | Generating llama-stack config from /opt/app-root/lightspeed-stack.yaml (mode auto-detected)...llama-stack | Using generated config: /tmp/generated-run.yamldocker compose ps: llama-stack, lightspeed-stack, mock-mcp, mock-tls-inference allhealthycurl -X POST http://localhost:8080/v1/query …→{"response":"unified ogx works",…}lightspeed-stack.yaml(CI server-mode config, no synthesis input) withrun.yaml=run-ci.yaml, recreate the two containers:docker compose up -d --force-recreate llama-stack lightspeed-stackExpected: enrichment path taken (the config has no synthesis input, so only
generate_configurationover the mounted run.yaml can produce the generated config); containers healthy;/v1/queryanswers.Actual (post-rebase): all containers
healthy; BYOK enrichment visible in the generated config (backend: byok_e2e-test-docs_storage);curl …→{"response":"legacy ogx works",…}uv run make format/uv run make verifyActual: clean, except 14 pre-existing mypy errors in
tests/unit/utils/test_models_dumper.pythat reproduce identically on untouched upstream/main.Summary by CodeRabbit
New Features
Bug Fixes