Remove amphora_health record on revert CreateAmphoraInDB

Record in amphora_health created by first UDP request from booted
amphora.
It's possible to run creation/failover LB where Amphora will be booted
and reverted on the final tasks (e.g. AmphoraPostVIPPlug) as result
record in amphora_health will be created, but not be removed on revert.
This patch adds logic to remove amphora_health record on revert flow.

Closes-Bug: 2085530

Change-Id: Ia8b4612cd25a68b44d55b113f704756818eb8a5b

octavia/controller/worker/v2/tasks/database_tasks.py

@@ -132,13 +132,17 @@ class CreateAmphoraInDB(BaseDatabaseTask):
         LOG.warning("Reverting create amphora in DB for amp id %s ", result)
 
         # Delete the amphora for now. May want to just update status later
-        try:
+        with db_apis.session().begin() as session:
-            with db_apis.session().begin() as session:
+            try:
                 self.amphora_repo.delete(session, id=result)
-        except Exception as e:
+            except Exception as e:
-            LOG.error("Failed to delete amphora %(amp)s "
+                LOG.error("Failed to delete amphora %(amp)s "
-                      "in the database due to: "
+                          "in the database due to: "
-                      "%(except)s", {'amp': result, 'except': str(e)})
+                          "%(except)s", {'amp': result, 'except': str(e)})
+            try:
+                self.amp_health_repo.delete(session, amphora_id=result)
+            except Exception:
+                pass
 
 
 class MarkLBAmphoraeDeletedInDB(BaseDatabaseTask):

octavia/tests/unit/controller/worker/v2/tasks/test_database_tasks.py

@@ -184,7 +184,9 @@ class TestDatabaseTasks(base.TestCase):
 
     @mock.patch('octavia.db.repositories.AmphoraRepository.create',
                 return_value=_db_amphora_mock)
+    @mock.patch('octavia.db.repositories.AmphoraHealthRepository.delete')
     def test_create_amphora_in_db(self,
+                                  mock_amphora_health_repo_delete,
                                   mock_create,
                                   mock_generate_uuid,
                                   mock_LOG,
@@ -210,23 +212,43 @@ class TestDatabaseTasks(base.TestCase):
 
         # Test the revert
         create_amp_in_db.revert(_tf_failure_mock)
-        self.assertFalse(mock_amphora_repo_delete.called)
+        mock_amphora_repo_delete.assert_not_called()
+        mock_amphora_health_repo_delete.assert_not_called()
 
+        amp_id = 'AMP'
         mock_amphora_repo_delete.reset_mock()
-        create_amp_in_db.revert(result='AMP')
+        mock_amphora_health_repo_delete.reset_mock()
+        create_amp_in_db.revert(result=amp_id)
         self.assertTrue(mock_amphora_repo_delete.called)
+        self.assertTrue(mock_amphora_health_repo_delete.called)
         mock_amphora_repo_delete.assert_called_once_with(
             mock_session,
-            id='AMP')
+            id=amp_id)
+        mock_amphora_health_repo_delete.assert_called_once_with(
+            mock_session,
+            amphora_id=amp_id)
+        mock_LOG.error.assert_not_called()
+        mock_LOG.debug.assert_not_called()
 
         # Test revert with exception
         mock_amphora_repo_delete.reset_mock()
-        mock_amphora_repo_delete.side_effect = Exception('fail')
+        mock_amphora_health_repo_delete.reset_mock()
-        create_amp_in_db.revert(result='AMP')
+        err1_msg, err2_msg = ('fail', 'fail2')
+        mock_amphora_repo_delete.side_effect = Exception(err1_msg)
+        mock_amphora_health_repo_delete.side_effect = Exception(err2_msg)
+        create_amp_in_db.revert(result=amp_id)
         self.assertTrue(mock_amphora_repo_delete.called)
+        self.assertTrue(mock_amphora_health_repo_delete.called)
         mock_amphora_repo_delete.assert_called_once_with(
             mock_session,
-            id='AMP')
+            id=amp_id)
+        mock_amphora_health_repo_delete.assert_called_once_with(
+            mock_session,
+            amphora_id=amp_id)
+        mock_LOG.error.assert_called_once_with(
+            "Failed to delete amphora %(amp)s "
+            "in the database due to: "
+            "%(except)s", {'amp': amp_id, 'except': err1_msg})
 
     @mock.patch('octavia.db.repositories.ListenerRepository.delete')
     def test_delete_listener_in_db(self,

releasenotes/notes/delete_amphora_health_row_on_amphora_revert-082f94459ecacaa2.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Remove record in amphora_health table on revert. It's necessary, because
+    record in amphora table for corresponding amphora also deleted.
+    It allows to avoid false positive react of failover threshold due to
+    orphan records in amphora_health table.