Merge "Avoid loadbalancer stuck in PENDING_X if delete_vip_port fails"
This commit is contained in:
Zuul
Gerrit Code Review
commit
f82c69421e
|
|
@ -362,8 +362,8 @@ class OvnProviderHelper():
|
|||
@tenacity.retry(
|
||||
retry=tenacity.retry_if_exception_type(
|
||||
driver_exceptions.UpdateStatusError),
|
||||
wait=tenacity.wait_exponential(),
|
||||
stop=tenacity.stop_after_delay(10),
|
||||
wait=tenacity.wait_exponential(max=75),
|
||||
stop=tenacity.stop_after_attempt(15),
|
||||
reraise=True)
|
||||
def _update_status_to_octavia(self, status):
|
||||
status = OvnProviderHelper._delete_disabled_from_status(status)
|
||||
|
|
@ -1005,9 +1005,15 @@ class OvnProviderHelper():
|
|||
LOG.exception(ovn_const.EXCEPTION_MSG, "creation of loadbalancer")
|
||||
# Any Exception set the status to ERROR
|
||||
if isinstance(port, dict):
|
||||
self.delete_vip_port(port.get('id'))
|
||||
LOG.warning("Deleting the VIP port %s since LB went into "
|
||||
"ERROR state", str(port.get('id')))
|
||||
try:
|
||||
self.delete_vip_port(port.get('id'))
|
||||
LOG.warning("Deleting the VIP port %s since LB went into "
|
||||
"ERROR state", str(port.get('id')))
|
||||
except Exception:
|
||||
LOG.exception("Error deleting the VIP port %s upon "
|
||||
"loadbalancer %s creation failure",
|
||||
str(port.get('id')),
|
||||
str(loadbalancer[constants.ID]))
|
||||
status = {
|
||||
constants.LOADBALANCERS: [
|
||||
{constants.ID: loadbalancer[constants.ID],
|
||||
|
|
@ -1034,6 +1040,23 @@ class OvnProviderHelper():
|
|||
LOG.warning("Loadbalancer %s not found in OVN Northbound DB. "
|
||||
"Setting the Loadbalancer status to DELETED "
|
||||
"in Octavia", str(loadbalancer[constants.ID]))
|
||||
|
||||
# NOTE(ltomasbo): In case the previous loadbalancer deletion
|
||||
# action failed at VIP deletion step, this ensures the VIP
|
||||
# is not leaked
|
||||
try:
|
||||
vip_port_id = self._get_vip_port_from_loadbalancer_id(
|
||||
loadbalancer[constants.ID])
|
||||
if vip_port_id:
|
||||
LOG.warning("Deleting the VIP port %s associated to LB "
|
||||
"missing in OVN DBs", str(vip_port_id))
|
||||
self.delete_vip_port(vip_port_id)
|
||||
except Exception:
|
||||
LOG.exception("Error deleting the VIP port %s",
|
||||
str(vip_port_id))
|
||||
lbalancer_status[constants.PROVISIONING_STATUS] = (
|
||||
constants.ERROR)
|
||||
lbalancer_status[constants.OPERATING_STATUS] = constants.ERROR
|
||||
return status
|
||||
|
||||
try:
|
||||
|
|
@ -1046,12 +1069,13 @@ class OvnProviderHelper():
|
|||
# dict while iterating over it. So first get a list of keys.
|
||||
# https://cito.github.io/blog/never-iterate-a-changing-dict/
|
||||
status = {key: value for key, value in status.items() if value}
|
||||
# Delete VIP port from neutron.
|
||||
self.delete_vip_port(port_id)
|
||||
except Exception:
|
||||
LOG.exception(ovn_const.EXCEPTION_MSG, "deletion of loadbalancer")
|
||||
lbalancer_status[constants.PROVISIONING_STATUS] = constants.ERROR
|
||||
lbalancer_status[constants.OPERATING_STATUS] = constants.ERROR
|
||||
# Delete VIP port from neutron.
|
||||
self.delete_vip_port(port_id)
|
||||
|
||||
return status
|
||||
|
||||
def _lb_delete(self, loadbalancer, ovn_lb, status):
|
||||
|
|
@ -1154,6 +1178,11 @@ class OvnProviderHelper():
|
|||
lb_status[constants.OPERATING_STATUS] = constants.ERROR
|
||||
return status
|
||||
|
||||
def _get_vip_port_from_loadbalancer_id(self, lb_id):
|
||||
lb = self._octavia_driver_lib.get_loadbalancer(lb_id)
|
||||
if lb and lb.vip_port_id:
|
||||
return lb.vip_port_id
|
||||
|
||||
def listener_create(self, listener):
|
||||
ovn_lb = self._get_or_create_ovn_lb(
|
||||
listener[constants.LOADBALANCER_ID],
|
||||
|
|
@ -1897,6 +1926,12 @@ class OvnProviderHelper():
|
|||
LOG.debug('VIP Port already exists, uuid: %s', port['id'])
|
||||
return {'port': port}
|
||||
|
||||
@tenacity.retry(
|
||||
retry=tenacity.retry_if_exception_type(
|
||||
n_exc.NeutronClientException),
|
||||
wait=tenacity.wait_exponential(max=75),
|
||||
stop=tenacity.stop_after_attempt(15),
|
||||
reraise=True)
|
||||
def delete_vip_port(self, port_id):
|
||||
neutron_client = clients.get_neutron_client()
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -543,9 +543,12 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
|
|||
self.ovn_lb.uuid)
|
||||
del_port.assert_called_once_with('foo_port')
|
||||
|
||||
@mock.patch.object(ovn_helper.OvnProviderHelper,
|
||||
'_get_vip_port_from_loadbalancer_id')
|
||||
@mock.patch.object(ovn_helper.OvnProviderHelper, 'delete_vip_port')
|
||||
def test_lb_delete_row_not_found(self, del_port):
|
||||
def test_lb_delete_row_not_found(self, del_port, get_vip_port):
|
||||
self.helper._find_ovn_lbs.side_effect = [idlutils.RowNotFound]
|
||||
get_vip_port.return_value = None
|
||||
status = self.helper.lb_delete(self.lb)
|
||||
self.assertEqual(status['loadbalancers'][0]['provisioning_status'],
|
||||
constants.DELETED)
|
||||
|
|
@ -553,6 +556,22 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
|
|||
constants.OFFLINE)
|
||||
self.helper.ovn_nbdb_api.lb_del.assert_not_called()
|
||||
del_port.assert_not_called()
|
||||
get_vip_port.assert_called_once_with(self.lb['id'])
|
||||
|
||||
@mock.patch.object(ovn_helper.OvnProviderHelper,
|
||||
'_get_vip_port_from_loadbalancer_id')
|
||||
@mock.patch.object(ovn_helper.OvnProviderHelper, 'delete_vip_port')
|
||||
def test_lb_delete_row_not_found_vip_leak(self, del_port, get_vip_port):
|
||||
self.helper._find_ovn_lbs.side_effect = [idlutils.RowNotFound]
|
||||
get_vip_port.return_value = 'foo_port'
|
||||
status = self.helper.lb_delete(self.lb)
|
||||
self.assertEqual(status['loadbalancers'][0]['provisioning_status'],
|
||||
constants.DELETED)
|
||||
self.assertEqual(status['loadbalancers'][0]['operating_status'],
|
||||
constants.OFFLINE)
|
||||
self.helper.ovn_nbdb_api.lb_del.assert_not_called()
|
||||
del_port.assert_called_once_with('foo_port')
|
||||
get_vip_port.assert_called_once_with(self.lb['id'])
|
||||
|
||||
@mock.patch.object(ovn_helper.OvnProviderHelper, 'delete_vip_port')
|
||||
def test_lb_delete_exception(self, del_port):
|
||||
|
|
@ -564,7 +583,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
|
|||
constants.ERROR)
|
||||
self.helper.ovn_nbdb_api.lb_del.assert_called_once_with(
|
||||
self.ovn_lb.uuid)
|
||||
del_port.assert_called_once_with('foo_port')
|
||||
del_port.assert_not_called()
|
||||
|
||||
@mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
|
||||
@mock.patch.object(ovn_helper.OvnProviderHelper, 'delete_vip_port')
|
||||
|
|
@ -580,6 +599,18 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
|
|||
self.ovn_lb.uuid)
|
||||
del_port.assert_called_once_with('foo_port')
|
||||
|
||||
@mock.patch.object(ovn_helper.OvnProviderHelper, 'delete_vip_port')
|
||||
def test_lb_delete_port_exception(self, del_port):
|
||||
del_port.side_effect = [Exception]
|
||||
status = self.helper.lb_delete(self.ovn_lb)
|
||||
self.assertEqual(status['loadbalancers'][0]['provisioning_status'],
|
||||
constants.ERROR)
|
||||
self.assertEqual(status['loadbalancers'][0]['operating_status'],
|
||||
constants.ERROR)
|
||||
self.helper.ovn_nbdb_api.lb_del.assert_called_once_with(
|
||||
self.ovn_lb.uuid)
|
||||
del_port.assert_called_once_with('foo_port')
|
||||
|
||||
@mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
|
||||
def test_lb_delete_cascade(self, net_cli):
|
||||
net_cli.return_value.delete_port.return_value = None
|
||||
|
|
|
|||
Loading…
Reference in New Issue