feat(deploy): migrate stack management from play kube to systemd Quadlets - #416
Closed
sheepdestroyer wants to merge 6 commits into
Closed
feat(deploy): migrate stack management from play kube to systemd Quadlets#416sheepdestroyer wants to merge 6 commits into
sheepdestroyer wants to merge 6 commits into
Conversation
added 6 commits
July 22, 2026 23:46
…lets Replace 'podman play kube' with declarative Quadlet units rendered from quadlets/*.pod + quadlets/*.container templates into ~/.config/containers/systemd/llm-routing/. systemd's podman-user-generator turns them into llm-routing-*.service units, making systemd the single supervisor: boot auto-start (WantedBy=default.target + linger) and crash recovery (Restart=always) now come from systemd instead of podman's restartPolicy. Key points: - Templates use the same _PLACEHOLDER convention as pod.yaml; a new render_quadlets() in start-stack.sh reuses the existing env-derived values (ports, secrets, DATA_ROOT, POD_NAME) so dev/prod parity is preserved with plain string replacement (bare scalars, not YAML). - deploy_fresh_pod() now: render configs -> render quadlets -> daemon-reload -> start/restart llm-routing-pod.service. The bare 'restart existing' path and safe_pod_teardown() detect systemd-managed stacks and route through systemctl accordingly. - Entrypoint semantics: Quadlet Exec= only sets args (appended to the image entrypoint), so containers where pod.yaml used command: now set Entrypoint= explicitly (litellm venv python, valkey-server, redis-server, /bin/sh for the router). MinIO keeps its image entrypoint with Exec= providing the server args, matching pod.yaml args: behavior. - AddHost=<POD_NAME>:127.0.0.1 on all containers replicates the /etc/hosts entry play kube injected, required by langfuse 3.222+ which resolves the pod hostname at startup. - Healthchecks ported from livenessProbe exec commands (HealthCmd, HealthStartPeriod maps initialDelaySeconds). Verified on dev: all 9 containers healthy, 193/193 unit tests pass, crash recovery (kill -> systemd auto-restart <12s), canonical HTTPS endpoints green (model timeouts on free tier are pre-existing, identical on prod play-kube).
Contributor
Reviewer's GuideMigrates stack management from a Podman play kube-based pod to a systemd Quadlet-managed stack, introduces robust Quadlet rendering/deployment logic and ownership detection, and aligns external URL derivation and verification with new subdomain-based routing semantics. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughThe stack now uses systemd-managed Quadlets for deployment, adds nine service containers and a routing pod, derives service subdomain URLs, updates local llama endpoints, and expands endpoint verification, documentation, upgrade synchronization, and deployment contract tests. ChangesLLM service URLs
Quadlet deployment
Verification and documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant StartStack
participant UserSystemd
participant QuadletUnits
participant LLMRoutingPod
StartStack->>UserSystemd: reload and start or restart rendered units
UserSystemd->>QuadletUnits: generate and manage service units
QuadletUnits->>LLMRoutingPod: create or reconcile the routing pod
LLMRoutingPod-->>StartStack: report startup or failure status
Possibly related PRs
Suggested labels: ✨ Finishing Touches 💡 2⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The Quadlet renderer in
start-stack.shdereferences several environment variables withos.environ[...](e.g.OLLAMA_API_KEY,OPENROUTER_API_KEY,LANGFUSE_PUBLIC_KEY, etc.), which will hard-fail the deployment if any are missing; consider usingos.environ.get()with sane defaults for non-critical secrets to avoid making them implicitly mandatory. - The new
derive_external_service_urlsshell/Python helper,resolve_external_urlsinrouter/main.py, and the canonical URL verifier now each embed slightly different URL derivation logic for the same services; it would be safer to centralize this computation (or at least a shared reference implementation) to reduce the risk of these paths drifting over time.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- The Quadlet renderer in `start-stack.sh` dereferences several environment variables with `os.environ[...]` (e.g. `OLLAMA_API_KEY`, `OPENROUTER_API_KEY`, `LANGFUSE_PUBLIC_KEY`, etc.), which will hard-fail the deployment if any are missing; consider using `os.environ.get()` with sane defaults for non-critical secrets to avoid making them implicitly mandatory.
- The new `derive_external_service_urls` shell/Python helper, `resolve_external_urls` in `router/main.py`, and the canonical URL verifier now each embed slightly different URL derivation logic for the same services; it would be safer to centralize this computation (or at least a shared reference implementation) to reduce the risk of these paths drifting over time.
## Individual Comments### Comment 1
<locationpath="start-stack.sh"line_range="597-608" />
<code_context>
+import os
+from urllib.parse import urlparse
+public = (os.environ.get("PUBLIC_BASE_URL") or "").rstrip("/")
+routing_domain = os.environ.get("ROUTING_DOMAIN") or "vendeuvre.lan"
+parsed = urlparse(public if "://" in public else f"https://{public}")
+scheme = parsed.scheme if parsed.scheme in {"http", "https"} else "https"
+host = parsed.netloc or parsed.path.split("/", 1)[0] or routing_domain
+print(os.environ.get("PROXY_BASE_URL") or f"{scheme}://litellm.{host}")
+print(os.environ.get("NEXTAUTH_URL") or f"{scheme}://langfuse.{host}")
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Defaulting to a hard-coded routing domain when PUBLIC_BASE_URL is empty may surprise operators.
When `PUBLIC_BASE_URL` is empty you fall back to `routing_domain`, which defaults to the literal `"vendeuvre.lan"`. So a missing `PUBLIC_BASE_URL` and `ROUTING_DOMAIN` will silently produce URLs like `https://litellm.vendeuvre.lan`.
If this default is only valid in a specific environment, consider instead:
- Failing fast when both `PUBLIC_BASE_URL` and `ROUTING_DOMAIN` are unset, or
- Emitting a clear warning to stderr when the hard-coded default is used.
This makes configuration errors more visible and avoids hard-to-diagnose connectivity issues.
```suggestion local values values=$(python3 -c 'import osimport sysfrom urllib.parse import urlparsepublic = (os.environ.get("PUBLIC_BASE_URL") or "").rstrip("/")routing_domain = os.environ.get("ROUTING_DOMAIN") or ""# Fail fast if both PUBLIC_BASE_URL and ROUTING_DOMAIN are unset/emptyif not public and not routing_domain: print( "ERROR: PUBLIC_BASE_URL and ROUTING_DOMAIN are both unset; " "cannot derive external service URLs.", file=sys.stderr, ) sys.exit(1)# Warn when using the hard-coded default routing domainif not routing_domain: routing_domain = "vendeuvre.lan" print( \'WARNING: Using default routing domain "vendeuvre.lan". ' "Set ROUTING_DOMAIN or PUBLIC_BASE_URL to override.", file=sys.stderr, )# If PUBLIC_BASE_URL is set, use it; otherwise derive from routing_domainbase_for_parse = public if public else routing_domainparsed = urlparse(base_for_parse if "://" in base_for_parse else f"https://{base_for_parse}")scheme = parsed.scheme if parsed.scheme in {"http", "https"} else "https"host = parsed.netloc or parsed.path.split("/", 1)[0] or routing_domain# Only the derived URLs go to stdout; warnings/errors go to stderr aboveprint(os.environ.get("PROXY_BASE_URL") or f"{scheme}://litellm.{host}")print(os.environ.get("NEXTAUTH_URL") or f"{scheme}://langfuse.{host}")') || return 1```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+597
to
+608
| local values | ||
| values=$(python3 -c ' | ||
| import os | ||
| from urllib.parse import urlparse | ||
| public = (os.environ.get("PUBLIC_BASE_URL") or "").rstrip("/") | ||
| routing_domain = os.environ.get("ROUTING_DOMAIN") or "vendeuvre.lan" | ||
| parsed = urlparse(public if "://" in public else f"https://{public}") | ||
| scheme = parsed.scheme if parsed.scheme in {"http", "https"} else "https" | ||
| host = parsed.netloc or parsed.path.split("/", 1)[0] or routing_domain | ||
| print(os.environ.get("PROXY_BASE_URL") or f"{scheme}://litellm.{host}") | ||
| print(os.environ.get("NEXTAUTH_URL") or f"{scheme}://langfuse.{host}") | ||
| ') || return 1 |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Defaulting to a hard-coded routing domain when PUBLIC_BASE_URL is empty may surprise operators.
When PUBLIC_BASE_URL is empty you fall back to routing_domain, which defaults to the literal "vendeuvre.lan". So a missing PUBLIC_BASE_URL and ROUTING_DOMAIN will silently produce URLs like https://litellm.vendeuvre.lan.
If this default is only valid in a specific environment, consider instead:
- Failing fast when both
PUBLIC_BASE_URLandROUTING_DOMAINare unset, or - Emitting a clear warning to stderr when the hard-coded default is used.
This makes configuration errors more visible and avoids hard-to-diagnose connectivity issues.
Suggested change
| local values | |
| values=$(python3 -c ' | |
| import os | |
| from urllib.parse import urlparse | |
| public = (os.environ.get("PUBLIC_BASE_URL") or "").rstrip("/") | |
| routing_domain = os.environ.get("ROUTING_DOMAIN") or "vendeuvre.lan" | |
| parsed = urlparse(public if"://"in public else f"https://{public}") | |
| scheme = parsed.scheme if parsed.scheme in {"http", "https"} else"https" | |
| host = parsed.netloc or parsed.path.split("/", 1)[0] or routing_domain | |
| print(os.environ.get("PROXY_BASE_URL") or f"{scheme}://litellm.{host}") | |
| print(os.environ.get("NEXTAUTH_URL") or f"{scheme}://langfuse.{host}") | |
| ') || return 1 | |
| local values | |
| values=$(python3 -c ' | |
| import os | |
| import sys | |
| from urllib.parse import urlparse | |
| public = (os.environ.get("PUBLIC_BASE_URL") or "").rstrip("/") | |
| routing_domain = os.environ.get("ROUTING_DOMAIN") or "" | |
| # Fail fast if both PUBLIC_BASE_URL and ROUTING_DOMAIN are unset/empty | |
| if not public and not routing_domain: | |
| print( | |
| "ERROR: PUBLIC_BASE_URL and ROUTING_DOMAIN are both unset; " | |
| "cannot derive external service URLs.", | |
| file=sys.stderr, | |
| ) | |
| sys.exit(1) | |
| # Warn when using the hard-coded default routing domain | |
| if not routing_domain: | |
| routing_domain = "vendeuvre.lan" | |
| print( | |
| \'WARNING: Using default routing domain "vendeuvre.lan". ' | |
| "Set ROUTING_DOMAIN or PUBLIC_BASE_URL to override.", | |
| file=sys.stderr, | |
| ) | |
| # If PUBLIC_BASE_URL is set, use it; otherwise derive from routing_domain | |
| base_for_parse = public if public else routing_domain | |
| parsed = urlparse(base_for_parse if"://"in base_for_parse else f"https://{base_for_parse}") | |
| scheme = parsed.scheme if parsed.scheme in {"http", "https"} else"https" | |
| host = parsed.netloc or parsed.path.split("/", 1)[0] or routing_domain | |
| # Only the derived URLs go to stdout; warnings/errors go to stderr above | |
| print(os.environ.get("PROXY_BASE_URL") or f"{scheme}://litellm.{host}") | |
| print(os.environ.get("NEXTAUTH_URL") or f"{scheme}://langfuse.{host}") | |
| ') || return 1 |
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 freeto 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.
Consolidated Quadlet systemd deployment for LLM-Routing.
Summary by Sourcery
Migrate stack deployment from a Podman play-kube pod to a systemd Quadlet-managed stack, introduce subdomain-based external URLs for services, and update verification, docs, and tests to match the new deployment and routing model.
Enhancements:
Tests:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests