diff --git a/nova/compute/api.py b/nova/compute/api.py index 2c3d495dd6..90593de69b 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -912,40 +912,6 @@ class API(base.Base): # by the network quotas return base_options, max_network_count, key_pair - def _create_build_request(self, context, instance, base_options, - request_spec, security_groups, num_instances, index): - # Store the BuildRequest that will help populate an instance - # object for a list/show request - info_cache = objects.InstanceInfoCache() - info_cache.instance_uuid = instance.uuid - info_cache.network_info = network_model.NetworkInfo() - # NOTE: base_options['config_drive'] is either True or '' due - # to how it's represented in the instances table in the db. - # BuildRequest needs a boolean. - bool_config_drive = strutils.bool_from_string( - base_options['config_drive'], default=False) - build_request = objects.BuildRequest(context, - instance=instance, - instance_uuid=instance.uuid, - request_spec=request_spec, - project_id=context.project_id, - user_id=context.user_id, - display_name=instance.display_name, - instance_metadata=base_options['metadata'], - progress=0, - vm_state=vm_states.BUILDING, - task_state=task_states.SCHEDULING, - image_ref=base_options['image_ref'], - access_ip_v4=base_options['access_ip_v4'], - access_ip_v6=base_options['access_ip_v6'], - info_cache=info_cache, - security_groups=security_groups, - config_drive=bool_config_drive, - key_name=base_options['key_name'], - locked_by=None) - build_request.create() - return build_request - def _provision_instances(self, context, instance_type, min_count, max_count, base_options, boot_meta, security_groups, block_device_mapping, shutdown_terminate, @@ -984,9 +950,10 @@ class API(base.Base): block_device_mapping, num_instances, i, shutdown_terminate, create_instance=False) - build_request = self._create_build_request(context, - instance, base_options, req_spec, security_groups, - num_instances, i) + build_request = objects.BuildRequest(context, + instance=instance, instance_uuid=instance.uuid, + project_id=instance.project_id) + build_request.create() # Create an instance_mapping. The null cell_mapping indicates # that the instance doesn't yet exist in a cell, and lookups # for it need to instead look for the RequestSpec. diff --git a/nova/db/sqlalchemy/api_models.py b/nova/db/sqlalchemy/api_models.py index ec9b192ef5..413e4f2ac1 100644 --- a/nova/db/sqlalchemy/api_models.py +++ b/nova/db/sqlalchemy/api_models.py @@ -26,8 +26,6 @@ from sqlalchemy import schema from sqlalchemy import String from sqlalchemy import Text -from nova.db.sqlalchemy import types - class _NovaAPIBase(models.ModelBase, models.TimestampMixin): pass @@ -100,11 +98,6 @@ class RequestSpec(API_BASE): id = Column(Integer, primary_key=True) instance_uuid = Column(String(36), nullable=False) spec = Column(Text, nullable=False) - build_request = orm.relationship('BuildRequest', - back_populates='request_spec', - uselist=False, - primaryjoin=( - 'RequestSpec.id == BuildRequest.request_spec_id')) class Flavors(API_BASE): @@ -175,28 +168,16 @@ class BuildRequest(API_BASE): ) id = Column(Integer, primary_key=True) - request_spec_id = Column(Integer, ForeignKey('request_specs.id')) - request_spec = orm.relationship(RequestSpec, - foreign_keys=request_spec_id, - back_populates='build_request', - primaryjoin=request_spec_id == RequestSpec.id) instance_uuid = Column(String(36)) project_id = Column(String(255), nullable=False) - user_id = Column(String(255)) - display_name = Column(String(255)) - instance_metadata = Column(Text) - progress = Column(Integer) - vm_state = Column(String(255)) - task_state = Column(String(255)) - image_ref = Column(String(255)) - access_ip_v4 = Column(types.IPAddress()) - access_ip_v6 = Column(types.IPAddress()) - info_cache = Column(Text) - security_groups = Column(Text) - config_drive = Column(Boolean, default=False) - key_name = Column(String(255)) - locked_by = Column(Enum('owner', 'admin')) instance = 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', + # 'image_ref', 'access_ip_v4', 'access_ip_v6', 'info_cache', + # 'security_groups', 'config_drive', 'key_name', 'locked_by', + # 'reservation_id', 'launch_index', 'hostname', 'kernel_id', + # 'ramdisk_id', 'root_device_name', 'user_data'] class KeyPair(API_BASE): diff --git a/nova/objects/build_request.py b/nova/objects/build_request.py index d8b1456ff7..0289fe9574 100644 --- a/nova/objects/build_request.py +++ b/nova/objects/build_request.py @@ -15,7 +15,6 @@ from oslo_log import log as logging from oslo_serialization import jsonutils from oslo_versionedobjects import exception as ovoo_exc import six -from sqlalchemy.orm import joinedload from nova.db.sqlalchemy import api as db from nova.db.sqlalchemy import api_models @@ -28,10 +27,6 @@ from nova.objects import fields CONF = cfg.CONF LOG = logging.getLogger(__name__) -OBJECT_FIELDS = ['info_cache', 'security_groups', 'instance'] -JSON_FIELDS = ['instance_metadata'] -IP_FIELDS = ['access_ip_v4', 'access_ip_v6'] - @base.NovaObjectRegistry.register class BuildRequest(base.NovaObject): @@ -42,43 +37,14 @@ class BuildRequest(base.NovaObject): 'id': fields.IntegerField(), 'instance_uuid': fields.UUIDField(), 'project_id': fields.StringField(), - 'user_id': fields.StringField(), - 'display_name': fields.StringField(nullable=True), - 'instance_metadata': fields.DictOfStringsField(nullable=True), - 'progress': fields.IntegerField(nullable=True), - 'vm_state': fields.StringField(nullable=True), - 'task_state': fields.StringField(nullable=True), - 'image_ref': fields.StringField(nullable=True), - 'access_ip_v4': fields.IPV4AddressField(nullable=True), - 'access_ip_v6': fields.IPV6AddressField(nullable=True), - 'info_cache': fields.ObjectField('InstanceInfoCache', nullable=True), - 'security_groups': fields.ObjectField('SecurityGroupList'), - 'config_drive': fields.BooleanField(default=False), - 'key_name': fields.StringField(nullable=True), - 'locked_by': fields.EnumField(['owner', 'admin'], nullable=True), - 'request_spec': fields.ObjectField('RequestSpec'), 'instance': fields.ObjectField('Instance'), # 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. - # These fields should be carried over to the instance when it is - # scheduled and created in a cell database. 'created_at': fields.DateTimeField(nullable=True), 'updated_at': fields.DateTimeField(nullable=True), } - def _load_request_spec(self, db_spec): - self.request_spec = objects.RequestSpec._from_db_object(self._context, - objects.RequestSpec(), db_spec) - - def _load_info_cache(self, db_info_cache): - self.info_cache = objects.InstanceInfoCache.obj_from_primitive( - jsonutils.loads(db_info_cache)) - - def _load_security_groups(self, db_sec_group): - self.security_groups = objects.SecurityGroupList.obj_from_primitive( - jsonutils.loads(db_sec_group)) - def _load_instance(self, db_instance): # NOTE(alaski): Be very careful with instance loading because it # changes more than most objects. @@ -115,8 +81,6 @@ class BuildRequest(base.NovaObject): getattr(req, '_load_%s' % key)(db_req[key]) except AttributeError: LOG.exception(_LE('No load handler for %s'), key) - elif key in JSON_FIELDS and db_req[key] is not None: - setattr(req, key, jsonutils.loads(db_req[key])) else: setattr(req, key, db_req[key]) req.obj_reset_changes() @@ -126,9 +90,8 @@ class BuildRequest(base.NovaObject): @staticmethod @db.api_context_manager.reader def _get_by_instance_uuid_from_db(context, instance_uuid): - db_req = (context.session.query(api_models.BuildRequest) - .options(joinedload('request_spec')) - .filter_by(instance_uuid=instance_uuid)).first() + db_req = context.session.query(api_models.BuildRequest).filter_by( + instance_uuid=instance_uuid).first() if not db_req: raise exception.BuildRequestNotFound(uuid=instance_uuid) return db_req @@ -144,26 +107,13 @@ class BuildRequest(base.NovaObject): db_req = api_models.BuildRequest() db_req.update(updates) db_req.save(context.session) - # NOTE: This is done because a later access will trigger a lazy load - # outside of the db session so it will fail. We don't lazy load - # request_spec on the object later because we never need a BuildRequest - # without the RequestSpec. - db_req.request_spec return db_req def _get_update_primitives(self): updates = self.obj_get_changes() for key, value in six.iteritems(updates): - if key in OBJECT_FIELDS and value is not None: + if isinstance(self.fields[key], fields.ObjectField): updates[key] = jsonutils.dumps(value.obj_to_primitive()) - elif key in JSON_FIELDS and value is not None: - updates[key] = jsonutils.dumps(value) - elif key in IP_FIELDS and value is not None: - # These are stored as a string in the db and must be converted - updates[key] = str(value) - req_spec_obj = updates.pop('request_spec', None) - if req_spec_obj: - updates['request_spec_id'] = req_spec_obj.id return updates @base.remotable diff --git a/nova/tests/functional/db/api/test_migrations.py b/nova/tests/functional/db/api/test_migrations.py index 657fc6732d..45e8705afb 100644 --- a/nova/tests/functional/db/api/test_migrations.py +++ b/nova/tests/functional/db/api/test_migrations.py @@ -82,6 +82,17 @@ class NovaAPIModelsSync(test_migrations.ModelsMigrationsSync): # the database. They will be removed from the model at a later time. fkey_whitelist = {'build_requests': ['request_spec_id']} + # Define a whitelist of columns that will be removed from the + # DB at a later release and aren't on a model anymore. + + column_whitelist = { + 'build_requests': ['vm_state', 'instance_metadata', + 'display_name', 'access_ip_v6', 'access_ip_v4', 'key_name', + 'locked_by', 'image_ref', 'progress', 'request_spec_id', + 'info_cache', 'user_id', 'task_state', 'security_groups', + 'config_drive'] + } + for element in diff: if isinstance(element, list): # modify_nullable is a list @@ -96,6 +107,12 @@ class NovaAPIModelsSync(test_migrations.ModelsMigrationsSync): if (tablename in fkey_whitelist and column_keys == fkey_whitelist[tablename]): continue + elif element[0] == 'remove_column': + tablename = element[2] + column = element[3] + if (tablename in column_whitelist and + column.name in column_whitelist[tablename]): + continue new_diff.append(element) return new_diff diff --git a/nova/tests/functional/db/test_build_request.py b/nova/tests/functional/db/test_build_request.py index d307832a69..c9a424191f 100644 --- a/nova/tests/functional/db/test_build_request.py +++ b/nova/tests/functional/db/test_build_request.py @@ -22,7 +22,6 @@ from nova.objects import build_request from nova import test from nova.tests import fixtures from nova.tests.unit import fake_build_request -from nova.tests.unit import fake_request_spec from nova.tests.unit.objects import test_objects @@ -40,13 +39,8 @@ class BuildRequestTestCase(test.NoDBTestCase): self.project_id = 'fake-project' def _create_req(self): - req_spec = fake_request_spec.fake_spec_obj(remove_id=True) - req_spec.instance_uuid = self.instance_uuid - req_spec.create() - args = fake_build_request.fake_db_req( - request_spec_id=req_spec.id) + args = fake_build_request.fake_db_req() args.pop('id', None) - args.pop('request_spec', None) args['instance_uuid'] = self.instance_uuid args['project_id'] = self.project_id return build_request.BuildRequest._from_db_object(self.context, @@ -63,8 +57,8 @@ class BuildRequestTestCase(test.NoDBTestCase): db_req = self.build_req_obj._get_by_instance_uuid_from_db(self.context, self.instance_uuid) - flavor_comp = functools.partial(test_objects.compare_obj, self, - allow_missing=['deleted', 'deleted_at', 'created_at', + obj_comp = functools.partial(objects.base.obj_equal_prims, + ignore=['deleted', 'deleted_at', 'created_at', 'updated_at']) def date_comp(db_val, obj_val): @@ -78,20 +72,7 @@ class BuildRequestTestCase(test.NoDBTestCase): for key in self.build_req_obj.fields.keys(): expected = getattr(req, key) db_value = db_req[key] - if key == 'request_spec': - # NOTE: The object and db value can't be compared directly as - # objects, so serialize them to a comparable form. - db_value = jsonutils.dumps(objects.RequestSpec._from_db_object( - self.context, objects.RequestSpec(), - db_value).obj_to_primitive()) - expected = jsonutils.dumps(expected.obj_to_primitive()) - elif key in build_request.OBJECT_FIELDS: - expected = jsonutils.dumps(expected.obj_to_primitive()) - elif key in build_request.JSON_FIELDS: - expected = jsonutils.dumps(expected) - elif key in build_request.IP_FIELDS: - expected = str(expected) - elif key in ['created_at', 'updated_at']: + if key in ['created_at', 'updated_at']: # Objects store tz aware datetimes but the db does not. expected = expected.replace(tzinfo=None) elif key == 'instance': @@ -104,7 +85,8 @@ class BuildRequestTestCase(test.NoDBTestCase): 'ec2_ids', 'migration_context', 'metadata', 'vcpu_model', 'services', 'system_metadata', 'tags', 'fault'], - comparators={'flavor': flavor_comp, - 'created_at': date_comp}) + comparators={'flavor': obj_comp, + 'created_at': date_comp, + 'keypairs': obj_comp}) continue self.assertEqual(expected, db_value) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index c5f533a7c5..76123a2b98 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -3106,19 +3106,19 @@ class _ComputeAPIUnitTestMixIn(object): do_test() @mock.patch('nova.objects.RequestSpec.from_components') + @mock.patch('nova.objects.BuildRequest') @mock.patch('nova.objects.Instance') @mock.patch('nova.objects.InstanceMapping.create') def test_provision_instances_with_keypair(self, mock_im, mock_instance, - mock_rs): + mock_br, mock_rs): fake_keypair = objects.KeyPair(name='test') @mock.patch.object(self.compute_api, '_check_num_instances_quota') @mock.patch.object(self.compute_api, 'security_group_api') @mock.patch.object(self.compute_api, 'create_db_entry_for_new_instance') - @mock.patch.object(self.compute_api, '_create_build_request') @mock.patch.object(self.compute_api, '_create_block_device_mapping') - def do_test(mock_cbdm, mock_cbr, mock_cdb, mock_sg, mock_cniq): + def do_test(mock_cbdm, mock_cdb, mock_sg, mock_cniq): mock_cniq.return_value = 1, mock.MagicMock() self.compute_api._provision_instances(self.context, mock.sentinel.flavor, @@ -3163,6 +3163,8 @@ class _ComputeAPIUnitTestMixIn(object): req_spec_mock = mock.MagicMock() mock_req_spec_from_components.return_value = req_spec_mock inst_mocks = [mock.MagicMock() for i in range(max_count)] + for inst_mock in inst_mocks: + inst_mock.project_id = 'fake-project' mock_inst.side_effect = inst_mocks build_req_mocks = [mock.MagicMock() for i in range(max_count)] mock_build_req.side_effect = build_req_mocks @@ -3222,41 +3224,11 @@ class _ComputeAPIUnitTestMixIn(object): mock.call(ctxt, instance=instances[0], instance_uuid=instances[0].uuid, - request_spec=req_spec_mock, - project_id=ctxt.project_id, - user_id=ctxt.user_id, - display_name=instances[0].display_name, - instance_metadata=base_options['metadata'], - progress=0, - vm_state=vm_states.BUILDING, - task_state=task_states.SCHEDULING, - image_ref=base_options['image_ref'], - access_ip_v4=base_options['access_ip_v4'], - access_ip_v6=base_options['access_ip_v6'], - info_cache=mock.ANY, - security_groups=mock.ANY, - config_drive=False, - key_name=base_options['config_drive'], - locked_by=None), + project_id=instances[0].project_id), mock.call(ctxt, instance=instances[1], instance_uuid=instances[1].uuid, - request_spec=req_spec_mock, - project_id=ctxt.project_id, - user_id=ctxt.user_id, - display_name=instances[1].display_name, - instance_metadata=base_options['metadata'], - progress=0, - vm_state=vm_states.BUILDING, - task_state=task_states.SCHEDULING, - image_ref=base_options['image_ref'], - access_ip_v4=base_options['access_ip_v4'], - access_ip_v6=base_options['access_ip_v6'], - info_cache=mock.ANY, - security_groups=mock.ANY, - config_drive=False, - key_name=base_options['config_drive'], - locked_by=None), + project_id=instances[1].project_id), ] mock_build_req.assert_has_calls(build_req_calls) for build_req_mock in build_req_mocks: diff --git a/nova/tests/unit/fake_build_request.py b/nova/tests/unit/fake_build_request.py index b7e2a68f4e..534ad69884 100644 --- a/nova/tests/unit/fake_build_request.py +++ b/nova/tests/unit/fake_build_request.py @@ -15,56 +15,21 @@ import datetime from oslo_serialization import jsonutils from oslo_utils import uuidutils -from nova.compute import task_states -from nova.compute import vm_states from nova import context -from nova.network import model as network_model from nova import objects from nova.objects import fields from nova.tests.unit import fake_instance -from nova.tests.unit import fake_request_spec - - -def _req_spec_to_db_format(req_spec): - db_spec = {'spec': jsonutils.dumps(req_spec.obj_to_primitive()), - 'id': req_spec.id, - 'instance_uuid': req_spec.instance_uuid, - } - return db_spec def fake_db_req(**updates): ctxt = context.RequestContext('fake-user', 'fake-project') instance_uuid = uuidutils.generate_uuid() - info_cache = objects.InstanceInfoCache() - info_cache.instance_uuid = instance_uuid - info_cache.network_info = network_model.NetworkInfo() - req_spec = fake_request_spec.fake_spec_obj( - context.RequestContext('fake-user', 'fake-project')) - req_spec.id = 42 - req_spec.obj_reset_changes() instance = fake_instance.fake_instance_obj(ctxt, objects.Instance, uuid=instance_uuid) db_build_request = { 'id': 1, 'project_id': 'fake-project', 'instance_uuid': instance_uuid, - 'user_id': 'fake-user', - 'display_name': '', - 'instance_metadata': jsonutils.dumps({'foo': 'bar'}), - 'progress': 0, - 'vm_state': vm_states.BUILDING, - 'task_state': task_states.SCHEDULING, - 'image_ref': None, - 'access_ip_v4': '1.2.3.4', - 'access_ip_v6': '::1', - 'info_cache': jsonutils.dumps(info_cache.obj_to_primitive()), - 'security_groups': jsonutils.dumps( - objects.SecurityGroupList().obj_to_primitive()), - 'config_drive': False, - 'key_name': None, - 'locked_by': None, - 'request_spec': _req_spec_to_db_format(req_spec), 'instance': jsonutils.dumps(instance.obj_to_primitive()), 'created_at': datetime.datetime(2016, 1, 16), 'updated_at': datetime.datetime(2016, 1, 16), @@ -97,19 +62,7 @@ def fake_req_obj(ctxt, db_req=None): continue if isinstance(req_obj.fields[field], fields.ObjectField): value = value - if field == 'request_spec': - req_spec = objects.RequestSpec._from_db_object(context, - objects.RequestSpec(), value) - req_obj.request_spec = req_spec - elif field == 'info_cache': - setattr(req_obj, field, - objects.InstanceInfoCache.obj_from_primitive( - jsonutils.loads(value))) - elif field == 'security_groups': - setattr(req_obj, field, - objects.SecurityGroupList.obj_from_primitive( - jsonutils.loads(value))) - elif field == 'instance': + if field == 'instance': req_obj.instance = objects.Instance.obj_from_primitive( jsonutils.loads(value)) elif field == 'instance_metadata': diff --git a/nova/tests/unit/objects/test_build_request.py b/nova/tests/unit/objects/test_build_request.py index 2531375057..41462924ca 100644 --- a/nova/tests/unit/objects/test_build_request.py +++ b/nova/tests/unit/objects/test_build_request.py @@ -33,11 +33,8 @@ class _TestBuildRequestObject(object): req_obj = build_request.BuildRequest.get_by_instance_uuid(self.context, fake_req['instance_uuid']) - self.assertEqual(fake_req['request_spec']['instance_uuid'], - req_obj.request_spec.instance_uuid) self.assertEqual(fake_req['instance_uuid'], req_obj.instance_uuid) self.assertEqual(fake_req['project_id'], req_obj.project_id) - self.assertIsInstance(req_obj.request_spec, objects.RequestSpec) self.assertIsInstance(req_obj.instance, objects.Instance) get_by_uuid.assert_called_once_with(self.context, fake_req['instance_uuid']) @@ -67,19 +64,13 @@ class _TestBuildRequestObject(object): build_request.BuildRequest.get_by_instance_uuid, self.context, fake_req['instance_uuid']) - @mock.patch.object(build_request.BuildRequest, - '_create_in_db') - def test_create(self, create_in_db): + def test_create(self): fake_req = fake_build_request.fake_db_req() req_obj = fake_build_request.fake_req_obj(self.context, fake_req) def _test_create_args(self2, context, changes): - for field in [fields for fields in - build_request.BuildRequest.fields if fields not in - ['created_at', 'updated_at', 'request_spec', 'id']]: + for field in ['instance_uuid', 'project_id']: self.assertEqual(fake_req[field], changes[field]) - self.assertEqual(fake_req['request_spec']['id'], - changes['request_spec_id']) self.assertEqual( jsonutils.dumps(req_obj.instance.obj_to_primitive()), changes['instance']) diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index 9f3b2e592b..0dde785bd6 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/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-fea0b079bddc45f3150f16be5515a2a8', + 'BuildRequest': '1.0-c6cd434db5cbdb4d1ebb935424261377', 'CellMapping': '1.0-7f1a7e85a22bbb7559fc730ab658b9bd', 'ComputeNode': '1.16-2436e5b836fa0306a3c4e6d9e5ddacec', 'ComputeNodeList': '1.14-3b6f4f5ade621c40e70cb116db237844',