From a764cfacbc6e5e5b79b8c1640db8b91323f5577d Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Mon, 28 Sep 2026 14:47:55 +0200 Subject: [PATCH] Find the PostgreSQL a machine has, rather than assume its names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit server init --postgres failed on Rocky with postgresql-setup not found. The PostgreSQL project's packages (PGDG) provide "postgresql-server", so dnf can satisfy that name with postgresql16-server — whose unit is postgresql-16.service, whose tools are in /usr/pgsql-16/bin and whose setup script is postgresql-16-setup. The code assumed the distribution's names for all three. The server unit is now picked from systemd's postgresql* unit files, its data directory read from the unit's own PGDATA, and initdb run directly as postgres when that directory is empty, with password logins over TCP. psql, initdb and the restart use what was found. The unit names and PGDATA values in the tests are the ones Rocky's and PGDG's packages ship. Not yet handled: a server initialised with ident for TCP logins, one on a port other than 5432, and several PGDG versions where the highest is not the one on 5432. Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/cli/serverinit.go | 1 + internal/cli/serverinit_postgres.go | 227 ++++++++++++++++++----- internal/cli/serverinit_postgres_test.go | 35 ++++ 3 files changed, 214 insertions(+), 49 deletions(-) diff --git a/internal/cli/serverinit.go b/internal/cli/serverinit.go index eddbbf1..a758640 100644 --- a/internal/cli/serverinit.go +++ b/internal/cli/serverinit.go @@ -72,6 +72,7 @@ type serverInit struct { // pgSetup prepares PostgreSQL and returns the store address; a seam for // tests. Nil does it for real. pgSetup func(context.Context) (string, error) + pg pgServer out io.Writer diff --git a/internal/cli/serverinit_postgres.go b/internal/cli/serverinit_postgres.go index 68ea5e3..138cd27 100644 --- a/internal/cli/serverinit_postgres.go +++ b/internal/cli/serverinit_postgres.go @@ -9,6 +9,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "time" @@ -194,6 +195,100 @@ func setAuthStore(path, address string) error { return os.WriteFile(path, []byte(strings.Join(lines, "\n")), info.Mode().Perm()) } +// pgServer is the PostgreSQL installation on this machine, as systemd and the +// file system describe it. +// +// Found rather than assumed. A distribution's own package calls its unit +// postgresql.service and puts initdb and psql on PATH; the PostgreSQL +// project's packages (PGDG) call it postgresql-16.service and keep them in +// /usr/pgsql-16/bin — and they provide "postgresql-server" too, so asking dnf +// for that name can install or find one of them. Assuming the first found a +// Rocky machine with the second unable to initialise the database it had. +type pgServer struct { + unit string // postgresql.service, postgresql-16.service + pgdata string // "" when the unit does not say, which is Debian's case + bin string // directory holding initdb and psql; "" means PATH +} + +func (p pgServer) tool(name string) string { + if p.bin != "" { + return filepath.Join(p.bin, name) + } + return name +} + +// pickPostgresUnit chooses the server unit from systemctl's list of +// postgresql* unit files: the plain one when there is one, else the highest +// version. Templates (postgresql@.service) are not a server by themselves. +func pickPostgresUnit(list string) string { + best, bestVersion := "", -1 + for _, line := range strings.Split(list, "\n") { + f := strings.Fields(line) + if len(f) == 0 { + continue + } + unit := f[0] + if !strings.HasSuffix(unit, ".service") || strings.Contains(unit, "@") { + continue + } + name := strings.TrimSuffix(unit, ".service") + if name == "postgresql" { + return unit + } + v := strings.TrimPrefix(name, "postgresql-") + if n, err := strconv.Atoi(strings.SplitN(v, ".", 2)[0]); err == nil && n > bestVersion { + best, bestVersion = unit, n + } + } + return best +} + +// pgdataFrom reads PGDATA out of `systemctl show -p Environment --value`. +func pgdataFrom(env string) string { + for _, kv := range strings.Fields(env) { + if v, ok := strings.CutPrefix(kv, "PGDATA="); ok { + return v + } + } + return "" +} + +func findPostgres(ctx context.Context) (pgServer, bool) { + out, _ := exec.CommandContext(ctx, "systemctl", "list-unit-files", "--type=service", + "--no-legend", "--plain", "postgresql*").Output() + unit := pickPostgresUnit(string(out)) + if unit == "" { + return pgServer{}, false + } + p := pgServer{unit: unit} + env, _ := exec.CommandContext(ctx, "systemctl", "show", "-p", "Environment", "--value", unit).Output() + p.pgdata = pgdataFrom(string(env)) + + if v, ok := strings.CutPrefix(strings.TrimSuffix(unit, ".service"), "postgresql-"); ok { + if dir := "/usr/pgsql-" + v + "/bin"; isDir(dir) { + p.bin = dir + } + if p.pgdata == "" && isDir("/var/lib/pgsql/"+v) { + p.pgdata = "/var/lib/pgsql/" + v + "/data" + } + } + if p.bin == "" { + if _, err := exec.LookPath("psql"); err != nil { + // Debian keeps the server's tools per version and puts only + // wrappers on PATH; without even those, look where it keeps them. + if dirs, _ := filepath.Glob("/usr/lib/postgresql/*/bin"); len(dirs) > 0 { + p.bin = dirs[len(dirs)-1] + } + } + } + return p, true +} + +func isDir(p string) bool { + st, err := os.Stat(p) + return err == nil && st.IsDir() +} + // setupPostgres makes sure a PostgreSQL on this machine holds a database for // llmgw, installing the server if there is none, and returns the store // address. @@ -202,11 +297,11 @@ func (in *serverInit) setupPostgres(ctx context.Context) (string, error) { if err != nil { return "", err } - if err := waitForPostgres(ctx); err != nil { + if err := in.waitForPostgres(ctx); err != nil { return "", err } - exists, err := psqlBool(ctx, "SELECT 1 FROM pg_roles WHERE rolname = '"+pgRole+"'") + exists, err := in.psqlBool(ctx, "SELECT 1 FROM pg_roles WHERE rolname = '"+pgRole+"'") if err != nil { return "", err } @@ -223,7 +318,7 @@ func (in *serverInit) setupPostgres(ctx context.Context) (string, error) { if exists { verb = "ALTER ROLE" } - if _, err := psql(ctx, verb+" "+pgRole+" WITH LOGIN PASSWORD '"+pass+"'"); err != nil { + if _, err := in.psql(ctx, verb+" "+pgRole+" WITH LOGIN PASSWORD '"+pass+"'"); err != nil { return "", err } dsn := fmt.Sprintf("postgres://%s:%s@127.0.0.1:5432/%s?sslmode=disable", pgRole, pass, pgDatabase) @@ -233,14 +328,14 @@ func (in *serverInit) setupPostgres(ctx context.Context) (string, error) { in.done("PostgreSQL role %s, its address in %s", pgRole, in.dsnPath()) } - exists, err = psqlBool(ctx, "SELECT 1 FROM pg_database WHERE datname = '"+pgDatabase+"'") + exists, err = in.psqlBool(ctx, "SELECT 1 FROM pg_database WHERE datname = '"+pgDatabase+"'") if err != nil { return "", err } if exists { in.kept("PostgreSQL database %s", pgDatabase) } else { - if _, err := psql(ctx, "CREATE DATABASE "+pgDatabase+" OWNER "+pgRole); err != nil { + if _, err := in.psql(ctx, "CREATE DATABASE "+pgDatabase+" OWNER "+pgRole); err != nil { return "", err } in.done("PostgreSQL database %s", pgDatabase) @@ -256,14 +351,14 @@ func (in *serverInit) setupPostgres(ctx context.Context) (string, error) { "ALTER SYSTEM SET shared_buffers = '96MB'", "ALTER SYSTEM SET max_connections = 40", } { - if _, err := psql(ctx, q); err != nil { + if _, err := in.psql(ctx, q); err != nil { return "", err } } - if out, err := exec.CommandContext(ctx, "systemctl", "restart", "postgresql").CombinedOutput(); err != nil { + if out, err := exec.CommandContext(ctx, "systemctl", "restart", in.pg.unit).CombinedOutput(); err != nil { return "", fmt.Errorf("restarting PostgreSQL: %v: %s", err, strings.TrimSpace(string(out))) } - if err := waitForPostgres(ctx); err != nil { + if err := in.waitForPostgres(ctx); err != nil { return "", err } in.done("PostgreSQL sized for a machine it shares (shared_buffers 96MB, 40 connections)") @@ -273,7 +368,8 @@ func (in *serverInit) setupPostgres(ctx context.Context) (string, error) { address := "@" + in.dsnPath() s, err := store.Open(ctx, address) if err != nil { - return "", fmt.Errorf("connecting with %s: %w", in.dsnPath(), err) + return "", fmt.Errorf("connecting with the address in %s: %w (PostgreSQL's pg_hba.conf "+ + "must allow password logins from 127.0.0.1: scram-sha-256 or md5, not ident)", in.dsnPath(), err) } _ = s.Close() return address, nil @@ -294,52 +390,85 @@ func (in *serverInit) writeDSN(dsn string) error { return in.chown(in.dsnPath(), true) } -// installPostgres installs the server when this machine has none, and says -// whether it did. +// installPostgres installs the server when this machine has none, makes sure +// its database is initialised and running, and says whether it installed it. func (in *serverInit) installPostgres(ctx context.Context) (bool, error) { - if exec.CommandContext(ctx, "systemctl", "cat", "postgresql.service").Run() == nil { - in.kept("PostgreSQL server") - return false, nil - } host := discovery.DiscoverHost() - family := host.Family() - if family == "" { - return false, fmt.Errorf("no PostgreSQL here and no package manager init knows (%s); "+ - "install PostgreSQL and run init again", host.OSID) - } - fmt.Fprintf(in.out, " .. installing PostgreSQL (a minute or two)\n") - pi := &provision.Installer{Host: host, Log: slog.Default()} - if err := pi.InstallPackages(ctx, map[string][]string{ - "deb": {"postgresql"}, - "rpm": {"postgresql-server", "postgresql"}, - }); err != nil { - return false, err - } - - if family == "rpm" { - // Debian's package creates and starts a cluster; RHEL's leaves an - // empty data directory. Password authentication over TCP is asked - // for, because RHEL's default for it is ident, which no service - // account passes. - if _, err := os.Stat("/var/lib/pgsql/data/PG_VERSION"); err != nil { - cmd := exec.CommandContext(ctx, "postgresql-setup", "--initdb") - cmd.Env = append(os.Environ(), "PGSETUP_INITDB_OPTIONS=--auth-host=scram-sha-256 --auth-local=peer") - if out, err := cmd.CombinedOutput(); err != nil { - return false, fmt.Errorf("postgresql-setup --initdb: %v: %s", err, strings.TrimSpace(string(out))) + pg, found := findPostgres(ctx) + installed := false + if found { + in.kept("PostgreSQL server (%s)", pg.unit) + } else { + if host.Family() == "" { + return false, fmt.Errorf("no PostgreSQL here and no package manager init knows (%s); "+ + "install PostgreSQL and run init again", host.OSID) + } + fmt.Fprintf(in.out, " .. installing PostgreSQL (a minute or two)\n") + pi := &provision.Installer{Host: host, Log: slog.Default()} + if err := pi.InstallPackages(ctx, map[string][]string{ + "deb": {"postgresql"}, + "rpm": {"postgresql-server", "postgresql"}, + }); err != nil { + return false, err + } + if pg, found = findPostgres(ctx); !found { + q := "rpm -qa 'postgresql*'" + if host.Family() == "deb" { + q = "dpkg -l 'postgresql*'" } + have, _ := exec.CommandContext(ctx, "sh", "-c", q).CombinedOutput() + return false, fmt.Errorf("PostgreSQL was installed but no postgresql service unit "+ + "appeared; what is installed:\n%s", strings.TrimSpace(string(have))) + } + installed = true + in.done("PostgreSQL installed (%s)", pg.unit) + } + in.pg = pg + + // Debian's package creates and starts a cluster itself. The RHEL family's, + // and PGDG's, leave the data directory empty until someone runs initdb. + if pg.pgdata != "" { + if _, err := os.Stat(filepath.Join(pg.pgdata, "PG_VERSION")); err != nil { + // Directly rather than through postgresql-setup, which is named + // differently by every packager. Password logins over TCP are + // asked for: RHEL's default for them is ident, which no service + // account passes. + if err := os.MkdirAll(pg.pgdata, 0o700); err != nil { + return false, err + } + if out, err := exec.CommandContext(ctx, "chown", "postgres:postgres", pg.pgdata).CombinedOutput(); err != nil { + return false, fmt.Errorf("chown %s: %v: %s", pg.pgdata, err, strings.TrimSpace(string(out))) + } + cmd := exec.CommandContext(ctx, "runuser", "-u", "postgres", "--", pg.tool("initdb"), + "-D", pg.pgdata, "--auth-host=scram-sha-256", "--auth-local=peer") + cmd.Dir = "/" + if out, err := cmd.CombinedOutput(); err != nil { + return false, fmt.Errorf("initdb %s: %v: %s", pg.pgdata, err, lastLines(out, 5)) + } + in.done("PostgreSQL database cluster initialised in %s", pg.pgdata) } } - if out, err := exec.CommandContext(ctx, "systemctl", "enable", "--now", "postgresql").CombinedOutput(); err != nil { - return false, fmt.Errorf("starting PostgreSQL: %v: %s", err, strings.TrimSpace(string(out))) + if exec.CommandContext(ctx, "systemctl", "is-active", "--quiet", pg.unit).Run() != nil { + if out, err := exec.CommandContext(ctx, "systemctl", "enable", "--now", pg.unit).CombinedOutput(); err != nil { + return false, fmt.Errorf("starting %s: %v: %s", pg.unit, err, strings.TrimSpace(string(out))) + } + in.done("%s started", pg.unit) } - in.done("PostgreSQL installed and started") - return true, nil + return installed, nil +} + +func lastLines(b []byte, n int) string { + lines := strings.Split(strings.TrimSpace(string(b)), "\n") + if len(lines) > n { + lines = lines[len(lines)-n:] + } + return strings.Join(lines, "\n") } // psql runs one statement as the postgres superuser, over the local socket. -func psql(ctx context.Context, query string) (string, error) { +func (in *serverInit) psql(ctx context.Context, query string) (string, error) { cmd := exec.CommandContext(ctx, "runuser", "-u", "postgres", "--", - "psql", "-v", "ON_ERROR_STOP=1", "-qAtX", "-c", query) + in.pg.tool("psql"), "-v", "ON_ERROR_STOP=1", "-qAtX", "-c", query) // postgres cannot enter root's home, and psql says so on every call. cmd.Dir = "/" out, err := cmd.CombinedOutput() @@ -351,17 +480,17 @@ func psql(ctx context.Context, query string) (string, error) { return strings.TrimSpace(string(out)), nil } -func psqlBool(ctx context.Context, query string) (bool, error) { - out, err := psql(ctx, query) +func (in *serverInit) psqlBool(ctx context.Context, query string) (bool, error) { + out, err := in.psql(ctx, query) return out == "1", err } // waitForPostgres waits for a server that was just started or restarted to // take connections. -func waitForPostgres(ctx context.Context) error { +func (in *serverInit) waitForPostgres(ctx context.Context) error { deadline := time.Now().Add(60 * time.Second) for { - _, err := psql(ctx, "SELECT 1") + _, err := in.psql(ctx, "SELECT 1") if err == nil { return nil } diff --git a/internal/cli/serverinit_postgres_test.go b/internal/cli/serverinit_postgres_test.go index d4563fd..66d5b2b 100644 --- a/internal/cli/serverinit_postgres_test.go +++ b/internal/cli/serverinit_postgres_test.go @@ -166,3 +166,38 @@ func TestInitWillNotMoveOnePostgresIntoAnother(t *testing.T) { t.Fatalf("expected a refusal, got %v\n%s", err, out) } } + +// The lists below are what `systemctl list-unit-files --type=service +// --no-legend --plain 'postgresql*'` prints on each kind of machine. +func TestPickPostgresUnit(t *testing.T) { + for _, c := range []struct{ name, list, want string }{ + {"rocky, distribution package", + "postgresql.service disabled disabled\npostgresql@.service disabled disabled\n", "postgresql.service"}, + {"rocky, PGDG package", + "postgresql-16.service disabled disabled\n", "postgresql-16.service"}, + {"two PGDG versions", + "postgresql-13.service enabled disabled\npostgresql-16.service disabled disabled\n", "postgresql-16.service"}, + {"debian", + "postgresql.service enabled enabled\npostgresql@.service indirect enabled\n", "postgresql.service"}, + {"only the template", "postgresql@.service indirect enabled\n", ""}, + {"none", "", ""}, + } { + if got := pickPostgresUnit(c.list); got != c.want { + t.Errorf("%s: got %q, want %q", c.name, got, c.want) + } + } +} + +// What `systemctl show -p Environment --value` prints for each unit, from +// the unit files the packages ship. +func TestPGDATAFromTheUnit(t *testing.T) { + for env, want := range map[string]string{ + "PG_OOM_ADJUST_FILE=/proc/self/oom_score_adj PG_OOM_ADJUST_VALUE=0 PGDATA=/var/lib/pgsql/data": "/var/lib/pgsql/data", + "PGDATA=/var/lib/pgsql/16/data/ PG_OOM_ADJUST_FILE=/proc/self/oom_score_adj": "/var/lib/pgsql/16/data/", + "": "", + } { + if got := pgdataFrom(env); got != want { + t.Errorf("%q: got %q, want %q", env, got, want) + } + } +}