Fix non-LSIO currentOsVariantOverride hidden by etc bind mount - #382
Merged
Conversation
Copilot review on PR #381 caught that the non-LSIO build-time RUN is hidden by the README's recommended bind-mount of /opt/${COMPANY_NAME}/mediaserver/etc — the host's empty etc directory shadows the image's pre-edited mediaserver.conf at runtime. Mirrors the LSIO fix from #380: drop the build-time RUN and inject idempotently at runtime via Docker/entrypoint.sh before exec'ing mediaserver. Both variants now follow the same pattern (LSIO does it in init-nx-relocate; non-LSIO does it in entrypoint.sh).
There was a problem hiding this comment.
Pull request overview
This PR fixes currentOsVariantOverride=docker being ineffective for non-LSIO images when users follow the README-recommended bind mount of /opt/${COMPANY_NAME}/mediaserver/etc, which can hide build-time edits to mediaserver.conf. The fix aligns non-LSIO behavior with the LSIO approach by performing an idempotent runtime injection at container start instead of during image build.
Changes:
- Move
currentOsVariantOverride=dockerinjection from Dockerfile build-timeRUN echo >>to non-LSIO runtime startup (Docker/entrypoint.sh). - Remove the now-ineffective build-time
RUNline from regenerated non-LSIO product Dockerfiles. - Update
CreateMatrix/Dockerfile.csto document why non-LSIO uses runtime injection.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Docker/entrypoint.sh | Adds idempotent runtime append of currentOsVariantOverride=docker before launching mediaserver. |
| CreateMatrix/Dockerfile.cs | Removes non-LSIO build-time injection and adds a clarifying note pointing to runtime injection. |
| Docker/NxWitness.Dockerfile | Regenerated to drop build-time currentOsVariantOverride RUN line. |
| Docker/NxMeta.Dockerfile | Regenerated to drop build-time currentOsVariantOverride RUN line. |
| Docker/NxGo.Dockerfile | Regenerated to drop build-time currentOsVariantOverride RUN line. |
| Docker/DWSpectrum.Dockerfile | Regenerated to drop build-time currentOsVariantOverride RUN line. |
| Docker/WisenetWAVE.Dockerfile | Regenerated to drop build-time currentOsVariantOverride RUN line. |
2 tasks
ptr727
added a commit
that referenced
this pull request
May 25, 2026
## Summary Copilot review on PR #381 round 2 flagged two related edge cases in the runtime injection added by #380 / #382: 1. **False positive on prefix match** — `grep -q "^currentOsVariantOverride=docker"` matches `currentOsVariantOverride=docker2` (or anything starting with `docker`). 2. **No upgrade path on existing different value** — if the user had previously set `currentOsVariantOverride=<other>`, the check fails and we append a *second* line for the same key, leaving precedence up to the parser. ## Fix In both [`Docker/entrypoint.sh`](Docker/entrypoint.sh) (non-LSIO) and [`Docker/s6-overlay/s6-rc.d/init-nx-relocate/run`](Docker/s6-overlay/s6-rc.d/init-nx-relocate/run) (LSIO), match the **key alone** — `^currentOsVariantOverride=` — and only append when the key is absent. This: - Avoids prefix-match false positives. - Never produces duplicate lines. - Respects a user's explicit setting (don't override what they typed). ## Test plan - [x] `dotnet test CreateMatrixTests/CreateMatrixTests.csproj` — 16/16 pass. - [ ] CI matrix passes.
ptr727
added a commit
that referenced
this pull request
May 25, 2026
## Summary Rolls up commits from develop. Brings the upstream-aligned `currentOsVariantOverride=docker` runtime injection, the README `ignoreRootTool` deviation note, refreshed compose examples (tmpfs `/tmp`, `/dev/dri` passthrough, storage-pool comments), and dependency bumps onto main. ## Why so many commits Initial #380 attempted a build-time RUN that worked for fresh installs but Copilot caught two bind-mount problems: - **LSIO** — `init-nx-relocate` replaces `/opt/.../mediaserver/etc` with a symlink to `/config/etc` on first start, erasing the build-time edit. Fixed in #380 round-2 by moving injection into the s6 init script. - **Non-LSIO** — the README's recommended setup bind-mounts `/opt/.../mediaserver/etc` from the host, hiding the build-time edit. Fixed in #382 by moving injection into `entrypoint.sh`. Subsequent iterations cleaned up the idempotence check (#383 — match the key alone, not the value) and made the write robust against non-writable bind mounts (#384 — guard the redirect, log a clear warning). ## Visible behavior changes for main - `mediaserver.conf` gets `currentOsVariantOverride=docker` appended on **container start** if the key is not already present. Injection happens in `entrypoint.sh` for non-LSIO and `init-nx-relocate/run` for LSIO. Match is key-only (`^currentOsVariantOverride=`) so a user-set value is never overridden and never duplicated. - README's Known Issues > Licensing now documents that upstream's `ignoreRootTool=true` deviation is **deliberately not adopted**. - Compose examples include `tmpfs: /tmp:size=1g,mode=1777` for RAM-backed temp files / Unix socket; production example shows `/dev/dri` iGPU passthrough and storage-pool layout. - Dependabot bumps (nuget-deps, actions-deps) included. ## Test plan The injection now happens on container start, so you must let the entrypoint run — `docker run --rm --entrypoint=cat ...` will skip it and produce a false negative. - [ ] Post-merge push to main fires `publish-release.yml` automatically. - [ ] Non-LSIO: start a container with the README compose example, then `docker exec <container> cat /opt/networkoptix/mediaserver/etc/mediaserver.conf | grep currentOsVariantOverride` shows the line. - [ ] LSIO: start with an empty `/config` volume, then `docker exec <container> cat /config/etc/mediaserver.conf | grep currentOsVariantOverride` shows the line. - [ ] Restart either container and confirm the line is not duplicated (idempotence). - [ ] Pre-populate `mediaserver.conf` with `currentOsVariantOverride=something-else`, start the container, confirm the existing value is preserved (no override). - [ ] Bind-mount a read-only etc directory in non-LSIO, start the container, confirm the entrypoint logs the "failed to write" warning to stderr and mediaserver still starts.
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
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.
Summary
Follow-up to #380. Copilot review on PR #381 (develop → main sync) caught that the non-LSIO build-time RUN from #380 is shadowed by the README's recommended bind-mount of
/opt/${COMPANY_NAME}/mediaserver/etc— the host's etc directory hides the image's pre-editedmediaserver.confat runtime.Same shape as the LSIO bug Copilot caught in PR #380's round 1, just for the other variant. Fix mirrors the LSIO approach: drop the build-time RUN and inject idempotently at container start.
Changes
CreateMatrix/Dockerfile.cs— remove the build-time RUN from the non-LSIOelsebranch; add a comment pointing readers to the runtime equivalent.Docker/entrypoint.sh— add the same idempotentgrep -q && echo >>pattern used ininit-nx-relocate/runfor LSIO. Runs after root-tool launch and beforeexec mediaserver.After this, both variants follow the same pattern: build is mediaserver-only; mediaserver.conf injection happens at runtime in the variant-appropriate init path.
Test plan
dotnet test CreateMatrixTests/CreateMatrixTests.csproj— 16/16 pass.grep -c currentOsVariantOverride=docker Docker/*.Dockerfile— 0/0 (all moved to runtime scripts).cat <bound-etc>/mediaserver.confshows the line.