From a14906fd35f5c12b33631456bb7bc3dfd0a0be36 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 21 Jan 2016 16:49:50 +0000 Subject: [PATCH 1/3] Fix 'run' behaviour with networks - Test that one-off containers join all networks - Don't set any aliases Signed-off-by: Aanand Prasad --- compose/service.py | 17 ++++++++++--- tests/acceptance/cli_test.py | 49 ++++++++++++++++++++++++++++++------ 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/compose/service.py b/compose/service.py index 8afc59c1..ebe7978c 100644 --- a/compose/service.py +++ b/compose/service.py @@ -430,10 +430,12 @@ class Service(object): return container def connect_container_to_networks(self, container): + one_off = (container.labels.get(LABEL_ONE_OFF) == "True") + for network in self.networks: self.client.connect_container_to_network( container.id, network, - aliases=[self.name], + aliases=self._get_aliases(one_off=one_off), links=self._get_links(False), ) @@ -505,6 +507,12 @@ class Service(object): 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: + return [] + + return [self.name] + def _get_links(self, link_to_self): links = {} @@ -610,7 +618,8 @@ class Service(object): override_options, one_off=one_off) - container_options['networking_config'] = self._get_container_networking_config() + container_options['networking_config'] = self._get_container_networking_config( + one_off=one_off) return container_options @@ -646,10 +655,10 @@ class Service(object): cpu_quota=options.get('cpu_quota'), ) - def _get_container_networking_config(self): + def _get_container_networking_config(self, one_off=False): return self.client.create_networking_config({ network_name: self.client.create_endpoint_config( - aliases=[self.name], + aliases=self._get_aliases(one_off=one_off), links=self._get_links(False), ) for network_name in self.networks diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 1806e707..6ae04ee5 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -903,14 +903,47 @@ class CLITestCase(DockerClientTestCase): self.assertEqual(container.name, name) @v2_only() - def test_run_with_networking(self): - self.base_dir = 'tests/fixtures/v2-simple' - self.dispatch(['run', 'simple', 'true'], None) - service = self.project.get_service('simple') - container, = service.containers(stopped=True, one_off=True) - networks = self.client.networks(names=[self.project.default_network.full_name]) - self.assertEqual(len(networks), 1) - self.assertEqual(container.human_readable_command, u'true') + def test_run_interactive_connects_to_network(self): + self.base_dir = 'tests/fixtures/networks' + + self.dispatch(['up', '-d']) + self.dispatch(['run', 'app', 'nslookup', 'app']) + self.dispatch(['run', 'app', 'nslookup', 'db']) + + containers = self.project.get_service('app').containers( + stopped=True, one_off=True) + assert len(containers) == 2 + + for container in containers: + networks = container.get('NetworkSettings.Networks') + + assert sorted(list(networks)) == [ + '{}_{}'.format(self.project.name, name) + for name in ['back', 'front'] + ] + + for _, config in networks.items(): + assert not config['Aliases'] + + @v2_only() + def test_run_detached_connects_to_network(self): + self.base_dir = 'tests/fixtures/networks' + self.dispatch(['up', '-d']) + self.dispatch(['run', '-d', 'app', 'top']) + + container = self.project.get_service('app').containers(one_off=True)[0] + networks = container.get('NetworkSettings.Networks') + + assert sorted(list(networks)) == [ + '{}_{}'.format(self.project.name, name) + for name in ['back', 'front'] + ] + + for _, config in networks.items(): + assert not config['Aliases'] + + assert self.lookup(container, 'app') + assert self.lookup(container, 'db') def test_run_handles_sigint(self): proc = start_process(self.base_dir, ['run', '-T', 'simple', 'top']) From 09dbc7b4cbbd27a36838bd60f62d6ff740da97f6 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 21 Jan 2016 18:04:50 +0000 Subject: [PATCH 2/3] Stop connecting to all networks on container creation This relies on an Engine behaviour which is a bug, not an intentional feature - we have to connect to networks one at a time Signed-off-by: Aanand Prasad --- compose/service.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/compose/service.py b/compose/service.py index ebe7978c..18322846 100644 --- a/compose/service.py +++ b/compose/service.py @@ -656,13 +656,17 @@ class Service(object): ) 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({ - network_name: self.client.create_endpoint_config( + self.net.mode: self.client.create_endpoint_config( aliases=self._get_aliases(one_off=one_off), links=self._get_links(False), ) - for network_name in self.networks - if network_name not in ['host', 'bridge'] }) def build(self, no_cache=False, pull=False, force_rm=False): From f3e55568d1a7c111398876bec84da8ed8b42da7f Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 21 Jan 2016 18:34:18 +0000 Subject: [PATCH 3/3] Fix interactive run with networking Make sure we connect the container to all required networks *after* starting the container and *before* hijacking the terminal. Signed-off-by: Aanand Prasad --- compose/cli/main.py | 8 +++++--- requirements.txt | 2 +- tests/unit/cli_test.py | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index 4be8536f..7409107d 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -41,7 +41,7 @@ from .utils import yesno if not IS_WINDOWS_PLATFORM: - import dockerpty + from dockerpty.pty import PseudoTerminal log = logging.getLogger(__name__) console_handler = logging.StreamHandler(sys.stderr) @@ -709,8 +709,10 @@ def run_one_off_container(container_options, project, service, options): signals.set_signal_handler_to_shutdown() try: try: - dockerpty.start(project.client, container.id, interactive=not options['-T']) - service.connect_container_to_networks(container) + pty = PseudoTerminal(project.client, container.id, interactive=not options['-T']) + sockets = pty.sockets() + service.start_container(container) + pty.start(sockets) exit_code = container.wait() except signals.ShutdownException: project.client.stop(container.id) diff --git a/requirements.txt b/requirements.txt index 563baa10..45e18528 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,8 @@ PyYAML==3.11 cached-property==1.2.0 -dockerpty==0.3.4 docopt==0.6.1 enum34==1.0.4 +git+https://github.com/d11wtq/dockerpty.git@29b1394108b017ef3e3deaf00604a9eb99880d5e#egg=dockerpty git+https://github.com/docker/docker-py.git@master#egg=docker-py jsonschema==2.5.1 requests==2.7.0 diff --git a/tests/unit/cli_test.py b/tests/unit/cli_test.py index a5767097..f9e3fb8f 100644 --- a/tests/unit/cli_test.py +++ b/tests/unit/cli_test.py @@ -72,8 +72,8 @@ class CLITestCase(unittest.TestCase): TopLevelCommand().dispatch(['help', 'nonexistent'], None) @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason="requires dockerpty") - @mock.patch('compose.cli.main.dockerpty', autospec=True) - def test_run_with_environment_merged_with_options_list(self, mock_dockerpty): + @mock.patch('compose.cli.main.PseudoTerminal', autospec=True) + def test_run_with_environment_merged_with_options_list(self, mock_pseudo_terminal): command = TopLevelCommand() mock_client = mock.create_autospec(docker.Client) mock_project = mock.Mock(client=mock_client)