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.

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

vmware_guest_network speedup module #2277

Conversation

GuideGlyph
Copy link
Contributor

SUMMARY

Speedup vmware_guest_network module. Current design loop though all networks and slow for big infrastructure with many networks (more then 100).

ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
COMPONENT NAME

vmware_guest_network.py

ADDITIONAL INFORMATION

In my scenario current module spend 5 min to find network (this function run twice during module work and lead to 10min waiting)

        if compute_resource:
            for network in compute_resource.network:
                if isinstance(network, vim.dvs.DistributedVirtualPortgroup):
                    dvs = network.config.distributedVirtualSwitch
                    if (switch_name and dvs.config.name == switch_name) or not switch_name:
                        if network.config.name == network_name:
                            return network
                        if hasattr(network.config.defaultPortConfig.vlan, 'vlanId') and \
                           network.config.defaultPortConfig.vlan.vlanId == vlan_id:
                            return network
                        if hasattr(network.config.defaultPortConfig.vlan, 'pvlanId') and \
                           network.config.defaultPortConfig.vlan.pvlanId == vlan_id:
                            return network
                elif isinstance(network, vim.Network):
                    if network_name and network_name == network.name:
                        return network
                    if vlan_id:
                        for k in pg_lookup.keys():
                            if vlan_id == pg_lookup[k]['vlan_id']:
                                if k == network.name:
                                    return network
                                break

This code loop through networks and check if its name equal to self.params['network_name']. Its slow. Its about 30s to check 100 networks.

vmware.py has perfect function for this. PyVmomi.find_network_by_name works perfect and find network immediately

@GuideGlyph
Copy link
Contributor Author

guess this failed because esxi MOB is disabled by default, gonna return part of old code to be compatible

Build failed. https://ansible.softwarefactory-project.io/zuul/buildset/8f9786e13bd147a482ee5496198a3336

✔️ ansible-tox-linters SUCCESS in 4m 22s ✔️ build-ansible-collection SUCCESS in 7m 30s ✔️ ansible-test-cloud-integration-vcenter7_only-stable218 SUCCESS in 17m 02s ✔️ ansible-test-cloud-integration-vcenter7_2esxi-stable218 SUCCESS in 17m 54s ❌ ansible-test-cloud-integration-vcenter7_1esxi-stable218_1_of_2 FAILURE in 42m 44s ⚠️ ansible-test-cloud-integration-vcenter7_1esxi-stable218_2_of_2 CANCELED ✔️ ansible-galaxy-importer SUCCESS in 4m 42s

@GuideGlyph GuideGlyph changed the title Speedup vmware_guest_network module vmware_guest_network speedup module Dec 24, 2024
@GuideGlyph
Copy link
Contributor Author

recheck

@mariolenz
Copy link
Collaborator

I'm sorry, I didn't find much time to work on this collection during the last weeks. I hope I'm able to review this PR soon.

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

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

@GuideGlyph Please a changelog fragment. Maybe something like:

changelogs/fragments/2277-vmware_guest_network.yml

minor_changes:
  - vmware_guest_network - Speedup network search
    (https://github.com/ansible-collections/community.vmware/pull/2277).

@GuideGlyph
Copy link
Contributor Author

@mariolenz ready

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

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

Thanks @GuideGlyph!

I'm not 100% sure that I understand your changes. But if this speeds up things for you and the CI is happy 🤷

FYI I'm planing to do a new release within the next few days, possibly next weekend.

@mariolenz
Copy link
Collaborator

recheck

@mariolenz
Copy link
Collaborator

recheck

@GuideGlyph
Copy link
Contributor Author

Thanks @GuideGlyph!

I'm not 100% sure that I understand your changes. But if this speeds up things for you and the CI is happy 🤷

FYI I'm planing to do a new release within the next few days, possibly next weekend.

Thanks for review. Some info how it works.

old code use a huge loop through all networks and check that name mach. For a small infrastructure it work nice, but when there are more then 500 networks it become slow.

        if compute_resource:
            for network in compute_resource.network:
                if isinstance(network, vim.dvs.DistributedVirtualPortgroup):
                    dvs = network.config.distributedVirtualSwitch
                    if (switch_name and dvs.config.name == switch_name) or not switch_name:
                        if network.config.name == network_name:
                            return network
                        if hasattr(network.config.defaultPortConfig.vlan, 'vlanId') and \
                           network.config.defaultPortConfig.vlan.vlanId == vlan_id:
                            return network
                        if hasattr(network.config.defaultPortConfig.vlan, 'pvlanId') and \
                           network.config.defaultPortConfig.vlan.pvlanId == vlan_id:
                            return network
                elif isinstance(network, vim.Network):
                    if network_name and network_name == network.name:
                        return network
                    if vlan_id:
                        for k in pg_lookup.keys():
                            if vlan_id == pg_lookup[k]['vlan_id']:
                                if k == network.name:
                                    return network
                                break

I add a code before loop that check if network can be find using PyVmomi.find_network_by_name(). It works faster because it use PyVmomi.get_managed_objects_properties()

    def get_managed_objects_properties(self, vim_type, properties=None):
        """
        Look up a Managed Object Reference in vCenter / ESXi Environment
        :param vim_type: Type of vim object e.g, for datacenter - vim.Datacenter
        :param properties: List of properties related to vim object e.g. Name
        :return: local content object
        """

Managed Object References doc
Managed Object Browser

PyVmomi.find_network_by_name() work instantly and fix time issue. If it cant find object (or disabled) old code should find it.

@mariolenz
Copy link
Collaborator

Thanks for the additional info @GuideGlyph! I'd have merged this already yesterday, but somehow our CI/CD didn't want me to. I'll give it another try now :-)

Copy link

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/9e2d5e5bcab64455b7b249c8d7d4a4e5

✔️ ansible-tox-linters SUCCESS in 5m 46s
✔️ build-ansible-collection SUCCESS in 6m 09s
✔️ ansible-galaxy-importer SUCCESS in 5m 11s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 817b93f into ansible-collections:main Jan 21, 2025
23 checks passed
@GuideGlyph GuideGlyph deleted the vmware_guest_network-speedup branch January 21, 2025 17:22
jpiron pushed a commit to jpiron/community.vmware that referenced this pull request Jan 23, 2025
SUMMARY
Speedup vmware_guest_network module. Current design loop though all networks and slow for big infrastructure with many networks (more then 100).
ISSUE TYPE

Bugfix Pull Request
Docs Pull Request

COMPONENT NAME
vmware_guest_network.py
ADDITIONAL INFORMATION
In my scenario current module spend 5 min to find network (this function run twice during module work and lead to 10min waiting)
        if compute_resource:
            for network in compute_resource.network:
                if isinstance(network, vim.dvs.DistributedVirtualPortgroup):
                    dvs = network.config.distributedVirtualSwitch
                    if (switch_name and dvs.config.name == switch_name) or not switch_name:
                        if network.config.name == network_name:
                            return network
                        if hasattr(network.config.defaultPortConfig.vlan, 'vlanId') and \
                           network.config.defaultPortConfig.vlan.vlanId == vlan_id:
                            return network
                        if hasattr(network.config.defaultPortConfig.vlan, 'pvlanId') and \
                           network.config.defaultPortConfig.vlan.pvlanId == vlan_id:
                            return network
                elif isinstance(network, vim.Network):
                    if network_name and network_name == network.name:
                        return network
                    if vlan_id:
                        for k in pg_lookup.keys():
                            if vlan_id == pg_lookup[k]['vlan_id']:
                                if k == network.name:
                                    return network
                                break
This code loop through networks and check if its name equal to self.params['network_name']. Its slow. Its about 30s to check 100 networks.
vmware.py has perfect function for this. PyVmomi.find_network_by_name works perfect and find network immediately

Reviewed-by: Mario Lenz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants