Merge "update allocation in binding profile during migrate"

This commit is contained in:
Zuul 2019-09-06 10:20:57 +00:00 committed by Gerrit Code Review
commit a2b814619b
8 changed files with 135 additions and 42 deletions

View File

@ -4203,8 +4203,8 @@ class ComputeManager(manager.Manager):
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.
@ -4214,6 +4214,8 @@ class ComputeManager(manager.Manager):
: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.
"""
@ -4238,9 +4240,8 @@ class ComputeManager(manager.Manager):
# 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,
@ -4292,10 +4293,12 @@ class ComputeManager(manager.Manager):
'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)
@ -4746,7 +4749,7 @@ class ComputeManager(manager.Manager):
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']
@ -4772,11 +4775,12 @@ class ComputeManager(manager.Manager):
# 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)
@ -4850,7 +4854,7 @@ class ComputeManager(manager.Manager):
"""
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
@ -4871,7 +4875,7 @@ class ComputeManager(manager.Manager):
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
@ -4883,7 +4887,8 @@ class ComputeManager(manager.Manager):
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
@ -7242,9 +7247,9 @@ class ComputeManager(manager.Manager):
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(

View File

@ -502,7 +502,8 @@ class API(base_api.NetworkAPI):
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(
@ -524,9 +525,9 @@ class API(base_api.NetworkAPI):
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."""

View File

@ -343,8 +343,16 @@ class NetworkAPI(base.Base):
"""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,

View File

@ -2774,11 +2774,12 @@ class API(base_api.NetworkAPI):
'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."""
@ -3249,8 +3250,10 @@ class API(base_api.NetworkAPI):
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}
@ -3269,9 +3272,6 @@ class API(base_api.NetworkAPI):
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
@ -3311,6 +3311,28 @@ class API(base_api.NetworkAPI):
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.

View File

@ -4866,7 +4866,7 @@ class ComputeTestCase(BaseTestCase,
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',

View File

@ -5263,7 +5263,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
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)
@ -5276,12 +5277,12 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
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
@ -7410,7 +7411,8 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
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'
@ -8459,7 +8461,8 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
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
)

View File

@ -279,14 +279,14 @@ class ApiTestCase(test.TestCase):
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):
@ -516,7 +516,8 @@ class ApiTestCase(test.TestCase):
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')

View File

@ -4531,6 +4531,57 @@ class TestNeutronv2WithMock(TestNeutronv2Base):
constants.BINDING_HOST_ID],
'new-host')
@mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
def test_update_port_bindings_for_instance_with_resource_req(
self, get_client_mock):
instance = fake_instance.fake_instance_obj(self.context)
fake_ports = {'ports': [
{'id': 'fake-port-1',
'binding:vnic_type': 'normal',
constants.BINDING_HOST_ID: 'old-host',
constants.BINDING_PROFILE:
{'allocation': uuids.source_compute_rp},
'resource_request': mock.sentinel.resource_request}]}
migration = {'status': 'confirmed',
'migration_type': "migration"}
list_ports_mock = mock.Mock(return_value=fake_ports)
get_client_mock.return_value.list_ports = list_ports_mock
self.api._update_port_binding_for_instance(
self.context, instance, 'new-host', migration,
{'fake-port-1': [uuids.dest_compute_rp]})
get_client_mock.return_value.update_port.assert_called_once_with(
'fake-port-1',
{'port': {'device_owner': 'compute:None',
'binding:profile': {'allocation': uuids.dest_compute_rp},
'binding:host_id': 'new-host'}})
@mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
def test_update_port_bindings_for_instance_with_resource_req_no_mapping(
self, get_client_mock):
instance = fake_instance.fake_instance_obj(self.context)
fake_ports = {'ports': [
{'id': 'fake-port-1',
'binding:vnic_type': 'normal',
constants.BINDING_HOST_ID: 'old-host',
constants.BINDING_PROFILE:
{'allocation': uuids.source_compute_rp},
'resource_request': mock.sentinel.resource_request}]}
migration = {'status': 'confirmed',
'migration_type': "migration"}
list_ports_mock = mock.Mock(return_value=fake_ports)
get_client_mock.return_value.list_ports = list_ports_mock
ex = self.assertRaises(
exception.PortUpdateFailed,
self.api._update_port_binding_for_instance, self.context,
instance, 'new-host', migration, provider_mappings=None)
self.assertIn(
"Provider mappings wasn't provided for the port with resource "
"request", six.text_type(ex))
def test_get_pci_mapping_for_migration(self):
instance = fake_instance.fake_instance_obj(self.context)
instance.migration_context = objects.MigrationContext()
@ -6262,7 +6313,8 @@ class TestNeutronv2Portbinding(TestNeutronv2Base):
'migrate_instance_finish',
self.context,
instance,
migration)
migration,
{})
def test_migrate_instance_finish_binding_true_exception(self):
migration = {'source_compute': self.instance.get('host'),
@ -6272,7 +6324,8 @@ class TestNeutronv2Portbinding(TestNeutronv2Base):
'migrate_instance_finish',
self.context,
instance,
migration)
migration,
{})
def test_setup_instance_network_on_host_true(self):
instance = self._fake_instance_object(self.instance)