feat: support native docker compose api

[prskr]

What does this PR do?

Deprecate shell-escape based LocalDockerCompose implementation and provide new docker/compose based one using the Docker API. Introduce a new API that takes context.Context into account. Furthermore it exposes more typed options.

#425

[mdelapenya]

@baez90 shall we change the PR to draft? We can start the review once you feel confortable with it

And thanks for your time here, using the native Go libraries for compose is a great addition that none of the other languages can do! :kudos:

[prskr]

We can do that if you prefer. I would actually appreciate a first review! To make the pipeline working I'd have to remove Go versions < 1.16. I could do that but wasn't sure if this is okay?

[mdelapenya]

We can do that if you prefer. I would actually appreciate a first review! To make the pipeline working I'd have to remove Go versions < 1.16. I could do that but wasn't sure if this is okay?

Fine for me about the removal 🙋‍♂️

[mdelapenya]

Great PR @baez90!!!

I'm starting to like the functional approach for optional configurations, although I need to practice more. Thanks for sharing!

At this stage of the review, this PR is a +1. Great great great job with the tests!

[codecov]

Codecov Report

Merging #476 (b231f60) into main (4c04bdf) will decrease coverage by 20.96%.
The diff coverage is 0.20%.

❗ Current head b231f60 differs from pull request most recent head 88f3c4b. Consider uploading reports for the commit 88f3c4b to get more accurate results

@@             Coverage Diff             @@
##             main     #476       +/-   ##
===========================================
- Coverage   34.50%   13.54%   -20.97%     
===========================================
  Files          13       15        +2     
  Lines        1759     2001      +242     
===========================================
- Hits          607      271      -336     
- Misses       1056     1684      +628     
+ Partials       96       46       -50     

Impacted Files	Coverage Δ
compose.go	0.00% <0.00%> (ø)
compose_api.go	0.00% <0.00%> (ø)
compose_local.go	0.00% <0.00%> (ø)
docker.go	20.25% <10.00%> (-21.02%)	⬇️
reaper.go	3.50% <0.00%> (-71.93%)	⬇️
mounts.go	0.00% <0.00%> (-50.00%)	⬇️
docker_mounts.go	9.75% <0.00%> (-36.59%)	⬇️
container.go	40.96% <0.00%> (-18.08%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[mdelapenya]

Thanks for this PR! Looks super promising!

It would be also awesome if we improve the docs about using compose with the new API design.

[prskr]

Sure, I'm actually wondering if I should try to get a PR into docker/compose to update their docker/docker dependency because this could resolve some replace issues I encountered and which we should otherwise document as well.

I'll give it a shot tomorrow to see if the dependency update would make things easier and update the docs either way accordingly 😊

[mdelapenya]

@baez90 no updates on this PR as I've been very busy with certain updates that will be public soon. I'll let you know once I'm 100% available to continue with this (hopefully soon)

[prskr]

Don't you worry I'm a bit busy myself as well. As previously mentioned I tried if upgrading the Docker version would help to get rid of the replacement stuff but doesn't work. Probably when Docker 22.xx is released or so 😅

I'll try to update the docs probably tomorrow or begining of next week and I also have to rebase the branch once more. Do you mind if I add something about Podman support as well when I'm already on it or shall I create another PR for that?

Until then good luck with whatever you're currently working on 😉

[mdelapenya]

Do you mind if I add something about Podman support as well when I'm already on it or shall I create another PR for that?

That would be perfect in a separate PR, faster to review and merge.

Until then good luck with whatever you're currently working on 😉

Thanks my friend!!

[prskr]

Alright @mdelapenya I prepared at least some docs, rebased everything once more and updated the dependencies.

Let me know if I should update the description once more or if the docs need further adjustments or anything else 😊 till then I'll have a look at #496

[kiview]

I just wanted to add a reference to an issue I remembered from the Compose project where someone asked if compose can be used as a library and actual API, only to realize that you opened it 🤯

So for future reference, this is the issue where they track the progress on this:
docker/roadmap#387

I'd suggest only releasing stable support for this, once upstream provides stable support. However, since you already distinguish by API within tc-go, it might be ok.

[prskr]

Well tc-go itself is also not release yet as stable hence I figured it should be okay to use the (fairly) stable compose API especially because everything compose related is wrapped so breaking changes (at least small-ish ones) could be adapted without breaking the tc-go public API.

I'll resolve the conflicts once more and afterwards we can discuss how to proceed with this PR?

[kiview]

Maybe tc-go is abstracting compose further away as tc-java does it, but Compose V2 has some pretty significant breaking changes, as we found while working on testcontainers/testcontainers-java#5608 (@eddumelendez can probably share the concrete details here).

Most prominent is the container/service naming, using hyphens instead of underscores.

Regarding:

Well tc-go itself is also not release yet as stable

while technically true, every breaking change will reflect on how users (and there are existing users of tc-go of course) perceive the maturity of the library, completely independent on what is actually communicated in the version number.

[prskr]

Funny that you mention the hyphens-vs-underscores problem as I took care of that a while ago 😄 (see here if you're interested) when I noticed that most of the test code was still running with compose V1 while I was running the tests locally with V2 but apart from that I haven't experienced any further problems with compose V1 vs. V2.

I totally understand your concern that the API should be as stable as possible to ensure tc-go users don't get frustrated by having to deal with breaking changes every time they update. I am using it on a daily basis myself and my first contribution was actually to rework a previous 'non-breaking-change' that kept the existing API but changed the behavior regarding bind mounts with the result that the code was still compiling but my tests were failing because mount source and target were inverted (previously the map key was the source and the value the target and suddenly it was the other way around) which is a great example that sometimes breaking the API is better than keeping it 😄

But as I said previously the new public API does not directly expose any compose details and also only includes some basic compose options so I'm pretty confident almost every breaking change can be hidden. The worst case I can think of is that some options aren't supported in the future and this can still be handled gracefully by deprecating the tc-go option and make it a noop option for a few releases.

Do I miss something?

[eddumelendez]

just want to add that besides the hyphens/underscore change in compose v2 the compose file version supported in v2 starts with 2. There should be services at the root of the yaml file

[prskr]

Is something missing here @mdelapenya ?

[prskr]

I incorporated the docs changes we spoke of and added another section regarding the service name API.
Also updated to the latest docker-compose version and rebased the branched once more to resolve conflicts.
Let me know if there's anything else missing 😉

[mdelapenya]

At the moment there is a force-push, I lose track of what I'd already reviewed and what I hadn't. I'd thank if we avoid pushing-force the PRs 🙏

We are squashing commits since the release of 0.14, so no worries about the story of the main branch: it will be now cleaner :)

[prskr]

I added another commit regarding the docs yesterday and did not change any previous commit, I only rebased it onto main again to resolve conflicts 😉 no rewrite of the history happened, you only need to review the latest commit 😊 hope that helps

If you prefer I'll merge main if necessary in the future to resolve conflicts, I'm just used to the rebase workflow 😉

[mdelapenya]

Minor comments or typos in docs

[mdelapenya]

Suggested change

	assert.NoError(t, err, "NewDockerComposeAPI()")
	assert.NoError(t, err, "NewDockerCompose()")

[mdelapenya]

Suggested change

	assert.NoError(t, err, "NewDockerComposeAPIWith()")
	assert.NoError(t, err, "NewDockerComposeWith()")

[mdelapenya]

Suggested change

	To interact with service containers after a stack was started it is possible to get an `*tc.DockerContainer` instance via the `ServiceContainer(...)` function.
	To interact with service containers after a stack was started it is possible to get a `*tc.DockerContainer` instance via the `ServiceContainer(...)` function.

[mdelapenya]

Suggested change

	Furthermore, there's the convenience function `Serices()` to get a list of all services **defined** by the current project.
	Furthermore, there's the convenience function `Services()` to get a list of all services **defined** by the current project.

[mdelapenya]

Suggested change

	_Node:_ this API is deprecated and superseded by `ComposeStack` which takes advantage of `docker-compose` v2 being
	_Node:_ this API is **deprecated** and superseded by `ComposeStack` which takes advantage of `docker-compose` v2 being

[mdelapenya]

If you prefer I'll merge main if necessary in the future to resolve conflicts, I'm just used to the rebase workflow 😉

Yes please 🙏 that would help!

Apart from that, the CI builds are failing, seems related to TestDockerCreateContainerWithDirs. Could you take a look?

[mdelapenya]

@baez90 I found #574, which it's fixing the CI

[mdelapenya]

@baez90 I've noticed this error in the pipeline, related to a concurrent access to a map (compose_api.go:294):

=== FAIL: . TestDockerComposeApiWithWaitForShortLifespanService (unknown)
Network b2e2bce1-ac00-48f3-82fc-8403274bef5c_default  Creating
Network b2e2bce1-ac00-48f3-82fc-8403274bef5c_default  Created
Container b2e2bce1-ac00-48f3-82fc-8403274bef5c-falafel-1  Creating
Container b2e2bce1-ac00-48f3-82fc-8403274bef5c-tzatziki-1  Creating
Container b2e2bce1-ac00-48f3-82fc-8403274bef5c-tzatziki-1  Created
Container b2e2bce1-ac00-48f3-82fc-8403274bef5c-falafel-1  Created
Container b2e2bce1-ac00-48f3-82fc-8403274bef5c-falafel-1  Starting
Container b2e2bce1-ac00-48f3-82fc-8403274bef5c-tzatziki-1  Starting
Container b2e2bce1-ac00-48f3-82fc-8403274bef5c-falafel-1  Started
Container b2e2bce1-ac00-48f3-82fc-8403274bef5c-tzatziki-1  Started
fatal error: concurrent map writes

goroutine 710 [running]:
runtime.throw({0x1f5e0d1?, 0x70?})
	/opt/hostedtoolcache/go/1.18.7/x64/src/runtime/panic.go:992 +0x71 fp=0xc0004e7c78 sp=0xc0004e7c48 pc=0x43a631
runtime.mapassign_faststr(0x100?, 0x0?, {0x1f45191, 0x7})
	/opt/hostedtoolcache/go/1.18.7/x64/src/runtime/map_faststr.go:212 +0x39c fp=0xc0004e7ce0 sp=0xc0004e7c78 pc=0x41551c
github.com/testcontainers/testcontainers-go.(*dockerCompose).lookupContainer(0xc000865200, {0x2277d08, 0xc00041d140}, {0x1f45191, 0x7})
	/home/runner/work/testcontainers-go/testcontainers-go/compose_api.go:294 +0x4fe fp=0xc0004e7f20 sp=0xc0004e7ce0 pc=0x19f65de
github.com/testcontainers/testcontainers-go.(*dockerCompose).Up.func1()
	/home/runner/work/testcontainers-go/testcontainers-go/compose_api.go:229 +0x57 fp=0xc0004e7f78 sp=0xc0004e7f20 pc=0x19f5a77
golang.org/x/sync/errgroup.(*Group).Go.func1()
	/home/runner/go/pkg/mod/golang.org/x/sync@v0.1.0/errgroup/errgroup.go:75 +0x64 fp=0xc0004e7fe0 sp=0xc0004e7f78 pc=0x62da84
runtime.goexit()
	/opt/hostedtoolcache/go/1.18.7/x64/src/runtime/asm_amd64.s:1571 +0x1 fp=0xc0004e7fe8 sp=0xc0004e7fe0 pc=0x46da61
created by golang.org/x/sync/errgroup.(*Group).Go
	/home/runner/go/pkg/mod/golang.org/x/sync@v0.1.0/errgroup/errgroup.go:72 +0xa5

Could you take a look? 🙏

[mdelapenya]

@baez90 test are passing! I pushed 4111cdc and b231f60 with the sole goal of making this PR into main ASAP. Hope you understand.

My plans are: release v0.15.0 this week(end) without this PR, and immediately merge this PR for a dedicated v0.16.0 to isolate the compose changes. Does it sound good to you?

[mdelapenya]

@baez90 I'm merging this PR now. Big big thanks for the huge refactor which simplifies a lot the maintenance of the compose module. We will eventually remove (and communicate) the local compose code after having the native implementation.

[prskr]

Thanks for taking care! I just couldn't manage to get some time to work on this PR so thanks a lot! Also for all the support, time for reviews and your valuable input all the time!