From d74dafec9050863406c0a8ef717993ee57a9999f Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Thu, 30 Jan 2025 15:04:43 +0200 Subject: [PATCH] consoles: Allow adding and editing VNC - The "Console" card is always present and let's people manage VNC server settings while the machine is off. - The "VNC console" tab is always present when the machine is running and let's people add VNC. - There is no way yet to change VNC server settings for a running machine since there is no good place for the button that would open the dialog. --- src/components/common/needsShutdown.jsx | 24 ++++++ src/components/vm/consoles/consoles.jsx | 31 ++++--- src/components/vm/consoles/vnc.jsx | 103 ++++++++++++++++++++++-- src/components/vm/consoles/vncBody.jsx | 2 +- src/components/vm/vmDetailsPage.jsx | 34 ++++---- test/check-machines-consoles | 99 +++++++++++++++++++++++ test/reference | 2 +- 7 files changed, 251 insertions(+), 44 deletions(-) diff --git a/src/components/common/needsShutdown.jsx b/src/components/common/needsShutdown.jsx index a9116cab0..0ea7eda57 100644 --- a/src/components/common/needsShutdown.jsx +++ b/src/components/common/needsShutdown.jsx @@ -85,6 +85,26 @@ export function needsShutdownSpice(vm) { return vm.hasSpice !== vm.inactiveXML.hasSpice; } +export function needsShutdownVnc(vm) { + function find_vnc(v) { + return v.displays && v.displays.find(d => d.type == "vnc"); + } + + const active_vnc = find_vnc(vm); + const inactive_vnc = find_vnc(vm.inactiveXML); + + if (inactive_vnc) { + if (!active_vnc) + return true; + if (inactive_vnc.port != -1 && active_vnc.port != inactive_vnc.port) + return true; + if (active_vnc.password != inactive_vnc.password) + return true; + } + + return false; +} + export function getDevicesRequiringShutdown(vm) { if (!vm.persistent) return []; @@ -125,6 +145,10 @@ export function getDevicesRequiringShutdown(vm) { if (needsShutdownSpice(vm)) devices.push(_("SPICE")); + // VNC + if (needsShutdownVnc(vm)) + devices.push(_("VNC")); + // TPM if (needsShutdownTpm(vm)) devices.push(_("TPM")); diff --git a/src/components/vm/consoles/consoles.jsx b/src/components/vm/consoles/consoles.jsx index 1827c4bb1..c09c7e8b4 100644 --- a/src/components/vm/consoles/consoles.jsx +++ b/src/components/vm/consoles/consoles.jsx @@ -22,8 +22,9 @@ import cockpit from 'cockpit'; import { AccessConsoles } from "@patternfly/react-console"; import SerialConsole from './serialConsole.jsx'; -import Vnc from './vnc.jsx'; +import Vnc, { VncState } from './vnc.jsx'; import DesktopConsole from './desktopConsole.jsx'; + import { domainCanConsole, domainDesktopConsole, @@ -34,14 +35,6 @@ import './consoles.css'; const _ = cockpit.gettext; -const VmNotRunning = () => { - return ( -
- {_("Please start the virtual machine to access its console.")} -
- ); -}; - class Consoles extends React.Component { constructor (props) { super(props); @@ -81,8 +74,9 @@ class Consoles extends React.Component { return 'SerialConsole'; } - // no console defined - return null; + // no console defined, but the VncConsole is always there and + // will instruct people how to enable it for real. + return 'VncConsole'; } onDesktopConsoleDownload (type) { @@ -114,9 +108,14 @@ class Consoles extends React.Component { const { serial } = this.state; const spice = vm.displays && vm.displays.find(display => display.type == 'spice'); const vnc = vm.displays && vm.displays.find(display => display.type == 'vnc'); + const inactive_vnc = vm.inactiveXML.displays && vm.inactiveXML.displays.find(display => display.type == 'vnc'); if (!domainCanConsole || !domainCanConsole(vm.state)) { - return (); + return ( +
+ +
+ ); } const onDesktopConsole = () => { // prefer spice over vnc @@ -134,14 +133,12 @@ class Consoles extends React.Component { connectionName={vm.connectionName} vmName={vm.name} spawnArgs={domainSerialConsoleCommand({ vm, alias: pty.alias })} />))} - {vnc && } + isExpanded={isExpanded} /> {(vnc || spice) && { + const Dialogs = useDialogs(); + + function add_vnc() { + Dialogs.show(); + } + + function edit_vnc() { + Dialogs.show(); + } + + if (vm.state == "running" && !vnc) { + return ( + + + {_("Graphical support not enabled.")} + + + + + + ); + } + + let vnc_info; + let vnc_action; + + if (!vnc) { + vnc_info = _("not supported"); + vnc_action = ( + + ); + } else { + if (vnc.port == -1) + vnc_info = _("VNC, dynamic port"); + else + vnc_info = cockpit.format(_("VNC, port $0"), vnc.port); + + vnc_action = ( + + ); + } + + return ( + <> +

+ { + vm.state == "running" + ? _("Shut down and restart the virtual machine to access the graphical console.") + : _("Please start the virtual machine to access its console.") + } +

+
+
+ + + {_("Graphical console:")} {vnc_info} + + + {vnc_action} + + +
+ + ); +}; + class Vnc extends React.Component { constructor(props) { super(props); @@ -115,9 +195,18 @@ class Vnc extends React.Component { } render() { - const { consoleDetail, connectionName, vmName, vmId, onAddErrorNotification, isExpanded } = this.props; + const { consoleDetail, inactiveConsoleDetail, vm, onAddErrorNotification, isExpanded } = this.props; const { path, isActionOpen } = this.state; - if (!consoleDetail || !path) { + + if (!consoleDetail) { + return ( +
+ +
+ ); + } + + if (!path) { // postpone rendering until consoleDetail is known and channel ready return null; } @@ -129,11 +218,11 @@ class Vnc extends React.Component { id={cockpit.format("ctrl-alt-$0", keyName)} key={cockpit.format("ctrl-alt-$0", keyName)} onClick={() => { - return domainSendKey({ connectionName, id: vmId, keyCodes: [Enum.KEY_LEFTCTRL, Enum.KEY_LEFTALT, Enum[cockpit.format("KEY_$0", keyName.toUpperCase())]] }) + return domainSendKey({ connectionName: vm.connectionName, id: vm.id, keyCodes: [Enum.KEY_LEFTCTRL, Enum.KEY_LEFTALT, Enum[cockpit.format("KEY_$0", keyName.toUpperCase())]] }) .catch(ex => onAddErrorNotification({ - text: cockpit.format(_("Failed to send key Ctrl+Alt+$0 to VM $1"), keyName, vmName), + text: cockpit.format(_("Failed to send key Ctrl+Alt+$0 to VM $1"), keyName, vm.name), detail: ex.message, - resourceId: vmId, + resourceId: vm.id, })); }}> {cockpit.format(_("Ctrl+Alt+$0"), keyName)} @@ -147,10 +236,10 @@ class Vnc extends React.Component { ]; const additionalButtons = [ ( this.setState({ isActionOpen: !isActionOpen })}> {_("Send key")} diff --git a/src/components/vm/consoles/vncBody.jsx b/src/components/vm/consoles/vncBody.jsx index 33731ffe4..1c9494189 100644 --- a/src/components/vm/consoles/vncBody.jsx +++ b/src/components/vm/consoles/vncBody.jsx @@ -21,7 +21,7 @@ import React, { useState } from 'react'; import PropTypes from 'prop-types'; import { FormGroup, FormHelperText, HelperText, HelperTextItem, - InputGroup, TextInput, Button, Checkbox, + InputGroup, TextInput, Button, Checkbox } from "@patternfly/react-core"; import { EyeIcon, EyeSlashIcon } from "@patternfly/react-icons"; diff --git a/src/components/vm/vmDetailsPage.jsx b/src/components/vm/vmDetailsPage.jsx index 98c38a335..a8e36465a 100644 --- a/src/components/vm/vmDetailsPage.jsx +++ b/src/components/vm/vmDetailsPage.jsx @@ -132,24 +132,22 @@ export const VmDetailsPage = ({ title: _("Usage"), body: , }, - ...(vm.displays.length - ? [{ - id: `${vmId(vm.name)}-consoles`, - className: "consoles-card", - title: _("Console"), - actions: vm.state != "shut off" - ? - : null, - body: , - }] - : []), + { + id: `${vmId(vm.name)}-consoles`, + className: "consoles-card", + title: _("Console"), + actions: vm.state != "shut off" + ? + : null, + body: , + }, { id: `${vmId(vm.name)}-disks`, className: "disks-card", diff --git a/test/check-machines-consoles b/test/check-machines-consoles index 3b8daf778..17b848515 100755 --- a/test/check-machines-consoles +++ b/test/check-machines-consoles @@ -19,6 +19,7 @@ import os import time +import xml.etree.ElementTree as ET import machineslib import testlib @@ -324,6 +325,104 @@ fullscreen=0 self.waitViewerDownload("vnc", my_ip) + def testAddEditVNC(self): + b = self.browser + + # Create a machine without any consoles + + name = "subVmTest1" + self.createVm(name) + + self.login_and_go("/machines") + self.waitPageInit() + self.waitVmRow(name) + self.goToVmPage(name) + + # "Console" card shows empty state + + b.wait_in_text(f"#vm-{name}-consoles .pf-v5-c-empty-state", "Graphical support not enabled.") + b.assert_pixels(f"#vm-{name}-consoles", "no-vnc") + + b.click(f"#vm-{name}-consoles .pf-v5-c-empty-state button:contains(Add VNC)") + + b.wait_visible("#add-vnc-dialog") + b.set_checked("#add-vnc-autoport", val=True) + b.set_input_text("#add-vnc-port", "5000") + b.click("#add-vnc-add") + b.wait_visible("#add-vnc-dialog .pf-m-error:contains('Port must be 5900 or larger.')") + b.assert_pixels("#add-vnc-dialog", "add") + b.set_input_text("#add-vnc-port", "100000000000") # for testing failed libvirt calls + b.set_input_text("#add-vnc-password", "foobar") + b.wait_attr("#add-vnc-password", "type", "password") + b.click("#add-vnc-dialog .pf-v5-c-input-group button") + b.wait_attr("#add-vnc-password", "type", "text") + b.click("#add-vnc-add") + b.wait_in_text("#add-vnc-dialog", "VNC device settings could not be saved") + b.wait_in_text("#add-vnc-dialog", "cannot parse vnc port 100000000000") + b.set_input_text("#add-vnc-port", "5901") + b.click("#add-vnc-add") + b.wait_not_present("#add-vnc-dialog") + + b.wait_in_text(f"#vm-{name}-consoles .pf-v5-c-console", "Shut down and restart") + b.wait_in_text(f"#vm-{name}-consoles .pf-v5-c-console", "Graphical console: VNC, port 5901") + b.wait_visible(f"#vm-{name}-needs-shutdown") + b.assert_pixels(f"#vm-{name}-consoles", "needs-shutdown") + + root = ET.fromstring(self.machine.execute(f"virsh dumpxml --inactive --security-info {name}")) + graphics = root.find('devices').findall('graphics') + self.assertEqual(len(graphics), 1) + self.assertEqual(graphics[0].get('port'), "5901") + self.assertEqual(graphics[0].get('passwd'), "foobar") + + b.click(f"#vm-{name}-consoles .pf-v5-c-console button:contains(Edit)") + b.wait_visible("#edit-vnc-dialog") + b.wait_val("#edit-vnc-port", "5901") + b.wait_val("#edit-vnc-password", "foobar") + b.assert_pixels("#edit-vnc-dialog", "edit") + b.set_input_text("#edit-vnc-port", "100000000000") # for testing failed libvirt calls + b.click("#edit-vnc-save") + b.wait_in_text("#edit-vnc-dialog", "VNC settings could not be saved") + b.wait_in_text("#edit-vnc-dialog", "cannot parse vnc port 100000000000") + b.set_checked("#edit-vnc-autoport", val=False) + b.click("#edit-vnc-save") + b.wait_not_present("#edit-vnc-dialog") + + b.wait_in_text(f"#vm-{name}-consoles .pf-v5-c-console", "Graphical console: VNC, dynamic port") + + root = ET.fromstring(self.machine.execute(f"virsh dumpxml --inactive --security-info {name}")) + graphics = root.find('devices').findall('graphics') + self.assertEqual(len(graphics), 1) + self.assertEqual(graphics[0].get('port'), "-1") + self.assertEqual(graphics[0].get('passwd'), "foobar") + + # Shut down machine + + self.performAction("subVmTest1", "forceOff") + b.wait_in_text("#vm-not-running-message", "Graphical console: VNC, dynamic port") + b.assert_pixels(f"#vm-{name}-consoles", "shutoff") + + # Remove VNC from the outside and do the whole dance again + + self.machine.execute(f"virt-xml --remove-device --graphics vnc {name}") + b.wait_in_text("#vm-not-running-message", "Graphical console: not supported") + + b.click("#vm-not-running-message button:contains(Add VNC)") + b.wait_visible("#add-vnc-dialog") + b.click("#add-vnc-add") + b.wait_not_present("#add-vnc-dialog") + b.wait_in_text("#vm-not-running-message", "Graphical console: VNC, dynamic port") + + b.click("#vm-not-running-message button:contains(Edit)") + b.wait_visible("#edit-vnc-dialog") + b.set_checked("#edit-vnc-autoport", val=True) + b.set_input_text("#edit-vnc-port", "5000") + b.click("#edit-vnc-save") + b.wait_visible("#edit-vnc-dialog .pf-m-error:contains('Port must be 5900 or larger.')") + b.set_input_text("#edit-vnc-port", "5900") + b.click("#edit-vnc-save") + b.wait_not_present("#edit-vnc-dialog") + b.wait_in_text("#vm-not-running-message", "Graphical console: VNC, port 5900") + if __name__ == '__main__': testlib.test_main() diff --git a/test/reference b/test/reference index fef884db2..405dc8e13 160000 --- a/test/reference +++ b/test/reference @@ -1 +1 @@ -Subproject commit fef884db2f402038be5b2f37e0df9d45a7aca4c2 +Subproject commit 405dc8e13d990ab893659e4ed12285c40f3304a1