You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Make Castle's release-management operations report failures in a machine-detectable way so bin/castle can return a non-zero exit status when unpack/install/commit/remove fail.
This issue intentionally no longer proposes extending Castle.generate/1. The long-term config-materialization architecture is tracked in #13, which aims to remove/reduce the bespoke generation path in favour of the target release's normal Elixir Config.Provider pipeline running in a temporary peer.
Problem
unpack/1, install/1, commit/1 and remove/1 catch release_handler errors, print them, and return normally:
case:release_handler.unpack_release(to_charlist(name))do{:ok,vsn}->IO.puts("Unpacked #{vsn} ok"){:error,reason}->IO.puts("Failed to unpack #{name}. #{inspect(reason)}")end
Because these operations are driven through bin/castle/RPC, a missing tarball, missing relup, unknown version, or invalid release state can appear successful to deployment automation.
Desired shape
Keep the runtime functions testable and make the command boundary responsible for process status. For example:
Part 1 (report operational failures) stands. It is the same defect as #10,
described once from Castle's side and once from Forecastle's adversarial review.
One change; both issues close together.
Part 2 (generate/1 → generate/2 with a caller-chosen path) is superseded by #13 and should not be implemented. Adding a destination parameter invests further
in the boot-time config-generation path that #13 removes. Once target configuration
is materialised in a peer running the target release's code, Castle stops
reproducing Elixir's config-provider reduction loop and Castle.generate/1 goes
away as a public function. The sys.config race described in part 2 disappears with
it: normal starts no longer regenerate that file at all, because they use Elixir's
own config providers.
One correction to part 1's suggested fix: System.halt(1) is wrong here. These
commands run over bin/<release> rpc → :erpc.call on the running release
node, so System.halt would halt the production node rather than the CLI.
Raising is the correct mechanism — Kernel.CLI.rpc_eval/1 catches remotely and
re-raises locally, so the local rpc VM prints the stacktrace and exits non-zero
while the running node is untouched. Note also that returning a plain {:error, reason} is invisible to the shell, since process_command only
special-cases :ok. So: keep an internal function returning :ok | {:error, reason} for testability, and raise at the command boundary.
Two latent bugs belong in the same change: releases/0 raises Enum.EmptyError
(Enum.max on []) when no releases are installed, and install/1 has no clause
for :release_handler.install_release/1's restart-required and {:error, {:illegal_option, _}} shapes, so those become a CaseClauseError.
Summary
Make Castle's release-management operations report failures in a machine-detectable way so
bin/castlecan return a non-zero exit status when unpack/install/commit/remove fail.This issue intentionally no longer proposes extending
Castle.generate/1. The long-term config-materialization architecture is tracked in #13, which aims to remove/reduce the bespoke generation path in favour of the target release's normal ElixirConfig.Providerpipeline running in a temporary peer.Problem
unpack/1,install/1,commit/1andremove/1catchrelease_handlererrors, print them, and return normally:Because these operations are driven through
bin/castle/RPC, a missing tarball, missing relup, unknown version, or invalid release state can appear successful to deployment automation.Desired shape
Keep the runtime functions testable and make the command boundary responsible for process status. For example:
:ok | {:error, reason}(or an equivalent structured result);bin/castlemaps failures to stderr + non-zero exit status;Acceptance criteria
bin/castle unpack ...,install ...,commit ..., andremove ...return non-zero on operational failure.generate/2API or other expansion of the legacy config-generation path is introduced here; config work follows Materialize target release configuration before install using a temporary peer #13.Scope update (1.0.0 planning)
Part 1 (report operational failures) stands. It is the same defect as #10,
described once from Castle's side and once from Forecastle's adversarial review.
One change; both issues close together.
Part 2 (
generate/1→generate/2with a caller-chosen path) is superseded by#13 and should not be implemented. Adding a destination parameter invests further
in the boot-time config-generation path that #13 removes. Once target configuration
is materialised in a peer running the target release's code, Castle stops
reproducing Elixir's config-provider reduction loop and
Castle.generate/1goesaway as a public function. The
sys.configrace described in part 2 disappears withit: normal starts no longer regenerate that file at all, because they use Elixir's
own config providers.
One correction to part 1's suggested fix:
System.halt(1)is wrong here. Thesecommands run over
bin/<release> rpc→:erpc.callon the running releasenode, so
System.haltwould halt the production node rather than the CLI.Raising is the correct mechanism —
Kernel.CLI.rpc_eval/1catches remotely andre-raises locally, so the local rpc VM prints the stacktrace and exits non-zero
while the running node is untouched. Note also that returning a plain
{:error, reason}is invisible to the shell, sinceprocess_commandonlyspecial-cases
:ok. So: keep an internal function returning:ok | {:error, reason}for testability, and raise at the command boundary.Two latent bugs belong in the same change:
releases/0raisesEnum.EmptyError(
Enum.maxon[]) when no releases are installed, andinstall/1has no clausefor
:release_handler.install_release/1's restart-required and{:error, {:illegal_option, _}}shapes, so those become aCaseClauseError.