Skip to content

VNC Container Implementation#2

Open
cooperj wants to merge 57 commits intomainfrom
display
Open

VNC Container Implementation#2
cooperj wants to merge 57 commits intomainfrom
display

Conversation

@cooperj
Copy link
Copy Markdown
Member

@cooperj cooperj commented Feb 16, 2026

This pull request introduces a major overhaul of the container build system and documentation for the repository. The changes improve modularity, maintainability, and usability of the Docker image build workflows, add new automation for image variant requests, and update the documentation to better guide users. The most important changes are grouped below.

CI/CD Workflow Refactor and Improvements:

  • Replaces the monolithic docker-build-and-push.yaml workflow with a new modular system: introduces build-docker-images.yaml as the main entry point, which calls reusable, specialized workflows for building standalone and ROS-based images. This allows for more maintainable and scalable image builds. [1] [2]
  • Adds a new reusable workflow, _build-ros-image.yaml, specifically for building ROS and CUDA-based images, supporting both base and desktop variants, with improved tagging and caching strategies.
  • Renames and updates the standalone image workflow from _build-image.yaml to _build-standalone-image.yaml, modernizing actions versions, simplifying inputs, and improving tag logic. [1] [2] [3] [4]

Documentation and Usability:

  • Significantly rewrites and expands README.md to clarify the purpose and selection of each container image, provide visual diagrams of image relationships, and offer workflow guidance for users.

Issue and Variant Management:

  • Adds a new GitHub issue template request-variant.yaml to streamline requests for new image variants, especially for ROS and CUDA combinations, ensuring user needs are captured in a structured way.

General Maintenance:

  • Minor Dockerfile formatting improvements for consistency.

These changes collectively modernize the image build infrastructure, make it easier to extend and maintain, and improve the experience for both maintainers and users.

@cooperj
Copy link
Copy Markdown
Member Author

cooperj commented Feb 16, 2026

I do want to slim this down further, but not change too much to make it too much harder to use.

The rough plan was to install docker tooling inside this container and sharing the docker.sock allowing for us to have desktop shortcuts for running docker exec into other containers, and I propose we should configure that with docker labels.

@cooperj cooperj marked this pull request as ready for review February 16, 2026 14:34
this is the code I have been working on locally to create a display container that is **seperate** and **not ros enabled**

the idea with it not being ros enabled and running on debian:trixie-slim is to keep the size of this display container down, and reduce complexity.
@cooperj
Copy link
Copy Markdown
Member Author

cooperj commented Feb 16, 2026

@marc-hanheide do we want to rebase this onto the main branch and merge this after #1?

@marc-hanheide
Copy link
Copy Markdown
Member

I do want to slim this down further, but not change too much to make it too much harder to use.

The rough plan was to install docker tooling inside this container and sharing the docker.sock allowing for us to have desktop shortcuts for running docker exec into other containers, and I propose we should configure that with docker labels.

I'm not convinced by making an assumption of access to docker socket. That's a deployment question that shouldn't have an impact on the container.

we can use devcontainer labels to facilitate attaching to it

@marc-hanheide
Copy link
Copy Markdown
Member

@marc-hanheide do we want to rebase this onto the main branch and merge this after #1?

yes, feel free to merge #1 if happy with it and bring it all together

…inal

tidy: run formatter and linter on all the dockerfiles to make them nicer
@marc-hanheide
Copy link
Copy Markdown
Member

Regarding authentication and listen addresses, I did spend some time trying to play with getting this working previously.

The way I see copilot suggesting is letting the dev have an option of setting a password on the noVNC interface which is disabled by default.

What I think we should do is the following options:

  1. If we don't need to auth (i.e. student facing robot) - we don't need it!
  2. If we are deploying with VPN access into the robot, have in the compose file the port binding tagged to the VPN interface (i.e. 10.8.0.123:5801:5801 where 10.8.0.123 is the IP on VPN)
  3. Sit the robot interface(s) behind a reverse proxy and put auth on that, this can be anything we like from nice OIDC to basic auth.

I feel like we should be going in the order of preference to 2->1->3, with options 1 and 2 being the easiest to implement.

VNC auth is insecure anyway. So, we should just disable it and for applications where we need protection we do this by protecting the route (e.g. using caddy auth with OIDC). Let's not put a VNC password

@marc-hanheide
Copy link
Copy Markdown
Member

I had a play with this, @cooperj

I may need a bit more context and documentation.

Slightly concerned about what you have in the readme:

A repository of verstile ROS-enabled Docker containers, orginally developed as apart of the Agri-OpenCore (AOC) project.

Container Name Tags Purpose
lcas.lincoln.ac.uk/ros { humble, jazzy } Base ROS Container, the minimal environment you need for ROS
lcas.lincoln.ac.uk/ros_cuda { humble, jazzy } ROS + Nvidia. When you need to use a GPU in your ROS environment for either better quality simulation or AI workloads.
lcas.lincoln.ac.uk/ros_cuda_desktop { humble, jazzy } ROS + Nvidia + Packages. Installs the ros-{distro}-desktop varient so there is the full ROS stack available.
lcas.lincoln.ac.uk/vnc { latest } X11 destination accessible over a website using NoVNC.

Are you suggesting overwriting those existing images (e.g. ros)?

Otherwise, I like the approach.

I have added some docker compose setups for testing in 107fd67

One real concern I have though: I have tried using your VNC container (with GPU) and another ros container (starting rviz or simply /opt/VirtualGL/bin/glxspheres64 for testing. But it is not possible to utilise 3D OpenGL (at least I ddin't make it work).

I still vote for a ROS-enabled, VGL, and GPU enable VNC ROS visualisation image. It doesn't need cuda, but ROS installed. Rather than having a "pure desktop" that other containers connect to (which works but looses OpenGL), we have a base VNC enabled ROS image (NOT cuda!), and a version derived from it that has all standard ROS visualisations install (like rviz, rqt,...), and another that has gazebo installed?

cooperj added 4 commits March 23, 2026 16:40
Updated repository name and improved container documentation.
Added sections for Development and Deployment in README.
Updated sections in README to reflect ROS and VNC usage.
base.dockerfile Outdated
@@ -39,8 +39,7 @@ RUN groupadd --gid $USER_GID $USERNAME \
&& useradd -s /bin/bash --uid $USER_UID --gid $USER_GID -m $USERNAME \
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.

I suggest to always use user 1000, if it already exists we shoudn't create it, independent of base image. We should also ensure it has sudo

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.

also remove all personal preferences and alias etc.

marc-hanheide and others added 11 commits March 24, 2026 14:11
…nhance Dockerfiles for VirtualGL support; remove users from ROS containers
… between containers and not isolate or rely on the host for security benefits
* implement changes made to `base` into `cuda` container

* add `compose.cuda.yml` file for testing that `cuda` configuration.

* remove VirtualGL from VNC container as this is not needed and will just add bulk

* run dockerfmt to tidy up the dockerfiles
* started work on moving ros building to a matrix rather than seperate steps as discused with Marc.

* Simplified the build steps for non ros enabled images (i.e. for the vnc container)
…uration

* Break the desktop step into a part of the ros image builder

* Clean up the matrix
Fix issues with building to ensure that we are bundling correctly
* rename from build image to build standalone image for extra clarity

* name each workflow run

* fix the semver tagging
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a standalone, non-ROS VNC/noVNC “display” container (Debian-based) and updates the repo’s build/publish automation and documentation to support building/displaying ROS GUIs via a shared X11 socket (optionally with GPU overrides).

Changes:

  • Introduces a Debian vnc image with TurboVNC + noVNC + XFCE and an entrypoint to bring up the desktop.
  • Adds compose examples (compose.yml, compose.cuda.yml, compose.gpu.yml) showing a VNC desktop alongside a ROS container sharing /tmp/.X11-unix.
  • Reworks GitHub Actions workflows into reusable ROS/standalone builders and updates docs/issue templates accordingly.

Reviewed changes

Copilot reviewed 17 out of 21 changed files in this pull request and generated 13 comments.

Show a summary per file
File Description
vnc.dockerfile Defines the standalone Debian VNC/noVNC + XFCE image and runtime defaults.
docker/vnc-entrypoint.sh Starts TurboVNC, XFCE, and noVNC proxy; sets X access and wallpaper.
docker/wallpapers/aoc.jpg Adds an additional wallpaper asset for the VNC desktop.
docker/nvidia-egl-vendor.json Adds GLVND EGL vendor registration for NVIDIA (used by ROS/CUDA images).
base.dockerfile Updates ROS base image setup; adds VirtualGL + GLVND NVIDIA EGL registration.
cuda.dockerfile Updates ROS+CUDA image setup; adds VirtualGL + GLVND NVIDIA EGL registration; removes CycloneDDS config copy.
desktop.dockerfile Adds a desktop-variant Dockerfile used by the updated build workflow.
compose.yml Adds VNC+ROS compose example (no GPU).
compose.cuda.yml Adds VNC+ROS compose example for ROS CUDA image.
compose.gpu.yml Adds GPU override compose file for both services.
README.md Updates repository description and container matrix; adds diagrams and VNC section.
.github/workflows/build-docker-images.yaml New top-level workflow orchestrating ROS and standalone builds.
.github/workflows/_build-standalone-image.yaml New reusable workflow for standalone image build/push.
.github/workflows/_build-ros-image.yaml New reusable workflow for ROS image build/push + optional desktop variant.
.github/ISSUE_TEMPLATE/request-variant.yaml Adds an issue template for requesting new image variants.
docker/cyclonedds.xml Removed CycloneDDS config file (no longer copied by Dockerfiles).
cuda_desktop.dockerfile Removed in favor of the new desktop.dockerfile.
.github/workflows/docker-build-and-push.yaml Replaced by the new workflow structure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

README.md Outdated
# AOC Container Base

A repository of verstile ROS-enabled Docker containers, orginally developed as apart of the [Agri-OpenCore (AOC) project](https://agri-opencore.org).
A repository of verstile Docker containers, orginally developed as apart of the [Agri-OpenCore (AOC) project](https://agri-opencore.org). Designed for the execution of simple and reliable containerised robotics solutions.
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

Typo in README: "orginally" should be "originally".

Suggested change
A repository of verstile Docker containers, orginally developed as apart of the [Agri-OpenCore (AOC) project](https://agri-opencore.org). Designed for the execution of simple and reliable containerised robotics solutions.
A repository of verstile Docker containers, originally developed as apart of the [Agri-OpenCore (AOC) project](https://agri-opencore.org). Designed for the execution of simple and reliable containerised robotics solutions.

Copilot uses AI. Check for mistakes.
dockerfile: ../vnc.dockerfile
target: xfce
ports:
- "5801" # noVNC web interface
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

ports: - "5801" maps port 5801 to a random host port, but the header comment instructs users to open http://localhost:5801/.... Use an explicit "5801:5801" mapping (or adjust the instructions).

Suggested change
- "5801" # noVNC web interface
- "5801:5801" # noVNC web interface

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +10
screen -dmS turbovnc bash -c '/opt/TurboVNC/bin/vncserver :1 -depth 24 -noxstartup -securitytypes TLSNone,X509None,None 2>&1 | tee /tmp/vnc.log; read -p "Press any key to continue..."'

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

TurboVNC is started with -securitytypes TLSNone,X509None,None, which disables authentication/encryption. If port 5901 is reachable (directly or via a misconfigured proxy), this exposes an unauthenticated remote desktop. Make the default secure (e.g., require a password / TLS, or bind VNC to localhost and gate insecure mode behind an explicit env flag).

Suggested change
screen -dmS turbovnc bash -c '/opt/TurboVNC/bin/vncserver :1 -depth 24 -noxstartup -securitytypes TLSNone,X509None,None 2>&1 | tee /tmp/vnc.log; read -p "Press any key to continue..."'
# Configure VNC security: by default, use TurboVNC's secure settings.
# Set VNC_INSECURE=1 to re-enable insecure, unauthenticated access (not recommended).
if [ "${VNC_INSECURE:-0}" = "1" ]; then
VNC_SECURITY_OPTS="-securitytypes TLSNone,X509None,None"
else
VNC_SECURITY_OPTS=""
fi
screen -dmS turbovnc bash -c "/opt/TurboVNC/bin/vncserver :1 -depth 24 -noxstartup ${VNC_SECURITY_OPTS} 2>&1 | tee /tmp/vnc.log; read -p 'Press any key to continue...'"

Copilot uses AI. Check for mistakes.
cooperj and others added 2 commits March 30, 2026 14:36
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@cooperj cooperj requested a review from marc-hanheide March 30, 2026 15:39
@cooperj
Copy link
Copy Markdown
Member Author

cooperj commented Mar 30, 2026

@marc-hanheide I have made the changes as we have discussed, and it is now ready for re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants