Merge pull request #2723 from aanand/fix-run-with-networking

Fix 'run' behaviour with networks

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'])
+            pty = PseudoTerminal(project.client, container.id, interactive=not options['-T'])
-            service.connect_container_to_networks(container)
+            sockets = pty.sockets()
+            service.start_container(container)
+            pty.start(sockets)
             exit_code = container.wait()
         except signals.ShutdownException:
             project.client.stop(container.id)

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,14 +655,18 @@ class Service(object):
             cpu_quota=options.get('cpu_quota'),
         )
 
-    def _get_container_networking_config(self):
+    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.name],
+                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):

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

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):
+    def test_run_interactive_connects_to_network(self):
-        self.base_dir = 'tests/fixtures/v2-simple'
+        self.base_dir = 'tests/fixtures/networks'
-        self.dispatch(['run', 'simple', 'true'], None)
+
-        service = self.project.get_service('simple')
+        self.dispatch(['up', '-d'])
-        container, = service.containers(stopped=True, one_off=True)
+        self.dispatch(['run', 'app', 'nslookup', 'app'])
-        networks = self.client.networks(names=[self.project.default_network.full_name])
+        self.dispatch(['run', 'app', 'nslookup', 'db'])
-        self.assertEqual(len(networks), 1)
+
-        self.assertEqual(container.human_readable_command, u'true')
+        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'])

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)
+    @mock.patch('compose.cli.main.PseudoTerminal', autospec=True)
-    def test_run_with_environment_merged_with_options_list(self, mock_dockerpty):
+    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)