From b263b592b9a2f4bdca1d62b6482342e299c53f70 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Wed, 17 Oct 2018 10:23:28 +0300 Subject: [PATCH] NSX|V3: Fix LB VIP advertisement The LB VIP should be advertised by the Tier1 router only if it is on the external network. To do that, the global advertise vp flag will not be set, and instead a rule with a filter to advertise only the VIPs on the external network is added. In addition, an admin utility is added to update already existing routers with loadbalancers. Since VPNaaS also uses the router advertisement rules, its code was also updated so that each application will handle only its own rules. Change-Id: Ibfac0406a8c3009c323828cc42c96012e70cb0a9 --- doc/source/admin_util.rst | 5 ++ .../lbaas/nsx_v3/implementation/lb_utils.py | 19 +++++ .../nsx_v3/implementation/loadbalancer_mgr.py | 7 +- .../lbaas/nsx_v3/implementation/member_mgr.py | 7 +- .../services/vpnaas/nsxv3/ipsec_driver.py | 11 ++- .../shell/admin/plugins/common/constants.py | 1 + .../plugins/nsxv3/resources/loadbalancer.py | 57 +++++++++++++++ vmware_nsx/shell/resources.py | 2 + .../unit/services/lbaas/test_nsxv3_driver.py | 73 +++++++++++++++++++ 9 files changed, 172 insertions(+), 10 deletions(-) diff --git a/doc/source/admin_util.rst b/doc/source/admin_util.rst index b9098ab1ba..4134ef93d7 100644 --- a/doc/source/admin_util.rst +++ b/doc/source/admin_util.rst @@ -534,6 +534,11 @@ LBaaS nsxadmin -r lb-monitors -o list +- Update advertisement of LB vips on routers:: + + nsxadmin -r lb-advertisement -o nsx-update + + Rate Limit ~~~~~~~~~~ - Show the current NSX rate limit:: diff --git a/vmware_nsx/services/lbaas/nsx_v3/implementation/lb_utils.py b/vmware_nsx/services/lbaas/nsx_v3/implementation/lb_utils.py index 10ef69bc38..df0c4723b5 100644 --- a/vmware_nsx/services/lbaas/nsx_v3/implementation/lb_utils.py +++ b/vmware_nsx/services/lbaas/nsx_v3/implementation/lb_utils.py @@ -20,6 +20,9 @@ from neutron_lib import exceptions as n_exc from vmware_nsx._i18n import _ from vmware_nsx.db import db as nsx_db from vmware_nsx.services.lbaas import lb_const +from vmware_nsxlib.v3 import nsx_constants + +ADV_RULE_NAME = 'LB external VIP advertisement' def get_tags(plugin, resource_id, resource_type, project_id, project_name): @@ -208,3 +211,19 @@ def remove_rule_from_policy(rule): def update_rule_in_policy(rule): remove_rule_from_policy(rule) rule['policy']['rules'].append(rule) + + +def update_router_lb_vip_advertisement(context, core_plugin, router, + nsx_router_id): + # Add a rule to advertise external vips on the router + external_subnets = core_plugin._find_router_gw_subnets(context, router) + external_cidrs = [s['cidr'] for s in external_subnets] + if external_cidrs: + adv_rule = { + 'display_name': ADV_RULE_NAME, + 'action': nsx_constants.FW_ACTION_ALLOW, + 'networks': external_cidrs, + 'rule_filter': {'prefix_operator': 'EQ', + 'match_route_types': ['T1_LB_VIP']}} + core_plugin.nsxlib.logical_router.update_advertisement_rules( + nsx_router_id, [adv_rule], name_prefix=ADV_RULE_NAME) diff --git a/vmware_nsx/services/lbaas/nsx_v3/implementation/loadbalancer_mgr.py b/vmware_nsx/services/lbaas/nsx_v3/implementation/loadbalancer_mgr.py index 51bf2ab2cf..0ac4a2d7e3 100644 --- a/vmware_nsx/services/lbaas/nsx_v3/implementation/loadbalancer_mgr.py +++ b/vmware_nsx/services/lbaas/nsx_v3/implementation/loadbalancer_mgr.py @@ -93,10 +93,11 @@ class EdgeLoadBalancerManagerFromDict(base_mgr.Nsxv3LoadbalancerBaseManager): try: service_client.delete(lb_service_id) # If there is no lb service attached to the router, - # update the router advertise_lb_vip flag to false. + # delete the router advertise_lb_vip rule. router_client = self.core_plugin.nsxlib.logical_router - router_client.update_advertisement( - nsx_router_id, advertise_lb_vip=False) + router_client.update_advertisement_rules( + nsx_router_id, [], + name_prefix=lb_utils.ADV_RULE_NAME) except nsxlib_exc.ManagerError: completor(success=False) msg = (_('Failed to delete lb service %(lbs)s from nsx' diff --git a/vmware_nsx/services/lbaas/nsx_v3/implementation/member_mgr.py b/vmware_nsx/services/lbaas/nsx_v3/implementation/member_mgr.py index 2f9472e06e..a7e7f7531d 100644 --- a/vmware_nsx/services/lbaas/nsx_v3/implementation/member_mgr.py +++ b/vmware_nsx/services/lbaas/nsx_v3/implementation/member_mgr.py @@ -74,9 +74,10 @@ class EdgeMemberManagerFromDict(base_mgr.Nsxv3LoadbalancerBaseManager): LOG.error("Failed to create LB service: %s", e) return - # Update router to enable advertise_lb_vip flag - self.core_plugin.nsxlib.logical_router.update_advertisement( - nsx_router_id, advertise_lb_vip=True) + # Add rule to advertise external vips + lb_utils.update_router_lb_vip_advertisement( + context, self.core_plugin, router, nsx_router_id) + return lb_service def _get_updated_pool_members(self, context, lb_pool, member): diff --git a/vmware_nsx/services/vpnaas/nsxv3/ipsec_driver.py b/vmware_nsx/services/vpnaas/nsxv3/ipsec_driver.py index 7b86446d9d..828944846e 100644 --- a/vmware_nsx/services/vpnaas/nsxv3/ipsec_driver.py +++ b/vmware_nsx/services/vpnaas/nsxv3/ipsec_driver.py @@ -148,6 +148,7 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver): filters = {'router_id': [router_id], 'status': [constants.ACTIVE]} services = self.vpn_plugin.get_vpnservices( context.elevated(), filters=filters) + rule_name_pref = 'VPN advertisement service' for srv in services: # use only services with active connections filters = {'vpnservice_id': [srv['id']], @@ -159,13 +160,15 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver): subnet = self.l3_plugin.get_subnet( context.elevated(), srv['subnet_id']) rules.append({ - 'display_name': 'VPN advertisement service ' + srv['id'], + 'display_name': "%s %s" % (rule_name_pref, srv['id']), 'action': consts.FW_ACTION_ALLOW, 'networks': [subnet['cidr']]}) - logical_router_id = db.get_nsx_router_id(context.session, router_id) - self._nsxlib.logical_router.update_advertisement_rules( - logical_router_id, rules) + if rules: + logical_router_id = db.get_nsx_router_id(context.session, + router_id) + self._nsxlib.logical_router.update_advertisement_rules( + logical_router_id, rules, name_prefix=rule_name_pref) def _update_status(self, context, vpn_service_id, ipsec_site_conn_id, status, updated_pending_status=True): diff --git a/vmware_nsx/shell/admin/plugins/common/constants.py b/vmware_nsx/shell/admin/plugins/common/constants.py index 20423c673e..f3fe2936a3 100644 --- a/vmware_nsx/shell/admin/plugins/common/constants.py +++ b/vmware_nsx/shell/admin/plugins/common/constants.py @@ -44,6 +44,7 @@ LB_SERVICES = 'lb-services' LB_VIRTUAL_SERVERS = 'lb-virtual-servers' LB_POOLS = 'lb-pools' LB_MONITORS = 'lb-monitors' +LB_ADVERTISEMENT = 'lb-advertisement' RATE_LIMIT = 'rate-limit' CLUSTER = 'cluster' ORPHANED_FIREWALL_SECTIONS = 'orphaned-firewall-sections' diff --git a/vmware_nsx/shell/admin/plugins/nsxv3/resources/loadbalancer.py b/vmware_nsx/shell/admin/plugins/nsxv3/resources/loadbalancer.py index e26c84a02e..39a7cf96e0 100644 --- a/vmware_nsx/shell/admin/plugins/nsxv3/resources/loadbalancer.py +++ b/vmware_nsx/shell/admin/plugins/nsxv3/resources/loadbalancer.py @@ -14,10 +14,16 @@ from oslo_log import log as logging +from neutron_lib.callbacks import registry +from neutron_lib import context as neutron_context + +from vmware_nsx.db import db as nsx_db +from vmware_nsx.services.lbaas.nsx_v3.implementation import lb_utils from vmware_nsx.shell.admin.plugins.common import constants from vmware_nsx.shell.admin.plugins.common import formatters from vmware_nsx.shell.admin.plugins.common import utils as admin_utils from vmware_nsx.shell.admin.plugins.nsxv3.resources import utils +from vmware_nsx.shell import resources as shell from vmware_nsxlib.v3 import nsx_constants as consts LOG = logging.getLogger(__name__) @@ -91,3 +97,54 @@ def nsx_list_lb_monitors(resource, event, trigger, **kwargs): constants.LB_MONITORS, lb_monitors['results'], ['display_name', 'id', 'resource_type'])) return bool(lb_monitors) + + +@admin_utils.output_header +def nsx_update_router_lb_advertisement(resource, event, trigger, **kwargs): + """The implementation of the VIP advertisement changed. + + This utility will update existing LB/routers + """ + nsxlib = utils.get_connected_nsxlib() + if not nsxlib.feature_supported(consts.FEATURE_LOAD_BALANCER): + LOG.error("This utility is not available for NSX version %s", + nsxlib.get_version()) + return + + # Get the list of neutron routers used by LB + lb_services = nsxlib.load_balancer.service.list()['results'] + lb_routers = [] + for lb_srv in lb_services: + for tag in lb_srv.get('tags', []): + if tag['scope'] == 'os-neutron-router-id': + lb_routers.append(tag['tag']) + lb_routers = set(lb_routers) + LOG.info("Going to update LB advertisement on %(num)s router(s): " + "%(routers)s", + {'num': len(lb_routers), 'routers': lb_routers}) + + context = neutron_context.get_admin_context() + with utils.NsxV3PluginWrapper() as plugin: + for rtr_id in lb_routers: + nsx_router_id = nsx_db.get_nsx_router_id(context.session, rtr_id) + if not nsx_router_id: + LOG.error("Router %s NSX Id was not found.", rtr_id) + continue + try: + # disable the global vip advertisement flag + plugin.nsxlib.logical_router.update_advertisement( + nsx_router_id, advertise_lb_vip=False) + # Add an advertisement rule for the external network + router = plugin.get_router(context, rtr_id) + lb_utils.update_router_lb_vip_advertisement( + context, plugin, router, nsx_router_id) + except Exception as e: + LOG.error("Failed updating router %(id)s: %(e)s", + {'id': rtr_id, 'e': e}) + + LOG.info("Done.") + + +registry.subscribe(nsx_update_router_lb_advertisement, + constants.LB_ADVERTISEMENT, + shell.Operations.NSX_UPDATE.value) diff --git a/vmware_nsx/shell/resources.py b/vmware_nsx/shell/resources.py index 26c38d245d..2f786d38b5 100644 --- a/vmware_nsx/shell/resources.py +++ b/vmware_nsx/shell/resources.py @@ -142,6 +142,8 @@ nsxv3_resources = { constants.RATE_LIMIT: Resource(constants.RATE_LIMIT, [Operations.SHOW.value, Operations.NSX_UPDATE.value]), + constants.LB_ADVERTISEMENT: Resource(constants.LB_ADVERTISEMENT, + [Operations.NSX_UPDATE.value]), constants.CLUSTER: Resource(constants.CLUSTER, [Operations.SHOW.value]) } diff --git a/vmware_nsx/tests/unit/services/lbaas/test_nsxv3_driver.py b/vmware_nsx/tests/unit/services/lbaas/test_nsxv3_driver.py index 8621eb4eda..f2228a7925 100644 --- a/vmware_nsx/tests/unit/services/lbaas/test_nsxv3_driver.py +++ b/vmware_nsx/tests/unit/services/lbaas/test_nsxv3_driver.py @@ -601,6 +601,79 @@ class TestEdgeLbaasV2Member(BaseTestEdgeLbaasV2): def test_create_existing_binding(self): self._test_create(LB_BINDING, POOL_BINDING) + def test_create_with_service(self): + ext_cidr = '1.1.1.0/24' + 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' + ) as mock_get_pool_members, \ + mock.patch.object(lb_utils, 'get_network_from_subnet' + ) as mock_get_network, \ + mock.patch.object(lb_utils, 'get_router_from_network' + ) as mock_get_router, \ + mock.patch.object(nsx_db, 'get_nsx_lbaas_pool_binding' + ) as mock_get_pool_binding, \ + mock.patch.object(nsx_db, 'get_nsx_lbaas_loadbalancer_binding' + ) as mock_get_lb_binding, \ + mock.patch.object(nsx_db, 'get_nsx_router_id' + ) as mock_get_nsx_router_id, \ + mock.patch.object(self.service_client, 'get_router_lb_service' + ) as mock_get_lb_service, \ + mock.patch.object(self.service_client, 'create' + ) as mock_create_lb_service, \ + mock.patch.object(nsx_db, 'add_nsx_lbaas_loadbalancer_binding' + ) as mock_add_loadbalancer_bidning, \ + mock.patch.object(self.service_client, + 'add_virtual_server' + ) as mock_add_vs_to_service, \ + mock.patch.object(self.pool_client, 'get' + ) as mock_get_pool, \ + mock.patch.object(self.pool_client, 'update_pool_with_members' + ) as mock_update_pool_with_members,\ + mock.patch.object(self.core_plugin.nsxlib.logical_router, + 'update_advertisement_rules') as update_adv,\ + mock.patch.object(self.core_plugin, '_find_router_gw_subnets' + ) as mock_get_subnets,\ + mock.patch.object(self.core_plugin, 'get_router' + ) as mock_core_get_router: + mock_validate_lb_subnet.return_value = True + 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_nsx_router_id.return_value = LB_ROUTER_ID + mock_get_lb_service.return_value = {} + mock_create_lb_service.return_value = {'id': LB_SERVICE_ID} + mock_get_pool.return_value = LB_POOL + mock_core_get_router.return_value = { + 'id': LB_ROUTER_ID, + 'name': 'router1', + 'external_gateway_info': 'dummy'} + mock_get_subnets.return_value = [{'cidr': ext_cidr}] + + 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) + 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]) + update_adv.assert_called_with( + LB_ROUTER_ID, + [{'networks': [ext_cidr], + 'display_name': lb_utils.ADV_RULE_NAME, + 'rule_filter': {'match_route_types': ['T1_LB_VIP'], + 'prefix_operator': 'EQ'}, + 'action': 'ALLOW'}], + name_prefix=lb_utils.ADV_RULE_NAME) + mock_successful_completion = ( + self.lbv2_driver.member.successful_completion) + mock_successful_completion.assert_called_with(self.context, + self.member, + delete=False) + def test_create_lbs_no_router_gateway(self): with mock.patch.object(lb_utils, 'validate_lb_member_subnet' ) as mock_validate_lb_subnet, \