-
-
Notifications
You must be signed in to change notification settings - Fork 515
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 deploying with docker compose using API #425
Comments
Thanks for creating this issue! Yes, it's on the plans, exactly the same as the Java folks do with the ContainerisedDockerCompose (https://github.com/testcontainers/testcontainers-java/blob/06fc95f5233b0c11d2ba319b4b02587db0b5028b/core/src/main/java/org/testcontainers/containers/DockerComposeContainer.java#L610) We are currently using the Local docker compose choice, but it would be more than helpful if you want to contribute it, and we will be glad to review it |
I'm actually wondering why not directly 'including' upstream |
Great feedback!! I think the original purpose of the issue is too match the Java APIs, but in Go we have the Docker SDK natively. I'd see with great eyes having it implemented natively. |
Hi @mdelapenya I added Please check my another PR #468 |
@mdelapenya I know you're not going to like this but: how about another breaking change/deprecating the The current interface: type DockerCompose interface {
Down() ExecError
Up() ExecError
WaitForService(string, wait.Strategy) DockerCompose
WithCommand([]string) DockerCompose
WithEnv(map[string]string) DockerCompose
WithExposedService(string, int, wait.Strategy) DockerCompose
} Isn't really idiomatic Go and doesn't really fit when working with the compose Go API. type ComposeStack interface {
Up(ctx context.Context) error
Down(ctx context.Context) error
WaitForService(string, wait.Strategy) ComposeStack
WithEnv(map[string]string) ComposeStack
WithExposedService(string, int, wait.Strategy) ComposeStack
} type name is only a placeholder, I'm not so fond of it tbh and I'm also wondering if the fluent/builder approach is actually that good. Personally I prefer options not only because the map a lot better to the underlying compose API but also because with the builder approach you're always 'loosing' type information without having a generic interface and with having a generic interface it is actually rather ugly 😄 but I'm open for opinions. @fiftin I had a look at your PR with the |
@baez90 because docker-compose has a lot of commands: 'up', 'down', etc and a lot of flags which parsed by cobra library. Do you want to do it yourself? What if authors of docker-compose add new flag in new version, you will modify your code? Now I just send all args received from user to docker-compose CLI object and it works. Yes, this solution require some hack with environment variables (I will add it if @mdelapenya agrees this solution), but it is reason why dockerized docker-compose much better. |
Apart from all that: you're free to do what you want. I only tried to give some feedback/hint on what could be done. What you make out of it is totally you thing, no reason to be offended or whatsoever... EDIT: please see here a still very rough example on how to directly use the compose API to I thought about some more convenience methods e.g. to be able to get a Probably this helps a bit. |
@baez90 , I just follow current API. And it has methods |
@baez90 Did you find public documentation for docker compose API. I'm didn't. Why you sure authors don't change API soon? I'm not sure that it is designed to be used as a library in external projects. But you can be sure that CLI interface will not change. |
I'm aware that the current API isn't compatible with my proposal (even though it could be adopted for the time being) but we released breaking changes earlier and I think having an API that takes Regarding your concerns with the public API of compose I rather feel it the other way around. Everything I use (if I remember correctly) is part of the Last but not least: either way the public API may change and break our API in the future but that can be (at least partially) avoided by encapsulation, right? So I wouldn't worry about that too much yet as long as Testcontainers-Go 1.0 isn't released? Eventually it's a matter of taste, I would like to introduce a breaking change - if necessary - as soon as possible and I generally think it would be helpful no matter which approach(es) are to be supported in the future. |
@baez90 @fiftin I LOVE where this discussion is going driven by you two 👏 Wdyt if you both work on a complete revamp of the compose code to support the native API? @mazitovt already created a PR in #463, so probably could be of some help here. If you all agree, we can freeze all compose-related PRs until this discussion is finished and we as a community like your proposal. |
I wouldn't mind to work on this together? I'd like to wait what the Regarding the other commands: I would also keep in mind the option to run AFAIK there's also no reaper integration for |
It appears so far neither of the approaches guarantees a stable API of What do you think? Shall we give it a try, Introduce a new API and add a |
I like deprecations, so if we go that way, I'm in 🙋 |
To improve further discussions I created a WIP PR containing the API based implementation I've been working on. I also duplicated the old test suite and adopted it for the API based one (including some additional tests). I also added some options to showcase how it can be done in an The API is more or less like I described it above and 'felt' not too bad while working on the tests. Due to dependencies to Go 1.16 in Let me know what you think 😊 |
How do you feel if I convert this thread to a Discussion? |
Fine for me if there's more to discuss? |
I'd like to give a bit of traction leveraging GH discussions, therefore I'm moving issues that are actually discussions to there, and this one seemed a great candidate |
Given #476 has been merged, I'm closing this issue. Thanks all for participating in the discussion! 👏 |
I'm attempting to run tests from a container without docker installed. This isn't an issue for ContainerRequests since it's using the API, but compose relies on
docker-compose
being present on the machine. It would be beneficial to be able to deploy a compose file without the requirement for external dependencies.The text was updated successfully, but these errors were encountered: