From 5c50a45a1e5b66e6ef97192bbaec6fdd54f9c2a4 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Fri, 14 May 2021 15:29:47 +0200 Subject: [PATCH] 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 --- nova/objects/request_spec.py | 7 +++++-- nova/tests/unit/objects/test_request_spec.py | 13 ++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/nova/objects/request_spec.py b/nova/objects/request_spec.py index 11334335e0ea..5478a1e9c9aa 100644 --- a/nova/objects/request_spec.py +++ b/nova/objects/request_spec.py @@ -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: diff --git a/nova/tests/unit/objects/test_request_spec.py b/nova/tests/unit/objects/test_request_spec.py index f31fa832e76e..7edc84bf62dc 100644 --- a/nova/tests/unit/objects/test_request_spec.py +++ b/nova/tests/unit/objects/test_request_spec.py @@ -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)