Remove restriction on image and dockerfile, one of them is now enough

master/buildbot/buildslave/docker.py

@@ -56,11 +56,11 @@ def __init__(self, name, password, docker_host, image=None, command=None,
                  version=None, tls=None):
 
         if not client:
-            config.error("The python module 'docker-py' is needed "
-                         "to use a DockerLatentBuildSlave")
-        if not image:
-            config.error("DockerLatentBuildSlave: You need to specify an"
-                         " image name")
+            config.error("The python module 'docker-py' is needed to use a"
+                         " DockerLatentBuildSlave")
+        if not image and not dockerfile:
+            config.error("DockerLatentBuildSlave: You need to specify at least"
+                         " an image name, or a dockerfile")
 
         self.volumes = []
         self.binds = {}
@@ -96,13 +96,15 @@ def start_instance(self, build):
             raise ValueError('instance active')
         return threads.deferToThread(self._thd_start_instance)
 
-    def _image_exists(self, client):
+    def _image_exists(self, client, name=None):
+        if name is None:
+            name = self.image
         # Make sure the container exists
         for image in client.images():
             for tag in image['RepoTags']:
-                if ':' in self.image and tag == self.image:
+                if ':' in name and tag == name:
                     return True
-                if tag.startswith(self.image + ':'):
+                if tag.startswith(name + ':'):
                     return True
         return False
 
@@ -117,23 +119,28 @@ def _get_client_params(self):
     def _thd_start_instance(self):
         docker_client = client.Client(**self._get_client_params())
 
-        found = self._image_exists(docker_client)
+        found = False
+        if self.image is not None:
+            found = self._image_exists(docker_client)
+            image = self.image
+        else:
+            image = '%s_%s_image' % (self.slavename, id(self))
         if (not found) and (self.dockerfile is not None):
             log.msg("Image '%s' not found, building it from scratch" %
                     self.image)
             for line in docker_client.build(fileobj=BytesIO(self.dockerfile.encode('utf-8')),
-                                            tag=self.image):
+                                            tag=image):
                 for streamline in handle_stream_line(line):
                     log.msg(streamline)
 
-        if not self._image_exists(docker_client):
+        if (not self._image_exists(docker_client, image)):
             log.msg("Image '%s' not found" % self.image)
             raise interfaces.LatentBuildSlaveFailedToSubstantiate(
                 'Image "%s" not found on docker host.' % self.image
             )
 
         instance = docker_client.create_container(
-            self.image,
+            image,
             self.command,
             name='%s_%s' % (self.slavename, id(self)),
             volumes=self.volumes,
@@ -146,8 +153,10 @@ def _thd_start_instance(self):
             )
 
         log.msg('Container created, Id: %s...' % instance['Id'][:6])
+        instance['image'] = image
         self.instance = instance
         docker_client.start(instance['Id'], binds=self.binds)
+        log.msg('Container started')
         return [instance['Id'], self.image]
 
     def stop_instance(self, fast=False):
@@ -167,3 +176,5 @@ def _thd_stop_instance(self, instance, fast):
         if not fast:
             docker_client.wait(instance['Id'])
         docker_client.remove_container(instance['Id'], v=True, force=True)
+        if self.image is None:
+            docker_client.remove_image(image=instance['image'])

master/buildbot/test/unit/test_buildslave_docker.py

@@ -34,9 +34,19 @@ def test_constructor_nodocker(self):
         self.patch(dockerbuildslave, 'client', None)
         self.assertRaises(config.ConfigErrors, self.ConcreteBuildSlave, 'bot', 'pass', 'unix://tmp.sock', 'debian:wheezy', [])
 
-    def test_constructor_noimage(self):
+    def test_constructor_noimage_nodockerfile(self):
         self.assertRaises(config.ConfigErrors, self.ConcreteBuildSlave, 'bot', 'pass', 'http://localhost:2375')
 
+    def test_constructor_noimage_dockerfile(self):
+        bs = self.ConcreteBuildSlave('bot', 'pass', 'http://localhost:2375', dockerfile="FROM ubuntu")
+        self.assertEqual(bs.dockerfile, "FROM ubuntu")
+        self.assertEqual(bs.image, None)
+
+    def test_constructor_image_nodockerfile(self):
+        bs = self.ConcreteBuildSlave('bot', 'pass', 'http://localhost:2375', image="myslave")
+        self.assertEqual(bs.dockerfile, None)
+        self.assertEqual(bs.image, 'myslave')
+
     def test_constructor_minimal(self):
         bs = self.ConcreteBuildSlave('bot', 'pass', 'tcp://1234:2375', 'slave', ['bin/bash'])
         self.assertEqual(bs.slavename, 'bot')

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

@@ -151,7 +151,7 @@ In addition to the arguments available for any :ref:`Latent-Buildslaves`, :class
     This is the adress the master will use to connect with a running Docker instance.
 
 ``image``
-    (mandatory)
+    (optional if ``dockerfile`` is given)
     This is the name of the image that will be started by the build master. It should start a buildslave.
 
 ``command``
@@ -163,13 +163,11 @@ In addition to the arguments available for any :ref:`Latent-Buildslaves`, :class
     See `Setting up Volumes`_
 
 ``dockerfile``
-    (optional)
+    (optional if ``image`` is given)
     This is the content of the Dockerfile that will be used to build the specified image if the image is not found by Docker.
     It should be a multiline string.
 
-    .. note:: This parameter will be used only once as the next times the image will already be available.
-
-    .. note:: No attempt is made to compare the image with the content of the Dockerfile parameter if the image is found.
+    .. 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)