diff --git a/docs/swarm/credentials.md b/docs/swarm/credentials.md index a1aaf9b7..cfee085e 100644 --- a/docs/swarm/credentials.md +++ b/docs/swarm/credentials.md @@ -174,6 +174,7 @@ single attempt, not for the retries: an attempt that fails ends that start, so the dependent starts with whatever is already on disk. When a later attempt lands a changed value, the homeserver, Grafana and collector readers restart the service that loaded it (`nix/host-modules/lib/refresh-consumer.nix`). +When the fetch unit can't read the credential file's mtime or the service's start time, it logs `cannot compare the mtime of … — leaving as it is` at `err` in its own journal, and the service keeps running on the old value until the next timer tick. ## Progressive enhancement diff --git a/nix/checks.nix b/nix/checks.nix index bc04772b..6551916c 100644 --- a/nix/checks.nix +++ b/nix/checks.nix @@ -55,6 +55,9 @@ in inherit pkgs self nixosSystem; inherit (pkgs) lib; }; + # Runs ./host-modules/lib/refresh-consumer.nix's restart decision under a + # comma-decimal locale, against a stub `systemctl`. + script-test-refresh-consumer = import ./script-tests/refresh-consumer.nix { inherit pkgs; }; module-eval-nats-tls = import ./module-eval/nats-tls.nix { inherit pkgs self nixosSystem; inherit (pkgs) lib; diff --git a/nix/host-modules/lib/refresh-consumer.nix b/nix/host-modules/lib/refresh-consumer.nix index c27923df..8f74567e 100644 --- a/nix/host-modules/lib/refresh-consumer.nix +++ b/nix/host-modules/lib/refresh-consumer.nix @@ -31,27 +31,43 @@ [ ! -f "$1" ] || [ "$(< "$1")" != "$2" ] } - # [systemctl option…]. Nothing when last entered - # `active` after was last written. Otherwise a running 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. - # The options are passed to every `systemctl` call, which is how - # `refresh_consumer` reaches into a container. `--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_unit() { + # [systemctl option…]. Returns 0 when was last + # written after last entered `active` (or never has), 1 when + # it was not, and 2, logged at `err`, when either time does not read. Both + # are bare microsecond counts, with no decimal separator for a locale to + # change. A caller acts only on 0: the mtime stays, so the first run that + # can read both still acts, where acting on doubt acts on every run. The + # options are passed to `systemctl`, which is how a caller reaches into a + # container. + written_after_start() { local unit="$1" path="$2" started started_us=0 written_us shift 2 # Empty for a unit that has never been active, which `date` would # otherwise read as today's midnight. started="$(systemctl "$@" show --timestamp=us+utc -p ActiveEnterTimestamp --value "$unit")" if [ -n "$started" ]; then - started_us="$(date -u -d "$started" +%s%6N)" + started_us="$(date -u -d "$started" +%s%6N)" || started_us= + fi + written_us="$(date -u -r "$path" +%s%6N)" || written_us= + if [[ ! "$started_us" =~ ^[0-9]+$ || ! "$written_us" =~ ^[0-9]+$ ]]; then + echo "<3>cannot compare the mtime of $path ('$written_us') with the last start of $unit ('$started') — leaving $unit as it is" >&2 + return 2 fi - written_us="$(stat -c %.6Y "$path" | tr -d .)" if [ "$written_us" -le "$started_us" ]; then - return 0 + return 1 fi + } + + # [systemctl option…]. Nothing unless `written_after_start`. + # Otherwise a running 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_unit() { + local unit="$1" path="$2" + shift 2 + written_after_start "$unit" "$path" "$@" || return 0 if systemctl "$@" is-failed --quiet "$unit"; then echo "$path changed after $unit last started, and $unit had failed — starting it" systemctl "$@" reset-failed "$unit" diff --git a/nix/host-modules/swarm-bao.nix b/nix/host-modules/swarm-bao.nix index 5fd0df35..db792c8b 100644 --- a/nix/host-modules/swarm-bao.nix +++ b/nix/host-modules/swarm-bao.nix @@ -3429,6 +3429,7 @@ in echo "wrote ${agentCaCachePath}" fi + ${refreshConsumer} ${composeListenerBundle} compose_listener_bundle @@ -3437,11 +3438,12 @@ in echo "openbao in $machine is not running; it reads ${listenerClientCaPath} when it starts" exit 0 fi - started="$(systemctl --machine="$machine" show --timestamp=us+utc -p ActiveEnterTimestamp --value openbao.service)" - started_us="$(date -u -d "$started" +%s%6N)" - written_us="$(stat -c %.6Y ${listenerClientCaPath} | tr -d .)" - if [ "$written_us" -le "$started_us" ]; then + newer=0 + written_after_start openbao.service ${listenerClientCaPath} --machine="$machine" || newer=$? + if [ "$newer" = 1 ]; then echo "openbao started after ${listenerClientCaPath} last changed; nothing to pick up" + fi + if [ "$newer" != 0 ]; then exit 0 fi diff --git a/nix/module-eval/bao-basics.nix b/nix/module-eval/bao-basics.nix index a1d74de4..c4614e67 100644 --- a/nix/module-eval/bao-basics.nix +++ b/nix/module-eval/bao-basics.nix @@ -269,16 +269,26 @@ let { # openbao reads its client-CA file only at start, so picking up the # agent CA is a restart: automatic where the store unseals itself, - # printed where a human has to. The shamir arm is the control. + # printed where a human has to. The shamir arm is the control. Both + # decide on the shared locale-independent compare + # (../host-modules/lib/refresh-consumer.nix, run by + # `script-test-refresh-consumer`) and act only on its 0. name = "the agent PKI unit restarts openbao under pkcs11 and only prints the step under shamir"; ok = let restart = ''systemctl --machine="$machine" restart openbao.service''; + compare = '' + written_after_start openbao.service ${tlsDir}/listener-client-ca.pem --machine="$machine" || newer=$? + ''; p = baoPkcs11.systemd.services.swarm-bao-agent-pki.script; sh = baoShamir.systemd.services.swarm-bao-agent-pki.script; in lib.hasInfix restart p - && lib.hasInfix ''if [ "$written_us" -le "$started_us" ]; then'' p + && lib.all (s: lib.hasInfix compare s && lib.hasInfix ''if [ "$newer" != 0 ]; then'' s) [ + p + sh + ] + && !(lib.hasInfix "%.6Y" p) && !(lib.hasInfix restart sh) && lib.hasInfix "# then unseal" sh; } diff --git a/nix/script-tests/refresh-consumer.nix b/nix/script-tests/refresh-consumer.nix new file mode 100644 index 00000000..601ced46 --- /dev/null +++ b/nix/script-tests/refresh-consumer.nix @@ -0,0 +1,19 @@ +# `checks.script-test-refresh-consumer` — runs ../host-modules/lib/refresh-consumer.nix's +# `written_after_start`, `refresh_unit` and `refresh_consumer` against a stub +# `systemctl`, under a locale whose decimal separator is a comma. The cases +# are in ./refresh-consumer.sh. +# +# What runs is the rendered helper, sourced as its callers' scripts include it. +{ + pkgs, + ... +}: +pkgs.runCommand "hyperhive-script-test-refresh-consumer" + { + HELPER = pkgs.writeText "refresh-consumer.sh" (import ../host-modules/lib/refresh-consumer.nix { }); + LOCALE_ARCHIVE = "${pkgs.glibcLocales}/lib/locale/locale-archive"; + } + '' + ${pkgs.bash}/bin/bash ${./refresh-consumer.sh} + touch "$out" + '' diff --git a/nix/script-tests/refresh-consumer.sh b/nix/script-tests/refresh-consumer.sh new file mode 100644 index 00000000..55b55293 --- /dev/null +++ b/nix/script-tests/refresh-consumer.sh @@ -0,0 +1,153 @@ +# Cases for ./refresh-consumer.nix. Each case sets the credential file's mtime +# and the consumer's last start, runs the helper with a stub `systemctl` that +# records every call it is asked to act on, and compares that record and what +# the helper wrote to stderr. +set -uo pipefail + +failures=0 +count=0 + +fail() { + echo "FAILED: $case: $*" >&2 + failures=$((failures + 1)) +} + +dir=$(mktemp -d) +mkdir "$dir/bin" +calls=$dir/calls +path=$dir/token +: >"$path" + +# `show` prints $STARTED as systemd's `--timestamp=us+utc` does; `is-active` +# and `is-failed` answer from $ACTIVE and $FAILED; every other verb is +# appended to $calls. +cat >"$dir/bin/systemctl" <>"$calls" ;; +esac +STUB +chmod +x "$dir/bin/systemctl" +export PATH="$dir/bin:$PATH" ACTIVE=1 FAILED=0 STARTED= + +export LC_ALL=de_DE.UTF-8 + +# shellcheck source=/dev/null +. "$HELPER" + +# The locale the cases run under has to print the comma the helper must not +# depend on, or none of them tests it. +case="setup" +touch -d @1791232888.193159 "$path" +got=$(stat -c %.6Y "$path") +if [ "$got" != "1791232888,193159" ]; then fail "stat under $LC_ALL printed '$got', wanted 1791232888,193159"; fi + +# systemd's `--timestamp=us+utc` rendering of .. +systemd_ts() { + LC_ALL=C date -u -d "@$1" '+%a %Y-%m-%d %H:%M:%S.%6N UTC' +} + +# check +# +check() { + case=$1 + local want_calls=$2 want_err=$3 got err + shift 3 + count=$((count + 1)) + : >"$calls" + # `set -e` as the callers' NixOS `script`s run: a failed read must not end the + # caller. Not under `||` or `if`, which would switch `set -e` off. + local rc + ( + set -e + "$@" >/dev/null 2>"$dir/err" + ) + rc=$? + if [ "$rc" != 0 ]; then fail "exited $rc"; fi + err=$(cat "$dir/err") + got=$(cat "$calls") + if [ "$got" != "$want_calls" ]; then + fail "systemctl was asked:"$'\n'"$got"$'\n'"wanted:"$'\n'"$want_calls" + fi + # A failed read also leaves `date`'s own message ahead of the helper's line. + if [ -n "$want_err" ]; then err=$(printf '%s\n' "$err" | tail -n 1); fi + if [ "$err" != "$want_err" ]; then + fail "stderr:"$'\n'"$err"$'\n'"wanted:"$'\n'"$want_err" + fi +} + +touch -d @1791232888.193159 "$path" + +STARTED=$(systemd_ts 1791232900.000001) +check "a file older than the start restarts nothing" "" "" \ + refresh_unit gitea-runner-swarm.service "$path" +check "nor through a container" "" "" \ + refresh_consumer swarm-ci-runner gitea-runner-swarm.service "$path" + +STARTED=$(systemd_ts 1791232888.193159) +check "a file written in the start's microsecond restarts nothing" "" "" \ + refresh_unit gitea-runner-swarm.service "$path" + +STARTED=$(systemd_ts 1791232888.193158) +check "a file one microsecond newer than the start restarts the unit" \ + "try-restart --no-block gitea-runner-swarm.service" "" \ + refresh_unit gitea-runner-swarm.service "$path" +check "through a container, with --machine" \ + "--machine=swarm-ci-runner try-restart --no-block gitea-runner-swarm.service" "" \ + refresh_consumer swarm-ci-runner gitea-runner-swarm.service "$path" + +FAILED=1 +check "a failed unit is reset and started" \ + "reset-failed gitea-runner-swarm.service"$'\n'"start --no-block gitea-runner-swarm.service" "" \ + refresh_unit gitea-runner-swarm.service "$path" +FAILED=0 + +STARTED= +check "a unit never active is restarted" \ + "try-restart --no-block gitea-runner-swarm.service" "" \ + refresh_unit gitea-runner-swarm.service "$path" + +ACTIVE=0 +check "nothing while the container is down" "" "" \ + refresh_consumer swarm-ci-runner gitea-runner-swarm.service "$path" +ACTIVE=1 + +STARTED="not a time" +check "an unreadable start restarts nothing and says so at err" "" \ + "<3>cannot compare the mtime of $path ('1791232888193159') with the last start of gitea-runner-swarm.service ('not a time') — leaving gitea-runner-swarm.service as it is" \ + refresh_unit gitea-runner-swarm.service "$path" + +STARTED=$(systemd_ts 1791232900.000001) +check "a missing file restarts nothing and says so at err" "" \ + "<3>cannot compare the mtime of $dir/missing ('') with the last start of gitea-runner-swarm.service ('$STARTED') — leaving gitea-runner-swarm.service as it is" \ + refresh_unit gitea-runner-swarm.service "$dir/missing" + +# compare . `written_after_start` as +# `swarm-bao-agent-pki` calls it, which acts only on 0. +compare() { + case=$1 + count=$((count + 1)) + local rc + ( + set -e + written_after_start openbao.service "$3" --machine=swarm-bao 2>/dev/null + ) + rc=$? + if [ "$rc" != "$2" ]; then fail "returned $rc, wanted $2"; fi +} + +STARTED=$(systemd_ts 1791232900.000001) +compare "the compare is 1 for a file older than the start" 1 "$path" +STARTED=$(systemd_ts 1791232888.193158) +compare "the compare is 0 for a file newer than the start" 0 "$path" +STARTED="not a time" +compare "the compare is 2 for an unreadable start" 2 "$path" +STARTED=$(systemd_ts 1791232900.000001) +compare "the compare is 2 for a missing file" 2 "$dir/missing" + +echo "$count cases, $failures failed" +[ "$failures" = 0 ]