Get a ReqSpec in evacuate API and pass it to scheduler
Since the RequestSpec object is now persisted in the API layer every time that we start an instance, we can now fetch it from the API DB and pass it thru the ComputeTask API to the conductor so it can directly call the scheduler with it. NOTE(sbauza): Yeah, it's ugly to see the ReqsSpec object being dehydrated to legacy dicts and then rehydrated by the next call, but I'll modify that in my next change. Partially-Implements: blueprint check-destination-on-migrations Change-Id: I7fe694175bb47f53d281bd62ac200f1c8416682b
This commit is contained in:
parent
07573aff23
commit
c46dde6d29
|
@ -3329,6 +3329,13 @@ class API(base.Base):
|
|||
compute_utils.notify_about_instance_usage(
|
||||
self.notifier, context, instance, "evacuate")
|
||||
|
||||
try:
|
||||
request_spec = objects.RequestSpec.get_by_instance_uuid(
|
||||
context, instance.uuid)
|
||||
except exception.RequestSpecNotFound:
|
||||
# Some old instances can still have no RequestSpec object attached
|
||||
# to them, we need to support the old way
|
||||
request_spec = None
|
||||
return self.compute_task_api.rebuild_instance(context,
|
||||
instance=instance,
|
||||
new_pass=admin_password,
|
||||
|
@ -3339,7 +3346,9 @@ class API(base.Base):
|
|||
bdms=None,
|
||||
recreate=True,
|
||||
on_shared_storage=on_shared_storage,
|
||||
host=host)
|
||||
host=host,
|
||||
request_spec=request_spec,
|
||||
)
|
||||
|
||||
def get_migrations(self, context, filters):
|
||||
"""Get all migrations for the given filters."""
|
||||
|
|
|
@ -127,7 +127,8 @@ class LocalComputeTaskAPI(object):
|
|||
def rebuild_instance(self, context, instance, orig_image_ref, image_ref,
|
||||
injected_files, new_pass, orig_sys_metadata,
|
||||
bdms, recreate=False, on_shared_storage=False,
|
||||
preserve_ephemeral=False, host=None, kwargs=None):
|
||||
preserve_ephemeral=False, host=None,
|
||||
request_spec=None, kwargs=None):
|
||||
# kwargs unused but required for cell compatibility.
|
||||
utils.spawn_n(self._manager.rebuild_instance, context,
|
||||
instance=instance,
|
||||
|
@ -140,7 +141,8 @@ class LocalComputeTaskAPI(object):
|
|||
recreate=recreate,
|
||||
on_shared_storage=on_shared_storage,
|
||||
host=host,
|
||||
preserve_ephemeral=preserve_ephemeral)
|
||||
preserve_ephemeral=preserve_ephemeral,
|
||||
request_spec=request_spec)
|
||||
|
||||
|
||||
class API(LocalAPI):
|
||||
|
@ -234,7 +236,8 @@ class ComputeTaskAPI(object):
|
|||
def rebuild_instance(self, context, instance, orig_image_ref, image_ref,
|
||||
injected_files, new_pass, orig_sys_metadata,
|
||||
bdms, recreate=False, on_shared_storage=False,
|
||||
preserve_ephemeral=False, host=None, kwargs=None):
|
||||
preserve_ephemeral=False, host=None,
|
||||
request_spec=None, kwargs=None):
|
||||
# kwargs unused but required for cell compatibility
|
||||
self.conductor_compute_rpcapi.rebuild_instance(context,
|
||||
instance=instance,
|
||||
|
@ -247,4 +250,5 @@ class ComputeTaskAPI(object):
|
|||
recreate=recreate,
|
||||
on_shared_storage=on_shared_storage,
|
||||
preserve_ephemeral=preserve_ephemeral,
|
||||
host=host)
|
||||
host=host,
|
||||
request_spec=request_spec)
|
||||
|
|
|
@ -144,7 +144,7 @@ class ComputeTaskManager(base.Base):
|
|||
may involve coordinating activities on multiple compute nodes.
|
||||
"""
|
||||
|
||||
target = messaging.Target(namespace='compute_task', version='1.11')
|
||||
target = messaging.Target(namespace='compute_task', version='1.12')
|
||||
|
||||
def __init__(self):
|
||||
super(ComputeTaskManager, self).__init__()
|
||||
|
@ -505,18 +505,32 @@ class ComputeTaskManager(base.Base):
|
|||
def rebuild_instance(self, context, instance, orig_image_ref, image_ref,
|
||||
injected_files, new_pass, orig_sys_metadata,
|
||||
bdms, recreate, on_shared_storage,
|
||||
preserve_ephemeral=False, host=None):
|
||||
preserve_ephemeral=False, host=None,
|
||||
request_spec=None):
|
||||
|
||||
with compute_utils.EventReporter(context, 'rebuild_server',
|
||||
instance.uuid):
|
||||
node = limits = None
|
||||
if not host:
|
||||
# NOTE(lcostantino): Retrieve scheduler filters for the
|
||||
# instance when the feature is available
|
||||
if not request_spec:
|
||||
# NOTE(sbauza): We were unable to find an original
|
||||
# RequestSpec object - probably because the instance is old
|
||||
# We need to mock that the old way
|
||||
filter_properties = {'ignore_hosts': [instance.host]}
|
||||
try:
|
||||
request_spec = scheduler_utils.build_request_spec(
|
||||
context, image_ref, [instance])
|
||||
else:
|
||||
# NOTE(sbauza): Augment the RequestSpec object by excluding
|
||||
# the source host for avoiding the scheduler to pick it
|
||||
request_spec.ignore_hosts = request_spec.ignore_hosts or []
|
||||
request_spec.ignore_hosts.append(instance.host)
|
||||
# TODO(sbauza): Provide directly the RequestSpec object
|
||||
# when _schedule_instances() and _set_vm_state_and_notify()
|
||||
# accept it
|
||||
filter_properties = request_spec.\
|
||||
to_legacy_filter_properties_dict()
|
||||
request_spec = request_spec.to_legacy_request_spec_dict()
|
||||
try:
|
||||
hosts = self._schedule_instances(
|
||||
context, request_spec, filter_properties)
|
||||
host_dict = hosts.pop(0)
|
||||
|
|
|
@ -268,7 +268,7 @@ class ComputeTaskAPI(object):
|
|||
1.9 - Converted requested_networks to NetworkRequestList object
|
||||
1.10 - Made migrate_server() and build_instances() send flavor objects
|
||||
1.11 - Added clean_shutdown to migrate_server()
|
||||
|
||||
1.12 - Added request_spec to rebuild_instance()
|
||||
"""
|
||||
|
||||
def __init__(self):
|
||||
|
@ -339,13 +339,23 @@ class ComputeTaskAPI(object):
|
|||
def rebuild_instance(self, ctxt, instance, new_pass, injected_files,
|
||||
image_ref, orig_image_ref, orig_sys_metadata, bdms,
|
||||
recreate=False, on_shared_storage=False, host=None,
|
||||
preserve_ephemeral=False, kwargs=None):
|
||||
cctxt = self.client.prepare(version='1.8')
|
||||
cctxt.cast(ctxt, 'rebuild_instance',
|
||||
instance=instance, new_pass=new_pass,
|
||||
injected_files=injected_files, image_ref=image_ref,
|
||||
orig_image_ref=orig_image_ref,
|
||||
orig_sys_metadata=orig_sys_metadata, bdms=bdms,
|
||||
recreate=recreate, on_shared_storage=on_shared_storage,
|
||||
preserve_ephemeral=preserve_ephemeral,
|
||||
host=host)
|
||||
preserve_ephemeral=False, request_spec=None, kwargs=None):
|
||||
version = '1.12'
|
||||
kw = {'instance': instance,
|
||||
'new_pass': new_pass,
|
||||
'injected_files': injected_files,
|
||||
'image_ref': image_ref,
|
||||
'orig_image_ref': orig_image_ref,
|
||||
'orig_sys_metadata': orig_sys_metadata,
|
||||
'bdms': bdms,
|
||||
'recreate': recreate,
|
||||
'on_shared_storage': on_shared_storage,
|
||||
'preserve_ephemeral': preserve_ephemeral,
|
||||
'host': host,
|
||||
'request_spec': request_spec,
|
||||
}
|
||||
if not self.client.can_send_version(version):
|
||||
version = '1.8'
|
||||
del kw['request_spec']
|
||||
cctxt = self.client.prepare(version=version)
|
||||
cctxt.cast(ctxt, 'rebuild_instance', **kw)
|
||||
|
|
|
@ -93,7 +93,7 @@ class EvacuateJsonTest(test_servers.ServersSampleBase):
|
|||
injected_files=mock.ANY, new_pass="MySecretPass",
|
||||
orig_sys_metadata=mock.ANY, bdms=mock.ANY, recreate=mock.ANY,
|
||||
on_shared_storage=False, preserve_ephemeral=mock.ANY,
|
||||
host='testHost')
|
||||
host='testHost', request_spec=mock.ANY)
|
||||
|
||||
@mock.patch('nova.conductor.manager.ComputeTaskManager.rebuild_instance')
|
||||
def test_server_evacuate_find_host(self, rebuild_mock):
|
||||
|
@ -109,7 +109,7 @@ class EvacuateJsonTest(test_servers.ServersSampleBase):
|
|||
injected_files=mock.ANY, new_pass="MySecretPass",
|
||||
orig_sys_metadata=mock.ANY, bdms=mock.ANY, recreate=mock.ANY,
|
||||
on_shared_storage=False, preserve_ephemeral=mock.ANY,
|
||||
host=None)
|
||||
host=None, request_spec=mock.ANY)
|
||||
|
||||
|
||||
class EvacuateJsonTestV214(EvacuateJsonTest):
|
||||
|
@ -130,7 +130,7 @@ class EvacuateJsonTestV214(EvacuateJsonTest):
|
|||
injected_files=mock.ANY, new_pass="MySecretPass",
|
||||
orig_sys_metadata=mock.ANY, bdms=mock.ANY, recreate=mock.ANY,
|
||||
on_shared_storage=None, preserve_ephemeral=mock.ANY,
|
||||
host='testHost')
|
||||
host='testHost', request_spec=mock.ANY)
|
||||
|
||||
@mock.patch('nova.conductor.manager.ComputeTaskManager.rebuild_instance')
|
||||
def test_server_evacuate_find_host(self, rebuild_mock):
|
||||
|
@ -145,4 +145,4 @@ class EvacuateJsonTestV214(EvacuateJsonTest):
|
|||
injected_files=mock.ANY, new_pass="MySecretPass",
|
||||
orig_sys_metadata=mock.ANY, bdms=mock.ANY, recreate=mock.ANY,
|
||||
on_shared_storage=None, preserve_ephemeral=mock.ANY,
|
||||
host=None)
|
||||
host=None, request_spec=mock.ANY)
|
||||
|
|
|
@ -10140,23 +10140,45 @@ class ComputeAPITestCase(BaseTestCase):
|
|||
instance = self._create_fake_instance_obj(services=True)
|
||||
self.assertIsNone(instance.task_state)
|
||||
|
||||
def fake_service_is_up(*args, **kwargs):
|
||||
return False
|
||||
ctxt = self.context.elevated()
|
||||
|
||||
fake_spec = objects.RequestSpec()
|
||||
|
||||
def fake_rebuild_instance(*args, **kwargs):
|
||||
instance.host = kwargs['host']
|
||||
instance.save()
|
||||
|
||||
self.stubs.Set(self.compute_api.servicegroup_api, 'service_is_up',
|
||||
fake_service_is_up)
|
||||
self.stubs.Set(self.compute_api.compute_task_api, 'rebuild_instance',
|
||||
fake_rebuild_instance)
|
||||
self.compute_api.evacuate(self.context.elevated(),
|
||||
@mock.patch.object(self.compute_api.compute_task_api,
|
||||
'rebuild_instance')
|
||||
@mock.patch.object(objects.RequestSpec,
|
||||
'get_by_instance_uuid')
|
||||
@mock.patch.object(self.compute_api.servicegroup_api, 'service_is_up')
|
||||
def do_test(service_is_up, get_by_instance_uuid, rebuild_instance):
|
||||
service_is_up.return_value = False
|
||||
get_by_instance_uuid.return_value = fake_spec
|
||||
rebuild_instance.side_effect = fake_rebuild_instance
|
||||
|
||||
self.compute_api.evacuate(ctxt,
|
||||
instance,
|
||||
host='fake_dest_host',
|
||||
on_shared_storage=True,
|
||||
admin_password=None)
|
||||
|
||||
rebuild_instance.assert_called_once_with(
|
||||
ctxt,
|
||||
instance=instance,
|
||||
new_pass=None,
|
||||
injected_files=None,
|
||||
image_ref=None,
|
||||
orig_image_ref=None,
|
||||
orig_sys_metadata=None,
|
||||
bdms=None,
|
||||
recreate=True,
|
||||
on_shared_storage=True,
|
||||
request_spec=fake_spec,
|
||||
host='fake_dest_host')
|
||||
do_test()
|
||||
|
||||
instance.refresh()
|
||||
self.assertEqual(instance.task_state, task_states.REBUILDING)
|
||||
self.assertEqual(instance.host, 'fake_dest_host')
|
||||
|
|
|
@ -316,13 +316,18 @@ class _BaseTaskTestCase(object):
|
|||
'recreate': False,
|
||||
'on_shared_storage': False,
|
||||
'preserve_ephemeral': False,
|
||||
'host': 'compute-host'}
|
||||
'host': 'compute-host',
|
||||
'request_spec': None}
|
||||
if update_args:
|
||||
rebuild_args.update(update_args)
|
||||
compute_rebuild_args = copy.deepcopy(rebuild_args)
|
||||
compute_rebuild_args['migration'] = migration
|
||||
compute_rebuild_args['node'] = node
|
||||
compute_rebuild_args['limits'] = limits
|
||||
|
||||
# Args that are passed in to the method but don't get passed to RPC
|
||||
compute_rebuild_args.pop('request_spec')
|
||||
|
||||
return rebuild_args, compute_rebuild_args
|
||||
|
||||
@mock.patch('nova.objects.Migration')
|
||||
|
@ -959,6 +964,53 @@ class _BaseTaskTestCase(object):
|
|||
instance=inst_obj,
|
||||
**compute_args)
|
||||
|
||||
def test_rebuild_instance_with_request_spec(self):
|
||||
inst_obj = self._create_fake_instance_obj()
|
||||
inst_obj.host = 'noselect'
|
||||
expected_host = 'thebesthost'
|
||||
expected_node = 'thebestnode'
|
||||
expected_limits = 'fake-limits'
|
||||
request_spec = {}
|
||||
filter_properties = {'ignore_hosts': [(inst_obj.host)]}
|
||||
fake_spec = objects.RequestSpec(ignore_hosts=[])
|
||||
augmented_spec = objects.RequestSpec(ignore_hosts=[inst_obj.host])
|
||||
rebuild_args, compute_args = self._prepare_rebuild_args(
|
||||
{'host': None, 'node': expected_node, 'limits': expected_limits,
|
||||
'request_spec': fake_spec})
|
||||
with test.nested(
|
||||
mock.patch.object(self.conductor_manager.compute_rpcapi,
|
||||
'rebuild_instance'),
|
||||
mock.patch.object(scheduler_utils, 'setup_instance_group',
|
||||
return_value=False),
|
||||
mock.patch.object(objects.RequestSpec, 'from_primitives',
|
||||
return_value=augmented_spec),
|
||||
mock.patch.object(self.conductor_manager.scheduler_client,
|
||||
'select_destinations',
|
||||
return_value=[{'host': expected_host,
|
||||
'nodename': expected_node,
|
||||
'limits': expected_limits}]),
|
||||
mock.patch.object(fake_spec, 'to_legacy_request_spec_dict',
|
||||
return_value=request_spec),
|
||||
mock.patch.object(fake_spec, 'to_legacy_filter_properties_dict',
|
||||
return_value=filter_properties),
|
||||
) as (rebuild_mock, sig_mock, fp_mock, select_dest_mock, to_reqspec,
|
||||
to_filtprops):
|
||||
self.conductor_manager.rebuild_instance(context=self.context,
|
||||
instance=inst_obj,
|
||||
**rebuild_args)
|
||||
to_reqspec.assert_called_once_with()
|
||||
to_filtprops.assert_called_once_with()
|
||||
fp_mock.assert_called_once_with(self.context, request_spec,
|
||||
filter_properties)
|
||||
select_dest_mock.assert_called_once_with(self.context,
|
||||
augmented_spec)
|
||||
compute_args['host'] = expected_host
|
||||
rebuild_mock.assert_called_once_with(self.context,
|
||||
instance=inst_obj,
|
||||
**compute_args)
|
||||
self.assertEqual('compute.instance.rebuild.scheduled',
|
||||
fake_notifier.NOTIFICATIONS[0].event_type)
|
||||
|
||||
|
||||
class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
|
||||
"""ComputeTaskManager Tests."""
|
||||
|
|
Loading…
Reference in New Issue