Whole lot of ansible clean up - #2272
Open
mmlb wants to merge 7 commits into
Open
Conversation
Collaborator
Author
|
Turns out qemu mode is still necessary, its used to build the multigres k8s images. Will rebase this PR once #2274 is merged. Funnily enough I bet it'll be easier to re-introduce the qemu ansible tasks on top of these commits instead of never removing it. Will see... |
mmlb
marked this pull request as draft
July 15, 2026 18:40
PostgreSQL Extension Dependency Analysis: PR #2272
SummaryNo extensions had dependencies with MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Extension DependenciesPostgreSQL 17 Extension DependenciesOrioleDB 17 Extension Dependencies |
PostgreSQL Package Dependency Analysis: PR #2272
SummaryNo packages had MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Dependency ChangesExtracting PostgreSQL 15 dependencies...
Runtime Closure Size
Raw Dependency ClosurePostgreSQL 17 Dependency ChangesExtracting PostgreSQL 17 dependencies...
Runtime Closure Size
Raw Dependency Closure |
mmlb
force-pushed
the
push-yqluszprqoly
branch
6 times, most recently
from
July 24, 2026 18:28
9e0aeb8 to
d680636
Compare
mmlb
marked this pull request as ready for review
July 24, 2026 18:29
mmlb
force-pushed
the
push-yqluszprqoly
branch
3 times, most recently
from
July 27, 2026 16:13
1900f82 to
e9ec288
Compare
Contributor
ReviewOverviewThis PR cleans up the AMI-build Ansible playbook:
Issues found1. Likely bug: 2. Dead code left behind, contradicting the PR's own stated goal. - name: Setting arch (amd64)
when: platform == "amd64"
set_fact:
arch: "amd64"
- name: Setting arch (arm)
when: platform == "arm64"
set_fact:
arch: "arm64"
3. Removed the 4. Two checksums are excluded from the new verification tooling. 5. Minor inconsistency: What's good here
|
Now that we don't have debpkg_mode I noticed that we always call ansible with nixpkg_mode = !stage2_nix. We can use just one var in that case, lets go with stage2 since it already sort of exists. qemu_mode can be cleaned up a bit too by always having it present. Tasks and templates can now check simple bool vs defined or not.
2 things going on here, 1) `when:` is always just one line and 2) `when:` are directly after `name:` 1. Most of our `when:` clauses are a simple statements or `or`d. Only 4/74 are ands that benefit from list mode. Having the full condition on one line is just plain nicer for grep-ability. Best to just do it one way. 2. We have a bunch of files that mix stage1 and stage2 and even some that have multiple mixed `when:`s. By having the `when`s up top with the name we can skip over/to when reading the files for a specific stage of the ami build.
Mostly just merging consecutive `when: stage2` or `when: not stage2` tasks & blocks. Some slight re-arrangements happened but should be no-ops because they happen a little later during `stage2` but should have been no behavioral change due to it running originally in `stage1`. Other things done: * I renamed setup-docker -> setup-supautils because there was nothing to do with docker in the file. * Dropped explicit 'install gpg explicitly for qemu artifacts' because gpg has been used by ansible before we get here so gpg is definitely already installed.
… everywhere It makes no sense to create a new fact (arch) that is same as platform, and even less sense to do so multiple times. Lets just do it once and early in the playbook so its available effectively always.
This way similar tasks look (sort of) similar instead of everyone doing their own thing, which leads to bugs.
Every download *must* be verified, by either checksum or signature. Naturally this includes signature files that are fetched from the internets. All fetches need to be verified so we can detect modified/tampered/corrupt transfers. I moved the urls here too so that we can have a relatively trivial script to update the checksums, which I've also added in this commit. All checksums are sha256 to avoid sha1 brokeness and complicating the upgrade/verify script.
Collaborator
Author
copy pasted and forgot to change to false, fixed now.
Rebase bug, fixed now.
This is fine, nothing uses ansible at runtime and we don't ever pass any tags when calling ansible from this repo. This was added ~5y ago and no longer relevant.
Fixed.
I wanted to avoid the |
hunleyd
approved these changes
Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Maintenance
What is the current behavior?
Lots of dead weight from dead code such as debpkg_mode, qemu_mode.
Using 2 variables nixpkg_mode and stage2_nix to represent 2 states when 1 variable will do.
Different ways to conditionally run ansible tasks.
Bunch of unverified downloads... 🕛 💣 if you ask me...
What is the new behavior?
Dead code deleted
Lots of clean ups of tasks themselves
All downloads are verified via checksum or signatures (including d/l of signature files)
Added script with 2 modes: update artifact version & checksum, verify checksums are all still good
Additional context
More stuff to come in future PRs.
Closes PSQL-1539