ajnart / homarr

Customizable browser's home page to interact with your homeserver's Docker containers (e.g. Sonarr/Radarr)
https://homarr.dev
MIT License
5.52k stars 255 forks source link

Add Proxmox integration/widget #1903

Closed dslatt closed 3 months ago

dslatt commented 4 months ago

Thank you for contributing to Homarr! So that your Pull Request can be handled effectively, please populate the following fields (delete sections that are not applicable)

Category

Feature

Overview

Comments

Issue Number (if applicable)

Related issue: #889

Screenshot (if applicable)

Basic View Screenshot from 2024-02-15 20-43-40

Integration Screenshot from 2024-02-15 20-44-16

Settings Screenshot from 2024-02-15 20-44-59 Screenshot from 2024-02-15 20-45-08 Screenshot from 2024-02-15 20-45-19

Filter by node name (to not show the entire cluster if you want; by default its the whole thing) Screenshot from 2024-02-15 20-45-38 Screenshot from 2024-02-15 20-45-43 Screenshot from 2024-02-15 20-46-02

Lists of 'items' in the cluster. Nodes, VMs, LXCs, and storage Screenshot from 2024-02-15 20-46-16 Screenshot from 2024-02-15 20-46-23 Screenshot from 2024-02-15 20-46-29 Screenshot from 2024-02-15 20-46-36

On-click detail (node) Screenshot from 2024-02-15 20-46-49

On-click detail (VM) Screenshot from 2024-02-15 20-46-59

On-click detail (LXC) Screenshot from 2024-02-15 20-47-11 Screenshot from 2024-02-15 20-47-16 Screenshot from 2024-02-15 20-47-21

On-click detail (storage) Screenshot from 2024-02-15 20-47-30 Screenshot from 2024-02-15 20-47-37 Screenshot from 2024-02-15 20-47-41

SeDemal commented 4 months ago

I'm a bit torn on that one, this looks like something that could just expend on another PR #1879. But I think for now we can just put them side by side and combine them later on. Generally that would make a system health monitoring widget, which the one for OMV has already been renamed to.

I'll take a look at the code later on. Just leaving that first thought here.

dslatt commented 4 months ago

I get where you're coming from with the OMV widget, they do have some information overlap after all. I could see merging the dash. one with that, at least for just showing basic system info but I assume you would lose the graphing in that case.

For proxmox, you could probably use the OMV widget ui to show cpu/ram in one row, nodes/vms/lxcs in another, and storage in the final row. I think the progress circle icons would need some labels in that case though. The few challenges I see would be:

Right course depends on what you all want I guess. I'm sure its possible to merge and make it work, it just feels complicated because all the extra proxmox stuff would have to live alongside the simpler data from the 'system' widget .

Open to any ideas really.

SeDemal commented 4 months ago

Alright, since the OMV widget was there before and was already intended for dashdot in the future, I was wondering if you could integrate to that widget instead? You could just add all your elements into that widget as is and make them appear only when the linked app is proxmox, meaning you wouldn't loose any of the elements you integrated. It makes it a bit confusing in the options because of how the system is right now but since we're changing the system in the future, we will have a way to hide those options later. Any elements that extend further than the available space can simply be handled in a scrollview.

dslatt commented 4 months ago

still have some refinement to do, but basic merge is done

SeDemal commented 4 months ago

I saw your comment on the OMV integration PR, you're right about making options toggleable. Like the current dashdot widget where each and every stats is toggleable. Tell us when you think you're done, eager to check it out. And thanks again for your efforts.

dslatt commented 4 months ago

Ready to be checked whenever

Merged my proxmox widget into the system health one. If both integrations are present, they are split into separate tabs. If only one is used, then no tabs are used.

Only thing I changed in the existing omv widget was to make the cputemp rpc optional; it relies on a plugin not installed by default and was causing errors.

Screenshot from 2024-03-05 14-36-09 Screenshot from 2024-03-05 14-36-19 Screenshot from 2024-03-05 14-40-34

manuel-rw commented 3 months ago

@dslatt can you fix the build errors before I merge this?

dslatt commented 3 months ago

i'll add some documentation soon

manuel-rw commented 3 months ago

PR still has merge conflicts. Can you resolve them? Planning to merge this soon

dslatt commented 3 months ago

will do, was just waiting on final omv stuff to merge