Clean up Dockerfiles and add tag-triggered image publishing#204
Clean up Dockerfiles and add tag-triggered image publishing#204foolnotion wants to merge 2 commits into
Conversation
- 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)
There was a problem hiding this comment.
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
baseDockerfileby removing large commented blocks and consolidatingsed+aptinto a singleRUNlayer. - Add
ARG BASE_IMAGE=srbench/baseto algorithm Dockerfiles so CI can inject a published base image reference. - Add
.github/workflows/docker.ymlto 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.
| env: | ||
| ORG: ${{ secrets.DOCKER_HUB_USERNAME }} | ||
|
|
||
| jobs: |
| context: . | ||
| file: baseDockerfile | ||
| push: true | ||
| tags: ${{ env.ORG }}/srbench:base |
| 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)
|
The 9 CI failures are all pre-existing issues unrelated to this PR. They were triggered because changing
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 |
Summary
baseDockerfile(oldFROMalternatives, Nvidia GPU block, commented proxy config); combinesed+aptinto a singleRUNlayer. No functional change to the image.ARG BASE_IMAGE=srbench/basetoalg-Dockerfileandalgorithms/tir/Dockerfileso that CI workflows can inject the published registry tag via--build-arg BASE_IMAGE=...without breaking local builds (the default remainssrbench/base)..github/workflows/docker.ymlto 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
baseDockerfilebuilds successfully (docker build -f baseDockerfile -t srbench/base .)alg-Dockerfilebuilds with--build-arg BASE_IMAGE=srbench/basefor bothgplearn(pure pip) andoperon(native extension)bash test.shpasses all tests inside both containers (test_algorithm.py3/3,test_evaluate_model.py1/1) for gplearn and operon