Watch
0
0
Fork
You've already forked hyperhive
0

refresh-consumer: key the restart on the file's mtime, not a pre-write compare

The restart decision was a shell variable set by comparing the fetched
value with the file just before overwriting it. A run that wrote the
file and then failed before the restart (the matrix unit's registration
render, or `systemctl --machine` finding no bus yet) left a retry that
saw an unchanged file and never restarted the consumer.

The file is now written only when the value differs, so its mtime marks
the last real change, and `refresh_consumer <machine> <unit> <path>`
compares that mtime with the consumer's ActiveEnterTimestamp on every
run, the shape the openbao client-CA refresh in swarm-bao.nix already
uses. A consumer that started after the last change is left alone; a
running one is try-restarted, a failed one reset and started, all with
--no-block, and nothing happens while the container is down.

The helper's comment block also exceeded the 30-line limit
(`comment-block lint` failed on d871467d); its per-function notes now
sit beside the functions.

module-eval-bao-grants asserts the gated write, the path the refresh is
keyed on, and the mtime-vs-start comparison for each consumer.

Refs #4662
This commit is contained in:
atlas 2026-09-30 00:27:33 +02:00 • committed by mara
commit b68fd7306e
6 changed files with 73 additions and 58 deletions

View file

@ -234,11 +234,11 @@ in
exit 0 exit 0
fi fi
changed=0 if secret_differs ${lib.escapeShellArg (toString deployCfg.matrix.appserviceTokenFile)} "$token"; then
if secret_differs ${lib.escapeShellArg (toString deployCfg.matrix.appserviceTokenFile)} "$token"; then changed=1; fi atomic_write_secret 0600 "" ${lib.escapeShellArg (toString deployCfg.matrix.appserviceTokenFile)} "$token"
atomic_write_secret 0600 "" ${lib.escapeShellArg (toString deployCfg.matrix.appserviceTokenFile)} "$token" fi
# Re-stamp the registration file from the token just written. The # Re-stamp the registration file from the token on disk. The
# token is half an agreement — the registration the homeserver loads # token is half an agreement — the registration the homeserver loads
# has to carry the same value — so writing the file and stopping # has to carry the same value — so writing the file and stopping
# would leave the homeserver authenticating hive-c0re against # would leave the homeserver authenticating hive-c0re against
@ -253,9 +253,7 @@ in
# tuwunel loads the registration through `LoadCredential`, a copy # tuwunel loads the registration through `LoadCredential`, a copy
# taken at start, while hive-c0re reads the token file on every call: # taken at start, while hive-c0re reads the token file on every call:
# a changed token splits the two until the homeserver restarts. # a changed token splits the two until the homeserver restarts.
if [ "$changed" = 1 ]; then refresh_consumer ${lib.escapeShellArg matrixMachine} tuwunel.service ${lib.escapeShellArg (toString deployCfg.matrix.appserviceTokenFile)}
refresh_consumer ${lib.escapeShellArg matrixMachine} tuwunel.service
fi
''; '';
}; };
}) })

View file

@ -1,56 +1,64 @@
# Shared shell steps for a host oneshot that fetches a credential into a # Shared shell steps for a host oneshot that fetches a credential into a
# file a service inside a container loads at start (`LoadCredential`, or a # file a service inside a container loads at start (`LoadCredential`, or a
# config file expanded once while parsing). Such a service never sees a # config file expanded once while parsing). Such a service never sees a
# value that lands after it started — a late fetch or a rotation — until it # value that lands after it started — late or rotated — until it restarts.
# is restarted, so the fetch restarts it, and only when the value changed:
# a routine re-fetch of the same value leaves it alone.
# #
# secret_differs <path> <value> # The file's mtime is the record of a change, so write the file only when
# True when <path> is missing or holds something other than <value>. # `secret_differs` says so. The restart decision is then re-derived from
# Call it BEFORE `atomic_write_secret` writes <path>. Compares with # the file on every run: a run that wrote the file but failed before the
# `$(< path)`, a bash builtin, so the value never becomes an argument in # restart leaves a retry that still sees the file newer than the service.
# /proc; both sides lose their trailing newlines, which is how
# `atomic_write_secret` writes and `$(bao …)` reads.
#
# refresh_consumer <machine> <unit>
# Nothing while `container@<machine>` is not active: a container that
# starts later loads the file as it starts. Otherwise a running
# consumer is restarted, and a failed one —
# a consumer that found no credential may have hit its start limit — is
# reset and started. A consumer stopped on purpose stays stopped.
# `--no-block` throughout: a caller ordered `Before=` its consumer must
# not wait on a job that waits on the caller (the deadlock stated at
# `swarm-services-cert`'s propagation in ../hive-tls.nix).
# #
# Pure function — NOT a NixOS module. Call it from a module's `let`: # Pure function — NOT a NixOS module. Call it from a module's `let`:
# #
# refreshConsumer = import ./lib/refresh-consumer.nix { }; # refreshConsumer = import ./lib/refresh-consumer.nix { };
# script = '' # script = ''
# ${refreshConsumer} # ${refreshConsumer}
# changed=0 # if secret_differs "$path" "$secret"; then
# if secret_differs "$path" "$secret"; then changed=1; fi # atomic_write_secret 0400 root:root "$path" "$secret"
# atomic_write_secret 0400 root:root "$path" "$secret" # fi
# if [ "$changed" = 1 ]; then refresh_consumer my-machine my.service; fi # refresh_consumer my-machine my.service "$path"
# ''; # '';
# #
# Requires `systemd` on the caller's `path`. # Requires `systemd` and `coreutils` on the caller's `path`.
{ }: { }:
'' ''
# True when $1 is missing or holds something other than $2. `$(< path)`
# is a bash builtin, so the value never becomes an argument in /proc;
# both sides lose their trailing newlines, which is how
# `atomic_write_secret` writes and `$(bao …)` reads.
secret_differs() { secret_differs() {
[ ! -f "$1" ] || [ "$(< "$1")" != "$2" ] [ ! -f "$1" ] || [ "$(< "$1")" != "$2" ]
} }
# <machine> <unit> <path>. Nothing while `container@<machine>` is not
# active (a container that starts later loads the file as it starts), or
# when <unit> last entered `active` after <path> was last written.
# Otherwise a running consumer is restarted and a failed one — it may
# have hit its start limit without the credential — is reset and started;
# one stopped on purpose stays stopped. `--no-block` because a caller
# ordered `Before=` its consumer must not wait on a job that waits on the
# caller (see `swarm-services-cert`'s propagation in ../hive-tls.nix).
refresh_consumer() { refresh_consumer() {
local machine="$1" unit="$2" local machine="$1" unit="$2" path="$3" started started_us=0 written_us
if ! systemctl is-active --quiet "container@$machine.service"; then if ! systemctl is-active --quiet "container@$machine.service"; then
return 0 return 0
fi fi
# Empty for a unit that has never been active, which `date` would
# otherwise read as today's midnight.
started="$(systemctl --machine="$machine" show --timestamp=us+utc -p ActiveEnterTimestamp --value "$unit")"
if [ -n "$started" ]; then
started_us="$(date -u -d "$started" +%s%6N)"
fi
written_us="$(stat -c %.6Y "$path" | tr -d .)"
if [ "$written_us" -le "$started_us" ]; then
return 0
fi
if systemctl --machine="$machine" is-failed --quiet "$unit"; then if systemctl --machine="$machine" is-failed --quiet "$unit"; then
echo "the credential for $unit in $machine changed and $unit had failed — starting it" echo "$path changed after $unit in $machine last started, and $unit had failed — starting it"
systemctl --machine="$machine" reset-failed "$unit" systemctl --machine="$machine" reset-failed "$unit"
systemctl --machine="$machine" start --no-block "$unit" systemctl --machine="$machine" start --no-block "$unit"
else else
echo "the credential for $unit in $machine changed — restarting it if it runs" echo "$path changed after $unit in $machine last started — restarting it if it runs"
systemctl --machine="$machine" try-restart --no-block "$unit" systemctl --machine="$machine" try-restart --no-block "$unit"
fi fi
} }

View file

@ -2451,15 +2451,13 @@ in
# an argument in /proc the way `install <<<"$secret"` or an `echo` # an argument in /proc the way `install <<<"$secret"` or an `echo`
# from `path` would. # from `path` would.
install -d -m 0755 ${lib.escapeShellArg forwarderHostSecretDir} install -d -m 0755 ${lib.escapeShellArg forwarderHostSecretDir}
changed=0 if secret_differs ${lib.escapeShellArg forwarderHostSecretPath} "$secret"; then
if secret_differs ${lib.escapeShellArg forwarderHostSecretPath} "$secret"; then changed=1; fi atomic_write_secret 0400 root:root ${lib.escapeShellArg forwarderHostSecretPath} "$secret"
atomic_write_secret 0400 root:root ${lib.escapeShellArg forwarderHostSecretPath} "$secret" fi
# `LoadCredential` copies the file at start only: a rotated secret # `LoadCredential` copies the file at start only: a rotated secret
# reaches the forwarder by restarting it. # reaches the forwarder by restarting it.
if [ "$changed" = 1 ]; then refresh_consumer ${lib.escapeShellArg cfg.machine} opentelemetry-collector.service ${lib.escapeShellArg forwarderHostSecretPath}
refresh_consumer ${lib.escapeShellArg cfg.machine} opentelemetry-collector.service
fi
''; '';
}; };
}) })

View file

@ -761,14 +761,12 @@ in
# grants the group nothing; if this mode ever widens, the gid has to be # grants the group nothing; if this mode ever widens, the gid has to be
# discovered at runtime rather than assumed. # discovered at runtime rather than assumed.
install -d -m 0755 ${lib.escapeShellArg hostSecretDir} install -d -m 0755 ${lib.escapeShellArg hostSecretDir}
changed=0 if secret_differs ${lib.escapeShellArg hostSecretPath} "$secret"; then
if secret_differs ${lib.escapeShellArg hostSecretPath} "$secret"; then changed=1; fi atomic_write_secret 0400 ${lib.escapeShellArg "${toString config.ids.uids.grafana}:0"} ${lib.escapeShellArg hostSecretPath} "$secret"
atomic_write_secret 0400 ${lib.escapeShellArg "${toString config.ids.uids.grafana}:0"} ${lib.escapeShellArg hostSecretPath} "$secret" fi
# `$__file{}` is expanded once, while Grafana parses its config. # `$__file{}` is expanded once, while Grafana parses its config.
if [ "$changed" = 1 ]; then refresh_consumer ${lib.escapeShellArg cfg.machine} grafana.service ${lib.escapeShellArg hostSecretPath}
refresh_consumer ${lib.escapeShellArg cfg.machine} grafana.service
fi
''; '';
}; };

View file

@ -867,15 +867,13 @@ in
# no uid to give this to — `LoadCredential` reads it as root before # no uid to give this to — `LoadCredential` reads it as root before
# the sandbox exists and re-exposes it to whichever uid the unit got. # the sandbox exists and re-exposes it to whichever uid the unit got.
install -d -m 0755 ${lib.escapeShellArg collectorHostSecretDir} install -d -m 0755 ${lib.escapeShellArg collectorHostSecretDir}
changed=0 if secret_differs ${lib.escapeShellArg collectorHostSecretPath} "$secret"; then
if secret_differs ${lib.escapeShellArg collectorHostSecretPath} "$secret"; then changed=1; fi atomic_write_secret 0400 root:root ${lib.escapeShellArg collectorHostSecretPath} "$secret"
atomic_write_secret 0400 root:root ${lib.escapeShellArg collectorHostSecretPath} "$secret" fi
# `LoadCredential` copies the file at start only, and a collector that # `LoadCredential` copies the file at start only, and a collector that
# started without it is in its start limit. # started without it is in its start limit.
if [ "$changed" = 1 ]; then refresh_consumer ${lib.escapeShellArg cfg.machine} opentelemetry-collector.service ${lib.escapeShellArg collectorHostSecretPath}
refresh_consumer ${lib.escapeShellArg cfg.machine} opentelemetry-collector.service
fi
''; '';
}; };

View file

@ -1565,8 +1565,10 @@ let
} }
] ]
# A consumer that loads its credential at start never sees one a later # A consumer that loads its credential at start never sees one a later
# attempt lands, so the fetch that lands it restarts that consumer — only # attempt lands, so the fetch restarts it when the file is newer than the
# on a changed value, so a routine re-fetch leaves it running. # consumer's last start. The file is written only when the value differs,
# so its mtime moves only on a real change, and the decision is re-read on
# every run instead of carried in a variable a failed run would lose.
++ ++
map map
( (
@ -1576,13 +1578,26 @@ let
consumer, consumer,
}: }:
{ {
name = "${fetch} restarts ${consumer} in ${machine} when the value it fetched changed"; name = "${fetch} restarts ${consumer} in ${machine} when its file is newer than the consumer's start";
ok = ok =
let let
s = baoGrantWithConsumers.systemd.services.${fetch}.script; # Text only: the script refers to store paths, and a string cut from it
# keeps that context, which `splitString` refuses as a separator.
s = builtins.unsafeDiscardStringContext baoGrantWithConsumers.systemd.services.${fetch}.script;
call = "refresh_consumer ${lib.escapeShellArg machine} ${consumer} ";
# The path the refresh is keyed on, as rendered.
path = lib.head (lib.splitString "\n" (lib.last (lib.splitString call s)));
# Everything from the gate on that path to the end of its `if`.
gated = lib.head (
lib.splitString "fi\n" (lib.last (lib.splitString "if secret_differs ${path} " s))
);
in in
lib.hasInfix "secret_differs " s lib.hasInfix call s
&& lib.hasInfix "refresh_consumer ${lib.escapeShellArg machine} ${consumer}" s; && lib.hasInfix "atomic_write_secret " gated
&& lib.hasInfix path gated
&& lib.hasInfix "ActiveEnterTimestamp" s
&& lib.hasInfix ''if [ "$written_us" -le "$started_us" ]; then'' s
&& !(lib.hasInfix "$changed" s);
} }
) )
[ [