Skip to content

Commit 888fab6

Browse files
fix(infra): serialize the upgrade and write compose through a unique temp path
The compose reinstall added last round is not safe under overlap, and `main` never wrote compose from this document, so the race arrives with that commit. Two executions — a double run, a retry while the first is still pulling, two operators — shared the fixed `/opt/gitlawb/compose.yaml.new`. The second truncates the file the first is about to install; and once the first renames, the second's open descriptor keeps writing into the live `compose.yaml` inode. Both outcomes stay valid YAML, so the `up -d --remove-orphans` on the next line accepts a blended or truncated service set and deletes whatever fell off — on a live node, the local postgres or the TLS terminator. Both reviewers reproduced it independently; one measured a short file in 12 of 12 trials. The whole step now runs under `flock -n` on /opt/gitlawb/.upgrade.lock and exits non-zero if the lock is held, so a second run cannot restart against a file the first is mid-install. The temp file comes from `mktemp`, so concurrent writers cannot truncate each other, with a trap to clean it up on failure. The compose body is base64-encoded rather than written through a nested heredoc. The rendering carries `$${VAR}` passthroughs that must reach the file unexpanded, which needs a quoted delimiter, and a shell heredoc nested inside an indented Terraform heredoc depends on the dedent landing that delimiter at column 0. Decoding one line has neither failure mode and no delimiter can collide with the content. Not validated locally: terraform is not installed on this machine, so this change has had no `fmt`, `validate`, or `plan` run against it.
1 parent 579eee0 commit 888fab6

1 file changed

Lines changed: 51 additions & 19 deletions

File tree

‎infra/aws/main.tf‎

Lines changed: 51 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -448,25 +448,57 @@ resource "aws_ssm_document" "upgrade" {
448448
action = "aws:runShellScript"
449449
name = "upgrade"
450450
inputs = {
451-
runCommand = [
452-
"set -euo pipefail",
453-
# Reinstall the CURRENT rendering before restarting.
454-
#
455-
# `aws_instance` ignores user_data drift, so an instance created before a
456-
# template change keeps the compose file it was born with. Compose passes
457-
# only the variables named in its `environment:` block into the container,
458-
# so a node setting added to the template never reaches an existing
459-
# instance — the operator writes it to /opt/gitlawb/.env, restarts, and the
460-
# node silently keeps the built-in default. `docker compose pull && up -d`
461-
# cannot fix that: the authoritative file is the one on disk.
462-
#
463-
# Idempotent: writing the same bytes and restarting is a no-op. This does
464-
# overwrite hand-edits to compose.yaml, which is intended — the file is
465-
# Terraform-owned; per-instance settings belong in /opt/gitlawb/.env.
466-
"install -d -m 0755 /opt/gitlawb",
467-
"cat > /opt/gitlawb/compose.yaml.new <<'COMPOSE_EOF'\n${local.compose_yaml}\nCOMPOSE_EOF",
468-
"mv /opt/gitlawb/compose.yaml.new /opt/gitlawb/compose.yaml",
469-
"cd /opt/gitlawb && docker compose pull && docker compose up -d --remove-orphans && docker image prune -f"
451+
# Reinstall the CURRENT rendering before restarting.
452+
#
453+
# `aws_instance` ignores user_data drift, so an instance created before a
454+
# template change keeps the compose file it was born with. Compose passes
455+
# only the variables named in its `environment:` block into the container,
456+
# so a node setting added to the template never reaches an existing
457+
# instance — the operator writes it to /opt/gitlawb/.env, restarts, and the
458+
# node silently keeps the built-in default. `docker compose pull && up -d`
459+
# cannot fix that: the authoritative file is the one on disk.
460+
#
461+
# Idempotent: writing the same bytes and restarting is a no-op. This does
462+
# overwrite hand-edits to compose.yaml, which is intended — the file is
463+
# Terraform-owned; per-instance settings belong in /opt/gitlawb/.env.
464+
#
465+
# One shell step, under flock, writing through mktemp. Two overlapping
466+
# executions — a double run, a retry while the first is still pulling, two
467+
# operators — otherwise share one fixed temp path: the second truncates the
468+
# file the first is about to install, and once the first renames, the
469+
# second's open descriptor keeps writing into the live compose.yaml inode.
470+
# Both outcomes stay valid YAML, so the `up -d --remove-orphans` on the next
471+
# line accepts a blended or truncated service set and deletes whatever fell
472+
# off — on a live node, the local postgres or the TLS terminator. The lock
473+
# also covers pull and up, so a second run cannot restart against a file the
474+
# first is mid-install.
475+
runCommand = [<<-UPGRADE
476+
set -euo pipefail
477+
install -d -m 0755 /opt/gitlawb
478+
479+
exec 9>/opt/gitlawb/.upgrade.lock
480+
if ! flock -n 9; then
481+
echo "another upgrade is already running on this instance" >&2
482+
exit 1
483+
fi
484+
485+
tmp="$(mktemp /opt/gitlawb/compose.yaml.XXXXXX)"
486+
trap 'rm -f "$tmp"' EXIT
487+
# base64 rather than a nested heredoc: the rendered compose contains
488+
# `$${VAR}` passthroughs that must reach the file unexpanded, and a
489+
# heredoc inside an indented Terraform heredoc depends on the dedent
490+
# landing the delimiter at column 0. Decoding a single line has neither
491+
# failure mode, and no delimiter can collide with the content.
492+
echo '${base64encode(local.compose_yaml)}' | base64 -d > "$tmp"
493+
chmod 0644 "$tmp"
494+
mv "$tmp" /opt/gitlawb/compose.yaml
495+
trap - EXIT
496+
497+
cd /opt/gitlawb
498+
docker compose pull
499+
docker compose up -d --remove-orphans
500+
docker image prune -f
501+
UPGRADE
470502
]
471503
}
472504
}]

0 commit comments

Comments
 (0)