-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Implement container network mode #13886
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement container network mode #13886
Conversation
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Based on discussions, preferring easier code readability, maintenance, debugging until we do perf testing. Also consistency with existing codebase for similar scenarios
…t/WSL into user/ptrivedi/stop-cont
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements container network mode support for WSLA containers, enabling users to specify network configurations when creating containers. The implementation adds network type options to the container API, updates the nerdctl command generation to pass appropriate network flags, and includes the Stop() method implementation for container lifecycle management. The changes also include a mutex type upgrade from std::mutex to std::recursive_mutex to support nested locking patterns.
Key changes:
- Added
WSLA_CONTAINER_NETWORK_TYPEenum andWSLA_CONTAINER_NETWORKstruct to container options - Implemented network mode handling for Host and None modes (Bridge mode pending VHD updates)
- Implemented the previously stubbed
Stop()method for container lifecycle management
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/windows/wslaservice/inc/wslaservice.idl | Added WSLA_CONTAINER_NETWORK_TYPE enum and WSLA_CONTAINER_NETWORK struct to define container network configuration options |
| src/windows/wslaservice/exe/WSLAContainer.h | Changed m_lock from std::mutex to std::recursive_mutex to support nested locking when State() is called from locked methods |
| src/windows/wslaservice/exe/WSLAContainer.cpp | Implemented network mode handling in PrepareNerdctlCreateCommand() and implemented Stop() method for container lifecycle management |
| src/windows/common/WSLAProcessLauncher.h | Added FormatResult(int) overload declaration for formatting exit codes without full process output |
| src/windows/common/WSLAProcessLauncher.cpp | Implemented FormatResult(int) overload for Stop() method error messages |
| src/windows/common/WSLAContainerLauncher.h | Added containerNetworkType parameter to constructor and m_containerNetworkType member variable |
| src/windows/common/WSLAContainerLauncher.cpp | Updated constructor and LaunchNoThrow() to pass network type to container options |
| test/windows/WSLATests.cpp | Updated all container test cases with network type parameter and added ContainerNetwork test method to validate network mode functionality |
| const std::string& EntryPoint, | ||
| const std::vector<std::string>& Arguments, | ||
| const std::vector<std::string>& Environment, | ||
| int containerNetworkType, |
Copilot
AI
Dec 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter name 'containerNetworkType' uses camelCase which is inconsistent with other parameter names in this constructor that use PascalCase (Image, Name, EntryPoint, Arguments, Environment, Flags). It should be renamed to 'ContainerNetworkType' for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
| const std::string& EntryPoint = "", | ||
| const std::vector<std::string>& Arguments = {}, | ||
| const std::vector<std::string>& Environment = {}, | ||
| int containerNetworkType = 1, |
Copilot
AI
Dec 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter name 'containerNetworkType' uses camelCase which is inconsistent with other parameter names in this function declaration that use PascalCase (Image, Name, EntryPoint, Arguments, Environment, Flags). It should be renamed to 'ContainerNetworkType' for consistency.
| int containerNetworkType = 1, | |
| int ContainerNetworkType = 1, |
| private: | ||
| std::string m_image; | ||
| std::string m_name; | ||
| int m_containerNetworkType; |
Copilot
AI
Dec 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The member variable type should be 'WSLA_CONTAINER_NETWORK_TYPE' instead of 'int' for type safety and consistency with the struct field it's assigned to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
* Add container network type to container options * Create containers with the specified network mode option * Currently supported network modes: - Host - None Bridge network mode will be added when the VHD is ready Custom network mode will be added at a later time
1a7785b to
7286ff9
Compare
…microsoft/WSL into user/ptrivedi/cont-network-mode
9691a2a to
41776f6
Compare
dc8946a to
87b7671
Compare
network type being passed in
87b7671 to
606dc8d
Compare
Add container network type to container options
Create containers with the specified network mode option
Currently supported network modes:
- Host
- None