Introduce @serial test execution decorator

Tempest provides a LockFixture to avoid two potentially interfering
tests to run in parallel. However, this solution does not scale when
we want to separate a set of tests from many other test cases. For
example, host aggregate and availability zone testing needs compute
hosts without any nova servers to be able to test moving computes
between aggregates but a lot of other tests are creating nova
servers. To fully separate these aggregate tests from the rest of
the tempest test cases, this patch proposes a @serial class decorator
to mark a test class to be run totally independently of any other test
classes.

Under the hood, the @serial decorator is implemented with a tempest-wide
interprocess read-write lock. The serial test classes always take the
write lock, while the non-serial classes take the read lock. The lock
allows in many readers OR a single writer. So the serial tests are run
independently from the rest.

To minimize the time a serial test blocks other tempest tests run in
parallel, this patch also introduced a serial_tests test directory to
store the serial tests. The current test ordering in a fresh env
uses alphabetical order so the serial tests will run at the end of
the execution not randomly in the middle. The gate uses fresh VMs
for every run so we can rely on this optimization there. In local
envs where tests are re-run, the subsequent runs will be ordered at
runtime by stestr. Therfore, a longer runtime might be observed due to
locking, but the correctness of the test execution is still kept.

Related-Bug: #821732
Change-Id: I0181517edab75f586464a38c4811417f888783b1
diff --git a/HACKING.rst b/HACKING.rst
index dc28e4e..17e2a49 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -322,7 +322,14 @@
 
 - If the execution of a set of tests is required to be serialized then locking
   can be used to perform this. See usage of ``LockFixture`` for examples of
-  using locking.
+  using locking. However, LockFixture only helps if you want to separate the
+  execution of two small sets of test cases. On the other hand, if you need to
+  run a set of tests separately from potentially all other tests then
+  ``LockFixture`` does not scale as you would need to take the lock in all the
+  other tests too. In this case, you can use the ``@serial`` decorator on top
+  of the test class holding the tests that need to run separately from the
+  potentially parallel test set. See more in :ref:`tempest_test_writing`.
+
 
 Sample Configuration File
 -------------------------
diff --git a/doc/source/write_tests.rst b/doc/source/write_tests.rst
index 34df089..3626a3f 100644
--- a/doc/source/write_tests.rst
+++ b/doc/source/write_tests.rst
@@ -256,6 +256,33 @@
 worth checking the immediate parent for what is set to determine if your
 class needs to override that setting.
 
+Running some tests in serial
+----------------------------
+Tempest potentially runs test cases in parallel, depending on the configuration.
+However, sometimes you need to make sure that tests are not interfering with
+each other via OpenStack resources. Tempest creates separate projects for each
+test class to separate project based resources between test cases.
+
+If your tests use resources outside of projects, e.g. host aggregates then
+you might need to explicitly separate interfering test cases. If you only need
+to separate a small set of testcases from each other then you can use the
+``LockFixture``.
+
+However, in some cases a small set of tests needs to be run independently from
+the rest of the test cases. For example, some of the host aggregate and
+availability zone testing needs compute nodes without any running nova server
+to be able to move compute hosts between availability zones. But many tempest
+tests start one or more nova servers. In this scenario you can mark the small
+set of tests that needs to be independent from the rest with the ``@serial``
+class decorator. This will make sure that even if tempest is configured to run
+the tests in parallel the tests in the marked test class will always be executed
+separately from the rest of the test cases.
+
+Please note that due to test ordering optimization reasons test cases marked
+for ``@serial`` execution need to be put under ``tempest/serial_tests``
+directory. This will ensure that the serial tests will block the parallel tests
+in the least amount of time.
+
 Interacting with Credentials and Clients
 ========================================
 
diff --git a/requirements.txt b/requirements.txt
index a118856..6e66046 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -22,3 +22,4 @@
 urllib3>=1.21.1 # MIT
 debtcollector>=1.2.0 # Apache-2.0
 defusedxml>=0.7.1 # PSFL
+fasteners>=0.16.0 # Apache-2.0
diff --git a/tempest/lib/decorators.py b/tempest/lib/decorators.py
index bb588be..7d54c1a 100644
--- a/tempest/lib/decorators.py
+++ b/tempest/lib/decorators.py
@@ -221,3 +221,10 @@
             # class is the caller
             owner.cleanup = owner.addClassResourceCleanup
             return MethodType(self.func, owner)
+
+
+def serial(cls):
+    """A decorator to mark a test class for serial execution"""
+    cls._serial = True
+    LOG.debug('marked %s for serial execution', cls.__name__)
+    return cls
diff --git a/tempest/serial_tests/__init__.py b/tempest/serial_tests/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tempest/serial_tests/__init__.py
diff --git a/tempest/serial_tests/api/__init__.py b/tempest/serial_tests/api/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tempest/serial_tests/api/__init__.py
diff --git a/tempest/serial_tests/api/admin/__init__.py b/tempest/serial_tests/api/admin/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tempest/serial_tests/api/admin/__init__.py
diff --git a/tempest/api/compute/admin/test_aggregates.py b/tempest/serial_tests/api/admin/test_aggregates.py
similarity index 99%
rename from tempest/api/compute/admin/test_aggregates.py
rename to tempest/serial_tests/api/admin/test_aggregates.py
index a6c6535..2ca91aa 100644
--- a/tempest/api/compute/admin/test_aggregates.py
+++ b/tempest/serial_tests/api/admin/test_aggregates.py
@@ -26,6 +26,7 @@
 CONF = config.CONF
 
 
+@decorators.serial
 class AggregatesAdminTestBase(base.BaseV2ComputeAdminTest):
     """Tests Aggregates API that require admin privileges"""
 
diff --git a/tempest/serial_tests/scenario/__init__.py b/tempest/serial_tests/scenario/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tempest/serial_tests/scenario/__init__.py
diff --git a/tempest/scenario/test_aggregates_basic_ops.py b/tempest/serial_tests/scenario/test_aggregates_basic_ops.py
similarity index 99%
rename from tempest/scenario/test_aggregates_basic_ops.py
rename to tempest/serial_tests/scenario/test_aggregates_basic_ops.py
index 58e234f..ba31d84 100644
--- a/tempest/scenario/test_aggregates_basic_ops.py
+++ b/tempest/serial_tests/scenario/test_aggregates_basic_ops.py
@@ -20,6 +20,7 @@
 from tempest.scenario import manager
 
 
+@decorators.serial
 class TestAggregatesBasicOps(manager.ScenarioTest):
     """Creates an aggregate within an availability zone
 
diff --git a/tempest/test.py b/tempest/test.py
index dba2695..d49458e 100644
--- a/tempest/test.py
+++ b/tempest/test.py
@@ -18,7 +18,9 @@
 import sys
 
 import debtcollector.moves
+from fasteners import process_lock
 import fixtures
+from oslo_concurrency import lockutils
 from oslo_log import log as logging
 import testtools
 
@@ -123,6 +125,23 @@
     # A way to adjust slow test classes
     TIMEOUT_SCALING_FACTOR = 1
 
+    # An interprocess lock to implement serial test execution if requested.
+    # The serial test classes are the writers as only one of them can be
+    # executed. The rest of the test classes are the readers as many of them
+    # can be run in parallel.
+    # Only classes can be decorated with @serial decorator not individual test
+    # cases as tempest allows test class level resource setup which could
+    # interfere with serialized execution on test cases level. I.e. the class
+    # setup of one of the test cases could run before taking a test case level
+    # lock.
+    # We cannot init the lock here as external lock needs oslo configuration
+    # to be loaded first to get the lock_path
+    serial_rw_lock = None
+
+    # Defines if the tests in this class should be run without any parallelism
+    # Use the @serial decorator on your test class to indicate such requirement
+    _serial = False
+
     @classmethod
     def _reset_class(cls):
         cls.__setup_credentials_called = False
@@ -134,14 +153,33 @@
         cls._teardowns = []
 
     @classmethod
+    def is_serial_execution_requested(cls):
+        return cls._serial
+
+    @classmethod
     def setUpClass(cls):
         cls.__setupclass_called = True
+
+        if cls.serial_rw_lock is None:
+            path = os.path.join(
+                lockutils.get_lock_path(CONF), 'tempest-serial-rw-lock')
+            cls.serial_rw_lock = (
+                process_lock.InterProcessReaderWriterLock(path)
+            )
+
         # Reset state
         cls._reset_class()
         # It should never be overridden by descendants
         if hasattr(super(BaseTestCase, cls), 'setUpClass'):
             super(BaseTestCase, cls).setUpClass()
         try:
+            if cls.is_serial_execution_requested():
+                LOG.debug('%s taking the write lock', cls.__name__)
+                cls.serial_rw_lock.acquire_write_lock()
+                LOG.debug('%s took the write lock', cls.__name__)
+            else:
+                cls.serial_rw_lock.acquire_read_lock()
+
             cls.skip_checks()
 
             if not cls.__skip_checks_called:
@@ -184,35 +222,44 @@
         # If there was no exception during setup we shall re-raise the first
         # exception in teardown
         re_raise = (etype is None)
-        while cls._teardowns:
-            name, teardown = cls._teardowns.pop()
-            # Catch any exception in tearDown so we can re-raise the original
-            # exception at the end
-            try:
-                teardown()
-                if name == 'resources':
-                    if not cls.__resource_cleanup_called:
-                        raise RuntimeError(
-                            "resource_cleanup for %s did not call the "
-                            "super's resource_cleanup" % cls.__name__)
-            except Exception as te:
-                sys_exec_info = sys.exc_info()
-                tetype = sys_exec_info[0]
-                # TODO(andreaf): Resource cleanup is often implemented by
-                # storing an array of resources at class level, and cleaning
-                # them up during `resource_cleanup`.
-                # In case of failure during setup, some resource arrays might
-                # not be defined at all, in which case the cleanup code might
-                # trigger an AttributeError. In such cases we log
-                # AttributeError as info instead of exception. Once all
-                # cleanups are migrated to addClassResourceCleanup we can
-                # remove this.
-                if tetype is AttributeError and name == 'resources':
-                    LOG.info("tearDownClass of %s failed: %s", name, te)
-                else:
-                    LOG.exception("teardown of %s failed: %s", name, te)
-                if not etype:
-                    etype, value, trace = sys_exec_info
+        try:
+            while cls._teardowns:
+                name, teardown = cls._teardowns.pop()
+                # Catch any exception in tearDown so we can re-raise the
+                # original exception at the end
+                try:
+                    teardown()
+                    if name == 'resources':
+                        if not cls.__resource_cleanup_called:
+                            raise RuntimeError(
+                                "resource_cleanup for %s did not call the "
+                                "super's resource_cleanup" % cls.__name__)
+                except Exception as te:
+                    sys_exec_info = sys.exc_info()
+                    tetype = sys_exec_info[0]
+                    # TODO(andreaf): Resource cleanup is often implemented by
+                    # storing an array of resources at class level, and
+                    # cleaning them up during `resource_cleanup`.
+                    # In case of failure during setup, some resource arrays
+                    # might not be defined at all, in which case the cleanup
+                    # code might trigger an AttributeError. In such cases we
+                    # log AttributeError as info instead of exception. Once all
+                    # cleanups are migrated to addClassResourceCleanup we can
+                    # remove this.
+                    if tetype is AttributeError and name == 'resources':
+                        LOG.info("tearDownClass of %s failed: %s", name, te)
+                    else:
+                        LOG.exception("teardown of %s failed: %s", name, te)
+                    if not etype:
+                        etype, value, trace = sys_exec_info
+        finally:
+            if cls.is_serial_execution_requested():
+                LOG.debug('%s releasing the write lock', cls.__name__)
+                cls.serial_rw_lock.release_write_lock()
+                LOG.debug('%s released the write lock', cls.__name__)
+            else:
+                cls.serial_rw_lock.release_read_lock()
+
         # If exceptions were raised during teardown, and not before, re-raise
         # the first one
         if re_raise and etype is not None:
diff --git a/tempest/test_discover/test_discover.py b/tempest/test_discover/test_discover.py
index a19f20b..679d58b 100644
--- a/tempest/test_discover/test_discover.py
+++ b/tempest/test_discover/test_discover.py
@@ -25,7 +25,7 @@
     base_path = os.path.split(os.path.dirname(os.path.abspath(__file__)))[0]
     base_path = os.path.split(base_path)[0]
     # Load local tempest tests
-    for test_dir in ['api', 'scenario']:
+    for test_dir in ['api', 'scenario', 'serial_tests']:
         full_test_dir = os.path.join(base_path, 'tempest', test_dir)
         if not pattern:
             suite.addTests(loader.discover(full_test_dir,
diff --git a/tempest/tests/test_test.py b/tempest/tests/test_test.py
index cbb81e2..26e8079 100644
--- a/tempest/tests/test_test.py
+++ b/tempest/tests/test_test.py
@@ -17,12 +17,14 @@
 import unittest
 from unittest import mock
 
+from oslo_concurrency import lockutils
 from oslo_config import cfg
 import testtools
 
 from tempest import clients
 from tempest import config
 from tempest.lib.common import validation_resources as vr
+from tempest.lib import decorators
 from tempest.lib import exceptions as lib_exc
 from tempest.lib.services.compute import base_compute_client
 from tempest.lib.services.placement import base_placement_client
@@ -33,6 +35,8 @@
 from tempest.tests.lib import fake_credentials
 from tempest.tests.lib.services import registry_fixture
 
+CONF = config.CONF
+
 
 class LoggingTestResult(testtools.TestResult):
 
@@ -594,6 +598,52 @@
             str(log[0][2]['traceback']).replace('\n', ' '),
             RuntimeError.__name__ + ': .* ' + OverridesSetup.__name__)
 
+    @mock.patch.object(test.process_lock, 'InterProcessReaderWriterLock')
+    def test_serial_execution_if_requested(self, mock_lock):
+
+        @decorators.serial
+        class SerialTests(self.parent_test):
+            pass
+
+        class ParallelTests(self.parent_test):
+            pass
+
+        @decorators.serial
+        class SerialTests2(self.parent_test):
+            pass
+
+        suite = unittest.TestSuite(
+            (SerialTests(), ParallelTests(), SerialTests2()))
+        log = []
+        result = LoggingTestResult(log)
+        suite.run(result)
+
+        expected_lock_path = os.path.join(
+            lockutils.get_lock_path(CONF), 'tempest-serial-rw-lock')
+
+        # We except that each test class has a lock with the _same_ external
+        # path so that if they would run by different processes they would
+        # still use the same lock
+        # Also we expect that each serial class takes and releases the
+        # write-lock while each non-serial class takes and releases the
+        # read-lock.
+        self.assertEqual(
+            [
+                mock.call(expected_lock_path),
+                mock.call().acquire_write_lock(),
+                mock.call().release_write_lock(),
+
+                mock.call(expected_lock_path),
+                mock.call().acquire_read_lock(),
+                mock.call().release_read_lock(),
+
+                mock.call(expected_lock_path),
+                mock.call().acquire_write_lock(),
+                mock.call().release_write_lock(),
+            ],
+            mock_lock.mock_calls
+        )
+
 
 class TestTempestBaseTestClassFixtures(base.TestCase):
 
@@ -750,6 +800,11 @@
 
 class TestAPIMicroversionTest1(test.BaseTestCase):
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.useFixture(fake_config.ConfigFixture())
+        config.TempestConfigPrivate = fake_config.FakePrivate
+
     @classmethod
     def resource_setup(cls):
         super(TestAPIMicroversionTest1, cls).resource_setup()
@@ -812,6 +867,11 @@
 
 class TestAPIMicroversionTest2(test.BaseTestCase):
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.useFixture(fake_config.ConfigFixture())
+        config.TempestConfigPrivate = fake_config.FakePrivate
+
     @classmethod
     def resource_setup(cls):
         super(TestAPIMicroversionTest2, cls).resource_setup()
diff --git a/tox.ini b/tox.ini
index c784293..618f9e0 100644
--- a/tox.ini
+++ b/tox.ini
@@ -124,7 +124,7 @@
 commands =
     find . -type f -name "*.pyc" -delete
     tempest run --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.api)' {posargs}
-    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)' {posargs}
+    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)|(^tempest\.serial_tests)' {posargs}
 
 [testenv:full-parallel]
 envdir = .tox/tempest
@@ -135,7 +135,7 @@
 # The regex below is used to select all tempest scenario and including the non slow api tests
 commands =
     find . -type f -name "*.pyc" -delete
-    tempest run --regex '(^tempest\.scenario.*)|(?!.*\[.*\bslow\b.*\])(^tempest\.api)' {posargs}
+    tempest run --regex '(^tempest\.scenario.*)|(^tempest\.serial_tests)|(?!.*\[.*\bslow\b.*\])(^tempest\.api)' {posargs}
 
 [testenv:api-microversion-tests]
 envdir = .tox/tempest
@@ -160,7 +160,7 @@
 commands =
     find . -type f -name "*.pyc" -delete
     tempest run --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.api)' --exclude-list ./tools/tempest-integrated-gate-networking-exclude-list.txt {posargs}
-    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)' --exclude-list ./tools/tempest-integrated-gate-networking-exclude-list.txt {posargs}
+    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)|(^tempest\.serial_tests)' --exclude-list ./tools/tempest-integrated-gate-networking-exclude-list.txt {posargs}
 
 [testenv:integrated-compute]
 envdir = .tox/tempest
@@ -173,7 +173,7 @@
 commands =
     find . -type f -name "*.pyc" -delete
     tempest run --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.api)' --exclude-list ./tools/tempest-integrated-gate-compute-exclude-list.txt {posargs}
-    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)' --exclude-list ./tools/tempest-integrated-gate-compute-exclude-list.txt {posargs}
+    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)|(^tempest\.serial_tests)' --exclude-list ./tools/tempest-integrated-gate-compute-exclude-list.txt {posargs}
 
 [testenv:integrated-placement]
 envdir = .tox/tempest
@@ -186,7 +186,7 @@
 commands =
     find . -type f -name "*.pyc" -delete
     tempest run --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.api)' --exclude-list ./tools/tempest-integrated-gate-placement-exclude-list.txt {posargs}
-    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)' --exclude-list ./tools/tempest-integrated-gate-placement-exclude-list.txt {posargs}
+    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)|(^tempest\.serial_tests)' --exclude-list ./tools/tempest-integrated-gate-placement-exclude-list.txt {posargs}
 
 [testenv:integrated-storage]
 envdir = .tox/tempest
@@ -199,7 +199,7 @@
 commands =
     find . -type f -name "*.pyc" -delete
     tempest run --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.api)' --exclude-list ./tools/tempest-integrated-gate-storage-exclude-list.txt {posargs}
-    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)' --exclude-list ./tools/tempest-integrated-gate-storage-exclude-list.txt {posargs}
+    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)|(^tempest\.serial_tests)' --exclude-list ./tools/tempest-integrated-gate-storage-exclude-list.txt {posargs}
 
 [testenv:integrated-object-storage]
 envdir = .tox/tempest
@@ -212,7 +212,7 @@
 commands =
     find . -type f -name "*.pyc" -delete
     tempest run --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.api)' --exclude-list ./tools/tempest-integrated-gate-object-storage-exclude-list.txt {posargs}
-    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)' --exclude-list ./tools/tempest-integrated-gate-object-storage-exclude-list.txt {posargs}
+    tempest run --combine --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.scenario)|(^tempest\.serial_tests)' --exclude-list ./tools/tempest-integrated-gate-object-storage-exclude-list.txt {posargs}
 
 [testenv:full-serial]
 envdir = .tox/tempest
@@ -225,7 +225,7 @@
 # FIXME: We can replace it with the `--exclude-regex` option to exclude tests now.
 commands =
     find . -type f -name "*.pyc" -delete
-    tempest run --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.(api|scenario))' {posargs}
+    tempest run --serial --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.(api|scenario|serial_tests))' {posargs}
 
 [testenv:scenario]
 envdir = .tox/tempest