Adopt currentOsVariantOverride from upstream nxvms-docker - #380
Merged
Conversation
Three changes from investigating issue #326 (upstream Docker tweaks): - CreateMatrix/Dockerfile.cs: append a RUN step that writes currentOsVariantOverride=docker to mediaserver.conf during image build, matching upstream's CI-3164 change. Regenerates 10 product Dockerfiles (5 products x stock/LSIO); the two base images are unaffected because they do not install mediaserver. - README.md: add a Licensing sub-bullet noting that upstream's later VMS-60430 change disables root-tool via ignoreRootTool=true and explaining why NxWitness deliberately does not follow (license enforcement depends on root-tool). - README.md: refresh the compose examples to add tmpfs /tmp (matches upstream's VMS-60430 tmpfs swap) and incorporate production-grade patterns from a real homelab deployment in the headline example (devices: /dev/dri for iGPU passthrough, storage-pool comments, Traefik routing label tweaks). Upstream's ignoreRootTool=true and the plugin-extension Dockerfile and ENTRYPOINT_SCRIPTS_DIR init-script hook are documented in the plan as deliberately not adopted (untested or conflicting with NxWitness's root-tool-based architecture).
There was a problem hiding this comment.
Pull request overview
This PR ports a small set of upstream nxvms-docker tweaks into this repo: it tries to force mediaserver to report its OS variant as Docker by appending currentOsVariantOverride=docker during image build, and refreshes README guidance (licensing note + compose examples including tmpfs /tmp and a more realistic “production” snippet).
Changes:
- Add a build step (via
CreateMatrix) to appendcurrentOsVariantOverride=dockertomediaserver.conf, regenerating 10 product Dockerfiles. - Update README licensing “Known Issues” to document upstream’s
ignoreRootTool=truedeviation and why this repo does not adopt it. - Refresh README compose examples (add
tmpfs /tmp, add/dev/dripassthrough example, tweak Traefik rule, add storage comments).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates compose examples (tmpfs/devices/labels) and documents upstream ignoreRootTool=true rationale. |
| CreateMatrix/Dockerfile.cs | Adds the generated Dockerfile step that appends currentOsVariantOverride=docker to mediaserver.conf. |
| Docker/NxWitness.Dockerfile | Regenerated to include the config append step. |
| Docker/NxWitness-LSIO.Dockerfile | Regenerated to include the config append step (but LSIO runtime relocation currently makes it ineffective). |
| Docker/NxMeta.Dockerfile | Regenerated to include the config append step. |
| Docker/NxMeta-LSIO.Dockerfile | Regenerated to include the config append step (but LSIO runtime relocation currently makes it ineffective). |
| Docker/NxGo.Dockerfile | Regenerated to include the config append step. |
| Docker/NxGo-LSIO.Dockerfile | Regenerated to include the config append step (but LSIO runtime relocation currently makes it ineffective). |
| Docker/DWSpectrum.Dockerfile | Regenerated to include the config append step. |
| Docker/DWSpectrum-LSIO.Dockerfile | Regenerated to include the config append step (but LSIO runtime relocation currently makes it ineffective). |
| Docker/WisenetWAVE.Dockerfile | Regenerated to include the config append step. |
| Docker/WisenetWAVE-LSIO.Dockerfile | Regenerated to include the config append step (but LSIO runtime relocation currently makes it ineffective). |
Copilot review on PR #380 caught that the build-time RUN was a no-op for LSIO variants: init-nx-relocate replaces /opt/${COMPANY_NAME}/mediaserver/etc with a symlink to /config/etc on first start, erasing the build-time edit. - CreateMatrix/Dockerfile.cs: emit the RUN only for non-LSIO variants (the etc directory survives there). Add an inline comment pointing readers to the LSIO runtime equivalent. - Docker/s6-overlay/s6-rc.d/init-nx-relocate/run: idempotently append currentOsVariantOverride=docker to /config/etc/mediaserver.conf at container start, after the symlink is in place. Uses grep -q to avoid duplicate lines on subsequent starts. - Regenerated Dockerfiles: 5 product LSIO files lose the RUN, 5 product non-LSIO files keep it, 2 base files still unaffected.
This was referenced May 25, 2026
Merged
ptr727
added a commit
that referenced
this pull request
May 25, 2026
## 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-edited `mediaserver.conf` at 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`](CreateMatrix/Dockerfile.cs) — remove the build-time RUN from the non-LSIO `else` branch; add a comment pointing readers to the runtime equivalent. - [`Docker/entrypoint.sh`](Docker/entrypoint.sh) — add the same idempotent `grep -q && echo >>` pattern used in [`init-nx-relocate/run`](Docker/s6-overlay/s6-rc.d/init-nx-relocate/run) for LSIO. Runs after root-tool launch and before `exec mediaserver`. - 5 non-LSIO product Dockerfiles regenerate to drop the RUN. LSIO Dockerfiles unchanged. 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 - [x] `dotnet test CreateMatrixTests/CreateMatrixTests.csproj` — 16/16 pass. - [x] `grep -c currentOsVariantOverride=docker Docker/*.Dockerfile` — 0/0 (all moved to runtime scripts). - [ ] Build a non-LSIO image, run with a bind-mounted empty etc dir, then `cat <bound-etc>/mediaserver.conf` shows the 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
Closes part of #326 (investigation of Nx upstream Docker tweaks). Plan deliberately scoped to three changes:
currentOsVariantOverride=docker— append the line tomediaserver.confat build time viaCreateMatrix/Dockerfile.cs, regenerating 10 product Dockerfiles. Mirrors upstream's54bbd16(CI-3164). The two base images are untouched (they don't install mediaserver).ignoreRootTool=truedeviation — add a sub-bullet under Known Issues > Licensing noting that upstream's4285f93(VMS-60430) disabled root-tool entirely, and explaining why NxWitness deliberately does not follow (license enforcement depends on root-tool).tmpfs /tmp(matches upstream's VMS-60430 switch) to all four compose snippets; bring the headline production example up to a real homelab pattern (devices: /dev/drifor iGPU passthrough, storage-pool comments, Traefik routing tweaks).Deliberately not adopted (rationale in plan)
ignoreRootTool=trueas a build-time default — would break license enforcement for paying users; opposite of NxWitness's architecture.extentions/nxai-plugin/Dockerfile) — no point shipping example code we can't test end-to-end with a real plugin.ENTRYPOINT_SCRIPTS_DIRuser init script hook — same "untested surface area" reason; revisit if a concrete need arises.File-by-file
CreateMatrix/Dockerfile.cs— one new RUN step inserted inCreateInstall()after the mediaserver deb install, before the LSIO/non-LSIO branching. Applies to both variants.Docker/*.Dockerfile(10 files) — regenerated viaMake/Create.sh; mechanical 5-line addition per product image, no other diff.README.md— one new bullet under Licensing;tmpfsadded to 4 compose blocks; production example expanded withdevices, comments, routing label.Test plan
dotnet build CreateMatrix/CreateMatrix.csproj— clean (0 warnings, 0 errors).dotnet test CreateMatrixTests/CreateMatrixTests.csproj— 16/16 pass.grep -c currentOsVariantOverride=docker Docker/*.Dockerfile— 10/10 product images, 0/2 base images.docker run --rm --entrypoint=cat <img> /opt/<company>/mediaserver/etc/mediaserver.conf | grep currentOsVariantOverrideshows the line.