Merge "Fixes: bfv vm reboot ends up in an error state."

nova/compute/manager.py

@@ -4222,8 +4222,7 @@ class ComputeManager(manager.Manager):
         nova_attachments = []
         bdms_to_delete = []
         for bdm in bdms.objects:
-            if bdm.volume_id and bdm.source_type == 'volume' and \
-                bdm.destination_type == 'volume':
+            if bdm.volume_id and bdm.attachment_id:
                 try:
                     self.volume_api.attachment_get(context, bdm.attachment_id)
                 except exception.VolumeAttachmentNotFound:

nova/tests/fixtures/cinder.py

@@ -101,7 +101,9 @@ class CinderFixture(fixtures.Fixture):
         # multi-attach, as some flows create a blank 'reservation' attachment
         # before deleting another attachment. However, a non-multiattach volume
         # can only have at most one attachment with a host connector at a time.
+        self.volumes = collections.defaultdict(dict)
         self.volume_to_attachment = collections.defaultdict(dict)
+        self.volume_snapshots = collections.defaultdict(dict)
 
     def setUp(self):
         super().setUp()
@@ -165,6 +167,15 @@ class CinderFixture(fixtures.Fixture):
         self.useFixture(fixtures.MockPatch(
             'nova.volume.cinder.API.attachment_get_all',
             side_effect=self.fake_attachment_get_all, autospec=False))
+        self.useFixture(fixtures.MockPatch(
+            'nova.volume.cinder.API.create_snapshot_force',
+            side_effect=self.fake_create_snapshot_force, autospec=False))
+        self.useFixture(fixtures.MockPatch(
+            'nova.volume.cinder.API.get_snapshot',
+            side_effect=self.fake_get_snapshot, autospec=False))
+        self.useFixture(fixtures.MockPatch(
+            'nova.volume.cinder.API.create',
+            side_effect=self.fake_vol_create, autospec=False))
 
     def _is_multiattach(self, volume_id):
         return volume_id in [
@@ -218,13 +229,16 @@ class CinderFixture(fixtures.Fixture):
         return {'save_volume_id': new_volume_id}
 
     def fake_get(self, context, volume_id, microversion=None):
-        volume = {
-            'display_name': volume_id,
-            'id': volume_id,
-            'size': 1,
-            'multiattach': self._is_multiattach(volume_id),
-            'availability_zone': self.az
-        }
+        if volume_id in self.volumes:
+            volume = self.volumes[volume_id]
+        else:
+            volume = {
+                'display_name': volume_id,
+                'id': volume_id,
+                'size': 1,
+                'multiattach': self._is_multiattach(volume_id),
+                'availability_zone': self.az
+            }
 
         # Add any attachment details the fixture has
         fixture_attachments = self.volume_to_attachment[volume_id]
@@ -466,3 +480,36 @@ class CinderFixture(fixtures.Fixture):
 
     def delete_vol_attachment(self, vol_id):
         del self.volume_to_attachment[vol_id]
+
+    def fake_create_snapshot_force(self, _ctxt, volume_id, name, description):
+        _id = uuidutils.generate_uuid()
+        snapshot = {
+            'id': _id,
+            'volume_id': volume_id,
+            'display_name': name,
+            'display_description': description,
+            'status': 'creating',
+            }
+        self.volume_snapshots[_id] = snapshot
+        return snapshot
+
+    def fake_get_snapshot(self, _ctxt, snap_id):
+        if snap_id in self.volume_snapshots:
+            # because instance is getting unquiesce_instance
+            self.volume_snapshots[snap_id]['status'] = 'available'
+            return self.volume_snapshots[snap_id]
+
+    def fake_vol_create(self, _ctxt, size, name, description,
+            snapshot=None, image_id=None, volume_type=None, metadata=None,
+            availability_zone=None):
+        _id = uuidutils.generate_uuid()
+        volume = {
+            'id': _id,
+            'status': 'available',
+            'display_name': name or 'fake-cinder-vol',
+            'attach_status': 'detached',
+            'size': size,
+            'display_description': description or 'fake-description',
+        }
+        self.volumes[_id] = volume
+        return volume

nova/tests/functional/integrated_helpers.py

@@ -27,6 +27,7 @@ import time
 from oslo_concurrency import lockutils
 from oslo_log import log as logging
 import oslo_messaging as messaging
+from oslo_utils.fixture import uuidsentinel as uuids
 
 from nova.compute import instance_actions
 from nova.compute import rpcapi as compute_rpcapi
@@ -702,6 +703,42 @@ class InstanceHelperMixin:
             self.cinder.create_vol_attachment(
                 volume_id, server['id'])
 
+    def _create_server_boot_from_volume(self):
+        bfv_image_id = uuids.bfv_image_uuid
+        timestamp = datetime.datetime(2011, 1, 1, 1, 2, 3)
+
+        image = {
+            'id': bfv_image_id,
+            'name': 'fake_image_name',
+            'created_at': timestamp,
+            'updated_at': timestamp,
+            'deleted_at': None,
+            'deleted': False,
+            'status': 'active',
+            'container_format': 'raw',
+            'disk_format': 'raw',
+            'min_disk': 0
+        }
+
+        self.glance.create(None, image)
+
+        # for bfv, image is not required in server request
+        server = self._build_server()
+        server.pop('imageRef')
+
+        # as bfv-image will be used as source in block_device_mapping_v2
+        # here block device will be created based on bfv-image
+        # i.e bfvimage_id
+        server['block_device_mapping_v2'] = [{
+            'source_type': 'image',
+            'destination_type': 'volume',
+            'boot_index': 0,
+            'uuid': bfv_image_id,
+            'volume_size': 1,
+        }]
+        server = self.api.post_server({'server': server})
+        return self._wait_for_state_change(server, 'ACTIVE')
+
 
 class PlacementHelperMixin:
     """A helper mixin for interacting with placement."""

nova/tests/functional/test_boot_from_volume.py

@@ -200,6 +200,22 @@ class BootFromVolumeTest(integrated_helpers._IntegratedTestBase):
         self.assertIn('You specified more local devices than the limit allows',
                       str(ex))
 
+    def test_reboot_bfv_instance(self):
+        # verify bdm 'source_type': 'image' and 'destination_type': 'volume'
+        server = self._create_server_boot_from_volume()
+        server = self._reboot_server(server, hard=True)
+        self.assertEqual(server['status'], 'ACTIVE')
+
+    def test_reboot_bfv_instance_snapshot(self):
+        # verify bdm 'source_type': 'snapshot' and 'destination_type': 'volume'
+        server = self._create_server_boot_from_volume()
+        self._snapshot_server(server, 'snap1')
+        images = self.api.get_images()
+        snap_img = [img for img in images if img['name'] == 'snap1'][0]
+        server1 = self._create_server(image_uuid=snap_img['id'])
+        self._reboot_server(server1, hard=True)
+        self.assertEqual(server['status'], 'ACTIVE')
+
 
 class BootFromVolumeLargeRequestTest(test.TestCase,
                                      integrated_helpers.InstanceHelperMixin):