Add block_device_mappings to BuildRequest

Due to the os-extended-volumes API it is necessary to be able to
retrieve block device mapping info for an instance at any time. In order
to do so it needs to be stored with the build request.

This also makes it available during instance deletion where it may be
useful to look up whether delete_on_termination is set on a bdm so that
it can be cleaned up.

Change-Id: Ib774a43e49b7153b3f7b099a59483c62003ee7a8
Partially-Implements: bp add-buildrequest-obj

nova/compute/api.py

@@ -942,11 +942,10 @@ class API(base.Base):
                     self._bdm_validate_set_size_and_instance(context,
                         instance, instance_type, block_device_mapping))
 
-                # TODO(alaski): Pass block_device_mapping here when the object
-                # supports it.
                 build_request = objects.BuildRequest(context,
                         instance=instance, instance_uuid=instance.uuid,
-                        project_id=instance.project_id)
+                        project_id=instance.project_id,
+                        block_device_mappings=block_device_mapping)
                 build_request.create()
                 # Create an instance_mapping.  The null cell_mapping indicates
                 # that the instance doesn't yet exist in a cell, and lookups

nova/db/sqlalchemy/api_migrations/migrate_repo/versions/019_build_request_add_block_device_mapping.py

@@ -0,0 +1,26 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from sqlalchemy import Column
+from sqlalchemy import MetaData
+from sqlalchemy import Table
+from sqlalchemy import Text
+
+
+def upgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+    build_requests = Table('build_requests', meta, autoload=True)
+
+    if not hasattr(build_requests.c, 'block_device_mappings'):
+        build_requests.create_column(Column('block_device_mappings', Text()))

nova/db/sqlalchemy/api_models.py

@@ -234,6 +234,7 @@ class BuildRequest(API_BASE):
     instance_uuid = Column(String(36))
     project_id = Column(String(255), nullable=False)
     instance = Column(Text)
+    block_device_mappings = Column(Text)
     # TODO(alaski): Drop these from the db in Ocata
     # columns_to_drop = ['request_spec_id', 'user_id', 'display_name',
     #         'instance_metadata', 'progress', 'vm_state', 'task_state',

nova/objects/build_request.py

@@ -13,6 +13,7 @@
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
+from oslo_utils import versionutils
 from oslo_versionedobjects import exception as ovoo_exc
 import six
 
@@ -31,13 +32,15 @@ LOG = logging.getLogger(__name__)
 @base.NovaObjectRegistry.register
 class BuildRequest(base.NovaObject):
     # Version 1.0: Initial version
-    VERSION = '1.0'
+    # Version 1.1: Added block_device_mappings
+    VERSION = '1.1'
 
     fields = {
         'id': fields.IntegerField(),
         'instance_uuid': fields.UUIDField(),
         'project_id': fields.StringField(),
         'instance': fields.ObjectField('Instance'),
+        'block_device_mappings': fields.ObjectField('BlockDeviceMappingList'),
         # NOTE(alaski): Normally these would come from the NovaPersistentObject
         # mixin but they're being set explicitly because we only need
         # created_at/updated_at. There is no soft delete for this object.
@@ -45,6 +48,13 @@ class BuildRequest(base.NovaObject):
         'updated_at': fields.DateTimeField(nullable=True),
     }
 
+    def obj_make_compatible(self, primitive, target_version):
+        super(BuildRequest, self).obj_make_compatible(primitive,
+                                                      target_version)
+        target_version = versionutils.convert_version_to_tuple(target_version)
+        if target_version < (1, 1) and 'block_device_mappings' in primitive:
+            del primitive['block_device_mappings']
+
     def _load_instance(self, db_instance):
         # NOTE(alaski): Be very careful with instance loading because it
         # changes more than most objects.
@@ -69,6 +79,22 @@ class BuildRequest(base.NovaObject):
                               'BuildRequest'))
             raise exception.BuildRequestNotFound(uuid=self.instance_uuid)
 
+    def _load_block_device_mappings(self, db_bdms):
+        # 'db_bdms' is a serialized BlockDeviceMappingList object. If it's None
+        # we're in a mixed version nova-api scenario and can't retrieve the
+        # actual list. Set it to an empty list here which will cause a
+        # temporary API inconsistency that will be resolved as soon as the
+        # instance is scheduled and on a compute.
+        if db_bdms is None:
+            LOG.debug('Failed to load block_device_mappings from BuildRequest '
+                      'for instance %s because it is None', self.instance_uuid)
+            self.block_device_mappings = objects.BlockDeviceMappingList()
+            return
+
+        self.block_device_mappings = (
+            objects.BlockDeviceMappingList.obj_from_primitive(
+                jsonutils.loads(db_bdms)))
+
     @staticmethod
     def _from_db_object(context, req, db_req):
         # Set this up front so that it can be pulled for error messages or

nova/tests/functional/db/api/test_migrations.py

@@ -441,6 +441,10 @@ class NovaAPIMigrationsWalk(test_migrations.WalkVersionsMixin):
         self.assertIndexExists(engine, 'instance_group_member',
                                'instance_group_member_instance_idx')
 
+    def _check_019(self, engine, data):
+        self.assertColumnExists(engine, 'build_requests',
+                                'block_device_mappings')
+
 
 class TestNovaAPIMigrationsWalkSQLite(NovaAPIMigrationsWalk,
                                       test_base.DbTestCase,

nova/tests/functional/db/test_build_request.py

@@ -89,6 +89,17 @@ class BuildRequestTestCase(test.NoDBTestCase):
                                      'created_at': date_comp,
                                      'keypairs': obj_comp})
                 continue
+            elif key == 'block_device_mappings':
+                db_bdms = objects.BlockDeviceMappingList.obj_from_primitive(
+                    jsonutils.loads(db_value))
+                self.assertEqual(1, len(db_bdms))
+                # Can't compare list objects directly, just compare the single
+                # item they contain.
+                test_objects.compare_obj(self, expected[0], db_bdms[0],
+                                         allow_missing=['instance'],
+                                         comparators={'created_at': date_comp,
+                                                      'updated_at': date_comp})
+                continue
             self.assertEqual(expected, db_value)
 
     def test_destroy(self):

nova/tests/unit/compute/test_compute_api.py

@@ -3143,14 +3143,15 @@ class _ComputeAPIUnitTestMixIn(object):
         @mock.patch.object(objects, 'Instance')
         @mock.patch.object(self.compute_api.security_group_api,
                 'ensure_default')
-        @mock.patch.object(self.compute_api, '_validate_bdm')
+        @mock.patch.object(self.compute_api,
+                           '_bdm_validate_set_size_and_instance')
         @mock.patch.object(self.compute_api, '_create_block_device_mapping')
         @mock.patch.object(objects.RequestSpec, 'from_components')
         @mock.patch.object(objects, 'BuildRequest')
         @mock.patch.object(objects.InstanceMapping, 'create')
         def do_test(_mock_inst_mapping_create, mock_build_req,
                 mock_req_spec_from_components, _mock_create_bdm,
-                _mock_validate_bdm, _mock_ensure_default, mock_inst,
+                mock_bdm_validate, _mock_ensure_default, mock_inst,
                 mock_check_num_inst_quota):
             quota_mock = mock.MagicMock()
 
@@ -3163,6 +3164,8 @@ class _ComputeAPIUnitTestMixIn(object):
             for inst_mock in inst_mocks:
                 inst_mock.project_id = 'fake-project'
             mock_inst.side_effect = inst_mocks
+            bdm_mocks = [mock.MagicMock() for i in range(max_count)]
+            mock_bdm_validate.side_effect = bdm_mocks
             build_req_mocks = [mock.MagicMock() for i in range(max_count)]
             mock_build_req.side_effect = build_req_mocks
 
@@ -3222,11 +3225,13 @@ class _ComputeAPIUnitTestMixIn(object):
                     mock.call(ctxt,
                               instance=instances[0],
                               instance_uuid=instances[0].uuid,
-                              project_id=instances[0].project_id),
+                              project_id=instances[0].project_id,
+                              block_device_mappings=bdm_mocks[0]),
                     mock.call(ctxt,
                               instance=instances[1],
                               instance_uuid=instances[1].uuid,
-                              project_id=instances[1].project_id),
+                              project_id=instances[1].project_id,
+                              block_device_mappings=bdm_mocks[1]),
                     ]
             mock_build_req.assert_has_calls(build_req_calls)
             for build_req_mock in build_req_mocks:

nova/tests/unit/fake_build_request.py

@@ -18,7 +18,9 @@ from oslo_utils import uuidutils
 from nova import context
 from nova import objects
 from nova.objects import fields
+from nova.tests.unit import fake_block_device
 from nova.tests.unit import fake_instance
+from nova.tests import uuidsentinel as uuids
 
 
 def fake_db_req(**updates):
@@ -26,11 +28,23 @@ def fake_db_req(**updates):
     instance_uuid = uuidutils.generate_uuid()
     instance = fake_instance.fake_instance_obj(ctxt, objects.Instance,
             uuid=instance_uuid)
+    block_devices = objects.BlockDeviceMappingList(
+        objects=[fake_block_device.fake_bdm_object(
+            context,
+            fake_block_device.FakeDbBlockDeviceDict(
+                source_type='blank', destination_type='local',
+                guest_format='foo', device_type='disk', disk_bus='',
+                boot_index=1, device_name='xvda', delete_on_termination=False,
+                snapshot_id=None, volume_id=None, volume_size=0,
+                image_id='bar', no_device=False, connection_info=None,
+                tag='', instance_uuid=uuids.instance))])
     db_build_request = {
             'id': 1,
             'project_id': 'fake-project',
             'instance_uuid': instance_uuid,
             'instance': jsonutils.dumps(instance.obj_to_primitive()),
+            'block_device_mappings': jsonutils.dumps(
+                block_devices.obj_to_primitive()),
             'created_at': datetime.datetime(2016, 1, 16),
             'updated_at': datetime.datetime(2016, 1, 16),
     }
@@ -65,6 +79,10 @@ def fake_req_obj(ctxt, db_req=None):
             if field == 'instance':
                 req_obj.instance = objects.Instance.obj_from_primitive(
                         jsonutils.loads(value))
+            elif field == 'block_device_mappings':
+                req_obj.block_device_mappings = (
+                    objects.BlockDeviceMappingList.obj_from_primitive(
+                        jsonutils.loads(value)))
         elif field == 'instance_metadata':
             setattr(req_obj, field, jsonutils.loads(value))
         else:

nova/tests/unit/objects/test_objects.py

@@ -1106,7 +1106,7 @@ object_data = {
     'BandwidthUsageList': '1.2-5fe7475ada6fe62413cbfcc06ec70746',
     'BlockDeviceMapping': '1.17-5e094927f1251770dcada6ab05adfcdb',
     'BlockDeviceMappingList': '1.17-1e568eecb91d06d4112db9fd656de235',
-    'BuildRequest': '1.0-c6cd434db5cbdb4d1ebb935424261377',
+    'BuildRequest': '1.1-5a5ce31c2f4dcd67088342a9164d13b4',
     'CellMapping': '1.0-7f1a7e85a22bbb7559fc730ab658b9bd',
     'CellMappingList': '1.0-4ee0d9efdfd681fed822da88376e04d2',
     'ComputeNode': '1.16-2436e5b836fa0306a3c4e6d9e5ddacec',