diff --git a/octavia/controller/worker/v1/flows/load_balancer_flows.py b/octavia/controller/worker/v1/flows/load_balancer_flows.py index 96433be439..0a1de08c07 100644 --- a/octavia/controller/worker/v1/flows/load_balancer_flows.py +++ b/octavia/controller/worker/v1/flows/load_balancer_flows.py @@ -85,12 +85,17 @@ class LoadBalancerFlows(object): post_amp_prefix = constants.POST_LB_AMP_ASSOCIATION_SUBFLOW lb_create_flow.add( - self.get_post_lb_amp_association_flow( - post_amp_prefix, topology, mark_active=(not listeners))) + self.get_post_lb_amp_association_flow(post_amp_prefix, topology)) if listeners: lb_create_flow.add(*self._create_listeners_flow()) + lb_create_flow.add( + database_tasks.MarkLBActiveInDB( + mark_subobjects=True, + requires=constants.LOADBALANCER + ) + ) return lb_create_flow def _create_single_topology(self): @@ -220,16 +225,9 @@ class LoadBalancerFlows(object): flows.append( self.listener_flows.get_create_all_listeners_flow() ) - flows.append( - database_tasks.MarkLBActiveInDB( - mark_subobjects=True, - requires=constants.LOADBALANCER - ) - ) return flows - def get_post_lb_amp_association_flow(self, prefix, topology, - mark_active=True): + def get_post_lb_amp_association_flow(self, prefix, topology): """Reload the loadbalancer and create networking subflows for created/allocated amphorae. @@ -253,10 +251,6 @@ class LoadBalancerFlows(object): post_create_LB_flow.add(database_tasks.UpdateLoadbalancerInDB( requires=[constants.LOADBALANCER, constants.UPDATE_DICT])) - if mark_active: - post_create_LB_flow.add(database_tasks.MarkLBActiveInDB( - name=sf_name + '-' + constants.MARK_LB_ACTIVE_INDB, - requires=constants.LOADBALANCER)) return post_create_LB_flow def _get_delete_listeners_flow(self, lb): diff --git a/octavia/controller/worker/v1/tasks/database_tasks.py b/octavia/controller/worker/v1/tasks/database_tasks.py index f8e23cec21..86613dcd71 100644 --- a/octavia/controller/worker/v1/tasks/database_tasks.py +++ b/octavia/controller/worker/v1/tasks/database_tasks.py @@ -926,6 +926,8 @@ class MarkLBActiveInDB(BaseDatabaseTask): loadbalancer.id) for listener in loadbalancer.listeners: self._mark_listener_status(listener, constants.ACTIVE) + for pool in loadbalancer.pools: + self._mark_pool_status(pool, constants.ACTIVE) LOG.info("Mark ACTIVE in DB for load balancer id: %s", loadbalancer.id) @@ -1000,14 +1002,20 @@ class MarkLBActiveInDB(BaseDatabaseTask): """ if self.mark_subobjects: - LOG.debug("Marking all listeners of loadbalancer %s ERROR", - loadbalancer.id) + LOG.debug("Marking all listeners and pools of loadbalancer %s" + " ERROR", loadbalancer.id) for listener in loadbalancer.listeners: try: self._mark_listener_status(listener, constants.ERROR) except Exception: LOG.warning("Error updating listener %s provisioning " "status", listener.id) + for pool in loadbalancer.pools: + try: + self._mark_pool_status(pool, constants.ERROR) + except Exception: + LOG.warning("Error updating pool %s provisioning " + "status", pool.id) class UpdateLBServerGroupInDB(BaseDatabaseTask): diff --git a/octavia/controller/worker/v2/flows/load_balancer_flows.py b/octavia/controller/worker/v2/flows/load_balancer_flows.py index 53a290bb70..baf87c1bb0 100644 --- a/octavia/controller/worker/v2/flows/load_balancer_flows.py +++ b/octavia/controller/worker/v2/flows/load_balancer_flows.py @@ -87,12 +87,18 @@ class LoadBalancerFlows(object): post_amp_prefix = constants.POST_LB_AMP_ASSOCIATION_SUBFLOW lb_create_flow.add( - self.get_post_lb_amp_association_flow( - post_amp_prefix, topology, mark_active=(not listeners))) + self.get_post_lb_amp_association_flow(post_amp_prefix, topology)) if listeners: lb_create_flow.add(*self._create_listeners_flow()) + lb_create_flow.add( + database_tasks.MarkLBActiveInDB( + mark_subobjects=True, + requires=constants.LOADBALANCER + ) + ) + return lb_create_flow def _create_single_topology(self): @@ -214,16 +220,9 @@ class LoadBalancerFlows(object): flows.append( self.listener_flows.get_create_all_listeners_flow() ) - flows.append( - database_tasks.MarkLBActiveInDB( - mark_subobjects=True, - requires=constants.LOADBALANCER - ) - ) return flows - def get_post_lb_amp_association_flow(self, prefix, topology, - mark_active=True): + def get_post_lb_amp_association_flow(self, prefix, topology): """Reload the loadbalancer and create networking subflows for created/allocated amphorae. @@ -246,10 +245,6 @@ class LoadBalancerFlows(object): post_create_LB_flow.add(database_tasks.UpdateLoadbalancerInDB( requires=[constants.LOADBALANCER, constants.UPDATE_DICT])) - if mark_active: - post_create_LB_flow.add(database_tasks.MarkLBActiveInDB( - name=sf_name + '-' + constants.MARK_LB_ACTIVE_INDB, - requires=constants.LOADBALANCER)) return post_create_LB_flow def _get_delete_listeners_flow(self, listeners): diff --git a/octavia/controller/worker/v2/tasks/database_tasks.py b/octavia/controller/worker/v2/tasks/database_tasks.py index cd682cd083..bac411868d 100644 --- a/octavia/controller/worker/v2/tasks/database_tasks.py +++ b/octavia/controller/worker/v2/tasks/database_tasks.py @@ -1000,6 +1000,8 @@ class MarkLBActiveInDB(BaseDatabaseTask): id=loadbalancer[constants.LOADBALANCER_ID]) for listener in db_lb.listeners: self._mark_listener_status(listener, constants.ACTIVE) + for pool in db_lb.pools: + self._mark_pool_status(pool, constants.ACTIVE) LOG.info("Mark ACTIVE in DB for load balancer id: %s", loadbalancer[constants.LOADBALANCER_ID]) @@ -1074,8 +1076,8 @@ class MarkLBActiveInDB(BaseDatabaseTask): """ if self.mark_subobjects: - LOG.debug("Marking all listeners of loadbalancer %s ERROR", - loadbalancer[constants.LOADBALANCER_ID]) + LOG.debug("Marking all listeners and pools of loadbalancer %s" + " ERROR", loadbalancer[constants.LOADBALANCER_ID]) db_lb = self.loadbalancer_repo.get( db_apis.get_session(), id=loadbalancer[constants.LOADBALANCER_ID]) @@ -1085,6 +1087,12 @@ class MarkLBActiveInDB(BaseDatabaseTask): except Exception: LOG.warning("Error updating listener %s provisioning " "status", listener.id) + for pool in db_lb.pools: + try: + self._mark_pool_status(pool, constants.ERROR) + except Exception: + LOG.warning("Error updating POOL %s provisioning " + "status", pool.id) class MarkLBActiveInDBByListener(BaseDatabaseTask): diff --git a/octavia/tests/unit/controller/worker/v1/flows/test_load_balancer_flows.py b/octavia/tests/unit/controller/worker/v1/flows/test_load_balancer_flows.py index f4e87b65bc..e3ec1d9b5a 100644 --- a/octavia/tests/unit/controller/worker/v1/flows/test_load_balancer_flows.py +++ b/octavia/tests/unit/controller/worker/v1/flows/test_load_balancer_flows.py @@ -160,7 +160,6 @@ class TestLoadBalancerFlows(base.TestCase): self.assertEqual(4, len(amp_flow.provides)) self.assertEqual(2, len(amp_flow.requires)) - # Test mark_active=False amp_flow = self.LBFlow.get_post_lb_amp_association_flow( '123', constants.TOPOLOGY_ACTIVE_STANDBY) 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 2a1cd17cae..680859d113 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 @@ -1139,13 +1139,15 @@ class TestDatabaseTasks(base.TestCase): provisioning_status=constants.ACTIVE), mock.call('TEST', listeners[1].id, provisioning_status=constants.ACTIVE)]) - self.assertEqual(2, repo.PoolRepository.update.call_count) - repo.PoolRepository.update.has_calls( + self.assertEqual(5, repo.PoolRepository.update.call_count) + repo.PoolRepository.update.assert_has_calls( [mock.call('TEST', default_pool.id, provisioning_status=constants.ACTIVE), mock.call('TEST', redirect_pool.id, + provisioning_status=constants.ACTIVE), + mock.call('TEST', unused_pool.id, provisioning_status=constants.ACTIVE)]) - self.assertEqual(4, repo.MemberRepository.update.call_count) + self.assertEqual(8, repo.MemberRepository.update.call_count) repo.MemberRepository.update.has_calls( [mock.call('TEST', members1[0].id, provisioning_status=constants.ACTIVE), @@ -1155,7 +1157,7 @@ class TestDatabaseTasks(base.TestCase): provisioning_status=constants.ACTIVE), mock.call('TEST', members2[1].id, provisioning_status=constants.ACTIVE)]) - self.assertEqual(1, repo.HealthMonitorRepository.update.call_count) + self.assertEqual(2, repo.HealthMonitorRepository.update.call_count) repo.HealthMonitorRepository.update.has_calls( [mock.call('TEST', health_monitor.id, provisioning_status=constants.ACTIVE)]) @@ -1184,13 +1186,13 @@ class TestDatabaseTasks(base.TestCase): provisioning_status=constants.ERROR), mock.call('TEST', listeners[1].id, provisioning_status=constants.ERROR)]) - self.assertEqual(2, repo.PoolRepository.update.call_count) + self.assertEqual(5, repo.PoolRepository.update.call_count) repo.PoolRepository.update.has_calls( [mock.call('TEST', default_pool.id, provisioning_status=constants.ERROR), mock.call('TEST', redirect_pool.id, provisioning_status=constants.ERROR)]) - self.assertEqual(4, repo.MemberRepository.update.call_count) + self.assertEqual(8, repo.MemberRepository.update.call_count) repo.MemberRepository.update.has_calls( [mock.call('TEST', members1[0].id, provisioning_status=constants.ERROR), @@ -1200,7 +1202,7 @@ class TestDatabaseTasks(base.TestCase): provisioning_status=constants.ERROR), mock.call('TEST', members2[1].id, provisioning_status=constants.ERROR)]) - self.assertEqual(1, repo.HealthMonitorRepository.update.call_count) + self.assertEqual(2, repo.HealthMonitorRepository.update.call_count) repo.HealthMonitorRepository.update.has_calls( [mock.call('TEST', health_monitor.id, provisioning_status=constants.ERROR)]) diff --git a/octavia/tests/unit/controller/worker/v2/flows/test_load_balancer_flows.py b/octavia/tests/unit/controller/worker/v2/flows/test_load_balancer_flows.py index 4ffec76be8..ee343304c5 100644 --- a/octavia/tests/unit/controller/worker/v2/flows/test_load_balancer_flows.py +++ b/octavia/tests/unit/controller/worker/v2/flows/test_load_balancer_flows.py @@ -183,7 +183,6 @@ class TestLoadBalancerFlows(base.TestCase): self.assertEqual(2, len(amp_flow.requires), amp_flow.requires) self.assertEqual(4, len(amp_flow.provides), amp_flow.provides) - # Test mark_active=False amp_flow = self.LBFlow.get_post_lb_amp_association_flow( '123', constants.TOPOLOGY_ACTIVE_STANDBY) 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 378a0bf271..50fad39073 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 @@ -1274,6 +1274,8 @@ class TestDatabaseTasks(base.TestCase): [mock.call('TEST', default_pool.id, provisioning_status=constants.ACTIVE), mock.call('TEST', redirect_pool.id, + provisioning_status=constants.ACTIVE), + mock.call('TEST', unused_pool.id, provisioning_status=constants.ACTIVE)]) repo.HealthMonitorRepository.update.has_calls( [mock.call('TEST', health_monitor.id, @@ -1306,8 +1308,11 @@ class TestDatabaseTasks(base.TestCase): [mock.call('TEST', default_pool.id, provisioning_status=constants.ERROR), mock.call('TEST', redirect_pool.id, - provisioning_status=constants.ERROR)]) - self.assertEqual(1, repo.HealthMonitorRepository.update.call_count) + provisioning_status=constants.ERROR), + mock.call('TEST', unused_pool.id, + provisioning_status=constants.ERROR) + ]) + self.assertEqual(2, repo.HealthMonitorRepository.update.call_count) repo.HealthMonitorRepository.update.has_calls( [mock.call('TEST', health_monitor.id, provisioning_status=constants.ERROR)]) diff --git a/releasenotes/notes/fix-pool-prov-status-on-lb-single-create-897070aee0a42da6.yaml b/releasenotes/notes/fix-pool-prov-status-on-lb-single-create-897070aee0a42da6.yaml new file mode 100644 index 0000000000..c95b7883b1 --- /dev/null +++ b/releasenotes/notes/fix-pool-prov-status-on-lb-single-create-897070aee0a42da6.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixed a bug that didn't set the correct provisioning_status for unattached + pools when creating a fully-populated load balancer.