Loosen currentOsVariantOverride idempotence check - #383
Merged
Merged
Conversation
Copilot review on PR #381 round 2 flagged that grep for "^currentOsVariantOverride=docker" has two problems: - False positive: matches "currentOsVariantOverride=docker2" or any other value that happens to start with "docker". - If a user has set currentOsVariantOverride=<something-else>, the check fails and we append a duplicate key, leaving two lines whose precedence depends on how mediaserver parses the file. Fix in both entrypoint.sh (non-LSIO) and init-nx-relocate/run (LSIO): match the key alone — "^currentOsVariantOverride=" — and only append when the key is absent. Respects an explicit user value, no duplicates, no false positives.
There was a problem hiding this comment.
Pull request overview
This PR adjusts the runtime injection logic for currentOsVariantOverride in both the non-LSIO entrypoint and the LSIO s6 init script so the idempotence check matches the key (currentOsVariantOverride=) rather than the full docker value.
Changes:
- Update the grep check to
^currentOsVariantOverride=sodocker2(or other prefixes) no longer produce false positives. - Avoid appending a second
currentOsVariantOverrideline when the key already exists with a different value. - Expand inline comments clarifying the “do not duplicate / do not override user value” intent.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Docker/entrypoint.sh | Loosens the idempotence check to detect any existing currentOsVariantOverride= key before appending. |
| Docker/s6-overlay/s6-rc.d/init-nx-relocate/run | Same key-only match for LSIO runtime injection to prevent duplicates and respect user-provided values. |
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
Copilot review on PR #381 round 2 flagged two related edge cases in the runtime injection added by #380 / #382:
grep -q "^currentOsVariantOverride=docker"matchescurrentOsVariantOverride=docker2(or anything starting withdocker).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(non-LSIO) andDocker/s6-overlay/s6-rc.d/init-nx-relocate/run(LSIO), match the key alone —^currentOsVariantOverride=— and only append when the key is absent. This:Test plan
dotnet test CreateMatrixTests/CreateMatrixTests.csproj— 16/16 pass.