Merge "Revert "Make tempest plugin sanity job non voting""
diff --git a/releasenotes/notes/image-wait-multiple-79c55305b584b1ba.yaml b/releasenotes/notes/image-wait-multiple-79c55305b584b1ba.yaml
new file mode 100644
index 0000000..6f63ebd
--- /dev/null
+++ b/releasenotes/notes/image-wait-multiple-79c55305b584b1ba.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    The wait_for_image_status() waiter now allows a list of status values
+    instead of just a string, and returns the state the image was in when we
+    stopped waiting.
diff --git a/requirements.txt b/requirements.txt
index d2f13a5..a1eff53 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,6 +1,3 @@
-# The order of packages is significant, because pip processes them in the order
-# of appearance. Changing the order has an impact on the overall integration
-# process, which may cause wedges in the gate later.
 pbr!=2.1.0,>=2.0.0 # Apache-2.0
 cliff!=2.9.0,>=2.8.0 # Apache-2.0
 jsonschema>=3.2.0 # MIT
diff --git a/tempest/api/identity/admin/v3/test_groups.py b/tempest/api/identity/admin/v3/test_groups.py
index b5b3c5d..96218bb 100644
--- a/tempest/api/identity/admin/v3/test_groups.py
+++ b/tempest/api/identity/admin/v3/test_groups.py
@@ -128,7 +128,7 @@
         for g in user_groups:
             if 'membership_expires_at' in g:
                 self.assertIsNone(g['membership_expires_at'])
-                del(g['membership_expires_at'])
+                del g['membership_expires_at']
         self.assertEqual(sorted(groups, key=lambda k: k['name']),
                          sorted(user_groups, key=lambda k: k['name']))
         self.assertEqual(2, len(user_groups))
diff --git a/tempest/common/utils/linux/remote_client.py b/tempest/common/utils/linux/remote_client.py
index dd18190..79cc09c 100644
--- a/tempest/common/utils/linux/remote_client.py
+++ b/tempest/common/utils/linux/remote_client.py
@@ -59,14 +59,14 @@
         output = self.exec_command(command)
         selected = []
         pos = None
-        for l in output.splitlines():
-            if pos is None and l.find("TYPE") > 0:
-                pos = l.find("TYPE")
+        for line in output.splitlines():
+            if pos is None and line.find("TYPE") > 0:
+                pos = line.find("TYPE")
                 # Show header line too
-                selected.append(l)
+                selected.append(line)
             # lsblk lists disk type in a column right-aligned with TYPE
-            elif pos is not None and pos > 0 and l[pos:pos + 4] == "disk":
-                selected.append(l)
+            elif pos is not None and pos > 0 and line[pos:pos + 4] == "disk":
+                selected.append(line)
 
         if selected:
             return "\n".join(selected)
@@ -121,9 +121,9 @@
     def _get_dns_servers(self):
         cmd = 'cat /etc/resolv.conf'
         resolve_file = self.exec_command(cmd).strip().split('\n')
-        entries = (l.split() for l in resolve_file)
-        dns_servers = [l[1] for l in entries
-                       if len(l) and l[0] == 'nameserver']
+        entries = (line.split() for line in resolve_file)
+        dns_servers = [line[1] for line in entries
+                       if len(line) and line[0] == 'nameserver']
         return dns_servers
 
     def get_dns_servers(self, timeout=5):
diff --git a/tempest/common/utils/net_downtime.py b/tempest/common/utils/net_downtime.py
index 6f2a947..ec1a4c8 100644
--- a/tempest/common/utils/net_downtime.py
+++ b/tempest/common/utils/net_downtime.py
@@ -50,10 +50,10 @@
 
 
 class NetDowntimeMeter(fixtures.Fixture):
-    def __init__(self, dest_ip, interval='0.2'):
+    def __init__(self, dest_ip, interval=0.2):
         self.dest_ip = dest_ip
         # Note: for intervals lower than 0.2 ping requires root privileges
-        self.interval = interval
+        self.interval = float(interval)
         self.ping_process = None
 
     def _setUp(self):
@@ -61,18 +61,18 @@
 
     def start_background_pinger(self):
         cmd = ['ping', '-q', '-s1']
-        cmd.append('-i{}'.format(self.interval))
+        cmd.append('-i%g' % self.interval)
         cmd.append(self.dest_ip)
-        LOG.debug("Starting background pinger to '{}' with interval {}".format(
-            self.dest_ip, self.interval))
+        LOG.debug("Starting background pinger to '%s' with interval %g",
+                  self.dest_ip, self.interval)
         self.ping_process = subprocess.Popen(
             cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         self.addCleanup(self.cleanup)
 
     def cleanup(self):
         if self.ping_process and self.ping_process.poll() is None:
-            LOG.debug('Terminating background pinger with pid {}'.format(
-                self.ping_process.pid))
+            LOG.debug('Terminating background pinger with pid %d',
+                      self.ping_process.pid)
             self.ping_process.terminate()
         self.ping_process = None
 
@@ -83,7 +83,7 @@
         output = self.ping_process.stderr.readline().strip().decode('utf-8')
         if output and len(output.split()[0].split('/')) == 2:
             succ, total = output.split()[0].split('/')
-            return (int(total) - int(succ)) * float(self.interval)
+            return (int(total) - int(succ)) * self.interval
         else:
             LOG.warning('Unexpected output obtained from the pinger: %s',
                         output)
@@ -115,8 +115,9 @@
         chmod_cmd = 'chmod +x {}'.format(self.script_path)
         self.ssh_client.exec_command(';'.join((echo_cmd, chmod_cmd)))
         LOG.debug('script created: %s', self.script_path)
-        LOG.debug(self.ssh_client.exec_command(
-            'cat {}'.format(self.script_path)))
+        output = self.ssh_client.exec_command(
+            'cat {}'.format(self.script_path))
+        LOG.debug('script content: %s', output)
 
     def run_metadata_script(self):
         self.ssh_client.exec_command('{} > {} &'.format(self.script_path,
diff --git a/tempest/common/waiters.py b/tempest/common/waiters.py
index 9e97f47..b4312b7 100644
--- a/tempest/common/waiters.py
+++ b/tempest/common/waiters.py
@@ -154,13 +154,21 @@
 
 
 def wait_for_image_status(client, image_id, status):
-    """Waits for an image to reach a given status.
+    """Waits for an image to reach a given status (or list of them).
 
     The client should have a show_image(image_id) method to get the image.
     The client should also have build_interval and build_timeout attributes.
+
+    status can be either a string or a list of strings that constitute a
+    terminal state that we will return.
     """
     show_image = client.show_image
 
+    if isinstance(status, str):
+        terminal_status = [status]
+    else:
+        terminal_status = status
+
     current_status = 'An unknown status'
     start = int(time.time())
     while int(time.time()) - start < client.build_timeout:
@@ -171,8 +179,8 @@
             image = image['image']
 
         current_status = image['status']
-        if current_status == status:
-            return
+        if current_status in terminal_status:
+            return current_status
         if current_status.lower() == 'killed':
             raise exceptions.ImageKilledException(image_id=image_id,
                                                   status=status)
@@ -184,7 +192,7 @@
     message = ('Image %(image_id)s failed to reach %(status)s state '
                '(current state %(current_status)s) within the required '
                'time (%(timeout)s s).' % {'image_id': image_id,
-                                          'status': status,
+                                          'status': ','.join(terminal_status),
                                           'current_status': current_status,
                                           'timeout': client.build_timeout})
     caller = test_utils.find_test_caller()
@@ -327,8 +335,7 @@
     # Check if image have last store location
     if len(available_stores) == 1:
         exc_cls = lib_exc.OtherRestClientException
-        message = ('Delete from last store location not allowed'
-                   % (image, image_store_deleted))
+        message = 'Delete from last store location not allowed'
         raise exc_cls(message)
     start = int(time.time())
     while int(time.time()) - start < client.build_timeout:
@@ -548,7 +555,7 @@
     interface_status = body['port_state']
     start = int(time.time())
 
-    while(interface_status != status):
+    while interface_status != status:
         time.sleep(client.build_interval)
         body = (client.show_interface(server_id, port_id)
                 ['interfaceAttachment'])
diff --git a/tempest/hacking/checks.py b/tempest/hacking/checks.py
index 1c9c55b..c81ec03 100644
--- a/tempest/hacking/checks.py
+++ b/tempest/hacking/checks.py
@@ -16,7 +16,6 @@
 import re
 
 from hacking import core
-import pycodestyle
 
 
 PYTHON_CLIENTS = ['cinder', 'glance', 'keystone', 'nova', 'swift', 'neutron',
@@ -40,22 +39,22 @@
 
 
 @core.flake8ext
-def import_no_clients_in_api_and_scenario_tests(physical_line, filename):
+def import_no_clients_in_api_and_scenario_tests(logical_line, filename):
     """Check for client imports from tempest/api & tempest/scenario tests
 
     T102: Cannot import OpenStack python clients
     """
 
     if "tempest/api" in filename or "tempest/scenario" in filename:
-        res = PYTHON_CLIENT_RE.match(physical_line)
+        res = PYTHON_CLIENT_RE.match(logical_line)
         if res:
-            return (physical_line.find(res.group(1)),
+            return (logical_line.find(res.group(1)),
                     ("T102: python clients import not allowed"
                      " in tempest/api/* or tempest/scenario/* tests"))
 
 
 @core.flake8ext
-def scenario_tests_need_service_tags(physical_line, filename,
+def scenario_tests_need_service_tags(logical_line, filename,
                                      previous_logical):
     """Check that scenario tests have service tags
 
@@ -63,28 +62,28 @@
     """
 
     if 'tempest/scenario/' in filename and '/test_' in filename:
-        if TEST_DEFINITION.match(physical_line):
+        if TEST_DEFINITION.match(logical_line):
             if not SCENARIO_DECORATOR.match(previous_logical):
-                return (physical_line.find('def'),
+                return (logical_line.find('def'),
                         "T104: Scenario tests require a service decorator")
 
 
 @core.flake8ext
-def no_setup_teardown_class_for_tests(physical_line, filename):
+def no_setup_teardown_class_for_tests(logical_line, filename, noqa):
 
-    if pycodestyle.noqa(physical_line):
+    if noqa:
         return
 
     if 'tempest/test.py' in filename or 'tempest/lib/' in filename:
         return
 
-    if SETUP_TEARDOWN_CLASS_DEFINITION.match(physical_line):
-        return (physical_line.find('def'),
+    if SETUP_TEARDOWN_CLASS_DEFINITION.match(logical_line):
+        return (logical_line.find('def'),
                 "T105: (setUp|tearDown)Class can not be used in tests")
 
 
 @core.flake8ext
-def service_tags_not_in_module_path(physical_line, filename):
+def service_tags_not_in_module_path(logical_line, filename):
     """Check that a service tag isn't in the module path
 
     A service tag should only be added if the service name isn't already in
@@ -96,14 +95,14 @@
     # created for services like heat which would cause false negatives for
     # those tests, so just exclude the scenario tests.
     if 'tempest/scenario' not in filename:
-        matches = SCENARIO_DECORATOR.match(physical_line)
+        matches = SCENARIO_DECORATOR.match(logical_line)
         if matches:
             services = matches.group(1).split(',')
             for service in services:
                 service_name = service.strip().strip("'")
                 modulepath = os.path.split(filename)[0]
                 if service_name in modulepath:
-                    return (physical_line.find(service_name),
+                    return (logical_line.find(service_name),
                             "T107: service tag should not be in path")
 
 
@@ -140,28 +139,27 @@
                "decorators.skip_because from tempest.lib")
 
 
-def _common_service_clients_check(logical_line, physical_line, filename):
+def _common_service_clients_check(logical_line, filename, noqa):
+    if noqa:
+        return False
+
     if not re.match('tempest/(lib/)?services/.*', filename):
         return False
 
-    if not METHOD.match(physical_line):
-        return False
-
-    if pycodestyle.noqa(physical_line):
+    if not METHOD.match(logical_line):
         return False
 
     return True
 
 
 @core.flake8ext
-def get_resources_on_service_clients(physical_line, logical_line, filename,
-                                     line_number, lines):
+def get_resources_on_service_clients(logical_line, filename,
+                                     line_number, lines, noqa):
     """Check that service client names of GET should be consistent
 
     T110
     """
-    if not _common_service_clients_check(logical_line, physical_line,
-                                         filename):
+    if not _common_service_clients_check(logical_line, filename, noqa):
         return
 
     for line in lines[line_number:]:
@@ -182,14 +180,13 @@
 
 
 @core.flake8ext
-def delete_resources_on_service_clients(physical_line, logical_line, filename,
-                                        line_number, lines):
+def delete_resources_on_service_clients(logical_line, filename,
+                                        line_number, lines, noqa):
     """Check that service client names of DELETE should be consistent
 
     T111
     """
-    if not _common_service_clients_check(logical_line, physical_line,
-                                         filename):
+    if not _common_service_clients_check(logical_line, filename, noqa):
         return
 
     for line in lines[line_number:]:
@@ -262,7 +259,7 @@
             'oslo_config' in logical_line):
         msg = ('T114: tempest.lib can not have any dependency on tempest '
                'config.')
-        yield(0, msg)
+        yield (0, msg)
 
 
 @core.flake8ext
@@ -281,7 +278,7 @@
 
     if not re.match(r'.\/tempest\/api\/.*\/admin\/.*', filename):
         msg = 'T115: All admin tests should exist under admin path.'
-        yield(0, msg)
+        yield (0, msg)
 
 
 @core.flake8ext
@@ -293,11 +290,11 @@
     result = EX_ATTRIBUTE.search(logical_line)
     msg = ("[T116] Unsupported 'message' Exception attribute in PY3")
     if result:
-        yield(0, msg)
+        yield (0, msg)
 
 
 @core.flake8ext
-def negative_test_attribute_always_applied_to_negative_tests(physical_line,
+def negative_test_attribute_always_applied_to_negative_tests(logical_line,
                                                              filename):
     """Check ``@decorators.attr(type=['negative'])`` applied to negative tests.
 
@@ -307,13 +304,13 @@
 
     if re.match(r'.\/tempest\/api\/.*_negative.*', filename):
 
-        if NEGATIVE_TEST_DECORATOR.match(physical_line):
+        if NEGATIVE_TEST_DECORATOR.match(logical_line):
             _HAVE_NEGATIVE_DECORATOR = True
             return
 
-        if TEST_DEFINITION.match(physical_line):
+        if TEST_DEFINITION.match(logical_line):
             if not _HAVE_NEGATIVE_DECORATOR:
-                return (
+                yield (
                     0, "T117: Must apply `@decorators.attr(type=['negative'])`"
                        " to all negative API tests"
                 )
diff --git a/tempest/lib/common/http.py b/tempest/lib/common/http.py
index d163968..5bdcecd 100644
--- a/tempest/lib/common/http.py
+++ b/tempest/lib/common/http.py
@@ -60,6 +60,14 @@
             retry = urllib3.util.Retry(redirect=False)
         r = super(ClosingProxyHttp, self).request(method, url, retries=retry,
                                                   *args, **new_kwargs)
+
+        # Clearing the pool is necessary to free memory that holds certificates
+        # loaded by the HTTPConnection class in urllib3. This line can be
+        # removed once we require a newer version of urllib3 (e.g., 2.2.3) that
+        # does not retain certificates in memory for each HTTPConnection
+        # managed by the PoolManager.
+        self.clear()
+
         if not kwargs.get('preload_content', True):
             # This means we asked urllib3 for streaming content, so we
             # need to return the raw response and not read any data yet
@@ -114,6 +122,14 @@
             retry = urllib3.util.Retry(redirect=False)
         r = super(ClosingHttp, self).request(method, url, retries=retry,
                                              *args, **new_kwargs)
+
+        # Clearing the pool is necessary to free memory that holds certificates
+        # loaded by the HTTPConnection class in urllib3. This line can be
+        # removed once we require a newer version of urllib3 (e.g., 2.2.3) that
+        # does not retain certificates in memory for each HTTPConnection
+        # managed by the PoolManager.
+        self.clear()
+
         if not kwargs.get('preload_content', True):
             # This means we asked urllib3 for streaming content, so we
             # need to return the raw response and not read any data yet
diff --git a/tempest/scenario/manager.py b/tempest/scenario/manager.py
index 01c42c8..369efcc 100644
--- a/tempest/scenario/manager.py
+++ b/tempest/scenario/manager.py
@@ -851,7 +851,7 @@
                     kernel_img_path = os.path.join(extract_dir, fname)
                 elif re.search(r'(.*-initrd.*|ari-.*/image$)', fname):
                     ramdisk_img_path = os.path.join(extract_dir, fname)
-                elif re.search(f'(.*\\.img$|ami-.*/image$)', fname):
+                elif re.search(r'(.*\\.img$|ami-.*/image$)', fname):
                     img_path = os.path.join(extract_dir, fname)
             # Create the kernel image.
             kparams = {
@@ -1561,8 +1561,8 @@
             floating_ip = (self.floating_ips_client.
                            show_floatingip(floatingip_id)['floatingip'])
             if status == floating_ip['status']:
-                LOG.info("FloatingIP: {fp} is at status: {st}"
-                         .format(fp=floating_ip, st=status))
+                LOG.info("FloatingIP: %s is at status: %s",
+                         floating_ip, status)
             return status == floating_ip['status']
 
         if not test_utils.call_until_true(refresh,
diff --git a/tempest/tests/test_hacking.py b/tempest/tests/test_hacking.py
index 464e66a..3f603e8 100644
--- a/tempest/tests/test_hacking.py
+++ b/tempest/tests/test_hacking.py
@@ -51,25 +51,34 @@
 
     def test_no_setup_teardown_class_for_tests(self):
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def setUpClass(cls):", './tempest/tests/fake_test.py'))
+            "  def setUpClass(cls):", './tempest/tests/fake_test.py', False))
         self.assertIsNone(checks.no_setup_teardown_class_for_tests(
-            "  def setUpClass(cls): # noqa", './tempest/tests/fake_test.py'))
+            "  def setUpClass(cls):", './tempest/tests/fake_test.py',
+            True))
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def setUpClass(cls):", './tempest/api/fake_test.py'))
+            "  def setUpClass(cls):", './tempest/api/fake_test.py',
+            False))
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def setUpClass(cls):", './tempest/scenario/fake_test.py'))
+            "  def setUpClass(cls):", './tempest/scenario/fake_test.py',
+            False))
         self.assertFalse(checks.no_setup_teardown_class_for_tests(
-            "  def setUpClass(cls):", './tempest/test.py'))
+            "  def setUpClass(cls):", './tempest/test.py',
+            False))
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def tearDownClass(cls):", './tempest/tests/fake_test.py'))
+            "  def tearDownClass(cls):", './tempest/tests/fake_test.py',
+            False))
         self.assertIsNone(checks.no_setup_teardown_class_for_tests(
-            "  def tearDownClass(cls): # noqa", './tempest/tests/fake_test.py'))
+            "  def tearDownClass(cls):", './tempest/tests/fake_test.py',
+            True))
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def tearDownClass(cls):", './tempest/api/fake_test.py'))
+            "  def tearDownClass(cls):", './tempest/api/fake_test.py',
+            False))
         self.assertTrue(checks.no_setup_teardown_class_for_tests(
-            "  def tearDownClass(cls):", './tempest/scenario/fake_test.py'))
+            "  def tearDownClass(cls):", './tempest/scenario/fake_test.py',
+            False))
         self.assertFalse(checks.no_setup_teardown_class_for_tests(
-            "  def tearDownClass(cls):", './tempest/test.py'))
+            "  def tearDownClass(cls):", './tempest/test.py',
+            False))
 
     def test_import_no_clients_in_api_and_scenario_tests(self):
         for client in checks.PYTHON_CLIENTS:
@@ -198,22 +207,26 @@
             # arbitrarily many decorators. These insert decorators above the
             # @decorators.attr(type=['negative']) decorator.
             for decorator in other_decorators:
-                self.assertIsNone(check(" %s" % decorator, filename))
+                self.assertFalse(
+                    list(check(" %s" % decorator, filename)))
         if with_negative_decorator:
-            self.assertIsNone(
-                check("@decorators.attr(type=['negative'])", filename))
+            self.assertFalse(
+                list(check("@decorators.attr(type=['negative'])", filename)))
         if with_other_decorators:
             # Include multiple decorators to verify that this check works with
             # arbitrarily many decorators. These insert decorators between
             # the test and the @decorators.attr(type=['negative']) decorator.
             for decorator in other_decorators:
-                self.assertIsNone(check(" %s" % decorator, filename))
-        final_result = check(" def test_some_negative_case", filename)
+                self.assertFalse(
+                    list(check(" %s" % decorator, filename)))
+        final_result = list(check(" def test_some_negative_case", filename))
         if expected_success:
-            self.assertIsNone(final_result)
+            self.assertFalse(final_result)
         else:
-            self.assertIsInstance(final_result, tuple)
-            self.assertFalse(final_result[0])
+            self.assertEqual(1, len(final_result))
+            self.assertIsInstance(final_result[0], tuple)
+            self.assertEqual(0, final_result[0][0])
+            self.assertTrue(final_result[0][1])
 
     def test_no_negatve_test_attribute_applied_to_negative_test(self):
         # Check negative filename, negative decorator passes
diff --git a/tempest/tests/test_test.py b/tempest/tests/test_test.py
index 80825a4..7fb9bb3 100644
--- a/tempest/tests/test_test.py
+++ b/tempest/tests/test_test.py
@@ -303,7 +303,7 @@
         # [0]: test, err, details [1] -> exc_info
         # Type, Exception, traceback [1] -> MultipleException
         found_exc = log[0][1][1]
-        self.assertTrue(isinstance(found_exc, testtools.MultipleExceptions))
+        self.assertIsInstance(found_exc, testtools.MultipleExceptions)
         self.assertEqual(2, len(found_exc.args))
         # Each arg is exc_info - match messages and order
         self.assertIn('mock3 resource', str(found_exc.args[0][1]))
@@ -332,7 +332,7 @@
         # [0]: test, err, details [1] -> exc_info
         # Type, Exception, traceback [1] -> RuntimeError
         found_exc = log[0][1][1]
-        self.assertTrue(isinstance(found_exc, RuntimeError))
+        self.assertIsInstance(found_exc, RuntimeError)
         self.assertIn(BadResourceCleanup.__name__, str(found_exc))
 
     def test_super_skip_checks_not_invoked(self):
diff --git a/test-requirements.txt b/test-requirements.txt
index 17fa9f1..bd4d772 100644
--- a/test-requirements.txt
+++ b/test-requirements.txt
@@ -1,8 +1,4 @@
-# The order of packages is significant, because pip processes them in the order
-# of appearance. Changing the order has an impact on the overall integration
-# process, which may cause wedges in the gate later.
-hacking>=3.0.1,<3.1.0;python_version>='3.5' # Apache-2.0
+hacking>=6.1.0,<6.2.0
 coverage!=4.4,>=4.0 # Apache-2.0
 oslotest>=3.2.0 # Apache-2.0
-pycodestyle>=2.0.0,<2.6.0 # MIT
-flake8-import-order==0.11 # LGPLv3
+flake8-import-order>=0.18.0,<0.19.0 # LGPLv3
diff --git a/tox.ini b/tox.ini
index e3c8fcf..d9d2bad 100644
--- a/tox.ini
+++ b/tox.ini
@@ -411,7 +411,8 @@
 # E129 skipped because it is too limiting when combined with other rules
 # W504 skipped because it is overeager and unnecessary
 # H405 skipped because it arbitrarily forces doctring "title" lines
-ignore = E125,E123,E129,W504,H405
+# I201 and I202 skipped because the rule does not allow new line between 3rd party modules and own modules
+ignore = E125,E123,E129,W504,H405,I201,I202,T117
 show-source = True
 exclude = .git,.venv,.tox,dist,doc,*egg,build
 enable-extensions = H106,H203,H904