feat(docker): expose memory (RAM) limit as an Advanced View field (OS-467) - #2673
Conversation
WalkthroughThe Docker container form now accepts a memory limit, documents its syntax and precedence, persists the value in XML, and includes it in generated ChangesDocker memory limit
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Form
participant XML
participant Command
participant Docker
Form->>XML: submit contMemory
XML->>Command: provide Memory
Command->>Docker: execute docker create with --memory
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
…(OS-469) (#2674) ## Summary Implements [OS-469](https://linear.app/lime-technology/issue/OS-469/pr-test-plugin-ship-a-diffpatch-so-multiple-pr-plugins-can-stack-on). The per-PR test plugin used to package changed files as **whole-file copies**, overwrite them on install, and **abort** if another `webgui-pr-*` plugin already managed any of the same files — so two PRs touching one file (e.g. [#2672](#2672) + [#2673](#2673), both editing `CreateDocker.php`) couldn't be tested together. Now it ships a **unified diff** and `patch`-applies it, so non-overlapping edits to the same file stack. ## How it works - **Build** (`pr-plugin-build.yml`): for changed **text** files, stage base + head versions in system layout and emit `pr.patch` (`diff -ruN`, paths apply with `patch -p1` at `/`). Changed **binary** files are copied whole into `binary/` with a `binary_files.txt` list. Both go in the same tarball. - **Plugin** (`generate-pr-plugin.sh`): on install, `patch -p1 --dry-run --forward` first — **abort with a clear message on a real reject** (overlapping change), otherwise apply and save `applied.patch`. Binaries are whole-file replaced with a per-binary conflict guard. On remove (and before update), reverse the patch (`patch -R`) and restore binary backups. - **Upload/R2 plumbing unchanged** — still a single tarball with the same URL/SHA wiring. ## Validation (local simulation) - Two patches editing the same file at different lines → **both apply, both changes coexist**. - Reversing one → **only that change is removed, the other stays**. - Overlapping edits → **dry-run fails → install aborts** cleanly. - Generated `.plg` is well-formed XML (all placeholders substituted); `bash -n` + YAML parse clean. ## Notes / still to verify - Real validation needs CI + an Unraid box installing two stacked PR plugins. Conveniently, **this PR touches `.github/**`, so it will itself build a PR test plugin with the new code** — a live smoke test of the generator. - Binary deletions aren't handled (rare); text add/modify/delete are. - `patch` is present in the Unraid base; uses `--forward`/`--batch` for non-interactive apply with fuzz. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Improved plugin install/update and uninstall to use a deterministic, patch-based manifest workflow for safer reversibility. * Enhanced plugin package generation to build a patch-oriented payload, distinguishing text changes from full binary replacements. * Updated the uninstall confirmation flow to reflect the new removal behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Add a "Memory limit" field to the Docker Add/Edit form (Advanced View) that maps to docker --memory, instead of requiring users to hand-write it in Extra Parameters. Empty leaves the container unlimited (unchanged default). The value round-trips through the container XML template (<Memory>) and is emitted as --memory=<value> in the docker create command. Closes OS-467 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
d441925 to
4d18192
Compare
…rop shared-file edits - Save mode is now much faster: pipe the single-pass streaming zip (scripts/flash_backup) into the destination file instead of appending each top-level item to a growing archive (one zip invocation vs many). - On completion the save job raises an Unraid notification (the tray bell) with a click-to-download link, exposes the saved file for download via a docroot symlink, and the GUI shows a Download button next to the saved path. - Revert the helptext.txt and .gitignore edits to base so this PR touches only its own files. helptext.txt was the single file shared with other in-flight PR test plugins (#2671, #2673), which made the stacked PR-plugin patch fail to apply; with it gone, #2677 stacks cleanly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@emhttp/plugins/dynamix.docker.manager/include/CreateDocker.php`:
- Around line 1151-1152: Update the contMemory handling in the Docker creation
flow, including its server-side validation before docker create, to reject or
normalize numeric values below the documented 6 MiB minimum, including 0, 1, and
5m. Align the input validation pattern or related form validation with this
minimum while preserving the unlimited option and valid unit formats.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 415baf6e-7c51-49c0-bc01-835969934acd
📒 Files selected for processing (3)
emhttp/languages/en_US/helptext.txtemhttp/plugins/dynamix.docker.manager/include/CreateDocker.phpemhttp/plugins/dynamix.docker.manager/include/Helpers.php
Summary
Docker memory limits were only available through Extra Parameters; this adds a first-class Advanced View field and preserves it through template save, edit-mode loading, and Docker command generation.
Why This Exists
OS-467 asks for the existing
docker --memorycapability to be available in the Add/Edit Container UI. The initial implementation saved<Memory>to the user template, but QA found thatxmlToVar()dropped the element before both edit-mode settings andxmlToCommand()consumed it.Resolution
The Advanced View form accepts a byte count or Docker byte suffix,
postToXML()stores it as<Memory>, andxmlToVar()now decodes that element into the canonical settings array.xmlToCommand()emits the first-class--memoryargument before Extra Parameters so the documented duplicate-flag precedence remains unchanged.Reviewer Considerations
Memorybelongs alongsideCPUsetin the common XML-to-settings conversion; that path feeds both edit-mode repopulation and command generation.<Memory>, so the conversion intentionally defaults the value to an empty string.--memoryin Extra Parameters remains later in the command and therefore wins; the help text documents this behavior.Behavior Changes
512m,2g, or a raw byte count persist in the container XML and repopulate when editing.--memory=<value>in the Docker create command.512mbare rejected by browser validation.Implementation Summary
contMemoryAdvanced View form field and localized help text.<Memory>in the container XML template.<Memory>inxmlToVar()so settings and command generation receive it.Verification
php -d short_open_tag=1 -l emhttp/plugins/dynamix.docker.manager/include/Helpers.php— passed.Risk
Low. The QA fix adds one optional XML mapping with an empty fallback; existing templates and containers keep their prior behavior when no memory value is present.
Summary by CodeRabbit
b,k,m, orgsuffixes.