openstack/octavia
Code Issues Proposed changes

Fix LB set in ERROR too early in the revert flow

Browse Source 
When an error occurs in a flow, the provisioning status of the load
balancer should be set to ERROR in the revert method of the first task
of the flow. This update acts as an unlock of the LB object and cannot
occur in any other revert method because the API might consider the LB
as mutable before finishing a task/flow.

Remove all occurrences of mark_loadbalancer_prov_status_error calls in
revert method of tasks that are not specifically designed for unlocking
the load balancers. Add a LoadBalancerToErrorOnRevertTask task in the
amphora failover flow to prevent a LB to be in an immutable state.

Story 2009651
Task 43810
Story 2009652
Task 43811

Change-Id: I48b0f5a773209b1c1b056d71c0da05d6fd82ca73
(cherry picked from commit 4b8b198fecee802e1031c014f811eb766329a1a0)
This commit is contained in:
Gregory Thiemonge 2021-10-28 22:13:26 +02:00
parent 45c992c1f3
commit 4039d35ce2
11 changed files with 46 additions and 360 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
octavia/controller/worker/v1/tasks/amphora_driver_tasks.py
View File

@ -254,7 +254,6 @@ class AmphoraPostVIPPlug(BaseAmphoraTask):
return
LOG.warning("Reverting post vip plug.")
self.task_utils.mark_amphora_status_error(amphora.id)
self.task_utils.mark_loadbalancer_prov_status_error(loadbalancer.id)
class AmphoraePostVIPPlug(BaseAmphoraTask):
@ -268,13 +267,6 @@ class AmphoraePostVIPPlug(BaseAmphoraTask):
loadbalancer,
amphorae_network_config)
def revert(self, result, loadbalancer, *args, **kwargs):
"""Handle a failed amphora vip plug notification."""
if isinstance(result, failure.Failure):
return
LOG.warning("Reverting amphorae post vip plug.")
self.task_utils.mark_loadbalancer_prov_status_error(loadbalancer.id)
class AmphoraCertUpload(BaseAmphoraTask):
"""Upload a certificate to the amphora."""

46
octavia/controller/worker/v1/tasks/database_tasks.py
View File

@ -1009,10 +1009,6 @@ class MarkLBActiveInDB(BaseDatabaseTask):
LOG.warning("Error updating listener %s provisioning "
"status", listener.id)
LOG.warning("Reverting mark load balancer deleted in DB "
"for load balancer id %s", loadbalancer.id)
self.task_utils.mark_loadbalancer_prov_status_error(loadbalancer.id)
class UpdateLBServerGroupInDB(BaseDatabaseTask):
"""Update the server group id info for load balancer in DB."""
@ -1072,17 +1068,6 @@ class MarkLBDeletedInDB(BaseDatabaseTask):
loadbalancer.id,
provisioning_status=constants.DELETED)
def revert(self, loadbalancer, *args, **kwargs):
"""Mark the load balancer as broken and ready to be cleaned up.
:param loadbalancer: Load balancer object that failed to update
:returns: None
"""
LOG.warning("Reverting mark load balancer deleted in DB "
"for load balancer id %s", loadbalancer.id)
self.task_utils.mark_loadbalancer_prov_status_error(loadbalancer.id)
class MarkLBPendingDeleteInDB(BaseDatabaseTask):
"""Mark the load balancer pending delete in the DB.
@ -1104,17 +1089,6 @@ class MarkLBPendingDeleteInDB(BaseDatabaseTask):
provisioning_status=(constants.
PENDING_DELETE))
def revert(self, loadbalancer, *args, **kwargs):
"""Mark the load balancer as broken and ready to be cleaned up.
:param loadbalancer: Load balancer object that failed to update
:returns: None
"""
LOG.warning("Reverting mark load balancer pending delete in DB "
"for load balancer id %s", loadbalancer.id)
self.task_utils.mark_loadbalancer_prov_status_error(loadbalancer.id)
class MarkLBAndListenersActiveInDB(BaseDatabaseTask):
"""Mark the load balancer and specified listeners active in the DB.
@ -1148,12 +1122,10 @@ class MarkLBAndListenersActiveInDB(BaseDatabaseTask):
:returns: None
"""
LOG.warning("Reverting mark load balancer and listeners active in DB "
"for load balancer id %(LB)s and listener ids: %(list)s",
{'LB': loadbalancer.id,
'list': ', '.join([listener.id
LOG.warning("Reverting mark listeners active in DB "
"for listener ids: %(list)s",
{'list': ', '.join([listener.id
for listener in listeners])})
self.task_utils.mark_loadbalancer_prov_status_error(loadbalancer.id)
for listener in listeners:
self.task_utils.mark_listener_prov_status_error(listener.id)
@ -1240,18 +1212,6 @@ class UpdateLoadbalancerInDB(BaseDatabaseTask):
self.loadbalancer_repo.update(db_apis.get_session(), loadbalancer.id,
**update_dict)
def revert(self, loadbalancer, *args, **kwargs):
"""Mark the loadbalancer ERROR since the update couldn't happen
:param loadbalancer: The load balancer that couldn't be updated
:returns: None
"""
LOG.warning("Reverting update loadbalancer in DB "
"for loadbalancer id %s", loadbalancer.id)
self.task_utils.mark_loadbalancer_prov_status_error(loadbalancer.id)
class UpdateHealthMonInDB(BaseDatabaseTask):
"""Update the health monitor in the DB.

4
octavia/controller/worker/v2/flows/amphora_flows.py
View File

@ -433,6 +433,10 @@ class AmphoraFlows(object):
failover_amp_flow = linear_flow.Flow(
constants.FAILOVER_AMPHORA_FLOW)
# Revert LB to provisioning_status ERROR if this flow goes wrong
failover_amp_flow.add(lifecycle_tasks.LoadBalancerToErrorOnRevertTask(
requires=constants.LOADBALANCER))
# Revert amphora to status ERROR if this flow goes wrong
failover_amp_flow.add(lifecycle_tasks.AmphoraToErrorOnRevertTask(
requires=constants.AMPHORA,

10
octavia/controller/worker/v2/tasks/amphora_driver_tasks.py
View File

@ -359,8 +359,6 @@ class AmphoraPostVIPPlug(BaseAmphoraTask):
return
LOG.warning("Reverting post vip plug.")
self.task_utils.mark_amphora_status_error(amphora.get(constants.ID))
self.task_utils.mark_loadbalancer_prov_status_error(
loadbalancer[constants.LOADBALANCER_ID])
class AmphoraePostVIPPlug(BaseAmphoraTask):
@ -376,14 +374,6 @@ class AmphoraePostVIPPlug(BaseAmphoraTask):
loadbalancer,
amphorae_network_config)
def revert(self, result, loadbalancer, *args, **kwargs):
"""Handle a failed amphora vip plug notification."""
if isinstance(result, failure.Failure):
return
LOG.warning("Reverting amphorae post vip plug.")
self.task_utils.mark_loadbalancer_prov_status_error(
loadbalancer[constants.LOADBALANCER_ID])
class AmphoraCertUpload(BaseAmphoraTask):
"""Upload a certificate to the amphora."""

81
octavia/controller/worker/v2/tasks/database_tasks.py
View File

@ -1086,12 +1086,6 @@ class MarkLBActiveInDB(BaseDatabaseTask):
LOG.warning("Error updating listener %s provisioning "
"status", listener.id)
LOG.warning("Reverting mark load balancer deleted in DB "
"for load balancer id %s",
loadbalancer[constants.LOADBALANCER_ID])
self.task_utils.mark_loadbalancer_prov_status_error(
loadbalancer[constants.LOADBALANCER_ID])
class MarkLBActiveInDBByListener(BaseDatabaseTask):
"""Mark the load balancer active in the DB using a listener dict.
@ -1112,22 +1106,6 @@ class MarkLBActiveInDBByListener(BaseDatabaseTask):
listener[constants.LOADBALANCER_ID],
provisioning_status=constants.ACTIVE)
def revert(self, listener, *args, **kwargs):
"""Mark the load balancer as broken and ready to be cleaned up.
This also puts all sub-objects of the load balancer to ERROR state if
self.mark_subobjects is True
:param listener: Listener dictionary
:returns: None
"""
LOG.warning("Reverting mark load balancer active in DB "
"for load balancer id %s",
listener[constants.LOADBALANCER_ID])
self.task_utils.mark_loadbalancer_prov_status_error(
listener[constants.LOADBALANCER_ID])
class UpdateLBServerGroupInDB(BaseDatabaseTask):
"""Update the server group id info for load balancer in DB."""
@ -1187,19 +1165,6 @@ class MarkLBDeletedInDB(BaseDatabaseTask):
loadbalancer[constants.LOADBALANCER_ID],
provisioning_status=constants.DELETED)
def revert(self, loadbalancer, *args, **kwargs):
"""Mark the load balancer as broken and ready to be cleaned up.
:param loadbalancer: Load balancer object that failed to update
:returns: None
"""
LOG.warning("Reverting mark load balancer deleted in DB "
"for load balancer id %s",
loadbalancer[constants.LOADBALANCER_ID])
self.task_utils.mark_loadbalancer_prov_status_error(
loadbalancer[constants.LOADBALANCER_ID])
class MarkLBPendingDeleteInDB(BaseDatabaseTask):
"""Mark the load balancer pending delete in the DB.
@ -1221,19 +1186,6 @@ class MarkLBPendingDeleteInDB(BaseDatabaseTask):
provisioning_status=(constants.
PENDING_DELETE))
def revert(self, loadbalancer, *args, **kwargs):
"""Mark the load balancer as broken and ready to be cleaned up.
:param loadbalancer: Load balancer object that failed to update
:returns: None
"""
LOG.warning("Reverting mark load balancer pending delete in DB "
"for load balancer id %s",
loadbalancer[constants.LOADBALANCER_ID])
self.task_utils.mark_loadbalancer_prov_status_error(
loadbalancer[constants.LOADBALANCER_ID])
class MarkLBAndListenersActiveInDB(BaseDatabaseTask):
"""Mark the load balancer and specified listeners active in the DB.
@ -1272,20 +1224,11 @@ class MarkLBAndListenersActiveInDB(BaseDatabaseTask):
:param listeners: Listener objects that failed to update
:returns: None
"""
lb_id = None
if loadbalancer_id:
lb_id = loadbalancer_id
elif listeners:
lb_id = listeners[0][constants.LOADBALANCER_ID]
if lb_id:
lists = ', '.join([listener[constants.LISTENER_ID]
for listener in listeners])
LOG.warning("Reverting mark load balancer and listeners active in "
"DB for load balancer id %(LB)s and listener ids: "
"%(list)s", {'LB': lb_id,
'list': lists})
self.task_utils.mark_loadbalancer_prov_status_error(lb_id)
lists = ', '.join([listener[constants.LISTENER_ID]
for listener in listeners])
LOG.warning("Reverting mark listeners active in "
"DB for listener ids: "
"%(list)s", {'list': lists})
for listener in listeners:
self.task_utils.mark_listener_prov_status_error(
@ -1376,20 +1319,6 @@ class UpdateLoadbalancerInDB(BaseDatabaseTask):
loadbalancer[constants.LOADBALANCER_ID],
**update_dict)
def revert(self, loadbalancer, *args, **kwargs):
"""Mark the loadbalancer ERROR since the update couldn't happen
:param loadbalancer: The load balancer that couldn't be updated
:returns: None
"""
LOG.warning("Reverting update loadbalancer in DB "
"for loadbalancer id %s",
loadbalancer[constants.LOADBALANCER_ID])
self.task_utils.mark_loadbalancer_prov_status_error(
loadbalancer[constants.LOADBALANCER_ID])
class UpdateHealthMonInDB(BaseDatabaseTask):
"""Update the health monitor in the DB.

37
octavia/tests/unit/controller/worker/v1/tasks/test_amphora_driver_tasks.py
View File

@ -496,10 +496,7 @@ class TestAmphoraDriverTasks(base.TestCase):
_session_mock,
id=AMP_ID,
status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_called_once_with(
_session_mock,
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertIsNone(amp)
@ -513,10 +510,7 @@ class TestAmphoraDriverTasks(base.TestCase):
_session_mock,
id=AMP_ID,
status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_called_once_with(
_session_mock,
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertIsNone(amp)
@ -546,33 +540,6 @@ class TestAmphoraDriverTasks(base.TestCase):
mock_driver.post_vip_plug.assert_called_once_with(
_amphora_mock, _LB_mock, amphorae_net_config_mock)
# Test revert
amp = amphora_post_vip_plug_obj.revert(None, _LB_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
_session_mock,
id=LB_ID,
provisioning_status=constants.ERROR)
self.assertIsNone(amp)
# Test revert with exception
repo.LoadBalancerRepository.update.reset_mock()
mock_loadbalancer_repo_update.side_effect = Exception('fail')
amp = amphora_post_vip_plug_obj.revert(None, _LB_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
_session_mock,
id=LB_ID,
provisioning_status=constants.ERROR)
self.assertIsNone(amp)
# Test revert when this task failed
repo.AmphoraRepository.update.reset_mock()
amp = amphora_post_vip_plug_obj.revert(
failure.Failure.from_exception(Exception('boom')), _amphora_mock,
None)
repo.AmphoraRepository.update.assert_not_called()
def test_amphora_cert_upload(self,
mock_driver,
mock_generate_uuid,

78
octavia/tests/unit/controller/worker/v1/tasks/test_database_tasks.py
View File

@ -955,10 +955,7 @@ class TestDatabaseTasks(base.TestCase):
'TEST',
id=LISTENER_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
# Test the revert with exceptions
mock_loadbalancer_repo_update.reset_mock()
@ -973,10 +970,7 @@ class TestDatabaseTasks(base.TestCase):
'TEST',
id=LISTENER_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
@mock.patch('octavia.common.tls_utils.cert_parser.get_cert_expiration',
return_value=_cert_mock)
@ -1040,10 +1034,7 @@ class TestDatabaseTasks(base.TestCase):
mock_loadbalancer_repo_update.reset_mock()
mark_loadbalancer_active.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertEqual(0, repo.ListenerRepository.update.call_count)
# Test the revert with exception
@ -1051,10 +1042,7 @@ class TestDatabaseTasks(base.TestCase):
mock_loadbalancer_repo_update.side_effect = Exception('fail')
mark_loadbalancer_active.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertEqual(0, repo.ListenerRepository.update.call_count)
def test_mark_LB_active_in_db_and_listeners(self,
@ -1086,10 +1074,7 @@ class TestDatabaseTasks(base.TestCase):
mock_listener_repo_update.reset_mock()
mark_lb_active.revert(lb)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=lb.id,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertEqual(2, repo.ListenerRepository.update.call_count)
repo.ListenerRepository.update.has_calls(
[mock.call('TEST', listeners[0].id,
@ -1192,10 +1177,7 @@ class TestDatabaseTasks(base.TestCase):
mock_l7r_repo_update.reset_mock()
mark_lb_active.revert(lb)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=lb.id,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertEqual(2, repo.ListenerRepository.update.call_count)
repo.ListenerRepository.update.has_calls(
[mock.call('TEST', listeners[0].id,
@ -1252,20 +1234,14 @@ class TestDatabaseTasks(base.TestCase):
mock_loadbalancer_repo_update.reset_mock()
mark_loadbalancer_deleted.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
# Test the revert with exception
mock_loadbalancer_repo_update.reset_mock()
mock_loadbalancer_repo_update.side_effect = Exception('fail')
mark_loadbalancer_deleted.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
def test_mark_LB_pending_deleted_in_db(self,
mock_generate_uuid,
@ -1285,25 +1261,6 @@ class TestDatabaseTasks(base.TestCase):
LB_ID,
provisioning_status=constants.PENDING_DELETE)
# Test the revert
mock_loadbalancer_repo_update.reset_mock()
mark_loadbalancer_pending_delete.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
# Test the revert with exception
mock_loadbalancer_repo_update.reset_mock()
mock_loadbalancer_repo_update.side_effect = Exception('fail')
mark_loadbalancer_pending_delete.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
@mock.patch('octavia.db.repositories.HealthMonitorRepository.update')
def test_update_health_monitor_in_db(self,
mock_health_mon_repo_update,
@ -1361,25 +1318,6 @@ class TestDatabaseTasks(base.TestCase):
LB_ID,
name='test', description='test2')
# Test the revert
mock_loadbalancer_repo_update.reset_mock()
update_load_balancer.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
# Test the revert with exception
mock_loadbalancer_repo_update.reset_mock()
mock_loadbalancer_repo_update.side_effect = Exception('fail')
update_load_balancer.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
@mock.patch('octavia.db.repositories.VipRepository.update')
def test_update_vip_in_db_during_update_loadbalancer(self,
mock_vip_update,

3
octavia/tests/unit/controller/worker/v2/flows/test_amphora_flows.py
View File

@ -342,10 +342,11 @@ class TestAmphoraFlows(base.TestCase):
self.assertIsInstance(amp_flow, flow.Flow)
self.assertIn(constants.LOADBALANCER_ID, amp_flow.requires)
self.assertIn(constants.LOADBALANCER, amp_flow.requires)
self.assertIn(constants.VIP_SG_ID, amp_flow.provides)
self.assertEqual(1, len(amp_flow.requires))
self.assertEqual(2, len(amp_flow.requires))
self.assertEqual(1, len(amp_flow.provides))
def test_cert_rotate_amphora_flow(self, mock_get_net_driver):

37
octavia/tests/unit/controller/worker/v2/tasks/test_amphora_driver_tasks.py
View File

@ -557,10 +557,7 @@ class TestAmphoraDriverTasks(base.TestCase):
_session_mock,
id=AMP_ID,
status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_called_once_with(
_session_mock,
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertIsNone(amp)
@ -574,10 +571,7 @@ class TestAmphoraDriverTasks(base.TestCase):
_session_mock,
id=AMP_ID,
status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_called_once_with(
_session_mock,
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertIsNone(amp)
@ -664,33 +658,6 @@ class TestAmphoraDriverTasks(base.TestCase):
_db_amphora_mock, _db_load_balancer_mock, amphorae_net_config_mock,
vip_subnet=vip_subnet, vrrp_port=vrrp_port)
# Test revert
amp = amphora_post_vip_plug_obj.revert(None, _LB_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
_session_mock,
id=LB_ID,
provisioning_status=constants.ERROR)
self.assertIsNone(amp)
# Test revert with exception
repo.LoadBalancerRepository.update.reset_mock()
mock_loadbalancer_repo_update.side_effect = Exception('fail')
amp = amphora_post_vip_plug_obj.revert(None, _LB_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
_session_mock,
id=LB_ID,
provisioning_status=constants.ERROR)
self.assertIsNone(amp)
# Test revert when this task failed
repo.AmphoraRepository.update.reset_mock()
amp = amphora_post_vip_plug_obj.revert(
failure.Failure.from_exception(Exception('boom')), _amphora_mock,
None)
repo.AmphoraRepository.update.assert_not_called()
def test_amphora_cert_upload(self,
mock_driver,
mock_generate_uuid,

93
octavia/tests/unit/controller/worker/v2/tasks/test_database_tasks.py
View File

@ -1027,10 +1027,7 @@ class TestDatabaseTasks(base.TestCase):
'TEST',
id=LISTENER_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
# Test the revert LB_ID from listeners
mock_loadbalancer_repo_update.reset_mock()
@ -1042,10 +1039,7 @@ class TestDatabaseTasks(base.TestCase):
'TEST',
id=LISTENER_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
# Test the revert no LB_ID
mock_loadbalancer_repo_update.reset_mock()
@ -1068,10 +1062,7 @@ class TestDatabaseTasks(base.TestCase):
'TEST',
id=LISTENER_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
@mock.patch('octavia.common.tls_utils.cert_parser.get_cert_expiration',
return_value=_cert_mock)
@ -1135,10 +1126,7 @@ class TestDatabaseTasks(base.TestCase):
mock_loadbalancer_repo_update.reset_mock()
mark_loadbalancer_active.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertEqual(0, repo.ListenerRepository.update.call_count)
# Test the revert with exception
@ -1146,10 +1134,7 @@ class TestDatabaseTasks(base.TestCase):
mock_loadbalancer_repo_update.side_effect = Exception('fail')
mark_loadbalancer_active.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertEqual(0, repo.ListenerRepository.update.call_count)
def test_mark_LB_active_in_db_by_listener(self,
@ -1175,10 +1160,7 @@ class TestDatabaseTasks(base.TestCase):
mock_loadbalancer_repo_update.reset_mock()
mark_loadbalancer_active.revert(listener_dict)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertEqual(0, repo.ListenerRepository.update.call_count)
# Test the revert with exception
@ -1186,10 +1168,7 @@ class TestDatabaseTasks(base.TestCase):
mock_loadbalancer_repo_update.side_effect = Exception('fail')
mark_loadbalancer_active.revert(listener_dict)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertEqual(0, repo.ListenerRepository.update.call_count)
@mock.patch('octavia.db.repositories.LoadBalancerRepository.get')
@ -1224,10 +1203,7 @@ class TestDatabaseTasks(base.TestCase):
mock_listener_repo_update.reset_mock()
mark_lb_active.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=lb.id,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertEqual(2, repo.ListenerRepository.update.call_count)
repo.ListenerRepository.update.has_calls(
[mock.call('TEST', listeners[0].id,
@ -1319,10 +1295,7 @@ class TestDatabaseTasks(base.TestCase):
mock_l7r_repo_update.reset_mock()
mark_lb_active.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=lb.id,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
self.assertEqual(2, repo.ListenerRepository.update.call_count)
repo.ListenerRepository.update.has_calls(
[mock.call('TEST', listeners[0].id,
@ -1368,20 +1341,14 @@ class TestDatabaseTasks(base.TestCase):
mock_loadbalancer_repo_update.reset_mock()
mark_loadbalancer_deleted.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
# Test the revert with exception
mock_loadbalancer_repo_update.reset_mock()
mock_loadbalancer_repo_update.side_effect = Exception('fail')
mark_loadbalancer_deleted.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
repo.LoadBalancerRepository.update.assert_not_called()
def test_mark_LB_pending_deleted_in_db(self,
mock_generate_uuid,
@ -1401,25 +1368,6 @@ class TestDatabaseTasks(base.TestCase):
LB_ID,
provisioning_status=constants.PENDING_DELETE)
# Test the revert
mock_loadbalancer_repo_update.reset_mock()
mark_loadbalancer_pending_delete.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
# Test the revert with exception
mock_loadbalancer_repo_update.reset_mock()
mock_loadbalancer_repo_update.side_effect = Exception('fail')
mark_loadbalancer_pending_delete.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
@mock.patch('octavia.db.repositories.HealthMonitorRepository.update')
def test_update_health_monitor_in_db(self,
mock_health_mon_repo_update,
@ -1477,25 +1425,6 @@ class TestDatabaseTasks(base.TestCase):
LB_ID,
name='test', description='test2')
# Test the revert
mock_loadbalancer_repo_update.reset_mock()
update_load_balancer.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
# Test the revert with exception
mock_loadbalancer_repo_update.reset_mock()
mock_loadbalancer_repo_update.side_effect = Exception('fail')
update_load_balancer.revert(self.loadbalancer_mock)
repo.LoadBalancerRepository.update.assert_called_once_with(
'TEST',
id=LB_ID,
provisioning_status=constants.ERROR)
@mock.patch('octavia.db.repositories.VipRepository.update')
def test_update_vip_in_db_during_update_loadbalancer(self,
mock_vip_update,

9
releasenotes/notes/fix-provisioning-status-on-errors-7f3736ef6e94d453.yaml Normal file
View File

@ -0,0 +1,9 @@
---
fixes:
- |
Fix an issue with the provisioning status of a load balancer that was set
to ERROR too early when an error occurred, making the load balancer mutable
while the execution of the tasks for this resources haven't finished yet.
- |
Fix an issue that could set the provisioning status of a load balancer to a
PENDING_UPDATE state when an error occurred in the amphora failover flow.