Skip to content

Commit fdfa7f5

Browse files
ausimianclaude
andcommitted
fix: require RELEASES to be consultable, not merely readable
release_handler:init/1 reads the file with file:consult/1, so a file whose permissions are fine but whose terms are malformed fails in exactly the same way as an unreadable one - and the hook leaves an existing file alone whatever is in it. The message said readable, which sent an operator with a corrupt RELEASES round the same restart loop the message exists to end. Absent, or consultable, is the condition. Also corrects the record-check entry higher in the release note, which still described the remedy as a bare restart and the cause as the file not being there. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 615b69d commit fdfa7f5

4 files changed

Lines changed: 35 additions & 23 deletions

File tree

AGENTS.md

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -453,9 +453,17 @@ Castle's job is configuration and release management on a running node.
453453
operator told to check whether the file is "absent" or "present and unreadable"
454454
finds it is neither and has no applicable advice. A plain restart is exactly
455455
right for them. So the message asks for `releases/RELEASES` to be *absent or
456-
readable* before the restart, which covers all three states and is shorter than
457-
the branch it replaced. Do not turn it back into a case analysis of the cause,
458-
and do not collapse it into a bare "restart the system" either.
456+
consultable* before the restart, which covers all three states and is shorter
457+
than the branch it replaced. Do not turn it back into a case analysis of the
458+
cause, and do not collapse it into a bare "restart the system" either.
459+
460+
**"Consultable", not "readable".** `init/1` reads the file with
461+
`file:consult/1`, so a file whose permissions are fine but whose terms are
462+
malformed fails in exactly the same way, and the hook leaves an existing file
463+
alone whatever is in it. An earlier version of this message said "readable" and
464+
so sent an operator with a corrupt `RELEASES` round the same restart loop it was
465+
written to end. Permissions are not the whole of the condition; being
466+
consultable by `:release_handler` is.
459467

460468
**And it must not name `releases/RELEASES` unqualified**, because that is the
461469
file the *release* creates, not necessarily the one the handler reads — see

RELEASE.md

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,17 @@
4747
- `Castle.unpack/1` and `Castle.install/1` now refuse a system that cannot be
4848
upgraded from, and refuse it in the same call that would otherwise have done
4949
the work. `:release_handler` reads `releases/RELEASES` once, as it starts, and
50-
when the file is not there it makes a release record up out of the boot
51-
script's name and version — a record that names no applications at all.
50+
when it cannot — the file absent, or there but not consultable — it makes a
51+
release record up out of the boot script's name and version — a record that
52+
names no applications at all.
5253
Upgrading a system in that state is worse than being stopped: the install
5354
reports success, and every application whose version changed but whose code the
5455
upgrade does not explicitly load goes on running its old code out of the
5556
directory of the release that was just replaced, until a later `remove` deletes
5657
it. Nothing can repair the running system afterwards, because creating the file
57-
changes no record the node holds — so what the refusal says is to restart,
58-
which is the one thing that does: the release creates the file before it
59-
starts.
58+
changes no record the node holds — so what the refusal says is to restart, with
59+
the file either absent or consultable first. See *Fixed* below for what that
60+
condition is and why a bare restart is not always enough.
6061

6162
The question is asked of the node's own records rather than of the filesystem,
6263
which is the only way to see the case where the file exists but the boot that
@@ -70,7 +71,7 @@
7071
operation that *writes* release records: an unpack on such a node would put the
7172
made-up record into `releases/RELEASES`, where the next boot would read it back
7273
as though it belonged there — which takes away the restart that is the way out,
73-
since the file is only created when it is missing. Committing and removing are
74+
since the file is only created when it is absent. Committing and removing are
7475
unaffected: neither can write that record back, and refusing them could strand
7576
a version that was already installed.
7677
- `Castle.upgradable/0`, which answers the same question on its own, for an
@@ -152,13 +153,14 @@
152153

153154
- The refusal for a system running from a synthesised release record now names a
154155
remedy that works. It said to restart, and a restart alone is enough only when
155-
the `RELEASES` file `:release_handler` reads is absent or readable: the release
156-
creates that file when it is missing, so one that is present but unreadable is
157-
stepped over on every start and the system comes back on another synthesised
158-
record. An operator following the old message would have restarted
156+
the `RELEASES` file `:release_handler` reads is absent or consultable: it reads
157+
that file with `file:consult/1`, so a malformed one fails just as an unreadable
158+
one does, and the release creates the file only when it is *absent* — so
159+
anything left in place that cannot be consulted is stepped over on every start
160+
and the system comes back on another synthesised record. An operator following the old message would have restarted
159161
indefinitely.
160162

161-
The refusal now asks for that file to be absent or readable before the restart,
163+
The refusal now asks for that file to be absent or consultable before the restart,
162164
and identifies it rather than assuming: `releases/RELEASES` under the release
163165
root, unless `RELDIR` or the `sasl` `releases_dir` parameter points elsewhere.
164166
Where one of those does, the two are different files and a restart cannot fix it

lib/castle/commands.ex

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -346,14 +346,16 @@ defmodule Castle.Commands do
346346
"upgrade does not load, running its old code. Creating the file now would not " <>
347347
"change the record this node works from, so the system has to be restarted. " <>
348348
"Before restarting, make sure the RELEASES file :release_handler reads is " <>
349-
"either readable or absent, because the release creates that file only when it " <>
350-
"is absent - an unreadable one left in place is stepped over on every start " <>
351-
"and the system comes back on the same synthesised record. Absent, or " <>
352-
"readable, is what a restart needs. That file is releases/RELEASES under the " <>
353-
"release root unless RELDIR or the sasl releases_dir parameter points " <>
354-
"elsewhere; where one of those does, the release creates a file at the root " <>
355-
"that the handler will not read, so the one it does read has to be put there " <>
356-
"and made readable by hand."}
349+
"either absent or one it can consult - it reads that file with file:consult/1, " <>
350+
"so a malformed one fails exactly as an unreadable one does, and permissions " <>
351+
"are not the whole of the condition. The release creates that file only when " <>
352+
"it is absent, so anything left in place that cannot be consulted is stepped " <>
353+
"over on every start and the system comes back on the same synthesised " <>
354+
"record. Absent, or consultable, is what a restart needs. That file is " <>
355+
"releases/RELEASES under the release root unless RELDIR or the sasl " <>
356+
"releases_dir parameter points elsewhere; where one of those does, the release " <>
357+
"creates a file at the root that the handler will not read, so the one it does " <>
358+
"read has to be put there by hand."}
357359

358360
nil ->
359361
{:error, "#{refusal}: no release is running."}

test/castle/commands_test.exs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ defmodule Castle.CommandsTest do
9393
# absence - and a case analysis of the cause, which is what this said first,
9494
# has no advice at all for a file that was absent at boot and has been
9595
# created readably since, where a plain restart is all that is needed.
96-
assert message =~ "Absent, or readable, is what a restart needs."
96+
assert message =~ "Absent, or consultable, is what a restart needs."
9797
end
9898

9999
test "asks the release the system is running, and not another one" do

0 commit comments

Comments
 (0)