Repository navigation
Conversation
|
Once #1006 gets merged I will rebase this PR and it will be ready for review. |
|
These tests hang for me like half the time or more. I wonder if there's a simpler way to test these, maybe just a fixture file comparison? |
The hang Jeff saw was the old port_used trying nc first. On RHEL and Fedora nc is ncat, where -w only limits the connect, so checking the port the skip test held open (accepting nothing) waited forever. shuf picked that port first about half the time. Rebasing onto #1002, which reads /proc/net/tcp instead, removes it. The tests now mostly check the generated script, as Jeff suggested. Two still run the startup, to show the picked port reaches vncserver and is the one the job reports. Those run under a 30s timeout, with BASH_ENV unset (Lmod makes every bash source its init), and the fake vncserver waits for its Xvnc like the real one does, so pgrep can't miss it. Skipping a port in use is find_port's job and is already tested in template_test.rb, so that test is gone.
3f3540b to
b3d3ff8
Compare
|
Found it I think: the old |
| #{vnc_port_pick} | ||
| # Attempt to start VNC server | ||
| VNC_OUT=$(#{vncserver_cmd} -log "#{vnc_log}" -rfbauth "#{vnc_passwd}" $HTTPD_OPT -noxstartup #{vnc_args} 2>&1) | ||
| VNC_OUT=$(#{vncserver_cmd} -log "#{vnc_log}" -rfbauth "#{vnc_passwd}" $HTTPD_OPT -noxstartup #{vnc_port_arg} #{vnc_args} 2>&1) |
There was a problem hiding this comment.
I'm now wondering if we should just force the port argument all the time. Reviewing this output, the port can be set above in vnc_port_pick or set below in vnc_default_port.
I wonder if it's easier for debugging and maintenance to just force the -rfbport all the time and only have one spot where the variable port can be set whether a minimum or maximum is set or the defaults used.
Trying to think why a center would need 5900+display and have setup machinery (like firewalld rules) around that and all I can think is they opened these ports specifically in which case they'd want to use mins and maxes anyhow.
What does this PR do?
VNC apps now keep the VNC server's own port inside
min_port/max_portwhen either is set, in the cluster'sbatch_connect.vncconfig or in the app'ssubmit.yml.erb.Before, the range only applied to the websocket port, and the VNC server always listened on 5900 + its display number. With a range set, the template picks a free port in it and starts
vncserverwith-rfbport. Without one, the generated script is the same as before (apart from whitespace). Covers both thevncandvnc_containertemplates.-rfbportworks with TurboVNC 2.x and 3.x. 3.x handles it invncserver; 2.x passes it to Xvnc after its own default, and Xvnc uses the last value.Related issue
Closes #933
Testing
test/batch_connect/vnc_port_range_test.rbruns the template's startup against a fakevncserverand checks the default port (5900 + display), a port inside the range, skipping a port already in use, a range with only one end set, and the container template.Anything else?
This reuses the existing
min_port/max_portrather than adding VNC-specific options, because that's what the issue reporter tried first. The tradeoff: a site that already sets them for VNC apps will see the VNC server move off 59xx into that range. That only matters for users connecting a native VNC client directly to the node, and the port in the connection info is still correct. If you'd rather it be opt-in, separatevnc_min_port/vnc_max_portoptions are a small change.