Updating the go version for local Travis build and set wait strategy for compose services

[shashank-g172]

With the older version of go within the local docker container, I found this issue: #260

I'm updating the docs to fix it.

[mdelapenya]

In the interest of speeding up the failures, I ended up removing the failedStrategyMap and failed fast : 2a0da88

As a consumer of the docker compose version, I'd consider any service in the compose file to be satisfied as a test time dependency. If any is not ready, I'd like it to fail.

[mdelapenya]

Hey @shashank-g172, great job here! I'm looking forward to seeing this into the library!

Please let's discuss about my comments in the PR.

One thing I'd do too is to revert the commits not related to the WaitFor strategy for compose, sending in a separate PR, so that we can review and merge separately, unless you consider them related.

Thanks, and well done again!

[shashank-g172]

@mdelapenya, each commit is all part of the same change. I needed to update the local go version in the contributing docs, I had to update the travis yaml file because of a dependency on the docker version, and all the other changes are generated except in compose_test.go and compose.go. I'd need to commit all the files for this to work.

[mdelapenya]

Sorry for the late response, I'm still on vacations and did not check comms that much.

Yeah, I'd like to merge it sooner then later, as it brings a lot of value to the compose feature. I'd still suggest renaming the withExposedService method to waitForService , although I'd love to hear @gianarb's voice here

[gianarb]

Thank you for your contribution @shashank-g172 and I am sorry if it didn't get merged on time and it now requires a rebase. @mdelapenya withExposedService is the way TestContainers-Java does it and I think we should keep it consistent 👍 even if I agree wait is clearer.

Overall this PR can be merged if rebased

[shashank-g172]

@gianarb @mdelapenya @mbroshi - The tests and the functionality works if I'm able to list the running containers so that I can apply the filter to apply the Wait Strategy for the one being specified. If any of you have a Go-Docker SDK agnostic way of listing the containers that I can't seem to find (which is why I had to pin down the version to 1.38.0), I can modify the file.

https://travis-ci.org/github/testcontainers/testcontainers-go/builds/751999670 (Look at the failed builds without the docker version set)

Considering that there isn't another way to list the containers easily - https://godoc.org/github.com/docker/docker/client#Client.ContainerList

[gianarb]

I am not sure if I understand the context of your question @shashank-g172 but I don't think we need to be Docker agnostic when it comes to the scope of Docker Compose, because Docker is a pretty strong compose requirement.
Happy new year btw!

[shashank-g172]

Happy New Year @gianarb!

I think I could phrase my question better:

1. There's one way of listing all the running containers using the Docker GO SDK: https://godoc.org/github.com/docker/docker/client#Client.ContainerList
2. I'm listing the running containers the same way while trying to apply the wait strategy to them (in compose.go)
3. However, I think that unless the DOCKER version is set explicitly to 1.38.0 on the CI agent, the functionality doesn't work. I can't seem to find an alternate way of listing running containers with later versions of the DOCKER API using the Go SDK.
4. Example: When these changes ran- only the agents with the Docker version set 1.38.0 pass, the others fail with this error message:

one or more wait strategies could not be applied to the running containers: error Error response from daemon: client version 1.40 is too new. The maximum supported API version is 1.38 occured while filtering the service nginx_1: 80 by name and published port

Is this a result of TestContainers making an assumption about the version of docker? The checks for this PR seem to be passing through:

https://github.com/testcontainers/testcontainers-go/pull/261/checks?check_run_id=1622559133
https://github.com/testcontainers/testcontainers-go/pull/261/checks?check_run_id=1622559172
https://github.com/testcontainers/testcontainers-go/pull/261/checks?check_run_id=1622559155

[shashank-g172]

Thoughts?

[mdelapenya]

Thoughts?

Thanks for your time here @shashank-g172. I already expressed my concerns about using the port for any wait strategy type, but I'm not against it: in deed I think this is a really great contribution. We can commit and iterate for improvements.

LGTM 👍

[mdelapenya]

Hey @shashank-g172, we removed support for travis. Would you mind rebasing/merging with upstream?

I think this PR brings a lot of value to the docker-compose consumers, so we'd like to review and merge the soonest. Thanks for your patience.

[gianarb]

@mdelapenya do you have time to cherry-pick, rebase and help to make this work ready to get merged?

Thanks a lot!

[mdelapenya]

@mdelapenya do you have time to cherry-pick, rebase and help to make this work ready to get merged?

Thanks a lot!

Yes, I believe I'll have time this Friday morning. Let me do that then. 👍

[mdelapenya]

I added a branch in my remote with exactly this PR plus the merge commit and a commit running go mod tidy: https://github.com/mdelapenya/testcontainers-go/tree/shashank-g172-master-resolve-conflicts

[gianarb]

Ok great, feel free to close this one and open a new one 👍