Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for working with already running containers #506

Closed
mathiasbn opened this issue Jul 5, 2022 · 3 comments
Closed

Support for working with already running containers #506

mathiasbn opened this issue Jul 5, 2022 · 3 comments
Labels
duplicate This issue or pull request already exists enhancement New feature or request help wanted Extra attention is needed

Comments

@mathiasbn
Copy link

mathiasbn commented Jul 5, 2022

Is your feature request related to a problem? Please describe.
When running integration tests with containers, I would like to reuse a running container on my local machine. So as not to recreate it between every test run. So along with StartAsync():

I propose a EnsureStartedAsync(Filter? filter)

Describe the solution you'd like
As of v2.0.1 it seems to me that a TestcontainersContainer's internally need two things.

  • An ITestcontainersConfiguration which is hidden from public inside the builders
  • An ContainerInspectResponse representing the running container

So in order to accomplish my "ensure started" method I actually have everything.

The test will use TestcontainersBuilder to construct an ITestcontainersContainer with a configuration inside.
If the container is running, I can easily do a ListContainersAsync and InspectContainerAsync to get the ContainerInspectResponse. (in this case, some kind of filter could be supplied)
If the container is not running, I can just call startAsync.

Describe alternatives you've considered
So if this solution is to "specialized" or niche, an alternative could be to open up the API a bit. At the moment it is very closed for extensions.
From an package user perspective I have the following problems:

  • I cannot get a hold of the ITestcontainersConfiguration, it is private inside the builder and inside the testcontainer
  • Without the configuration I cannot extend TestcontainersContainer or any of its subtypes (it takes a configuration in the constructor)
  • The ContainerInspectResponse container of TestcontainersContainer is private and I can't find a way to "extend my way" to set it. Therefore I cannot represent the "running state"

Additional context
I have not used this lib for a long time, so please bare with me, if I missed something obvious.
But working with this lib, I have banged my head against internal classes, and private fields in hierarchies a lot. Of cause you should not expose internal state, but is all this exactly internal?
Couldn't some of it be made public, and thereby improve how extensible this library is?

@HofmeisterAn
Copy link
Collaborator

Relates to #407. Assigning an existing or running container (ContainerInspectResponse) to the respective field in TestcontainersContainer sounds good. Then we can decide if it's even necessary to create and (or) start a container. We just need something to identify a container reliable. A contribution and pull request is more than welcome. Maybe someone else from the other issue would like to join.

I have banged my head against internal classes, and private fields in hierarchies a lot.

If you have anything better in mind, please share your ideas, proposals and contribute. Anything that improves the development experience and makes Testcontainers more convenient is welcome. However, just exposing for example the remote Docker API doesn't add much value, IMO. Then devs can deal straight with the REST API.

@HofmeisterAn HofmeisterAn added duplicate This issue or pull request already exists enhancement New feature or request help wanted Extra attention is needed labels Jul 5, 2022
@PSanetra
Copy link
Contributor

I have started to work on this, but I am not sure when I am able to finish. (Sorry, I have no public WIP branch.)

I agree with @HofmeisterAn that the public API should be as restricted as possible so that further development can continue without unnoticed or too many breaking changes.

@HofmeisterAn
Copy link
Collaborator

I will close this issue. While there are still a few topics left, and there may be cases that do not work with the current reuse implementation, I think we have made a big step forward by implementing #1051. Let's continue the discussion in #407.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants