From bec0c8c4a06ffc33ce7914a5f017b981d086435c Mon Sep 17 00:00:00 2001 From: Michael Kerrin Date: Tue, 23 Jul 2013 09:49:57 +0000 Subject: [PATCH] Create volume from snapshot must be in the same AZ as snapshot This issue and patch also apply to cloning volumes. When creating a volume from a snapshot we need to pick the availability zone of the snapshot's source volume. This patch goes further and enforces that the new volume must be in the same AZ as the snapshot. It raises an user error if the user tries to create a volume in a different AZ to the snapshot. This is enforced across all drivers because creating a volume from a snapshot is implemented in the drivers and not all drivers are guaranteed to support creating a volume from snapshot is a foreign AZ. More to point if you don't support create a volume like this, and we allow this then you can create volumes and instances that get stuck in some weird states that require a support call to fix. If you do support cross AZ functionality then you can override the enforcement of that cloned volumes most be in the same AZ as their source via the 'cloned_volume_same_az' option. Change-Id: Iafc8f35ecc6a6b51dbe6df8bf44eaa3e79c3bd01 Fixes: bug #1202648 --- cinder/tests/test_volume.py | 74 ++++++++++++++++++++++++++++++++++++- cinder/volume/api.py | 33 ++++++++++++++--- 2 files changed, 100 insertions(+), 7 deletions(-) diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index f616fffd7..4d2c6bcb3 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -89,7 +89,8 @@ class VolumeTestCase(test.TestCase): @staticmethod def _create_volume(size=0, snapshot_id=None, image_id=None, - source_volid=None, metadata=None, status="creating"): + source_volid=None, metadata=None, status="creating", + availability_zone=None): """Create a volume object.""" vol = {} vol['size'] = size @@ -98,7 +99,8 @@ class VolumeTestCase(test.TestCase): vol['source_volid'] = source_volid vol['user_id'] = 'fake' vol['project_id'] = 'fake' - vol['availability_zone'] = CONF.storage_availability_zone + vol['availability_zone'] = \ + availability_zone or CONF.storage_availability_zone vol['status'] = status vol['attach_status'] = "detached" vol['host'] = CONF.host @@ -353,6 +355,41 @@ class VolumeTestCase(test.TestCase): description='fake_desc', snapshot=snapshot) + def test_create_volume_from_snapshot_fail_wrong_az(self): + """Test volume can't be created from snapshot in a different az.""" + volume_api = cinder.volume.api.API() + + def fake_list_availability_zones(): + return ({'name': 'nova', 'available': True}, + {'name': 'az2', 'available': True}) + + self.stubs.Set(volume_api, + 'list_availability_zones', + fake_list_availability_zones) + + volume_src = self._create_volume(availability_zone='az2') + self.volume.create_volume(self.context, volume_src['id']) + snapshot = self._create_snapshot(volume_src['id']) + self.volume.create_snapshot(self.context, volume_src['id'], + snapshot['id']) + snapshot = db.snapshot_get(self.context, snapshot['id']) + + volume_dst = volume_api.create(self.context, + size=1, + name='fake_name', + description='fake_desc', + snapshot=snapshot) + self.assertEqual(volume_dst['availability_zone'], 'az2') + + self.assertRaises(exception.InvalidInput, + volume_api.create, + self.context, + size=1, + name='fake_name', + description='fake_desc', + snapshot=snapshot, + availability_zone='nova') + def test_create_volume_with_invalid_exclusive_options(self): """Test volume create with multiple exclusive options fails.""" volume_api = cinder.volume.api.API() @@ -1394,6 +1431,39 @@ class VolumeTestCase(test.TestCase): self.volume.delete_volume(self.context, volume_dst['id']) self.volume.delete_volume(self.context, volume_src['id']) + def test_create_volume_from_sourcevol_fail_wrong_az(self): + """Test volume can't be cloned from an other volume in different az.""" + volume_api = cinder.volume.api.API() + + def fake_list_availability_zones(): + return ({'name': 'nova', 'available': True}, + {'name': 'az2', 'available': True}) + + self.stubs.Set(volume_api, + 'list_availability_zones', + fake_list_availability_zones) + + volume_src = self._create_volume(availability_zone='az2') + self.volume.create_volume(self.context, volume_src['id']) + + volume_src = db.volume_get(self.context, volume_src['id']) + + volume_dst = volume_api.create(self.context, + size=1, + name='fake_name', + description='fake_desc', + source_volume=volume_src) + self.assertEqual(volume_dst['availability_zone'], 'az2') + + self.assertRaises(exception.InvalidInput, + volume_api.create, + self.context, + size=1, + name='fake_name', + description='fake_desc', + source_volume=volume_src, + availability_zone='nova') + def test_create_volume_from_sourcevol_with_glance_metadata(self): """Test glance metadata can be correctly copied to new volume.""" def fake_create_cloned_volume(volume, src_vref): diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 43d4a99e1..2fe3290ff 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -45,9 +45,14 @@ volume_host_opt = cfg.BoolOpt('snapshot_same_host', default=True, help='Create volume from snapshot at the host ' 'where snapshot resides') +volume_same_az_opt = cfg.BoolOpt('cloned_volume_same_az', + default=True, + help='Ensure that the new volumes are the ' + 'same AZ as snapshot or source volume') CONF = cfg.CONF CONF.register_opt(volume_host_opt) +CONF.register_opt(volume_same_az_opt) CONF.import_opt('storage_availability_zone', 'cinder.volume.manager') LOG = logging.getLogger(__name__) @@ -160,6 +165,29 @@ class API(base.Base): msg = _('Image minDisk size is larger than the volume size.') raise exception.InvalidInput(reason=msg) + if availability_zone is None: + if snapshot is not None: + availability_zone = snapshot['volume']['availability_zone'] + elif source_volume is not None: + availability_zone = source_volume['availability_zone'] + else: + availability_zone = CONF.storage_availability_zone + else: + self._check_availabilty_zone(availability_zone) + + if CONF.cloned_volume_same_az: + if (snapshot and + snapshot['volume']['availability_zone'] != + availability_zone): + msg = _("Volume must be in the same " + "availability zone as the snapshot") + raise exception.InvalidInput(reason=msg) + elif source_volume and \ + source_volume['availability_zone'] != availability_zone: + msg = _("Volume must be in the same " + "availability zone as the source volume") + raise exception.InvalidInput(reason=msg) + if not volume_type and not source_volume: volume_type = volume_types.get_default_volume_type() @@ -198,11 +226,6 @@ class API(base.Base): 'd_consumed': _consumed(over)}) raise exception.VolumeLimitExceeded(allowed=quotas[over]) - if availability_zone is None: - availability_zone = CONF.storage_availability_zone - else: - self._check_availabilty_zone(availability_zone) - self._check_metadata_properties(context, metadata) options = {'size': size, 'user_id': context.user_id, -- 2.45.2