DependsOn Support: Design Discussion

[Mathew-Estafanous]

Context

This design discussion aims to solve how DependsOn would be implemented such that it complies with the following behaviour:

• Wait until dependant containers are ready before starting the current container. Similar to what Docker Compose and dependsOn() do as of now.
• Start the dependency container (if not already started) and wait for a ready status based on its wait.Strategy.

Currently, users must manually sequence the startup order or depend on Docker Compose for a cluster of dependent containers. The addition of the DependsOn option would eliminate this manual effort.

Design Option

Extending DependsOn as a Wait Strategy

Initially I considered adding DependsOn as a type of wait strategy such that interfacing with DependsOn would be familiar to other (similar) waiting strategies. However, in closer observation it seems that wait.Strategy describes the post-startup phase of the container lifecycle while DependsOn concerns itself with pre-startup.

I think it's still feasible to add DependsOn as a wait strategy, but that would require a fundamental change with how wait strategies are understood.

req := testcontainers.ContainerRequest{
	Image:      "docker.io/alpine",
	WaitingFor: wait.OnDependency(containerA, containerB),
}

Adding DependsOn option to ContainerRequest

Addition of DependsOn to the ContainerRequest which would be used to supply the list of dependant containers that must run prior to itself.

type ContainerRequest struct {
// other attributes
      WaitingFor wait.Strategy
      DependsOn []Container
// other attributes
}

DependsOn should then be included as part of the pre-startup lifecycle and each dependant container is waited on according to their respective wait strategies. Only if all dependency are ready will the container move forward with starting itself.

Handling Cyclical Dependencies

Cyclical dependency are considered as errors and should cause startup to fail. To prevent side-effects, dependency-cycles should be detected before any attempts are made to start dependency containers.

Detection would follow a straight forward depth (or breadth) search that maintains a record of visited dependencies. If a container is revisited during the search (signalling a cyclical dependency) an error is triggered, and no containers are started.

Related Issues

• PR: feat: Add DependsOn support #2035
• issue [Enhancement]: dependsOn() implementation #1791

[hughesjj]

I completely agree with the proposed implementation of handling DependsOn separate from a WaitStrategy given it more closely maps to the docker-compose interface, and thus will be less surprising to users of the interface. I see WaitingFor as more of a utility function to avoid bash-hacking via a new container and RunScripts, which would only be a viable option if we have DependsOn to begin with.

I think handling dependency cycles is a nice to have unless it's particularly easy to foot-gun yourself. the Current proposed implementation makes any footguns arduous to negligently discharge, so I don't think it's required to have, but would be nice to have.

We could implement a DFS/disjoint set cycle detection if need be, but I don't think it need be.

While the exception/error path handling for cyclical dependencies is not mentioned, I'm fine with either returning a golang error or even just logging an error/warning to stderr if we feel it worthy to implement upfront.

That said, I would not personally consider cyclical dependency avoidance required functionality given the overall context of this issue and currently proposed implementation.

[mdelapenya]

I think handling dependency cycles is a nice to have unless it's particularly easy to foot-gun yourself. the Current proposed implementation makes any footguns arduous to negligently discharge, so I don't think it's required to have, but would be nice to have.

We could implement a DFS/disjoint set cycle detection if need be, but I don't think it need be.

Agreed. Let's move that part to a follow-up implementation. Thanks for setting the basis here.

[mdelapenya]

I think it's still feasible to add DependsOn as a wait strategy, but that would require a fundamental change with how wait strategies are understood.

Please take a look at #1922, where we propose a new lifecycle event: Ready, the wait strategies are moved there. But I think it does not affect the discussion here, as the dependsOn should be considered in a PreCreate or PreStart hook.

[Mathew-Estafanous]

Thanks for pointing those changes out. Is there a timeline on when that PR gets merged?

[mdelapenya]

I'll be on PTO for Xmas, so let me get back to it right after that.

[Mathew-Estafanous]

After discussing dependsOn with @mdelapenya, we came to an agreement to keep the first iteration simple with the option of extending it in the future as needed. Depends on will work similarly to the java implementation works.

1. Dependencies are started prior to starting the parent container.
   • Or reuse a running dependency if a container with the same name exists.
2. A parent container will not start until all dependencies have started and waited on.

The depends on interface will look similar to the following:

dependencies := testcontainers.NewContainerDependencies(reqDepA, reqDepB)

req := testcontainers.ContainerRequest{
   // ..options
   DependsOn: dependencies
}

// start parent container with req.

---

Question up for discussion:

1. I'd like to hear others' thoughts on the interface that we've come up with.
2. Should dependencies terminate alongside their parent or remain active?

[pablochacin]

If I understand then you decided to drop the callback? This takes me to my original comment about the limited usability of the feature as the container that has the dependencies cannot use a reference to the other containers.

I understand that the first iteration should be simple, but I think the callback you suggested is a simple mechanism that allows this customization of the container with the dependencies. Besides, you already have an implementation and from what I could see, it's pretty straightforward.

[mdelapenya]

What I'm concerned is to be reimplementing docker compose here. The Java flavour just starts the dependencies a container depends on. What I discussed with @Mathew-Estafanous was about having a very basic impl, and if the community needs any improvement on top of that, work on that.

Regarding the access to the dependencies, we can leverage the container lifecycle, ideally preCreates, to start the dependencies, and from there, transfer their container representations to the final container.

I'd like to also say that any of you will probably have a broader view of the feature, so I'm open to any improvement. I just want to start small and iterate if/when needed.

[pablochacin]

@mdelapenya

What I'm concerned is to be reimplementing docker compose here

I totally agree. That's why I'm not in favor of injecting env variables, for instance.

Regarding the access to the dependencies, we can leverage the container lifecycle, ideally preCreates, to start the dependencies, and from there, transfer their container representations to the final container.

Also agree. Have you a suggestion of how to get this reference to the dependencies in the pre-create life-cycle hook?
My suggestion of following @Mathew-Estafanous proposal of a callback is to allow this. But I'm open to other options.

[mdelapenya]

I had this in my mind, but of course it's not prescriptive at all:

// add the dependingOnContainers slice private variable to the DockerContainer struct

// add a local variable to the lifecycle code. It will capture the values from the lifecycle execution
var dependantContainers []*DockerContainer
...
// inside the preCreates hook:
// start dependant container requests, getting the containers and assigning them to the dependantContainers slice

// inside the preStarts hook, where we already have a reference to the container representation of the initial req:
dockerContainer.dependingOnContainers = dependantContainers

// figure out how to allow consumers access those containers:
// - a public method in the DockerContainer struct, but then we would need to add it to the container interface
// - create a new interface for StartedContainer (which is what I'd choose to separate concerns for a future v1 to break the huge Container interface into smaller ones)

Thoughts?

[mdelapenya]

BTW, can you illustrate an in-depth example use case for the callback? 🙏 I'm afraid I'm not able to get it yet 🤦