From 0503979ca44aa7c66964e39b258d843dc4813e35 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Wed, 9 Aug 2017 15:54:22 +0300 Subject: [PATCH] NSX|V: skip interface actions on old lbaas member create For older lbaas objects, which are still on an exclusive router edges, there is no need to add an interface to the edge when a member is created. If we do try to add the edge - the nsx backend will issue an error. Change-Id: I8819aaafabdd8deaa4e5423b1da3c366bc42ab47 --- .../services/lbaas/nsx_v/lbaas_common.py | 28 +++++++++++++- .../lbaas/nsx_v/v2/loadbalancer_mgr.py | 4 +- .../services/lbaas/nsx_v/v2/member_mgr.py | 15 +++++--- .../nsx_v/test_edge_loadbalancer_driver_v2.py | 37 ++++++++++++++++++- 4 files changed, 75 insertions(+), 9 deletions(-) diff --git a/vmware_nsx/services/lbaas/nsx_v/lbaas_common.py b/vmware_nsx/services/lbaas/nsx_v/lbaas_common.py index bc9d1dc502..1bcfb09a46 100644 --- a/vmware_nsx/services/lbaas/nsx_v/lbaas_common.py +++ b/vmware_nsx/services/lbaas/nsx_v/lbaas_common.py @@ -15,6 +15,8 @@ import netaddr +from oslo_log import log as logging + from neutron_lib import constants from neutron_lib import exceptions as n_exc @@ -23,7 +25,10 @@ from vmware_nsx.common import locking from vmware_nsx.db import nsxv_db from vmware_nsx.plugins.nsx_v.vshield import edge_utils +LOG = logging.getLogger(__name__) + MEMBER_ID_PFX = 'member-' +RESOURCE_ID_PFX = 'lbaas-' def get_member_id(member_id): @@ -31,7 +36,7 @@ def get_member_id(member_id): def get_lb_resource_id(lb_id): - return ('lbaas-' + lb_id)[:36] + return (RESOURCE_ID_PFX + lb_id)[:36] def get_lb_edge_name(context, lb_id): @@ -278,3 +283,24 @@ def enable_edge_acceleration(vcns, edge_id): config['enabled'] = True config['featureType'] = 'loadbalancer_4.0' vcns.enable_service_loadbalancer(edge_id, config) + + +def is_lb_on_router_edge(context, core_plugin, edge_id): + binding = nsxv_db.get_nsxv_router_binding_by_edge( + context.session, edge_id) + router_id = binding['router_id'] + if router_id.startswith(RESOURCE_ID_PFX): + # New lbaas edge + return False + + # verify that this is a router (and an exclusive one) + try: + router = core_plugin.get_router(context, router_id) + if router.get('router_type') == 'exclusive': + return True + except Exception: + pass + LOG.error("Edge %(edge)s router %(rtr)s is not an lbaas edge, but also " + "not an exclusive router", + {'edge': edge_id, 'rtr': router_id}) + return False diff --git a/vmware_nsx/services/lbaas/nsx_v/v2/loadbalancer_mgr.py b/vmware_nsx/services/lbaas/nsx_v/v2/loadbalancer_mgr.py index aa0467886d..3deeb54b2c 100644 --- a/vmware_nsx/services/lbaas/nsx_v/v2/loadbalancer_mgr.py +++ b/vmware_nsx/services/lbaas/nsx_v/v2/loadbalancer_mgr.py @@ -96,7 +96,9 @@ class EdgeLoadBalancerManager(base_mgr.EdgeLoadbalancerBaseManager): lb_common.set_lb_firewall_default_rule( self.vcns, binding['edge_id'], 'deny') if edge_binding: - if edge_binding['router_id'].startswith('lbaas-'): + old_lb = lb_common.is_lb_on_router_edge( + context, self.core_plugin, binding['edge_id']) + if not old_lb: resource_id = lb_common.get_lb_resource_id(lb.id) self.core_plugin.edge_manager.delete_lrouter( context, resource_id, dist=False) diff --git a/vmware_nsx/services/lbaas/nsx_v/v2/member_mgr.py b/vmware_nsx/services/lbaas/nsx_v/v2/member_mgr.py index 168e81b4f6..918d32a391 100644 --- a/vmware_nsx/services/lbaas/nsx_v/v2/member_mgr.py +++ b/vmware_nsx/services/lbaas/nsx_v/v2/member_mgr.py @@ -61,12 +61,15 @@ class EdgeMemberManager(base_mgr.EdgeLoadbalancerBaseManager): edge_pool_id = pool_binding['edge_pool_id'] with locking.LockManager.get_lock(edge_id): - # Verify that Edge appliance is connected to the member's subnet - if not lb_common.get_lb_interface( - context, self.core_plugin, lb_id, member.subnet_id): - lb_common.create_lb_interface( - context, self.core_plugin, lb_id, member.subnet_id, - member.tenant_id) + if not lb_common.is_lb_on_router_edge( + context.elevated(), self.core_plugin, edge_id): + # Verify that Edge appliance is connected to the member's + # subnet (only if this is a dedicated loadbalancer edge) + if not lb_common.get_lb_interface( + context, self.core_plugin, lb_id, member.subnet_id): + lb_common.create_lb_interface( + context, self.core_plugin, lb_id, member.subnet_id, + member.tenant_id) edge_pool = self.vcns.get_pool(edge_id, edge_pool_id)[1] edge_member = { diff --git a/vmware_nsx/tests/unit/nsx_v/test_edge_loadbalancer_driver_v2.py b/vmware_nsx/tests/unit/nsx_v/test_edge_loadbalancer_driver_v2.py index 39f59d5c91..e52cd2640e 100644 --- a/vmware_nsx/tests/unit/nsx_v/test_edge_loadbalancer_driver_v2.py +++ b/vmware_nsx/tests/unit/nsx_v/test_edge_loadbalancer_driver_v2.py @@ -220,7 +220,7 @@ class TestEdgeLbaasV2Loadbalancer(BaseTestEdgeLbaasV2): self.lbv2_driver.load_balancer.successful_completion) mock_successful_completion.assert_called_with(self.context, new_lb) - def test_delete(self): + def test_delete_old(self): with mock.patch.object(nsxv_db, 'get_nsxv_lbaas_loadbalancer_binding' ) as mock_get_binding, \ mock.patch.object(lb_common, 'del_vip_fw_rule') as mock_del_fwr, \ @@ -232,6 +232,8 @@ class TestEdgeLbaasV2Loadbalancer(BaseTestEdgeLbaasV2): ) as mock_del_binding, \ mock.patch.object(self.core_plugin, 'get_ports' ) as mock_get_ports, \ + mock.patch.object(self.core_plugin, 'get_router', + return_value={'router_type': 'exclusive'}), \ mock.patch.object(nsxv_db, 'get_nsxv_router_binding_by_edge' ) as mock_get_r_binding: mock_get_binding.return_value = LB_BINDING @@ -255,6 +257,37 @@ class TestEdgeLbaasV2Loadbalancer(BaseTestEdgeLbaasV2): self.lb, delete=True) + def test_delete_new(self): + with mock.patch.object(nsxv_db, 'get_nsxv_lbaas_loadbalancer_binding' + ) as mock_get_binding, \ + mock.patch.object(lb_common, 'set_lb_firewall_default_rule' + ) as mock_set_fw_rule, \ + mock.patch.object(nsxv_db, 'del_nsxv_lbaas_loadbalancer_binding', + ) as mock_del_binding, \ + mock.patch.object(self.core_plugin, 'get_ports' + ) as mock_get_ports, \ + mock.patch.object(self.core_plugin.edge_manager, 'delete_lrouter' + ) as mock_delete_lrouter, \ + mock.patch.object(nsxv_db, 'get_nsxv_router_binding_by_edge' + ) as mock_get_r_binding: + mock_get_binding.return_value = LB_BINDING + mock_get_ports.return_value = [] + router_id = 'lbaas-xxxx' + mock_get_r_binding.return_value = {'router_id': router_id} + self.edge_driver.loadbalancer.delete(self.context, self.lb) + + mock_del_binding.assert_called_with(self.context.session, + LB_ID) + mock_set_fw_rule.assert_called_with( + self.edge_driver.vcns, LB_EDGE_ID, 'deny') + mock_delete_lrouter.assert_called_with( + mock.ANY, 'lbaas-' + LB_ID, dist=False) + mock_successful_completion = ( + self.lbv2_driver.load_balancer.successful_completion) + mock_successful_completion.assert_called_with(self.context, + self.lb, + delete=True) + def test_stats(self): pass @@ -494,6 +527,8 @@ class TestEdgeLbaasV2Member(BaseTestEdgeLbaasV2): ) as mock_get_lb_binding, \ mock.patch.object(nsxv_db, 'get_nsxv_lbaas_pool_binding' ) as mock_get_pool_binding, \ + mock.patch.object(nsxv_db, 'get_nsxv_router_binding_by_edge' + ), \ mock.patch.object(self.edge_driver.vcns, 'get_pool' ) as mock_get_pool, \ mock.patch.object(self.edge_driver.vcns, 'update_pool'