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

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index f616fffd7383ebff37374ef6371f19f34977011e..4d2c6bcb3de2ba59cf770ea0bc38d60b319a5789 100644 (file)
--- 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 43d4a99e14f8cd759be1284d7da46375ce150a57..2fe3290ffd19aa131eb00789db0a87c51679106a 100644 (file)
--- 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,