ansible_modules icon indicating copy to clipboard operation
ansible_modules copied to clipboard

[Bug]: Swapped behaviour for vCPUs on VMs between pre and post 3.0.0

Open bvergnaud opened this issue 2 years ago • 1 comments

Ansible NetBox Collection version

v3.7.1

Ansible version

ansible [core 2.13.1]
  config file = /home/bvergnaud/git/scw/platform/vm-containers/iaas-playbooks/ansible.cfg
  configured module search path = ['/home/bvergnaud/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/bvergnaud/.local/share/virtualenvs/iaas-playbooks-3YYUb2kg/lib/python3.8/site-packages/ansible
  ansible collection location = /home/bvergnaud/git/scw/platform/vm-containers/iaas-playbooks/.collections
  executable location = /home/bvergnaud/.local/share/virtualenvs/iaas-playbooks-3YYUb2kg/bin/ansible
  python version = 3.8.13 (default, Apr  4 2022, 10:14:53) [GCC 11.2.0]
  jinja version = 3.1.2
  libyaml = True

NetBox version

v2.10.10

Python version

3.8

Steps to Reproduce

Running a simple netbox.netbox.netbox_virtual_machine task twice, with the vcpus parameter set:

- name: "Create/delete VM on NetBox"
  netbox.netbox.netbox_virtual_machine:
    netbox_url: "{{ netbox_url }}"
    netbox_token: "{{ netbox_token }}"
    data:
      name: test-vm
      vcpus: 1
    state: present
  delegate_to: localhost

Expected Behavior

The task should be idempotent and report status=ok with no changes.

Observed Behavior

On the second run, it produces a diff:

--- before
+++ after
@@ -1 +1 @@
-vcpus: 1
+vcpus: '1.00'

changed: [lab1dc2-inf-dev-hv16 -> localhost] => changed=true
  msg: virtual_machine test-vm updated
  virtual_machine:
    comments: ''
    config_context: {}
    created: '2022-09-06'
    custom_fields:
      vmid: null
    id: 9094
    last_updated: '2022-09-06T14:00:18.040069Z'
    local_context_data: null
    name: test-vm
    status: active
    url: https://netbox-staging.internal.scaleway.com/api/virtualization/virtual-machines/9094/
    vcpus: 1.0

Note:

  • Output filtered manually from testing with more parameters, in case anything looks odd.
  • The same behaviour holds true if we cast the value of vcpus as int or float.

bvergnaud avatar Sep 06 '22 16:09 bvergnaud

Hello 👋🏻

I already have an idea of where this might be coming from. In plugins/module_utils/netbox_utils.py L1258, we check the current version of netbox:

version_pre_30 = self._version_check_greater("3.0", self.version)

This is then re-used L1272-1273 to decide how to format the vcpus parameter:

if serialized_nb_obj.get("vcpus") and data.get("vcpus") and version_pre_30:
    updated_obj["vcpus"] = "{0:.2f}".format(data["vcpus"])

vcpus is a float if netbox >= 3.0.0, but an int if netbox < 3.0.0 (at least from the perspective of pynetbox, I'm unsure if those data types are the same in netbox itself):

>>> nb2_vm.vcpus
2
>>> nb3_vm.vcpus
2.0

# Same if serialized
>>> nb2_vm.serialize().get('vcpus')
2
>>> nb3_vm.serialize().get('vcpus')
2.0

Which means that the condition L1272 should say and not version_pre_30 instead. I haven't tested it yet, but I'm assuming that if >= v3.0.0 there is a similar diff but inverted, where the parameter is not formatted as a float when it should be.

If someone who knows the codebase better than me confirms that this makes some sense, I'll be happy to take a closer look and put a patch together. 🙂

bvergnaud avatar Sep 06 '22 16:09 bvergnaud