Spike: Docker Compose VNC test servers for compatibility testing - #334
Conversation
Adds a compatibility test bed of small, version-pinned Linux VNC servers (tigervnc no-auth, tigervnc VNC-password-auth, x11vnc) built from in-repo Dockerfiles, driven by tests/servers/fleet.mk (servers-up/servers-down/ test-fleet, included from the main Makefile) and exercised by tests/functional/test_fleet.py, which connects with vncdotool.api.connect, sends a key, and captures a screenshot per server -- skipping cleanly per server when its port isn't up. Wires a spike-fleet GitHub Actions workflow that builds/starts the fleet with compose healthchecks, runs the tests, and uploads captured screenshots as artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STWkReabCK9sJW2qQ45brM
The inline artifact-capture script never called vncdotool.api.shutdown(), so the background Twisted reactor thread api.connect() starts kept the step's python process alive indefinitely after the loop finished. Also cap the step at 3 minutes so a similar regression fails fast instead of eating the whole job timeout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STWkReabCK9sJW2qQ45brM
Address review on the server spike: - rename "fleet" throughout: spike-servers.yml workflow, servers.mk, tests/functional/test_servers.py, make test-servers - replace the inline heredoc python in the workflow with tests/functional/capture_screenshots.py Screenshots are now easy to look at rather than thrown away in a tempfile: tests write them to tests/servers/screenshots (override with VNCDOTOOL_SCREENSHOT_DIR), the capture script builds a self-contained index.html gallery of every server, and in CI it writes a summary table to the job summary so the run page shows what was captured without downloading the artifact. `make screenshots` does the same locally. Also make the per-server test say what it proves: the capture must match the size the server serves and must not be a single flat colour, so a handshake that yields no framebuffer content fails instead of passing on PNG magic alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJeE1y3aj3j6L7ByKEtaZj
sibson
changed the title
Running it as a script puts tests/functional on sys.path but not the repo root, so the CI step failed with ModuleNotFoundError: vncdotool -- CI installs requirements-dev.txt, not the package itself. Put the repo root on sys.path too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJeE1y3aj3j6L7ByKEtaZj
Screenshots are only reachable from the web as a zipped artifact today. Record publishing the generated gallery to Pages as the Tier 1 follow-up, with the constraints found while looking at it (no artifact preview, data: URIs stripped from job summaries) and the orphan-branch interim option. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJeE1y3aj3j6L7ByKEtaZj
| run: docker compose -f tests/servers/docker-compose.yml logs | ||
|
|
||
| - name: Upload screenshots | ||
| if: always() |
There was a problem hiding this comment.
Because those trailing steps are diagnostics and teardown, and the case you most want them in is the failing one: when a test fails, the container status, a screenshot of what the server was actually showing, and the server logs are the evidence you need — without if: always() a failed test step skips all of it, and down would leak containers. Added a comment in the workflow saying so.
The steps that are part of the test itself (start servers, run tests) deliberately don't have it, so a failure there still fails the job.
Generated by Claude Code
| tigervnc-tools \ | ||
| x11-apps \ | ||
| xauth \ | ||
| netcat-openbsd \ |
There was a problem hiding this comment.
It was only there for the HEALTHCHECK/compose readiness probe (nc -z 127.0.0.1 5900), which is what makes up --wait a reliable barrier. Not worth a package: switched to bash -c 'exec 3<>/dev/tcp/127.0.0.1/5900', same probe using bash's built-in, and netcat is gone from the images.
Generated by Claude Code
| @@ -0,0 +1,25 @@ | |||
| # Minimal TigerVNC (Xvnc) server, no authentication. | |||
There was a problem hiding this comment.
can we layer these dockerfiles to avoid duplication?
There was a problem hiding this comment.
Done — the three Dockerfiles are now one multi-stage tests/servers/Dockerfile: a base stage with what every server shares (X client to draw, EXPOSE, HEALTHCHECK), then a stage per server that installs only its own packages. Compose picks the stage with target:. Adding a server is one stage rather than another near-copy.
Generated by Claude Code
| @@ -0,0 +1,25 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
can we not use a single entrypoint that optionally spawns with password?
There was a problem hiding this comment.
Yes — merged. tigervnc-entrypoint.sh reads VNC_PASSWORD: unset/empty gives -SecurityTypes None, set writes a password file with vncpasswd and gives -SecurityTypes VncAuth -PasswordFile .... The tigervnc-auth image is gone; both services are the same image (pinned to one build via image: in compose) and differ only by that env var.
The wait-for-X-then-draw block both entrypoints duplicated is now a shared draw-content.sh in the base image, so x11vnc-entrypoint.sh is down to three lines of actual work.
Generated by Claude Code
| # third-party desktop image so the exact server version is controlled by | ||
| # apt in the base distro rather than by whatever a random upstream image | ||
| # happens to bundle. | ||
| FROM debian:bookworm-slim |
There was a problem hiding this comment.
will gh actions cache these custom images or are we paying a build tax everytime. Address as todo rather than blocker.
There was a problem hiding this comment.
We're paying it. GitHub-hosted runners start each job with an empty Docker cache, so layer caching only helps within a run — every run rebuilds from scratch, ~35s of the ~70s total. Recorded as a Tier 1 follow-up in docs/server-compatibility-plan.md with the two options: docker/build-push-action with cache-from/to: type=gha, or build once into GHCR and have CI pull pinned digests (which folds into the digest-pinning item). Not treating it as a blocker.
The layering change above helps a little in the meantime — the TigerVNC services now share one image instead of building two, so it's two builds per run rather than three, over a shared base stage.
Generated by Claude Code
| help: | ||
| @echo "test: run unit tests" | ||
| @echo "test-func: run functional tests" | ||
| @echo "servers-up: start the docker VNC test servers" |
There was a problem hiding this comment.
Address review: - one multi-stage tests/servers/Dockerfile: a shared base stage (X client, healthcheck) with a stage per server, selected by compose `target:`, instead of three near-identical Dockerfiles - one TigerVNC entrypoint and image for both the no-auth and password services; VNC_PASSWORD unset means SecurityTypes None, set means VncAuth. The tigervnc-auth image is gone - shared draw-content.sh for the wait-for-X-then-draw logic both servers had - drop netcat: the healthcheck uses bash's /dev/tcp for the same probe - explain in the workflow why the diagnostic steps carry `if: always()` - Makefile help was missing servers-down Also record the CI image build tax (empty Docker cache per run, ~35s) as a plan follow-up with the gha-cache/GHCR options, per review. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJeE1y3aj3j6L7ByKEtaZj
Test server functional tests was failing on tigervnc and tigervnc-auth with "capture is a single flat colour": the image HEALTHCHECK (and therefore `docker compose up --wait`) only probed that the RFB TCP port accepted a connection. For Xvnc that port opens the instant the process starts, well before the backgrounded draw-content.sh has waited for the display and mapped its xlogo window, so the compose wait can return before there is anything on screen. x11vnc happened to be slow enough starting up (Xvfb, then attaching x11vnc to it) that xlogo usually won the race, masking the same bug there. draw-content.sh now waits for the xlogo window to actually be mapped (via xwininfo) before touching a /tmp/draw-content-ready marker, and the Dockerfile HEALTHCHECK requires that marker in addition to the open port. This ties container health to real screen content for all three servers instead of relying on incidental startup-time differences. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJeE1y3aj3j6L7ByKEtaZj
Now that #334 has landed, carry its review outcomes across: * the two OS jobs were near-copies differing only by shell and script, so they collapse into one matrix job with fail-fast disabled, which keeps the "one OS failing must not take the other down" property without the duplication; * say in the workflow why the trailing steps are if: always(), since that is the question the shape invites; * probe the port with bash's own /dev/tcp rather than nc, the way the Docker servers now do; * record why an open port *is* sufficient readiness here, given the Docker servers needed a content marker on top of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V2PPxkQw8HZ6YUU4hbeigc
ep.sh landed at the repo root in #334: a debug copy of tests/servers/tigervnc-entrypoint.sh with the trap removed and an echo in front of Xvnc, so it prints the command rather than serving. Nothing references it. The `# codeql[py/weak-cryptographic-algorithm]` comment on des_encrypt() did not suppress anything -- the alert was raised on that line anyway and had to be dismissed in the UI -- so it was claiming to do something it doesn't. The comment explaining why RFB mandates DES stays.
2 participants
What this is
Viability spike for Tier 1 of the server compatibility plan: a Docker-Compose-defined set of Linux VNC servers usable identically for local dev and CI. Verdict: viable — green end to end (~70s: image builds + healthcheck-gated startup, all per-server tests pass, screenshots captured and published, clean teardown).
Contents
tests/servers/Dockerfile— one multi-stage build: abasestage with what every server shares (X client to draw,EXPOSE,HEALTHCHECK) plus a stage per server, so adding a server is a stage rather than another near-copy. Built ondebian:bookworm-slimso versions come from apt in a distro we pin, not from whatever an upstream desktop image bundles.tests/servers/docker-compose.yml— servicestigervnc(no auth, :5931),tigervnc-auth(VNC password, :5932),x11vnc(Xvfb-backed, :5933), each selecting its stage withtarget:. The two TigerVNC services are the same image, distinguished only byVNC_PASSWORD.tests/servers/{tigervnc,x11vnc}-entrypoint.sh+ shareddraw-content.sh— the TigerVNC entrypoint servesSecurityTypes Noneor VncAuth depending onVNC_PASSWORD;draw-content.shdraws an X client and signals readiness once its window is mapped.tests/servers/servers.mk(included fromMakefile) —servers-up,servers-down,test-servers,screenshotstests/functional/test_servers.py— parameterized connect → type → capture test per server; skips cleanly when a server port isn't opentests/functional/capture_screenshots.py— screenshots every running server and builds the gallery; used by bothmake screenshotsand CI.github/workflows/spike-servers.yml— branch-push + manual-dispatch workflow proving the servers onubuntu-latestScreenshots are kept, not thrown away
tests/servers/screenshots/(gitignored,VNCDOTOOL_SCREENSHOT_DIRoverrides) instead of a tempfile, so they survive the runindex.htmlgallery with every server's screenshot inlined — one file to open, locally or out of the CI artifactmake screenshotsdoes all of the above locally against whatever servers are upWhat the per-server test proves
Passing means, in order: the RFB handshake completed for that server's security type (None, or VNC password auth), a key event was accepted, a framebuffer update was decoded to PNG, and that PNG is both the size the server serves and not a single flat colour. Previously an all-black framebuffer passed on PNG magic alone.
Findings worth keeping
api.connect()leaves the Twisted reactor thread running untilapi.shutdown()is called — without it, test processes/CI steps hang forever after passing. Hit twice during the spike; now guarded withtearDownModuleand an explicit shutdown in the capture script. Also fresh evidence for the plan's Phase 1 "fail loudly, never hang" workstream.vncpasswdlives intigervnc-tools, needed alongsidetigervnc-standalone-server.Spike status
This is spike-quality code proving the approach. Follow-ups are recorded in
docs/server-compatibility-plan.md: publish the screenshot gallery to GitHub Pages, stop paying the per-run image build tax (gha cache or GHCR), pin images by digest, fold into the main CI workflow, extend coverage (TightVNC, QEMU, TLS variants), deepen what the scenario suite asserts, and wire captures into the fixture/recording pipeline.