From 95d5eb316c8c2274f4c10a22f3837a6c62f3e09e Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 15 Apr 2019 13:19:16 -0400 Subject: [PATCH] Remove ComputeDriver.macs_for_instance method The only in-tree driver that implemented the macs_for_instance method was the ironic driver but that hasn't been used since I4d70423ca978885a982c7eb5bd1efcc024d2b777 in Ocata, so this drops the now unused ComputeDriver method from the ComputeManager. There are only two known out-of-tree drivers that have defined that method in their driver code [1] but they are simply returning None which is what the base ComputeDriver class does so those drivers would be unaffected by this change. A follow up change will continue to remove the "macs" kwarg from the allocate_for_instance network API. [1] http://codesearch.openstack.org/?q=def%20macs_for_instance&i=nope&files=&repos= Change-Id: Ie18ef2386d0a18c970b5c5cba86958cd23f3a228 --- nova/compute/manager.py | 11 ++++---- nova/tests/unit/compute/test_compute.py | 27 ------------------ nova/tests/unit/compute/test_compute_mgr.py | 27 ++++++------------ nova/virt/driver.py | 31 --------------------- 4 files changed, 13 insertions(+), 83 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 8524e11a5b6b..a8bf74f7c7eb 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1462,7 +1462,7 @@ class ComputeManager(manager.Manager): raise exception.InstanceExists(name=instance.name) def _allocate_network_async(self, context, instance, requested_networks, - macs, security_groups, is_vpn, + security_groups, is_vpn, resource_provider_mapping): """Method used to allocate networks in the background. @@ -1486,7 +1486,7 @@ class ComputeManager(manager.Manager): nwinfo = self.network_api.allocate_for_instance( context, instance, vpn=is_vpn, requested_networks=requested_networks, - macs=macs, + macs=None, # TODO(mriedem): Remove macs kwarg. security_groups=security_groups, bind_host_id=bind_host_id, resource_provider_mapping=resource_provider_mapping) @@ -1533,14 +1533,13 @@ class ComputeManager(manager.Manager): if not self.is_neutron_security_groups: security_groups = [] - macs = self.driver.macs_for_instance(instance) network_info = self._allocate_network(context, instance, - requested_networks, macs, security_groups, + requested_networks, security_groups, resource_provider_mapping) return network_info - def _allocate_network(self, context, instance, requested_networks, macs, + def _allocate_network(self, context, instance, requested_networks, security_groups, resource_provider_mapping): """Start network allocation asynchronously. Return an instance of NetworkInfoAsyncWrapper that can be used to retrieve the @@ -1556,7 +1555,7 @@ class ComputeManager(manager.Manager): is_vpn = False return network_model.NetworkInfoAsyncWrapper( self._allocate_network_async, context, instance, - requested_networks, macs, security_groups, is_vpn, + requested_networks, security_groups, is_vpn, resource_provider_mapping) def _default_root_device_name(self, instance, image_meta, root_bdm): diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index c33f47e8ebd4..ba2dfc429634 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -4540,33 +4540,6 @@ class ComputeTestCase(BaseTestCase, self.context) self.assertEqual(payload['image_ref_url'], image_ref_url) - @mock.patch.object(fake.FakeDriver, "macs_for_instance") - def test_run_instance_queries_macs(self, mock_mac): - # run_instance should ask the driver for node mac addresses and pass - # that to the network_api in use. - fake_network.unset_stub_network_methods(self) - instance = self._create_fake_instance_obj() - - macs = set(['01:23:45:67:89:ab']) - - mock_mac.return_value = macs - - with mock.patch.object(self.compute.network_api, - 'allocate_for_instance') as mock_allocate: - mock_allocate.return_value = ( - fake_network.fake_get_instance_nw_info(self, 1, 1)) - self.compute._build_networks_for_instance(self.context, instance, - requested_networks=None, security_groups=None, - resource_provider_mapping=None) - - security_groups = None if CONF.use_neutron else [] - mock_allocate.assert_called_once_with(self.context, instance, - vpn=False, requested_networks=None, macs=macs, - security_groups=security_groups, - bind_host_id=self.compute.host, - resource_provider_mapping=None) - mock_mac.assert_called_once_with(test.MatchType(instance_obj.Instance)) - def test_instance_set_to_error_on_uncaught_exception(self): # Test that instance is set to error state when exception is raised. instance = self._create_fake_instance_obj() diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 87bbf9f6a9fe..e2086901addc 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -562,7 +562,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, is_vpn = 'fake-is-vpn' req_networks = objects.NetworkRequestList( objects=[objects.NetworkRequest(network_id='fake')]) - macs = 'fake-macs' sec_groups = 'fake-sec-groups' final_result = 'meow' rp_mapping = {} @@ -574,7 +573,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, side_effect=[test.TestingException()] * 7 + [final_result]): res = self.compute._allocate_network_async(self.context, instance, req_networks, - macs, sec_groups, is_vpn, rp_mapping) @@ -593,7 +591,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, is_vpn = 'fake-is-vpn' req_networks = objects.NetworkRequestList( objects=[objects.NetworkRequest(network_id='fake')]) - macs = 'fake-macs' sec_groups = 'fake-sec-groups' rp_mapping = {} @@ -602,12 +599,12 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, side_effect=test.TestingException) as mock_allocate: self.assertRaises(test.TestingException, self.compute._allocate_network_async, - self.context, instance, req_networks, macs, + self.context, instance, req_networks, sec_groups, is_vpn, rp_mapping) mock_allocate.assert_called_once_with( self.context, instance, vpn=is_vpn, - requested_networks=req_networks, macs=macs, + requested_networks=req_networks, macs=None, security_groups=sec_groups, bind_host_id=instance.get('host'), resource_provider_mapping=rp_mapping) @@ -623,7 +620,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, is_vpn = 'fake-is-vpn' req_networks = objects.NetworkRequestList( objects=[objects.NetworkRequest(network_id='fake')]) - macs = 'fake-macs' sec_groups = 'fake-sec-groups' final_result = 'zhangtralon' rp_mapping = {} @@ -634,7 +630,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, final_result]): res = self.compute._allocate_network_async(self.context, instance, req_networks, - macs, sec_groups, is_vpn, rp_mapping) @@ -647,7 +642,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, req_networks = objects.NetworkRequestList( objects=[objects.NetworkRequest(network_id='none')]) nwinfo = self.compute._allocate_network_async( - self.context, mock.sentinel.instance, req_networks, macs=None, + self.context, mock.sentinel.instance, req_networks, security_groups=['default'], is_vpn=False, resource_provider_mapping={}) self.assertEqual(0, len(nwinfo)) @@ -5235,9 +5230,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): @mock.patch.object(objects.InstanceActionEvent, 'event_start') @mock.patch.object(objects.InstanceActionEvent, 'event_finish_with_failure') - @mock.patch.object(virt_driver.ComputeDriver, 'macs_for_instance') def test_rescheduled_exception_with_network_allocated(self, - mock_macs_for_instance, mock_event_finish, + mock_event_finish, mock_event_start, mock_ins_save, mock_build_ins, mock_build_and_run): self.flags(use_neutron=False) @@ -5246,7 +5240,6 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): system_metadata={'network_allocated': 'True'}, expected_attrs=['metadata', 'system_metadata', 'info_cache']) mock_ins_save.return_value = instance - mock_macs_for_instance.return_value = [] mock_build_and_run.side_effect = exception.RescheduledException( reason='', instance_uuid=self.instance.uuid) @@ -5284,9 +5277,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): @mock.patch.object(objects.InstanceActionEvent, 'event_start') @mock.patch.object(objects.InstanceActionEvent, 'event_finish_with_failure') - @mock.patch.object(virt_driver.ComputeDriver, 'macs_for_instance') def test_rescheduled_exception_with_network_allocated_with_neutron(self, - mock_macs_for_instance, mock_event_finish, mock_event_start, + mock_event_finish, mock_event_start, mock_ins_save, mock_build_ins, mock_cleanup_network, mock_build_and_run): """Tests that we always cleanup allocated networks for the instance @@ -5297,7 +5289,6 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): system_metadata={'network_allocated': 'True'}, expected_attrs=['metadata', 'system_metadata', 'info_cache']) mock_ins_save.return_value = instance - mock_macs_for_instance.return_value = [] mock_build_and_run.side_effect = exception.RescheduledException( reason='', instance_uuid=self.instance.uuid) @@ -5332,9 +5323,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): @mock.patch.object(objects.InstanceActionEvent, 'event_start') @mock.patch.object(objects.InstanceActionEvent, 'event_finish_with_failure') - @mock.patch.object(virt_driver.ComputeDriver, 'macs_for_instance') def test_rescheduled_exception_with_sriov_network_allocated(self, - mock_macs_for_instance, mock_event_finish, + mock_event_finish, mock_event_start, mock_ins_save, mock_cleanup_network, mock_build_ins, mock_build_and_run): vif1 = fake_network_cache_model.new_vif() @@ -5353,7 +5343,6 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): instance.info_cache = info_cache mock_ins_save.return_value = instance - mock_macs_for_instance.return_value = [] mock_build_and_run.side_effect = exception.RescheduledException( reason='', instance_uuid=self.instance.uuid) @@ -6374,7 +6363,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): self.resource_provider_mapping) mock_allocate.assert_called_once_with(self.context, instance, - self.requested_networks, None, self.security_groups, + self.requested_networks, self.security_groups, self.resource_provider_mapping) self.assertTrue(hasattr(nw_info_obj, 'wait'), "wait must be there") @@ -6390,7 +6379,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): self.resource_provider_mapping) mock_allocate.assert_called_once_with(self.context, instance, - self.requested_networks, None, self.security_groups, + self.requested_networks, self.security_groups, self.resource_provider_mapping) self.assertTrue(hasattr(nw_info_obj, 'wait'), "wait must be there") diff --git a/nova/virt/driver.py b/nova/virt/driver.py index 97a9fa3aa35e..2fec09a229a6 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -1515,37 +1515,6 @@ class ComputeDriver(object): """Does the driver want networks deallocated on reschedule?""" return False - # NOTE(vsaienko) This method is deprecated, don't use it! - # TODO(vsaienko) Remove this function in Ocata. - def macs_for_instance(self, instance): - """What MAC addresses must this instance have? - - Some hypervisors (such as bare metal) cannot do freeform virtualization - of MAC addresses. This method allows drivers to return a set of MAC - addresses that the instance is to have. allocate_for_instance will take - this into consideration when provisioning networking for the instance. - - Mapping of MAC addresses to actual networks (or permitting them to be - freeform) is up to the network implementation layer. For instance, - with openflow switches, fixed MAC addresses can still be virtualized - onto any L2 domain, with arbitrary VLANs etc, but regular switches - require pre-configured MAC->network mappings that will match the - actual configuration. - - Most hypervisors can use the default implementation which returns None. - Hypervisors with MAC limits should return a set of MAC addresses, which - will be supplied to the allocate_for_instance call by the compute - manager, and it is up to that call to ensure that all assigned network - details are compatible with the set of MAC addresses. - - This is called during spawn_instance by the compute manager. - - :return: None, or a set of MAC ids (e.g. set(['12:34:56:78:90:ab'])). - None means 'no constraints', a set means 'these and only these - MAC addresses'. - """ - return None - def manage_image_cache(self, context, all_instances): """Manage the driver's local image cache.