Fix regression when listing build_requests with marker and ip filter
Change Ic02206e887e3fea7752d615bbed680510c482097 attempted to optimize the GET /servers flow by skipping filtering on build requests if the ip or ip6 filters were used, since servers that are not yet scheduled (build requests) can't have ips. However, if a marker is provided and the marker is in the build_requests table, we fail to look there and then check the cells for the marker, which won't exist and result in a 400 MarkerNotFound error. This fixes the issue by *not* skipping build requests if there is a marker specified. A functional test is added which will show the 400 MarkerNotFound error if the code fix is removed. Change-Id: Ibdd157d06252c3c0219539ec798c8d9d3a8ae26f Closes-Bug: #1777458
This commit is contained in:
parent
d80c23a835
commit
8cea14abf3
|
@ -2443,7 +2443,9 @@ class API(base.Base):
|
|||
skip_build_request = False
|
||||
orig_limit = limit
|
||||
if filter_ip:
|
||||
skip_build_request = True
|
||||
# We cannot skip build requests if there is a marker since the
|
||||
# the marker could be a build request.
|
||||
skip_build_request = marker is None
|
||||
if self.network_api.has_substr_port_filtering_extension(context):
|
||||
# We're going to filter by IP using Neutron so set filter_ip
|
||||
# to False so we don't attempt post-DB query filtering in
|
||||
|
|
|
@ -319,6 +319,35 @@ class ServersPreSchedulingTestCase(test.TestCase,
|
|||
# should have removed them.
|
||||
self.assertNotIn(volume_id, cinder.attachments[server['id']])
|
||||
|
||||
def test_instance_list_build_request_marker_ip_filter(self):
|
||||
"""Tests listing instances with a marker that is in the build_requests
|
||||
table and also filtering by ip, in which case the ip filter can't
|
||||
possibly find anything because instances that are not yet scheduled
|
||||
can't have ips, but the point is to find the marker in the build
|
||||
requests table.
|
||||
"""
|
||||
self.useFixture(nova_fixtures.AllServicesCurrent())
|
||||
# Create the server.
|
||||
body = {
|
||||
'server': {
|
||||
'name': 'test_instance_list_build_request_marker_ip_filter',
|
||||
'imageRef': fake_image.get_valid_image_id(),
|
||||
'flavorRef': '1',
|
||||
'networks': 'none'
|
||||
}
|
||||
}
|
||||
server = self.api.post_server(body)
|
||||
# Now list servers using the one we just created as the marker and
|
||||
# include the ip filter (see bug 1764685).
|
||||
search_opts = {
|
||||
'marker': server['id'],
|
||||
'ip': '192.168.159.150'
|
||||
}
|
||||
servers = self.api.get_servers(search_opts=search_opts)
|
||||
# We'll get 0 servers back because there are none with the specified
|
||||
# ip filter.
|
||||
self.assertEqual(0, len(servers))
|
||||
|
||||
|
||||
class EnforceVolumeBackedForZeroDiskFlavorTestCase(
|
||||
test.TestCase, integrated_helpers.InstanceHelperMixin):
|
||||
|
|
|
@ -6107,7 +6107,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
|||
|
||||
self.compute_api.get_all(
|
||||
self.context, search_opts={'ip': 'fake'},
|
||||
limit=None, marker='fake-marker', sort_keys=['baz'],
|
||||
limit=None, marker=None, sort_keys=['baz'],
|
||||
sort_dirs=['desc'])
|
||||
|
||||
mock_list_port.assert_called_once_with(
|
||||
|
@ -6116,7 +6116,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
|||
fields = ['metadata', 'info_cache', 'security_groups']
|
||||
mock_inst_get.assert_called_once_with(
|
||||
self.context, {'ip': 'fake', 'uuid': ['fake_device_id']},
|
||||
None, 'fake-marker', fields, ['baz'], ['desc'])
|
||||
None, None, fields, ['baz'], ['desc'])
|
||||
|
||||
@mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension')
|
||||
@mock.patch.object(neutron_api.API, 'list_ports')
|
||||
|
@ -6135,7 +6135,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
|||
|
||||
self.compute_api.get_all(
|
||||
self.context, search_opts={'ip6': 'fake'},
|
||||
limit=None, marker='fake-marker', sort_keys=['baz'],
|
||||
limit=None, marker=None, sort_keys=['baz'],
|
||||
sort_dirs=['desc'])
|
||||
|
||||
mock_list_port.assert_called_once_with(
|
||||
|
@ -6144,7 +6144,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
|||
fields = ['metadata', 'info_cache', 'security_groups']
|
||||
mock_inst_get.assert_called_once_with(
|
||||
self.context, {'ip6': 'fake', 'uuid': ['fake_device_id']},
|
||||
None, 'fake-marker', fields, ['baz'], ['desc'])
|
||||
None, None, fields, ['baz'], ['desc'])
|
||||
|
||||
@mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension')
|
||||
@mock.patch.object(neutron_api.API, 'list_ports')
|
||||
|
@ -6164,7 +6164,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
|||
|
||||
self.compute_api.get_all(
|
||||
self.context, search_opts={'ip': 'fake1', 'ip6': 'fake2'},
|
||||
limit=None, marker='fake-marker', sort_keys=['baz'],
|
||||
limit=None, marker=None, sort_keys=['baz'],
|
||||
sort_dirs=['desc'])
|
||||
|
||||
mock_list_port.assert_has_calls([
|
||||
|
@ -6179,7 +6179,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
|||
mock_inst_get.assert_called_once_with(
|
||||
self.context, {'ip': 'fake1', 'ip6': 'fake2',
|
||||
'uuid': ['fake_device_id', 'fake_device_id']},
|
||||
None, 'fake-marker', fields, ['baz'], ['desc'])
|
||||
None, None, fields, ['baz'], ['desc'])
|
||||
|
||||
@mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension')
|
||||
@mock.patch.object(neutron_api.API, 'list_ports')
|
||||
|
|
Loading…
Reference in New Issue