search

testcontainers / testcontainers-dotnet

A library to support tests with throwaway instances of Docker containers for all compatible .NET Standard versions.
https://dotnet.testcontainers.org
MIT License
3.65k stars 250 forks source link

[Enhancement]: Add Resource Reaper (Ryuk) WCOW support #1140

Open narcis-ro opened 3 months ago

narcis-ro commented 3 months ago

Testcontainers version

3.8.0 bet

Using the latest Testcontainers version?

Yes

Host OS

Windows

Host arch

X64

.NET version

8

Docker version

Latest

Docker info

latest

What happened?

Ryuk latest version supports Windows containers. Can the default ryuk version be updated to latest version please?

Relevant log output

No response

Additional information

No response

HofmeisterAn commented 3 months ago

I looked into it yesterday and prepared a branch that adds and integrates Ryuk's new version (with WCOW support). Unfortunately, I had forgotten about the limitations of GitHub-hosted runners/Microsoft-hosted agents on Windows (https://github.com/actions/runner-images/issues/6688). Without updating Docker's daemon configuration, we cannot mount the named pipe:

$daemonSettings = New-Object PSObject
$daemonSettings | Add-Member NoteProperty hosts @("npipe://")
$daemonSettings | Add-Member NoteProperty group "Users"
$daemonSettings | ConvertTo-Json | Out-File -FilePath "$($env:ProgramData)\docker\config\daemon.json" -Encoding ASCII
Get-Service -Name *docker* | Restart-Service

If we adjust the configuration before running the tests, Ryuk runs fine, but adding Ryuk support will break the tests for others (using the mentioned runners/agents) if they do not apply the same configuration. As long as the runners/agents do not support it, I do not think we can really add support for this feature (/cc @mdelapenya).

mdelapenya commented 3 months ago

@HofmeisterAn we must update the docs then. And be clear that without that configuration it won't work. wdyt?

HofmeisterAn commented 3 months ago

Developers can opt out in CI (ephemeral) using TESTCONTAINERS_RYUK_DISABLED. It is probably simpler than applying the configuration. This would be similar to existing configurations (that rely on the Linux Engine) too.

@HofmeisterAn we must update the docs then. And be clear that without that configuration it won't work. wdyt?

Yep, docs + a proper log message sounds good.

narcis-ro commented 3 months ago

The problem is that if the reaper detects it's windows, it won't start it, even with the custom config to use the new ryuk version

image

https://github.com/testcontainers/testcontainers-dotnet/blob/14f9ad92ece7b5539aed0c8f2f7c5415ab97aab8/src/Testcontainers/Configurations/TestcontainersSettings.cs

narcis-ro commented 3 months ago

Could we at least remove that if until complete support for WCOW ? This was at least we can use the new images of ruyk that supports windows

The problem is that if the reaper detects it's windows, it won't start it, even with the custom config to use the new ryuk version

image

https://github.com/testcontainers/testcontainers-dotnet/blob/14f9ad92ece7b5539aed0c8f2f7c5415ab97aab8/src/Testcontainers/Configurations/TestcontainersSettings.cs

HofmeisterAn commented 3 months ago

Could we at least remove that if until complete support for WCOW ? This was at least we can use the new images of ruyk that supports windows

I assume you are referring to the if-statement. Removing this alone is not sufficient to enable or support Ryuk on WCOW.