Merge "Use skip_checks if all testcases have same skip conditions"
diff --git a/releasenotes/notes/add-return-value-to-retype-volume-a401aa619aaa2457.yaml b/releasenotes/notes/add-return-value-to-retype-volume-a401aa619aaa2457.yaml
index 4abfe9e..ca42014 100644
--- a/releasenotes/notes/add-return-value-to-retype-volume-a401aa619aaa2457.yaml
+++ b/releasenotes/notes/add-return-value-to-retype-volume-a401aa619aaa2457.yaml
@@ -1,5 +1,7 @@
 ---
 fixes:
-  Add a missing return statement to the retype_volume API in the v2 volumes_client library.
-  This changes the response body from None to an empty dictionary.
+  - |
+    Add a missing return statement to the retype_volume API in the v2
+    volumes_client library: Bug#1703997
 
+    This changes the response body from None to an empty dictionary.
diff --git a/requirements.txt b/requirements.txt
index aaecaaa..36b9efa 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -10,7 +10,7 @@
 testrepository>=0.0.18 # Apache-2.0/BSD
 oslo.concurrency>=3.8.0 # Apache-2.0
 oslo.config!=4.3.0,!=4.4.0,>=4.0.0 # Apache-2.0
-oslo.log>=3.22.0 # Apache-2.0
+oslo.log>=3.30.0 # Apache-2.0
 oslo.serialization!=2.19.1,>=1.10.0 # Apache-2.0
 oslo.utils>=3.20.0 # Apache-2.0
 six>=1.9.0 # MIT
diff --git a/tempest/api/compute/admin/test_auto_allocate_network.py b/tempest/api/compute/admin/test_auto_allocate_network.py
index ea92563..0c80252 100644
--- a/tempest/api/compute/admin/test_auto_allocate_network.py
+++ b/tempest/api/compute/admin/test_auto_allocate_network.py
@@ -16,7 +16,6 @@
 
 from tempest.api.compute import base
 from tempest.common import compute
-from tempest.common import credentials_factory as credentials
 from tempest.common import utils
 from tempest import config
 from tempest.lib.common.utils import test_utils
@@ -46,11 +45,6 @@
     @classmethod
     def skip_checks(cls):
         super(AutoAllocateNetworkTest, cls).skip_checks()
-        identity_version = cls.get_identity_version()
-        if not credentials.is_admin_available(
-                identity_version=identity_version):
-            msg = "Missing Identity Admin API credentials in configuration."
-            raise cls.skipException(msg)
         if not CONF.service_available.neutron:
             raise cls.skipException('Neutron is required')
         if not utils.is_extension_enabled('auto-allocated-topology',
diff --git a/tempest/api/compute/test_extensions.py b/tempest/api/compute/test_extensions.py
index cd5418f..34faf5f 100644
--- a/tempest/api/compute/test_extensions.py
+++ b/tempest/api/compute/test_extensions.py
@@ -26,7 +26,7 @@
 LOG = logging.getLogger(__name__)
 
 
-class ExtensionsTestJSON(base.BaseV2ComputeTest):
+class ExtensionsTest(base.BaseV2ComputeTest):
 
     @decorators.idempotent_id('3bb27738-b759-4e0d-a5fa-37d7a6df07d1')
     def test_list_extensions(self):
diff --git a/tempest/api/compute/volumes/test_attach_volume.py b/tempest/api/compute/volumes/test_attach_volume.py
index 502bc1b..e0fed58 100644
--- a/tempest/api/compute/volumes/test_attach_volume.py
+++ b/tempest/api/compute/volumes/test_attach_volume.py
@@ -13,8 +13,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import testtools
-
 from tempest.api.compute import base
 from tempest.common import compute
 from tempest.common.utils.linux import remote_client
@@ -143,6 +141,10 @@
             self.assertEqual(server['id'], body['serverId'])
             self.assertEqual(attachment['volumeId'], body['volumeId'])
             self.assertEqual(attachment['id'], body['id'])
+            self.servers_client.detach_volume(server['id'],
+                                              attachment['volumeId'])
+            waiters.wait_for_volume_resource_status(
+                self.volumes_client, attachment['volumeId'], 'available')
 
 
 class AttachVolumeShelveTestJSON(AttachVolumeTestJSON):
@@ -155,6 +157,12 @@
     min_microversion = '2.20'
     max_microversion = 'latest'
 
+    @classmethod
+    def skip_checks(cls):
+        super(AttachVolumeShelveTestJSON, cls).skip_checks()
+        if not CONF.compute_feature_enabled.shelve:
+            raise cls.skipException('Shelve is not available.')
+
     def _count_volumes(self, server):
         # Count number of volumes on an instance
         volumes = 0
@@ -202,8 +210,6 @@
             self.assertEqual(number_of_volumes, counted_volumes)
 
     @decorators.idempotent_id('13a940b6-3474-4c3c-b03f-29b89112bfee')
-    @testtools.skipUnless(CONF.compute_feature_enabled.shelve,
-                          'Shelve is not available.')
     def test_attach_volume_shelved_or_offload_server(self):
         # Create server, count number of volumes on it, shelve
         # server and attach pre-created volume to shelved server
@@ -229,8 +235,6 @@
         self.assertIsNotNone(volume_attachment['device'])
 
     @decorators.idempotent_id('b54e86dd-a070-49c4-9c07-59ae6dae15aa')
-    @testtools.skipUnless(CONF.compute_feature_enabled.shelve,
-                          'Shelve is not available.')
     def test_detach_volume_shelved_or_offload_server(self):
         # Count number of volumes on instance, shelve
         # server and attach pre-created volume to shelved server
diff --git a/tempest/api/compute/volumes/test_volume_snapshots.py b/tempest/api/compute/volumes/test_volume_snapshots.py
index 0f436eb..b8ca81d 100644
--- a/tempest/api/compute/volumes/test_volume_snapshots.py
+++ b/tempest/api/compute/volumes/test_volume_snapshots.py
@@ -13,8 +13,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import testtools
-
 from tempest.api.compute import base
 from tempest.common import waiters
 from tempest import config
@@ -38,6 +36,9 @@
         if not CONF.service_available.cinder:
             skip_msg = ("%s skipped as Cinder is not available" % cls.__name__)
             raise cls.skipException(skip_msg)
+        if not CONF.volume_feature_enabled.snapshot:
+            skip_msg = ("Cinder volume snapshots are disabled")
+            raise cls.skipException(skip_msg)
 
     @classmethod
     def setup_clients(cls):
@@ -46,8 +47,6 @@
         cls.snapshots_client = cls.snapshots_extensions_client
 
     @decorators.idempotent_id('cd4ec87d-7825-450d-8040-6e2068f2da8f')
-    @testtools.skipUnless(CONF.volume_feature_enabled.snapshot,
-                          'Cinder volume snapshots are disabled')
     def test_volume_snapshot_create_get_list_delete(self):
         volume = self.create_volume()
         self.addCleanup(self.delete_volume, volume['id'])
diff --git a/tempest/api/network/test_allowed_address_pair.py b/tempest/api/network/test_allowed_address_pair.py
index 37f5ec2..a471bd6 100644
--- a/tempest/api/network/test_allowed_address_pair.py
+++ b/tempest/api/network/test_allowed_address_pair.py
@@ -41,6 +41,8 @@
         api_extensions
     """
 
+    _project_network_cidr = CONF.network.project_network_cidr
+
     @classmethod
     def skip_checks(cls):
         super(AllowedAddressPairTestJSON, cls).skip_checks()
@@ -103,7 +105,7 @@
     @decorators.idempotent_id('4d6d178f-34f6-4bff-a01c-0a2f8fe909e4')
     def test_update_port_with_cidr_address_pair(self):
         # Update allowed address pair with cidr
-        cidr = str(netaddr.IPNetwork(CONF.network.project_network_cidr))
+        cidr = str(netaddr.IPNetwork(self._project_network_cidr))
         self._update_port_with_address(cidr)
 
     @decorators.idempotent_id('b3f20091-6cd5-472b-8487-3516137df933')
@@ -133,3 +135,4 @@
 
 class AllowedAddressPairIpV6TestJSON(AllowedAddressPairTestJSON):
     _ip_version = 6
+    _project_network_cidr = CONF.network.project_network_v6_cidr
diff --git a/tempest/api/network/test_networks.py b/tempest/api/network/test_networks.py
index 206d1a8..88340c1 100644
--- a/tempest/api/network/test_networks.py
+++ b/tempest/api/network/test_networks.py
@@ -374,22 +374,36 @@
     @testtools.skipUnless(CONF.network.public_network_id,
                           'The public_network_id option must be specified.')
     def test_external_network_visibility(self):
-        """Verifies user can see external networks but not subnets."""
+        public_network_id = CONF.network.public_network_id
+
+        # find external network matching public_network_id
         body = self.networks_client.list_networks(**{'router:external': True})
-        networks = [network['id'] for network in body['networks']]
-        self.assertNotEmpty(networks, "No external networks found")
+        external_network = next((network for network in body['networks']
+                                 if network['id'] == public_network_id), None)
+        self.assertIsNotNone(external_network, "Public network %s not found "
+                                               "in external network list"
+                             % public_network_id)
 
         nonexternal = [net for net in body['networks'] if
                        not net['router:external']]
         self.assertEmpty(nonexternal, "Found non-external networks"
                                       " in filtered list (%s)." % nonexternal)
-        self.assertIn(CONF.network.public_network_id, networks)
+
         # only check the public network ID because the other networks may
         # belong to other tests and their state may have changed during this
         # test
-        body = self.subnets_client.list_subnets(
-            network_id=CONF.network.public_network_id)
-        self.assertEmpty(body['subnets'], "Public subnets visible")
+        body = self.subnets_client.list_subnets(network_id=public_network_id)
+
+        # check subnet visibility of external_network
+        if external_network['shared']:
+            self.assertNotEmpty(body['subnets'], "Subnets should be visible "
+                                                 "for shared public network %s"
+                                % public_network_id)
+        else:
+            self.assertEmpty(body['subnets'], "Subnets should not be visible "
+                                              "for non-shared public "
+                                              "network %s"
+                             % public_network_id)
 
     @decorators.idempotent_id('c72c1c0c-2193-4aca-ccc4-b1442640bbbb')
     @utils.requires_ext(extension="standard-attr-description",
diff --git a/tempest/api/volume/admin/test_volumes_actions.py b/tempest/api/volume/admin/test_volumes_actions.py
index f99e03d..8d09217 100644
--- a/tempest/api/volume/admin/test_volumes_actions.py
+++ b/tempest/api/volume/admin/test_volumes_actions.py
@@ -30,6 +30,8 @@
         if status:
             self.admin_volume_client.reset_volume_status(
                 temp_volume['id'], status=status)
+            waiters.wait_for_volume_resource_status(
+                self.volumes_client, temp_volume['id'], status)
         self.admin_volume_client.force_delete_volume(temp_volume['id'])
         self.volumes_client.wait_for_resource_deletion(temp_volume['id'])
 
@@ -37,14 +39,15 @@
     def test_volume_reset_status(self):
         # test volume reset status : available->error->available
         volume = self.create_volume()
+        self.addCleanup(waiters.wait_for_volume_resource_status,
+                        self.volumes_client, volume['id'], 'available')
         self.addCleanup(self.admin_volume_client.reset_volume_status,
                         volume['id'], status='available')
         for status in ['error', 'available', 'maintenance']:
             self.admin_volume_client.reset_volume_status(
                 volume['id'], status=status)
-            volume_get = self.admin_volume_client.show_volume(
-                volume['id'])['volume']
-            self.assertEqual(status, volume_get['status'])
+            waiters.wait_for_volume_resource_status(
+                self.volumes_client, volume['id'], status)
 
     @decorators.idempotent_id('21737d5a-92f2-46d7-b009-a0cc0ee7a570')
     def test_volume_force_delete_when_volume_is_creating(self):
@@ -88,6 +91,8 @@
 
         # Reset volume's status to error
         self.admin_volume_client.reset_volume_status(volume_id, status='error')
+        waiters.wait_for_volume_resource_status(self.volumes_client,
+                                                volume_id, 'error')
 
         # Force detach volume
         self.admin_volume_client.force_detach_volume(
diff --git a/tempest/api/volume/test_volumes_actions.py b/tempest/api/volume/test_volumes_actions.py
index eea65f1..be5638e 100644
--- a/tempest/api/volume/test_volumes_actions.py
+++ b/tempest/api/volume/test_volumes_actions.py
@@ -112,6 +112,10 @@
         waiters.wait_for_volume_resource_status(self.volumes_client,
                                                 self.volume['id'], 'available')
 
+        image_info = self.images_client.show_image(image_id)
+        self.assertEqual(image_name, image_info['name'])
+        self.assertEqual(CONF.volume.disk_format, image_info['disk_format'])
+
     @decorators.idempotent_id('92c4ef64-51b2-40c0-9f7e-4749fbaaba33')
     def test_reserve_unreserve_volume(self):
         # Mark volume as reserved.
diff --git a/tempest/api/volume/test_volumes_list.py b/tempest/api/volume/test_volumes_list.py
index 8593d3a..b5f98ea 100644
--- a/tempest/api/volume/test_volumes_list.py
+++ b/tempest/api/volume/test_volumes_list.py
@@ -50,7 +50,7 @@
             return
 
         def str_vol(vol):
-            return "%s:%s" % (vol['id'], vol[self.name])
+            return "%s:%s" % (vol['id'], vol['name'])
 
         raw_msg = "Could not find volumes %s in expected list %s; fetched %s"
         self.fail(raw_msg % ([str_vol(v) for v in missing_vols],
@@ -60,7 +60,6 @@
     @classmethod
     def resource_setup(cls):
         super(VolumesListTestJSON, cls).resource_setup()
-        cls.name = cls.VOLUME_FIELDS[1]
 
         existing_volumes = cls.volumes_client.list_volumes()['volumes']
         cls.volume_id_list = [vol['id'] for vol in existing_volumes]
@@ -117,22 +116,20 @@
     @decorators.idempotent_id('a28e8da4-0b56-472f-87a8-0f4d3f819c02')
     def test_volume_list_by_name(self):
         volume = self.volume_list[data_utils.rand_int_id(0, 2)]
-        params = {self.name: volume[self.name]}
+        params = {'name': volume['name']}
         fetched_vol = self.volumes_client.list_volumes(
             params=params)['volumes']
         self.assertEqual(1, len(fetched_vol), str(fetched_vol))
-        self.assertEqual(fetched_vol[0][self.name],
-                         volume[self.name])
+        self.assertEqual(fetched_vol[0]['name'], volume['name'])
 
     @decorators.idempotent_id('2de3a6d4-12aa-403b-a8f2-fdeb42a89623')
     def test_volume_list_details_by_name(self):
         volume = self.volume_list[data_utils.rand_int_id(0, 2)]
-        params = {self.name: volume[self.name]}
+        params = {'name': volume['name']}
         fetched_vol = self.volumes_client.list_volumes(
             detail=True, params=params)['volumes']
         self.assertEqual(1, len(fetched_vol), str(fetched_vol))
-        self.assertEqual(fetched_vol[0][self.name],
-                         volume[self.name])
+        self.assertEqual(fetched_vol[0]['name'], volume['name'])
 
     @decorators.idempotent_id('39654e13-734c-4dab-95ce-7613bf8407ce')
     def test_volumes_list_by_status(self):
@@ -213,7 +210,7 @@
     def test_volume_list_param_display_name_and_status(self):
         # Test to list volume when display name and status param is given
         volume = self.volume_list[data_utils.rand_int_id(0, 2)]
-        params = {self.name: volume[self.name],
+        params = {'name': volume['name'],
                   'status': 'available'}
         self._list_by_param_value_and_assert(params)
 
@@ -221,7 +218,7 @@
     def test_volume_list_with_detail_param_display_name_and_status(self):
         # Test to list volume when name and status param is given
         volume = self.volume_list[data_utils.rand_int_id(0, 2)]
-        params = {self.name: volume[self.name],
+        params = {'name': volume['name'],
                   'status': 'available'}
         self._list_by_param_value_and_assert(params, with_detail=True)
 
diff --git a/tempest/common/compute.py b/tempest/common/compute.py
index 47196ec..df0f5a5 100644
--- a/tempest/common/compute.py
+++ b/tempest/common/compute.py
@@ -198,9 +198,27 @@
         body = rest_client.ResponseBody(body.response, body['server'])
         servers = [body]
 
-    # The name of the method to associate a floating IP to as server is too
-    # long for PEP8 compliance so:
-    assoc = clients.compute_floating_ips_client.associate_floating_ip_to_server
+    def _setup_validation_fip():
+        if CONF.service_available.neutron:
+            ifaces = clients.interfaces_client.list_interfaces(server['id'])
+            validation_port = None
+            for iface in ifaces['interfaceAttachments']:
+                if iface['net_id'] == tenant_network['id']:
+                    validation_port = iface['port_id']
+                    break
+            if not validation_port:
+                # NOTE(artom) This will get caught by the catch-all clause in
+                # the wait_until loop below
+                raise ValueError('Unable to setup floating IP for validation: '
+                                 'port not found on tenant network')
+            clients.floating_ips_client.update_floatingip(
+                validation_resources['floating_ip']['id'],
+                port_id=validation_port)
+        else:
+            fip_client = clients.compute_floating_ips_client
+            fip_client.associate_floating_ip_to_server(
+                floating_ip=validation_resources['floating_ip']['ip'],
+                server_id=servers[0]['id'])
 
     if wait_until:
         for server in servers:
@@ -212,9 +230,7 @@
                 # creation will fail with the condition above (l.58).
                 if CONF.validation.run_validation and validatable:
                     if CONF.validation.connect_method == 'floating':
-                        assoc(floating_ip=validation_resources[
-                              'floating_ip']['ip'],
-                              server_id=servers[0]['id'])
+                        _setup_validation_fip()
 
             except Exception:
                 with excutils.save_and_reraise_exception():
diff --git a/tempest/test.py b/tempest/test.py
index f390ae4..a4cc2cc 100644
--- a/tempest/test.py
+++ b/tempest/test.py
@@ -220,10 +220,13 @@
         resource_setup or at test level.
         """
         identity_version = cls.get_identity_version()
-        if 'admin' in cls.credentials and not credentials.is_admin_available(
-                identity_version=identity_version):
-            msg = "Missing Identity Admin API credentials in configuration."
-            raise cls.skipException(msg)
+        # setting force_tenant_isolation to True also needs admin credentials.
+        if ('admin' in cls.credentials or
+                getattr(cls, 'force_tenant_isolation', False)):
+            if not credentials.is_admin_available(
+                    identity_version=identity_version):
+                raise cls.skipException(
+                    "Missing Identity Admin API credentials in configuration.")
         if 'alt' in cls.credentials and not credentials.is_alt_available(
                 identity_version=identity_version):
             msg = "Missing a 2nd set of API credentials in configuration."
diff --git a/tempest/tests/cmd/test_run.py b/tempest/tests/cmd/test_run.py
index 7ac347d..6e1250f 100644
--- a/tempest/tests/cmd/test_run.py
+++ b/tempest/tests/cmd/test_run.py
@@ -13,6 +13,7 @@
 # under the License.
 
 import argparse
+import atexit
 import os
 import shutil
 import subprocess
@@ -25,6 +26,7 @@
 from tempest.tests import base
 
 DEVNULL = open(os.devnull, 'wb')
+atexit.register(DEVNULL.close)
 
 
 class TestTempestRun(base.TestCase):
@@ -68,6 +70,34 @@
         self.assertEqual('i_am_a_fun_little_regex',
                          self.run_cmd._build_regex(args))
 
+    def test__build_whitelist_file(self):
+        args = mock.Mock(spec=argparse.Namespace)
+        setattr(args, 'smoke', False)
+        setattr(args, 'regex', None)
+        self.tests = tempfile.NamedTemporaryFile(
+            prefix='whitelist', delete=False)
+        self.tests.write(b"volume \n compute")
+        self.tests.close()
+        setattr(args, 'whitelist_file', self.tests.name)
+        setattr(args, 'blacklist_file', None)
+        self.assertEqual("volume|compute",
+                         self.run_cmd._build_regex(args))
+        os.unlink(self.tests.name)
+
+    def test__build_blacklist_file(self):
+        args = mock.Mock(spec=argparse.Namespace)
+        setattr(args, 'smoke', False)
+        setattr(args, 'regex', None)
+        self.tests = tempfile.NamedTemporaryFile(
+            prefix='blacklist', delete=False)
+        self.tests.write(b"volume \n compute")
+        self.tests.close()
+        setattr(args, 'whitelist_file', None)
+        setattr(args, 'blacklist_file', self.tests.name)
+        self.assertEqual("^((?!compute|volume).)*$",
+                         self.run_cmd._build_regex(args))
+        os.unlink(self.tests.name)
+
 
 class TestRunReturnCode(base.TestCase):
     def setUp(self):