diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index a49131b64d8e..3f92a731ae1e 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 @@ class ComputeTaskManager(base.Base): 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 @@ class ComputeTaskManager(base.Base): 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 @@ class ComputeTaskManager(base.Base): 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 @@ class ComputeTaskManager(base.Base): 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 @@ class ComputeTaskManager(base.Base): 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 @@ class ComputeTaskManager(base.Base): :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 @@ class ComputeTaskManager(base.Base): 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 @@ class ComputeTaskManager(base.Base): 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 706aff82ba54..f1a2bb489de7 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 @@ class ComputeTaskAPI(object): 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 9fbe01888bfd..d1bb0d03590f 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -20,6 +20,7 @@ import copy 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 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): 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 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): 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 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): 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 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): 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 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): 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 @@ class ConductorTaskRPCAPITestCase(_BaseTaskTestCase, 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."""