Refactor list servers negative test and fix remaining gate bugs

test_list_servers_negative had so many issues with it, it's hard
to list them here... but at least the following has been fixed:

* No longer will the test flatly delete all instances the tenant
  and alt tenant have during every test method run (!!)
* Fixtures -- 2 active and 1 deleted server -- are created in the
  class' setUpClass() method instead of having test methods
  constantly re-launch instances -- this reduces the test time
  down about 3 minutes
* Removed all non-negative tests
* Removed all tests that were skipped due to bugs that have been
* Skip the XML create_service() test for Identity Admin tests
  because of the upstream Keystone bug 1061738

This patch also includes patches for bug fixes that Sean Gallagher
contributed but needed to be rebased into this one in order
to get the full gate to pass.

Change-Id: I0b38cf045520d93031c7916dae2479daf69d0411
fixes: LP bug #1059802
fixes: LP bug #1060373
fixes: LP bug #1061740
fixes: LP bug #1061167
diff --git a/tempest/manager.py b/tempest/manager.py
index dc4b289..6487d0c 100644
--- a/tempest/manager.py
+++ b/tempest/manager.py
@@ -127,7 +127,7 @@
         client_args = (username, password, tenant_name, auth_url)
 
         # Create our default Nova client to use in testing
-        service_type = self.config.compute.catalog_type,
+        service_type = self.config.compute.catalog_type
         return novaclient.client.Client(self.NOVACLIENT_VERSION,
                                         *client_args,
                                         service_type=service_type,
diff --git a/tempest/services/nova/json/servers_client.py b/tempest/services/nova/json/servers_client.py
index 32ec740..df239dd 100644
--- a/tempest/services/nova/json/servers_client.py
+++ b/tempest/services/nova/json/servers_client.py
@@ -164,7 +164,7 @@
 
             server_status = body['status']
             if server_status == 'ERROR' and not ignore_error:
-                raise exceptions.BuildErrorException
+                raise exceptions.BuildErrorException(server_id=server_id)
 
             if int(time.time()) - start_time >= self.build_timeout:
                 raise exceptions.TimeoutException
diff --git a/tempest/tests/compute/base.py b/tempest/tests/compute/base.py
index f36c8f2..d9c1592 100644
--- a/tempest/tests/compute/base.py
+++ b/tempest/tests/compute/base.py
@@ -111,7 +111,7 @@
                 msg = ('Unable to create isolated tenant %s because ' +
                        'it already exists. If this is related to a ' +
                        'previous test failure, try using ' +
-                       'allow_tentant_reuse in tempest.conf') % tenant_name
+                       'allow_tenant_reuse in tempest.conf') % tenant_name
                 raise exceptions.Duplicate(msg)
 
         try:
@@ -125,10 +125,10 @@
                                                          username)
                 LOG.info('Re-using existing user %s' % user)
             else:
-                msg = ('Unable to create isolated tenant %s because ' +
+                msg = ('Unable to create isolated user %s because ' +
                        'it already exists. If this is related to a ' +
                        'previous test failure, try using ' +
-                       'allow_tentant_reuse in tempest.conf') % tenant_name
+                       'allow_tenant_reuse in tempest.conf') % tenant_name
                 raise exceptions.Duplicate(msg)
 
         # Store the complete creds (including UUID ids...) for later
@@ -149,19 +149,14 @@
             admin_client.delete_tenant(tenant['id'])
 
     @classmethod
-    def clear_remaining_servers(cls):
-        # NOTE(danms): Only nuke all left-over servers if we're in our
-        # own isolated tenant
-        if not cls.isolated_creds:
-            return
-        resp, servers = cls.servers_client.list_servers()
-        for server in servers['servers']:
+    def clear_servers(cls):
+        for server in cls.servers:
             try:
                 cls.servers_client.delete_server(server['id'])
             except Exception:
                 pass
 
-        for server in servers['servers']:
+        for server in cls.servers:
             try:
                 cls.servers_client.wait_for_server_termination(server['id'])
             except Exception:
@@ -169,20 +164,21 @@
 
     @classmethod
     def tearDownClass(cls):
-        cls.clear_remaining_servers()
+        cls.clear_servers()
         cls.clear_isolated_creds()
 
-    def create_server(self, image_id=None):
+    @classmethod
+    def create_server(cls, image_id=None):
         """Wrapper utility that returns a test server"""
-        server_name = rand_name(self.__class__.__name__ + "-instance")
-        flavor = self.flavor_ref
+        server_name = rand_name(cls.__name__ + "-instance")
+        flavor = cls.flavor_ref
         if not image_id:
-            image_id = self.image_ref
+            image_id = cls.image_ref
 
-        resp, server = self.servers_client.create_server(
+        resp, server = cls.servers_client.create_server(
                                                 server_name, image_id, flavor)
-        self.servers_client.wait_for_server_status(server['id'], 'ACTIVE')
-        self.servers.append(server)
+        cls.servers_client.wait_for_server_status(server['id'], 'ACTIVE')
+        cls.servers.append(server)
         return server
 
     def wait_for(self, condition):
diff --git a/tempest/tests/compute/test_list_servers_negative.py b/tempest/tests/compute/test_list_servers_negative.py
index 2279959..1be7480 100644
--- a/tempest/tests/compute/test_list_servers_negative.py
+++ b/tempest/tests/compute/test_list_servers_negative.py
@@ -19,6 +19,7 @@
 import sys
 
 import unittest2 as unittest
+import nose
 
 from tempest import exceptions
 from tempest import openstack
@@ -47,212 +48,97 @@
                 cls.alt_manager = openstack.AltManager()
             cls.alt_client = cls.alt_manager.servers_client
 
-    def tearDown(self):
-        """Terminate instances created by tests"""
-        try:
-            for server in self.servers:
-                resp, body = self.client.delete_server(server)
-                if resp['status'] == '204':
-                    self.client.wait_for_server_termination(server)
-        except exceptions.NotFound:
-            pass
-
-    def get_active_servers(self, server_count):
-        """Returns active test instances to calling test methods"""
-        resp, body = self.client.list_servers_with_detail()
+        # Under circumstances when there is not a tenant/user
+        # created for the test case, the test case checks
+        # to see if there are existing servers for the
+        # either the normal user/tenant or the alt user/tenant
+        # and if so, the whole test is skipped. We do this
+        # because we assume a baseline of no servers at the
+        # start of the test instead of destroying any existing
+        # servers.
+        resp, body = cls.client.list_servers()
         servers = body['servers']
-        active_servers = [
-            server for server in servers if server['status'] == 'ACTIVE'
-        ]
-        num_of_active_servers = len(active_servers)
+        num_servers = len(servers)
+        if num_servers > 0:
+            username = cls.os.username
+            tenant_name = cls.os.tenant_name
+            msg = ("User/tenant %(username)s/%(tenant_name)s already have "
+                   "existing server instances. Skipping test.") % locals()
+            raise nose.SkipTest(msg)
 
-        # Check if we already have enough active servers
-        if active_servers and num_of_active_servers >= server_count:
-            return active_servers[0:server_count]
+        resp, body = cls.alt_client.list_servers()
+        servers = body['servers']
+        num_servers = len(servers)
+        if num_servers > 0:
+            username = cls.alt_manager.username
+            tenant_name = cls.alt_manager.tenant_name
+            msg = ("Alt User/tenant %(username)s/%(tenant_name)s already have "
+                   "existing server instances. Skipping test.") % locals()
+            raise nose.SkipTest(msg)
 
-        # Otherwise create the remaining shortfall of servers
-        servers_needed = server_count - num_of_active_servers
+        # The following servers are created for use
+        # by the test methods in this class. These
+        # servers are cleaned up automatically in the
+        # tearDownClass method of the super-class.
+        cls.existing_fixtures = []
+        cls.deleted_fixtures = []
+        for x in xrange(2):
+            srv = cls.create_server()
+            cls.existing_fixtures.append(srv)
 
-        for i in range(0, servers_needed):
-            srv = self.create_server()
-            active_servers.append(srv)
+        srv = cls.create_server()
+        cls.client.delete_server(srv['id'])
+        # We ignore errors on termination because the server may
+        # be put into ERROR status on a quick spawn, then delete,
+        # as the compute node expects the instance local status
+        # to be spawning, not deleted. See LP Bug#1061167
+        cls.client.wait_for_server_termination(srv['id'],
+                                               ignore_error=True)
+        cls.deleted_fixtures.append(srv)
 
-        return active_servers
-
-    def test_list_servers_when_no_servers_running(self):
-        """Return an empty list when there are no active servers"""
-        # Delete Active servers
-        try:
-            resp, body = self.client.list_servers()
-            for server in body['servers']:
-                resp, body = self.client.delete_server(server['id'])
-                self.client.wait_for_server_termination(server['id'])
-        except exceptions.NotFound:
-            pass
-        # Verify empty list
+    def test_list_servers_with_a_deleted_server(self):
+        """Verify deleted servers do not show by default in list servers"""
+        # List servers and verify server not returned
         resp, body = self.client.list_servers()
         servers = body['servers']
+        deleted_ids = [s['id'] for s in self.deleted_fixtures]
+        actual = [srv for srv in servers
+                  if srv['id'] in deleted_ids]
+        self.assertEqual('200', resp['status'])
+        self.assertEqual([], actual)
+
+    def test_list_servers_by_non_existing_image(self):
+        """Listing servers for a non existing image returns empty list"""
+        non_existing_image = '1234abcd-zzz0-aaa9-ppp3-0987654abcde'
+        resp, body = self.client.list_servers(dict(image=non_existing_image))
+        servers = body['servers']
         self.assertEqual('200', resp['status'])
         self.assertEqual([], servers)
 
-    def test_list_servers_with_a_deleted_server(self):
-        """Create and delete a server and verify empty list"""
-        server = self.get_active_servers(1)[0]
-
-        # Delete the server
-        self.client.delete_server(server['id'])
-        self.client.wait_for_server_termination(server['id'])
-
-        # List servers and verify server not returned
-        resp, body = self.client.list_servers()
-        servers = body['servers']
-        actual = [srv for srv in servers if srv['id'] == server['id']]
-        self.assertEqual('200', resp['status'])
-        self.assertEqual([], actual)
-
-    def test_list_servers_by_existing_image(self):
-        """Server list is returned for a specific image and snapshot images"""
-        try:
-            base_server = self.get_active_servers(1)[0]
-
-            # Create a snapshot of the server
-            snapshot_name = "%s_snapshot" % base_server['id']
-            resp, body = self.client.create_image(base_server['id'],
-                                                  snapshot_name)
-            snapshot_url = resp['location']
-            match = re.search('/images/(?P<image_id>.+)', snapshot_url)
-            self.assertIsNotNone(match)
-            snapshot_id = match.groupdict()['image_id']
-
-            self.images_client.wait_for_image_status(snapshot_id, 'ACTIVE')
-
-            # Create a server from the snapshot
-            snap_server = self.create_server(image_id=snapshot_id)
-            self.servers.append(snap_server['id'])
-
-            # List base servers by image
-            resp, body = self.client.list_servers({'image': self.image_ref})
-            servers = body['servers']
-            self.assertEqual('200', resp['status'])
-            self.assertIn(base_server['id'], [server['id'] for server in
-                          servers])
-            self.assertTrue(len(body['servers']) > 0)
-
-            # List snapshotted server by image
-            resp, body = self.client.list_servers({'image': snapshot_id})
-            servers = body['servers']
-            self.assertEqual('200', resp['status'])
-            self.assertIn(snap_server['id'],
-                          [server['id'] for server in servers])
-            self.assertEqual(1, len(body['servers']))
-
-        finally:
-            self.images_client.delete_image(snapshot_id)
-
-    @unittest.skip("Until Bug 1002911 is fixed")
-    def test_list_servers_by_non_existing_image(self):
-        """Listing servers for a non existing image raises error"""
-        self.assertRaises(exceptions.NotFound, self.client.list_servers,
-                          {'image': '1234abcd-zzz0-aaa9-ppp3-0987654abcde'})
-
-    @unittest.skip("Until Bug 1002911 is fixed")
-    def test_list_servers_by_image_pass_overlimit_image(self):
-        """Return an error while listing server with too large image id"""
-        self.assertRaises(exceptions.OverLimit, self.client.list_servers,
-                          {'image': sys.maxint + 1})
-
-    @unittest.skip("Until Bug 1002911 is fixed")
-    def test_list_servers_by_image_pass_negative_id(self):
-        """Return an error while listing server with negative image id"""
-        self.assertRaises(exceptions.BadRequest, self.client.list_servers,
-                          {'image': -1})
-
-    def test_list_servers_by_existing_flavor(self):
-        """List servers by flavor"""
-        self.get_active_servers(1)
-
-        resp, body = self.client.list_servers({'flavor': self.flavor_ref})
-        self.assertEqual('200', resp['status'])
-        self.assertTrue(len(body['servers']) > 0)
-
-    @unittest.skip("Until Bug 1002918 is fixed")
     def test_list_servers_by_non_existing_flavor(self):
-        """Return an error while listing server by non existing flavor"""
-        self.assertRaises(exceptions.NotFound, self.client.list_servers,
-                          {'flavor': 1234})
-
-    @unittest.skip("Until Bug 1002918 is fixed")
-    def test_list_servers_by_flavor_pass_overlimit_flavor(self):
-        """Return an error while listing server with too large flavor value"""
-        self.assertRaises(exceptions.OverLimit, self.client.list_servers,
-                          {'flavor': sys.maxint + 1})
-
-    @unittest.skip("Until Bug 1002918 is fixed")
-    def test_list_servers_by_flavor_pass_negative_value(self):
-        """Return an error while listing server with negative flavor value"""
-        self.assertRaises(exceptions.BadRequest, self.client.list_servers,
-                          {'flavor': -1})
-
-    def test_list_servers_by_server_name(self):
-        """Returns a list of servers containing an existing server name"""
-        server_name = rand_name('test-vm-')
-        resp, server = self.client.create_server(server_name, self.image_ref,
-                                                 self.flavor_ref)
-        self.servers.append(server['id'])
-
-        resp, body = self.client.list_servers({'name': server_name})
+        """Listing servers by non existing flavor returns empty list"""
+        non_existing_flavor = 1234
+        resp, body = self.client.list_servers(dict(flavor=non_existing_flavor))
+        servers = body['servers']
         self.assertEqual('200', resp['status'])
-        self.assertEqual(server_name, body['servers'][0]['name'])
+        self.assertEqual([], servers)
 
-    @unittest.skip("Until Bug 1002892 is fixed")
     def test_list_servers_by_non_existing_server_name(self):
-        """Return an error while listing for a non existent server"""
-        resp, body = self.client.list_servers({'name': 'junk_serv_100'})
-        self.assertRaises(exceptions.NotFound, self.client.list_servers,
-                          {'name': 'junk_serv_100'})
-
-    @unittest.skip("Until Bug 1002892 is fixed")
-    def test_list_servers_by_server_name_empty(self):
-        """Return an error when an empty server name is passed"""
-        self.assertRaises(exceptions.BadRequest, self.client.list_servers,
-                          {'name': ''})
-
-    @unittest.skip("Until Bug 1002892 is fixed")
-    def test_list_servers_by_server_name_too_large(self):
-        """Return an error for a very large value for server name listing"""
-        self.assertRaises(exceptions.OverLimit, self.client.list_servers,
-                          {'name': 'a' * 65})
-
-    @unittest.skip("Until Bug 1002892 is fixed")
-    def test_list_servers_by_name_pass_numeric_name(self):
-        """Return an error for a numeric server name listing"""
-        self.assertRaises(exceptions.BadRequest, self.client.list_servers,
-                          {'name': 99})
-
-    def test_list_servers_by_active_status(self):
-        """Return a listing of servers by active status"""
-        self.create_server()
-        resp, body = self.client.list_servers({'status': 'ACTIVE'})
+        """Listing servers for a non existent server name returns empty list"""
+        non_existing_name = 'junk_server_1234'
+        resp, body = self.client.list_servers(dict(name=non_existing_name))
+        servers = body['servers']
         self.assertEqual('200', resp['status'])
-        self.assertTrue(len(body['servers']) > 0)
+        self.assertEqual([], servers)
 
-    def test_list_servers_by_building_status(self):
-        """Return a listing of servers in build state"""
-        server_name = rand_name('test-vm-')
-        resp, server = self.client.create_server(server_name, self.image_ref,
-                                                 self.flavor_ref)
-        self.servers.append(server['id'])
-        resp, body = self.client.list_servers({'status': 'BUILD'})
+    @unittest.skip("Skip until bug 1061712 is resolved")
+    def test_list_servers_status_non_existing(self):
+        """Return an empty list when invalid status is specified"""
+        non_existing_status = 'BALONEY'
+        resp, body = self.client.list_servers(dict(status=non_existing_status))
+        servers = body['servers']
         self.assertEqual('200', resp['status'])
-        self.assertEqual(1, len(body['servers']))
-        self.assertEqual(server['id'], body['servers'][0]['id'])
-
-        self.client.wait_for_server_status(server['id'], 'ACTIVE')
-
-    def test_list_servers_status_is_invalid(self):
-        """Return an error when invalid status is specified"""
-        self.assertRaises(exceptions.BadRequest, self.client.list_servers,
-                          {'status': 'DEAD'})
+        self.assertEqual([], servers)
 
     def test_list_servers_pass_numeric_status(self):
         """Return an error when a numeric value for status is specified"""
@@ -261,17 +147,15 @@
 
     def test_list_servers_by_limits(self):
         """List servers by specifying limits"""
-        self.get_active_servers(2)
         resp, body = self.client.list_servers({'limit': 1})
         self.assertEqual('200', resp['status'])
         self.assertEqual(1, len(body['servers']))
 
     def test_list_servers_by_limits_greater_than_actual_count(self):
         """List servers by specifying a greater value for limit"""
-        self.get_active_servers(2)
         resp, body = self.client.list_servers({'limit': 100})
         self.assertEqual('200', resp['status'])
-        self.assertTrue(len(body['servers']) >= 2)
+        self.assertEqual(len(self.existing_fixtures), len(body['servers']))
 
     def test_list_servers_by_limits_pass_string(self):
         """Return an error if a string value is passed for limit"""
@@ -283,120 +167,34 @@
         self.assertRaises(exceptions.BadRequest, self.client.list_servers,
                           {'limit': -1})
 
-    @unittest.skip("Until Bug 1002924 is fixed")
-    def test_list_servers_by_limits_pass_overlimit_value(self):
-        """Return an error if too large value for limit is passed"""
-        self.assertRaises(exceptions.OverLimit, self.client.list_servers,
-                          {'limit': sys.maxint + 1})
-
     def test_list_servers_by_changes_since(self):
         """Servers are listed by specifying changes-since date"""
-        self.get_active_servers(2)
         resp, body = self.client.list_servers(
                          {'changes-since': '2011-01-01T12:34:00Z'})
         self.assertEqual('200', resp['status'])
-        self.assertTrue(len(body['servers']) >= 2)
+        # changes-since returns all instances, including deleted.
+        num_expected = (len(self.existing_fixtures) +
+                        len(self.deleted_fixtures))
+        self.assertEqual(num_expected, len(body['servers']))
 
     def test_list_servers_by_changes_since_invalid_date(self):
         """Return an error when invalid date format is passed"""
         self.assertRaises(exceptions.BadRequest, self.client.list_servers,
                           {'changes-since': '2011/01/01'})
 
-    @unittest.skip("Until Bug 1002926 is fixed")
     def test_list_servers_by_changes_since_future_date(self):
-        """Return an error when a date in the future is passed"""
-        self.assertRaises(exceptions.BadRequest, self.client.list_servers,
-                          {'changes-since': '2051-01-01T12:34:00Z'})
-
-    @unittest.skip("Until Bug 1002935 is fixed")
-    def test_list_servers_list_another_tenant_servers(self):
-        """Return an error when a user lists servers in another tenant"""
-        # Create a server by a user in it's tenant
-        server_name = rand_name('test-vm-')
-        resp, server = self.client.create_server(server_name, self.image_ref,
-                                                 self.flavor_ref)
-        self.servers.append(server['id'])
-
-        # List the servers by alternate user in the base user's tenant
-        self.assertRaises(exceptions.NotFound, self.alt_client.list_servers,
-                          {'name': server_name})
-
-    def test_list_servers_detail_when_no_servers_running(self):
-        """Return an empty list for servers detail when no active servers"""
-        # Delete active servers
-        try:
-            resp, body = self.client.list_servers()
-            for server in body['servers']:
-                resp, body = self.client.delete_server(server['id'])
-                self.client.wait_for_server_termination(server['id'])
-        except exceptions.NotFound:
-            pass
-        # Verify empty list
-        resp, body = self.client.list_servers_with_detail()
+        """Return an empty list when a date in the future is passed"""
+        resp, body = self.client.list_servers(
+                         {'changes-since': '2051-01-01T12:34:00Z'})
         self.assertEqual('200', resp['status'])
-        servers = body['servers']
-        actual = [srv for srv in servers if srv['status'] == 'ACTIVE']
-        self.assertEqual([], actual)
-
-    def test_list_servers_detail_server_is_building(self):
-        """Server in Build state is listed"""
-        server_name = rand_name('test-vm-')
-        resp, server = self.client.create_server(server_name, self.image_ref,
-                                                 self.flavor_ref)
-
-        self.servers.append(server['id'])
-        resp, body = self.client.list_servers_with_detail()
-        self.assertEqual('200', resp['status'])
-        self.assertEqual('BUILD', body['servers'][0]['status'])
+        self.assertEqual(0, len(body['servers']))
 
     def test_list_servers_detail_server_is_deleted(self):
         """Server details are not listed for a deleted server"""
-        server = self.get_active_servers(1)[0]
-
-        self.client.delete_server(server['id'])
-        self.client.wait_for_server_termination(server['id'])
+        deleted_ids = [s['id'] for s in self.deleted_fixtures]
         resp, body = self.client.list_servers_with_detail()
         servers = body['servers']
-        actual = [srv for srv in servers if srv['id'] == server['id']]
+        actual = [srv for srv in servers
+                  if srv['id'] in deleted_ids]
         self.assertEqual('200', resp['status'])
         self.assertEqual([], actual)
-
-    def test_get_server_details_non_existent_id(self):
-        """Return an error during get server details using non-existent id"""
-        self.assertRaises(exceptions.NotFound, self.client.get_server,
-                          'junk-123ab-45cd')
-
-    def test_get_server_details_another_tenant_server(self):
-        """Return an error when querying details of server in another tenant"""
-        server_name = rand_name('test-vm-')
-        resp, server = self.client.create_server(server_name, self.image_ref,
-                                                 self.flavor_ref)
-        self.servers.append(server['id'])
-        self.assertRaises(exceptions.NotFound, self.alt_client.get_server,
-                          server['id'])
-
-    def test_get_server_details_pass_string_uuid(self):
-        """Return an error when a string value is passed for uuid"""
-        try:
-            self.assertRaises(exceptions.NotFound, self.client.get_server,
-                              'junk-server-uuid')
-        except Exception as e:
-            self.fail("Incorrect Exception raised: %s" % e)
-
-    @unittest.skip("Until Bug 1002901 is fixed")
-    def test_get_server_details_pass_negative_uuid(self):
-        """Return an error when a negative value is passed for uuid"""
-        try:
-            self.assertRaises(exceptions.BadRequest, self.client.get_server,
-                              -1)
-        except Exception as e:
-            self.fail("Incorrect Exception raised: %s" % e)
-
-    @unittest.skip("Until Bug 1002901 is fixed")
-    def test_get_server_details_pass_overlimit_length_uuid(self):
-        """Return an error when a very large value is passed for uuid"""
-        try:
-            self.assertRaises(exceptions.OverLimit, self.client.get_server,
-                              'a' * 37)
-        except Exception as e:
-            self.fail("Incorrect Exception raised: %s" % e)
diff --git a/tempest/tests/identity/admin/test_services.py b/tempest/tests/identity/admin/test_services.py
index b8b99f7..da697ab 100644
--- a/tempest/tests/identity/admin/test_services.py
+++ b/tempest/tests/identity/admin/test_services.py
@@ -15,8 +15,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from nose.plugins.attrib import attr
-import unittest2 as unittest
+import nose
 
 from tempest import exceptions
 from tempest.common.utils.data_utils import rand_name
@@ -77,3 +76,4 @@
     @classmethod
     def setUpClass(cls):
         super(ServicesTestXML, cls).setUpClass()
+        raise nose.SkipTest("Skipping until Bug #1061738 resolved")