Reduce the number of ifconfig calls made in add_topo #19119
Conversation
`ifconfig -a` is called for every VM in a topology within ansible/roles/vm_set/library/vm_topology.py. For topologies with a couple hundred VMs, this can take anywhere from seconds to minutes for each call to `ifconfig -a`. Multiplying this across the number of VMs results in these calls taking up the majority of time for add_topo to complete.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
The pre-commit check failed with: flake8...................................................................Failed
ansible/roles/vm_set/library/vm_topology.py:304:31: E201 whitespace after '[' @ccroy-arista can you help fix the issue? |
| # dualtor, hence why the former approach is preserved above for that case. | ||
| else: | ||
| # Get the intf information in a tabulated format. Discard the header row. | ||
| intf_rows = VMTopology.cmd('ifconfig -a -s').split('\n')[1:] |
There was a problem hiding this comment.
Can ip -j addr (JSON output of ip addr) be used? That way, manual string parsing isn't needed, and it also replaces one use of ifconfig with ip.
There was a problem hiding this comment.
Updated to use ip -j addr.
There was a problem hiding this comment.
@saiarcot895 I'm looking into using a sysfs approach with os.listdir, which should be an even more portable and general solution; I will provide an update once I have tested this approach, and if it works well then I'll update the PR as well.
Fix formatting errors encountered during pre-commit check.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
hi @ccroy-arista , anything happened to this PR? |
|
Hi @r12f, I am updating this PR to a more portable and robust solution per: #19119 (comment) I will mark the PR active again once the change is updated. |
|
Ah, I see. Got it and no worries. Feel free to leave the PR in open state. It is just a regular "review-and-address-comment" cycles :D |
|
Thanks Riff, will do :) |
Description of PR
Within
VMTopology.init,ifconfig -ais called for every VM. For topologies with several hundred VMs, each call toifconfig -acan take an appreciable amount of time (from seconds to minutes, depending on exact number of VMs and processing power of the testbed server). Multiplying this across hundreds of VMs results inadd_topotaking a significant amount of time to complete.This change decreases the
add_toposetup time by reducing the number ofifconfig -acalls withinVMTopology.initfrom to 1.This change reduces setup time for non-multi-DUT configurations only; it does not impact the setup time for multi-DUT configurations.
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
To reduce the setup time for
add_topo, particularly for topologies with a larger number of VMs.How did you do it?
Reduced the number of
ifconfig -acalls withinVMTopology.init(...)from to 1.How did you verify/test it?
Ran
add_topoon a t1-isolated topology and confirmed that the setup time was reduced.Any platform specific information?
This change reduces setup time for non-multi-DUT configurations only; it does not impact the setup time for multi-DUT configurations.