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 "Graphical 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.
cockpit-project · Jan 30, 2025 · 7e51acd · 7e51acd
1 parent 600be40
commit 7e51acd
Show file tree

Hide file tree

Showing 6 changed files with 208 additions and 44 deletions.
diff --git 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
@@ -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 (
-        <div id="vm-not-running-message">
-            {_("Please start the virtual machine to access its console.")}
-        </div>
-    );
-};
-
 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 (<VmNotRunning />);
+            return (
+                <div id="vm-not-running-message">
+                    <VncState vm={vm} vnc={inactive_vnc} />
+                </div>
+            );
         }
 
         const onDesktopConsole = () => { // prefer spice over vnc
@@ -127,21 +126,19 @@ class Consoles extends React.Component {
             <AccessConsoles preselectedType={this.getDefaultConsole()}
                             textSelectConsoleType={_("Select console type")}
                             textSerialConsole={_("Serial console")}
-                            textVncConsole={_("VNC console")}
+                            textVncConsole={_("Graphical console")}
                             textDesktopViewerConsole={_("Desktop viewer")}>
                 {serial.map((pty, idx) => (<SerialConsole type={serial.length == 1 ? "SerialConsole" : cockpit.format(_("Serial console ($0)"), pty.alias || idx)}
                                                   key={"pty-" + idx}
                                                   connectionName={vm.connectionName}
                                                   vmName={vm.name}
                                                   spawnArgs={domainSerialConsoleCommand({ vm, alias: pty.alias })} />))}
-                {vnc &&
                 <Vnc type="VncConsole"
-                     vmName={vm.name}
-                     vmId={vm.id}
-                     connectionName={vm.connectionName}
+                     vm={vm}
                      consoleDetail={vnc}
+                     inactiveConsoleDetail={inactive_vnc}
                      onAddErrorNotification={onAddErrorNotification}
-                     isExpanded={isExpanded} />}
+                     isExpanded={isExpanded} />
                 {(vnc || spice) &&
                 <DesktopConsole type="DesktopViewer"
                                 onDesktopConsole={onDesktopConsole}

diff --git a/src/components/vm/consoles/vnc.jsx b/src/components/vm/consoles/vnc.jsx
@@ -22,10 +22,17 @@ import cockpit from 'cockpit';
 import { VncConsole } from '@patternfly/react-console';
 import { Dropdown, DropdownItem, DropdownList } from "@patternfly/react-core/dist/esm/components/Dropdown";
 import { MenuToggle } from "@patternfly/react-core/dist/esm/components/MenuToggle";
+import { Button } from "@patternfly/react-core/dist/esm/components/Button";
 import { Divider } from "@patternfly/react-core/dist/esm/components/Divider";
+import { EmptyState, EmptyStateBody, EmptyStateFooter } from "@patternfly/react-core/dist/esm/components/EmptyState";
+import { Split, SplitItem } from "@patternfly/react-core/dist/esm/layouts/Split/index.js";
+
+import { useDialogs } from 'dialogs.jsx';
 
 import { logDebug } from '../../../helpers.js';
 import { domainSendKey } from '../../../libvirtApi/domain.js';
+import { AddVNC } from './vncAdd.jsx';
+import { EditVNCModal } from './vncEdit.jsx';
 
 const _ = cockpit.gettext;
 // https://github.com/torvalds/linux/blob/master/include/uapi/linux/input-event-codes.h
@@ -48,6 +55,79 @@ const Enum = {
     KEY_DELETE: 111,
 };
 
+export const VncState = ({ vm, vnc }) => {
+    const Dialogs = useDialogs();
+
+    function add_vnc() {
+        Dialogs.show(<AddVNC idPrefix="add-vnc" vm={vm} />);
+    }
+
+    function edit_vnc() {
+        Dialogs.show(<EditVNCModal idPrefix="edit-vnc" vm={vm} consoleDetail={vnc} />);
+    }
+
+    if (vm.state == "running" && !vnc) {
+        return (
+            <EmptyState>
+                <EmptyStateBody>
+                    {_("Graphical support not enabled.")}
+                </EmptyStateBody>
+                <EmptyStateFooter>
+                    <Button variant="secondary" onClick={add_vnc}>
+                        {_("Add VNC")}
+                    </Button>
+                </EmptyStateFooter>
+            </EmptyState>
+        );
+    }
+
+    let vnc_info;
+    let vnc_action;
+
+    if (!vnc) {
+        vnc_info = _("not supported");
+        vnc_action = (
+            <Button variant="link" isInline onClick={add_vnc}>
+                {_("Add support")}
+            </Button>
+        );
+    } else {
+        if (vnc.port == -1)
+            vnc_info = _("VNC, dynamic port");
+        else
+            vnc_info = cockpit.format(_("VNC, port $0"), vnc.port);
+
+        vnc_action = (
+            <Button variant="link" isInline onClick={edit_vnc}>
+                {_("Edit")}
+            </Button>
+        );
+    }
+
+    return (
+        <>
+            <p>
+                {
+                    vm.state == "running"
+                        ? _("Shut down and restart the virtual machine to access the graphical console.")
+                        : _("Please start the virtual machine to access its console.")
+                }
+            </p>
+            <br />
+            <div>
+                <Split hasGutter>
+                    <SplitItem isFilled>
+                        <span><b>{_("Graphical console:")}</b> {vnc_info}</span>
+                    </SplitItem>
+                    <SplitItem>
+                        {vnc_action}
+                    </SplitItem>
+                </Split>
+            </div>
+        </>
+    );
+};
+
 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 (
+                <div className="pf-v5-c-console__vnc">
+                    <VncState vm={vm} vnc={inactiveConsoleDetail} />
+                </div>
+            );
+        }
+
+        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 = [
             <Dropdown onSelect={this.onExtraKeysDropdownToggle}
-                key={cockpit.format("$0-$1-vnc-sendkey", vmName, connectionName)}
+                key={cockpit.format("$0-$1-vnc-sendkey", vm.name, vm.connectionName)}
                 toggle={(toggleRef) => (
                     <MenuToggle
-                        id={cockpit.format("$0-$1-vnc-sendkey", vmName, connectionName)}
+                        id={cockpit.format("$0-$1-vnc-sendkey", vm.name, vm.connectionName)}
                         ref={toggleRef}
                         onClick={(_event) => this.setState({ isActionOpen: !isActionOpen })}>
                         {_("Send key")}

diff --git a/src/components/vm/consoles/vncBody.jsx b/src/components/vm/consoles/vncBody.jsx
@@ -21,8 +21,10 @@ import React, { useState } from 'react';
 import PropTypes from 'prop-types';
 import {
     FormGroup, FormHelperText, HelperText, HelperTextItem,
-    InputGroup, TextInput, Button, Checkbox,
+    Grid, GridItem,
+    InputGroup, TextInput, Button, Checkbox
 } from "@patternfly/react-core";
+import { Split, SplitItem } from "@patternfly/react-core/dist/esm/layouts/Split/index.js";
 import { EyeIcon, EyeSlashIcon } from "@patternfly/react-icons";
 
 import cockpit from 'cockpit';

diff --git a/src/components/vm/vmDetailsPage.jsx b/src/components/vm/vmDetailsPage.jsx
@@ -132,24 +132,22 @@ export const VmDetailsPage = ({
             title: _("Usage"),
             body: <VmUsageTab vm={vm} />,
         },
-        ...(vm.displays.length
-            ? [{
-                id: `${vmId(vm.name)}-consoles`,
-                className: "consoles-card",
-                title: _("Console"),
-                actions: vm.state != "shut off"
-                    ? <Button variant="link"
-                          onClick={() => {
-                              const urlOptions = { name: vm.name, connection: vm.connectionName };
-                              return cockpit.location.go(["vm", "console"], { ...cockpit.location.options, ...urlOptions });
-                          }}
-                          icon={<ExpandIcon />}
-                          iconPosition="right">{_("Expand")}</Button>
-                    : null,
-                body: <Consoles vm={vm} config={config}
-                            onAddErrorNotification={onAddErrorNotification} />,
-            }]
-            : []),
+        {
+            id: `${vmId(vm.name)}-consoles`,
+            className: "consoles-card",
+            title: _("Console"),
+            actions: vm.state != "shut off"
+                ? <Button variant="link"
+                      onClick={() => {
+                          const urlOptions = { name: vm.name, connection: vm.connectionName };
+                          return cockpit.location.go(["vm", "console"], { ...cockpit.location.options, ...urlOptions });
+                      }}
+                      icon={<ExpandIcon />}
+                      iconPosition="right">{_("Expand")}</Button>
+                : null,
+            body: <Consoles vm={vm} config={config}
+                      onAddErrorNotification={onAddErrorNotification} />,
+        },
         {
             id: `${vmId(vm.name)}-disks`,
             className: "disks-card",

diff --git 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,59 @@ 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 console not supported")
+
+        # Shut down machine, card shows a button to add VNC
+
+        self.performAction("subVmTest1", "forceOff")
+        b.wait_in_text("#vm-not-running-message", "Graphical console: not supported")
+        b.click("#vm-not-running-message button:contains(Add support)")
+
+        b.wait_visible("#add-vnc-dialog")
+        b.set_input_text("#add-vnc-address", "0.0.0.0")
+        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.set_input_text("#add-vnc-port", "5901")
+        b.click("#add-vnc-add")
+        b.wait_not_present("#add-vnc-dialog")
+
+        b.wait_in_text("#vm-not-running-message", "Graphical console: supported, port 5901")
+
+        root = ET.fromstring(self.machine.execute(f"virsh dumpxml {name}"))
+        graphics = root.find('devices').findall('graphics')
+        self.assertEqual(len(graphics), 1)
+        self.assertEqual(graphics[0].get('port'), "5901")
+        self.assertEqual(graphics[0].get('listen'), "0.0.0.0")
+
+        b.click("#vm-not-running-message button:contains(Edit)")
+        b.wait_visible("#edit-vnc-dialog")
+        b.set_input_text("#edit-vnc-address", "")
+        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 {name}"))
+        graphics = root.find('devices').findall('graphics')
+        self.assertEqual(len(graphics), 1)
+        self.assertEqual(graphics[0].get('port'), "-1")
+        self.assertEqual(graphics[0].get('listen'), None)
+
 
 if __name__ == '__main__':
     testlib.test_main()