Skip to content

fix(deploy): guard legacy Quadlet ownership#352

Closed
sheepdestroyer wants to merge 10 commits into
masterfrom
fix/quadlet-environment-isolation
Closed

fix(deploy): guard legacy Quadlet ownership#352
sheepdestroyer wants to merge 10 commits into
masterfrom
fix/quadlet-environment-isolation

Conversation

@sheepdestroyer

@sheepdestroyer sheepdestroyer commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Supersedes #351.

Final review fixes:

  • Verify the generic legacy Quadlet unit's PodName against the current stack before accepting ownership, both when the pod exists and when it is absent.
  • Prevent dev/prod cross-environment teardown through stale legacy unit metadata.
  • Document the actual local safety-net hop (local-qwen-3.6) in all fallback diagrams and routing bullets.
  • Preserve environment-specific Quadlet namespaces and worktree-scoped data roots.

Validation:

  • 15 targeted Quadlet tests passed.
  • bash -n start-stack.sh passed.
  • git diff --check passed.
  • Existing-pod legacy mismatch isolation harness passed.
  • Dev canonical verification previously passed 29/29.
  • Local safety-net http://127.0.0.1:8083 is reachable with the approved models loaded.

Summary by Sourcery

Isolate dev and prod Quadlet deployments by introducing environment-specific namespaces, updating unit naming and ownership handling to safely coexist with legacy deployments, and documenting the new behavior and local safety-net routing.

New Features:

  • Support environment-specific Quadlet namespaces for dev and production with separate rendered unit directories and data roots.

Bug Fixes:

  • Guard stack ownership and teardown logic to only treat the legacy generic Quadlet unit as owned when it explicitly targets the current pod, preventing cross-environment teardown.

Enhancements:

  • Namespace generated Quadlet unit filenames and internal dependencies based on the configured namespace while preserving non-identifier configuration values.
  • Validate QUADLET_NAMESPACE format at startup to avoid invalid systemd unit names and ensure predictable rendering behavior.

Documentation:

  • Update README and scripts documentation to reflect environment-specific Quadlet unit names, directories, and the local-qwen-3.6 safety-net hop in routing diagrams.

Tests:

  • Add tests covering environment-specific Quadlet namespaces, namespaced rendering behavior, ownership detection semantics, documentation expectations, and worktree-scoped default data roots.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@sourcery-ai

sourcery-ai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Makes Quadlet deployment environment-specific (dev vs prod), tightens legacy Quadlet ownership detection to avoid cross-environment teardown, and updates docs/tests to reflect namespaced units and a local safety-net model hop.

Sequence diagram for Quadlet stack ownership detection and safe teardown

sequenceDiagram
    actor User
    participant start_stack_sh
    participant podman
    participant systemd

    User->>start_stack_sh: safe_pod_teardown
    activate start_stack_sh
    start_stack_sh->>start_stack_sh: stack_ownership

    alt pod exists
        start_stack_sh->>podman: podman pod exists POD_NAME
        podman-->>start_stack_sh: exists
        start_stack_sh->>podman: podman pod inspect POD_NAME
        podman-->>start_stack_sh: InfraContainerID
        start_stack_sh->>podman: podman inspect PODMAN_SYSTEMD_UNIT
        podman-->>start_stack_sh: infra_unit
        alt infra_unit == LLM_ROUTING_POD_UNIT
            start_stack_sh-->>start_stack_sh: ownership=quadlet:LLM_ROUTING_POD_UNIT
        else infra_unit == LEGACY_LLM_ROUTING_POD_UNIT
            start_stack_sh->>systemd: systemctl --user cat LEGACY_LLM_ROUTING_POD_UNIT
            systemd-->>start_stack_sh: unit contents
            alt legacy_unit_owns_pod
                start_stack_sh-->>start_stack_sh: ownership=quadlet:LEGACY_LLM_ROUTING_POD_UNIT
            else
                start_stack_sh-->>start_stack_sh: ownership=absent
            end
        else other unit
            start_stack_sh-->>start_stack_sh: ownership=legacy
        end
    else pod absent
        start_stack_sh->>systemd: systemctl --user show LLM_ROUTING_POD_UNIT
        systemd-->>start_stack_sh: LoadState
        alt LLM_ROUTING_POD_UNIT loaded
            start_stack_sh-->>start_stack_sh: ownership=quadlet:LLM_ROUTING_POD_UNIT
        else
            start_stack_sh->>systemd: systemctl --user show LEGACY_LLM_ROUTING_POD_UNIT
            systemd-->>start_stack_sh: LoadState
            alt LEGACY_LLM_ROUTING_POD_UNIT loaded and legacy_unit_owns_pod
                start_stack_sh-->>start_stack_sh: ownership=quadlet:LEGACY_LLM_ROUTING_POD_UNIT
            else
                start_stack_sh-->>start_stack_sh: ownership=absent
            end
        end
    end

    start_stack_sh-->>start_stack_sh: result ownership

    alt ownership starts with quadlet:
        start_stack_sh->>start_stack_sh: owner_unit="${ownership#quadlet:}"
        start_stack_sh->>systemd: systemctl --user stop owner_unit
        systemd-->>start_stack_sh: stopped
        start_stack_sh->>systemd: systemctl --user reset-failed owner_unit
        systemd-->>start_stack_sh: reset
        start_stack_sh->>podman: podman pod rm -f POD_NAME
        podman-->>start_stack_sh: removed
    else legacy or absent
        start_stack_sh-->>User: skip Quadlet teardown
    end
    deactivate start_stack_sh
Loading

Flow diagram for updated premium routing fallback with local-qwen-3.6 safety net

flowchart LR
    A[agent-advanced-core] --> L[local-qwen-3.6]
    L --> O[llm-routing-ollama]
    O --> R[openrouter-auto]
Loading

File-Level Changes

Change Details Files
Introduce environment-specific Quadlet namespace and data root so dev/prod use separate unit names and directories.
  • Default QUADLET_NAMESPACE to llm-routing-prod with validation and export in start-stack.sh
  • Use QUADLET_NAMESPACE to derive LLM_ROUTING_POD_UNIT and QUADLET_DIR paths
  • Adjust README and scripts/README to describe dev/prod-specific unit names and locations
  • Add tests ensuring QUADLET_NAMESPACE is set in .env.dev and that DATA_ROOT defaults to a worktree-scoped directory
start-stack.sh
README.md
scripts/README.md
tests/test_quadlet_templates.py
Namespace Quadlet rendering so generated filenames and intra-unit references are environment-specific while preserving non-identifier strings.
  • Pass QUADLET_NAMESPACE into the embedded Python Quadlet renderer and compute a namespace-specific identifier prefix
  • Rewrite Pod/After/Wants/BindsTo/Requires/PartOf lines to replace llm-routing-* identifiers and pod references with namespaced equivalents only
  • Change rendered filenames and the rendered_names set to substitute llm-routing with the namespace
  • Add tests that the renderer preserves non-identifier occurrences of llm-routing while correctly namespacing identifiers and pod references
start-stack.sh
tests/test_quadlet_templates.py
Harden Quadlet stack ownership detection to guard legacy shared units and preserve the exact owning unit through teardown/restart.
  • Introduce LEGACY_LLM_ROUTING_POD_UNIT and a legacy_unit_owns_pod helper that checks PodName against the current POD_NAME via systemctl --user cat
  • Make stack_ownership return quadlet: for both namespaced and verified-legacy units, or absent/legacy otherwise
  • Change safe_pod_teardown and deployment logic to parse the quadlet: prefix and operate on the specific owning unit instead of a fixed name
  • Extend tests to assert new ownership markers, legacy guard behavior, and diagnostics messaging for the owning unit
start-stack.sh
tests/test_quadlet_templates.py
Document the local safety-net model (local-qwen-3.6) in routing diagrams and descriptions.
  • Update README routing diagrams to insert local-qwen-3.6 between advanced-core and llm-routing-ollama in all relevant fallback trees
  • Align the bullet-list descriptions under the diagrams with the new local-qwen-3.6 hop and clarify its role as a local safety net
README.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@sheepdestroyer, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 46 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b9980786-6a09-4964-a443-69196c0ae180

📥 Commits

Reviewing files that changed from the base of the PR and between 8e7eb90 and 8ad1e17.

📒 Files selected for processing (5)
  • .env.dev
  • README.md
  • scripts/README.md
  • start-stack.sh
  • tests/test_quadlet_templates.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/quadlet-environment-isolation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added documentation Improvements or additions to documentation scripts tests labels Jul 23, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • In the embedded Quadlet rendering Python, namespace_identifier has overlapping handling for Pod (both in the main if field in {...} branch and the subsequent elif field == "Pod"), which is currently unreachable and could be simplified to avoid double-replacement pitfalls and make the namespace logic easier to reason about.
  • The legacy_unit_owns_pod helper relies on grep -Fqx "PodName=${POD_NAME}", which is very strict; consider matching more robustly (e.g., allowing trailing whitespace or multiple PodName lines) so small unit-file formatting changes don’t silently break legacy ownership detection.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the embedded Quadlet rendering Python, `namespace_identifier` has overlapping handling for `Pod` (both in the main `if field in {...}` branch and the subsequent `elif field == "Pod"`), which is currently unreachable and could be simplified to avoid double-replacement pitfalls and make the namespace logic easier to reason about.
- The `legacy_unit_owns_pod` helper relies on `grep -Fqx "PodName=${POD_NAME}"`, which is very strict; consider matching more robustly (e.g., allowing trailing whitespace or multiple `PodName` lines) so small unit-file formatting changes don’t silently break legacy ownership detection.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@sheepdestroyer

Copy link
Copy Markdown
Owner Author

Superseded by #353 with the final foreign legacy-pod conflict abort and complete fallback documentation consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation scripts tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant