Skip to content

Commit 5f497b5

Browse files
Fix device_role -> role and add testing for Netbox 3.6 (#1066)
* Add testing for v3.6 of Netbox. * Add custom logic to account for device_role -> role for Netbox 3.6+ but can be removed eventually. * Remove update_vc_child as query_param if statement as that is handled further down in code.
1 parent 4211b71 commit 5f497b5

File tree

110 files changed

+20032
-27
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

110 files changed

+20032
-27
lines changed

.github/workflows/main.yml

+2
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ jobs:
8282
NETBOX_DOCKER_VERSION: 2.5.3
8383
- VERSION: "v3.5"
8484
NETBOX_DOCKER_VERSION: 2.6.1
85+
- VERSION: "v3.6"
86+
NETBOX_DOCKER_VERSION: 2.7.0
8587
# If we want to integration test wiht all supported Python:
8688
#python-version: ["3.9", "3.10", "3.11"]
8789

hacking/local-test.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,5 @@
99
ansible-galaxy collection install netbox-netbox-*.tar.gz -p .
1010

1111
# You can now cd into the installed version and run tests
12-
(cd ansible_collections/netbox/netbox/ && ansible-test units -v --python 3.6 && ansible-test sanity --requirements -v --python 3.6 --skip-test pep8 plugins/)
12+
(cd ansible_collections/netbox/netbox/ && ansible-test units -v --python 3.10 && ansible-test sanity --requirements -v --python 3.10 --skip-test pep8 plugins/)
1313
rm -rf ansible_collections

plugins/module_utils/netbox_dcim.py

+9-13
Original file line numberDiff line numberDiff line change
@@ -211,19 +211,15 @@ def run(self):
211211
)
212212

213213
# This is logic to handle interfaces on a VC
214-
if self.endpoint == "interfaces":
215-
if self.nb_object:
216-
device = self.nb.dcim.devices.get(self.nb_object.device.id)
217-
if (
218-
device["virtual_chassis"]
219-
and self.nb_object.device.id != self.data["device"]
220-
):
221-
if self.module.params.get("update_vc_child"):
222-
data["device"] = self.nb_object.device.id
223-
else:
224-
self._handle_errors(
225-
msg="Must set update_vc_child to True to allow child device interface modification"
226-
)
214+
if self.endpoint == "interfaces" and self.nb_object:
215+
child = self.nb.dcim.devices.get(self.nb_object.device.id)
216+
if child["virtual_chassis"] and child.id != data["device"]:
217+
if self.module.params.get("update_vc_child"):
218+
data["device"] = child.id
219+
else:
220+
self._handle_errors(
221+
msg="Must set update_vc_child to True to allow child device interface modification"
222+
)
227223

228224
if self.state == "present":
229225
self._ensure_object_exists(nb_endpoint, endpoint_name, name, data)

plugins/module_utils/netbox_utils.py

+27-11
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,7 @@
562562
"cluster_type": "type",
563563
"cluster_group": "group",
564564
"contact_group": "group",
565+
"device_role": "role",
565566
"fhrp_group": "group",
566567
"inventory_item_role": "role",
567568
"parent_contact_group": "parent",
@@ -819,9 +820,15 @@ def _convert_identical_keys(self, data):
819820
if self._version_check_greater(self.version, "2.7", greater_or_equal=True):
820821
if data.get("form_factor"):
821822
temp_dict["type"] = data.pop("form_factor")
823+
822824
for key in data:
823825
if self.endpoint == "power_panels" and key == "rack_group":
824826
temp_dict[key] = data[key]
827+
# TODO: Remove this once the lowest supported Netbox version is 3.6 or greater as we can use default logic of CONVERT_KEYS moving forward.
828+
elif key == "device_role" and not self._version_check_greater(
829+
self.version, "3.6", greater_or_equal=True
830+
):
831+
temp_dict[key] = data[key]
825832
elif key in CONVERT_KEYS:
826833
# This will keep the original key for keys in list, but also convert it.
827834
if key in ("assigned_object", "scope"):
@@ -855,19 +862,18 @@ def _get_query_param_id(self, match, data):
855862
"""
856863
if isinstance(data.get(match), int):
857864
return data[match]
858-
else:
859-
endpoint = CONVERT_TO_ID[match]
860-
app = self._find_app(endpoint)
861-
nb_app = getattr(self.nb, app)
862-
nb_endpoint = getattr(nb_app, endpoint)
865+
endpoint = CONVERT_TO_ID[match]
866+
app = self._find_app(endpoint)
867+
nb_app = getattr(self.nb, app)
868+
nb_endpoint = getattr(nb_app, endpoint)
863869

864-
query_params = {QUERY_TYPES.get(match): data[match]}
865-
result = self._nb_endpoint_get(nb_endpoint, query_params, match)
870+
query_params = {QUERY_TYPES.get(match): data[match]}
871+
result = self._nb_endpoint_get(nb_endpoint, query_params, match)
866872

867-
if result:
868-
return result.id
869-
else:
870-
return data
873+
if result:
874+
return result.id
875+
else:
876+
return data
871877

872878
def _build_query_params(
873879
self, parent, module_data, user_query_params=None, child=None
@@ -909,6 +915,16 @@ def _build_query_params(
909915

910916
if parent == "vlan_group" and match == "site":
911917
query_dict.update({match: query_id})
918+
elif (
919+
parent == "interface"
920+
and "device" in module_data
921+
and self._version_check_greater(
922+
self.version, "3.6", greater_or_equal=True
923+
)
924+
):
925+
query_dict.update(
926+
{"virtual_chassis_member_id": module_data["device"]}
927+
)
912928
else:
913929
query_dict.update({match + "_id": query_id})
914930
else:

tests/integration/netbox-deploy.py

+5
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,11 @@ def make_netbox_calls(endpoint, payload):
343343
devices[0]["location"] = created_rack_groups[0].id
344344
devices[1]["location"] = created_rack_groups[0].id
345345
devices[3]["location"] = created_rack_groups[0].id
346+
# TODO: Remove this logic and adjust payload from device_role -> role once Netbox 3.6 or greater is supported.
347+
if nb_version >= version.parse("3.6"):
348+
for device in devices:
349+
if "device_role" in device:
350+
device["role"] = device.pop("device_role")
346351

347352
created_devices = make_netbox_calls(nb.dcim.devices, devices)
348353
### Device variables to be used later on
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
runme_config
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# https://docs.ansible.com/ansible/devel/dev_guide/testing/sanity/integration-aliases.html
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
#!/usr/bin/env python
2+
3+
# Inspired by community.aws collection script_inventory_ec2 test
4+
# https://github.com/ansible-collections/community.aws/blob/master/tests/integration/targets/script_inventory_ec2/inventory_diff.py
5+
6+
from __future__ import absolute_import, division, print_function
7+
8+
__metaclass__ = type
9+
10+
import argparse
11+
import json
12+
import sys
13+
from operator import itemgetter
14+
15+
from deepdiff import DeepDiff
16+
17+
# NetBox includes "created" and "last_updated" times on objects. These end up in the interfaces objects that are included verbatim from the NetBox API.
18+
# "url" may be different if local tests use a different host/port
19+
# Remove these from files saved in git as test data
20+
KEYS_REMOVE = frozenset(["created", "last_updated", "url"])
21+
22+
# Ignore these when performing diffs as they will be different for each test run
23+
# (Was previously keys specific to NetBox 2.6)
24+
KEYS_IGNORE = frozenset()
25+
26+
# Rack Groups became hierarchical in NetBox 2.8. Don't bother comparing against test data in NetBox 2.7
27+
KEYS_IGNORE_27 = frozenset(
28+
[
29+
"rack_groups", # host var
30+
"rack_group_parent_rack_group", # group, group_names_raw = False
31+
"parent_rack_group", # group, group_names_raw = True
32+
]
33+
)
34+
35+
36+
# Assume the object will not be recursive, as it originally came from JSON
37+
def remove_keys(obj, keys):
38+
if isinstance(obj, dict):
39+
keys_to_remove = keys.intersection(obj.keys())
40+
for key in keys_to_remove:
41+
del obj[key]
42+
43+
for key, value in obj.items():
44+
remove_keys(value, keys)
45+
46+
elif isinstance(obj, list):
47+
# Iterate over temporary copy, as we may remove items
48+
for item in obj[:]:
49+
if isinstance(item, str) and item in keys:
50+
# List contains a string that we want to remove
51+
# eg. a group name in list of groups
52+
obj.remove(item)
53+
remove_keys(item, keys)
54+
55+
56+
def sort_hostvar_arrays(obj):
57+
meta = obj.get("_meta")
58+
if not meta:
59+
return
60+
61+
hostvars = meta.get("hostvars")
62+
if not hostvars:
63+
return
64+
65+
for _, host in hostvars.items():
66+
if interfaces := host.get("interfaces"):
67+
host["interfaces"] = sorted(interfaces, key=itemgetter("id"))
68+
69+
if services := host.get("services"):
70+
host["services"] = sorted(services, key=itemgetter("id"))
71+
72+
73+
def read_json(filename):
74+
with open(filename, "r", encoding="utf-8") as file:
75+
return json.loads(file.read())
76+
77+
78+
def write_json(filename, data):
79+
with open(filename, "w", encoding="utf-8") as file:
80+
json.dump(data, file, indent=4)
81+
82+
83+
def main():
84+
parser = argparse.ArgumentParser(description="Diff Ansible inventory JSON output")
85+
parser.add_argument(
86+
"filename_a",
87+
metavar="ORIGINAL.json",
88+
type=str,
89+
help="Original json to test against",
90+
)
91+
parser.add_argument(
92+
"filename_b",
93+
metavar="NEW.json",
94+
type=str,
95+
help="Newly generated json to compare against original",
96+
)
97+
parser.add_argument(
98+
"--write",
99+
action="store_true",
100+
help=(
101+
"When comparing files, various keys are removed. "
102+
"This option will not compare the files, and instead writes ORIGINAL.json to NEW.json after removing these keys. "
103+
"This is used to clean the test json files before saving to the git repo. "
104+
"For example, this removes dates. "
105+
),
106+
)
107+
parser.add_argument(
108+
"--netbox-version",
109+
metavar="VERSION",
110+
type=str,
111+
help=(
112+
"Apply comparison specific to NetBox version. "
113+
"For example, rack_groups arrays will only contain a single item in v2.7, so are ignored in the comparison."
114+
),
115+
)
116+
117+
args = parser.parse_args()
118+
119+
data_a = read_json(args.filename_a)
120+
121+
if args.write:
122+
# When writing test data, only remove "remove_keys" that will change on every git commit.
123+
# This makes diffs more easily readable to ensure changes to test data look correct.
124+
remove_keys(data_a, KEYS_REMOVE)
125+
sort_hostvar_arrays(data_a)
126+
write_json(args.filename_b, data_a)
127+
128+
else:
129+
data_b = read_json(args.filename_b)
130+
131+
# Ignore keys that we don't want to diff, in addition to the ones removed that change on every commit
132+
keys = KEYS_REMOVE.union(KEYS_IGNORE)
133+
remove_keys(data_a, keys)
134+
remove_keys(data_b, keys)
135+
136+
sort_hostvar_arrays(data_a)
137+
sort_hostvar_arrays(data_b)
138+
139+
# Perform the diff
140+
result = DeepDiff(data_a, data_b, ignore_order=True)
141+
142+
if result:
143+
# Dictionary is not empty - print differences
144+
print(json.dumps(result, sort_keys=True, indent=4))
145+
sys.exit(1)
146+
else:
147+
# Success, no differences
148+
sys.exit(0)
149+
150+
151+
if __name__ == "__main__":
152+
main()

0 commit comments

Comments
 (0)