diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index bf70c777d62a..ceb2215e4bdd 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -2491,10 +2491,73 @@ class API(base_api.NetworkAPI): client.update_floatingip(fip['id'], {'floatingip': {'port_id': None}}) def migrate_instance_start(self, context, instance, migration): - """Start to migrate the network of an instance.""" - # NOTE(wenjianhn): just pass to make migrate instance doesn't - # raise for now. - pass + """Start to migrate the network of an instance. + + If the instance has port bindings on the destination compute host, + they are activated in this method which will atomically change the + source compute host port binding to inactive and also change the port + "binding:host_id" attribute to the destination host. + + If there are no binding resources for the attached ports on the given + destination host, this method is a no-op. + + :param context: The user request context. + :param instance: The instance being migrated. + :param migration: dict with required keys:: + + "source_compute": The name of the source compute host. + "dest_compute": The name of the destination compute host. + + :raises: nova.exception.PortBindingActivationFailed if any port binding + activation fails + """ + if not self.supports_port_binding_extension(context): + # If neutron isn't new enough yet for the port "binding-extended" + # API extension, we just no-op. The port binding host will be + # be updated in migrate_instance_finish, which is functionally OK, + # it's just not optimal. + LOG.debug('Neutron is not new enough to perform early destination ' + 'host port binding activation. Port bindings will be ' + 'updated later.', instance=instance) + return + + client = _get_ksa_client(context, admin=True) + dest_host = migration['dest_compute'] + for vif in instance.get_network_info(): + # Not all compute migration flows use the port binding-extended + # API yet, so first check to see if there is a binding for the + # port and destination host. + resp = client.get('/v2.0/ports/%s/bindings/%s' % + (vif['id'], dest_host), raise_exc=False) + if resp: + if resp.json()['binding']['status'] != 'ACTIVE': + self.activate_port_binding(context, vif['id'], dest_host) + # TODO(mriedem): Do we need to call + # _clear_migration_port_profile? migrate_instance_finish + # would normally take care of clearing the "migrating_to" + # attribute on each port when updating the port's + # binding:host_id to point to the destination host. + else: + # We might be racing with another thread that's handling + # post-migrate operations and already activated the port + # binding for the destination host. + LOG.debug('Port %s binding to destination host %s is ' + 'already ACTIVE.', vif['id'], dest_host, + instance=instance) + elif resp.status_code == 404: + # If there is no port binding record for the destination host, + # we can safely assume none of the ports attached to the + # instance are using the binding-extended API in this flow and + # exit early. + return + else: + # We don't raise an exception here because we assume that + # port bindings will be updated correctly when + # migrate_instance_finish runs. + LOG.error('Unexpected error trying to get binding info ' + 'for port %s and destination host %s. Code: %s. ' + 'Error: %s', vif['id'], dest_host, resp.status_code, + resp.text) def migrate_instance_finish(self, context, instance, migration): """Finish migrating the network of an instance.""" diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index 2a1ec35f9f1a..aa0eb90986dd 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -5162,6 +5162,110 @@ class TestNeutronv2WithMock(_TestNeutronv2Common): self.context, instance, '172.24.5.15', '10.1.0.9') + @mock.patch('nova.network.neutronv2.api._get_ksa_client', + new_callable=mock.NonCallableMock) # asserts not called + def test_migrate_instance_start_no_binding_ext(self, get_client_mock): + """Tests that migrate_instance_start exits early if neutron doesn't + have the binding-extended API extension. + """ + with mock.patch.object(self.api, 'supports_port_binding_extension', + return_value=False): + self.api.migrate_instance_start( + self.context, mock.sentinel.instance, {}) + + @mock.patch('nova.network.neutronv2.api._get_ksa_client') + def test_migrate_instance_start_activate(self, get_client_mock): + """Tests the happy path for migrate_instance_start where the binding + for the port(s) attached to the instance are activated on the + destination host. + """ + binding = {'binding': {'status': 'INACTIVE'}} + resp = fake_req.FakeResponse(200, content=jsonutils.dumps(binding)) + get_client_mock.return_value.get.return_value = resp + # Just create a simple instance with a single port. + instance = objects.Instance(info_cache=objects.InstanceInfoCache( + network_info=model.NetworkInfo([model.VIF(uuids.port_id)]))) + migration = {'source_compute': 'source', 'dest_compute': 'dest'} + with mock.patch.object(self.api, 'activate_port_binding') as activate: + with mock.patch.object(self.api, 'supports_port_binding_extension', + return_value=True): + self.api.migrate_instance_start( + self.context, instance, migration) + activate.assert_called_once_with(self.context, uuids.port_id, 'dest') + get_client_mock.return_value.get.assert_called_once_with( + '/v2.0/ports/%s/bindings/dest' % uuids.port_id, raise_exc=False) + + @mock.patch('nova.network.neutronv2.api._get_ksa_client') + def test_migrate_instance_start_already_active(self, get_client_mock): + """Tests the case that the destination host port binding is already + ACTIVE when migrate_instance_start is called so we don't try to + activate it again, which would result in a 409 from Neutron. + """ + binding = {'binding': {'status': 'ACTIVE'}} + resp = fake_req.FakeResponse(200, content=jsonutils.dumps(binding)) + get_client_mock.return_value.get.return_value = resp + # Just create a simple instance with a single port. + instance = objects.Instance(info_cache=objects.InstanceInfoCache( + network_info=model.NetworkInfo([model.VIF(uuids.port_id)]))) + migration = {'source_compute': 'source', 'dest_compute': 'dest'} + with mock.patch.object(self.api, 'activate_port_binding', + new_callable=mock.NonCallableMock): + with mock.patch.object(self.api, 'supports_port_binding_extension', + return_value=True): + self.api.migrate_instance_start( + self.context, instance, migration) + get_client_mock.return_value.get.assert_called_once_with( + '/v2.0/ports/%s/bindings/dest' % uuids.port_id, raise_exc=False) + + @mock.patch('nova.network.neutronv2.api._get_ksa_client') + def test_migrate_instance_start_no_bindings(self, get_client_mock): + """Tests the case that migrate_instance_start is running against new + enough neutron for the binding-extended API but the ports don't have + a binding resource against the destination host, so no activation + happens. + """ + get_client_mock.return_value.get.return_value = ( + fake_req.FakeResponse(404)) + # Create an instance with two ports so we can test the short circuit + # when we find that the first port doesn't have a dest host binding. + instance = objects.Instance(info_cache=objects.InstanceInfoCache( + network_info=model.NetworkInfo([ + model.VIF(uuids.port1), model.VIF(uuids.port2)]))) + migration = {'source_compute': 'source', 'dest_compute': 'dest'} + with mock.patch.object(self.api, 'activate_port_binding', + new_callable=mock.NonCallableMock): + with mock.patch.object(self.api, 'supports_port_binding_extension', + return_value=True): + self.api.migrate_instance_start( + self.context, instance, migration) + get_client_mock.return_value.get.assert_called_once_with( + '/v2.0/ports/%s/bindings/dest' % uuids.port1, raise_exc=False) + + @mock.patch('nova.network.neutronv2.api._get_ksa_client') + def test_migrate_instance_start_get_error(self, get_client_mock): + """Tests the case that migrate_instance_start is running against new + enough neutron for the binding-extended API but getting the port + binding information results in an error response from neutron. + """ + get_client_mock.return_value.get.return_value = ( + fake_req.FakeResponse(500)) + instance = objects.Instance(info_cache=objects.InstanceInfoCache( + network_info=model.NetworkInfo([ + model.VIF(uuids.port1), model.VIF(uuids.port2)]))) + migration = {'source_compute': 'source', 'dest_compute': 'dest'} + with mock.patch.object(self.api, 'activate_port_binding', + new_callable=mock.NonCallableMock): + with mock.patch.object(self.api, 'supports_port_binding_extension', + return_value=True): + self.api.migrate_instance_start( + self.context, instance, migration) + self.assertEqual(2, get_client_mock.return_value.get.call_count) + get_client_mock.return_value.get.assert_has_calls([ + mock.call('/v2.0/ports/%s/bindings/dest' % uuids.port1, + raise_exc=False), + mock.call('/v2.0/ports/%s/bindings/dest' % uuids.port2, + raise_exc=False)]) + class TestNeutronv2ModuleMethods(test.NoDBTestCase):