Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
worker: fix invalid DockerLatent volumes name
Volumes from DockerLatentWorker were named against the 'bind:volume'
string instead of the 'volume' part only.

According to docker-py documentation, `volumes` parameter from
create_container() must only contain volumes name (ie. '/mnt/vol1') and
not the complete definition (ie. '/home/user1:/mnt/vol1').

This lead to unwanted entry in the container filesystem (ie.
'/home/user1:/mnt/vol1/' folder was created inside the container).
  • Loading branch information
faudebert authored and tardyp committed Aug 23, 2016
1 parent 930a458 commit 62a7381
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
6 changes: 3 additions & 3 deletions master/buildbot/test/unit/test_worker_docker.py
Expand Up @@ -86,14 +86,14 @@ 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'])
self.assertEqual(bs.volumes, ['/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.volumes, ['/opt/webapp'])
self.assertEqual(
bs.binds, {'/src/webapp': {'bind': '/opt/webapp', 'ro': False}})

Expand All @@ -104,7 +104,7 @@ def test_volume_ro_rw(self):
'~:/backup:rw'])
yield bs.start_instance(self.build)
self.assertEqual(
bs.volumes, ['/src/webapp:/opt/webapp:ro', '~:/backup:rw'])
bs.volumes, ['/opt/webapp', '/backup'])
self.assertEqual(bs.binds, {'/src/webapp': {'bind': '/opt/webapp', 'ro': True},
'~': {'bind': '/backup', 'ro': False}})

Expand Down
15 changes: 8 additions & 7 deletions master/buildbot/worker/docker.py
Expand Up @@ -80,7 +80,7 @@ def __init__(self, name, password, docker_host, image=None, command=None,
if not isinstance(volume_string, str):
continue
try:
volume, bind = volume_string.split(":", 1)
bind, volume = volume_string.split(":", 1)
except ValueError:
config.error("Invalid volume definition for docker "
"%s. Skipping..." % volume_string)
Expand Down Expand Up @@ -110,18 +110,19 @@ def parse_volumes(self, volumes):
self.volumes = []
for volume_string in (volumes or []):
try:
volume, bind = volume_string.split(":", 1)
bind, volume = 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}
if volume.endswith(':ro') or volume.endswith(':rw'):
ro = volume[-2:] == 'ro'
volume = volume[:-3]

self.volumes.append(volume)
self.binds[bind] = {'bind': volume, 'ro': ro}

def createEnvironment(self):
result = {
Expand Down

0 comments on commit 62a7381

Please sign in to comment.