Merge pull request #183 from ClusterHQ/gear-links-150

Add a links argument to `GearClient.add` (fixes #150)

flocker/node/functional/docker/Dockerfile.in

@@ -0,0 +1,4 @@
+FROM busybox
+MAINTAINER ClusterHQ <[email protected]>
+ADD . /
+CMD ["/bin/sh",  "-e", "run.sh", "{host}", "{port}", "{bytes}", "{timeout}"]

flocker/node/functional/docker/run.sh

@@ -0,0 +1,56 @@
+#!/bin/sh
+set -e
+help() {
+    cat <<EOF
+Usage: run.sh [options] HOST PORT BYTES TIMEOUT
+
+Send BYTES to HOST:PORT using a TCP connection.
+
+Retry until a connection can be established or until the TIMEOUT period is
+reached.
+
+This is the init script for the Docker container described in the neighbouring
+Dockerfile.
+
+Options:
+ --help: Print help.
+
+EOF
+}
+
+while [[ $# -gt 0 ]]; do
+    case "$1" in
+        -h | --help)
+            help
+            exit 0
+            ;;
+        --)
+            shift
+            break
+            ;;
+        -*)
+            help >&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

flocker/node/functional/test_gear.py

@@ -10,12 +10,18 @@
 
 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
+from twisted.internet import reactor
 
 from treq import request, content
 
-from ...testtools import loop_until, find_free_port
+from ...testtools import (
+    loop_until, find_free_port, make_capture_protocol,
+    ProtocolPoppingFactory, DockerImageBuilder)
+
 from ..test.test_gear import make_igearclient_tests, random_name
 from ..gear import GearClient, GearError, GEAR_PORT, PortMap
 
@@ -53,22 +59,31 @@ class GearClientTests(TestCase):
     def setUp(self):
         pass
 
-    def start_container(self, name, ports=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 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 with the ``GearClient`` when the unit
             is running.
         """
         client = GearClient("127.0.0.1")
-        d = client.add(name, u"openshift/busybox-http-app", ports=ports)
-        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(units):
             return [unit for unit in units if
-                    (unit.name == name and
+                    (unit.name == unit_name and
                      unit.activation_state == u"active")]
 
         def check_if_started():
@@ -218,7 +233,8 @@ 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_port=8080,
+                                 external_port=external_port)])
 
         d.addCallback(
             lambda ignored: self.request_until_response(external_port))
@@ -229,3 +245,55 @@ def started(response):
             return d
         d.addCallback(started)
         return d
+
+    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
+        expected_bytes = b'foo bar baz'
+        image_name = b'flocker/send_bytes_to'
+        # Create a Docker image
+        image = DockerImageBuilder(
+            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
+            )
+        )
+
+        # This is the target of the proxy which will be created.
+        server = TCP4ServerEndpoint(reactor, 0)
+        capture_finished, protocol = make_capture_protocol()
+
+        def check_data(data):
+            self.assertEqual(expected_bytes, data)
+        capture_finished.addCallback(check_data)
+
+        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):
+            return capture_finished
+        d.addCallback(started)
+
+        return d

flocker/node/gear.py

@@ -44,7 +44,7 @@ class Unit(object):
 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.
@@ -55,6 +55,9 @@ def add(unit_name, image_name, ports=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.
+
         :return: ``Deferred`` that fires on success, or errbacks with
             :class:`AlreadyExists` if a unit by that name already exists.
         """
@@ -133,6 +136,7 @@ def _request(self, method, path, data=None):
         url = self._base_url + path
         if data is not None:
             data = json.dumps(data)
+
         return request(method, url, data=data, persistent=False)
 
     def _ensure_ok(self, response):
@@ -154,14 +158,45 @@ 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):
+        """
+        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
+
+        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 = []
 
-        data = {u"Image": image_name, u"Started": True, u'Ports': []}
+        if links is None:
+            links = []
+
+        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})
+                {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_port,
+                 u'ToHost': u'127.0.0.1',
+                 u'ToPort': link.external_port}
+            )
 
         checked = self.exists(unit_name)
         checked.addCallback(
@@ -212,15 +247,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)
 
@@ -239,12 +277,12 @@ def list(self):
         return succeed(result)
 
 
-@attributes(['internal', 'external'])
+@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 int internal: The port number exposed by the container.
-    :ivar int external: The port number exposed by the host
+    :ivar int internal_port: The port number exposed by the container.
+    :ivar int external_port: The port number exposed by the host.
     """

flocker/node/test/test_gear.py

@@ -139,8 +139,8 @@ class PortMapInitTests(
         make_with_init_tests(
             record_type=PortMap,
             kwargs=dict(
-                internal=1234,
-                external=5678,
+                internal_port=5678,
+                external_port=910,
             )
         )
 ):
@@ -162,8 +162,8 @@ def test_repr(self):
         ``PortMap.__repr__`` shows the internal and external ports.
         """
         self.assertEqual(
-            "<PortMap(internal=1234, external=5678)>",
-            repr(PortMap(internal=1234, external=5678))
+            "<PortMap(internal_port=5678, external_port=910)>",
+            repr(PortMap(internal_port=5678, external_port=910))
         )
 
     def test_equal(self):
@@ -172,16 +172,16 @@ def test_equal(self):
         equal.
         """
         self.assertEqual(
-            PortMap(internal=1234, external=5678),
-            PortMap(internal=1234, external=5678)
+            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 and external ports
-        do not compare equal.
+        ``PortMap`` instances with the different internal and external ports do
+        not compare equal.
         """
         self.assertNotEqual(
-            PortMap(internal=5678, external=1234),
-            PortMap(internal=1234, external=5678)
+            PortMap(internal_port=5678, external_port=910),
+            PortMap(internal_port=1516, external_port=1718)
         )