refactoring: reimplement Docker strategy #3162
Merged
Conversation
This file contains 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
masci
changed the title
ZanSara
suggested changes
Sep 8, 2022
ZanSara
reviewed
Sep 9, 2022
|
|
||
| - name: Release Haystack images | ||
| uses: docker/bake-action@v2 | ||
| if: startsWith(github.ref, 'refs/tags/') |
There was a problem hiding this comment.
Not sure about this line, what is it doing? Are we not building the api-latest in some cases?
There was a problem hiding this comment.
We only build api-latest during a proper release and not when merging in main. But now I see a potential problem with bugfix releases, let me think about this.
There was a problem hiding this comment.
Ok there was indeed a problem with the bugfix releases, now I compare the current version being built with the latest Haystack and only tag if the current one is greater
Co-authored-by: Sara Zan <[email protected]>
brandenchan
pushed a commit
that referenced
this pull request
Sep 21, 2022
* setup base images * add cpu flavor * use the same Dockerfile for cpu and gpu * better naming, add docs * add docker workflow * add missing image input * change cwd for bake * also push api images * try conditional tagging for releases * revert testing code * update docker readme * document variable override * use Python 3.10 * allow empty HAYSTACK_EXTRAS * Apply suggestions from code review Co-authored-by: Sara Zan <[email protected]> * remove repo description step, can't make it work so far * add docs to the last step as it's tricky * manage tags for the newest images * tests are passing, checking in the last bit Co-authored-by: Sara Zan <[email protected]>
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.
Related Issues
Related to #1952
Proposed Changes:
This PR contains several improvements to our Docker images and we can group the changes under 2 main topics.
New tagging strategy
Right now we offer different images for different image variants, e.g.
haystack-cpu,haystack-gpu. After this PR we'll have one image,deepset/haystackproperly tagged to indicate the version and the variant, e.g.deepset/haystack:gpu-1.9.0. This will be the new tags layout:cpu,cpu-1.10.0cpu-1.10.0gpu,gpu-1.9.0gpu-1.9.0Note: the new images won't have the
latesttag by design.New building strategy
We now use
baketo orchestrate the build, which will be defined in thedocker-bake.hclfile. See the README for examples about how to use it.The builds leverage multiple stages and we'll distribute the
basevariants of the images which might be useful in the development process.Notes for the reviewer
Currently adding automation, still WIP.The CI is good to go, not sure why the last step of the job is failing I suspect access management issues, worst case I'll wipe it offChecklist