Merge "Only construct SchedulerReportClient on first access from API"

nova/api/openstack/compute/services.py

@@ -42,7 +42,13 @@ class ServiceController(wsgi.Controller):
         self.actions = {"enable": self._enable,
                         "disable": self._disable,
                         "disable-log-reason": self._disable_log_reason}
-        self.placementclient = report.SchedulerReportClient()
+        self._placementclient = None  # Lazy-load on first access.
+
+    @property
+    def placementclient(self):
+        if self._placementclient is None:
+            self._placementclient = report.SchedulerReportClient()
+        return self._placementclient
 
     def _get_services(self, req):
         # The API services are filtered out since they are not RPC services

nova/compute/api.py

@@ -76,7 +76,8 @@ from nova.policies import servers as servers_policies
 import nova.policy
 from nova import profiler
 from nova import rpc
-from nova.scheduler import client as scheduler_client
+from nova.scheduler.client import query as queryclient
+from nova.scheduler.client import report as reportclient
 from nova.scheduler import utils as scheduler_utils
 from nova import servicegroup
 from nova import utils
@@ -253,13 +254,7 @@ class API(base.Base):
         self.image_api = image_api or image.API()
         self.network_api = network_api or network.API()
         self.volume_api = volume_api or cinder.API()
-        # NOTE(mriedem): This looks a bit weird but we get the reportclient
-        # via SchedulerClient since it lazy-loads SchedulerReportClient on
-        # the first usage which helps to avoid a bunch of lockutils spam in
-        # the nova-api logs every time the service is restarted (remember
-        # that pretty much all of the REST API controllers construct this
-        # API class).
-        self.placementclient = scheduler_client.SchedulerClient().reportclient
+        self._placementclient = None  # Lazy-load on first access.
         self.security_group_api = (security_group_api or
             openstack_driver.get_openstack_security_group_driver())
         self.consoleauth_rpcapi = consoleauth_rpcapi.ConsoleAuthAPI()
@@ -2136,6 +2131,12 @@ class API(base.Base):
             if 'id' in bdm:
                 bdm.destroy()
 
+    @property
+    def placementclient(self):
+        if self._placementclient is None:
+            self._placementclient = reportclient.SchedulerReportClient()
+        return self._placementclient
+
     def _local_delete(self, context, instance, bdms, delete_type, cb):
         if instance.vm_state == vm_states.SHELVED_OFFLOADED:
             LOG.info("instance is in SHELVED_OFFLOADED state, cleanup"
@@ -5301,10 +5302,16 @@ class AggregateAPI(base.Base):
     """Sub-set of the Compute Manager API for managing host aggregates."""
     def __init__(self, **kwargs):
         self.compute_rpcapi = compute_rpcapi.ComputeAPI()
-        self.scheduler_client = scheduler_client.SchedulerClient()
-        self.placement_client = self.scheduler_client.reportclient
+        self.scheduler_client = queryclient.SchedulerQueryClient()
+        self._placement_client = None  # Lazy-load on first access.
         super(AggregateAPI, self).__init__(**kwargs)
 
+    @property
+    def placement_client(self):
+        if self._placement_client is None:
+            self._placement_client = reportclient.SchedulerReportClient()
+        return self._placement_client
+
     @wrap_exception()
     def create_aggregate(self, context, aggregate_name, availability_zone):
         """Creates the model for the aggregate."""

nova/tests/unit/compute/test_compute.py

@@ -67,7 +67,6 @@ from nova.objects import block_device as block_device_obj
 from nova.objects import fields as obj_fields
 from nova.objects import instance as instance_obj
 from nova.objects import migrate_data as migrate_data_obj
-from nova.scheduler import client as scheduler_client
 from nova import test
 from nova.tests import fixtures
 from nova.tests.unit.compute import eventlet_utils
@@ -12288,6 +12287,18 @@ class ComputeAPIAggrTestCase(BaseTestCase):
         hosts = aggregate.hosts if 'hosts' in aggregate else None
         self.assertIn(values[0][1][0], hosts)
 
+    @mock.patch('nova.scheduler.client.report.SchedulerReportClient')
+    def test_placement_client_init(self, mock_report_client):
+        """Tests to make sure that the construction of the placement client
+        only happens once per AggregateAPI class instance.
+        """
+        self.assertIsNone(self.api._placement_client)
+        # Access the property twice to make sure SchedulerReportClient is
+        # only loaded once.
+        for x in range(2):
+            self.api.placement_client
+        mock_report_client.assert_called_once_with()
+
 
 class ComputeAPIAggrCallsSchedulerTestCase(test.NoDBTestCase):
     """This is for making sure that all Aggregate API methods which are
@@ -12300,13 +12311,15 @@ class ComputeAPIAggrCallsSchedulerTestCase(test.NoDBTestCase):
         self.api = compute_api.AggregateAPI()
         self.context = context.RequestContext('fake', 'fake')
 
-    @mock.patch.object(scheduler_client.SchedulerClient, 'update_aggregates')
+    @mock.patch('nova.scheduler.client.query.SchedulerQueryClient.'
+                'update_aggregates')
     def test_create_aggregate(self, update_aggregates):
         with mock.patch.object(objects.Aggregate, 'create'):
             agg = self.api.create_aggregate(self.context, 'fake', None)
         update_aggregates.assert_called_once_with(self.context, [agg])
 
-    @mock.patch.object(scheduler_client.SchedulerClient, 'update_aggregates')
+    @mock.patch('nova.scheduler.client.query.SchedulerQueryClient.'
+                'update_aggregates')
     def test_update_aggregate(self, update_aggregates):
         self.api.is_safe_to_update_az = mock.Mock()
         agg = objects.Aggregate()
@@ -12315,7 +12328,8 @@ class ComputeAPIAggrCallsSchedulerTestCase(test.NoDBTestCase):
             self.api.update_aggregate(self.context, 1, {})
         update_aggregates.assert_called_once_with(self.context, [agg])
 
-    @mock.patch.object(scheduler_client.SchedulerClient, 'update_aggregates')
+    @mock.patch('nova.scheduler.client.query.SchedulerQueryClient.'
+                'update_aggregates')
     def test_update_aggregate_metadata(self, update_aggregates):
         self.api.is_safe_to_update_az = mock.Mock()
         agg = objects.Aggregate()
@@ -12325,7 +12339,8 @@ class ComputeAPIAggrCallsSchedulerTestCase(test.NoDBTestCase):
             self.api.update_aggregate_metadata(self.context, 1, {})
         update_aggregates.assert_called_once_with(self.context, [agg])
 
-    @mock.patch.object(scheduler_client.SchedulerClient, 'delete_aggregate')
+    @mock.patch('nova.scheduler.client.query.SchedulerQueryClient.'
+                'delete_aggregate')
     def test_delete_aggregate(self, delete_aggregate):
         self.api.is_safe_to_update_az = mock.Mock()
         agg = objects.Aggregate(uuid=uuids.agg, name='fake-aggregate',
@@ -12340,7 +12355,8 @@ class ComputeAPIAggrCallsSchedulerTestCase(test.NoDBTestCase):
                 'aggregate_add_host')
     @mock.patch('nova.compute.utils.notify_about_aggregate_action')
     @mock.patch('nova.compute.rpcapi.ComputeAPI.add_aggregate_host')
-    @mock.patch.object(scheduler_client.SchedulerClient, 'update_aggregates')
+    @mock.patch('nova.scheduler.client.query.SchedulerQueryClient.'
+                'update_aggregates')
     def test_add_host_to_aggregate(self, update_aggregates, mock_add_agg,
                                    mock_notify, mock_add_host):
         self.api.is_safe_to_update_az = mock.Mock()
@@ -12365,7 +12381,8 @@ class ComputeAPIAggrCallsSchedulerTestCase(test.NoDBTestCase):
                 'aggregate_remove_host')
     @mock.patch('nova.compute.utils.notify_about_aggregate_action')
     @mock.patch('nova.compute.rpcapi.ComputeAPI.remove_aggregate_host')
-    @mock.patch.object(scheduler_client.SchedulerClient, 'update_aggregates')
+    @mock.patch('nova.scheduler.client.query.SchedulerQueryClient.'
+                'update_aggregates')
     def test_remove_host_from_aggregate(self, update_aggregates,
                                         mock_remove_agg, mock_notify,
                                         mock_remove_host):

nova/tests/unit/compute/test_compute_api.py

@@ -6718,6 +6718,18 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
         self.assertTrue(hasattr(self.compute_api, 'host'))
         self.assertEqual(CONF.host, self.compute_api.host)
 
+    @mock.patch('nova.scheduler.client.report.SchedulerReportClient')
+    def test_placement_client_init(self, mock_report_client):
+        """Tests to make sure that the construction of the placement client
+        only happens once per API class instance.
+        """
+        self.assertIsNone(self.compute_api._placementclient)
+        # Access the property twice to make sure SchedulerReportClient is
+        # only loaded once.
+        for x in range(2):
+            self.compute_api.placementclient
+        mock_report_client.assert_called_once_with()
+
 
 class Cellsv1DeprecatedTestMixIn(object):
     @mock.patch.object(objects.BuildRequestList, 'get_by_filters')