Ensures that COMPUTE_RESOURCE_SEMAPHORE usage is fair

This patch poisons the synchronized decorator in the unit test
to prevent adding you synchronized methods without the fair=True
flag.

Change-Id: I739025dacbcaa0f7adbe612c064f979bf6390880
Related-Bug: #1864122

nova/test.py

@@ -96,6 +96,39 @@ class TestingException(Exception):
 mock_fixture.patch_mock_module()
 
 
+def _poison_unfair_compute_resource_semaphore_locking():
+    """Ensure that every locking on COMPUTE_RESOURCE_SEMAPHORE is called with
+    fair=True.
+    """
+    orig_synchronized = utils.synchronized
+
+    def poisoned_synchronized(*args, **kwargs):
+        # Only check fairness if the decorator is used with
+        # COMPUTE_RESOURCE_SEMAPHORE. But the name of the semaphore can be
+        # passed as args or as kwargs.
+        # Note that we cannot import COMPUTE_RESOURCE_SEMAPHORE as that would
+        # apply the decorators we want to poison here.
+        if len(args) >= 1:
+            name = args[0]
+        else:
+            name = kwargs.get("name")
+        if name == "compute_resources" and not kwargs.get("fair", False):
+            raise AssertionError(
+                'Locking on COMPUTE_RESOURCE_SEMAPHORE should always be fair. '
+                'See bug 1864122.')
+        # go and act like the original decorator
+        return orig_synchronized(*args, **kwargs)
+
+    # replace the synchronized decorator factory with our own that checks the
+    # params passed in
+    utils.synchronized = poisoned_synchronized
+
+
+# NOTE(gibi): This poisoning needs to be done in import time as decorators are
+# applied in import time on the ResourceTracker
+_poison_unfair_compute_resource_semaphore_locking()
+
+
 class NovaExceptionReraiseFormatError(object):
     real_log_exception = exception.NovaException._log_exception
 

nova/tests/unit/compute/test_resource_tracker.py

@@ -43,6 +43,7 @@ from nova.tests.unit import fake_instance
 from nova.tests.unit import fake_notifier
 from nova.tests.unit.objects import test_pci_device as fake_pci_device
 from nova.tests.unit import utils
+from nova import utils as nova_utils
 from nova.virt import driver
 
 _HOSTNAME = 'fake-host'
@@ -3896,3 +3897,21 @@ class ResourceTrackerTestCase(test.NoDBTestCase):
         rt = resource_tracker.ResourceTracker(
             _HOSTNAME, mock.sentinel.driver, mock.sentinel.reportclient)
         self.assertIs(rt.reportclient, mock.sentinel.reportclient)
+
+    def test_that_unfair_usage_of_compute_resource_semaphore_is_caught(self):
+        def _test_explict_unfair():
+            class MyResourceTracker(resource_tracker.ResourceTracker):
+                @nova_utils.synchronized(
+                    resource_tracker.COMPUTE_RESOURCE_SEMAPHORE, fair=False)
+                def foo(self):
+                    pass
+
+        def _test_implicit_unfair():
+            class MyResourceTracker(resource_tracker.ResourceTracker):
+                @nova_utils.synchronized(
+                    resource_tracker.COMPUTE_RESOURCE_SEMAPHORE)
+                def foo(self):
+                    pass
+
+        self.assertRaises(AssertionError, _test_explict_unfair)
+        self.assertRaises(AssertionError, _test_implicit_unfair)