Skip to content

Commit

Permalink
Merge "update allocation in binding profile during migrate"
Browse files Browse the repository at this point in the history
  • Loading branch information
Zuul authored and openstack-gerrit committed Sep 6, 2019
2 parents 9651424 + a2984b6 commit a2b8146
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 42 deletions.
37 changes: 21 additions & 16 deletions nova/compute/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -4203,8 +4203,8 @@ def revert_resize(self, context, instance, migration, request_spec=None):
self.compute_rpcapi.finish_revert_resize(context, instance,
migration, migration.source_compute, request_spec)

def _finish_revert_resize_network_migrate_finish(self, context, instance,
migration):
def _finish_revert_resize_network_migrate_finish(
self, context, instance, migration, provider_mappings):
"""Causes port binding to be updated. In some Neutron or port
configurations - see NetworkModel.get_bind_time_events() - we
expect the vif-plugged event from Neutron immediately and wait for it.
Expand All @@ -4214,6 +4214,8 @@ def _finish_revert_resize_network_migrate_finish(self, context, instance,
:param context: The request context.
:param instance: The instance undergoing the revert resize.
:param migration: The Migration object of the resize being reverted.
:param provider_mappings: a dict of list of resource provider uuids
keyed by port uuid
:raises: eventlet.timeout.Timeout or
exception.VirtualInterfacePlugException.
"""
Expand All @@ -4238,9 +4240,8 @@ def _finish_revert_resize_network_migrate_finish(self, context, instance,
# the migration.dest_compute to source host at here.
with utils.temporary_mutation(
migration, dest_compute=migration.source_compute):
self.network_api.migrate_instance_finish(context,
instance,
migration)
self.network_api.migrate_instance_finish(
context, instance, migration, provider_mappings)
except eventlet.timeout.Timeout:
with excutils.save_and_reraise_exception():
LOG.error('Timeout waiting for Neutron events: %s', events,
Expand Down Expand Up @@ -4292,10 +4293,12 @@ def finish_revert_resize(
'migration_uuid': migration.uuid})
raise

provider_mappings = self._get_request_group_mapping(request_spec)

self.network_api.setup_networks_on_host(context, instance,
migration.source_compute)
self._finish_revert_resize_network_migrate_finish(
context, instance, migration)
context, instance, migration, provider_mappings)
network_info = self.network_api.get_instance_nw_info(context,
instance)

Expand Down Expand Up @@ -4746,7 +4749,7 @@ def _complete_volume_attachments(self, context, bdms):
context, bdm.attachment_id)

def _finish_resize(self, context, instance, migration, disk_info,
image_meta, bdms):
image_meta, bdms, request_spec):
resize_instance = False # indicates disks have been resized
old_instance_type_id = migration['old_instance_type_id']
new_instance_type_id = migration['new_instance_type_id']
Expand All @@ -4772,11 +4775,12 @@ def _finish_resize(self, context, instance, migration, disk_info,
# NOTE(tr3buchet): setup networks on destination host
self.network_api.setup_networks_on_host(context, instance,
migration.dest_compute)
provider_mappings = self._get_request_group_mapping(request_spec)

# For neutron, migrate_instance_finish updates port bindings for this
# host including any PCI devices claimed for SR-IOV ports.
self.network_api.migrate_instance_finish(context,
instance,
migration)
self.network_api.migrate_instance_finish(
context, instance, migration, provider_mappings)

network_info = self.network_api.get_instance_nw_info(context, instance)

Expand Down Expand Up @@ -4850,7 +4854,7 @@ def finish_resize(self, context, disk_info, image, instance,
"""
try:
self._finish_resize_helper(context, disk_info, image, instance,
migration)
migration, request_spec)
except Exception:
with excutils.save_and_reraise_exception():
# At this point, resize_instance (which runs on the source) has
Expand All @@ -4871,7 +4875,7 @@ def finish_resize(self, context, disk_info, image, instance,
context, instance, migration)

def _finish_resize_helper(self, context, disk_info, image, instance,
migration):
migration, request_spec):
"""Completes the migration process.
The caller must revert the instance's allocations if the migration
Expand All @@ -4883,7 +4887,8 @@ def _finish_resize_helper(self, context, disk_info, image, instance,
with self._error_out_instance_on_exception(context, instance):
image_meta = objects.ImageMeta.from_dict(image)
network_info = self._finish_resize(context, instance, migration,
disk_info, image_meta, bdms)
disk_info, image_meta, bdms,
request_spec)

# TODO(melwitt): We should clean up instance console tokens here. The
# instance is on a new host and will need to establish a new console
Expand Down Expand Up @@ -7242,9 +7247,9 @@ def post_live_migration_at_destination(self, context, instance,
migration = {'source_compute': instance.host,
'dest_compute': self.host,
'migration_type': 'live-migration'}
self.network_api.migrate_instance_finish(context,
instance,
migration)
# TODO(gibi): calculate and pass resource_provider_mapping
self.network_api.migrate_instance_finish(
context, instance, migration, provider_mappings=None)

network_info = self.network_api.get_instance_nw_info(context, instance)
self._notify_about_instance_usage(
Expand Down
9 changes: 5 additions & 4 deletions nova/network/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,8 @@ def migrate_instance_start(self, context, instance, migration):

self.network_rpcapi.migrate_instance_start(context, **args)

def migrate_instance_finish(self, context, instance, migration):
def migrate_instance_finish(
self, context, instance, migration, provider_mappings):
"""Finish migrating the network of an instance."""
flavor = instance.get_flavor()
args = dict(
Expand All @@ -524,9 +525,9 @@ def migrate_instance_finish(self, context, instance, migration):
def setup_instance_network_on_host(self, context, instance, host,
migration=None):
"""Setup network for specified instance on host."""
self.migrate_instance_finish(context, instance,
{'source_compute': None,
'dest_compute': host})
self.migrate_instance_finish(
context, instance, {'source_compute': None, 'dest_compute': host},
None)

def cleanup_instance_network_on_host(self, context, instance, host):
"""Cleanup network for specified instance on host."""
Expand Down
12 changes: 10 additions & 2 deletions nova/network/base_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,16 @@ def migrate_instance_start(self, context, instance, migration):
"""Start to migrate the network of an instance."""
raise NotImplementedError()

def migrate_instance_finish(self, context, instance, migration):
"""Finish migrating the network of an instance."""
def migrate_instance_finish(
self, context, instance, migration, provider_mappings):
"""Finish migrating the network of an instance.
:param context: The request context.
:param instance: nova.objects.instance.Instance object.
:param migration: nova.objects.migration.Migration object.
:param provider_mappings: a dict of list of resource provider uuids
keyed by port uuid
"""
raise NotImplementedError()

def setup_instance_network_on_host(self, context, instance, host,
Expand Down
40 changes: 31 additions & 9 deletions nova/network/neutronv2/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2774,11 +2774,12 @@ def migrate_instance_start(self, context, instance, migration):
'Error: %s', vif['id'], dest_host, resp.status_code,
resp.text)

def migrate_instance_finish(self, context, instance, migration):
def migrate_instance_finish(
self, context, instance, migration, provider_mappings):
"""Finish migrating the network of an instance."""
self._update_port_binding_for_instance(context, instance,
migration['dest_compute'],
migration=migration)
self._update_port_binding_for_instance(
context, instance, migration['dest_compute'], migration=migration,
provider_mappings=provider_mappings)

def add_network_to_project(self, context, project_id, network_uuid=None):
"""Force add a network to the project."""
Expand Down Expand Up @@ -3249,8 +3250,10 @@ def _get_pci_mapping_for_migration(self, instance, migration):
migration.get('status') == 'reverted')
return instance.migration_context.get_pci_mapping_for_migration(revert)

def _update_port_binding_for_instance(self, context, instance, host,
migration=None):
def _update_port_binding_for_instance(
self, context, instance, host, migration=None,
provider_mappings=None):

neutron = get_client(context, admin=True)
search_opts = {'device_id': instance.uuid,
'tenant_id': instance.project_id}
Expand All @@ -3269,9 +3272,6 @@ def _update_port_binding_for_instance(self, context, instance, host,
vif_type = p.get('binding:vif_type')
if (p.get(constants.BINDING_HOST_ID) != host or
vif_type in FAILED_VIF_TYPES):
# TODO(gibi): To support ports with resource request during
# server move operations we need to take care of 'allocation'
# key in the binding profile per binding.

updates[constants.BINDING_HOST_ID] = host
# If the host changed, the AZ could have also changed so we
Expand Down Expand Up @@ -3311,6 +3311,28 @@ def _update_port_binding_for_instance(self, context, instance, host,
reason=_("Unable to correlate PCI slot %s") %
pci_slot)

if p.get('resource_request'):
if not provider_mappings:
# NOTE(gibi): This should not happen as the API level
# minimum compute service version check ensures that the
# compute services already send the RequestSpec during
# the move operations between the source and the
# destination and the dest compute calculates the
# mapping based on that.
raise exception.PortUpdateFailed(
port_id=p['id'],
reason=_("Provider mappings wasn't provided for the "
"port with resource request"))

# NOTE(gibi): In the resource provider mapping there can be
# more than one RP fulfilling a request group. But resource
# requests of a Neutron port is always mapped to a
# numbered request group that is always fulfilled by one
# resource provider. So we only pass that single RP UUID here.
binding_profile[constants.ALLOCATION] = \
provider_mappings[p['id']][0]
updates[constants.BINDING_PROFILE] = binding_profile

port_updates.append((p['id'], updates))

# Avoid rolling back updates if we catch an error above.
Expand Down
2 changes: 1 addition & 1 deletion nova/tests/unit/compute/test_compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -4866,7 +4866,7 @@ def _instance_save1(expected_task_state=None):
mock_setup.assert_called_once_with(self.context, instance,
'fake-mini')
mock_net_mig.assert_called_once_with(self.context,
test.MatchType(objects.Instance), migration)
test.MatchType(objects.Instance), migration, None)
mock_get_nw.assert_called_once_with(self.context, instance)
mock_notify.assert_has_calls([
mock.call(self.context, instance, 'finish_resize.start',
Expand Down
13 changes: 8 additions & 5 deletions nova/tests/unit/compute/test_compute_mgr.py
Original file line number Diff line number Diff line change
Expand Up @@ -5263,7 +5263,8 @@ def _test_finish_revert_resize_network_migrate_finish(
source_compute='fake-source',
dest_compute='fake-dest')

def fake_migrate_instance_finish(context, instance, migration):
def fake_migrate_instance_finish(
context, instance, migration, mapping):
# NOTE(artom) This looks weird, but it's checking that the
# temporaty_mutation() context manager did its job.
self.assertEqual(migration.dest_compute, migration.source_compute)
Expand All @@ -5276,12 +5277,12 @@ def fake_migrate_instance_finish(context, instance, migration):
side_effect=fake_migrate_instance_finish)
) as (mock_wait, mock_migrate_instance_finish):
self.compute._finish_revert_resize_network_migrate_finish(
self.context, instance, migration)
self.context, instance, migration, mock.sentinel.mapping)
mock_wait.assert_called_once_with(
instance, events, deadline=CONF.vif_plugging_timeout,
error_callback=self.compute._neutron_failed_migration_callback)
mock_migrate_instance_finish.assert_called_once_with(
self.context, instance, migration)
self.context, instance, migration, mock.sentinel.mapping)

def test_finish_revert_resize_network_migrate_finish_wait(self):
"""Test that we wait for bind-time events if we have a hybrid-plugged
Expand Down Expand Up @@ -7410,7 +7411,8 @@ def test_revert_resize_instance_destroy_disks_non_shared_storage(self):
def test_finish_revert_resize_network_calls_order(self):
self.nw_info = None

def _migrate_instance_finish(context, instance, migration):
def _migrate_instance_finish(
context, instance, migration, provider_mappings):
# The migration.dest_compute is temporarily set to source_compute.
self.assertEqual(migration.source_compute, migration.dest_compute)
self.nw_info = 'nw_info'
Expand Down Expand Up @@ -8459,7 +8461,8 @@ def _do_test(mock_notify, post_live_migration_at_destination,
self.context, self.instance,
{'source_compute': cn_old,
'dest_compute': self.compute.host,
'migration_type': 'live-migration'})
'migration_type': 'live-migration'},
provider_mappings=None)
_get_instance_block_device_info.assert_called_once_with(
self.context, self.instance
)
Expand Down
7 changes: 4 additions & 3 deletions nova/tests/unit/network/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,14 +279,14 @@ def test_migrate_instance_finish_with_multihost(self):
arg1, arg2, expected = self._stub_migrate_instance_calls(
'migrate_instance_finish', True, info)
expected['host'] = 'fake_compute_dest'
self.network_api.migrate_instance_finish(self.context, arg1, arg2)
self.network_api.migrate_instance_finish(self.context, arg1, arg2, {})
self.assertEqual(info['kwargs'], expected)

def test_migrate_instance_finish_without_multihost(self):
info = {'kwargs': {}}
arg1, arg2, expected = self._stub_migrate_instance_calls(
'migrate_instance_finish', False, info)
self.network_api.migrate_instance_finish(self.context, arg1, arg2)
self.network_api.migrate_instance_finish(self.context, arg1, arg2, {})
self.assertEqual(info['kwargs'], expected)

def test_is_multi_host_instance_has_no_fixed_ip(self):
Expand Down Expand Up @@ -516,7 +516,8 @@ def test_setup_instance_network_on_host(self, fake_migrate_finish):
self.context, instance, 'fake_compute_source')
fake_migrate_finish.assert_called_once_with(
self.context, instance,
{'source_compute': None, 'dest_compute': 'fake_compute_source'})
{'source_compute': None, 'dest_compute': 'fake_compute_source'},
None)

@mock.patch('oslo_concurrency.lockutils.lock')
@mock.patch.object(api.API, '_get_instance_nw_info')
Expand Down
Loading

0 comments on commit a2b8146

Please sign in to comment.