Skip to content

Commit

Permalink
Alias containers by short id
Browse files Browse the repository at this point in the history
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
  • Loading branch information
aanand committed Jan 25, 2016
1 parent 35788f4 commit 313c584
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 24 deletions.
31 changes: 9 additions & 22 deletions compose/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,12 +430,16 @@ def start_container(self, container):
return container

def connect_container_to_networks(self, container):
one_off = (container.labels.get(LABEL_ONE_OFF) == "True")
connected_networks = container.get('NetworkSettings.Networks')

for network in self.networks:
if network in connected_networks:

This comment has been minimized.

Copy link
@thecloudtaylor

thecloudtaylor Mar 7, 2016

Any specific reason you are first disconnecting the containers before connecting? This causes a problem with Windows Containers as we don't support disconnecting running containers from the network currently...

This comment has been minimized.

Copy link
@dnephin

dnephin Mar 7, 2016

Yes, we have to do this to add the container short id as an alias. The order has to be:

  1. Create a container, connect it to a single network. If we don't connect it to any networks we won't be able to connect later (I believe).
  2. Disconnect it from the networks so that we can reconnect with the proper aliases.
  3. Connect to all networks (one at a time), with the correct aliases, which we didn't have in (1) because we didn't have a container id yet.

If networks provided the container short id as a default alias, I believe we could avoid this.

self.client.disconnect_container_from_network(
container.id, network)

self.client.connect_container_to_network(
container.id, network,
aliases=self._get_aliases(one_off=one_off),
aliases=self._get_aliases(container),
links=self._get_links(False),
)

Expand Down Expand Up @@ -507,11 +511,11 @@ def _next_container_number(self, one_off=False):
numbers = [c.number for c in containers]
return 1 if not numbers else max(numbers) + 1

def _get_aliases(self, one_off):
if one_off:
def _get_aliases(self, container):
if container.labels.get(LABEL_ONE_OFF) == "True":
return []

return [self.name]
return [self.name, container.short_id]

def _get_links(self, link_to_self):
links = {}
Expand Down Expand Up @@ -618,9 +622,6 @@ def _get_container_create_options(
override_options,
one_off=one_off)

container_options['networking_config'] = self._get_container_networking_config(
one_off=one_off)

return container_options

def _get_container_host_config(self, override_options, one_off=False):
Expand Down Expand Up @@ -655,20 +656,6 @@ def _get_container_host_config(self, override_options, one_off=False):
cpu_quota=options.get('cpu_quota'),
)

def _get_container_networking_config(self, one_off=False):
if self.net.mode in ['host', 'bridge']:
return None

if self.net.mode not in self.networks:
return None

return self.client.create_networking_config({
self.net.mode: self.client.create_endpoint_config(
aliases=self._get_aliases(one_off=one_off),
links=self._get_links(False),
)
})

def build(self, no_cache=False, pull=False, force_rm=False):
log.info('Building %s' % self.name)

Expand Down
8 changes: 6 additions & 2 deletions tests/acceptance/cli_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,12 @@ def test_up(self):
container = containers[0]
self.assertIn(container.id, network['Containers'])

networks = list(container.get('NetworkSettings.Networks'))
self.assertEqual(networks, [network['Name']])
networks = container.get('NetworkSettings.Networks')
self.assertEqual(list(networks), [network['Name']])

self.assertEqual(
sorted(networks[network['Name']]['Aliases']),
sorted([service.name, container.short_id]))

for service in services:
assert self.lookup(container, service.name)
Expand Down

0 comments on commit 313c584

Please sign in to comment.