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

(#25) Create very basic implementation of Local Docker Compose #97

Merged
merged 33 commits into from Apr 20, 2020

Conversation

mdelapenya
Copy link
Collaborator

@mdelapenya mdelapenya commented Sep 11, 2019

What is this PR doing?

This is the very beginning of an initiative to port the Docker-Compose approach from the Java implementation to Golang.

It simply creates the Local Docker Compose approach (no socat container at this moment), which forces developers to have the docker-compose binary in PATH.

Following Java's approach, a LocalDockerCompose (LDC) object needs to be created with an array of paths to docker-compose.yml files, and an identifier (maybe this second field could be internally created).

It's possible to configure the LDC with a command (i.e. "up -d"), which will be executed after the method Invoke is called.

We have added a convenient Down() method, to be executed after tests run to destroy resources created by docker-compose.

Besides that, we are parsing Docker Compose YAML file to capture service names, and put into an array, so that it's possible to perform operations over the services in the compose file, accessing the compose.Services field.

Why is it important?

This will allow developers to configure more elaborated setups for testing, like an Elasticsearch + Kibana, or Metricbeat + MySQL, as an example, being those services coordinated and scheduled by Docker Compose under the hood, with no need to orchestrate them on the programatic side.

On the other hand, using declarative way of describing services is more familiar to developers (🤞), so the entry barrier could be lower.

How to test it?

I added a few unit tests, which verify that there are no errors after executing the compose, checking that environment variables are properly propagated from compose to the containers, and destroying it in a deferred manner.

Related Issues

Closes #25

@mdelapenya mdelapenya force-pushed the 25-docker-compose branch 2 times, most recently from b8622f4 to 38810f1 Compare September 11, 2019 07:22
@mdelapenya
Copy link
Collaborator Author

Hey @gianarb, any feedback with this one?

@mdelapenya
Copy link
Collaborator Author

Closing for travis rebuild

@mdelapenya mdelapenya closed this Sep 18, 2019
@mdelapenya mdelapenya reopened this Sep 18, 2019
@gianarb
Copy link
Collaborator

gianarb commented Sep 23, 2019

Sweet! The direction is right! I am trying it just now!

@gianarb
Copy link
Collaborator

gianarb commented Sep 23, 2019

I am looking at how testcontainers-java does that.

https://www.testcontainers.org/modules/docker_compose/

Note that it is not necessary to define ports to be exposed in the YAML file; this would inhibit reuse/inclusion of the file in other contexts.

Instead, 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.

We should do the same, mainly because we can avoid the ports collision, but I am not sure about how LocalCompose in used in Java. Do developers tend to use a single docker-compose file for development and all tests or they write more of them (worst case one per test) using it as a specification language? @bsideup

@mdelapenya
Copy link
Collaborator Author

Yeah, this first proposal is trying to address the easy one: the local docker-compose approach, which is totally decoupled from the one with the minimal socat container.

As decoupled they are, I'd review them separately. Wdyt?

@gianarb
Copy link
Collaborator

gianarb commented Sep 29, 2019

Something I do not see from the examples is around how do you think we can get URLs to access containers deployed with docker-compose? Let's say we have a MySQL and we should like to get the service endpoint for it? @mdelapenya
Thanks!

@mdelapenya
Copy link
Collaborator Author

Ok, let me check out Java implementation for that. I agree that there should be a manner to access the container from the compose

Thanks!

@mdelapenya
Copy link
Collaborator Author

Hey @gianarb I don't think the local compose for Java supports accessing a service. I looked through the code and asked in their repo (testcontainers/testcontainers-java#1928) and did not find a manner to access the service 🤷‍♂

@mdelapenya mdelapenya changed the title [WIP] (#25) Create very basic implementation of Local Docker Compose (#25) Create very basic implementation of Local Docker Compose Oct 29, 2019
@mdelapenya
Copy link
Collaborator Author

I think this PR is ready for review

@mdelapenya mdelapenya closed this Oct 29, 2019
@mdelapenya
Copy link
Collaborator Author

Reopening for relaunching travis

@mdelapenya mdelapenya reopened this Oct 29, 2019
@mdelapenya
Copy link
Collaborator Author

Tests are passing locally, closing and reopening again

@mdelapenya mdelapenya closed this Oct 30, 2019
@mdelapenya mdelapenya reopened this Oct 30, 2019
@mdelapenya mdelapenya closed this Oct 30, 2019
@mdelapenya mdelapenya reopened this Oct 30, 2019
@mdelapenya
Copy link
Collaborator Author

@gianarb not sure why, but Travis fails consistently with:

--- FAIL: TestLocalDockerComposeWithMultipleComposeFiles (2.35s)
    compose_test.go:114: {<nil> read |0: file already closed read |0: file already closed}
FAIL
FAIL	github.com/testcontainers/testcontainers-go	129.580s

although tests are passing locally. Can you double check please?

@mdelapenya mdelapenya closed this Nov 12, 2019
@mdelapenya
Copy link
Collaborator Author

Reopening and checking Travis 🤞

@mdelapenya mdelapenya reopened this Nov 12, 2019
@mdelapenya
Copy link
Collaborator Author

@gianarb Any feedback about this one?

Copy link
Contributor

@ClaytonNorthey92 ClaytonNorthey92 left a comment

Choose a reason for hiding this comment

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

I know it is a bit of work, but would it be possible to add a test that has more than one service in docker-compose and test the interaction between them?

for example, if we could use nginx or envoy to reverse proxy a request, for example, I might want to write to redis, and read that value from redis, so I might want a docker compose that I could test functionality that

write <key/value> --> nginx/envoy --> redis
read <key/value> --> nginx/envoy --> redis

I think this would be a common use case that would be a good use case for docker-compose to perform, thoughts?

utility.go Outdated
var alfabet = []rune("123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz")

// RandomString returns a random string
func RandomString(length int) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be better to generate a UUID? rather than write our own RandomString function here? if it is just being used as an identifier, we just need to guarantee its uniqueness, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Totally agree, and I'm using it only for tests, which makes no sense to maintain new code.

Thanks for the feedback!


// LocalDockerCompose represents a Docker Compose execution using local binary
// docker-compose or docker-compose.exe, depending on the underlying platform
type LocalDockerCompose struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to add a usage example to the README? I like that I can read through the tests for examples, but I think our README is lacking features in general

@mdelapenya
Copy link
Collaborator Author

I know it is a bit of work, but would it be possible to add a test that has more than one service in docker-compose and test the interaction between them?

for example, if we could use nginx or envoy to reverse proxy a request, for example, I might want to write to redis, and read that value from redis, so I might want a docker compose that I could test functionality that

write <key/value> --> nginx/envoy --> redis
read <key/value> --> nginx/envoy --> redis

I think this would be a common use case that would be a good use case for docker-compose to perform, thoughts?

Wouldn't it mean testing that docker-compose works? I agree in adding a new test with a combination of services that interact with each other (i.e. a service that populates a database), but testing the network and the dns discovery would be a responsibility for docker guys imho, which I'm sure it's something they are already doing and it's working 😃

@ClaytonNorthey92
Copy link
Contributor

I know it is a bit of work, but would it be possible to add a test that has more than one service in docker-compose and test the interaction between them?
for example, if we could use nginx or envoy to reverse proxy a request, for example, I might want to write to redis, and read that value from redis, so I might want a docker compose that I could test functionality that
write <key/value> --> nginx/envoy --> redis
read <key/value> --> nginx/envoy --> redis
I think this would be a common use case that would be a good use case for docker-compose to perform, thoughts?

Wouldn't it mean testing that docker-compose works? I agree in adding a new test with a combination of services that interact with each other (i.e. a service that populates a database), but testing the network and the dns discovery would be a responsibility for docker guys imho, which I'm sure it's something they are already doing and it's working 😃

ah yes, you're right! good point!

It is shared by invoke and down
It could disclose environment variables
I'm not adding information about the environment to avoid disclosing
sensible values, but it could be easily added
It was just a matter of setting the container name at docker-compose
level, because we just want to access the container to read its
environment variables and do some calculations with them.
@mdelapenya
Copy link
Collaborator Author

Hey @gianarb, I rebased the branch and updated the PR

Copy link
Collaborator

@gianarb gianarb 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 your patent!! 😄

@gianarb gianarb merged commit a911c8e into testcontainers:master Apr 20, 2020
@mdelapenya mdelapenya deleted the 25-docker-compose branch April 20, 2020 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality or new behaviors on the existing one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker compose support
3 participants