Fix two idempotency issues when updating VMs.

[bvergnaud]

Related Issue

#823

New Behavior

Using netbox.netbox.netbox_virtual_machine to update the vCPU count of a VM in netbox is now idempotent from version 2.10 onward.
Using netbox.netbox.netbox_virtual_machine to update a VM in netbox that has custom_fields with no values is also idempotent from version 2.10 onward.

Plus a couple of pretty minor changes (that can be removed easily if undesired):

• Method _version_check_greater now returns a type consistent value (i.e. False instead of None by default).
• Fix for a typo in an error message.

Contrast to Current Behavior

In the current behaviour, I'd get a float value replacing my integer value on the vcpus field every time (netbox 2.10).
And if I had a custom_fields with value None, the module would systematically return a state: changed with an empty diff (netbox 2.10 and 3.2).

Discussion: Benefits and Drawbacks

To do that, I dropped the use of a formatted string and instead use the same types as pynetbox, i.e. integer / float, depending on the situation. The user can use either type and it'll work fine, so long as pynetbox and the targeted netbox are able to handle it. Otherwise, an error is raised, e.g. pynetbox.core.query.RequestError: The request failed with code 400 Bad Request: {'vcpus': ['Ensure that there are no more than 6 digits in total.']}

I manually tested on 2.10 and 3.2 instances, with different vcpus values (2, 2.0, 2.1, 2.10, 2.12), and I get a consistent, idempotent behaviour.

Additionally, there was a piece of code that would always generate diffs if the original or updated objects contained a custom field with a value of None.
I chose to replicate the existing behaviour on the updated object rather than delete the loop over the original object. But admittedly, I do not understand the purpose of this piece of code. If anyone has knowledge about this, I'd be happy to learn more.

Existing tests pass, and I've also added an idempotence test on the execution of the netbox_virtual_machine module.

Proposed Release Note Entry

Fix #823: Fix idempotence of netbox_virtual_machine module.

Double Check

• I have read the comments and followed the CONTRIBUTING.md.
• I have explained my PR according to the information in the comments or in a linked issue.
• My PR targets the devel branch.

[sc68cal]

I don't get why you added the @staticmethod decorator to _version_check_greater yet all the callers to the function are still via self._version_check_greater instead of calling the class.

I think that change should be dropped from this PR.

[bvergnaud]

I don't get why you added the @staticmethod decorator to _version_check_greater yet all the callers to the function are still via self._version_check_greater instead of calling the class.

I think that change should be dropped from this PR.

Calling the class instead of self would merely spawn a new instance of that class. Since we're already in the class, that would be quite unnecessary, since self is also an instance of the class. A static method does not HAVE to be called via a new instance of the class.

But granted, this is not necessary for this PR so if you don't feel comfortable merging it, I don't mind dropping the commit.
EDIT: Done, commit dropped

[bvergnaud]

Update:

• Tests pass, and I've added a new idempotence test for the virtual_machine module.
• Fixed another issue I didn't anticipate: if a VM has a custom_field with a value of None, it would systematically cause a status: changed, even with an empty diff. I'm not sure the way I fixed it is the best way. Here, there's a loop that removes such custom fields from the original, serialized object. I added the same behaviour on the updated object to make the comparison fair, and moved all that before the updated_obj gets updated to avoid dropping intentional changes. But I don't get the purpose of this behaviour. It could be cleaner to instead not do it at all, but I didn't want to risk silently breaking something.

As is, the PR is good to go, if there are no further comments / changes requests.