bb-Ricardo / netbox-sync

Sync objects from VMware or redfish sources to NetBox
MIT License
279 stars 64 forks source link

Divide VM memory by 1.024 to accomodate 'metric' interpretation by Netbox #400

Open yaiqsa opened 2 months ago

yaiqsa commented 2 months ago

After the Netbox 4.0 upgrade, the web GUI started reporting VM memory in 'metric' units like GB, instead of 'binary' units like GiB, which it did prior to 4.0.

image

I opened an issue for this in the Netbox project, and was told that this is the way that its going to be.

However, Netbox-sync still sets the memory field in MiB (for a VM with 12 GiB RAM, netbox-sync sets the MB field to 12288, while Netbox 'expects' 12000).

Could you please reflect the changes that Netbox made? So the import divides the memory value by 1.024 somewhere in the process?

I think this is pretty a low-priority problem, but it would be nice if it could be fixed eventually 🌈

bb-Ricardo commented 2 months ago

Hi,

I checked and it worked fine with 4.0.1 but looks different in 4.0.6.

also could't easily find the issue in the change log of the versions in between.

The problem here is: the value only "changed" in the UI. If you also use the API then the value there remains the same.

Technically this would change the API output as well if we adjust the value.

yaiqsa commented 1 month ago

Technically this would change the API output as well if we adjust the value.

I understand what you're saying, however the Netbox maintainers are saying that the 'binary' display we used to have was incorrect. (Implying that the API should always have reported 'decimal' values). I wish that wasn't the case, because I think 'binary' representation is more relevant, but my opinion doesn't matter in this case.

So I think that in the eyes of Netbox, any application using this value from the API should always have interpreted it as MB, not MiB.

I can ask you the same thing I asked Jeremy; could you make it configurable? But I would understand it if you're hesitant to put time in a 'feature' you don't agree with 😅

bb-Ricardo commented 1 month ago

It's easy to make it configurable. Had this in mind to not break any existing API integration based on this project.

Will add a config option like vm_momery_in_mib. Or do you have a more fitting name?

stavr666 commented 1 month ago

Upgraded from 4.0.3 to 4.0.8 and got this unexpected behavior. Question to maintainers: Where did you find real cases, where RAM expected to be entered as MB/GB/TB instead of MiB/GiB/TiB? MS HV, VMware vSphere, LXC, QEMU, OpenStack, many cloud providers (~10 that I used) - all of them use 1024 base for memory.

Why change divisor now, instead of fixing obvious visual error (show correct MiB, GiB, TiB for memory)?

bb-Ricardo commented 1 month ago

Upgraded from 4.0.3 to 4.0.8 and got this unexpected behavior.

Question to maintainers: Where did you find real cases, where RAM expected to be entered as MB/GB/TB instead of MiB/GiB/TiB? MS HV, VMware vSphere, LXC, QEMU, OpenStack, many cloud providers (~10 that I used) - all of them use 1024 base for memory.

Why change divisor now, instead of fixing obvious visual error (show correct MiB, GiB, TiB for memory)?

Hi @stavr666,

You would need to direct your questions towarda the NetBox project. Preferably in the related issue https://github.com/netbox-community/netbox/issues/16770

Here we can only mitigate the change.

yaiqsa commented 1 month ago

@bb-Ricardo

vm_memory_in_mib might imply that it always shows the memory in MiB, not binary multiples in general (like KiB,GiB etc).

Looking at this wikipedia article I would go for vm_memory_in_binary_multiples? It's a bit long, but its clear at least 😅 .