Skip to content

Add Ubuntu 26.04 builds (#609)#5

Draft
richardsimmonds wants to merge 1 commit into
mainfrom
fix/609-ubuntu-26.04-builds
Draft

Add Ubuntu 26.04 builds (#609)#5
richardsimmonds wants to merge 1 commit into
mainfrom
fix/609-ubuntu-26.04-builds

Conversation

@richardsimmonds

Copy link
Copy Markdown
Owner

Summary

Adds ubuntu26.04 (resolute) as a new OS target to the DEB packaging matrix, per documentdb#609.

Changes

  • packaging/build_packages.sh — accept --os ubuntu26.04 in the whitelist and map it to the ubuntu:26.04 Docker base image; updated help text and error message
  • packaging/gateway/build_gateway_packages.sh — same: whitelist, ubuntu:26.04 build + test images, reuses the existing Dockerfile_gateway_ubuntu (parameterized via BASE_IMAGE)
  • .github/workflows/build_deb_packages.yml — added ubuntu26.04 to the full build matrix (the reduced PR matrix is unchanged to keep PR checks fast)
  • packaging/README.md — documented the new distro in both supported-distro lists

Notes

  • The deb Dockerfiles (packaging/deb/Dockerfile-deb, Dockerfile_gateway_ubuntu) take the base image as an ARG, so no Dockerfile changes were needed.
  • No source changes; CI will validate that the extension and gateway build cleanly on the new base image.

Closes documentdb#609

Adds ubuntu26.04 as a new OS target across:
- packaging/build_packages.sh (--os whitelist, docker image)
- packaging/gateway/build_gateway_packages.sh (same)
- packaging/README.md (both supported-distro lists)
- .github/workflows/build_deb_packages.yml (full matrix)

Closes documentdb#609

Signed-off-by: richardsimmonds <richardsimmonds314@gmail.com>

@patty-chow patty-chow left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Verdict: NEEDS CHANGES — 1 blocking issue, 1 process gap, 1 nit.

🔴 Blocking: gateway build will fail on ubuntu26.04 (resolute)

scripts/install_llvm.sh (run by Dockerfile_gateway_ubuntu) hard-pins LLVM_VERSION=20 and installs via apt.llvm.org's llvm.sh. Verified directly against apt.llvm.org:

  • dists/llvm-toolchain-resolute-20/Release404 (no LLVM 20 repo for resolute)
  • dists/llvm-toolchain-resolute-21/Release → 200

llvm.sh only check_urls the codename base path (which exists), so it adds the nonexistent llvm-toolchain-resolute-20 repo; the subsequent apt-get update fails and, with set -e, the Docker build dies. The gateway builds on the pg_version == '17' legs, so both ubuntu26.04 jobs (amd64 + arm64) in the full matrix will fail. scripts/install_llvm.sh needs distro awareness (LLVM 21 on resolute) before this matrix entry is safe.

⚠️ Process gap: nothing in this PR actually exercises the new leg

The PR description says CI will validate the new base image — but it won't. The workflow skips drafts, and the reduced PR matrix (deliberately unchanged) excludes ubuntu26.04. The new leg first runs post-merge on main, where it will hit the LLVM failure above. Please run a manual workflow_dispatch of build_deb_packages.yml on this branch (dispatch uses the full matrix) and link the run here before merge.

✅ Looks Good

  • Extension (non-gateway) path checks out: Dockerfile-deb takes BASE_IMAGE as ARG and uses pgdg via $(lsb_release -cs)-pgdg. Verified resolute-pgdg exists on apt.postgresql.org with pg16/17/18 packages for amd64 and arm64, and ubuntu:26.04 exists on Docker Hub.
  • Codename "resolute" correct; README updated in both supported-distro lists.
  • Keeping the reduced PR matrix untouched is the right call for check speed.
  • DCO: commit daaf317 carries a proper Signed-off-by. Scope is minimal — no unrelated changes.

Reviewed by Hermes reviewer agent

TEST_DOCKER_IMAGE="ubuntu:24.04"
DOCKERFILE="${script_dir}/packaging/gateway/deb/Dockerfile_gateway_ubuntu"
;;
ubuntu26.04)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: This case routes to Dockerfile_gateway_ubuntu, which runs scripts/install_llvm.sh — and that script hard-pins LLVM_VERSION=20. apt.llvm.org has no llvm-toolchain-resolute-20 repo (404; only -21 exists for resolute). The repo add succeeds but apt-get update then fails, killing the Docker build on both ubuntu26.04 pg17 legs (the only legs that build the gateway). install_llvm.sh needs distro awareness (LLVM 21 on resolute) before this entry works.

(github.event_name == 'pull_request' && !inputs.use_full_matrix) &&
'{"include":[{"os":"deb12","arch":"amd64","runner":"ubuntu-24.04","pg_version":"17"},{"os":"ubuntu22.04","arch":"arm64","runner":"ubuntu-24.04-arm","pg_version":"18"}]}' ||
'{"os":["deb11","deb12","deb13","ubuntu22.04","ubuntu24.04"],"arch":["amd64","arm64"],"include":[{"arch":"amd64","runner":"ubuntu-24.04"},{"arch":"arm64","runner":"ubuntu-24.04-arm"}],"pg_version":["16","17","18"]}'
'{"os":["deb11","deb12","deb13","ubuntu22.04","ubuntu24.04","ubuntu26.04"],"arch":["amd64","arm64"],"include":[{"arch":"amd64","runner":"ubuntu-24.04"},{"arch":"arm64","runner":"ubuntu-24.04-arm"}],"pg_version":["16","17","18"]}'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Correct placement (full matrix only), but note: nothing in this PR's lifecycle exercises this new leg — drafts skip the workflow, and the reduced PR matrix excludes ubuntu26.04. Please run a manual workflow_dispatch of this workflow on the branch (dispatch uses the full matrix) and link the run here before merge. As-is, the first ubuntu26.04 build happens post-merge on main, where it will fail on the gateway LLVM issue.

echo ""
echo "Mandatory Arguments:"
echo " --os OS to build packages for. Possible values: [deb11, deb12, ubuntu22.04, ubuntu24.04, rhel8, rhel9]"
echo " --os OS to build packages for. Possible values: [deb11, deb12, ubuntu22.04, ubuntu24.04, ubuntu26.04, rhel8, rhel9]"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Nit: this help text (and the error message at line 50) omits deb13, which the case statement at line 41 accepts. Pre-existing inconsistency, but since this PR touches both lines anyway, add deb13 while you're here. The gateway script's equivalent text already lists it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Ubuntu 26.04 builds

2 participants