Browse Source

Use deepcopy when process filters in db api

In db API when we process filters, we didn't
use deepcopy. In cases of "tags" and "not-tags"
we used pop to get the first tag, filtered out
results, and then joined with other tags for
later filtering. When we did pop(), the original
value was deleted, the key "tags"/"not-tags" remains.

In the cell scenario, both single cell(we will
query cell0 and the other cell) and multicell,
as we have to query all the cells in a loop and
the tags list in the filter will keep popping,
this will lead to either a HTTP 500 error(popping
from an empty list) or incorrect result(when
number of tags in the list is larger than cell
number, no HTTP 500 will show, but the filter
results for each cell will be different as
each loop will pop one tag).

closes-bug: #1682693

Change-Id: Ia2738dd0c7d1842b68c83d0a9e75e26b2f8d492a
Kevin_Zheng 3 years ago
committed by Matt Riedemann
3 changed files with 29 additions and 8 deletions
  1. +1
  2. +2
  3. +26

+ 1
- 1
nova/db/sqlalchemy/ View File

@@ -2147,7 +2147,7 @@ def instance_get_all_by_filters_sort(context, filters, limit=None, marker=None,

# Make a copy of the filters dictionary to use going forward, as we'll
# be modifying it and we shouldn't affect the caller's use of it.
filters = filters.copy()
filters = copy.deepcopy(filters)

if 'changes-since' in filters:
changes_since = timeutils.normalize_time(filters['changes-since'])

+ 2
- 7
nova/tests/functional/regressions/ View File

@@ -14,7 +14,6 @@

from nova import test
from nova.tests import fixtures as nova_fixtures
from nova.tests.functional.api import client as api_client
from nova.tests.functional import integrated_helpers
from nova.tests.unit.image import fake as image_fake
from nova.tests.unit import policy_fixture
@@ -90,9 +89,5 @@ class ServerTagsFilteringTest(test.TestCase,
self.assertEqual(['bar', 'foo'], sorted(server['tags']))

# query for the shared tag and we should get two servers back
# FIXME(mriedem): This causes a 500 error until bug 1682693 is fixed.
ex = self.assertRaises(api_client.OpenStackApiException,
self.assertEqual(500, ex.response.status_code)
self.assertIn('IndexError', ex.message)
servers = self.api.get_servers(search_opts=dict(tags='foo'))
self.assertEqual(2, len(servers))

+ 26
- 0
nova/tests/unit/db/ View File

@@ -10348,6 +10348,32 @@ class TestInstanceTagsFiltering(test.TestCase):
self._assertEqualInstanceUUIDs([uuids[0], uuids[1], uuids[3], uuids[4],
uuids[6], uuids[7]], result)

def test_instance_get_all_by_filters_not_tags_multiple_cells(self):
"""Test added for bug 1682693.

In cells v2 scenario, db.instance_get_all_by_filters() will
be called multiple times to search across all cells. This
test tests that filters for all cells remain the same in the
uuids = self._create_instances(8)

db.instance_tag_set(self.ctxt, uuids[0], [u't1'])
db.instance_tag_set(self.ctxt, uuids[1], [u't2'])
db.instance_tag_set(self.ctxt, uuids[2], [u't1', u't2'])
db.instance_tag_set(self.ctxt, uuids[3], [u't2', u't3'])
db.instance_tag_set(self.ctxt, uuids[4], [u't3'])
db.instance_tag_set(self.ctxt, uuids[5], [u't1', u't2', u't3'])
db.instance_tag_set(self.ctxt, uuids[6], [u't3', u't4'])
db.instance_tag_set(self.ctxt, uuids[7], [])

filters = {'not-tags': [u't1', u't2']}

result = db.instance_get_all_by_filters(self.ctxt, filters)

self._assertEqualInstanceUUIDs([uuids[0], uuids[1], uuids[3], uuids[4],
uuids[6], uuids[7]], result)

def test_instance_get_all_by_filters_not_tags_any(self):
uuids = self._create_instances(8)