Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Conversation

dariko
Copy link

@dariko dariko commented Mar 30, 2016

In a docker-compose (v2) environment this image could not be used to link containers outside the project compose project.

This PR proposes a solution:

  • An environment variable parameter ADDITIONAL_SERVICES is added to to the image.
  • ADDITIONAL_SERVICES must be set as a comma-separated list of "%s_%s" % (project, service) entries.
  • Backend container discovery is based on matching the given project,service names with the one set in the com.docker.compose.[project|service] container labels.
  • If there is no network shared between HAProxy and a backend server, that server will be ignored.

compose_service = container.get("Config", {}).get("Labels", {}).get("com.docker.compose.service", "")
for _service in additional_services:
terms = _service.strip().split("_")
if terms[0].strip() == compose_project and terms[1].strip() == compose_service:
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to test the length of terms before referencing its element directly.

@tifayuki
Copy link
Contributor

@dariko
Thank you very much for the PR.

A few comments:

  1. In the README, it is better to add something like "this environment variable only works on compose v2", or change the envvar name to something like ADDITIONAL_NEWLINK_SERVICE, so that people will not try to use it in dockercloud.
  2. "%s_%s" % (project, service): there could be a problem if either the project name or the service name contains _. Maybe "%s:%s" is better?
  3. Test the length of terms before referencing its element directly.

Also, can you squash the commit and open the PR against staging ?

Thank you.

@dariko dariko force-pushed the additional_services branch from 9632e43 to e58cbfd Compare March 30, 2016 11:38
@dariko
Copy link
Author

dariko commented Mar 30, 2016

ok on all 3 points.
Squashed the commits, I'll reopen against staging

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants