Skip to content

Commit 1794a61

Browse files
Zuulopenstack-gerrit
Zuul
authored and
openstack-gerrit
committed
Merge "Fix pool creation with single LB create call"
2 parents f48146b + 3a157c0 commit 1794a61

File tree

9 files changed

+63
-48
lines changed

9 files changed

+63
-48
lines changed

‎octavia/controller/worker/v1/flows/load_balancer_flows.py

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,17 @@ def get_create_load_balancer_flow(self, topology, listeners=None):
8585

8686
post_amp_prefix = constants.POST_LB_AMP_ASSOCIATION_SUBFLOW
8787
lb_create_flow.add(
88-
self.get_post_lb_amp_association_flow(
89-
post_amp_prefix, topology, mark_active=not listeners))
88+
self.get_post_lb_amp_association_flow(post_amp_prefix, topology))
9089

9190
if listeners:
9291
lb_create_flow.add(*self._create_listeners_flow())
9392

93+
lb_create_flow.add(
94+
database_tasks.MarkLBActiveInDB(
95+
mark_subobjects=True,
96+
requires=constants.LOADBALANCER
97+
)
98+
)
9499
return lb_create_flow
95100

96101
def _create_single_topology(self):
@@ -220,16 +225,9 @@ def _create_listeners_flow(self):
220225
flows.append(
221226
self.listener_flows.get_create_all_listeners_flow()
222227
)
223-
flows.append(
224-
database_tasks.MarkLBActiveInDB(
225-
mark_subobjects=True,
226-
requires=constants.LOADBALANCER
227-
)
228-
)
229228
return flows
230229

231-
def get_post_lb_amp_association_flow(self, prefix, topology,
232-
mark_active=True):
230+
def get_post_lb_amp_association_flow(self, prefix, topology):
233231
"""Reload the loadbalancer and create networking subflows for
234232
235233
created/allocated amphorae.
@@ -253,10 +251,6 @@ def get_post_lb_amp_association_flow(self, prefix, topology,
253251

254252
post_create_LB_flow.add(database_tasks.UpdateLoadbalancerInDB(
255253
requires=[constants.LOADBALANCER, constants.UPDATE_DICT]))
256-
if mark_active:
257-
post_create_LB_flow.add(database_tasks.MarkLBActiveInDB(
258-
name=sf_name + '-' + constants.MARK_LB_ACTIVE_INDB,
259-
requires=constants.LOADBALANCER))
260254
return post_create_LB_flow
261255

262256
def _get_delete_listeners_flow(self, lb):

‎octavia/controller/worker/v1/tasks/database_tasks.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -936,6 +936,8 @@ def execute(self, loadbalancer):
936936
loadbalancer.id)
937937
for listener in loadbalancer.listeners:
938938
self._mark_listener_status(listener, constants.ACTIVE)
939+
for pool in loadbalancer.pools:
940+
self._mark_pool_status(pool, constants.ACTIVE)
939941

940942
LOG.info("Mark ACTIVE in DB for load balancer id: %s",
941943
loadbalancer.id)
@@ -1010,14 +1012,20 @@ def revert(self, loadbalancer, *args, **kwargs):
10101012
"""
10111013

10121014
if self.mark_subobjects:
1013-
LOG.debug("Marking all listeners of loadbalancer %s ERROR",
1014-
loadbalancer.id)
1015+
LOG.debug("Marking all listeners and pools of loadbalancer %s"
1016+
" ERROR", loadbalancer.id)
10151017
for listener in loadbalancer.listeners:
10161018
try:
10171019
self._mark_listener_status(listener, constants.ERROR)
10181020
except Exception:
10191021
LOG.warning("Error updating listener %s provisioning "
10201022
"status", listener.id)
1023+
for pool in loadbalancer.pools:
1024+
try:
1025+
self._mark_pool_status(pool, constants.ERROR)
1026+
except Exception:
1027+
LOG.warning("Error updating pool %s provisioning "
1028+
"status", pool.id)
10211029

10221030

10231031
class UpdateLBServerGroupInDB(BaseDatabaseTask):

‎octavia/controller/worker/v2/flows/load_balancer_flows.py

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,18 @@ def get_create_load_balancer_flow(self, topology, listeners=None):
9191

9292
post_amp_prefix = constants.POST_LB_AMP_ASSOCIATION_SUBFLOW
9393
lb_create_flow.add(
94-
self.get_post_lb_amp_association_flow(
95-
post_amp_prefix, topology, mark_active=not listeners))
94+
self.get_post_lb_amp_association_flow(post_amp_prefix, topology))
9695

9796
if listeners:
9897
lb_create_flow.add(*self._create_listeners_flow())
9998

99+
lb_create_flow.add(
100+
database_tasks.MarkLBActiveInDB(
101+
mark_subobjects=True,
102+
requires=constants.LOADBALANCER
103+
)
104+
)
105+
100106
if CONF.controller_worker.event_notifications:
101107
lb_create_flow.add(
102108
notification_tasks.SendCreateNotification(
@@ -225,16 +231,9 @@ def _create_listeners_flow(self):
225231
flows.append(
226232
self.listener_flows.get_create_all_listeners_flow()
227233
)
228-
flows.append(
229-
database_tasks.MarkLBActiveInDB(
230-
mark_subobjects=True,
231-
requires=constants.LOADBALANCER
232-
)
233-
)
234234
return flows
235235

236-
def get_post_lb_amp_association_flow(self, prefix, topology,
237-
mark_active=True):
236+
def get_post_lb_amp_association_flow(self, prefix, topology):
238237
"""Reload the loadbalancer and create networking subflows for
239238
240239
created/allocated amphorae.
@@ -257,10 +256,6 @@ def get_post_lb_amp_association_flow(self, prefix, topology,
257256

258257
post_create_LB_flow.add(database_tasks.UpdateLoadbalancerInDB(
259258
requires=[constants.LOADBALANCER, constants.UPDATE_DICT]))
260-
if mark_active:
261-
post_create_LB_flow.add(database_tasks.MarkLBActiveInDB(
262-
name=sf_name + '-' + constants.MARK_LB_ACTIVE_INDB,
263-
requires=constants.LOADBALANCER))
264259
return post_create_LB_flow
265260

266261
def _get_delete_listeners_flow(self, listeners):

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,6 +1038,8 @@ def execute(self, loadbalancer):
10381038
id=loadbalancer[constants.LOADBALANCER_ID])
10391039
for listener in db_lb.listeners:
10401040
self._mark_listener_status(listener, constants.ACTIVE)
1041+
for pool in db_lb.pools:
1042+
self._mark_pool_status(pool, constants.ACTIVE)
10411043

10421044
LOG.info("Mark ACTIVE in DB for load balancer id: %s",
10431045
loadbalancer[constants.LOADBALANCER_ID])
@@ -1112,8 +1114,8 @@ def revert(self, loadbalancer, *args, **kwargs):
11121114
"""
11131115

11141116
if self.mark_subobjects:
1115-
LOG.debug("Marking all listeners of loadbalancer %s ERROR",
1116-
loadbalancer[constants.LOADBALANCER_ID])
1117+
LOG.debug("Marking all listeners and pools of loadbalancer %s"
1118+
" ERROR", loadbalancer[constants.LOADBALANCER_ID])
11171119
db_lb = self.loadbalancer_repo.get(
11181120
db_apis.get_session(),
11191121
id=loadbalancer[constants.LOADBALANCER_ID])
@@ -1123,6 +1125,12 @@ def revert(self, loadbalancer, *args, **kwargs):
11231125
except Exception:
11241126
LOG.warning("Error updating listener %s provisioning "
11251127
"status", listener.id)
1128+
for pool in db_lb.pools:
1129+
try:
1130+
self._mark_pool_status(pool, constants.ERROR)
1131+
except Exception:
1132+
LOG.warning("Error updating POOL %s provisioning "
1133+
"status", pool.id)
11261134

11271135

11281136
class MarkLBActiveInDBByListener(BaseDatabaseTask):

‎octavia/tests/unit/controller/worker/v1/flows/test_load_balancer_flows.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ def test_get_post_lb_amp_association_flow(self, mock_get_net_driver):
160160
self.assertEqual(4, len(amp_flow.provides))
161161
self.assertEqual(2, len(amp_flow.requires))
162162

163-
# Test mark_active=False
164163
amp_flow = self.LBFlow.get_post_lb_amp_association_flow(
165164
'123', constants.TOPOLOGY_ACTIVE_STANDBY)
166165

‎octavia/tests/unit/controller/worker/v1/tasks/test_database_tasks.py

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,14 +1139,16 @@ def test_mark_LB_active_in_db_full_graph(self,
11391139
provisioning_status=constants.ACTIVE),
11401140
mock.call('TEST', listeners[1].id,
11411141
provisioning_status=constants.ACTIVE)])
1142-
self.assertEqual(2, repo.PoolRepository.update.call_count)
1142+
self.assertEqual(5, repo.PoolRepository.update.call_count)
11431143
repo.PoolRepository.update.assert_has_calls(
11441144
[mock.call('TEST', default_pool.id,
11451145
provisioning_status=constants.ACTIVE),
11461146
mock.call('TEST', redirect_pool.id,
1147+
provisioning_status=constants.ACTIVE),
1148+
mock.call('TEST', unused_pool.id,
11471149
provisioning_status=constants.ACTIVE)])
1148-
self.assertEqual(4, repo.MemberRepository.update.call_count)
1149-
repo.MemberRepository.update.assert_has_calls(
1150+
self.assertEqual(8, repo.MemberRepository.update.call_count)
1151+
repo.MemberRepository.update.has_calls(
11501152
[mock.call('TEST', members1[0].id,
11511153
provisioning_status=constants.ACTIVE),
11521154
mock.call('TEST', members1[1].id,
@@ -1155,8 +1157,8 @@ def test_mark_LB_active_in_db_full_graph(self,
11551157
provisioning_status=constants.ACTIVE),
11561158
mock.call('TEST', members2[1].id,
11571159
provisioning_status=constants.ACTIVE)])
1158-
self.assertEqual(1, repo.HealthMonitorRepository.update.call_count)
1159-
repo.HealthMonitorRepository.update.assert_has_calls(
1160+
self.assertEqual(2, repo.HealthMonitorRepository.update.call_count)
1161+
repo.HealthMonitorRepository.update.has_calls(
11601162
[mock.call('TEST', health_monitor.id,
11611163
provisioning_status=constants.ACTIVE)])
11621164
self.assertEqual(1, repo.L7PolicyRepository.update.call_count)
@@ -1184,14 +1186,14 @@ def test_mark_LB_active_in_db_full_graph(self,
11841186
provisioning_status=constants.ERROR),
11851187
mock.call('TEST', listeners[1].id,
11861188
provisioning_status=constants.ERROR)])
1187-
self.assertEqual(2, repo.PoolRepository.update.call_count)
1188-
repo.PoolRepository.update.assert_has_calls(
1189+
self.assertEqual(5, repo.PoolRepository.update.call_count)
1190+
repo.PoolRepository.update.has_calls(
11891191
[mock.call('TEST', default_pool.id,
11901192
provisioning_status=constants.ERROR),
11911193
mock.call('TEST', redirect_pool.id,
11921194
provisioning_status=constants.ERROR)])
1193-
self.assertEqual(4, repo.MemberRepository.update.call_count)
1194-
repo.MemberRepository.update.assert_has_calls(
1195+
self.assertEqual(8, repo.MemberRepository.update.call_count)
1196+
repo.MemberRepository.update.has_calls(
11951197
[mock.call('TEST', members1[0].id,
11961198
provisioning_status=constants.ERROR),
11971199
mock.call('TEST', members1[1].id,
@@ -1200,8 +1202,8 @@ def test_mark_LB_active_in_db_full_graph(self,
12001202
provisioning_status=constants.ERROR),
12011203
mock.call('TEST', members2[1].id,
12021204
provisioning_status=constants.ERROR)])
1203-
self.assertEqual(1, repo.HealthMonitorRepository.update.call_count)
1204-
repo.HealthMonitorRepository.update.assert_has_calls(
1205+
self.assertEqual(2, repo.HealthMonitorRepository.update.call_count)
1206+
repo.HealthMonitorRepository.update.has_calls(
12051207
[mock.call('TEST', health_monitor.id,
12061208
provisioning_status=constants.ERROR)])
12071209
self.assertEqual(1, repo.L7PolicyRepository.update.call_count)

‎octavia/tests/unit/controller/worker/v2/flows/test_load_balancer_flows.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,6 @@ def test_get_post_lb_amp_association_flow(self, mock_get_net_driver,
208208
self.assertEqual(2, len(amp_flow.requires), amp_flow.requires)
209209
self.assertEqual(4, len(amp_flow.provides), amp_flow.provides)
210210

211-
# Test mark_active=False
212211
amp_flow = self.LBFlow.get_post_lb_amp_association_flow(
213212
'123', constants.TOPOLOGY_ACTIVE_STANDBY)
214213

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,6 +1315,8 @@ def test_mark_LB_active_in_db_full_graph(self,
13151315
[mock.call('TEST', default_pool.id,
13161316
provisioning_status=constants.ACTIVE),
13171317
mock.call('TEST', redirect_pool.id,
1318+
provisioning_status=constants.ACTIVE),
1319+
mock.call('TEST', unused_pool.id,
13181320
provisioning_status=constants.ACTIVE)])
13191321
repo.HealthMonitorRepository.update.assert_has_calls(
13201322
[mock.call('TEST', health_monitor.id,
@@ -1347,9 +1349,12 @@ def test_mark_LB_active_in_db_full_graph(self,
13471349
[mock.call('TEST', default_pool.id,
13481350
provisioning_status=constants.ERROR),
13491351
mock.call('TEST', redirect_pool.id,
1350-
provisioning_status=constants.ERROR)])
1351-
self.assertEqual(1, repo.HealthMonitorRepository.update.call_count)
1352-
repo.HealthMonitorRepository.update.assert_has_calls(
1352+
provisioning_status=constants.ERROR),
1353+
mock.call('TEST', unused_pool.id,
1354+
provisioning_status=constants.ERROR)
1355+
])
1356+
self.assertEqual(2, repo.HealthMonitorRepository.update.call_count)
1357+
repo.HealthMonitorRepository.update.has_calls(
13531358
[mock.call('TEST', health_monitor.id,
13541359
provisioning_status=constants.ERROR)])
13551360
self.assertEqual(1, repo.L7PolicyRepository.update.call_count)

‎releasenotes/notes/fix-pool-prov-status-on-lb-single-create-897070aee0a42da6.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
Fixed a bug that didn't set the correct provisioning_status for unattached
5+
pools when creating a fully-populated load balancer.

0 commit comments

Comments
 (0)