diff --git a/AGENTS.md b/AGENTS.md index 1a3f46b..8ea88cb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,28 +9,24 @@ build-time dependency. Castle's job is configuration and release management on a running node. -- **`Castle.generate/1`** — reads `build.config` from the release's version - directory, folds the stashed config providers over it, and writes the result - as that version's `sys.config`. This is the whole reason the pair exists: - Mix expands runtime configuration once, at boot, from the version it booted; - Castle re-expands it for the version being upgraded *to*, before the relup - runs. It is now the older of two ways to do that — see the next bullet — and - goes away with the third step of - [#13](https://github.com/ausimian/castle/issues/13). - **Materialising the target's configuration**, which `install/1` and `commit/1` - do before they hand a version to `:release_handler`. Which way depends on - whether `releases//build.config` exists, and that is a sound - discriminator because it is Forecastle that creates it: assembling a release - today strips the providers out, stashes their initialised state under - `:castle`, and renames the `sys.config` Mix wrote to `build.config`. So the - file is present exactly when the configuration was intercepted at build time, - and `Castle.generate/1` is then the only thing that can expand it. Note that - it has to be the *presence of `build.config`* rather than the absence of - `sys.config`: once such a release has booted once, it has both. - - When it is absent, Mix's pipeline is intact and `Castle.Peer` materialises the - configuration instead: a `:peer` reached over a loopback socket — so no epmd, - cookie, node name or distribution; the peer reports `nonode@nohost` and + do before they hand a version to `:release_handler`. This is the whole reason + the pair exists: Mix expands runtime configuration once, at boot, from the + version it booted; Castle expands it for the version being upgraded *to*, + before the relup runs. + + There is one way it happens, and there used to be two. The other read a + `build.config` — the `sys.config` Forecastle renamed at assembly time, having + stripped the providers out and stashed their initialised state under + `:castle` — and folded that state over it in the running node, which is + `Castle.generate/1`. Both halves of that are gone: forecastle#6 stopped + intercepting configuration at build time, and the third step of + [#13](https://github.com/ausimian/castle/issues/13) deleted the path that read + it. Do not reintroduce either. A release whose providers ran in the version + that happens to be running was configured by the wrong code. + + What is left is `Castle.Peer`: a `:peer` reached over a loopback socket — so no + epmd, cookie, node name or distribution; the peer reports `nonode@nohost` and `is_alive() == false` — booted on the target's own `preboot` script and its own emulator, which runs `Config.Provider.boot/1` over the target's own 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. scratch will be created with that group and the mode bits will be honoured against it. - `Castle.Commands.write_sys_config/2`, on the `build.config` path, is the one - place this rule is not applied: it creates `sys.config` with the process umask - when the file does not exist yet. There is no transient exposure there — the - mode it is granted is the mode it keeps — and that path is deleted in step 3, - while "nothing observable changes for a release assembled by today's - Forecastle" pins its behaviour until then. It is a real gap, recorded rather - than fixed here. - A base that cannot be read as a configuration is refused, naming the remedy, rather than resolved from: it is preferred to `sys.config` by definition, so failing loudly is the only safe thing left. - This is permanent design: the `build.config` path has always had a pristine - base — - `build.config` *is* one — and this is what carries that property forward when - step 3 deletes it. It is deliberately not called `build.config`, since that - name is the discriminator and would send the release back down the path being - removed. `sys.config` gains a `CASTLE_MATERIALISED` comment line, which makes + This is permanent design: the path this replaced always had a pristine base — + `build.config` *was* one, and nothing ever wrote it — and this is what carries + that property forward now that it is gone. + `sys.config` gains a `CASTLE_MATERIALISED` comment line, which makes the invariant checkable: written by Castle, so a base must exist. A version that says that and has no base beside it is refused, with the remedy (unpack 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. - **`Castle.make_releases/0`** — creates the `RELEASES` file from the running permanent release if it does not already exist, so a release assembled by Mix - can manage its own upgrades. + can manage its own upgrades. The directory is derived from `code:root_dir()`, + which is the root `:release_handler` resolves *its* relative paths against + (`consult/2` is `file:consult(root_dir_relative_path(File))`, and + `do_write_release/3` the same), so no caller has to change directory and none + should: the working directory was only ever visible to the `File.exists?/1` + guard, which is what let the file this looked for and the file OTP wrote be + different ones. It calls **`create_RELEASES/3`**, never `/4` with the root + supplied: `/3` is `create_RELEASES("", RelDir, RelFile, LibDirs)`, and + `check_rel_data/4` stores library directories as `lib/-` when the + root is empty and as absolute paths under it when it is not — "to make it easy + to create a relocatable RELEASES file", in OTP's own words. Passing the root + would bake this machine's paths into a file whose point is that it can be + moved, and no end-state test would see it. +- **`Castle.upgradable/0`** — succeeds when the running release can be upgraded + from, and refuses when `:release_handler` is working from the record it + synthesises for itself. It reads `RELEASES` once, in `init/1`, and when it + cannot it builds a record out of the boot script's name and version with the + `libs` field left at `[]`. Nothing can replace that afterwards, and creating + the file later does not: the first operation that changes anything writes the + in-memory record back over it. Upgrading from it is silently wrong rather than + refused — the relup's `point_of_no_return` switches code paths for + `get_new_libs(Current, New)`, which folds over the *current* release's + applications and so yields nothing at all, leaving any application whose + version changed but whose code the relup does not load running from the + directory of the release being replaced. The discriminator is that empty + application list, and it is exact: `which_releases/0` reports + `mk_lib_name(Libs)`, `mk_lib_name([]) -> []`, and a record read from a + `RELEASES` file names at least `kernel` and `stdlib`. It has to be asked of the + node rather than of the filesystem, which is why this is here and not in + `bin/castle`: a file that appeared *after* the boot that looked for it passes + a shell test for the file and still leaves the node on the synthesised record. + The remedy the message names is a restart, because that is the only thing that + changes the answer. - **`unpack/1`, `install/1`, `commit/1`, `remove/1`, `releases/0`** — wrappers over `:release_handler`, with the target version's configuration materialised 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. rebooted, and an emulator upgrade finishes on the way back up, where it can still roll back. So Castle answers the question and leaves the asking to Forecastle: `bin/castle install` repeats it rather than trusting the reply, - from Forecastle 1.0.0 — the revision pinned in this project's `mix.lock` - installs with a single rpc and never calls this, so do not describe the - polling as something Castle's own integrated state does. Two conditions. The + from Forecastle 1.0.0 — so the polling is Forecastle's, and not something + Castle's own state does. Two conditions. The version is the running release: the `current` one, or the `permanent` one when none is current — `install` leaves 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 the operations themselves, returning their outcome instead of acting on the process, which is what makes them testable. -Forecastle is what arranges for these to be reachable: it renames `sys.config` -to `build.config` at assembly time, adds a `:preboot` script that starts -`:castle`, and writes the `env.sh` fragment and `bin/castle` wrapper that call -into this module. +Forecastle is what arranges for these to be reachable: it leaves the +configuration Mix wrote alone, adds a `:preboot` script that starts `:castle`, +and writes the `env.sh` fragment and `bin/castle` wrapper that call into this +module. ## Layout @@ -370,11 +387,18 @@ into this module. `mix test` covers `Castle.Commands` as units. `:release_handler`, `:init` and `Castle.Peer` are reached through module arguments that default to them, so the tests hand them `Castle.ReleaseHandlerStub`, `Castle.InitStub` and -`Castle.PeerStub` instead; `generate/1` and `materialise/2` take the version -directory they work on, so the tests give them a `tmp_dir`. +`Castle.PeerStub` instead; `materialise/2` takes the version directory it works +on and `make_releases/2` the releases directory, so the tests give them a +`tmp_dir` — and neither the commands nor their tests touch the working +directory, which is what lets them all run async. `test/castle_test.exs` drives the boundary itself against the real `:release_handler` — which is running under `mix test`, because castle depends -on sasl — and the real `:init`, naming releases that do not exist. +on sasl — and the real `:init`, naming releases that do not exist. One test +there is not about the boundary: `upgradable/0` rests on a claim about OTP's own +data, that a record read from a `RELEASES` file names applications, so it is +asserted against the record the real `:release_handler` read from the OTP +installation's own file rather than against a stub. It fails, loudly and with +the reason visible, on an installation that has no `releases/RELEASES`. `test/castle/peer_test.exs` is the exception: it starts real peers. Stubbing the 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 ok`, ## Known limitations -- **Concurrent boots race on `sys.config`.** `generate/1` writes into the - version directory, so simultaneous `start`/`daemon`/`eval` invocations with - differing environments overwrite each other's configuration. Do not fix this - by letting callers choose where the configuration is written: it goes with - `generate/1` itself, once - [forecastle#6](https://github.com/ausimian/forecastle/issues/6) has stopped - intercepting configuration at build time and the third step of - [#13](https://github.com/ausimian/castle/issues/13) has deleted the path that - reads `build.config`. The peer path does not have it — nothing boots to - configure a target — but a boot still goes through `generate/1` until then. - **How the materialised `sys.config` and a later cold boot of the same version - interact is not verified yet.** Both write the same file. Materialisation - resolves from `sys.config.pristine` and leaves no `config_provider_booted` - marker behind, so a cold boot re-runs the providers over the materialised - result — which is what the issue expects, and what the header Mix wrote is - preserved for. It only becomes reachable with - [forecastle#6](https://github.com/ausimian/forecastle/issues/6), and belongs - there. + interact is not verified.** Both write the same file. Materialisation resolves + from `sys.config.pristine` and leaves no `config_provider_booted` marker + behind, so a cold boot re-runs the providers over the materialised result — + which is what the issue expects, and what the header Mix wrote is preserved + for. That is now reachable, since + [forecastle#6](https://github.com/ausimian/forecastle/issues/6) landed, but + nothing asserts it: Forecastle's `:e2e` suite installs and commits without + restarting afterwards. It belongs there, because it takes a booted release. - **The public API is undocumented.** `@moduledoc` is still the generated placeholder and there are no `@doc` or `@spec` annotations ([#11](https://github.com/ausimian/castle/issues/11)). diff --git a/RELEASE.md b/RELEASE.md index 8ff93aa..b611d01 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -41,13 +41,23 @@ found to be unbootable — which, for an upgrade that restarts, is found on the way back up with a rollback as the only way out. - Which way a release is configured is settled by the release itself. One whose - configuration was intercepted at build time — every release assembled by the - Forecastle this is released alongside, recognisable by the `build.config` in - its version directory — is expanded exactly as it was before, so nothing about - installing or committing such a release changes. The new path is taken by a - release whose ordinary Mix provider pipeline is intact, which is the shape - Forecastle stops interfering with in its own next release. + This is how every release is configured now, and the only way: the path that + read a `build.config` is gone, along with the build-time interception that + produced one — see *Removed* below. +- `Castle.upgradable/0`, which succeeds when the release the system is running + can be upgraded from, and fails when it cannot. `:release_handler` reads + `releases/RELEASES` once, as it starts, and when the file is not there it makes + a release record up out of the boot script's name and version — a record that + names no applications at all. Upgrading a system in that state is worse than + being stopped: the install reports success, and every application whose version + changed but whose code the upgrade does not explicitly load goes on running its + old code out of the directory of the release that was just replaced, until a + later `remove` deletes it. Nothing can repair the running system afterwards, + because creating the file changes no record the node holds — so what the + failure says is to restart the system before upgrading it, which is the one + thing that does. The question is asked of the node's own records rather than of + the filesystem, which is the only way to see the case where the file exists but + the boot that went looking for it was earlier. - `Castle.Error`, the exception raised by a release-management command that did not succeed. - `Castle.running/1`, which succeeds when the version it is given is the @@ -91,7 +101,12 @@ ### Changed - Raised the minimum Elixir requirement to 1.18. -- `unpack/1`, `install/1`, `commit/1`, `remove/1`, `generate/1` and +- `make_releases/0` no longer depends on the working directory. It looks for + `releases/RELEASES` under the root of the release - `code:root_dir()`, which is + the root `:release_handler` resolves its own relative paths against - so the + file it looks for is necessarily the file OTP writes, and a caller that used to + change directory before calling it no longer has to. +- `unpack/1`, `install/1`, `commit/1`, `remove/1` and `make_releases/0` now fail when the operation fails, instead of printing the reason and returning normally. These are invoked over `bin/castle`, which reaches them by `rpc`, and by the launcher's preboot `eval`, so the reason @@ -102,6 +117,17 @@ node, re-raised in the short-lived VM that made the call, and it is that VM which exits. What a successful command reports is unchanged. +### Removed + +- `Castle.generate/1`, and with it the path through `install/1` and `commit/1` + that read a `build.config`. Expanding the target's configuration by folding + provider state stashed at build time over a renamed `sys.config`, in whichever + version happens to be running, is what the temporary VM above replaces - and + from Forecastle 1.0.0 nothing assembles a release that has a `build.config` to + read. Runtime configuration on a normal boot is Mix's own again, and the + configuration of a version being installed is expanded by that version's own + providers. + ### Fixed - `install/1` reports the emulator restart that an upgrade to a new emulator, @@ -109,5 +135,5 @@ `CaseClauseError` while the upgrade proceeds. - `releases/0` reports nothing at all, rather than raising `Enum.EmptyError`, when no releases are installed. -- `generate/1` and `make_releases/0` say what went wrong - which file could not - be read or written, and why - rather than raising `MatchError`. +- `make_releases/0` says what went wrong - which file could not be read or + written, and why - rather than raising `MatchError`. diff --git a/lib/castle.ex b/lib/castle.ex index caaabda..b16b50c 100644 --- a/lib/castle.ex +++ b/lib/castle.ex @@ -7,9 +7,10 @@ defmodule Castle do # Every function in this module is a command entry point: `bin/castle` sends # each one to the running node over `bin/ rpc`, and the launcher's - # env.sh fragment evaluates generate/1 and make_releases/0 in the preboot VM. - # There is no separate CLI layer to carry the process status, so these - # functions are the command boundary, and it is here that a failure raises. + # env.sh fragment evaluates make_releases/0 in the preboot VM, on the first + # start of a deployment. There is no separate CLI layer to carry the process + # status, so these functions are the command boundary, and it is here that a + # failure raises. # # Raising, rather than halting or returning: the rpc expression runs on the # running release node, so halting there would halt the system under @@ -22,11 +23,11 @@ defmodule Castle do # rather than acting on the process, so that they can be tested. def make_releases do - report!(Commands.make_releases()) + report!(Commands.make_releases(rel_dir())) end - def generate(vsn) do - report!(Commands.generate(rel_vsn_dir(vsn))) + def upgradable do + report!(Commands.upgradable()) end def unpack(name) when is_binary(name) do @@ -64,12 +65,17 @@ defmodule Castle do # may fail without saying that an install happened. defp materialise(vsn), do: report!(Commands.materialise(rel_vsn_dir(vsn))) - # The version directory of the release being operated on, under the root of - # the release that is running. Derived, never chosen by the caller: which file - # the configuration lands in is a property of the installation, not an - # argument. It resolves for any version the running release knows about, - # because `:release_handler` unpacks every version into this same root. - defp rel_vsn_dir(vsn), do: Path.join([:code.root_dir(), "releases", vsn]) + # The release directory, and the version directory of the release being + # operated on beneath it. Derived, never chosen by the caller: which file the + # configuration lands in, and which file the release records go in, are + # properties of the installation rather than arguments. `code:root_dir()` is + # the root because that is the root `:release_handler` itself resolves + # relative paths against, so these name the files it will read, and a caller's + # working directory cannot make them name different ones. The version + # directory resolves for any version the running release knows about, because + # `:release_handler` unpacks every version into this same root. + defp rel_dir, do: Path.join(to_string(:code.root_dir()), "releases") + defp rel_vsn_dir(vsn), do: Path.join(rel_dir(), vsn) defp report!({:ok, lines}), do: Enum.each(lines, &IO.puts/1) defp report!({:error, message}), do: raise(Castle.Error, message) diff --git a/lib/castle/commands.ex b/lib/castle/commands.ex index 1ce0d1d..695416d 100644 --- a/lib/castle/commands.ex +++ b/lib/castle/commands.ex @@ -14,41 +14,54 @@ defmodule Castle.Commands do # signatures - the ones `bin/castle` and `env.sh` call - unchanged. Nothing # outside `Castle` is meant to call this module. - @app Mix.Project.config()[:app] - @typedoc """ The outcome of a command: the lines to report, or the message to fail with. """ @type result :: {:ok, [String.t()]} | {:error, String.t()} - @reldir "releases" - @doc """ - Creates the `RELEASES` file from the running permanent release. - - Does nothing if the file already exists. Resolved relative to the current - working directory, which the caller is expected to have set to the release - root. + Creates the `RELEASES` file in `rel_dir` from the running permanent release. + + Does nothing if the file already exists. + + The directory is an argument because that is what makes this testable, not + because a caller gets to choose it: `Castle.make_releases/0` derives it from + the root of the release, and that is the same root `:release_handler` resolves + its own relative paths against - `code:root_dir()`, never the working + directory (`consult/2` is `file:consult(root_dir_relative_path(File))`, and + `do_write_release/3` the same). So the working directory was only ever visible + to the check below, which is what made it possible for the file this looked + for and the file `create_RELEASES/3` wrote to be different ones. Nothing has + to change directory to call this, and nothing should. """ - @spec make_releases(module()) :: result() - def make_releases(handler \\ :release_handler) do - releases_file = Path.join(@reldir, "RELEASES") + @spec make_releases(Path.t(), module()) :: result() + def make_releases(rel_dir, handler \\ :release_handler) do + releases_file = Path.join(rel_dir, "RELEASES") if File.exists?(releases_file) do {:ok, []} else {:ok, _} = Application.ensure_all_started(:sasl) - create_releases(releases_file, handler) + create_releases(rel_dir, releases_file, handler) end end - defp create_releases(releases_file, handler) do + # `create_RELEASES/3`, and not `/4` with the root supplied. The three-argument + # form is `create_RELEASES("", RelDir, RelFile, LibDirs)`, and `check_rel_data/4` + # keys off that empty `Root`: with it, each library directory is stored as + # `lib/-` - "to make it easy to create a relocatable RELEASES file", + # in OTP's own comment - and with a root supplied, as an absolute path under + # it. Passing the root would therefore write this machine's paths into a file + # whose whole point is that it can be moved, and nothing would notice until it + # was. + defp create_releases(rel_dir, releases_file, handler) do case handler.which_releases(:permanent) do [{name, vsn, _, _}] -> - relfile = Path.join([@reldir, vsn, "#{name}.rel"]) + # `vsn` arrives as a charlist, which `Path.join/1` takes as chardata. + relfile = Path.join([rel_dir, vsn, "#{name}.rel"]) # credo:disable-for-next-line Credo.Check.Readability.FunctionNames - case handler.create_RELEASES(to_charlist(@reldir), relfile, []) do + case handler.create_RELEASES(to_charlist(rel_dir), relfile, []) do :ok -> {:ok, []} @@ -67,90 +80,95 @@ defmodule Castle.Commands do end @doc """ - Materialises the configuration of the release in `rel_vsn_dir`. - - Two shapes of release reach this, and the presence of `build.config` is what - tells them apart. Forecastle used to intercept configuration at assembly time: - it stripped the providers out of the release, stashed their initialised state - under this application's key, and renamed the `sys.config` Mix had written to - `build.config`. The only thing that can expand a release assembled that way is - `generate/1`, folding the stashed state over the file it was taken from - and - the presence of `build.config` is the test rather than the absence of - `sys.config`, because from its first boot onwards such a release has both: - writing one beside the other is what `generate/1` does. - - A release Mix configured normally has its providers where Mix put them and its - `sys.config` under the name Mix gave it, and nothing has been renamed - so the - absence of `build.config` says the pipeline is intact, and the target can be - evaluated the way Elixir intends: in a VM of its own, running its own - providers, which is what `Castle.Peer` does. That is the only sound way to do - it, since a provider module can differ between the version that is running and - the version being installed. - - The module is an argument for the same reason `:release_handler` is: so that a - test can watch which way the decision went without starting a VM. + Confirms that the running release can be upgraded from. + + `:release_handler` reads `RELEASES` once, in its `init/1`, and when it cannot + it synthesises a record out of the boot script's name and version instead - + `[#release{name = Name, vsn = Vsn, status = permanent}]`, leaving the `libs` + field at its default of `[]`. That record is what such a node then works from + for the rest of its life: creating the file afterwards changes nothing, and + the first operation that changes anything writes the in-memory record straight + back over it. + + Upgrading from it is worse than being refused. The library directories a + release is loaded from are switched at the relup's `point_of_no_return`, which + calls `code:replace_path/2` over `get_new_libs(Current, New)` - the + applications whose version differs between the two records - together with + whichever ones the relup loads object code for. `get_new_libs/2` folds over + the *current* release's applications, and `get_new_libs([], _) -> []`, so a + record that names none switches nothing: an application whose version did + change, and whose code the relup does not explicitly load, goes on running + from the library directory of the release being replaced. The install reports + success, and that directory survives until the next `remove` deletes it. + + The discriminator is that empty application list, and it is exact: the list + `which_releases/0` reports is `mk_lib_name(Libs)`, `mk_lib_name([]) -> []`, + and a record read from `RELEASES` names at least `kernel` and `stdlib`. So + emptiness distinguishes the synthesised record from every real one, which + testing for the file cannot do - a file that appeared after the boot that + looked for it passes that test and leaves the node on the synthesised record + regardless. + + The running release is selected the way `running/3` selects it: the `current` + one if there is one, and the `permanent` one otherwise. """ - @spec materialise(Path.t(), module()) :: result() - def materialise(rel_vsn_dir, peer \\ Castle.Peer) do - cond do - File.exists?(Path.join(rel_vsn_dir, "build.config")) -> - generate(rel_vsn_dir) - - File.dir?(rel_vsn_dir) -> - peer.materialise(rel_vsn_dir) + @spec upgradable(module()) :: result() + def upgradable(handler \\ :release_handler) do + case running_release(handler) do + {_vsn, [_ | _]} -> + {:ok, []} - true -> + {vsn, []} -> {:error, - "Cannot configure #{Path.basename(rel_vsn_dir)}: #{rel_vsn_dir} does not exist. " <> - "Unpack the release first."} + "#{vsn} is running from a release record OTP built from the boot script, which " <> + "names no applications: releases/RELEASES was missing, or could not be read, " <> + "when the system booted. An upgrade from it reports success and leaves any " <> + "application whose version changed, but whose code the upgrade does not load, " <> + "running its old code. Creating the file now would not change the record this " <> + "node works from. Restart the system before upgrading it: the release creates " <> + "the file before it starts."} + + nil -> + {:error, "No release is running, so this system cannot be upgraded."} end end @doc """ - Expands the build-time configuration in `rel_vsn_dir` into its `sys.config`. + Materialises the configuration of the release in `rel_vsn_dir`. - The directory is the version directory of the release being configured. It is - an argument because that is what makes this testable, not because a caller - gets to choose it: `Castle.generate/1` derives it from the running release, - and the configuration always lands beside the `build.config` it came from. - """ - @spec generate(Path.t()) :: result() - def generate(rel_vsn_dir) do - build_config_path = Path.join(rel_vsn_dir, "build.config") + There is one way to do that: in a VM of its own, running the target's own + provider modules over the target's own configuration, which is what + `Castle.Peer` does. A provider module can differ between the version that is + running and the version being installed - that is precisely what an upgrade + may change - so the running node is not a place where the answer can be + worked out. - case :file.consult(to_charlist(build_config_path)) do - {:ok, [build_config]} -> - write_sys_config(rel_vsn_dir, expand(build_config)) + What is left here is the one thing the peer cannot say well: that there is no + release at that path to configure at all. The peer's own refusals name a file + the version is missing, or something its providers did - the right answers for + a release that was unpacked and then damaged, and the wrong ones for a version + that was never unpacked. So a version directory that is absent, empty, or not + a directory is answered here, where the remedy can be named. - {:ok, terms} -> - {:error, "Cannot read #{build_config_path}: expected one term, found #{length(terms)}."} + The module is an argument for the same reason `:release_handler` is: so that a + test can see what was asked of it without starting a VM. + """ + @spec materialise(Path.t(), module()) :: result() + def materialise(rel_vsn_dir, peer \\ Castle.Peer) do + case File.ls(rel_vsn_dir) do + {:ok, [_ | _]} -> + peer.materialise(rel_vsn_dir) - {:error, reason} -> - {:error, "Cannot read #{build_config_path}. #{:file.format_error(reason)}"} + nothing -> + {:error, + "Cannot configure #{Path.basename(rel_vsn_dir)}: " <> + "#{rel_vsn_dir} #{describe(nothing)}. Unpack the release first."} end end - # The providers were stashed under this application's key at build time, each - # already initialised, so all that is left to do is fold them over the - # configuration they were built from. An exception raised by a provider - a - # runtime.exs that cannot find what it needs, say - is left to propagate: it - # describes the problem better than anything that could be said here. - defp expand(build_config) do - build_config - |> Keyword.get(@app, []) - |> Keyword.get(:config_providers, []) - |> Enum.reduce(build_config, fn {mod, arg}, cfg -> mod.load(cfg, arg) end) - end - - defp write_sys_config(rel_vsn_dir, sys_config) do - path = Path.join(rel_vsn_dir, "sys.config") - contents = :io_lib.format(~c"%% coding: utf-8~n~tp.~n", [sys_config]) - - case File.write(path, contents) do - :ok -> {:ok, []} - {:error, reason} -> {:error, "Cannot write #{path}. #{:file.format_error(reason)}"} - end - end + defp describe({:ok, []}), do: "is empty" + defp describe({:error, :enoent}), do: "does not exist" + defp describe({:error, reason}), do: "cannot be read (#{:file.format_error(reason)})" @doc """ Unpacks the named release tarball. @@ -219,9 +237,9 @@ defmodule Castle.Commands do @spec running(String.t(), module(), module()) :: result() def running(vsn, handler \\ :release_handler, init \\ :init) do case running_release(handler) do - ^vsn -> booted(vsn, init) + {^vsn, _apps} -> booted(vsn, init) nil -> {:error, "#{vsn} is not the running release. No release is running."} - other -> {:error, "#{vsn} is not the running release. #{other} is."} + {other, _apps} -> {:error, "#{vsn} is not the running release. #{other} is."} end end @@ -257,17 +275,22 @@ defmodule Castle.Commands do end end + # The release the system is running, as `{vsn, apps}`, or `nil` if there is + # none. The application list comes along because `upgradable/1` reads it, and + # both questions have to be asked of the same release. defp running_release(handler) do - releases = for {_, vsn, _, status} <- handler.which_releases(), do: {to_string(vsn), status} + releases = + for {_, vsn, apps, status} <- handler.which_releases(), + do: {to_string(vsn), apps, status} case with_status(releases, :current) do nil -> with_status(releases, :permanent) - vsn -> vsn + running -> running end end defp with_status(releases, wanted) do - Enum.find_value(releases, fn {vsn, status} -> if status == wanted, do: vsn end) + Enum.find_value(releases, fn {vsn, apps, status} -> if status == wanted, do: {vsn, apps} end) end @doc """ diff --git a/lib/castle/peer.ex b/lib/castle/peer.ex index b47ddf7..819d37e 100644 --- a/lib/castle/peer.ex +++ b/lib/castle/peer.ex @@ -131,13 +131,12 @@ defmodule Castle.Peer do # nothing reads it: an install cannot tell its own leftovers from another # install's work in progress, so it does not try. # - # This is permanent, not a step in the migration. The `build.config` path this - # sits beside has always had a pristine base - `build.config` *is* one, and - # `generate/1` only ever reads it - and that is the one thing the old - # mechanism got right. When step 3 of castle#13 deletes that path, this is - # what carries the property forward. It is deliberately not *called* - # `build.config`: that name is the discriminator between the two paths, and a - # file by that name would send the release back down the one being removed. + # This is permanent, not a step in the migration. The path this replaced - + # `Castle.generate/1`, folding provider state stashed at build time over the + # `build.config` Forecastle renamed `sys.config` to - always had a pristine + # base, because `build.config` *was* one and nothing ever wrote it. That was + # the one thing the old mechanism got right, and this is what carries the + # property forward now that it is gone. # # `sys.config` gains a `CASTLE_MATERIALISED` line when it is written, which is # what makes the invariant checkable: written by Castle, so a base must exist. @@ -399,7 +398,7 @@ defmodule Castle.Peer do sys_config = Path.join(rel_vsn_dir, @sys_config) vsn = Path.basename(rel_vsn_dir) - with :ok <- regular(sys_config, "#{vsn} has neither a sys.config nor a build.config."), + with :ok <- regular(sys_config, "#{vsn} has no configuration to evaluate."), :ok <- regular(boot <> ".boot", "Its configuration is evaluated on that script."), {:ok, erl} <- emulator(root, rel_vsn_dir), {:ok, work} <- work_dir(rel_vsn_dir) do diff --git a/test/castle/commands_test.exs b/test/castle/commands_test.exs index 694d6ba..e340e23 100644 --- a/test/castle/commands_test.exs +++ b/test/castle/commands_test.exs @@ -2,46 +2,22 @@ defmodule Castle.CommandsTest do use ExUnit.Case, async: true alias Castle.Commands - alias Castle.ConfigProviderStub alias Castle.InitStub alias Castle.PeerStub alias Castle.ReleaseHandlerStub, as: Stub describe "materialise/2" do @tag :tmp_dir - test "expands build.config, and starts nothing, when there is one", %{tmp_dir: dir} do - write_build_config(dir, - castle: [config_providers: [{ConfigProviderStub, merge: [sample: [greeting: "runtime"]]}]], - sample: [greeting: "build"] - ) + test "hands an unpacked version to the peer", %{tmp_dir: dir} do + unpacked(dir) - # The peer stub has no registered reply, so it raises if it is reached. - assert Commands.materialise(dir, PeerStub) == {:ok, []} - assert PeerStub.calls() == [] - assert read_sys_config(dir)[:sample][:greeting] == "runtime" - end - - @tag :tmp_dir - test "keeps expanding build.config once it has written a sys.config", %{tmp_dir: dir} do - # Which is the state every release assembled by today's Forecastle is in - # from its first boot onwards, so the discriminator has to be the presence - # of build.config and not the absence of sys.config. - write_build_config(dir, sample: [greeting: "build"]) - File.write!(Path.join(dir, "sys.config"), "[].\n") - - assert Commands.materialise(dir, PeerStub) == {:ok, []} - assert PeerStub.calls() == [] - assert read_sys_config(dir) == [sample: [greeting: "build"]] - end - - @tag :tmp_dir - test "hands a release whose pipeline is intact to the peer", %{tmp_dir: dir} do assert Commands.materialise(dir, PeerStub.stub({:ok, []})) == {:ok, []} assert PeerStub.calls() == [dir] end @tag :tmp_dir test "reports what the peer could not do", %{tmp_dir: dir} do + unpacked(dir) peer = PeerStub.stub({:error, "DATABASE_URL is not set"}) assert Commands.materialise(dir, peer) == {:error, "DATABASE_URL is not set"} @@ -53,6 +29,7 @@ defmodule Castle.CommandsTest do # ready to accept an install that must not be asked for. Everything able # to refuse belongs on this side of the mutation, and a configuration that # could not be materialised is the whole of what this adds to that list. + unpacked(dir) peer = PeerStub.stub({:error, "the compile environment does not agree"}) Stub.stub(:install_release, {:ok, ~c"1.2.2", ~c"upgrade"}) @@ -63,79 +40,72 @@ defmodule Castle.CommandsTest do @tag :tmp_dir test "reports a version that has not been unpacked", %{tmp_dir: dir} do + # The peer stub has no registered reply, so it raises if it is reached. missing = Path.join(dir, "9.9.9") assert {:error, message} = Commands.materialise(missing, PeerStub) - assert message =~ "Cannot configure 9.9.9" + assert message =~ "Cannot configure 9.9.9: #{missing} does not exist." assert message =~ "Unpack the release first" assert PeerStub.calls() == [] end - end - - describe "generate/1" do - @tag :tmp_dir - test "expands the build configuration through the config providers", %{tmp_dir: dir} do - write_build_config(dir, - castle: [config_providers: [{ConfigProviderStub, merge: [sample: [greeting: "runtime"]]}]], - sample: [greeting: "build", untouched: true] - ) - - assert Commands.generate(dir) == {:ok, []} - - config = read_sys_config(dir) - assert config[:sample][:greeting] == "runtime" - assert config[:sample][:untouched] == true - end - - @tag :tmp_dir - test "writes the build configuration as-is when there are no providers", %{tmp_dir: dir} do - write_build_config(dir, sample: [greeting: "build"]) - - assert Commands.generate(dir) == {:ok, []} - assert read_sys_config(dir) == [sample: [greeting: "build"]] - end @tag :tmp_dir - test "reports a missing build configuration", %{tmp_dir: dir} do - assert {:error, message} = Commands.generate(dir) - assert message =~ Path.join(dir, "build.config") - assert message =~ "no such file or directory" + test "reports a version directory with nothing in it", %{tmp_dir: dir} do + # Which is not the same thing as a version that was unpacked and then had + # its configuration removed - the peer names the file that is missing for + # that - so it does not claim to be, and it does say what to do about it. + empty = Path.join(dir, "9.9.9") + File.mkdir!(empty) + + assert {:error, message} = Commands.materialise(empty, PeerStub) + assert message =~ "Cannot configure 9.9.9: #{empty} is empty." + assert message =~ "Unpack the release first" + assert PeerStub.calls() == [] end + end - @tag :tmp_dir - test "reports an unreadable build configuration", %{tmp_dir: dir} do - File.write!(Path.join(dir, "build.config"), "]].\n") + describe "upgradable/1" do + test "confirms a system whose release record was read from RELEASES" do + handler = + Stub.stub(:which_releases, [ + {~c"sample", ~c"1.2.3", [~c"kernel-10.5", ~c"stdlib-7.2"], :permanent} + ]) - assert {:error, message} = Commands.generate(dir) - assert message =~ Path.join(dir, "build.config") + assert Commands.upgradable(handler) == {:ok, []} end - @tag :tmp_dir - test "reports a build configuration holding more than one term", %{tmp_dir: dir} do - File.write!(Path.join(dir, "build.config"), "[].\n[].\n") + test "refuses a system running on a record OTP synthesised" do + # release_handler could not read RELEASES when it started, so it built a + # record out of the boot script's name and version, whose libs field is + # empty - and mk_lib_name([]) is [], which no real record reports. + handler = Stub.stub(:which_releases, [{~c"sample", ~c"1.2.3", [], :permanent}]) - assert {:error, message} = Commands.generate(dir) - assert message =~ "expected one term, found 2" + assert {:error, message} = Commands.upgradable(handler) + assert message =~ "1.2.3 is running from a release record OTP built from the boot script" + assert message =~ "names no applications" + assert message =~ "running its old code" + assert message =~ "Restart the system before upgrading it" end - @tag :tmp_dir - test "reports a sys.config it cannot write", %{tmp_dir: dir} do - write_build_config(dir, sample: [greeting: "build"]) - # Whatever the reason, the operator has to be told which file it was. - File.mkdir_p!(Path.join(dir, "sys.config")) + test "asks the release the system is running, and not another one" do + # The synthesised record is the permanent one, and an install leaves its + # target current - so which release is asked has to be the running one, + # the way running/3 selects it, or a system that has already upgraded once + # would be refused for the state of the record it came from. + handler = + Stub.stub(:which_releases, [ + {~c"sample", ~c"1.2.3", [~c"kernel-10.5"], :current}, + {~c"sample", ~c"1.2.2", [], :permanent} + ]) - assert {:error, message} = Commands.generate(dir) - assert message =~ Path.join(dir, "sys.config") + assert Commands.upgradable(handler) == {:ok, []} end - @tag :tmp_dir - test "lets a failing config provider speak for itself", %{tmp_dir: dir} do - write_build_config(dir, - castle: [config_providers: [{ConfigProviderStub, raise: "DATABASE_URL is not set"}]] - ) + test "refuses a system with no release running at all" do + handler = Stub.stub(:which_releases, [{~c"sample", ~c"1.2.3", [], :unpacked}]) - assert_raise RuntimeError, "DATABASE_URL is not set", fn -> Commands.generate(dir) end - refute File.exists?(Path.join(dir, "sys.config")) + assert Commands.upgradable(handler) == + {:error, "No release is running, so this system cannot be upgraded."} end end @@ -345,15 +315,8 @@ defmodule Castle.CommandsTest do # What a node reports once its boot script has run to the end. defp booted, do: InitStub.stub({:starting, :started}) - defp write_build_config(dir, config) do - File.write!( - Path.join(dir, "build.config"), - :io_lib.format(~c"%% coding: utf-8~n~tp.~n", [config]) - ) - end - - defp read_sys_config(dir) do - assert {:ok, [config]} = :file.consult(to_charlist(Path.join(dir, "sys.config"))) - config - end + # Enough of an unpacked version directory for `materialise/2`: what is in it is + # the peer's business, and the peer is a stub here. Everything it would look + # for is covered against a real one in `Castle.PeerTest`. + defp unpacked(dir), do: File.write!(Path.join(dir, "sys.config"), "[].\n") end diff --git a/test/castle/make_releases_test.exs b/test/castle/make_releases_test.exs index 9065a4e..e4567aa 100644 --- a/test/castle/make_releases_test.exs +++ b/test/castle/make_releases_test.exs @@ -1,37 +1,49 @@ defmodule Castle.MakeReleasesTest do - # make_releases/1 resolves the releases directory relative to the working - # directory, the way the launcher's env.sh fragment calls it, and the working - # directory belongs to the whole VM. - use ExUnit.Case, async: false + # The releases directory is an argument, so nothing here touches the working + # directory and these can run alongside everything else. `Castle.make_releases/0` + # derives it from `code:root_dir()`, which is what `:release_handler` resolves + # its own relative paths against - and which, under `mix test`, is the OTP + # installation, so the derivation itself is exercised at the boundary in + # `CastleTest` rather than here. + use ExUnit.Case, async: true alias Castle.Commands alias Castle.ReleaseHandlerStub, as: Stub @moduletag :tmp_dir - describe "make_releases/1" do + describe "make_releases/2" do test "leaves an existing RELEASES file alone", %{tmp_dir: dir} do - File.mkdir_p!(Path.join(dir, "releases")) - File.write!(Path.join([dir, "releases", "RELEASES"]), "") + rel_dir = rel_dir(dir) + File.write!(Path.join(rel_dir, "RELEASES"), "") # The stub has no registered replies, so it raises if it is consulted. - assert make_releases(dir, Stub) == {:ok, []} + assert Commands.make_releases(rel_dir, Stub) == {:ok, []} assert Stub.calls(:which_releases) == [] end test "creates it from the release running as permanent", %{tmp_dir: dir} do + rel_dir = rel_dir(dir) handler = Stub.stub(:which_releases, [{~c"sample", ~c"0.1.0", [], :permanent}]) Stub.stub(:create_RELEASES, :ok) - assert make_releases(dir, handler) == {:ok, []} - assert Stub.calls(:create_RELEASES) == [[~c"releases", "releases/0.1.0/sample.rel", []]] + assert Commands.make_releases(rel_dir, handler) == {:ok, []} + + # The release directory it was given, and the .rel file beneath it. The + # root is *not* passed: create_RELEASES/3 is create_RELEASES("", RelDir, + # RelFile, LibDirs), and the empty root is what makes the library paths in + # the file relative, so that the release can be moved. A fourth argument + # here would bake this machine's paths into it. + assert Stub.calls(:create_RELEASES) == [ + [to_charlist(rel_dir), Path.join(rel_dir, "0.1.0/sample.rel"), []] + ] end test "reports having nothing to create it from", %{tmp_dir: dir} do handler = Stub.stub(:which_releases, []) - assert {:error, message} = make_releases(dir, handler) - assert message =~ "releases/RELEASES" + assert {:error, message} = Commands.make_releases(rel_dir(dir), handler) + assert message =~ Path.join([dir, "releases", "RELEASES"]) assert message =~ "no release is running as permanent" end @@ -42,7 +54,7 @@ defmodule Castle.MakeReleasesTest do {~c"sample", ~c"0.2.0", [], :permanent} ]) - assert {:error, message} = make_releases(dir, handler) + assert {:error, message} = Commands.make_releases(rel_dir(dir), handler) assert message =~ "expected one permanent release, found 0.1.0, 0.2.0" end @@ -50,13 +62,16 @@ defmodule Castle.MakeReleasesTest do handler = Stub.stub(:which_releases, [{~c"sample", ~c"0.1.0", [], :permanent}]) Stub.stub(:create_RELEASES, {:error, :eacces}) - assert {:error, message} = make_releases(dir, handler) - assert message =~ "Cannot create releases/RELEASES from releases/0.1.0/sample.rel." + assert {:error, message} = Commands.make_releases(rel_dir(dir), handler) + assert message =~ "Cannot create #{Path.join(dir, "releases/RELEASES")}" + assert message =~ "from #{Path.join(dir, "releases/0.1.0/sample.rel")}." assert message =~ "eacces" end end - defp make_releases(dir, handler) do - File.cd!(dir, fn -> Commands.make_releases(handler) end) + defp rel_dir(dir) do + rel_dir = Path.join(dir, "releases") + File.mkdir_p!(rel_dir) + rel_dir end end diff --git a/test/castle/peer_test.exs b/test/castle/peer_test.exs index 929c582..ff5ef7e 100644 --- a/test/castle/peer_test.exs +++ b/test/castle/peer_test.exs @@ -224,7 +224,7 @@ defmodule Castle.PeerTest do assert {:error, message} = Castle.Peer.materialise(vsn_dir) assert message =~ Path.join(vsn_dir, "sys.config") - assert message =~ "neither a sys.config nor a build.config" + assert message =~ "has no configuration to evaluate" end test "reports a version directory with no preboot script", %{tmp_dir: root} do diff --git a/test/castle_test.exs b/test/castle_test.exs index feaec19..d796273 100644 --- a/test/castle_test.exs +++ b/test/castle_test.exs @@ -20,8 +20,14 @@ defmodule CastleTest do end end - test "raises when the configuration of the target version cannot be read" do - assert_raise Castle.Error, ~r/9\.9\.9\/build\.config/, fn -> Castle.generate("9.9.9") end + # The gate rests on a claim about OTP's own data: a release record read from + # a RELEASES file names applications, and only the record release_handler + # synthesises when it cannot read one names none. The release_handler running + # here read the OTP installation's own RELEASES file, so what this checks is + # that claim, against a real record rather than a stub. + test "confirms a system whose record came from a RELEASES file, silently" do + assert [{_, _, [_ | _], _} | _] = :release_handler.which_releases() + assert capture_io(&Castle.upgradable/0) == "" end # The configuration of the target has to exist before the target is handed diff --git a/test/support/config_provider_stub.ex b/test/support/config_provider_stub.ex deleted file mode 100644 index 0f68ed3..0000000 --- a/test/support/config_provider_stub.ex +++ /dev/null @@ -1,27 +0,0 @@ -defmodule Castle.ConfigProviderStub do - @moduledoc false - - # A `Config.Provider` that does something small and observable, so that - # `Castle.Commands.generate/1` can be tested without a real runtime.exs. - # - # Its state - which is what Forecastle stashes in build.config, having called - # init/1 at build time - says what to do: - # - # * `merge:` a keyword list, to merge it into the configuration, and - # * `raise:` a message, to fail the way a provider that cannot find what it - # needs fails. - - @behaviour Config.Provider - - @impl Config.Provider - def init(opts) when is_list(opts), do: opts - - @impl Config.Provider - def load(config, opts) do - if message = opts[:raise] do - raise message - end - - Config.Reader.merge(config, Keyword.get(opts, :merge, [])) - end -end