Do not fail on revert PlugVIPAmphora due undefined db_lb

- changed order: get db_lb before getting subnet info, to avoid useless
  message without db_lb.vip
- added default db_lb value for case, when DB calls are failed.
  in this case error message will be generated without information about
  VIP

Closes-Bug: 2085427

Change-Id: I97acfbd9b26fefbc38b61e36f72f483072390b23
(cherry picked from commit 7c08b63a37)
This commit is contained in:
Sergey Kraynev 2024-10-23 15:24:50 +04:00
parent c0ca1a5e30
commit 5a039fcd9f
3 changed files with 67 additions and 9 deletions

View File

@ -536,11 +536,10 @@ class PlugVIPAmphora(BaseNetworkTask):
"""Handle a failure to plumb a vip."""
if isinstance(result, failure.Failure):
return
lb_id = loadbalancer[constants.LOADBALANCER_ID]
LOG.warning("Unable to plug VIP for amphora id %s "
"load balancer id %s",
amphora.get(constants.ID),
loadbalancer[constants.LOADBALANCER_ID])
amphora.get(constants.ID), lb_id)
try:
session = db_apis.get_session()
with session.begin():
@ -550,15 +549,16 @@ class PlugVIPAmphora(BaseNetworkTask):
db_amp.ha_port_id = result[constants.HA_PORT_ID]
db_subnet = self.network_driver.get_subnet(
subnet[constants.ID])
db_lb = self.loadbalancer_repo.get(
session,
id=loadbalancer[constants.LOADBALANCER_ID])
db_lb = self.loadbalancer_repo.get(session, id=lb_id)
self.network_driver.unplug_aap_port(db_lb.vip,
db_amp, db_subnet)
except Exception as e:
LOG.error('Failed to unplug AAP port. Resources may still be in '
'use for VIP: %s due to error: %s', db_lb.vip, str(e))
LOG.error(
'Failed to unplug AAP port for load balancer: %s. '
'Resources may still be in use for VRRP port: %s. '
'Due to error: %s',
lb_id, result[constants.VRRP_PORT_ID], str(e)
)
class UnplugVIP(BaseNetworkTask):

View File

@ -1526,6 +1526,57 @@ class TestNetworkTasks(base.TestCase):
mock_driver.unplug_aap_port.assert_called_once_with(
LB.vip, self.db_amphora_mock, mockSubnet)
@mock.patch('octavia.db.repositories.AmphoraRepository.get')
@mock.patch('octavia.db.repositories.LoadBalancerRepository.get')
@mock.patch('octavia.db.api.get_session', return_value=_session_mock)
def test_revert_plug_vip_amphora_subnet_not_found(
self, mock_session, mock_lb_get, mock_get, mock_get_net_driver):
mock_driver = mock.MagicMock()
mock_lb_get.return_value = LB
mock_get.return_value = self.db_amphora_mock
mock_get_net_driver.return_value = mock_driver
net = network_tasks.PlugVIPAmphora()
amphora = {constants.ID: AMPHORA_ID,
constants.LB_NETWORK_IP: IP_ADDRESS}
subnet = {constants.ID: SUBNET_ID}
err_msg = 'Subnet not found'
mock_driver.get_subnet.side_effect = net_base.SubnetNotFound(err_msg)
result = AMPS_DATA[0].to_dict()
net.revert(result, self.load_balancer_mock, amphora, subnet)
mock_driver.unplug_aap_port.assert_not_called()
network_tasks.LOG.error.assert_called_once_with(
'Failed to unplug AAP port for load balancer: %s. '
'Resources may still be in use for VRRP port: %s. '
'Due to error: %s',
self.load_balancer_mock[constants.LOADBALANCER_ID],
result[constants.VRRP_PORT_ID], err_msg
)
@mock.patch('octavia.db.repositories.AmphoraRepository.get')
@mock.patch('octavia.db.repositories.LoadBalancerRepository.get')
@mock.patch('octavia.db.api.get_session', return_value=_session_mock)
def test_revert_plug_vip_amphora_raise_db_error(
self, mock_session, mock_lb_get, mock_get, mock_get_net_driver):
mock_driver = mock.MagicMock()
mock_lb_get.return_value = LB
err_msg = 'Some Error'
mock_get.side_effect = Exception(err_msg)
net = network_tasks.PlugVIPAmphora()
amphora = {constants.ID: AMPHORA_ID,
constants.LB_NETWORK_IP: IP_ADDRESS}
subnet = {constants.ID: SUBNET_ID}
result = AMPS_DATA[0].to_dict()
net.revert(result, self.load_balancer_mock, amphora, subnet)
mock_driver.unplug_aap_port.assert_not_called()
mock_lb_get.assert_not_called()
network_tasks.LOG.error.assert_called_once_with(
'Failed to unplug AAP port for load balancer: %s. '
'Resources may still be in use for VRRP port: %s. '
'Due to error: %s',
self.load_balancer_mock[constants.LOADBALANCER_ID],
result[constants.VRRP_PORT_ID], err_msg
)
@mock.patch('octavia.controller.worker.v2.tasks.network_tasks.DeletePort.'
'update_progress')
def test_delete_port(self, mock_update_progress, mock_get_net_driver):

View File

@ -0,0 +1,7 @@
---
fixes:
- |
Fix error on revert PlugVIPAmphora task, when db_lb is not defined
and get_subnet raises NotFound error. It could happen when Amphora
creation failed by timeout and before it VIP network was removed.
As result revert failed with exception.