From 05935b5e5448436f210cb5488a65338aae6e722f Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Mon, 4 Jan 2016 15:10:32 -0800 Subject: [PATCH 1/6] Don't recreate pre-existing volumes. During the initialize_volumes phase, if a volume using the non-namespaced name already exists, don't create the namespaced equivalent. Signed-off-by: Joffrey F --- compose/project.py | 6 ++++++ compose/volume.py | 11 +++++++++++ tests/integration/project_test.py | 26 ++++++++++++++++++++++++-- tests/integration/volume_test.py | 10 ++++++++++ 4 files changed, 51 insertions(+), 2 deletions(-) diff --git a/compose/project.py b/compose/project.py index fa3eace20..f5bb8bebf 100644 --- a/compose/project.py +++ b/compose/project.py @@ -235,6 +235,12 @@ class Project(object): def initialize_volumes(self): try: for volume in self.volumes: + if volume.is_user_created: + log.info( + 'Found user-created volume "{0}". No new namespaced ' + 'volume will be created.'.format(volume.name) + ) + continue volume.create() except NotFound: raise ConfigurationError( diff --git a/compose/volume.py b/compose/volume.py index fb8bd5809..9bd98fa5b 100644 --- a/compose/volume.py +++ b/compose/volume.py @@ -1,6 +1,8 @@ from __future__ import absolute_import from __future__ import unicode_literals +from docker.errors import NotFound + class Volume(object): def __init__(self, client, project, name, driver=None, driver_opts=None): @@ -21,6 +23,15 @@ class Volume(object): def inspect(self): return self.client.inspect_volume(self.full_name) + @property + def is_user_created(self): + try: + self.client.inspect_volume(self.name) + except NotFound: + return False + + return True + @property def full_name(self): return '{0}_{1}'.format(self.project, self.name) diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index a3e0f33a0..d1b1fdf07 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -4,6 +4,7 @@ from __future__ import unicode_literals import random import py +from docker.errors import NotFound from .testcases import DockerClientTestCase from compose.config import config @@ -624,7 +625,7 @@ class ProjectTest(DockerClientTestCase): self.assertEqual(volume_data['Name'], full_vol_name) self.assertEqual(volume_data['Driver'], 'local') - def test_project_up_invalid_volume_driver(self): + def test_initialize_volumes_invalid_volume_driver(self): vol_name = '{0:x}'.format(random.getrandbits(32)) config_data = config.Config( @@ -642,7 +643,7 @@ class ProjectTest(DockerClientTestCase): with self.assertRaises(config.ConfigurationError): project.initialize_volumes() - def test_project_up_updated_driver(self): + def test_initialize_volumes_updated_driver(self): vol_name = '{0:x}'.format(random.getrandbits(32)) full_vol_name = 'composetest_{0}'.format(vol_name) @@ -675,3 +676,24 @@ class ProjectTest(DockerClientTestCase): assert 'Configuration for volume {0} specifies driver smb'.format( vol_name ) in str(e.exception) + + def test_initialize_volumes_user_created_volumes(self): + # Use composetest_ prefix so it gets garbage-collected in tearDown() + vol_name = 'composetest_{0:x}'.format(random.getrandbits(32)) + full_vol_name = 'composetest_{0}'.format(vol_name) + self.client.create_volume(vol_name) + config_data = config.Config( + version=2, services=[{ + 'name': 'web', + 'image': 'busybox:latest', + 'command': 'top' + }], volumes={vol_name: {'driver': 'local'}} + ) + project = Project.from_config( + name='composetest', + config_data=config_data, client=self.client + ) + project.initialize_volumes() + + with self.assertRaises(NotFound): + self.client.inspect_volume(full_vol_name) diff --git a/tests/integration/volume_test.py b/tests/integration/volume_test.py index 8ae35378a..8bcce0e14 100644 --- a/tests/integration/volume_test.py +++ b/tests/integration/volume_test.py @@ -54,3 +54,13 @@ class VolumeTest(DockerClientTestCase): vol.remove() volumes = self.client.volumes()['Volumes'] assert len([v for v in volumes if v['Name'] == vol.full_name]) == 0 + + def test_is_user_created(self): + vol = Volume(self.client, 'composetest', 'uservolume01') + try: + self.client.create_volume('uservolume01') + assert vol.is_user_created is True + finally: + self.client.remove_volume('uservolume01') + vol2 = Volume(self.client, 'composetest', 'volume01') + assert vol2.is_user_created is False From 9cb58b796e2ea70f37ea857909fa4a3044d8861b Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Tue, 12 Jan 2016 16:53:49 -0800 Subject: [PATCH 2/6] Implement ability to specify external volumes External volumes are created and managed by the user. They are not namespaced. They are expected to exist at the beginning of the up phase. Signed-off-by: Joffrey F --- compose/config/fields_schema_v2.json | 7 +++++++ compose/project.py | 14 ++++++++++--- compose/volume.py | 21 ++++++++++++++----- tests/integration/project_test.py | 24 ++++++++++++++++++++-- tests/integration/volume_test.py | 30 ++++++++++++++++++---------- tests/unit/config/config_test.py | 18 +++++++++++++++++ 6 files changed, 93 insertions(+), 21 deletions(-) diff --git a/compose/config/fields_schema_v2.json b/compose/config/fields_schema_v2.json index 22ff839fb..61bd7628b 100644 --- a/compose/config/fields_schema_v2.json +++ b/compose/config/fields_schema_v2.json @@ -41,6 +41,13 @@ "^.+$": {"type": ["string", "number"]} }, "additionalProperties": false + }, + "external": { + "type": ["boolean", "object"], + "properties": { + "name": {"type": "string"} + }, + "additionalProperties": false } } } diff --git a/compose/project.py b/compose/project.py index f5bb8bebf..49b54e10f 100644 --- a/compose/project.py +++ b/compose/project.py @@ -77,7 +77,9 @@ class Project(object): project.volumes.append( Volume( client=client, project=name, name=vol_name, - driver=data.get('driver'), driver_opts=data.get('driver_opts') + driver=data.get('driver'), + driver_opts=data.get('driver_opts'), + external=data.get('external', False) ) ) return project @@ -235,11 +237,17 @@ class Project(object): def initialize_volumes(self): try: for volume in self.volumes: - if volume.is_user_created: + if volume.external: log.info( - 'Found user-created volume "{0}". No new namespaced ' + 'Volume {0} declared as external. No new ' 'volume will be created.'.format(volume.name) ) + if not volume.exists(): + raise ConfigurationError( + 'Volume {0} declared as external, but could not be' + ' found. Please create the volume manually and try' + ' again.'.format(volume.full_name) + ) continue volume.create() except NotFound: diff --git a/compose/volume.py b/compose/volume.py index 9bd98fa5b..64671ca9b 100644 --- a/compose/volume.py +++ b/compose/volume.py @@ -5,12 +5,19 @@ from docker.errors import NotFound class Volume(object): - def __init__(self, client, project, name, driver=None, driver_opts=None): + def __init__(self, client, project, name, driver=None, driver_opts=None, + external=False): self.client = client self.project = project self.name = name self.driver = driver self.driver_opts = driver_opts + self.external_name = None + if external: + if isinstance(external, dict): + self.external_name = external.get('name') + else: + self.external_name = self.name def create(self): return self.client.create_volume( @@ -23,15 +30,19 @@ class Volume(object): def inspect(self): return self.client.inspect_volume(self.full_name) - @property - def is_user_created(self): + def exists(self): try: - self.client.inspect_volume(self.name) + self.inspect() except NotFound: return False - return True + @property + def external(self): + return bool(self.external_name) + @property def full_name(self): + if self.external_name: + return self.external_name return '{0}_{1}'.format(self.project, self.name) diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index d1b1fdf07..36b736b42 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -677,7 +677,7 @@ class ProjectTest(DockerClientTestCase): vol_name ) in str(e.exception) - def test_initialize_volumes_user_created_volumes(self): + def test_initialize_volumes_external_volumes(self): # Use composetest_ prefix so it gets garbage-collected in tearDown() vol_name = 'composetest_{0:x}'.format(random.getrandbits(32)) full_vol_name = 'composetest_{0}'.format(vol_name) @@ -687,7 +687,7 @@ class ProjectTest(DockerClientTestCase): 'name': 'web', 'image': 'busybox:latest', 'command': 'top' - }], volumes={vol_name: {'driver': 'local'}} + }], volumes={vol_name: {'external': True}} ) project = Project.from_config( name='composetest', @@ -697,3 +697,23 @@ class ProjectTest(DockerClientTestCase): with self.assertRaises(NotFound): self.client.inspect_volume(full_vol_name) + + def test_initialize_volumes_inexistent_external_volume(self): + vol_name = '{0:x}'.format(random.getrandbits(32)) + + config_data = config.Config( + version=2, services=[{ + 'name': 'web', + 'image': 'busybox:latest', + 'command': 'top' + }], volumes={vol_name: {'external': True}} + ) + project = Project.from_config( + name='composetest', + config_data=config_data, client=self.client + ) + with self.assertRaises(config.ConfigurationError) as e: + project.initialize_volumes() + assert 'Volume {0} declared as external'.format( + vol_name + ) in str(e.exception) diff --git a/tests/integration/volume_test.py b/tests/integration/volume_test.py index 8bcce0e14..fbb4aaa27 100644 --- a/tests/integration/volume_test.py +++ b/tests/integration/volume_test.py @@ -18,9 +18,10 @@ class VolumeTest(DockerClientTestCase): except DockerException: pass - def create_volume(self, name, driver=None, opts=None): + def create_volume(self, name, driver=None, opts=None, external=False): vol = Volume( - self.client, 'composetest', name, driver=driver, driver_opts=opts + self.client, 'composetest', name, driver=driver, driver_opts=opts, + external=external ) self.tmp_volumes.append(vol) return vol @@ -55,12 +56,19 @@ class VolumeTest(DockerClientTestCase): volumes = self.client.volumes()['Volumes'] assert len([v for v in volumes if v['Name'] == vol.full_name]) == 0 - def test_is_user_created(self): - vol = Volume(self.client, 'composetest', 'uservolume01') - try: - self.client.create_volume('uservolume01') - assert vol.is_user_created is True - finally: - self.client.remove_volume('uservolume01') - vol2 = Volume(self.client, 'composetest', 'volume01') - assert vol2.is_user_created is False + def test_external_volume(self): + vol = self.create_volume('volume01', external=True) + assert vol.external is True + assert vol.full_name == vol.name + vol.create() + info = vol.inspect() + assert info['Name'] == vol.name + + def test_external_aliased_volume(self): + alias_name = 'alias01' + vol = self.create_volume('volume01', external={'name': alias_name}) + assert vol.external is True + assert vol.full_name == alias_name + vol.create() + info = vol.inspect() + assert info['Name'] == alias_name diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 25483e526..679125bc9 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -775,6 +775,24 @@ class ConfigTest(unittest.TestCase): 'extends': {'service': 'foo'} } + def test_external_volume_config(self): + config_details = build_config_details({ + 'version': 2, + 'services': { + 'bogus': {'image': 'busybox'} + }, + 'volumes': { + 'ext': {'external': True}, + 'ext2': {'external': {'name': 'aliased'}} + } + }) + config_result = config.load(config_details) + volumes = config_result.volumes + assert 'ext' in volumes + assert volumes['ext']['external'] is True + assert 'ext2' in volumes + assert volumes['ext2']['external']['name'] == 'aliased' + class PortsTest(unittest.TestCase): INVALID_PORTS_TYPES = [ From f774422d18c32f3d9233e7c697ccf65e30a3fc06 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Tue, 12 Jan 2016 16:58:24 -0800 Subject: [PATCH 3/6] Test Volume.exists() behavior Signed-off-by: Joffrey F --- tests/integration/volume_test.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/integration/volume_test.py b/tests/integration/volume_test.py index fbb4aaa27..2e65f0be3 100644 --- a/tests/integration/volume_test.py +++ b/tests/integration/volume_test.py @@ -57,7 +57,7 @@ class VolumeTest(DockerClientTestCase): assert len([v for v in volumes if v['Name'] == vol.full_name]) == 0 def test_external_volume(self): - vol = self.create_volume('volume01', external=True) + vol = self.create_volume('composetest_volume_ext', external=True) assert vol.external is True assert vol.full_name == vol.name vol.create() @@ -65,10 +65,28 @@ class VolumeTest(DockerClientTestCase): assert info['Name'] == vol.name def test_external_aliased_volume(self): - alias_name = 'alias01' + alias_name = 'composetest_alias01' vol = self.create_volume('volume01', external={'name': alias_name}) assert vol.external is True assert vol.full_name == alias_name vol.create() info = vol.inspect() assert info['Name'] == alias_name + + def test_exists(self): + vol = self.create_volume('volume01') + assert vol.exists() is False + vol.create() + assert vol.exists() is True + + def test_exists_external(self): + vol = self.create_volume('volume01', external=True) + assert vol.exists() is False + vol.create() + assert vol.exists() is True + + def test_exists_external_aliased(self): + vol = self.create_volume('volume01', external={'name': 'composetest_alias01'}) + assert vol.exists() is False + vol.create() + assert vol.exists() is True From 8616b2de515f64d0d2541bcc61f8abcac15ac070 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 13 Jan 2016 11:22:57 -0800 Subject: [PATCH 4/6] Update error message when external volume is missing Signed-off-by: Joffrey F --- compose/project.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/compose/project.py b/compose/project.py index 49b54e10f..08843a6ee 100644 --- a/compose/project.py +++ b/compose/project.py @@ -238,15 +238,18 @@ class Project(object): try: for volume in self.volumes: if volume.external: - log.info( + log.debug( 'Volume {0} declared as external. No new ' 'volume will be created.'.format(volume.name) ) if not volume.exists(): raise ConfigurationError( - 'Volume {0} declared as external, but could not be' - ' found. Please create the volume manually and try' - ' again.'.format(volume.full_name) + 'Volume {name} declared as external, but could' + ' not be found. Please create the volume manually' + ' using `{command}{name}` and try again.'.format( + name=volume.full_name, + command='docker volume create --name=' + ) ) continue volume.create() From d601199eb5d31da117179c113d77f9d070b3507b Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 13 Jan 2016 12:07:08 -0800 Subject: [PATCH 5/6] Normalize external_name Signed-off-by: Joffrey F --- compose/config/config.py | 7 +++++++ compose/project.py | 2 +- compose/volume.py | 9 ++------- tests/integration/project_test.py | 8 ++++++-- tests/integration/volume_test.py | 10 ++++++---- 5 files changed, 22 insertions(+), 14 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 918946b39..887223189 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -273,6 +273,13 @@ def load_volumes(config_files): for config_file in config_files: for name, volume_config in config_file.config.get('volumes', {}).items(): volumes.update({name: volume_config}) + external = volume_config.get('external') + if external: + if isinstance(external, dict): + volume_config['external_name'] = external.get('name') + else: + volume_config['external_name'] = name + return volumes diff --git a/compose/project.py b/compose/project.py index 08843a6ee..e882713c2 100644 --- a/compose/project.py +++ b/compose/project.py @@ -79,7 +79,7 @@ class Project(object): client=client, project=name, name=vol_name, driver=data.get('driver'), driver_opts=data.get('driver_opts'), - external=data.get('external', False) + external_name=data.get('external_name') ) ) return project diff --git a/compose/volume.py b/compose/volume.py index 64671ca9b..b78aa029f 100644 --- a/compose/volume.py +++ b/compose/volume.py @@ -6,18 +6,13 @@ from docker.errors import NotFound class Volume(object): def __init__(self, client, project, name, driver=None, driver_opts=None, - external=False): + external_name=None): self.client = client self.project = project self.name = name self.driver = driver self.driver_opts = driver_opts - self.external_name = None - if external: - if isinstance(external, dict): - self.external_name = external.get('name') - else: - self.external_name = self.name + self.external_name = external_name def create(self): return self.client.create_volume( diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 36b736b42..467eb7861 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -687,7 +687,9 @@ class ProjectTest(DockerClientTestCase): 'name': 'web', 'image': 'busybox:latest', 'command': 'top' - }], volumes={vol_name: {'external': True}} + }], volumes={ + vol_name: {'external': True, 'external_name': vol_name} + } ) project = Project.from_config( name='composetest', @@ -706,7 +708,9 @@ class ProjectTest(DockerClientTestCase): 'name': 'web', 'image': 'busybox:latest', 'command': 'top' - }], volumes={vol_name: {'external': True}} + }], volumes={ + vol_name: {'external': True, 'external_name': vol_name} + } ) project = Project.from_config( name='composetest', diff --git a/tests/integration/volume_test.py b/tests/integration/volume_test.py index 2e65f0be3..706179ed2 100644 --- a/tests/integration/volume_test.py +++ b/tests/integration/volume_test.py @@ -18,10 +18,12 @@ class VolumeTest(DockerClientTestCase): except DockerException: pass - def create_volume(self, name, driver=None, opts=None, external=False): + def create_volume(self, name, driver=None, opts=None, external=None): + if external and isinstance(external, bool): + external = name vol = Volume( self.client, 'composetest', name, driver=driver, driver_opts=opts, - external=external + external_name=external ) self.tmp_volumes.append(vol) return vol @@ -66,7 +68,7 @@ class VolumeTest(DockerClientTestCase): def test_external_aliased_volume(self): alias_name = 'composetest_alias01' - vol = self.create_volume('volume01', external={'name': alias_name}) + vol = self.create_volume('volume01', external=alias_name) assert vol.external is True assert vol.full_name == alias_name vol.create() @@ -86,7 +88,7 @@ class VolumeTest(DockerClientTestCase): assert vol.exists() is True def test_exists_external_aliased(self): - vol = self.create_volume('volume01', external={'name': 'composetest_alias01'}) + vol = self.create_volume('volume01', external='composetest_alias01') assert vol.exists() is False vol.create() assert vol.exists() is True From e76b2679eb8f3117c7c0e0444e70bfc4d7fa502d Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 13 Jan 2016 13:52:59 -0800 Subject: [PATCH 6/6] external volume disallows other config keys Signed-off-by: Joffrey F --- compose/config/fields_schema_v2.json | 42 +++++++++++++++++----------- tests/unit/config/config_test.py | 13 +++++++++ 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/compose/config/fields_schema_v2.json b/compose/config/fields_schema_v2.json index 61bd7628b..310dbf961 100644 --- a/compose/config/fields_schema_v2.json +++ b/compose/config/fields_schema_v2.json @@ -32,24 +32,32 @@ "definitions": { "volume": { "id": "#/definitions/volume", - "type": "object", - "properties": { - "driver": {"type": "string"}, - "driver_opts": { - "type": "object", - "patternProperties": { - "^.+$": {"type": ["string", "number"]} - }, - "additionalProperties": false + "oneOf": [{ + "type": "object", + "properties": { + "driver": {"type": "string"}, + "driver_opts": { + "type": "object", + "patternProperties": { + "^.+$": {"type": ["string", "number"]} + }, + "additionalProperties": false + } }, - "external": { - "type": ["boolean", "object"], - "properties": { - "name": {"type": "string"} - }, - "additionalProperties": false - } - } + "additionalProperties": false + }, { + "type": "object", + "properties": { + "external": { + "type": ["boolean", "object"], + "properties": { + "name": {"type": "string"} + }, + "additionalProperties": false + } + }, + "additionalProperties": false + }] } }, "additionalProperties": false diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 679125bc9..b17598804 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -793,6 +793,19 @@ class ConfigTest(unittest.TestCase): assert 'ext2' in volumes assert volumes['ext2']['external']['name'] == 'aliased' + def test_external_volume_invalid_config(self): + config_details = build_config_details({ + 'version': 2, + 'services': { + 'bogus': {'image': 'busybox'} + }, + 'volumes': { + 'ext': {'external': True, 'driver': 'foo'} + } + }) + with self.assertRaises(ConfigurationError): + config.load(config_details) + class PortsTest(unittest.TestCase): INVALID_PORTS_TYPES = [