feat(neutron): dynamic lookup network nodeId #1377
Conversation
haseebsyed12
force-pushed
the
puc-1297-dynamic-lookup-network-node
branch
2 times, most recently
from
0a15219 to
e72d9aa
Compare
haseebsyed12
force-pushed
the
puc-1297-dynamic-lookup-network-node
branch
from
e72d9aa to
cfbcc73
Compare
| return bool(uuid_pattern.match(value)) | ||
|
|
||
|
|
||
| def fetch_network_node_trunk_id() -> str: |
There was a problem hiding this comment.
This funtion is quite long and does a lot of things, consider splitting it into small functions. Also I see no test for it and it seems to be important to test it.
There was a problem hiding this comment.
This is a pretty expensive function to be called regularly in some of the other flows as well.
cardoe
left a comment
cardoe
left a comment
There was a problem hiding this comment.
Honestly the big O on this is gonna be costly. I would change this around to not get all trunks and walk them and call Ironic over and over again. You can call Ironic once for the gateway node to get its ID then get the ports owned by that ID. Then get the trunks owned by those ports. Now you've got a much smaller list to iterate over.
In the current code if we have 1000 trunk ports (which isn't even the possible max for a site) you would make 1000 calls to Ironic inside of a function that's expected to complete in 20 seconds or less.
| utils.fetch_trunk_plugin().add_subports( | ||
| context=n_context.get_admin_context(), | ||
| trunk_id=cfg.CONF.ml2_understack.network_node_trunk_uuid, | ||
| trunk_id=utils.fetch_network_node_trunk_id(), | ||
| subports=subports, | ||
| ) |
There was a problem hiding this comment.
I would probably get the trunk_id = utils.fetch_network_node_trunk_id() on its own line so that if there's an issue we'll get a clearer backtrace for the line number.
| def _is_uuid(value: str) -> bool: | ||
| """Check if a string is a UUID.""" | ||
| uuid_pattern = re.compile( | ||
| r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$", re.IGNORECASE | ||
| ) | ||
| return bool(uuid_pattern.match(value)) |
There was a problem hiding this comment.
| def _is_uuid(value: str) -> bool: | |
| """Check if a string is a UUID.""" | |
| uuid_pattern = re.compile( | |
| r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$", re.IGNORECASE | |
| ) | |
| return bool(uuid_pattern.match(value)) | |
| def _is_uuid(value: str) -> bool: | |
| """Check if a string is a UUID.""" | |
| try: | |
| uuid.UUID(value) | |
| return True | |
| except ValueError: | |
| return False |
| return bool(uuid_pattern.match(value)) | ||
|
|
||
|
|
||
| def fetch_network_node_trunk_id() -> str: |
There was a problem hiding this comment.
This is a pretty expensive function to be called regularly in some of the other flows as well.
No description provided.