Merge "NSX|V: skip interface actions on old lbaas member create"

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

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)

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.is_lb_on_router_edge(
-            if not lb_common.get_lb_interface(
+                context.elevated(), self.core_plugin, edge_id):
-                    context, self.core_plugin, lb_id, member.subnet_id):
+                # Verify that Edge appliance is connected to the member's
-                lb_common.create_lb_interface(
+                # subnet (only if this is a dedicated loadbalancer edge)
-                    context, self.core_plugin, lb_id, member.subnet_id,
+                if not lb_common.get_lb_interface(
-                    member.tenant_id)
+                        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 = {

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'