navidrome: beets writes tags (write: yes) (REN-350) #1069

2026-09-29T07:17:43Z

Ghost commented

2026-09-29 07:17:43 +00:00

What

kubernetes/apps/navidrome/beets.yaml:

import.write: yes
run.sh: beet write after import. Incremental import does not revisit the 74 albums matched in the dry runs, so this writes their DB tags into the files. The log reports counts before and after.
music mount is now writable (readOnly removed). move/copy/delete/link stay no, and art plugins stay off.

Why

REN-350 gate. Two dry runs were checked. Second run (beets-manual, exit 0): 560 albums, 74 matched, 486 skipped, 0 duplicates, 990 tracks to write. Spot check: Reiter "All You Need Is Love" now 2000 XE (was a 2004 RU reissue), Dio "Holy Diver" now 1983 (was JP 1994), Fresh Dumbledore OK, Annihilator "Metal" 2007 OK, and Judas Priest "Stained Class" is now skipped (safe).

Safety

Tags only; no file is moved, renamed or deleted. Lidarr already mounts this PVC read-write as uid/gid 1000 with fsGroup 1000, so there is no recursive chown. Lidarr has writeAudioTags: no. Home Assistant is not affected.

Verified

Shell syntax checked (sh -n). Linted only; not yet in the cluster.

Revert

Revert this commit. Tags already written stay in the files, which are otherwise unchanged.

🤖 Generated with Claude Code

## What `kubernetes/apps/navidrome/beets.yaml`: - `import.write: yes` - run.sh: `beet write` after import. Incremental import does not revisit the 74 albums matched in the dry runs, so this writes their DB tags into the files. The log reports counts before and after. - `music` mount is now writable (readOnly removed). move/copy/delete/link stay `no`, and art plugins stay off. ## Why REN-350 gate. Two dry runs were checked. Second run (`beets-manual`, exit 0): 560 albums, 74 matched, 486 skipped, 0 duplicates, 990 tracks to write. Spot check: Reiter "All You Need Is Love" now 2000 XE (was a 2004 RU reissue), Dio "Holy Diver" now 1983 (was JP 1994), Fresh Dumbledore OK, Annihilator "Metal" 2007 OK, and Judas Priest "Stained Class" is now skipped (safe). ## Safety Tags only; no file is moved, renamed or deleted. Lidarr already mounts this PVC read-write as uid/gid 1000 with fsGroup 1000, so there is no recursive chown. Lidarr has `writeAudioTags: no`. Home Assistant is not affected. ## Verified Shell syntax checked (`sh -n`). Linted only; not yet in the cluster. ## Revert Revert this commit. Tags already written stay in the files, which are otherwise unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Ghost added 1 commit 2026-09-29 07:17:43 +00:00

navidrome: beets writes tags (write: yes, beet write) (REN-350)

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

ci/woodpecker/pr/ci-manifests-test Pipeline was successful

Details

ci/woodpecker/pr/pr-review Pipeline was successful

Details

af0c0829af

Two dry runs were checked; the spot check now matches original releases.
Music mount becomes writable for tag writes; move/copy/delete stay off.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

review-bot reviewed

2026-09-29 07:21:03 +00:00

review-bot left a comment

🤖 Automated review — model CLI default (complexity: complex), commit af0c0829af5adb8218be3f465e9161d6001f284b.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

This PR flips the beets sidecar from dry-run to live tag writing for REN-350: import.write: yes, an explicit beet write step in run.sh to catch up the 74 albums already matched during the dry-run phase, and removal of readOnly: true from the music volume mount so the write can actually land. The change is small, self-contained, and the reasoning in the comments/PR body is consistent with the rest of the file (Lidarr already mounts the same PVC read-write at uid/gid 1000, writeAudioTags stays no there, move/copy/delete/link stay off in beets' own import config). No blocking issues found.

🟡 Suggestions

kubernetes/apps/navidrome/beets.yaml:84-89: beet write (and both write -p preview calls) run unconditionally, even when beet import itself failed (rc != 0, e.g. crash, config error, or beet not executing at all). The final exit code still correctly reflects the import's failure ([ "$rc" -eq 0 ] && rc=$wrc only applies wrc when import succeeded), so job status is right — but a hard-failed import could still leave you writing tags for the backlog in the same run where something already went wrong. This is probably intentional (writes are idempotent/tags-only per the PR's safety argument), but worth a one-line comment confirming that's deliberate rather than an oversight, since it's not obvious from a first read of the script.
kubernetes/apps/navidrome/beets.yaml:84,89: The two beet write -p preview invocations don't capture their own exit status. If beets fails outright on the preview call (e.g. DB locked), the count reported by grep -vc '^ ' ... would silently read from a truncated/empty file rather than surfacing a preview failure. Low risk given these are reporting-only, non-gating steps, but if you ever want the "items to write" / "items still differing" counters to be trustworthy signals, consider checking their exit codes too.

Tests

No automated test covers this script: ci-manifests-test.yml explicitly documents that its shellcheck step does not reach shell embedded inside YAML (init containers, this run.sh block), and that gap is pre-existing, not introduced here. The PR body states verification was limited to sh -n syntax checking plus two dry runs inspected by hand (match report + 5-album spot check) rather than a live cluster run of the write path. Given this cron job now performs live file writes to a shared PVC, that's a reasonable bar for a homelab repo, but it means the actual beet write behavior (including the new "items still differing after write" sanity check) is unverified until it runs in the cluster. Nothing further to add beyond what the PR body already discloses.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `af0c0829af5adb8218be3f465e9161d6001f284b`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR flips the beets sidecar from dry-run to live tag writing for REN-350: `import.write: yes`, an explicit `beet write` step in `run.sh` to catch up the 74 albums already matched during the dry-run phase, and removal of `readOnly: true` from the `music` volume mount so the write can actually land. The change is small, self-contained, and the reasoning in the comments/PR body is consistent with the rest of the file (Lidarr already mounts the same PVC read-write at uid/gid 1000, `writeAudioTags` stays `no` there, move/copy/delete/link stay off in beets' own import config). No blocking issues found. ## 🟡 Suggestions - `kubernetes/apps/navidrome/beets.yaml:84-89`: `beet write` (and both `write -p` preview calls) run unconditionally, even when `beet import` itself failed (`rc != 0`, e.g. crash, config error, or `beet` not executing at all). The final exit code still correctly reflects the import's failure (`[ "$rc" -eq 0 ] && rc=$wrc` only applies `wrc` when import succeeded), so job status is right — but a hard-failed import could still leave you writing tags for the backlog in the same run where something already went wrong. This is probably intentional (writes are idempotent/tags-only per the PR's safety argument), but worth a one-line comment confirming that's deliberate rather than an oversight, since it's not obvious from a first read of the script. - `kubernetes/apps/navidrome/beets.yaml:84,89`: The two `beet write -p` preview invocations don't capture their own exit status. If beets fails outright on the preview call (e.g. DB locked), the count reported by `grep -vc '^ ' ...` would silently read from a truncated/empty file rather than surfacing a preview failure. Low risk given these are reporting-only, non-gating steps, but if you ever want the "items to write" / "items still differing" counters to be trustworthy signals, consider checking their exit codes too. ## Tests No automated test covers this script: `ci-manifests-test.yml` explicitly documents that its `shellcheck` step does not reach shell embedded inside YAML (init containers, this run.sh block), and that gap is pre-existing, not introduced here. The PR body states verification was limited to `sh -n` syntax checking plus two dry runs inspected by hand (match report + 5-album spot check) rather than a live cluster run of the write path. Given this cron job now performs live file writes to a shared PVC, that's a reasonable bar for a homelab repo, but it means the actual `beet write` behavior (including the new "items still differing after write" sanity check) is unverified until it runs in the cluster. Nothing further to add beyond what the PR body already discloses. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

Ghost approved these changes

2026-09-29 07:21:32 +00:00

Ghost left a comment

Approved at af0c0829. Checked: diff is one file; import write: yes, copy/move/link/hardlink/reflink/delete stay no, duplicate_action skip, fetchart/embedart auto no. run.sh: sh -n OK; beet write exit is folded into rc only when import rc=0. /music RW: PVC navidrome-music-bulk is RWX cephfs, Lidarr mounts it RW with fsGroup 1000 OnRootMismatch, so no recursive chown. Undo for written tags: Velero schedule navidrome-daily (09:15, 30d TTL, PVCs incl. library) holds pre-write copies; first write run is 03:30 Europe/Berlin. Not HA scope. CI: ci-scripts-test, ci-manifests-test, pr-review success.

Approved at af0c0829. Checked: diff is one file; import write: yes, copy/move/link/hardlink/reflink/delete stay no, duplicate_action skip, fetchart/embedart auto no. run.sh: sh -n OK; beet write exit is folded into rc only when import rc=0. /music RW: PVC navidrome-music-bulk is RWX cephfs, Lidarr mounts it RW with fsGroup 1000 OnRootMismatch, so no recursive chown. Undo for written tags: Velero schedule navidrome-daily (09:15, 30d TTL, PVCs incl. library) holds pre-write copies; first write run is 03:30 Europe/Berlin. Not HA scope. CI: ci-scripts-test, ci-manifests-test, pr-review success.

Ghost merged commit 9ab8bc9735 into master

2026-09-29 07:21:33 +00:00

Ghost referenced this issue from a commit

2026-09-29 07:21:34 +00:00

Merge pull request 'navidrome: beets writes tags (write: yes) (REN-350)' (#1069) from atlas/ren-350-beets-write into master

Ghost referenced this pull request

2026-09-29 10:00:41 +00:00

navidrome: beets retries skipped albums (partial-album match trial, no writes) (REN-350) #1070

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#1069