openstack
/
nova
Code Issues Proposed changes

Fix RequestLevelParams persistence handling in RequestSpec 

The request_level_params field of the RequestSpec are intentionally not
persistent and at the same time it is declared as non nullable. The
current code handling the persistence of this field actually set the
request_level_params field to None breaking the non nullable invariant
of the field.

So far this is error is not triggered as the request_level_params field
was only used lazy loading, which is defaulted the field to a new
RequestLevelParam object on the RequestSpec object that was never saved after
such lazy load. A later patch will initialize request_level_params field
in the RequestSpec.from_components() factory method to handle this
parameters from Neutron ports and that triggers a failure when the
RequestSpec is saved.

There are a set of non nullable, non persisted RequestSpec fields that
are handled individually during create / save. This patch applies the
same logic to request_level_params as well.

Change-Id: I7d11ef8abb30686f9d125cac0e48369eab839b0f
This commit is contained in:
Balazs Gibizer 2021-05-14 15:29:47 +02:00
parent b0cd985f0c
commit 5c50a45a1e
2 changed files with 17 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
nova/objects/request_spec.py
View File

@ -674,8 +674,7 @@ class RequestSpec(base.NovaObject):
spec.instance_group.hosts = None
# NOTE(mriedem): Don't persist these since they are per-request
for excluded in ('retry', 'requested_destination',
'requested_resources', 'ignore_hosts',
'request_level_params'):
'requested_resources', 'ignore_hosts'):
if excluded in spec and getattr(spec, excluded):
setattr(spec, excluded, None)
# NOTE(stephenfin): Don't persist network metadata since we have
@ -686,6 +685,10 @@ class RequestSpec(base.NovaObject):
# no need for it after scheduling
if 'requested_networks' in spec and spec.requested_networks:
del spec.requested_networks
# NOTE(gibi): Don't persist requested_networks since we have
# no need for it after scheduling
if 'request_level_params' in spec and spec.request_level_params:
del spec.request_level_params
db_updates = {'spec': jsonutils.dumps(spec.obj_to_primitive())}
if 'instance_uuid' in updates:

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

@ -674,6 +674,9 @@ class _TestRequestSpecObject(object):
req_obj.retry = expected_retry
nr = objects.NetworkRequest()
req_obj.requested_networks = objects.NetworkRequestList(objects=[nr])
req_lvl_params = objects.RequestLevelParams(
root_required={"CUSTOM_FOO"})
req_obj.request_level_params = req_lvl_params
orig_create_in_db = request_spec.RequestSpec._create_in_db
with mock.patch.object(request_spec.RequestSpec, '_create_in_db') \
@ -688,13 +691,16 @@ class _TestRequestSpecObject(object):
# 3. requested_resources
# 4. retry
# 5. requested_networks
# 6. request_level_params
data = jsonutils.loads(updates['spec'])['nova_object.data']
self.assertNotIn('network_metadata', data)
self.assertIsNone(data['requested_destination'])
self.assertIsNone(data['requested_resources'])
self.assertIsNone(data['retry'])
self.assertIsNotNone(data['instance_uuid'])
self.assertNotIn('requested_networks', data)
self.assertNotIn('request_level_params', data)
self.assertIsNotNone(data['instance_uuid'])
# also we expect that the following fields are not reset after create
# 1. network_metadata
@ -702,6 +708,7 @@ class _TestRequestSpecObject(object):
# 3. requested_resources
# 4. retry
# 5. requested_networks
# 6. request_level_params
self.assertIsNotNone(req_obj.network_metadata)
self.assertJsonEqual(expected_network_metadata.obj_to_primitive(),
req_obj.network_metadata.obj_to_primitive())
@ -717,6 +724,10 @@ class _TestRequestSpecObject(object):
self.assertIsNotNone(req_obj.requested_networks)
self.assertJsonEqual(nr.obj_to_primitive(),
req_obj.requested_networks[0].obj_to_primitive())
self.assertIsNotNone(req_obj.request_level_params)
self.assertJsonEqual(
req_lvl_params.obj_to_primitive(),
req_obj.request_level_params.obj_to_primitive())
def test_save_does_not_persist_requested_fields(self):
req_obj = fake_request_spec.fake_spec_obj(remove_id=True)