From 1bb4da1216e2478fcf032bef0cd32b9b65d25399 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Thu, 26 Jun 2014 17:30:21 +0100 Subject: [PATCH 01/34] A skeleton test --- flocker/node/functional/test_gear.py | 29 ++++++++++++++++++++++++++-- flocker/node/gear.py | 10 ++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index 4c6fdc52d3..7df11a8575 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -53,15 +53,17 @@ class GearClientTests(TestCase): def setUp(self): pass - def start_container(self, name, ports=None): + def start_container(self, name, ports=None, links=None): """Start a unit and wait until it's up and running. :param unicode name: The name of the unit. + :param list links: A list of ``PortMap`` instances describing the + network links between the container and the host. :return: Deferred that fires when the unit is running. """ client = GearClient("127.0.0.1") - d = client.add(name, u"openshift/busybox-http-app", ports=ports) + d = client.add(name, u"openshift/busybox-http-app", ports=ports, links=links) self.addCleanup(client.remove, name) def is_started(data): @@ -218,3 +220,26 @@ def test_add_error_if_external_port_in_use(self): Raises error if the chosen external port is already exposed. """ self.fail() + + + def test_add_with_links(self): + """ + GearClient.add accepts a links argument which sets up links between + container local ports and host local ports. + """ + # This is the target of the proxy which will be created. + server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + server.setblocking(0) + server.bind((b'127.0.0.1', 0)) + server.listen(1) + host_port = server.getsockname()[1] + name = random_name() + d = self.start_container( + name, links=[PortMap(internal=8080, external=host_port)]) + + def started(ignored): + accepted, client_address = server.accept() + self.assertEqual(b'XXX', accepted.read()) + d.addCallback(started) + + return d diff --git a/flocker/node/gear.py b/flocker/node/gear.py index 0ab53f0beb..e54c1bb008 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -28,7 +28,7 @@ class GearError(Exception): class IGearClient(Interface): """A client for the geard HTTP API.""" - def add(unit_name, image_name, ports=None): + def add(unit_name, image_name, ports=None, links=None): """Install and start a new unit. :param unicode unit_name: The name of the unit to create. @@ -38,6 +38,9 @@ def add(unit_name, image_name, ports=None): :param list ports: A list of ``PortMap``\ s mapping ports exposed in the container to ports exposed on the host. + :param list links: A list of ``PortMap``\ s mapping ports forwarded from + the container to ports on the host. + :return: ``Deferred`` that fires on success, or errbacks with :class:`AlreadyExists` if a unit by that name already exists. """ @@ -116,10 +119,13 @@ def _ensure_ok(self, response): d.addCallback(lambda data: fail(GearError(response.code, data))) return d - def add(self, unit_name, image_name, ports=None): + def add(self, unit_name, image_name, ports=None, links=None): if ports is None: ports = [] + if links is None: + links = [] + data = {u"Image": image_name, u"Started": True, u'Ports': []} for port in ports: data['Ports'].append( From 79724bbdee34204788a388c6503789a8448bcbec Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Thu, 26 Jun 2014 17:31:31 +0100 Subject: [PATCH 02/34] Dockerfile to run nc against a fixed local port --- flocker/node/functional/Dockerfile | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 flocker/node/functional/Dockerfile diff --git a/flocker/node/functional/Dockerfile b/flocker/node/functional/Dockerfile new file mode 100644 index 0000000000..29d41a295c --- /dev/null +++ b/flocker/node/functional/Dockerfile @@ -0,0 +1,3 @@ +FROM busybox +MAINTAINER Hybrid Logic +CMD ["/usr/bin/nc", "127.0.0.1", "31337"] From d5c78a5ba93e3b93a686a2f8f839d8b96dec0b7d Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Thu, 26 Jun 2014 17:52:19 +0100 Subject: [PATCH 03/34] A helper to build the Dockerfile for the test --- flocker/node/functional/test_gear.py | 38 +++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index 7df11a8575..160c6436a3 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -15,6 +15,8 @@ from treq import request, content +from characteristic import attributes + from ...testtools import loop_until, loop_until2 from ..test.test_gear import make_igearclient_tests, random_name from ..gear import GearClient, GearError, GEAR_PORT, PortMap @@ -53,7 +55,7 @@ class GearClientTests(TestCase): def setUp(self): pass - def start_container(self, name, ports=None, links=None): + def start_container(self, name, image_name=u"openshift/busybox-http-app", ports=None, links=None): """Start a unit and wait until it's up and running. :param unicode name: The name of the unit. @@ -63,7 +65,7 @@ def start_container(self, name, ports=None, links=None): :return: Deferred that fires when the unit is running. """ client = GearClient("127.0.0.1") - d = client.add(name, u"openshift/busybox-http-app", ports=ports, links=links) + d = client.add(name, image_name, ports=ports, links=links) self.addCleanup(client.remove, name) def is_started(data): @@ -227,6 +229,16 @@ def test_add_with_links(self): GearClient.add accepts a links argument which sets up links between container local ports and host local ports. """ + internal_port = 31337 + image_name = b'flocker/send_xxx_to_31337' + # Create a Docker image + image = DockerImageBuilder( + docker_dir=os.path.dirname(__file__), + tag=image_name + ) + image.build() + self.addCleanup(image.remove) + # This is the target of the proxy which will be created. server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) server.setblocking(0) @@ -235,7 +247,7 @@ def test_add_with_links(self): host_port = server.getsockname()[1] name = random_name() d = self.start_container( - name, links=[PortMap(internal=8080, external=host_port)]) + name, image_name=image_name, links=[PortMap(internal=internal_port, external=host_port)]) def started(ignored): accepted, client_address = server.accept() @@ -243,3 +255,23 @@ def started(ignored): d.addCallback(started) return d + + +@attributes(['docker_dir', 'tag']) +class DockerImageBuilder(object): + def build(self): + command = [ + b'docker', b'build', + b'--force-rm', + b'--tag=%s' % (self.tag,), + self.docker_dir + ] + subprocess.check_call(command) + + def remove(self): + command = [ + b'docker', b'rmi', + b'--force', + self.tag + ] + subprocess.check_call(command) From d30af41cd76a8adcc0085b78c9ecd8e05e3bac35 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Fri, 27 Jun 2014 14:41:59 +0100 Subject: [PATCH 04/34] twiddling --- flocker/node/functional/test_gear.py | 31 ++++++++++++++++++---------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index 185367067f..e9bb32b402 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -56,24 +56,30 @@ class GearClientTests(TestCase): def setUp(self): pass - def start_container(self, name, image_name=u"openshift/busybox-http-app", ports=None, links=None): + def start_container(self, unit_name, + image_name=u"openshift/busybox-http-app", + ports=None, links=None): """Start a unit and wait until it's up and running. - :param unicode name: The name of the unit. - :param list links: A list of ``PortMap`` instances describing the - network links between the container and the host. - + :param unicode unit_name: See ``IGearClient.add``. + :param unicode image_name: See ``IGearClient.add``. :param list ports: See ``IGearClient.add``. + :param list links: See ``IGearClient.add``. :return: Deferred that fires when the unit is running. """ client = GearClient("127.0.0.1") - d = client.add(name, image_name, ports=ports, links=links) - self.addCleanup(client.remove, name) + d = client.add( + unit_name=unit_name, + image_name=image_name, + ports=ports, + links=links, + ) + self.addCleanup(client.remove, unit_name) def is_started(data): return [container for container in data[u"Containers"] if - (container[u"Id"] == name and + (container[u"Id"] == unit_name and container[u"SubState"] == u"running")] def check_if_started(): @@ -233,13 +239,16 @@ def test_add_with_links(self): server.bind((b'127.0.0.1', 0)) server.listen(1) host_port = server.getsockname()[1] - name = random_name() + name = (self.id().replace('.', '_') + b'_' + random_name())[-24:] d = self.start_container( - name, image_name=image_name, links=[PortMap(internal=internal_port, external=host_port)]) + unit_name=name, + image_name=image_name, + links=[PortMap(internal=internal_port, external=host_port)] + ) def started(ignored): accepted, client_address = server.accept() - self.assertEqual(b'XXX', accepted.read()) + self.assertEqual(b'xxx', accepted.read()) d.addCallback(started) return d From 4c512adcfc4269582827e438c534814ddfff14e4 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Fri, 27 Jun 2014 15:40:50 +0100 Subject: [PATCH 05/34] gear and systemd react badly to long names starting with _ --- flocker/node/functional/test_gear.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index e9bb32b402..044b7d4f72 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -239,7 +239,7 @@ def test_add_with_links(self): server.bind((b'127.0.0.1', 0)) server.listen(1) host_port = server.getsockname()[1] - name = (self.id().replace('.', '_') + b'_' + random_name())[-24:] + name = random_name() d = self.start_container( unit_name=name, image_name=image_name, From 6fb2e8098ab3f8c8b60c29fb189f4407b3ec336e Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Mon, 30 Jun 2014 15:07:02 +0100 Subject: [PATCH 06/34] Committing something ugly that sort of works --- flocker/node/functional/Dockerfile | 2 +- flocker/node/functional/test_gear.py | 8 +++++--- flocker/node/gear.py | 12 +++++++++++- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/flocker/node/functional/Dockerfile b/flocker/node/functional/Dockerfile index 29d41a295c..38ed5ea9aa 100644 --- a/flocker/node/functional/Dockerfile +++ b/flocker/node/functional/Dockerfile @@ -1,3 +1,3 @@ FROM busybox MAINTAINER Hybrid Logic -CMD ["/usr/bin/nc", "127.0.0.1", "31337"] +CMD ["/bin/sh", "-c", "sleep 1 && echo 'xxx' | nc 127.0.0.1 31337"] diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index c86bbcdac5..2d5cb93a64 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -6,6 +6,7 @@ import json import subprocess import socket +import time from unittest import skipIf from twisted.trial.unittest import TestCase @@ -236,9 +237,9 @@ def test_add_with_links(self): # This is the target of the proxy which will be created. server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) server.setblocking(0) - server.bind((b'127.0.0.1', 0)) + server.bind((b'10.0.2.15', 0)) server.listen(1) - host_port = server.getsockname()[1] + host_ip, host_port = server.getsockname()[:2] name = random_name() d = self.start_container( unit_name=name, @@ -247,8 +248,9 @@ def test_add_with_links(self): ) def started(ignored): + time.sleep(5) accepted, client_address = server.accept() - self.assertEqual(b'xxx', accepted.read()) + self.assertEqual(b'xxx\n', accepted.recv(1024)) d.addCallback(started) return d diff --git a/flocker/node/gear.py b/flocker/node/gear.py index 716a3816cc..38379c982b 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -127,11 +127,21 @@ def add(self, unit_name, image_name, ports=None, links=None): if links is None: links = [] - data = {u"Image": image_name, u"Started": True, u'Ports': []} + data = { + u"Image": image_name, u"Started": True, u'Ports': [], u'NetworkLinks': []} + for port in ports: data['Ports'].append( {u'Internal': port.internal, u'External': port.external}) + for link in links: + data['NetworkLinks'].append( + {u'FromHost': u'127.0.0.1', + u'FromPort': link.internal, + u'ToHost': u'10.0.2.15', + u'ToPort': link.external} + ) + checked = self.exists(unit_name) checked.addCallback( lambda exists: fail(AlreadyExists(unit_name)) if exists else None) From ca758f30a70a6f463bb9a5ace9e82c49196ef305 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Mon, 30 Jun 2014 15:59:22 +0100 Subject: [PATCH 07/34] Disable the docker image cleanup step which was failing intermittently and test that links also work with other local addresses. --- flocker/node/functional/test_gear.py | 4 ++-- flocker/node/gear.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index 2d5cb93a64..f1d876f209 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -232,12 +232,12 @@ def test_add_with_links(self): tag=image_name ) image.build() - self.addCleanup(image.remove) +# self.addCleanup(image.remove) # This is the target of the proxy which will be created. server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) server.setblocking(0) - server.bind((b'10.0.2.15', 0)) + server.bind((b'172.17.42.1', 0)) server.listen(1) host_ip, host_port = server.getsockname()[:2] name = random_name() diff --git a/flocker/node/gear.py b/flocker/node/gear.py index 38379c982b..0ca7a40b66 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -138,7 +138,7 @@ def add(self, unit_name, image_name, ports=None, links=None): data['NetworkLinks'].append( {u'FromHost': u'127.0.0.1', u'FromPort': link.internal, - u'ToHost': u'10.0.2.15', + u'ToHost': u'172.17.42.1', u'ToPort': link.external} ) From c897ee89ce6f066b01457eda865239901f9e1b96 Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Mon, 30 Jun 2014 08:33:32 -0700 Subject: [PATCH 08/34] Start of including IP address as part of PortMap --- flocker/node/functional/test_gear.py | 19 +++++++++++++++++-- flocker/node/gear.py | 4 ++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index 2d5cb93a64..2a1b463191 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -219,6 +219,19 @@ def started(response): return d + def _first_non_loopback_address(self): + """ + Return an IPv4 address found in system configuration. + + :return: An ``IPv4Address`` address the machine is configured with. + """ + from netifaces import interfaces, ifaddresses, AF_INET + + for interface in interfaces(): + for link in ifaddresses(interface)[AF_INET]: + if link['addr'] != b'127.0.0.1': + return link['addr'] + def test_add_with_links(self): """ GearClient.add accepts a links argument which sets up links between @@ -237,14 +250,16 @@ def test_add_with_links(self): # This is the target of the proxy which will be created. server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) server.setblocking(0) - server.bind((b'10.0.2.15', 0)) + + address = self._first_non_loopback_address() + server.bind((address, 0)) server.listen(1) host_ip, host_port = server.getsockname()[:2] name = random_name() d = self.start_container( unit_name=name, image_name=image_name, - links=[PortMap(internal=internal_port, external=host_port)] + links=[PortMap(internal=internal_port, external=host_port, internal_address=address)] ) def started(ignored): diff --git a/flocker/node/gear.py b/flocker/node/gear.py index 38379c982b..4dd4fcc931 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -138,7 +138,7 @@ def add(self, unit_name, image_name, ports=None, links=None): data['NetworkLinks'].append( {u'FromHost': u'127.0.0.1', u'FromPort': link.internal, - u'ToHost': u'10.0.2.15', + u'ToHost': link.internal_address, u'ToPort': link.external} ) @@ -212,7 +212,7 @@ def remove(self, unit_name): return succeed(None) -@attributes(['internal', 'external']) +@attributes(['internal', 'external', 'internal_address']) class PortMap(object): """ A record representing the mapping between a port exposed internally by a From 4417166e214025bc59c1276b4f4b28d03dfdc4e4 Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Mon, 30 Jun 2014 08:38:43 -0700 Subject: [PATCH 09/34] Fixed merge conflict --- flocker/node/gear.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/flocker/node/gear.py b/flocker/node/gear.py index 479d448262..4dd4fcc931 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -138,11 +138,7 @@ def add(self, unit_name, image_name, ports=None, links=None): data['NetworkLinks'].append( {u'FromHost': u'127.0.0.1', u'FromPort': link.internal, -<<<<<<< HEAD u'ToHost': link.internal_address, -======= - u'ToHost': u'172.17.42.1', ->>>>>>> ca758f30a70a6f463bb9a5ace9e82c49196ef305 u'ToPort': link.external} ) From a81fddba3991f5b05a89fb7166d020c12ebe8d0d Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Mon, 30 Jun 2014 16:50:22 +0100 Subject: [PATCH 10/34] Continuation of adding internal_address to PortMap --- flocker/node/gear.py | 7 ++++--- flocker/node/test/test_gear.py | 23 ++++++++++++----------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/flocker/node/gear.py b/flocker/node/gear.py index 4dd4fcc931..f886a803fe 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -212,12 +212,13 @@ def remove(self, unit_name): return succeed(None) -@attributes(['internal', 'external', 'internal_address']) +@attributes(['internal_address', 'internal_port', 'external_port']) class PortMap(object): """ A record representing the mapping between a port exposed internally by a docker container and the corresponding external port on the host. - :ivar int internal: The port number exposed by the container. - :ivar int external: The port number exposed by the host + :ivar IPv4Address internal_address: An IP address of the container. + :ivar int internal_port: The port number exposed by the container. + :ivar int external_port: The port number exposed by the host """ diff --git a/flocker/node/test/test_gear.py b/flocker/node/test/test_gear.py index 62b7181ddf..3f5ff35802 100644 --- a/flocker/node/test/test_gear.py +++ b/flocker/node/test/test_gear.py @@ -127,29 +127,30 @@ class PortMapTests(TestCase): """ def test_repr(self): """ - ``PortMap.__repr__`` shows the internal and external ports. + ``PortMap.__repr__`` shows the internal address and the internal and + external ports. """ self.assertEqual( - "", - repr(PortMap(internal=1234, external=5678)) + "", + repr(PortMap(internal_address='1.2.3.4', internal_port=5678, external_port=910)) ) def test_equal(self): """ - ``PortMap`` instances with the same internal and external ports compare - equal. + ``PortMap`` instances with the same internal address and the same + internal and external ports compare equal. """ self.assertEqual( - PortMap(internal=1234, external=5678), - PortMap(internal=1234, external=5678) + PortMap(internal_address='1.2.3.4', internal_port=5678, external_port=910), + PortMap(internal_address='11.12.13.14', internal_port=1516, external_port=1718) ) def test_not_equal(self): """ - ``PortMap`` instances with the different internal and external ports - do not compare equal. + ``PortMap`` instances with the different internal addresses and + internal and external ports do not compare equal. """ self.assertNotEqual( - PortMap(internal=5678, external=1234), - PortMap(internal=1234, external=5678) + PortMap(internal_address='1.2.3.4', internal_port=5678, external_port=910), + PortMap(internal_address='11.12.13.14', internal_port=1516, external_port=1718) ) From 3c3a61238f365043751e5fc30879e9f841f37f84 Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Mon, 30 Jun 2014 16:59:15 +0100 Subject: [PATCH 11/34] Account for PortMap changes --- flocker/node/functional/test_gear.py | 4 ++-- flocker/node/gear.py | 6 +++--- flocker/node/test/test_gear.py | 9 +++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index 364657783f..60f081e3b5 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -206,7 +206,7 @@ def test_add_with_port(self): external_port = find_free_port()[1] name = random_name() d = self.start_container( - name, ports=[PortMap(internal=8080, external=external_port)]) + name, ports=[PortMap(internal_address='1.2.3.4', internal_port=8080, external_port=external_port)]) d.addCallback( lambda ignored: self.request_until_response(external_port)) @@ -258,7 +258,7 @@ def test_add_with_links(self): d = self.start_container( unit_name=name, image_name=image_name, - links=[PortMap(internal=internal_port, external=host_port, internal_address=address)] + links=[PortMap(internal_address=address, internal_port=internal_port, external_port=host_port)] ) def started(ignored): diff --git a/flocker/node/gear.py b/flocker/node/gear.py index f886a803fe..40fb27836b 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -132,14 +132,14 @@ def add(self, unit_name, image_name, ports=None, links=None): for port in ports: data['Ports'].append( - {u'Internal': port.internal, u'External': port.external}) + {u'Internal': port.internal_port, u'External': port.external_port}) for link in links: data['NetworkLinks'].append( {u'FromHost': u'127.0.0.1', - u'FromPort': link.internal, + u'FromPort': link.internal_port, u'ToHost': link.internal_address, - u'ToPort': link.external} + u'ToPort': link.external_port} ) checked = self.exists(unit_name) diff --git a/flocker/node/test/test_gear.py b/flocker/node/test/test_gear.py index 3f5ff35802..b7a64d0480 100644 --- a/flocker/node/test/test_gear.py +++ b/flocker/node/test/test_gear.py @@ -107,8 +107,9 @@ class PortMapInitTests( make_with_init_tests( record_type=PortMap, kwargs=dict( - internal=1234, - external=5678, + internal_address='1.2.3.4', + internal_port=5678, + external_port=910, ) ) ): @@ -131,7 +132,7 @@ def test_repr(self): external ports. """ self.assertEqual( - "", + "", repr(PortMap(internal_address='1.2.3.4', internal_port=5678, external_port=910)) ) @@ -142,7 +143,7 @@ def test_equal(self): """ self.assertEqual( PortMap(internal_address='1.2.3.4', internal_port=5678, external_port=910), - PortMap(internal_address='11.12.13.14', internal_port=1516, external_port=1718) + PortMap(internal_address='1.2.3.4', internal_port=5678, external_port=910), ) def test_not_equal(self): From 157d1b051ae5ac361c52eb722bbdb241bfde62bf Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Mon, 30 Jun 2014 09:07:52 -0700 Subject: [PATCH 12/34] Changed long lines for flake8 --- flocker/node/functional/test_gear.py | 8 ++++++-- flocker/node/gear.py | 10 ++++++---- flocker/node/test/test_gear.py | 18 ++++++++++++------ 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index 60f081e3b5..0cbbe3ab20 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -206,7 +206,9 @@ def test_add_with_port(self): external_port = find_free_port()[1] name = random_name() d = self.start_container( - name, ports=[PortMap(internal_address='1.2.3.4', internal_port=8080, external_port=external_port)]) + name, ports=[PortMap(internal_address='1.2.3.4', + internal_port=8080, + external_port=external_port)]) d.addCallback( lambda ignored: self.request_until_response(external_port)) @@ -258,7 +260,9 @@ def test_add_with_links(self): d = self.start_container( unit_name=name, image_name=image_name, - links=[PortMap(internal_address=address, internal_port=internal_port, external_port=host_port)] + links=[PortMap(internal_address=address, + internal_port=internal_port, + external_port=host_port)] ) def started(ignored): diff --git a/flocker/node/gear.py b/flocker/node/gear.py index 40fb27836b..494cf06e1c 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -39,8 +39,8 @@ def add(unit_name, image_name, ports=None, links=None): the container to ports exposed on the host. Default ``None`` means that no port mappings will be configured for this unit. - :param list links: A list of ``PortMap``\ s mapping ports forwarded from - the container to ports on the host. + :param list links: A list of ``PortMap``\ s mapping ports forwarded + from the container to ports on the host. :return: ``Deferred`` that fires on success, or errbacks with :class:`AlreadyExists` if a unit by that name already exists. @@ -128,11 +128,13 @@ def add(self, unit_name, image_name, ports=None, links=None): links = [] data = { - u"Image": image_name, u"Started": True, u'Ports': [], u'NetworkLinks': []} + u"Image": image_name, u"Started": True, u'Ports': [], + u'NetworkLinks': []} for port in ports: data['Ports'].append( - {u'Internal': port.internal_port, u'External': port.external_port}) + {u'Internal': port.internal_port, + u'External': port.external_port}) for link in links: data['NetworkLinks'].append( diff --git a/flocker/node/test/test_gear.py b/flocker/node/test/test_gear.py index b7a64d0480..f49e652113 100644 --- a/flocker/node/test/test_gear.py +++ b/flocker/node/test/test_gear.py @@ -132,8 +132,10 @@ def test_repr(self): external ports. """ self.assertEqual( - "", - repr(PortMap(internal_address='1.2.3.4', internal_port=5678, external_port=910)) + (""), + repr(PortMap(internal_address='1.2.3.4', internal_port=5678, + external_port=910)) ) def test_equal(self): @@ -142,8 +144,10 @@ def test_equal(self): internal and external ports compare equal. """ self.assertEqual( - PortMap(internal_address='1.2.3.4', internal_port=5678, external_port=910), - PortMap(internal_address='1.2.3.4', internal_port=5678, external_port=910), + PortMap(internal_address='1.2.3.4', internal_port=5678, + external_port=910), + PortMap(internal_address='1.2.3.4', internal_port=5678, + external_port=910), ) def test_not_equal(self): @@ -152,6 +156,8 @@ def test_not_equal(self): internal and external ports do not compare equal. """ self.assertNotEqual( - PortMap(internal_address='1.2.3.4', internal_port=5678, external_port=910), - PortMap(internal_address='11.12.13.14', internal_port=1516, external_port=1718) + PortMap(internal_address='1.2.3.4', internal_port=5678, + external_port=910), + PortMap(internal_address='11.12.13.14', internal_port=1516, + external_port=1718) ) From af938466127486014728a747885af73b0d4146c5 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Mon, 30 Jun 2014 17:42:52 +0100 Subject: [PATCH 13/34] Don't bother trying to cleanup the image. --- flocker/node/functional/test_gear.py | 1 - 1 file changed, 1 deletion(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index 0cbbe3ab20..4af6052521 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -247,7 +247,6 @@ def test_add_with_links(self): tag=image_name ) image.build() -# self.addCleanup(image.remove) # This is the target of the proxy which will be created. server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) From a65032dcaa3a1443ed8974d7b48b2fe45ca280f6 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Mon, 30 Jun 2014 18:26:21 +0100 Subject: [PATCH 14/34] Ok I realise now that when you specify HostAddress 127.0.0.1 it actually routes to the host's non-loopback ip. So I just need to listen on all interfaces for this test to pass --- flocker/node/functional/test_gear.py | 22 +++------------------- flocker/node/gear.py | 6 +++--- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index 4af6052521..c24df6624e 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -206,8 +206,7 @@ def test_add_with_port(self): external_port = find_free_port()[1] name = random_name() d = self.start_container( - name, ports=[PortMap(internal_address='1.2.3.4', - internal_port=8080, + name, ports=[PortMap(internal_port=8080, external_port=external_port)]) d.addCallback( @@ -221,19 +220,6 @@ def started(response): return d - def _first_non_loopback_address(self): - """ - Return an IPv4 address found in system configuration. - - :return: An ``IPv4Address`` address the machine is configured with. - """ - from netifaces import interfaces, ifaddresses, AF_INET - - for interface in interfaces(): - for link in ifaddresses(interface)[AF_INET]: - if link['addr'] != b'127.0.0.1': - return link['addr'] - def test_add_with_links(self): """ GearClient.add accepts a links argument which sets up links between @@ -251,16 +237,14 @@ def test_add_with_links(self): # This is the target of the proxy which will be created. server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) server.setblocking(0) - address = self._first_non_loopback_address() - server.bind((address, 0)) + server.bind(('0.0.0.0', 0)) server.listen(1) host_ip, host_port = server.getsockname()[:2] name = random_name() d = self.start_container( unit_name=name, image_name=image_name, - links=[PortMap(internal_address=address, - internal_port=internal_port, + links=[PortMap(internal_port=internal_port, external_port=host_port)] ) diff --git a/flocker/node/gear.py b/flocker/node/gear.py index 494cf06e1c..8b8d621bac 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -99,6 +99,7 @@ def _request(self, method, unit_name, operation=None, data=None): url += b"/" + operation if data is not None: data = json.dumps(data) + return request(method, url, data=data, persistent=False) def _ensure_ok(self, response): @@ -140,7 +141,7 @@ def add(self, unit_name, image_name, ports=None, links=None): data['NetworkLinks'].append( {u'FromHost': u'127.0.0.1', u'FromPort': link.internal_port, - u'ToHost': link.internal_address, + u'ToHost': u'127.0.0.1', u'ToPort': link.external_port} ) @@ -214,13 +215,12 @@ def remove(self, unit_name): return succeed(None) -@attributes(['internal_address', 'internal_port', 'external_port']) +@attributes(['internal_port', 'external_port'],) class PortMap(object): """ A record representing the mapping between a port exposed internally by a docker container and the corresponding external port on the host. - :ivar IPv4Address internal_address: An IP address of the container. :ivar int internal_port: The port number exposed by the container. :ivar int external_port: The port number exposed by the host """ From 8277080adccb089a33dfd525e329050849b43d15 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Mon, 30 Jun 2014 18:39:21 +0100 Subject: [PATCH 15/34] Attempt to connect in a loop which allows us to remove all the sleeps --- flocker/node/functional/Dockerfile | 3 --- flocker/node/functional/docker/Dockerfile | 4 ++++ flocker/node/functional/docker/run.sh | 8 ++++++++ flocker/node/functional/test_gear.py | 3 +-- 4 files changed, 13 insertions(+), 5 deletions(-) delete mode 100644 flocker/node/functional/Dockerfile create mode 100644 flocker/node/functional/docker/Dockerfile create mode 100644 flocker/node/functional/docker/run.sh diff --git a/flocker/node/functional/Dockerfile b/flocker/node/functional/Dockerfile deleted file mode 100644 index 38ed5ea9aa..0000000000 --- a/flocker/node/functional/Dockerfile +++ /dev/null @@ -1,3 +0,0 @@ -FROM busybox -MAINTAINER Hybrid Logic -CMD ["/bin/sh", "-c", "sleep 1 && echo 'xxx' | nc 127.0.0.1 31337"] diff --git a/flocker/node/functional/docker/Dockerfile b/flocker/node/functional/docker/Dockerfile new file mode 100644 index 0000000000..5e842ad132 --- /dev/null +++ b/flocker/node/functional/docker/Dockerfile @@ -0,0 +1,4 @@ +FROM busybox +MAINTAINER Hybrid Logic +ADD . / +CMD ["/bin/sh", "-e", "run.sh"] diff --git a/flocker/node/functional/docker/run.sh b/flocker/node/functional/docker/run.sh new file mode 100644 index 0000000000..ce9afbd2aa --- /dev/null +++ b/flocker/node/functional/docker/run.sh @@ -0,0 +1,8 @@ +#!/bin/sh +set -e + +while true; do + if echo "xxx" | nc "127.0.0.1" "31337"; then + break + fi +done diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index c24df6624e..5c65271a0d 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -229,7 +229,7 @@ def test_add_with_links(self): image_name = b'flocker/send_xxx_to_31337' # Create a Docker image image = DockerImageBuilder( - docker_dir=os.path.dirname(__file__), + docker_dir=os.path.join(os.path.dirname(__file__), 'docker'), tag=image_name ) image.build() @@ -249,7 +249,6 @@ def test_add_with_links(self): ) def started(ignored): - time.sleep(5) accepted, client_address = server.accept() self.assertEqual(b'xxx\n', accepted.recv(1024)) d.addCallback(started) From b03d285bd5280aad5ccf5510e234fbb3dce20d69 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Mon, 30 Jun 2014 19:22:35 +0100 Subject: [PATCH 16/34] use twisted to capture the lines sent from the container --- flocker/node/functional/test_gear.py | 42 +++++++++++++++++----------- flocker/testtools.py | 37 +++++++++++++++++++++++- 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index 5c65271a0d..c0c5181dc4 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -6,19 +6,22 @@ import json import subprocess import socket -import time from unittest import skipIf from twisted.trial.unittest import TestCase from twisted.python.procutils import which from twisted.internet.defer import succeed from twisted.internet.error import ConnectionRefusedError +from twisted.internet.endpoints import TCP4ServerEndpoint +from twisted.internet import reactor from treq import request, content from characteristic import attributes -from ...testtools import loop_until, find_free_port +from ...testtools import ( + loop_until, find_free_port, make_line_capture_protocol, + ProtocolPoppingFactory) from ..test.test_gear import make_igearclient_tests, random_name from ..gear import GearClient, GearError, GEAR_PORT, PortMap @@ -222,7 +225,7 @@ def started(response): def test_add_with_links(self): """ - GearClient.add accepts a links argument which sets up links between + ``GearClient.add`` accepts a links argument which sets up links between container local ports and host local ports. """ internal_port = 31337 @@ -235,22 +238,27 @@ def test_add_with_links(self): image.build() # This is the target of the proxy which will be created. - server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - server.setblocking(0) - server.bind(('0.0.0.0', 0)) - server.listen(1) - host_ip, host_port = server.getsockname()[:2] - name = random_name() - d = self.start_container( - unit_name=name, - image_name=image_name, - links=[PortMap(internal_port=internal_port, - external_port=host_port)] - ) + server = TCP4ServerEndpoint(reactor, 0) + capture_finished, protocol = make_line_capture_protocol() + def check_lines(lines): + self.assertEqual([b'xxx'], lines) + capture_finished.addCallback(check_lines) + + factory = ProtocolPoppingFactory(protocols=[protocol]) + d = server.listen(factory) + def start_container(port): + self.addCleanup(port.stopListening) + host_port = port.getHost().port + return self.start_container( + unit_name=random_name(), + image_name=image_name, + links=[PortMap(internal_port=internal_port, + external_port=host_port)] + ) + d.addCallback(start_container) def started(ignored): - accepted, client_address = server.accept() - self.assertEqual(b'xxx\n', accepted.recv(1024)) + return capture_finished d.addCallback(started) return d diff --git a/flocker/testtools.py b/flocker/testtools.py index 8fbe9448c1..d4ab999300 100644 --- a/flocker/testtools.py +++ b/flocker/testtools.py @@ -18,9 +18,12 @@ from twisted.internet.interfaces import IProcessTransport, IReactorProcess from twisted.python.filepath import FilePath from twisted.internet.task import Clock, deferLater -from twisted.internet.defer import maybeDeferred +from twisted.internet.defer import maybeDeferred, Deferred +from twisted.internet.error import ConnectionDone from twisted.internet import reactor from twisted.trial.unittest import SynchronousTestCase +from twisted.protocols.basic import LineReceiver +from twisted.internet.protocol import Factory from . import __version__ from .common.script import ( @@ -351,3 +354,35 @@ def find_free_port(interface='127.0.0.1', socket_family=socket.AF_INET, return probe.getsockname() finally: probe.close() + + +def make_line_capture_protocol(): + """ + Return a deferred, and a protocol which will capture lines and fire the + deferred when its connection is lost. + """ + d = Deferred() + lines = [] + class LineRecorder(LineReceiver): + delimiter = b'\n' + + def lineReceived(self, line): + lines.append(line) + + def connectionLost(self, reason): + if reason.check(ConnectionDone): + d.callback(lines) + else: + d.errback(reason) + return d, LineRecorder() + + +class ProtocolPoppingFactory(Factory): + """ + A factory which only creates a fixed list of protocols. + """ + def __init__(self, protocols): + self.protocols = protocols + + def buildProtocol(self, addr): + return self.protocols.pop() From 1104bf709e6f15d0771a4f509679a85d68ba0f27 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Mon, 30 Jun 2014 19:30:54 +0100 Subject: [PATCH 17/34] remove some of the changes associated with the earlier PortMap.external_address experiment. --- flocker/node/gear.py | 7 +++++-- flocker/node/test/test_gear.py | 30 +++++++++++------------------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/flocker/node/gear.py b/flocker/node/gear.py index 8b8d621bac..fdc6dd42bd 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -194,15 +194,18 @@ class FakeGearClient(object): def __init__(self): self._units = {} - def add(self, unit_name, image_name, ports=None): + def add(self, unit_name, image_name, ports=None, links=None): if ports is None: ports = [] + if links is None: + links = [] if unit_name in self._units: return fail(AlreadyExists(unit_name)) self._units[unit_name] = { 'unit_name': unit_name, 'image_name': image_name, - 'ports': ports + 'ports': ports, + 'links': links, } return succeed(None) diff --git a/flocker/node/test/test_gear.py b/flocker/node/test/test_gear.py index f49e652113..7580b96ca8 100644 --- a/flocker/node/test/test_gear.py +++ b/flocker/node/test/test_gear.py @@ -107,7 +107,6 @@ class PortMapInitTests( make_with_init_tests( record_type=PortMap, kwargs=dict( - internal_address='1.2.3.4', internal_port=5678, external_port=910, ) @@ -128,36 +127,29 @@ class PortMapTests(TestCase): """ def test_repr(self): """ - ``PortMap.__repr__`` shows the internal address and the internal and - external ports. + ``PortMap.__repr__`` shows the internal and external ports. """ self.assertEqual( - (""), - repr(PortMap(internal_address='1.2.3.4', internal_port=5678, - external_port=910)) + (""), + repr(PortMap(internal_port=5678, external_port=910)) ) def test_equal(self): """ - ``PortMap`` instances with the same internal address and the same - internal and external ports compare equal. + ``PortMap`` instances with the same internal and external ports compare + equal. """ self.assertEqual( - PortMap(internal_address='1.2.3.4', internal_port=5678, - external_port=910), - PortMap(internal_address='1.2.3.4', internal_port=5678, - external_port=910), + PortMap(internal_port=5678, external_port=910), + PortMap(internal_port=5678, external_port=910), ) def test_not_equal(self): """ - ``PortMap`` instances with the different internal addresses and - internal and external ports do not compare equal. + ``PortMap`` instances with the different internal and external ports do + not compare equal. """ self.assertNotEqual( - PortMap(internal_address='1.2.3.4', internal_port=5678, - external_port=910), - PortMap(internal_address='11.12.13.14', internal_port=1516, - external_port=1718) + PortMap(internal_port=5678, external_port=910), + PortMap(internal_port=1516, external_port=1718) ) From 6a07879c54f9e9c57961787d2a62320111f211e5 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Mon, 30 Jun 2014 19:38:36 +0100 Subject: [PATCH 18/34] Move DockerImageBuilder to testtools and document it. --- flocker/node/functional/test_gear.py | 24 +------------------ flocker/testtools.py | 35 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index c0c5181dc4..ec2a7794c2 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -17,11 +17,9 @@ from treq import request, content -from characteristic import attributes - from ...testtools import ( loop_until, find_free_port, make_line_capture_protocol, - ProtocolPoppingFactory) + ProtocolPoppingFactory, DockerImageBuilder) from ..test.test_gear import make_igearclient_tests, random_name from ..gear import GearClient, GearError, GEAR_PORT, PortMap @@ -262,23 +260,3 @@ def started(ignored): d.addCallback(started) return d - - -@attributes(['docker_dir', 'tag']) -class DockerImageBuilder(object): - def build(self): - command = [ - b'docker', b'build', - b'--force-rm', - b'--tag=%s' % (self.tag,), - self.docker_dir - ] - subprocess.check_call(command) - - def remove(self): - command = [ - b'docker', b'rmi', - b'--force', - self.tag - ] - subprocess.check_call(command) diff --git a/flocker/testtools.py b/flocker/testtools.py index d4ab999300..e21b1eb9ad 100644 --- a/flocker/testtools.py +++ b/flocker/testtools.py @@ -11,6 +11,7 @@ from collections import namedtuple from contextlib import contextmanager from random import random +import subprocess from zope.interface import implementer from zope.interface.verify import verifyClass @@ -25,6 +26,8 @@ from twisted.protocols.basic import LineReceiver from twisted.internet.protocol import Factory +from characteristic import attributes + from . import __version__ from .common.script import ( FlockerScriptRunner, ICommandLineScript) @@ -386,3 +389,35 @@ def __init__(self, protocols): def buildProtocol(self, addr): return self.protocols.pop() + + +@attributes(['docker_dir', 'tag']) +class DockerImageBuilder(object): + """ + Build a docker image tag it, and optionally remove the image later. + + :ivar bytes docker_dir: The path to the directory containing a `Dockerfile`. + :ivar bytes tag: The tag name to be applied to the built image. + """ + def build(self): + """ + Build an image and tag it in the local Docker repository. + """ + command = [ + b'docker', b'build', + b'--force-rm', + b'--tag=%s' % (self.tag,), + self.docker_dir + ] + subprocess.check_call(command) + + def remove(self): + """ + Forcefully remove the image from the local Docker repository. + """ + command = [ + b'docker', b'rmi', + b'--force', + self.tag + ] + subprocess.check_call(command) From b19f6fcb38dbb6e3ab04f72a59e2df3759b8da78 Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Tue, 1 Jul 2014 07:04:04 -0700 Subject: [PATCH 19/34] Fixed linting errors --- flocker/node/functional/test_gear.py | 2 ++ flocker/testtools.py | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index ec2a7794c2..35e37c19a2 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -238,12 +238,14 @@ def test_add_with_links(self): # This is the target of the proxy which will be created. server = TCP4ServerEndpoint(reactor, 0) capture_finished, protocol = make_line_capture_protocol() + def check_lines(lines): self.assertEqual([b'xxx'], lines) capture_finished.addCallback(check_lines) factory = ProtocolPoppingFactory(protocols=[protocol]) d = server.listen(factory) + def start_container(port): self.addCleanup(port.stopListening) host_port = port.getHost().port diff --git a/flocker/testtools.py b/flocker/testtools.py index a942f88961..aab16dce8c 100644 --- a/flocker/testtools.py +++ b/flocker/testtools.py @@ -367,6 +367,7 @@ def make_line_capture_protocol(): """ d = Deferred() lines = [] + class LineRecorder(LineReceiver): delimiter = b'\n' @@ -397,7 +398,8 @@ class DockerImageBuilder(object): """ Build a docker image tag it, and optionally remove the image later. - :ivar bytes docker_dir: The path to the directory containing a `Dockerfile`. + :ivar bytes docker_dir: The path to the directory containing a + `Dockerfile`. :ivar bytes tag: The tag name to be applied to the built image. """ def build(self): From c444ef8cbdaef4a5cb306d40546b4beedf53b3f3 Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Tue, 1 Jul 2014 07:09:06 -0700 Subject: [PATCH 20/34] Grammar fix --- flocker/testtools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flocker/testtools.py b/flocker/testtools.py index 907e19fca2..50eb57ed0c 100644 --- a/flocker/testtools.py +++ b/flocker/testtools.py @@ -411,7 +411,7 @@ def buildProtocol(self, addr): @attributes(['docker_dir', 'tag']) class DockerImageBuilder(object): """ - Build a docker image tag it, and optionally remove the image later. + Build a docker image, tag it, and optionally remove the image later. :ivar bytes docker_dir: The path to the directory containing a `Dockerfile`. From f67ec330ba9ec85c24eda424f4ce28a0ac1a9932 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 09:46:10 +0100 Subject: [PATCH 21/34] Use clusterhq for maintainer details. --- flocker/node/functional/docker/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flocker/node/functional/docker/Dockerfile b/flocker/node/functional/docker/Dockerfile index 5e842ad132..8aa90e807f 100644 --- a/flocker/node/functional/docker/Dockerfile +++ b/flocker/node/functional/docker/Dockerfile @@ -1,4 +1,4 @@ FROM busybox -MAINTAINER Hybrid Logic +MAINTAINER ClusterHQ ADD . / CMD ["/bin/sh", "-e", "run.sh"] From 55e721149ed0b40e85bb343078c78de9e9d7ce0c Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 10:39:08 +0100 Subject: [PATCH 22/34] Make the Docker run script timeout after a period of time. --- flocker/node/functional/docker/Dockerfile | 2 +- flocker/node/functional/docker/run.sh | 54 +++++++++++++++++++++-- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/flocker/node/functional/docker/Dockerfile b/flocker/node/functional/docker/Dockerfile index 8aa90e807f..7ab56c4cf5 100644 --- a/flocker/node/functional/docker/Dockerfile +++ b/flocker/node/functional/docker/Dockerfile @@ -1,4 +1,4 @@ FROM busybox MAINTAINER ClusterHQ ADD . / -CMD ["/bin/sh", "-e", "run.sh"] +CMD ["/bin/sh", "-e", "run.sh", "127.0.0.1", "31337", "xxx", "10"] diff --git a/flocker/node/functional/docker/run.sh b/flocker/node/functional/docker/run.sh index ce9afbd2aa..edb427521e 100644 --- a/flocker/node/functional/docker/run.sh +++ b/flocker/node/functional/docker/run.sh @@ -1,8 +1,56 @@ #!/bin/sh -set -e +set -e +help() { + cat <&2 + echo "ERROR: Unknown option: $1" >&2 + exit 1 + ;; + *) + break + ;; + esac +done + +HOST=${1:?"Error: Missing parameter 1:HOST"} +PORT=${2:?"Error: Missing parameter 2:PORT"} +BYTES=${3:?"Error: Missing parameter 3:BYTES"} +TIMEOUT=${4:?"Error: Missing parameter 3:TIMEOUT"} + +start_time=$(date +"%s") +# Attempt to connect +# NB nc -w 10 means connection timeout after 10s +while ! echo -n "${BYTES}" | nc -w 10 "${HOST}" "${PORT}"; do + usleep 100000 + if test "$(date +'%s')" -gt "$((start_time+${TIMEOUT}))"; then + echo "ERROR: unable to connect to after ${TIMEOUT} seconds." >&2 break fi done From d86ea75d04487086a8720c0e914d5f764c202bc5 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 10:40:35 +0100 Subject: [PATCH 23/34] The Dockerfile can be a template --- flocker/node/functional/docker/{Dockerfile => Dockerfile.in} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename flocker/node/functional/docker/{Dockerfile => Dockerfile.in} (100%) diff --git a/flocker/node/functional/docker/Dockerfile b/flocker/node/functional/docker/Dockerfile.in similarity index 100% rename from flocker/node/functional/docker/Dockerfile rename to flocker/node/functional/docker/Dockerfile.in From 26f1fa7a934361db4c1c2f3576f445e44d3a962d Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 11:46:22 +0100 Subject: [PATCH 24/34] Implement a Dockerfile template feature which allows the host, port and bytes to be supplied from within the test. --- flocker/node/functional/docker/Dockerfile.in | 2 +- flocker/node/functional/test_gear.py | 29 +++++++--- flocker/testtools.py | 59 +++++++++++++++----- 3 files changed, 65 insertions(+), 25 deletions(-) diff --git a/flocker/node/functional/docker/Dockerfile.in b/flocker/node/functional/docker/Dockerfile.in index 7ab56c4cf5..a0fcaa570a 100644 --- a/flocker/node/functional/docker/Dockerfile.in +++ b/flocker/node/functional/docker/Dockerfile.in @@ -1,4 +1,4 @@ FROM busybox MAINTAINER ClusterHQ ADD . / -CMD ["/bin/sh", "-e", "run.sh", "127.0.0.1", "31337", "xxx", "10"] +CMD ["/bin/sh", "-e", "run.sh", "{host}", "{port}", "{bytes}", "{timeout}"] diff --git a/flocker/node/functional/test_gear.py b/flocker/node/functional/test_gear.py index 2115f7d1a2..5df08b1484 100644 --- a/flocker/node/functional/test_gear.py +++ b/flocker/node/functional/test_gear.py @@ -10,6 +10,7 @@ from twisted.trial.unittest import TestCase from twisted.python.procutils import which +from twisted.python.filepath import FilePath from twisted.internet.defer import succeed from twisted.internet.error import ConnectionRefusedError from twisted.internet.endpoints import TCP4ServerEndpoint @@ -18,7 +19,7 @@ from treq import request, content from ...testtools import ( - loop_until, find_free_port, make_line_capture_protocol, + loop_until, find_free_port, make_capture_protocol, ProtocolPoppingFactory, DockerImageBuilder) from ..test.test_gear import make_igearclient_tests, random_name @@ -251,21 +252,31 @@ def test_add_with_links(self): container local ports and host local ports. """ internal_port = 31337 - image_name = b'flocker/send_xxx_to_31337' + expected_bytes = b'foo bar baz' + image_name = b'flocker/send_bytes_to' # Create a Docker image image = DockerImageBuilder( - docker_dir=os.path.join(os.path.dirname(__file__), 'docker'), - tag=image_name + source_dir=FilePath( + os.path.join(os.path.dirname(__file__), 'docker')), + tag=image_name, + working_dir=FilePath(self.mktemp()) + ) + image.build( + dockerfile_variables=dict( + host=b'127.0.0.1', + port=internal_port, + bytes=expected_bytes, + timeout=30 + ) ) - image.build() # This is the target of the proxy which will be created. server = TCP4ServerEndpoint(reactor, 0) - capture_finished, protocol = make_line_capture_protocol() + capture_finished, protocol = make_capture_protocol() - def check_lines(lines): - self.assertEqual([b'xxx'], lines) - capture_finished.addCallback(check_lines) + def check_data(data): + self.assertEqual(expected_bytes, data) + capture_finished.addCallback(check_data) factory = ProtocolPoppingFactory(protocols=[protocol]) d = server.listen(factory) diff --git a/flocker/testtools.py b/flocker/testtools.py index 0b500a1318..96ce329bca 100644 --- a/flocker/testtools.py +++ b/flocker/testtools.py @@ -13,6 +13,7 @@ from collections import namedtuple from contextlib import contextmanager from random import random +import shutil from subprocess import check_call from functools import wraps @@ -33,8 +34,7 @@ from twisted.conch.openssh_compat.factory import OpenSSHFactory from twisted.conch.unix import UnixConchUser from twisted.trial.unittest import SynchronousTestCase, SkipTest -from twisted.protocols.basic import LineReceiver -from twisted.internet.protocol import Factory +from twisted.internet.protocol import Factory, Protocol from characteristic import attributes @@ -529,26 +529,24 @@ def find_free_port(interface='127.0.0.1', socket_family=socket.AF_INET, probe.close() -def make_line_capture_protocol(): +def make_capture_protocol(): """ - Return a deferred, and a protocol which will capture lines and fire the + Return a deferred, and a protocol which will capture bytes and fire the deferred when its connection is lost. """ d = Deferred() - lines = [] + captured_data = [] - class LineRecorder(LineReceiver): - delimiter = b'\n' - - def lineReceived(self, line): - lines.append(line) + class Recorder(Protocol): + def dataReceived(self, data): + captured_data.append(data) def connectionLost(self, reason): if reason.check(ConnectionDone): - d.callback(lines) + d.callback(b''.join(captured_data)) else: d.errback(reason) - return d, LineRecorder() + return d, Recorder() class ProtocolPoppingFactory(Factory): @@ -562,7 +560,7 @@ def buildProtocol(self, addr): return self.protocols.pop() -@attributes(['docker_dir', 'tag']) +@attributes(['source_dir', 'tag', 'working_dir']) class DockerImageBuilder(object): """ Build a docker image, tag it, and optionally remove the image later. @@ -571,15 +569,46 @@ class DockerImageBuilder(object): `Dockerfile`. :ivar bytes tag: The tag name to be applied to the built image. """ - def build(self): + def _process_template(self, template_file, target_file, replacements): + """ + Fill in the placeholders in `template_file` with the `replacements` and + write the result to `target_file`. + + :param FilePath template_file: The file containing the placeholders. + :param FilePath target_file: The file to which the result will be written. + :param dict replacements: A dictionary of variable names and replacement + values. + """ + with template_file.open() as f: + template = f.read().decode('utf8') + target_file.setContent(template.format(**replacements)) + + def build(self, dockerfile_variables=None): """ Build an image and tag it in the local Docker repository. + + :param dict dockerfile_variables: A dictionary of replacements which + will be applied to a `Dockerfile.in` template file if such a file + exists. """ + if dockerfile_variables is None: + dockerfile_variables = {} + + if not self.working_dir.exists(): + self.working_dir.makedirs() + + docker_dir = self.working_dir.child('docker') + shutil.copytree(self.source_dir.path, docker_dir.path) + template_file = docker_dir.child('Dockerfile.in') + docker_file = docker_dir.child('Dockerfile') + if template_file.exists() and not docker_file.exists(): + self._process_template( + template_file, docker_file, dockerfile_variables) command = [ b'docker', b'build', b'--force-rm', b'--tag=%s' % (self.tag,), - self.docker_dir + docker_dir.path ] check_call(command) From 1d2ff08f95a4135577fe7642dfc631ca92b09b48 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 11:47:38 +0100 Subject: [PATCH 25/34] full stop --- flocker/node/gear.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flocker/node/gear.py b/flocker/node/gear.py index f485eea620..4ed1830681 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -268,5 +268,5 @@ class PortMap(object): docker container and the corresponding external port on the host. :ivar int internal_port: The port number exposed by the container. - :ivar int external_port: The port number exposed by the host + :ivar int external_port: The port number exposed by the host. """ From 1e168f65795a44ffa948ac61632b6aab901f377c Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 11:49:09 +0100 Subject: [PATCH 26/34] Remove unnecessary brackets --- flocker/node/test/test_gear.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flocker/node/test/test_gear.py b/flocker/node/test/test_gear.py index 304d1d35b3..e155183654 100644 --- a/flocker/node/test/test_gear.py +++ b/flocker/node/test/test_gear.py @@ -162,7 +162,7 @@ def test_repr(self): ``PortMap.__repr__`` shows the internal and external ports. """ self.assertEqual( - (""), + "", repr(PortMap(internal_port=5678, external_port=910)) ) From a9f21a1158c622b3229f5914cca3f621e71626b5 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 11:55:26 +0100 Subject: [PATCH 27/34] Highlight Deferred etc in docstring. Add return docs --- flocker/testtools.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/flocker/testtools.py b/flocker/testtools.py index 96ce329bca..debdbccfef 100644 --- a/flocker/testtools.py +++ b/flocker/testtools.py @@ -531,8 +531,11 @@ def find_free_port(interface='127.0.0.1', socket_family=socket.AF_INET, def make_capture_protocol(): """ - Return a deferred, and a protocol which will capture bytes and fire the - deferred when its connection is lost. + Return a ``Deferred``, and a ``Protocol`` which will capture bytes and fire + the ``Deferred`` when its connection is lost. + + :returns: A 2-tuple of ``Deferred`` and ``Protocol`` instance. + :rtype: tuple """ d = Deferred() captured_data = [] From bbd97ca6c03ed92847811f0fbb28e02af54ab31a Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 11:57:20 +0100 Subject: [PATCH 28/34] A note about --force-rm --- flocker/testtools.py | 1 + 1 file changed, 1 insertion(+) diff --git a/flocker/testtools.py b/flocker/testtools.py index debdbccfef..b691af2418 100644 --- a/flocker/testtools.py +++ b/flocker/testtools.py @@ -609,6 +609,7 @@ def build(self, dockerfile_variables=None): template_file, docker_file, dockerfile_variables) command = [ b'docker', b'build', + # Always clean up intermediate containers in case of failures. b'--force-rm', b'--tag=%s' % (self.tag,), docker_dir.path From 10dd47a6fbfd2e177ff9a5be38ece6a781ea777a Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 12:32:47 +0100 Subject: [PATCH 29/34] Document the motivation for ProtocolPoppingFactory --- flocker/testtools.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/flocker/testtools.py b/flocker/testtools.py index b691af2418..8cb8922f07 100644 --- a/flocker/testtools.py +++ b/flocker/testtools.py @@ -555,8 +555,16 @@ def connectionLost(self, reason): class ProtocolPoppingFactory(Factory): """ A factory which only creates a fixed list of protocols. + + For example if in a test you want to ensure that a test server only handles + a single connection, you'd supply a list of one ``Protocol`` + instance. Subsequent requests will result in an ``IndexError``. """ def __init__(self, protocols): + """ + :param list protocols: A list of ``Protocol`` instances which will be + used for successive connections. + """ self.protocols = protocols def buildProtocol(self, addr): From ecacef8c5632028ca65eed2892074822f76c521c Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 12:45:50 +0100 Subject: [PATCH 30/34] A note about localhost links --- flocker/node/gear.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/flocker/node/gear.py b/flocker/node/gear.py index 4ed1830681..632bf7f019 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -159,6 +159,16 @@ def _ensure_ok(self, response): return d def add(self, unit_name, image_name, ports=None, links=None): + """ + See ``IGearClient.add`` for base documentation. + + Gear `NetworkLinks` are currently fixed to destination localhost. This + allows us to control the actual target of the link using proxy / nat + rules on the host machine without having to restart the gear unit. + + XXX: If gear allowed us to reconfigure links this wouldn't be + necessary. See https://github.com/openshift/geard/issues/223 + """ if ports is None: ports = [] From 808920d1321a8f02b925d3986918c078f2383f5a Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 12:56:13 +0100 Subject: [PATCH 31/34] A note about the behaviour of geard loopback link forwarding behaviour. --- flocker/node/gear.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/flocker/node/gear.py b/flocker/node/gear.py index 632bf7f019..d08ecb9439 100644 --- a/flocker/node/gear.py +++ b/flocker/node/gear.py @@ -168,6 +168,12 @@ def add(self, unit_name, image_name, ports=None, links=None): XXX: If gear allowed us to reconfigure links this wouldn't be necessary. See https://github.com/openshift/geard/issues/223 + + XXX: As long as we need to set the target as 127.0.0.1 its also worth + noting that gear will actually route the traffic to a non-loopback + address on the host. So if your service or NAT rule on the host is + configured for 127.0.0.1 only, it won't receive any traffic. See + https://github.com/openshift/geard/issues/224 """ if ports is None: ports = [] From dd5f3d6076b579faa9ded01c97600a8534945d9b Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 13:04:01 +0100 Subject: [PATCH 32/34] A note about ugly (but useful) debug output --- flocker/testtools.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/flocker/testtools.py b/flocker/testtools.py index 8cb8922f07..301e8c03fa 100644 --- a/flocker/testtools.py +++ b/flocker/testtools.py @@ -615,6 +615,10 @@ def build(self, dockerfile_variables=None): if template_file.exists() and not docker_file.exists(): self._process_template( template_file, docker_file, dockerfile_variables) + # XXX: This dumps lots of debug output to stderr which messes up the + # test results output. It's useful debug info incase of a test failure + # so it would be better to send it to the test.log file. See + # https://github.com/ClusterHQ/flocker/issues/171 command = [ b'docker', b'build', # Always clean up intermediate containers in case of failures. From ef7d3278f4cea1d72a72bd162afa866e4f078d2d Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 13:22:29 +0100 Subject: [PATCH 33/34] Remove unused DockerImage.remove method --- flocker/testtools.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/flocker/testtools.py b/flocker/testtools.py index 301e8c03fa..e3b90732a6 100644 --- a/flocker/testtools.py +++ b/flocker/testtools.py @@ -628,17 +628,6 @@ def build(self, dockerfile_variables=None): ] check_call(command) - def remove(self): - """ - Forcefully remove the image from the local Docker repository. - """ - command = [ - b'docker', b'rmi', - b'--force', - self.tag - ] - check_call(command) - def skip_on_broken_permissions(test_method): """ From 31cf086482a29dda3de56fcb23afc5e66ed1c6d2 Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 2 Jul 2014 13:24:46 +0100 Subject: [PATCH 34/34] flake8 cleanup --- flocker/testtools.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/flocker/testtools.py b/flocker/testtools.py index e3b90732a6..81f5efd308 100644 --- a/flocker/testtools.py +++ b/flocker/testtools.py @@ -586,9 +586,10 @@ def _process_template(self, template_file, target_file, replacements): write the result to `target_file`. :param FilePath template_file: The file containing the placeholders. - :param FilePath target_file: The file to which the result will be written. - :param dict replacements: A dictionary of variable names and replacement - values. + :param FilePath target_file: The file to which the result will be + written. + :param dict replacements: A dictionary of variable names and + replacement values. """ with template_file.open() as f: template = f.read().decode('utf8') @@ -603,7 +604,7 @@ def build(self, dockerfile_variables=None): exists. """ if dockerfile_variables is None: - dockerfile_variables = {} + dockerfile_variables = {} if not self.working_dir.exists(): self.working_dir.makedirs()