fix(vmgui): pre-flight X display, stop failing silently
`vmgui` sent the child's stderr to /dev/null, so when the X display was unreachable (the common case: sudo -u ci-runner loses the desktop user's XAUTHORITY and xhost wasn't granted) the GUI died with "cannot open display" and the command still printed success — "nothing opens". Now it: - pre-flights the display with xdpyinfo and aborts with the exact `xhost +SI:localuser:ci-runner` hint instead of spawning a doomed process; - drops an inherited XAUTHORITY that this user can't read (falls back to host-based xhost auth); - logs the GUI's stdio to /tmp/vmgui-*.log and grace-polls 1.5s, surfacing the log if the GUI exits immediately; - errors (non-zero) when no DISPLAY is resolvable. Runbook §4.7 updated with the "if nothing opens" guidance. 13 tests; green ≥90%.
This commit is contained in:
@@ -15,6 +15,7 @@ from __future__ import annotations
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import tempfile
|
||||
import time
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from pathlib import Path
|
||||
@@ -488,22 +489,49 @@ def vm_new(
|
||||
# ────────────────────────────────────────────────────────────────────────── open
|
||||
|
||||
|
||||
def _launch_gui(argv: list[str], env: dict[str, str]) -> int:
|
||||
"""Spawn the VMware Workstation GUI detached. Returns the child PID.
|
||||
def _x_display_reachable(env: dict[str, str]) -> tuple[bool, str]:
|
||||
"""Probe whether the X display in ``env`` is usable.
|
||||
|
||||
Uses ``xdpyinfo`` when available (it exits non-zero and prints the reason
|
||||
if the display cannot be opened). Returns ``(ok, detail)``; if the probe
|
||||
tool is missing we optimistically return ``(True, "")`` so the launch is
|
||||
still attempted.
|
||||
"""
|
||||
probe = shutil.which("xdpyinfo")
|
||||
if probe is None:
|
||||
return True, ""
|
||||
try:
|
||||
res = subprocess.run( # fixed argv, no shell
|
||||
[probe],
|
||||
env=env,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.PIPE,
|
||||
timeout=10,
|
||||
check=False,
|
||||
)
|
||||
except (OSError, subprocess.TimeoutExpired) as exc:
|
||||
return False, str(exc)
|
||||
if res.returncode == 0:
|
||||
return True, ""
|
||||
return False, (res.stderr or b"").decode(errors="replace").strip()
|
||||
|
||||
|
||||
def _launch_gui(argv: list[str], env: dict[str, str], log_path: Path) -> subprocess.Popen[bytes]:
|
||||
"""Spawn the VMware Workstation GUI detached, logging stdio to ``log_path``.
|
||||
|
||||
Isolated so tests can monkeypatch it without spawning a real process. The
|
||||
GUI must outlive the CLI, so the child is started in its own session with
|
||||
its stdio detached.
|
||||
GUI must outlive the CLI, so the child runs in its own session; its output
|
||||
goes to a log file (not /dev/null) so failures are diagnosable.
|
||||
"""
|
||||
proc = subprocess.Popen( # argv is a fixed list, never a shell string
|
||||
log_fh = log_path.open("wb")
|
||||
return subprocess.Popen( # argv is a fixed list, never a shell string
|
||||
argv,
|
||||
env=env,
|
||||
stdin=subprocess.DEVNULL,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
stdout=log_fh,
|
||||
stderr=subprocess.STDOUT,
|
||||
start_new_session=True,
|
||||
)
|
||||
return proc.pid
|
||||
|
||||
|
||||
@click.command("vmgui")
|
||||
@@ -575,14 +603,35 @@ def vmgui(
|
||||
env = dict(os.environ)
|
||||
if display:
|
||||
env["DISPLAY"] = display
|
||||
if not env.get("DISPLAY"):
|
||||
disp = env.get("DISPLAY", "")
|
||||
if not disp:
|
||||
raise click.ClickException(
|
||||
"no X display set ($DISPLAY empty and --display omitted). Pass "
|
||||
"--display :0 (and authorise this user on the desktop session: "
|
||||
"run `xhost +SI:localuser:ci-runner` as the logged-in user)."
|
||||
)
|
||||
|
||||
# An inherited XAUTHORITY pointing at another user's cookie (e.g. the
|
||||
# desktop user's ~/.Xauthority) is unreadable by ci-runner and blocks the
|
||||
# connection with "Authorization required". Drop it so the launch relies on
|
||||
# host-based auth (xhost) instead.
|
||||
xauth = env.get("XAUTHORITY")
|
||||
if xauth and not os.access(xauth, os.R_OK):
|
||||
env.pop("XAUTHORITY", None)
|
||||
click.echo(
|
||||
"[vmgui] WARNING: no X display set ($DISPLAY empty and --display "
|
||||
"omitted). The GUI will fail to start. Pass --display :0 and run "
|
||||
"`xhost +SI:localuser:ci-runner` from a desktop session first.",
|
||||
f"[vmgui] dropping unreadable XAUTHORITY ({xauth}); relying on xhost.",
|
||||
err=True,
|
||||
)
|
||||
|
||||
# Pre-flight: fail loudly with the cause instead of silently spawning a
|
||||
# GUI that immediately dies on "cannot open display".
|
||||
ok, detail = _x_display_reachable(env)
|
||||
if not ok:
|
||||
raise click.ClickException(
|
||||
f"cannot open X display {disp!r}: {detail or 'unknown error'}. "
|
||||
"From the desktop session run: xhost +SI:localuser:ci-runner"
|
||||
)
|
||||
|
||||
argv = [gui]
|
||||
if new_window:
|
||||
argv.append("-n")
|
||||
@@ -599,12 +648,32 @@ def vmgui(
|
||||
err=True,
|
||||
)
|
||||
|
||||
ts = datetime.now(tz=UTC).strftime("%Y%m%d-%H%M%S")
|
||||
log_path = Path(tempfile.gettempdir()) / f"vmgui-{os.getpid()}-{ts}.log"
|
||||
|
||||
click.echo(f"[vmgui] launching VMware GUI: {' '.join(argv)}")
|
||||
try:
|
||||
pid = _launch_gui(argv, env)
|
||||
proc = _launch_gui(argv, env, log_path)
|
||||
except (OSError, ValueError) as exc:
|
||||
raise click.ClickException(f"failed to launch VMware GUI: {exc}") from exc
|
||||
click.echo(f"[vmgui] VMware GUI started (pid {pid}); display={env.get('DISPLAY', '')}")
|
||||
|
||||
# Brief grace: if the GUI dies right away, surface its log instead of
|
||||
# claiming success.
|
||||
time.sleep(1.5)
|
||||
rc = proc.poll()
|
||||
if rc is not None and rc != 0:
|
||||
try:
|
||||
tail = log_path.read_text(errors="replace").strip().splitlines()[-5:]
|
||||
except OSError:
|
||||
tail = []
|
||||
detail = "\n ".join(tail) if tail else "(no output captured)"
|
||||
raise click.ClickException(
|
||||
f"VMware GUI exited immediately (code {rc}). Log {log_path}:\n {detail}"
|
||||
)
|
||||
|
||||
click.echo(
|
||||
f"[vmgui] VMware GUI started (pid {proc.pid}); display={disp}; log={log_path}"
|
||||
)
|
||||
|
||||
|
||||
__all__ = ["cleanup_orphans", "vm", "vmgui"]
|
||||
|
||||
Reference in New Issue
Block a user