NSX|P: Fix handling LB member on external subnet

If the LB has an external vip, the member must have a local subnet-id
connected to a router with an uplink,
or the member must have an IP which is a FIP address

In addition, remove one leftover log, and remove a lock on router
id when it is None

Change-Id: Iefb492e43b5cc47a84ce82e4dfbcb0d1e5e6bffe

vmware_nsx/services/lbaas/nsx_p/implementation/listener_mgr.py

@@ -46,7 +46,6 @@ class EdgeListenerManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager):
         tags.append({
             'scope': lb_const.LB_LB_TYPE,
             'tag': listener['loadbalancer_id']})
-        LOG.error("DEBUG ADIT _get_listener_tags end")
         return tags
 
     def _upload_certificate(self, listener_id, cert_href, tags,

vmware_nsx/services/lbaas/nsx_p/implementation/loadbalancer_mgr.py

@@ -102,7 +102,15 @@ class EdgeLoadBalancerManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager):
                 else:
                     tier1_srv = self.core_plugin.nsxpolicy.tier1
                     connectivity_path = tier1_srv.get_path(router_id)
-                with p_utils.get_lb_rtr_lock(router_id):
+                if connectivity_path:
+                    with p_utils.get_lb_rtr_lock(router_id):
+                        service_client.create_or_overwrite(
+                            lb_name, lb_service_id=lb['id'],
+                            description=lb['description'],
+                            tags=tags, size=lb_size,
+                            connectivity_path=connectivity_path)
+                else:
+                    # no lock
                     service_client.create_or_overwrite(
                         lb_name, lb_service_id=lb['id'],
                         description=lb['description'],

vmware_nsx/services/lbaas/nsx_p/implementation/member_mgr.py

@@ -31,17 +31,23 @@ def _translate_member_state(state):
 
 
 class EdgeMemberManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager):
-    def _get_info_from_fip(self, context, fip):
+    def _get_fip_object(self, context, fip):
         filters = {'floating_ip_address': [fip]}
         floating_ips = self.core_plugin.get_floatingips(context,
                                                         filters=filters)
         if floating_ips:
-            return floating_ips[0]['fixed_ip_address']
+            return floating_ips[0]
         else:
             msg = (_('Member IP %(fip)s is an external IP, and is expected to '
                      'be a floating IP') % {'fip': fip})
             raise n_exc.BadRequest(resource='lbaas-vip', msg=msg)
 
+    def _get_info_from_fip(self, context, fip):
+        return self._get_fip_object(context, fip)['fixed_ip_address']
+
+    def _get_router_from_fip(self, context, fip):
+        return self._get_fip_object(context, fip)['router_id']
+
     def _validate_member_lb_connectivity(self, context, member, completor):
         lb = member['pool'].get('loadbalancer')
 
@@ -70,8 +76,32 @@ class EdgeMemberManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager):
             raise n_exc.BadRequest(resource='lbaas-router', msg=msg)
 
         if not service.get('connectivity_path'):
-            router_id = lb_utils.get_router_from_network(
+            # Find the router of the local subnet
+            network = lb_utils.get_network_from_subnet(
                 context, self.core_plugin, member['subnet_id'])
+            if network and network.get('router:external'):
+                # member ip should be a fip
+                try:
+                    router_id = self._get_router_from_fip(
+                        context, member['address'])
+                except n_exc.BadRequest:
+                    with excutils.save_and_reraise_exception():
+                        completor(success=False)
+            else:
+                try:
+                    router_id = lb_utils.get_router_from_network(
+                        context, self.core_plugin, member['subnet_id'])
+                except Exception:
+                    completor(success=False)
+                    msg = (_('Cannot find router attached to member '
+                             '%(mem_id)s') % {'mem_id': member['id']})
+                    raise n_exc.BadRequest(resource='lbaas-router', msg=msg)
+                if not router_id:
+                    completor(success=False)
+                    msg = (_('Cannot find router with uplink attached to '
+                             'member %(mem_id)s') % {'mem_id': member['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)
@@ -124,7 +154,11 @@ class EdgeMemberManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager):
         else:
             network = None
         if network and network.get('router:external'):
-            fixed_ip = self._get_info_from_fip(context, member['address'])
+            try:
+                fixed_ip = self._get_info_from_fip(context, member['address'])
+            except n_exc.BadRequest:
+                with excutils.save_and_reraise_exception():
+                    completor(success=False)
         else:
             fixed_ip = member['address']
         pool_id = member['pool']['id']
@@ -149,7 +183,12 @@ class EdgeMemberManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager):
         network = lb_utils.get_network_from_subnet(
             context, self.core_plugin, new_member['subnet_id'])
         if network and network.get('router:external'):
-            fixed_ip = self._get_info_from_fip(context, new_member['address'])
+            try:
+                fixed_ip = self._get_info_from_fip(
+                    context, new_member['address'])
+            except n_exc.BadRequest:
+                with excutils.save_and_reraise_exception():
+                    completor(success=False)
         else:
             fixed_ip = new_member['address']
         pool_id = old_member['pool']['id']
@@ -176,7 +215,11 @@ class EdgeMemberManagerFromDict(base_mgr.NsxpLoadbalancerBaseManager):
         network = lb_utils.get_network_from_subnet(
             context, self.core_plugin, member['subnet_id'])
         if network and network.get('router:external'):
-            fixed_ip = self._get_info_from_fip(context, member['address'])
+            try:
+                fixed_ip = self._get_info_from_fip(context, member['address'])
+            except n_exc.BadRequest:
+                with excutils.save_and_reraise_exception():
+                    completor(success=False)
         else:
             fixed_ip = member['address']
         pool_id = member['pool']['id']

vmware_nsx/tests/unit/services/lbaas/test_nsxp_driver.py

@@ -1618,7 +1618,8 @@ class TestEdgeLbaasV2Member(BaseTestEdgeLbaasV2):
                               return_value=[]),\
             mock.patch.object(self.core_plugin, 'get_floatingips',
                               return_value=[{
-                                  'fixed_ip_address': MEMBER_ADDRESS}]),\
+                                  'fixed_ip_address': MEMBER_ADDRESS,
+                                  'router_id': LB_ROUTER_ID}]),\
             mock.patch.object(self.pool_client,
                               'create_pool_member_and_add_to_pool'
                               ) as mock_update_pool_with_members:
@@ -1663,7 +1664,42 @@ class TestEdgeLbaasV2Member(BaseTestEdgeLbaasV2):
                               return_value=[]),\
             mock.patch.object(self.core_plugin, 'get_floatingips',
                               return_value=[{
-                                  'fixed_ip_address': MEMBER_ADDRESS}]):
+                                  'fixed_ip_address': MEMBER_ADDRESS,
+                                  'router_id': LB_ROUTER_ID}]):
+            mock_get_pool_members.return_value = [self.member]
+            mock_get_network.return_value = EXT_LB_NETWORK
+            mock_get_router.return_value = LB_ROUTER_ID
+            mock_get_lb_service.return_value = {'id': LB_SERVICE_ID}
+            mock_get_pool.return_value = LB_POOL
+
+            self.assertRaises(
+                n_exc.BadRequest, self.edge_driver.member.create,
+                self.context, self.member_dict, self.completor)
+            self.assertTrue(self.last_completor_called)
+            self.assertFalse(self.last_completor_succees)
+
+    def test_create_external_vip_no_fip(self):
+        self.reset_completor()
+        lb_service = {'id': LB_SERVICE_ID}
+        with 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(self.service_client, 'get_router_lb_service'
+                              ) as mock_get_lb_service, \
+            mock.patch.object(self.core_plugin.nsxpolicy, 'search_by_tags',
+                              return_value={'results': [lb_service]}),\
+            mock.patch.object(self.core_plugin,
+                              'service_router_has_loadbalancers',
+                              return_value=True),\
+            mock.patch.object(self.pool_client, 'get'
+                              ) as mock_get_pool, \
+            mock.patch.object(self.core_plugin, '_find_router_gw_subnets',
+                              return_value=[]),\
+            mock.patch.object(self.core_plugin, 'get_floatingips',
+                              return_value=[]):
             mock_get_pool_members.return_value = [self.member]
             mock_get_network.return_value = EXT_LB_NETWORK
             mock_get_router.return_value = LB_ROUTER_ID