diff --git a/octavia/controller/worker/v1/tasks/amphora_driver_tasks.py b/octavia/controller/worker/v1/tasks/amphora_driver_tasks.py index 52c5ba01ea..f2fb420de3 100644 --- a/octavia/controller/worker/v1/tasks/amphora_driver_tasks.py +++ b/octavia/controller/worker/v1/tasks/amphora_driver_tasks.py @@ -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.""" diff --git a/octavia/controller/worker/v1/tasks/database_tasks.py b/octavia/controller/worker/v1/tasks/database_tasks.py index e42f6efaba..7a8203668a 100644 --- a/octavia/controller/worker/v1/tasks/database_tasks.py +++ b/octavia/controller/worker/v1/tasks/database_tasks.py @@ -1070,10 +1070,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.""" @@ -1133,17 +1129,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. @@ -1165,17 +1150,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. @@ -1209,12 +1183,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) @@ -1301,18 +1273,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. diff --git a/octavia/controller/worker/v2/flows/amphora_flows.py b/octavia/controller/worker/v2/flows/amphora_flows.py index 1ae186fbc1..4b4bbdc60a 100644 --- a/octavia/controller/worker/v2/flows/amphora_flows.py +++ b/octavia/controller/worker/v2/flows/amphora_flows.py @@ -578,6 +578,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, diff --git a/octavia/controller/worker/v2/tasks/amphora_driver_tasks.py b/octavia/controller/worker/v2/tasks/amphora_driver_tasks.py index fdf91d23d1..c06e00ea51 100644 --- a/octavia/controller/worker/v2/tasks/amphora_driver_tasks.py +++ b/octavia/controller/worker/v2/tasks/amphora_driver_tasks.py @@ -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.""" diff --git a/octavia/controller/worker/v2/tasks/database_tasks.py b/octavia/controller/worker/v2/tasks/database_tasks.py index 9b8e81c098..a875191536 100644 --- a/octavia/controller/worker/v2/tasks/database_tasks.py +++ b/octavia/controller/worker/v2/tasks/database_tasks.py @@ -1140,12 +1140,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. @@ -1166,22 +1160,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.""" @@ -1241,19 +1219,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. @@ -1275,19 +1240,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. @@ -1326,20 +1278,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( @@ -1430,20 +1373,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. diff --git a/octavia/tests/unit/controller/worker/v1/tasks/test_amphora_driver_tasks.py b/octavia/tests/unit/controller/worker/v1/tasks/test_amphora_driver_tasks.py index fa92aae320..aff0bae77f 100644 --- a/octavia/tests/unit/controller/worker/v1/tasks/test_amphora_driver_tasks.py +++ b/octavia/tests/unit/controller/worker/v1/tasks/test_amphora_driver_tasks.py @@ -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, diff --git a/octavia/tests/unit/controller/worker/v1/tasks/test_database_tasks.py b/octavia/tests/unit/controller/worker/v1/tasks/test_database_tasks.py index a4c5f1262d..f905096ff9 100644 --- a/octavia/tests/unit/controller/worker/v1/tasks/test_database_tasks.py +++ b/octavia/tests/unit/controller/worker/v1/tasks/test_database_tasks.py @@ -1043,10 +1043,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() @@ -1061,10 +1058,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) @@ -1128,10 +1122,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 @@ -1139,10 +1130,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, @@ -1174,10 +1162,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, @@ -1280,10 +1265,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, @@ -1340,20 +1322,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, @@ -1373,25 +1349,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, @@ -1449,25 +1406,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, diff --git a/octavia/tests/unit/controller/worker/v2/flows/test_amphora_flows.py b/octavia/tests/unit/controller/worker/v2/flows/test_amphora_flows.py index 8cf8996513..a55578ab77 100644 --- a/octavia/tests/unit/controller/worker/v2/flows/test_amphora_flows.py +++ b/octavia/tests/unit/controller/worker/v2/flows/test_amphora_flows.py @@ -362,10 +362,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_get_failover_flow_spare(self, mock_get_net_driver): @@ -376,10 +377,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): diff --git a/octavia/tests/unit/controller/worker/v2/tasks/test_amphora_driver_tasks.py b/octavia/tests/unit/controller/worker/v2/tasks/test_amphora_driver_tasks.py index 28de5af50e..723048a3e5 100644 --- a/octavia/tests/unit/controller/worker/v2/tasks/test_amphora_driver_tasks.py +++ b/octavia/tests/unit/controller/worker/v2/tasks/test_amphora_driver_tasks.py @@ -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, diff --git a/octavia/tests/unit/controller/worker/v2/tasks/test_database_tasks.py b/octavia/tests/unit/controller/worker/v2/tasks/test_database_tasks.py index 5641b572dc..690d16b52d 100644 --- a/octavia/tests/unit/controller/worker/v2/tasks/test_database_tasks.py +++ b/octavia/tests/unit/controller/worker/v2/tasks/test_database_tasks.py @@ -1115,10 +1115,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() @@ -1130,10 +1127,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() @@ -1156,10 +1150,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) @@ -1223,10 +1214,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 @@ -1234,10 +1222,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, @@ -1263,10 +1248,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 @@ -1274,10 +1256,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') @@ -1312,10 +1291,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, @@ -1407,10 +1383,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, @@ -1456,20 +1429,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, @@ -1489,25 +1456,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, @@ -1565,25 +1513,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, diff --git a/releasenotes/notes/fix-provisioning-status-on-errors-7f3736ef6e94d453.yaml b/releasenotes/notes/fix-provisioning-status-on-errors-7f3736ef6e94d453.yaml new file mode 100644 index 0000000000..f4688bb3e9 --- /dev/null +++ b/releasenotes/notes/fix-provisioning-status-on-errors-7f3736ef6e94d453.yaml @@ -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.