CLI: Initialize shm-size and configure stop-signal#40303
CLI: Initialize shm-size and configure stop-signal#40303AmelBawa-msft wants to merge 2 commits intofeature/wsl-for-appsfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the WSLC CLI container create/run flows to support configuring container default stop signal and /dev/shm size, wiring the new CLI arguments through validation and into WSLCContainerOptions passed to the session/container COM APIs.
Changes:
- Add
--stop-signaland--shm-sizearguments tocontainer createandcontainer run, including localized help text. - Validate and parse the new arguments, then plumb them through
ContainerOptionsintoWSLCContainerLauncher/WSLCContainerOptions. - Add E2E coverage for the new CLI flags and update the expected help output.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp | Adds E2E coverage for container run --stop-signal/--shm-size and updates expected help output. |
| test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp | Adds E2E coverage for container create --stop-signal/--shm-size and updates expected help output. |
| src/windows/wslc/tasks/ContainerTasks.cpp | Populates ContainerOptions.StopSignal / ShmSize from parsed CLI args. |
| src/windows/wslc/services/ContainerService.cpp | Applies StopSignal / ShmSize to the launcher prior to container creation. |
| src/windows/wslc/services/ContainerModel.h | Adds StopSignal and ShmSize fields to the CLI-facing ContainerOptions model. |
| src/windows/wslc/commands/ContainerRunCommand.cpp | Registers --stop-signal / --shm-size for container run. |
| src/windows/wslc/commands/ContainerCreateCommand.cpp | Registers --stop-signal / --shm-size for container create. |
| src/windows/wslc/arguments/ArgumentValidation.h | Declares memory-size validation/parsing helpers. |
| src/windows/wslc/arguments/ArgumentValidation.cpp | Implements memory-size validation/parsing and hooks it into arg validation. |
| src/windows/wslc/arguments/ArgumentDefinitions.h | Adds new argument definitions and localized descriptions. |
| src/windows/common/WSLCContainerLauncher.h | Adds a SetShmSize() API and stores shm size on the launcher. |
| src/windows/common/WSLCContainerLauncher.cpp | Sets WSLCContainerOptions.ShmSize from the launcher state. |
| localization/strings/en-US/Resources.resw | Adds localized CLI descriptions for --stop-signal and --shm-size. |
| ULONGLONG GetMemorySizeFromString(const std::wstring& input, const std::wstring& argName) | ||
| { | ||
| auto narrowInput = wsl::windows::common::string::WideToMultiByte(input); | ||
| auto parsed = wsl::shared::string::ParseMemorySize(narrowInput.c_str()); | ||
| if (!parsed.has_value()) | ||
| { | ||
| throw ArgumentException( | ||
| std::format(L"Invalid {} argument value: '{}'. Expected a memory size (e.g. 64m, 256m, 1g)", argName, input)); | ||
| } |
There was a problem hiding this comment.
GetMemorySizeFromString() uses wsl::shared::string::ParseMemorySize(), which is currently case-sensitive for units (expects K/M/G/T or KB/MB/...). With the help text/tests using lowercase (e.g. 128m, 1g), valid Docker-style inputs will be rejected. Consider normalizing the suffix to uppercase (or otherwise doing a case-insensitive parse) and make sure the error message examples match the accepted formats.
|
|
||
| WSLC_TEST_METHOD(WSLCE2E_Container_Run_ShmSize) | ||
| { | ||
| auto result = RunWslc(std::format(L"container run --rm --shm-size 128m {} df -h /dev/shm", DebianImage.NameAndTag())); |
There was a problem hiding this comment.
This test uses --shm-size 128m (lowercase unit). The current ParseMemorySize() helper is case-sensitive (expects M/MB etc.), so this test will fail unless the parser/validation is updated to accept lowercase units (or the test uses 128M).
| auto result = RunWslc(std::format(L"container run --rm --shm-size 128m {} df -h /dev/shm", DebianImage.NameAndTag())); | |
| auto result = RunWslc(std::format(L"container run --rm --shm-size 128M {} df -h /dev/shm", DebianImage.NameAndTag())); |
| WSLC_TEST_METHOD(WSLCE2E_Container_Create_ShmSize) | ||
| { | ||
| auto result = RunWslc(std::format( | ||
| L"container create --shm-size 128m --name {} {} df -h /dev/shm", WslcContainerName, DebianImage.NameAndTag())); | ||
| result.Verify({.Stderr = L"", .ExitCode = 0}); | ||
|
|
||
| result = RunWslc(std::format(L"container start -a {}", WslcContainerName)); | ||
| result.Verify({.Stderr = L"", .ExitCode = 0}); | ||
| VERIFY_IS_TRUE(result.Stdout->find(L"128M") != std::wstring::npos); |
There was a problem hiding this comment.
This test uses --shm-size 128m (lowercase unit). The current ParseMemorySize() helper is case-sensitive (expects M/MB etc.), so this test will fail unless the parser/validation is updated to accept lowercase units (or the test uses 128M).
| <comment>{FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated</comment> | ||
| </data> | ||
| <data name="WSLCCLI_ShmSizeArgDescription" xml:space="preserve"> | ||
| <value>Size of /dev/shm (e.g. 64m, 1g)</value> |
There was a problem hiding this comment.
The --shm-size description shows examples 64m, 1g, but the underlying ParseMemorySize() helper is case-sensitive for units (expects M/MB, G/GB, etc.). Either update parsing/validation to accept lowercase units or adjust this string so the documented examples match what is accepted.
| <value>Size of /dev/shm (e.g. 64m, 1g)</value> | |
| <value>Size of /dev/shm (e.g. 64M, 1G)</value> |
Summary of the Pull Request
--shm-sizeand--stop-signaloptions to container create and container runPR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed