Skip to content

consoles: Make a non-shared connection when asking remote to resize#2268

Merged
mvollmer merged 1 commit intocockpit-project:mainfrom
mvollmer:console-resize-is-non-shared
Sep 16, 2025
Merged

consoles: Make a non-shared connection when asking remote to resize#2268
mvollmer merged 1 commit intocockpit-project:mainfrom
mvollmer:console-resize-is-non-shared

Conversation

@mvollmer
Copy link
Copy Markdown
Member

@mvollmer mvollmer commented Aug 8, 2025

It doesn't feel like a good idea to let multiple clients each try to resize the remote end.

This also gives us a easy way to make non-shared connections in our tests, which we will use in the future.


Machines: Cockpit can make exclusive VNC connections

A VNC viewer can tell the VNC server whether or not it wants exclusive access to it, or whether it is okay that multiple viewers display and interact with the same console. Until now, Cockpit has always allowed shared access when it connects.

Now, Cockpit will request exclusive access when it is in the "Remote resizing" mode, to avoid having multiple viewers make conflicting resize requests.

The feature requires the web server from Cockpit 346.

@mvollmer mvollmer force-pushed the console-resize-is-non-shared branch 2 times, most recently from 37381be to 35bc33d Compare September 1, 2025 10:51
@mvollmer mvollmer force-pushed the console-resize-is-non-shared branch from 35bc33d to 4c5c848 Compare September 2, 2025 08:43
@mvollmer
Copy link
Copy Markdown
Member Author

fedora-43 now has cockpit 346, which has cockpit-project/cockpit#22376.

@mvollmer mvollmer force-pushed the console-resize-is-non-shared branch 2 times, most recently from 605af3a to ece9cf3 Compare September 12, 2025 10:37
@mvollmer mvollmer marked this pull request as ready for review September 12, 2025 10:47

b2.select_PF("#vm-console-vnc-scaling", "Remote resizing")

cockpit_version = int(m.execute('(/usr/libexec/cockpit-ws --version'
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can simplify this by finding the list of images that don't have 346. That's probably just rhel-9 and centos-9, and those will never get it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, agreed to keep a shrinking static OS l ist. This is quite a lot of error prone version checking here. Or if you prefer a runtime check, perhaps check b.eval_js('cockpit.transport.options.capabilities')?

@mvollmer
Copy link
Copy Markdown
Member Author

I think tumbleweed and ubuntu-stable will get 346 pretty soon, no? We could wait for them, then we have a stable list of images that don't have the fix, I think.

@mvollmer mvollmer force-pushed the console-resize-is-non-shared branch from 9d18401 to 6448a14 Compare September 12, 2025 12:36
@martinpitt
Copy link
Copy Markdown
Member

@mvollmer u-stable will never get 346. That's only in backports (once it gets approved), but we need to support the shipped version, too.

@mvollmer mvollmer force-pushed the console-resize-is-non-shared branch from 6448a14 to 5a3f069 Compare September 12, 2025 15:44
@mvollmer mvollmer requested a review from martinpitt September 12, 2025 18:08
@mvollmer
Copy link
Copy Markdown
Member Author

Ubuntu is hard to get green in general. I blame the Browser.open bidi timeout..

@mvollmer
Copy link
Copy Markdown
Member Author

@martinpitt, what do you think? Should we check the cockpit version to figure out what cockpit is going to do, or have a list of OSes?

I think the list is better and it is probably very stable once opensuse-tumbleweed has Cockpit 346.


const caps = cockpit.transport.options.capabilities;
const bug_is_fixed = Array.isArray(caps) && caps.includes("websocket-channel-close-fix");
const shared = !(resizeSession && bug_is_fixed);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea in general, but what happens to an existing connection? If it starts out with "resize server" and you switch it to e.g. "local scaling", it won't automatically become shared. Worse, if you start with "local scaling" and change to "resize server", it will stay private. Or does that reconnect internally?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will reconnect. Every change to the scale/resize settings will lead to a reconnection, because of the way the React wrapper is written. (That wrapper is now in our control, so we could improve that.)


b2.select_PF("#vm-console-vnc-scaling", "Remote resizing")

cockpit_version = int(m.execute('(/usr/libexec/cockpit-ws --version'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, agreed to keep a shrinking static OS l ist. This is quite a lot of error prone version checking here. Or if you prefer a runtime check, perhaps check b.eval_js('cockpit.transport.options.capabilities')?

' | grep ^Version: | cut -d" " -f2'))

if cockpit_version >= 346:
if not m.image.startswith(("rhel-9", "centos-9", "ubuntu-", "debian-trixie", "opensuse-")):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, this looks better. But for clarity, perhaps spell out "opensuse-tumbleweed", just in case we get SLES at some point. We'll have to do a lockstep machines/bots update when tumbleweed updates, but we gain robustness.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roger.

It doesn't feel like a good idea to let multiple clients each try to
resize the remote end.

This also gives us a easy way to make non-shared connections in our
tests, which we will use in the future.
@mvollmer mvollmer force-pushed the console-resize-is-non-shared branch from 5a3f069 to 7a7972d Compare September 16, 2025 05:55
Copy link
Copy Markdown
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kiitos!

@mvollmer mvollmer merged commit 063f7f6 into cockpit-project:main Sep 16, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants