Remove unused amphora flow and status constants

Remove unused code that was used in the spare pool feature.

Cleaned up unused amphora-related code including:
- get_create_amphora_flow function and its wrapper
- MarkAmphoraReadyInDB task class and related tests
- MARK_AMPHORA_READY_INDB constant
- AMPHORA_READY status constant and all references

Updated amphora deletion logic to only allow deletion when
amphora status is ERROR, which is the correct behavior since
AMPHORA_ALLOCATED amphorae should not be deletable.

Change-Id: I9ad69c7998509669cb6f76379810bbb56d887e80
Generated-By: Claude Code
Signed-off-by: Gregory Thiemonge <gthiemon@redhat.com>

octavia/common/constants.py

@@ -159,8 +159,6 @@ PROVISIONING_STATUS = lib_consts.PROVISIONING_STATUS
 AMPHORA_ALLOCATED = lib_consts.AMPHORA_ALLOCATED
 # Amphora is being built 'BOOTING'
 AMPHORA_BOOTING = lib_consts.AMPHORA_BOOTING
-# Amphora is ready to be allocated to a load balancer 'READY'
-AMPHORA_READY = lib_consts.AMPHORA_READY
 # 'FAILOVER_STOPPED'. Failover threshold level has been reached.
 AMPHORA_FAILOVER_STOPPED = lib_consts.AMPHORA_FAILOVER_STOPPED
 # 'ACTIVE'
@@ -541,7 +539,6 @@ COMPUTE_WAIT = 'octavia-compute-wait'
 UPDATE_AMPHORA_INFO = 'octavia-update-amphora-info'
 AMPHORA_FINALIZE = 'octavia-amphora-finalize'
 MARK_AMPHORA_ALLOCATED_INDB = 'octavia-mark-amphora-allocated-indb'
-MARK_AMPHORA_READY_INDB = 'octavia-mark-amphora-ready-indb'
 MARK_LB_ACTIVE_INDB = 'octavia-mark-lb-active-indb'
 MARK_AMP_MASTER_INDB = 'octavia-mark-amp-master-indb'
 MARK_AMP_BACKUP_INDB = 'octavia-mark-amp-backup-indb'

octavia/controller/worker/v2/flows/amphora_flows.py

@@ -36,57 +36,6 @@ LOG = logging.getLogger(__name__)
 
 class AmphoraFlows:
 
-    def get_create_amphora_flow(self):
-        """Creates a flow to create an amphora.
-
-        :returns: The flow for creating the amphora
-        """
-        create_amphora_flow = linear_flow.Flow(constants.CREATE_AMPHORA_FLOW)
-        create_amphora_flow.add(database_tasks.CreateAmphoraInDB(
-                                provides=constants.AMPHORA_ID))
-        create_amphora_flow.add(lifecycle_tasks.AmphoraIDToErrorOnRevertTask(
-            requires=constants.AMPHORA_ID))
-        create_amphora_flow.add(cert_task.GenerateServerPEMTask(
-                                provides=constants.SERVER_PEM))
-        create_amphora_flow.add(
-            database_tasks.UpdateAmphoraDBCertExpiration(
-                requires=(constants.AMPHORA_ID, constants.SERVER_PEM)))
-        create_amphora_flow.add(compute_tasks.CertComputeCreate(
-            requires=(constants.AMPHORA_ID, constants.SERVER_PEM,
-                      constants.SERVER_GROUP_ID,
-                      constants.BUILD_TYPE_PRIORITY, constants.FLAVOR),
-            provides=constants.COMPUTE_ID))
-        create_amphora_flow.add(database_tasks.MarkAmphoraBootingInDB(
-            requires=(constants.AMPHORA_ID, constants.COMPUTE_ID)))
-        retry_subflow = linear_flow.Flow(
-            constants.COMPUTE_CREATE_RETRY_SUBFLOW,
-            retry=compute_tasks.ComputeRetry())
-        retry_subflow.add(
-            compute_tasks.ComputeWait(
-                requires=(constants.COMPUTE_ID, constants.AMPHORA_ID),
-                provides=constants.COMPUTE_OBJ))
-        create_amphora_flow.add(retry_subflow)
-        create_amphora_flow.add(database_tasks.UpdateAmphoraInfo(
-            requires=(constants.AMPHORA_ID, constants.COMPUTE_OBJ),
-            provides=constants.AMPHORA))
-        retry_subflow = linear_flow.Flow(
-            constants.CREATE_AMPHORA_RETRY_SUBFLOW,
-            retry=amphora_driver_tasks.AmpRetry())
-        retry_subflow.add(
-            amphora_driver_tasks.AmphoraComputeConnectivityWait(
-                requires=constants.AMPHORA,
-                inject={'raise_retry_exception': True}))
-        create_amphora_flow.add(retry_subflow)
-        create_amphora_flow.add(database_tasks.ReloadAmphora(
-            requires=constants.AMPHORA,
-            provides=constants.AMPHORA))
-        create_amphora_flow.add(amphora_driver_tasks.AmphoraFinalize(
-            requires=constants.AMPHORA))
-        create_amphora_flow.add(database_tasks.MarkAmphoraReadyInDB(
-            requires=constants.AMPHORA))
-
-        return create_amphora_flow
-
     def get_amphora_for_lb_subflow(self, prefix, role):
         """Create a new amphora for lb."""
 

octavia/controller/worker/v2/flows/flow_utils.py

@@ -78,10 +78,6 @@ def get_update_load_balancer_flow():
     return LB_FLOWS.get_update_load_balancer_flow()
 
 
-def get_create_amphora_flow():
-    return AMP_FLOWS.get_create_amphora_flow()
-
-
 def get_delete_amphora_flow(amphora, retry_attempts=None, retry_interval=None):
     return AMP_FLOWS.get_delete_amphora_flow(amphora, retry_attempts,
                                              retry_interval)

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

@@ -944,58 +944,6 @@ class MarkAmphoraPendingUpdateInDB(BaseDatabaseTask):
         self.task_utils.mark_amphora_status_error(amphora.get(constants.ID))
 
 
-class MarkAmphoraReadyInDB(BaseDatabaseTask):
-    """This task will mark an amphora as ready in the database.
-
-    Assume sqlalchemy made sure the DB got
-    retried sufficiently - so just abort
-    """
-
-    def execute(self, amphora):
-        """Mark amphora as ready in DB.
-
-        :param amphora: Amphora to be updated.
-        :returns: None
-        """
-
-        LOG.info("Mark READY in DB for amphora: %(amp)s with compute "
-                 "id %(comp)s",
-                 {"amp": amphora.get(constants.ID),
-                  "comp": amphora[constants.COMPUTE_ID]})
-        with db_apis.session().begin() as session:
-            self.amphora_repo.update(
-                session,
-                amphora.get(constants.ID),
-                status=constants.AMPHORA_READY,
-                compute_id=amphora[constants.COMPUTE_ID],
-                lb_network_ip=amphora[constants.LB_NETWORK_IP])
-
-    def revert(self, amphora, *args, **kwargs):
-        """Mark the amphora as broken and ready to be cleaned up.
-
-        :param amphora: Amphora that was updated.
-        :returns: None
-        """
-
-        LOG.warning("Reverting mark amphora ready in DB for amp "
-                    "id %(amp)s and compute id %(comp)s",
-                    {'amp': amphora.get(constants.ID),
-                     'comp': amphora[constants.COMPUTE_ID]})
-        try:
-            with db_apis.session().begin() as session:
-                self.amphora_repo.update(
-                    session,
-                    amphora.get(constants.ID),
-                    status=constants.ERROR,
-                    compute_id=amphora[constants.COMPUTE_ID],
-                    lb_network_ip=amphora[constants.LB_NETWORK_IP])
-        except Exception as e:
-            LOG.error("Failed to update amphora %(amp)s "
-                      "status to ERROR due to: "
-                      "%(except)s", {'amp': amphora.get(constants.ID),
-                                     'except': str(e)})
-
-
 class UpdateAmphoraComputeId(BaseDatabaseTask):
     """Associate amphora with a compute in DB."""
 

octavia/db/repositories.py

@@ -1436,8 +1436,8 @@ class AmphoraRepository(BaseRepository):
         """Tests and sets an amphora status.
 
         Puts a lock on the amphora table to check the status of the
-        amphora. The status must be either AMPHORA_READY or ERROR to
+        amphora. The status must be ERROR to successfully update the
-        successfully update the amphora status.
+        amphora status.
 
         :param lock_session: A Sql Alchemy database session.
         :param id: id of Load Balancer
@@ -1451,7 +1451,7 @@ class AmphoraRepository(BaseRepository):
                .with_for_update()
                .filter_by(id=id)
                .filter(self.model_class.status != consts.DELETED).one())
-        if amp.status not in [consts.AMPHORA_READY, consts.ERROR]:
+        if amp.status != consts.ERROR:
             raise exceptions.ImmutableObject(resource=consts.AMPHORA, id=id)
         amp.status = consts.PENDING_DELETE
         lock_session.flush()

octavia/tests/functional/api/v2/test_amphora.py

@@ -107,7 +107,7 @@ class TestAmphora(base.BaseAPITest):
             'cert_expiration': None,
             'cert_busy': False,
             'role': constants.ROLE_MASTER,
-            'status': constants.AMPHORA_READY,
+            'status': constants.AMPHORA_ALLOCATED,
             'vrrp_interface': 'eth1',
             'vrrp_id': 1,
             'vrrp_priority': 100,
@@ -135,7 +135,7 @@ class TestAmphora(base.BaseAPITest):
     def test_delete(self, mock_cast):
         self.amp_args = {
             'id': uuidutils.generate_uuid(),
-            'status': constants.AMPHORA_READY,
+            'status': constants.ERROR,
         }
         with self.session.begin():
             amp = self.amphora_repo.create(self.session, **self.amp_args)
@@ -175,7 +175,7 @@ class TestAmphora(base.BaseAPITest):
     def test_delete_authorized(self, mock_cast):
         self.amp_args = {
             'id': uuidutils.generate_uuid(),
-            'status': constants.AMPHORA_READY,
+            'status': constants.ERROR,
         }
         with self.session.begin():
             amp = self.amphora_repo.create(self.session, **self.amp_args)
@@ -216,7 +216,7 @@ class TestAmphora(base.BaseAPITest):
     def test_delete_not_authorized(self, mock_cast):
         self.amp_args = {
             'id': uuidutils.generate_uuid(),
-            'status': constants.AMPHORA_READY,
+            'status': constants.ERROR,
         }
         with self.session.begin():
             amp = self.amphora_repo.create(self.session, **self.amp_args)

octavia/tests/functional/db/test_repositories.py

@@ -3898,7 +3898,7 @@ class AmphoraRepositoryTest(BaseRepositoryTest):
 
         amphora = self.create_amphora(self.FAKE_UUID_1)
         self.amphora_repo.update(self.session, amphora.id,
-                                 status=constants.AMPHORA_READY)
+                                 status='READY')
         new_amphora = self.amphora_repo.allocate_and_associate(self.session,
                                                                self.lb.id)
         self.assertIsNotNone(new_amphora)
@@ -4052,7 +4052,7 @@ class AmphoraRepositoryTest(BaseRepositoryTest):
     def test_and_set_status_for_delete(self):
         # Normal path
         amphora = self.create_amphora(self.FAKE_UUID_1,
-                                      status=constants.AMPHORA_READY)
+                                      status=constants.ERROR)
         self.amphora_repo.test_and_set_status_for_delete(self.session,
                                                          amphora.id)
         new_amphora = self.amphora_repo.get(self.session, id=amphora.id)

octavia/tests/unit/amphorae/drivers/keepalived/test_vrrp_rest_driver.py

@@ -87,7 +87,7 @@ class TestVRRPRestDriver(base.TestCase):
         self.clients[API_VERSION].upload_vrrp_config.reset_mock()
         ready_amphora_mock = mock.MagicMock()
         ready_amphora_mock.id = uuidutils.generate_uuid()
-        ready_amphora_mock.status = constants.AMPHORA_READY
+        ready_amphora_mock.status = constants.ERROR
         ready_amphora_mock.api_version = API_VERSION
 
         self.keepalived_mixin.update_vrrp_conf(
@@ -117,7 +117,7 @@ class TestVRRPRestDriver(base.TestCase):
         self.clients[API_VERSION].start_vrrp.reset_mock()
         ready_amphora_mock = mock.MagicMock()
         ready_amphora_mock.id = uuidutils.generate_uuid()
-        ready_amphora_mock.status = constants.AMPHORA_READY
+        ready_amphora_mock.status = constants.ERROR
         ready_amphora_mock.api_version = API_VERSION
 
         self.keepalived_mixin.start_vrrp_service(ready_amphora_mock)

octavia/tests/unit/controller/worker/v2/flows/test_amphora_flows.py

@@ -47,45 +47,6 @@ class TestAmphoraFlows(base.TestCase):
         self.lb = data_models.LoadBalancer(
             id=4, amphorae=[self.amp1, self.amp2, self.amp3])
 
-    def test_get_create_amphora_flow(self, mock_get_net_driver):
-
-        amp_flow = self.AmpFlow.get_create_amphora_flow()
-
-        self.assertIsInstance(amp_flow, flow.Flow)
-
-        self.assertIn(constants.AMPHORA, amp_flow.provides)
-        self.assertIn(constants.AMPHORA_ID, amp_flow.provides)
-        self.assertIn(constants.COMPUTE_ID, amp_flow.provides)
-        self.assertIn(constants.COMPUTE_OBJ, amp_flow.provides)
-        self.assertIn(constants.SERVER_PEM, amp_flow.provides)
-
-        self.assertIn(constants.BUILD_TYPE_PRIORITY, amp_flow.requires)
-        self.assertIn(constants.FLAVOR, amp_flow.requires)
-        self.assertIn(constants.AVAILABILITY_ZONE, amp_flow.requires)
-
-        self.assertEqual(5, len(amp_flow.provides))
-        self.assertEqual(4, len(amp_flow.requires))
-
-    def test_get_create_amphora_flow_cert(self, mock_get_net_driver):
-        self.AmpFlow = amphora_flows.AmphoraFlows()
-
-        amp_flow = self.AmpFlow.get_create_amphora_flow()
-
-        self.assertIsInstance(amp_flow, flow.Flow)
-
-        self.assertIn(constants.AMPHORA, amp_flow.provides)
-        self.assertIn(constants.AMPHORA_ID, amp_flow.provides)
-        self.assertIn(constants.COMPUTE_ID, amp_flow.provides)
-        self.assertIn(constants.COMPUTE_OBJ, amp_flow.provides)
-        self.assertIn(constants.SERVER_PEM, amp_flow.provides)
-
-        self.assertIn(constants.BUILD_TYPE_PRIORITY, amp_flow.requires)
-        self.assertIn(constants.FLAVOR, amp_flow.requires)
-        self.assertIn(constants.AVAILABILITY_ZONE, amp_flow.requires)
-
-        self.assertEqual(5, len(amp_flow.provides))
-        self.assertEqual(4, len(amp_flow.requires))
-
     def test_get_amphora_for_lb_flow(self, mock_get_net_driver):
 
         amp_flow = self.AmpFlow.get_amphora_for_lb_subflow(

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

@@ -938,54 +938,6 @@ class TestDatabaseTasks(base.TestCase):
             id=AMP_ID,
             status=constants.ERROR)
 
-    def test_mark_amphora_ready_in_db(self,
-                                      mock_generate_uuid,
-                                      mock_LOG,
-                                      mock_get_session,
-                                      mock_loadbalancer_repo_update,
-                                      mock_listener_repo_update,
-                                      mock_amphora_repo_update,
-                                      mock_amphora_repo_delete):
-
-        self.amphora['lb_network_ip'] = LB_NET_IP
-
-        mark_amp_ready_in_db = database_tasks.MarkAmphoraReadyInDB()
-        mark_amp_ready_in_db.execute(self.amphora)
-
-        mock_session = mock_get_session().begin().__enter__()
-
-        repo.AmphoraRepository.update.assert_called_once_with(
-            mock_session,
-            AMP_ID,
-            status=constants.AMPHORA_READY,
-            compute_id=COMPUTE_ID,
-            lb_network_ip=LB_NET_IP)
-
-        # Test the revert
-
-        mock_amphora_repo_update.reset_mock()
-        mark_amp_ready_in_db.revert(self.amphora)
-
-        repo.AmphoraRepository.update.assert_called_once_with(
-            mock_session,
-            AMP_ID,
-            status=constants.ERROR,
-            compute_id=COMPUTE_ID,
-            lb_network_ip=LB_NET_IP)
-
-        # Test the revert with exception
-
-        mock_amphora_repo_update.reset_mock()
-        mock_amphora_repo_update.side_effect = Exception('fail')
-        mark_amp_ready_in_db.revert(self.amphora)
-
-        repo.AmphoraRepository.update.assert_called_once_with(
-            mock_session,
-            AMP_ID,
-            status=constants.ERROR,
-            compute_id=COMPUTE_ID,
-            lb_network_ip=LB_NET_IP)
-
     @mock.patch('octavia.db.repositories.AmphoraRepository.get')
     def test_update_amphora_info(self,
                                  mock_amphora_repo_get,

octavia/tests/unit/controller/worker/v2/test_controller_worker.py

@@ -1547,7 +1547,7 @@ class TestControllerWorker(base.TestCase):
         mock_amphora.load_balancer_id = None
         mock_amphora.id = AMP_ID
         mock_amphora.load_balancer_id = LB_ID
-        mock_amphora.status = constants.AMPHORA_READY
+        mock_amphora.status = constants.AMPHORA_ALLOCATED
         mock_amp_repo_get.return_value = mock_amphora
         flavor_dict = {constants.LOADBALANCER_TOPOLOGY:
                        constants.TOPOLOGY_SINGLE}
@@ -1604,7 +1604,7 @@ class TestControllerWorker(base.TestCase):
         mock_amphora.load_balancer_id = None
         mock_amphora.id = AMP_ID
         mock_amphora.load_balancer_id = LB_ID
-        mock_amphora.status = constants.AMPHORA_READY
+        mock_amphora.status = constants.AMPHORA_ALLOCATED
         mock_amp_repo_get.return_value = mock_amphora
         flavor_dict = {constants.LOADBALANCER_TOPOLOGY:
                        constants.TOPOLOGY_ACTIVE_STANDBY}
@@ -1661,7 +1661,7 @@ class TestControllerWorker(base.TestCase):
         mock_amphora.load_balancer_id = None
         mock_amphora.id = AMP_ID
         mock_amphora.load_balancer_id = LB_ID
-        mock_amphora.status = constants.AMPHORA_READY
+        mock_amphora.status = constants.AMPHORA_ALLOCATED
         mock_amp_repo_get.return_value = mock_amphora
         flavor_dict = {constants.LOADBALANCER_TOPOLOGY:
                        constants.TOPOLOGY_ACTIVE_STANDBY}
@@ -1715,7 +1715,7 @@ class TestControllerWorker(base.TestCase):
         mock_amphora.load_balancer_id = None
         mock_amphora.id = AMP_ID
         mock_amphora.load_balancer_id = LB_ID
-        mock_amphora.status = constants.AMPHORA_READY
+        mock_amphora.status = constants.AMPHORA_ALLOCATED
         mock_amp_repo_get.return_value = mock_amphora
         flavor_dict = {constants.LOADBALANCER_TOPOLOGY: mock_lb.topology}
         expected_stored_params = {
@@ -1771,7 +1771,7 @@ class TestControllerWorker(base.TestCase):
         mock_amphora.load_balancer_id = None
         mock_amphora.id = AMP_ID
         mock_amphora.load_balancer_id = LB_ID
-        mock_amphora.status = constants.AMPHORA_READY
+        mock_amphora.status = constants.AMPHORA_ALLOCATED
         mock_amp_repo_get.return_value = mock_amphora
         flavor_dict = {constants.LOADBALANCER_TOPOLOGY:
                        constants.TOPOLOGY_SINGLE, 'taste': 'spicy'}
@@ -1829,7 +1829,7 @@ class TestControllerWorker(base.TestCase):
         mock_amphora.load_balancer_id = None
         mock_amphora.id = AMP_ID
         mock_amphora.load_balancer_id = LB_ID
-        mock_amphora.status = constants.AMPHORA_READY
+        mock_amphora.status = constants.AMPHORA_ALLOCATED
         mock_amp_repo_get.return_value = mock_amphora
         flavor_dict = {constants.LOADBALANCER_TOPOLOGY:
                        constants.TOPOLOGY_SINGLE}
@@ -1887,7 +1887,7 @@ class TestControllerWorker(base.TestCase):
         mock_amphora.load_balancer_id = None
         mock_amphora.id = AMP_ID
         mock_amphora.load_balancer_id = LB_ID
-        mock_amphora.status = constants.AMPHORA_READY
+        mock_amphora.status = constants.AMPHORA_ALLOCATED
         mock_amp_repo_get.return_value = mock_amphora
         flavor_dict = {constants.LOADBALANCER_TOPOLOGY:
                        constants.TOPOLOGY_SINGLE}
@@ -2006,7 +2006,7 @@ class TestControllerWorker(base.TestCase):
         mock_amphora = mock.MagicMock()
         mock_amphora.load_balancer_id = None
         mock_amphora.id = AMP_ID
-        mock_amphora.status = constants.AMPHORA_READY
+        mock_amphora.status = constants.AMPHORA_ALLOCATED
         mock_amp_repo_get.return_value = mock_amphora
         expected_stored_params = {constants.AVAILABILITY_ZONE: {},
                                   constants.BUILD_TYPE_PRIORITY:

releasenotes/notes/remove-unused-amphora-flow-and-status-constants-a3b2c4d5e6f7a8b9.yaml

@@ -0,0 +1,8 @@
+---
+other:
+  - |
+    Removed unused amphora-related code including the get_create_amphora_flow
+    function, MarkAmphoraReadyInDB task class, MARK_AMPHORA_READY_INDB
+    constant, and AMPHORA_READY status constant. Updated amphora deletion
+    logic to only allow deletion when amphora status is ERROR, which is the
+    correct behavior since AMPHORA_ALLOCATED amphorae should not be deletable.

tools/flow-list-v2.txt

@@ -2,7 +2,6 @@
 # Some flows are used by other flows, so just list the primary flows here
 # Format:
 # module class flow
-octavia.controller.worker.v2.flows.amphora_flows AmphoraFlows get_create_amphora_flow
 octavia.controller.worker.v2.flows.amphora_flows AmphoraFlows get_failover_amphora_flow
 octavia.controller.worker.v2.flows.amphora_flows AmphoraFlows cert_rotate_amphora_flow
 octavia.controller.worker.v2.flows.load_balancer_flows LoadBalancerFlows get_create_load_balancer_flow