AMI: Clean up messy apt handling - #2351
Merged
mmlb merged 8 commits intoAug 31, 2026
Merged
Conversation
Crispy1975
approved these changes
Aug 8, 2026
mmlb
force-pushed
the
manny/psql-1450-minimize-apt-get-updatesupgrades-1-at-start-1-at-end
branch
6 times, most recently
from
August 10, 2026 19:59
a722383 to
4eae171
Compare
Collaborator
Author
|
Reverting the PR/commit that caused this and converting this to draft. Will re-introduce everything with fixes in here after bugs are ironed out. |
mmlb
marked this pull request as draft
August 11, 2026 14:38
mmlb
force-pushed
the
manny/psql-1450-minimize-apt-get-updatesupgrades-1-at-start-1-at-end
branch
from
August 17, 2026 15:52
237d765 to
5e8c157
Compare
Collaborator
Author
PostgreSQL Package Dependency Analysis: PR #2351
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 |
PostgreSQL Extension Dependency Analysis: PR #2351
SummaryNo extensions had dependencies with MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Extension DependenciesPostgreSQL 17 Extension DependenciesOrioleDB 17 Extension Dependencies |
mmlb
force-pushed
the
manny/psql-1450-minimize-apt-get-updatesupgrades-1-at-start-1-at-end
branch
14 times, most recently
from
August 19, 2026 13:57
b35a08f to
2a24f46
Compare
mmlb
force-pushed
the
manny/psql-1450-minimize-apt-get-updatesupgrades-1-at-start-1-at-end
branch
3 times, most recently
from
August 25, 2026 19:54
dd89896 to
dbb8d87
Compare
This comment has been minimized.
This comment has been minimized.
mmlb
force-pushed
the
manny/psql-1450-minimize-apt-get-updatesupgrades-1-at-start-1-at-end
branch
4 times, most recently
from
August 26, 2026 20:31
e4725ba to
399bd3a
Compare
Just like the AMIs, good to know here too.
mmlb
force-pushed
the
manny/psql-1450-minimize-apt-get-updatesupgrades-1-at-start-1-at-end
branch
from
August 31, 2026 14:24
5415d14 to
afcd909
Compare
This comment has been minimized.
This comment has been minimized.
mmlb
marked this pull request as ready for review
August 31, 2026 16:04
hunleyd
requested changes
Aug 31, 2026
hunleyd
left a comment
hunleyd
left a comment
Contributor
There was a problem hiding this comment.
- qemu-bootstrap-nix.sh setup_apt — three stacked defects:
- [[ $ARCH == x86_64 ]] — ARCH is never set in this script and qemu.pkr.hcl only passes GIT_SHA/POSTGRES_MAJOR_VERSION. Dead branch, always false, no nounset to catch it.
- Missing the if [[ $ubumirror == "$defmirror" ]]; then return; fi guard the sibling surrogate-bootstrap-nix.sh has. On arm64 (the only arch CI actually builds), defmirror == ubumirror (ports.ubuntu.com both), so this fires today, appending a duplicate URI to the sources file every single qemu build (apt "configured multiple times" warnings, double index fetch).
- Hardcodes noble instead of deriving $CODENAME like its sibling. rewrite this function to match surrogate-bootstrap-nix.sh's copy exactly (derive ARCH via dpkg --print-architecture with a :? guard, compare amd64, derive $CODENAME from /etc/os-release, add the early return).
- surrogate-bootstrap-nix.sh:182 — new mirror=$(awk '/^URIs:/{uri=$2} /^Suites:.*<CODENAME-updates>/{...}') uses GNU-only </> word-boundary syntax. I reproduced this in a real Ubuntu 24.04 container: default /usr/bin/awk there is mawk, and the exact pattern returns empty output; mirror comes out "", fed straight into debootstrap ... "$mirror". Fix: don't use </> — anchor on whitespace/field boundaries instead.
- chroot-bootstrap-nix.sh — deleted the APT_OPTIONS array (Install-Recommends=false, Install-Suggests=false, Acquire::Languages=none) without replacing it. One of the two apt-get install calls in the same file still has --no-install-recommends, the other doesn't which directly contradicts this PR's own stated goal ("all scripts use apt with the same settings"). Also: unquoted fonts-dejavu*/linux-headers* in the cleanup.sh package array (real glob-expansion hazard, currently harmless only because cwd has no matching files).
- qemu-bootstrap-nix.sh — setup_apt/update_and_upgrade_apt now run before waitfor_boot_finished (the old code and the sibling script both wait first). Cloud-init can still be rewriting ubuntu.sources at that point, silently discarding the sed edit, and/or apt lock-contending with cloud-init's own apt-daily.service and hard-failing under errexit. Didn't fail today's CI run, but it's a timing race, not a guarantee.
It turns out that most of the code deleted here was actually not doing
anything useful. For example, switch_mirror modifies
/etc/apt/sources.list in place but the file doesn't have any mirrors
configured there so it's really a no-op! Here's the contents from an
instance I just fired up[^1]:
ubuntu@ip-172-31-26-227:~$ tail -n+1 /etc/apt/sources.list /etc/apt/sources.list.d/*
==> /etc/apt/sources.list <==
# Ubuntu sources have moved to the /etc/apt/sources.list.d/ubuntu.sources
# file, which uses the deb822 format. Use deb822-formatted .sources files
# to manage package sources in the /etc/apt/sources.list.d/ directory.
# See the sources.list(5) manual page for details.
==> /etc/apt/sources.list.d/ubuntu.sources <==
## Note, this file is written by cloud-init on first boot of an instance
## modifications made here will not survive a re-bundle.
--- 8< ---
Types: deb
URIs: http://us-east-1.ec2.ports.ubuntu.com/ubuntu-ports/
Suites: noble noble-updates noble-backports
Components: main universe restricted multiverse
Signed-By: /usr/share/keyrings/ubuntu-archive-keyring.gpg
## Ubuntu security updates. Aside from URIs and Suites,
## this should mirror your choices in the previous section.
Types: deb
URIs: http://ports.ubuntu.com/ubuntu-ports
Suites: noble-security
Components: main universe restricted multiverse
Signed-By: /usr/share/keyrings/ubuntu-archive-keyring.gpg
We get fallback handling by apt itself by adding multiple mirrors in
URIs, apt tries first (the regional) and falls back to global ubuntu
repos if there's an issue. We also setup a temporary apt config that
forces the APT_OPTIONS for all apt calls so we don't need to do it in
every call site.
I ended up dropping the in-repo sources file since they are strictly
worse than what we get from AWS in the build. There's no change in
suites or components between old and new, just that we get ubuntu
upstream as a fallback. Well technically there's a slight difference
since we are basing off of cloud-init generated files and they can
theoretically change under us but I'll gamble that it'll be fine or
better off. Besides, one day we'll be on NixOS as the ultimate "make
sure we know everything in the instance" ;).
I also got rid of the `add-apt-repository --yes universe` because
universe is already enabled.
[^1]: AMI=ubuntu/images/hvm-ssd-gp3/ubuntu-noble-24.04-amd64-server-20260604
We are wasting a bunch of time and brain power keeping track of package installs, so lets minimize them while keeping the grouping/messages intact. I did move the package installs from setup_apparmor && setup_grub into update_install_packages because why not just install everything in one place and leave the _setup_ to somewhere else. I moved them out of because package installation should all be done early and ASAP IMO so it can fail fast if its going to fail. Also I moved ec2-hibinit-agent, ec2-instance-connect, hibagent here since this is only run for AMIs and the comment isn't currently accurate. Either through `export DEBIAN_FRONTEND=noninteractive`, `disable_services` or maybe it was treated as a bug and is now fixed in the package I'm not sure but I ran this out of AWS and it was fine.
Lets be a little cleaner about setup/update/cleanup, also drop the wrapper functions to plain old apt-get calls since they no longer do anything useful. I created setup_apt because I want to move the Install-Recommends=False into it later and configure all apt calls to use it seamlessly, this way we will have a better/more visible list of packages being installed.
No yum handling necessary and commented out code should be deleted.
mmlb
force-pushed
the
manny/psql-1450-minimize-apt-get-updatesupgrades-1-at-start-1-at-end
branch
from
August 31, 2026 18:49
afcd909 to
a7cb4bf
Compare
Collaborator
Author
Fixed all of these, I run qemu builds on my x86_64 machine so need to keep it.
Yep looks like this was missed when I switched over to
All scripts use apt with the same settings is no longer a goal for this PR (will be done in a separate one). Will handle the glob expansion note.
Fixed |
There's no point run updates/upgrades unnecessarily while also not
ensuring that we start with latest and end with latest. So I've dropped
any unecessary updates and upgrades from middle of runs. Now we update
and upgrade after start up ({surrogate,qemu}-bootstrap-nix, chroot was
already good) and then once at the end (qemu-bootstrap-nix and
nix-provision). Ansible's apt_repository module does update when
necessary and has not changed.
mmlb
force-pushed
the
manny/psql-1450-minimize-apt-get-updatesupgrades-1-at-start-1-at-end
branch
from
August 31, 2026 19:01
a7cb4bf to
6d66689
Compare
hunleyd
approved these changes
Aug 31, 2026
mmlb
deleted the
manny/psql-1450-minimize-apt-get-updatesupgrades-1-at-start-1-at-end
branch
August 31, 2026 21:24
hunleyd
added a commit
that referenced
this pull request
Sep 1, 2026
…pgbackrest-archive-async-spool-path-defaults-onto-the-10gb-root-volume * origin/develop: AMI: Clean up messy apt handling (#2351) feat: enable built-in database_wraparound collector in postgres_exporter (#2375) chore: bump postgres_release to cut fresh AMIs (includes #2399) (#2406) fix : idle and max connection duration timeouts to Envoy HTTP listeners (#2403) fix(cron): revoke TRIGGER on cron.job_run_details with CASCADE (#2399) ci: fix update-flake-lock workflow (app token + update-flake-lock action) (#2281)
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
3 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?
So much confusing code here when apt can do fallbacks better.
Bunch of dead/unused code hanging around.
We run apt-get update/upgrade all over the ami/qemu image build stages.
What is the new behavior?
We now let apt handle fallbacks itself and apt usage is cleaner overall.