feat: allow overriding the session ID

[jcmfernandes]

What does this PR do?

Makes the session ID configurable, allowing for long-running sessions.

Why is it important?

Speaking of my use-case, Apache Pulsar takes a long time to boot, so I wanted testcontainers to boot it up, eventually take it down, but allow me to use the same container across several runs. This wasn't the case, since I was getting a new reaper running on every test run. I wanted a rolling-window behavior, hence this change.

Related issues

• Closes [Feature]: Support a reuseWindow for cleaning up reusable containers if no tests run within a certain duration #2804

Follow-ups

Update the documentation.

[netlify]

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	50fa9f3
🔍 Latest deploy log	https://app.netlify.com/projects/testcontainers-go/deploys/68c5f3deee13a200084e7fa1
😎 Deploy Preview	https://deploy-preview-3051--testcontainers-go.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[kiview]

Hey @jcmfernandes, I am actually surprised your setup doesn't work with the current Test Session Semantics.

Also, your change doesn't take the parent PID into account anymore, so it would be a breaking change for downstream integrations, that rely on the current session ID semantics.

[jcmfernandes]

Hey @jcmfernandes, I am actually surprised your setup doesn't work with the current Test Session Semantics.

Also, your change doesn't take the parent PID into account anymore, so it would be a breaking change for downstream integrations, that rely on the current session ID semantics.

Thanks for getting back to me.

Without these changes, every go test ./... run create a new instance of the reaper, and I'm not alone (#2804).

As stated in the PR description:

Regarding point 3, I must admit that I don't understand the advantages of the hashing logic I ended up deleting. I'm happy to revert it, though.

I'm happy to revert those changes, but so far, setting the session ID myself (through an env var) has been the only solution that worked for me.

[kiview]

Without these changes, every go test ./... run create a new instance of the reaper

Alright, that is how it is supposed to work.
But what it sounds to me from what you are trying to achieve, it sounds you need Reusable Containers.

Either way, the semantics around sessions IDs and cleanup behavior are fairly involved. I'd suggest to not proceed with this work without specifically syncing with @mdelapenya and @stevenh on the design.

[jcmfernandes]

Without these changes, every go test ./... run create a new instance of the reaper

Alright, that is how it is supposed to work. But what it sounds to me from what you are trying to achieve, it sounds you need Reusable Containers.

Either way, the semantics around sessions IDs and cleanup behavior are fairly involved. I'd suggest to not proceed with this work without specifically syncing with @mdelapenya and @stevenh on the design.

Thanks for the pointers. @mdelapenya and @stevenh, your input is very much welcome.

But what it sounds to me from what you are trying to achieve, it sounds you need Reusable Containers.

I am using a reusable container, and the container was always reused as expected; no issues there. The problem is that the reaper isn't reused because the session ID keeps changing.

[jcmfernandes]

Also, your change doesn't take the parent PID into account anymore, so it would be a breaking change for downstream integrations, that rely on the current session ID semantics.

I pushed a new commit that brings back the logic I had previously removed. This shouldn't break things for anyone anymore.

[stevenh]

Hi @jcmfernandes can you clarify to me the problem you're trying to solve?

Your description seems to indicate to me you want to share containers between tests due to the slow startup time?

[jcmfernandes]

Hi @jcmfernandes can you clarify to me the problem you're trying to solve?

Your description seems to indicate to me you want to share containers between tests due to the slow startup time?

Hi @stevenh! Correct. In other words, I want:

1. Reusable containers (all good!)
2. Reusable reapers for reusable containers (not good)

Copying from #2804, this is what I see when I enable reusable containers:

CONTAINER ID  IMAGE                                                  COMMAND     CREATED             STATUS             PORTS                                                                   NAMES
b551cb564516  docker.io/testcontainers/ryuk:0.8.1                    /bin/ryuk   2 minutes ago       Up 2 minutes       0.0.0.0:39033->8080/tcp, 8080/tcp                                       reaper_42d84025974814469f367f10224b4657f7ad888e2a5d827e76703f06292c39ec
5a7876b38a26  docker.io/mysql:8.0.36                                 mysqld      2 minutes ago       Up 2 minutes       0.0.0.0:42559->3306/tcp, 0.0.0.0:44633->33060/tcp, 3306/tcp, 33060/tcp  tests-mysql
c93fdf315582  docker.io/testcontainers/ryuk:0.8.1                    /bin/ryuk   2 minutes ago       Up 2 minutes       0.0.0.0:34995->8080/tcp, 8080/tcp                                       reaper_3ccf995c484a246a36990a92f85c3fc17ffb3912f867ba70b029aaf323f48547
e63820a62ce3  docker.io/testcontainers/ryuk:0.8.1                    /bin/ryuk   2 minutes ago       Up 2 minutes       0.0.0.0:44359->8080/tcp, 8080/tcp                                       reaper_fe49fa57fcc7031f3d1e9dbb6f305a2f4b392d1b10921d42b6a0df56e4970667
56d082f1fbbd  docker.io/testcontainers/ryuk:0.8.1                    /bin/ryuk   About a minute ago  Up About a minute  0.0.0.0:38461->8080/tcp, 8080/tcp                                       reaper_669ec11b7226f1c446fddd1a0e3f1aadfd1627687ed6adf3b5c846e66ecbe276
9922c527850c  docker.io/testcontainers/ryuk:0.8.1                    /bin/ryuk   About a minute ago  Up About a minute  0.0.0.0:34041->8080/tcp, 8080/tcp                                       reaper_74976de06285397df7d0edffbdd2e5fd284cf833f5b3de025705fbcad5e3608b
c76bcd09a892  docker.io/testcontainers/ryuk:0.8.1                    /bin/ryuk   52 seconds ago      Up 52 seconds      0.0.0.0:32785->8080/tcp, 8080/tcp                                       reaper_6e46bea7477b04a07464880892df435bdbd55753551da0d80b51d8917f4ab094
df476617ebdf  docker.io/testcontainers/ryuk:0.8.1                    /bin/ryuk   42 seconds ago      Up 42 seconds      0.0.0.0:39403->8080/tcp, 8080/tcp                                       reaper_24fdf990fc0d0ce6e34d030f619611233f7b7e714e623e1b6c635f7a85fdee4a
fc398aaabe82  docker.io/testcontainers/ryuk:0.8.1                    /bin/ryuk   27 seconds ago      Up 27 seconds      0.0.0.0:34295->8080/tcp, 8080/tcp                                       reaper_f9596c3bfefde5404c227e99286798894109b8d3b1de2fe207580a1baf340ac6
8ac0c88b5298  docker.io/testcontainers/ryuk:0.8.1                    /bin/ryuk   18 seconds ago      Up 18 seconds      0.0.0.0:42809->8080/tcp, 8080/tcp                                       reaper_8a5edab68adc986d01ff49f0774b013c3933ee8854bda319df02d26dfa233ded
64edfcfa3ec0  docker.io/testcontainers/ryuk:0.8.1                    /bin/ryuk   8 seconds ago       Up 9 seconds       0.0.0.0:35387->8080/tcp, 8080/tcp                                       reaper_3d76e1e0b630828fad795b4fcc603edaf11179da4d56148248f89862e92139d1

I want to see one reaper for the reusable container. As a side effect, having one reaper creates a “rolling window” behavior, where the reaper resets its timeout upon a new connection to the container, which is precisely what I would like to see: keep the container up and running between test runs and terminate it after the defined period of inactivity.

Am I making sense?

[kiview]

Oh yes, that makes absolutely sense and I believe this is more of a bug and something that Testcontainers should already handle correctly, rather than having a user to specify a session id manually.

[stevenh]

Thanks for the confirmation @jcmfernandes.

To facilitate this are you intending to set a long ReconnectionTimeout for all containers? I ask as if so I think thats going to prevent clean up of all other containers too, which I'm not sure is your intention?

[jcmfernandes]

@stevenh yes, because I want to keep the testcontainer running for the entirety of my work session.

I usually start a watcher that runs tests automatically as I save changes. I want the testcontainer to boot on the first test run and to stay around for, say, one hour, waiting for new connections. Furthermore, I then want new connections to reset that timeout. Example.

1. Run a test that starts the testcontainer at 9:05 AM with a reconnection timeout of 1 hour. Ryuk must wait for new connections until 10:05 AM, and GC the testcontainer if that doesn't happen.
2. Run another test at 9:35 AM --> I now want ryuk to wait until 10:35 AM for new connections.

This is precisely the behavior I get by manually setting the session ID and RYUK_RECONNECTION_TIMEOUT to 1 hour. Now, I'm happy to discuss ways to make the session ID stable and auto-generated.

[stevenh]

Thanks again for the clarification, the challenge I see with that is not only does it impact the long lived container but all containers for example:

You run a test which starts the long lived DB container as well a short lived test container. The desired behaviour is the DB container stays running between tests, but the test container itself exits after the test is run.

However in the case where the test fails and the short lived contain didn't exit as expected, with a standard ryuk setup this wouldn't be an issue as the test container would be quickly cleaned up, but with RYUK_RECONNECTION_TIMEOUT set to one hour no clean up will happen until that hour expires.

Does this make sense?

To handle this situation properly I think we'd need more fine grained control of when specific containers are cleaned up.

[jcmfernandes]

Thanks for the prompt reply, @stevenh.

That's a fair concern — it seems that testcontainers is missing some kind of "group" concept to which certain GC options apply. Currently, that's a "session", and this PR won't change it. That is, I don't see how this PR makes things worse regarding your concern, and it solves an actual reported issue.

[mdelapenya]

Hi @jcmfernandes, thanks for opening this PR, and following-up after @kiview's comments. I'm returning today after summer break, working on the open PRs/issues in the repo.

I'm not against the changes in this PR, although it introduces some breaking changes (BCs) after removing public API functions. See my comments for that. We could avoid it offering a deprecation path when/if possible, although we sometimes introduce BCs if needed.

I'd rename the PR to feat: allow overriding the sessionID and would focus on that, only. The added value if we do this is we would also enable Bazel folks to contribute their own env var for the session ID at build time. Nevertheless, we must check how the reaper behaves in that scenario. As discussed here, it's by design that the reaper is spawned just once per test session (a shell), but it's true that the design could be inconsistent with reusable containers used in multiple shells (different session IDs). Sharing state, even in tests, is a complex thing to maintain, so we probably need a more robust design for both features: reusable containers and the reaper, and it could need dedicated work on that direction.

@kiview @stevenh wdyt?

[jcmfernandes]

@mdelapenya thanks for your input. I modified this PR, making it focus solely on allowing devs to set the session ID, and I opened a separate PR #3269 where I delete TestcontainersConfig.

[stevenh]

I'm concerned calling config.Read in init could have some unintended side effects which are impossible to address, something we'll need to confirm.

[stevenh]

@mdelapenya could calling config.Read() in init cause undesired behaviour due to how early it called?

I'm thinking this would prevent things like changing the environment variables in a test from having the desired effect as an example.

[jcmfernandes]

Thanks for the feedback, @stevenh. I understand your concern: reading the config in an init() function felt wrong.

I change the code so that config.Read falls back to the session in core/bootstrap (I moved bootstrap.go to a new package to avoid import cycle between internal/config and internal/core). Please let me know what you think.