General CI Fix

[ryanmerolle]

Closes Issue #567

• Corrects integration testing failures because mismatched netbox-docker and NetBox version related to latest assuming v2.11 for both repos (NetBox 3.0.x release broke that logic)
• Correct sanity checks related to missing cert references
• Correct sanity checks related to tags (type: list) not having a element defined
• Removed unneeded ignore references to parameter-list-no-elements that were causing CI to fail
• Introduce but commented out NetBox v3.0 integration testing

[felixfontein]

The PR #534 which added this option (but failed to document them) was also missing version_added for all the new options added. It's probably a good idea to also add that in this PR.

[nahun]

>       cert = self.module.params["cert"]
E       KeyError: 'cert'

I believe its because cert doesn't have a default value so it doesn't get set in module params. But I could easily be wrong

I noticed this too on:

ansible_modules/plugins/module_utils/netbox_utils.py

Line 545 in c612612

session.cert = tuple(i for i in cert)

All modules fail because cert is None if the user doesn't provide that parameter in their task and it can't loop on None.

[felixfontein]

@nahun all module options have the implicit default None. This failure happens in the tests because the tests have a hard-coded fake params dictionary: https://github.com/netbox-community/ansible_modules/blob/devel/tests/unit/module_utils/test_netbox_base_class.py#L79

[felixfontein]

@ryanmerolle adding "cert": None, to

ansible_modules/tests/unit/module_utils/test_netbox_base_class.py

Lines 47 to 60 in 4d3f69c

	return {
	"netbox_url": "http://netbox.local/",
	"netbox_token": "0123456789",
	"data": {
	"name": "Test Device1",
	"device_role": "Core Switch",
	"device_type": "Cisco Switch",
	"manufacturer": "Cisco",
	"site": "Test Site",
	"asset_tag": "1001",
	},
	"state": "present",
	"validate_certs": False,
	}

should fix the remaining issues, resp. at least proceed them a bit longer until failure :)

[ryanmerolle]

Thanks for the pointer @felixfontein. I felt like playing whack a mole with each CI linting_unit_testing stage Error I would find as I would fix each one. I just got tired and had to call it quits for the night. ON to the integration testing errors now.

[ryanmerolle]

@FragmentedPacket I got a lot of the CI fixed up, but could I bother you this once to review? Totally understand if not.

Summary: I got us unblocked for lint unit testing but hitting some snags on integration testing.

[rodvand]

As @nahun mentioned earlier the line

ansible_modules/plugins/module_utils/netbox_utils.py

Line 545 in c612612

session.cert = tuple(i for i in cert)

is troublesome when cert is None. So I suggest we just do a simple

if cert:
    session.cert = tuple(i for i in cert)

to avoid the issue. The cert addition to the modules does need a more thorough walkthrough I think when things have "settled" a bit.

[ryanmerolle]

All that is left is either skipping v3.0 integration requiring success or fixing the integration testing for 3.0.

Of course I need to document all the changes since this became a bigger PR than I intended.

[nahun]

I say we let 3.0 fail for now as we haven't done any real work for 3.0 yet.

Great work!

[rodvand]

I agree. 3.0 support should be a separate PR.

[ryanmerolle]

I am commenting out NetBox 3.0 CI.

I agree on it being its own PR, but at the same time it was so close and no major API changes were introduce, so it should not be failing.

Future PRs should address:

• Tests for current ansible-core and previous major version (ansible-base in the case of 2.10).
• Any new Ansible requirements with regard to continuing the collection inclusion with the "fat" ansible install.
• NetBox 3.0 testing.
• Cleanup of old references in integration and unit tests (python2, old netbox versions below 2.11, etc).
• Local CI & contribution documentation updates. The current barrier to entry on updating CI or running CI locally is quite high, and I only go this far due to my more than average involvement in the netbox-docker repo and its deployment in my own job.

[ryanmerolle]

@jeremystretch Ready to Merge...

[gundalow]

Thanks :)