feat: Add DependsOn support

[Mathew-Estafanous]

What does this PR do?

This PR adds DependsOn field to ContainerRequest which can be used to supply a list of containers that must be running prior to starting the current container. On startup of the container, all containers it depends on will be started (if they are not already running).

Dependencies are started within the same network such that the parent container is able to connect to its dependencies.

Dependencies are added to the parent's container request as follows:

dep := NewContainerDependency(nginxReq, "MY_DEPENDENCY").
	WithCallback(func(c Container) {
		// callback logic
	}).
	WithKeepAlive(false)

req := ContainerRequest{
	Image:     "my-web-app",
        /* Other configuration */
	DependsOn: []ContainerDependency{dep}
}

The supplied environment variable (ie MY_DEPENDENCY) will be injected into the parent container and can be used to resolve the dependency's IP address.

Why is it important?

• Enables a streamlined and explicit description of container dependencies.
• Supply feature parity between testcontainer-java's own dependsOn implementation and testcontainer-go.

Related issues

• Closes [Enhancement]: dependsOn() implementation #1791

How to test this PR

A test suite for this new field accompanies the PR. See dependency_test.go

[netlify]

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	a2f0c45
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-go/deploys/6626a2edbe570f000986965d
😎 Deploy Preview	https://deploy-preview-2035--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 site configuration.

[mdelapenya]

Hey @Mathew-Estafanous thanks for submitting this PR. Wdyt if we first create a GH discussion about the design of this feature? As an example, I had in mind having a way to detect cycles in the dependsOn: i.e. srv1 depends on srv2, but what happens if srv2 depends on srv1 (users doing crazy things?) how would we handle that error?

In that sense, feel free to create the discussion so we can elaborate a great design. Please take a look at #559 as an example

[hughesjj]

LGTM! Thanks, been following from otel side, this will be super useful for us.

[hughesjj]

@ preventing circular dependencies, sounds reasonable but not sure it should be a blocker, can always followup with such in a different PR

circular dependencies are inherently un-resolvable so imho it wouldn't change the contract API

@ design I would be okay with adding dependson as another WaitingFor/wait.Strategy target but I think it keeps with upstream/docker compose better as a separate array

(also, probably should be a set rather than array but idk if it's a hashable type and golang set semantics are fairly arduous so for sake of simplicity I'm okay with not enforcing such)

[Mathew-Estafanous]

Thanks for reviewing! I appreciate it.

@ preventing circular dependencies, sounds reasonable but not sure it should be a blocker, can always followup with such in a different PR

circular dependencies are inherently un-resolvable so imho it wouldn't change the contract API

Come to think of it, given the current API design, how could someone create a circular dependency? You'd already need to have created both containers. Am I missing something?

[hughesjj]

Given DependsOn requires an instantiated container, and DependsOn is part of ContainerRequest, it does inherently seem like any runtime modifications to the DependsOn array of an existing container would be such an obvious and arduous anti-pattern I doubt it'd affect many/any users, and imho if they're knowledgeable about the semantics required to do so they should be knowledgeable enough to realize it probably wouldn't work as expected.

That said, I suppose that is a design consideration (whether DependsOn should take in actual instantiated containers or if they should be dependent upon other container requests).

I see in the container dependencies can be chained together integration test, the dependent container needs to exist before instantiating the DependsOn for dependent containers.

It's still an improvement but imho having container requests depending on a container name/some other reference instead of the go object would be preferable.

[hughesjj]

I think you're aligned with "upstream" (assuming the java impl is the reference spec for testcontainers)

Looks like all their dependsOn are of type Startable which aligns with a container instance and not containerrequest instance.

that said, I don't think java has the notion of containerrequest to begin with, so I think that puts the ball of design back in our court.

[Mathew-Estafanous]

....if they're knowledgeable about the semantics required to do so they should be knowledgeable enough to realize it probably wouldn't work as expected.

Agreed, it would be quite the feat to shoe-horn such a blatant anti-pattern in the current design.

But you do bring up a good point on maybe generalizing the api to take in something less concrete than a Container. I think referencing just a name/container ID might not be enough considering the need to respect each container's wait.strategy and I'm not aware of a way to get that with just a name as the reference.

[hughesjj]

Perhaps we can make the type of DependsOn a custom type/interface (pseudo-type union) we could then implement it as-is and extend in the future?

FWIW docker-compose just uses a reference to the container in depends_on, and as far as I'm concerned that's exactly the ID semantics you've implemented. Lifecycle and representation is a different question, but dependency wise I don't see a difference between the proposed implementation and the docker-compose spec

[Mathew-Estafanous]

Perhaps we can make the type of DependsOn a custom type/interface (pseudo-type union) we could then implement it as-is and extend in the future?

Not sure what you mean by make DependsOn a custom type. Are you referring to having a custom interface (similar to Startable in the java impl)?

[hughesjj]

Sorry. So, currently, DependsOn is of type []Container. I was trying to say that, if we wish to support some other non-Container type in the future, we could make it of type []Dependable, for some interface with a better name than Dependable, of which Container would be a type. My background is more heavily java/kotlin/python, so apologies if I'm using improper terminology anywhere here, but the gist of my comment is "Hey a TypeUnion could be useful for forwards compatibility", in case we don't wish to require a container instance in the future.

That said, theoretically anything that implements Container would also implement my hypothetical type union, so even as-is it should be forwards compatible afaik.

[mdelapenya]

Hi folks, sorry for the radio silence during this month but Xmas holidays and company trips made it difficult to me to jump in without all the context.

First thank you all for the great community work you've been doing here with your proposals 🙇 I've enjoyed a lot seeing you collaborating here.

And now to the topic. The library defines two different concepts for a container:

• a ContainerRequest struct, which represents the definition for the future container, used by developers to define and configure the containers they need. This struct will be used to start a container.
• a Container, an interface representing an already created/started/stopped container, created from a ContainerRequest struct. Could this interface be the aforementioned union type?

So, coming back to the DependsOn thing, when defining my container, I'd like to declare all the containers I need to be running before a given containerA is started. But because we are declaring containerA with a ContainerRequest_A, as a developer, I would have two options:

1. define and create the containers that are needed by containerA, but without starting them (not passing the Started: true field to the container request)
2. define multiple container requests and set them as dependencies for containerA at this declaration level.

The 1) option would be simpler, but less developer friendly, as I'd be forcing developers to not start the supporting containers; then the library, using the lifecycle hooks, would start them before containerA.

The 2) option seems more developer friendly, as I'd declare all the container requests, would set the DependsOn dependencies, and would start the containerA. Then the library, using the lifecycle hooks, would start all the dependencies.

I see this PR more aligned with option 1, which fairly works, although I see it less easy to use. At least at the moment.

For options 2, we would need to depend on another container request, but still relying on the users not calling the GenericContainer(...) method on the dependent containers before calling containerA.

So, what would you prefer?

[Mathew-Estafanous]

I agree that the implemented solution isn't as easy to use and I think it would be be better to work on a solution similar to your proposed 2nd solution.

However, I'm concerned that with the use of ContainerRequest we would be left without a Container for each dependency. GenericContainer(..) would only return the 'parent' container interface, leaving out the others. If maintaining access to dependant to containers isn't necessary, then option 2 would suffice.

We could introduce a Dependency struct as a wrapper for ContainerRequests with a callback function that is triggered when the container is started.

type Dependency struct {
	ContainerRequest ContainerRequest
	OnStarted func(Container) error
}

It's not the most elegant solution, but it offers a workaround in gaining access to the Container of dependant containers.

Alternatively, (if not already possible) we could consider introducing a 'find' container mechanism, possibly similar to the internal findContainerByName(..) method.

What are your thoughts? Is having access to dependant containers necessary?

[Mathew-Estafanous]

Hey @mdelapenya

I just want to bump this conversation. Have you had time to review my comments? I understand if this on hold due to scheduling. Thanks!

[pablochacin]

Hi. As a potential user of this feature, first at all thanks @Mathew-Estafanous for moving this forward.

I tend to agree with @mdelapenya that using containers is not natural in this context, mostly because the feature will automatically start dependencies if they are not already started.

What are your thoughts? Is having access to dependant containers necessary?

I think in general it is needed. For instance, an application (container App ) needs the IP addresses of the databases (MySQL, Redis) to setup db connections.

What I don't see is how this could work, as we need this address before starting the container App, to pass it as Environment variables for example.

The way Docker compose solves this is by injecting the dependencies as environment variables. In my example above, the App container would have environment variables with the addresses of the Mysql and Redis containers.

Alternatively, (if not already possible) we could consider introducing a 'find' container mechanism, possibly similar to the internal findContainerByName(..) method

I think this could work better, but still, I don't see how this would work in the case I described above,

[Mathew-Estafanous]

Hey @pablochacin, thanks for your input! And sorry for not getting back earlier.

What I don't see is how this could work, as we need this address before starting the container App, to pass it as Environment variables for example.

The way Docker compose solves this is by injecting the dependencies as environment variables. In my example above, the App container would have environment variables with the addresses of the Mysql and Redis containers.

You make a good point. We can enable a similar behaviour by adding all dependant containers and the parent to the same network. This should allow the parent (or dependant) container to reference each other's IP address through the use of docker's built-in DNS service. That should allow for calls like http://my-redis:6379.

What do you think? Would that suffice in this case?

[pablochacin]

@Mathew-Estafanous

I think putting the containers in the same network and using a fixed name for the containers could work, but with that many requirements, the solution looks fragile and somehow forced.

I think the callback you proposed could work as a more robust mechanism. if it is called after the dependency is running but before the container with the dependencies is created, we can inject any parameter we need in the container request, host names, environment variables, etc.

I lean towards this initial approach and enhance developer experience by providing some options that implement frequent use cases like:

• WithDNSDependency(depdency ContainerRequest, hostname string) injects the IP address of the dependency with the given hostname
• WithEnvVarDependency(depdency ContainerRequest, var string) injects the IP address of the dependency as an environment variable

[Mathew-Estafanous]

@pablochacin

I don't believe we'd necessitate the use of fixed names.

We could have a callback similar to WithDNSDependency(dependency ContainerRequest, hostname string). In this case, the containers would be in the same network then we'd inject an environment variable with the same name as hostnameEnv that supplies the name (hostname) of the container it depends on.

---

For example. caching_service might depend on a redis instance.

...WithDNSDependency(myRedisService, "REDIS_HOSTNAME")

On creation of the redis container, it might be assigned some arbitrary name which we then inject to the parent container as an env variable REDIS_HOSTNAME=<name>.

Internally, the containers would be created in a way similar to this:

> docker network create testcontainers

# start dependent redis container
> docker run -itd --network=testcontainers redis:latest
# image named 'youthful_elgamal' is created

> docker run -itd --network=testcontainers -e  REDIS_HOSTNAME=youthful_elgamal caching_service:latest
# creates `caching_service` container which can rely on DNS resolution based on the environment variable passed to it.

What do you think? Does this work or am I misunderstanding your concerns?

[pablochacin]

I don't believe we'd necessitate the use of fixed names.

@Mathew-Estafanous I explained poorly. I meant that if we don't introduce a mechanism like the one I suggested, we would need fixed names. With the options I suggested, that is not necessary, as you explained.

So, in summary, I think implementing the dependency with a callback is a good first step and the the options for automatically setting DNS or ENV with the container's IP can be built on top of it.

[Mathew-Estafanous]

Here's the interface that I came up with for dependencies. I still have to add more tests and work on edge cases, but thought I'd push this up to get some eyes on it.

[Mathew-Estafanous]

I was hoping to use network.New for this, but that causes a cyclical dependency - which is why I'm using these depreciated functions.

I'd appreciate any suggestions on how to use network.New instead of doing this.

[Mathew-Estafanous]

I've added a KeepAlive flag since I thought it'd be helpful to define whether or not the dependency should terminate alongside its parent.

I set the default to true in NewContainerDependency but I'd be interested to hear other's opinions of the flag itself and if true is an appropriate default.

[pablochacin]

The environment variable can be injected by the callback function and not all containers can take advantage of it, so I'm not convinced this should be implemented in the dependency struct as a default behavior.

[Mathew-Estafanous]

Sorry, i'm not sure what you mean by the env variable can be injected by the callback func. The container would be already running by the time the callback is called and I'm not aware if injecting env vars into a live container would be possible at that point.

[Mathew-Estafanous]

Do you mean that we should make Env injection optional through the use of something like WithEnvName(envKey string) ?

[pablochacin]

The container would be already running by the time the callback is called
The dependency will be running, but not the container that depends on it. See please an additional note that I made about the callback function signature.

Do you mean that we should make Env injection optional through the use of something like WithEnvName(envKey string) ?
Yes, that was my original proposal

[pablochacin]

@Mathew-Estafanous Overall I like the approach. My only observation is that I think injecting the environment variable by default is not necessary. The callback function can do this. I would remove that from the interface.

[pablochacin]

I think the callback should receive also the container request of the container that has the dependency. In this way the callback can modify the request (e.g. add env variables, setup the hosts file, etc)

[Mathew-Estafanous]

@pablochacin I discussed this issue at length with Manuel and wrote a quick explanation of what was decided on in the DependsOn discussion post.