Repository navigation
Add option to disable Ryuk from code #3136
Description
I know that there is a way to disable this with env variable. But for my scenario I don't need Ryuk to be spawn every time I run test containers (I have separate gradle tasks to setup and destroy containers which summons Ryuk when it is needed). Currently I have to use really janky hacks to inject env variable form code. Other option is to set env variable on every machine for our gradle build to work normally which is not ideal either.
For everyone who is wondering what is the scenario. You really don't want to spawn Oracle 20Gb container with Apex and Ords and wait minutes for db to load every time you want to recheck typo in your integration test. Instead we spin containers once at the start and than they run until clean task is called.
Activity
bsideup commentedon Aug 23, 2020
Have you considered the reusable containers feature introduced in #1781?
asafm commentedon Oct 22, 2020
@bsideup I ran into another scenario recently answering to the same requirement: Turn off ryuk programmatically.
The scenario was this: We have a Benchmark test, which spins up multiple virtual machines (EC2) that has docker installed on them (Linux machine) - one serves as client which bomards the other machine which serves as the server being tested. the benchmark test, which is a a normal JUnit, spins up both the client docker image on the client machine and the server docker image on the server machine using docker client, which communicates over tcp to the remote docker daemon. This all works well, but it's highly verbose code.
I was trying out a new test, which uses GenericContainer which was much less verbose, but it failed on two reasons, but for clarity, I will focus in this ticket on the ryuk reason:
When starting RYUK, the container starts successfully, then it tries to connect to it's exposed port, which is randomly selected. Since I'm communicating from the machine running the benchmark test (can it an EC2 machine or my laptop), to the machine running ryuk, it goes through a firewall dictates by Amazon Security Group. Since the port is random, I cant really set one up in the security group.
Also, those server and client machines, are ephemeral - they are started in the beginning of the test and terminated when the test is done (it's quite rare to run those tests anyway since code is not changed that much), so we don't truly enjoy the benefit of ryuk, so no point in running it.
For those two reasons, I wanted a way to programmatically, turn off ryuk. Couple of options come to mind:
TestcontainersConfiguration- I can set a file on the classpath of the benchmark test module and problem solved. They only issue with this is that it's not "programmatic".WDYT?
My current work-around is the same as was suggested: Pretty ugly reflections hack (taken from JUnit Jupiter to modify Env Vars using Reflection, and un-modify it when I'm done).
bsideup commentedon Oct 22, 2020
@asafm what you described made me thinking that, for this very exotic use case, the existing option (environment variable) should be enough. Env var isn't a hack, because disabled Ryuk is a property of an environment, not project.
Ryuk (as well as the containers cleanup in general) is an important, essential part of Testcontainers. People are free to use the library the way they want to, but we don't want to provide an API that would allow doing bad things with the library. Even the environment variable to disable Ryuk was a bit of a trade off for exotic CI systems that are taking care of the cleanup themselves.
Since I think that reusable containers may/will solve @ksawerykarwacki's case, I am closing this issue. There are (inconvenient for you ,but still) options for disabling Ryuk, and also some workarounds/hacks, which makes me think that you're not blocking.
Providing a first class API to disable Ryuk would simply send a wrong message that we support disabling it. We don't :)
asafm commentedon Oct 22, 2020
Another use case, is simply working with remote docker daemon instead of your own to reduce load on your laptop. We have a script which instead spinning up dockers which are must have for tests (base) locally, we spin up a remote EC2 machine, and spin the dockers there. When running tests, some spin up specific docker images using GenericContainers. When working locally all good, but when you switch to this mode, you are again forced to exit IntelliJ and start it again, just for environment variable to kick in.
What am saying is that, environment can change, while working , be it locally, remote, etc. Having the option to set this disabled flag as part of the testcontainer config, or even a system property, would make it much easier to work with it.
The other option is to use plain Docker-client and not enjoy the benefit of the Testcontainer DSL to communicate Docker Daemon.
I think most people, the normal use case you're referring would hardly notice that extra option to disable it as System Property vs Env variable, as it would be called an Advance Setting. What I mean to say: It's not your public API as explicit as the Setters in GenericContainer or the a bit more hidden TestcontainersConfiguration. You have to dig deep in the code to know this option exists and then make a concious decision to turn it off, hence I think it's ok to not include it as API, thus still make 100% of users happy - the normal use case and the more exotic one, without sacrificing accidental misuse of this option, since it's not that public as the other configuration options are (I only mean this single addition of ability to set it via System Property).
I respect your and the rest of the library maintainers opinion and decision, what ever you decide of course.
kiview commentedon Oct 22, 2020
I agree with @bsideup that having an easily discoverable public API is a bit too much and a bit too potentially dangerous for normal users.
However, I think @asafm idea of adding an additional configuration mechanism, e.g. System Property, is not too involved and seems more or less symmetric to the ENV approach to me, while still being more accessible from within the JVM.
asafm commentedon Oct 22, 2020
Just to have TL; DR , the proposed change is:
from:
to
kiview commentedon Oct 22, 2020
Or rather
I assume ^^
asafm commentedon Oct 27, 2020
@bsideup WDYT?
bsideup commentedon Oct 27, 2020
I would still not add it. If there is a UX problem with Ryuk (e.g. the random port, although every other container would have a random port, too) then we should fix it.
IMO disabling Ryuk isn't how we should approach these UX issues.
As a last resort, there are projects like https://stefanbirkner.github.io/system-rules/ in case you really need to disable it programmatically bypassing the measures that we added.
asafm commentedon Nov 3, 2020
IMO both actions are valid: We can both fix the Ryuk UX of choosing a fixed port, and also allow the user to decide if they need to use RYUK, just like they configure any other feature they choose to use or not in Testcontainers.
Regarding port - Is it o.k. from your side if I open a PR / Issue to have the port of ryuk optionally fixed and supply it as a configuration parameter? (e.g. testcontainers.properties file)?
That's not always the case. In my scenario, some containers have fixed port, from the same reason I stated before - I need to communicate from one container to the other, each on it's own EC2 machine, there for I have to know the port in advance to setup the firewall between them (i.e. Security Group rules).
Regarding the disabling property of Ryuk: Your view is that terminating rouge containers started by Testcontainers - i.e. Ryuk - is an integral part of the system, in such a way that it's not configurable/optional other than per environment (i.e. environment variable).
My view is a bit different: I "fell in love" with this library a very long time ago because it gave you this concise, non-verbose, API to Docker, with additional behaviors which are exactly what you need when you're using Docker for testing purposes. As time goes by, more and more behaviors have been added, which augment the way you work. All of them AFAIK have been optional to use, to fit your need, and configurable at runtime via properties / parameters.
IMO, RYUK stands in contrast to this. It's another behavior added to the system, which is a must when running on shared build nodes, or your own laptop. Yet, there are cases - one-off build nodes for example, which don't require that behavior. The contrast here is that this behavior is non-configurable at runtime, only per environment (env vars). Fixing RYUK configuration to make it configurable via System Property or configuration file only aligns it with the other behaviors this library has added since @rnorth launched it.
bsideup commentedon Nov 6, 2020
Following @rnorth's vision was always the top priority for me :)
And, given that, I would say that "What would Richard do" here is to e.g. communicate with Ryuk over stdin/stdout (aka the already established channel via Docker) instead of opening a separate TCP connection on a random port - and that's something I am already researching ;)
asafm commentedon Nov 16, 2020
Sounds good. If you have an issue for that that I can watch , it will help me. Thanks @bsideup
bsideup commentedon Nov 24, 2020
@asafm I just submitted #3521 that uses
docker execinstead of the TCP connection to Ryukbsideup commentedon Nov 27, 2020
@asafm any chance you can try #3521 in your environment? :)