Fix two idempotency issues when updating VMs. (#859)

bvergnaud · web-flow · commit 1f15f7e2c8bc · 2022-11-07T08:23:01.000+01:00
* fix: make _version_check_greater return type consistent values
* fix: typo in form_factor error message
* fix: make netbox_virtual_machine updates idempotent on netbox v2.10+
diff --git a/plugins/module_utils/netbox_utils.py b/plugins/module_utils/netbox_utils.py
@@ -676,6 +676,8 @@ def _version_check_greater(self, greater, lesser, greater_or_equal=False):
         elif g_major == l_major and g_minor > l_minor:
             return True
 
+        return False
+
     def _connect_netbox_api(self, url, token, ssl_verify, cert):
         try:
             session = requests.Session()
@@ -1255,12 +1257,28 @@ def _update_netbox_object(self, data):
         """
         serialized_nb_obj = self.nb_object.serialize()
         updated_obj = serialized_nb_obj.copy()
+
+        if serialized_nb_obj.get("custom_fields"):
+            serialized_nb_obj["custom_fields"] = {
+                key: value
+                for key, value in serialized_nb_obj["custom_fields"].items()
+                if value is not None
+            }
+
+        if updated_obj.get("custom_fields"):
+            updated_obj["custom_fields"] = {
+                key: value
+                for key, value in updated_obj["custom_fields"].items()
+                if value is not None
+            }
+
         updated_obj.update(data)
+
         if serialized_nb_obj.get("tags") and data.get("tags"):
             serialized_nb_obj["tags"] = set(serialized_nb_obj["tags"])
             updated_obj["tags"] = set(data["tags"])
 
-        # Ensure idempotency for site and virtual machine on version pre-3.0
+        # Ensure idempotency for site on older netbox versions
         version_pre_30 = self._version_check_greater("3.0", self.version)
         if (
             serialized_nb_obj.get("latitude")
@@ -1275,15 +1293,13 @@ def _update_netbox_object(self, data):
         ):
             updated_obj["longitude"] = str(data["longitude"])
 
-        if serialized_nb_obj.get("vcpus") and data.get("vcpus") and version_pre_30:
-            updated_obj["vcpus"] = "{0:.2f}".format(data["vcpus"])
-
-        if serialized_nb_obj.get("custom_fields"):
-            serialized_nb_obj["custom_fields"] = {
-                key: value
-                for key, value in serialized_nb_obj["custom_fields"].items()
-                if value is not None
-            }
+        # Ensure idempotency for virtual machine on older netbox versions
+        version_pre_211 = self._version_check_greater("2.11", self.version)
+        if serialized_nb_obj.get("vcpus") and data.get("vcpus"):
+            if version_pre_211:
+                updated_obj["vcpus"] = int(data["vcpus"])
+            else:
+                updated_obj["vcpus"] = float(data["vcpus"])
 
         if serialized_nb_obj == updated_obj:
             return serialized_nb_obj, None
@@ -1296,7 +1312,7 @@ def _update_netbox_object(self, data):
                         data_after[key] = updated_obj[key]
                 except KeyError:
                     if key == "form_factor":
-                        msg = "form_factor is not valid for NetBox 2.7 onword. Please use the type key instead."
+                        msg = "form_factor is not valid for NetBox 2.7 onward. Please use the type key instead."
                     else:
                         msg = (
                             "%s does not exist on existing object. Check to make sure valid field."
diff --git a/tests/integration/targets/v3.1/tasks/netbox_virtual_machine.yml b/tests/integration/targets/v3.1/tasks/netbox_virtual_machine.yml
@@ -76,24 +76,53 @@
       - test_three['virtual_machine']['tags'][0] == 4
       - test_three['msg'] == "virtual_machine Test VM One updated"
 
-- name: "VIRTUAL_MACHINE 4: Delete"
+- name: "VIRTUAL_MACHINE 4: Test idempotence"
+  netbox.netbox.netbox_virtual_machine:
+    netbox_url: http://localhost:32768
+    netbox_token: 0123456789abcdef0123456789abcdef01234567
+    data:
+      name: "Test VM One"
+      cluster: "Test Cluster"
+      vcpus: 8.5
+      memory: 8
+      status: "Planned"
+      virtual_machine_role: "Test VM Role"
+      tags:
+        - "Schnozzberry"
+    state: present
+  register: test_four_idempotence
+
+- name: "VIRTUAL_MACHINE 4: ASSERT - Not changed"
+  assert:
+    that:
+      - test_four_idempotence is not changed
+      - test_four_idempotence['virtual_machine']['name'] == "Test VM One"
+      - test_four_idempotence['virtual_machine']['cluster'] == 1
+      - test_four_idempotence['virtual_machine']['vcpus'] == 8.5
+      - test_four_idempotence['virtual_machine']['memory'] == 8
+      - test_four_idempotence['virtual_machine']['status'] == "planned"
+      - test_four_idempotence['virtual_machine']['role'] == 2
+      - test_four_idempotence['virtual_machine']['tags'][0] == 4
+      - test_four_idempotence['msg'] == "virtual_machine Test VM One already exists"
+
+- name: "VIRTUAL_MACHINE 5: Delete"
   netbox.netbox.netbox_virtual_machine:
     netbox_url: http://localhost:32768
     netbox_token: 0123456789abcdef0123456789abcdef01234567
     data:
       name: "Test VM One"
     state: absent
-  register: test_four
+  register: test_five
 
-- name: "VIRTUAL_MACHINE 4: ASSERT - Delete"
+- name: "VIRTUAL_MACHINE 5: ASSERT - Delete"
   assert:
     that:
-      - test_four is changed
-      - test_four['virtual_machine']['name'] == "Test VM One"
-      - test_four['virtual_machine']['cluster'] == 1
-      - test_four['virtual_machine']['vcpus'] == 8.5
-      - test_four['virtual_machine']['memory'] == 8
-      - test_four['virtual_machine']['status'] == "planned"
-      - test_four['virtual_machine']['role'] == 2
-      - test_four['virtual_machine']['tags'][0] == 4
-      - test_four['msg'] == "virtual_machine Test VM One deleted"
+      - test_five is changed
+      - test_five['virtual_machine']['name'] == "Test VM One"
+      - test_five['virtual_machine']['cluster'] == 1
+      - test_five['virtual_machine']['vcpus'] == 8.5
+      - test_five['virtual_machine']['memory'] == 8
+      - test_five['virtual_machine']['status'] == "planned"
+      - test_five['virtual_machine']['role'] == 2
+      - test_five['virtual_machine']['tags'][0] == 4
+      - test_five['msg'] == "virtual_machine Test VM One deleted"
diff --git a/tests/integration/targets/v3.2/tasks/netbox_virtual_machine.yml b/tests/integration/targets/v3.2/tasks/netbox_virtual_machine.yml
@@ -76,24 +76,53 @@
       - test_three['virtual_machine']['tags'][0] == 4
       - test_three['msg'] == "virtual_machine Test VM One updated"
 
-- name: "VIRTUAL_MACHINE 4: Delete"
+- name: "VIRTUAL_MACHINE 4: Test idempotence"
+  netbox.netbox.netbox_virtual_machine:
+    netbox_url: http://localhost:32768
+    netbox_token: 0123456789abcdef0123456789abcdef01234567
+    data:
+      name: "Test VM One"
+      cluster: "Test Cluster"
+      vcpus: 8.5
+      memory: 8
+      status: "Planned"
+      virtual_machine_role: "Test VM Role"
+      tags:
+        - "Schnozzberry"
+    state: present
+  register: test_four_idempotence
+
+- name: "VIRTUAL_MACHINE 4: ASSERT - Not changed"
+  assert:
+    that:
+      - test_four_idempotence is not changed
+      - test_four_idempotence['virtual_machine']['name'] == "Test VM One"
+      - test_four_idempotence['virtual_machine']['cluster'] == 1
+      - test_four_idempotence['virtual_machine']['vcpus'] == 8.5
+      - test_four_idempotence['virtual_machine']['memory'] == 8
+      - test_four_idempotence['virtual_machine']['status'] == "planned"
+      - test_four_idempotence['virtual_machine']['role'] == 2
+      - test_four_idempotence['virtual_machine']['tags'][0] == 4
+      - test_four_idempotence['msg'] == "virtual_machine Test VM One already exists"
+
+- name: "VIRTUAL_MACHINE 5: Delete"
   netbox.netbox.netbox_virtual_machine:
     netbox_url: http://localhost:32768
     netbox_token: 0123456789abcdef0123456789abcdef01234567
     data:
       name: "Test VM One"
     state: absent
-  register: test_four
+  register: test_five
 
-- name: "VIRTUAL_MACHINE 4: ASSERT - Delete"
+- name: "VIRTUAL_MACHINE 5: ASSERT - Delete"
   assert:
     that:
-      - test_four is changed
-      - test_four['virtual_machine']['name'] == "Test VM One"
-      - test_four['virtual_machine']['cluster'] == 1
-      - test_four['virtual_machine']['vcpus'] == 8.5
-      - test_four['virtual_machine']['memory'] == 8
-      - test_four['virtual_machine']['status'] == "planned"
-      - test_four['virtual_machine']['role'] == 2
-      - test_four['virtual_machine']['tags'][0] == 4
-      - test_four['msg'] == "virtual_machine Test VM One deleted"
+      - test_five is changed
+      - test_five['virtual_machine']['name'] == "Test VM One"
+      - test_five['virtual_machine']['cluster'] == 1
+      - test_five['virtual_machine']['vcpus'] == 8.5
+      - test_five['virtual_machine']['memory'] == 8
+      - test_five['virtual_machine']['status'] == "planned"
+      - test_five['virtual_machine']['role'] == 2
+      - test_five['virtual_machine']['tags'][0] == 4
+      - test_five['msg'] == "virtual_machine Test VM One deleted"
diff --git a/tests/integration/targets/v3.3/tasks/netbox_virtual_machine.yml b/tests/integration/targets/v3.3/tasks/netbox_virtual_machine.yml
@@ -76,24 +76,53 @@
       - test_three['virtual_machine']['tags'][0] == 4
       - test_three['msg'] == "virtual_machine Test VM One updated"
 
-- name: "VIRTUAL_MACHINE 4: Delete"
+- name: "VIRTUAL_MACHINE 4: Test idempotence"
+  netbox.netbox.netbox_virtual_machine:
+    netbox_url: http://localhost:32768
+    netbox_token: 0123456789abcdef0123456789abcdef01234567
+    data:
+      name: "Test VM One"
+      cluster: "Test Cluster"
+      vcpus: 8.5
+      memory: 8
+      status: "Planned"
+      virtual_machine_role: "Test VM Role"
+      tags:
+        - "Schnozzberry"
+    state: present
+  register: test_four_idempotence
+
+- name: "VIRTUAL_MACHINE 4: ASSERT - Not changed"
+  assert:
+    that:
+      - test_four_idempotence is not changed
+      - test_four_idempotence['virtual_machine']['name'] == "Test VM One"
+      - test_four_idempotence['virtual_machine']['cluster'] == 1
+      - test_four_idempotence['virtual_machine']['vcpus'] == 8.5
+      - test_four_idempotence['virtual_machine']['memory'] == 8
+      - test_four_idempotence['virtual_machine']['status'] == "planned"
+      - test_four_idempotence['virtual_machine']['role'] == 2
+      - test_four_idempotence['virtual_machine']['tags'][0] == 4
+      - test_four_idempotence['msg'] == "virtual_machine Test VM One already exists"
+
+- name: "VIRTUAL_MACHINE 5: Delete"
   netbox.netbox.netbox_virtual_machine:
     netbox_url: http://localhost:32768
     netbox_token: 0123456789abcdef0123456789abcdef01234567
     data:
       name: "Test VM One"
     state: absent
-  register: test_four
+  register: test_five
 
-- name: "VIRTUAL_MACHINE 4: ASSERT - Delete"
+- name: "VIRTUAL_MACHINE 5: ASSERT - Delete"
   assert:
     that:
-      - test_four is changed
-      - test_four['virtual_machine']['name'] == "Test VM One"
-      - test_four['virtual_machine']['cluster'] == 1
-      - test_four['virtual_machine']['vcpus'] == 8.5
-      - test_four['virtual_machine']['memory'] == 8
-      - test_four['virtual_machine']['status'] == "planned"
-      - test_four['virtual_machine']['role'] == 2
-      - test_four['virtual_machine']['tags'][0] == 4
-      - test_four['msg'] == "virtual_machine Test VM One deleted"
+      - test_five is changed
+      - test_five['virtual_machine']['name'] == "Test VM One"
+      - test_five['virtual_machine']['cluster'] == 1
+      - test_five['virtual_machine']['vcpus'] == 8.5
+      - test_five['virtual_machine']['memory'] == 8
+      - test_five['virtual_machine']['status'] == "planned"
+      - test_five['virtual_machine']['role'] == 2
+      - test_five['virtual_machine']['tags'][0] == 4
+      - test_five['msg'] == "virtual_machine Test VM One deleted"
