Merge "Fix LB set in ERROR too early in the revert flow" into stable/train
This commit is contained in:
commit
bb1488b5c5
|
@ -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."""
|
||||
|
|
|
@ -1064,10 +1064,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."""
|
||||
|
@ -1127,17 +1123,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.
|
||||
|
@ -1159,17 +1144,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.
|
||||
|
@ -1203,11 +1177,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([l.id for l in listeners])})
|
||||
self.task_utils.mark_loadbalancer_prov_status_error(loadbalancer.id)
|
||||
LOG.warning("Reverting mark listeners active in DB "
|
||||
"for listener ids: %(list)s",
|
||||
{'list': ', '.join([listener.id
|
||||
for listener in listeners])})
|
||||
for listener in listeners:
|
||||
self.task_utils.mark_listener_prov_status_error(listener.id)
|
||||
|
||||
|
@ -1294,18 +1267,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.
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -998,10 +998,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()
|
||||
|
@ -1016,10 +1013,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)
|
||||
|
@ -1083,10 +1077,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
|
||||
|
@ -1094,10 +1085,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,
|
||||
|
@ -1129,10 +1117,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,
|
||||
|
@ -1235,10 +1220,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,
|
||||
|
@ -1295,20 +1277,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,
|
||||
|
@ -1328,25 +1304,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,
|
||||
|
@ -1404,25 +1361,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,
|
||||
|
|
|
@ -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.
|
Loading…
Reference in New Issue