From 901ec06ae8e1cda57d4223365c5c1e3f8e7bdee3 Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Mon, 8 Dec 2025 19:46:23 -0500 Subject: [PATCH 01/14] Implement Stop Container --- src/windows/wslaservice/exe/WSLAContainer.cpp | 33 ++++++++++++++++++- test/windows/WSLATests.cpp | 33 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/windows/wslaservice/exe/WSLAContainer.cpp b/src/windows/wslaservice/exe/WSLAContainer.cpp index 3b770e9d4..c82cf78dc 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.cpp +++ b/src/windows/wslaservice/exe/WSLAContainer.cpp @@ -69,9 +69,40 @@ HRESULT WSLAContainer::Start() } HRESULT WSLAContainer::Stop(int Signal, ULONG TimeoutMs) +try { - return E_NOTIMPL; + std::lock_guard lock{m_lock}; + + /* 'nerdctl stop ...' + * returns success and on stdout if the container is running or already stopped + * returns error "No such container: " on stderr if the container is in 'Created' state or does not exist + * + * For our case, we consider stopping a container that is not running or does not exist as a no-op and return success. + * TODO: Discuss and return stdout/stderr or corresponding HRESULT from nerdctl stop for better diagnostics. + */ + + if (m_state == WslaContainerStateExited) + { + return S_OK; + } + // Validate that the container is in the exited state. + RETURN_HR_IF_MSG( + HRESULT_FROM_WIN32(ERROR_INVALID_STATE), + m_state != WslaContainerStateRunning, + "No such running or exited container '%hs', state: %i", + m_name.c_str(), + m_state); + ServiceProcessLauncher launcher(nerdctlPath, {nerdctlPath, "stop", m_name}); + // TODO: Figure out how we want to handle custom signals and timeout values. + // nerdctl stop has a --time and a --signal option that can be used + // By default, it uses SIGTERM and a default timeout of 10 seconds. + auto result = launcher.Launch(*m_parentVM).Wait(); + THROW_HR_IF(E_FAIL, result.first != 0); + + m_state = WslaContainerStateExited; + return S_OK; } +CATCH_RETURN(); HRESULT WSLAContainer::Delete() try diff --git a/test/windows/WSLATests.cpp b/test/windows/WSLATests.cpp index 031611cf4..1af05433e 100644 --- a/test/windows/WSLATests.cpp +++ b/test/windows/WSLATests.cpp @@ -1233,6 +1233,32 @@ class WSLATests VERIFY_SUCCEEDED(container.Get().Delete()); } + // Test StopContainer + { + // Create a container + WSLAContainerLauncher launcher( + "debian:latest", "test-container-2", "/bin/cat", {}, {}, ProcessFlags::Stdin | ProcessFlags::Stdout | ProcessFlags::Stderr); + + auto container = launcher.Launch(*session); + + // Verify that the container is in running state. + VERIFY_ARE_EQUAL(container.State(), WslaContainerStateRunning); + + VERIFY_SUCCEEDED(container.Get().Stop(9, 10000)); + + expectContainerList( + {{"exited-container", "debian:latest", WslaContainerStateExited}, + {"test-container-2", "debian:latest", WslaContainerStateExited}}); + + // Verify that the container is in exited state. + VERIFY_ARE_EQUAL(container.State(), WslaContainerStateExited); + + // Verify that deleting a container stopped via Stop() works. + VERIFY_SUCCEEDED(container.Get().Delete()); + + expectContainerList({{"exited-container", "debian:latest", WslaContainerStateExited}}); + } + // Verify that trying to open a non existing container fails. { wil::com_ptr sameContainer; @@ -1274,9 +1300,16 @@ class WSLATests {{"exited-container", "debian:latest", WslaContainerStateExited}, {"test-unique-name", "debian:latest", WslaContainerStateExited}}); + // Verify that calling Stop() on exited containers is a no-op and state remains as WslaContainerStateExited. + VERIFY_SUCCEEDED(container.Get().Stop(15, 1000)); + VERIFY_ARE_EQUAL(container.State(), WslaContainerStateExited); + // Verify that stopped containers can be deleted. VERIFY_SUCCEEDED(container.Get().Delete()); + // Verify that stopping a deleted container returns ERROR_INVALID_STATE. + VERIFY_ARE_EQUAL(container.Get().Stop(15, 1000), HRESULT_FROM_WIN32(ERROR_INVALID_STATE)); + // Verify that deleted containers can't be deleted again. VERIFY_ARE_EQUAL(container.Get().Delete(), HRESULT_FROM_WIN32(ERROR_INVALID_STATE)); From be9cdce0ddee555b2df429e8f61d131d9985dccf Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Mon, 8 Dec 2025 19:51:09 -0500 Subject: [PATCH 02/14] Add TODO for testing Stop() on a container in 'Created' state --- test/windows/WSLATests.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/windows/WSLATests.cpp b/test/windows/WSLATests.cpp index 1af05433e..ba1ca3db3 100644 --- a/test/windows/WSLATests.cpp +++ b/test/windows/WSLATests.cpp @@ -1246,6 +1246,9 @@ class WSLATests VERIFY_SUCCEEDED(container.Get().Stop(9, 10000)); + // TODO: Once 'container run' is split into 'container create' + 'container start', + // validate that Stop() on a container in 'Created' state returns ERROR_INVALID_STATE. + expectContainerList( {{"exited-container", "debian:latest", WslaContainerStateExited}, {"test-container-2", "debian:latest", WslaContainerStateExited}}); From d1aab6400cb9c66e6e2a45779a46e228ebf768e6 Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Mon, 8 Dec 2025 19:52:46 -0500 Subject: [PATCH 03/14] Update src/windows/wslaservice/exe/WSLAContainer.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/windows/wslaservice/exe/WSLAContainer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windows/wslaservice/exe/WSLAContainer.cpp b/src/windows/wslaservice/exe/WSLAContainer.cpp index c82cf78dc..65936ccf1 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.cpp +++ b/src/windows/wslaservice/exe/WSLAContainer.cpp @@ -85,7 +85,7 @@ try { return S_OK; } - // Validate that the container is in the exited state. + // Validate that the container is in the running state. RETURN_HR_IF_MSG( HRESULT_FROM_WIN32(ERROR_INVALID_STATE), m_state != WslaContainerStateRunning, From 63ec07cc7c33e176d19beb1c558d73b209a5bab0 Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Mon, 8 Dec 2025 23:14:30 -0500 Subject: [PATCH 04/14] Update src/windows/wslaservice/exe/WSLAContainer.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/windows/wslaservice/exe/WSLAContainer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/windows/wslaservice/exe/WSLAContainer.cpp b/src/windows/wslaservice/exe/WSLAContainer.cpp index 65936ccf1..6ce87d8dc 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.cpp +++ b/src/windows/wslaservice/exe/WSLAContainer.cpp @@ -77,7 +77,8 @@ try * returns success and on stdout if the container is running or already stopped * returns error "No such container: " on stderr if the container is in 'Created' state or does not exist * - * For our case, we consider stopping a container that is not running or does not exist as a no-op and return success. + * For our case, we treat stopping an already-exited container as a no-op and return success. + * Stopping a deleted or created container returns ERROR_INVALID_STATE. * TODO: Discuss and return stdout/stderr or corresponding HRESULT from nerdctl stop for better diagnostics. */ From 38d53f6f6d5d7284052998ba5ecdb3d9be233b8b Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Mon, 8 Dec 2025 23:15:07 -0500 Subject: [PATCH 05/14] Update src/windows/wslaservice/exe/WSLAContainer.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/windows/wslaservice/exe/WSLAContainer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windows/wslaservice/exe/WSLAContainer.cpp b/src/windows/wslaservice/exe/WSLAContainer.cpp index 6ce87d8dc..432f7aeb8 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.cpp +++ b/src/windows/wslaservice/exe/WSLAContainer.cpp @@ -90,7 +90,7 @@ try RETURN_HR_IF_MSG( HRESULT_FROM_WIN32(ERROR_INVALID_STATE), m_state != WslaContainerStateRunning, - "No such running or exited container '%hs', state: %i", + "Container '%hs' is not in a stoppable state: %i", m_name.c_str(), m_state); ServiceProcessLauncher launcher(nerdctlPath, {nerdctlPath, "stop", m_name}); From c614d36fc997bbdfcaf2c35582957c5ca58e12f5 Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Tue, 9 Dec 2025 00:22:07 -0500 Subject: [PATCH 06/14] Fix timeout, state, etc. --- src/windows/common/WSLAProcessLauncher.cpp | 5 +++++ src/windows/common/WSLAProcessLauncher.h | 1 + src/windows/wslaservice/exe/WSLAContainer.cpp | 19 +++++++++++++------ src/windows/wslaservice/exe/WSLAContainer.h | 1 + test/windows/WSLATests.cpp | 6 +++--- 5 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/windows/common/WSLAProcessLauncher.cpp b/src/windows/common/WSLAProcessLauncher.cpp index 5409b723c..3a3b7bda3 100644 --- a/src/windows/common/WSLAProcessLauncher.cpp +++ b/src/windows/common/WSLAProcessLauncher.cpp @@ -118,6 +118,11 @@ std::string WSLAProcessLauncher::FormatResult(const RunningWSLAProcess::ProcessR stdErr != result.Output.end() ? stdErr->second : ""); } +std::string WSLAProcessLauncher::FormatResult(const int code) +{ + return std::format("{} [{}] exited with: {}.", m_executable, wsl::shared::string::Join(m_arguments, ','), code); +} + std::pair RunningWSLAProcess::Wait(DWORD TimeoutMs) { THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_TIMEOUT), !GetExitEvent().wait(TimeoutMs)); diff --git a/src/windows/common/WSLAProcessLauncher.h b/src/windows/common/WSLAProcessLauncher.h index aa573cfe2..ea0f42fb4 100644 --- a/src/windows/common/WSLAProcessLauncher.h +++ b/src/windows/common/WSLAProcessLauncher.h @@ -95,6 +95,7 @@ class WSLAProcessLauncher ClientRunningWSLAProcess Launch(IWSLASession& Session); std::tuple> LaunchNoThrow(IWSLASession& Session); std::string FormatResult(const RunningWSLAProcess::ProcessResult& result); + std::string FormatResult(const int code); protected: std::tuple, std::vector> CreateProcessOptions(); diff --git a/src/windows/wslaservice/exe/WSLAContainer.cpp b/src/windows/wslaservice/exe/WSLAContainer.cpp index 432f7aeb8..4ff85df8c 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.cpp +++ b/src/windows/wslaservice/exe/WSLAContainer.cpp @@ -73,6 +73,11 @@ try { std::lock_guard lock{m_lock}; + if (StateNoLock() == WslaContainerStateExited) + { + return S_OK; + } + /* 'nerdctl stop ...' * returns success and on stdout if the container is running or already stopped * returns error "No such container: " on stderr if the container is in 'Created' state or does not exist @@ -82,10 +87,6 @@ try * TODO: Discuss and return stdout/stderr or corresponding HRESULT from nerdctl stop for better diagnostics. */ - if (m_state == WslaContainerStateExited) - { - return S_OK; - } // Validate that the container is in the running state. RETURN_HR_IF_MSG( HRESULT_FROM_WIN32(ERROR_INVALID_STATE), @@ -97,8 +98,8 @@ try // TODO: Figure out how we want to handle custom signals and timeout values. // nerdctl stop has a --time and a --signal option that can be used // By default, it uses SIGTERM and a default timeout of 10 seconds. - auto result = launcher.Launch(*m_parentVM).Wait(); - THROW_HR_IF(E_FAIL, result.first != 0); + auto result = launcher.Launch(*m_parentVM).Wait(TimeoutMs); + THROW_HR_IF_MSG(E_FAIL, result.first != 0, "%hs", launcher.FormatResult(result.first).c_str()); m_state = WslaContainerStateExited; return S_OK; @@ -131,6 +132,12 @@ WSLA_CONTAINER_STATE WSLAContainer::State() noexcept { std::lock_guard lock{m_lock}; + // If the container is running, refresh the init process state before returning. + return StateNoLock(); +} + +WSLA_CONTAINER_STATE WSLAContainer::StateNoLock() noexcept +{ // If the container is running, refresh the init process state before returning. if (m_state == WslaContainerStateRunning && m_containerProcess.State() != WSLAProcessStateRunning) { diff --git a/src/windows/wslaservice/exe/WSLAContainer.h b/src/windows/wslaservice/exe/WSLAContainer.h index 53370bcef..a79180666 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.h +++ b/src/windows/wslaservice/exe/WSLAContainer.h @@ -50,6 +50,7 @@ class DECLSPEC_UUID("B1F1C4E3-C225-4CAE-AD8A-34C004DE1AE4") WSLAContainer WSLAVirtualMachine* m_parentVM = nullptr; std::mutex m_lock; + WSLA_CONTAINER_STATE StateNoLock() noexcept; static std::vector PrepareNerdctlRunCommand(const WSLA_CONTAINER_OPTIONS& options, std::vector&& inputOptions); }; } // namespace wsl::windows::service::wsla \ No newline at end of file diff --git a/test/windows/WSLATests.cpp b/test/windows/WSLATests.cpp index ba1ca3db3..bffe13008 100644 --- a/test/windows/WSLATests.cpp +++ b/test/windows/WSLATests.cpp @@ -1244,7 +1244,7 @@ class WSLATests // Verify that the container is in running state. VERIFY_ARE_EQUAL(container.State(), WslaContainerStateRunning); - VERIFY_SUCCEEDED(container.Get().Stop(9, 10000)); + VERIFY_SUCCEEDED(container.Get().Stop(15, 50000)); // TODO: Once 'container run' is split into 'container create' + 'container start', // validate that Stop() on a container in 'Created' state returns ERROR_INVALID_STATE. @@ -1304,14 +1304,14 @@ class WSLATests {"test-unique-name", "debian:latest", WslaContainerStateExited}}); // Verify that calling Stop() on exited containers is a no-op and state remains as WslaContainerStateExited. - VERIFY_SUCCEEDED(container.Get().Stop(15, 1000)); + VERIFY_SUCCEEDED(container.Get().Stop(15, 50000)); VERIFY_ARE_EQUAL(container.State(), WslaContainerStateExited); // Verify that stopped containers can be deleted. VERIFY_SUCCEEDED(container.Get().Delete()); // Verify that stopping a deleted container returns ERROR_INVALID_STATE. - VERIFY_ARE_EQUAL(container.Get().Stop(15, 1000), HRESULT_FROM_WIN32(ERROR_INVALID_STATE)); + VERIFY_ARE_EQUAL(container.Get().Stop(15, 50000), HRESULT_FROM_WIN32(ERROR_INVALID_STATE)); // Verify that deleted containers can't be deleted again. VERIFY_ARE_EQUAL(container.Get().Delete(), HRESULT_FROM_WIN32(ERROR_INVALID_STATE)); From d7f3e27992e2eae708c5aeed53709218128de078 Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Tue, 9 Dec 2025 16:33:55 -0500 Subject: [PATCH 07/14] Change to using a recursive_mutex for now Based on discussions, preferring easier code readability, maintenance, debugging until we do perf testing. Also consistency with existing codebase for similar scenarios --- src/windows/wslaservice/exe/WSLAContainer.cpp | 16 +++++----------- src/windows/wslaservice/exe/WSLAContainer.h | 2 +- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/windows/wslaservice/exe/WSLAContainer.cpp b/src/windows/wslaservice/exe/WSLAContainer.cpp index 4ff85df8c..2e1c5b2c6 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.cpp +++ b/src/windows/wslaservice/exe/WSLAContainer.cpp @@ -71,9 +71,9 @@ HRESULT WSLAContainer::Start() HRESULT WSLAContainer::Stop(int Signal, ULONG TimeoutMs) try { - std::lock_guard lock{m_lock}; + std::lock_guard lock(m_lock); - if (StateNoLock() == WslaContainerStateExited) + if (State() == WslaContainerStateExited) { return S_OK; } @@ -109,12 +109,12 @@ CATCH_RETURN(); HRESULT WSLAContainer::Delete() try { - std::lock_guard lock{m_lock}; + std::lock_guard lock(m_lock); // Validate that the container is in the exited state. RETURN_HR_IF_MSG( HRESULT_FROM_WIN32(ERROR_INVALID_STATE), - m_state != WslaContainerStateExited, + State() != WslaContainerStateExited, "Cannot delete container '%hs', state: %i", m_name.c_str(), m_state); @@ -130,14 +130,8 @@ CATCH_RETURN(); WSLA_CONTAINER_STATE WSLAContainer::State() noexcept { - std::lock_guard lock{m_lock}; + std::lock_guard lock(m_lock); - // If the container is running, refresh the init process state before returning. - return StateNoLock(); -} - -WSLA_CONTAINER_STATE WSLAContainer::StateNoLock() noexcept -{ // If the container is running, refresh the init process state before returning. if (m_state == WslaContainerStateRunning && m_containerProcess.State() != WSLAProcessStateRunning) { diff --git a/src/windows/wslaservice/exe/WSLAContainer.h b/src/windows/wslaservice/exe/WSLAContainer.h index a79180666..d4aa807ef 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.h +++ b/src/windows/wslaservice/exe/WSLAContainer.h @@ -48,7 +48,7 @@ class DECLSPEC_UUID("B1F1C4E3-C225-4CAE-AD8A-34C004DE1AE4") WSLAContainer std::string m_image; WSLA_CONTAINER_STATE m_state = WslaContainerStateInvalid; WSLAVirtualMachine* m_parentVM = nullptr; - std::mutex m_lock; + std::recursive_mutex m_lock; WSLA_CONTAINER_STATE StateNoLock() noexcept; static std::vector PrepareNerdctlRunCommand(const WSLA_CONTAINER_OPTIONS& options, std::vector&& inputOptions); From 06435720f3868bddc84a6f420a506350248d03b0 Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Wed, 10 Dec 2025 09:45:52 -0500 Subject: [PATCH 08/14] Fix merge issues and change to recursive_mutex after merge --- src/windows/wslaservice/exe/WSLAContainer.cpp | 4 ++-- src/windows/wslaservice/exe/WSLAContainer.h | 1 - test/windows/WSLATests.cpp | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/windows/wslaservice/exe/WSLAContainer.cpp b/src/windows/wslaservice/exe/WSLAContainer.cpp index 2d0bd858e..abde4d967 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.cpp +++ b/src/windows/wslaservice/exe/WSLAContainer.cpp @@ -44,7 +44,7 @@ const std::string& WSLAContainer::Image() const noexcept void WSLAContainer::Start(const WSLA_CONTAINER_OPTIONS& Options) { - std::lock_guard lock{m_lock}; + std::lock_guard lock(m_lock); THROW_HR_IF_MSG( HRESULT_FROM_WIN32(ERROR_INVALID_STATE), @@ -185,7 +185,7 @@ CATCH_RETURN(); HRESULT WSLAContainer::GetInitProcess(IWSLAProcess** Process) try { - std::lock_guard lock{m_lock}; + std::lock_guard lock(m_lock); THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !m_containerProcess.has_value()); return m_containerProcess->Get().QueryInterface(__uuidof(IWSLAProcess), (void**)Process); diff --git a/src/windows/wslaservice/exe/WSLAContainer.h b/src/windows/wslaservice/exe/WSLAContainer.h index bc9deac55..2a8b7fab0 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.h +++ b/src/windows/wslaservice/exe/WSLAContainer.h @@ -49,7 +49,6 @@ class DECLSPEC_UUID("B1F1C4E3-C225-4CAE-AD8A-34C004DE1AE4") WSLAContainer std::optional GetNerdctlStatus(); - std::mutex m_lock; wil::unique_event m_startedEvent{wil::EventOptions::ManualReset}; std::optional m_containerProcess; std::string m_name; diff --git a/test/windows/WSLATests.cpp b/test/windows/WSLATests.cpp index 4d39edb7f..384d4a83c 100644 --- a/test/windows/WSLATests.cpp +++ b/test/windows/WSLATests.cpp @@ -1299,7 +1299,7 @@ class WSLATests { // Create a container WSLAContainerLauncher launcher( - "debian:latest", "test-container-2", "/bin/cat", {}, {}, ProcessFlags::Stdin | ProcessFlags::Stdout | ProcessFlags::Stderr); + "debian:latest", "test-container-2", "sleep", {"sleep", "99999"}, {}, ProcessFlags::Stdout | ProcessFlags::Stderr); auto container = launcher.Launch(*session); From 7286ff9e35dcb7d69dc039cbff6e479036bb57e2 Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Fri, 12 Dec 2025 12:17:20 -0500 Subject: [PATCH 09/14] Implement container network mode * 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 --- src/windows/common/WSLAContainerLauncher.cpp | 4 +- src/windows/common/WSLAContainerLauncher.h | 2 + src/windows/wslaservice/exe/WSLAContainer.cpp | 37 +++- src/windows/wslaservice/inc/wslaservice.idl | 15 ++ test/windows/WSLATests.cpp | 173 +++++++++++++++++- 5 files changed, 218 insertions(+), 13 deletions(-) diff --git a/src/windows/common/WSLAContainerLauncher.cpp b/src/windows/common/WSLAContainerLauncher.cpp index 838654017..31090f9bb 100644 --- a/src/windows/common/WSLAContainerLauncher.cpp +++ b/src/windows/common/WSLAContainerLauncher.cpp @@ -48,8 +48,9 @@ WSLAContainerLauncher::WSLAContainerLauncher( const std::string& EntryPoint, const std::vector& Arguments, const std::vector& Environment, + WSLA_CONTAINER_NETWORK_TYPE containerNetworkType, ProcessFlags Flags) : - WSLAProcessLauncher(EntryPoint, Arguments, Environment, Flags), m_image(Image), m_name(Name) + WSLAProcessLauncher(EntryPoint, Arguments, Environment, Flags), m_image(Image), m_name(Name), m_containerNetworkType(containerNetworkType) { } @@ -60,6 +61,7 @@ std::pair> WSLAContainerLauncher::L options.Name = m_name.c_str(); auto [processOptions, commandLinePtrs, environmentPtrs] = CreateProcessOptions(); options.InitProcessOptions = processOptions; + options.ContainerNetwork.ContainerNetworkType = static_cast(m_containerNetworkType); if (m_executable.empty()) { diff --git a/src/windows/common/WSLAContainerLauncher.h b/src/windows/common/WSLAContainerLauncher.h index dd8b037a4..47f5e6ec5 100644 --- a/src/windows/common/WSLAContainerLauncher.h +++ b/src/windows/common/WSLAContainerLauncher.h @@ -45,6 +45,7 @@ class WSLAContainerLauncher : private WSLAProcessLauncher const std::string& EntryPoint = "", const std::vector& Arguments = {}, const std::vector& Environment = {}, + WSLA_CONTAINER_NETWORK_TYPE containerNetworkType = WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_HOST, ProcessFlags Flags = ProcessFlags::Stdout | ProcessFlags::Stderr); void AddVolume(const std::string& HostPath, const std::string& ContainerPath, bool ReadOnly); @@ -56,5 +57,6 @@ class WSLAContainerLauncher : private WSLAProcessLauncher private: std::string m_image; std::string m_name; + int m_containerNetworkType; }; } // namespace wsl::windows::common \ No newline at end of file diff --git a/src/windows/wslaservice/exe/WSLAContainer.cpp b/src/windows/wslaservice/exe/WSLAContainer.cpp index abde4d967..9a99abef0 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.cpp +++ b/src/windows/wslaservice/exe/WSLAContainer.cpp @@ -18,11 +18,10 @@ Module Name: using wsl::windows::service::wsla::WSLAContainer; -// Constants for required default arguments for "nerdctl run..." -static std::vector defaultNerdctlRunArgs{//"--pull=never", // TODO: Uncomment once PullImage() is implemented. - "--net=host", // TODO: default for now, change later - "--ulimit", - "nofile=65536:65536"}; +// Constants for required default arguments for "nerdctl create..." +static std::vector defaultNerdctlCreateArgs{//"--pull=never", // TODO: Uncomment once PullImage() is implemented. + "--ulimit", + "nofile=65536:65536"}; WSLAContainer::WSLAContainer(WSLAVirtualMachine* parentVM, const WSLA_CONTAINER_OPTIONS& Options, std::string&& Id, ContainerEventTracker& tracker) : m_parentVM(parentVM), m_name(Options.Name), m_image(Options.Image), m_id(std::move(Id)) @@ -262,6 +261,30 @@ std::vector WSLAContainer::PrepareNerdctlCreateCommand(const WSLA_C args.push_back("create"); args.push_back("--name"); args.push_back(options.Name); + + if (options.ContainerNetwork.ContainerNetworkType == WSLA_CONTAINER_NETWORK_HOST) + { + args.push_back("--net=host"); + } + else if (options.ContainerNetwork.ContainerNetworkType == WSLA_CONTAINER_NETWORK_NONE) + { + args.push_back("--net=none"); + } + else if (options.ContainerNetwork.ContainerNetworkType == WSLA_CONTAINER_NETWORK_BRIDGE) + { + args.push_back("--net=bridge"); + } + // TODO: uncomment and implement when we have custom networks + /* else if (options.ContainerNetwork == WSLA_CONTAINER_NETWORK_CUSTOM) + { + args.push_back(std::format("--net={}", options.ContainerNetwork.ContainerNetworkName)); + } */ + else + { + // TODO: change the default to bridge once VHD has been fixed + args.push_back("--net=host"); + } + if (options.ShmSize > 0) { args.push_back(std::format("--shm-size={}m", options.ShmSize)); @@ -273,7 +296,7 @@ std::vector WSLAContainer::PrepareNerdctlCreateCommand(const WSLA_C args.push_back("all"); } - args.insert(args.end(), defaultNerdctlRunArgs.begin(), defaultNerdctlRunArgs.end()); + args.insert(args.end(), defaultNerdctlCreateArgs.begin(), defaultNerdctlCreateArgs.end()); args.insert(args.end(), inputOptions.begin(), inputOptions.end()); for (ULONG i = 0; i < options.InitProcessOptions.EnvironmentCount; i++) @@ -332,4 +355,4 @@ std::optional WSLAContainer::GetNerdctlStatus() // N.B. nerdctl inspect can return with exit code 0 and no output. Return an empty optional if that happens. return status.empty() ? std::optional{} : status; -} +} \ No newline at end of file diff --git a/src/windows/wslaservice/inc/wslaservice.idl b/src/windows/wslaservice/inc/wslaservice.idl index 2e31b205f..d7a733993 100644 --- a/src/windows/wslaservice/inc/wslaservice.idl +++ b/src/windows/wslaservice/inc/wslaservice.idl @@ -135,6 +135,20 @@ struct WSLA_PORT_MAPPING USHORT ContainerPort; }; +enum WSLA_CONTAINER_NETWORK_TYPE +{ + WSLA_CONTAINER_NETWORK_BRIDGE = 0, + WSLA_CONTAINER_NETWORK_HOST = 1, + WSLA_CONTAINER_NETWORK_NONE = 2, + // WSLA_CONTAINER_NETWORK_CUSTOM = 3 // TODO: Implement when implementing custom networks +}; +struct WSLA_CONTAINER_NETWORK +{ + // TODO: Change default to bridge when implemented + enum WSLA_CONTAINER_NETWORK_TYPE ContainerNetworkType; + LPCSTR ContainerNetworkName; +}; + enum WSLA_CONTAINER_FLAGS { WSLA_CONTAINER_FLAG_ENABLE_GPU = 1 @@ -154,6 +168,7 @@ struct WSLA_CONTAINER_OPTIONS // TODO: List specific GPU devices. // TODO: Flags on wether the caller wants to override entrypoint, args, or both. ULONGLONG ShmSize; + struct WSLA_CONTAINER_NETWORK ContainerNetwork; }; enum WSLA_CONTAINER_STATE diff --git a/test/windows/WSLATests.cpp b/test/windows/WSLATests.cpp index 384d4a83c..00db9000f 100644 --- a/test/windows/WSLATests.cpp +++ b/test/windows/WSLATests.cpp @@ -1141,7 +1141,13 @@ class WSLATests { WSLAContainerLauncher launcher( - "debian:latest", "test-default-entrypoint", "/bin/cat", {}, {}, ProcessFlags::Stdin | ProcessFlags::Stdout | ProcessFlags::Stderr); + "debian:latest", + "test-default-entrypoint", + "/bin/cat", + {}, + {}, + WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_HOST, + ProcessFlags::Stdin | ProcessFlags::Stdout | ProcessFlags::Stderr); // For now, validate that trying to use stdin without a tty returns the appropriate error. auto result = wil::ResultFromException([&]() { auto container = launcher.Launch(*session); }); @@ -1255,7 +1261,13 @@ class WSLATests // Create a stuck container. WSLAContainerLauncher launcher( - "debian:latest", "test-container-1", "sleep", {"sleep", "99999"}, {}, ProcessFlags::Stdout | ProcessFlags::Stderr); + "debian:latest", + "test-container-1", + "sleep", + {"sleep", "99999"}, + {}, + WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_HOST, + ProcessFlags::Stdout | ProcessFlags::Stderr); auto container = launcher.Launch(*session); @@ -1299,7 +1311,13 @@ class WSLATests { // Create a container WSLAContainerLauncher launcher( - "debian:latest", "test-container-2", "sleep", {"sleep", "99999"}, {}, ProcessFlags::Stdout | ProcessFlags::Stderr); + "debian:latest", + "test-container-2", + "sleep", + {"sleep", "99999"}, + {}, + WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_HOST, + ProcessFlags::Stdout | ProcessFlags::Stderr); auto container = launcher.Launch(*session); @@ -1333,7 +1351,13 @@ class WSLATests // Validate that container names are unique. { WSLAContainerLauncher launcher( - "debian:latest", "test-unique-name", "sleep", {"sleep", "99999"}, {}, ProcessFlags::Stdout | ProcessFlags::Stderr); + "debian:latest", + "test-unique-name", + "sleep", + {"sleep", "99999"}, + {}, + WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_HOST, + ProcessFlags::Stdout | ProcessFlags::Stderr); auto container = launcher.Launch(*session); VERIFY_ARE_EQUAL(container.State(), WslaContainerStateRunning); @@ -1383,11 +1407,150 @@ class WSLATests // Verify that the same name can be reused now that the container is deleted. WSLAContainerLauncher otherLauncher( - "debian:latest", "test-unique-name", "echo", {"OK"}, {}, ProcessFlags::Stdout | ProcessFlags::Stderr); + "debian:latest", + "test-unique-name", + "echo", + {"OK"}, + {}, + WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_HOST, + ProcessFlags::Stdout | ProcessFlags::Stderr); auto result = otherLauncher.Launch(*session).GetInitProcess().WaitAndCaptureOutput(); VERIFY_ARE_EQUAL(result.Output[1], "OK\n"); VERIFY_ARE_EQUAL(result.Code, 0); } } + + TEST_METHOD(ContainerNetwork) + { + WSL2_TEST_ONLY(); + SKIP_TEST_ARM64(); + + auto storagePath = std::filesystem::current_path() / "test-storage"; + + auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() { + std::error_code error; + + std::filesystem::remove_all(storagePath, error); + if (error) + { + LogError("Failed to cleanup storage path %ws: %hs", storagePath.c_str(), error.message().c_str()); + } + }); + + auto settings = GetDefaultSessionSettings(); + settings.NetworkingMode = WSLANetworkingModeNAT; + settings.StoragePath = storagePath.c_str(); + settings.MaximumStorageSizeMb = 1024; + + auto session = CreateSession(settings); + + auto expectContainerList = [&](const std::vector>& expectedContainers) { + wil::unique_cotaskmem_array_ptr containers; + + VERIFY_SUCCEEDED(session->ListContainers(&containers, containers.size_address())); + VERIFY_ARE_EQUAL(expectedContainers.size(), containers.size()); + + for (size_t i = 0; i < expectedContainers.size(); i++) + { + const auto& [expectedName, expectedImage, expectedState] = expectedContainers[i]; + VERIFY_ARE_EQUAL(expectedName, containers[i].Name); + VERIFY_ARE_EQUAL(expectedImage, containers[i].Image); + VERIFY_ARE_EQUAL(expectedState, containers[i].State); + } + }; + + // Verify that containers launch successfully when host and none are used as network modes + // TODO: Test bridge network container launch when VHD with bridge cni is ready + // TODO: Add port mapping related tests when port mapping is implemented + { + WSLAContainerLauncher launcher( + "debian:latest", + "test-network", + {}, + {"sleep", "99999"}, + {}, + WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_HOST, + ProcessFlags::Stdout | ProcessFlags::Stderr); + + auto container = launcher.Launch(*session); + VERIFY_ARE_EQUAL(container.State(), WslaContainerStateRunning); + auto result = ExpectCommandResult( + session.get(), + {"/usr/bin/nerdctl", "inspect", "-f", "'{{ index .Config.Labels \"nerdctl/networks\" }}'", "test-network"}, + 0); + VERIFY_ARE_EQUAL(result.Output[1], "'[\"host\"]'\n"); + + VERIFY_SUCCEEDED(container.Get().Stop(15, 50000)); + + expectContainerList({{"test-network", "debian:latest", WslaContainerStateExited}}); + + // Verify that the container is in exited state. + VERIFY_ARE_EQUAL(container.State(), WslaContainerStateExited); + + // Verify that deleting a container stopped via Stop() works. + VERIFY_SUCCEEDED(container.Get().Delete()); + + expectContainerList({}); + } + + { + WSLAContainerLauncher launcher( + "debian:latest", + "test-network", + {}, + {"sleep", "99999"}, + {}, + WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_NONE, + ProcessFlags::Stdout | ProcessFlags::Stderr); + + auto container = launcher.Launch(*session); + VERIFY_ARE_EQUAL(container.State(), WslaContainerStateRunning); + auto result = ExpectCommandResult( + session.get(), + {"/usr/bin/nerdctl", "inspect", "-f", "'{{ index .Config.Labels \"nerdctl/networks\" }}'", "test-network"}, + 0); + VERIFY_ARE_EQUAL(result.Output[1], "'[\"none\"]'\n"); + + VERIFY_SUCCEEDED(container.Get().Stop(15, 50000)); + + expectContainerList({{"test-network", "debian:latest", WslaContainerStateExited}}); + + // Verify that the container is in exited state. + VERIFY_ARE_EQUAL(container.State(), WslaContainerStateExited); + + // Verify that deleting a container stopped via Stop() works. + VERIFY_SUCCEEDED(container.Get().Delete()); + + expectContainerList({}); + } + + // Test bridge when ready + /* + { + WSLAContainerLauncher launcher( + "debian:latest", "test-network", {}, {"sleep", "99999"}, {}, WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_BRIDGE, ProcessFlags::Stdout | ProcessFlags::Stderr); + + auto container = launcher.Launch(*session); + VERIFY_ARE_EQUAL(container.State(), WslaContainerStateRunning); + auto result = ExpectCommandResult( + session.get(), + {"/usr/bin/nerdctl", "inspect", "-f", "'{{ index .Config.Labels \"nerdctl/networks\" }}'", "test-network"}, + 0); + VERIFY_ARE_EQUAL(result.Output[1], "'[\"bridge\"]'\n"); + + VERIFY_SUCCEEDED(container.Get().Stop(15, 50000)); + + expectContainerList({{"test-network", "debian:latest", WslaContainerStateExited}}); + + // Verify that the container is in exited state. + VERIFY_ARE_EQUAL(container.State(), WslaContainerStateExited); + + // Verify that deleting a container stopped via Stop() works. + VERIFY_SUCCEEDED(container.Get().Delete()); + + expectContainerList({}); + } + */ + } }; From e0153c3bd07a010680c67dd30d61012b80a62060 Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Fri, 12 Dec 2025 12:33:35 -0500 Subject: [PATCH 10/14] Change container network type to enum --- src/windows/common/WSLAContainerLauncher.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windows/common/WSLAContainerLauncher.h b/src/windows/common/WSLAContainerLauncher.h index 47f5e6ec5..ef79ae874 100644 --- a/src/windows/common/WSLAContainerLauncher.h +++ b/src/windows/common/WSLAContainerLauncher.h @@ -57,6 +57,6 @@ class WSLAContainerLauncher : private WSLAProcessLauncher private: std::string m_image; std::string m_name; - int m_containerNetworkType; + WSLA_CONTAINER_NETWORK_TYPE m_containerNetworkType; }; } // namespace wsl::windows::common \ No newline at end of file From 40007d7a841569a5bf5c8173623c8d5fbe3ae9e2 Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Mon, 15 Dec 2025 12:09:41 -0500 Subject: [PATCH 11/14] Implement code review feedback --- src/windows/wslaservice/exe/WSLAContainer.cpp | 35 ++++++++++--------- test/windows/WSLATests.cpp | 14 ++++++++ 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/windows/wslaservice/exe/WSLAContainer.cpp b/src/windows/wslaservice/exe/WSLAContainer.cpp index 7292bd69e..8d5b9d902 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.cpp +++ b/src/windows/wslaservice/exe/WSLAContainer.cpp @@ -101,7 +101,7 @@ void WSLAContainer::OnEvent(ContainerEvent event) HRESULT WSLAContainer::Stop(int Signal, ULONG TimeoutMs) try { - std::lock_guard lock(m_lock); + std::lock_guard lock(m_lock); if (State() == WslaContainerStateExited) { @@ -263,27 +263,28 @@ std::vector WSLAContainer::PrepareNerdctlCreateCommand(const WSLA_C args.push_back("--name"); args.push_back(options.Name); - if (options.ContainerNetwork.ContainerNetworkType == WSLA_CONTAINER_NETWORK_HOST) + switch (options.ContainerNetwork.ContainerNetworkType) { + case WSLA_CONTAINER_NETWORK_HOST: args.push_back("--net=host"); - } - else if (options.ContainerNetwork.ContainerNetworkType == WSLA_CONTAINER_NETWORK_NONE) - { + break; + case WSLA_CONTAINER_NETWORK_NONE: args.push_back("--net=none"); - } - else if (options.ContainerNetwork.ContainerNetworkType == WSLA_CONTAINER_NETWORK_BRIDGE) - { + break; + case WSLA_CONTAINER_NETWORK_BRIDGE: args.push_back("--net=bridge"); - } + break; // TODO: uncomment and implement when we have custom networks - /* else if (options.ContainerNetwork == WSLA_CONTAINER_NETWORK_CUSTOM) - { - args.push_back(std::format("--net={}", options.ContainerNetwork.ContainerNetworkName)); - } */ - else - { - // TODO: change the default to bridge once VHD has been fixed - args.push_back("--net=host"); + // case WSLA_CONTAINER_NETWORK_CUSTOM: + // args.push_back(std::format("--net={}", options.ContainerNetwork.ContainerNetworkName)); + // break; + default: + THROW_HR_MSG( + HRESULT_FROM_WIN32(ERROR_INVALID_STATE), + "No such network: type: %i, name: %hs", + options.ContainerNetwork.ContainerNetworkType, + options.ContainerNetwork.ContainerNetworkName); + break; } if (options.ShmSize > 0) diff --git a/test/windows/WSLATests.cpp b/test/windows/WSLATests.cpp index 123f7b72f..975a42db3 100644 --- a/test/windows/WSLATests.cpp +++ b/test/windows/WSLATests.cpp @@ -1524,6 +1524,20 @@ class WSLATests expectContainerList({}); } + { + WSLAContainerLauncher launcher( + "debian:latest", + "test-network", + {}, + {"sleep", "99999"}, + {}, + (WSLA_CONTAINER_NETWORK_TYPE)6, // WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_NONE, + ProcessFlags::Stdout | ProcessFlags::Stderr); + + auto retVal = launcher.LaunchNoThrow(*session); + VERIFY_ARE_EQUAL(retVal.first, HRESULT_FROM_WIN32(ERROR_INVALID_STATE)); + } + // Test bridge when ready /* { From 592439ccfc167642c9440079904dadd885e80d9b Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Mon, 15 Dec 2025 12:31:30 -0500 Subject: [PATCH 12/14] Fix merge issue: Pass container network type to WSLAContainerLauncher constructor --- test/windows/WSLATests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/windows/WSLATests.cpp b/test/windows/WSLATests.cpp index fee637f28..f4a054278 100644 --- a/test/windows/WSLATests.cpp +++ b/test/windows/WSLATests.cpp @@ -1593,7 +1593,7 @@ class WSLATests // Create a container. WSLAContainerLauncher launcher( - "debian:latest", "test-container-exec", {}, {"sleep", "99999"}, {}, ProcessFlags::Stdout | ProcessFlags::Stderr); + "debian:latest", "test-container-exec", {}, {"sleep", "99999"}, {}, WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_NONE, ProcessFlags::Stdout | ProcessFlags::Stderr); auto container = launcher.Launch(*session); From 4cf5c11f9a133c1398b1bdc23c3e0a09f21d4f4e Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Mon, 15 Dec 2025 13:37:22 -0500 Subject: [PATCH 13/14] Incorporate copilot code review suggestion --- src/windows/common/WSLAContainerLauncher.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windows/common/WSLAContainerLauncher.cpp b/src/windows/common/WSLAContainerLauncher.cpp index 31090f9bb..d355845d8 100644 --- a/src/windows/common/WSLAContainerLauncher.cpp +++ b/src/windows/common/WSLAContainerLauncher.cpp @@ -61,7 +61,7 @@ std::pair> WSLAContainerLauncher::L options.Name = m_name.c_str(); auto [processOptions, commandLinePtrs, environmentPtrs] = CreateProcessOptions(); options.InitProcessOptions = processOptions; - options.ContainerNetwork.ContainerNetworkType = static_cast(m_containerNetworkType); + options.ContainerNetwork.ContainerNetworkType = m_containerNetworkType; if (m_executable.empty()) { From 606dc8da80f23c4187a60301cd9d32ac842bf9fa Mon Sep 17 00:00:00 2001 From: Pooja Trivedi Date: Mon, 15 Dec 2025 14:42:39 -0500 Subject: [PATCH 14/14] Fix formatting and change hresult return to E_INVALIDARG for invalid network type being passed in --- src/windows/wslaservice/exe/WSLAContainer.cpp | 2 +- test/windows/WSLATests.cpp | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/windows/wslaservice/exe/WSLAContainer.cpp b/src/windows/wslaservice/exe/WSLAContainer.cpp index 547063407..af7a7ee41 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.cpp +++ b/src/windows/wslaservice/exe/WSLAContainer.cpp @@ -341,7 +341,7 @@ std::vector WSLAContainer::PrepareNerdctlCreateCommand(const WSLA_C // break; default: THROW_HR_MSG( - HRESULT_FROM_WIN32(ERROR_INVALID_STATE), + E_INVALIDARG, "No such network: type: %i, name: %hs", options.ContainerNetwork.ContainerNetworkType, options.ContainerNetwork.ContainerNetworkName); diff --git a/test/windows/WSLATests.cpp b/test/windows/WSLATests.cpp index f4a054278..afddfb71d 100644 --- a/test/windows/WSLATests.cpp +++ b/test/windows/WSLATests.cpp @@ -1549,7 +1549,7 @@ class WSLATests ProcessFlags::Stdout | ProcessFlags::Stderr); auto retVal = launcher.LaunchNoThrow(*session); - VERIFY_ARE_EQUAL(retVal.first, HRESULT_FROM_WIN32(ERROR_INVALID_STATE)); + VERIFY_ARE_EQUAL(retVal.first, E_INVALIDARG); } // Test bridge when ready @@ -1593,7 +1593,13 @@ class WSLATests // Create a container. WSLAContainerLauncher launcher( - "debian:latest", "test-container-exec", {}, {"sleep", "99999"}, {}, WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_NONE, ProcessFlags::Stdout | ProcessFlags::Stderr); + "debian:latest", + "test-container-exec", + {}, + {"sleep", "99999"}, + {}, + WSLA_CONTAINER_NETWORK_TYPE::WSLA_CONTAINER_NETWORK_NONE, + ProcessFlags::Stdout | ProcessFlags::Stderr); auto container = launcher.Launch(*session);