Make docker volumes a renderable property

* Make docker volumes a renderable property

Adds support for volumes as a renderable property,
cleanup volume code and unit tests a bit

* Add documentation for renderables

Update release notes, add example unit tests for renderables

master/buildbot/test/fake/docker.py

@@ -17,7 +17,7 @@
 class Client(object):
 
     def __init__(self, base_url):
-        self._images = [{'RepoTags': ['busybox:latest']}]
+        self._images = [{'RepoTags': ['busybox:latest', 'worker:latest']}]
 
     def images(self):
         return self._images

master/buildbot/test/unit/test_worker_docker.py

@@ -19,6 +19,7 @@
 from buildbot import interfaces
 from buildbot.process.properties import Properties
 from buildbot.process.properties import Property
+from buildbot.process.properties import Interpolate
 from buildbot.test.fake import docker
 from buildbot.worker import docker as dockerworker
 
@@ -29,7 +30,7 @@ class ConcreteWorker(dockerworker.DockerLatentWorker):
         pass
 
     def setUp(self):
-        self.build = Properties(image="busybox:latest")
+        self.build = Properties(image="busybox:latest", builder="docker_worker")
         self.patch(dockerworker, 'client', docker)
 
     def test_constructor_nodocker(self):
@@ -77,15 +78,26 @@ def test_constructor_all_docker_parameters(self):
         self.assertEqual(bs.client_args, {'base_url': 'unix:///var/run/docker.sock', 'version': '1.9', 'tls': True})
         self.assertEqual(bs.hostconfig, {'network_mode': 'fake', 'dns': ['1.1.1.1', '1.2.3.4']})
 
+    @defer.inlineCallbacks
+    def test_start_instance_volume_renderable(self):
+        bs = self.ConcreteWorker('bot', 'pass', 'tcp://1234:2375', 'worker', ['bin/bash'],
+                                 volumes=[Interpolate('/data:/buildslave/%(kw:builder)s/build', builder=Property('builder'))])
+        id, name = yield bs.start_instance(self.build)
+        self.assertEqual(bs.volumes, ['/data:/buildslave/docker_worker/build'])
+
+    @defer.inlineCallbacks
     def test_volume_no_suffix(self):
         bs = self.ConcreteWorker('bot', 'pass', 'tcp://1234:2375', 'worker', ['bin/bash'], volumes=['/src/webapp:/opt/webapp'])
+        yield bs.start_instance(self.build)
         self.assertEqual(bs.volumes, ['/src/webapp:/opt/webapp'])
         self.assertEqual(bs.binds, {'/src/webapp': {'bind': '/opt/webapp', 'ro': False}})
 
-    def test_ro_rw_volume(self):
+    @defer.inlineCallbacks
+    def test_volume_ro_rw(self):
         bs = self.ConcreteWorker('bot', 'pass', 'tcp://1234:2375', 'worker', ['bin/bash'],
                                  volumes=['/src/webapp:/opt/webapp:ro',
                                           '~:/backup:rw'])
+        yield bs.start_instance(self.build)
         self.assertEqual(bs.volumes, ['/src/webapp:/opt/webapp:ro', '~:/backup:rw'])
         self.assertEqual(bs.binds, {'/src/webapp': {'bind': '/opt/webapp', 'ro': True},
                                     '~': {'bind': '/backup', 'ro': False}})
@@ -94,6 +106,12 @@ def test_volume_bad_format(self):
         self.assertRaises(config.ConfigErrors, self.ConcreteWorker, 'bot', 'pass', 'http://localhost:2375', image="worker",
                           volumes=['abcd=efgh'])
 
+    @defer.inlineCallbacks
+    def test_volume_bad_format_renderable(self):
+        bs = self.ConcreteWorker('bot', 'pass', 'http://localhost:2375', image="worker",
+                                 volumes=[Interpolate('/data==/buildslave/%(kw:builder)s/build', builder=Property('builder'))])
+        yield self.assertRaises(AttributeError, bs.start_instance(self.build))
+
     @defer.inlineCallbacks
     def test_start_instance_image_no_version(self):
         bs = self.ConcreteWorker('bot', 'pass', 'tcp://1234:2375', 'busybox', ['bin/bash'])

master/buildbot/worker/docker.py

@@ -69,24 +69,24 @@ def __init__(self, name, password, docker_host, image=None, command=None,
             config.error("DockerLatentWorker: You need to specify at least"
                          " an image name, or a dockerfile")
 
-        self.volumes = []
+        self.volumes = volumes or []
         self.binds = {}
         self.networking_config = networking_config
         self.followStartupLogs = followStartupLogs
-        for volume_string in (volumes or []):
-            try:
-                volume, bind = volume_string.split(":", 1)
-            except ValueError:
-                config.error("Invalid volume definition for docker "
-                             "%s. Skipping..." % volume_string)
-                continue
-            self.volumes.append(volume_string)
 
-            ro = False
-            if bind.endswith(':ro') or bind.endswith(':rw'):
-                ro = bind[-2:] == 'ro'
-                bind = bind[:-3]
-            self.binds[volume] = {'bind': bind, 'ro': ro}
+        # Following block is only for checking config errors,
+        # actual parsing happens in self.parse_volumes()
+        # Renderables can be direct volumes definition or list member
+        if isinstance(volumes, list):
+            for volume_string in (volumes or []):
+                if not isinstance(volume_string, str):
+                    continue
+                try:
+                    volume, bind = volume_string.split(":", 1)
+                except ValueError:
+                    config.error("Invalid volume definition for docker "
+                                 "%s. Skipping..." % volume_string)
+                    continue
 
         # Set build_wait_timeout to 0 if not explicitely set: Starting a
         # container is almost immediate, we can affort doing so for each build.
@@ -108,6 +108,23 @@ def __init__(self, name, password, docker_host, image=None, command=None,
         if tls is not None:
             self.client_args['tls'] = tls
 
+    def parse_volumes(self, volumes):
+        self.volumes = []
+        for volume_string in (volumes or []):
+            try:
+                volume, bind = volume_string.split(":", 1)
+            except ValueError:
+                config.error("Invalid volume definition for docker "
+                             "%s. Skipping..." % volume_string)
+                continue
+            self.volumes.append(volume_string)
+
+            ro = False
+            if bind.endswith(':ro') or bind.endswith(':rw'):
+                ro = bind[-2:] == 'ro'
+                bind = bind[:-3]
+            self.binds[volume] = {'bind': bind, 'ro': ro}
+
     def createEnvironment(self):
         result = {
             "BUILDMASTER": self.masterFQDN,
@@ -123,7 +140,8 @@ def start_instance(self, build):
         if self.instance is not None:
             raise ValueError('instance active')
         image = yield build.render(self.image)
-        res = yield threads.deferToThread(self._thd_start_instance, image)
+        volumes = yield build.render(self.volumes)
+        res = yield threads.deferToThread(self._thd_start_instance, image, volumes)
         defer.returnValue(res)
 
     def _image_exists(self, client, name):
@@ -136,7 +154,7 @@ def _image_exists(self, client, name):
                     return True
         return False
 
-    def _thd_start_instance(self, image):
+    def _thd_start_instance(self, image, volumes):
         docker_client = client.Client(**self.client_args)
 
         found = False
@@ -158,6 +176,7 @@ def _thd_start_instance(self, image):
                 'Image "%s" not found on docker host.' % image
             )
 
+        self.parse_volumes(volumes)
         self.hostconfig['binds'] = self.binds
         host_conf = docker_client.create_host_config(**self.hostconfig)
 

master/docs/developer/tests.rst

@@ -52,6 +52,27 @@ Unit test modules are be named after the package or class they test, replacing
 trivial classes, can be tested in a single test module. For more complex
 situations, prefer to use multiple test modules.
 
+Unit tests using renderables require special handling. The following example
+shows how the same test would be written with the 'param' parameter and with the
+same parameter as a renderable.::
+
+    def test_param(self):
+        f = self.ConcreteClass(param='val')
+        self.assertEqual(f.param, 'val')
+
+When the parameter is renderable, you need to instantiate the Class before you
+can you renderables::
+
+    def setUp(self):
+        self.build = Properties(paramVal='val')
+
+    @defer.inlineCallbacks
+    def test_param_renderable(self):
+        f = self.ConcreteClass(param=Interpolate('%(kw:rendered_val)s',
+                               rendered_val=Property('paramVal'))
+        yield f.start_instance(self.build)
+        self.assertEqual(f.param, 'val')
+
 Interface Tests
 ~~~~~~~~~~~~~~~
 

master/docs/manual/cfg-properties.rst

@@ -277,6 +277,8 @@ For example::
 
 You can think of ``renderer`` as saying "call this function when the step starts".
 
+Note: Config errors with Renderables may not always be caught via checkconfig
+
 .. index:: single: Properties; Transform
 
 .. _Transform:

master/docs/relnotes/index.rst

@@ -24,6 +24,8 @@ Features
 * Add GitLab authentication plugin for web UI.
   See :class:`buildbot.www.oauth2.GitLabAuth`.
 * :class:`DockerLatentWorker` now has a ``hostconfig`` parameter that can be used to setup host configuration when creating a new container.
+* :class:`DockerLatentWorker` now has a ``networking_config`` parameter that can be used to setup container networks.
+* The :class:`DockerLatentWorker` ``volumes`` attribute is now renderable.
 * :bb:step:`CMake` build step is added.
   It provides a convenience interface to `CMake <https://cmake.org/cmake/help/latest/>`_ build system.
 * MySQL InnoDB tables are now supported.