nimmo/nixos-config

Watch

Fork 0

Migrate Redview to Lyra native service #163

Merged

nimmo merged 1 commit from feature/lyra-redview-cutover into main

2026-08-12 17:18:06 +01:00

nimmo commented

2026-08-12 11:28:24 +01:00

Owner

Refs #35

Summary

Migrates Redview from Nova’s Docker stack to Lyra’s native NixOS service.

Adds an NFS-backed, SOPS-configured Redview module for Lyra.
Preserves the migrated Redview database and BDFR state under /var/lib/redview.
Uses a per-rip userspace WireGuard SOCKS proxy: only BDFR traffic is tunnelled; the service has no network-administration capabilities.
Pins PRAW 7.7.1 for BDFR 2.6.2 compatibility.
Starts after the NFS mount and SOPS material are ready, and includes Redview state in backups and failure monitoring.

Validation

nix flake check passes.
Lyra just test activation succeeded.
redview.service is healthy locally at /health.
Existing media database paths were migrated successfully.
A live rip starts successfully through the userspace WireGuard proxy.

Remaining before merge

Validate a representative completed rip and its downloaded media.
Switch the Newt/Pangolin backend to Lyra only after local validation.
Confirm the public route and rollback path.

Refs #35 ## Summary Migrates Redview from Nova’s Docker stack to Lyra’s native NixOS service. - Adds an NFS-backed, SOPS-configured Redview module for Lyra. - Preserves the migrated Redview database and BDFR state under `/var/lib/redview`. - Uses a per-rip userspace WireGuard SOCKS proxy: only BDFR traffic is tunnelled; the service has no network-administration capabilities. - Pins PRAW 7.7.1 for BDFR 2.6.2 compatibility. - Starts after the NFS mount and SOPS material are ready, and includes Redview state in backups and failure monitoring. ## Validation - `nix flake check` passes. - Lyra `just test` activation succeeded. - `redview.service` is healthy locally at `/health`. - Existing media database paths were migrated successfully. - A live rip starts successfully through the userspace WireGuard proxy. ## Remaining before merge - Validate a representative completed rip and its downloaded media. - Switch the Newt/Pangolin backend to Lyra only after local validation. - Confirm the public route and rollback path.

nimmo added this to the Lyra Service Stack NixOS Migration milestone

2026-08-12 11:28:24 +01:00

nimmo added the

host:lyra

project:lyra-service-stack-migration

phase:cutover

phase:validation

labels

2026-08-12 11:28:24 +01:00

nimmo added 1 commit

2026-08-12 11:28:24 +01:00

feat(lyra): add native redview service

Build and Push Attic Cache / test (pull_request) Successful in 1m4s

Details

Build and Push Attic Cache / build (pull_request) Has been skipped

Details

Build and Push Attic Cache / pin-lifecycle (pull_request) Has been skipped

Details

Build and Push Attic Cache / remediate (pull_request) Has been skipped

Details

AI pull-request review / review (pull_request) Successful in 4m57s

Details

989753ed40

nimmo changed title from ~~WIP: Migrate Redview to Lyra native service~~ to Migrate Redview to Lyra native service

2026-08-12 11:34:45 +01:00

nimmo commented

2026-08-12 11:39:52 +01:00

Author

Owner

Automated code review

Reviewed commit: 989753ed4020ad12e0fc1cd366b4041b7d6f4193

Verdict: Requires further work

Resolve the blocking or important findings and investigate failed deterministic checks before merging.

Overall assessment

The PR introduces a native Redview service module with SOPS secret management, NFS mount configuration, and systemd dependency ordering. Three actionable findings were identified: heavy CUDA package evaluation outside the conditional guard, a potential startup failure due to a missing subdirectory in ExecStartPre, and redundant systemd mount options.

Blocking findings

None.

Important findings

Heavy CUDA evaluation outside conditional guard (modules/services/redview.nix:18; high confidence)
cudaPkgs = import inputs.nixpkgs {
system = pkgs.stdenv.hostPlatform.system;
config = {
allowUnfree = true;
cudaSupport = true;
cudaCapabilities = [ "8.9" ];
};
};
Impact: The CUDA Python environment and its dependencies are evaluated at module load time, even when nixosConfig.redview.enable is false. This wastes CI/local build resources and violates standard NixOS module scoping practices.
Suggested fix: Move the cudaPkgs and redviewPython definitions inside the mkIf cfg.enable { ... } block so they are only evaluated when the service is actually enabled.
ExecStartPre may fail due to missing subdirectory (modules/services/redview.nix:194; high confidence)
${pkgs.coreutils}/bin/install -m 0600 ${
config.sops.templates."redview-bdfr-config".path
} /var/lib/redview/bdfr-config/config.cfg
Impact: The install command copies the BDFR config to a nested path. If /var/lib/redview/bdfr-config/ does not exist (e.g., upstream tmpfiles only creates /var/lib/redview), systemd will fail the pre-start step and block service activation.
Suggested fix: Add mkdir -p /var/lib/redview/bdfr-config to ExecStartPre, or verify that the upstream redview module's tmpfiles rules create this subdirectory and adjust the dependency accordingly.

Suggestions

Redundant systemd mount ordering options (modules/services/redview.nix:112; medium confidence)
"x-systemd.requires=network-online.target"
"x-systemd.after=network-online.target"
Impact: Both requires and after imply ordering relative to network-online.target. Using both is redundant and slightly inflates the fstab entry without changing behavior.
Suggested fix: Remove x-systemd.requires (keeping after is standard for NFS mounts).

Tests and validation

No configured deterministic checks.

Questions

Does the upstream inputs.redview module's tmpfiles configuration create /var/lib/redview/bdfr-config/, or should ExecStartPre explicitly mkdir -p it?
Is there a repository policy regarding top-level package evaluation in NixOS modules, or is this purely an optimization concern for Lyra-only deployments?

Review limitations

Cannot execute nix flake check or just test to validate evaluation and deployment behavior.
External flake contents (inputs.redview/bdfr-patches/) are not supplied, so patch paths and upstream tmpfiles behavior are assumed based on PR description.
SOPS encrypted values in secrets/redview.yaml cannot be decrypted or verified against the .sops.yaml key groups beyond structural matching.

Diff coverage

.sops.yaml: reviewed — included in a context-limited batch
README.md: reviewed — included in a context-limited batch
flake.lock: reviewed — included in a context-limited batch
flake.nix: reviewed — included in a context-limited batch
hosts/lyra/default.nix: reviewed — included in a context-limited batch
modules/services/redview.nix: reviewed — included in a context-limited batch
secrets/redview.yaml: reviewed — included in a context-limited batch

Review metadata

Reviewer: forgejo-ai-review v1
Model: qwen3.6:35b-a3b-q8_0
Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, README.md
Commits considered: 1
Previous automated review: none found
Findings: 3

## Automated code review Reviewed commit: `989753ed4020ad12e0fc1cd366b4041b7d6f4193` ## Verdict: Requires further work Resolve the blocking or important findings and investigate failed deterministic checks before merging. ### Overall assessment The PR introduces a native Redview service module with SOPS secret management, NFS mount configuration, and systemd dependency ordering. Three actionable findings were identified: heavy CUDA package evaluation outside the conditional guard, a potential startup failure due to a missing subdirectory in ExecStartPre, and redundant systemd mount options. ### Blocking findings None. ### Important findings - **Heavy CUDA evaluation outside conditional guard** (modules/services/redview.nix:18; high confidence) cudaPkgs = import inputs.nixpkgs { system = pkgs.stdenv.hostPlatform.system; config = { allowUnfree = true; cudaSupport = true; cudaCapabilities = [ "8.9" ]; }; }; Impact: The CUDA Python environment and its dependencies are evaluated at module load time, even when nixosConfig.redview.enable is false. This wastes CI/local build resources and violates standard NixOS module scoping practices. Suggested fix: Move the cudaPkgs and redviewPython definitions inside the mkIf cfg.enable { ... } block so they are only evaluated when the service is actually enabled. - **ExecStartPre may fail due to missing subdirectory** (modules/services/redview.nix:194; high confidence) ${pkgs.coreutils}/bin/install -m 0600 ${ config.sops.templates."redview-bdfr-config".path } /var/lib/redview/bdfr-config/config.cfg Impact: The install command copies the BDFR config to a nested path. If /var/lib/redview/bdfr-config/ does not exist (e.g., upstream tmpfiles only creates /var/lib/redview), systemd will fail the pre-start step and block service activation. Suggested fix: Add mkdir -p /var/lib/redview/bdfr-config to ExecStartPre, or verify that the upstream redview module's tmpfiles rules create this subdirectory and adjust the dependency accordingly. ### Suggestions - **Redundant systemd mount ordering options** (modules/services/redview.nix:112; medium confidence) "x-systemd.requires=network-online.target" "x-systemd.after=network-online.target" Impact: Both requires and after imply ordering relative to network-online.target. Using both is redundant and slightly inflates the fstab entry without changing behavior. Suggested fix: Remove x-systemd.requires (keeping after is standard for NFS mounts). ### Tests and validation - No configured deterministic checks. ### Questions - Does the upstream inputs.redview module's tmpfiles configuration create /var/lib/redview/bdfr-config/, or should ExecStartPre explicitly mkdir -p it? - Is there a repository policy regarding top-level package evaluation in NixOS modules, or is this purely an optimization concern for Lyra-only deployments? ### Review limitations - Cannot execute nix flake check or just test to validate evaluation and deployment behavior. - External flake contents (inputs.redview/bdfr-patches/) are not supplied, so patch paths and upstream tmpfiles behavior are assumed based on PR description. - SOPS encrypted values in secrets/redview.yaml cannot be decrypted or verified against the .sops.yaml key groups beyond structural matching. ### Diff coverage - `.sops.yaml`: **reviewed** — included in a context-limited batch - `README.md`: **reviewed** — included in a context-limited batch - `flake.lock`: **reviewed** — included in a context-limited batch - `flake.nix`: **reviewed** — included in a context-limited batch - `hosts/lyra/default.nix`: **reviewed** — included in a context-limited batch - `modules/services/redview.nix`: **reviewed** — included in a context-limited batch - `secrets/redview.yaml`: **reviewed** — included in a context-limited batch <details><summary>Review metadata</summary> - Reviewer: forgejo-ai-review v1 - Model: qwen3.6:35b-a3b-q8_0 - Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, README.md - Commits considered: 1 - Previous automated review: none found - Findings: 3 </details>

nimmo commented

2026-08-12 17:15:46 +01:00

Author

Owner

Thanks for the review. I investigated all three findings against the evaluated configuration and live Lyra deployment; no code change is needed.

CUDA binding scope — Nix let bindings are lazy. cudaPkgs and redviewPython are referenced only inside mkIf cfg.enable, so they are not forced when Redview is disabled. In addition, this module is imported only by Lyra. nix flake check passes across all hosts.
BDFR config directory — the upstream Redview module declares a tmpfiles rule for ${cfg.bdfrConfigDir}. On the real Lyra activation, the ExecStartPre install step successfully created /var/lib/redview/bdfr-config/config.cfg, and the service started and completed its health check.
NFS ordering — After=network-online.target provides ordering only if that target is pulled in; Requires=network-online.target also pulls it into the transaction. Keeping both is intentional for an NFS mount.

The live service additionally completed a WireGuard-proxied BDFR rip startup with no host network-administration capability.

Thanks for the review. I investigated all three findings against the evaluated configuration and live Lyra deployment; no code change is needed. 1. **CUDA binding scope** — Nix `let` bindings are lazy. `cudaPkgs` and `redviewPython` are referenced only inside `mkIf cfg.enable`, so they are not forced when Redview is disabled. In addition, this module is imported only by Lyra. `nix flake check` passes across all hosts. 2. **BDFR config directory** — the upstream Redview module declares a tmpfiles rule for `${cfg.bdfrConfigDir}`. On the real Lyra activation, the `ExecStartPre install` step successfully created `/var/lib/redview/bdfr-config/config.cfg`, and the service started and completed its health check. 3. **NFS ordering** — `After=network-online.target` provides ordering only if that target is pulled in; `Requires=network-online.target` also pulls it into the transaction. Keeping both is intentional for an NFS mount. The live service additionally completed a WireGuard-proxied BDFR rip startup with no host network-administration capability.

nimmo merged commit 33364eb259 into main

2026-08-12 17:18:06 +01:00

nimmo referenced this pull request from a commit

2026-08-12 17:18:07 +01:00

Merge pull request 'Migrate Redview to Lyra native service' (#163) from feature/lyra-redview-cutover into main

nimmo deleted branch feature/lyra-redview-cutover

2026-08-12 17:18:08 +01:00