From 42106d4696aaa8e91e458783b15c90335ff5f7e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20J=C3=B3zefczyk?= Date: Fri, 17 Jan 2020 15:08:27 +0000 Subject: [PATCH] Don't fail if VIP already exist or has been deleted before Sometimes there is a race condition on creation of VIP port or deleting it that ends with exception and blocks LB stack creation / deletion. Because for now we don't have running master branch for OVN provider driver this change will be applied first on stable/train in networking-ovn tree and then cherry-picked. Change-Id: I2aaae7c407caba7a57e2ca2d4ed524f3bb63953f Closes-Bug: #1860141 --- ovn_octavia_provider/driver.py | 37 ++++++++++--- .../tests/unit/test_driver.py | 54 ++++++++++++++++++- 2 files changed, 81 insertions(+), 10 deletions(-) diff --git a/ovn_octavia_provider/driver.py b/ovn_octavia_provider/driver.py index 703c18cf..d4e3a74f 100644 --- a/ovn_octavia_provider/driver.py +++ b/ovn_octavia_provider/driver.py @@ -1015,12 +1015,8 @@ class OvnProviderHelper(object): 'loadbalancers': [{"id": loadbalancer['id'], "provisioning_status": constants.ERROR, "operating_status": constants.ERROR}]} - try: - # Delete VIP port from neutron. - self.delete_vip_port(port_id) - except n_exc.PortNotFoundClient: - LOG.warning("Port %s could not be found. Please " - "check Neutron logs", port_id) + # Delete VIP port from neutron. + self.delete_vip_port(port_id) return status def _lb_delete(self, loadbalancer, ovn_lb, status): @@ -1756,11 +1752,36 @@ class OvnProviderHelper(object): except KeyError: pass network_driver = get_network_driver() - return network_driver.neutron_client.create_port(port) + try: + return network_driver.neutron_client.create_port(port) + except n_exc.IpAddressAlreadyAllocatedClient: + # Sometimes the VIP is already created (race-conditions) + # Lets get the it from Neutron API. + ports = network_driver.neutron_client.list_ports( + network_id=vip_d['vip_network_id'], + name='%s%s' % (ovn_const.LB_VIP_PORT_PREFIX, lb_id)) + if not ports['ports']: + LOG.error('Cannot create/get LoadBalancer VIP port with ' + 'fixed IP: %s', vip_d['vip_address']) + status = {'loadbalancers': [{ + "id": lb_id, + "provisioning_status": constants.ERROR, + "operating_status": constants.ERROR}]} + self._update_status_to_octavia(status) + return + # there should only be one port returned + port = ports['ports'][0] + LOG.debug('VIP Port already exists, uuid: %s', port['id']) + return {'port': port} def delete_vip_port(self, port_id): network_driver = get_network_driver() - network_driver.neutron_client.delete_port(port_id) + try: + network_driver.neutron_client.delete_port(port_id) + except n_exc.PortNotFoundClient: + LOG.warning("Port %s could not be found. Please " + "check Neutron logs. Perhaps port " + "was already deleted.", port_id) def handle_vip_fip(self, fip_info): ovn_lb = fip_info['ovn_lb'] diff --git a/ovn_octavia_provider/tests/unit/test_driver.py b/ovn_octavia_provider/tests/unit/test_driver.py index f353bcdb..26f4ccef 100644 --- a/ovn_octavia_provider/tests/unit/test_driver.py +++ b/ovn_octavia_provider/tests/unit/test_driver.py @@ -1004,8 +1004,8 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): @mock.patch('ovn_octavia_provider.driver.get_network_driver') @mock.patch.object(ovn_driver.OvnProviderHelper, 'delete_vip_port') def test_lb_delete_port_not_found(self, del_port, net_dr): - net_dr.return_value.neutron_client.delete_port.return_value = None - del_port.side_effect = [n_exc.PortNotFoundClient] + net_dr.return_value.neutron_client.delete_port.side_effect = ( + [n_exc.PortNotFoundClient]) status = self.helper.lb_delete(self.ovn_lb) self.assertEqual(status['loadbalancers'][0]['provisioning_status'], constants.DELETED) @@ -2519,6 +2519,56 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): mock.call().neutron_client.create_port(expected_dict)] gn.assert_has_calls(expected_call) + @mock.patch('ovn_octavia_provider.driver.get_network_driver') + def test_create_vip_port_vip_selected_already_exist(self, net_dr): + net_dr.return_value.neutron_client.create_port.side_effect = [ + n_exc.IpAddressAlreadyAllocatedClient] + net_dr.return_value.neutron_client.list_ports.return_value = { + 'ports': [ + {'name': 'ovn-lb-vip-' + self.loadbalancer_id, + 'id': self.loadbalancer_id}]} + self.vip_dict['vip_address'] = '10.1.10.1' + ret = self.helper.create_vip_port( + self.project_id, + self.loadbalancer_id, + self.vip_dict) + expected = { + 'port': { + 'name': '%s%s' % (ovn_const.LB_VIP_PORT_PREFIX, + self.loadbalancer_id), + 'id': self.loadbalancer_id}} + self.assertDictEqual(expected, ret) + expected_call = [ + mock.call().neutron_client.list_ports( + network_id='%s' % self.vip_dict['vip_network_id'], + name='%s%s' % (ovn_const.LB_VIP_PORT_PREFIX, + self.loadbalancer_id))] + net_dr.assert_has_calls(expected_call) + + @mock.patch('ovn_octavia_provider.driver.get_network_driver') + def test_create_vip_port_vip_selected_other_allocation_exist(self, net_dr): + net_dr.return_value.neutron_client.create_port.side_effect = [ + n_exc.IpAddressAlreadyAllocatedClient] + net_dr.return_value.neutron_client.list_ports.return_value = { + 'ports': []} + self.vip_dict['vip_address'] = '10.1.10.1' + ret = self.helper.create_vip_port( + self.project_id, + self.loadbalancer_id, + self.vip_dict) + self.assertIsNone(ret) + expected_call = [ + mock.call().neutron_client.list_ports( + network_id='%s' % self.vip_dict['vip_network_id'], + name='%s%s' % (ovn_const.LB_VIP_PORT_PREFIX, + self.loadbalancer_id))] + net_dr.assert_has_calls(expected_call) + self.helper._update_status_to_octavia.assert_called_once_with( + {'loadbalancers': + [{'id': self.loadbalancer_id, + 'provisioning_status': 'ERROR', + 'operating_status': 'ERROR'}]}) + def test_get_member_info(self): ret = self.helper.get_member_info(self.pool_id) self.assertEqual([(self.member_id, '%s:%s' % (self.member_address,