Feature request: Associate virtual machines to devices

[TiagoTT]

It would be useful to link virtual machines to the hypervisor devices in Netbox.

The following needs to happen:

• associate virtual machines to the virtualization cluster (already supported)
• associate hypervisor devices to the virtualization cluster
• associate virtual machines to the hypervisor device

The netbox-agent could discover the virtual machines when executed on the hypervisor devices, by executing a command given in the configuration.

For each virtual machine found on the hypervisor, it could search for the virtual machine object in Netbox, create it if missing or update it if it already exists, and set the device of the virtual machines to match the hypervisor device.

For each virtual machine already associated with the hypervisor in Netbox, if could check if it is still found on the hypervisor device and remove the association in Netbox otherwise.

A proposed implementation was submitted in #327 .

[CllaudiaB]

Hello @TiagoTT
Thanks for your sugesstion, I have tested it and made some changes.
Could you please give me rebase access to your branch?

[TiagoTT]

Hello @CllaudiaB ,
Thank you for your reply.
Unfortunately I can't grant you access to our repository due to company policy, but I have rebased now.
If you need to make changes before merging, feel free to cherry-pick my commit into a new branch of yours and make the necessary changes there. Or let me know which changes I need to do.

[CllaudiaB]

Hello @TiagoTT,
I made some changes in this PR: #345
If you have time, could you also rebase it, please?
I want to prepare the new release for next week
Thank you :)

[TiagoTT]

Hello @CllaudiaB ,

Thank you for your time!
I fixed some bugs after my PR was open and amended the commit and pushed with force to keep it all in one commit.
And it seems that you started testing/fixing the first version. I am sorry to not having properly tested before opening the PR, and cause you unnecessary work.
I am trying to apply your changes on top of the most current version of my branch (which I already rebased on master branch).

If I understand correctly, the new parse_output function expects the output of virsh list, which includes a header and for each VM it starts with the libvirt domain ID, then has the name of the VM and finally the state of the VM. In our deployment we are using virsh list --name to list the running VMs, which doesn't contain header nor IDs nor state, just the VM names (one per line). And I imagined that the user would figure out the command to use on any other virtualization system that would output the VM names separated by whitespace (newlines, tabs, spaces... the separators that split() considers by default).
Should we go with the more generic approach used by other settings with driver and regex to facilitate the support of other virtualization platforms? If we use the regexp you have for the virsh list output, it will be harder for users of other virtualization platforms to use this feature, as they would have to figure out how to list their VMs and have a number in front of each VM name on each line.
This change was not ported to my PR yet.

The commit in which you switched from get to filter to find the virtual machines associated with the hypervisor, makes sense in the early version of my code, but in the latest version, I have changed that logic a bit to handle the creation of VMs when they are not found. To list the VMs already associated to the hypervisor I also switched to filter, but I keep using get to get the record of individual VMs (which may not exist yet or may still be associated with another hypervisor).

In have ported the use of shlex into my branch:

• 0e42da3

And removed the duplicated definition of get_netbox_cluster() from server.py:

• 33cf48e

The other typos that I had in the earlier version have been fixed.

[CllaudiaB]

Hello @TiagoTT,

Thank you as well for your time.
The version I proposed has been tested on a Proxmox hypervisor.
In the configuration file, I pass the command qm list to list_guests_cmd.
This command lists the VMs on the hypervisor.

Example:

VMID    NAME        STATUS
100     vm1         running  
101     vm2         stopped  
102     vm3         running 

I use the parse_output function to retrieve the VMs names. :)

I need to check for other types of hypervisors to see if there are commands that list the name directly.
To make sure that what we propose is compatible with the greatest number of hypervisors.

[CllaudiaB]

Hello @TiagoTT ,

Can you please retrieve these commits into your branch?

• d2eef97
• 3604531
• 1945c91

Thank you

[TiagoTT]

Hello @CllaudiaB,

Happy New Year!

I have ported your changes and our branches are now equal. I have tested and I am happy with the result.

While I tested the current version successfully, I noticed that the changes introduced to the get_virtual_guests method cause the command given to the virtual.list_guests_cmd option to no longer support shell pipes. Which works fine for me with --virtual.list_guests_cmd="/usr/bin/virsh list --name", as libvirt command line tools can output just the clean VM names but may not work for other virtualization platforms, as you had already observed with Proxmox.

Originally I had imagined that whatever the format of the output of the VM list command on the hypervisor, the user could always | grep or | sed or | awk to produce a clean list of VM names.

If we keep the new and more secure and robust command execution mechanism, we will need to add a parameter so that the user can optionally configure a regular expression to apply to the output of the command, as it is no longer possible to use any other shell commands to format the text.

What do you think?
I was looking at the code of driver.cmd but it only returns a single value, not a list.
Should we write a new driver to support multi line output and return an array filtering each element by an optional regular expression?
Or should we just implement this in the hypervisor.py module?

Also, while trying to test on some of our servers I was faced with installation problems, which I think are caused by the transition from setup.py to pyproject.toml, for which I did a quick fix and will open a separate issue and PR.

[TiagoTT]

I have implemented and tested a simple solution for the configurable parsing of the output of the command to list the virtualization guests, but did not include it in the branch of the current PR.
You can see it here: https://github.com/brandwatch/netbox-agent/pull/1/files
If you like it, I can include it in the branch of the current PR.

[CllaudiaB]

Hello @TiagoTT,

Thank you, Happy New Year!

About the VM names, we'll go with your suggestion. It will be up to the user to create a script to produce a clean list of VM names. In my case, for Proxmox, I created a script using awk to get only the VM names.
Thank you very much for your feedback. As for the transition from setup.py to pyproject.toml, I will take a look on Monday :)