main() is one call into internal/cli (closes #160)

cmd/netwatch-server/main.go parsed the command line, ran the check-cidr
and prepare-data-dir subcommands and built the fx application itself;
the Go style guide puts all of that in internal/ or pkg/. The new
package internal/cli does it, with the same arguments, messages and
exit codes. main() is now os.Exit(cli.Main(Appname, Version,
os.Args[1:], cli.RunServer)): Main takes the function that runs the
server, so its tests call it with one that fails them. Appname and
Version stay in main, where the build sets Version. Each subcommand
writes its error to a writer it is given, and prepare-data-dir also
takes the volume and the user's name, so its own tests give it a
temporary directory and the user they run as.

Model: opus-5-5

README.md

@@ -199,9 +199,7 @@ stops. Its routes:
   address may send a limited number a minute, and the report files are capped in
   size, the oldest deleted first
 - `GET /.well-known/healthcheck` — answers 200 with `"status":"ok"`, the
-  server's version and its uptime; while reports cannot be stored, as the last
-  write of a report file failed, it answers 503 with `"status":"error"` and the
-  same fields (see [Report storage](#report-storage))
+  server's version and its uptime
 - `GET /metrics` — Prometheus metrics behind basic auth, only when
   `METRICS_USERNAME` and `METRICS_PASSWORD` are set; each client address may
   make a limited number of requests to it a minute
@@ -279,11 +277,10 @@ The reports of a failed write stay in memory, and the next flush, within a
 minute, tries again to write them. The server writes one file at a time, so a
 file holds its reports in the order they arrived, those of a failed write
 included. From the failed write until a write succeeds, `POST /api/v1/reports`
-refuses every report with 500 and keeps nothing of it, and the health check
-answers 503 with `"status":"error"`. The page sends the refused samples again in
-its next report, as long as it still holds them: at the 1-second interval one
-refused report already loses some of those in the history (see
-[Reporting](#reporting)). Reports already answered 200 are lost only if the
+refuses every report with 500 and keeps nothing of it. The page sends the
+refused samples again in its next report, as long as it still holds them: at the
+1-second interval one refused report already loses some of those in the history
+(see [Reporting](#reporting)). Reports already answered 200 are lost only if the
 server stops before a write succeeds; the final flush at stop tries them too,
 and if it fails, the process exits non-zero.
 
@@ -557,16 +554,9 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs:
       stops the container. Empty, the backend sends nothing to Sentry
 - **Health check:** the image's `HEALTHCHECK` requests
   `/.well-known/healthcheck` through nginx every 30 seconds, so it fails unless
-  both nginx and the backend answer. It also fails while reports cannot be
-  stored, as when the volume is full or `DATA_DIR` cannot be written: the health
-  check answers 503 from a failed write of a report file until a write succeeds,
-  and the backend tries the write again every minute. Three failed checks in a
-  row make the container `unhealthy`, and one that passes makes it `healthy`
-  again. upaas reads the container's health once, 60 seconds after a deploy, and
-  fails the deploy unless it is `healthy`. A container that turns `unhealthy`
-  after that is neither restarted nor reported by upaas, whose `unless-stopped`
-  restart policy ignores health; only `docker ps` and `docker inspect` show it.
-  The container also stops when either process exits.
+  both nginx and the backend answer. upaas reads the container's health 60
+  seconds after a deploy and fails the deploy unless it is `healthy`. The
+  container also stops when either process exits.
 
 ## Browser Compatibility
 

TODO.md

@@ -27,22 +27,12 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of
   the `check-cidr` and `prepare-data-dir` subcommands, with the same arguments,
   messages and exit codes, and otherwise the server, with the `fx` wiring.
   `Appname` and `Version` stay in `main`, where the build sets `Version`, and
-  `main` gives `cli.Main` stderr and the function that runs the server, which
-  the tests replace with a buffer and a function that fails them. Unit tests
-  call `cli.Main` with `check-cidr` and a CIDR it takes and one it refuses, and
-  check the exit code, and with `prepare-data-dir` and a path outside `/data`,
-  and check that it exits 1 with the error from looking up the `netwatch` user,
-  which the test image does not have. Others give each subcommand input it takes
-  and input it refuses, and check the exit code and what it writes to stderr
-- 2026-10-19: while report files cannot be written, the health check answers 503
-  with `"status":"error"` and the same fields, so the image's `HEALTHCHECK`
-  reports the container `unhealthy`
-  ([#142](https://git.eeqj.de/sneak/netwatch/issues/142)). It answered 200
-  `"status":"ok"` whatever happened, so the platform saw a healthy container
-  while every report was refused with 500. It answers 200 at start, before any
-  write has failed, and again from the first write that succeeds. A unit test
-  removes `DATA_DIR` to make a write fail and checks the 503, then puts it back
-  and checks the 200 after the next write
+  `main` gives `cli.Main` the function that runs the server, which the tests
+  replace with one that fails them. Unit tests call `cli.Main` with `check-cidr`
+  and a CIDR it takes and one it refuses, and with `prepare-data-dir` and a path
+  outside `/data`, and check the exit code. Others give each subcommand input it
+  takes and input it refuses, and check the exit code and what it writes to
+  stderr
 - 2026-10-19: report file writes no longer overlap
   ([#150](https://git.eeqj.de/sneak/netwatch/issues/150)), so the reports of
   failed writes are written in the order they arrived. Two writes that failed at

cmd/netwatch-server/main.go

@@ -15,5 +15,5 @@ var (
 )
 
 func main() {
-	os.Exit(cli.Main(os.Stderr, Appname, Version, os.Args[1:], cli.RunServer))
+	os.Exit(cli.Main(Appname, Version, os.Args[1:], cli.RunServer))
 }

internal/cli/cli.go

@@ -7,6 +7,7 @@ package cli
 import (
 	"fmt"
 	"io"
+	"os"
 	"os/user"
 
 	"sneak.berlin/go/netwatch/internal/config"
@@ -23,17 +24,15 @@ import (
 )
 
 // Main runs netwatch-server with args, its command line after the
-// program's name, and returns the exit code; the subcommands write
-// their errors to stderr. Unless args are "check-cidr CIDR" or
-// "prepare-data-dir DATA_DIR", Main runs the server by calling serve
-// with appname and version, the name and version the server logs and
-// reports. main passes os.Stderr and RunServer; the tests pass a
-// buffer and a function that fails them, so that no test starts the
+// program's name, and returns the exit code. Unless args are
+// "check-cidr CIDR" or "prepare-data-dir DATA_DIR", Main runs the
+// server by calling serve with appname and version, the name and
+// version the server logs and reports. main passes RunServer; the
+// tests pass a function that fails them, so that no test starts the
 // server. When serve is RunServer and the server fails to start or to
 // stop, fx's Run exits the process with status 1 and Main never
 // returns.
 func Main(
-	stderr io.Writer,
 	appname, version string,
 	args []string,
 	serve func(appname, version string),
@@ -42,7 +41,7 @@ func Main(
 	// this server would refuse CIDR in its TRUSTED_PROXIES.
 	// bin/entrypoint.sh runs it on each entry it gives nginx.
 	if len(args) == 2 && args[0] == "check-cidr" {
-		return checkCIDR(stderr, args[1])
+		return checkCIDR(os.Stderr, args[1])
 	}
 
 	// "netwatch-server prepare-data-dir DATA_DIR" gets DATA_DIR ready
@@ -50,7 +49,7 @@ func Main(
 	// reportbuf.PrepareDataDir. bin/entrypoint.sh runs it as root
 	// before it starts this server as that user.
 	if len(args) == 2 && args[0] == "prepare-data-dir" {
-		return prepareDataDir(stderr, "/data", args[1], "netwatch")
+		return prepareDataDir(os.Stderr, "/data", args[1], "netwatch")
 	}
 
 	serve(appname, version)

internal/cli/cli_test.go

@@ -2,7 +2,6 @@ package cli_test
 
 import (
 	"bytes"
-	"io"
 	"os"
 	"os/user"
 	"path/filepath"
@@ -21,31 +20,25 @@ func TestMainRunsCheckCIDR(t *testing.T) {
 	t.Parallel()
 
 	for cidr, want := range map[string]int{"10.0.0.0/8": 0, "1.2.3/32": 1} {
-		code := cli.Main(io.Discard, "netwatch", "test",
-			[]string{"check-cidr", cidr}, failIfServerStarts(t))
+		code := cli.Main("netwatch", "test", []string{"check-cidr", cidr},
+			failIfServerStarts(t))
 		if code != want {
 			t.Errorf("check-cidr %q: exit %d, want %d", cidr, code, want)
 		}
 	}
 }
 
-// TestMainRunsPrepareDataDir: Main given prepare-data-dir runs the
-// prepare-data-dir subcommand for the netwatch user, and does not start
-// the server. The test image has no netwatch user, so that subcommand
-// exits 1 with the error from looking the user up;
-// TestPrepareDataDirRefusesPathOutsideVolume checks the path.
+// TestMainRunsPrepareDataDir: Main given prepare-data-dir returns 1 for
+// a DATA_DIR outside /data, and does not start the server. The test
+// image has no netwatch user, so there the 1 comes from looking that
+// user up; TestPrepareDataDirRefusesPathOutsideVolume checks the path.
 func TestMainRunsPrepareDataDir(t *testing.T) {
 	t.Parallel()
 
-	var stderr bytes.Buffer
-
-	code := cli.Main(&stderr, "netwatch", "test",
-		[]string{"prepare-data-dir", "/etc"}, failIfServerStarts(t))
-
-	want := user.UnknownUserError("netwatch").Error() + "\n"
-	if code != 1 || stderr.String() != want {
-		t.Errorf("prepare-data-dir /etc: exit %d, stderr %q; want exit 1 and %q",
-			code, stderr.String(), want)
+	code := cli.Main("netwatch", "test", []string{"prepare-data-dir", "/etc"},
+		failIfServerStarts(t))
+	if code != 1 {
+		t.Errorf("prepare-data-dir /etc: exit %d, want 1", code)
 	}
 }
 

internal/config/config_test.go

@@ -13,8 +13,8 @@ import (
 	"go.uber.org/fx"
 )
 
-// requireConfigError builds the config as cli.RunServer does and fails
-// the test unless that fails with an error naming setting. It uses
+// requireConfigError builds the config as main does and fails the
+// test unless that fails with an error naming setting. It uses
 // fx.New, because fxtest.New fails the test itself on an error.
 func requireConfigError(t *testing.T, setting string) {
 	t.Helper()
@@ -31,8 +31,8 @@ func requireConfigError(t *testing.T, setting string) {
 	}
 }
 
-// requireTrustedProxies builds the config as cli.RunServer does and
-// fails the test unless its TrustedProxies are want.
+// requireTrustedProxies builds the config as main does and fails the
+// test unless its TrustedProxies are want.
 func requireTrustedProxies(t *testing.T, want []string) {
 	t.Helper()
 

internal/handlers/healthcheck.go

@@ -3,17 +3,9 @@ package handlers
 import "net/http"
 
 // HandleHealthCheck returns a handler for the health check
-// endpoint. It answers 200 while the status is "ok" and 503 otherwise,
-// so that the image's HEALTHCHECK fails.
+// endpoint.
 func (s *Handlers) HandleHealthCheck() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		resp := s.hc.Healthcheck()
-
-		code := http.StatusOK
-		if resp.Status != "ok" {
-			code = http.StatusServiceUnavailable
-		}
-
-		s.respondJSON(w, r, resp, code)
+		s.respondJSON(w, r, s.hc.Healthcheck(), http.StatusOK)
 	}
 }

internal/handlers/healthcheck_test.go

@@ -9,19 +9,17 @@ import (
 	"testing"
 	"time"
 
-	"sneak.berlin/go/netwatch/internal/config"
 	"sneak.berlin/go/netwatch/internal/globals"
 	"sneak.berlin/go/netwatch/internal/handlers"
 	"sneak.berlin/go/netwatch/internal/healthcheck"
 	"sneak.berlin/go/netwatch/internal/logger"
-	"sneak.berlin/go/netwatch/internal/reportbuf"
 
 	"go.uber.org/fx/fxtest"
 )
 
 // newStartedHandlers builds Handlers with a real health check for the
-// server named in g, and a report buffer in a temporary directory, and
-// starts them, which records the time the uptime counts from.
+// server named in g, and starts them, which records the time the
+// uptime counts from.
 func newStartedHandlers(t *testing.T, g *globals.Globals) *handlers.Handlers {
 	t.Helper()
 
@@ -32,23 +30,14 @@ func newStartedHandlers(t *testing.T, g *globals.Globals) *handlers.Handlers {
 		t.Fatalf("logger: %v", err)
 	}
 
-	buf, err := reportbuf.New(lc, reportbuf.Params{
-		Config: &config.Config{DataDir: t.TempDir(), DataDirMaxBytes: 1 << 20},
-		Logger: log,
-	})
-	if err != nil {
-		t.Fatalf("report buffer: %v", err)
-	}
-
 	hc, err := healthcheck.New(lc,
-		healthcheck.Params{Buffer: buf, Globals: g, Logger: log})
+		healthcheck.Params{Globals: g, Logger: log})
 	if err != nil {
 		t.Fatalf("health check: %v", err)
 	}
 
-	h, err := handlers.New(lc, handlers.Params{
-		Buffer: buf, Globals: g, Healthcheck: hc, Logger: log,
-	})
+	h, err := handlers.New(lc,
+		handlers.Params{Globals: g, Healthcheck: hc, Logger: log})
 	if err != nil {
 		t.Fatalf("handlers: %v", err)
 	}
@@ -59,12 +48,10 @@ func newStartedHandlers(t *testing.T, g *globals.Globals) *handlers.Handlers {
 	return h
 }
 
-// TestHandleHealthCheck checks the health check's answer at start, when
-// no write of a report file has failed: 200, a JSON content type, and a
-// JSON object with exactly the fields of healthcheck.HealthcheckResponse,
-// carrying "status":"ok", this server's name and version and an uptime
-// counted from its start. The answer while writes fail is checked in
-// internal/reportbuf, whose tests can make a write fail.
+// TestHandleHealthCheck checks the health check's answer: 200, a JSON
+// content type, and a JSON object with exactly the fields of
+// healthcheck.HealthcheckResponse, carrying this server's name and
+// version and an uptime counted from its start.
 func TestHandleHealthCheck(t *testing.T) {
 	t.Parallel()
 

internal/healthcheck/healthcheck.go

@@ -10,7 +10,6 @@ import (
 	"sneak.berlin/go/netwatch/internal/config"
 	"sneak.berlin/go/netwatch/internal/globals"
 	"sneak.berlin/go/netwatch/internal/logger"
-	"sneak.berlin/go/netwatch/internal/reportbuf"
 
 	"go.uber.org/fx"
 )
@@ -19,7 +18,6 @@ import (
 type Params struct {
 	fx.In
 
-	Buffer  *reportbuf.Buffer
 	Config  *config.Config
 	Globals *globals.Globals
 	Logger  *logger.Logger
@@ -70,18 +68,12 @@ func New(
 	return s, nil
 }
 
-// Healthcheck returns the current health status of the application:
-// "ok", or "error" while report files cannot be written.
+// Healthcheck returns the current health status of the application.
 func (s *Healthcheck) Healthcheck() *HealthcheckResponse {
-	status := "ok"
-	if s.params.Buffer.LastWriteFailed() {
-		status = "error"
-	}
-
 	return &HealthcheckResponse{
 		Appname:       s.params.Globals.Appname,
 		Now:           time.Now().UTC().Format(time.RFC3339Nano),
-		Status:        status,
+		Status:        "ok",
 		UptimeHuman:   s.uptime().String(),
 		UptimeSeconds: int64(s.uptime().Seconds()),
 		Version:       s.params.Globals.Version,

internal/reportbuf/reportbuf.go

@@ -255,16 +255,6 @@ func (b *Buffer) Append(v any) error {
 	return nil
 }
 
-// LastWriteFailed reports whether the last write of a report file to
-// finish failed: false at start until a write fails, and false again
-// once a write succeeds. While it is true, Append refuses reports.
-func (b *Buffer) LastWriteFailed() bool {
-	b.mu.Lock()
-	defer b.mu.Unlock()
-
-	return b.writeErr != nil
-}
-
 // deleteOldestFiles deletes report files, oldest first, until n more
 // bytes fit under maxBytes. It deletes none when the reports waiting
 // to be written leave no room for n even with every file gone, since

internal/reportbuf/reportbuf_test.go

@@ -334,9 +334,9 @@ func stopWhileHeld(
 	return <-stopped
 }
 
-// startApp starts a Buffer through fx, as cli.RunServer does, with the
-// DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set, and returns
-// it with the app, for tests that stop the app themselves.
+// startApp starts a Buffer through fx, as main does, with the DATA_DIR
+// and DATA_DIR_MAX_BYTES the calling test has set, and returns it with
+// the app, for tests that stop the app themselves.
 func startApp(t *testing.T) (*reportbuf.Buffer, *fxtest.App) {
 	t.Helper()
 
@@ -358,8 +358,8 @@ func startApp(t *testing.T) (*reportbuf.Buffer, *fxtest.App) {
 	return buf, app
 }
 
-// startBuffer starts a Buffer through fx, as cli.RunServer does, with
-// the DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set.
+// startBuffer starts a Buffer through fx, as main does, with the
+// DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set.
 func startBuffer(t *testing.T) *reportbuf.Buffer {
 	t.Helper()
 
@@ -1074,54 +1074,9 @@ func TestReportsRefusedWhileWritesFail(t *testing.T) {
 	}
 }
 
-// TestHealthCheckFailsWhileWritesFail asks the health check handler
-// for its answer around a failed write. It answers 200 with
-// "status":"ok" at start, 503 with "status":"error" from the failed
-// write until a write succeeds, and 200 with "status":"ok" again after.
-func TestHealthCheckFailsWhileWritesFail(t *testing.T) {
-	dir := t.TempDir()
-	t.Setenv("DATA_DIR", dir)
-
-	buf, h := startBufferWithHandlers(t)
-
-	checkHealthCheck(t, h, "at start", http.StatusOK, "ok")
-
-	// A flush writes a file only when a report is waiting.
-	err := buf.Append(map[string]string{"id": "kept"})
-	if err != nil {
-		t.Fatalf("append: %v", err)
-	}
-
-	// With the data directory gone, the write has nowhere to go.
-	err = os.Remove(dir)
-	if err != nil {
-		t.Fatalf("remove data dir: %v", err)
-	}
-
-	err = buf.Flush()
-	if err == nil {
-		t.Fatal("flush succeeded with the data directory gone")
-	}
-
-	checkHealthCheck(t, h, "after the failed write",
-		http.StatusServiceUnavailable, "error")
-
-	err = os.Mkdir(dir, 0o750)
-	if err != nil {
-		t.Fatalf("make data dir: %v", err)
-	}
-
-	err = buf.Flush()
-	if err != nil {
-		t.Fatalf("flush with the data directory back: %v", err)
-	}
-
-	checkHealthCheck(t, h, "after a write succeeded", http.StatusOK, "ok")
-}
-
 // startBufferWithHandlers starts a Buffer and the Handlers that store
-// reports in it through fx, as cli.RunServer does, with the DATA_DIR
-// the calling test has set.
+// reports in it through fx, as main does, with the DATA_DIR the calling
+// test has set.
 func startBufferWithHandlers(
 	t *testing.T,
 ) (*reportbuf.Buffer, *handlers.Handlers) {
@@ -1166,39 +1121,6 @@ func postReport(t *testing.T, h *handlers.Handlers, clientID string) int {
 	return rec.Code
 }
 
-// checkHealthCheck asks the health check handler for its answer, and
-// fails the test, saying when it asked, unless the answer has status
-// code wantCode and a "status" field of wantStatus.
-func checkHealthCheck(
-	t *testing.T,
-	h *handlers.Handlers,
-	when string,
-	wantCode int,
-	wantStatus string,
-) {
-	t.Helper()
-
-	rec := httptest.NewRecorder()
-	req := httptest.NewRequestWithContext(t.Context(),
-		http.MethodGet, "/.well-known/healthcheck", http.NoBody)
-	h.HandleHealthCheck().ServeHTTP(rec, req)
-
-	var body struct {
-		Status string `json:"status"`
-	}
-
-	err := json.Unmarshal(rec.Body.Bytes(), &body)
-	if err != nil {
-		t.Fatalf("health check %s: body not JSON: %v (%q)",
-			when, err, rec.Body.String())
-	}
-
-	if rec.Code != wantCode || body.Status != wantStatus {
-		t.Fatalf("health check %s: %d with status %q, want %d with %q",
-			when, rec.Code, body.Status, wantCode, wantStatus)
-	}
-}
-
 // TestConcurrentAppendsStopAtCap appends from many goroutines at once
 // with room for exactly roomFor reports: exactly that many must be
 // taken, which holds only if Append checks and counts each report

internal/server/routes_test.go

@@ -22,9 +22,9 @@ import (
 	"go.uber.org/fx/fxtest"
 )
 
-// newServer builds a Server from the same constructors as
-// cli.RunServer, configured from the environment. It is never started,
-// so nothing listens.
+// newServer builds a Server from the same constructors as main,
+// configured from the environment. It is never started, so nothing
+// listens.
 func newServer(t *testing.T) *server.Server {
 	t.Helper()