Skip to content

Commit

Permalink
Support tag instances when boot(3/4)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
Kevin_Zheng committed Jul 7, 2017
1 parent 9261b85 commit b3cf0ab
Show file tree
Hide file tree
Showing 3 changed files with 162 additions and 20 deletions.
35 changes: 29 additions & 6 deletions nova/conductor/manager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__()
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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.
"""
Expand All @@ -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):
Expand All @@ -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
19 changes: 13 additions & 6 deletions nova/conductor/rpcapi.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand Down
128 changes: 120 additions & 8 deletions nova/tests/unit/conductor/test_conductor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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))

Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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,
Expand Down Expand Up @@ -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."""
Expand Down

0 comments on commit b3cf0ab

Please sign in to comment.