diff --git a/nova/compute/manager.py b/nova/compute/manager.py index a706c7fae9e3..dbd63348a98d 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -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( diff --git a/nova/network/api.py b/nova/network/api.py index c8f46e6ce9ff..b379ea060092 100644 --- a/nova/network/api.py +++ b/nova/network/api.py @@ -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.""" diff --git a/nova/network/base_api.py b/nova/network/base_api.py index 2163e17d79f4..289e51f809c3 100644 --- a/nova/network/base_api.py +++ b/nova/network/base_api.py @@ -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, diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index 4a263342edfe..9b0aab0d1892 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -2749,11 +2749,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.""" @@ -3224,8 +3225,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} @@ -3244,9 +3247,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 @@ -3286,6 +3286,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. diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 95b36938e735..123c0ba1e6e4 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -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', diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 389cd4a961c6..1fa4eab60427 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -5250,7 +5250,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) @@ -5263,12 +5264,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 @@ -7397,7 +7398,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' @@ -8446,7 +8448,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 ) diff --git a/nova/tests/unit/network/test_api.py b/nova/tests/unit/network/test_api.py index ba73d4c89a22..6280f352b1f0 100644 --- a/nova/tests/unit/network/test_api.py +++ b/nova/tests/unit/network/test_api.py @@ -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') diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index faeb7436ef89..38a73ef69a72 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -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() @@ -6211,7 +6262,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'), @@ -6221,7 +6273,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)