openstack/nova
Code Issues Proposed changes

Merge "Change BuildRequest to contain a serialized instance"

Browse Source
This commit is contained in:
Jenkins
2016-04-25 16:51:21 +00:00
committed by Gerrit Code Review
parent f6be862827 a5d3b57c3d
commit 60e01bf140
7 changed files with 166 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
nova/compute/api.py
View File

@@ -910,30 +910,25 @@ class API(base.Base):
# by the network quotas
return base_options, max_network_count
def _create_build_request(self, context, instance_uuid, base_options,
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.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)
# display_name follows a whole set of rules
display_name = base_options['display_name']
if display_name is None:
display_name = self._default_display_name(instance_uuid)
if num_instances > 1:
display_name = self._new_instance_name_from_template(instance_uuid,
display_name, index)
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=display_name,
display_name=instance.display_name,
instance_metadata=base_options['metadata'],
progress=0,
vm_state=vm_states.BUILDING,
@@ -973,9 +968,6 @@ class API(base.Base):
base_options['pci_requests'], filter_properties,
instance_group, base_options['availability_zone'])
req_spec.create()
build_request = self._create_build_request(context,
instance_uuid, base_options, req_spec, security_groups,
num_instances, i)
# Create an instance object, but do not store in db yet.
instance = objects.Instance(context=context)
@@ -986,6 +978,9 @@ 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)
# 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.

41
nova/objects/build_request.py
View File

@@ -13,6 +13,7 @@
from oslo_config import cfg
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
@@ -27,7 +28,7 @@ from nova.objects import fields
CONF = cfg.CONF
LOG = logging.getLogger(__name__)
OBJECT_FIELDS = ['info_cache', 'security_groups']
OBJECT_FIELDS = ['info_cache', 'security_groups', 'instance']
JSON_FIELDS = ['instance_metadata']
IP_FIELDS = ['access_ip_v4', 'access_ip_v6']
@@ -39,6 +40,7 @@ class BuildRequest(base.NovaObject):
fields = {
'id': fields.IntegerField(),
'instance_uuid': fields.UUIDField(),
'project_id': fields.StringField(),
'user_id': fields.StringField(),
'display_name': fields.StringField(nullable=True),
@@ -55,6 +57,7 @@ class BuildRequest(base.NovaObject):
'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.
@@ -76,8 +79,36 @@ class BuildRequest(base.NovaObject):
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.
try:
self.instance = objects.Instance.obj_from_primitive(
jsonutils.loads(db_instance))
except TypeError:
LOG.debug('Failed to load instance from BuildRequest with uuid '
'%s because it is None' % (self.instance_uuid))
raise exception.BuildRequestNotFound(uuid=self.instance_uuid)
except ovoo_exc.IncompatibleObjectVersion as exc:
# This should only happen if proper service upgrade strategies are
# not followed. Log the exception and raise BuildRequestNotFound.
# If the instance can't be loaded this object is useless and may
# as well not exist.
LOG.debug('Could not deserialize instance store in BuildRequest '
'with uuid %(instance_uuid)s. Found version %(version)s '
'which is not supported here.',
dict(instance_uuid=self.instance_uuid,
version=exc.objver))
LOG.exception(_LE('Could not deserialize instance in '
'BuildRequest'))
raise exception.BuildRequestNotFound(uuid=self.instance_uuid)
@staticmethod
def _from_db_object(context, req, db_req):
# Set this up front so that it can be pulled for error messages or
# logging at any point.
req.instance_uuid = db_req['instance_uuid']
for key in req.fields:
if isinstance(req.fields[key], fields.ObjectField):
try:
@@ -97,9 +128,7 @@ class BuildRequest(base.NovaObject):
def _get_by_instance_uuid_from_db(context, instance_uuid):
db_req = (context.session.query(api_models.BuildRequest)
.options(joinedload('request_spec'))
.filter(
api_models.RequestSpec.instance_uuid == instance_uuid)
).first()
.filter_by(instance_uuid=instance_uuid)).first()
if not db_req:
raise exception.BuildRequestNotFound(uuid=instance_uuid)
return db_req
@@ -142,6 +171,10 @@ class BuildRequest(base.NovaObject):
if self.obj_attr_is_set('id'):
raise exception.ObjectActionError(action='create',
reason='already created')
if not self.obj_attr_is_set('instance_uuid'):
# We can't guarantee this is not null in the db so check here
raise exception.ObjectActionError(action='create',
reason='instance_uuid must be set')
updates = self._get_update_primitives()
db_req = self._create_in_db(self._context, updates)

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

@@ -10,6 +10,8 @@
# License for the specific language governing permissions and limitations
# under the License.
import functools
from oslo_serialization import jsonutils
from oslo_utils import uuidutils
@@ -21,6 +23,7 @@ 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
class BuildRequestTestCase(test.NoDBTestCase):
@@ -44,6 +47,7 @@ class BuildRequestTestCase(test.NoDBTestCase):
request_spec_id=req_spec.id)
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,
self.build_req_obj,
@@ -58,6 +62,19 @@ class BuildRequestTestCase(test.NoDBTestCase):
req = self._create_req()
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',
'updated_at'])
def date_comp(db_val, obj_val):
# We have this separate comparison method because compare_obj below
# assumes that db datetimes are tz unaware. That's normally true
# but not when they're part of a serialized object and not a
# dedicated datetime column.
self.assertEqual(db_val.replace(tzinfo=None),
obj_val.replace(tzinfo=None))
for key in self.build_req_obj.fields.keys():
expected = getattr(req, key)
db_value = db_req[key]
@@ -77,4 +94,17 @@ class BuildRequestTestCase(test.NoDBTestCase):
elif key in ['created_at', 'updated_at']:
# Objects store tz aware datetimes but the db does not.
expected = expected.replace(tzinfo=None)
elif key == 'instance':
db_instance = objects.Instance.obj_from_primitive(
jsonutils.loads(db_value))
test_objects.compare_obj(self, expected, db_instance,
# These objects are not loaded in the test instance
allow_missing=['pci_requests', 'numa_topology',
'pci_devices', 'security_groups', 'info_cache',
'ec2_ids', 'migration_context', 'metadata',
'vcpu_model', 'services', 'system_metadata',
'tags', 'fault'],
comparators={'flavor': flavor_comp,
'created_at': date_comp})
continue
self.assertEqual(expected, db_value)

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

@@ -3030,6 +3030,11 @@ class _ComputeAPIUnitTestMixIn(object):
'access_ip_v6': None,
'config_drive': None,
'key_name': None,
'reservation_id': None,
'kernel_id': None,
'ramdisk_id': None,
'root_device_name': None,
'user_data': None,
'numa_topology': None,
'pci_requests': None}
security_groups = {}
@@ -3066,8 +3071,7 @@ class _ComputeAPIUnitTestMixIn(object):
def test_provision_instances_creates_destroys_build_request(self):
@mock.patch.object(self.compute_api, '_check_num_instances_quota')
@mock.patch.object(objects.Instance, 'create')
@mock.patch.object(objects.Instance, 'save')
@mock.patch.object(objects, 'Instance')
@mock.patch.object(self.compute_api.security_group_api,
'ensure_default')
@mock.patch.object(self.compute_api, '_validate_bdm')
@@ -3077,20 +3081,22 @@ class _ComputeAPIUnitTestMixIn(object):
@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_create,
_mock_inst_save, mock_check_num_inst_quota):
_mock_validate_bdm, _mock_ensure_default, mock_inst,
mock_check_num_inst_quota):
quota_mock = mock.MagicMock()
req_spec_mock = mock.MagicMock()
build_req_mock = mock.MagicMock()
min_count = 1
max_count = 2
mock_check_num_inst_quota.return_value = (2, quota_mock)
req_spec_mock = mock.MagicMock()
mock_req_spec_from_components.return_value = req_spec_mock
mock_build_req.return_value = build_req_mock
inst_mocks = [mock.MagicMock() for i in range(max_count)]
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
ctxt = context.RequestContext('fake-user', 'fake-project')
flavor = self._create_flavor()
min_count = 1
max_count = 2
boot_meta = {
'id': 'fake-image-id',
'properties': {'mappings': []},
@@ -3105,6 +3111,11 @@ class _ComputeAPIUnitTestMixIn(object):
'access_ip_v6': None,
'config_drive': None,
'key_name': None,
'reservation_id': None,
'kernel_id': None,
'ramdisk_id': None,
'root_device_name': None,
'user_data': None,
'numa_topology': None,
'pci_requests': None}
security_groups = {}
@@ -3129,15 +3140,20 @@ class _ComputeAPIUnitTestMixIn(object):
security_groups, block_device_mapping, shutdown_terminate,
instance_group, check_server_group_quota,
filter_properties)
self.assertTrue(uuidutils.is_uuid_like(instances[0].uuid))
for instance in instances:
self.assertTrue(uuidutils.is_uuid_like(instance.uuid))
for inst_mock in inst_mocks:
inst_mock.create.assert_called_once_with()
display_names = ['fake-name-1', 'fake-name-2']
build_req_calls = [
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=display_names[0],
display_name=instances[0].display_name,
instance_metadata=base_options['metadata'],
progress=0,
vm_state=vm_states.BUILDING,
@@ -3150,13 +3166,13 @@ class _ComputeAPIUnitTestMixIn(object):
config_drive=False,
key_name=base_options['config_drive'],
locked_by=None),
mock.call().create(),
mock.call().destroy(),
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=display_names[1],
display_name=instances[1].display_name,
instance_metadata=base_options['metadata'],
progress=0,
vm_state=vm_states.BUILDING,
@@ -3169,10 +3185,11 @@ class _ComputeAPIUnitTestMixIn(object):
config_drive=False,
key_name=base_options['config_drive'],
locked_by=None),
mock.call().create(),
mock.call().destroy()
]
mock_build_req.assert_has_calls(build_req_calls)
for build_req_mock in build_req_mocks:
build_req_mock.create.assert_called_once_with()
build_req_mock.destroy.assert_called_once_with()
do_test()
@@ -3213,6 +3230,11 @@ class _ComputeAPIUnitTestMixIn(object):
'access_ip_v6': None,
'config_drive': None,
'key_name': None,
'reservation_id': None,
'kernel_id': None,
'ramdisk_id': None,
'root_device_name': None,
'user_data': None,
'numa_topology': None,
'pci_requests': None}
security_groups = {}

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

@@ -21,6 +21,7 @@ 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
@@ -33,6 +34,7 @@ def _req_spec_to_db_format(req_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
@@ -41,10 +43,12 @@ def fake_db_req(**updates):
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': None,
'instance_uuid': instance_uuid,
'user_id': 'fake-user',
'display_name': '',
'instance_metadata': jsonutils.dumps({'foo': 'bar'}),
@@ -61,7 +65,7 @@ def fake_db_req(**updates):
'key_name': None,
'locked_by': None,
'request_spec': _req_spec_to_db_format(req_spec),
'instance': None,
'instance': jsonutils.dumps(instance.obj_to_primitive()),
'created_at': datetime.datetime(2016, 1, 16),
'updated_at': datetime.datetime(2016, 1, 16),
}
@@ -82,10 +86,10 @@ def fake_db_req(**updates):
return db_build_request
def fake_req_obj(context, db_req=None):
def fake_req_obj(ctxt, db_req=None):
if db_req is None:
db_req = fake_db_req()
req_obj = objects.BuildRequest(context)
req_obj = objects.BuildRequest(ctxt)
for field in req_obj.fields:
value = db_req[field]
# create() can't be called if this is set
@@ -105,6 +109,9 @@ def fake_req_obj(context, db_req=None):
setattr(req_obj, field,
objects.SecurityGroupList.obj_from_primitive(
jsonutils.loads(value)))
elif field == 'instance':
req_obj.instance = objects.Instance.obj_from_primitive(
jsonutils.loads(value))
elif field == 'instance_metadata':
setattr(req_obj, field, jsonutils.loads(value))
else:

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

@@ -12,10 +12,13 @@
import mock
from oslo_serialization import jsonutils
from nova import exception
from nova import objects
from nova.objects import build_request
from nova.tests.unit import fake_build_request
from nova.tests.unit import fake_instance
from nova.tests.unit.objects import test_objects
@@ -28,14 +31,41 @@ class _TestBuildRequestObject(object):
get_by_uuid.return_value = fake_req
req_obj = build_request.BuildRequest.get_by_instance_uuid(self.context,
fake_req['request_spec']['instance_uuid'])
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['request_spec']['instance_uuid'])
fake_req['instance_uuid'])
@mock.patch.object(build_request.BuildRequest,
'_get_by_instance_uuid_from_db')
def test_get_by_instance_uuid_instance_none(self, get_by_uuid):
fake_req = fake_build_request.fake_db_req()
fake_req['instance'] = None
get_by_uuid.return_value = fake_req
self.assertRaises(exception.BuildRequestNotFound,
build_request.BuildRequest.get_by_instance_uuid, self.context,
fake_req['instance_uuid'])
@mock.patch.object(build_request.BuildRequest,
'_get_by_instance_uuid_from_db')
def test_get_by_instance_uuid_instance_version_too_new(self, get_by_uuid):
fake_req = fake_build_request.fake_db_req()
instance = fake_instance.fake_instance_obj(self.context,
objects.Instance, uuid=fake_req['instance_uuid'])
instance.VERSION = '99'
fake_req['instance'] = jsonutils.dumps(instance.obj_to_primitive)
get_by_uuid.return_value = fake_req
self.assertRaises(exception.BuildRequestNotFound,
build_request.BuildRequest.get_by_instance_uuid, self.context,
fake_req['instance_uuid'])
@mock.patch.object(build_request.BuildRequest,
'_create_in_db')
@@ -50,6 +80,9 @@ class _TestBuildRequestObject(object):
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'])
return fake_req
with mock.patch.object(build_request.BuildRequest, '_create_in_db',
@@ -62,6 +95,11 @@ class _TestBuildRequestObject(object):
self.assertRaises(exception.ObjectActionError, req_obj.create)
def test_create_uuid_set(self):
req_obj = build_request.BuildRequest(self.context)
self.assertRaises(exception.ObjectActionError, req_obj.create)
@mock.patch.object(build_request.BuildRequest, '_destroy_in_db')
def test_destroy(self, destroy_in_db):
req_obj = build_request.BuildRequest(self.context)

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-e4ca475cabb07f73d8176f661afe8c55',
'BuildRequest': '1.0-fea0b079bddc45f3150f16be5515a2a8',
'CellMapping': '1.0-7f1a7e85a22bbb7559fc730ab658b9bd',
'ComputeNode': '1.16-2436e5b836fa0306a3c4e6d9e5ddacec',
'ComputeNodeList': '1.14-3b6f4f5ade621c40e70cb116db237844',