Publish Quickstart images to ghcr #571
Conversation
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
.github/workflows/build.yml
Outdated
| username: ${{ github.repository_owner }} | ||
| password: ${{ secrets.GITHUB_TOKEN }} | ||
| - name: Build and push docker images to GitHub container registry | ||
| if: matrix.target_os != 'darwin' |
There was a problem hiding this comment.
Tanvi, this change does not make sense because it is cloning all the workflow from dapr/dapr. We need to change the current quickstart workflow to do a push to GHCR in addition to Docker Hub. The change is probably much smaller.
There was a problem hiding this comment.
@artursouza As discussed earlier, In case of Docker we publish quickstarts images to a different repository than Dapr-
Line 8 in 870ed62
With this PR, quickstarts images also will be published at the same place as Dapr components.
There was a problem hiding this comment.
Yes, they will. We just add them under /samples folder. I talked to @yaron2 about that already. This PR, on the other hand, does not work at all for quickstarts.
.github/workflows/build.yml
Outdated
| popd | ||
| done | ||
| echo "Build docker image and push image..." | ||
| make docker-push TARGET_OS=${{ matrix.target_os }} TARGET_ARCH=${{ matrix.target_arch }} DAPR_REGISTRY=ghcr.io/${{ github.repository_owner }} DAPR_TAG=${{ env.REL_VERSION }} |
There was a problem hiding this comment.
We can check this file for what make command we already have for pushing images.
Line 1 in 870ed62
make docker-push is not here I guess. We can think of adding docker-push or reuse what we have already.Overall flow would look like this -
- Build docker images - Already being done
- Docker login - Already being done
- ghcr login - It is added in this PR
- push images to docker - Already being done
- push images to ghcr - We only need to add this.
We can combine 4 & 5 together or keep it separate.
There was a problem hiding this comment.
@tanvigour and @artursouza the docker images I'm most worried about are the ones referenced in compose file for kafka and zookeeper. Are we porting these two aimages to GHCR also for the sake of test stability? https://github.com/dapr/quickstarts/blob/master/bindings/docker-compose-single-kafka.yml#L8
@artursouza what do you think? |
We can put those under the samples/3rdparty/ folder, this way we can also pull them from there. |
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
.github/workflows/build.yml
Outdated
| for sample in "${SAMPLE_LIST[@]}"; do | ||
| echo "Push image for ${sample}..." | ||
| pushd ${sample} | ||
| make push | ||
| popd | ||
| done | ||
| - name: GitHub container registry login | ||
| if: matrix.target_os != 'darwin' && github.event_name != 'pull_request' |
There was a problem hiding this comment.
Nit, but do we need this OS check?
There was a problem hiding this comment.
Yes. We don't want to publish the image per OS build.
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
docker.mk
Outdated
| @@ -6,6 +6,7 @@ | |||
| # Common make targets for samples' Docker images. | |||
|
|
|||
| SAMPLE_REGISTRY ?= docker.io/dapriosamples | |||
| GHRC_REGISTRY ?= ghrc.io/dapr/samples | |||
There was a problem hiding this comment.
We can revert the changes in this file and simply set SAMPLE_REGISTRY=ghrc.io/dapr/samples in the workflow and reuse the existing make targets.
Signed-off-by: tanvigour <tanvi.gour@gmail.com>
Signed-off-by: tanvigour tanvi.gour@gmail.com
Description
Please explain the changes you've made
Issue reference
We strive to have all PRs being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #567
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: