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:
parent
7a1e1a4034
commit
610fd5f024
|
@ -517,33 +517,6 @@ def _get_cidr_from_subnetpool(**kwargs):
|
||||||
.format(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):
|
def revoke_expose_ports(port_id):
|
||||||
sgs = app.neutron.list_security_groups(
|
sgs = app.neutron.list_security_groups(
|
||||||
name=utils.get_sg_expose_name(port_id))
|
name=utils.get_sg_expose_name(port_id))
|
||||||
|
@ -1735,9 +1708,7 @@ def ipam_request_address():
|
||||||
fixed_ips.append(fixed_ip)
|
fixed_ips.append(fixed_ip)
|
||||||
|
|
||||||
if num_ports:
|
if num_ports:
|
||||||
existing_port = filtered_ports[0]
|
created_port = filtered_ports[0]
|
||||||
created_port = _update_existing_port(
|
|
||||||
existing_port, fixed_ip, req_mac_address)
|
|
||||||
# REVISIT(yedongcan) For tag-ext extension not
|
# REVISIT(yedongcan) For tag-ext extension not
|
||||||
# supported, the Neutron existing port still can not
|
# supported, the Neutron existing port still can not
|
||||||
# be deleted in ipam_release_address.
|
# be deleted in ipam_release_address.
|
||||||
|
|
|
@ -134,6 +134,7 @@ class Driver(object):
|
||||||
'name': port['name'],
|
'name': port['name'],
|
||||||
'device_owner': lib_const.DEVICE_OWNER,
|
'device_owner': lib_const.DEVICE_OWNER,
|
||||||
'binding:host_id': lib_utils.get_hostname(),
|
'binding:host_id': lib_utils.get_hostname(),
|
||||||
|
'admin_state_up': True,
|
||||||
}
|
}
|
||||||
if not port.get('device_id'):
|
if not port.get('device_id'):
|
||||||
updated_port['device_id'] = endpoint_id
|
updated_port['device_id'] = endpoint_id
|
||||||
|
|
|
@ -114,7 +114,8 @@ class TestVethDriver(base.TestKuryrBase):
|
||||||
'name': fake_port_name,
|
'name': fake_port_name,
|
||||||
'device_owner': lib_const.DEVICE_OWNER,
|
'device_owner': lib_const.DEVICE_OWNER,
|
||||||
'binding:host_id': lib_utils.get_hostname(),
|
'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,
|
mock_update_port.assert_called_with(fake_neutron_port_id,
|
||||||
|
@ -145,7 +146,8 @@ class TestVethDriver(base.TestKuryrBase):
|
||||||
'port': {
|
'port': {
|
||||||
'name': fake_port_name,
|
'name': fake_port_name,
|
||||||
'device_owner': lib_const.DEVICE_OWNER,
|
'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,
|
mock_update_port.assert_called_with(fake_neutron_port_id,
|
||||||
|
@ -183,7 +185,8 @@ class TestVethDriver(base.TestKuryrBase):
|
||||||
'device_owner': lib_const.DEVICE_OWNER,
|
'device_owner': lib_const.DEVICE_OWNER,
|
||||||
'binding:host_id': lib_utils.get_hostname(),
|
'binding:host_id': lib_utils.get_hostname(),
|
||||||
'device_id': fake_endpoint_id,
|
'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,
|
mock_update_port.assert_called_with(fake_neutron_port_id,
|
||||||
|
|
|
@ -303,7 +303,8 @@ class TestVlanDriver(base.TestKuryrBase):
|
||||||
'name': fake_port_name,
|
'name': fake_port_name,
|
||||||
'device_owner': lib_const.DEVICE_OWNER,
|
'device_owner': lib_const.DEVICE_OWNER,
|
||||||
'binding:host_id': lib_utils.get_hostname(),
|
'binding:host_id': lib_utils.get_hostname(),
|
||||||
'mac_address': fake_neutron_mac_address2
|
'mac_address': fake_neutron_mac_address2,
|
||||||
|
'admin_state_up': True,
|
||||||
}})
|
}})
|
||||||
|
|
||||||
|
|
||||||
|
|
|
@ -811,14 +811,13 @@ class TestKuryrIpam(base.TestKuryrBase):
|
||||||
|
|
||||||
@mock.patch('kuryr_libnetwork.controllers._neutron_port_add_tag')
|
@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.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_ports')
|
||||||
@mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets')
|
@mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets')
|
||||||
@mock.patch('kuryr_libnetwork.controllers.app')
|
@mock.patch('kuryr_libnetwork.controllers.app')
|
||||||
@ddt.data((False), (True))
|
@ddt.data((False), (True))
|
||||||
def test_ipam_driver_request_specific_address_existing_port(self,
|
def test_ipam_driver_request_specific_address_existing_port(self,
|
||||||
use_tag_ext, mock_app, mock_list_subnets, mock_list_ports,
|
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
|
mock_app.tag_ext = use_tag_ext
|
||||||
# faking list_subnets
|
# faking list_subnets
|
||||||
neutron_network_id = uuidutils.generate_uuid()
|
neutron_network_id = uuidutils.generate_uuid()
|
||||||
|
@ -876,13 +875,6 @@ class TestKuryrIpam(base.TestKuryrBase):
|
||||||
mock_list_ports.side_effect = [
|
mock_list_ports.side_effect = [
|
||||||
fake_ports_response, fake_ports_response_2]
|
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
|
# Testing container ip allocation
|
||||||
fake_request = {
|
fake_request = {
|
||||||
'PoolID': fake_kuryr_subnetpool_id,
|
'PoolID': fake_kuryr_subnetpool_id,
|
||||||
|
@ -917,8 +909,6 @@ class TestKuryrIpam(base.TestKuryrBase):
|
||||||
mock_list_ports.assert_has_calls([
|
mock_list_ports.assert_has_calls([
|
||||||
mock.call(fixed_ips=fixed_ip_existing),
|
mock.call(fixed_ips=fixed_ip_existing),
|
||||||
mock.call(fixed_ips=fixed_ipv6_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:
|
if mock_app.tag_ext:
|
||||||
self.assertEqual(2, mock_port_add_tag.call_count)
|
self.assertEqual(2, mock_port_add_tag.call_count)
|
||||||
else:
|
else:
|
||||||
|
@ -926,14 +916,13 @@ class TestKuryrIpam(base.TestKuryrBase):
|
||||||
|
|
||||||
@mock.patch('kuryr_libnetwork.controllers._neutron_port_add_tag')
|
@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.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_ports')
|
||||||
@mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets')
|
@mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets')
|
||||||
@mock.patch('kuryr_libnetwork.controllers.app')
|
@mock.patch('kuryr_libnetwork.controllers.app')
|
||||||
@ddt.data((False), (True))
|
@ddt.data((False), (True))
|
||||||
def test_ipam_driver_request_specific_mac_address_existing_port(self,
|
def test_ipam_driver_request_specific_mac_address_existing_port(self,
|
||||||
use_tag_ext, mock_app, mock_list_subnets, mock_list_ports,
|
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
|
mock_app.tag_ext = use_tag_ext
|
||||||
# faking list_subnets
|
# faking list_subnets
|
||||||
neutron_network_id = uuidutils.generate_uuid()
|
neutron_network_id = uuidutils.generate_uuid()
|
||||||
|
@ -991,13 +980,6 @@ class TestKuryrIpam(base.TestKuryrBase):
|
||||||
mock_list_ports.side_effect = [
|
mock_list_ports.side_effect = [
|
||||||
fake_ports_response, fake_ports_response_2]
|
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
|
# Testing container ip allocation
|
||||||
fake_request = {
|
fake_request = {
|
||||||
'PoolID': fake_kuryr_subnetpool_id,
|
'PoolID': fake_kuryr_subnetpool_id,
|
||||||
|
@ -1034,8 +1016,6 @@ class TestKuryrIpam(base.TestKuryrBase):
|
||||||
fixed_ips='subnet_id=%s' % subnet_v4_id),
|
fixed_ips='subnet_id=%s' % subnet_v4_id),
|
||||||
mock.call(mac_address=requested_mac_address,
|
mock.call(mac_address=requested_mac_address,
|
||||||
fixed_ips='subnet_id=%s' % subnet_v6_id)])
|
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:
|
if mock_app.tag_ext:
|
||||||
self.assertEqual(2, mock_port_add_tag.call_count)
|
self.assertEqual(2, mock_port_add_tag.call_count)
|
||||||
else:
|
else:
|
||||||
|
|
Loading…
Reference in New Issue