feat: debian support#381
Open
jason-lynch wants to merge 11 commits into
Open
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds multi-OS support (Rocky, Ubuntu, Debian) to the pgEdge Control Plane, removes the gosigar dependency, and makes database ownership dynamically resolved. It includes Lima templates for different operating systems, Ansible provisioning roles with OS-specific package management, new Apt package manager support alongside DNF, and comprehensive systemd service documentation for both RPM and Deb installations. ChangesMulti-OS Support with Dynamic Database Ownership
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Up to standards ✅🟢 Issues
|
| Category | Results |
|---|---|
| Complexity | 1 medium |
🟢 Metrics 53 complexity · 2 duplication
Metric Results Complexity 53 Duplication 2
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
packaging/rpm/postinstall.sh (1)
3-3: 💤 Low valueConsider quoting
$1for robustness.While
$1is always numeric in RPM scriptlets, quoting it would satisfy ShellCheck and improve defensive coding.🔧 Suggested fix
-if [ $1 -ge 1 ]; then +if [ "$1" -ge 1 ]; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packaging/rpm/postinstall.sh` at line 3, The if test uses an unquoted positional parameter: change the conditional in the postinstall script from using $1 unquoted to a quoted form (e.g., "$1") so the shell handles empty or special values safely and satisfies ShellCheck; update the if line in packaging/rpm/postinstall.sh where the snippet reads if [ $1 -ge 1 ]; then to use the quoted positional parameter.lima/roles/deb_prerequisites/tasks/main.yaml (1)
92-95: ⚡ Quick winPin Delve to a specific version instead of
@latest.Go is pinned to 1.25.5 in this role, but Delve uses
@latest, creating non-reproducible provisioning. When Delve releases a new version, provisioning may fail or behave unexpectedly if the new release doesn't align with the Go 1.25.x toolchain. Pin Delve to v1.25.2 (the latest Delve release supporting Go 1.25):Suggested fix
- name: Install delve debugger ansible.builtin.command: /usr/local/go/bin/go install github.com/go-delve/delve/cmd/dlv@latest + ansible.builtin.command: /usr/local/go/bin/go install github.com/go-delve/delve/cmd/dlv@v1.25.2 args: creates: /root/go/bin/dlv🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lima/roles/deb_prerequisites/tasks/main.yaml` around lines 92 - 95, The task "Install delve debugger" currently uses ansible.builtin.command to run "/usr/local/go/bin/go install github.com/go-delve/delve/cmd/dlv@latest", which makes provisioning non-reproducible; update that command to pin Delve to the v1.25.2 tag (e.g. replace "@latest" with "@v1.25.2") so the task named "Install delve debugger" and the creates check "/root/go/bin/dlv" always install a known compatible Delve build for Go 1.25.x.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/installation/systemd.md`:
- Line 157: The doc uses "sudo apt install
pgedge-control-plane_${VERSION#v}_linux_${ARCH}.deb" (and the same pattern at
the other occurrence) which can be misinterpreted by apt; change both
occurrences to pass a local-file path (e.g., prefix with ./) so apt installs the
downloaded .deb file reliably by path rather than attempting package-name
resolution.
- Around line 51-52: Update the sentence that currently mentions installing the
`pgedge-control-plane` RPM to use inclusive package-format wording; change the
reference to something like "after installing the `pgedge-control-plane`
package" or "after installing the `pgedge-control-plane` package (RPM or DEB)"
so Debian/Ubuntu users are not confused—locate the occurrence of the string
`pgedge-control-plane` in docs/installation/systemd.md and replace the
RPM-specific phrasing accordingly.
- Around line 319-320: The package name in the upgrade command is inconsistent:
replace the literal "pgedge-postgresql18" with the Debian-style package name
"pgedge-postgresql-18" to match the earlier documented pattern
"pgedge-postgresql-<major>" so the apt upgrade command will succeed; update the
text containing "pgedge-postgresql18" accordingly.
In `@e2e/whole_cluster_test.go`:
- Around line 127-133: The loop currently treats any error from row.Scan as
transient; change it to inspect the error returned by row.Scan: if it's
pgx.ErrNoRows or a pgconn.PgError with Code == "42P01" (table not found) then
treat as transient (sleep and continue), otherwise immediately fail the test
(use t.Fatalf or return the error) so real SQL/connection errors aren't masked;
locate the check around conn.QueryRow(...).Scan(&actual) and replace the blanket
continue with explicit error-type checks for pgx.ErrNoRows and pg error code
"42P01".
In `@lima/roles/deb_prerequisites/tasks/main.yaml`:
- Around line 86-91: The task named "Restart chronyd" uses
ansible.builtin.systemd_service with name set to "chronyd" which is incorrect on
Debian/Ubuntu; change the service name to "chrony" (i.e., update the
ansible.builtin.systemd_service task in the "Restart chronyd" task so name:
chrony, keeping state: restarted, enabled: true and the when: chronycfg.changed
condition intact) so the unit matches Debian/Ubuntu's chrony.service.
In `@lima/roles/deb_prerequisites/templates/pgedge-old.sources.tmpl`:
- Around line 1-5: The apt source template pgedge-old.sources.tmpl currently
contains the unsafe token "Trusted: yes"; remove that line and instead add a
"Signed-By" entry that references the repository key installed by the pgEdge
release package (e.g., Signed-By: pgedge-archive-keyring.gpg), ensuring the repo
uses apt-secure signature verification; update the template so the block reads
the same (Types/Suites/Components/URIs) but replaces "Trusted: yes" with the
Signed-By directive and confirm the release package provides the named keyring.
In `@server/internal/orchestrator/systemd/memory_test.go`:
- Line 14: The require.Equal call in memory_test.go currently passes the actual
value first and the expected value second; update the call in the test (the
require.Equal invocation) to use the correct Testify argument order by swapping
the second and third parameters so that expected is passed before actual (i.e.,
require.Equal(t, expected, out)).
In `@server/internal/orchestrator/systemd/patroni_unit.go`:
- Around line 17-18: PatroniUnitOptions currently only accepts the database
owner's UID; update its signature to also accept the numeric GID (e.g.,
databaseOwnerGID / ownerGID) and propagate that value into the generated systemd
options by adding a "Group" unit option alongside the existing "User" option (in
the code that constructs the []*unit.UnitOption returned by PatroniUnitOptions).
Ensure the new parameter type matches the existing owner UID type, update all
callers to pass the computed OwnerGID, and set the systemd directive to the
numeric GID so the unit runs with the configured group.
In `@server/internal/orchestrator/systemd/provide.go`:
- Around line 79-99: The getOSFamily function currently scans /etc/os-release
but doesn't check for scanning errors; after the for scanner.Scan() loop add a
check of scanner.Err() and if non-nil return OSFamilyUnknown (or empty) with
that error so I/O errors aren't silently ignored; update references in
getOSFamily to propagate the scanner error instead of always returning nil.
---
Nitpick comments:
In `@lima/roles/deb_prerequisites/tasks/main.yaml`:
- Around line 92-95: The task "Install delve debugger" currently uses
ansible.builtin.command to run "/usr/local/go/bin/go install
github.com/go-delve/delve/cmd/dlv@latest", which makes provisioning
non-reproducible; update that command to pin Delve to the v1.25.2 tag (e.g.
replace "@latest" with "@v1.25.2") so the task named "Install delve debugger"
and the creates check "/root/go/bin/dlv" always install a known compatible Delve
build for Go 1.25.x.
In `@packaging/rpm/postinstall.sh`:
- Line 3: The if test uses an unquoted positional parameter: change the
conditional in the postinstall script from using $1 unquoted to a quoted form
(e.g., "$1") so the shell handles empty or special values safely and satisfies
ShellCheck; update the if line in packaging/rpm/postinstall.sh where the snippet
reads if [ $1 -ge 1 ]; then to use the quoted positional parameter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18b28578-4b07-4127-a687-554338d082cb
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (46)
.goreleaser.yamlMakefileNOTICE.txtdocs/development/running-locally.mddocs/installation/configuration.mddocs/installation/systemd.mde2e/whole_cluster_test.gogo.modlima/Makefilelima/ansible.cfglima/debian-13-template.yamllima/deploy.yamllima/rocky-9-template.yamllima/roles/deb_prerequisites/tasks/main.yamllima/roles/deb_prerequisites/templates/pgedge-old.sources.tmpllima/roles/deb_prerequisites/vars/main.yamllima/roles/rhel_prerequisites/tasks/main.yamllima/roles/rhel_prerequisites/vars/main.yamllima/roles/start_vms/tasks/main.yamllima/ubuntu-24.04-template.yamllima/vars.yamlpackaging/deb/postinstall.shpackaging/preremove.shpackaging/rpm/postinstall.shserver/internal/config/config.goserver/internal/orchestrator/common/pgbackrest_config.goserver/internal/orchestrator/swarm/orchestrator.goserver/internal/orchestrator/swarm/pgbackrest_config.goserver/internal/orchestrator/systemd/apt.goserver/internal/orchestrator/systemd/apt_test.goserver/internal/orchestrator/systemd/dnf.goserver/internal/orchestrator/systemd/dnf_test.goserver/internal/orchestrator/systemd/golden_test/TestUnitOptions/PatroniUnitOptions/cpu_limit.serviceserver/internal/orchestrator/systemd/golden_test/TestUnitOptions/PatroniUnitOptions/fractional_cpu_limit.serviceserver/internal/orchestrator/systemd/golden_test/TestUnitOptions/PatroniUnitOptions/memory_max.serviceserver/internal/orchestrator/systemd/golden_test/TestUnitOptions/PatroniUnitOptions/minimal.serviceserver/internal/orchestrator/systemd/memory.goserver/internal/orchestrator/systemd/memory_test.goserver/internal/orchestrator/systemd/orchestrator.goserver/internal/orchestrator/systemd/packages.goserver/internal/orchestrator/systemd/packages_test.goserver/internal/orchestrator/systemd/patroni_unit.goserver/internal/orchestrator/systemd/provide.goserver/internal/orchestrator/systemd/unit_options_test.goserver/internal/pgbackrest/config.goserver/internal/pgbackrest/config_test.go
💤 Files with no reviewable changes (2)
- NOTICE.txt
- go.mod
jason-lynch
force-pushed
the
feat/PLAT-579/debian-support-ii
branch
3 times, most recently
from
925a7ef to
a703265
Compare
Adds package scripts to manage the control plane unit during upgrades and removal. The upgrade scriptlet restarts the control plane service after the upgrade and the removal scriptlet stops and disables the control plane before the control plane package is removed.
We removed the replica check from this test to fix an incompatibility with spock 5.0.7. This commit re-adds that check using polling to wait for the replica to catch up rather than calling `wait_for_sync_event`.
This package does not require CGO on Linux, but it does require it on macOS. This causes problems with debugging tests inside the the systemd package. This commit adds a simple mechanism to read memory information from `/proc/meminfo`, which will work on all Linux distributions.
pgBackRest uses this directory to prevent multiple pgBackRest instances from operating on the database at the same time. The default lock directory is `/tmp/pgbackrest`, so multiple instances on the same host could interfere with each other. This was not an issue in Swarm because `/tmp` is an ephemeral directory inside the container, but for systemd clusters this is an issue. This commit resolves this issue by computing an instance-specific lock directory when we're generating the pgBackRest configuration.
Unlike the RPM packages, the Postgres deb packages do not use a well known UID or GID for the postgres user. Instead they use the next available UID/GID, and the GID does not necessarily match the UID. This commit makes it so that we look up the UID and GID of the postgres user at runtime in systemd clusters. It also adds a separate override configuration for the database owner GID. This does also remove the default database owner UID from the config file, but it retains the current behavior for Swarm by moving the default value into the Swarm orchestrator package. PLAT-579
Adds support for Debian-based operating systems by adding an new `PackageManager` implementation for `apt` as well as a mechanism to detect the OS family at startup. PLAT-579
Adds the ability to deploy either Ubuntu 24.04 or Debian 13 when running `make dev-lima-deploy`: ``` make dev-lima-deploy DEV_LIMA_OS=ubuntu-24.04 ``` Or ``` make dev-lima-deploy DEV_LIMA_OS=debian-13 ``` PLAT-579
Adds a `.deb` package to our GitHub releases. PLAT-579
Adds the new `database_owner_gid` field to the configuration guide and updates the description for `database_owner_uid` to match the new behavior. PLAT-579
jason-lynch
force-pushed
the
feat/PLAT-512/update-database-version-ii
branch
from
d2aa2c0 to
8c65137
Compare
jason-lynch
force-pushed
the
feat/PLAT-579/debian-support-ii
branch
2 times, most recently
from
5fdbc15 to
da5f81c
Compare
Adds instructions for installing the Control Plane for systemd onto debian-based hosts. PLAT-579
jason-lynch
force-pushed
the
feat/PLAT-579/debian-support-ii
branch
from
da5f81c to
a97d5b3
Compare
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
1 participant
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.
Summary
This PR adds support for Debian-based Linux distributions, such as Debian and Ubuntu. After this PR, we will start publishing a
.debfile in our releases that users can download and install, as they can with the.rpmfile we currently publish.Changes
This PR is split into granular commits. I highly recommend reviewing the commits separately rather than looking at the entire diff at once. Some of the more notable commits are:
feat: debian supportdocs: add deb instructions to systemd docdebpackages.Testing
Notes for Reviewers
PLAT-579