openstack/deb-nova
Code Issues Proposed changes

Drop fields from BuildRequest object and model

Browse Source 
Now that there's a serialized instance object on the BuildRequest object
and in the build_requests table a lot of fields/columns are no longer
necessary. To prepare for removal of the columns from the db at a later
time references to them have been removed. This starts a deprecation
timer so that they can be removed in Ocata at the earliest.

Note that the BuildRequest object was never passed over the wire, and is
just created and immediately destroyed at this point so there is no need
to handle backwards compatibility for these changes.

Change-Id: Ie86ba66d5f73c153b68bef83661d4c8bdb0ae9dc
This commit is contained in:
Andrew Laski
2016-04-14 11:37:02 -04:00
parent 4a02f34736
commit 98a05bc637
9 changed files with 49 additions and 236 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
nova/compute/api.py
View File

@@ -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.

33
nova/db/sqlalchemy/api_models.py
View File

@@ -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):

56
nova/objects/build_request.py
View File

@@ -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

17
nova/tests/functional/db/api/test_migrations.py
View File

@@ -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

32
nova/tests/functional/db/test_build_request.py
View File

@@ -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)

42
nova/tests/unit/compute/test_compute_api.py
View File

@@ -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:

49
nova/tests/unit/fake_build_request.py
View File

@@ -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':

13
nova/tests/unit/objects/test_build_request.py
View File

@@ -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'])

2
nova/tests/unit/objects/test_objects.py
View File

@@ -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',