Add regression test for server filtering by tags bug 1682693

There was a latent bug in the DB API code such that when we
process filters when listing instances, the various tags
filters have their values popped out of the filters dict and
the values (which are lists) for the filter also have the
first item in the list popped out to build the query.

This latent bug was exposed in Newton when we started listing
instances in the API from both cell0 and the main cell database,
because the query to cell0 would pop an item and then it
would not be in the 2nd query to the main cell database. If we
only had one tag in the filter list, we get an IndexError on
the 2nd pop() call.

Note that we also use the build_requests table in the API to
list instances, but there would not be any tagged servers in
that table since a server has to be ACTIVE before you can tag it,
and build_requests only exist until the instance is put into a
cell and starts building there (so it won't be ACTIVE yet).

Change-Id: I65bdfadd406368775c794ee4d8a16302a8a134bc
Related-Bug: #1682693
(cherry picked from commit 01dd1a05a213c0cbd0097188418cabe915291c8d)

diff --git a/nova/tests/functional/api/client.py b/nova/tests/functional/api/client.py
index 7ebc2579c0544c5cc0bb7e421806cd1eefcf566c..e0e9fe4cf1cec2750a2aeb5c516bced1765b52bd 100644 (file)
--- a/nova/tests/functional/api/client.py
+++ b/nova/tests/functional/api/client.py
@@ -82,6 +82,10 @@ class OpenStackApiException(Exception):
                         '_body': _body})
 
         super(OpenStackApiException, self).__init__(message)
+        # py35 does not give special meaning to the first arg and store it
+        # as the message variable.
+        if not hasattr(self, 'message'):
+            self.message = message
 
 
 class OpenStackApiAuthenticationException(OpenStackApiException):
@@ -397,3 +401,11 @@ class TestOpenStackClient(object):
 
     def get_limits(self):
         return self.api_get('/limits').body['limits']
+
+    def put_server_tags(self, server_id, tags):
+        """Put (or replace) a list of tags on the given server.
+
+        Returns the list of tags from the response.
+        """
+        return self.api_put('/servers/%s/tags' % server_id,
+                            {'tags': tags}).body['tags']

diff --git a/nova/tests/functional/integrated_helpers.py b/nova/tests/functional/integrated_helpers.py
index 192b465592c0a10d3e91a0f1b592a305a9675a1a..a6014ace5ebf11501c4e3f7a163e3cafcd00f8c4 100644 (file)
--- a/nova/tests/functional/integrated_helpers.py
+++ b/nova/tests/functional/integrated_helpers.py
@@ -234,7 +234,7 @@ class InstanceHelperMixin(object):
         return server
 
     def _build_minimal_create_server_request(self, api, name, image_uuid=None,
-                                             flavor_id=None):
+                                             flavor_id=None, networks=None):
         server = {}
 
         # We now have a valid imageId
@@ -245,4 +245,6 @@ class InstanceHelperMixin(object):
             flavor_id = api.get_flavors()[1]['id']
         server['flavorRef'] = ('http://fake.server/%s' % flavor_id)
         server['name'] = name
+        if networks is not None:
+            server['networks'] = networks
         return server

diff --git a/nova/tests/functional/regressions/test_bug_1682693.py b/nova/tests/functional/regressions/test_bug_1682693.py
new file mode 100644 (file)
index 0000000..03249a9
--- /dev/null
+++ b/nova/tests/functional/regressions/test_bug_1682693.py
@@ -0,0 +1,98 @@
+# Copyright 2017 Huawei Technologies Co.,LTD.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+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
+
+
+class ServerTagsFilteringTest(test.TestCase,
+                              integrated_helpers.InstanceHelperMixin):
+    """Simple tests to create servers with tags and then list servers using
+    the various tag filters.
+
+    This is a regression test for bug 1682693 introduced in Newton when we
+    started pulling instances from cell0 and the main cell.
+    """
+
+    def setUp(self):
+        super(ServerTagsFilteringTest, self).setUp()
+        self.useFixture(policy_fixture.RealPolicyFixture())
+        # The NeutronFixture is needed to stub out validate_networks in API.
+        self.useFixture(nova_fixtures.NeutronFixture(self))
+        # Use the PlacementFixture to avoid annoying warnings in the logs.
+        self.useFixture(nova_fixtures.PlacementFixture())
+        api_fixture = self.useFixture(nova_fixtures.OSAPIFixture(
+            api_version='v2.1'))
+        self.api = api_fixture.api
+
+        # the image fake backend needed for image discovery
+        image_fake.stub_out_image_service(self)
+        self.addCleanup(image_fake.FakeImageService_reset)
+        # We have to get the image before we use 2.latest otherwise we'll get
+        # a 404 on the /images proxy API because of 2.36.
+        image_id = self.api.get_images()[0]['id']
+        # Use the latest microversion available to make sure something does
+        # not regress in new microversions; cap as necessary.
+        self.api.microversion = 'latest'
+
+        self.start_service('conductor')
+        self.flags(driver='chance_scheduler', group='scheduler')
+        self.start_service('scheduler')
+        self.start_service('compute')
+        # The consoleauth service is needed for deleting console tokens when
+        # the server is deleted.
+        self.start_service('consoleauth')
+
+        # create two test servers
+        self.servers = []
+        for x in range(2):
+            server = self.api.post_server(
+                dict(server=self._build_minimal_create_server_request(
+                    self.api, 'test-list-server-tag-filters%i' % x, image_id,
+                    networks='none')))
+            self.addCleanup(self.api.delete_server, server['id'])
+            server = self._wait_for_state_change(self.api, server, 'ACTIVE')
+            self.servers.append(server)
+
+        # now apply two tags to the first server
+        self.two_tag_server = self.servers[0]
+        self.api.put_server_tags(self.two_tag_server['id'], ['foo', 'bar'])
+        # apply one tag to the second server which intersects with one tag
+        # from the first server
+        self.one_tag_server = self.servers[1]
+        self.api.put_server_tags(self.one_tag_server['id'], ['foo'])
+
+    def test_list_servers_filter_by_tags(self):
+        """Tests listing servers and filtering by the 'tags' query
+        parameter which uses AND logic.
+        """
+        servers = self.api.get_servers(search_opts=dict(tags='foo,bar'))
+        # we should get back our server that has both tags
+        self.assertEqual(1, len(servers))
+        server = servers[0]
+        self.assertEqual(self.two_tag_server['id'], server['id'])
+        self.assertEqual(2, len(server['tags']))
+        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.api.get_servers,
+                               search_opts=dict(tags='foo'))
+        self.assertEqual(500, ex.response.status_code)
+        self.assertIn('IndexError', ex.message)