pg bouncer upgrade

[samrose]

What kind of change does this PR introduce?

pgbouncer needs updating for for prepared statement support, addressing incorporation of upgrades

This PR updates the version to 1.24.1

This PR performs the upgrade, repackages the build of pgbouncer from source in a nix package, and will cover automated testing in testinfra as well.

Summary by CodeRabbit

• New Features

  • PgBouncer is now available as a package with enhanced build configuration and optional systemd support on Linux.

• Updates

  • Upgraded PgBouncer to 1.25.1 with refreshed checksums and build inputs (including pandoc and Python 3).

• Behavioral

  • PgBouncer installation now runs in an additional build mode; placeholder config creation is gated to the Nix-style build.

• Tests

  • Tests now validate the expected PgBouncer version and binary path.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[samrose]

Next steps are to work on the tests for this upgrade + default settings.

[encima]

No config to add/change here as defaults are good; customisable config can be added to API later. Errors occur when testing in test_ami_nix as it seems like the service isn't loaded yet (or the binary is not on the path at least)

[samrose]

@encima re-ran the failed test.

[encima]

Tests for pgbouncer still to be written:

• Start pgbouncer with config
• Connect using prepared statements

[encima]

Noticed the make step seems to make source and docs now; fixing

[encima]

Benefit of 1.240 is that we do not need to change the default settings as max_prepared_statements is enabled by default and set to 200; we can expose configs later and optimise by instance

Only clients connecting with prepared statements enabled (i.e. sqlx) would be affected anyways

[coderabbitai]

Walkthrough

Adds PgBouncer 1.25.1: a Nix derivation and package export, updates Ansible to install in stage2_nix builds and add pandoc/python3 deps, and extends tests to resolve expected PgBouncer version and binary path.

Changes

Cohort / File(s)	Summary
Ansible playbook & tasks ansible/playbook.yml, ansible/tasks/setup-pgbouncer.yml	"Install PgBouncer" task now also runs when stage2_nix is true; apt list gains pandoc and python3; touch of /etc/pgbouncer-custom/{{ pgbouncer_config_item }} guarded to run only when nixpkg_mode is true.
Variables ansible/vars.yml	Bumped pgbouncer_release 1.19.0 → 1.25.1 and updated SHA256 checksum.
Nix package integration nix/packages/default.nix, nix/pgbouncer.nix	Added pgbouncer to public packages and introduced new Nix derivation for pgbouncer 1.25.1 with build/runtime deps (libevent, openssl, c-ares), nativeBuildInputs (pkg-config, pandoc, python3), conditional systemd flags on Linux, tests passthru and metadata.
Tests / Testinfra testinfra/test_ami_nix.py	Added load_expected_pgbouncer_version() and module constants EXPECTED_PGBOUNCER_VERSION, PGBOUNCER_BINARY; adjusted SSH retry exception handling to catch Exception (no bound var).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Test as "Testinfra"
  participant Vars as "Ansible vars.yml / nix/pgbouncer.nix"
  participant Nix as "Nix build / package registry"
  participant Ansible as "Ansible playbook"
  participant Host as "Target host / pgbouncer"

  rect rgba(200,230,255,0.5)
  Test->>Vars: load_expected_pgbouncer_version()
  Vars-->>Test: return version (1.25.1)
  end

  rect rgba(200,255,200,0.5)
  Ansible->>Nix: (stage2_nix) request pgbouncer package
  Nix-->>Ansible: provide pgbouncer derivation/profile
  end

  rect rgba(255,230,200,0.5)
  Ansible->>Host: install pgbouncer (pandoc, python3 deps)
  Host-->>Ansible: installed
  Ansible->>Host: create config (guarded by nixpkg_mode)
  Host-->>Ansible: config created
  end

  rect rgba(230,200,255,0.5)
  Test->>Host: verify binary at PGBOUNCER_BINARY matches EXPECTED_PGBOUNCER_VERSION
  Host-->>Test: version check result
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• staaldraad

Poem

🐇 I hopped in code to fetch a new release,

Pandoc, Python packed—what a gentle breeze.
Nix builds a bouncer, Ansible gives a shove,
Tests look for version, the rabbit nods above. ✨

🚥 Pre-merge checks | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description lacks the required template structure. The description_template specifies using either 'Default' or 'Extension Upgrade' sub-templates, but the provided description uses an ad-hoc format without the structured sections these templates require.	Follow the specified PR description template by selecting and using either the Default or Extension Upgrade template with proper section headings and structured content.
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The PR title 'pg bouncer upgrade' is vague and generic. While it indicates a general category of change, it does not specify the actual upgrade version (1.25.1) or provide concrete information about what was accomplished.	Make the title more specific by including the version upgrade, e.g., 'Upgrade pgbouncer to 1.25.1 for prepared statement support' to clearly convey the main change.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

ansible/tasks/setup-pgbouncer.yml (1)

51-51: Typo: nolign should be nologin.

The shell path /usr/sbin/nolign appears to be a typo. This will cause user creation to fail or set an invalid shell.

🐛 Proposed fix

-    shell: '/usr/sbin/nolign'
+    shell: '/usr/sbin/nologin'

🤖 Fix all issues with AI agents

In `@ansible/tasks/setup-pgbouncer.yml`:
- Line 80: Change the task conditional that currently reads "when: nixpkg_mode"
so the placeholder files are created for all relevant build modes; update the
when expression to "debpkg_mode or nixpkg_mode or stage2_nix" (so downstream
lineinfile tasks that omit create and the subsequent file permission tasks will
find the files). Edit the task containing the current when: nixpkg_mode
condition (the placeholder file creation task) to use the expanded boolean
expression to match the parent import and ensure compatibility with the
lineinfile (lines ~103-106) and permission tasks (lines ~108-116).

In `@nix/pgbouncer.nix`:
- Around line 1-52: The pgbouncer derivation currently lists c-ares in
buildInputs but never enables it in configureFlags, so either remove c-ares from
the buildInputs list or enable it by adding the "--with-cares" configure flag;
locate the stdenv.mkDerivation block for pgbouncer and update the buildInputs
array (remove c-ares) if you don't want c-ares support, or append "--with-cares"
to the configureFlags expression (alongside the existing "--with-systemd"
option) if you do want c-ares DNS resolver enabled.

In `@testinfra/test_ami_nix.py`:
- Around line 174-175: The module declares EXPECTED_PGBOUNCER_VERSION
(initialized by load_expected_pgbouncer_version()) and PGBOUNCER_BINARY but
never uses them; either remove these unused module-level constants or add tests
that exercise them. To fix: if PgBouncer behavior is not needed, delete
EXPECTED_PGBOUNCER_VERSION, PGBOUNCER_BINARY and the call to
load_expected_pgbouncer_version() to avoid import-time work; otherwise implement
tests (e.g., test_pgbouncer_binary_exists and test_pgbouncer_version_matches)
that reference PGBOUNCER_BINARY and EXPECTED_PGBOUNCER_VERSION and validate the
binary exists and its version matches load_expected_pgbouncer_version() (using
subprocess or fixture helpers). Ensure references use the exact symbol names
EXPECTED_PGBOUNCER_VERSION, PGBOUNCER_BINARY, and
load_expected_pgbouncer_version() so future reviewers can locate the change.

🧹 Nitpick comments (1)

testinfra/test_ami_nix.py (1)

150-171: Consider more robust YAML/Nix parsing.

The current string-based parsing is fragile—it doesn't handle quoted values consistently, comments on the same line, or whitespace variations. For YAML, consider using yaml.safe_load() (already have PyYAML via boto3 dependencies). For Nix, regex would be more reliable.

♻️ Optional: Use YAML parser for vars.yml

 def load_expected_pgbouncer_version() -> str:
+    import yaml
     repo_root = Path(__file__).resolve().parent.parent
     ansible_vars = repo_root / "ansible" / "vars.yml"
     if ansible_vars.exists():
         with ansible_vars.open() as f:
-            for raw_line in f:
-                line = raw_line.strip()
-                if line.startswith("pgbouncer_release:"):
-                    return line.split(":", 1)[1].strip().strip('"')
+            data = yaml.safe_load(f)
+            if data and "pgbouncer_release" in data:
+                return str(data["pgbouncer_release"])
 
     nix_file = repo_root / "nix" / "pgbouncer.nix"
     if nix_file.exists():
         with nix_file.open() as f:
-            for raw_line in f:
-                line = raw_line.strip()
-                if line.startswith("version ="):
-                    value = line.split("=", 1)[1].strip()
-                    return value.strip(";").strip('"')
+            import re
+            content = f.read()
+            match = re.search(r'version\s*=\s*"([^"]+)"', content)
+            if match:
+                return match.group(1)
 
     raise RuntimeError(
         "Could not determine expected PgBouncer version from configuration files"
     )

[delgado3d]

need to bump these versions