Skip to content

Commit

Permalink
Explicitly set the host option for the VNC websocket channel
Browse files Browse the repository at this point in the history
Without setting the host option Cockpit connects to the main Cockpit's
machine VNC port instead of the currently open host VNC port. This is a
workaround for a bug in cockpit-ws which should set the host option
automatically for us depending on what iframe makes the connection.

Resolves: RHEL-3959
  • Loading branch information
jelly committed Nov 29, 2023
1 parent d376ba7 commit 6024303
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 5 deletions.
3 changes: 3 additions & 0 deletions src/components/vm/consoles/vnc.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ class Vnc extends React.Component {
binary: "raw",
address: consoleDetail.address,
port: parseInt(consoleDetail.tlsPort || consoleDetail.port, 10),
// https://issues.redhat.com/browse/COCKPIT-870
// https://issues.redhat.com/browse/RHEL-3959
host: cockpit.transport.host,
});
this.setState({
path: `${prefix.slice(1)}?${window.btoa(query)}`,
Expand Down
96 changes: 96 additions & 0 deletions test/check-machines-multi-host-consoles
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
#!/usr/bin/python3 -cimport os, sys; os.execv(os.path.dirname(sys.argv[1]) + "/common/pywrap", sys.argv)

# This file is part of Cockpit.
#
# Copyright (C) 2021 Red Hat, Inc.
#
# Cockpit is free software; you can redistribute it and/or modify it
# under the terms of the GNU Lesser General Public License as published by
# the Free Software Foundation; either version 2.1 of the License, or
# (at your option) any later version.
#
# Cockpit is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
# Lesser General Public License for more details.
#
# You should have received a copy of the GNU Lesser General Public License
# along with Cockpit; If not, see <http://www.gnu.org/licenses/>.

import testlib
from machineslib import VirtualMachinesCase


class TestMultiMachineVNC(VirtualMachinesCase):
"""
Test for showing the wrong VNC console when adding another host via the add
host feature the raw stream channel does not set the host option so would
connect to the console of machine you connect to instead of the newly added
host.
https://issues.redhat.com/browse/RHEL-3959
"""

provision = { # noqa: RUF012
"machine1": {"address": "10.111.113.1/20", "memory_mb": 660},
"machine2": {"address": "10.111.113.2/20", "memory_mb": 660},
}

def setUp(self):
super().setUp()
self.machine1 = self.machines['machine1']
self.machine2 = self.machines['machine2']

self.setup_ssh_auth()

def testBasic(self):
b = self.browser
m = self.machine
m2 = self.machine2
m2_host = "10.111.113.2"

m.execute(f"ssh-keyscan {m2_host} > /etc/ssh/ssh_known_hosts")
self.startLibvirt(m2)

# Start a VM with a VNC on machine1
name = "subVmTest1"
self.createVm(name, graphics="vnc", ptyconsole=True)

self.login_and_go("/machines")
self.waitPageInit()

self.waitVmRow(name)
self.goToVmPage(name)
b.wait_in_text(f"#vm-{name}-system-state", "Running")

self.add_machine(m2_host, known_host=True, password=None)

b.switch_to_top()
b.click("#hosts-sel button")

b.click(f"a[href='/@{m2_host}']")

b.wait_js_cond(f'window.location.pathname == "/@{m2_host}/system"')
b.enter_page("/system", host=m2_host)
b.become_superuser()

b.go(f"/@{m2_host}/machines")
b.enter_page("/machines", host=m2_host)

self.createVm(name, graphics="vnc", ptyconsole=True, machine=m2)
self.waitVmRow("subVmTest1")
b.wait_in_text("#vm-subVmTest1-system-state", "Running")
self.goToVmPage("subVmTest1")

# Wait till we have a VNC connection
b.wait_visible(".pf-v5-c-console__vnc canvas")

# Both machines should have the equal amount of VNC connections
m1_open_connections = m.execute("ss -tupn | grep 127.0.0.1:5900 | grep qemu").splitlines()
m2_open_connections = m2.execute("ss -tupn | grep 127.0.0.1:5900 | grep qemu").splitlines()
self.assertTrue(len(m1_open_connections) == 1)
self.assertTrue(len(m2_open_connections) == 1)


if __name__ == '__main__':
testlib.test_main()
10 changes: 5 additions & 5 deletions test/machineslib.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ def startLibvirt(self, m):
m.execute("virsh net-start default || true")
m.execute(r"until virsh net-info default | grep 'Active:\s*yes'; do sleep 1; done")

def createVm(self, name, graphics='none', ptyconsole=False, running=True, memory=128, connection='system'):
m = self.machine
def createVm(self, name, graphics='none', ptyconsole=False, running=True, memory=128, connection='system', machine=None):
m = machine or self.machine

image_file = m.pull("cirros")

Expand Down Expand Up @@ -190,7 +190,7 @@ def createVm(self, name, graphics='none', ptyconsole=False, running=True, memory
command.append(
f'[ "$(virsh -c qemu:///{connection} domstate {name})" = {state} ] || \
{{ virsh -c qemu:///{connection} dominfo {name} >&2; cat /var/log/libvirt/qemu/{name}.log >&2; exit 1; }}')
self.run_admin("; ".join(command), connection)
self.run_admin("; ".join(command), connection, machine=machine)

# TODO check if kernel is booted
# Ideally we would like to check guest agent event for that
Expand Down Expand Up @@ -228,8 +228,8 @@ def prepareStorageDeviceOnISCSI(self, target_iqn):
self.addCleanup(m.execute, "targetcli /iscsi delete %s; iscsiadm -m node -o delete || true" % target_iqn)
return orig_iqn

def run_admin(self, cmd, connectionName='system'):
m = self.machine
def run_admin(self, cmd, connectionName='system', machine=None):
m = machine or self.machine

if connectionName == 'session':
return m.execute(f"su - admin -c 'export XDG_RUNTIME_DIR=/run/user/$(id -u admin); {cmd}'")
Expand Down

0 comments on commit 6024303

Please sign in to comment.