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

Netbox Inventory plugin enhancements #54285

Open
wants to merge 12 commits into
base: devel
from

Conversation

@mkeetman
Copy link

mkeetman commented Mar 23, 2019

SUMMARY
  • Added the ability for requests for VM inventory information to collect config_context data issue #53393
  • Added a function to extract dcim device interface information
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

Netbox inventory plugin

ADDITIONAL INFORMATION

The inventory extraction is quite expensive and slow. The Netbox API doesn't appear to lend itself to easy access to this information (except requesting each interface individually)

Tested against Netbox v2.5.3

@ansibot

This comment has been minimized.

@resmo

resmo approved these changes Mar 25, 2019

Copy link
Member

resmo left a comment

LGTM

@ansibot ansibot removed the needs_triage label Mar 25, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 26, 2019

@mkeetman this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@resmo

resmo approved these changes Mar 26, 2019

Copy link
Member

resmo left a comment

shipit

Show resolved Hide resolved lib/ansible/plugins/inventory/netbox.py Outdated

@mkeetman mkeetman force-pushed the mkeetman:devel branch to a008fab Mar 30, 2019

def extract_interfaces(self, host):
try:
if self.interfaces:
url = urljoin(self.api_endpoint, "/api/dcim/interfaces/?limit=0&device_id=%s" % (to_text(host["id"])))

This comment has been minimized.

Copy link
@kb-light

kb-light Mar 30, 2019

You do not differentiate between dcim and VMs, so this would raise a similar bug as #53393.

This comment has been minimized.

Copy link
@mkeetman

mkeetman Mar 31, 2019

Author

This was a conscious decision, and probably short sighted. I've included it. It is admittedly not as useful in the context of a VM as it for network devices - I needed it originally to extract VLANs present on the network device for configuration purposes. IMO for this to be useful for VMs a secondary lookup should be executed to retrieve the associated IP details. Can that be a future PR?

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 31, 2019

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/plugins/inventory/netbox.py:317:119: E225 missing whitespace around operator

click here for bot help

@ansibot ansibot added the ci_verified label Mar 31, 2019

Michael Keetman

@ansibot ansibot removed the ci_verified label Mar 31, 2019

@FragmentedPacket
Copy link
Contributor

FragmentedPacket left a comment

LGTM

@FragmentedPacket

This comment has been minimized.

Copy link
Contributor

FragmentedPacket commented Apr 14, 2019

shipit

@empi89

This comment has been minimized.

Copy link

empi89 commented Apr 23, 2019

Would you also like to add this for ip addresses? When dealing with networking configuration the ip addresses of the network interfaces which are stored in netbox are quite essential.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.