-
Notifications
You must be signed in to change notification settings - Fork 5
docker: add new way to build and push docker images, #248
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
Conversation
…d and push docker image, add version for each of the docker
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.
Pull Request Overview
Unifies Docker image builds across postgis, pgvector, and node-sqitch by parameterizing Dockerfiles, introducing per-image version.yaml files, and adding a GitHub Actions workflow to build/push multi-arch images.
- Introduces centralized docker/Makefile to build/push images for multiple processes and versions.
- Parameterizes Dockerfiles with BASE and BASE_VERSION args and adds repository source labels.
- Adds .github/workflows/docker.yaml to build (PRs) and build+push (push/dispatch) images to GHCR.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| docker/postgis/version.yaml | Declares base image and versions for postgis build matrix. |
| docker/postgis/Dockerfile | Parameterized base image; added label; installs packages. |
| docker/pgvector/version.yaml | Declares base image and versions for pgvector build matrix. |
| docker/pgvector/Dockerfile | Parameterized base; revised build steps for pgvector extension. |
| docker/node-sqitch/version.yaml | Declares base image and versions for node-sqitch build matrix. |
| docker/node-sqitch/Dockerfile | Parameterized base in multi-stage build; added labels. |
| docker/Makefile | New unified targets to build/push per process and per version from version.yaml. |
| .github/workflows/docker.yaml | New workflow to build (PR) and build+push (push/dispatch) images to GHCR. |
Comments suppressed due to low confidence (1)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
docker/Makefile
Outdated
| @BASE=$$(sed -n 's/^base:[[:space:]]*//p' $(PROCESS)/version.yaml | head -n1); \ | ||
| VERSIONS=$$(sed -n '/^versions:/,$$p' $(PROCESS)/version.yaml | sed -n 's/^-\s*//p'); \ | ||
| if [ -z "$$BASE" ] || [ -z "$$VERSIONS" ]; then \ |
Copilot
AI
Oct 16, 2025
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.
'\s' is not a valid character class in POSIX/GNU sed; VERSIONS will be empty and the build will fail. Use POSIX classes and allow optional indentation: replace the inner sed with sed -n 's/^[[:space:]]-[[:space:]]//p'.
docker/Makefile
Outdated
| @BASE=$$(sed -n 's/^base:[[:space:]]*//p' $(PROCESS)/version.yaml | head -n1); \ | ||
| VERSIONS=$$(sed -n '/^versions:/,$$p' $(PROCESS)/version.yaml | sed -n 's/^-\s*//p'); \ | ||
| if [ -z "$$BASE" ] || [ -z "$$VERSIONS" ]; then \ |
Copilot
AI
Oct 16, 2025
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.
Same as above: '\s' is not recognized by sed. Update to sed -n 's/^[[:space:]]-[[:space:]]//p' so version items are parsed correctly.
docker/postgis/version.yaml
Outdated
| @@ -0,0 +1,3 @@ | |||
| base: postgres | |||
| versions: | |||
| - 13.3-alpine | |||
Copilot
AI
Oct 16, 2025
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.
[nitpick] YAML list items should be indented under the key; as written many YAML parsers will reject this. Prefer a valid YAML structure and (with the Makefile changes above) allow optional indentation: change to ' - 13.3-alpine'.
| - 13.3-alpine | |
| - 13.3-alpine |
| @@ -0,0 +1,3 @@ | |||
| base: node | |||
| versions: | |||
| - 20.12.0-alpine3.19 | |||
Copilot
AI
Oct 16, 2025
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.
[nitpick] Indent the list item for valid YAML: ' - 20.12.0-alpine3.19'.
| - 20.12.0-alpine3.19 | |
| - 20.12.0-alpine3.19 |
| strategy: | ||
| matrix: | ||
| process: [pgvector, node-sqitch, postgis] | ||
| max-parallel: 3 |
Copilot
AI
Oct 16, 2025
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.
workflow_dispatch inputs (process, version) are defined but unused; this matrix always builds all processes, ignoring the user's selection. Gate the matrix on event type or add a dispatch-specific step/job that uses inputs.process and inputs.version (e.g., run make PROCESS='${{ inputs.process }}' VERSION='${{ inputs.version }}' build-push-process-version for workflow_dispatch).
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Closes: #247
New way to build and push docker images.
Create workflow to build and push docker image, add version for each of the docker