womax/nova
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Use the RequestSpec when getting scheduler_hints in compute

Browse Source 
For the first chosen host during an instance build, conductor
will pass both filter_properties and request_spec and they
both have scheduler_hints.

We want to eventually get rid of the legacy filter_properties,
so this change uses the request_spec, if available, to get the
scheduler_hints in the compute during the build. These are only
used when validating server group (anti-)affinity policies.

If the request_spec isn't available, because we rescheduled to
another host for example, then we fallback to the filter_properties
to get the hints.

Part of blueprint request-spec-use-by-compute

Change-Id: I49ffebcd129990f1835f404d98b51732a32171eb
This commit is contained in:
Matt Riedemann
2017-10-26 12:16:00 -04:00
parent 8fd4f9a029
commit 1b2deeb7e6
2 changed files with 86 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nova/compute/manager.py
+35 -6
View File
@@ -1284,7 +1284,7 @@ class ComputeManager(manager.Manager):
return [_decode(f) for f in injected_files]
def _validate_instance_group_policy(self, context, instance,
filter_properties):
scheduler_hints):
# NOTE(russellb) Instance group policy is enforced by the scheduler.
# However, there is a race condition with the enforcement of
# the policy. Since more than one instance may be scheduled at the
@@ -1293,12 +1293,16 @@ class ComputeManager(manager.Manager):
# multiple instances with an affinity policy could end up on different
# hosts. This is a validation step to make sure that starting the
# instance here doesn't violate the policy.
scheduler_hints = filter_properties.get('scheduler_hints') or {}
group_hint = scheduler_hints.get('group')
if not group_hint:
return
# The RequestSpec stores scheduler_hints as key=list pairs so we need
# to check the type on the value and pull the single entry out. The
# API request schema validates that the 'group' hint is a single value.
if isinstance(group_hint, list):
group_hint = group_hint[0]
@utils.synchronized(group_hint)
def _do_validation(context, instance, group_hint):
group = objects.InstanceGroup.get_by_hint(context, group_hint)
@@ -1844,7 +1848,7 @@ class ComputeManager(manager.Manager):
self._build_and_run_instance(context, instance, image,
decoded_files, admin_password, requested_networks,
security_groups, block_device_mapping, node, limits,
filter_properties)
filter_properties, request_spec)
LOG.info('Took %0.2f seconds to build instance.',
timer.elapsed(), instance=instance)
return build_results.ACTIVE
@@ -1893,6 +1897,9 @@ class ComputeManager(manager.Manager):
instance.task_state = task_states.SCHEDULING
instance.save()
# TODO(mriedem): Pass the request_spec back to conductor so that
# it gets to the next chosen host during the reschedule and we
# can hopefully eventually get rid of the legacy filter_properties.
self.compute_task_api.build_instances(context, [instance],
image, filter_properties, admin_password,
injected_files, requested_networks, security_groups,
@@ -1950,9 +1957,29 @@ class ComputeManager(manager.Manager):
return True
return False
@staticmethod
def _get_scheduler_hints(filter_properties, request_spec=None):
"""Helper method to get scheduler hints.
This method prefers to get the hints out of the request spec, but that
might not be provided. Conductor will pass request_spec down to the
first compute chosen for a build but older computes will not pass
the request_spec to conductor's build_instances method for a
a reschedule, so if we're on a host via a retry, request_spec may not
be provided so we need to fallback to use the filter_properties
to get scheduler hints.
"""
hints = {}
if request_spec is not None and 'scheduler_hints' in request_spec:
hints = request_spec.scheduler_hints
if not hints:
hints = filter_properties.get('scheduler_hints') or {}
return hints
def _build_and_run_instance(self, context, instance, image, injected_files,
admin_password, requested_networks, security_groups,
block_device_mapping, node, limits, filter_properties):
block_device_mapping, node, limits, filter_properties,
request_spec=None):
image_name = image.get('name')
self._notify_about_instance_usage(context, instance, 'create.start',
@@ -1970,13 +1997,15 @@ class ComputeManager(manager.Manager):
self._check_device_tagging(requested_networks, block_device_mapping)
try:
scheduler_hints = self._get_scheduler_hints(filter_properties,
request_spec)
rt = self._get_resource_tracker()
with rt.instance_claim(context, instance, node, limits):
# NOTE(russellb) It's important that this validation be done
# *after* the resource tracker instance claim, as that is where
# the host is set on the instance.
self._validate_instance_group_policy(context, instance,
filter_properties)
scheduler_hints)
image_meta = objects.ImageMeta.from_dict(image)
with self._build_resources(context, instance,
requested_networks, security_groups, image_meta,
nova/tests/unit/compute/test_compute_mgr.py
+51 -16
View File
@@ -4193,6 +4193,31 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
mock.call(self.context, inst_obj, 'fake-mini',
action='soft_delete', phase='end')])
def test_get_scheduler_hints(self):
# 1. No hints and no request_spec.
self.assertEqual({}, self.compute._get_scheduler_hints({}))
# 2. Hints come from the filter_properties.
hints = {'foo': 'bar'}
filter_properties = {'scheduler_hints': hints}
self.assertEqual(
hints, self.compute._get_scheduler_hints(filter_properties))
# 3. Hints come from filter_properties because reqspec is empty.
reqspec = objects.RequestSpec.from_primitives(self.context, {}, {})
self.assertEqual(
hints, self.compute._get_scheduler_hints(
filter_properties, reqspec))
# 4. Hints come from the request spec.
reqspec_hints = {'boo': 'baz'}
reqspec = objects.RequestSpec.from_primitives(
self.context, {}, {'scheduler_hints': reqspec_hints})
# The RequestSpec unconditionally stores hints as a key=list
# unlike filter_properties which just stores whatever came in from
# the API request.
expected_reqspec_hints = {'boo': ['baz']}
self.assertDictEqual(
expected_reqspec_hints, self.compute._get_scheduler_hints(
filter_properties, reqspec))
class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
def setUp(self):
@@ -4312,7 +4337,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.image, self.injected_files, self.admin_pass,
self.requested_networks, self.security_groups,
self.block_device_mapping, self.node, self.limits,
self.filter_properties)
self.filter_properties, {})
# This test when sending an icehouse compatible rpc call to juno compute
# node, NetworkRequest object can load from three items tuple.
@@ -4378,7 +4403,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.image, self.injected_files, self.admin_pass,
self.requested_networks, self.security_groups,
self.block_device_mapping, self.node, self.limits,
self.filter_properties)
self.filter_properties, {})
mock_clean_net.assert_called_once_with(self.context, self.instance,
self.requested_networks)
mock_clean_vol.assert_called_once_with(self.context,
@@ -4427,7 +4452,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.image, self.injected_files, self.admin_pass,
self.requested_networks, self.security_groups,
self.block_device_mapping, self.node, self.limits,
self.filter_properties)
self.filter_properties, {})
mock_clean.assert_called_once_with(self.context, self.instance,
self.compute.host)
mock_nil.assert_called_once_with(self.instance)
@@ -4512,7 +4537,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.image, self.injected_files, self.admin_pass,
self.requested_networks, self.security_groups,
self.block_device_mapping, self.node, self.limits,
self.filter_properties)
self.filter_properties, {})
mock_cleanup_network.assert_called_once_with(
self.context, instance, self.compute.host)
mock_build_ins.assert_called_once_with(self.context,
@@ -4567,7 +4592,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.image, self.injected_files, self.admin_pass,
self.requested_networks, self.security_groups,
self.block_device_mapping, self.node, self.limits,
self.filter_properties)
self.filter_properties, {})
mock_cleanup_network.assert_called_once_with(
self.context, instance, self.requested_networks)
mock_build_ins.assert_called_once_with(self.context,
@@ -4611,7 +4636,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
mock_build_run.assert_called_once_with(self.context, self.instance,
self.image, self.injected_files, self.admin_pass,
self.requested_networks, self.security_groups,
self.block_device_mapping, self.node, self.limits, {})
self.block_device_mapping, self.node, self.limits, {}, {})
mock_clean_net.assert_called_once_with(self.context, self.instance,
self.requested_networks)
mock_clean_vol.assert_called_once_with(self.context,
@@ -4665,7 +4690,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.image, self.injected_files, self.admin_pass,
self.requested_networks, self.security_groups,
self.block_device_mapping, self.node, self.limits,
self.filter_properties)
self.filter_properties, {})
mock_deallocate.assert_called_once_with(self.instance)
mock_clean_inst.assert_called_once_with(self.context, self.instance,
self.compute.host)
@@ -4713,7 +4738,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.image, self.injected_files, self.admin_pass,
self.requested_networks, self.security_groups,
self.block_device_mapping, self.node, self.limits,
self.filter_properties)
self.filter_properties, {})
mock_deallocate.assert_called_once_with(self.instance)
mock_clean.assert_called_once_with(self.context, self.instance,
self.requested_networks)
@@ -4773,7 +4798,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.image, self.injected_files, self.admin_pass,
self.requested_networks, self.security_groups,
self.block_device_mapping, self.node, self.limits,
self.filter_properties)
self.filter_properties, {})
mock_clean_net.assert_called_once_with(self.context, self.instance,
self.requested_networks)
@@ -5034,7 +5059,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.limits, self.filter_properties)
_validate_instance_group_policy.assert_called_once_with(
self.context, self.instance, self.filter_properties)
self.context, self.instance, {})
_build_networks_for_instance.assert_has_calls(
[mock.call(self.context, self.instance,
self.requested_networks, self.security_groups)])
@@ -5188,23 +5213,33 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
@mock.patch('nova.objects.InstanceGroup.get_by_hint')
def test_validate_policy_honors_workaround_disabled(self, mock_get):
instance = objects.Instance(uuid=uuids.instance)
filter_props = {'scheduler_hints': {'group': 'foo'}}
hints = {'group': 'foo'}
mock_get.return_value = objects.InstanceGroup(policies=[])
self.compute._validate_instance_group_policy(self.context,
instance,
filter_props)
instance, hints)
mock_get.assert_called_once_with(self.context, 'foo')
@mock.patch('nova.objects.InstanceGroup.get_by_hint')
def test_validate_policy_honors_workaround_enabled(self, mock_get):
self.flags(disable_group_policy_check_upcall=True, group='workarounds')
instance = objects.Instance(uuid=uuids.instance)
filter_props = {'scheduler_hints': {'group': 'foo'}}
hints = {'group': 'foo'}
self.compute._validate_instance_group_policy(self.context,
instance,
filter_props)
instance, hints)
self.assertFalse(mock_get.called)
@mock.patch('nova.objects.InstanceGroup.get_by_hint')
def test_validate_instance_group_policy_handles_hint_list(self, mock_get):
"""Tests that _validate_instance_group_policy handles getting
scheduler_hints from a RequestSpec which stores the hints as a key=list
pair.
"""
instance = objects.Instance(uuid=uuids.instance)
hints = {'group': [uuids.group_hint]}
self.compute._validate_instance_group_policy(self.context,
instance, hints)
mock_get.assert_called_once_with(self.context, uuids.group_hint)
def test_failed_bdm_prep_from_delete_raises_unexpected(self):
with test.nested(
mock.patch.object(self.compute,