Skip to content

Commit 2346b96

Browse files
ausimianclaude
andcommitted
docs: correct what the deleted path left behind
AGENTS.md described the two-shape dispatch as a load-bearing property and recorded the umask gap in write_sys_config as a known one; both belonged to the path that has gone, and a note about a gap in deleted code reads as a gap in the code that is left. The boot-time race on sys.config went with it too - nothing Castle does runs at boot any more - and the cold-boot interaction it listed as unreachable is reachable now that forecastle#6 has landed, so what is missing there is a test rather than a dependency. RELEASE.md is notes for an unreleased 1.0.0, so generate/1 is not a function this release changes but one it removes: 0.3.1 published it. The gate and the make_releases change are operator-visible in their own right - one refuses an upgrade that used to be accepted, the other stops a caller having to change directory first. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C4oaMvbR1cbxrZBj8qwkqN
1 parent d00e1e9 commit 2346b96

2 files changed

Lines changed: 114 additions & 73 deletions

File tree

AGENTS.md

Lines changed: 78 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -9,28 +9,24 @@ build-time dependency.
99

1010
Castle's job is configuration and release management on a running node.
1111

12-
- **`Castle.generate/1`** — reads `build.config` from the release's version
13-
directory, folds the stashed config providers over it, and writes the result
14-
as that version's `sys.config`. This is the whole reason the pair exists:
15-
Mix expands runtime configuration once, at boot, from the version it booted;
16-
Castle re-expands it for the version being upgraded *to*, before the relup
17-
runs. It is now the older of two ways to do that — see the next bullet — and
18-
goes away with the third step of
19-
[#13](https://github.com/ausimian/castle/issues/13).
2012
- **Materialising the target's configuration**, which `install/1` and `commit/1`
21-
do before they hand a version to `:release_handler`. Which way depends on
22-
whether `releases/<vsn>/build.config` exists, and that is a sound
23-
discriminator because it is Forecastle that creates it: assembling a release
24-
today strips the providers out, stashes their initialised state under
25-
`:castle`, and renames the `sys.config` Mix wrote to `build.config`. So the
26-
file is present exactly when the configuration was intercepted at build time,
27-
and `Castle.generate/1` is then the only thing that can expand it. Note that
28-
it has to be the *presence of `build.config`* rather than the absence of
29-
`sys.config`: once such a release has booted once, it has both.
30-
31-
When it is absent, Mix's pipeline is intact and `Castle.Peer` materialises the
32-
configuration instead: a `:peer` reached over a loopback socket — so no epmd,
33-
cookie, node name or distribution; the peer reports `nonode@nohost` and
13+
do before they hand a version to `:release_handler`. This is the whole reason
14+
the pair exists: Mix expands runtime configuration once, at boot, from the
15+
version it booted; Castle expands it for the version being upgraded *to*,
16+
before the relup runs.
17+
18+
There is one way it happens, and there used to be two. The other read a
19+
`build.config` — the `sys.config` Forecastle renamed at assembly time, having
20+
stripped the providers out and stashed their initialised state under
21+
`:castle` — and folded that state over it in the running node, which is
22+
`Castle.generate/1`. Both halves of that are gone: forecastle#6 stopped
23+
intercepting configuration at build time, and the third step of
24+
[#13](https://github.com/ausimian/castle/issues/13) deleted the path that read
25+
it. Do not reintroduce either. A release whose providers ran in the version
26+
that happens to be running was configured by the wrong code.
27+
28+
What is left is `Castle.Peer`: a `:peer` reached over a loopback socket — so no
29+
epmd, cookie, node name or distribution; the peer reports `nonode@nohost` and
3430
`is_alive() == false` — booted on the target's own `preboot` script and its
3531
own emulator, which runs `Config.Provider.boot/1` over the target's own
3632
provider modules and hands the resolved configuration back to be written.
@@ -214,24 +210,14 @@ Castle's job is configuration and release management on a running node.
214210
scratch will be created with that group and the mode bits will be honoured
215211
against it.
216212

217-
`Castle.Commands.write_sys_config/2`, on the `build.config` path, is the one
218-
place this rule is not applied: it creates `sys.config` with the process umask
219-
when the file does not exist yet. There is no transient exposure there — the
220-
mode it is granted is the mode it keeps — and that path is deleted in step 3,
221-
while "nothing observable changes for a release assembled by today's
222-
Forecastle" pins its behaviour until then. It is a real gap, recorded rather
223-
than fixed here.
224-
225213
A base that cannot be read as a configuration is refused, naming the remedy,
226214
rather than resolved from: it is preferred to `sys.config` by definition, so
227215
failing loudly is the only safe thing left.
228216

229-
This is permanent design: the `build.config` path has always had a pristine
230-
base —
231-
`build.config` *is* one — and this is what carries that property forward when
232-
step 3 deletes it. It is deliberately not called `build.config`, since that
233-
name is the discriminator and would send the release back down the path being
234-
removed. `sys.config` gains a `CASTLE_MATERIALISED` comment line, which makes
217+
This is permanent design: the path this replaced always had a pristine base —
218+
`build.config` *was* one, and nothing ever wrote it — and this is what carries
219+
that property forward now that it is gone.
220+
`sys.config` gains a `CASTLE_MATERIALISED` comment line, which makes
235221
the invariant checkable: written by Castle, so a base must exist. A version
236222
that says that and has no base beside it is refused, with the remedy (unpack
237223
it again) named, rather than having a once-resolved configuration captured as
@@ -285,7 +271,39 @@ Castle's job is configuration and release management on a running node.
285271

286272
- **`Castle.make_releases/0`** — creates the `RELEASES` file from the running
287273
permanent release if it does not already exist, so a release assembled by Mix
288-
can manage its own upgrades.
274+
can manage its own upgrades. The directory is derived from `code:root_dir()`,
275+
which is the root `:release_handler` resolves *its* relative paths against
276+
(`consult/2` is `file:consult(root_dir_relative_path(File))`, and
277+
`do_write_release/3` the same), so no caller has to change directory and none
278+
should: the working directory was only ever visible to the `File.exists?/1`
279+
guard, which is what let the file this looked for and the file OTP wrote be
280+
different ones. It calls **`create_RELEASES/3`**, never `/4` with the root
281+
supplied: `/3` is `create_RELEASES("", RelDir, RelFile, LibDirs)`, and
282+
`check_rel_data/4` stores library directories as `lib/<app>-<vsn>` when the
283+
root is empty and as absolute paths under it when it is not — "to make it easy
284+
to create a relocatable RELEASES file", in OTP's own words. Passing the root
285+
would bake this machine's paths into a file whose point is that it can be
286+
moved, and no end-state test would see it.
287+
- **`Castle.upgradable/0`** — succeeds when the running release can be upgraded
288+
from, and refuses when `:release_handler` is working from the record it
289+
synthesises for itself. It reads `RELEASES` once, in `init/1`, and when it
290+
cannot it builds a record out of the boot script's name and version with the
291+
`libs` field left at `[]`. Nothing can replace that afterwards, and creating
292+
the file later does not: the first operation that changes anything writes the
293+
in-memory record back over it. Upgrading from it is silently wrong rather than
294+
refused — the relup's `point_of_no_return` switches code paths for
295+
`get_new_libs(Current, New)`, which folds over the *current* release's
296+
applications and so yields nothing at all, leaving any application whose
297+
version changed but whose code the relup does not load running from the
298+
directory of the release being replaced. The discriminator is that empty
299+
application list, and it is exact: `which_releases/0` reports
300+
`mk_lib_name(Libs)`, `mk_lib_name([]) -> []`, and a record read from a
301+
`RELEASES` file names at least `kernel` and `stdlib`. It has to be asked of the
302+
node rather than of the filesystem, which is why this is here and not in
303+
`bin/castle`: a file that appeared *after* the boot that looked for it passes
304+
a shell test for the file and still leaves the node on the synthesised record.
305+
The remedy the message names is a restart, because that is the only thing that
306+
changes the answer.
289307
- **`unpack/1`, `install/1`, `commit/1`, `remove/1`, `releases/0`** — wrappers
290308
over `:release_handler`, with the target version's configuration materialised
291309
ahead of `install` and `commit` so that it exists before the version is
@@ -296,9 +314,8 @@ Castle's job is configuration and release management on a running node.
296314
rebooted, and an emulator upgrade finishes on the way back up, where it can
297315
still roll back. So Castle answers the question and leaves the asking to
298316
Forecastle: `bin/castle install` repeats it rather than trusting the reply,
299-
from Forecastle 1.0.0 — the revision pinned in this project's `mix.lock`
300-
installs with a single rpc and never calls this, so do not describe the
301-
polling as something Castle's own integrated state does. Two conditions. The
317+
from Forecastle 1.0.0 — so the polling is Forecastle's, and not something
318+
Castle's own state does. Two conditions. The
302319
version is the running release: the
303320
`current` one, or the `permanent` one when none is current — `install` leaves
304321
its target `current` and `commit` promotes it, so both count; `unpacked` (a
@@ -335,10 +352,10 @@ re-raises in the calling VM, and only that VM exits. `Castle.Commands` holds
335352
the operations themselves, returning their outcome instead of acting on the
336353
process, which is what makes them testable.
337354

338-
Forecastle is what arranges for these to be reachable: it renames `sys.config`
339-
to `build.config` at assembly time, adds a `:preboot` script that starts
340-
`:castle`, and writes the `env.sh` fragment and `bin/castle` wrapper that call
341-
into this module.
355+
Forecastle is what arranges for these to be reachable: it leaves the
356+
configuration Mix wrote alone, adds a `:preboot` script that starts `:castle`,
357+
and writes the `env.sh` fragment and `bin/castle` wrapper that call into this
358+
module.
342359

343360
## Layout
344361

@@ -370,11 +387,18 @@ into this module.
370387
`mix test` covers `Castle.Commands` as units. `:release_handler`, `:init` and
371388
`Castle.Peer` are reached through module arguments that default to them, so the
372389
tests hand them `Castle.ReleaseHandlerStub`, `Castle.InitStub` and
373-
`Castle.PeerStub` instead; `generate/1` and `materialise/2` take the version
374-
directory they work on, so the tests give them a `tmp_dir`.
390+
`Castle.PeerStub` instead; `materialise/2` takes the version directory it works
391+
on and `make_releases/2` the releases directory, so the tests give them a
392+
`tmp_dir` — and neither the commands nor their tests touch the working
393+
directory, which is what lets them all run async.
375394
`test/castle_test.exs` drives the boundary itself against the real
376395
`:release_handler` — which is running under `mix test`, because castle depends
377-
on sasl — and the real `:init`, naming releases that do not exist.
396+
on sasl — and the real `:init`, naming releases that do not exist. One test
397+
there is not about the boundary: `upgradable/0` rests on a claim about OTP's own
398+
data, that a record read from a `RELEASES` file names applications, so it is
399+
asserted against the record the real `:release_handler` read from the OTP
400+
installation's own file rather than against a stub. It fails, loudly and with
401+
the reason visible, on an installation that has no `releases/RELEASES`.
378402

379403
`test/castle/peer_test.exs` is the exception: it starts real peers. Stubbing the
380404
peer would prove nothing about the one thing it exists to do, which is to run a
@@ -478,24 +502,15 @@ each command prints. Those strings — `Unpacked <vsn> ok`,
478502

479503
## Known limitations
480504

481-
- **Concurrent boots race on `sys.config`.** `generate/1` writes into the
482-
version directory, so simultaneous `start`/`daemon`/`eval` invocations with
483-
differing environments overwrite each other's configuration. Do not fix this
484-
by letting callers choose where the configuration is written: it goes with
485-
`generate/1` itself, once
486-
[forecastle#6](https://github.com/ausimian/forecastle/issues/6) has stopped
487-
intercepting configuration at build time and the third step of
488-
[#13](https://github.com/ausimian/castle/issues/13) has deleted the path that
489-
reads `build.config`. The peer path does not have it — nothing boots to
490-
configure a target — but a boot still goes through `generate/1` until then.
491505
- **How the materialised `sys.config` and a later cold boot of the same version
492-
interact is not verified yet.** Both write the same file. Materialisation
493-
resolves from `sys.config.pristine` and leaves no `config_provider_booted`
494-
marker behind, so a cold boot re-runs the providers over the materialised
495-
result — which is what the issue expects, and what the header Mix wrote is
496-
preserved for. It only becomes reachable with
497-
[forecastle#6](https://github.com/ausimian/forecastle/issues/6), and belongs
498-
there.
506+
interact is not verified.** Both write the same file. Materialisation resolves
507+
from `sys.config.pristine` and leaves no `config_provider_booted` marker
508+
behind, so a cold boot re-runs the providers over the materialised result —
509+
which is what the issue expects, and what the header Mix wrote is preserved
510+
for. That is now reachable, since
511+
[forecastle#6](https://github.com/ausimian/forecastle/issues/6) landed, but
512+
nothing asserts it: Forecastle's `:e2e` suite installs and commits without
513+
restarting afterwards. It belongs there, because it takes a booted release.
499514
- **The public API is undocumented.** `@moduledoc` is still the generated
500515
placeholder and there are no `@doc` or `@spec` annotations
501516
([#11](https://github.com/ausimian/castle/issues/11)).

RELEASE.md

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,23 @@
4141
found to be unbootable — which, for an upgrade that restarts, is found on the
4242
way back up with a rollback as the only way out.
4343

44-
Which way a release is configured is settled by the release itself. One whose
45-
configuration was intercepted at build time — every release assembled by the
46-
Forecastle this is released alongside, recognisable by the `build.config` in
47-
its version directory — is expanded exactly as it was before, so nothing about
48-
installing or committing such a release changes. The new path is taken by a
49-
release whose ordinary Mix provider pipeline is intact, which is the shape
50-
Forecastle stops interfering with in its own next release.
44+
This is how every release is configured now, and the only way: the path that
45+
read a `build.config` is gone, along with the build-time interception that
46+
produced one — see *Removed* below.
47+
- `Castle.upgradable/0`, which succeeds when the release the system is running
48+
can be upgraded from, and fails when it cannot. `:release_handler` reads
49+
`releases/RELEASES` once, as it starts, and when the file is not there it makes
50+
a release record up out of the boot script's name and version — a record that
51+
names no applications at all. Upgrading a system in that state is worse than
52+
being stopped: the install reports success, and every application whose version
53+
changed but whose code the upgrade does not explicitly load goes on running its
54+
old code out of the directory of the release that was just replaced, until a
55+
later `remove` deletes it. Nothing can repair the running system afterwards,
56+
because creating the file changes no record the node holds — so what the
57+
failure says is to restart the system before upgrading it, which is the one
58+
thing that does. The question is asked of the node's own records rather than of
59+
the filesystem, which is the only way to see the case where the file exists but
60+
the boot that went looking for it was earlier.
5161
- `Castle.Error`, the exception raised by a release-management command that did
5262
not succeed.
5363
- `Castle.running/1`, which succeeds when the version it is given is the
@@ -91,7 +101,12 @@
91101
### Changed
92102

93103
- Raised the minimum Elixir requirement to 1.18.
94-
- `unpack/1`, `install/1`, `commit/1`, `remove/1`, `generate/1` and
104+
- `make_releases/0` no longer depends on the working directory. It looks for
105+
`releases/RELEASES` under the root of the release - `code:root_dir()`, which is
106+
the root `:release_handler` resolves its own relative paths against - so the
107+
file it looks for is necessarily the file OTP writes, and a caller that used to
108+
change directory before calling it no longer has to.
109+
- `unpack/1`, `install/1`, `commit/1`, `remove/1` and
95110
`make_releases/0` now fail when the operation fails, instead of printing the
96111
reason and returning normally. These are invoked over `bin/castle`, which
97112
reaches them by `rpc`, and by the launcher's preboot `eval`, so the reason
@@ -102,12 +117,23 @@
102117
node, re-raised in the short-lived VM that made the call, and it is that VM
103118
which exits. What a successful command reports is unchanged.
104119

120+
### Removed
121+
122+
- `Castle.generate/1`, and with it the path through `install/1` and `commit/1`
123+
that read a `build.config`. Expanding the target's configuration by folding
124+
provider state stashed at build time over a renamed `sys.config`, in whichever
125+
version happens to be running, is what the temporary VM above replaces - and
126+
from Forecastle 1.0.0 nothing assembles a release that has a `build.config` to
127+
read. Runtime configuration on a normal boot is Mix's own again, and the
128+
configuration of a version being installed is expanded by that version's own
129+
providers.
130+
105131
### Fixed
106132

107133
- `install/1` reports the emulator restart that an upgrade to a new emulator,
108134
or to a new kernel, stdlib or sasl, needs - rather than failing with a
109135
`CaseClauseError` while the upgrade proceeds.
110136
- `releases/0` reports nothing at all, rather than raising `Enum.EmptyError`,
111137
when no releases are installed.
112-
- `generate/1` and `make_releases/0` say what went wrong - which file could not
113-
be read or written, and why - rather than raising `MatchError`.
138+
- `make_releases/0` says what went wrong - which file could not be read or
139+
written, and why - rather than raising `MatchError`.

0 commit comments

Comments
 (0)