openstack/nova
Code Issues Proposed changes

Merge "Drop fields from BuildRequest object and model"

Browse Source
This commit is contained in:
Jenkins
2016-05-23 15:45:12 +00:00
committed by Gerrit Code Review
parent 23c3d18a41 98a05bc637
commit 5b01be8fc2
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

@@ -27,8 +27,6 @@ from sqlalchemy import String
from sqlalchemy import Text
from sqlalchemy import Unicode
from nova.db.sqlalchemy import types
class _NovaAPIBase(models.ModelBase, models.TimestampMixin):
pass
@@ -101,11 +99,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):
@@ -176,28 +169,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

@@ -3131,19 +3131,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,
@@ -3188,6 +3188,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
@@ -3247,41 +3249,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',