Test shutdown exit codes, Sentry startup failure and the processing wait (closes #86)

These tests fail until the change that follows: runApp and
WaitForProcessing do not exist yet, the server has no shutdowner, and a
Sentry DSN that cannot be used exits the process from a goroutine
instead of failing the server's start hook.

runApp must return the exit code a shutdown request carries, 0 without
one, and 1 when the app fails to start or stop. A listen error must ask
fx to shut down with exit code 1. WaitForProcessing must wait for an
image being processed and report it when its context ends first.

Model: opus-5-5

README.md

@@ -40,9 +40,8 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
 
 - **Port:** pixa listens on container port `8080`.
 - **Volume:** container path `/var/lib/pixa`, where pixa keeps its
-  database and cache. upaas bind-mounts the host path it is given and
+  database and cache. Creating the host directory when it is missing is
-  does not create it, so the host directory must exist before the first
+  upaas's job, tracked in https://git.eeqj.de/sneak/upaas/issues/235.
-  deploy.
 - **Environment variables:**
     - `PIXA_SIGNING_KEY` (required): secret for signed and encrypted URLs
       and login, 32+ characters, for example from
@@ -58,10 +57,6 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
   `healthy`. The probe uses the port from `PORT` (default `8080`), so a
   port changed only in a mounted config file is not seen by it: change
   the port with `PORT`.
-- **First run:** create the host directory, owned by root or by uid
-  `65532` and gid `65532`. The server runs as the container's `pixad`
-  user, which has that uid and gid, and the container gives the
-  directory to `pixad` when it starts.
 
 ## Rationale
 

TODO.md

@@ -29,6 +29,13 @@ P2: security: referer blacklist
 
 # Completed Steps
 
+- 2026-09-29 the container makes `/var/lib/pixa` usable by itself (closes
+  #159): `deploy/docker-entrypoint.sh` creates the directory if it is missing,
+  gives the directory and everything in it to `pixad` when the directory or one
+  of its top-level entries belongs to another user or group, sets its mode to
+  `750`, then runs the server as `pixad`; data left by an earlier run under
+  another uid is taken over this way; "Running under upaas" in `README.md` no
+  longer tells the operator to create or chown the host directory.
 - 2026-09-29 variant content types kept in memory (closes #70):
   `Cache.metaCache` holds the content types of up to 10,000 variants in an LRU
   (`github.com/hashicorp/golang-lru/v2`), filled by `StoreVariant` and by

cmd/pixad/main_internal_test.go

@@ -0,0 +1,80 @@
+package main
+
+import (
+	"errors"
+	"testing"
+
+	"go.uber.org/fx"
+)
+
+// errTestHook is the error returned by the test hooks that fail.
+var errTestHook = errors.New("test hook failed")
+
+// TestRunAppExitCode checks the exit code runApp returns: the one a
+// shutdown request carries, 0 for a request without one (as for SIGINT or
+// SIGTERM), and 1 when the app fails to start or to stop.
+func TestRunAppExitCode(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name string
+		hook func(shutdowner fx.Shutdowner) fx.Hook
+		want int
+	}{
+		{
+			name: "shutdown requested with exit code 1",
+			hook: func(shutdowner fx.Shutdowner) fx.Hook {
+				return fx.StartHook(func() error {
+					return shutdowner.Shutdown(fx.ExitCode(1))
+				})
+			},
+			want: 1,
+		},
+		{
+			name: "shutdown requested without an exit code",
+			hook: func(shutdowner fx.Shutdowner) fx.Hook {
+				return fx.StartHook(func() error {
+					return shutdowner.Shutdown()
+				})
+			},
+			want: 0,
+		},
+		{
+			name: "start fails",
+			hook: func(fx.Shutdowner) fx.Hook {
+				return fx.StartHook(func() error { return errTestHook })
+			},
+			want: 1,
+		},
+		{
+			name: "stop fails",
+			hook: func(shutdowner fx.Shutdowner) fx.Hook {
+				return fx.StartStopHook(
+					func() error { return shutdowner.Shutdown() },
+					func() error { return errTestHook },
+				)
+			},
+			want: 1,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			app := fx.New(
+				fx.NopLogger,
+				fx.Invoke(func(lc fx.Lifecycle, shutdowner fx.Shutdowner) {
+					lc.Append(tc.hook(shutdowner))
+				}),
+			)
+
+			got := runApp(app)
+			t.Logf("runApp() = %d", got)
+
+			if got != tc.want {
+				t.Errorf("runApp() = %d, want %d", got, tc.want)
+			}
+		})
+	}
+}

deploy/docker-entrypoint.sh

@@ -1,14 +1,22 @@
 #!/bin/sh
 # deploy/docker-entrypoint.sh: the Docker image's ENTRYPOINT. It runs as
-# root only to give /var/lib/pixa to pixad: a host directory
+# root only to make /var/lib/pixa usable by pixad: a host directory
-# bind-mounted there keeps its host owner, often root, and pixad could
+# bind-mounted there keeps its host owner, often root, and data from an
-# not write to it. The server itself always runs as pixad.
+# earlier run may belong to another uid. The server itself always runs
+# as pixad.
 set -eu
 
 main() {
-    if [ "$(stat -c %U /var/lib/pixa)" != pixad ]; then
+    mkdir -p /var/lib/pixa
-        chown pixad:pixad /var/lib/pixa
+    # Only the directory and its top-level entries are checked, so a
+    # normal start does not walk the cache. -depth gives each directory
+    # to pixad after its contents, so a start stopped part way leaves
+    # something at the top for the next start to find; -h changes a
+    # symlink itself, never the file it points to.
+    if [ -n "$(find /var/lib/pixa -maxdepth 1 \( ! -user pixad -o ! -group pixad \))" ]; then
+        find /var/lib/pixa -depth -exec chown -h pixad:pixad {} +
     fi
+    chmod 750 /var/lib/pixa
     exec su-exec pixad /usr/local/bin/pixad "$@"
 }
 

internal/imageprocessor/max_concurrent_processing_internal_test.go

@@ -300,3 +300,69 @@ func TestProcessReleasesSlotOnError(t *testing.T) {
 		})
 	}
 }
+
+// TestWaitForProcessing holds a processing slot with a Process call that
+// cannot finish reading its input. WaitForProcessing must report that image
+// when its context ends first, wait for it otherwise, return 0 once it has
+// finished, and give back the slots it took while waiting.
+func TestWaitForProcessing(t *testing.T) {
+	t.Parallel()
+
+	proc := New(Params{MaxConcurrentProcessing: 2})
+
+	gate := make(chan struct{})
+	entered := make(chan struct{}, 1)
+	results := make(chan error, 1)
+
+	openGate := sync.OnceFunc(func() { close(gate) })
+	t.Cleanup(openGate)
+
+	processInBackground(proc, &gatedReader{
+		data: bytes.NewReader(createTestJPEG(t, 10, 10)), gate: gate,
+		entered: entered, counter: &readingCounter{},
+	}, results)
+	waitForEntries(t, entered, 1)
+
+	ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
+	defer cancel()
+
+	stillProcessing := proc.WaitForProcessing(ctx)
+	t.Logf("WaitForProcessing() after its context ended: %d", stillProcessing)
+
+	if stillProcessing != 1 {
+		t.Errorf("WaitForProcessing() after its context ended = %d, want 1",
+			stillProcessing)
+	}
+
+	waited := make(chan int, 1)
+
+	go func() { waited <- proc.WaitForProcessing(t.Context()) }()
+
+	select {
+	case got := <-waited:
+		t.Fatalf("WaitForProcessing() = %d while an image was being processed",
+			got)
+	case <-time.After(100 * time.Millisecond):
+	}
+
+	openGate()
+
+	err := <-results
+	if err != nil {
+		t.Errorf("Process() error = %v, want nil", err)
+	}
+
+	select {
+	case got := <-waited:
+		if got != 0 {
+			t.Errorf("WaitForProcessing() once processing finished = %d, want 0",
+				got)
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("WaitForProcessing() did not return once processing finished")
+	}
+
+	if held := len(proc.processingSemaphore); held != 0 {
+		t.Errorf("%d slots still held after WaitForProcessing() returned", held)
+	}
+}

internal/server/shutdown_internal_test.go

@@ -0,0 +1,96 @@
+package server
+
+import (
+	"log/slog"
+	"net"
+	"testing"
+	"time"
+
+	"go.uber.org/fx"
+	"go.uber.org/fx/fxtest"
+
+	"sneak.berlin/go/pixa/internal/config"
+	"sneak.berlin/go/pixa/internal/globals"
+	"sneak.berlin/go/pixa/internal/logger"
+)
+
+// shutdownRecorder is an fx.Shutdowner that sends the options of each
+// shutdown request on requests.
+type shutdownRecorder struct {
+	requests chan []fx.ShutdownOption
+}
+
+func (r shutdownRecorder) Shutdown(opts ...fx.ShutdownOption) error {
+	r.requests <- opts
+
+	return nil
+}
+
+// TestSentryInitFailureFailsStartup checks that a Sentry DSN that cannot be
+// used makes the server's start hook fail, so fx stops what has already
+// started, instead of the process exiting from a goroutine.
+func TestSentryInitFailureFailsStartup(t *testing.T) {
+	t.Parallel()
+
+	lc := fxtest.NewLifecycle(t)
+
+	log, err := logger.New(lc, logger.Params{Globals: &globals.Globals{}})
+	if err != nil {
+		t.Fatalf("logger.New() error = %v", err)
+	}
+
+	_, err = New(lc, Params{
+		Logger:  log,
+		Globals: &globals.Globals{Appname: "pixad"},
+		Config:  &config.Config{SentryDSN: "not-a-dsn"},
+	})
+	if err != nil {
+		t.Fatalf("New() error = %v", err)
+	}
+
+	err = lc.Start(t.Context())
+	t.Logf("Start() error = %v", err)
+
+	if err == nil {
+		t.Fatal("Start() error = nil, want the Sentry initialization error")
+	}
+}
+
+// TestListenErrorRequestsShutdownWithExitCode1 occupies the server's port
+// and checks that the listen error asks fx to shut down with exit code 1.
+func TestListenErrorRequestsShutdownWithExitCode1(t *testing.T) {
+	t.Parallel()
+
+	busy, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", ":0")
+	if err != nil {
+		t.Fatalf("Listen() error = %v", err)
+	}
+
+	t.Cleanup(func() { _ = busy.Close() })
+
+	addr, ok := busy.Addr().(*net.TCPAddr)
+	if !ok {
+		t.Fatalf("listener address %v is not a TCP address", busy.Addr())
+	}
+
+	requests := make(chan []fx.ShutdownOption, 1)
+	s := &Server{
+		log:        slog.New(slog.DiscardHandler),
+		config:     &config.Config{Port: addr.Port},
+		shutdowner: shutdownRecorder{requests: requests},
+	}
+	s.httpServer = s.newHTTPServer()
+
+	go s.serveUntilShutdown()
+
+	select {
+	case opts := <-requests:
+		t.Logf("shutdown options = %v", opts)
+
+		if len(opts) != 1 || opts[0] != fx.ExitCode(1) {
+			t.Errorf("shutdown options = %v, want [fx.ExitCode(1)]", opts)
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("no shutdown was requested after the listen error")
+	}
+}