Ensure NSX VS is always associated with NSX LBS

Add NSX VS to NSX LBS also if NSX load balancer bindings are
found in the database. This alone indeed does not imply the virtual
server is already associated with the load balancer as the bindings
could have been created when a member for another pool was added.

In order to avoid wasteful NSX round-trips this patch moves the check
for VS-LBS association after the addition of the member to the pool.
This allows for executing the check only for the first member of
a given pool.

Change-Id: I239f4c6bcf07279c523ba861abfe27e8029015c6
This commit is contained in:
Salvatore Orlando 2018-11-03 14:24:09 -07:00 committed by Adit Sarfaty
parent 2f0bc19f36
commit 0f41d6ec49
2 changed files with 44 additions and 19 deletions

View File

@ -152,11 +152,14 @@ class EdgeMemberManagerFromDict(base_mgr.Nsxv3LoadbalancerBaseManager):
lb_pool_id = binding.get('lb_pool_id')
lb_binding = nsx_db.get_nsx_lbaas_loadbalancer_binding(
context.session, lb_id)
lb_service = None
if not lb_binding:
nsx_router_id = nsx_db.get_nsx_router_id(context.session,
router_id)
lb_service = service_client.get_router_lb_service(
nsx_router_id)
virtual_server_ids = (lb_service and
lb_service.get('virtual_server_ids', []))
if not lb_service:
lb_size = lb_utils.get_lb_flavor_size(
self.flavor_plugin, context,
@ -169,17 +172,6 @@ class EdgeMemberManagerFromDict(base_mgr.Nsxv3LoadbalancerBaseManager):
self._add_loadbalancer_binding(
context, loadbalancer['id'], lb_service_id,
nsx_router_id, loadbalancer['vip_address'])
if vs_id:
try:
service_client.add_virtual_server(lb_service_id,
vs_id)
except nsxlib_exc.ManagerError:
completor(success=False)
msg = (_('Failed to attach virtual server %(vs)s '
'to lb service %(service)s') %
{'vs': vs_id, 'service': lb_service_id})
raise n_exc.BadRequest(resource='lbaas-member',
msg=msg)
else:
completor(success=False)
msg = (_('Failed to get lb service to attach virtual '
@ -197,6 +189,31 @@ class EdgeMemberManagerFromDict(base_mgr.Nsxv3LoadbalancerBaseManager):
'weight': member['weight']}]
members = (old_m + new_m) if old_m else new_m
pool_client.update_pool_with_members(lb_pool_id, members)
# Check whether the virtual server should be added to the load
# balancing server. It is safe to perform this operation after the
# member has been added to the pool. This allows us to skip this
# check if there is already a member in the pool
if vs_id and not old_m:
# load the LB service is not already loaded
if not lb_service:
nsx_router_id = nsx_db.get_nsx_router_id(context.session,
router_id)
lb_service = service_client.get_router_lb_service(
nsx_router_id)
lb_service_id = lb_service['id']
virtual_server_ids = lb_service.get('virtual_server_ids',
[])
if vs_id not in virtual_server_ids:
try:
service_client.add_virtual_server(lb_service_id, vs_id)
except nsxlib_exc.ManagerError:
completor(success=False)
msg = (_('Failed to attach virtual server %(vs)s '
'to lb service %(service)s') %
{'vs': vs_id, 'service': lb_service_id})
raise n_exc.BadRequest(resource='lbaas-member',
msg=msg)
else:
completor(success=False)
msg = (_('Failed to get pool binding to add member %s') %

View File

@ -543,7 +543,7 @@ class TestEdgeLbaasV2Member(BaseTestEdgeLbaasV2):
def _tested_entity(self):
return 'member'
def test_create(self):
def _test_create(self, lb_binding, pool_binding):
with mock.patch.object(lb_utils, 'validate_lb_member_subnet'
) as mock_validate_lb_subnet, \
mock.patch.object(self.lbv2_driver.plugin, 'get_pool_members'
@ -561,7 +561,7 @@ class TestEdgeLbaasV2Member(BaseTestEdgeLbaasV2):
mock.patch.object(self.service_client, 'get_router_lb_service'
) as mock_get_lb_service, \
mock.patch.object(nsx_db, 'add_nsx_lbaas_loadbalancer_binding'
) as mock_add_loadbalancer_bidning, \
) as mock_add_loadbalancer_binding, \
mock.patch.object(self.service_client,
'add_virtual_server'
) as mock_add_vs_to_service, \
@ -573,17 +573,19 @@ class TestEdgeLbaasV2Member(BaseTestEdgeLbaasV2):
mock_get_pool_members.return_value = [self.member]
mock_get_network.return_value = LB_NETWORK
mock_get_router.return_value = LB_ROUTER_ID
mock_get_pool_binding.return_value = POOL_BINDING
mock_get_lb_binding.return_value = None
mock_get_pool_binding.return_value = pool_binding
mock_get_lb_binding.return_value = lb_binding
mock_get_nsx_router_id.return_value = LB_ROUTER_ID
mock_get_lb_service.return_value = {'id': LB_SERVICE_ID}
mock_get_pool.return_value = LB_POOL
self.edge_driver.member.create(self.context, self.member)
mock_add_loadbalancer_bidning.assert_called_with(
self.context.session, LB_ID, LB_SERVICE_ID, LB_ROUTER_ID,
LB_VIP)
if not lb_binding:
mock_add_loadbalancer_binding.assert_called_with(
self.context.session, LB_ID, LB_SERVICE_ID, LB_ROUTER_ID,
LB_VIP)
else:
mock_add_loadbalancer_binding.assert_not_called()
mock_add_vs_to_service.assert_called_with(LB_SERVICE_ID, LB_VS_ID)
mock_update_pool_with_members.assert_called_with(LB_POOL_ID,
[LB_MEMBER])
@ -593,6 +595,12 @@ class TestEdgeLbaasV2Member(BaseTestEdgeLbaasV2):
self.member,
delete=False)
def test_create(self):
self._test_create(None, POOL_BINDING)
def test_create_existing_binding(self):
self._test_create(LB_BINDING, POOL_BINDING)
def test_create_lbs_no_router_gateway(self):
with mock.patch.object(lb_utils, 'validate_lb_member_subnet'
) as mock_validate_lb_subnet, \