Skip to content

Add extra_hosts to yml configuration --add-hosts #1158

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.

Sign up for GitHub

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

Merged
aanand merged 4 commits into from
Apr 27, 2015
Merged

Add extra_hosts to yml configuration --add-hosts #1158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fb81c37
added the extra_hosts option to the yml configuration which exposes t…
sampwing Jan 15, 2015
8098b65
Fix when pyyaml has interpreted line as a dictionary
tdesvenain Jan 21, 2015
25ee3f0
Remove extra s from --add-host
chernjie Mar 24, 2015
86a08c0
See https://github.com/docker/compose/pull/1158#discussion_r29063218
chernjie Apr 27, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compose/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
'entrypoint',
'env_file',
'environment',
'extra_hosts',
'hostname',
'image',
'links',
Expand All @@ -41,6 +42,9 @@

DOCKER_CONFIG_HINTS = {
'cpu_share': 'cpu_shares',
'add_host': 'extra_hosts',
Copy link

@dnephin dnephin Apr 24, 2015

Choose a reason for hiding this comment

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

maybe 'extra_host': 'extra_hosts', as well ?

Copy link
Author

@chernjie chernjie Apr 27, 2015

Choose a reason for hiding this comment

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

Added @ 86a08c0

'hosts': 'extra_hosts',
'extra_host': 'extra_hosts',
'link': 'links',
'port': 'ports',
'privilege': 'privileged',
Expand Down
29 changes: 29 additions & 0 deletions compose/service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
'dns',
'dns_search',
'env_file',
'extra_hosts',
'net',
'pid',
'privileged',
Expand Down Expand Up @@ -448,6 +449,8 @@ def _get_container_host_config(self, override_options, one_off=False, intermedia

restart = parse_restart_spec(options.get('restart', None))

extra_hosts = build_extra_hosts(options.get('extra_hosts', None))

return create_host_config(
links=self._get_links(link_to_self=one_off),
port_bindings=port_bindings,
Expand All @@ -460,6 +463,7 @@ def _get_container_host_config(self, override_options, one_off=False, intermedia
restart_policy=restart,
cap_add=cap_add,
cap_drop=cap_drop,
extra_hosts=extra_hosts,
pid_mode=pid
)

Expand Down Expand Up @@ -619,3 +623,28 @@ def split_port(port):

external_ip, external_port, internal_port = parts
return internal_port, (external_ip, external_port or None)


def build_extra_hosts(extra_hosts_config):
if not extra_hosts_config:
return {}

if isinstance(extra_hosts_config, list):
extra_hosts_dict = {}
for extra_hosts_line in extra_hosts_config:
if not isinstance(extra_hosts_line, six.string_types):
raise ConfigError(
"extra_hosts_config \"%s\" must be either a list of strings or a string->string mapping," %
extra_hosts_config
)
host, ip = extra_hosts_line.split(':')
extra_hosts_dict.update({host.strip(): ip.strip()})
extra_hosts_config = extra_hosts_dict

if isinstance(extra_hosts_config, dict):
return extra_hosts_config
Copy link

@dnephin dnephin Apr 24, 2015

Choose a reason for hiding this comment

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

It might be a good idea to only support one of these formats. We've supported both in the past, but I'm not sure that's necessary going forward. It seems to cause confusion and extra code.

If we decide to keep lists (or both), I would suggest <host>=<ip> (equals instead of colon) syntax instead. If you forget the quotes it confuses yaml into things its a mapping, and that can be very counter-intuitive to users. If we use an equals, it should understand that it is a string, not a mapping.

If we decide to keep mappings (or both), we should probably do some basic validation of the keys and values, maybe even just assert that they are Truthy (not None or empty strings).

Copy link

@dnephin dnephin Apr 24, 2015

Choose a reason for hiding this comment

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

Oh, I just realized this discussion has already happened. I'll defer to whatever @aanand decides now that I've registered my concerns.


raise ConfigError(
"extra_hosts_config \"%s\" must be either a list of strings or a string->string mapping," %
extra_hosts_config
)
17 changes: 17 additions & 0 deletions docs/yml.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,23 @@ external_links:
- project_db_1:postgresql
```

### extra_hosts

Add hostname mappings. Use the same values as the docker client `--add-host` parameter.

```
extra_hosts:
- "somehost:162.242.195.82"
- "otherhost:50.31.209.229"
```
Copy link

@aanand aanand Apr 14, 2015

Choose a reason for hiding this comment

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

"docker" and "fig" are strange host names - why would I have a host called "fig"? (cc @moxiegirl - any best practices here?)

Could you add an example with a dict? Best if the strings are quoted, e.g.

extra_hosts:
  somehost: "162.242.195.82"
  otherhost: "50.31.209.229"

Copy link
Author

@chernjie chernjie Apr 14, 2015

Choose a reason for hiding this comment

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

Updated examples


An entry with the ip address and hostname will be created in `/etc/hosts` inside containers for this service, e.g:

```
162.242.195.82 somehost
50.31.209.229 otherhost
```

### ports

Expose ports. Either specify both ports (`HOST:CONTAINER`), or just the container
Expand Down
62 changes: 61 additions & 1 deletion tests/integration/service_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
import mock

from compose import Service
from compose.service import CannotBeScaledError
from compose.service import (
CannotBeScaledError,
build_extra_hosts,
ConfigError,
)
from compose.container import Container
from docker.errors import APIError
from .testcases import DockerClientTestCase
Expand Down Expand Up @@ -107,6 +111,62 @@ def test_create_container_with_cpu_shares(self):
service.start_container(container)
self.assertEqual(container.inspect()['Config']['CpuShares'], 73)

def test_build_extra_hosts(self):
# string
self.assertRaises(ConfigError, lambda: build_extra_hosts("www.example.com: 192.168.0.17"))

# list of strings
self.assertEqual(build_extra_hosts(
["www.example.com:192.168.0.17"]),
{'www.example.com': '192.168.0.17'})
self.assertEqual(build_extra_hosts(
["www.example.com: 192.168.0.17"]),
{'www.example.com': '192.168.0.17'})
self.assertEqual(build_extra_hosts(
["www.example.com: 192.168.0.17",
"static.example.com:192.168.0.19",
"api.example.com: 192.168.0.18"]),
{'www.example.com': '192.168.0.17',
'static.example.com': '192.168.0.19',
'api.example.com': '192.168.0.18'})

# list of dictionaries
self.assertRaises(ConfigError, lambda: build_extra_hosts(
[{'www.example.com': '192.168.0.17'},
{'api.example.com': '192.168.0.18'}]))

# dictionaries
self.assertEqual(build_extra_hosts(
{'www.example.com': '192.168.0.17',
'api.example.com': '192.168.0.18'}),
{'www.example.com': '192.168.0.17',
'api.example.com': '192.168.0.18'})

def test_create_container_with_extra_hosts_list(self):
extra_hosts = ['somehost:162.242.195.82', 'otherhost:50.31.209.229']
service = self.create_service('db', extra_hosts=extra_hosts)
container = service.create_container()
service.start_container(container)
self.assertEqual(set(container.get('HostConfig.ExtraHosts')), set(extra_hosts))

def test_create_container_with_extra_hosts_string(self):
extra_hosts = 'somehost:162.242.195.82'
service = self.create_service('db', extra_hosts=extra_hosts)
self.assertRaises(ConfigError, lambda: service.create_container())

def test_create_container_with_extra_hosts_list_of_dicts(self):
extra_hosts = [{'somehost': '162.242.195.82'}, {'otherhost': '50.31.209.229'}]
service = self.create_service('db', extra_hosts=extra_hosts)
self.assertRaises(ConfigError, lambda: service.create_container())

def test_create_container_with_extra_hosts_dicts(self):
extra_hosts = {'somehost': '162.242.195.82', 'otherhost': '50.31.209.229'}
extra_hosts_list = ['somehost:162.242.195.82', 'otherhost:50.31.209.229']
service = self.create_service('db', extra_hosts=extra_hosts)
container = service.create_container()
service.start_container(container)
self.assertEqual(set(container.get('HostConfig.ExtraHosts')), set(extra_hosts_list))

def test_create_container_with_specified_volume(self):
host_path = '/tmp/host-path'
container_path = '/container-path'
Expand Down