Skip to content

Comments

[WIP] Docker compose refactor#156

Closed
rnorth wants to merge 29 commits intomasterfrom
docker-compose-refactor
Closed

[WIP] Docker compose refactor#156
rnorth wants to merge 29 commits intomasterfrom
docker-compose-refactor

Conversation

@rnorth
Copy link
Member

@rnorth rnorth commented Jun 5, 2016

Refactoring (incomplete) of DockerComposeContainer to more cleanly use multiple invocations of docker-compose during the test lifecycle. DockerComposeContainer is now no longer a container itself; it's merely a JUnit rule that creates and runs a docker-compose container as necessary.

I suspect this may break things if people are (for example) mapping volumes or injecting env vars into their compose container, so it'd be good to understand these kind of use cases and work to support them.

/**
* Approach to determine whether a container has 'started up' correctly.
*/
public abstract class StartupCheckStrategy {
Copy link
Member Author

Choose a reason for hiding this comment

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

@ahaines: I've refactored startup checks somewhat in this branch - I wonder if this might help with #154

ambassadorContainer.start();
localProfiler.start("Start ambassador container");

ambassadorContainer.start();
Copy link
Member Author

Choose a reason for hiding this comment

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

See #147 and #146 - must implement fail-fast so that an unrecoverable failure isn't repeatedly retried. Not yet done.

@haines
Copy link
Contributor

haines commented Jun 5, 2016

Injecting environment variables is important for our use-case because we want to be able to specify the version of an image in CI, to support multiple branches building in parallel. Our docker-compose file will look something like:

version: "2"

services:
  foo:
    image: our-registry:5000/repo/foo:${version}

version will be something like the commit hash to ensure that images built for different branches will not overwrite each other, and would be injected into our tests with something like -DdockerImageVersion=...

@rnorth
Copy link
Member Author

rnorth commented Jun 5, 2016

@haines thanks - thought that would be the case. I will make sure environment variables are carried over. Also, volume mounts probably should be too, to allow for things like mounting target/something.jar to be bundled into the docker image.

ResourceReaper.instance().registerContainerForCleanup(container.getId(), container.getNames()[0]));

// Ensure that the default network for this compose environment, if any, is also cleaned up
ResourceReaper.instance().registerNetworkForCleanup(identifier + "_default");
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to do this manually rather than replacing docker-compose rm -f -v with docker-compose down -v?

rnorth added 16 commits July 3, 2016 14:27
Use a TCP-unix socket proxy instead of relying on netty to support unix sockets. More tests are disabled due to OS X / docker machine coupling assumptions.
Needs testing for reliability and performance, but seems to function reasonably well so far.

Add a configuration strategy for proxied connections to Docker for Mac daemon on OS X

Include temporary workaround for docker-java/docker-java#588
… polling of Docker API (using rate limiter for testing new JDBC container connections)
…mpatibility issues

Refactor Docker compose support, to allow scaling, better output logs, and eventually docker-compose v2 format. Refs #146, #147

Start support for docker-compose v2 environments, putting ambassador containers onto the right network to be able to access the compose-launched containers.

Fix Netty conflicts by using Jedis for testing (simpler dependencies)
rnorth added 12 commits July 3, 2016 14:28
…that don't have a slash prepended, i.e. non docker compose containers

Use partial name matching for creating links between containers (in line with linked container network identification)

Remove automatic setting of network mode to 'bridge' when a linked container is not on any network, as this causes an error in the current Docker API and is unnecessary.
Tidy up Docker client configuration to avoid repetitive creation of temporary clients
Add recommended logback.xml entry for testcontainers logs to be at INFO level
…onger necessary, so removal simplifies reasoning about control flow.
@rnorth rnorth force-pushed the docker-compose-refactor branch from 0274634 to 127715e Compare July 3, 2016 13:29
@rnorth
Copy link
Member Author

rnorth commented Jul 3, 2016

Closing in deference to #167

@rnorth rnorth closed this Jul 3, 2016
@rnorth rnorth deleted the docker-compose-refactor branch October 8, 2018 16:39
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.

2 participants