Merge pull request #6 from LINUXexpert-org/docker-container-launcher

Run the recovery cycle in a container
2026-08-28 17:16:56 -07:00
parent 5871c5a6b2 0698235d4c
commit f34cd95454
3 changed files with 413 additions and 1 deletions
@@ -0,0 +1,193 @@
+// SPDX-FileCopyrightText: 2026 LINUXexpert-org
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package recovery
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"os/exec"
+	"strconv"
+	"strings"
+	"time"
+)
+
+// DefaultDockerBinary is what shells out unless a caller names another.
+// Named rather than inlined because podman answers the same CLI closely
+// enough to be worth pointing this at one day - see issue #3, where that is
+// explicitly not this pass's work.
+const DefaultDockerBinary = "docker"
+
+// ContainerMount is one mount to give the recovery container. These come
+// from the live container's own `docker inspect`, not from a template: the
+// store has to be the one Stalwart was already using, and anything this
+// tool invented instead would migrate an empty directory and report
+// success.
+type ContainerMount struct {
+	Source      string // volume name or host path
+	Destination string // path inside the container
+	ReadOnly    bool
+}
+
+func (m ContainerMount) arg() string {
+	s := m.Source + ":" + m.Destination
+	if m.ReadOnly {
+		s += ":ro"
+	}
+	return s
+}
+
+// ContainerLauncher runs the target version as a throwaway container
+// against the live data, which is what a container deployment's recovery
+// cycle is. See ARCHITECTURE.md §4.4 and issue #3.
+//
+// The live container must already be stopped. Two Stalwart processes
+// against one store is corruption, and nothing here can detect that the
+// other one is running - `run` stops the service before this phase for
+// exactly that reason.
+type ContainerLauncher struct {
+	// Image is the staged target image, by digest where possible: a tag
+	// can move between staging and running, and then this would migrate
+	// the store with something other than what was verified.
+	Image string
+
+	// Mounts is the live container's own mounts.
+	Mounts []ContainerMount
+
+	// Name for the recovery container. It must not be the live container's
+	// name - that one still exists while stopped, and reusing it would
+	// collide. Empty gets a generated one.
+	Name string
+
+	// Publish maps host to container ports, e.g. "127.0.0.1:8080:8080", so
+	// the health check on this side can reach recovery mode's listener.
+	Publish []string
+
+	// DockerBinary defaults to DefaultDockerBinary.
+	DockerBinary string
+}
+
+func (c ContainerLauncher) docker() string {
+	if c.DockerBinary == "" {
+		return DefaultDockerBinary
+	}
+	return c.DockerBinary
+}
+
+func (c ContainerLauncher) name() string {
+	if c.Name != "" {
+		return c.Name
+	}
+	return fmt.Sprintf("stalwart-migrate-recovery-%d", os.Getpid())
+}
+
+// Launch starts the container in the foreground and returns once the OS
+// has started the client. Foreground rather than detached so the server's
+// own output arrives on this process's pipes: a recovery boot that fails
+// on a bind conflict or a rejected config says so immediately, and a
+// detached container would leave that in `docker logs` for nobody.
+func (c ContainerLauncher) Launch(ctx context.Context, o LaunchOptions) (Supervised, error) {
+	if c.Image == "" {
+		return nil, fmt.Errorf("recovery: no image to launch - stage the target image first")
+	}
+	if len(c.Mounts) == 0 {
+		return nil, fmt.Errorf("recovery: no mounts for the recovery container - it would migrate an empty store and report success")
+	}
+
+	name := c.name()
+	args := []string{"run", "--rm", "--name", name}
+	if o.RecoveryMode {
+		args = append(args,
+			"-e", "STALWART_RECOVERY_MODE=1",
+			"-e", fmt.Sprintf("STALWART_RECOVERY_ADMIN=%s:%s", o.AdminUser, o.AdminPassword),
+		)
+	}
+	for _, e := range o.ExtraEnv {
+		args = append(args, "-e", e)
+	}
+	for _, m := range c.Mounts {
+		args = append(args, "-v", m.arg())
+	}
+	for _, p := range c.Publish {
+		args = append(args, "-p", p)
+	}
+	args = append(args, c.Image)
+	if o.ConfigPath != "" {
+		args = append(args, "--config", o.ConfigPath)
+	}
+
+	proc := &Process{}
+	if err := proc.start(exec.CommandContext(ctx, c.docker(), args...), "container "+name); err != nil {
+		return nil, err
+	}
+	return &containerProcess{proc: proc, name: name, dockerBin: c.docker()}, nil
+}
+
+// containerProcess supervises the `docker run` client and, separately, the
+// container it started.
+//
+// Those are two things, and conflating them is how a migration ends up
+// with two processes on one store. Signals reach the container through an
+// attached client, so the ordinary path works - but Process.Stop escalates
+// to SIGKILL when the grace period expires, and killing the client does
+// not kill the container. It would be left running, holding the store
+// open, while the run moved on to the next phase believing it had stopped.
+// So the container is stopped by name first, and its absence confirmed.
+type containerProcess struct {
+	proc      *Process
+	name      string
+	dockerBin string
+}
+
+func (c *containerProcess) Output() string { return c.proc.Output() }
+
+func (c *containerProcess) Stop(gracePeriod time.Duration) error {
+	if gracePeriod <= 0 {
+		gracePeriod = 10 * time.Second
+	}
+	// docker's own timeout, so it SIGKILLs the container's PID 1 if the
+	// server does not exit - the container's equivalent of Process.Stop's
+	// escalation, done where it actually reaches the process.
+	secs := strconv.Itoa(int(gracePeriod.Seconds()))
+	stopOut, stopErr := exec.Command(c.docker(), "stop", "--time", secs, c.name).CombinedOutput()
+
+	// Reap the client regardless: it owns the output buffer, and its
+	// output is most valuable exactly when the stop went wrong.
+	procErr := c.proc.Stop(gracePeriod)
+
+	if stopErr != nil {
+		// A container that has already exited is not an error - `--rm`
+		// means a clean shutdown removes it before this runs.
+		if gone, checkErr := c.absent(); checkErr == nil && gone {
+			return procErr
+		}
+		return fmt.Errorf("recovery: stop container %s: %w (%s)", c.name, stopErr, strings.TrimSpace(string(stopOut)))
+	}
+	if gone, checkErr := c.absent(); checkErr == nil && !gone {
+		return fmt.Errorf("recovery: container %s is still running after being stopped - it still holds the store open, "+
+			"so nothing should touch that data until it is gone", c.name)
+	}
+	return procErr
+}
+
+func (c *containerProcess) docker() string {
+	if c.dockerBin == "" {
+		return DefaultDockerBinary
+	}
+	return c.dockerBin
+}
+
+// absent reports whether the container is gone or stopped.
+func (c *containerProcess) absent() (bool, error) {
+	out, err := exec.Command(c.docker(), "inspect", "-f", "{{.State.Running}}", c.name).Output()
+	if err != nil {
+		// inspect fails when there is no such container, which with --rm
+		// is the successful outcome.
+		return true, nil
+	}
+	return strings.TrimSpace(string(out)) != "true", nil
+}
+
+var _ Launcher = ContainerLauncher{}
+var _ Supervised = (*containerProcess)(nil)
@@ -0,0 +1,209 @@
+// SPDX-FileCopyrightText: 2026 LINUXexpert-org
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package recovery
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+// fakeDocker installs a `docker` that logs every invocation's arguments and
+// behaves as scripted per subcommand. runBody is what `docker run` does;
+// stateBody is what `docker inspect -f {{.State.Running}}` prints.
+func fakeDocker(t *testing.T, runBody, stopExit, stateBody string) (log string) {
+	t.Helper()
+	dir := t.TempDir()
+	log = filepath.Join(dir, "docker-args.log")
+	script := fmt.Sprintf(`#!/bin/sh
+echo "$@" >> %q
+case "$1" in
+  run) %s ;;
+  stop) exit %s ;;
+  inspect) %s ;;
+esac
+`, log, runBody, stopExit, stateBody)
+	if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
+	return log
+}
+
+// waitForArgs polls until the fake has recorded an invocation. Launch
+// returns once the OS has started the client, which is before the shell it
+// started has run anything.
+func waitForArgs(t *testing.T, log string) string {
+	t.Helper()
+	deadline := time.Now().Add(5 * time.Second)
+	for time.Now().Before(deadline) {
+		if got := readArgsFile(t, log); got != "" {
+			return got
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	t.Fatalf("fake docker was never invoked (log %s stayed empty)", log)
+	return ""
+}
+
+// waitForOutput polls until the supervised process has produced some, for
+// the same reason waitForArgs exists: Launch returns before the thing it
+// started has said anything, and stopping it first would race the output
+// away.
+func waitForOutput(t *testing.T, sup Supervised) string {
+	t.Helper()
+	deadline := time.Now().Add(5 * time.Second)
+	for time.Now().Before(deadline) {
+		if got := sup.Output(); strings.TrimSpace(got) != "" {
+			return got
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	return sup.Output()
+}
+
+func mounts() []ContainerMount {
+	return []ContainerMount{{Source: "stalwart-data", Destination: "/opt/stalwart"}}
+}
+
+func TestContainerLauncherBuildsTheRunCommand(t *testing.T) {
+	log := fakeDocker(t, "exec sleep 300", "0", "echo false")
+
+	sup, err := ContainerLauncher{
+		Image: "stalwartlabs/stalwart@sha256:abc", Mounts: mounts(),
+		Name: "recov", Publish: []string{"127.0.0.1:8080:8080"},
+	}.Launch(context.Background(), LaunchOptions{
+		ConfigPath: "/opt/stalwart/etc/config.json", RecoveryMode: true,
+		AdminUser: "admin", AdminPassword: "s3cret", ExtraEnv: []string{"FOO=bar"},
+	})
+	if err != nil {
+		t.Fatalf("Launch: %v", err)
+	}
+	defer sup.Stop(time.Second)
+
+	args := waitForArgs(t, log)
+	for _, want := range []string{
+		"run --rm --name recov",
+		"-e STALWART_RECOVERY_MODE=1",
+		"-e STALWART_RECOVERY_ADMIN=admin:s3cret",
+		"-e FOO=bar",
+		"-v stalwart-data:/opt/stalwart",
+		"-p 127.0.0.1:8080:8080",
+		"stalwartlabs/stalwart@sha256:abc --config /opt/stalwart/etc/config.json",
+	} {
+		if !strings.Contains(args, want) {
+			t.Errorf("docker run args missing %q\ngot: %s", want, args)
+		}
+	}
+}
+
+// An ordinary boot has no recovery variables. Leaving them set is the
+// documented footgun cutover strips from a unit; the container path must
+// not reintroduce it.
+func TestContainerLauncherOmitsRecoveryEnvOnAnOrdinaryBoot(t *testing.T) {
+	log := fakeDocker(t, "exec sleep 300", "0", "echo false")
+	sup, err := ContainerLauncher{Image: "img", Mounts: mounts(), Name: "recov"}.
+		Launch(context.Background(), LaunchOptions{RecoveryMode: false})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer sup.Stop(time.Second)
+
+	if args := waitForArgs(t, log); strings.Contains(args, "STALWART_RECOVERY") {
+		t.Errorf("ordinary boot set recovery env:\n%s", args)
+	}
+}
+
+// Migrating an empty store and reporting success is the worst outcome
+// available here, so no mounts is refused rather than defaulted.
+func TestContainerLauncherRefusesWithoutMounts(t *testing.T) {
+	fakeDocker(t, "exec sleep 300", "0", "echo false")
+	_, err := ContainerLauncher{Image: "img", Name: "recov"}.
+		Launch(context.Background(), LaunchOptions{})
+	if err == nil {
+		t.Fatal("expected a refusal when no mounts were given")
+	}
+	if !strings.Contains(err.Error(), "empty store") {
+		t.Errorf("refusal should say why it matters, got: %v", err)
+	}
+}
+
+func TestContainerLauncherRefusesWithoutAnImage(t *testing.T) {
+	_, err := ContainerLauncher{Mounts: mounts()}.Launch(context.Background(), LaunchOptions{})
+	if err == nil {
+		t.Fatal("expected a refusal when no image was given")
+	}
+}
+
+// Stop must stop the container by name, not just the client. A killed
+// client leaves the container running and holding the store, and the run
+// would move on believing it had stopped.
+func TestStopStopsTheContainerAndNotOnlyTheClient(t *testing.T) {
+	log := fakeDocker(t, "exec sleep 300", "0", "echo false")
+	sup, err := ContainerLauncher{Image: "img", Mounts: mounts(), Name: "recov"}.
+		Launch(context.Background(), LaunchOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := sup.Stop(2 * time.Second); err != nil {
+		t.Fatalf("Stop: %v", err)
+	}
+	args := waitForArgs(t, log)
+	if !strings.Contains(args, "stop --time 2 recov") {
+		t.Errorf("Stop did not stop the container by name:\n%s", args)
+	}
+}
+
+// The hazard this exists for: docker reported the stop fine, but the
+// container is still running. Nothing may touch that store, so Stop has to
+// fail loudly rather than let the run continue.
+func TestStopFailsWhenTheContainerSurvives(t *testing.T) {
+	fakeDocker(t, "exec sleep 300", "0", "echo true")
+	sup, err := ContainerLauncher{Image: "img", Mounts: mounts(), Name: "recov"}.
+		Launch(context.Background(), LaunchOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = sup.Stop(time.Second)
+	if err == nil {
+		t.Fatal("Stop reported success while the container was still running")
+	}
+	if !strings.Contains(err.Error(), "still holds the store open") {
+		t.Errorf("error should say why it matters, got: %v", err)
+	}
+}
+
+// With --rm a clean shutdown removes the container before Stop runs, so
+// `docker stop` failing on a container that is already gone is the normal
+// path, not a failure.
+func TestStopToleratesAnAlreadyGoneContainer(t *testing.T) {
+	fakeDocker(t, "exec sleep 300", "1", "exit 1")
+	sup, err := ContainerLauncher{Image: "img", Mounts: mounts(), Name: "recov"}.
+		Launch(context.Background(), LaunchOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := sup.Stop(time.Second); err != nil {
+		t.Fatalf("Stop should tolerate an already-removed container, got: %v", err)
+	}
+}
+
+// The server's own words are the diagnosis; a container must not lose them.
+func TestContainerOutputIsCaptured(t *testing.T) {
+	fakeDocker(t, "echo 'Failed to bind to [::]:8080: Address already in use'; exit 1", "0", "exit 1")
+	sup, err := ContainerLauncher{Image: "img", Mounts: mounts(), Name: "recov"}.
+		Launch(context.Background(), LaunchOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	out := waitForOutput(t, sup)
+	_ = sup.Stop(time.Second)
+	if !strings.Contains(out, "Address already in use") {
+		t.Errorf("container output was not captured, got: %q", out)
+	}
+}
@@ -114,11 +114,21 @@ func (p *Process) Start(ctx context.Context, o ProcessOptions) error {

 	cmd := exec.CommandContext(ctx, o.BinaryPath, "--config", o.ConfigPath)
 	cmd.Env = append(os.Environ(), env...)
+	return p.start(cmd, o.BinaryPath)
+}
+
+// start attaches the output buffer and launches cmd. Split out of Start so
+// a deployment that runs the target version some other way - a container,
+// where the child is `docker run` rather than the server itself - gets the
+// same supervision: the same captured output, and the same Stop.
+//
+// what names the thing being started, for the error if it will not.
+func (p *Process) start(cmd *exec.Cmd, what string) error {
 	p.output = &outputBuffer{}
 	cmd.Stdout = p.output
 	cmd.Stderr = p.output
 	if err := cmd.Start(); err != nil {
-		return fmt.Errorf("recovery: start %s: %w", o.BinaryPath, err)
+		return fmt.Errorf("recovery: start %s: %w", what, err)
 	}
 	p.cmd = cmd
 	return nil