netdevopsbr / netbox-proxbox

Netbox Plugin for integration between Proxmox and Netbox
Apache License 2.0
338 stars 50 forks source link

Proxbox plugin crashing when adding an extra network interface to generated devices #98

Closed RWL-Dittrich closed 1 year ago

RWL-Dittrich commented 1 year ago

When setting up the devices to include the idrac interfaces and virtual bridges the actual physical proxmox nodes have, the plugin started crashing.

Whenever I add an extra interface to one of the hosts that gets updated by proxbox the plugin fails with an error that relates to the MTU that's missing. When I add an MTU to the interface it gets another error.

Is adding extra network interfaces to the proxbox nodes unsupported or is this a bug?

So after adding another interface (idrac) and giving it an MTU I get the following error:

Mar 20 15:03:35 netbox-ubuntu gunicorn[90603]: [Proxbox - Netbox plugin | Update All]
Mar 20 15:03:35 netbox-ubuntu gunicorn[90603]: CLUSTER...
Mar 20 15:03:35 netbox-ubuntu gunicorn[90603]: [OK] CLUSTER created. -> PVE-app-cluster
Mar 20 15:03:35 netbox-ubuntu gunicorn[90603]: NODES...
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]: Internal Server Error: /plugins/proxbox/full_update/
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]: Traceback (most recent call last):
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:   File "/opt/netbox/venv/lib/python3.10/site-packages/django/core/handlers/exception.py", line 56, in inner
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:     response = get_response(request)
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:   File "/opt/netbox/venv/lib/python3.10/site-packages/django/core/handlers/base.py", line 197, in _get_response
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:     response = wrapped_callback(request, *callback_args, **callback_kwargs)
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:   File "/opt/netbox/venv/lib/python3.10/site-packages/django/views/generic/base.py", line 103, in view
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:     return self.dispatch(request, *args, **kwargs)
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:   File "/opt/netbox/venv/lib/python3.10/site-packages/django/contrib/auth/mixins.py", line 109, in dispatch
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:     return super().dispatch(request, *args, **kwargs)
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:   File "/opt/netbox/venv/lib/python3.10/site-packages/django/views/generic/base.py", line 142, in dispatch
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:     return handler(request, *args, **kwargs)
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:   File "/opt/netbox/netbox/netbox-proxbox/netbox_proxbox/views.py", line 93, in get
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:     "virtualmachines_table": VMUpdateResult(proxbox_api.update.all(remove_unused = True)["virtualmachines"]),
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:   File "/opt/netbox/netbox/netbox-proxbox/netbox_proxbox/proxbox_api/update.py", line 434, in all
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:     node_updated = nodes(proxmox_json = px_node_each, proxmox_cluster = proxmox_cluster)
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:   File "/opt/netbox/netbox/netbox-proxbox/netbox_proxbox/proxbox_api/update.py", line 392, in nodes
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:     full_update = node_full_update(netbox_node, proxmox_json, proxmox_cluster)
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:   File "/opt/netbox/netbox/netbox-proxbox/netbox_proxbox/proxbox_api/update.py", line 61, in node_full_update
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:     interfaces_updated = updates.node.interfaces(netbox_node, proxmox_json)
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:   File "/opt/netbox/netbox/netbox-proxbox/netbox_proxbox/proxbox_api/updates/node.py", line 219, in interfaces
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]:     ntb_iface = list(nb.dcim.interfaces.filter(device_id=netbox_node.id, name=iface['name']))
Mar 20 15:03:36 netbox-ubuntu gunicorn[90603]: TypeError: string indices must be integers
MrBE4R commented 1 year ago

Hello,

Whenever I add an extra interface to one of the hosts that gets updated by proxbox the plugin fails with an error that relates to the MTU that's missing. When I add an MTU to the interface it gets another error.

To be sure, it's when you add the interface in netbox ? I didn't test this in our setup as I didn't have fully imported the components of our nodes in netbox.

I think the problem is here : https://github.com/netdevopsbr/netbox-proxbox/blob/v0.0.5/netbox_proxbox/proxbox_api/updates/node.py#L104

We can go with something like this :

'mtu' : int(iface.mtu) if iface.mtu else 1500

So after adding another interface (idrac) and giving it an MTU I get the following error: [...]

This one is fairly simple, here : https://github.com/netdevopsbr/netbox-proxbox/blob/v0.0.5/netbox_proxbox/proxbox_api/updates/node.py#L219

ntb_iface = list(nb.dcim.interfaces.filter(device_id=netbox_node.id, name=iface['name']))

should be

ntb_iface = list(nb.dcim.interfaces.filter(device_id=netbox_node.id, name=iface))

BUT, we should check if the interface is a management only interface and skip the deletion.

RWL-Dittrich commented 1 year ago

Yes. Because the imported nodes are missing the management interface and the virtual bridges / bonds I add these myself. But when I add these interfaces manually and run the plugin again it crashes out with the logs I sent earlier

RWL-Dittrich commented 1 year ago

I just tried the changes and they seem to work! Though the interfaces I create manually do get deleted if they are not marked as management interfaces.

Would it be possible to manually add some non-management interfaces. Maybe by creating a custom field with a flag that marks the interfaces as not-managed by the netbox-proxbox plugin?

MrBE4R commented 1 year ago

Hum, this should do the trick :

Add a custom field as following Required values (must be equal)

And change the line

if not ntb_iface[0].mgmt_only:

to

if not ntb_iface[0].mgmt_only or not ntb_iface[0].custom_fields.get('proxmox_keep_interface', False):
RWL-Dittrich commented 1 year ago

Should I implement this myself and create a pull request or can you add that to your PR?

MrBE4R commented 1 year ago

I've added it to the PR but it's based on the develop branch.

RWL-Dittrich commented 1 year ago

Ah good I see now! I'll keep using that branch for now. When the changes get merged in I'll switch back to develop/master 😄

Thank you!

emersonfelipesp commented 1 year ago

Ah good I see now! I'll keep using that branch for now. When the changes get merged in I'll switch back to develop/master 😄

Thank you!

It just got merged!