From de49cdaf9fa3295789fee3ff030e3e9034bc2880 Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Fri, 7 Feb 2025 11:39:49 +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.css | 4 + src/components/vm/consoles/consoles.jsx | 108 ++++++++++-- src/components/vm/consoles/vncAddEdit.jsx | 194 ++++++++++++++++++++++ src/components/vm/vmDetailsPage.jsx | 34 ++-- src/libvirtApi/domain.js | 26 +++ test/check-machines-consoles | 116 +++++++++++++ 7 files changed, 473 insertions(+), 33 deletions(-) create mode 100644 src/components/vm/consoles/vncAddEdit.jsx 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.css b/src/components/vm/consoles/consoles.css index 53ac629af..ec4312605 100644 --- a/src/components/vm/consoles/consoles.css +++ b/src/components/vm/consoles/consoles.css @@ -28,3 +28,7 @@ #pf-v5-c-console__send-shortcut { display: none; } + +.consoles-card .pf-v5-c-empty-state__icon { + color: var(--pf-v5-global--custom-color--200); +} diff --git a/src/components/vm/consoles/consoles.jsx b/src/components/vm/consoles/consoles.jsx index 1827c4bb1..11210fb11 100644 --- a/src/components/vm/consoles/consoles.jsx +++ b/src/components/vm/consoles/consoles.jsx @@ -20,10 +20,19 @@ import React from 'react'; import PropTypes from 'prop-types'; import cockpit from 'cockpit'; import { AccessConsoles } from "@patternfly/react-console"; +import { Button } from "@patternfly/react-core/dist/esm/components/Button"; +import { + EmptyState, EmptyStateHeader, EmptyStateBody, EmptyStateFooter, EmptyStateActions, EmptyStateIcon +} from "@patternfly/react-core/dist/esm/components/EmptyState"; +import { PendingIcon } from "@patternfly/react-icons"; + +import { useDialogs } from 'dialogs.jsx'; import SerialConsole from './serialConsole.jsx'; import Vnc from './vnc.jsx'; import DesktopConsole from './desktopConsole.jsx'; +import { AddEditVNCModal } from './vncAddEdit.jsx'; + import { domainCanConsole, domainDesktopConsole, @@ -34,11 +43,70 @@ import './consoles.css'; const _ = cockpit.gettext; -const VmNotRunning = () => { +const ConsoleEmptyState = ({ vm }) => { + const Dialogs = useDialogs(); + const serials = vm.displays && vm.displays.filter(display => display.type == 'pty'); + const inactive_vnc = vm.inactiveXML.displays && vm.inactiveXML.displays.find(display => display.type == 'vnc'); + + function add_vnc() { + Dialogs.show(); + } + + function edit_vnc() { + Dialogs.show(); + } + + let icon = null; + let top_message = null; + let bot_message = _("Graphical support not enabled."); + + if (serials.length == 0 && !inactive_vnc) { + bot_message = _("Console support not enabled."); + } else { + if (vm.state != "running") { + if (serials.length > 0 && !inactive_vnc) { + top_message = _("Please start the virtual machine to access its serial console."); + } else { + top_message = _("Please start the virtual machine to access its console."); + } + } else if (inactive_vnc) { + icon = ; + if (serials.length > 0) { + top_message = _("Please restart the virtual machine to access its graphical console."); + } else { + top_message = _("Please restart the virtual machine to access its console."); + } + } + } + return ( -
- {_("Please start the virtual machine to access its console.")} -
+ + + + {top_message} + + { !inactive_vnc + ? <> + + {bot_message} + + + + + + + + : + + + + + } + ); }; @@ -81,8 +149,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) { @@ -116,7 +185,11 @@ class Consoles extends React.Component { const vnc = vm.displays && vm.displays.find(display => display.type == 'vnc'); if (!domainCanConsole || !domainCanConsole(vm.state)) { - return (); + return ( +
+ +
+ ); } const onDesktopConsole = () => { // prefer spice over vnc @@ -134,14 +207,19 @@ class Consoles extends React.Component { connectionName={vm.connectionName} vmName={vm.name} spawnArgs={domainSerialConsoleCommand({ vm, alias: pty.alias })} />))} - {vnc && - } + { vnc + ? + :
+ +
+ } {(vnc || spice) && . + */ + +import cockpit from 'cockpit'; + +import React, { useState } from 'react'; +import PropTypes from 'prop-types'; +import { + Form, Modal, ModalVariant, + FormGroup, FormHelperText, HelperText, HelperTextItem, + InputGroup, TextInput, Button, +} from "@patternfly/react-core"; +import { EyeIcon, EyeSlashIcon } from "@patternfly/react-icons"; + +import { ModalError } from 'cockpit-components-inline-notification.jsx'; +import { DialogsContext } from 'dialogs.jsx'; +import { domainChangeVncSettings, domainAttachVnc, domainGet } from '../../../libvirtApi/domain.js'; +import { NeedsShutdownAlert } from '../../common/needsShutdown.jsx'; + +const _ = cockpit.gettext; + +const VncBody = ({ idPrefix, onValueChanged, dialogValues, validationErrors }) => { + const [showPassword, setShowPassword] = useState(false); + + return ( + <> + + onValueChanged('vncPort', event.target.value)} /> + + + { validationErrors?.vncPort + ? {validationErrors?.vncPort} + : + {_("Leave empty to automatically assign a free port when machine starts")} + + } + + + + + + onValueChanged('vncPassword', event.target.value)} /> + + + + + ); +}; + +function validateDialogValues(values) { + const res = { }; + + if (values.vncPort != "" && (!values.vncPort.match("^[0-9]+$") || Number(values.vncPort) < 5900)) + res.vncPort = _("Port must be 5900 or larger."); + + return Object.keys(res).length > 0 ? res : null; +} + +VncBody.propTypes = { + idPrefix: PropTypes.string.isRequired, + onValueChanged: PropTypes.func.isRequired, + dialogValues: PropTypes.object.isRequired, + validationErrors: PropTypes.object, +}; + +export class AddEditVNCModal extends React.Component { + static contextType = DialogsContext; + + constructor(props) { + super(props); + + this.state = { + dialogError: undefined, + vncPort: Number(props.consoleDetail?.port) == -1 ? "" : props.consoleDetail?.port || "", + vncPassword: props.consoleDetail?.password || "", + validationErrors: null, + }; + + this.save = this.save.bind(this); + this.onValueChanged = this.onValueChanged.bind(this); + this.dialogErrorSet = this.dialogErrorSet.bind(this); + } + + onValueChanged(key, value) { + const stateDelta = { [key]: value, validationErrors: null }; + this.setState(stateDelta); + } + + dialogErrorSet(text, detail) { + this.setState({ dialogError: text, dialogErrorDetail: detail }); + } + + save() { + const Dialogs = this.context; + const { vm } = this.props; + + const errors = validateDialogValues(this.state); + if (errors) { + this.setState({ validationErrors: errors }); + return; + } + + const vncParams = { + connectionName: vm.connectionName, + vmName: vm.name, + vncAddress: this.props.consoleDetail?.address || "", + vncPort: this.state.vncPort || "", + vncPassword: this.state.vncPassword || "", + }; + + (this.props.consoleDetail ? domainChangeVncSettings(vncParams) : domainAttachVnc(vncParams)) + .then(() => { + domainGet({ connectionName: vm.connectionName, id: vm.id }); + Dialogs.close(); + }) + .catch((exc) => { + this.dialogErrorSet(_("VNC settings could not be saved"), exc.message); + }); + } + + render() { + const Dialogs = this.context; + const { idPrefix, vm } = this.props; + + const defaultBody = ( +
e.preventDefault()} isHorizontal> + + + ); + + return ( + + + + + }> + <> + { vm.state === 'running' && !this.state.dialogError && } + {this.state.dialogError && } + {defaultBody} + + + ); + } +} + +AddEditVNCModal.propTypes = { + idPrefix: PropTypes.string.isRequired, + vm: PropTypes.object.isRequired, + consoleDetail: PropTypes.object, +}; 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/src/libvirtApi/domain.js b/src/libvirtApi/domain.js index eab234179..b7f8ea42e 100644 --- a/src/libvirtApi/domain.js +++ b/src/libvirtApi/domain.js @@ -1089,3 +1089,29 @@ export async function domainAddTPM({ connectionName, vmName }) { const args = ["virt-xml", "-c", `qemu:///${connectionName}`, "--add-device", "--tpm", "default", vmName]; return cockpit.spawn(args, { err: "message", superuser: connectionName === "system" ? "try" : null }); } + +export function domainAttachVnc({ connectionName, vmName, vncAddress, vncPort, vncPassword }) { + const args = ['virt-xml', '-c', `qemu:///${connectionName}`, vmName, '--add-device', '--graphics', `vnc,listen=${vncAddress},port=${vncPort},passwd=${vncPassword}`]; + const options = { err: "message" }; + + if (connectionName === "system") + options.superuser = "try"; + + return cockpit.spawn(args, options); +} + +export function domainChangeVncSettings({ + connectionName, + vmName, + vncAddress, + vncPort, + vncPassword, +}) { + const options = { err: "message" }; + if (connectionName === "system") + options.superuser = "try"; + + const args = ["virt-xml", "-c", `qemu:///${connectionName}`, vmName, "--edit", "--graphics", `vnc,listen=${vncAddress},port=${vncPort},passwd=${vncPassword}`]; + + return cockpit.spawn(args, options); +} diff --git a/test/check-machines-consoles b/test/check-machines-consoles index 3b8daf778..6053068c9 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,121 @@ fullscreen=0 self.waitViewerDownload("vnc", my_ip) + def testAddEditVNC(self, withSerial=True): + b = self.browser + + # Create a machine without any consoles + + name = "subVmTest1" + self.createVm(name, ptyconsole=withSerial) + + self.login_and_go("/machines") + self.waitPageInit() + self.waitVmRow(name) + self.goToVmPage(name) + + def assert_state(text): + b.wait_in_text(f"#vm-{name}-consoles .pf-v5-c-empty-state", text) + + def assert_not_state(text): + b.wait_not_in_text(f"#vm-{name}-consoles .pf-v5-c-empty-state", text) + + if withSerial: + b.click("#pf-v5-c-console__type-selector") + b.wait_visible("#pf-v5-c-console__type-selector + .pf-v5-c-select__menu") + b.click("#VncConsole button") + b.wait_not_present("#pf-v5-c-console__type-selector + .pf-v5-c-select__menu") + + # "Console" card shows empty state + + if withSerial: + assert_state("Graphical support not enabled.") + b.assert_pixels(f"#vm-{name}-consoles", "no-vnc") + else: + assert_state("Console support not enabled.") + + b.click(f"#vm-{name}-consoles .pf-v5-c-empty-state button:contains(Add VNC)") + + b.wait_visible("#add-vnc-dialog") + b.set_input_text("#add-vnc-port", "5000") + b.click("#add-vnc-save") + b.wait_visible("#add-vnc-dialog .pf-m-error:contains('Port must be 5900 or larger.')") + b.set_input_text("#add-vnc-port", "Newport, Rhode Island") + b.wait_not_present("#add-vnc-dialog .pf-m-error") + b.click("#add-vnc-save") + 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-save") + b.wait_in_text("#add-vnc-dialog", "VNC 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-save") + b.wait_not_present("#add-vnc-dialog") + + if withSerial: + assert_state("Please restart the virtual machine to access its graphical console.") + b.wait_visible(f"#vm-{name}-needs-shutdown") + b.assert_pixels(f"#vm-{name}-consoles", "needs-shutdown") + else: + assert_state("Please restart the virtual machine to access its console.") + + 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-empty-state button:contains(VNC server settings)") + 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", "") + b.click("#edit-vnc-save") + b.wait_not_present("#edit-vnc-dialog") + + 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") + assert_state("Please start the virtual machine to access its console.") + 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}") + + if withSerial: + assert_state("Please start the virtual machine to access its serial console.") + assert_state("Graphical support not enabled.") + b.assert_pixels(f"#vm-{name}-consoles", "shutoff-no-vnc") + else: + assert_state("Console support not enabled.") + + b.click("#vm-not-running-message button:contains(Add VNC)") + b.wait_visible("#add-vnc-dialog") + b.click("#add-vnc-save") + b.wait_not_present("#add-vnc-dialog") + + if withSerial: + assert_not_state("Graphical support not enabled.") + else: + assert_not_state("Console support not enabled.") + assert_state("Please start the virtual machine to access its console.") + + def testAddEditVNCNoSerial(self): + self.testAddEditVNC(withSerial=False) + if __name__ == '__main__': testlib.test_main()