-
Notifications
You must be signed in to change notification settings - Fork 4
add docker-build/push to CI #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #147 +/- ##
=======================================
Coverage 76.73% 76.73%
=======================================
Files 41 41
Lines 1831 1831
=======================================
Hits 1405 1405
Misses 426 426 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Zeitsperre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be more convenient to use the docker/build-push-action workflows here.
.github/workflows/tests.yml
Outdated
| - name: Get Tag Version | ||
| id: version | ||
| shell: bash | ||
| run: | | ||
| if [[ "${GITHUB_REF}" == "refs/heads/master" ]]; then | ||
| echo "::set-output name=TAG_VERSION::latest" | ||
| else | ||
| echo "::set-output name=TAG_VERSION::${GITHUB_REF##*/}" | ||
| fi | ||
| - name: Login to DockerHub | ||
| uses: docker/login-action@v1 | ||
| with: | ||
| username: ${{ secrets.DOCKERHUB_USERNAME }} | ||
| password: ${{ secrets.DOCKERHUB_TOKEN }} | ||
| - name: Build Docker | ||
| run: make APP_VERSION=${{ steps.version.outputs.TAG_VERSION }} docker-build | ||
| - name: Push to DockerHub | ||
| run: make APP_VERSION=${{ steps.version.outputs.TAG_VERSION }} docker-push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you come across this workflow example? https://github.com/docker/build-push-action/
No need to manually fetch and apply tags on build if you can parse it directly from the trigger:
tags: name/app:latest,name/app:${{ GITHUB_REF##*/ }}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I've used it somewhere else, but I don't recall where exactly.
I simply copied this one over from another repo and didn't want to take more time on it.
Can you commit on the branch? I don't mind using that approach instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Zeitsperre
Did you want to attempt adjusting the CI with that action? I do not have enough time to deal with this, so I will merge as-is otherwise since I will need it for another release for urllib3 security fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. I can take a look at this tomorrow if you can wait until then. Sorry for the delay!
Co-authored-by: Trevor James Smith <[email protected]>
…cker deployment job Signed-off-by: Trevor James Smith <[email protected]>
Signed-off-by: Trevor James Smith <[email protected]>
|
@fmigneault I think this is a "good enough" version. I'll see if I can open some more Pull Requests tomorrow to split things up a bit. We should also consider generating a token on DockerHub and setting up Trusted Publisher for PyPI. |
Since DockerHub does not provide cloud build anymore for free tier, build it ourselves here and push it. For the recent https://github.com/bird-house/twitcher/releases/tag/v0.11.0, I pushed it manually (https://hub.docker.com/repository/docker/birdhouse/twitcher/tags/v0.11.0/sha256:4d88c6afc3011fe4960bf81584b16244954d15db99babb7fcd591f8768d1143f). This CI update should do the same once tagged or on master.
I've set up a token with CRIM user for now. Can be updated to a birdhouse-specific token as desired (I do not have access to birdhouse org tokens within DockerHub).