Browse Source

Don't persist could-be-stale InstanceGroup fields in RequestSpec

Not only might these be stale, but they can also be very large, especially
if InstanceGroup.members has a lot of peer instance uuids. This can overrun
a 64kb TEXT field and cause a failure.

Related-Bug: #1738094

Change-Id: Ie70c77db753711e1449e99534d3b83669871943f
(cherry picked from commit 94fd36f058)
changes/25/528225/2
Dan Smith 3 years ago
committed by Artom Lifshitz
parent
commit
922c95a03d
4 changed files with 33 additions and 4 deletions
  1. +20
    -1
      nova/objects/request_spec.py
  2. +3
    -0
      nova/tests/functional/db/test_request_spec.py
  3. +2
    -1
      nova/tests/unit/fake_request_spec.py
  4. +8
    -2
      nova/tests/unit/objects/test_request_spec.py

+ 20
- 1
nova/objects/request_spec.py View File

@ -441,6 +441,18 @@ class RequestSpec(base.NovaObject):
else:
setattr(spec, key, getattr(spec_obj, key))
spec._context = context
if 'instance_group' in spec and spec.instance_group:
# NOTE(danms): We don't store the full instance group in
# the reqspec since it would be stale almost immediately.
# Instead, load it by uuid here so it's up-to-date.
try:
spec.instance_group = objects.InstanceGroup.get_by_uuid(
context, spec.instance_group.uuid)
except exception.InstanceGroupNotFound:
# NOTE(danms): Instance group may have been deleted
spec.instance_group = None
spec.obj_reset_changes()
return spec
@ -480,7 +492,14 @@ class RequestSpec(base.NovaObject):
# NOTE(alaski): The db schema is the full serialized object in a
# 'spec' column. If anything has changed we rewrite the full thing.
if updates:
db_updates = {'spec': jsonutils.dumps(self.obj_to_primitive())}
# NOTE(danms): Don't persist the could-be-large and could-be-stale
# properties of InstanceGroup
spec = self.obj_clone()
if 'instance_group' in spec and spec.instance_group:
spec.instance_group.members = None
spec.instance_group.hosts = None
db_updates = {'spec': jsonutils.dumps(spec.obj_to_primitive())}
if 'instance_uuid' in updates:
db_updates['instance_uuid'] = updates['instance_uuid']
return db_updates


+ 3
- 0
nova/tests/functional/db/test_request_spec.py View File

@ -30,6 +30,9 @@ class RequestSpecTestCase(test.NoDBTestCase):
def setUp(self):
super(RequestSpecTestCase, self).setUp()
self.useFixture(fixtures.Database(database='api'))
# NOTE(danms): Only needed for the fallback legacy main db loading
# code in InstanceGroup.
self.useFixture(fixtures.Database(database='main'))
self.context = context.RequestContext('fake-user', 'fake-project')
self.spec_obj = request_spec.RequestSpec()
self.instance_uuid = None


+ 2
- 1
nova/tests/unit/fake_request_spec.py View File

@ -16,6 +16,7 @@ from oslo_utils import uuidutils
from nova import context
from nova import objects
from nova.tests.unit import fake_flavor
from nova.tests import uuidsentinel as uuids
INSTANCE_NUMA_TOPOLOGY = objects.InstanceNUMATopology(
@ -77,7 +78,7 @@ def fake_spec_obj(remove_id=False):
req_obj.flavor = fake_flavor.fake_flavor_obj(ctxt)
req_obj.retry = objects.SchedulerRetries()
req_obj.limits = objects.SchedulerLimits()
req_obj.instance_group = objects.InstanceGroup()
req_obj.instance_group = objects.InstanceGroup(uuid=uuids.instgroup)
req_obj.project_id = 'fake'
req_obj.num_instances = 1
req_obj.availability_zone = None


+ 8
- 2
nova/tests/unit/objects/test_request_spec.py View File

@ -494,9 +494,11 @@ class _TestRequestSpecObject(object):
@mock.patch.object(request_spec.RequestSpec,
'_get_by_instance_uuid_from_db')
def test_get_by_instance_uuid(self, get_by_uuid):
@mock.patch('nova.objects.InstanceGroup.get_by_uuid')
def test_get_by_instance_uuid(self, mock_get_ig, get_by_uuid):
fake_spec = fake_request_spec.fake_db_spec()
get_by_uuid.return_value = fake_spec
mock_get_ig.return_value = objects.InstanceGroup(name='fresh')
req_obj = request_spec.RequestSpec.get_by_instance_uuid(self.context,
fake_spec['instance_uuid'])
@ -517,6 +519,7 @@ class _TestRequestSpecObject(object):
self.assertIsInstance(req_obj.retry, objects.SchedulerRetries)
self.assertIsInstance(req_obj.limits, objects.SchedulerLimits)
self.assertIsInstance(req_obj.instance_group, objects.InstanceGroup)
self.assertEqual('fresh', req_obj.instance_group.name)
def _check_update_primitive(self, req_obj, changes):
self.assertEqual(req_obj.instance_uuid, changes['instance_uuid'])
@ -532,11 +535,14 @@ class _TestRequestSpecObject(object):
# object fields
for field in ['image', 'numa_topology', 'pci_requests', 'flavor',
'retry', 'limits', 'instance_group']:
'retry', 'limits']:
self.assertEqual(
getattr(req_obj, field).obj_to_primitive(),
getattr(serialized_obj, field).obj_to_primitive())
self.assertIsNone(serialized_obj.instance_group.members)
self.assertIsNone(serialized_obj.instance_group.hosts)
def test_create(self):
req_obj = fake_request_spec.fake_spec_obj(remove_id=True)


Loading…
Cancel
Save