-
Notifications
You must be signed in to change notification settings - Fork 3
GitHub Action for building docker images #44
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,109 @@ | ||||||||||
| name: "Appserver: Build & Push Docker Image" | ||||||||||
| run-name: Build & Push appserver/${{ github.base_ref || github.ref_name }} | ||||||||||
| on: | ||||||||||
| push: | ||||||||||
| branches: [main] | ||||||||||
| defaults: | ||||||||||
| run: | ||||||||||
| shell: bash | ||||||||||
| jobs: | ||||||||||
| build-and-push: | ||||||||||
| runs-on: ubuntu-latest | ||||||||||
| env: | ||||||||||
| REGISTRY: harbor.delivery.iqgeo.cloud/${{ vars.harbor_project_name }} | ||||||||||
| platform_version: 7.2 | ||||||||||
| steps: | ||||||||||
| - | ||||||||||
| name: Checkout code | ||||||||||
| uses: actions/checkout@v4 | ||||||||||
| with: | ||||||||||
| lfs: true | ||||||||||
|
|
||||||||||
| - | ||||||||||
| name: Authenticate Docker to Harbor | ||||||||||
| uses: docker/login-action@v3 | ||||||||||
| with: | ||||||||||
| registry: harbor.delivery.iqgeo.cloud | ||||||||||
| username: ${{ secrets.container_registry_username }} | ||||||||||
| password: ${{ secrets.container_registry_password }} | ||||||||||
|
|
||||||||||
| - | ||||||||||
| name: Set up QEMU | ||||||||||
| uses: docker/setup-qemu-action@v3 | ||||||||||
|
|
||||||||||
| - | ||||||||||
| name: Set up Docker Buildx | ||||||||||
| uses: docker/setup-buildx-action@v3 | ||||||||||
|
|
||||||||||
| - | ||||||||||
| name: "Build: Platform" | ||||||||||
| id: platform | ||||||||||
| uses: docker/build-push-action@v6 | ||||||||||
| with: | ||||||||||
| push: false | ||||||||||
| pull: false | ||||||||||
| context: . | ||||||||||
| file: deployment/dockerfile.build | ||||||||||
| build-args: | | ||||||||||
| CONTAINER_REGISTRY=${{ env.REGISTRY }} | ||||||||||
| platforms: linux/amd64,linux/arm64 | ||||||||||
| tags: | | ||||||||||
| ${{ env.REGISTRY }}/platform-build:${{ env.platform_version }} | ||||||||||
| iqgeo-myproj-build | ||||||||||
|
Comment on lines
+51
to
+52
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @luiscamachopt @mstrong98 @tomthetommy @ianrobertson-iqgeo so the record shows it, this is NOT a good tagging strategy. I see this, adding a CI/CD pipeline definition to this project template, as a golden opportunity for us to establish a tagging strategy; let's be sure to capitalize on it, and let's work toward consensus on what our tagging strategy is. If ever we wanted to get tagging right, here is our chance!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm considering taking another pass at this @luiscamachopt to reflect my most recent (and most current iteration of this workflow) which, personally, I feel to be my best iteration to-date. But, I don't want to jump the gun on the conversation here. Will you @luiscamachopt take a look at this implementation too? https://github.com/IQGeo/project-nmti-trials/blob/dba9e106e092009df2678083b66368de944dad89/.github/workflows/sre-image-build.yml |
||||||||||
| cache-to: type=gha,mode=max | ||||||||||
| cache-from: type=gha | ||||||||||
|
|
||||||||||
| - | ||||||||||
| name: Extract Appserver metadata (tags, labels) for Docker | ||||||||||
| id: meta | ||||||||||
| uses: docker/metadata-action@v5 | ||||||||||
| with: | ||||||||||
| images: | | ||||||||||
| ${{ env.REGISTRY }}/platform-appserver | ||||||||||
| tags: | | ||||||||||
| type=raw,value={{branch}}-{{sha}} | ||||||||||
| type=sha,prefix={{branch}}-,enable={{!is_default_branch}} | ||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not entirely sure that this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we have not adopted github enterprise should we get this working in Jenkins first or also?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great point @LucasThelen! Glad you mentioned it. Is this the fish you wish? #45 |
||||||||||
| type=raw,value=${{ env.platform_version }},enable={{is_default_branch}} | ||||||||||
|
|
||||||||||
| - | ||||||||||
| name: "Build: Appserver" | ||||||||||
| uses: docker/build-push-action@v6 | ||||||||||
| with: | ||||||||||
| push: true | ||||||||||
| pull: true | ||||||||||
| context: deployment | ||||||||||
| file: deployment/dockerfile.appserver | ||||||||||
| # secret-files: | | ||||||||||
| # oidc=${{ secrets.oidc_config }} | ||||||||||
|
Comment on lines
+76
to
+77
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This, along with the included changes recommended to the |
||||||||||
| build-args: | | ||||||||||
| CONTAINER_REGISTRY=${{ env.REGISTRY }} | ||||||||||
| platforms: linux/amd64,linux/arm64 | ||||||||||
| tags: ${{ steps.meta.outputs.tags }} | ||||||||||
| cache-from: type=gha | ||||||||||
|
|
||||||||||
| - | ||||||||||
| name: Extract Appserver Tools metadata (tags, labels) for Docker | ||||||||||
| id: tools_metadata | ||||||||||
| uses: docker/metadata-action@v5 | ||||||||||
| with: | ||||||||||
| images: | | ||||||||||
| ${{ env.REGISTRY }}/platform-tools | ||||||||||
| tags: | | ||||||||||
| type=raw,value={{branch}}-{{sha}} | ||||||||||
| type=sha,prefix={{branch}}-,enable={{!is_default_branch}} | ||||||||||
| type=raw,value=${{ env.platform_version }},enable={{is_default_branch}} | ||||||||||
|
|
||||||||||
| - | ||||||||||
| name: "Build: Appserver Tools" | ||||||||||
| uses: docker/build-push-action@v6 | ||||||||||
| with: | ||||||||||
| push: true | ||||||||||
| pull: true | ||||||||||
| context: deployment | ||||||||||
| file: deployment/dockerfile.tools | ||||||||||
| build-args: | | ||||||||||
| CONTAINER_REGISTRY=${{ env.REGISTRY }} | ||||||||||
| platforms: linux/amd64,linux/arm64 | ||||||||||
| tags: ${{ steps.tools_metadata.outputs.tags }} | ||||||||||
| cache-from: type=gha | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||
| ARG CONTAINER_REGISTRY=harbor.delivery.iqgeo.cloud/releases/ | ||||||||
| ARG CONTAINER_REGISTRY=${CONTAINER_REGISTRY:-harbor.delivery.iqgeo.cloud/releases} | ||||||||
|
|
||||||||
| FROM iqgeo-myproj-build AS iqgeo_builder | ||||||||
|
|
||||||||
|
|
@@ -16,7 +16,7 @@ RUN rm -rf ${MODULES}/*/node_modules \ | |||||||
| && rm -rf ${MODULES}/*/native | ||||||||
|
|
||||||||
| ############################################## project appserver image | ||||||||
| FROM ${CONTAINER_REGISTRY}platform-appserver:7.2 | ||||||||
| FROM ${CONTAINER_REGISTRY}/platform-appserver:7.2 | ||||||||
|
|
||||||||
| USER root | ||||||||
|
|
||||||||
|
|
@@ -49,3 +49,4 @@ USER www-data | |||||||
| COPY --chown=www-data:www-data entrypoint.d/* /entrypoint.d/ | ||||||||
|
|
||||||||
| COPY --chown=www-data:www-data appserver_config/ /opt/iqgeo/config/ | ||||||||
| # RUN --mount=type=secret,id=oidc,target=/opt/iqgeo/config/oidc/conf.json | ||||||||
|
Comment on lines
51
to
+52
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This will need some re-touching if it is to account for all 'appserver_config' scenarios, but does illustrate exposing secrets to the build as a best practice. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have to add this to the tools image as well?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @LucasThelen Negative. At least, not in current implementation. It does not Now, that said, if tools image does need the |
||||||||
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.
dont we want version to be a variable? that would allow you to have one action that can build multiple images.
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 considered how that might go, yes (and spent way more time than I'd have liked to). But, in the end, the problem as I see it is; what is the 'source of truth' for that? The GitHub repository? It could work for the workflow, yes, but what about dev environments...? The
.iqgeorcfile? Yes, probably. Or, at least, that is the status-quo. While the source of truth it's NOT the Action's workflow, it's parameterized here for convenience and ease-of-use for when the time comes that it does need to be changed.Or, that's my $0.02 on the matter. RFC!