From a5765179201ee03ed26bb8f3bcda9f75bbdb191c Mon Sep 17 00:00:00 2001 From: Avishay Balderman Date: Tue, 29 Jul 2014 18:15:29 +0300 Subject: [PATCH] Radware: When a pip is needed, reuse the Port 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 --- .../loadbalancer/drivers/radware/driver.py | 57 +++++++++++++------ .../drivers/radware/test_plugin_driver.py | 23 ++++++++ 2 files changed, 62 insertions(+), 18 deletions(-) diff --git a/neutron/services/loadbalancer/drivers/radware/driver.py b/neutron/services/loadbalancer/drivers/radware/driver.py index efa972d439c..c6d7bc9239a 100644 --- a/neutron/services/loadbalancer/drivers/radware/driver.py +++ b/neutron/services/loadbalancer/drivers/radware/driver.py @@ -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.""" + def _get_pip(self, context, tenant_id, port_name, + network_id, subnet_id): + """Get proxy 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 + 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: diff --git a/neutron/tests/unit/services/loadbalancer/drivers/radware/test_plugin_driver.py b/neutron/tests/unit/services/loadbalancer/drivers/radware/test_plugin_driver.py index 4edda4527e0..9a7e660416e 100644 --- a/neutron/tests/unit/services/loadbalancer/drivers/radware/test_plugin_driver.py +++ b/neutron/tests/unit/services/loadbalancer/drivers/radware/test_plugin_driver.py @@ -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']