chore: Get a VM running

lib/hyper/cluster/routing.ex

@@ -28,6 +28,26 @@ defmodule Hyper.Cluster.Routing do
   @spec via(term()) :: {:via, module(), {atom(), term()}}
   def via(key), do: {:via, Horde.Registry, {@name, key}}
 
+  @doc """
+  Register the calling process under `key` from inside its own `init`.
+
+  Prefer this over starting a process with a `{:via, Horde.Registry, _}` name.
+  OTP's post-start name check (`gen:get_proc_name`) calls `whereis_name`
+  immediately after the synchronous `register`, but Horde materialises the name
+  into its local ETS only asynchronously, via the DeltaCRDT diff loop. Under
+  registry churn that read loses the race and OTP aborts startup with
+  `{:process_not_registered_via, Horde.Registry}`. Registering from within
+  `init` carries no such self-check, while leaving the name cluster-resolvable
+  through `via/1` once the diff propagates (callers already tolerate that lag).
+  """
+  @spec register_self(term()) :: :ok | {:error, {:already_registered, pid()}}
+  def register_self(key) do
+    case Horde.Registry.register(@name, key, nil) do
+      {:ok, _pid} -> :ok
+      {:error, {:already_registered, _pid}} = err -> err
+    end
+  end
+
   @doc "Which node currently runs `vm_id`? `nil` if unknown."
   @spec whereis(Hyper.Vm.Id.t()) :: node() | nil
   @decorate with_span("Hyper.Cluster.Routing.whereis", include: [:vm_id])

lib/hyper/node.ex

@@ -161,7 +161,7 @@ defmodule Hyper.Node do
   @spec test_system :: :ok | {:error, term()}
   def test_system do
     with {:ok, _} <- Hyper.Cfg.Budget.load(),
-         :ok <- Hyper.Node.FireVMM.Provider.ensure_installed(),
+         :ok <- check_firecracker_bins(),
          :ok <- Hyper.Node.FireVMM.VmLinux.Provider.ensure_installed(),
          :ok <- Hyper.Node.Vmlinux.test_system(),
          :ok <- Hyper.Img.OciLoader.Umoci.ensure_installed(),
@@ -175,6 +175,24 @@ defmodule Hyper.Node do
     end
   end
 
+  @spec check_firecracker_bins ::
+          :ok
+          | {:error, {:firecracker_bin_missing | :jailer_bin_missing, Path.t()}}
+          | {:error, :firecracker_not_configured | :jailer_not_configured}
+  defp check_firecracker_bins do
+    with {:fc, {:ok, fc}} <- {:fc, Hyper.Cfg.Tools.firecracker_configured()},
+         {:jail, {:ok, jail}} <- {:jail, Hyper.Cfg.Tools.jailer_configured()} do
+      cond do
+        not Sys.Posix.executable?(fc) -> {:error, {:firecracker_bin_missing, fc}}
+        not Sys.Posix.executable?(jail) -> {:error, {:jailer_bin_missing, jail}}
+        true -> :ok
+      end
+    else
+      {:fc, :error} -> {:error, :firecracker_not_configured}
+      {:jail, :error} -> {:error, :jailer_not_configured}
+    end
+  end
+
   @spec check_helper_base(Path.t()) ::
           :ok | {:error, {:suid_helper_base_mismatch, Path.t(), Path.t()}}
   defp check_helper_base(base) do

lib/hyper/node/fire_vmm.ex

@@ -47,14 +47,15 @@ defmodule Hyper.Node.FireVMM do
 
   @spec start_link(Opts.t()) :: Supervisor.on_start()
   def start_link(opts) do
-    Supervisor.start_link(__MODULE__, opts, name: via(opts.vm_id))
+    Supervisor.start_link(__MODULE__, opts)
   end
 
+  @spec child_spec(Opts.t()) :: Supervisor.child_spec()
   def child_spec(opts) do
     # Keyed by VM id and :transient so a cleanly-stopped VM is not rebooted by
     # the node-level DynamicSupervisor.
     %{
-      vm_id: {__MODULE__, opts.vm_id},
+      id: {__MODULE__, opts.vm_id},
       start: {__MODULE__, :start_link, [opts]},
       type: :supervisor,
       restart: :transient
@@ -63,18 +64,26 @@ defmodule Hyper.Node.FireVMM do
 
   @impl true
   def init(opts) do
-    children = [
-      # Client must be registered before Core: Core starts the State machine,
-      # which calls Client.run while waiting for the daemon's API. Client depends
-      # only on vm_id (an independent peer), so it has no reverse dependency.
-      {Client, %Client.Opts{vm_id: opts.vm_id}},
-      {Core, opts}
-    ]
+    # Self-register the cluster routing entry here rather than via a start name;
+    # see `Hyper.Cluster.Routing.register_self/1`. A fresh random vm_id never
+    # collides, so `:already_registered` only happens against a stale dead
+    # incarnation - decline the start and let the supervisor retry clean.
+    case Hyper.Cluster.Routing.register_self({opts.vm_id, :supervisor}) do
+      :ok ->
+        children = [
+          # Client must be registered before Core: Core starts the State machine,
+          # which calls Client.run while waiting for the daemon's API. Client
+          # depends only on vm_id (an independent peer), so no reverse dependency.
+          {Client, %Client.Opts{vm_id: opts.vm_id}},
+          {Core, opts}
+        ]
 
-    Supervisor.init(children, strategy: :one_for_one)
-  end
+        Supervisor.init(children, strategy: :one_for_one)
 
-  defp via(vm_id), do: Hyper.Cluster.Routing.via({vm_id, :supervisor})
+      {:error, _} ->
+        :ignore
+    end
+  end
 
   @doc "Test whether the system can run firecracker VMMs."
   @spec test_system() :: :ok | {:error, term()}

lib/hyper/node/fire_vmm/client.ex

@@ -55,15 +55,12 @@ defmodule Hyper.Node.FireVMM.Client do
     @type t :: %__MODULE__{socket_path: Path.t()}
   end
 
+  # Prod path (vm_id, no explicit name) starts unnamed and self-registers in
+  # `init` - see `Hyper.Cluster.Routing.register_self/1`. A `:name` override
+  # (test stand-ins) is honoured as a plain local name and skips registration.
   @spec start_link(Opts.t()) :: GenServer.on_start()
   def start_link(%Opts{} = opts) do
-    name =
-      case opts.name do
-        nil when not is_nil(opts.vm_id) -> via(opts.vm_id)
-        other -> other
-      end
-
-    GenServer.start_link(__MODULE__, opts, gen_opts(name))
+    GenServer.start_link(__MODULE__, opts, gen_opts(opts.name))
   end
 
   @spec via(Hyper.Vm.Id.t()) :: GenServer.name()
@@ -79,12 +76,26 @@ defmodule Hyper.Node.FireVMM.Client do
   end
 
   @impl true
-  @spec init(Opts.t()) :: {:ok, State.t()}
+  @spec init(Opts.t()) :: {:ok, State.t()} | {:stop, {:already_registered, pid()}}
   def init(%Opts{} = opts) do
-    socket_path = opts.socket_path || Jailer.host_socket(opts.vm_id)
-    {:ok, %State{socket_path: socket_path}}
+    with :ok <- register(opts) do
+      socket_path = opts.socket_path || Jailer.host_socket(opts.vm_id)
+      {:ok, %State{socket_path: socket_path}}
+    end
+  end
+
+  # Register cluster-wide under {vm_id, :client} on the prod path. With an
+  # explicit name (test stand-in), the name is the local registration, so skip.
+  @spec register(Opts.t()) :: :ok | {:stop, {:already_registered, pid()}}
+  defp register(%Opts{name: nil, vm_id: vm_id}) when not is_nil(vm_id) do
+    case Hyper.Cluster.Routing.register_self({vm_id, :client}) do
+      :ok -> :ok
+      {:error, reason} -> {:stop, reason}
+    end
   end
 
+  defp register(%Opts{}), do: :ok
+
   @impl true
   def handle_call({:run, op_fun}, _from, %State{socket_path: socket_path} = state) do
     {:reply, op_fun.(socket_path: socket_path), state}

lib/hyper/node/fire_vmm/core.ex

@@ -18,8 +18,9 @@ defmodule Hyper.Node.FireVMM.Core do
     * firecracker crash -> the `Daemon` child exits; both restart; `Daemon`
       resets the stale jail and relaunches, and the fresh controller cold-boots.
 
-  `MuonTrap` kills the OS process when its port closes (teardown or BEAM death),
-  so no firecracker process outlives the supervisor.
+  `Daemon` guarantees firecracker is dead on teardown via the helper's
+  `cgroup.kill` (MuonTrap's port-close kill misses the setsid'd firecracker), so
+  no firecracker process outlives a graceful supervisor shutdown.
   """
 
   use Supervisor
@@ -28,9 +29,12 @@ defmodule Hyper.Node.FireVMM.Core do
   alias Hyper.Node.FireVMM.Daemon
   alias Hyper.Node.FireVMM.State
 
+  # Started unnamed: nothing resolves the core by name (it is addressed as a
+  # child of `Hyper.Node.FireVMM`), so it needs no registry entry - and avoids a
+  # needless racy Horde registration at startup.
   @spec start_link(FireVMM.Opts.t()) :: Supervisor.on_start()
   def start_link(opts) do
-    Supervisor.start_link(__MODULE__, opts, name: Hyper.Cluster.Routing.via({opts.vm_id, :core}))
+    Supervisor.start_link(__MODULE__, opts)
   end
 
   @impl true

lib/hyper/node/fire_vmm/daemon.ex

@@ -3,26 +3,37 @@ defmodule Hyper.Node.FireVMM.Daemon do
   The jailed firecracker OS process for one microVM, as a static child of
   `Hyper.Node.FireVMM.Core`.
 
-  Lifecycle is supervisor-owned. On every (re)start it first resets any stale
-  jail left by a prior incarnation - the firecracker jailer refuses to reuse an
-  existing chroot - then builds the jailer command and runs it under
-  `MuonTrap.Daemon`, which kills the OS process when its port closes (controller
-  crash, container teardown, or BEAM death). So no firecracker process outlives
-  the supervisor, and `Core`'s `:one_for_all` restarting this child (e.g. after a
-  firecracker crash) cleanly cold-boots against a fresh jail.
-
-  The supervised process *is* the `MuonTrap.Daemon` - `start_link/1` does the
-  reset, then delegates and returns that pid.
+  A `trap_exit` GenServer that owns firecracker's lifetime end to end:
+
+    * on every (re)start it resets any stale jail left by a prior incarnation —
+      the firecracker jailer refuses to reuse an existing chroot — then launches
+      the jailer under a linked `MuonTrap.Daemon`. The supervised process is
+      `hyper-suidhelper jailer ...`, which `execve`s into the jailer (same pid).
+    * if firecracker exits, the linked `MuonTrap.Daemon` exits and this server
+      stops with that reason, so `Core`'s `:one_for_all` cold-boots the pair.
+    * on teardown it **guarantees firecracker is dead**: `MuonTrap`'s port-close
+      kills by process group, but the jailer `setsid`s firecracker into its own
+      session, so it escapes that kill and would leak (holding the cgroup, the
+      rootfs dm device, loop devices). `terminate/2` therefore runs the helper's
+      `cgroup.kill` teardown (`ChrootJail.remove`), which SIGKILLs the whole leaf
+      cgroup regardless of session. The same call on (re)start cleans up after a
+      prior incarnation the BEAM could not (a SIGKILL'd node leaves no
+      `terminate/2`); the periodic `Hyper.Node.Reaper` is the final backstop.
   """
 
+  use GenServer
+  use OpenTelemetryDecorator
+
   alias Hyper.Node.FireVMM.{Jailer, Opts}
   alias Hyper.SuidHelper
   alias Unit.Time
 
-  use OpenTelemetryDecorator
+  require Logger
 
   @shutdown_timeout Time.s(5)
 
+  defstruct [:opts, :muontrap]
+
   @spec child_spec(Opts.t()) :: Supervisor.child_spec()
   def child_spec(%Opts{} = opts) do
     %{
@@ -34,20 +45,82 @@ defmodule Hyper.Node.FireVMM.Daemon do
     }
   end
 
-  @doc """
-  Reset the VM's stale jail, then launch the jailer under `MuonTrap.Daemon` and
-  return its pid. Fails (so the supervisor retries) if the reset cannot run.
-  """
-  @spec start_link(Opts.t()) :: {:ok, pid()} | {:error, term()}
-  @decorate with_span("Hyper.Node.FireVMM.Daemon.start_link", include: [:id])
-  def start_link(%Opts{vm_id: id} = opts) do
-    with :ok <- SuidHelper.ChrootJail.remove(Jailer.chroot_dir(id), Jailer.cgroup_dir(id)) do
-      cmd = Jailer.command(opts)
-
-      case MuonTrap.Daemon.start_link(cmd.binary, cmd.args, []) do
-        {:ok, pid} -> {:ok, pid}
-        {:error, _} = err -> err
-      end
+  @spec start_link(Opts.t()) :: GenServer.on_start()
+  def start_link(%Opts{} = opts) do
+    GenServer.start_link(__MODULE__, opts)
+  end
+
+  @impl true
+  @decorate with_span("Hyper.Node.FireVMM.Daemon.init", include: [:id])
+  def init(%Opts{vm_id: id} = opts) do
+    # Trap exits so the linked MuonTrap's exit reaches `handle_info` (not a silent
+    # link kill) and so `terminate/2` runs on supervisor shutdown.
+    Process.flag(:trap_exit, true)
+
+    with :ok <- reset_stale_jail(id),
+         {:ok, muontrap} <- launch(opts) do
+      {:ok, %__MODULE__{opts: opts, muontrap: muontrap}}
+    else
+      {:error, reason} -> {:stop, reason}
+    end
+  end
+
+  # firecracker (the linked MuonTrap.Daemon) exited: stop with its reason so
+  # `Core`'s `:one_for_all` discards the controller too and cold-boots the pair.
+  @impl true
+  def handle_info({:EXIT, muontrap, reason}, %__MODULE__{muontrap: muontrap} = state) do
+    {:stop, reason, state}
+  end
+
+  def handle_info(_msg, state), do: {:noreply, state}
+
+  # Guarantee firecracker is dead and its jail cleared. MuonTrap cannot kill the
+  # setsid'd firecracker; the helper's `cgroup.kill` can. Best-effort: a failure
+  # here is logged, and the `Reaper` will retry, but it must not crash teardown.
+  @impl true
+  @decorate with_span("Hyper.Node.FireVMM.Daemon.terminate", include: [:id])
+  def terminate(_reason, %__MODULE__{opts: %Opts{vm_id: id}}) do
+    case clear_jail(id) do
+      :ok ->
+        :ok
+
+      {:error, reason} ->
+        Logger.error("vm #{id}: teardown failed to clear jail: #{inspect(reason)}")
+    end
+  end
+
+  @spec reset_stale_jail(Hyper.Vm.Id.t()) :: :ok | {:error, term()}
+  defp reset_stale_jail(id), do: clear_jail(id)
+
+  @spec clear_jail(Hyper.Vm.Id.t()) :: :ok | {:error, term()}
+  defp clear_jail(id) do
+    SuidHelper.ChrootJail.remove(Jailer.chroot_dir(id), Jailer.cgroup_dir(id))
+  end
+
+  @spec launch(Opts.t()) :: {:ok, pid()} | {:error, term()}
+  defp launch(%Opts{vm_id: id} = opts) do
+    cmd = Jailer.command(opts)
+
+    # Surface what the jailed process actually does: `log_output` routes the
+    # helper/jailer/firecracker stdout+stderr (guest serial console included)
+    # to the Logger, and `exit_status_to_reason` turns MuonTrap's opaque
+    # `:error_exit_status` into `{:firecracker_exited, status}` so a crash
+    # report names the real exit code instead of hiding it.
+    daemon_opts = [
+      log_output: :info,
+      log_prefix: "vm #{id} firecracker: ",
+      stderr_to_stdout: true,
+      exit_status_to_reason: &{:firecracker_exited, &1}
+    ]
+
+    case MuonTrap.Daemon.start_link(cmd.binary, cmd.args, daemon_opts) do
+      {:ok, pid} ->
+        Logger.info("vm #{id}: jailer launched under MuonTrap (#{inspect(pid)})")
+        {:ok, pid}
+
+      {:error, reason} = err ->
+        Logger.error("vm #{id}: jailer failed to launch: #{inspect(reason)}")
+        err
     end
   end
 end