Inventory: warning about query parameters that aren't valid for both devices and VMs

[DouglasHeriot]

ISSUE TYPE

• Bug Report

SOFTWARE VERSIONS

Ansible:

2.9.6

Netbox:

2.7.10

Collection:

v0.1.10

SUMMARY

STEPS TO REPRODUCE

Add to the inventory yml:

query_filters:
  - has_primary_ip: 'true'

EXPECTED RESULTS

Filter devices to only those that have primary IPs set (ie. exclude patch panels and passive devices), not show any warning.

ACTUAL RESULTS

Seems to work, but prints the following warning:

[WARNING]: Warning: has_primary_ip not in ('cluster', 'cluster_id', 'cluster_group', 'cluster_group_id', 'cluster_type',
'cluster_type_id', 'disk', 'mac_address', 'memory', 'name', 'platform', 'platform_id', 'region', 'region_id', 'role', 'role_id', 'site',
'site_id', 'status', 'tag', 'tenant', 'tenant_id', 'tenant_group', 'tenant_group_id', 'vcpus') or starting with cf (Custom field)

Not sure how this should be fixed. The fundamental issue is the query_filters setting is validated against 2 separate lists of parametes. Either need to only report a warning if a filter isn't valid for either, or have allow users to set a separate set of filters for devices and VMs.

I beleive this was introduced by #63 #103

[FragmentedPacket]

If we were to get rid of the warning, I'd be worried that if someone specified an invalid query_filter for either device or VM and they didn't see the expected device/vm in the results then they wouldn't have any clue as to what happened. Does the warning need to be more clear?

And tbh, we could have device_query_filters and vm_query_filters, but I'm not sure how everyone is using the plugin and what would be better so I'll leave it up to others' opinions.

[DouglasHeriot]

A more important bug related to this - if query_parameters are defined but none are valid for VMs, vm_url is set to None and no VMs are fetched at all. This actually blocks me from using this inventory for VMs.

ansible_modules/plugins/inventory/nb_inventory.py

Lines 563 to 564 in 2d6894b

	if len(vm_query_parameters) <= 1:
	vm_url = None

[FragmentedPacket]

Yeah I made an update to it since someone was having the issue of it returning all VMs if the query filter wasn't valid for VM.

More input would be helpful but I assume if something doesn't match a filter, you wouldn't just want to get everything, but really would want more input from others to get the proper functionality

[DouglasHeriot]

My particular use case is using the query_filters has_primary_ip: true to filter out all the patch panels and passive devices I don't care about having in the inventory. However has_primary_ip doesn't exist on VMs so I don't get any VMs at all.

I'm working right now on adding separate device_query_filters and vm_query_filters in addition to query_filters. The idea is you can put device or vm specific filters in the more specific options, and then query_filters continues to apply to both.

Maybe we can solve this by maintaining the existing behaviour - if you put a device or vm specific filter in query_filters then only devices or vms will be returned. And if you want to continue getting devices and vms then use the more specific options instead?

[DouglasHeriot]

Another thought - once the inventory is built there's no host variable that clearly identifies if a host is a physical device or a VM. You could infer it based on if variables like cluster or vcpus exist, but that depends on if they're configured in Netbox at all.

Might be nice if there was a group_by option to create a group for all devices and all vms?

And maybe a special query_filters option for only retrieving devices or vms?

[FragmentedPacket]

Should this be closed via merge of #153