openstack
/
kuryr-libnetwork
Code Issues Proposed changes

Remove unnecessary port update 

Kuryr update pre-created neutron port twice. The first one is in
'ipam_request_address' and it updates the following attributes:
* name
* admin_state_up
* mac_address

The second port update is in 'network_driver_create_endpoint' and
several attributes are written including name and mac_address.
This commit remove the first port update to optimize the performance.
The update of admin_state_up will be moved to the second port update.

Change-Id: I743b2088366d910902775cabefa43be2865e37c5
Partial-Bug: #1809306
This commit is contained in:
Hongbin Lu 2018-12-22 23:00:14 +00:00
parent 7a1e1a4034
commit 610fd5f024
5 changed files with 12 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
kuryr_libnetwork/controllers.py
View File

@ -517,33 +517,6 @@ def _get_cidr_from_subnetpool(**kwargs):
.format(kwargs))
def _update_existing_port(existing_port, fixed_ip, mac_address):
host = existing_port.get('binding:host_id')
vif_type = existing_port.get('binding:vif_type')
if not host and vif_type == 'unbound':
updated_port = {
'name': const.NEUTRON_UNBOUND_PORT,
'admin_state_up': True,
}
if mac_address:
updated_port['mac_address'] = mac_address
updated_port_resp = app.neutron.update_port(
existing_port['id'],
{'port': updated_port})
existing_port = updated_port_resp['port']
else:
port_name = existing_port.get('name', '')
if (str(port_name) != const.NEUTRON_UNBOUND_PORT or
len(existing_port['fixed_ips']) <= 1 or
host != lib_utils.get_hostname()):
raise exceptions.AddressInUseException(
"Requested ip address {0} already belongs to "
"a bound Neutron port: {1}".format(fixed_ip,
existing_port['id']))
return existing_port
def revoke_expose_ports(port_id):
sgs = app.neutron.list_security_groups(
name=utils.get_sg_expose_name(port_id))
@ -1735,9 +1708,7 @@ def ipam_request_address():
fixed_ips.append(fixed_ip)
if num_ports:
existing_port = filtered_ports[0]
created_port = _update_existing_port(
existing_port, fixed_ip, req_mac_address)
created_port = filtered_ports[0]
# REVISIT(yedongcan) For tag-ext extension not
# supported, the Neutron existing port still can not
# be deleted in ipam_release_address.

1
kuryr_libnetwork/port_driver/driver.py
View File

@ -134,6 +134,7 @@ class Driver(object):
'name': port['name'],
'device_owner': lib_const.DEVICE_OWNER,
'binding:host_id': lib_utils.get_hostname(),
'admin_state_up': True,
}
if not port.get('device_id'):
updated_port['device_id'] = endpoint_id

9
kuryr_libnetwork/tests/unit/port_driver/drivers/test_veth.py
View File

@ -114,7 +114,8 @@ class TestVethDriver(base.TestKuryrBase):
'name': fake_port_name,
'device_owner': lib_const.DEVICE_OWNER,
'binding:host_id': lib_utils.get_hostname(),
'mac_address': fake_mac_address2
'mac_address': fake_mac_address2,
'admin_state_up': True,
}
}
mock_update_port.assert_called_with(fake_neutron_port_id,
@ -145,7 +146,8 @@ class TestVethDriver(base.TestKuryrBase):
'port': {
'name': fake_port_name,
'device_owner': lib_const.DEVICE_OWNER,
'binding:host_id': lib_utils.get_hostname()
'binding:host_id': lib_utils.get_hostname(),
'admin_state_up': True,
}
}
mock_update_port.assert_called_with(fake_neutron_port_id,
@ -183,7 +185,8 @@ class TestVethDriver(base.TestKuryrBase):
'device_owner': lib_const.DEVICE_OWNER,
'binding:host_id': lib_utils.get_hostname(),
'device_id': fake_endpoint_id,
'mac_address': fake_mac_address2
'mac_address': fake_mac_address2,
'admin_state_up': True,
}
}
mock_update_port.assert_called_with(fake_neutron_port_id,

3
kuryr_libnetwork/tests/unit/port_driver/drivers/test_vlan.py
View File

@ -303,7 +303,8 @@ class TestVlanDriver(base.TestKuryrBase):
'name': fake_port_name,
'device_owner': lib_const.DEVICE_OWNER,
'binding:host_id': lib_utils.get_hostname(),
'mac_address': fake_neutron_mac_address2
'mac_address': fake_neutron_mac_address2,
'admin_state_up': True,
}})

24
kuryr_libnetwork/tests/unit/test_kuryr_ipam.py
View File

@ -811,14 +811,13 @@ class TestKuryrIpam(base.TestKuryrBase):
@mock.patch('kuryr_libnetwork.controllers._neutron_port_add_tag')
@mock.patch('kuryr_libnetwork.controllers.app.neutron.create_port')
@mock.patch('kuryr_libnetwork.controllers.app.neutron.update_port')
@mock.patch('kuryr_libnetwork.controllers.app.neutron.list_ports')
@mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets')
@mock.patch('kuryr_libnetwork.controllers.app')
@ddt.data((False), (True))
def test_ipam_driver_request_specific_address_existing_port(self,
use_tag_ext, mock_app, mock_list_subnets, mock_list_ports,
mock_update_port, mock_create_port, mock_port_add_tag):
mock_create_port, mock_port_add_tag):
mock_app.tag_ext = use_tag_ext
# faking list_subnets
neutron_network_id = uuidutils.generate_uuid()
@ -876,13 +875,6 @@ class TestKuryrIpam(base.TestKuryrBase):
mock_list_ports.side_effect = [
fake_ports_response, fake_ports_response_2]
update_port = {
'name': const.NEUTRON_UNBOUND_PORT,
'admin_state_up': True,
'mac_address': requested_mac_address,
}
mock_update_port.return_value = fake_port
# Testing container ip allocation
fake_request = {
'PoolID': fake_kuryr_subnetpool_id,
@ -917,8 +909,6 @@ class TestKuryrIpam(base.TestKuryrBase):
mock_list_ports.assert_has_calls([
mock.call(fixed_ips=fixed_ip_existing),
mock.call(fixed_ips=fixed_ipv6_existing)])
mock_update_port.assert_called_with(fake_neutron_port_id,
{'port': update_port})
if mock_app.tag_ext:
self.assertEqual(2, mock_port_add_tag.call_count)
else:
@ -926,14 +916,13 @@ class TestKuryrIpam(base.TestKuryrBase):
@mock.patch('kuryr_libnetwork.controllers._neutron_port_add_tag')
@mock.patch('kuryr_libnetwork.controllers.app.neutron.create_port')
@mock.patch('kuryr_libnetwork.controllers.app.neutron.update_port')
@mock.patch('kuryr_libnetwork.controllers.app.neutron.list_ports')
@mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets')
@mock.patch('kuryr_libnetwork.controllers.app')
@ddt.data((False), (True))
def test_ipam_driver_request_specific_mac_address_existing_port(self,
use_tag_ext, mock_app, mock_list_subnets, mock_list_ports,
mock_update_port, mock_create_port, mock_port_add_tag):
mock_create_port, mock_port_add_tag):
mock_app.tag_ext = use_tag_ext
# faking list_subnets
neutron_network_id = uuidutils.generate_uuid()
@ -991,13 +980,6 @@ class TestKuryrIpam(base.TestKuryrBase):
mock_list_ports.side_effect = [
fake_ports_response, fake_ports_response_2]
update_port = {
'name': const.NEUTRON_UNBOUND_PORT,
'admin_state_up': True,
'mac_address': requested_mac_address,
}
mock_update_port.return_value = fake_port
# Testing container ip allocation
fake_request = {
'PoolID': fake_kuryr_subnetpool_id,
@ -1034,8 +1016,6 @@ class TestKuryrIpam(base.TestKuryrBase):
fixed_ips='subnet_id=%s' % subnet_v4_id),
mock.call(mac_address=requested_mac_address,
fixed_ips='subnet_id=%s' % subnet_v6_id)])
mock_update_port.assert_called_with(fake_neutron_port_id,
{'port': update_port})
if mock_app.tag_ext:
self.assertEqual(2, mock_port_add_tag.call_count)
else: