-
Notifications
You must be signed in to change notification settings - Fork 468
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
Simply test the built docker image in CI #1617
Conversation
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.
We can test before release, like https://github.com/korandoru/hawkeye/blob/75da87606f8f303c740d8d8d6738d8271af90d0a/.github/workflows/ci.yml#L94-L119
Or even add the extra test step at -
kvrocks/.github/workflows/kvrocks.yaml
Lines 280 to 292 in f3d3105
check-docker: | |
name: Check Docker image | |
needs: [precondition, check-and-lint] | |
if: ${{ needs.precondition.outputs.docs_only != 'true' }} | |
runs-on: ubuntu-20.04 | |
steps: | |
- uses: actions/checkout@v3 | |
- name: Get core numbers | |
run: echo "NPROC=$(nproc)" >> $GITHUB_ENV | |
- uses: docker/build-push-action@v3 | |
with: | |
context: . | |
build-args: MORE_BUILD_ARGS=-j${{ env.NPROC }} |
So we check before merge.
Seems a good idea, but I do not know how to get the docker image in the previous step ( kvrocks/.github/workflows/kvrocks.yaml Line 289 in f3d3105
|
And, a failed try by me, is that |
@PragmaTwice I already link to this snippet which has a step to "Build and load". - name: Build and load
uses: docker/build-push-action@v3
with:
context: .
file: ${{matrix.file}}
tags: ${{matrix.name}}:ci
outputs: type=docker
- name: Sanity check
run: docker run --rm -v $(pwd):/github/workspace ${{matrix.name}}:ci check Take care of |
Got it. Done. |
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.
LGTM. Please update the PR title according to the current change.
It closes #1580.