From b3cf0ab1ed5455e1557b0b2a047e80e0cac83594 Mon Sep 17 00:00:00 2001 From: Kevin_Zheng Date: Tue, 25 Apr 2017 16:47:29 +0800 Subject: [PATCH] Support tag instances when boot(3/4) This is the third patch of the series, this patch adds a new rpc version to ComputeTaskManager to accept tags as a parameter in schedule_and_build_instances() Change-Id: Iac54b9627cb4398e45f1f15a2f4e7d6f86242093 Implemetes: blueprint support-tag-instance-when-boot --- nova/conductor/manager.py | 35 +++++- nova/conductor/rpcapi.py | 19 ++- nova/tests/unit/conductor/test_conductor.py | 128 ++++++++++++++++++-- 3 files changed, 162 insertions(+), 20 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index a49131b64d8..3f92a731ae1 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -218,7 +218,7 @@ class ComputeTaskManager(base.Base): may involve coordinating activities on multiple compute nodes. """ - target = messaging.Target(namespace='compute_task', version='1.16') + target = messaging.Target(namespace='compute_task', version='1.17') def __init__(self): super(ComputeTaskManager, self).__init__() @@ -835,6 +835,16 @@ def _create_block_device_mapping(self, cell, instance_type, instance_uuid, bdm.update_or_create() return instance_block_device_mapping + def _create_tags(self, context, instance_uuid, tags): + """Create the Tags objects in the db.""" + if tags: + tag_list = [tag.tag for tag in tags] + instance_tags = objects.TagList.create( + context, instance_uuid, tag_list) + return instance_tags + else: + return tags + def _bury_in_cell0(self, context, request_spec, exc, build_requests=None, instances=None): """Ensure all provided build_requests and instances end up in cell0. @@ -901,7 +911,8 @@ def _bury_in_cell0(self, context, request_spec, exc, def schedule_and_build_instances(self, context, build_requests, request_specs, image, admin_password, injected_files, - requested_networks, block_device_mapping): + requested_networks, block_device_mapping, + tags=None): # Add all the UUIDs for the instances instance_uuids = [spec.instance_uuid for spec in request_specs] try: @@ -971,6 +982,7 @@ def schedule_and_build_instances(self, context, build_requests, want_result=False) instance_bdms = self._create_block_device_mapping( cell, instance.flavor, instance.uuid, block_device_mapping) + instance_tags = self._create_tags(cctxt, instance.uuid, tags) # Update mapping for instance. Normally this check is guarded by # a try/except but if we're here we know that a newer nova-api @@ -981,7 +993,8 @@ def schedule_and_build_instances(self, context, build_requests, inst_mapping.save() if not self._delete_build_request( - build_request, instance, cell, instance_bdms): + context, build_request, instance, cell, instance_bdms, + instance_tags): # The build request was deleted before/during scheduling so # the instance is gone and we don't have anything to build for # this one. @@ -1006,13 +1019,15 @@ def schedule_and_build_instances(self, context, build_requests, host=host['host'], node=host['nodename'], limits=host['limits']) - def _delete_build_request(self, build_request, instance, cell, - instance_bdms): + def _delete_build_request(self, context, build_request, instance, cell, + instance_bdms, instance_tags): """Delete a build request after creating the instance in the cell. This method handles cleaning up the instance in case the build request is already deleted by the time we try to delete it. + :param context: the context of the request being handled + :type context: nova.context.RequestContext :param build_request: the build request to delete :type build_request: nova.objects.BuildRequest :param instance: the instance created from the build_request @@ -1021,6 +1036,8 @@ def _delete_build_request(self, build_request, instance, cell, :type cell: nova.objects.CellMapping :param instance_bdms: list of block device mappings for the instance :type instance_bdms: nova.objects.BlockDeviceMappingList + :param instance_tags: list of tags for the instance + :type instance_tags: nova.objects.TagList :returns: True if the build request was successfully deleted, False if the build request was already deleted and the instance is now gone. """ @@ -1029,7 +1046,7 @@ def _delete_build_request(self, build_request, instance, cell, except exception.BuildRequestNotFound: # This indicates an instance deletion request has been # processed, and the build should halt here. Clean up the - # bdm and instance record. + # bdm, tags and instance record. with obj_target_cell(instance, cell) as cctxt: with compute_utils.notify_about_instance_delete( self.notifier, cctxt, instance): @@ -1051,5 +1068,11 @@ def _delete_build_request(self, build_request, instance, cell, bdm.destroy() except exception.ObjectActionError: pass + if instance_tags: + with try_target_cell(context, cell) as target_ctxt: + try: + objects.TagList.destroy(target_ctxt, instance.uuid) + except exception.InstanceNotFound: + pass return False return True diff --git a/nova/conductor/rpcapi.py b/nova/conductor/rpcapi.py index 706aff82ba5..f1a2bb489de 100644 --- a/nova/conductor/rpcapi.py +++ b/nova/conductor/rpcapi.py @@ -273,6 +273,7 @@ class ComputeTaskAPI(object): 1.14 - Added request_spec to unshelve_instance() 1.15 - Added live_migrate_instance 1.16 - Added schedule_and_build_instances + 1.17 - Added tags to schedule_and_build_instances() """ def __init__(self): @@ -353,18 +354,24 @@ def build_instances(self, context, instances, image, filter_properties, cctxt.cast(context, 'build_instances', **kw) def schedule_and_build_instances(self, context, build_requests, - request_specs, - image, admin_password, injected_files, - requested_networks, - block_device_mapping): - version = '1.16' + request_specs, + image, admin_password, injected_files, + requested_networks, + block_device_mapping, + tags=None): + version = '1.17' kw = {'build_requests': build_requests, 'request_specs': request_specs, 'image': jsonutils.to_primitive(image), 'admin_password': admin_password, 'injected_files': injected_files, 'requested_networks': requested_networks, - 'block_device_mapping': block_device_mapping} + 'block_device_mapping': block_device_mapping, + 'tags': tags} + + if not self.client.can_send_version(version): + version = '1.16' + del kw['tags'] cctxt = self.client.prepare(version=version) cctxt.cast(context, 'schedule_and_build_instances', **kw) diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 9fbe01888bf..d1bb0d03590 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -20,6 +20,7 @@ import mock from mox3 import mox import oslo_messaging as messaging +from oslo_serialization import jsonutils from oslo_utils import timeutils from oslo_versionedobjects import exception as ovo_exc import six @@ -1352,19 +1353,21 @@ def setUp(self): tag='')) params['block_device_mapping'] = objects.BlockDeviceMappingList( objects=[bdm]) + tag = objects.Tag(self.ctxt, tag='tag1') + params['tags'] = objects.TagList(objects=[tag]) self.params = params @mock.patch('nova.availability_zones.get_host_availability_zone') @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance') @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations') - def test_schedule_and_build_instances(self, select_destinations, - build_and_run_instance, - get_az): + def _do_schedule_and_build_instances_test(self, params, + select_destinations, + build_and_run_instance, + get_az): select_destinations.return_value = [{'host': 'fake-host', 'nodename': 'fake-nodename', 'limits': None}] get_az.return_value = 'myaz' - params = self.params details = {} def _build_and_run_instance(ctxt, *args, **kwargs): @@ -1382,24 +1385,53 @@ def _build_and_run_instance(ctxt, *args, **kwargs): instance_uuid = details['instance'].uuid bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( - self.context, instance_uuid) + self.ctxt, instance_uuid) ephemeral = list(filter(block_device.new_format_is_ephemeral, bdms)) self.assertEqual(1, len(ephemeral)) swap = list(filter(block_device.new_format_is_swap, bdms)) self.assertEqual(0, len(swap)) self.assertEqual(1, ephemeral[0].volume_size) + return instance_uuid - cells = objects.CellMappingList.get_all(self.context) + def test_schedule_and_build_instances(self): + instance_uuid = self._do_schedule_and_build_instances_test( + self.params) + cells = objects.CellMappingList.get_all(self.ctxt) # NOTE(danms): Assert that we created the InstanceAction in the # correct cell + # NOTE(Kevin Zheng): Also assert tags in the correct cell for cell in cells: - with context.target_cell(self.context, cell) as cctxt: + with context.target_cell(self.ctxt, cell) as cctxt: actions = objects.InstanceActionList.get_by_instance_uuid( cctxt, instance_uuid) if cell.name == 'cell1': self.assertEqual(1, len(actions)) + tags = objects.TagList.get_by_resource_id( + cctxt, instance_uuid) + self.assertEqual(1, len(tags)) + else: + self.assertEqual(0, len(actions)) + + def test_schedule_and_build_instances_no_tags_provided(self): + params = copy.deepcopy(self.params) + del params['tags'] + instance_uuid = self._do_schedule_and_build_instances_test(params) + cells = objects.CellMappingList.get_all(self.ctxt) + + # NOTE(danms): Assert that we created the InstanceAction in the + # correct cell + # NOTE(Kevin Zheng): Also assert tags in the correct cell + for cell in cells: + with context.target_cell(self.ctxt, cell) as cctxt: + actions = objects.InstanceActionList.get_by_instance_uuid( + cctxt, instance_uuid) + if cell.name == 'cell1': + self.assertEqual(1, len(actions)) + tags = objects.TagList.get_by_resource_id( + cctxt, instance_uuid) + self.assertEqual(0, len(tags)) else: self.assertEqual(0, len(actions)) @@ -1483,6 +1515,9 @@ def test_schedule_and_build_scheduler_failure(self, select_destinations): self.assertEqual('error', instance.vm_state) self.assertIsNone(instance.task_state) + @mock.patch('nova.conductor.manager.try_target_cell') + @mock.patch('nova.objects.TagList.destroy') + @mock.patch('nova.objects.TagList.create') @mock.patch('nova.compute.utils.notify_about_instance_usage') @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance') @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations') @@ -1494,7 +1529,10 @@ def test_schedule_and_build_delete_during_scheduling(self, target_cell, br_destroy, select_destinations, build_and_run, - notify): + notify, + taglist_create, + taglist_destroy, + try_target): br_destroy.side_effect = exc.BuildRequestNotFound(uuid='foo') self.start_service('compute', host='fake-host') @@ -1504,10 +1542,14 @@ def test_schedule_and_build_delete_during_scheduling(self, target_cell, targeted_context = mock.MagicMock( autospec='nova.context.RequestContext') target_cell.return_value.__enter__.return_value = targeted_context + try_target.return_value.__enter__.return_value = targeted_context + taglist_create.return_value = self.params['tags'] self.conductor.schedule_and_build_instances(**self.params) self.assertFalse(build_and_run.called) self.assertFalse(bury.called) self.assertTrue(br_destroy.called) + taglist_destroy.assert_called_once_with( + targeted_context, self.params['build_requests'][0].instance_uuid) test_utils.assert_instance_delete_notification_by_uuid( notify, self.params['build_requests'][0].instance_uuid, @@ -2375,6 +2417,76 @@ def test(self, context, instance): test(None, ctxt, inst)) mock_im.assert_called_once_with(ctxt, inst.uuid) + def test_schedule_and_build_instances_with_tags(self): + + build_request = fake_build_request.fake_req_obj(self.context) + request_spec = objects.RequestSpec( + instance_uuid=build_request.instance_uuid) + image = {'fake_data': 'should_pass_silently'} + admin_password = 'fake_password' + injected_file = 'fake' + requested_network = None + block_device_mapping = None + tags = ['fake_tag'] + version = '1.17' + cctxt_mock = mock.MagicMock() + + @mock.patch.object(self.conductor.client, 'can_send_version', + return_value=True) + @mock.patch.object(self.conductor.client, 'prepare', + return_value=cctxt_mock) + def _test(prepare_mock, can_send_mock): + self.conductor.schedule_and_build_instances( + self.context, build_request, request_spec, image, + admin_password, injected_file, requested_network, + block_device_mapping, tags=tags) + prepare_mock.assert_called_once_with(version=version) + kw = {'build_requests': build_request, + 'request_specs': request_spec, + 'image': jsonutils.to_primitive(image), + 'admin_password': admin_password, + 'injected_files': injected_file, + 'requested_networks': requested_network, + 'block_device_mapping': block_device_mapping, + 'tags': tags} + cctxt_mock.cast.assert_called_once_with( + self.context, 'schedule_and_build_instances', **kw) + _test() + + def test_schedule_and_build_instances_with_tags_cannot_send(self): + + build_request = fake_build_request.fake_req_obj(self.context) + request_spec = objects.RequestSpec( + instance_uuid=build_request.instance_uuid) + image = {'fake_data': 'should_pass_silently'} + admin_password = 'fake_password' + injected_file = 'fake' + requested_network = None + block_device_mapping = None + tags = ['fake_tag'] + cctxt_mock = mock.MagicMock() + + @mock.patch.object(self.conductor.client, 'can_send_version', + return_value=False) + @mock.patch.object(self.conductor.client, 'prepare', + return_value=cctxt_mock) + def _test(prepare_mock, can_send_mock): + self.conductor.schedule_and_build_instances( + self.context, build_request, request_spec, image, + admin_password, injected_file, requested_network, + block_device_mapping, tags=tags) + prepare_mock.assert_called_once_with(version='1.16') + kw = {'build_requests': build_request, + 'request_specs': request_spec, + 'image': jsonutils.to_primitive(image), + 'admin_password': admin_password, + 'injected_files': injected_file, + 'requested_networks': requested_network, + 'block_device_mapping': block_device_mapping} + cctxt_mock.cast.assert_called_once_with( + self.context, 'schedule_and_build_instances', **kw) + _test() + class ConductorTaskAPITestCase(_BaseTaskTestCase, test_compute.BaseTestCase): """Compute task API Tests."""