Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new workflow for building simtools production images and simplifies the naming scheme across all Docker images. The key changes include standardizing variable names (from avx_instruction to avx_flag), fixing a bug in CORSIKA configuration version handling, and updating image naming to be more consistent and traceable.
- Adds new
build-simtools-prod.ymlworkflow for production image builds - Standardizes variable naming across workflows and Dockerfiles (
avx_flaginstead ofavx_instruction) - Fixes bug where CORSIKA configuration was using wrong variable for git branch selection
- Simplifies Docker image naming scheme and tagging structure
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/changes/1954.feature.md | Documents new production image building workflow |
| docs/changes/1954.api.md | Documents API changes for Docker image naming |
| docker/Dockerfile-simtools-prod | Updates production Dockerfile with standardized variable names and dependency changes |
| docker/Dockerfile-simtools-legacy | Standardizes variable naming (SIMTOOLS_BRANCH/SIMTOOLS_REPO) |
| docker/Dockerfile-simtools-dev | Updates to use new image naming scheme and removes unused SIMTOOLS_REQUIREMENT arg |
| docker/Dockerfile-simtel_array | Adds CTA_PROD6 define to EXTRA_DEFINES |
| docker/Dockerfile-corsika7 | Fixes bug using correct CORSIKA_CONFIG_VERSION variable |
| .github/workflows/build-simtools-prod.yml | New workflow for building production images with multiple AVX variants |
| .github/workflows/build-simtools-legacy.yml | Standardizes variable names and marks as legacy |
| .github/workflows/build-simtools-dev.yml | Simplifies workflow and standardizes variable names |
| .github/workflows/build-sim_telarray.yml | Updates tagging scheme and removes production matrix |
| .github/workflows/build-corsika7.yml | Standardizes variable names and adds CORSIKA_CONFIG_VERSION build arg |
| .github/workflows/build-corsika-simtel-legacy.yml | Standardizes variable names and marks as legacy |
|
Before I start the actual review and forget this question, why is the simtools version included in the CORSIKA/sim_telarray images?
Is it because you expect the compilation process to be heavily dependent on the simtools version? |
orelgueta
left a comment
orelgueta
left a comment
There was a problem hiding this comment.
Some minor questions, nothing critical. Thanks!
Exactly - docker files, etc and build scripts are simtools dependent. |
|
Add a workflow for the building of simtools production images based in CORSIKA / sim_telarray from gitlab.
Simplified the package naming:
ghcr.io/gammasim/corsika7:0.26.0-78010-avx512fghcr.io/gammasim/sim_telarray:0.26.0-<sim_telarray version>ghcr.io/gammasim/simtools-dev:latest(save to use the latest tag here)ghcr.io/gammasim/simtools-prod:0.26.0-corsika<corsika version>-<avx flag>-simtel<sim_telarray version>The addition of integration tests after the building step for the production images is provided in PR #1955.
The legacy building scripts have been renamed including now
legacyin their file name.Additional fixes / changes:
avx_flagand sometimesavx_instruction. Tried to sync as much possible in the workflows to simplify maintenance.