From cedfc1811e1e90f8c52b0d859451e8340fffa997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Allard?= Date: Wed, 19 Nov 2014 00:40:36 +0100 Subject: [PATCH] Remove restriction on image and dockerfile, one of them is now enough --- master/buildbot/buildslave/docker.py | 35 ++++++++++++------- .../test/unit/test_buildslave_docker.py | 12 ++++++- master/docs/manual/cfg-buildslaves-docker.rst | 8 ++--- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/master/buildbot/buildslave/docker.py b/master/buildbot/buildslave/docker.py index 18294e92f46..54e7862545a 100644 --- a/master/buildbot/buildslave/docker.py +++ b/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']) diff --git a/master/buildbot/test/unit/test_buildslave_docker.py b/master/buildbot/test/unit/test_buildslave_docker.py index d64e0635665..62025d35100 100644 --- a/master/buildbot/test/unit/test_buildslave_docker.py +++ b/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') diff --git a/master/docs/manual/cfg-buildslaves-docker.rst b/master/docs/manual/cfg-buildslaves-docker.rst index f008e04d8ff..d45f1076d8f 100644 --- a/master/docs/manual/cfg-buildslaves-docker.rst +++ b/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)