Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NetBox changes: Unit size for VirtualMachine.disk and VirtualDisk.size changed from 1 gigabyte to 1 megabyte. #413

Closed
lucafabbri365 opened this issue Sep 12, 2024 · 10 comments
Labels
awaiting reply Awaiting reply from issue owner
Milestone

Comments

@lucafabbri365
Copy link

lucafabbri365 commented Sep 12, 2024

Hello,
based on the recent changes in version 4.1.0 of NetBox regarding the unit size for VirtualMachine.disk and VirtualDisk.size (from 1 gigabyte to 1 megabyte), NetBox-sync reports an incorrect value in the virtual disk size of virtual machines.

@bb-Ricardo
Copy link
Owner

Thank you. for this hint, will add a fix.

@bb-Ricardo
Copy link
Owner

Hey,

I just pushed another commit to development branch to fix this issue with NetBox 4.1.0. Would you be able to try it out?

Thank you

@cguadall
Copy link

Hello,

I've tried the last commit to development and it works correctly. Thank you.

Reviewing the commit a doubt have raised. According to this netbox discussion, it seems that the preferred option to convert units is using SI (divide by 1000)

netbox-community/netbox#17484

The information provided by vmware uses the SI units (decimal) or are binary based?

@lucafabbri365
Copy link
Author

lucafabbri365 commented Sep 16, 2024

Hello @bb-Ricardo,
as written by @cguadall NetBox uses Base 10 (MB or GB) in place of Base 2 (MiB or GiB).

While adding a new virtual disk, the size is entered in MB, but it is then displayed in GB (by dividing by 1000).
Before you answered to this issue, I applied a workaround the code inside /module/sources/vmware/connection.py

FROM:

disk_size = grab(vm_device, "capacityInKB", fallback=0)
disk_size_in_gb = int(disk_size / 1024 / 1024)
if disk_size_in_gb < 1:
	vm_device_description.append(f"Size: {int(disk_size / 1024)} MB")
	disk_size_in_gb = 1

disk_data.append({
	"name": grab(vm_device, "deviceInfo.label"),
	"size": disk_size_in_gb,
	"description": " / ".join(vm_device_description)
})

TO:

disk_size = grab(vm_device, "capacityInKB", fallback=0)
disk_size_in_mb = int(disk_size / 1024 / 1024 * 1000)
disk_data.append({
	"name": grab(vm_device, "deviceInfo.label"),
	"size": disk_size_in_mb,
	"description": " / ".join(vm_device_description)
})

Merging the above with your modified version in development, it could be:

disk_size_in_kb = grab(vm_device, "capacityInKB", fallback=0)
if version.parse(self.inventory.netbox_api_version) < version.parse("4.1.0"):
	disk_size = int(disk_size_in_kb / 1024 / 1024)
	if disk_size < 1:
		vm_device_description.append(f"Size: {int(disk_size_in_kb / 1024)} MB")
		disk_size = 1
# since NetBox 4.1.0 disk size is represented in MB
else:
	disk_size = int(disk_size_in_kb / 1024 / 1024 * 1000)

(Maybe there is a more elegant way to do that).

@bb-Ricardo
Copy link
Owner

Hi,

I noticed this as well the next day. Just reading this: https://en.wikipedia.org/wiki/Byte#Multiple-byte_units makes you realise what happens if you led toner heads run your company. What a mess.

Will try to fix it the next few days.

@bb-Ricardo
Copy link
Owner

The issue #400 is pretty much the same problem in a different location.

Because the values of the API have not changed (at least with the issue #400). So I will combine these two and with the config option the decission is up to the user. What do you think?

VMWare reports base 2 based values.

@bb-Ricardo
Copy link
Owner

Hi @lucafabbri365 and @cguadall,

I just pushed a new commit to the development branch with newly added config option:

; In NetBox version 4.1.0 and newer the VM disk and RAM values are displayed in power of
; 10 instead of power of 2. If this values is set to true 4GB of RAM will be set to a
; value of 4000 megabyte. If set to false 4GB of RAM will be reported as 4096MB. The same
; behavior also applies for VM disk sizes.
;vm_disk_and_ram_in_decimal = True

Would you be able to try it out and see if this fixes the issue?

@bb-Ricardo bb-Ricardo added the awaiting reply Awaiting reply from issue owner label Sep 23, 2024
@bb-Ricardo bb-Ricardo added this to the 1.7.0 milestone Sep 23, 2024
@lucafabbri365
Copy link
Author

Hello @bb-Ricardo,
tested and worked like a charm !!!

Thank you guy !!!

May I continue using the version from development branch ?

@bb-Ricardo
Copy link
Owner

same here. You can use the dev branch. hope to merge it soon to main.

@cguadall
Copy link

Hello @bb-Ricardo,

Tested and worked perfectly.

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting reply Awaiting reply from issue owner
Projects
None yet
Development

No branches or pull requests

3 participants