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

2 files changed, 17 insertions, 3 deletions

diff --git a/nova/objects/request_spec.py b/nova/objects/request_spec.py
index 11334335e0..5478a1e9c9 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 f31fa832e7..7edc84bf62 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)