From dee72c8e76c415851d5bcd1f6cdd0e31f3d16129 Mon Sep 17 00:00:00 2001 From: Hans Lindgren Date: Tue, 12 May 2015 19:53:07 +0200 Subject: [PATCH] Additional cleanup after compute RPC 3.x removal This change does some additional cleanup like removing the now orphaned _spawn() method and a related test. Furthermore it does away with some rpcapi test stuff that no longer applies and also makes sure we send objects instead of primitives in all rpcapi tests. Finally, some left- over parameters in the client-side attach_volume() method are removed since those are no longer needed. Change-Id: I74e270304e03a843e6051afcaae7812af5564875 --- nova/compute/api.py | 3 +- nova/compute/manager.py | 53 --------------------- nova/compute/rpcapi.py | 7 ++- nova/tests/unit/compute/test_compute.py | 24 ++-------- nova/tests/unit/compute/test_compute_mgr.py | 11 ----- nova/tests/unit/compute/test_rpcapi.py | 47 +++--------------- 6 files changed, 14 insertions(+), 131 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index aee3b2ec533c..7e415d4a8979 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -3000,8 +3000,7 @@ class API(base.Base): volume = self.volume_api.get(context, volume_id) self.volume_api.check_attach(context, volume, instance=instance) self.volume_api.reserve_volume(context, volume_id) - self.compute_rpcapi.attach_volume(context, instance=instance, - volume_id=volume_id, mountpoint=device, bdm=volume_bdm) + self.compute_rpcapi.attach_volume(context, instance, volume_bdm) except Exception: with excutils.save_and_reraise_exception(): volume_bdm.destroy() diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 174b14597aa3..a279db43c4e1 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1738,59 +1738,6 @@ class ComputeManager(manager.Manager): instance.launched_at = timeutils.utcnow() configdrive.update_instance(instance) - @object_compat - def _spawn(self, context, instance, image_meta, network_info, - block_device_info, injected_files, admin_password, - set_access_ip=False): - """Spawn an instance with error logging and update its power state.""" - instance.vm_state = vm_states.BUILDING - instance.task_state = task_states.SPAWNING - instance.save(expected_task_state=task_states.BLOCK_DEVICE_MAPPING) - try: - self.driver.spawn(context, instance, image_meta, - injected_files, admin_password, - network_info, - block_device_info) - except Exception: - with excutils.save_and_reraise_exception(): - LOG.exception(_LE('Instance failed to spawn'), - instance=instance) - - self._update_instance_after_spawn(context, instance) - - def _set_access_ip_values(): - """Add access ip values for a given instance. - - If CONF.default_access_ip_network_name is set, this method will - grab the corresponding network and set the access ip values - accordingly. Note that when there are multiple ips to choose - from, an arbitrary one will be chosen. - """ - - network_name = CONF.default_access_ip_network_name - if not network_name: - return - - for vif in network_info: - if vif['network']['label'] == network_name: - for ip in vif.fixed_ips(): - if ip['version'] == 4: - instance.access_ip_v4 = ip['address'] - if ip['version'] == 6: - instance.access_ip_v6 = ip['address'] - return - - if set_access_ip: - _set_access_ip_values() - - network_info.wait(do_raise=True) - instance.info_cache.network_info = network_info - # NOTE(JoshNang) This also saves the changes to the instance from - # _allocate_network_async, as they aren't saved in that function - # to prevent races. - instance.save(expected_task_state=task_states.SPAWNING) - return instance - def _update_scheduler_instance_info(self, context, instance): """Sends an InstanceList with created or updated Instance objects to the Scheduler client. diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index b2583bb899ec..325a233f24c1 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -357,12 +357,11 @@ class ComputeAPI(object): instance=instance, network_id=network_id, port_id=port_id, requested_ip=requested_ip) - def attach_volume(self, ctxt, instance, volume_id, mountpoint, bdm=None): - kw = {'instance': instance, 'bdm': bdm} + def attach_volume(self, ctxt, instance, bdm): version = '4.0' cctxt = self.client.prepare(server=_compute_host(None, instance), - version=version) - cctxt.cast(ctxt, 'attach_volume', **kw) + version=version) + cctxt.cast(ctxt, 'attach_volume', instance=instance, bdm=bdm) def change_instance_metadata(self, ctxt, instance, diff): version = '4.0' diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index b10ce52a8cf8..3f79a09a5e5f 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -140,22 +140,6 @@ def unify_instance(instance): return newdict -class FakeSchedulerAPI(object): - - def run_instance(self, ctxt, request_spec, admin_password, - injected_files, requested_networks, is_first_time, - filter_properties): - pass - - def live_migration(self, ctxt, block_migration, disk_over_commit, - instance, dest): - pass - - def prep_resize(self, ctxt, instance, instance_type, image, request_spec, - filter_properties, reservations, clean_shutown): - pass - - class FakeComputeTaskAPI(object): def resize_instance(self, context, instance, extra_instance_updates, @@ -9205,10 +9189,8 @@ class ComputeAPITestCase(BaseTestCase): mock_reserve_vol.assert_called_once_with( self.context, 'fake-volume-id') a, kw = mock_attach.call_args - self.assertEqual(kw['volume_id'], 'fake-volume-id') - self.assertEqual(kw['mountpoint'], '/dev/vdb') - self.assertEqual(kw['bdm'].device_name, '/dev/vdb') - self.assertEqual(kw['bdm'].volume_id, 'fake-volume-id') + self.assertEqual(a[2].device_name, '/dev/vdb') + self.assertEqual(a[2].volume_id, 'fake-volume-id') def test_attach_volume_no_device(self): @@ -9224,7 +9206,7 @@ class ComputeAPITestCase(BaseTestCase): called['fake_volume_get'] = True return {'id': volume_id} - def fake_rpc_attach_volume(self, context, **kwargs): + def fake_rpc_attach_volume(self, context, instance, bdm): called['fake_rpc_attach_volume'] = True def fake_rpc_reserve_block_device_name(self, context, instance, device, diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 136f3340288f..6f14dc2e4529 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -3032,17 +3032,6 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): self.instance, self.block_device_mapping, self.requested_networks, try_deallocate_networks=True) - @mock.patch('nova.compute.manager.ComputeManager._get_power_state') - def test_spawn_waits_for_network_and_saves_info_cache(self, gps): - inst = mock.MagicMock() - network_info = mock.MagicMock() - with mock.patch.object(self.compute, 'driver'): - self.compute._spawn(self.context, inst, {}, network_info, None, - None, None) - network_info.wait.assert_called_once_with(do_raise=True) - self.assertEqual(network_info, inst.info_cache.network_info) - inst.save.assert_called_with(expected_task_state=task_states.SPAWNING) - @mock.patch('nova.utils.spawn_n') def test_reschedule_on_resources_unavailable(self, mock_spawn): mock_spawn.side_effect = lambda f, *a, **k: f(*a, **k) diff --git a/nova/tests/unit/compute/test_rpcapi.py b/nova/tests/unit/compute/test_rpcapi.py index 308ada0ad71c..a547f6df3958 100644 --- a/nova/tests/unit/compute/test_rpcapi.py +++ b/nova/tests/unit/compute/test_rpcapi.py @@ -41,16 +41,12 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): 'instance_type_id': 1} self.fake_instance_obj = fake_instance.fake_instance_obj(self.context, **instance_attr) - self.fake_instance = jsonutils.to_primitive(self.fake_instance_obj) - self.fake_volume_bdm = jsonutils.to_primitive( - fake_block_device.FakeDbBlockDeviceDict( + self.fake_volume_bdm = objects_block_dev.BlockDeviceMapping( + **fake_block_device.FakeDbBlockDeviceDict( {'source_type': 'volume', 'destination_type': 'volume', - 'instance_uuid': self.fake_instance['uuid'], + 'instance_uuid': self.fake_instance_obj.uuid, 'volume_id': 'fake-volume-id'})) - def test_serialized_instance_has_name(self): - self.assertIn('name', self.fake_instance) - def _test_compute_api(self, method, rpc_method, assert_dict=False, **kwargs): ctxt = context.RequestContext('fake_user', 'fake_project') @@ -60,41 +56,14 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): self.assertEqual(rpcapi.client.target.topic, CONF.compute_topic) orig_prepare = rpcapi.client.prepare - # TODO(danms): Remove this special case when we drop 3.x - if CONF.upgrade_levels.compute == '3.40': - base_version = '3.0' - else: - base_version = rpcapi.client.target.version + base_version = rpcapi.client.target.version expected_version = kwargs.pop('version', base_version) - nova_network = kwargs.pop('nova_network', False) expected_kwargs = kwargs.copy() - if ('requested_networks' in expected_kwargs and - expected_version == '3.23'): - expected_kwargs['requested_networks'] = [] - for requested_network in kwargs['requested_networks']: - if not nova_network: - expected_kwargs['requested_networks'].append( - (requested_network.network_id, - str(requested_network.address), - requested_network.port_id)) - else: - expected_kwargs['requested_networks'].append( - (requested_network.network_id, - str(requested_network.address))) if 'host_param' in expected_kwargs: expected_kwargs['host'] = expected_kwargs.pop('host_param') else: expected_kwargs.pop('host', None) - if 'legacy_limits' in expected_kwargs: - expected_kwargs['limits'] = expected_kwargs.pop('legacy_limits') - kwargs.pop('legacy_limits', None) - expected_kwargs.pop('destination', None) - - if 'mountpoint' in expected_kwargs and expected_version == '4.0': - # TODO(danms): Remove me when we drop 3.x - del expected_kwargs['mountpoint'] - del expected_kwargs['volume_id'] if assert_dict: expected_kwargs['instance'] = jsonutils.to_primitive( @@ -108,8 +77,6 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): kwargs['do_cast'] = False if 'host' in kwargs: host = kwargs['host'] - elif 'destination' in kwargs: - host = kwargs['destination'] elif 'instances' in kwargs: host = kwargs['instances'][0]['host'] else: @@ -160,8 +127,8 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): def test_attach_volume(self): self._test_compute_api('attach_volume', 'cast', - instance=self.fake_instance_obj, volume_id='id', - mountpoint='mp', bdm=self.fake_volume_bdm, version='4.0') + instance=self.fake_instance_obj, bdm=self.fake_volume_bdm, + version='4.0') def test_change_instance_metadata(self): self._test_compute_api('change_instance_metadata', 'cast', @@ -355,7 +322,7 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): def test_remove_volume_connection(self): self._test_compute_api('remove_volume_connection', 'call', - instance=self.fake_instance, volume_id='id', host='host', + instance=self.fake_instance_obj, volume_id='id', host='host', version='4.0') def test_rescue_instance(self):