Files
spinifex/cmd/rds-agent/engine_postgres.go
T
Ben Duncan 18853cef31 fix: close the four defects keeping the e2e nightly red (#1007)
* fix: stop the e2e analyser inventing failures and let a detach outlast its seal

go-junit-report emits <error message="No test result found"> for a === RUN with
no matching result line, and attaches the test's own captured log as the body.
The analyser counted those as failures, so the baremetal cell reported 25
failures against a junit file recording 1, with log output reading as
assertions. Separate the marker from a real error in both the parse and the
roll-up, count the unresolved cases and say so in the summary.

The gateway gave a detach 30s while the daemon budgets 180s for the ebs.unmount
seal and 180s more for the QMP unplug chain, so any detach whose seal replayed a
WAL was reported as ServerInternal while it went on to succeed.

Also switch four nightly job gates from always() to !cancelled(): always()
overrides cancellation, so a cancelled run kept starting jobs.

* fix: serialise the e2e workflows on one group and retry a dropped libvirt

e2e.yml had no concurrency block at all, so every push to dev could provision
alongside every other push and alongside the nightly. All four workflows that
build on these hypervisors now share e2e-hypervisors, queued rather than
cancelled. A per-environment group is not expressible: GitHub picks the group
before it picks a runner, and the environment is a property of the runner.

tofu apply also failed outright on a transient libvirt connect, which fails a
45-minute job in 90 seconds and reports it as a test failure. retry_run retries
only when that attempt's own output names a connection failure, so a real
failure still fails once.

Also collect the unfiltered journal from the matrix cells and the node journal
from cell-21: the unit filter drops the kernel and OOM lines that explain why
one node behaved unlike its siblings, and a passing run's journal is what makes
a failing one readable.

* fix: build the baremetal junit from the unfiltered suite log

run-baremetal-e2e.sh streams the full suite output to suite-output.log and a
grep-filtered summary to its stdout, which the workflow tees into
test-baremetal.log. That filter anchors --- PASS at column 0 and Go indents
every subtest result by four spaces, so the junit was generated from a log with
no subtest results in it: go-junit-report saw each === RUN with no outcome and
emitted <error message="No test result found">.

17 of cell-28's 39 testcases and 93 of cell-20's 120 were that. Regenerating
cell-28 from suite-output.log gives 39 tests, 0 failures, 0 errors.

* fix: converge released EIPs instead of trusting a fire-and-forget teardown

A disassociate leaves the ENI running, so the orphan sweep's owning-port test
never fires for it. The teardown then rests entirely on vpc.delete-nat, which is
core NATS and is lost whenever vpcd is restarting — and the intent KV watch is
UpdatesOnly, so the same window hides the write that would have driven a repair
pass. The dnat_and_snat row and its host /32 route to a released public address
survived until something else happened to touch them, which for a live ENI was
nothing at all.

Give the sweep the second test it was missing: AddEIP is the only writer of
dnat_and_snat, so the floating IPs intent asks for are the complete set of
addresses that may hold one, and a row for any other address is stale however
alive its port. Ports and external IPs travel together in LiveEIPs, both unioned
from the start-of-pass snapshot and the fresh re-read, so a mid-pass launch is
still spared. The host unbind now runs even when the row had already gone, which
is the state a crash between the OVN delete and the unbind leaves.

Run that sweep on the startup pass too. Startup is the pass that follows the
blind window, so skipping it deferred repair of exactly the changes that were
missed. The port-group and ENI-port sweeps stay gated there for the in-flight
create they were written for; the EIP sweep re-reads intent immediately before
deciding and abandons the whole sweep if that read fails, which is the same
protection by a stronger route.

* fix: keep a release verify off the hypervisors an e2e run is using

release.yml is the fifth workflow that provisions on the ci-single pool and had
no concurrency block at all, so a tag push could land a tofu apply on the same
libvirt host a nightly cell was mid-provision on. The group goes on the verify
job rather than the workflow: the build jobs touch no hypervisor and should not
queue behind a nightly.

* fix: stop cell-27 handing a 50-minute suite a 50-minute job

The rds suite is budgeted 50m in suite-sets.sh and cell-27 took the matrix
default of 50 minutes for the whole job, so provisioning, install, the suite and
teardown had to fit inside the suite's allowance alone. It never could: the last
measured run spent 6 minutes reaching the suite and was killed 44 minutes into
it, mid-TestAutomatedBackups. A job timeout reports "The operation was canceled"
and nothing else, which is why this cell has no verdict in any nightly rather
than a failure anyone could read.

cell-27 gets 75 minutes, and the select job now refuses to start a run whose job
budget does not clear its suite budget plus a setup allowance. The inner timeout
firing first is the property that matters: go test says where it stopped and
what failed, and the job timeout says nothing at all. cell-29 already had the
same fix applied by hand; nothing stopped the next suite from repeating it.

* fix: converge host EIP plumbing against intent, not against the NAT row

The released-EIP sweep landed and did not fire. Its premise was that a stranded
dnat_and_snat row is what a stranded host route can be found by, and on the path
that actually strands one there is no row left. AddEIP scrubs the predecessor
row sharing the private IP before installing the new one, so when reconcile
re-added the instance's auto-assigned address it silently deleted the released
EIP's row — and applyEIPs runs ahead of the sweep, which then walked a table
with nothing stale in it while the /32 route and its proxy-ARP stayed up.

Give the sweep the host's own inventory. ListEIPIngress reads the EIPs back from
the FORWARD accepts EnsureEIPIngress stamps with its own comment, so a binding is
found by what installed it rather than by a row some other path is free to
delete. Anything plumbed that intent no longer asks for is torn down, which also
covers a crash between the OVN delete and the unbind. A failed inventory fails
the pass: an empty list and an unreadable one are indistinguishable, and reading
the second as the first is a sweep that reports success having done nothing.

Close the window inside the pass too. The predecessor scrub now reads the row it
is about to drop and releases that address's host state, so the strand does not
exist between applyEIPs and the sweep rather than merely being repaired a step
later.

* fix: retry the release verify's libvirt calls like every other job does

The verify job drives the same libvirt provider on the same hypervisors as the
e2e cells, where a dropped connection under concurrent load is the steady state
and is retried. Here it was a hard failure that reads as a broken release. It
was skipped earlier only because the job checks out mulga and not spinifex, so
the shared helpers were not on disk — a sparse checkout of .github/scripts costs
nothing and makes apply and destroy behave like their e2e counterparts.

* fix: do not report a rebooting DB instance available on the pre-reboot agent's health

RegisterDBInstance stamped a fresh LastSeen while leaving Agent.EngineHealth at
the value the previous agent process reported. After a reboot the new agent
registers before it has probed anything, so the record read as a healthy engine
with a beat newer than the transition, and the reconciler moved the instance to
available while PostgreSQL was still starting up. A client that connects the
moment the status turns green then gets "the database system is starting up".

A register now discards the previous process's verdict and records when this
agent process started. The restart reconciler requires that registration to be
newer than the transition, which also closes the window where a beat lands
between the engine being stopped and the VM going down.

* test: record the credential kubectl sent when an EKS call comes back Unauthorized

The IRSA pod subtest's first kubectl is the gateway-CA configmap, which lands
before the pod dumps are registered, so a 401 there left no evidence at all. A
verbose read-only probe reports whether an Authorization header was sent, which
separates a token the apiserver rejected from one the exec plugin never made.

* fix: report a starting engine promptly, and budget CI jobs from the suite

The agent waited a full heartbeat interval before its first beat, so a restart
could not be observed as finished for up to 30 seconds after the engine was
already serving. Beat at startup instead, and report every 5 seconds until the
engine has answered once — the control plane is actively waiting on that beat,
and the steady-state cadence it hands back is for a database already serving.

The rds suite is the one most sensitive to what else the hypervisor is doing:
its DB VMs serialise behind a 4 GiB semaphore, so its wall clock is set by how
long it waits for the budget. The same pass ran 45m beside four nightly cells
and overran 50m beside nine, panicking with two tests still running. Raise its
budget to 60m.

That budget was then larger than the jobs meant to contain it. A job cancelled
on timeout reports only "The operation was canceled" and takes the artifacts
with it, so the inner `go test` timeout must always fire first — and it could
not for rds on e2e.yml, nor for storagefault or diskperf, which have asked for
90m and 180m inside a 60m job since they were added. Both workflows now resolve
their job budget from suite-sets.sh rather than carrying a literal: e2e.yml
from the largest requested suite, e2e-suites.yml per matrix leg, so a leg is
bounded by its own suite and not by the longest one beside it. The default push
set resolves to exactly the timeouts both workflows had before.

* fix: check a permutation cell's job budget against the suites it runs

The guard only saw cells that name a suite, so the nine that run a topology's
nightly subset were never checked against it — the same class of mistake the
guard exists to catch, in the majority of the cells. Bound each by the longest
suite in its subset: max() and not sum(), because whichever suite hangs must
panic on its own timeout first, and a sum would demand a budget the set has
never come close to using. Every cell clears it today.

* fix: report why a PostgreSQL engine would not start, and stop failing a stop that landed

Three defects, all found by the first rds pass that ever ran deep enough to
reach them.

A wait that saw an instance reach `failed` called t.Fatalf from inside the
polling condition, and testify runs that condition on its own goroutine. The
Goexit killed the tick and left the collector empty, which reads as the
condition being satisfied — so the wait returned successfully on an instance it
had just declared broken, logged "reached status available" about one that was
failed, and let the test carry on against it. Six conditions across four suites
call Fatal or require the same way. EventuallyErr now runs its condition on the
test's goroutine, which makes all six behave as written.

A DB VM stop has two drivers: the API call and the reconciler resuming one whose
caller it assumes has died. When they overlap the loser is told the VM is not in
a state to be stopped — which is the state the stop was heading for — and the
customer's StopDBInstance returned InvalidDBInstanceState for a stop that had
just completed. Both paths now treat a refused command the way they already
treat an unanswered one: it says nothing about the VM, so the fleet view
settles it.

PostgreSQL had no equivalent of the engine log tail MariaDB has carried since it
was added, so a postmaster that refuses a setting produced "engine did not
respond on /run/postgresql:5432" and nothing else — on a guest with no SSH,
whose console records only that pg_ctl could not start the server. The probe now
quotes the tail of the log the image already directs the postmaster to, and the
reboot and modify paths carry the agent's reason into the failure the way the
create path always has. The reason reaches the customer through StatusInfos.

The rds suite's budget goes to 90m and cell-27's job to 100m. Its wall clock is
what the DB-VM queue costs rather than what the tests do — single tests logged
11m, 22m and 26m waiting for budget — and the pass above still had three of
seventeen tests running at 60m.

* fix: keep a DB VM recoverable when its agent cannot start, and flush the volume before a snapshot

An rds-agent that failed to construct exited, so a transient IMDS credential
resolution took the guest down for good: rds-datadir and rds-init cascade on the
missing handoff, the engine never starts, and the control plane sees only silence
until it declares the instance failed ten minutes later. Construction is retried
like every other boot step instead.

OpenRC skips start_post when the start itself fails, which is the one case with
no diagnostic at all — the console carried nothing but "died". The log mirror
moves out of the handoff wait so a failed start uses it too.

A snapshot quiesced the engine and nothing else. The engine repairs its own files
from the WAL, but postgresql.conf sits beside them and it does not: a restore
came up on a torn tail and the postmaster stopped on a syntax error at the
unterminated include hook. The quiesce now flushes the whole data mount, and the
hook is renamed into place rather than appended, so a boot that finds a half
written one replaces it instead of appending after its open quote.

* fix: flush the config cloud-init wrote before a reset can take it

A DB instance rebooted seconds after it went available came back with no
RDS_GATEWAY_URL, so rds-agent could not reach the control plane and the engine
never started. cloud-init writes agent.env once, per-instance, and never again;
a reboot is a QMP system_reset with no guest shutdown, so the file was still
dirty page cache when the machine reset under it and the next boot read nothing.
The handoff wait now syncs, which is the point at which the agent has proved it
read that file.

* fix: stop dropping an EIP teardown published while vpcd is restarting

DisassociateAddress publishes vpc.delete-nat fire-and-forget. On run
34046316024 vpcd disconnected from NATS at 03:51:26.73 and its replacement
subscribed at 03:51:27.09; the publish landed at 03:51:26.92, in the gap, and
went nowhere. ReleaseAddress then returned the address to the pool two minutes
later with its /32 host route still delivering to the guest that had held it.

vpc.delete-nat now uses request-reply like vpc.add-nat, and retries only on
no-responders — which returns immediately, so the cost is paid solely when
nothing is listening. A teardown that still fails is logged as an error naming
the address rather than lost silently. Neither is fatal to the caller: the
reconciler's orphan sweep remains the backstop.

vpc.add-nat on the EIP path stays fire-and-forget. The shared helper blocks it
on the flows barrier for up to 45s, and associating an address should not
inherit that.

Two reconcile passes in that run had the address absent from intent and did not
sweep its host route, and the journal cannot say why: the sweep logged only when
it acted, so a route it kept was indistinguishable from one it never saw. It now
logs the bindings it found, kept and wanted.

* ci: render one run-level table for the e2e nightly

Reading a nightly meant opening nineteen jobs, and the Actions UI does not
separate a cell that failed from one cancelled out from under it — a
distinction every investigation of this suite has needed, and one the run
34046316024 analysis had to reach for the jobs API to get.

A final job, needs: every cell and if: always(), renders cell, topology,
result, duration, runner and per-cell test counts into the step summary, with
run-level totals, the three slowest cells and the failing test names.

Result, duration and runner come from the jobs API so a cell that died before
writing junit still gets a row. Counts come from a new junit-only artifact per
cell rather than the full cell artifact, so the roll-up does not pull nineteen
journals it has no use for.

Verified against run 34046316024: 14 success, 2 cancelled, 1 failure, with the
two cancellations shown as cancelled rather than folded into failure.

* fix: assert EC2's actual EIP semantics, and probe EKS auth on an authenticated path

natuplink asserted that an instance's auto-assigned public IP is still plumbed
after an EIP associated to the same instance is torn down. EC2 does the
opposite: associating an EIP releases the auto-assigned address back into the
pool, and it cannot be reused. Our datapath already does that — run 34059879666
shows 192.168.0.72's ingress removed at 07:09:44.000, 36ms before .73's was
installed — so the test was asserting behaviour neither AWS nor we have.

It now asserts the release at the point it happens, and that no public delivery
remains after the teardown. This surfaced only once the teardown itself started
working: the run's other assertions passed, including .73's removal, which was
the failure before.

Our records still disagree with our datapath — the ENI keeps the released
address, which is why termination tears the old one down twice and never the
current one. That is the remaining half of mulga-7zc9w and is not fixed here;
it needs an IPAM release on the associate path.

Separately, AuthDiagnostics read /version, which is anonymously readable, so its
200 was consistent with both branches it claimed to distinguish. It now runs
auth whoami, reads kube-system, and invokes the credential plugin directly,
reporting exit code, stderr and whether a token was produced — never the token,
since CI logs outlive it.

* test: capture per-guest state when a spread guest never answers SSH

TestMultinodeSpread's only failure dump is dumpPlacementNATDiag, which collects
OVN, OVS, ARP and route state aimed at the NAT and EIP half of the test. The
failure it actually hits is a guest that never accepted SSH — run 34040324968,
the guest on node3 at 10.100.2.5 — and for that it recorded no console, no port
binding and no tap state. There was no way to tell from the artifact whether the
guest booted at all.

Now dumps, per guest: the console, the NB logical switch port, the SB port
binding, the OVS interface backing its tap, and br-int's port list. Probed on
every node rather than the expected one, because a port bound on the wrong
chassis is exactly what a hosting-node-only dump cannot show.

Covers all three guests, not just the failing one, so a working guest is
available as the control.

* ci: sample hypervisor load either side of every cell

Eight runner names map onto three hypervisors, so cells that never share a
runner still share a host: when cell 10's SSH wait expired on run 34040324968,
cells 22, 17, 18 and 28 were concurrently live, two of them multi-node. Nothing
in the artifacts measured host load, which left co-tenancy as an explanation
that could be neither confirmed nor ruled out.

Records loadavg, MemAvailable, SwapFree, the cpu line carrying steal, and the
QEMU process count before and after each cell, into the cell artifact.

* ci: note that main's missing concurrency block is why the nightly runs ungrouped

Three refs carry three different groupings: main has no concurrency block at
all, dev uses e2e-nightly, this branch uses e2e-hypervisors. Scheduled runs load
the workflow from the default branch, so every cron nightly runs ungrouped and
cannot queue against anything — which is how a dispatch of 34050630627 ran
alongside the scheduled 34046316024 and cost it cells 24 and 27.

e2e-hypervisors is the right group and deliberately broader than dev's, since
e2e.yml, e2e-suites.yml and e2e-ddil.yml draw on the same runner pools. Nothing
on a feature branch can fix this; the block has to reach main.
2026-09-06 16:53:32 -05:00

554 lines
22 KiB
Go

package main
import (
"context"
"errors"
"fmt"
"log/slog"
"os"
"path/filepath"
"strconv"
"strings"
"time"
handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds"
)
// The engine is reached over its unix socket under peer authentication, so the
// agent drops to the postgres OS user rather than holding a password of its own.
type postgresEngine struct {
quiesceState
parameterManager
// The control plane's own metadata for this engine, resolved once at
// startup: the live password apply runs as the cluster superuser, so it
// re-checks the role name against the same reserved set the API validates.
meta handlers_rds.Engine
run commandRunner
startSess sessionRunner
psql string
rcService string
service string
pgData string
socketDir string
osUser string
}
var _ engine = (*postgresEngine)(nil)
// pg_isready gives PostgreSQL three states for free: exit 0 is serving, exit 1
// is a postmaster up and rejecting connections, anything else is an engine that
// is not there. Resolved on PATH, where the client package puts it.
const postgresProbeBinary = "pg_isready"
// The socket directory rather than loopback: the engine binds the customer ENI
// and nothing else, so there is no TCP path for the probe to take. It also keeps
// liveness independent of the network path, of the pg_hba scope and of the TLS
// enforcement rule — libpq reads a host beginning with / as a socket directory,
// and the generated `local ... peer` line covers it.
func newPostgresProbe(cfg config, run probeRunner) *engineProbe {
return newEngineProbe(cfg.EnginePort, postgresProbeState(cfg.SocketDir, cfg.EngineErrorLog, run))
}
func postgresProbeState(host, errorLog string, run probeRunner) probeStateFn {
return func(ctx context.Context, port int64) (engineState, string) {
portArg := strconv.FormatInt(port, 10)
code, _, err := run(ctx, postgresProbeBinary, "-h", host, "-p", portArg, "-q")
switch {
case err != nil:
// A missing binary or broken image. Reporting healthy on the strength of
// nothing would hide it, so this reads as absent like an engine that did
// not answer.
return engineAbsent, fmt.Sprintf("engine probe could not run: %v", err)
case code == 0:
return engineServing, ""
case code == 1:
return engineRecovering, withEngineLogTail(
"engine is rejecting connections (startup or recovery)", errorLog)
default:
// A postmaster that refused a setting never opens the socket, so this is
// the state a bad parameter lands in. The reason for it is only in the
// server's own log, and it reaches the customer through StatusInfos.
return engineAbsent, withEngineLogTail(
fmt.Sprintf("engine did not respond on %s:%s", host, portArg), errorLog)
}
}
}
// The include the resolved set is rendered to, and the copy of the last one the
// engine accepted. Both live beside the data rather than in /etc, matching
// rds-init: a class change boots a fresh root volume and would revert them.
const (
postgresParametersFile = "10-rds-parameters.conf"
// Deliberately not a .conf name: include_dir globs *.conf, so the rollback
// copy must not be read as a second set of settings.
postgresLastGoodFile = "10-rds-parameters.last-good"
)
// The pg_hba rule that implements TLS enforcement, and the directory the
// generated pg_hba includes it from. Its content is a constant, so the copy
// rds-init writes in shell and the one written here have no template to drift
// apart on: enforcement is the presence of the file and nothing more.
const (
postgresHBADir = "hba.d"
postgresForceSSLRuleFile = "20-rds-force-ssl.conf"
// An explicit reject rather than an omitted permit: both refuse the
// connection, and only this one puts the reason in the server log and in the
// client's error.
postgresForceSSLRules = `# Managed by the platform: the rule that requires TLS of client connections.
# Written by rds-init at boot and by rds-agent on a parameter apply.
hostnossl all all 0.0.0.0/0 reject
hostnossl all all ::/0 reject
`
// What the read-back must count once the file is in place. Asserting the shape
// rather than only the absence of parse errors is what catches the failure
// that will actually happen: the derivation above writing the wrong thing.
postgresForceSSLRuleCount = 2
)
// The layout's factory resolves the control plane metadata during startup.
func newPostgresEngineFromCatalog(cfg config, run commandRunner, startSess sessionRunner, probe *engineProbe) (engine, error) {
meta, err := handlers_rds.LookupEngine(enginePostgres)
if err != nil {
return nil, fmt.Errorf("this image bakes %s, which this build's control plane does not offer: %w", enginePostgres, err)
}
return newPostgresEngine(cfg, meta, run, startSess, probe), nil
}
func newPostgresEngine(cfg config, meta handlers_rds.Engine, run commandRunner, startSess sessionRunner, probe *engineProbe) *postgresEngine {
return &postgresEngine{
meta: meta,
run: run,
startSess: startSess,
psql: filepath.Join(cfg.EngineBinDir, "psql"),
rcService: cfg.RCService,
service: cfg.EngineService,
pgData: cfg.EngineDataDir,
socketDir: cfg.SocketDir,
osUser: cfg.EngineUser,
parameterManager: parameterManager{
probe: probe,
params: parameterStore{
dir: filepath.Join(cfg.EngineDataDir, "conf.d"),
installed: postgresParametersFile,
lastGood: postgresLastGoodFile,
osUser: cfg.EngineUser,
engine: enginePostgres,
},
repairTimeout: parameterRepairTimeout,
repairPoll: parameterRepairPoll,
},
}
}
// The role name and the password ride the environment and are re-quoted by
// psql, so neither reaches a shell word or an argv another process can read.
func (e *postgresEngine) SetPassword(ctx context.Context, username, password string) error {
if username == "" || password == "" {
return fmt.Errorf("set-password requires both %s and %s",
handlers_rds.CommandParamMasterUsername, handlers_rds.CommandParamMasterUserPassword)
}
// The statement below runs as the cluster superuser over the socket under
// peer auth, so a reserved name in the command payload would hand the
// customer the bootstrap superuser rather than rotate their own role.
if err := e.meta.ValidateUsernameNotReserved(username); err != nil {
return fmt.Errorf("refusing to set the password of a role the engine reserves: %w", err)
}
// psql interpolates the password into the ALTER ROLE before the server sees
// it, and these three are what would write it to the log — the last on any
// failure at its own default. SUSET, so the parameter group cannot win.
const sql = `SET log_statement = 'none';
SET log_min_duration_statement = -1;
SET log_min_error_statement = 'panic';
\getenv master RDS_MASTER_USERNAME
\getenv password RDS_MASTER_PASSWORD
ALTER ROLE :"master" WITH LOGIN PASSWORD :'password';
`
_, err := e.psqlRun(ctx, sql,
"RDS_MASTER_USERNAME="+username,
"RDS_MASTER_PASSWORD="+password,
)
if err != nil {
// psql echoes the failing statement, which here would carry the new
// password back to the control plane and into the event ring.
return fmt.Errorf("apply the master password: %s", redact(err.Error(), password))
}
return nil
}
// Installs the resolved set, validates it with the engine's own parser before
// the engine adopts it, and reloads. A refused value is rolled back here rather
// than left on the data volume, where it would survive every VM replace.
func (e *postgresEngine) ApplyParameters(ctx context.Context, params []handlers_rds.Parameter) ([]string, error) {
e.paramMu.Lock()
defer e.paramMu.Unlock()
initialState, _ := e.probe.state(ctx)
if initialState == engineRecovering {
return nil, errors.New("apply parameters while the engine is still starting or recovering")
}
if initialState == engineServing {
// A command can arrive before the first heartbeat. Seed the configuration
// currently being served before replacing its file in that window.
if err := e.recordServingParametersLocked(ctx); err != nil {
return nil, fmt.Errorf("record the parameters serving before the apply: %w", err)
}
}
// The check runs against the file in place, because the engine parses the
// datadir's own include_dir. The window that leaves is closed by the rollback
// below and the last-known-good restore the agent runs at boot.
restoreParameters, err := e.params.install(params)
if err != nil {
return nil, err
}
if err := e.checkConfig(ctx); err != nil {
return nil, errors.Join(fmt.Errorf("the engine rejected the parameter set: %w", err), restoreParameters())
}
state, _ := e.probe.state(ctx)
switch state {
case engineAbsent:
return e.restartOnRepairSetLocked(ctx)
case engineRecovering:
return nil, errors.Join(errors.New("the engine entered startup or recovery during the parameter apply"), restoreParameters())
}
// After the engine's own parser has passed the set and before the reload that
// adopts both files at once.
enforce, restoreEnforcement, err := e.syncTLSEnforcement(ctx, state)
if err != nil {
return nil, errors.Join(fmt.Errorf("apply the TLS enforcement of the parameter set: %w", err), restoreParameters())
}
// The two files carry one apply from here: a rejected set withdrawn without
// its enforcement rule would leave the instance serving the old parameters
// under the new set's TLS posture.
restore := func() error { return errors.Join(restoreEnforcement(), restoreParameters()) }
if err := e.reload(ctx); err != nil {
// The engine may have gone down after the first probe. In that case start
// it on the new, parser-checked repair set rather than restoring the set
// that already left it unable to serve.
if state, _ := e.probe.state(ctx); state == engineAbsent {
return e.restartOnRepairSetLocked(ctx)
}
return nil, errors.Join(err, restore())
}
if err := e.verifyTLSEnforcement(ctx, enforce); err != nil {
// The reload already happened, so putting the files back is not enough: a
// pg_hba that parsed but says the wrong thing has been adopted, and only a
// second reload takes the engine off it.
return nil, errors.Join(err, restore(), e.reload(ctx))
}
return e.pendingRestartParameters(ctx)
}
// Re-derived before the restart: the engine reads pg_hba when it starts, so a
// repair set started under the previous set's enforcement would serve one TLS
// posture while the API reported the other.
func (e *postgresEngine) restartOnRepairSetLocked(ctx context.Context) ([]string, error) {
if _, _, err := e.syncTLSEnforcement(ctx, engineAbsent); err != nil {
return nil, fmt.Errorf("apply the TLS enforcement of the repair set: %w", err)
}
return awaitRepairedEngine(ctx, e.probe, e.Restart, e.pendingRestartParameters, e.repairTimeout, e.repairPoll)
}
// The function's own answer is the reload, not psql's exit status: it returns f
// when it could not signal the postmaster, and the read-back that follows parses
// pg_hba from disk, so an unsignalled reload would otherwise verify clean.
func (e *postgresEngine) reload(ctx context.Context) error {
out, err := e.psqlRun(ctx, "SELECT pg_reload_conf();\n")
if err != nil {
return fmt.Errorf("reload the engine configuration: %w", err)
}
if result := strings.TrimSpace(out); result != "t" {
return fmt.Errorf("reload the engine configuration: pg_reload_conf() returned %q", result)
}
return nil
}
// The postmaster's own answer: a static setting it has stored but not adopted is
// what it reports here, so nothing in the guest has to classify one.
func (e *postgresEngine) pendingRestartParameters(ctx context.Context) ([]string, error) {
out, err := e.psqlRun(ctx, "SELECT name FROM pg_settings WHERE pending_restart ORDER BY name;\n")
if err != nil {
return nil, fmt.Errorf("read the settings pending a restart: %w", err)
}
var pending []string
for line := range strings.SplitSeq(out, "\n") {
if name := strings.TrimSpace(line); name != "" {
pending = append(pending, name)
}
}
return pending, nil
}
// Snapshots the include only when the running postmaster has no settings still
// pending a restart. The parameter mutex keeps an apply from replacing the file
// between that check and the copy.
func (e *postgresEngine) RecordServingParameters(ctx context.Context) error {
e.paramMu.Lock()
defer e.paramMu.Unlock()
return e.recordServingParametersLocked(ctx)
}
func (e *postgresEngine) recordServingParametersLocked(ctx context.Context) error {
return recordLastGood(ctx, e.params, e.pendingRestartParameters)
}
// Puts the last set the engine accepted back in place, for a restart that failed
// after a parameter change.
func (e *postgresEngine) RestoreLastKnownGoodParameters(ctx context.Context) (bool, error) {
e.paramMu.Lock()
defer e.paramMu.Unlock()
restored, err := restoreLastGood(ctx, e.params, e.probe)
if err != nil || !restored {
return restored, err
}
// A service restart does not re-run rds-init, so a restore that stopped at the
// parameter file would bring the last known good set up under the enforcement
// of the set that was just rejected. restoreLastGood only replaces the file of
// an engine that is down.
if _, _, err := e.syncTLSEnforcement(ctx, engineAbsent); err != nil {
return true, fmt.Errorf("apply the TLS enforcement of the restored parameter set: %w", err)
}
return true, nil
}
// Turns the installed enforcement value into the pg_hba fact that implements it.
// PostgreSQL has no server setting for this, so the parameter file on its own
// would report the change applied and enforce nothing.
//
// The serving-TLS guard runs only against an engine that is up. The restore and
// repair paths reach this with the engine down, where the rule is read by the
// start that follows rather than by a reload.
func (e *postgresEngine) syncTLSEnforcement(ctx context.Context, state engineState) (enforce bool, restore func() error, err error) {
enforce, err = installedTLSEnforcement(e.meta.TLSEnforcementParameter(), e.params.installedPath())
if err != nil {
return false, nil, err
}
if enforce && state == engineServing {
if err := e.requireServingTLS(ctx); err != nil {
return false, nil, err
}
}
restore, err = e.writeTLSEnforcement(enforce)
if err != nil {
return false, nil, err
}
return enforce, restore, nil
}
// Writes or removes the enforcement rule, returning a func putting back whatever
// was there. Removing matters as much as writing: a restored data volume carries
// hba.d with it, so a stale rule would keep enforcing under a parameter group
// that turns enforcement off.
func (e *postgresEngine) writeTLSEnforcement(enforce bool) (func() error, error) {
path := e.forceSSLRulePath()
previous, readErr := os.ReadFile(path)
if readErr != nil && !os.IsNotExist(readErr) {
return nil, fmt.Errorf("read the TLS enforcement rule at %s: %w", path, readErr)
}
existed := readErr == nil
restore := func() error {
if !existed {
return e.removeTLSEnforcement()
}
return e.params.write(path, previous)
}
if !enforce {
if err := e.removeTLSEnforcement(); err != nil {
return nil, err
}
return restore, nil
}
// rds-init lays the directory down on every boot, so this covers an agent that
// reached an apply before one ran rather than the ordinary case.
dir := filepath.Dir(path)
if err := os.MkdirAll(dir, 0o700); err != nil {
return nil, fmt.Errorf("create %s: %w", dir, err)
}
if err := e.params.chownToEngine(dir); err != nil {
return nil, err
}
if err := e.params.write(path, []byte(postgresForceSSLRules)); err != nil {
return nil, err
}
return restore, nil
}
func (e *postgresEngine) removeTLSEnforcement() error {
path := e.forceSSLRulePath()
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("withdraw the TLS enforcement rule at %s: %w", path, err)
}
return nil
}
func (e *postgresEngine) forceSSLRulePath() string {
return filepath.Join(e.pgData, postgresHBADir, postgresForceSSLRuleFile)
}
// Refuses to require TLS of clients from an engine that is not serving it, which
// would reject every connection while the parameter path reported success.
func (e *postgresEngine) requireServingTLS(ctx context.Context) error {
out, err := e.psqlRun(ctx, "SHOW ssl;\n")
if err != nil {
return fmt.Errorf("read whether the engine is serving TLS: %w", err)
}
if serving := strings.TrimSpace(out); serving != "on" {
return fmt.Errorf("the engine is serving without TLS (ssl = %q), so requiring it of clients would reject every connection", serving)
}
return nil
}
// PostgreSQL discards a pg_hba it cannot parse on reload and keeps the rules it
// is already serving, which presents as an apply that succeeded and enforces
// nothing. So the file is read back.
//
// pg_hba_file_rules re-parses from disk, so this proves the file would be
// adopted rather than that the postmaster has adopted it. The inference holds
// because a reloaded pg_hba is discarded only on a parse error.
const postgresHBAReadBack = `SELECT count(*) FILTER (WHERE type = 'hostnossl' AND auth_method = 'reject'),
count(*) FILTER (WHERE error IS NOT NULL) FROM pg_hba_file_rules;
`
func (e *postgresEngine) verifyTLSEnforcement(ctx context.Context, enforce bool) error {
out, err := e.psqlRun(ctx, postgresHBAReadBack)
if err != nil {
return fmt.Errorf("read back the client authentication rules: %w", err)
}
rules, unparsable, err := parseHBAReadBack(out)
if err != nil {
return err
}
if unparsable > 0 {
return fmt.Errorf("the client authentication file carries %d rules the engine could not parse, so it would keep the rules it is already serving", unparsable)
}
want := 0
if enforce {
want = postgresForceSSLRuleCount
}
if rules != want {
return fmt.Errorf("the client authentication file carries %d TLS enforcement rules, want %d", rules, want)
}
return nil
}
func parseHBAReadBack(out string) (rules, unparsable int, err error) {
answer := strings.TrimSpace(out)
malformed := fmt.Errorf("the client authentication read-back returned %q, want two counts", answer)
counts := strings.Split(answer, "|")
if len(counts) != 2 {
return 0, 0, malformed
}
parsed := make([]int, len(counts))
for i, count := range counts {
if parsed[i], err = strconv.Atoi(strings.TrimSpace(count)); err != nil {
return 0, 0, malformed
}
}
return parsed[0], parsed[1], nil
}
// The engine's own parser, run offline against the datadir. Reading one setting
// back is enough: postgres parses every include first and exits non-zero naming
// the file and line of an unknown parameter or an out-of-range value.
func (e *postgresEngine) checkConfig(ctx context.Context) error {
if _, err := e.run(ctx, command{
Name: filepath.Join(filepath.Dir(e.psql), "postgres"),
Args: []string{"-D", e.pgData, "-C", "shared_buffers"},
Env: []string{"PATH=" + defaultGuestPath},
User: e.osUser,
}); err != nil {
return err
}
return nil
}
func (e *postgresEngine) Stop(ctx context.Context) error {
return serviceAction(ctx, e.run, e.rcService, e.service, "stop")
}
// Only the parameter rollback calls this: a restart the control plane wants goes
// through RebootDBInstance, which cycles the VM.
func (e *postgresEngine) Restart(ctx context.Context) error {
return serviceAction(ctx, e.run, e.rcService, e.service, "restart")
}
// Feeds sql on stdin rather than as an argument, so a statement is never
// visible in the process table.
func (e *postgresEngine) psqlRun(ctx context.Context, sql string, env ...string) (string, error) {
return e.run(ctx, command{
Name: e.psql,
Args: e.psqlArgs(),
Env: append([]string{"PATH=" + defaultGuestPath}, env...),
Stdin: sql,
User: e.osUser,
})
}
// Reading the script from stdin is what lets one invocation serve both a
// one-shot run and a session held open across several statements.
func (e *postgresEngine) psqlArgs() []string {
return []string{
"--no-psqlrc", "--quiet", "--no-align", "--tuples-only",
"-v", "ON_ERROR_STOP=1",
"-h", e.socketDir,
"-p", strconv.FormatInt(e.probe.port.Load(), 10),
"-U", e.osUser,
"-d", "postgres",
"-f", "-",
}
}
// Puts the engine into backup mode: the datadir is checkpointed and the engine
// stops writing over the pages a snapshot is about to read. The hold is released
// by Unquiesce, or by its own deadline, whichever comes first.
func (e *postgresEngine) Quiesce(ctx context.Context, label string, hold time.Duration) error {
if err := validateQuiesceRequest(label, hold); err != nil {
return err
}
// Held across the whole start, so a second quiesce waits and then finds the
// first one's hold rather than opening a concurrent backup alongside it.
e.mu.Lock()
defer e.mu.Unlock()
if e.held != nil {
return fmt.Errorf("the engine is already quiesced for backup %s", e.held.label)
}
// Deliberately not the command's context: the session has to outlive the call
// that started it, and its own deadline is what bounds it instead.
session, err := e.startSess(context.WithoutCancel(ctx), command{
Name: e.psql,
Args: e.psqlArgs(),
Env: []string{"PATH=" + defaultGuestPath, "RDS_BACKUP_LABEL=" + label},
User: e.osUser,
SentinelStatement: `\echo ` + sessionSentinel + "\n",
})
if err != nil {
return fmt.Errorf("open a backup session: %w", err)
}
// fast forces an immediate checkpoint rather than spreading it over the
// checkpoint interval, which would hold the snapshot open for minutes.
const sql = `\getenv label RDS_BACKUP_LABEL
SELECT pg_backup_start(:'label', fast => true);
`
if err := session.Exec(ctx, sql); err != nil {
if closeErr := session.Close(); closeErr != nil {
slog.Warn("rds-agent: closing a failed backup session", "err", closeErr)
}
return fmt.Errorf("put the engine into backup mode: %w", err)
}
e.beginHoldLocked(label, session, hold)
return nil
}
func (e *postgresEngine) Unquiesce(ctx context.Context) error {
return e.releaseHold(ctx, "SELECT pg_backup_stop();\n")
}