summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatt Riedemann <mriedem.os@gmail.com>2017-12-12 21:27:28 -0500
committerMatt Riedemann <mriedem.os@gmail.com>2018-01-04 10:02:08 -0500
commit808d36475103e373f1deb3344b6829ce68d6cdd5 (patch)
tree3cefb51179ed3ea6634d7d02ac93ecad59839ff1
parent506e4f5fce02c743e149c051e0d2924291aa51aa (diff)
Raise MarkerNotFound if BuildRequestList.get_by_filters doesn't find markernewton-eol14.1.0
For some reason, probably because build requests are meant to be short lived and we don't get a lot of bugs about paging misbehavior, when paging instances with a marker, we didn't raise MarkerNotFound if we didn't find the marker in the list of build requests. Doing so would match what we do when paging over cells and listing instances using a marker. Once we find the marker, be that in build_requests, or one of the cells, we need to set the marker to None to stop looking for it elsewhere if we have more space to fill our limit. For example, see change I8a957bebfcecd6ac712103c346e028d80f1ecd7c. This patch fixes the issue by raising MarkerNotFound from BuildRequestList get_by_filters if there is a marker and we didn't find a build request for it. The compute API get_all() method handles that as normal and continues looking for the marker in one of the cells. Change-Id: I1aa3ca6cc70cef65d24dec1e7db9491c9b73f7ab Closes-Bug: #1737856 (cherry picked from commit 1706e3989157f912bce99beb99c75216a064eb2d) (cherry picked from commit 344029b94ad7ff4667403a851d33c3ddb4e97b4b) (cherry picked from commit b00b2fe9be4394e7c9cf73c168d435a2333d12f6)
Notes
Notes (review): Verified+1: IBM PowerKVM CI <kvmpower@linux.vnet.ibm.com> Code-Review+2: Dan Smith <dms@danplanet.com> Code-Review+2: Matthew Treinish <mtreinish@kortar.org> Workflow+1: Matthew Treinish <mtreinish@kortar.org> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Tue, 09 Jan 2018 11:55:01 +0000 Reviewed-on: https://review.openstack.org/530982 Project: openstack/nova Branch: refs/heads/stable/newton
-rw-r--r--nova/compute/api.py14
-rw-r--r--nova/objects/build_request.py5
-rw-r--r--nova/tests/functional/db/test_build_request.py7
-rw-r--r--nova/tests/unit/compute/test_compute_api.py16
4 files changed, 31 insertions, 11 deletions
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 6eb269d..35d0c82 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -2315,9 +2315,17 @@ class API(base.Base):
2315 # [sorted instances with no host] + [sorted instances with host]. 2315 # [sorted instances with no host] + [sorted instances with host].
2316 # This means BuildRequest and cell0 instances first, then cell 2316 # This means BuildRequest and cell0 instances first, then cell
2317 # instances 2317 # instances
2318 build_requests = objects.BuildRequestList.get_by_filters( 2318 try:
2319 context, filters, limit=limit, marker=marker, sort_keys=sort_keys, 2319 build_requests = objects.BuildRequestList.get_by_filters(
2320 sort_dirs=sort_dirs) 2320 context, filters, limit=limit, marker=marker,
2321 sort_keys=sort_keys, sort_dirs=sort_dirs)
2322 # If we found the marker in we need to set it to None
2323 # so we don't expect to find it in the cells below.
2324 marker = None
2325 except exception.MarkerNotFound:
2326 # If we didn't find the marker in the build requests then keep
2327 # looking for it in the cells.
2328 build_requests = objects.BuildRequestList()
2321 build_req_instances = objects.InstanceList( 2329 build_req_instances = objects.InstanceList(
2322 objects=[build_req.instance for build_req in build_requests]) 2330 objects=[build_req.instance for build_req in build_requests])
2323 # Only subtract from limit if it is not None 2331 # Only subtract from limit if it is not None
diff --git a/nova/objects/build_request.py b/nova/objects/build_request.py
index e0f2c6a..9404e1b 100644
--- a/nova/objects/build_request.py
+++ b/nova/objects/build_request.py
@@ -370,7 +370,8 @@ class BuildRequestList(base.ObjectListBase, base.NovaObject):
370 370
371 filtered_build_reqs.append(build_req) 371 filtered_build_reqs.append(build_req)
372 372
373 if (len(filtered_build_reqs) < 2) or (not sort_keys): 373 if (((len(filtered_build_reqs) < 2) or (not sort_keys))
374 and not marker):
374 # No need to sort 375 # No need to sort
375 return cls(context, objects=filtered_build_reqs) 376 return cls(context, objects=filtered_build_reqs)
376 377
@@ -383,6 +384,8 @@ class BuildRequestList(base.ObjectListBase, base.NovaObject):
383 if build_req.instance.uuid == marker: 384 if build_req.instance.uuid == marker:
384 marker_index = i 385 marker_index = i
385 break 386 break
387 else:
388 raise exception.MarkerNotFound(marker=marker)
386 len_build_reqs = len(sorted_build_reqs) 389 len_build_reqs = len(sorted_build_reqs)
387 limit_index = len_build_reqs 390 limit_index = len_build_reqs
388 if limit: 391 if limit:
diff --git a/nova/tests/functional/db/test_build_request.py b/nova/tests/functional/db/test_build_request.py
index c3a60f0..d65ab41 100644
--- a/nova/tests/functional/db/test_build_request.py
+++ b/nova/tests/functional/db/test_build_request.py
@@ -526,6 +526,13 @@ class BuildRequestListTestCase(test.NoDBTestCase):
526 objects.base.obj_equal_prims(req.instance, 526 objects.base.obj_equal_prims(req.instance,
527 req_list[i].instance) 527 req_list[i].instance)
528 528
529 def test_get_by_filters_marker_not_found(self):
530 self._create_req()
531 self.assertRaises(exception.MarkerNotFound,
532 build_request.BuildRequestList.get_by_filters,
533 self.context, {}, marker=uuidutils.generate_uuid(),
534 sort_keys=['id'], sort_dirs=['asc'])
535
529 def test_get_by_filters_limit(self): 536 def test_get_by_filters_limit(self):
530 reqs = [self._create_req(), 537 reqs = [self._create_req(),
531 self._create_req(), 538 self._create_req(),
diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py
index 19080372..05f3ebd 100644
--- a/nova/tests/unit/compute/test_compute_api.py
+++ b/nova/tests/unit/compute/test_compute_api.py
@@ -4345,7 +4345,7 @@ class _ComputeAPIUnitTestMixIn(object):
4345 self.context, {'foo': 'bar'}, limit=None, marker='fake-marker', 4345 self.context, {'foo': 'bar'}, limit=None, marker='fake-marker',
4346 sort_keys=['baz'], sort_dirs=['desc']) 4346 sort_keys=['baz'], sort_dirs=['desc'])
4347 mock_inst_get.assert_called_once_with( 4347 mock_inst_get.assert_called_once_with(
4348 self.context, {'foo': 'bar'}, limit=None, marker='fake-marker', 4348 self.context, {'foo': 'bar'}, limit=None, marker=None,
4349 expected_attrs=None, sort_keys=['baz'], sort_dirs=['desc']) 4349 expected_attrs=None, sort_keys=['baz'], sort_dirs=['desc'])
4350 for i, instance in enumerate(build_req_instances + cell_instances): 4350 for i, instance in enumerate(build_req_instances + cell_instances):
4351 self.assertEqual(instance, instances[i]) 4351 self.assertEqual(instance, instances[i])
@@ -4379,7 +4379,7 @@ class _ComputeAPIUnitTestMixIn(object):
4379 self.context, {'foo': 'bar'}, limit=None, marker='fake-marker', 4379 self.context, {'foo': 'bar'}, limit=None, marker='fake-marker',
4380 sort_keys=['baz'], sort_dirs=['desc']) 4380 sort_keys=['baz'], sort_dirs=['desc'])
4381 mock_inst_get.assert_called_once_with( 4381 mock_inst_get.assert_called_once_with(
4382 self.context, {'foo': 'bar'}, limit=None, marker='fake-marker', 4382 self.context, {'foo': 'bar'}, limit=None, marker=None,
4383 expected_attrs=None, sort_keys=['baz'], sort_dirs=['desc']) 4383 expected_attrs=None, sort_keys=['baz'], sort_dirs=['desc'])
4384 for i, instance in enumerate(build_req_instances + cell_instances): 4384 for i, instance in enumerate(build_req_instances + cell_instances):
4385 self.assertEqual(instance, instances[i]) 4385 self.assertEqual(instance, instances[i])
@@ -4413,14 +4413,15 @@ class _ComputeAPIUnitTestMixIn(object):
4413 self.context, {'foo': 'bar'}, limit=10, marker='fake-marker', 4413 self.context, {'foo': 'bar'}, limit=10, marker='fake-marker',
4414 sort_keys=['baz'], sort_dirs=['desc']) 4414 sort_keys=['baz'], sort_dirs=['desc'])
4415 mock_inst_get.assert_called_once_with( 4415 mock_inst_get.assert_called_once_with(
4416 self.context, {'foo': 'bar'}, limit=8, marker='fake-marker', 4416 self.context, {'foo': 'bar'}, limit=8, marker=None,
4417 expected_attrs=None, sort_keys=['baz'], sort_dirs=['desc']) 4417 expected_attrs=None, sort_keys=['baz'], sort_dirs=['desc'])
4418 for i, instance in enumerate(build_req_instances + cell_instances): 4418 for i, instance in enumerate(build_req_instances + cell_instances):
4419 self.assertEqual(instance, instances[i]) 4419 self.assertEqual(instance, instances[i])
4420 4420
4421 @mock.patch.object(context, 'target_cell') 4421 @mock.patch.object(context, 'target_cell')
4422 @mock.patch.object(objects.BuildRequestList, 'get_by_filters', 4422 @mock.patch.object(objects.BuildRequestList, 'get_by_filters',
4423 return_value=objects.BuildRequestList(objects=[])) 4423 side_effect=exception.MarkerNotFound(
4424 marker='fake-marker'))
4424 @mock.patch.object(objects.CellMapping, 'get_by_uuid') 4425 @mock.patch.object(objects.CellMapping, 'get_by_uuid')
4425 def test_get_all_includes_cell0(self, mock_cell_mapping_get, 4426 def test_get_all_includes_cell0(self, mock_cell_mapping_get,
4426 mock_buildreq_get, mock_target_cell): 4427 mock_buildreq_get, mock_target_cell):
@@ -4496,11 +4497,11 @@ class _ComputeAPIUnitTestMixIn(object):
4496 mock_target_cell.assert_called_once_with(self.context, 4497 mock_target_cell.assert_called_once_with(self.context,
4497 cell_mapping) 4498 cell_mapping)
4498 inst_get_calls = [mock.call(self.context, {'foo': 'bar'}, 4499 inst_get_calls = [mock.call(self.context, {'foo': 'bar'},
4499 limit=8, marker='fake-marker', 4500 limit=8, marker=None,
4500 expected_attrs=None, sort_keys=['baz'], 4501 expected_attrs=None, sort_keys=['baz'],
4501 sort_dirs=['desc']), 4502 sort_dirs=['desc']),
4502 mock.call(self.context, {'foo': 'bar'}, 4503 mock.call(self.context, {'foo': 'bar'},
4503 limit=6, marker='fake-marker', 4504 limit=6, marker=None,
4504 expected_attrs=None, sort_keys=['baz'], 4505 expected_attrs=None, sort_keys=['baz'],
4505 sort_dirs=['desc']) 4506 sort_dirs=['desc'])
4506 ] 4507 ]
@@ -4513,7 +4514,8 @@ class _ComputeAPIUnitTestMixIn(object):
4513 4514
4514 @mock.patch.object(context, 'target_cell') 4515 @mock.patch.object(context, 'target_cell')
4515 @mock.patch.object(objects.BuildRequestList, 'get_by_filters', 4516 @mock.patch.object(objects.BuildRequestList, 'get_by_filters',
4516 return_value=objects.BuildRequestList(objects=[])) 4517 side_effect=exception.MarkerNotFound(
4518 marker=uuids.marker))
4517 @mock.patch.object(objects.CellMapping, 'get_by_uuid') 4519 @mock.patch.object(objects.CellMapping, 'get_by_uuid')
4518 def test_get_all_cell0_marker_not_found(self, mock_cell_mapping_get, 4520 def test_get_all_cell0_marker_not_found(self, mock_cell_mapping_get,
4519 mock_buildreq_get, 4521 mock_buildreq_get,