Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve perfomance of node network functions #14

Merged
merged 1 commit into from
Nov 15, 2024
Merged

Improve perfomance of node network functions #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 36 additions & 26 deletions esi/lib/networks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,37 @@ def network_and_port_list(connection, filter_network=None):
f1 = executor.submit(get_ports, connection, filter_network)
f2 = executor.submit(connection.network.networks)
f3 = executor.submit(connection.network.ips)
network_ports = list(f1.result())
network_ports_dict = {port.id: port for port in list(f1.result())}
networks_dict = {network.id: network for network in list(f2.result())}
floating_ips = list(f3.result())

for floating_ip in floating_ips:
# no need to do this for floating IPs associated with a port,
# as port forwarding is irrelevant in such a case
if not floating_ip.port_id:
pfwds = list(connection.network.port_forwardings(floating_ip=floating_ip))
fip_futures = []
for floating_ip in floating_ips:
fip_futures.append(executor.submit(
get_floating_ip_and_port_forwarding,
connection,
floating_ip))
for fip_future in fip_futures:
floating_ip, pfwds = fip_future.result()
if len(pfwds):
floating_ip.port_id = pfwds[0].internal_port_id
port_forwardings_dict[floating_ip.port_id] = pfwds
floating_ips_dict[floating_ip.port_id] = floating_ip
floating_ips_dict[floating_ip.port_id] = floating_ip

return network_ports, networks_dict, floating_ips_dict, port_forwardings_dict
return network_ports_dict, networks_dict, floating_ips_dict, port_forwardings_dict


def get_networks_from_port(connection, port, networks_dict={}, floating_ips_dict={}):
def get_floating_ip_and_port_forwarding(connection, floating_ip):
# no need to do this for floating IPs associated with a port,
# as port forwarding is irrelevant in such a case
pfwds = []
if not floating_ip.port_id:
pfwds = list(connection.network.port_forwardings(floating_ip=floating_ip))
if len(pfwds):
floating_ip.port_id = pfwds[0].internal_port_id
return floating_ip, pfwds


def get_networks_from_port(connection, port, networks_dict={}, network_ports_dict={}, floating_ips_dict={}):
"""Gets associated network objects from a port object

:param connection: An OpenStack connection
Expand All @@ -85,22 +98,19 @@ def get_networks_from_port(connection, port, networks_dict={}, floating_ips_dict
parent_network = connection.network.get_network(network=port.network_id)

if port.trunk_details:
with concurrent.futures.ThreadPoolExecutor() as executor:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the concurrent.futures.ThreadPoolExecutor() need to be removed? Does it lead to slower execution with ThreadPoolExecutor?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, simply because it should no longer be necessary. The port dictionary should contain all the ports the user can see, so the API query should never be made. The if/else is still there mostly as a just-in-case thing - similar to what the code does with networks - but uncluttering the code feels like a win to me.

subport_futures = []
subport_infos = port.trunk_details['sub_ports']
for subport_info in subport_infos:
subport_futures.append(executor.submit(
connection.network.get_port,
port=subport_info['port_id']
))
for subport_future in subport_futures:
subport = subport_future.result()
if subport.network_id in networks_dict:
trunk_network = networks_dict[subport.network_id]
else:
trunk_network = connection.network.get_network(subport.network_id)
trunk_ports.append(subport)
trunk_networks.append(trunk_network)
subport_infos = port.trunk_details['sub_ports']
for subport_info in subport_infos:
if subport_info['port_id'] in network_ports_dict:
subport = network_ports_dict[subport_info['port_id']]
else:
subport = connection.network.get_port(subport_info['port_id'])

if subport.network_id in networks_dict:
trunk_network = networks_dict[subport.network_id]
else:
trunk_network = connection.network.get_network(subport.network_id)
trunk_ports.append(subport)
trunk_networks.append(trunk_network)

floating_network_id = getattr(floating_ips_dict.get(port.id),
'floating_network_id', None)
Expand Down
6 changes: 3 additions & 3 deletions esi/lib/nodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def network_list(connection, filter_node=None, filter_network=None):
filter_network = f3.result()
f2 = executor.submit(networks.network_and_port_list, connection, filter_network)
baremetal_nodes, baremetal_ports = f1.result()
network_ports, networks_dict, floating_ips_dict, port_forwardings_dict = f2.result()
network_ports_dict, networks_dict, floating_ips_dict, port_forwardings_dict = f2.result()

data = []
for baremetal_node in baremetal_nodes:
Expand All @@ -102,14 +102,14 @@ def network_list(connection, filter_node=None, filter_network=None):
network_port_id = baremetal_port.internal_info.get('tenant_vif_port_id', None)

if network_port_id:
network_port = next((np for np in network_ports
if np.id == network_port_id), None)
network_port = network_ports_dict.get(network_port_id)

if network_port is not None and (not filter_network or filter_network.id == network_port.network_id):
parent_network, trunk_networks, trunk_ports, floating_network \
= networks.get_networks_from_port(connection,
network_port,
networks_dict,
network_ports_dict,
floating_ips_dict)

network_info.append({
Expand Down
14 changes: 7 additions & 7 deletions esi/tests/unit/lib/test_networks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,10 @@ def test_network_and_port_list(self):
actual = networks.network_and_port_list(self.connection)

expected = (
[
self.neutron_port1,
self.neutron_port2
],
{
'neutron_port_uuid_1': self.neutron_port1,
'neutron_port_uuid_2': self.neutron_port2
},
{
'network_uuid_1': self.network1,
'network_uuid_2': self.network2,
Expand All @@ -192,9 +192,9 @@ def test_network_and_port_list_network_filter(self):
actual = networks.network_and_port_list(self.connection, self.network1)

expected = (
[
self.neutron_port1,
],
{
'neutron_port_uuid_1': self.neutron_port1,
},
{
'network_uuid_1': self.network1,
'network_uuid_2': self.network2,
Expand Down
16 changes: 7 additions & 9 deletions esi/tests/unit/lib/test_nodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,14 +253,14 @@ def mock_baremetal_ports(details=False, node_id=None):

def mock_network_ports(network_id=None):
if network_id == 'network_uuid_1':
return [self.neutron_port1]
return [self.neutron_port1, self.sub_port1]
elif network_id == 'network_uuid_2':
return [self.neutron_port2]
return [self.neutron_port2, self.sub_port2]
if network_id == 'network_uuid_3':
return [self.neutron_port3]
elif network_id:
return []
return [self.neutron_port1, self.neutron_port2, self.neutron_port3]
return [self.neutron_port1, self.neutron_port2, self.neutron_port3, self.sub_port1, self.sub_port2]
self.connection.network.ports.side_effect = mock_network_ports

def mock_find_network(name_or_id=None, ignore_missing=True):
Expand Down Expand Up @@ -398,8 +398,7 @@ def test_network_list(self):
self.connection.network.ports.assert_called_once_with()
self.connection.network.port_forwardings.assert_called_once_with(floating_ip=self.floating_ip_pfw)
self.connection.network.get_network.assert_not_called()
self.connection.network.get_port.assert_any_call(port='sub_port_uuid_1')
self.connection.network.get_port.assert_any_call(port='sub_port_uuid_2')
self.connection.network.get_port.assert_not_called()

def test_network_list_filter_node(self):
filter_node = 'node1'
Expand Down Expand Up @@ -448,8 +447,7 @@ def test_network_list_filter_node(self):
self.connection.network.ports.assert_called_once_with()
self.connection.network.port_forwardings.assert_called_once_with(floating_ip=self.floating_ip_pfw)
self.connection.network.get_network.assert_not_called()
self.connection.network.get_port.assert_any_call(port='sub_port_uuid_1')
self.connection.network.get_port.assert_any_call(port='sub_port_uuid_2')
self.connection.network.get_port.assert_not_called()

def test_network_list_filter_network(self):
filter_node = None
Expand Down Expand Up @@ -487,8 +485,8 @@ def test_network_list_filter_network(self):
self.connection.network.ports.assert_called_once_with(network_id='network_uuid_3')
self.connection.network.port_forwardings.assert_called_once_with(floating_ip=self.floating_ip_pfw)
self.connection.network.get_network.assert_not_called()
self.connection.network.get_port.assert_any_call(port='sub_port_uuid_1')
self.connection.network.get_port.assert_any_call(port='sub_port_uuid_2')
self.connection.network.get_port.assert_any_call('sub_port_uuid_1')
self.connection.network.get_port.assert_any_call('sub_port_uuid_2')

def test_network_list_filter_node_network(self):
filter_node = '11111111-2222-3333-4444-bbbbbbbbbbbb'
Expand Down
Loading