virt: Remove various aggregate APIs

These were used to broadcast aggregate changes to XenAPI pools and
aren't used by any other in-tree driver. Remove them.

Change-Id: I18a01032a89bff84d71e879c5207157393849b7e
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

Author: stephenfin

nova/compute/api.py

@@ -6212,8 +6212,6 @@ def add_host_to_aggregate(self, context, aggregate_id, host_name):
                         node_name, err)
         self._update_az_cache_for_host(context, host_name, aggregate.metadata)
         # NOTE(jogo): Send message to host to support resource pools
-        self.compute_rpcapi.add_aggregate_host(context,
-                aggregate=aggregate, host_param=host_name, host=host_name)
         aggregate_payload.update({'name': aggregate.name})
         compute_utils.notify_about_aggregate_update(context,
                                                     "addhost.end",
@@ -6266,8 +6264,6 @@ def remove_host_from_aggregate(self, context, aggregate_id, host_name):
         aggregate.delete_host(host_name)
         self.query_client.update_aggregates(context, [aggregate])
         self._update_az_cache_for_host(context, host_name, aggregate.metadata)
-        self.compute_rpcapi.remove_aggregate_host(context,
-                aggregate=aggregate, host_param=host_name, host=host_name)
         compute_utils.notify_about_aggregate_update(context,
                                                     "removehost.end",
                                                     aggregate_payload)

nova/compute/manager.py

@@ -10092,39 +10092,17 @@ def _error_out_instance_on_exception(self, context, instance,
                 # NOTE(mriedem): Why don't we pass clean_task_state=True here?
                 self._set_instance_obj_error_state(instance)
 
+    # TODO(stephenfin): Remove this once we bump the compute API to v6.0
     @wrap_exception()
     def add_aggregate_host(self, context, aggregate, host, slave_info):
-        """Notify hypervisor of change (for hypervisor pools)."""
-        try:
-            self.driver.add_to_aggregate(context, aggregate, host,
-                                         slave_info=slave_info)
-        except NotImplementedError:
-            LOG.debug('Hypervisor driver does not support '
-                      'add_aggregate_host')
-        except exception.AggregateError:
-            with excutils.save_and_reraise_exception():
-                self.driver.undo_aggregate_operation(
-                                    context,
-                                    aggregate.delete_host,
-                                    aggregate, host)
+        """(REMOVED) Notify hypervisor of change (for hypervisor pools)."""
+        raise NotImplementedError()
 
+    # TODO(stephenfin): Remove this once we bump the compute API to v6.0
     @wrap_exception()
     def remove_aggregate_host(self, context, host, slave_info, aggregate):
-        """Removes a host from a physical hypervisor pool."""
-        try:
-            self.driver.remove_from_aggregate(context, aggregate, host,
-                                              slave_info=slave_info)
-        except NotImplementedError:
-            LOG.debug('Hypervisor driver does not support '
-                      'remove_aggregate_host')
-        except (exception.AggregateError,
-                exception.InvalidAggregateAction) as e:
-            with excutils.save_and_reraise_exception():
-                self.driver.undo_aggregate_operation(
-                                    context,
-                                    aggregate.add_host,
-                                    aggregate, host,
-                                    isinstance(e, exception.AggregateError))
+        """(REMOVED) Removes a host from a physical hypervisor pool."""
+        raise NotImplementedError()
 
     def _process_instance_event(self, instance, event):
         _event = self.instance_events.pop_instance_event(instance, event)

nova/compute/rpcapi.py

@@ -511,6 +511,7 @@ def get_client(self, target, version_cap, serializer):
                               serializer=serializer,
                               call_monitor_timeout=cmt)
 
+    # TODO(stephenfin): This is no longer used and can be removed in v6.0
     def add_aggregate_host(self, ctxt, host, aggregate, host_param,
                            slave_info=None):
         '''Add aggregate host.
@@ -1092,6 +1093,7 @@ def rebuild_instance(
                    recreate=recreate, on_shared_storage=on_shared_storage,
                    **msg_args)
 
+    # TODO(stephenfin): This is no longer used and can be removed in v6.0
     def remove_aggregate_host(self, ctxt, host, aggregate, host_param,
                               slave_info=None):
         '''Remove aggregate host.

nova/exception.py

@@ -1243,12 +1243,6 @@ class PortLimitExceeded(QuotaError):
     msg_fmt = _("Maximum number of ports exceeded")
 
 
-# TODO(stephenfin): Remove this XenAPI relic
-class AggregateError(NovaException):
-    msg_fmt = _("Aggregate %(aggregate_id)s: action '%(action)s' "
-                "caused an error: %(reason)s.")
-
-
 class AggregateNotFound(NotFound):
     msg_fmt = _("Aggregate %(aggregate_id)s could not be found.")
 

nova/tests/unit/compute/test_compute.py

@@ -12247,13 +12247,6 @@ def test_add_host_to_aggregate(self, mock_az, mock_notify, mock_add_host,
                 host=fake_host,
                 hypervisor_hostname=fake_host)])
 
-        def fake_add_aggregate_host(*args, **kwargs):
-            hosts = kwargs["aggregate"].hosts
-            self.assertIn(fake_host, hosts)
-
-        self.stub_out('nova.compute.rpcapi.ComputeAPI.add_aggregate_host',
-                       fake_add_aggregate_host)
-
         fake_notifier.NOTIFICATIONS = []
         aggr = self.api.add_host_to_aggregate(self.context,
                                               aggr.id, fake_host)
@@ -12449,13 +12442,6 @@ def test_remove_host_from_aggregate_active(
                                                   aggr.id, host)
         host_to_remove = values[0][1][0]
 
-        def fake_remove_aggregate_host(*args, **kwargs):
-            hosts = kwargs["aggregate"].hosts
-            self.assertNotIn(host_to_remove, hosts)
-
-        self.stub_out('nova.compute.rpcapi.ComputeAPI.remove_aggregate_host',
-                       fake_remove_aggregate_host)
-
         fake_notifier.NOTIFICATIONS = []
         mock_notify.reset_mock()
         mock_get_all_by_host.reset_mock()
@@ -12668,12 +12654,12 @@ def test_delete_aggregate(self, delete_aggregate):
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 'aggregate_add_host')
     @mock.patch('nova.compute.utils.notify_about_aggregate_action')
-    @mock.patch('nova.compute.rpcapi.ComputeAPI.add_aggregate_host')
     @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,
-                                   mock_get_all_by_host):
+    def test_add_host_to_aggregate(
+        self, update_aggregates, mock_notify, mock_add_host,
+        mock_get_all_by_host,
+    ):
         self.api.is_safe_to_update_az = mock.Mock()
         self.api._update_az_cache_for_host = mock.Mock()
         agg = objects.Aggregate(name='fake', metadata={}, uuid=uuids.agg)
@@ -12690,23 +12676,19 @@ def test_add_host_to_aggregate(self, update_aggregates, mock_add_agg,
                                   return_value=agg)):
             self.api.add_host_to_aggregate(self.context, 1, 'fakehost')
         update_aggregates.assert_called_once_with(self.context, [agg])
-        mock_add_agg.assert_called_once_with(self.context, aggregate=agg,
-                                             host_param='fakehost',
-                                             host='fakehost')
         mock_add_host.assert_called_once_with(
             self.context, agg.uuid, host_name='fakehost')
 
     @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 'aggregate_remove_host')
     @mock.patch('nova.compute.utils.notify_about_aggregate_action')
-    @mock.patch('nova.compute.rpcapi.ComputeAPI.remove_aggregate_host')
     @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,
-                                        mock_get_all_by_host):
+    def test_remove_host_from_aggregate(
+        self, update_aggregates, mock_notify, mock_remove_host,
+        mock_get_all_by_host,
+    ):
         self.api._update_az_cache_for_host = mock.Mock()
         agg = objects.Aggregate(name='fake', metadata={}, uuid=uuids.agg)
         agg.delete_host = mock.Mock()
@@ -12720,9 +12702,6 @@ def test_remove_host_from_aggregate(self, update_aggregates,
                                   return_value=agg)):
             self.api.remove_host_from_aggregate(self.context, 1, 'fakehost')
         update_aggregates.assert_called_once_with(self.context, [agg])
-        mock_remove_agg.assert_called_once_with(self.context, aggregate=agg,
-                                                host_param='fakehost',
-                                                host='fakehost')
         mock_notify.assert_has_calls([
             mock.call(context=self.context, aggregate=agg,
                       action='remove_host', phase='start'),
@@ -12732,72 +12711,6 @@ def test_remove_host_from_aggregate(self, update_aggregates,
             self.context, agg.uuid, 'fakehost')
 
 
-class ComputeAggrTestCase(BaseTestCase):
-    """This is for unit coverage of aggregate-related methods
-    defined in nova.compute.manager.
-    """
-
-    def setUp(self):
-        super(ComputeAggrTestCase, self).setUp()
-        self.context = context.get_admin_context()
-        az = {'availability_zone': 'test_zone'}
-        self.aggr = objects.Aggregate(self.context, name='test_aggr',
-                                      metadata=az)
-        self.aggr.create()
-
-    def test_add_aggregate_host(self):
-        def fake_driver_add_to_aggregate(self, context, aggregate, host,
-                                         **_ignore):
-            fake_driver_add_to_aggregate.called = True
-            return {"foo": "bar"}
-        self.stub_out("nova.virt.fake.FakeDriver.add_to_aggregate",
-                       fake_driver_add_to_aggregate)
-
-        self.compute.add_aggregate_host(self.context, host="host",
-            aggregate=self.aggr, slave_info=None)
-        self.assertTrue(fake_driver_add_to_aggregate.called)
-
-    def test_remove_aggregate_host(self):
-        def fake_driver_remove_from_aggregate(cls, context, aggregate, host,
-                                              **_ignore):
-            fake_driver_remove_from_aggregate.called = True
-            self.assertEqual("host", host, "host")
-            return {"foo": "bar"}
-        self.stub_out("nova.virt.fake.FakeDriver.remove_from_aggregate",
-                       fake_driver_remove_from_aggregate)
-
-        self.compute.remove_aggregate_host(self.context,
-            aggregate=self.aggr, host="host", slave_info=None)
-        self.assertTrue(fake_driver_remove_from_aggregate.called)
-
-    def test_add_aggregate_host_passes_slave_info_to_driver(self):
-        def driver_add_to_aggregate(cls, context, aggregate, host, **kwargs):
-            self.assertEqual(self.context, context)
-            self.assertEqual(aggregate.id, self.aggr.id)
-            self.assertEqual(host, "the_host")
-            self.assertEqual("SLAVE_INFO", kwargs.get("slave_info"))
-
-        self.stub_out("nova.virt.fake.FakeDriver.add_to_aggregate",
-                       driver_add_to_aggregate)
-
-        self.compute.add_aggregate_host(self.context, host="the_host",
-            slave_info="SLAVE_INFO", aggregate=self.aggr)
-
-    def test_remove_from_aggregate_passes_slave_info_to_driver(self):
-        def driver_remove_from_aggregate(cls, context, aggregate, host,
-                                         **kwargs):
-            self.assertEqual(self.context, context)
-            self.assertEqual(aggregate.id, self.aggr.id)
-            self.assertEqual(host, "the_host")
-            self.assertEqual("SLAVE_INFO", kwargs.get("slave_info"))
-
-        self.stub_out("nova.virt.fake.FakeDriver.remove_from_aggregate",
-                       driver_remove_from_aggregate)
-
-        self.compute.remove_aggregate_host(self.context,
-            aggregate=self.aggr, host="the_host", slave_info="SLAVE_INFO")
-
-
 class DisabledInstanceTypesTestCase(BaseTestCase):
     """Some instance-types are marked 'disabled' which means that they will not
     show up in customer-facing listings. We do, however, want those

nova/tests/unit/virt/test_virt_drivers.py

@@ -650,14 +650,6 @@ def test_host_power_action_shutdown(self):
     def test_host_power_action_startup(self):
         self.connection.host_power_action('startup')
 
-    @catch_notimplementederror
-    def test_add_to_aggregate(self):
-        self.connection.add_to_aggregate(self.ctxt, 'aggregate', 'host')
-
-    @catch_notimplementederror
-    def test_remove_from_aggregate(self):
-        self.connection.remove_from_aggregate(self.ctxt, 'aggregate', 'host')
-
     def test_events(self):
         got_events = []
 

nova/virt/driver.py

@@ -1544,54 +1544,6 @@ def cache_image(self, context, image_id):
         """
         raise NotImplementedError()
 
-    # TODO(stephenfin): This was only implemented (properly) for XenAPI and
-    # should be removed.
-    def add_to_aggregate(self, context, aggregate, host, **kwargs):
-        """Add a compute host to an aggregate.
-
-        The counter action to this is :func:`remove_from_aggregate`
-
-        :param nova.context.RequestContext context:
-            The security context.
-        :param nova.objects.aggregate.Aggregate aggregate:
-            The aggregate which should add the given `host`
-        :param str host:
-            The name of the host to add to the given `aggregate`.
-        :param dict kwargs:
-            A free-form thingy...
-
-        :return: None
-        """
-        # NOTE(jogo) Currently only used for XenAPI-Pool
-        raise NotImplementedError()
-
-    # TODO(stephenfin): This was only implemented (properly) for XenAPI and
-    # should be removed.
-    def remove_from_aggregate(self, context, aggregate, host, **kwargs):
-        """Remove a compute host from an aggregate.
-
-        The counter action to this is :func:`add_to_aggregate`
-
-        :param nova.context.RequestContext context:
-            The security context.
-        :param nova.objects.aggregate.Aggregate aggregate:
-            The aggregate which should remove the given `host`
-        :param str host:
-            The name of the host to remove from the given `aggregate`.
-        :param dict kwargs:
-            A free-form thingy...
-
-        :return: None
-        """
-        raise NotImplementedError()
-
-    # TODO(stephenfin): This was only implemented (properly) for XenAPI and
-    # should be removed.
-    def undo_aggregate_operation(self, context, op, aggregate,
-                                  host, set_error=True):
-        """Undo for Resource Pools."""
-        raise NotImplementedError()
-
     def get_volume_connector(self, instance):
         """Get connector information for the instance for attaching to volumes.