diff --git a/localization/strings/en-US/Resources.resw b/localization/strings/en-US/Resources.resw index 42030d93b..553ea3950 100644 --- a/localization/strings/en-US/Resources.resw +++ b/localization/strings/en-US/Resources.resw @@ -2126,6 +2126,10 @@ For privacy information about this product please visit https://aka.ms/privacy.< Failed to create volume '{}': {} {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + + Port {} is already in use, cannot start container {} + {FixedPlaceholder="{}"}Command line arguments, file names and string inserts should not be translated + Both Dockerfile and Containerfile found. Use -f to select the file to use {FixedPlaceholder="Dockerfile"}{FixedPlaceholder="Containerfile"}{FixedPlaceholder="-f"}Command line arguments, file names and string inserts should not be translated diff --git a/src/windows/wslc/services/ContainerService.cpp b/src/windows/wslc/services/ContainerService.cpp index df6ada836..b6da3b35a 100644 --- a/src/windows/wslc/services/ContainerService.cpp +++ b/src/windows/wslc/services/ContainerService.cpp @@ -291,7 +291,6 @@ int ContainerService::Run(Session& session, const std::string& image, ContainerO { // Create the container auto runningContainer = CreateInternal(session, image, runOptions); - runningContainer.SetDeleteOnClose(false); auto& container = runningContainer.Get(); // Start the created container @@ -299,6 +298,9 @@ int ContainerService::Run(Session& session, const std::string& image, ContainerO WI_SetFlagIf(startFlags, WSLCContainerStartFlagsAttach, !runOptions.Detach); THROW_IF_FAILED(container.Start(startFlags, nullptr)); // TODO: Error message, detach keys + // Disable auto-delete only after successful start + runningContainer.SetDeleteOnClose(false); + // Handle attach if requested if (WI_IsFlagSet(startFlags, WSLCContainerStartFlagsAttach)) { diff --git a/src/windows/wslcsession/WSLCContainer.cpp b/src/windows/wslcsession/WSLCContainer.cpp index 47080b2ad..a5541365c 100644 --- a/src/windows/wslcsession/WSLCContainer.cpp +++ b/src/windows/wslcsession/WSLCContainer.cpp @@ -231,6 +231,16 @@ std::string ExtractContainerName(const std::vector& names, const st return CleanContainerName(names[0]); } +std::string FormatPortEndpoint(const ContainerPortMapping& portMapping) +{ + auto addr = portMapping.VmMapping.BindingAddressString(); + return std::format( + "{}:{}/{}", + portMapping.VmMapping.IsIPv6() ? std::format("[{}]", addr) : addr, + portMapping.VmMapping.HostPort(), + portMapping.ProtocolString()); +} + WSLCContainerMetadataV1 ParseContainerMetadata(const std::string& json) { auto wrapper = wsl::shared::FromJson(json.c_str()); @@ -1497,12 +1507,10 @@ void WSLCContainerImpl::MapPorts() auto allocatedPort = m_virtualMachine.TryAllocatePort(e.ContainerPort, e.VmMapping.BindAddress.si_family, e.VmMapping.Protocol); - THROW_HR_IF_MSG( + THROW_HR_WITH_USER_ERROR_IF( HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), - !allocatedPort, - "Port %hu is in use, cannot start container %hs", - e.ContainerPort, - m_id.c_str()); + wsl::shared::Localization::MessageWslcPortInUse(FormatPortEndpoint(e), m_id), + !allocatedPort); e.VmMapping.AssignVmPort(allocatedPort); @@ -1510,7 +1518,20 @@ void WSLCContainerImpl::MapPorts() } } - m_virtualMachine.MapPort(e.VmMapping); + try + { + m_virtualMachine.MapPort(e.VmMapping); + } + catch (...) + { + auto result = wil::ResultFromCaughtException(); + if (result == HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS) || result == HRESULT_FROM_WIN32(WSAEADDRINUSE)) + { + THROW_HR_WITH_USER_ERROR( + HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), wsl::shared::Localization::MessageWslcPortInUse(FormatPortEndpoint(e), m_id)); + } + throw; + } } } diff --git a/src/windows/wslcsession/WSLCVirtualMachine.cpp b/src/windows/wslcsession/WSLCVirtualMachine.cpp index 734d0d049..be404eab2 100644 --- a/src/windows/wslcsession/WSLCVirtualMachine.cpp +++ b/src/windows/wslcsession/WSLCVirtualMachine.cpp @@ -167,6 +167,11 @@ bool VMPortMapping::IsLocalhost() const } } +bool VMPortMapping::IsIPv6() const +{ + return BindAddress.si_family == AF_INET6; +} + uint16_t VMPortMapping::HostPort() const { if (BindAddress.si_family == AF_INET6) diff --git a/src/windows/wslcsession/WSLCVirtualMachine.h b/src/windows/wslcsession/WSLCVirtualMachine.h index 99cd515ec..b5a3ebff5 100644 --- a/src/windows/wslcsession/WSLCVirtualMachine.h +++ b/src/windows/wslcsession/WSLCVirtualMachine.h @@ -86,6 +86,7 @@ struct VMPortMapping void Unmap(); void Release(); bool IsLocalhost() const; + bool IsIPv6() const; std::string BindingAddressString() const; void Attach(WSLCVirtualMachine& Vm); void Detach(); diff --git a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp index 8b22bce11..563c2f862 100644 --- a/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp +++ b/test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp @@ -40,6 +40,7 @@ class WSLCE2EContainerRunTests TEST_CLASS_CLEANUP(ClassCleanup) { EnsureContainerDoesNotExist(WslcContainerName); + EnsureContainerDoesNotExist(WslcContainerName2); EnsureImageIsDeleted(DebianImage); EnsureImageIsDeleted(PythonImage); @@ -54,6 +55,7 @@ class WSLCE2EContainerRunTests TEST_METHOD_SETUP(TestMethodSetup) { EnsureContainerDoesNotExist(WslcContainerName); + EnsureContainerDoesNotExist(WslcContainerName2); EnvTestFile1 = wsl::windows::common::filesystem::GetTempFilename(); EnvTestFile2 = wsl::windows::common::filesystem::GetTempFilename(); @@ -384,9 +386,6 @@ class WSLCE2EContainerRunTests WSLC_TEST_METHOD(WSLCE2E_Container_Run_PortAlreadyInUse) { - // Bug: https://github.com/microsoft/WSL/issues/14448 - SKIP_TEST_NOT_IMPL(); - // Start a container with a simple server listening on a port auto result1 = RunWslc(std::format( L"container run -d --name {} -p {}:{} {} {}", @@ -397,9 +396,28 @@ class WSLCE2EContainerRunTests GetPythonHttpServerScript(ContainerTestPort))); result1.Verify({.Stderr = L"", .ExitCode = 0}); - // Attempt to start another container mapping the same host port - auto result2 = RunWslc(std::format(L"container run -p {}:{} {}", HostTestPort1, ContainerTestPort, DebianImage.NameAndTag())); - result2.Verify({.ExitCode = 1}); + // Create a second container mapping the same host port to validate the full error message + auto createResult = + RunWslc(std::format(L"container create -p {}:{} {}", HostTestPort1, ContainerTestPort, DebianImage.NameAndTag())); + createResult.Verify({.Stderr = L"", .ExitCode = 0}); + auto containerId = createResult.GetStdoutOneLine(); + + // Attempt to start — should fail with port conflict + auto startResult = RunWslc(std::format(L"container start {}", containerId)); + startResult.Verify( + {.Stderr = std::format( + L"Port 127.0.0.1:{}/tcp is already in use, cannot start container {}\r\nError code: ERROR_ALREADY_EXISTS\r\n", HostTestPort1, containerId), + .ExitCode = 1}); + + // Clean up the created container + RunWslc(std::format(L"container rm {}", containerId)).Verify({.Stderr = L"", .ExitCode = 0}); + + // Verify 'container run' auto-cleans up on port conflict (no ghost container) + auto runResult = RunWslc(std::format( + L"container run --name {} -p {}:{} {}", WslcContainerName2, HostTestPort1, ContainerTestPort, DebianImage.NameAndTag())); + runResult.Verify({.ExitCode = 1}); + + VerifyContainerIsNotListed(WslcContainerName2); } // https://github.com/microsoft/WSL/issues/14433 @@ -538,6 +556,7 @@ class WSLCE2EContainerRunTests private: // Test container name const std::wstring WslcContainerName = L"wslc-test-container"; + const std::wstring WslcContainerName2 = L"wslc-test-container-2"; // Test environment variables const std::wstring HostEnvVariableName = L"WSLC_TEST_HOST_ENV";