From 38fc7f6f1602640b8e8c7b9cfa3e48766a5c5b95 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 23 Aug 2019 16:03:41 -0400 Subject: [PATCH] Trap and log errors from _update_inst_info_cache_for_disassociated_fip When associating a floating IP to instance A and the floating IP is already associated to instance B, the associate_floating_ip method updates the floating IP to be associated with instance A then tries to update the network info cache for instance B. That network info cache update is best effort and could fail in different ways, e.g. the original associated port or instance could be gone. Failing to refresh the cache for instance B should not affect the association operation for instance A, so this change traps any errors during the refresh and just logs them as a warning. Change-Id: Ib5a44e4fd2ec2bf43b761db29403810d8b730429 Related-Bug: #1826472 (cherry picked from commit 9cbedea0bf5535fc45d78c80e6d7499512e4c5f5) --- nova/network/neutronv2/api.py | 11 +++-- nova/tests/unit/network/test_neutronv2.py | 51 +++++++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index 97b1ffb4763b..c92c7513bd9c 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -2397,11 +2397,16 @@ class API(base_api.NetworkAPI): # the cache for that instance to avoid a window of time where multiple # servers in the API say they are using the same floating IP. if fip['port_id']: - # TODO(mriedem): Seems we should trap and log any errors from + # Trap and log any errors from # _update_inst_info_cache_for_disassociated_fip but not let them # raise back up to the caller since this refresh is best effort. - self._update_inst_info_cache_for_disassociated_fip( - context, instance, client, fip) + try: + self._update_inst_info_cache_for_disassociated_fip( + context, instance, client, fip) + except Exception as e: + LOG.warning('An error occurred while trying to refresh the ' + 'network info cache for an instance associated ' + 'with port %s. Error: %s', fip['port_id'], e) def _update_inst_info_cache_for_disassociated_fip(self, context, instance, client, fip): diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index fa599877b9cc..46dde6787699 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -5845,6 +5845,57 @@ class TestNeutronv2WithMock(_TestNeutronv2Common): self.context, instance, '172.24.5.15', '10.1.0.9') + @mock.patch('nova.network.neutronv2.api.get_client') + @mock.patch('nova.network.neutronv2.api.LOG.warning') + @mock.patch('nova.network.base_api.update_instance_cache_with_nw_info') + def test_associate_floating_ip_refresh_error_trap(self, mock_update_cache, + mock_log_warning, + mock_get_client): + """Tests that when _update_inst_info_cache_for_disassociated_fip + raises an exception, associate_floating_ip traps and logs it but + does not re-raise. + """ + ctxt = context.get_context() + instance = fake_instance.fake_instance_obj(ctxt) + floating_addr = '172.24.5.15' + fixed_addr = '10.1.0.9' + fip = {'id': uuids.floating_ip_id, 'port_id': uuids.old_port_id} + # Setup the mocks. + with test.nested( + mock.patch.object(self.api, '_get_port_id_by_fixed_address', + return_value=uuids.new_port_id), + mock.patch.object(self.api, '_get_floating_ip_by_address', + return_value=fip), + mock.patch.object(self.api, + '_update_inst_info_cache_for_disassociated_fip', + side_effect=exception.PortNotFound( + port_id=uuids.old_port_id)) + ) as ( + _get_port_id_by_fixed_address, + _get_floating_ip_by_address, + _update_inst_info_cache_for_disassociated_fip + ): + # Run the code. + self.api.associate_floating_ip( + ctxt, instance, floating_addr, fixed_addr) + # Assert the calls. + mock_get_client.assert_called_once_with(ctxt) + mock_client = mock_get_client.return_value + _get_port_id_by_fixed_address.assert_called_once_with( + mock_client, instance, fixed_addr) + _get_floating_ip_by_address.assert_called_once_with( + mock_client, floating_addr) + mock_client.update_floatingip.assert_called_once_with( + uuids.floating_ip_id, test.MatchType(dict)) + _update_inst_info_cache_for_disassociated_fip.assert_called_once_with( + ctxt, instance, mock_client, fip) + mock_log_warning.assert_called_once() + self.assertIn('An error occurred while trying to refresh the ' + 'network info cache for an instance associated ' + 'with port', mock_log_warning.call_args[0][0]) + mock_update_cache.assert_called_once_with( # from @refresh_cache + self.api, ctxt, instance, nw_info=None) + @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):