merged

master/buildbot/buildslave/docker.py

@@ -16,9 +16,10 @@
 # Needed so that this module name don't clash with docker-py on older python.
 from __future__ import absolute_import
 
-from io import BytesIO
 import json
 
+from io import BytesIO
+
 from twisted.internet import defer
 from twisted.internet import threads
 from twisted.python import log
@@ -33,6 +34,7 @@
 except ImportError:
     client = None
 
+
 def handle_stream_line(line):
     """\
     Input is the json representation of: {'stream': "Content\ncontent"}
@@ -87,13 +89,16 @@ def __init__(self, name, password, docker_host, image=None, command=None,
                                           missing_timeout, build_wait_timeout,
                                           properties, locks)
 
-        self.docker_host = docker_host
         self.image = image
         self.command = command or []
-
         self.dockerfile = dockerfile
-        self.version = version
-        self.tls = tls
+
+        # Prepare the parameters for the Docker Client object.
+        self.client_args = {'base_url': docker_host}
+        if version is not None:
+            self.client_args['version'] = version
+        if tls is not None:
+            self.client_args['tls'] = tls
 
     def start_instance(self, build):
         if self.instance is not None:
@@ -103,7 +108,7 @@ def start_instance(self, build):
     def _image_exists(self, client, name=None):
         if name is None:
             name = self.image
-        # Make sure the container exists
+        # Make sure the image exists
         for image in client.images():
             for tag in image['RepoTags']:
                 if ':' in name and tag == name:
@@ -112,16 +117,8 @@ def _image_exists(self, client, name=None):
                     return True
         return False
 
-    def _get_client_params(self):
-        kwargs = {'base_url': self.docker_host}
-        if self.version is not None:
-            kwargs['version'] = self.version
-        if self.tls is not None:
-            kwargs['tls'] = self.tls
-        return kwargs
-
     def _thd_start_instance(self):
-        docker_client = client.Client(**self._get_client_params())
+        docker_client = client.Client(**self.client_args)
 
         found = False
         if self.image is not None:
@@ -174,7 +171,7 @@ def stop_instance(self, fast=False):
         return threads.deferToThread(self._thd_stop_instance, instance, fast)
 
     def _thd_stop_instance(self, instance, fast):
-        docker_client = client.Client(**self._get_client_params())
+        docker_client = client.Client(**self.client_args)
         log.msg('Stopping container %s...' % instance['Id'][:6])
         docker_client.stop(instance['Id'])
         if not fast:

master/buildbot/test/unit/test_buildslave_docker.py

@@ -48,26 +48,25 @@ def test_constructor_image_nodockerfile(self):
         self.assertEqual(bs.image, 'myslave')
 
     def test_constructor_minimal(self):
-        bs = self.ConcreteBuildSlave('bot', 'pass', 'tcp://1234:2375', 'slave', ['bin/bash'])
+        # Minimal set of parameters
+        bs = self.ConcreteBuildSlave('bot', 'pass', 'tcp://1234:2375', 'slave')
         self.assertEqual(bs.slavename, 'bot')
         self.assertEqual(bs.password, 'pass')
-        self.assertEqual(bs.docker_host, 'tcp://1234:2375')
+        self.assertEqual(bs.client_args, {'base_url': 'tcp://1234:2375'})
         self.assertEqual(bs.image, 'slave')
-        self.assertEqual(bs.command, ['bin/bash'])
+        self.assertEqual(bs.command, [])
 
     def test_constructor_all_docker_parameters(self):
         # Volumes have their own tests
-        bs = self.ConcreteBuildSlave('bot', 'pass', 'unix:///var/run/docker.sock', 'slave_img', ['/bin/sh'], dockerfile="FROM ubuntu", version='1.9')
+        bs = self.ConcreteBuildSlave('bot', 'pass', 'unix:///var/run/docker.sock', 'slave_img', ['/bin/sh'], dockerfile="FROM ubuntu", version='1.9', tls=True)
         self.assertEqual(bs.slavename, 'bot')
         self.assertEqual(bs.password, 'pass')
-        self.assertEqual(bs.docker_host, 'unix:///var/run/docker.sock')
         self.assertEqual(bs.image, 'slave_img')
         self.assertEqual(bs.command, ['/bin/sh'])
         self.assertEqual(bs.dockerfile, "FROM ubuntu")
-        self.assertEqual(bs.version, '1.9')
         self.assertEqual(bs.volumes, [])
         self.assertEqual(bs.binds, {})
-        self.assertEqual(bs.tls, None)
+        self.assertEqual(bs.client_args, {'base_url': 'unix:///var/run/docker.sock', 'version': '1.9', 'tls': True})
 
     def test_rw_volume(self):
         bs = self.ConcreteBuildSlave('bot', 'pass', 'tcp://1234:2375', 'slave', ['bin/bash'], volumes=['/src/webapp:/opt/webapp'])
@@ -121,13 +120,13 @@ def test_start_instance_noimage_gooddockerfile(self):
         id, name = yield bs.start_instance(None)
         self.assertEqual(name, 'slave')
 
+
 class testDockerPyStreamLogs(unittest.TestCase):
 
     def compare(self, result, log):
         self.assertEquals(result,
                           list(dockerbuildslave.handle_stream_line(log)))
 
-
     def testEmpty(self):
         self.compare([], '{"stream":"\\n"}\r\n')
 

master/docs/manual/cfg-buildslaves-docker.rst

@@ -170,7 +170,7 @@ In addition to the arguments available for any :ref:`Latent-Buildslaves`, :class
     .. note:: In case ``image`` and ``dockerfile`` are given, no attempt is made to compare the image with the content of the Dockerfile parameter if the image is found.
 
 ``version``
-    (optional, default to the higher version known by docker-py)
+    (optional, default to the highest version known by docker-py)
     This will indicates wich API version must be used to communicate with Docker.
 
 ``tls``