[Bug]: Containers are not cleaned-up with DisposeAsync when Resource Reaper is disabled

[JSkimming]

Testcontainers version

2.4.0

Using the latest Testcontainers version?

Yes

Host OS

Windows (WSL2)

Host arch

x64

.NET version

7.0.102

Docker version

Client:
 Cloud integration: v1.0.29
 Version:           20.10.22
 API version:       1.41
 Go version:        go1.18.9
 Git commit:        3a2c30b
 Built:             Thu Dec 15 22:36:18 2022
 OS/Arch:           windows/amd64
 Context:           default
 Experimental:      true

Server: Docker Desktop 4.16.3 (96739)
 Engine:
  Version:          20.10.22
  API version:      1.41 (minimum version 1.12)
  Go version:       go1.18.9
  Git commit:       42c8b31
  Built:            Thu Dec 15 22:26:14 2022
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.14
  GitCommit:        9ba4b250366a5ddde94bb7c9d1def331423aa323
 runc:
  Version:          1.1.4
  GitCommit:        v1.1.4-0-g5fd4c4d
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Docker info

Client:
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc., v0.10.0)
  compose: Docker Compose (Docker Inc., v2.15.1)
  dev: Docker Dev Environments (Docker Inc., v0.0.5)
  extension: Manages Docker extensions (Docker Inc., v0.2.17)
  sbom: View the packaged-based Software Bill Of Materials (SBOM) for an image (Anchore Inc., 0.6.0)
  scan: Docker Scan (Docker Inc., v0.23.0)

Server:
 Containers: 0
  Running: 0
  Paused: 0
  Stopped: 0
 Images: 11
 Server Version: 20.10.22
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runtime.v1.linux runc io.containerd.runc.v2
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 9ba4b250366a5ddde94bb7c9d1def331423aa323
 runc version: v1.1.4-0-g5fd4c4d
 init version: de40ad0
 Security Options:
  seccomp
   Profile: default
 Kernel Version: 5.15.68.1-microsoft-standard-WSL2
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 32
 Total Memory: 31.32GiB
 Name: docker-desktop
 ID: FNB2:KOOG:BBSV:V7DY:H5ZR:6ZPE:ZBP5:3E36:NNTL:MZUA:X2B5:O5CO
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 No Proxy: hubproxy.docker.internal
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  hubproxy.docker.internal:5000
  127.0.0.0/8
 Live Restore Enabled: false

WARNING: No blkio throttle.read_bps_device support
WARNING: No blkio throttle.write_bps_device support
WARNING: No blkio throttle.read_iops_device support
WARNING: No blkio throttle.write_iops_device support

What happened?

Our CI uses BitBucket pipelines, and following the instructions here, I disabled Resource Reaper.

Sometime later, I discovered the containers were not being cleaned up, and after further investigation, I started calling CleanUpAsync() before calling DisposeAsync().

I upgraded to 2.4.0 today and started following the obsolete warnings (we treat warnings as errors), and I removed CleanUpAsync() as it has been marked as obsolete. If I upgrade to IContainer, it has no CleanUpAsync() method.

I have again discovered that containers are not cleaned up when calling DisposeAsync().

So I'm stuck in a bind.

1. I can selectively ignore the obsolete warnings and continue to call CleanUpAsync().
2. I can Accept containers are not cleaned up.
3. I can move away from BitBucket pipelines and enable Resource Reaper.
4. I can stick with v2.3.0

None of the options is ideal. I'm going for option 4 for now.

Relevant log output

No response

Additional information

Here is our (slightly simplified) xunit test fixture

public abstract class EventStoreContainerFixture : IAsyncLifetime
{
    private TestcontainersContainer _eventStoreContainer;

    protected EventStoreContainerFixture()
    {
        HostPort = 2113;
        ConnectionString = $"esdb://admin:changeit@localhost:{HostPort}?tls=false";

        // We get the following error in BitBucket pipelines. Setting ResourceReaperEnabled to false fixes it.
        // {"message":"authorization denied by plugin pipelines: --mounts is not allowed"}
        // See: https://github.com/testcontainers/testcontainers-dotnet/issues/492#issuecomment-1167174986
        TestcontainersSettings.ResourceReaperEnabled = false;

        _eventStoreContainer = new TestcontainersBuilder<TestcontainersContainer>()
            .WithImage("eventstore/eventstore:20.10.5-bionic")
            .WithCommand("--insecure")
            .WithCommand("--enable-external-tcp")
            .WithPortBinding(HostPort, 2113)
            .WithWaitStrategy(Wait.ForUnixContainer().UntilPortIsAvailable(2113))
            .Build();
    }

    public int HostPort { get; }

    public string ConnectionString { get; }

    public Task InitializeAsync()
    {
        return _eventStoreContainer.StartAsync();
    }

    public async Task DisposeAsync()
    {
        // Calling CleanUpAsync is supposed to be unnecessary, but our testing has shown the containers are left behind.
        // https://github.com/testcontainers/testcontainers-dotnet/discussions/410
        await _eventStoreContainer.CleanUpAsync();
        await _eventStoreContainer.DisposeAsync();
    }
}

[HofmeisterAn]

I agree, this is a bit unfortunate. I must admit that I did not have something like Bitbucket in mind when I made the change. I will try to add more information, and perhaps we can come up with a better implementation.

Disabling Ryuk is not recommended, as it is the only reliable way to clean up the environment. DisposeAsync() follows the .NET best practices, but it is not entirely reliable. If the test process crashes or dies, it will not clean up anything. Not using Ryuk can cause problems on developers' machines and CI environments.

This is not a significant issue on ephemeral CI, but I am not very familiar with the Bitbucket CI environment. Is it ephemeral? If not, it can get messy. DisposeAsync() will only clean up if a Resource Reaper (Ryuk) is assigned to the resources. Since it is disabled, it is not assigned, and therefore the container remains.

@eddumelendez @kiview do you know how Java does the "clean up" on Bitbucket? I would assume there is no clean up either, right?

[JSkimming]

@HofmeisterAn I've not investigated, though I suspect the Bitbucket CI environment is ephemeral. There's support for docker build image caching, though I suspect containers don't last long. Containers would soon pile up if they didn't clean up somehow.

I'll investigate further, I could add a check and only disable Ryuk in the CI.

Though I'd advocate a belt-and-braces approach, by allowing for explicit clean-up were possible and using Ryuk as a fallback.

[HofmeisterAn]

I think we can assign a Resource Reappear session ID to each resource, which is essentially the same as WithCleanUp(true). By doing this, DisposeAsync() will remove the resource, like CleanUpAsync() does in version 2.3.0. Up until now, we have utilized the Resource Reappear session ID to determine whether we need to create and start Ryuk. However, we can replace this with TestcontainersSettings.ResourceReaperEnabled. I am able to create a fix for this, and would like to ask if you could test it?

[JSkimming]

I am able to create a fix for this, and would like to ask if you could test it?

Of course, happy to test a fix. Just let me know.

[HofmeisterAn]

I've prepared a fix in #783 (bugfix/783-dispose-container-if-rr-is-disabled). Can you test the branch in your environment?

[JSkimming]

@HofmeisterAn I've just seen your reply and tested it against the latest develop branch. I can confirm it works wonderfully 🎉.

Here is the updated (slightly simplified) xunit test fixture

public class EventStoreContainerFixture : IAsyncLifetime
{
    private IContainer _eventStoreContainer;

    public EventStoreContainerFixture()
    {
        HostPort = 2113;
        ConnectionString = $"esdb://admin:changeit@localhost:{HostPort}?tls=false";

        // We get the following error in BitBucket pipelines. Setting ResourceReaperEnabled to false fixes it.
        // {"message":"authorization denied by plugin pipelines: --mounts is not allowed"}
        // See: https://github.com/testcontainers/testcontainers-dotnet/issues/492#issuecomment-1167174986
        TestcontainersSettings.ResourceReaperEnabled = false;

        _eventStoreContainer = new ContainerBuilder()
            .WithImage("eventstore/eventstore:20.10.5-bionic")
            .WithCommand("--insecure")
            .WithCommand("--enable-external-tcp")
            .WithPortBinding(HostPort, 2113)
            .WithWaitStrategy(Wait.ForUnixContainer().UntilPortIsAvailable(2113))
            .Build();
    }

    public int HostPort { get; }

    public string ConnectionString { get; }

    public Task InitializeAsync()
    {
        return _eventStoreContainer.StartAsync();
    }

    public Task DisposeAsync()
    {
        return _eventStoreContainer.DisposeAsync();
    }
}

[HofmeisterAn]

I would like to point out once again that disabling the Resource Reaper is not recommended. However, in your case, running on BitBucket, there may be no other option.

Therefore, I suggest that you set the TESTCONTAINERS_RYUK_DISABLED environment variable in your CI and do not touch the development configurations.

[JSkimming]

Duly noted, thank you.