cockpit-project / cockpit-machines

Cockpit UI for virtual machines
GNU Lesser General Public License v2.1
295 stars 75 forks source link

Details page redesign #1027

Open garrett opened 1 year ago

garrett commented 1 year ago

I've gone through a few iterations and revisions, and he's what I have so far:

cockpit-machines details redesign

Goals:

  1. Group metadata (all metadata about the VM is now in the header at the top)
  2. Group editable data together
  3. Make editable data consistent (autostart was odd as a live effect toggle prior to this)
  4. Group read-only values together
  5. Provide more space for the Console (trimming down the space used for other things, making the VM go flush to the edges, etc.)
  6. Group CPU related items together
  7. Group Memory related items
  8. Prioritize common machine-level editable
  9. Add in a place for description (when it's added from the kebab menu)
  10. Find a place for vsock

This will require changing a few other things (such as CPU editing, which would require merging the CPU-related modals and autostart becoming a modal, which isn't super ideal, but it does fix the inconsistency).

garrett commented 1 year ago

So: What do you think of this so far?

garrett commented 1 year ago

Parts of this was originally discussed in https://github.com/cockpit-project/cockpit-machines/issues/971 (good reference for some background info), but the redesign encompasses more than just changing one card (as mentioned above).

skobyda commented 1 year ago

The one thing which struck me the most that vCPU and CPU is now combined into 1 row with only one editable options. Does that mean that both CPU mode and vCPU dialogs: Screenshot from 2023-04-13 13-28-53 Screenshot from 2023-04-13 13-28-46

Would be combined into one dialog, something like this? Screenshot from 2023-04-13 13-31-53

garrett commented 1 year ago

Would be combined into one dialog, something like this?

Yes, correct. Something like that.

That dialog should be improved (the number entries shouldn't be quite that wide, for example, and this layout implies that vCPU and Sockets are related and that vCPU maximum and Cores per socket are also related), but all of that stuff would be in it, just reorganized a bit.

skobyda commented 1 year ago

And secondly, I see that description has an edit button: Screenshot from 2023-04-13 13-33-36

But VM title (which is what "System container" is, I presume) doesn't have one. How that would be edited? Screenshot from 2023-04-13 13-33-41

garrett commented 1 year ago

The description would be added with the kebab menu. It and the name would be edited by the kebab menu too. I added in edit there, as everything below has edit too. (We might leave it out.)

System container... whoops I don't mean container. :sweat_smile: It's not the VM title; it's the connection type of system or user. The title would be a pretty form of the the name (which has limitations). If it's renamed with a different enough title (could downcase both and strip non-alphabetic characters to compare), then we could show "Title (name)". If the title and name match enough, then just show the title, I think?

skobyda commented 1 year ago

Right, so this looks good and I like it. I'm thinking how to split this out into separate items. My idea is:

  1. I would perhaps first start with combining CPU configuration into 1 dialog, as a preparation PR.
  2. Then the main PR which does everything else, apart from vsock and VM title and description
  3. VSOCK can be worked on both simultaneously, and perhaps even merged independently, right? since I see that vsock would fit both into the old and new design, there is no reason to wait with vsock after the redesign is done, right?
  4. Title and description seems a bit more blocked by the new redesign, but perhaps we could at least for now add the Title and description into the kebab menu, and away from the current Overview?
skobyda commented 1 year ago

Would be combined into one dialog, something like this?

Yes, correct. Something like that.

That dialog should be improved (the number entries shouldn't be quite that wide, for example, and this layout implies that vCPU and Sockets are related and that vCPU maximum and Cores per socket are also related), but all of that stuff would be in it, just reorganized a bit.

But vCPU maximum and sockets ARE related. vCPU maximum number has to be a multiplier/divider of socket number, see how it works:

Screencast from 2023-04-13 13-50-03.webm

skobyda commented 1 year ago

Okay, I can now see that CPU redesign would need some time, and since the new design will not work without all cpu details in a single modal, I'll start implemening cpu redesign now.

garrett commented 1 year ago

Yeah, your plan for splitting it apart into separate steps sounds correct.

As for the description, it could be added in the places as shown in the mockup (that is, at the top and not in the overview). It's kinda-sorta blocked on the changes, but not entirely, if it goes into the place as shown in the mockup.

And vsock would also be unstuck, as you pointed out. However, vsock still needs more UI work aside from this. (But we'll work on that one too.)

I wanted to figure out the place for everything before adding it all in.

Larse99 commented 1 year ago

Looks good! But I would rather see the VNC console as a button, that opens a popup. Just like Proxmox does. This takes too much space imo, which can be used for other stats and stuff :).

garrett commented 1 year ago

The idea is that you can see what's going on and even run commands inside the VM, whether it's text-based or graphical.

What other stats would you want to see that aren't already on the page? (I'm not sure if we would have access to get the additional stats or not... I'm just wondering what you consider useful information that we're not already showing.)

Larse99 commented 1 year ago

The idea is that you can see what's going on and even run commands inside the VM, whether it's text-based or graphical.

What other stats would you want to see that aren't already on the page? (I'm not sure if we would have access to get the additional stats or not... I'm just wondering what you consider useful information that we're not already showing.)

I get what you're saying, and thats true. I would like to see a little bit more information about networks, Disk IO, etc. Could give some useful information about the VM's currently running. At the moment I use virt-top which shows me the information, but could be cool if it was included in the GUI.

edit: I know i can see some networking stuff here: https://node01.company.nl:9090/network/vnet5 for example. But would be nice if it was included in the Virtual Machine overview :). SInce I first have to check which VM uses what vnet. Ofcourse, I could script this, but it's not ideal.

Britz commented 1 year ago

I really like that redesign, it looks way cleaner, but I have some serious concerns about it:

The Name:

The description would be added with the kebab menu. It and the name would be edited by the kebab menu too. I added in edit there, as everything below has edit too. (We might leave it out.)

System container... whoops I don't mean container. 😅 It's not the VM title; it's the connection type of system or user. The title would be a pretty form of the the name (which has limitations). If it's renamed with a different enough title (could downcase both and strip non-alphabetic characters to compare), then we could show "Title (name)". If the title and name match enough, then just show the title, I think?

If you use virsh and Cockpit-Machines together – which is required if you do a little more than the most basic tasks – having only the title would be a mess. I would prefer to always have "Title (name)", if a title is set and "name" otherwise, perhaps except they are fully identical, than the title could be skipped. In virsh and probably other tools you need to use the name, since that is the unique ID of the vm and therefore, in my opinion, it should never be hidden. If you interpret the Domains/VMs name name as an identifier, it would perhaps make sense to put as "ID" to the machine detail box to always have it.

Another idea could be to always show the name inside the breadcrumb menu above, thus additionally also better reflecting the URL path / query, which will most probably will contain the name and not the title.

The Console:

Looks good! But I would rather see the VNC console as a button, that opens a popup. Just like Proxmox does. This takes too much space imo, which can be used for other stats and stuff :).

I agree to that suggestion, at least when it comes to the VNC console. A VNC console in this small square is only capable of giving you a quick glimpse of what your VM is doing, since the text size is to small and the mouse input is mostly strangely offset. So you need to expand it or even better also open it in a new tab without the cockpit's sidebar navigation to do serious work with it and since the output depends on the guest's screen settings, it most of the time shows black bars either vertically or horizontally to match the aspect ratio, which is waste of space. Also your design is lacking the send-key buttons, which can be quite important, e.g., when trying to get to the Bios. A pop-up on the other hand — as used by most tools, e.g., the already mentioned ProxMox, oVirt, Guacamole and all BMCs I came across so far — can restrict its window size to fit the screen ratio of the guest, thus enforcing mouse alignment and has space to show additional option, that would clutter the clean design of the overview page and are only important for VNC usage, e.g. the send key buttons, copy paste option, ...

For the serial console, that's totally different story, it does not need mouse input and supports native website interactions, as scrolling, font-size adaption, dynamic text-overflow, copy & paste, .... which makes it fully usable inside that small square.

Thus I would suggest to use the serial console, if configured, as the default one and provide a pop-up button for the VNC console. That would additionally allow to use both at the same time, which can be handy when debugging broken VM configurations.

Another, not directly design related issue with the console is, that it always automatically connects to the VNC console, even if it should not, e.g. if --noautoconsole is set or if the main console is set to the serial one. In the latter case, that is extremely annoying, since the serial console only streams it's output and logs in the boot sequence can be easily missed, if you do not switch fast enough. The button order and the missing connect/disconnect button let's assume that this behavior gets even worse.

garrett commented 1 year ago

If you use virsh and Cockpit-Machines together – which is required if you do a little more than the most basic tasks – having only the title would be a mess. I would prefer to always have "Title (name)", if a title is set and "name" otherwise, perhaps except they are fully identical, than the title could be skipped. In virsh and probably other tools you need to use the name, since that is the unique ID of the vm and therefore, in my opinion, it should never be hidden. If you interpret the Domains/VMs name name as an identifier, it would perhaps make sense to put as "ID" to the machine detail box to always have it.

It's basically what I said here: https://github.com/cockpit-project/cockpit-machines/issues/1027#issuecomment-1506810276

But I guess you don't want it to match unless it's exact? That is, you don't want to be case insensitive and strip out characters?

What if we did the fuzzy matching still and always had the ID breadcrumbs? Like this:

image

(In this mockup revision, the ID would be your-vms-name-here at the top.)

Would this work for you?

Also your design is lacking the send-key buttons, which can be quite important

I was working on that, but went on PTO and then have had to move on to other things for now. I intended on getting back to it with higher priority when the implementation of what we have in the mockup is picked up.

This was the start; it's not done; it's basically adding "Send key" as a dropdown back in as-is:

Board(1)

(The menu that you see is a placeholder for the ... menu. It doesn't have any items except disconnect so far. It's a rough WIP.)

Another, not directly design related issue with the console is, that it always automatically connects to the VNC console, even if it should not, e.g. if --noautoconsole is set or if the main console is set to the serial one. In the latter case, that is extremely annoying, since the serial console only streams it's output and logs in the boot sequence can be easily missed, if you do not switch fast enough. The button order and the missing connect/disconnect button let's assume that this behavior gets even worse.

That's definitely a bug that should be filed.

It's probably 2 bugs:

  1. Respecting the noautoconsole
  2. Considering "integration" options in the create modal, similar to what we have in Cockpit-Podman. We may want to have some kind of port mapping in there too (also similar to Cockpit-Podman, even if done in different ways), probably to automatically set up some firewall rules if we can (like what we do with firewalld on the Firewall subpage, as part of the Network page).
Britz commented 1 year ago

But I guess you don't want it to match unless it's exact? That is, you don't want to be case insensitive and strip out characters?

Yeah, exactly, since it is the ID that you need to use in CLI which is case sensitive. Therefore, I think it is important to have it somewhere. Of course, you can, e.g., use virsh list to get the vm names and from that you can probably guess which title corresponds to it but just having it in the overview page would be way more handy and convenient.

What if we did the fuzzy matching still and always had the ID breadcrumbs? Like this:

(In this mockup revision, the ID would be your-vms-name-here at the top.)

Would this work for you?

That would be fine for me, it's actually the same solution I describes above as an alternative:

Another idea could be to always show the name inside the breadcrumb menu above, thus additionally also better reflecting the URL path / query, which will most probably will contain the name and not the title.

This was the start; it's not done; it's basically adding "Send key" as a dropdown back in as-is:

(The menu that you see is a placeholder for the ... menu. It doesn't have any items except disconnect so far. It's a rough WIP.)

Ok, nice. also having it directly there and not as a sub menu in the burger menu is great, since when, e.g., trying to reach the Bios or UEFI settings, timing is a thing which could get hard, if you need to search through multiple layers of submenus.

That's definitely a bug that should be filed.

It's probably 2 bugs:

  1. Respecting the noautoconsole

Agree, the 'noautoconsole' is probably kind of a bug. It is a virt-install parameter and I'm currently not sure how and if it is represented in the resulting domain xml. Perhaps Cockpit-Machines could allow to set a preferred console, which could be Text, VNC or None.

  1. Considering "integration" options in the create modal, similar to what we have in Cockpit-Podman. We may want to have some kind of port mapping in there too (also similar to Cockpit-Podman, even if done in different ways), probably to automatically set up some firewall rules if we can (like what we do with firewalld on the Firewall subpage, as part of the Network page).

I don't see a relation with the "integration" options of Cockpit-Podman. These are basically given by Disks, Network interfaces, Host devices and Shared directories. Having a similar port and network management would require to create network namespaces per VM / network, wich is currently neither supported by libvirt nor by NetworkManager, but I would love to see that feature. Merging libvirt networking with podman networking would be genius.

Since I just compared Cockpit-Podman and Cockpit-Machine to better understand your thoughts, I get another one which probably should be a feature request. Having a health check option similar to Cockpit-Podman would be great. However, this would most probably be more complex and requires other technologies, e.g. using QEMU Guest Agent.