ci: Improve docker cache and test that the image works #349
Conversation
sdn4z
changed the title
|
/lgtm review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🦉 lgtm Review
Score: Needs Work 🔧
🔍 Summary
Thanks for this contribution. The goal of improving Docker caching and adding tests is great, and splitting the build jobs per architecture is the correct approach for improving caching.
However, there are a couple of critical issues in the implementation that need to be addressed. The publish.yml workflow has a broken testing mechanism that will not work as it is, and the build-test.yml workflow will fail due to an invalid Docker tag being used for the cache.
I've left detailed comments on these high-severity issues with suggestions for fixes, along with a couple of minor quality suggestions. Please address the workflow issues before this can be merged.
More information
- Id:
7a6c063af7ca47feae9957d28d878c86 - Model:
gemini-2.5-pro - Created at:
2025-09-26T15:21:13.325532+00:00
Usage summary
- Request count:
2 - Request tokens:
28,893 - Response tokens:
14,888 - Total tokens:
43,781
See the 📚 lgtm-ai repository for more information about lgtm.
|
/lgtm review |
There was a problem hiding this comment.
🦉 lgtm Review
Score: Needs Work 🔧
🔍 Summary
This is a great PR that significantly improves the robustness of our Docker image pipeline. Adding tests for the built images and fixing the multi-arch caching are excellent changes.
However, there is one critical issue in the publish.yml workflow that needs to be addressed before this can be merged. The matrix configuration for the multi-architecture build contains a typo in an architecture name and also includes an unused variable. This will likely cause the release to fail for the ARM platform.
Once this is fixed, the PR should be good to go. The score reflects the critical nature of the issue.
More information
- Id:
086a0691190140ff87cb6fcc0cff1ef3 - Model:
gemini-2.5-pro - Created at:
2025-09-26T15:51:16.855569+00:00
Usage summary
- Request count:
2 - Request tokens:
26,507 - Response tokens:
11,781 - Total tokens:
38,288
See the 📚 lgtm-ai repository for more information about lgtm.
| # Extract version IDs that do NOT have the 'buildcache' tag | ||
| ids_to_delete=$(echo "$versions" | jq -r '.[] | select(.metadata.container.tags | index("buildcache") | not) | .id') | ||
| # Extract version IDs that do NOT have any buildcache-* tags (buildcache-amd64, buildcache-arm64, etc.) | ||
| ids_to_delete=$(echo "$versions" | jq -r '.[] | select(.metadata.container.tags | map(test("^buildcache-")) | any | not) | .id') |
There was a problem hiding this comment.
This will not delete current cache, I have to do it manually.
2 participants
Test the docker image both in the PR and before publishing it.
I've split the build in the
publishworkflow so there's now one job per architecture. This is because gh cache does not properly work when more than one architecture is set, which results in the cache not being used when it should.closes #350