]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Radware: When a pip is needed, reuse the Port
authorAvishay Balderman <avishayb@radware.com>
Tue, 29 Jul 2014 15:15:29 +0000 (18:15 +0300)
committerAvishay Balderman <avishayb@radware.com>
Tue, 5 Aug 2014 10:57:00 +0000 (13:57 +0300)
When a pip (Proxy IP) is needed by the driver, do not create
a new Neutron Port every time a pip is needed. Reuse the
existing Port.

Change-Id: I769a9d85e217b30a1ea4d09449ff39bf1ab23c5a
Closes-bug: #1349895

neutron/services/loadbalancer/drivers/radware/driver.py
neutron/tests/unit/services/loadbalancer/drivers/radware/test_plugin_driver.py

index efa972d439c8680243662fee78a98d1f84c201e6..c6d7bc9239a7c81d137b3c57b3a3f382d2b37a18 100644 (file)
@@ -196,11 +196,12 @@ class LoadBalancerDriver(abstract_driver.LoadBalancerAbstractDriver):
         # if VIP and PIP are different, we need an IP address for the PIP
         # so create port on PIP's network and use its IP address
         if vip_network_id != pool_network_id:
-            pip_address = self._create_port_for_pip(
+            pip_address = self._get_pip(
                 context,
                 vip['tenant_id'],
                 _make_pip_name_from_vip(vip),
-                pool_network_id)
+                pool_network_id,
+                ext_vip['pool']['subnet_id'])
             ext_vip['pip_address'] = pip_address
         else:
             ext_vip['pip_address'] = vip['address']
@@ -613,24 +614,44 @@ class LoadBalancerDriver(abstract_driver.LoadBalancerAbstractDriver):
                 raise r_exc.WorkflowMissing(workflow=wf)
         self.workflow_templates_exists = True
 
-    def _create_port_for_pip(self, context, tenant_id, port_name, subnet):
-        """Creates port on subnet, returns that port's IP."""
-
-        # create port, we just want any IP allocated to the port based on the
-        # network id, so setting 'fixed_ips' to ATTR_NOT_SPECIFIED
-        port_data = {
-            'tenant_id': tenant_id,
-            'name': port_name,
-            'network_id': subnet,
-            'mac_address': attributes.ATTR_NOT_SPECIFIED,
-            'admin_state_up': False,
-            'device_id': '',
-            'device_owner': 'neutron:' + constants.LOADBALANCER,
-            'fixed_ips': attributes.ATTR_NOT_SPECIFIED
+    def _get_pip(self, context, tenant_id, port_name,
+                 network_id, subnet_id):
+        """Get proxy IP
+
+        Creates or get port on network_id, returns that port's IP
+        on the subnet_id.
+        """
+
+        port_filter = {
+            'name': [port_name],
         }
-        port = self.plugin._core_plugin.create_port(context,
-                                                    {'port': port_data})
-        return port['fixed_ips'][0]['ip_address']
+        ports = self.plugin._core_plugin.get_ports(context,
+                                                   filters=port_filter)
+        if not ports:
+            # create port, we just want any IP allocated to the port
+            # based on the network id and subnet_id
+            port_data = {
+                'tenant_id': tenant_id,
+                'name': port_name,
+                'network_id': network_id,
+                'mac_address': attributes.ATTR_NOT_SPECIFIED,
+                'admin_state_up': False,
+                'device_id': '',
+                'device_owner': 'neutron:' + constants.LOADBALANCER,
+                'fixed_ips': [{'subnet_id': subnet_id}]
+            }
+            port = self.plugin._core_plugin.create_port(context,
+                                                        {'port': port_data})
+        else:
+            port = ports[0]
+        ips_on_subnet = [ip for ip in port['fixed_ips']
+                         if ip['subnet_id'] == subnet_id]
+        if not ips_on_subnet:
+            raise Exception(_('Could not find or allocate '
+                              'IP address for subnet id %s'),
+                            subnet_id)
+        else:
+            return ips_on_subnet[0]['ip_address']
 
 
 class vDirectRESTClient:
index 4edda4527e0a8c75b95bea34d152ed247e856774..9a7e660416edf00737feb4cb6d9287622e8fd71d 100644 (file)
@@ -153,6 +153,29 @@ class TestLoadBalancerPlugin(TestLoadBalancerPluginBase):
 
         self.addCleanup(radware_driver.completion_handler.join)
 
+    def test_get_pip(self):
+        """Call _get_pip twice and verify that a Port is created once."""
+        port_dict = {'fixed_ips': [{'subnet_id': '10.10.10.10',
+                                    'ip_address': '11.11.11.11'}]}
+        self.plugin_instance._core_plugin.get_ports = mock.Mock(
+            return_value=[])
+        self.plugin_instance._core_plugin.create_port = mock.Mock(
+            return_value=port_dict)
+        radware_driver = self.plugin_instance.drivers['radware']
+        radware_driver._get_pip(context.get_admin_context(),
+                                'tenant_id', 'port_name',
+                                'network_id', '10.10.10.10')
+        self.plugin_instance._core_plugin.get_ports.assert_called_once()
+        self.plugin_instance._core_plugin.create_port.assert_called_once()
+        self.plugin_instance._core_plugin.create_port.reset_mock()
+        self.plugin_instance._core_plugin.get_ports.reset_mock()
+        self.plugin_instance._core_plugin.get_ports.return_value = [port_dict]
+        radware_driver._get_pip(context.get_admin_context(),
+                                'tenant_id', 'port_name',
+                                'network_id', '10.10.10.10')
+        self.plugin_instance._core_plugin.get_ports.assert_called_once()
+        self.plugin_instance._core_plugin.create_port.assert_has_calls([])
+
     def test_rest_client_recover_was_called(self):
         """Call the real REST client and verify _recover is called."""
         radware_driver = self.plugin_instance.drivers['radware']