From c4bd94f94dc1383d82a5356ebf8a50855c024cd8 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Wed, 19 Jun 2019 15:15:05 +0300 Subject: [PATCH] NSX|P: Fix LB VIP related issues 1. Make sure floating ip updates on vip ports affect the nsx LB service, and avoids NAT rules 2. Make sure loadbalancer created with a pre-existing vip port will also be affected by fip updates 3. prevent creation of multiple loadbalancers on teh same router as the NSX backed does not allow it. Change-Id: Ia4959b22a068b0053d7709e83c8809627e4f3e89 --- vmware_nsx/plugins/nsx_p/plugin.py | 82 ++++++++++++++++++- vmware_nsx/services/lbaas/lb_const.py | 2 + .../nsx_p/implementation/loadbalancer_mgr.py | 34 +++++++- .../lbaas/nsx_p/implementation/member_mgr.py | 10 +++ .../nsx_v3/implementation/loadbalancer_mgr.py | 3 + .../unit/services/lbaas/test_nsxp_driver.py | 3 + 6 files changed, 129 insertions(+), 5 deletions(-) diff --git a/vmware_nsx/plugins/nsx_p/plugin.py b/vmware_nsx/plugins/nsx_p/plugin.py index 8040322789..784a88d7a0 100644 --- a/vmware_nsx/plugins/nsx_p/plugin.py +++ b/vmware_nsx/plugins/nsx_p/plugin.py @@ -86,6 +86,7 @@ from vmware_nsx.services.lbaas.nsx_p.implementation import listener_mgr from vmware_nsx.services.lbaas.nsx_p.implementation import loadbalancer_mgr from vmware_nsx.services.lbaas.nsx_p.implementation import member_mgr from vmware_nsx.services.lbaas.nsx_p.implementation import pool_mgr +from vmware_nsx.services.lbaas.octavia import constants as oct_const from vmware_nsx.services.lbaas.octavia import octavia_listener from vmware_nsx.services.qos.common import utils as qos_com_utils from vmware_nsx.services.qos.nsx_v3 import driver as qos_driver @@ -2062,6 +2063,20 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): tier1_id, nat_rule_id=self._get_fip_dnat_rule_id(fip_id)) + def _update_lb_vip(self, port, vip_address): + # update the load balancer virtual server's VIP with + # floating ip, but don't add NAT rules + device_id = port['device_id'] + if device_id.startswith(oct_const.DEVICE_ID_PREFIX): + device_id = device_id[len(oct_const.DEVICE_ID_PREFIX):] + tags_to_search = [{'scope': 'os-lbaas-lb-id', 'tag': device_id}] + vs_client = self.nsxpolicy.load_balancer.virtual_server + vs_list = self.nsxpolicy.search_by_tags( + tags_to_search, vs_client.entry_def.resource_type() + )['results'] + for vs in vs_list: + vs_client.update(vs['id'], ip_address=vip_address) + def create_floatingip(self, context, floatingip): # First do some validations fip_data = floatingip['floatingip'] @@ -2079,6 +2094,20 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): if not router_id: return new_fip + if port_id: + device_owner = port_data.get('device_owner') + fip_address = new_fip['floating_ip_address'] + if (device_owner == const.DEVICE_OWNER_LOADBALANCERV2 or + device_owner == oct_const.DEVICE_OWNER_OCTAVIA or + device_owner == lb_const.VMWARE_LB_VIP_OWNER): + try: + self._update_lb_vip(port_data, fip_address) + except nsx_lib_exc.ManagerError: + with excutils.save_and_reraise_exception(): + super(NsxPolicyPlugin, self).delete_floatingip( + context, new_fip['id']) + return new_fip + try: self._add_fip_nat_rules( router_id, new_fip['id'], @@ -2093,7 +2122,26 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): def delete_floatingip(self, context, fip_id): fip = self.get_floatingip(context, fip_id) router_id = fip['router_id'] - if router_id: + port_id = fip['port_id'] + is_lb_port = False + if port_id: + port_data = self.get_port(context, port_id) + device_owner = port_data.get('device_owner') + fixed_ip_address = fip['fixed_ip_address'] + if (device_owner == const.DEVICE_OWNER_LOADBALANCERV2 or + device_owner == oct_const.DEVICE_OWNER_OCTAVIA or + device_owner == lb_const.VMWARE_LB_VIP_OWNER): + # If the port is LB VIP port, after deleting the FIP, + # update the virtual server VIP back to fixed IP. + is_lb_port = True + try: + self._update_lb_vip(port_data, fixed_ip_address) + except nsx_lib_exc.ManagerError as e: + LOG.error("Exception when updating vip ip_address" + "on vip_port %(port)s: %(err)s", + {'port': port_id, 'err': e}) + + if router_id and not is_lb_port: self._delete_fip_nat_rules(router_id, fip_id) super(NsxPolicyPlugin, self).delete_floatingip(context, fip_id) @@ -2101,6 +2149,7 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): def update_floatingip(self, context, fip_id, floatingip): fip_data = floatingip['floatingip'] old_fip = self.get_floatingip(context, fip_id) + old_port_id = old_fip['port_id'] new_status = (const.FLOATINGIP_STATUS_ACTIVE if fip_data.get('port_id') else const.FLOATINGIP_STATUS_DOWN) @@ -2114,14 +2163,41 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): new_fip = super(NsxPolicyPlugin, self).update_floatingip( context, fip_id, floatingip) router_id = new_fip['router_id'] + new_port_id = new_fip['port_id'] - if (old_fip['router_id'] and + # Delete old configuration NAT / vip + is_lb_port = False + if old_port_id: + old_port_data = self.get_port(context, old_port_id) + old_device_owner = old_port_data['device_owner'] + old_fixed_ip = old_fip['fixed_ip_address'] + if (old_device_owner == const.DEVICE_OWNER_LOADBALANCERV2 or + old_device_owner == oct_const.DEVICE_OWNER_OCTAVIA or + old_device_owner == lb_const.VMWARE_LB_VIP_OWNER): + # If the port is LB VIP port, after deleting the FIP, + # update the virtual server VIP back to fixed IP. + is_lb_port = True + self._update_lb_vip(old_port_data, old_fixed_ip) + + if (not is_lb_port and old_fip['router_id'] and (not router_id or old_fip['router_id'] != router_id)): # Delete the old rules (if the router did not change - rewriting # the rules with _add_fip_nat_rules is enough) self._delete_fip_nat_rules(old_fip['router_id'], fip_id) - if router_id: + # Update LB VIP if the new port is LB port + is_lb_port = False + if new_port_id: + new_port_data = self.get_port(context, new_port_id) + new_dev_own = new_port_data['device_owner'] + new_fip_address = new_fip['floating_ip_address'] + if (new_dev_own == const.DEVICE_OWNER_LOADBALANCERV2 or + new_dev_own == oct_const.DEVICE_OWNER_OCTAVIA or + new_dev_own == lb_const.VMWARE_LB_VIP_OWNER): + is_lb_port = True + self._update_lb_vip(new_port_data, new_fip_address) + + if router_id and not is_lb_port: self._add_fip_nat_rules( router_id, new_fip['id'], new_fip['floating_ip_address'], diff --git a/vmware_nsx/services/lbaas/lb_const.py b/vmware_nsx/services/lbaas/lb_const.py index 1bb3dbe809..e0ba714d6f 100644 --- a/vmware_nsx/services/lbaas/lb_const.py +++ b/vmware_nsx/services/lbaas/lb_const.py @@ -132,3 +132,5 @@ OFFLINE = 'OFFLINE' DEGRADED = 'DEGRADED' ENABLED = 'ENABLED' DISABLED = 'DISABLED' + +VMWARE_LB_VIP_OWNER = 'vmware-lb-vip' diff --git a/vmware_nsx/services/lbaas/nsx_p/implementation/loadbalancer_mgr.py b/vmware_nsx/services/lbaas/nsx_p/implementation/loadbalancer_mgr.py index e0234be0b9..b9ff73ecea 100644 --- a/vmware_nsx/services/lbaas/nsx_p/implementation/loadbalancer_mgr.py +++ b/vmware_nsx/services/lbaas/nsx_p/implementation/loadbalancer_mgr.py @@ -23,6 +23,7 @@ from vmware_nsx.services.lbaas import base_mgr from vmware_nsx.services.lbaas import lb_const from vmware_nsx.services.lbaas.nsx_p.implementation import lb_utils as p_utils from vmware_nsx.services.lbaas.nsx_v3.implementation import lb_utils +from vmware_nsx.services.lbaas.octavia import constants as oct_const from vmware_nsxlib.v3 import exceptions as nsxlib_exc from vmware_nsxlib.v3.policy import utils as lib_p_utils from vmware_nsxlib.v3 import utils @@ -68,9 +69,21 @@ class EdgeLoadBalancerManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager): {'lb_id': lb_id, 'subnet': lb['vip_subnet_id']}) raise n_exc.BadRequest(resource='lbaas-subnet', msg=msg) - if router_id and not self.core_plugin.service_router_has_services( + if router_id: + # Validate that there is no other LB on this router + # as NSX does not allow it + if self.core_plugin.service_router_has_loadbalancers( context.elevated(), router_id): - self.core_plugin.create_service_router(context, router_id) + completor(success=False) + msg = (_('Cannot create a loadbalancer %(lb_id)s on router ' + '%(router)s, as it already has a loadbalancer') % + {'lb_id': lb_id, 'router': router_id}) + raise n_exc.BadRequest(resource='lbaas-router', msg=msg) + + # Create the service router if it does not exist + if not self.core_plugin.service_router_has_services( + context.elevated(), router_id): + self.core_plugin.create_service_router(context, router_id) lb_name = utils.get_name_and_uuid(lb['name'] or 'lb', lb_id) @@ -103,6 +116,14 @@ class EdgeLoadBalancerManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager): LOG.error('Failed to create loadbalancer %(lb)s for lb with ' 'exception %(e)s', {'lb': lb['id'], 'e': e}) + # Make sure the vip port is marked with a device owner + port = self.core_plugin.get_port( + context.elevated(), lb['vip_port_id']) + if not port.get('device_owner'): + self.core_plugin.update_port( + context.elevated(), lb['vip_port_id'], + {'port': {'device_id': oct_const.DEVICE_ID_PREFIX + lb['id'], + 'device_owner': lb_const.VMWARE_LB_VIP_OWNER}}) completor(success=True) @log_helpers.log_method_call @@ -146,6 +167,15 @@ class EdgeLoadBalancerManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager): 'of loadbalancer %(lb)s with error %(err)s', {'lb': lb['id'], 'err': e}) + # Make sure the vip port is not marked with a vmware device owner + port = self.core_plugin.get_port( + context.elevated(), lb['vip_port_id']) + if port.get('device_owner') == lb_const.VMWARE_LB_VIP_OWNER: + self.core_plugin.update_port( + context.elevated(), lb['vip_port_id'], + {'port': {'device_id': '', + 'device_owner': ''}}) + completor(success=True) @log_helpers.log_method_call diff --git a/vmware_nsx/services/lbaas/nsx_p/implementation/member_mgr.py b/vmware_nsx/services/lbaas/nsx_p/implementation/member_mgr.py index 335bc4355d..593e1ec8e9 100644 --- a/vmware_nsx/services/lbaas/nsx_p/implementation/member_mgr.py +++ b/vmware_nsx/services/lbaas/nsx_p/implementation/member_mgr.py @@ -67,6 +67,16 @@ class EdgeMemberManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager): if not service.get('connectivity_path'): router_id = lb_utils.get_router_from_network( context, self.core_plugin, member['subnet_id']) + # Validate that there is no other LB on this router + # as NSX does not allow it + if self.core_plugin.service_router_has_loadbalancers( + context.elevated(), router_id): + completor(success=False) + msg = (_('Cannot attach a loadbalancer %(lb_id)s on router ' + '%(router)s, as it already has a loadbalancer') % + {'lb_id': lb['id'], 'router': router_id}) + raise n_exc.BadRequest(resource='lbaas-router', msg=msg) + if not self.core_plugin.service_router_has_services(context, router_id): self.core_plugin.create_service_router(context, router_id) 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 b5515f8f65..ae3b761594 100644 --- a/vmware_nsx/services/lbaas/nsx_v3/implementation/loadbalancer_mgr.py +++ b/vmware_nsx/services/lbaas/nsx_v3/implementation/loadbalancer_mgr.py @@ -35,6 +35,9 @@ class EdgeLoadBalancerManagerFromDict(base_mgr.Nsxv3LoadbalancerBaseManager): @log_helpers.log_method_call def create(self, context, lb, completor): + + # TODO(asarfaty): If the lb is created with a port_id, + # need to set octavia device owner & device id on it. if not lb_utils.validate_lb_subnet(context, self.core_plugin, lb['vip_subnet_id']): completor(success=False) diff --git a/vmware_nsx/tests/unit/services/lbaas/test_nsxp_driver.py b/vmware_nsx/tests/unit/services/lbaas/test_nsxp_driver.py index e5601e5133..bb28715c45 100644 --- a/vmware_nsx/tests/unit/services/lbaas/test_nsxp_driver.py +++ b/vmware_nsx/tests/unit/services/lbaas/test_nsxp_driver.py @@ -300,6 +300,9 @@ class TestEdgeLbaasV2Loadbalancer(BaseTestEdgeLbaasV2): return_value=neutron_router), \ mock.patch.object(self.core_plugin, '_find_router_gw_subnets', return_value=[]),\ + mock.patch.object(self.core_plugin, + 'service_router_has_loadbalancers', + return_value=False),\ mock.patch.object(self.service_client, 'get_router_lb_service', return_value=None),\ mock.patch.object(self.service_client, 'create_or_overwrite'