Add WSLAContainer flag to auto delete container after exit#14196
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Docker-style auto-remove behavior for WSLA containers by persisting an Rm-style flag in container metadata and triggering container deletion automatically after the container stops.
Changes:
- Persist/restore
WSLAContainerFlagsinWSLAContainerMetadataV1so flags survive service/session restarts. - Implement auto-delete on container stop events when
WSLAContainerFlagsRmis set. - Add a new Windows test covering auto-remove behavior and flag persistence across sessions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/windows/WSLATests.cpp | Adds ContainerAutoRemove test validating async deletion and persistence across sessions. |
| src/windows/wslaservice/exe/WSLAContainerMetadata.h | Extends persisted container metadata to include Flags. |
| src/windows/wslaservice/exe/WSLAContainer.cpp | Persists flags into metadata on create/open and triggers auto-delete on stop events. |
OneBlue
left a comment
OneBlue
left a comment
There was a problem hiding this comment.
LGTM, minor comments in the tests.
This turned out to be more complex problem than we thought. Thank you for doing this !
test/windows/WSLATests.cpp
Outdated
| launcher.SetContainerFlags(WSLAContainerFlagsRm); | ||
|
|
||
| auto container = launcher.Launch(*m_defaultSession); | ||
| auto process = container.GetInitProcess(); |
There was a problem hiding this comment.
nit: process is unused in this scope
test/windows/WSLATests.cpp
Outdated
| VERIFY_ARE_EQUAL(container.State(), WslaContainerStateRunning); | ||
| VERIFY_SUCCEEDED(container.Get().Stop(WSLASignalSIGKILL, 0)); | ||
|
|
||
| // verifyContainerDeleted("test-auto-remove"); |
There was a problem hiding this comment.
nit: We should remove that comment since it refers to a previous version of the tests
| wil::com_ptr<IWSLAContainer> notFound; | ||
| VERIFY_ARE_EQUAL(m_defaultSession->OpenContainer("test-auto-remove", ¬Found), HRESULT_FROM_WIN32(ERROR_NOT_FOUND)); | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: I think it would also be a good idea to add test coverage for the "stop detection" code path.
To do that we could create a container with the AutoRm flag that just does "echo foo", and validate that after the init process has exited, the container gets deleted (we'll probably have to check with a timeout since that won't be synchronous though, but we can use wsl::shared::retry::RetryWithTimeout for that)
2 participants
Summary of the Pull Request
Add support for Docker-style auto-remove (Rm) flag for containers
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed