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

nova/compute/api.py

@@ -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

nova/tests/functional/wsgi/test_servers.py

@@ -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):

nova/tests/unit/compute/test_compute_api.py

@@ -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')