Repository navigation
show clear errors for missing vnc - #1018
Conversation
| host=localhost | ||
| #{template.send(:before_script)} | ||
| BASH | ||
| out, status = Open3.capture2e('/bin/bash', '-c', script) |
There was a problem hiding this comment.
I think it's preferable to use standard in here to bash or write the file out. This here creates a giant single line command and there are limits to how many characters a single command can be.
There was a problem hiding this comment.
Good call, the old version passed the whole script to bash as one giant argument. It now writes the script to a file in the test's tmpdir and runs bash <file> instead.
I went with a file rather than piping into stdin: when bash reads the script from stdin, any command in the script that also reads stdin can eat the remaining lines of the script. Writing a file sidesteps that.
| %w[cat grep pgrep seq shuf sleep sh].each do |cmd| | ||
| path = ENV['PATH'].split(':').map { |p| File.join(p, cmd) }.find { |p| File.executable?(p) } | ||
| File.symlink(path, File.join(bin, cmd)) | ||
| end |
There was a problem hiding this comment.
Can't we just prepend the PATH with this bin directory so they show up first in the PATH instead of having to do all this?
There was a problem hiding this comment.
Yep, that's simpler. The fake bin is now prepended to the real PATH, and the symlink loop is gone. I also faked sleep there, so the retry test doesn't spend ~5s sleeping between attempts.
One catch: with the real PATH appended, the two "missing vncserver" tests can't hide a vncserver that's actually installed (my laptop has TigerVNC), so they now skip when one's in PATH. CI doesn't have one, so they still run there. I also added a test where VNCSERVER_CMD points at a path that doesn't exist, which covers the missing case on any machine.
- the up-front check and the TigerVNC message use vncserver_cmd, so a vncserver pinned outside PATH (#1006) isn't reported as missing - messages say 'VNC server' so no bare vncserver lands in the script - tests write the script to a file instead of a giant bash -c argument - tests prepend the fake commands to PATH instead of symlinking the real ones; the two missing-vncserver tests skip if a real one is installed - add tests for a pinned vncserver outside PATH, found and missing
55edc26 to
9f257ba
Compare
|
Rebased onto master. This picked up #1006, which this PR was quietly breaking: the up-front check used |
johrstrom
left a comment
johrstrom
left a comment
There was a problem hiding this comment.
Tested both templates and they both work well for me. Thanks!
What does this PR do?
VNC apps need TurboVNC, but when a node has TigerVNC (or no
vncserverat all) the job retries 10 times, dumps Xvnc's usage text each time, and exits with nothing saying what's wrong. Now:vncserverisn't in PATH (or, forvnc_container, in the container), the job says so and that TurboVNC is required, before trying anything.TurboVNC startup is unchanged. TigerVNC is detected only from its own output ("TigerVNC" / "Xtigervnc", or the "replaced by a systemd unit" warning some RHEL-family packages print), which TurboVNC never prints.
Related issue
Closes #768
Testing
test/batch_connect/vnc_requires_turbovnc_test.rbuses fakevncserverscripts for TigerVNC, the RHEL warning, a failing TurboVNC, and a missingvncserver, for both templates.Anything else?
Not yet tried against RHEL's
tigervnc-serverpackage specifically.