-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
feat: Build and publish a QEMU image artifact #1430
Conversation
* fix: only grant pg_read_all_data if it exists * fix: prevent `public` from being casted into `regrole`
…til PG is ready to accept connections (#1250)
* fix: disable pg_stat_monitor * chore: bump version
* fix: disable pg_stat_monitor * chore: bump version
…aming fix (#1259) Co-authored-by: Bobbie Soedirgo <[email protected]> Co-authored-by: Bobbie Soedirgo <[email protected]> Co-authored-by: Kang Ming <[email protected]> Co-authored-by: Stojan Dimitrovski <[email protected]> Co-authored-by: Sam Rose <[email protected]> fix(ci): respect postgresVersion input (#1237) fix: only grant pg_read_all_data if it exists (#1242) fix(15.6): disable pg_stat_monitor (#1260)
* bump pg_net to 0.11.0 * bump image to 15.6.1.135
…me; remove ansible pkg + ppa (#1295)
* fix: add more roles to reserved_roles & reserved_memberships * Update common-nix.vars.pkr.hcl
* upgrade pg_net to 0.13.0 on 15.6 * bump postgres-version
This reverts commit 8d5d14a.
07fa49d
to
1691dca
Compare
Probably simplest to squash-merge this change |
@@ -78,6 +78,7 @@ jobs: | |||
run: | | |||
packer init amazon-arm64-nix.pkr.hcl | |||
GIT_SHA=${{github.sha}} | |||
# why is postgresql_major defined here instead of where the _three_ other postgresql_* variables are defined? |
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.
@samrose not sure if you know?
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.
@darora this var is used in ansible, which in turn is run by packer, and so that is the most likely reason it was set here
git grep postgresql_major
.github/workflows/ami-release-nix.yml: packer build -var "git-head-version=${GIT_SHA}" -var "packer-execution-id=${GITHUB_RUN_ID}" -var-file="development-arm.vars.pkr.hcl" -var-file="common-nix.vars.pkr.hcl" -var "ansible_arguments=-e postgresql_major=${POSTGRES_MAJOR_VERSION}" amazon-arm64-nix.pkr.hcl
.github/workflows/testinfra-nix.yml: packer build -var "git-head-version=${GIT_SHA}" -var "packer-execution-id=${GITHUB_RUN_ID}" -var-file="development-arm.vars.pkr.hcl" -var-file="common-nix.vars.pkr.hcl" -var "ansible_arguments=" -var "postgres-version=${{ steps.random.outputs.random_string }}" -var "region=ap-southeast-1" -var 'ami_regions=["ap-southeast-1"]' -var "force-deregister=true" -var "ansible_arguments=-e postgresql_major=${POSTGRES_MAJOR_VERSION}" amazon-arm64-nix.pkr.hcl
CONTRIBUTING.md:ADD "https://github.com/supabase/pg_graphql/releases/download/v${pg_graphql_release}/pg_graphql-v${pg_graphql_release}-pg${postgresql_major}-${TARGETARCH}-linux-gnu.deb" \
Dockerfile-15:ARG postgresql_major=15
Dockerfile-15:ARG postgresql_release=${postgresql_major}.1
Dockerfile-orioledb-17:ARG postgresql_major=17-orioledb
Dockerfile-orioledb-17:ARG postgresql_release=${postgresql_major}.1
ansible/tasks/internal/collect-pg-binaries.yml: path: /tmp/pg_binaries/{{ postgresql_major }}/
ansible/tasks/internal/collect-pg-binaries.yml: src: /usr/lib/postgresql/{{ postgresql_major }}/{{ item }}/
ansible/tasks/internal/collect-pg-binaries.yml: dest: /tmp/pg_binaries/{{ postgresql_major }}/{{ item }}/
ansible/tasks/internal/collect-pg-binaries.yml: dest: /tmp/pg_binaries/{{ postgresql_major }}/lib/
ansible/tasks/internal/collect-pg-binaries.yml: dest: /tmp/pg_binaries/{{ postgresql_major }}/lib/
ansible/tasks/internal/collect-pg-binaries.yml: dest: /tmp/pg_binaries/{{ postgresql_major }}/lib/libpq.so.5
ansible/tasks/internal/collect-pg-binaries.yml: src: /usr/share/postgresql/{{ postgresql_major }}/
ansible/tasks/internal/collect-pg-binaries.yml: dest: /tmp/pg_binaries/{{ postgresql_major }}/share/
ansible/tasks/setup-postgres.yml: name: postgresql-{{ postgresql_major }}={{ postgresql_release }}-1.pgdg20.04+1
ansible/tasks/setup-postgres.yml: cmd: ln -s /usr/lib/postgresql/{{ postgresql_major }}/bin /usr/lib/postgresql/bin
ansible/tasks/setup-postgrest.yml: repo: "deb http://apt.postgresql.org/pub/repos/apt/ focal-pgdg {{ postgresql_major }}"
ansible/tasks/setup-postgrest.yml: repo: "deb http://apt.postgresql.org/pub/repos/apt/ focal-pgdg {{ postgresql_major }}"
Those are all of the places where it's used in ansible.
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.
There are some parts of ansible build that use postgresql_major, and some that use the full version of postgres. If you think there is a way to refactor and change that into just using the version, I am in favor of that.
Otherwise, this was the pattern in vars.yml
postgres_major:
- "15"
- "orioledb-17"
# Full version strings for each major version
postgres_release:
postgresorioledb-17: "17.0.1.032-orioledb"
postgres15: "15.8.1.036"
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.
Or if you just think it might be better to move that line to another spot where other vars are defined, probably it won't be an issue
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.
There's probably some easy minor cleanup we can do to ensure a little bit more consistency in how they're passed in, e.g. by getting rid of ansible_arguments
and populating that from the underlying variables...will take a look at it
VERSION=$(cat common-nix.vars.pkr.hcl | sed -e 's/postgres-version = "\(.*\)"/\1/g') | ||
echo "version=$VERSION" >> $GITHUB_OUTPUT | ||
|
||
# - name: Create nix flake revision tarball |
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.
leaving some things commented out for now - will be needed later
1691dca
to
100858b
Compare
100858b
to
e53bd62
Compare
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.
@darora everything looks good so far.
It seems the qemu-image-build.yml
only builds one of the versions of pg. But maybe you have it set up that way just to speed up development?
If there is some way I can help please let me know. I don't mind reviewing again too.
It is maybe possible we might be able to eventually refactor so that we don't need stages, etc, but it might be worth waiting until until you get through a first version of this, and then try to shift away from that.
Yeah - we're currently only iterating on the PG15 builds so it was simpler to constrain the build to that. I imagine it should be fairly straightforward to re-enable the PG17 build as a follow-up in the next couple of weeks. Thanks for the review - I'll have someone on infra review and approve it as well. |
Used black for python script, shfmt for shell. Default configurations were used for both.
Co-authored-by: angelico <[email protected]>
Creates and publishes a QEMU image for our Postgres distribution.
The build process is adapted from our AMI build, but is slightly different, as described in qemu_artifact.md