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

Inventory bug: services queried by device name doesn't work for VMs, or names with spaces #142

Closed
DouglasHeriot opened this issue Mar 25, 2020 · 6 comments · Fixed by #202
Labels

Comments

@DouglasHeriot
Copy link
Contributor

DouglasHeriot commented Mar 25, 2020

ISSUE TYPE
  • Bug Report
SOFTWARE VERSIONS
Ansible:

2.9.6

Netbox:

2.7.10

Collection:

0.10.1

SUMMARY

There are a few issues here: When the inventory queries the Netbox API for a list of services:

  • It passes the device name instead of an ID
  • If this device name has spaces, it is not cached properly and a warning appears
  • If the host is a VM and not a device, then the list of services is not fetched as it's hard-coded to always query by device name and not VM name/ID.

I plan to work on fixing this, by always querying for list of services by ID, and properly constructing the URL for either device or VM query.

STEPS TO REPRODUCE

Here is my inventory yaml file:

plugin: netbox.netbox.nb_inventory
api_endpoint: https://netbox
config_context: True

cache: True
cache_timeout: 3600
cache_plugin: jsonfile
cache_connection: /tmp/inventory_netbox

interfaces: False

group_by:
  - sites
  - tenants
  - racks
  - tags
  - device_roles
  - device_types
  - manufacturers
  - platforms

query_filters:
  - has_primary_ip: 'true'
EXPECTED RESULTS
  • No warnings
  • Services returned for both devices and VMs
ACTUAL RESULTS
[WARNING]: error in 'jsonfile' cache plugin while trying to read /tmp/inventory_netbox/netbox.netbox.nb_inventory_d46c3s_4f124 : b"[Errno
2] No such file or directory: '/tmp/inventory_netbox/netbox.netbox.nb_inventory_d46c3s_4f124'"
Fetching: https://netbox/api/ipam/services/?device=IMAG VMU
@bluikko
Copy link
Contributor

bluikko commented Apr 3, 2020

This issue is similar to #156. It might be a good idea to look at generalizing "use ID (device or VM) instead of device" to be used everywhere.

The Ansible bundled Netbox has had PRs for fixing it for ages but IIRC this module does not share code with it?

@DouglasHeriot
Copy link
Contributor Author

@bluikko Devices and VMs are totally separate things - their IDs and names do not share a namespace. That means you can have a device and a VM using the same IDs and same names. Everything in this collection will need to maintain the devices and VMs separately, and have separate parameters to specify if you're referring to a device or VM.

Unfortunately the implementation of the inventory plugin and the modules is completely separate right now, so there's 2 separate pieces of work right now.

The "Ansible bundled Netbox" no longer exists as of ansible/ansible@bdd82ad and will not be included in Ansible 2.10 - this collection is its new home. The benefit is that it's much easier to manage issues and pull requests separately than in the thousands and thousands of them held up in the Ansible project. (And we can also do better integration testing now)

@bluikko
Copy link
Contributor

bluikko commented Apr 3, 2020

I see, I've not looked into how devices/VMs are represented.

I am 100% sure there were PRs in the Ansible repo that were supposed to enable actions against VMs similarly to devices. Perhaps they then have a "type" parameter or separate device/vm name parameters.

@DouglasHeriot
Copy link
Contributor Author

Looking at working on this and #143 soon, once #155 is merged. I'm wondering if I should remove all the custom request handling from nb_inventory and have it use pynetbox instead. Not sure if there's any efficiency benefits to keeping it simple and not introducing pynetbox?

@FragmentedPacket
Copy link
Contributor

You know there was threading added to pynetbox for .all() and .get() or .filter(). I'm fine with moving to pynetbox as it should be easier to pass the object around for the API calls and not working about concatenating URLs, etc.

I think the threading does require Python 3.6 though so up to you.

@DouglasHeriot
Copy link
Contributor Author

Yeah I started having a closer look at how it handles threading compared to nb_inventory. The other feature I'll have to think about is caching - nb_inventory currently supports ansible's cache plugins and trying to implement that with pynetbox looks like a bit of a challenge. There's no easy way to insert custom caching inside pynetbox, and caching the resultant objects to/from JSON might be difficult as they'll end up being recursive (eg. device object to interface and back to device)

So, I'm thinking it'll be quicker to just resolve these last few issues without introducing pynetbox just yet.

DouglasHeriot added a commit to hillsong/ansible_modules that referenced this issue May 4, 2020
DouglasHeriot added a commit to hillsong/ansible_modules that referenced this issue May 4, 2020
DouglasHeriot added a commit to hillsong/ansible_modules that referenced this issue May 4, 2020
DouglasHeriot added a commit to hillsong/ansible_modules that referenced this issue May 5, 2020
DouglasHeriot added a commit to hillsong/ansible_modules that referenced this issue May 7, 2020
… interfaces/services.

A side effect is it resolves netbox-community#142 fetching services for VMs

Includes starting to better support virtual chasis - should only take the master device and not the children. Some work on this started in ansible/ansible#60642
DouglasHeriot added a commit to hillsong/ansible_modules that referenced this issue May 7, 2020
… interfaces/services.

A side effect is it resolves netbox-community#142 fetching services for VMs

Includes starting to better support virtual chasis - should only take the master device and not the children. Some work on this started in ansible/ansible#60642

See the TODO comments for work still to be done before being ready to merge.
DouglasHeriot added a commit to hillsong/ansible_modules that referenced this issue May 8, 2020
… interfaces/services.

A side effect is it resolves netbox-community#142 fetching services for VMs

Includes better support virtual chasis - only take the master device and not the children. Some of this from ansible/ansible#60642

See the TODO comments for work still to be done before being ready to merge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants