-
-
Notifications
You must be signed in to change notification settings - Fork 182
Testing: docker CI build for #2220 #2256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
tkurki
wants to merge
13
commits into
master
Choose a base branch
from
build-docker
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+118
−59
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change Docker images to use local npm install instead of global (npm -g). This avoids permission issues when installing plugins and running in rootless container environments like Podman. Changes: - Dockerfile: use npm install instead of npm i -g for tarballs - Dockerfile_rel: use npm install instead of npm i -g for release - Dockerfile_base_24.04: remove setcap on node binary (not needed) - startup.sh: update path to local node_modules/.bin/ - README.md: update documentation for new directory structure The server now installs in /home/node/signalk/node_modules/ and runs entirely in user space without requiring special capabilities.
When running with rootless Podman/Docker and the host's D-Bus socket mounted, the container should use the host's D-Bus and Avahi services instead of trying to start its own. Previously, startup.sh always tried to start D-Bus and Avahi, which produced confusing error messages like "Address already in use" and "Failed to contact D-Bus daemon" even though everything worked fine. Now we detect if the host D-Bus socket is already connected and skip starting container services in that case. This gives clean startup logs for rootless container deployments while maintaining backwards compatibility with Docker Compose setups that don't mount the socket.
Symlinks don't work reliably in all Docker/Compose configurations. Copy the @SignalK packages to the nested node_modules location where the admin-ui expects to find them.
Allow Dockerfile_base_24.04 to accept NODE_VERSION build arg (e.g., NODE_VERSION=22) in addition to existing NODE arg (e.g., NODE=22.x) for better compatibility with different build scenarios. Add shell logic to ensure NodeSource setup script receives proper version format (with .x suffix) regardless of input format. Maintains backward compatibility with CI/CD workflows that use --build-arg NODE=22.x while enabling manual builds with --build-arg NODE_VERSION=22.
Add automatic detection of container runtime environment and export CONTAINER_RUNTIME environment variable to differentiate between Docker, Podman, Kubernetes, containerd, CRI-O, and LXC at runtime. Detection uses multiple methods in priority order: - File markers (/.dockerenv for Docker, /run/.containerenv for Podman) - Environment variables (KUBERNETES_SERVICE_HOST for Kubernetes) - cgroup patterns (/kubepods, /lxc, /containerd, /docker, /libpod) - Runtime sockets (/var/run/crio, /var/run/containerd/containerd.sock) This enables plugins and addons to adapt behavior based on the actual runtime environment. The same container image now works across all runtimes while exposing runtime-specific information via process.env.CONTAINER_RUNTIME. Maintains backward compatibility with IS_IN_DOCKER environment variable which remains set to "true" in all containerized environments for existing server update detection logic. Users can override detection by setting CONTAINER_RUNTIME manually: docker run -e CONTAINER_RUNTIME=custom signalk/signalk-server Modified files: - docker/startup.sh: Add runtime detection logic for 6 container runtimes - docker/README.md: Add comprehensive documentation with usage examples and detection methods for all supported runtimes
Docker image build with minimal amount of build steps to minimize image size
Removes accidental trailing single quote from signalk-server start command that was introduced in 6a5c57d.
- Move IS_IN_DOCKER from Dockerfile ENV to startup.sh for centralized container detection (alongside CONTAINER_RUNTIME) - Simplify Container Runtime Detection section in README.md per maintainer feedback (remove verbose Detection Methods, Usage in Code, Manual Override, and Backward Compatibility sections)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a temporary PR for testing Docker build for #2220
Rebased on master with a branch name that should trigger CI to build dev container, like from master.