cockpit-machines icon indicating copy to clipboard operation
cockpit-machines copied to clipboard

Ability to add/edit/remove consoles

Open BrainwreckedTech opened this issue 2 years ago • 30 comments

Note: Current design is at https://github.com/cockpit-project/cockpit-machines/issues/553#issuecomment-1061581973

Also tracked in https://issues.redhat.com/browse/RHEL-17433 and https://issues.redhat.com/browse/RHEL-17434


Page: Virtual Machines -> [your_virtual_machine]

Allow configuration of VNC and SPICE remote viewing through the web interface instead of requiring manual XML configuration.

If SPICE viewing is disabled, disable relevent SPICE devices in the XML config file.

Mockup: mockup_remote_display

BrainwreckedTech avatar Feb 03 '22 12:02 BrainwreckedTech

Ah, this is a nice mockup.

However, we do have access to VNC and SPICE in-browser tat the top and you're supposed to be able to connect to it from there.

I set up a fresh CentOS-Stream-9, having Cockpit-Machines download the image and do an automated installation.

It's installing using a kickstart file in text mode and looks like this:

Screenshot 2022-02-03 at 17-33-41 Virtual Machines - garrett Bolt

When Anaconda is up and running, it's visible in VNC. Here's a crop:

Screenshot from 2022-02-03 17-41-02

If you switch to "Desktop viewer", it show instructions on how to access this from a dedicated SPICE or VNC viewer.

Screenshot 2022-02-03 at 17-34-11 Virtual Machines - garrett Bolt

(The layout of this was done a long time ago and ported across different versions of PatternFly and different layouts. It could use some polishing up, admittedly.)

We should probably re-visit this concept and make it a little more obvious. And we could make it editable too, with the featureset in your mockup. I think it probably makes sense to have it in the console area, but shown differently (and in a more obvious way than it currently is).

Would we need multiple connections for SPICE and VNC? Or is one each enough?

garrett avatar Feb 03 '22 16:02 garrett

Here's the desktop of CentOS Stream 9 in Cockpit Client (Cockpit's "Desktop" app):

Screenshot from 2022-02-03 17-46-59

And the same VM connected via SPICE using "Virtual Machine Viewer" (virt-viewer): Screenshot from 2022-02-03 17-50-48

But, yeah, thanks for opening this issue! We can make this more discoverable, and it should be editable as you suggest.

garrett avatar Feb 03 '22 16:02 garrett

I didn't miss the info portion. The Desktop Viewer part was actually really neat.

But it doesn't let you change anything.

In my case, I'm running libvirt VMs from a central home file server and would like to connect using a viewer on my desktop computer, and I'd like to not have to edit an XML config file to do it.

Screenshot_20220203_120155

BrainwreckedTech avatar Feb 03 '22 17:02 BrainwreckedTech

@BrainwreckedTech: Thanks for the quick reply!

I agree with you; you should be able to edit it without having to edit an XML.

I think it probably makes the most sense in the console area, as it's related and you wouldn't add multiples like the disks, network interface, etc.… The information is basically host, port, and user/password, so it's not too much information either. I think we can make it work.

(And if not there, perhaps in the overview?)

It's the end of the work week for me right now, but I have an idea and want to mock this up soon!

garrett avatar Feb 03 '22 17:02 garrett

The console area would be fine if SPICE and VNC are the only options. I just didn't want to assume that there weren't other options that the feature could be expanded to include.

I could also see people wanting to disable both options if a VM is considered live, only enabling a local (to the VM) VNC or SPICE option in emergency situations.

BrainwreckedTech avatar Feb 03 '22 17:02 BrainwreckedTech

I second this idea, and I think the mockup that BrainwreckedTech threw together is gorgeous. I too would like to be able to add, remove, and update specific remote access protocols from the web interface instead of using virsh edit every time I need to adjust something. I'm glad this issue was opened so recently, as I just started to use cockpit a few days ago (I am hosting a few VMs for some friends) and was looking for this exact feature request.

Xenophilicy avatar Feb 08 '22 05:02 Xenophilicy

I've been thinking things over, and between this request and my other one, I think my original mockup is the better way to go. While the page might get long, it's better to have an overview of all the options you can set rather than tucking options away in miscellaneous spots to save space.

With a web interface, people are more accepting of having to scroll. It's something that's expected when viewing things through a browser. Even if someone doesn't understand the internals of HTML, CSS, and JavaScript, and how layout control is loosey-goosey on the web, they get the overall feeling of it through all of the web sites they visit throughout their lifetime. So scrolling is no big deal.

Contrast this with desktop design, where precision is expected so we get endless debates on whether client-side decorations or the traditional title bar + menu bar + tool bar is the better use of space. :)

BrainwreckedTech avatar Feb 13 '22 16:02 BrainwreckedTech

@BrainwreckedTech: It's not a matter of scrolling. The remote display support is intertwined with the functionality of the console. It makes sense for them to be grouped together, not split apart to different parts of the page. Additionally, there would only be one SPICE and one VNC configuration for a VM, not multiple. (The other lists support 0 - any number of items, whereas SPICE/VNC just has 1 of each.)

Taking the above into consideration, I really think the way to go forward is to adjust the console area so it presents the information in a better way and let it be editable.

I haven't had time to properly mock this up. I've been busy on other things. But I would like to get to this soon.

I do appreciate your mockup and opening the issue. I think you're right that this should be configurable.

garrett avatar Feb 14 '22 15:02 garrett

Here's what I was thinking about having the remote options integrated into the console area:

VM console remote options

I don't have the edit modal dialog mocked up, but it would include both SPICE and VNC, probably both with a checkbox (so you can disable one or the other), with dropdowns to select the IP address (local only or one that can be accessed over a network) and a text entry (limited to numbers) as a port.

I'm currently unsure where the remote viewer button would go. Is it weird to have it in the popover? Otherwise, we could try to compress the line a bit more and have a button to launch remote or something like that.

Anyway, this mockup would certainly need more polishing; I wanted to make and share it ASAP to communicate my idea and so we have something more to talk about.

garrett avatar Feb 15 '22 17:02 garrett

That also "unhides" the options. :+1:

If Graphics/Text == VNC/Serial, I'd vote to stick with VNC/Serial.

What if "Remote" was a button, and the popup had modfied text from current layout?

Clicking "Remote" will download a .vv file and launch Remote Viewer. Search for "Remote Viewer" or virt-viewer in your software/package manager. (For Windows, download the MSI from virt-manager.org)

Maybe "Remote" could be renamed "Launch Viewer"?

BrainwreckedTech avatar Feb 18 '22 17:02 BrainwreckedTech

@BrainwreckedTech: Thanks! Those are great suggestions.

I've adjusted the console on the Virtual Machine details page:

VM console

And moved the remote viewer information in a dialog:

Remote viewer confirm modal (2)

And I think it might get a bit tedious to set up the handling and have to do multiple click throughs, so I added skip setting to bypass the dialog for the next time. But then that needs a way to undo the setting. It's a browser-based setting that would be "global" to all VMs. Right now, we don't have any settings for machines, and if we did, it would be at the main page listing all the VMs, not in the details page.

So, while this is a browser setting that applies to all VMs, I think it still makes sense to expose the toggle close to where you can set it. As a compromise, I've added it to the edit SPICE and VNC settings dialog (which is new here — it was just implied previously):

SPICE and VNC settings modal

It's wrong to have a global feature (that's browser-based) at this level, but it's also wrong (and probably even more wrong) to have a global setting out of sight, so "perfect is the enemy of good" and all that... and right now, I think this is the best approach, even if it's a little bit of a compromise.

garrett avatar Mar 08 '22 09:03 garrett

@BrainwreckedTech: Thanks! Those are great suggestions.

SPICE and VNC settings modal

It's wrong to have a global feature (that's browser-based) at this level, but it's also wrong (and probably even more wrong) to have a global setting out of sight, so "perfect is the enemy of good" and all that... and right now, I think this is the best approach, even if it's a little bit of a compromise.

Would it be possible to change those address input boxes to drop down boxes where a user could auto select any of the interfaces on the host?

For example, drop down lists all interfaces on the host dynamically like this:

  • virbr0
  • virbr1
  • lo
  • custom

Under custom, an input box shows up and the user can enter what ever IP they want. With the other options, the IP is pulled from the interfaces? Possibly mention somewhere that the firewall will need adjusting too.

Asleep-at-the-wheel avatar Aug 08 '22 03:08 Asleep-at-the-wheel

Cockpit Client needs the connection to work over SSH. Perhaps we should have the connection happen through SSH anyway, otherwise the ports won't matter, unless the firewall is bypassed.

So we'd need to either:

  • always use SSH
  • instruct the firewall to temporarily open a port
  • an option to switch (in some cases)

The simplest that should work virtually everywhere (that has SSH enabled at least, which is probably almost every machine that uses Cockpit) would be to just force the VNC and SPICE connections through SSH. With this being the case, the port selection doesn't make so much sense, right? And the connection would always be localhost once it's through SSH? This is all assuming that virt-viewer supports SSH in the vv file. (It does at the command line.)

Example virt-viewer connection over SSH:

virt-viewer --connect qemu+ssh://user@host:222/system guest

...but does big question is: the configuration file support this?

Meanwhile, our IP + port settings don't even work unless one (or more) is true:

  • the firewall is down
  • the firewall has a port poked through
  • someone is running Cockpit on the same machine as using VNC

...which means the desktop connection is currently almost entirely useless by default. And this is why I'm suggesting we use ssh instead (or at least by default).

While the mockups I posted above would mainly work for this, I think we'd probably want to retool them a bit more based on what's decided here.

garrett avatar Aug 08 '22 08:08 garrett

You are correct and sticking to SSH to provide an encrypted tunnel is always preferred. Users can however change the XML of the VM to bind VNC to an IP/port on the host and open a firewall port. This allows users to connect with any VNC client from anywhere.

Probably want to somehow incorporate this use case into your draft (which I suggested using drop downs to bind to IP of interface, and perhaps defaults to lo).

Asleep-at-the-wheel avatar Aug 09 '22 03:08 Asleep-at-the-wheel

I'm currently evaluating Cockpit Machines and one of the things I stumbled upon is the Spice integration. Launching the viewer with just one click is really something I miss.

In addition it would also be nice to launch the viewer with one click from the overview page (next to the Run/Shutdown buttons).

Regarding the "skip explanation dialog" proposal, that would not be a domain-related setting but a user-related setting (every user need to see the message once until he installed the Spice client). Perhaps a question mark with an explanation of the requirements when clicked would be an easier approach. Similar to the explanation of the different interface types of a network configuration.

I would have liked to test the SSH tunnel approach to compare its speed, but it doesn't seem to be supported by the Windows virt-viewer, which unfortunately is a must for the setup I'm evaluating.

In order to be able to use the viewer remotely via VPN, I had to enable the Spice compression. So my suggestion would be an option for it in the settings.

I'm also a bit surprised why the spice_listen option in /etc/libvirt/qemu.conf is copied to each domain? If it's a default, there shouldn't be a need to copy it.

1ras avatar Aug 09 '22 08:08 1ras

You are correct and sticking to SSH to provide an encrypted tunnel is always preferred.

Right. We should default to SSH. The gotcha is that it seems the remote viewer app doesn't support SSH in the config file?

@jelly added a bug about this upstream @ https://gitlab.com/virt-viewer/virt-viewer/-/issues/86

Users can however change the XML of the VM to bind VNC to an IP/port on the host and open a firewall port.

Editing an XML file and opening a firewall port for this really doesn't belong in Cockpit in such a manual way. (We're basically doing just part of that now, without any firewall manipulation or guidance. :frowning_face:)

Cockpit should make it easy to do, without any extra effort, and this includes opening a firewall port from the same place in Machines. We do have a firewall page for this, but it should be added from the Machines page, in this configuration area if cockpit-firewalld is installed. (Otherwise, it needs to provide information about opening a firewall port or possibly have an opt-in to use something like .)

Right now, we're "totally dropping the ball" on this feature, as the it really doesn't work anywhere without manual work, except on development machines.

I think adding a desktop viewer is an old feature, probably even added before we had firewall support, which would help to explain this.

Probably want to somehow incorporate this use case into your draft (which I suggested using drop downs to bind to IP of interface, and perhaps defaults to lo).

It's there already, as instruct the firewall to temporarily open a port and also implied in an option to switch (between SSH and the IP+port+firewall method), which is basically the IP + port of what we have now, with the addition of actually letting that port do something useful by Cockpit opening up the port in the firewall.

Since we already do have the IP and port UI and the new design relies on that for now, perhaps we should just finish it first, by opening up a port in the firewall. As this would automatically open up another means of access to the system (so it's actually useful from Cockpit), we'd need a warning that this will open up a firewall port.

And then, we would extend this by adding SSH and making it the default.

garrett avatar Aug 09 '22 08:08 garrett

Regarding the "skip explanation dialog" proposal, that would not be a domain-related setting but a user-related setting (every user need to see the message once until he installed the Spice client).

Yes, it would likely be in the browser LocalStorage, which means it's a setting stored in the browser... which means per-user, effectively.

In order to be able to use the viewer remotely via VPN, I had to enabled the Spice compression. So my suggestion would be an option for it in the settings.

We should probably have it enabled by default, unless there's a good reason not to? (I guess CPU overhead and latency might be of concern? But it's likely really negligible in practice on any modern machine.)

I'm also a bit surprised why the spice_listen option in /etc/libvirt/qemu.conf is copied to each domain? If it's a default, there shouldn't be a need to copy it.

This feature has been around for a while in its current state. There might be a reason for it, or it could be due to an old default. Or possibly some distributions we support might not have it enabled by default. We should reevaluate this. Thanks!

garrett avatar Aug 09 '22 08:08 garrett

Thanks again for all the feedback, everyone! Hopefully you'll still be happy to keep sharing your feedback as we work on this more.

I've added a ticket internally (to make this officially higher priority), where I outline my current plan:

  1. Improve web UI for VNC and serial connections
  2. Improve UI for local desktop-based access (using a .vv file)
  3. Add automatic firewall port opening for IP+port method (with a warning that this will open a port on the machine; a warning is needed as it increases the attack surface)
  4. Add the ability to connect over SSH (and make it default)
  5. Handle file transfers and launching in Cockpit Client

Some of these could be done out of order or in parallel. I'll need to adjust designs for the plan too (especially for SSH), but what I have above is probably still a good starting place.

(By-the-way, just to be clear: Despite this being also tracked with our internal tracking tool, we'll still work in the open here as always, and hope to get more feedback along the way too as we share our progress. Our internal tracking tool is secondary to GitHub's issues, but mainly used for planning larger & important items. In other words, I'm basically using the internal tool to try to prioritize this issue — and I do link to and refer to our conversation here. :wink:)

garrett avatar Aug 09 '22 09:08 garrett

From #virt on oftc I recieved an answer on what is the difference between remote-viewer and virt-viewer:

remote-viewer is a general purpose vnc/spice client where you must tell it the address of the server to connect to. virt-viewer > meanwhile is a specialization of remote-view, which takes a libvirt guest name, and connects to libvirt to auto-discover the > VNC/spice server address.

jelly avatar Aug 09 '22 17:08 jelly

The file handling part of Cockpit Client is being tracked in https://github.com/cockpit-project/cockpit/issues/17626.

As Cockpit Client connects through SSH (not using/needing the Cockpit port of 9090), it's all the more important for users of Cockpit Client that we make sure remoting VNC/SPICE works via SSH instead of setting a port and opening it up in the firewall. (Logic being that if someone doesn't want or need port 9090 open, they probably also wouldn't want a SPICE or VNC port open either.)

In other words: SSH is important; we should do this somehow. If the client doesn't support it via a launcher file yet, then we might need to show the command line to make it work.

(However, the command line would only work in Linux, not in Windows and macOS. WSL2 would be an exception, but that's still Linux... just in Windows. Same issue, really, and we shouldn't expect people to have to install and set up WSL2 enough to get remote-viewer / virt-viewer working.)

garrett avatar Aug 11 '22 07:08 garrett

Regarding the Spice compression: Currently when enabling Spice compression with virsh edit or virt-xml, it's still disabled by virt-install during installation. It works as intended after installation but not during installation.

It seems that virt-install uses a logic which explicitly disables Spice compression when accessed locally and with Cockpit every access is local.

Edit: The domain is created with "Create and edit", then edited and only then installed. One would think that virt-install then uses the settings of the domain, but this is not the case.

1ras avatar Aug 16 '22 16:08 1ras

There should be a password setting, which is necessary for remote access.

AnterCreeper avatar Oct 26 '22 14:10 AnterCreeper

I really like the mockups here @garrett ! I will check how easy it's to implement. This will need to be done upstream in the @partternfly/react-console component.

KKoukiou avatar Nov 29 '22 18:11 KKoukiou

@garrett for the serial and vncconsole there used to be a button to 'Send keys'. Also there used to be a 'Disconnect' button. Dropping this will probably will be unsatisfying some users. Can we fit this into the design?

Also the instructions on how to install the virt-viewer for the remote viewer are missing in this mockup. Was this deliberate change?

KKoukiou avatar Dec 09 '22 13:12 KKoukiou

@garrett for the serial and vncconsole there used to be a button to 'Send keys'. Also there used to be a 'Disconnect' button. Dropping this will probably will be unsatisfying some users. Can we fit this into the design?

Sure, good point.

Also the instructions on how to install the virt-viewer for the remote viewer are missing in this mockup. Was this deliberate change?

I thought they are in the mockup?

The first version was @ https://github.com/cockpit-project/cockpit-machines/issues/553#issuecomment-1040537199

The revised one moved it: https://github.com/cockpit-project/cockpit-machines/issues/553#issuecomment-1061581973:

And moved the remote viewer information in a dialog:

image

garrett avatar Dec 13 '22 13:12 garrett

@garrett for the serial and vncconsole there used to be a button to 'Send keys'. Also there used to be a 'Disconnect' button. Dropping this will probably will be unsatisfying some users. Can we fit this into the design?

This design is for the smaller version, BTW; the expanded version should have everything.

We could probably have everything in a kebab menu, like:

  • Send key
    • all the keys underneath as options
  • --- divider ---
  • Disconnection (as a dangerous link)

garrett avatar Dec 13 '22 13:12 garrett

Maybe setting the keyboard language as well?

renich avatar Apr 25 '23 20:04 renich

Fooling issue should be linked to this #73

madwax avatar Aug 13 '23 17:08 madwax

@garrett I have some questions wrt. the design above:

  • So you want to pull out "Desktop" from the Select dropdown, and replace the rest (Serial/VNC) with two buttons if there is more than one console available, and otherwise with nothing, and just show the one available console?
  • The "Launch viewer" button would replace the "Desktop" select dropdown. So it would show the "Remote viewer" info dialog, and "Launch viewer" then triggers the .vv download? and checking "Skip next time" would set a localstorage flag and immediately download the .vv next time?
  • The edit button is missing a way to delete consoles. This is by far the highest priority aspect of this bug (see the linked RHEL issues). We can add a :wastebasket: icon next to them. But then we also need a way to add them back I suppose? That starts to sound complicated then, but we can't "disable" a console -- there's no storage/API in libvirt to do that.

martinpitt avatar Jan 05 '24 11:01 martinpitt