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

feat: Add docker compose support (#122) #1082

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from

Conversation

nkz-soft
Copy link

@nkz-soft nkz-soft commented Jan 2, 2024

What does this PR do?

Adds docker compose support

Why is it important?

We have docker compose support for java implementations, but that's not here yet. This PR does not contain a complete implementation of all methods in the java library, but it is a starting point from which you can begin to port the rest of the methods.

Related issues

Additional information

At this time, there are only two methods in the DockerComposeBuilder class:

  • WithLocalCompose - Specifies the docker-compose file that needs to be executed.
  • WithComposeFile - Whether or not to execute docker-composer locally or in a container.

Additional methods are available for implementing in Java, listed in order of importance (to me):

  • withOptions - Adds options to the docker-compose command, e.g. docker-compose --compatibility.
  • withRemoveImages - Remove images after containers shutdown.
  • withRemoveVolumes - Remove volumes after containers shut down.
  • withBuild - Whether to always build images before starting containers.
  • withExposedService - Specifies the port for current service. According to the documentation, Testcontainers will spin up a small 'ambassador' container, which will proxy between the Compose-managed containers and ports that are accessible to your tests. This is done using a separate, minimal container that runs socat as a TCP proxy. I don't fully understand the necessity of creating a proxy (ambassador) container here, perhaps to avoid port conflicts or to allow the port to listen.
  • getServiceHost - Returns the IP address where the container is listening (via an ambassador container).
  • getServicePort(serviceName, servicePort) - Returns the Docker mapped port for a port that has been exposed (via an ambassador container).

It's my opinion that we should start with the first four methods that cover 90% of the functionality.
The implementation of proxy and port forwarding functions in docker-compose can be done directly through a yaml file.

Copy link

netlify bot commented Jan 2, 2024

Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit f1417d4
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-dotnet/deploys/66221036e421140008db4057
😎 Deploy Preview https://deploy-preview-1082--testcontainers-dotnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@nkz-soft nkz-soft changed the title Feature/support docker compose feat: Add docker compose support (#122) Jan 2, 2024
@HofmeisterAn
Copy link
Collaborator

HofmeisterAn commented Jan 3, 2024

Thanks for the initial PR. Could you please provide additional information in the PR description, specifying which parts are addressed by the PR and which parts are not covered, in comparison to the features provided by Java? Additionally, what aspects do we need to take care of afterward? What follow-ups are necessary, etc.?

@nkz-soft
Copy link
Author

nkz-soft commented Jan 3, 2024

Thanks for the initial PR. Could you please provide additional information in the PR description, specifying which parts are addressed by the PR and which parts are not covered, in comparison to the features provided by Java? Additionally, what aspects do we need to take care of afterward? What follow-ups are necessary, etc.?

I have added some additional information, but I am not sure that this is enough. Please take a look at this.

@HofmeisterAn
Copy link
Collaborator

I have added some additional information, but I am not sure that this is enough. Please take a look at this.

Thanks. I will need some days to review it. A lot of other topics have accumulated over the holidays.

@dmytro-dovhan-gl
Copy link

Hi, Any updates here?

@HofmeisterAn
Copy link
Collaborator

Hi, Any updates here?

Sorry, I haven't had the time yet, but it is the next item in my GH inbox. I cannot promise it, but I will try to look at it sometime next week.

Copy link
Collaborator

@HofmeisterAn HofmeisterAn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR. Sorry for the late response, I hadn't found the time earlier. I still need to run and test it locally. Meanwhile, I have a couple of suggestions and improvements. It would be great if we could discuss and address them in the meantime. Thanks again for your contribution.

src/Testcontainers.DockerCompose/DockerComposeBuilder.cs Outdated Show resolved Hide resolved
src/Testcontainers.DockerCompose/DockerComposeBuilder.cs Outdated Show resolved Hide resolved
src/Testcontainers.DockerCompose/DockerComposeBuilder.cs Outdated Show resolved Hide resolved
src/Testcontainers.DockerCompose/DockerComposeBuilder.cs Outdated Show resolved Hide resolved
src/Testcontainers.DockerCompose/DockerCompose.cs Outdated Show resolved Hide resolved
src/Testcontainers.DockerCompose/DockerComposeBuilder.cs Outdated Show resolved Hide resolved
@nkz-soft
Copy link
Author

Thank you for the PR. Sorry for the late response, I hadn't found the time earlier. I still need to run and test it locally. Meanwhile, I have a couple of suggestions and improvements. It would be great if we could discuss and address them in the meantime. Thanks again for your contribution.

Thanks, I think next week, I'll check and fix all the issues.

@nkz-soft
Copy link
Author

Thank you for the PR. Sorry for the late response, I hadn't found the time earlier. I still need to run and test it locally. Meanwhile, I have a couple of suggestions and improvements. It would be great if we could discuss and address them in the meantime. Thanks again for your contribution.

I think it's done, could you please review the code again?

@HofmeisterAn
Copy link
Collaborator

Please see my suggestions and comments here: nkz-soft#1. Overall, it looks good, but there are still a few things we need to discuss and sort out first.

…cker-compose-1

refactor: Reuse the the builder configuration in local mode
@dava2788
Copy link

dava2788 commented May 9, 2024

Hello all.
Any updates of this PR.
Really interesting in using this docker compose functionality

Best Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add missing function WithDockerCompose
4 participants