[Bug]: Unable to disable Ryuk in 0.0.32

[jonchammer]

Testcontainers version

0.0.32

Using the latest Testcontainers version?

Yes

Host OS

MacOS

Host arch

ARM

Go version

1.22

Docker version

Client:
 Version:           27.1.1
 API version:       1.46
 Go version:        go1.21.12
 Git commit:        6312585
 Built:             Tue Jul 23 19:54:12 2024
 OS/Arch:           darwin/arm64
 Context:           desktop-linux

Server: Docker Desktop 4.33.0 (160616)
 Engine:
  Version:          27.1.1
  API version:      1.46 (minimum version 1.24)
  Go version:       go1.21.12
  Git commit:       cc13f95
  Built:            Tue Jul 23 19:57:14 2024
  OS/Arch:          linux/arm64
  Experimental:     false
 containerd:
  Version:          1.7.19
  GitCommit:        2bf793ef6dc9a18e00cb12efb64355c2c9d5eb41
 runc:
  Version:          1.7.19
  GitCommit:        v1.1.13-0-g58aa920
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Docker info

Client:
 Version:    27.1.1
 Context:    desktop-linux
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.16.1-desktop.1
    Path:     /Users/jch002j/.docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.29.1-desktop.1
    Path:     /Users/jch002j/.docker/cli-plugins/docker-compose
  debug: Get a shell into any image or container (Docker Inc.)
    Version:  0.0.34
    Path:     /Users/jch002j/.docker/cli-plugins/docker-debug
  desktop: Docker Desktop commands (Alpha) (Docker Inc.)
    Version:  v0.0.14
    Path:     /Users/jch002j/.docker/cli-plugins/docker-desktop
  dev: Docker Dev Environments (Docker Inc.)
    Version:  v0.1.2
    Path:     /Users/jch002j/.docker/cli-plugins/docker-dev
  extension: Manages Docker extensions (Docker Inc.)
    Version:  v0.2.25
    Path:     /Users/jch002j/.docker/cli-plugins/docker-extension
  feedback: Provide feedback, right in your terminal! (Docker Inc.)
    Version:  v1.0.5
    Path:     /Users/jch002j/.docker/cli-plugins/docker-feedback
  init: Creates Docker-related starter files for your project (Docker Inc.)
    Version:  v1.3.0
    Path:     /Users/jch002j/.docker/cli-plugins/docker-init
  sbom: View the packaged-based Software Bill Of Materials (SBOM) for an image (Anchore Inc.)
    Version:  0.6.0
    Path:     /Users/jch002j/.docker/cli-plugins/docker-sbom
  scout: Docker Scout (Docker Inc.)
    Version:  v1.11.0
    Path:     /Users/jch002j/.docker/cli-plugins/docker-scout

Server:
 Containers: 0
  Running: 0
  Paused: 0
  Stopped: 0
 Images: 638
 Server Version: 27.1.1
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 2bf793ef6dc9a18e00cb12efb64355c2c9d5eb41
 runc version: v1.1.13-0-g58aa920
 init version: de40ad0
 Security Options:
  seccomp
   Profile: unconfined
  cgroupns
 Kernel Version: 6.10.0-linuxkit
 Operating System: Docker Desktop
 OSType: linux
 Architecture: aarch64
 CPUs: 10
 Total Memory: 15.6GiB
 Name: docker-desktop
 ID: 51665d58-29e3-4351-8667-02cd786cba3d
 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
 Labels:
  com.docker.desktop.address=unix:///Users/jch002j/Library/Containers/com.docker.docker/Data/docker-cli.sock
 Experimental: false
 Insecure Registries:
  hubproxy.docker.internal:5555
  127.0.0.0/8
 Live Restore Enabled: false

WARNING: daemon is not using the default seccomp profile

What happened?

Hello, all! We're upgrading one of our applications from testcontainers-go 0.0.31 to 0.0.32.

For various reasons, we've opted to disable Ryuk entirely by setting the TESTCONTAINERS_RYUK_DISABLED environment variable to true. (We're handling our own cleanup of all containers, and the application runs behind a corporate firewall, so we're not able to download public Docker images, like testcontainers/ryuk at runtime).

Our application is able to start up normally using testcontainers-go 0.0.31, and we can easily verify that Ryuk is not running via docker ps or the Docker desktop dashboard. When we upgrade to testcontainers-go 0.0.32 (including updating to v27.0.3+incompatible for github.com/docker/docker), it appears that the TESTCONTAINERS_RYUK_DISABLED environment variable is being ignored altogether, because testcontainers does start the Ryuk container, at least as long as testcontainers/ryuk:0.7.0 is present locally. Note that this is exactly the same codebase, with no changes except to the go.mod and go.sum files.

That Ryuk image leads to a related problem, as our machines will typically not have that Ryuk image cached locally, and they will have no ability to download it at runtime. This leads to crashes in the application with errors like these:

Error response from daemon: No such image: testcontainers/ryuk:0.7.0: creating network reaper failed: failed to create network

It looks to me like this is a regression that was introduced in 0.0.32, so the expectation here is that testcontainers would respect the TESTCONTAINERS_RYUK_DISABLED environment variable if set.

Relevant log output

No response

Additional information

Possibly relevant: our code is setting that environment variable as part of the main() function of the application, but before any testcontainers code is used.

E.g.

func main() {
    err := os.Setenv("TESTCONTAINERS_RYUK_DISABLED", "true")
    // Error handling
  
    // Other code
    // ...
  
    // Start leveraging testcontainers
    // ...
}

This is necessary in our situation because our users should not be responsible for knowing that the environment variable needs to be provided when they run the application. Ryuk will always be disabled for this application, so there's no reason for them to even know that it exists. (We'd much prefer some way to disable Ryuk programmatically, but that's more of a feature request than a bug).

All that being said, if testcontainers were examining that environment variable in (e.g.) an init() function, that could explain what's happening. (But that is pure speculation on my part.)

[stevenh]

I believe this is due to the internal config being initialised in logger.go which was introduced by 80e57e1.

As the config object is internal, you can't use the config.Reset() to reinitialise. So the only workaround for now would be to set the environment variable before running.

It might be possible to do this with an init hack but init ordering is tricky.

[jonchammer]

@stevenh, It sounds like this might be related to #2636 then. Similar questions about when configurations are read and all that. I 100% agree that adding more init() logic isn't ideal, though. That's pretty unlikely to be a long-term solution to the problem.

For what it's worth, I'm not sure I see the value added by telling the user that they're not using the garbage collector via the logger (referring to logger.go) . If disabling Ryuk is already an opt-in process (by setting the environment variable or updating the properties file), I think that the user would be fully aware of that decision on their end. Adding a log message is most likely going to tell them something they already know and will probably annoy them in the process (forcing them to build out a custom logger of some sort if they wanted to silence the warning). In our case, we're leveraging testcontainers as part of a larger application, so we certainly wouldn't want a "rogue" dependency taking over stdout/stderr.

Better might be to work towards allowing end users of the testcontainers library to specify everything about the configuration (e.g. which logger to use, whether Ryuk is enabled, etc.) upfront and programatically. That would help to reduce the number of edge cases like these that have to be dealt with and would likely be a bit more idiomatic in the Go world.

As it stands, I think the best course of action for our application is just to stay on 0.0.31 for the time being. We routinely update our dependencies as a means of trying to keep tech debt in check, but this one is non-critical, at least for now. We'd want to update eventually, but we can sit for a while if we have to.

P.S. Thanks so much for the quick response! Kudos for that. It's very much appreciated. :-D

[stevenh]

Oh I forgot to mention you can set ryuk.disabled=true in .testcontainers.properties as a workaround.

[stevenh]

Yes the same underlying issue as #2636

[stevenh]

@stevenh, It sounds like this might be related to #2636 then. Similar questions about when configurations are read and all that. I 100% agree that adding more init() logic isn't ideal, though. That's pretty unlikely to be a long-term solution to the problem.

For what it's worth, I'm not sure I see the value added by telling the user that they're not using the garbage collector via the logger (referring to logger.go) . If disabling Ryuk is already an opt-in process (by setting the environment variable or updating the properties file), I think that the user would be fully aware of that decision on their end. Adding a log message is most likely going to tell them something they already know and will probably annoy them in the process (forcing them to build out a custom logger of some sort if they wanted to silence the warning). In our case, we're leveraging testcontainers as part of a larger application, so we certainly wouldn't want a "rogue" dependency taking over stdout/stderr.

I agree, with the work done on the bug bash I was using testcontainers with the reaper disabled without any issues at all, it just needs a bit of care.

Better might be to work towards allowing end users of the testcontainers library to specify everything about the configuration (e.g. which logger to use, whether Ryuk is enabled, etc.) upfront and programatically. That would help to reduce the number of edge cases like these that have to be dealt with and would likely be a bit more idiomatic in the Go world.

I think that's a nice idea, I would suggest a functional options approach so we can do things like the following and is compatible with new options being added too.

// Just with a custom host.
testcontainers.Setup(testcontainers.WithHost("myhost"))

// With with Reaper disabled.
testcontainers.Setup(
    testcontainers.ReaperDisabled(),
    testcontainers.WithHost("myhost"),
)

// With a full config.
testcontainers.Setup(testcontainers.WithConfig(cfg))

Thoughts?

As it stands, I think the best course of action for our application is just to stay on 0.0.31 for the time being. We routinely update our dependencies as a means of trying to keep tech debt in check, but this one is non-critical, at least for now. We'd want to update eventually, but we can sit for a while if we have to.

PR up for review which fixes this. It does remove the warning so lets see what others think.