Skip to content

Clean up Dockerfiles and add tag-triggered image publishing#204

Open
foolnotion wants to merge 2 commits into
masterfrom
docker-robustness
Open

Clean up Dockerfiles and add tag-triggered image publishing#204
foolnotion wants to merge 2 commits into
masterfrom
docker-robustness

Conversation

@foolnotion
Copy link
Copy Markdown
Contributor

Summary

  • Strip the accumulated dead comments from baseDockerfile (old FROM alternatives, Nvidia GPU block, commented proxy config); combine sed + apt into a single RUN layer. No functional change to the image.
  • Add ARG BASE_IMAGE=srbench/base to alg-Dockerfile and algorithms/tir/Dockerfile so that CI workflows can inject the published registry tag via --build-arg BASE_IMAGE=... without breaking local builds (the default remains srbench/base).
  • Add .github/workflows/docker.yml to publish the base image and all per-algorithm images to DockerHub when a version tag is pushed (v*). Publishing is intentionally not triggered on every push to master — only on explicit releases.

Test plan

  • baseDockerfile builds successfully (docker build -f baseDockerfile -t srbench/base .)
  • alg-Dockerfile builds with --build-arg BASE_IMAGE=srbench/base for both gplearn (pure pip) and operon (native extension)
  • bash test.sh passes all tests inside both containers (test_algorithm.py 3/3, test_evaluate_model.py 1/1) for gplearn and operon

- Strip dead comments from baseDockerfile; combine sed+apt into one layer
- Add ARG BASE_IMAGE to alg-Dockerfile and tir/Dockerfile so CI can
  inject the published registry tag without affecting local builds
  (default remains srbench/base for local use)
- Add docker.yml workflow to publish base + per-algorithm images to
  DockerHub on version tags (not on every push)
Copilot AI review requested due to automatic review settings June 4, 2026 16:06
Copy link
Copy Markdown

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

This PR streamlines the project’s Docker build setup by cleaning up the base image Dockerfile, making algorithm builds configurable via a BASE_IMAGE build arg, and introducing a tag-triggered GitHub Actions workflow intended to publish Docker images on release tags (v*).

Changes:

  • Simplify baseDockerfile by removing large commented blocks and consolidating sed + apt into a single RUN layer.
  • Add ARG BASE_IMAGE=srbench/base to algorithm Dockerfiles so CI can inject a published base image reference.
  • Add .github/workflows/docker.yml to build and push base + per-algorithm images when a version tag is pushed.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
baseDockerfile Removes dead comments and consolidates apt setup into one layer.
alg-Dockerfile Adds configurable BASE_IMAGE for CI-driven base image selection.
algorithms/tir/Dockerfile Adds configurable BASE_IMAGE for the tir custom Dockerfile.
.github/workflows/docker.yml Introduces tag-triggered DockerHub publishing workflow for base + algorithm images.

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

Comment thread baseDockerfile
env:
ORG: ${{ secrets.DOCKER_HUB_USERNAME }}

jobs:
Comment thread .github/workflows/docker.yml Outdated
context: .
file: baseDockerfile
push: true
tags: ${{ env.ORG }}/srbench:base
Comment on lines +81 to +86
build-args: |
ALGORITHM=${{ matrix.algorithm }}
BASE_IMAGE=${{ env.ORG }}/srbench:base
push: true
tags: ${{ env.ORG }}/srbench-${{ matrix.algorithm }}:latest
cache-from: type=gha,scope=${{ matrix.algorithm }}
- Use ORG/base and ORG/<alg> naming to match make_docker_compose_file.sh
- Publish versioned tags alongside :latest when triggered by a v* tag
- Add permissions: contents: read (least-privilege)
@foolnotion
Copy link
Copy Markdown
Contributor Author

The 9 CI failures are all pre-existing issues unrelated to this PR. They were triggered because changing baseDockerfile and alg-Dockerfile correctly sets rebuild-all=true in ci-docker.yml, causing all algorithm images to be rebuilt and exposing failures that already exist on master:

Algorithm Failure Root cause
brush Build fails CMake/C++ compile error in pybrush wheel
bsr test_evaluate_model Sympy TypeError: Pow ^ Integer
e2et test_evaluate_model NameError: name 'model' is not defined in regressor
eql test_evaluate_model ModuleNotFoundError: No module named 'eql'
feat test_evaluate_model ImportError: libshogun.so.18 missing
nesymres test_evaluate_model Missing pretrained checkpoint nesymres_100M.ckpt
pysr test_evaluate_model AttributeError: _validate_data (sklearn API change)
sklearn test_evaluate_model ModuleNotFoundError: No module named 'methods.sklearn'
tpsr test_evaluate_model Missing pretrained model model1.pt

These should be tracked and fixed separately — happy to open issues for each if that's useful.

Also addressed the review feedback: image naming now matches make_docker_compose_file.sh convention (ORG/base, ORG/<alg>), versioned tags are published alongside :latest, and permissions: contents: read is set.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants