Inventory: IPaddresses of interfaces is lost when using cache.

[matte4711]

ISSUE TYPE

• Bug Report

SOFTWARE VERSIONS

Ansible:

ansible 2.9.10

Netbox:

v2.8.7

Collection:

v0.3.0

SUMMARY

When using caching together with nb_inventory and expanding interfaces IP-address mapping gets lost when using cache.

STEPS TO REPRODUCE

The issue is reproducible by configure nb_inventory and using cache.

plugin: netbox.netbox.nb_inventory
cache: true
cache_plugin: jsonfile
cache_timeout: 3600
api_endpoint: https://netbox-lab.netnod.se
interfaces: true

Take any device with a interface that have ip-address assigned to it and run the following command:

$ rm /tmp/ansible/ansible_inventory_netbox*
$ ansible-inventory --host ag1b-bck.gbg.netnod.se 2>/dev/null | jq '.interfaces[] | select(.name == "Ethernet1").ip_addresses[].address'
"172.20.9.2/31"
$ ansible-inventory --host ag1b-bck.gbg.netnod.se 2>/dev/null | jq '.interfaces[] | select(.name == "Ethernet1").ip_addresses[].address'
$ 

I have done some debugging and it looks like the issue is due to the ip-addresses query results from netbox is modified and the modified data is written to the cache.

Removal of interface mapping is done here:
(https://github.com/netbox-community/ansible_modules/blob/devel/plugins/inventory/nb_inventory.py#L837)

The ip-address-> interface mapping is needed to attach the ip-addresses to the interfaces.
And in consecutive runs this is not available.

EXPECTED RESULTS

ACTUAL RESULTS

[DouglasHeriot]

Firstly I have to say sorry to everyone I've been busy and not replied or worked on issues here for a while.

Now, just a quick look at this – I'm not sure what is causing this. I don't think it's related to the line of code you point out – caching is handled inside the _fetch_information method on the raw HTTP response immediately after it's converted from JSON, before any processing of the results happen.

I use this regularly with interfaces that have IPs, with caching, and haven't run into this issue. I'll have to think a bit harder about what could cause it.

[DouglasHeriot]

Actually, @matte4711 you could be right – I may have misunderstood how Ansible's caching works. I assumed they were written to disk before this modification happens.

[matte4711]

I made a draft PR that will fix the issue by copying the data before modifying it.

Would this be an acceptable solution?

[DouglasHeriot]

@matte4711 This looks like a reasonable fix to me.

It would be good to have the integration tests check for this case. Some of them are already configured to use the cache – but I guess the ones that use the cache may not have interfaces turned on.

• test-inventory-plurals.yml caching on, interfaces on
• test-inventory-options.yml caching on, interfaces off

The order of tests in runme.sh is not defined - it's just a bash for loop over *.yml files. But as long as we turn on caching and interfaces for at least 2 tests, the issue should be reproduced.

I'm not sure what is the best way to organise test inventory options. I tried to come up with every combination but didn't think to run each combination with and without caching.

[matte4711]

Is there any reason for not always using cache in the tests?
If so it would probably be best to setup tests for both with and without cache to really catch all potential issues.

Personally i'd probably go for the always cache option. :)

[DouglasHeriot]

There's lots of different options to test, that potentially have significant effects:

• with and without caching
• with and without interfaces
• with and without services
• plurals true and false
• flatten_… true and false
• fetch_all true and false

You could quickly get to hundreds of combinations, but we don't want running the tests to take forever.

[sc68cal]

I got bit by this issue recently, it would be great to get this fixed.

[FragmentedPacket]

I believe we're waiting for an update to tests to test the cache. I've got some stuff in my backlog that I want to get done before addressing this, but would be great if anyone else can help out.

[sc68cal]

Ok, I could investigate adding / updating tests, the only question is since @matte4711 's PR is based off his fork it may be difficult to get them added to his PR. I'll cherry pick in his patch and then add the tests and reference his PR in my PR, if that's acceptable

[matte4711]

I have had some extra time to make some tests into the regression tests. I have added a change to the regressions that now is verifying that the fix actually works.

Now both test-inventory-plurals.ym and test-inventory-options-flatten.yml
uses:

cache: True
interfaces: True
fetch_all: False