Clean BDM when snapshoting volume-backed instances

When snapshoting a volume-backed instance, its block device mapping is
copied entirely into the image properties of the resulting snapshot.

This may lead to some errors when booting from that snapshot because the
fields 'id', 'insance_uuid' and 'connection_info' must be generated for
each instance instead of reusing the ones from the original instance.
For example, sharing the connection_info will cause the new instance to
try to connect to the original volume instead of creating a new one.

This patch cleans all the database-specific plus the connection_info
fields from the block device mapping when creating the snapshot.

Change-Id: I4190e9519d641c61b00b5d86c3885f2ae4fa86f3
Partial-Bug: #1246327
(cherry picked from commit eddd8a1)

nova/compute/api.py

@@ -1927,13 +1927,19 @@ def snapshot_volume_backed(self, context, instance, image_meta, name,
             properties['root_device_name'] = instance['root_device_name']
         properties.update(extra_properties or {})
 
+        # TODO(xqueralt): Use new style BDM in volume snapshots
         bdms = self.get_instance_bdms(context, instance)
 
         mapping = []
         for bdm in bdms:
             if bdm['no_device']:
                 continue
 
+            # Clean the BDM of the database related fields to prevent
+            # duplicates in the future (e.g. the id was being preserved)
+            for field in block_device.BlockDeviceDict._db_only_fields:
+                bdm.pop(field, None)
+
             volume_id = bdm.get('volume_id')
             if volume_id:
                 # create snapshot based on volume_id
@@ -1945,7 +1951,11 @@ def snapshot_volume_backed(self, context, instance, image_meta, name,
                 snapshot = self.volume_api.create_snapshot_force(
                     context, volume['id'], name, volume['display_description'])
                 bdm['snapshot_id'] = snapshot['id']
-                bdm['volume_id'] = None
+
+                # Clean the extra volume related fields that will be generated
+                # when booting from the new snapshot.
+                bdm.pop('volume_id')
+                bdm.pop('connection_info')
 
             mapping.append(bdm)
 

nova/tests/compute/test_compute_api.py

@@ -13,10 +13,12 @@
 
 """Unit tests for compute API."""
 
+import copy
 import datetime
 import iso8601
 import mox
 
+from nova import block_device
 from nova.compute import api as compute_api
 from nova.compute import cells_api as compute_cells_api
 from nova.compute import flavors
@@ -37,6 +39,7 @@
 from nova import quota
 from nova import test
 from nova.tests.image import fake as fake_image
+from nova.tests import matchers
 from nova.tests.objects import test_migration
 from nova.tests.objects import test_service
 
@@ -1367,6 +1370,82 @@ def test_backup_with_base_image_ref(self):
         self._test_snapshot_and_backup(is_snapshot=False,
                                        with_base_ref=True)
 
+    def test_snapshot_volume_backed(self):
+        instance = self._create_instance_obj()
+        instance['root_device_name'] = 'vda'
+
+        instance_bdms = []
+
+        image_meta = {
+            'id': 'fake-image-id',
+            'properties': {'mappings': []},
+            'status': 'fake-status',
+            'location': 'far-away',
+        }
+
+        expect_meta = {
+            'name': 'test-snapshot',
+            'properties': {'root_device_name': 'vda', 'mappings': 'DONTCARE'},
+            'size': 0,
+        }
+
+        def fake_get_instance_bdms(context, instance):
+            return copy.deepcopy(instance_bdms)
+
+        def fake_image_create(context, image_meta, data):
+            self.assertThat(image_meta, matchers.DictMatches(expect_meta))
+
+        def fake_volume_get(context, volume_id):
+            return {'id': volume_id, 'display_description': ''}
+
+        def fake_volume_create_snapshot(context, volume_id, name, description):
+            return {'id': '%s-snapshot' % volume_id}
+
+        self.stubs.Set(self.compute_api, 'get_instance_bdms',
+                       fake_get_instance_bdms)
+        self.stubs.Set(self.compute_api.image_service, 'create',
+                       fake_image_create)
+        self.stubs.Set(self.compute_api.volume_api, 'get',
+                       fake_volume_get)
+        self.stubs.Set(self.compute_api.volume_api, 'create_snapshot_force',
+                       fake_volume_create_snapshot)
+
+        # No block devices defined
+        self.compute_api.snapshot_volume_backed(
+            self.context, instance, copy.deepcopy(image_meta), 'test-snapshot')
+
+        bdm = {'no_device': False, 'volume_id': '1',
+               'connection_info': 'inf', 'device_name': '/dev/vda'}
+
+        for key in block_device.BlockDeviceDict._db_only_fields:
+            bdm[key] = 'MUST DELETE'
+
+        instance_bdms.append(bdm)
+
+        expect_meta['properties']['block_device_mapping'] = []
+
+        expect_meta['properties']['block_device_mapping'].append(
+            {'no_device': False, 'snapshot_id': '1-snapshot',
+             'device_name': '/dev/vda'})
+
+        # All the db_only fields and the volume ones are removed
+        self.compute_api.snapshot_volume_backed(
+            self.context, instance, copy.deepcopy(image_meta), 'test-snapshot')
+
+        image_mappings = [{'device': 'vda', 'virtual': 'ephemeral0'},
+                          {'device': 'vdb', 'virtual': 'swap'},
+                          {'device': 'vdc', 'virtual': 'ephemeral1'}]
+
+        image_meta['properties']['mappings'] = image_mappings
+
+        expect_meta['properties']['block_device_mapping'].extend([
+            {'no_device': True, 'device_name': '/dev/vdb'},
+            {'no_device': True, 'device_name': '/dev/vdc'}])
+
+        # Check that the mappgins from the image properties are included
+        self.compute_api.snapshot_volume_backed(
+            self.context, instance, copy.deepcopy(image_meta), 'test-snapshot')
+
     def test_volume_snapshot_create(self):
         volume_id = '1'
         create_info = {'id': 'eyedee'}