From 38d35fe0cf27293ed29553cd78160d5d2ac423ab Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:30:40 -0400 Subject: [PATCH 01/21] Amend rustfs spec with verified CLI/asset facts from Task 1 Verified 2026-04-14 against rustfs 1.0.0-alpha.93: - Correct env vars are RUSTFS_ACCESS_KEY / RUSTFS_SECRET_KEY, not RUSTFS_ROOT_USER / RUSTFS_ROOT_PASSWORD. - --console-enable is required to bind port 9001. - Linux assets carry a -gnu / -musl variant suffix; pv uses -gnu. - /releases/latest returns 404 because RustFS marks every release as a prerelease, so FetchLatestVersion for rustfs uses /releases?per_page=1 and parses [0].tag_name. --- .../2026-04-14-rustfs-native-binary-design.md | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/docs/superpowers/specs/2026-04-14-rustfs-native-binary-design.md b/docs/superpowers/specs/2026-04-14-rustfs-native-binary-design.md index 83691e6..3ec32ac 100644 --- a/docs/superpowers/specs/2026-04-14-rustfs-native-binary-design.md +++ b/docs/superpowers/specs/2026-04-14-rustfs-native-binary-design.md @@ -95,9 +95,10 @@ The existing `Service` (Docker) interface is unchanged. - `Name()`: `"s3"` (matches the user-facing service name) - `DisplayName()`: `"S3 Storage (RustFS)"` - `Binary()`: `binaries.Rustfs` -- `Args(dataDir)`: `["server", dataDir, "--address", ":9000", "--console-address", ":9001"]` - - **VERIFY during implementation:** exact flag names by running `./rustfs server --help` on the downloaded binary. -- `Env()`: `["RUSTFS_ROOT_USER=rstfsadmin", "RUSTFS_ROOT_PASSWORD=rstfsadmin"]` +- `Args(dataDir)`: `["server", dataDir, "--address", ":9000", "--console-enable", "--console-address", ":9001"]` + - **Verified 2026-04-14** against `rustfs 1.0.0-alpha.93`. The positional `` is the data-dir. `--console-enable` is required to actually open port 9001; without it, the console UI never binds. +- `Env()`: `["RUSTFS_ACCESS_KEY=rstfsadmin", "RUSTFS_SECRET_KEY=rstfsadmin"]` + - **Verified 2026-04-14**: the `RUSTFS_ROOT_USER` / `RUSTFS_ROOT_PASSWORD` names in earlier drafts are invalid; the real env vars are `RUSTFS_ACCESS_KEY` and `RUSTFS_SECRET_KEY` (per `rustfs server --help`). - `Port()`: `9000`; `ConsolePort()`: `9001` - `WebRoutes()`: `[{s3 → 9001}, {s3-api → 9000}]` (unchanged from current Docker version) - `EnvVars(project)`: identical keys/values to the current Docker S3 so linked projects keep working @@ -105,15 +106,14 @@ The existing `Service` (Docker) interface is unchanged. ### `binaries.Rustfs` (`internal/binaries/rustfs.go`) -- Archive naming: `rustfs-{platform}-latest.zip`, where `{platform}` is: - - `darwin/arm64` → `macos-aarch64` (confirmed by user's curl) - - `darwin/amd64` → `macos-x86_64` (**VERIFY** on releases page) - - `linux/amd64` → `linux-x86_64` (**VERIFY**) - - `linux/arm64` → `linux-aarch64` (**VERIFY**) +- Archive naming: `rustfs-{platform}-latest.zip`, where `{platform}` is (verified 2026-04-14 against alpha.93): + - `darwin/arm64` → `macos-aarch64` + - `darwin/amd64` → `macos-x86_64` + - `linux/amd64` → `linux-x86_64-gnu` (RustFS publishes `-gnu` and `-musl` variants on Linux; pv uses the glibc build) + - `linux/arm64` → `linux-aarch64-gnu` - Download URL pattern: `https://github.com/rustfs/rustfs/releases/download/{version}/rustfs-{platform}-latest.zip` -- Latest version: `https://api.github.com/repos/rustfs/rustfs/releases/latest` -- `NeedsExtract: true` — `.zip` archive, extract to `~/.pv/internal/bin/rustfs` -- **VERIFY during implementation:** whether the `.zip` contains `rustfs` at the root or inside a subdirectory. +- Latest version: `https://api.github.com/repos/rustfs/rustfs/releases?per_page=1` — the `/releases/latest` endpoint returns 404 because RustFS marks every release as a prerelease. `FetchLatestVersion` must special-case rustfs to parse an array response and take `[0].tag_name`. +- `NeedsExtract: true` — `.zip` archive, extract to `~/.pv/internal/bin/rustfs`. The `rustfs` binary sits at the **root** of the zip (verified 2026-04-14). ### `supervisor` package (`internal/supervisor/`) @@ -462,14 +462,13 @@ Added as a new phase in `.github/workflows/e2e.yml`. - RustFS on-disk format compatibility across upgrades. Documented limitation: "RustFS is alpha; major version bumps may require `pv service:destroy s3` + re-add." - Performance / load. -## Verification Items (before implementation starts) +## Verification Items (verified 2026-04-14 against alpha.93) -These were assumptions that need ground-truth verification in the first task of the implementation plan: - -1. Asset names on RustFS releases for `macos-x86_64`, `linux-x86_64`, `linux-aarch64`. Only `macos-aarch64` is user-confirmed. -2. Exact CLI flags for RustFS — does `./rustfs server --help` accept `--address :9000 --console-address :9001`, or is the syntax different? -3. Archive contents — does the `.zip` have `rustfs` at the root or nested in a subdirectory? -4. Whether RustFS exposes a usable health endpoint (e.g. `/minio/health/live`) — if yes, we can upgrade `ReadyCheck` from TCP to HTTP; if no, TCP is sufficient. +1. Asset names on RustFS releases: confirmed. macOS uses `macos-{aarch64,x86_64}`; Linux uses `linux-{aarch64,x86_64}-{gnu,musl}`. pv ships the `-gnu` variant. +2. CLI flags: confirmed. `rustfs server ... [--address :9000] [--console-enable] [--console-address :9001]`. `` is a positional argument, and `--console-enable` is required to bind port 9001. +3. Archive contents: the `rustfs` binary is at the root of the `.zip` (no subdirectory). +4. Health endpoint: not investigated; `ReadyCheck` stays TCP-based per the original decision. +5. Latest-version API: RustFS marks every release as prerelease, so `/releases/latest` 404s. `FetchLatestVersion` for rustfs uses `/releases?per_page=1` and parses `[0].tag_name` instead. ## Deferred (explicit non-goals for this spec, listed for future reference) From 1b2391e8344399b0b8fa540e4b645471a560a51d Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:33:41 -0400 Subject: [PATCH 02/21] Add Rustfs binary descriptor Mirror the Mago/Composer binary-descriptor pattern for RustFS. Download URL + latest-version URL wired through manager.go. Rustfs is not added to Tools() because it is a backing service, not a user-facing CLI tool. LatestVersionURL points at /releases?per_page=1 because RustFS marks every release as a prerelease, making /releases/latest return 404. FetchLatestVersion gains a rustfs branch that parses the array response and picks [0].tag_name. Linux assets carry a -gnu / -musl variant suffix; pv uses the -gnu build for broadest compatibility. --- internal/binaries/manager.go | 4 ++ internal/binaries/rustfs.go | 47 +++++++++++++++++++++ internal/binaries/rustfs_test.go | 65 ++++++++++++++++++++++++++++++ internal/binaries/versions.go | 21 ++++++++++ internal/binaries/versions_test.go | 29 +++++++++++++ 5 files changed, 166 insertions(+) create mode 100644 internal/binaries/rustfs.go create mode 100644 internal/binaries/rustfs_test.go diff --git a/internal/binaries/manager.go b/internal/binaries/manager.go index 1139ff3..89587ca 100644 --- a/internal/binaries/manager.go +++ b/internal/binaries/manager.go @@ -36,6 +36,8 @@ func DownloadURL(b Binary, version string) (string, error) { return magoURL(version) case "composer": return composerURL(), nil + case "rustfs": + return rustfsURL(version) default: return "", fmt.Errorf("unknown binary: %s", b.Name) } @@ -56,6 +58,8 @@ func LatestVersionURL(b Binary) string { switch b.Name { case "mago": return "https://api.github.com/repos/carthage-software/mago/releases/latest" + case "rustfs": + return "https://api.github.com/repos/rustfs/rustfs/releases?per_page=1" default: return "" } diff --git a/internal/binaries/rustfs.go b/internal/binaries/rustfs.go new file mode 100644 index 0000000..7fd67e4 --- /dev/null +++ b/internal/binaries/rustfs.go @@ -0,0 +1,47 @@ +package binaries + +import ( + "fmt" + "runtime" +) + +var Rustfs = Binary{ + Name: "rustfs", + DisplayName: "RustFS", + NeedsExtract: true, +} + +// rustfsPlatformNames maps GOOS/GOARCH to the platform suffix RustFS uses in +// its release asset filenames. Linux releases publish -gnu (glibc) and -musl +// variants; pv ships the -gnu variant for the broadest compatibility with +// typical developer machines. +var rustfsPlatformNames = map[string]map[string]string{ + "darwin": { + "arm64": "macos-aarch64", + "amd64": "macos-x86_64", + }, + "linux": { + "amd64": "linux-x86_64-gnu", + "arm64": "linux-aarch64-gnu", + }, +} + +func rustfsArchiveName() (string, error) { + archMap, ok := rustfsPlatformNames[runtime.GOOS] + if !ok { + return "", fmt.Errorf("unsupported OS for RustFS: %s", runtime.GOOS) + } + platform, ok := archMap[runtime.GOARCH] + if !ok { + return "", fmt.Errorf("unsupported architecture for RustFS: %s/%s", runtime.GOOS, runtime.GOARCH) + } + return fmt.Sprintf("rustfs-%s-latest.zip", platform), nil +} + +func rustfsURL(version string) (string, error) { + archive, err := rustfsArchiveName() + if err != nil { + return "", err + } + return fmt.Sprintf("https://github.com/rustfs/rustfs/releases/download/%s/%s", version, archive), nil +} diff --git a/internal/binaries/rustfs_test.go b/internal/binaries/rustfs_test.go new file mode 100644 index 0000000..4f622ec --- /dev/null +++ b/internal/binaries/rustfs_test.go @@ -0,0 +1,65 @@ +package binaries + +import ( + "runtime" + "strings" + "testing" +) + +func TestRustfsURL_CurrentPlatform(t *testing.T) { + url, err := rustfsURL("1.0.0-alpha.93") + if err != nil { + t.Fatalf("unexpected error for %s/%s: %v", runtime.GOOS, runtime.GOARCH, err) + } + if !strings.HasPrefix(url, "https://github.com/rustfs/rustfs/releases/download/1.0.0-alpha.93/") { + t.Errorf("URL = %q; missing expected prefix", url) + } + if !strings.HasSuffix(url, ".zip") { + t.Errorf("URL = %q; expected .zip suffix", url) + } +} + +func TestRustfsArchiveName_AllPlatforms(t *testing.T) { + tests := []struct { + goos, goarch, want string + }{ + {"darwin", "arm64", "rustfs-macos-aarch64-latest.zip"}, + {"darwin", "amd64", "rustfs-macos-x86_64-latest.zip"}, + {"linux", "amd64", "rustfs-linux-x86_64-gnu-latest.zip"}, + {"linux", "arm64", "rustfs-linux-aarch64-gnu-latest.zip"}, + } + for _, tc := range tests { + archMap, ok := rustfsPlatformNames[tc.goos] + if !ok { + t.Errorf("no entry for GOOS=%s", tc.goos) + continue + } + platform, ok := archMap[tc.goarch] + if !ok { + t.Errorf("no entry for GOARCH=%s on %s", tc.goarch, tc.goos) + continue + } + got := "rustfs-" + platform + "-latest.zip" + if got != tc.want { + t.Errorf("%s/%s: got %q, want %q", tc.goos, tc.goarch, got, tc.want) + } + } +} + +func TestDownloadURL_RustfsCase(t *testing.T) { + url, err := DownloadURL(Rustfs, "1.0.0-alpha.93") + if err != nil { + t.Fatalf("DownloadURL returned error: %v", err) + } + if url == "" { + t.Error("DownloadURL returned empty string") + } +} + +func TestLatestVersionURL_RustfsCase(t *testing.T) { + got := LatestVersionURL(Rustfs) + want := "https://api.github.com/repos/rustfs/rustfs/releases?per_page=1" + if got != want { + t.Errorf("got %q, want %q", got, want) + } +} diff --git a/internal/binaries/versions.go b/internal/binaries/versions.go index 0d619b1..275b9dc 100644 --- a/internal/binaries/versions.go +++ b/internal/binaries/versions.go @@ -65,6 +65,14 @@ func FetchLatestVersion(client *http.Client, b Binary) (string, error) { return "", fmt.Errorf("no version URL for %s", b.Name) } + return fetchLatestVersionFromURL(client, b.Name, url) +} + +// fetchLatestVersionFromURL fetches the latest release tag from the given URL. +// When name is "rustfs", it expects the GitHub releases list endpoint (returns a +// JSON array) and picks the first entry. All other binaries expect the single +// /releases/latest object response. +func fetchLatestVersionFromURL(client *http.Client, name, url string) (string, error) { req, err := http.NewRequest("GET", url, nil) if err != nil { return "", err @@ -86,6 +94,19 @@ func FetchLatestVersion(client *http.Client, b Binary) (string, error) { return "", err } + if name == "rustfs" { + var releases []struct { + TagName string `json:"tag_name"` + } + if err := json.Unmarshal(body, &releases); err != nil { + return "", fmt.Errorf("parse GitHub response: %w", err) + } + if len(releases) == 0 { + return "", fmt.Errorf("no releases found for rustfs") + } + return releases[0].TagName, nil + } + var release struct { TagName string `json:"tag_name"` } diff --git a/internal/binaries/versions_test.go b/internal/binaries/versions_test.go index 1672729..551ab53 100644 --- a/internal/binaries/versions_test.go +++ b/internal/binaries/versions_test.go @@ -114,6 +114,35 @@ func TestFetchLatestVersion_GitHub(t *testing.T) { } } +func TestFetchLatestVersion_Rustfs(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`[{"tag_name":"1.0.0-alpha.93"}]`)) + })) + defer srv.Close() + + version, err := fetchLatestVersionFromURL(srv.Client(), "rustfs", srv.URL) + if err != nil { + t.Fatalf("fetchLatestVersionFromURL() error = %v", err) + } + if version != "1.0.0-alpha.93" { + t.Errorf("fetchLatestVersionFromURL() = %q, want %q", version, "1.0.0-alpha.93") + } +} + +func TestFetchLatestVersion_Rustfs_EmptyArray_Error(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`[]`)) + })) + defer srv.Close() + + _, err := fetchLatestVersionFromURL(srv.Client(), "rustfs", srv.URL) + if err == nil { + t.Error("fetchLatestVersionFromURL() with empty array should return error, got nil") + } +} + // urlRewriteTransport redirects all requests to a test server URL. type urlRewriteTransport struct { base http.RoundTripper From 839b01f1f15d1a5bc922c3411bed8807cae4954f Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:36:58 -0400 Subject: [PATCH 03/21] Add ExtractZip + installRustfs for RustFS binaries ExtractZip mirrors ExtractTarGz for the zip format used by RustFS releases. installRustfs follows the installMago pattern: download the .zip, extract the binary, delete the archive, chmod 0755. Wired into InstallBinary via a new rustfs case. --- internal/binaries/download.go | 42 +++++++++++++++++ internal/binaries/download_zip_test.go | 63 ++++++++++++++++++++++++++ internal/binaries/install.go | 21 ++++++++- 3 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 internal/binaries/download_zip_test.go diff --git a/internal/binaries/download.go b/internal/binaries/download.go index f40f06a..b254956 100644 --- a/internal/binaries/download.go +++ b/internal/binaries/download.go @@ -2,6 +2,7 @@ package binaries import ( "archive/tar" + "archive/zip" "compress/gzip" "crypto/sha256" "encoding/hex" @@ -168,3 +169,44 @@ func ExtractTarGz(archivePath, destPath, binaryName string) error { func MakeExecutable(path string) error { return os.Chmod(path, 0755) } + +// ExtractZip extracts a single binary from a .zip archive at archivePath, +// locating the file by basename and writing it to destPath with 0o755 mode. +// Mirrors the semantics of ExtractTarGz for .zip archives. +func ExtractZip(archivePath, destPath, binaryName string) error { + r, err := zip.OpenReader(archivePath) + if err != nil { + return fmt.Errorf("open zip %s: %w", archivePath, err) + } + defer r.Close() + + if err := os.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { + return err + } + + for _, f := range r.File { + if f.FileInfo().IsDir() { + continue + } + if filepath.Base(f.Name) != binaryName { + continue + } + rc, err := f.Open() + if err != nil { + return fmt.Errorf("open %s in zip: %w", f.Name, err) + } + out, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o755) + if err != nil { + rc.Close() + return fmt.Errorf("create %s: %w", destPath, err) + } + _, copyErr := io.Copy(out, rc) + rc.Close() + out.Close() + if copyErr != nil { + return fmt.Errorf("copy %s: %w", f.Name, copyErr) + } + return nil + } + return fmt.Errorf("binary %q not found in zip %s", binaryName, archivePath) +} diff --git a/internal/binaries/download_zip_test.go b/internal/binaries/download_zip_test.go new file mode 100644 index 0000000..e083171 --- /dev/null +++ b/internal/binaries/download_zip_test.go @@ -0,0 +1,63 @@ +package binaries + +import ( + "archive/zip" + "os" + "path/filepath" + "testing" +) + +func TestExtractZip_FlattensSingleBinary(t *testing.T) { + tmp := t.TempDir() + zipPath := filepath.Join(tmp, "test.zip") + f, err := os.Create(zipPath) + if err != nil { + t.Fatal(err) + } + w := zip.NewWriter(f) + fw, err := w.Create("nested/dir/rustfs") + if err != nil { + t.Fatal(err) + } + if _, err := fw.Write([]byte("#!/bin/sh\necho hi\n")); err != nil { + t.Fatal(err) + } + if err := w.Close(); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + + destPath := filepath.Join(tmp, "out", "rustfs") + if err := os.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { + t.Fatal(err) + } + if err := ExtractZip(zipPath, destPath, "rustfs"); err != nil { + t.Fatal(err) + } + + info, err := os.Stat(destPath) + if err != nil { + t.Fatalf("expected %s to exist: %v", destPath, err) + } + if info.Mode().Perm()&0o100 == 0 { + t.Errorf("expected %s to be executable, got mode %v", destPath, info.Mode()) + } +} + +func TestExtractZip_MissingBinary(t *testing.T) { + tmp := t.TempDir() + zipPath := filepath.Join(tmp, "test.zip") + f, _ := os.Create(zipPath) + w := zip.NewWriter(f) + fw, _ := w.Create("something-else") + _, _ = fw.Write([]byte("x")) + w.Close() + f.Close() + + err := ExtractZip(zipPath, filepath.Join(tmp, "out", "rustfs"), "rustfs") + if err == nil { + t.Fatal("expected error when binary not found in zip") + } +} diff --git a/internal/binaries/install.go b/internal/binaries/install.go index 4abb59c..c986805 100644 --- a/internal/binaries/install.go +++ b/internal/binaries/install.go @@ -27,10 +27,12 @@ func InstallBinaryProgress(client *http.Client, b Binary, version string, progre } switch b.Name { - case "mago": - return installMago(client, url, progress) case "composer": return installComposer(client, url, b, version, progress) + case "mago": + return installMago(client, url, progress) + case "rustfs": + return installRustfs(client, url, progress) default: return fmt.Errorf("unknown binary: %s", b.Name) } @@ -53,6 +55,21 @@ func installMago(client *http.Client, url string, progress ProgressFunc) error { return MakeExecutable(destPath) } +func installRustfs(client *http.Client, url string, progress ProgressFunc) error { + internalBin := config.InternalBinDir() + archivePath := filepath.Join(internalBin, "rustfs.zip") + destPath := filepath.Join(internalBin, "rustfs") + + if err := DownloadProgress(client, url, archivePath, progress); err != nil { + return err + } + if err := ExtractZip(archivePath, destPath, "rustfs"); err != nil { + return err + } + os.Remove(archivePath) + return MakeExecutable(destPath) +} + func installComposer(client *http.Client, url string, b Binary, version string, progress ProgressFunc) error { destPath := config.ComposerPharPath() From c614e9fb087ff1431a3bc24df8cb1163a365e830 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:38:27 -0400 Subject: [PATCH 04/21] Add Kind and Enabled fields to registry.ServiceInstance Kind is "docker" (default) or "binary". Enabled is only used for binary services; nil means enabled=true so existing entries written by older pv versions keep working unchanged. Image field gains omitempty so binary entries do not emit an empty image string. --- internal/registry/registry.go | 8 ++++- internal/registry/registry_test.go | 54 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 567b1a3..5a1c2ae 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -11,9 +11,15 @@ import ( ) type ServiceInstance struct { - Image string `json:"image"` + Image string `json:"image,omitempty"` Port int `json:"port"` ConsolePort int `json:"console_port,omitempty"` + // Kind is "docker" (default) or "binary". Empty/unset is treated as "docker" + // for backwards compatibility with registry files written by earlier pv versions. + Kind string `json:"kind,omitempty"` + // Enabled is only meaningful for Kind == "binary". nil is treated as enabled=true + // (same back-compat reason). A non-nil false means "registered but stopped". + Enabled *bool `json:"enabled,omitempty"` } type ProjectServices struct { diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go index 5af8951..8095756 100644 --- a/internal/registry/registry_test.go +++ b/internal/registry/registry_test.go @@ -1,8 +1,10 @@ package registry import ( + "encoding/json" "os" "path/filepath" + "strings" "testing" "github.com/prvious/pv/internal/config" @@ -680,3 +682,55 @@ func TestServiceSaveLoad_RoundTrip(t *testing.T) { t.Errorf("redis port = %d, want 6379", loaded.Services["redis"].Port) } } + +func TestServiceInstance_JSON_WithKindEnabled(t *testing.T) { + enabled := true + si := ServiceInstance{ + Image: "", + Port: 9000, + ConsolePort: 9001, + Kind: "binary", + Enabled: &enabled, + } + data, err := json.Marshal(si) + if err != nil { + t.Fatal(err) + } + var back ServiceInstance + if err := json.Unmarshal(data, &back); err != nil { + t.Fatal(err) + } + if back.Kind != "binary" { + t.Errorf("Kind round-trip: got %q", back.Kind) + } + if back.Enabled == nil || *back.Enabled != true { + t.Errorf("Enabled round-trip: got %v", back.Enabled) + } +} + +func TestServiceInstance_JSON_OldFormat_DefaultsToDocker(t *testing.T) { + // Entries written by earlier pv versions do not include Kind or Enabled. + blob := []byte(`{"image":"redis:7","port":6379}`) + var si ServiceInstance + if err := json.Unmarshal(blob, &si); err != nil { + t.Fatal(err) + } + if si.Kind != "" { + t.Errorf("old entry should deserialize with empty Kind; got %q", si.Kind) + } + if si.Enabled != nil { + t.Errorf("old entry should deserialize with nil Enabled; got %v", si.Enabled) + } +} + +func TestServiceInstance_JSON_EmptyFields_Omitted(t *testing.T) { + si := ServiceInstance{Image: "redis:7", Port: 6379} + data, _ := json.Marshal(si) + s := string(data) + if strings.Contains(s, "kind") { + t.Errorf("expected kind to be omitted when empty; got %s", s) + } + if strings.Contains(s, "enabled") { + t.Errorf("expected enabled to be omitted when nil; got %s", s) + } +} From 6632b4df9c3cb9b3de4456c7f16db0e7af5c38f3 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:39:48 -0400 Subject: [PATCH 05/21] Add BinaryService interface and binary registry Parallel abstraction to the existing Docker Service interface. Registry is populated by init() functions in per-service files; for now it stays empty. LookupBinary / AllBinary match the existing Service API surface. --- internal/services/binary.go | 68 ++++++++++++++++++++++++++++++++ internal/services/binary_test.go | 29 ++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 internal/services/binary.go create mode 100644 internal/services/binary_test.go diff --git a/internal/services/binary.go b/internal/services/binary.go new file mode 100644 index 0000000..ccd2770 --- /dev/null +++ b/internal/services/binary.go @@ -0,0 +1,68 @@ +package services + +import ( + "time" + + "github.com/prvious/pv/internal/binaries" +) + +// ReadyCheck describes how a supervisor verifies that a binary service has +// finished starting and is ready to accept requests. Exactly one of TCPPort +// or HTTPEndpoint must be set. +type ReadyCheck struct { + TCPPort int // probe 127.0.0.1:port until Dial succeeds + HTTPEndpoint string // GET this URL, expect a 2xx response + Timeout time.Duration // overall give-up time for the ready probe +} + +// BinaryService is the contract for services that run as native binaries +// supervised by the pv daemon, rather than as Docker containers. +type BinaryService interface { + Name() string + DisplayName() string + + // Binary returns the binaries.Binary descriptor that owns download / URL logic. + Binary() binaries.Binary + + // Args returns CLI args passed to the binary at spawn time. + // dataDir is the absolute path to this service's persistent data directory. + Args(dataDir string) []string + + // Env returns process env vars to add on top of os.Environ(). + Env() []string + + // Port is the primary service port exposed on 127.0.0.1. + Port() int + + // ConsolePort is a secondary port (admin UI), or 0 if none. + ConsolePort() int + + // WebRoutes exposes HTTP subdomains (e.g. s3.pv.test -> 9001) to FrankenPHP. + WebRoutes() []WebRoute + + // EnvVars returns the env vars injected into a linked project's .env file. + EnvVars(projectName string) map[string]string + + // ReadyCheck describes how to verify the spawned process is accepting requests. + ReadyCheck() ReadyCheck +} + +// binaryRegistry is populated by init() functions in per-service files +// (e.g. rustfs.go registers itself as "s3"). +var binaryRegistry = map[string]BinaryService{} + +// LookupBinary returns the BinaryService registered under name, or ok=false. +func LookupBinary(name string) (BinaryService, bool) { + svc, ok := binaryRegistry[name] + return svc, ok +} + +// AllBinary returns a snapshot of the binary-service registry. +// Callers must not mutate the returned map. +func AllBinary() map[string]BinaryService { + out := make(map[string]BinaryService, len(binaryRegistry)) + for k, v := range binaryRegistry { + out[k] = v + } + return out +} diff --git a/internal/services/binary_test.go b/internal/services/binary_test.go new file mode 100644 index 0000000..16385e9 --- /dev/null +++ b/internal/services/binary_test.go @@ -0,0 +1,29 @@ +package services + +import "testing" + +func TestLookupBinary_Unknown_ReturnsFalse(t *testing.T) { + _, ok := LookupBinary("does-not-exist") + if ok { + t.Error("expected ok=false for unknown name") + } +} + +func TestLookupBinary_KnownRegistered(t *testing.T) { + // This test is populated by Task 5 when RustFS is registered. + // For now we just assert the function exists and returns the empty-map result. + if binaryRegistry == nil { + t.Fatal("binaryRegistry should not be nil") + } +} + +func TestAllBinary_ReturnsRegistryMap(t *testing.T) { + m := AllBinary() + if m == nil { + t.Error("AllBinary should not return nil") + } + // Identity equality is not guaranteed by the interface, but content equality is. + if len(m) != len(binaryRegistry) { + t.Errorf("AllBinary size %d != binaryRegistry size %d", len(m), len(binaryRegistry)) + } +} From dbd5efbb0b93e734ec3a2ab1a6b5c28e5570fd70 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:43:06 -0400 Subject: [PATCH 06/21] Replace Docker S3 service with RustFS BinaryService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RustFS registers itself as "s3" in the binary registry. The old Docker-backed S3 implementation is removed — there is no longer a docker path for s3. Available() now returns the union of Docker and binary service names so CLI help messages stay correct. Uses verified RUSTFS_ACCESS_KEY / RUSTFS_SECRET_KEY env vars and --console-enable flag per rustfs 1.0.0-alpha.93 verification. --- internal/caddy/caddy.go | 10 ++- internal/services/rustfs.go | 68 +++++++++++++++ internal/services/rustfs_test.go | 137 ++++++++++++++++++++++++++++++ internal/services/s3.go | 76 ----------------- internal/services/s3_test.go | 69 --------------- internal/services/service.go | 15 +++- internal/services/service_test.go | 4 +- 7 files changed, 227 insertions(+), 152 deletions(-) create mode 100644 internal/services/rustfs.go create mode 100644 internal/services/rustfs_test.go delete mode 100644 internal/services/s3.go delete mode 100644 internal/services/s3_test.go diff --git a/internal/caddy/caddy.go b/internal/caddy/caddy.go index 537e895..144095c 100644 --- a/internal/caddy/caddy.go +++ b/internal/caddy/caddy.go @@ -364,12 +364,16 @@ func GenerateServiceSiteConfigs(reg *registry.Registry) error { svcName = key[:idx] } - svc, err := services.Lookup(svcName) - if err != nil { + var routes []services.WebRoute + if dockerSvc, err := services.Lookup(svcName); err == nil { + routes = dockerSvc.WebRoutes() + } else if binSvc, ok := services.LookupBinary(svcName); ok { + routes = binSvc.WebRoutes() + } else { continue } - for _, route := range svc.WebRoutes() { + for _, route := range routes { var buf bytes.Buffer if err := tmpl.Execute(&buf, serviceConsoleData{ Subdomain: route.Subdomain, diff --git a/internal/services/rustfs.go b/internal/services/rustfs.go new file mode 100644 index 0000000..823a3d0 --- /dev/null +++ b/internal/services/rustfs.go @@ -0,0 +1,68 @@ +package services + +import ( + "time" + + "github.com/prvious/pv/internal/binaries" +) + +type RustFS struct{} + +func (r *RustFS) Name() string { return "s3" } +func (r *RustFS) DisplayName() string { return "S3 Storage (RustFS)" } + +func (r *RustFS) Binary() binaries.Binary { return binaries.Rustfs } + +// Args builds the rustfs server invocation. --console-enable is required; +// without it RustFS does not bind the console port. +// Verified against rustfs 1.0.0-alpha.93. +func (r *RustFS) Args(dataDir string) []string { + return []string{ + "server", dataDir, + "--address", ":9000", + "--console-enable", + "--console-address", ":9001", + } +} + +// Env sets the RustFS admin credentials. The correct env var names per +// `rustfs server --help` are ACCESS_KEY / SECRET_KEY (NOT ROOT_USER / +// ROOT_PASSWORD — those are invalid). +func (r *RustFS) Env() []string { + return []string{ + "RUSTFS_ACCESS_KEY=rstfsadmin", + "RUSTFS_SECRET_KEY=rstfsadmin", + } +} + +func (r *RustFS) Port() int { return 9000 } +func (r *RustFS) ConsolePort() int { return 9001 } + +func (r *RustFS) WebRoutes() []WebRoute { + return []WebRoute{ + {Subdomain: "s3", Port: 9001}, + {Subdomain: "s3-api", Port: 9000}, + } +} + +func (r *RustFS) EnvVars(projectName string) map[string]string { + return map[string]string{ + "AWS_ACCESS_KEY_ID": "rstfsadmin", + "AWS_SECRET_ACCESS_KEY": "rstfsadmin", + "AWS_DEFAULT_REGION": "us-east-1", + "AWS_BUCKET": projectName, + "AWS_ENDPOINT": "http://127.0.0.1:9000", + "AWS_USE_PATH_STYLE_ENDPOINT": "true", + } +} + +func (r *RustFS) ReadyCheck() ReadyCheck { + return ReadyCheck{ + TCPPort: 9000, + Timeout: 30 * time.Second, + } +} + +func init() { + binaryRegistry["s3"] = &RustFS{} +} diff --git a/internal/services/rustfs_test.go b/internal/services/rustfs_test.go new file mode 100644 index 0000000..969149d --- /dev/null +++ b/internal/services/rustfs_test.go @@ -0,0 +1,137 @@ +package services + +import ( + "reflect" + "testing" +) + +func TestRustFS_RegisteredAsS3(t *testing.T) { + svc, ok := LookupBinary("s3") + if !ok { + t.Fatal("LookupBinary(\"s3\") returned ok=false") + } + if _, isRustfs := svc.(*RustFS); !isRustfs { + t.Errorf("expected *RustFS, got %T", svc) + } +} + +func TestRustFS_Name(t *testing.T) { + r := &RustFS{} + if r.Name() != "s3" { + t.Errorf("Name() = %q, want s3", r.Name()) + } +} + +func TestRustFS_Ports(t *testing.T) { + r := &RustFS{} + if r.Port() != 9000 { + t.Errorf("Port() = %d, want 9000", r.Port()) + } + if r.ConsolePort() != 9001 { + t.Errorf("ConsolePort() = %d, want 9001", r.ConsolePort()) + } +} + +func TestRustFS_WebRoutes(t *testing.T) { + r := &RustFS{} + want := []WebRoute{ + {Subdomain: "s3", Port: 9001}, + {Subdomain: "s3-api", Port: 9000}, + } + got := r.WebRoutes() + if !reflect.DeepEqual(got, want) { + t.Errorf("WebRoutes() = %#v, want %#v", got, want) + } +} + +func TestRustFS_EnvVars_MatchesDockerKeys(t *testing.T) { + // Linked projects rely on these exact .env keys; the binary migration + // must not silently change them. + r := &RustFS{} + vars := r.EnvVars("myproject") + wantKeys := []string{ + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "AWS_DEFAULT_REGION", + "AWS_BUCKET", + "AWS_ENDPOINT", + "AWS_USE_PATH_STYLE_ENDPOINT", + } + for _, k := range wantKeys { + if _, ok := vars[k]; !ok { + t.Errorf("EnvVars missing key %q", k) + } + } + if vars["AWS_BUCKET"] != "myproject" { + t.Errorf("AWS_BUCKET = %q, want myproject", vars["AWS_BUCKET"]) + } + if vars["AWS_ENDPOINT"] != "http://127.0.0.1:9000" { + t.Errorf("AWS_ENDPOINT = %q, want http://127.0.0.1:9000", vars["AWS_ENDPOINT"]) + } +} + +func TestRustFS_Args_IncludesDataDir(t *testing.T) { + r := &RustFS{} + args := r.Args("/tmp/data") + found := false + for _, a := range args { + if a == "/tmp/data" { + found = true + break + } + } + if !found { + t.Errorf("Args() did not include the data dir; got %v", args) + } +} + +func TestRustFS_Args_IncludesConsoleEnable(t *testing.T) { + // Verified 2026-04-14: port 9001 does not bind unless --console-enable + // is passed explicitly. + r := &RustFS{} + args := r.Args("/tmp/data") + found := false + for _, a := range args { + if a == "--console-enable" { + found = true + break + } + } + if !found { + t.Errorf("Args() missing --console-enable; got %v", args) + } +} + +func TestRustFS_Env_UsesAccessSecretKeys(t *testing.T) { + r := &RustFS{} + env := r.Env() + var sawAccess, sawSecret bool + for _, e := range env { + if e == "RUSTFS_ACCESS_KEY=rstfsadmin" { + sawAccess = true + } + if e == "RUSTFS_SECRET_KEY=rstfsadmin" { + sawSecret = true + } + } + if !sawAccess { + t.Errorf("Env() missing RUSTFS_ACCESS_KEY; got %v", env) + } + if !sawSecret { + t.Errorf("Env() missing RUSTFS_SECRET_KEY; got %v", env) + } +} + +func TestRustFS_ReadyCheck_TCP9000(t *testing.T) { + r := &RustFS{} + rc := r.ReadyCheck() + if rc.TCPPort != 9000 { + t.Errorf("ReadyCheck.TCPPort = %d, want 9000", rc.TCPPort) + } + if rc.HTTPEndpoint != "" { + t.Errorf("ReadyCheck.HTTPEndpoint = %q, want empty (TCP probe)", rc.HTTPEndpoint) + } + if rc.Timeout == 0 { + t.Error("ReadyCheck.Timeout must be non-zero") + } +} diff --git a/internal/services/s3.go b/internal/services/s3.go deleted file mode 100644 index 0df8c92..0000000 --- a/internal/services/s3.go +++ /dev/null @@ -1,76 +0,0 @@ -package services - -import ( - "github.com/prvious/pv/internal/config" - "github.com/prvious/pv/internal/container" -) - -type S3 struct{} - -func (s *S3) Name() string { return "s3" } -func (s *S3) DisplayName() string { return "S3 Storage" } - -func (s *S3) DefaultVersion() string { return "latest" } - -func (s *S3) ImageName(version string) string { - return "rustfs/rustfs:" + version -} - -func (s *S3) ContainerName(version string) string { - return "pv-s3-" + version -} - -func (s *S3) Port(_ string) int { return 9000 } -func (s *S3) ConsolePort(_ string) int { return 9001 } - -func (s *S3) WebRoutes() []WebRoute { - return []WebRoute{ - {Subdomain: "s3", Port: 9001}, - {Subdomain: "s3-api", Port: 9000}, - } -} - -func (s *S3) CreateOpts(version string) container.CreateOpts { - return container.CreateOpts{ - Name: s.ContainerName(version), - Image: s.ImageName(version), - Env: []string{ - "RUSTFS_ROOT_USER=rstfsadmin", - "RUSTFS_ROOT_PASSWORD=rstfsadmin", - }, - Ports: map[int]int{ - 9000: 9000, - 9001: 9001, - }, - Volumes: map[string]string{ - config.ServiceDataDir("s3", version): "/data", - }, - Labels: map[string]string{ - "dev.prvious.pv": "true", - "dev.prvious.pv.service": "s3", - "dev.prvious.pv.version": version, - }, - Cmd: []string{"server", "/data", "--console-address", ":9001"}, - HealthCmd: []string{"CMD-SHELL", "curl -f http://localhost:9000/minio/health/live"}, - HealthInterval: "2s", - HealthTimeout: "5s", - HealthRetries: 15, - } -} - -func (s *S3) EnvVars(projectName string, _ int) map[string]string { - return map[string]string{ - "AWS_ACCESS_KEY_ID": "rstfsadmin", - "AWS_SECRET_ACCESS_KEY": "rstfsadmin", - "AWS_DEFAULT_REGION": "us-east-1", - "AWS_BUCKET": projectName, - "AWS_ENDPOINT": "http://127.0.0.1:9000", - "AWS_USE_PATH_STYLE_ENDPOINT": "true", - } -} - -func (s *S3) CreateDatabase(_ *container.Engine, _, _ string) error { - return nil -} - -func (s *S3) HasDatabases() bool { return false } diff --git a/internal/services/s3_test.go b/internal/services/s3_test.go deleted file mode 100644 index e9adaf7..0000000 --- a/internal/services/s3_test.go +++ /dev/null @@ -1,69 +0,0 @@ -package services - -import "testing" - -func TestS3Ports(t *testing.T) { - s := &S3{} - if got := s.Port("latest"); got != 9000 { - t.Errorf("Port = %d, want 9000", got) - } - if got := s.ConsolePort("latest"); got != 9001 { - t.Errorf("ConsolePort = %d, want 9001", got) - } -} - -func TestS3ImageName(t *testing.T) { - s := &S3{} - if got := s.ImageName("latest"); got != "rustfs/rustfs:latest" { - t.Errorf("ImageName = %q, want %q", got, "rustfs/rustfs:latest") - } -} - -func TestS3EnvVars(t *testing.T) { - s := &S3{} - env := s.EnvVars("my_app", 9000) - if env["AWS_ACCESS_KEY_ID"] != "rstfsadmin" { - t.Errorf("AWS_ACCESS_KEY_ID = %q", env["AWS_ACCESS_KEY_ID"]) - } - if env["AWS_BUCKET"] != "my_app" { - t.Errorf("AWS_BUCKET = %q", env["AWS_BUCKET"]) - } - if env["AWS_ENDPOINT"] != "http://127.0.0.1:9000" { - t.Errorf("AWS_ENDPOINT = %q", env["AWS_ENDPOINT"]) - } - if env["AWS_USE_PATH_STYLE_ENDPOINT"] != "true" { - t.Errorf("AWS_USE_PATH_STYLE_ENDPOINT = %q", env["AWS_USE_PATH_STYLE_ENDPOINT"]) - } -} - -func TestS3WebRoutes(t *testing.T) { - s := &S3{} - routes := s.WebRoutes() - if len(routes) != 2 { - t.Fatalf("WebRoutes len = %d, want 2", len(routes)) - } - if routes[0].Subdomain != "s3" || routes[0].Port != 9001 { - t.Errorf("route[0] = %+v, want {s3 9001}", routes[0]) - } - if routes[1].Subdomain != "s3-api" || routes[1].Port != 9000 { - t.Errorf("route[1] = %+v, want {s3-api 9000}", routes[1]) - } -} - -func TestS3CreateOpts(t *testing.T) { - s := &S3{} - opts := s.CreateOpts("latest") - if len(opts.Cmd) == 0 { - t.Error("expected Cmd to be set for S3") - } - if opts.Ports[9001] != 9001 { - t.Error("expected console port 9001 mapping") - } -} - -func TestS3Name(t *testing.T) { - s := &S3{} - if s.Name() != "s3" { - t.Errorf("Name = %q, want %q", s.Name(), "s3") - } -} diff --git a/internal/services/service.go b/internal/services/service.go index 0d12f3d..8389fce 100644 --- a/internal/services/service.go +++ b/internal/services/service.go @@ -3,6 +3,7 @@ package services import ( "fmt" "regexp" + "sort" "strings" "github.com/prvious/pv/internal/container" @@ -37,19 +38,27 @@ var registry = map[string]Service{ "mysql": &MySQL{}, "postgres": &Postgres{}, "redis": &Redis{}, - "s3": &S3{}, } func Lookup(name string) (Service, error) { svc, ok := registry[name] if !ok { - return nil, fmt.Errorf("unknown service %q (available: mail, mysql, postgres, redis, s3)", name) + return nil, fmt.Errorf("unknown service %q (available: %s)", name, strings.Join(Available(), ", ")) } return svc, nil } +// Available returns the union of Docker and binary service names, sorted. func Available() []string { - return []string{"mail", "mysql", "postgres", "redis", "s3"} + names := make([]string, 0, len(registry)+len(binaryRegistry)) + for n := range registry { + names = append(names, n) + } + for n := range binaryRegistry { + names = append(names, n) + } + sort.Strings(names) + return names } // SanitizeProjectName converts a directory name to a database-safe identifier. diff --git a/internal/services/service_test.go b/internal/services/service_test.go index 7e015fd..1f9d9f8 100644 --- a/internal/services/service_test.go +++ b/internal/services/service_test.go @@ -5,7 +5,8 @@ import ( ) func TestLookup_Valid(t *testing.T) { - for _, name := range []string{"mail", "mysql", "postgres", "redis", "s3"} { + // s3 is now a BinaryService (RustFS), not in the Docker registry. + for _, name := range []string{"mail", "mysql", "postgres", "redis"} { svc, err := Lookup(name) if err != nil { t.Errorf("Lookup(%q) error = %v", name, err) @@ -43,6 +44,7 @@ func TestServiceKey(t *testing.T) { func TestAvailable(t *testing.T) { names := Available() + // 4 Docker services (mail, mysql, postgres, redis) + 1 binary service (s3 via RustFS). if len(names) != 5 { t.Errorf("Available() returned %d services, want 5", len(names)) } From e08cb6e3cb5c09c431ea7b60dda75c9735b5b8fd Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:46:16 -0400 Subject: [PATCH 07/21] Add supervisor package for child-process management Supervisor spawns binaries, waits for ReadyCheck, restarts on crash (with a 5-in-60s budget), and stops cleanly with SIGTERM->SIGKILL. Tests exercise the happy path plus ready-timeout, StopAll, TCP readiness, and log-file writes using /bin/sh as a stand-in binary. --- internal/supervisor/supervisor.go | 300 +++++++++++++++++++++++++ internal/supervisor/supervisor_test.go | 176 +++++++++++++++ 2 files changed, 476 insertions(+) create mode 100644 internal/supervisor/supervisor.go create mode 100644 internal/supervisor/supervisor_test.go diff --git a/internal/supervisor/supervisor.go b/internal/supervisor/supervisor.go new file mode 100644 index 0000000..216137e --- /dev/null +++ b/internal/supervisor/supervisor.go @@ -0,0 +1,300 @@ +// Package supervisor spawns and watches child binary processes for the pv +// daemon. Each Process is launched, watched for liveness, restarted on crash +// (up to a budget), and stopped cleanly on demand. The supervisor runs +// in-process inside the pv daemon — it is not a standalone long-running +// process of its own. +package supervisor + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "sync" + "syscall" + "time" +) + +// Process describes one supervised binary. +type Process struct { + Name string + Binary string // absolute path to executable + Args []string + Env []string // appended to os.Environ() + WorkingDir string + LogFile string // absolute path; stdout+stderr appended here + + // Ready returns nil when the process is serving requests. + Ready func(ctx context.Context) error + ReadyTimeout time.Duration +} + +// managed holds the supervisor's internal state for a single Process. +type managed struct { + proc Process + cmd *exec.Cmd + cancel context.CancelFunc // cancels the watcher goroutine + stopped bool // set true when Stop was called explicitly + restarts []time.Time // rolling window of restart timestamps +} + +// Supervisor manages a set of child processes. +type Supervisor struct { + mu sync.Mutex + processes map[string]*managed +} + +// New constructs an empty supervisor. +func New() *Supervisor { + return &Supervisor{processes: map[string]*managed{}} +} + +// Start spawns p, waits for p.Ready to succeed, and returns. +// The process continues in the background; crashes are restarted +// according to the crash-budget policy. +func (s *Supervisor) Start(ctx context.Context, p Process) error { + if p.Name == "" { + return errors.New("supervisor: Process.Name is required") + } + + // Pre-flight: ensure log file's parent directory exists. + if p.LogFile != "" { + if err := os.MkdirAll(filepath.Dir(p.LogFile), 0o755); err != nil { + return fmt.Errorf("supervisor: create log dir: %w", err) + } + } + + s.mu.Lock() + if _, exists := s.processes[p.Name]; exists { + s.mu.Unlock() + return fmt.Errorf("supervisor: %q is already supervised", p.Name) + } + s.mu.Unlock() + + m, err := s.spawn(p) + if err != nil { + return err + } + + s.mu.Lock() + s.processes[p.Name] = m + s.mu.Unlock() + + // Start the watch goroutine so a crash is observed immediately. + watchCtx, cancel := context.WithCancel(context.Background()) + m.cancel = cancel + go s.watch(watchCtx, p.Name) + + // Ready-wait (blocks Start). + if p.Ready != nil { + if err := s.waitReady(ctx, p); err != nil { + _ = s.Stop(p.Name, 5*time.Second) + return fmt.Errorf("supervisor: %s not ready: %w", p.Name, err) + } + } + return nil +} + +// spawn opens the log file, builds the command, and starts it. +func (s *Supervisor) spawn(p Process) (*managed, error) { + var logFile *os.File + if p.LogFile != "" { + f, err := os.OpenFile(p.LogFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) + if err != nil { + return nil, fmt.Errorf("supervisor: open log: %w", err) + } + logFile = f + } + + cmd := exec.Command(p.Binary, p.Args...) + cmd.Env = append(os.Environ(), p.Env...) + cmd.Dir = p.WorkingDir + if logFile != nil { + cmd.Stdout = logFile + cmd.Stderr = logFile + } + + if err := cmd.Start(); err != nil { + if logFile != nil { + _ = logFile.Close() + } + return nil, fmt.Errorf("supervisor: spawn %s: %w", p.Name, err) + } + return &managed{proc: p, cmd: cmd}, nil +} + +// waitReady polls p.Ready every 250ms until success or timeout. +func (s *Supervisor) waitReady(ctx context.Context, p Process) error { + deadline := time.Now().Add(p.ReadyTimeout) + if p.ReadyTimeout == 0 { + deadline = time.Now().Add(30 * time.Second) + } + for { + if err := p.Ready(ctx); err == nil { + return nil + } + if time.Now().After(deadline) { + return errors.New("ready-check timed out") + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(250 * time.Millisecond): + } + } +} + +// watch blocks on cmd.Wait and handles crash restarts. +func (s *Supervisor) watch(ctx context.Context, name string) { + for { + s.mu.Lock() + m, ok := s.processes[name] + s.mu.Unlock() + if !ok { + return + } + + waitErr := m.cmd.Wait() + + s.mu.Lock() + if m.stopped { + delete(s.processes, name) + s.mu.Unlock() + return + } + + // Crash recovery: enforce the restart budget (5 within 60s). + now := time.Now() + cutoff := now.Add(-60 * time.Second) + filtered := m.restarts[:0] + for _, t := range m.restarts { + if t.After(cutoff) { + filtered = append(filtered, t) + } + } + m.restarts = filtered + if len(m.restarts) >= 5 { + fmt.Fprintf(os.Stderr, "supervisor: %s exceeded restart budget (5/60s); giving up (last error: %v)\n", name, waitErr) + delete(s.processes, name) + s.mu.Unlock() + return + } + m.restarts = append(m.restarts, now) + s.mu.Unlock() + + // Pause briefly and respawn — no ready-wait on recovery. + select { + case <-ctx.Done(): + return + case <-time.After(2 * time.Second): + } + newM, err := s.spawn(m.proc) + if err != nil { + fmt.Fprintf(os.Stderr, "supervisor: %s respawn failed: %v\n", name, err) + s.mu.Lock() + delete(s.processes, name) + s.mu.Unlock() + return + } + s.mu.Lock() + newM.stopped = m.stopped + newM.cancel = m.cancel + newM.restarts = m.restarts + s.processes[name] = newM + s.mu.Unlock() + } +} + +// Stop sends SIGTERM, waits up to timeout, then SIGKILL. +// After Stop returns, IsRunning(name) is false. +func (s *Supervisor) Stop(name string, timeout time.Duration) error { + s.mu.Lock() + m, ok := s.processes[name] + if !ok { + s.mu.Unlock() + return nil + } + m.stopped = true + if m.cancel != nil { + m.cancel() + } + pid := m.cmd.Process.Pid + cmd := m.cmd + s.mu.Unlock() + + if err := cmd.Process.Signal(syscall.SIGTERM); err != nil && !errors.Is(err, os.ErrProcessDone) { + return fmt.Errorf("supervisor: SIGTERM %s (pid %d): %w", name, pid, err) + } + + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + + select { + case <-done: + case <-time.After(timeout): + _ = cmd.Process.Kill() + <-done + } + + s.mu.Lock() + delete(s.processes, name) + s.mu.Unlock() + return nil +} + +// StopAll stops every supervised process in parallel. +// timeout is per-process, not total. +func (s *Supervisor) StopAll(timeout time.Duration) error { + s.mu.Lock() + names := make([]string, 0, len(s.processes)) + for n := range s.processes { + names = append(names, n) + } + s.mu.Unlock() + + var wg sync.WaitGroup + for _, n := range names { + wg.Add(1) + go func(name string) { + defer wg.Done() + _ = s.Stop(name, timeout) + }(n) + } + wg.Wait() + return nil +} + +// IsRunning reports whether name is a supervised process with a live PID. +func (s *Supervisor) IsRunning(name string) bool { + s.mu.Lock() + m, ok := s.processes[name] + s.mu.Unlock() + if !ok { + return false + } + // Signal 0 checks that the process exists without delivering a signal. + return m.cmd.Process.Signal(syscall.Signal(0)) == nil +} + +// Pid returns the current pid of name, or 0 if not supervised. +func (s *Supervisor) Pid(name string) int { + s.mu.Lock() + defer s.mu.Unlock() + if m, ok := s.processes[name]; ok { + return m.cmd.Process.Pid + } + return 0 +} + +// SupervisedNames returns the set of currently-supervised process names. +func (s *Supervisor) SupervisedNames() []string { + s.mu.Lock() + defer s.mu.Unlock() + out := make([]string, 0, len(s.processes)) + for n := range s.processes { + out = append(out, n) + } + return out +} diff --git a/internal/supervisor/supervisor_test.go b/internal/supervisor/supervisor_test.go new file mode 100644 index 0000000..35e6020 --- /dev/null +++ b/internal/supervisor/supervisor_test.go @@ -0,0 +1,176 @@ +package supervisor + +import ( + "context" + "errors" + "io" + "net" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// newTestProcess returns a Process that runs `sh -c ` and writes logs +// to a file inside t.TempDir(). Ready is a no-op (returns nil immediately). +func newTestProcess(t *testing.T, name, shellCmd string) Process { + t.Helper() + logPath := filepath.Join(t.TempDir(), name+".log") + return Process{ + Name: name, + Binary: "/bin/sh", + Args: []string{"-c", shellCmd}, + LogFile: logPath, + Ready: func(ctx context.Context) error { + return nil + }, + ReadyTimeout: 2 * time.Second, + } +} + +func TestSupervisor_StartStop_Sleep(t *testing.T) { + s := New() + p := newTestProcess(t, "sleeper", "sleep 30") + if err := s.Start(context.Background(), p); err != nil { + t.Fatalf("Start: %v", err) + } + if !s.IsRunning("sleeper") { + t.Fatal("expected IsRunning=true after Start") + } + if s.Pid("sleeper") == 0 { + t.Error("Pid should be non-zero after successful Start") + } + if err := s.Stop("sleeper", 2*time.Second); err != nil { + t.Fatalf("Stop: %v", err) + } + if s.IsRunning("sleeper") { + t.Error("expected IsRunning=false after Stop") + } +} + +func TestSupervisor_StopAll(t *testing.T) { + s := New() + for _, name := range []string{"a", "b", "c"} { + if err := s.Start(context.Background(), newTestProcess(t, name, "sleep 30")); err != nil { + t.Fatalf("Start %s: %v", name, err) + } + } + if err := s.StopAll(2 * time.Second); err != nil { + t.Fatalf("StopAll: %v", err) + } + for _, name := range []string{"a", "b", "c"} { + if s.IsRunning(name) { + t.Errorf("%s still running after StopAll", name) + } + } +} + +func TestSupervisor_ReadyTimeout(t *testing.T) { + s := New() + p := newTestProcess(t, "never-ready", "sleep 30") + p.Ready = func(ctx context.Context) error { + return errors.New("not ready") + } + p.ReadyTimeout = 500 * time.Millisecond + + start := time.Now() + err := s.Start(context.Background(), p) + elapsed := time.Since(start) + + if err == nil { + t.Fatal("expected ready-timeout error") + } + if elapsed > 2*time.Second { + t.Errorf("Start took %v; expected close to ReadyTimeout", elapsed) + } + // The process should have been killed after the timeout. + if s.IsRunning("never-ready") { + t.Error("expected process to be stopped after ready timeout") + } +} + +func TestSupervisor_SupervisedNames(t *testing.T) { + s := New() + if len(s.SupervisedNames()) != 0 { + t.Error("expected empty list on fresh Supervisor") + } + if err := s.Start(context.Background(), newTestProcess(t, "x", "sleep 30")); err != nil { + t.Fatalf("Start: %v", err) + } + names := s.SupervisedNames() + if len(names) != 1 || names[0] != "x" { + t.Errorf("SupervisedNames = %v, want [x]", names) + } + if err := s.Stop("x", 2*time.Second); err != nil { + t.Fatalf("Stop: %v", err) + } + if len(s.SupervisedNames()) != 0 { + t.Error("expected empty list after Stop") + } +} + +func TestSupervisor_TCPReadyCheck(t *testing.T) { + // Bind a TCP listener on a random port inside the test and use it as the + // ready-check target to prove the dial-based ready logic works. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + addr := ln.Addr().String() + + s := New() + p := Process{ + Name: "ready-tcp", + Binary: "/bin/sh", + Args: []string{"-c", "sleep 30"}, + LogFile: filepath.Join(t.TempDir(), "ready-tcp.log"), + Ready: func(ctx context.Context) error { + c, err := net.DialTimeout("tcp", addr, 200*time.Millisecond) + if err != nil { + return err + } + c.Close() + return nil + }, + ReadyTimeout: 3 * time.Second, + } + if err := s.Start(context.Background(), p); err != nil { + t.Fatalf("Start: %v", err) + } + defer s.Stop("ready-tcp", 2*time.Second) + if !s.IsRunning("ready-tcp") { + t.Error("expected IsRunning=true after ready check passes") + } +} + +func TestSupervisor_LogFileIsWritten(t *testing.T) { + s := New() + p := newTestProcess(t, "logger", "echo hello-from-supervisor; sleep 30") + if err := s.Start(context.Background(), p); err != nil { + t.Fatalf("Start: %v", err) + } + defer s.Stop("logger", 2*time.Second) + + // Poll briefly for the log file to contain the expected line. + deadline := time.Now().Add(2 * time.Second) + var data []byte + for time.Now().Before(deadline) { + f, err := os.Open(p.LogFile) + if err == nil { + data, _ = io.ReadAll(f) + f.Close() + if len(data) > 0 { + break + } + } + time.Sleep(50 * time.Millisecond) + } + if len(data) == 0 { + t.Fatalf("log file %s was empty after 2s", p.LogFile) + } + if !strings.Contains(string(data), "hello-from-supervisor") { + t.Errorf("log file missing expected output; got %q", string(data)) + } +} From 3c265503f453dcb49eb250f8ab8cfc2e9afe09a0 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:48:42 -0400 Subject: [PATCH 08/21] Add supervisor-process builder and daemon-status.json writer buildSupervisorProcess translates a BinaryService into a supervisor.Process by resolving paths via internal/config and creating data + log directories. writeDaemonStatus captures the supervisor snapshot for CLI readers; ReadDaemonStatus rejects stale files when the recorded PID is dead. --- internal/server/binary_service.go | 150 +++++++++++++++++++++++++ internal/server/binary_service_test.go | 75 +++++++++++++ 2 files changed, 225 insertions(+) create mode 100644 internal/server/binary_service.go create mode 100644 internal/server/binary_service_test.go diff --git a/internal/server/binary_service.go b/internal/server/binary_service.go new file mode 100644 index 0000000..0cfa013 --- /dev/null +++ b/internal/server/binary_service.go @@ -0,0 +1,150 @@ +package server + +import ( + "context" + "encoding/json" + "fmt" + "net" + "net/http" + "os" + "path/filepath" + "syscall" + "time" + + "github.com/prvious/pv/internal/config" + "github.com/prvious/pv/internal/services" + "github.com/prvious/pv/internal/supervisor" +) + +// DaemonStatus is the JSON snapshot written to ~/.pv/daemon-status.json. +type DaemonStatus struct { + PID int `json:"pid"` + StartedAt time.Time `json:"started_at"` + Supervised map[string]SupervisedStatus `json:"supervised"` +} + +// SupervisedStatus holds the runtime state of a single supervised binary. +type SupervisedStatus struct { + PID int `json:"pid"` + Running bool `json:"running"` +} + +// daemonStartedAt is captured when the package is first initialized inside the +// daemon process. It is recorded in every status snapshot. +var daemonStartedAt = time.Now() + +// buildSupervisorProcess translates a BinaryService into a supervisor.Process. +// It resolves all paths via internal/config and creates the data + log directories. +func buildSupervisorProcess(svc services.BinaryService) (supervisor.Process, error) { + binaryName := svc.Binary().Name + binaryPath := filepath.Join(config.InternalBinDir(), binaryName) + + dataDir := config.ServiceDataDir(svc.Name(), "latest") + if err := os.MkdirAll(dataDir, 0o755); err != nil { + return supervisor.Process{}, fmt.Errorf("create data dir %s: %w", dataDir, err) + } + + logFile := filepath.Join(config.PvDir(), "logs", binaryName+".log") + if err := os.MkdirAll(filepath.Dir(logFile), 0o755); err != nil { + return supervisor.Process{}, fmt.Errorf("create log dir: %w", err) + } + + rc := svc.ReadyCheck() + ready := buildReadyFunc(rc) + + return supervisor.Process{ + Name: binaryName, + Binary: binaryPath, + Args: svc.Args(dataDir), + Env: svc.Env(), + LogFile: logFile, + Ready: ready, + ReadyTimeout: rc.Timeout, + }, nil +} + +// buildReadyFunc returns a ReadyFunc appropriate to the ReadyCheck variant. +func buildReadyFunc(rc services.ReadyCheck) func(context.Context) error { + switch { + case rc.HTTPEndpoint != "": + client := &http.Client{Timeout: 2 * time.Second} + url := rc.HTTPEndpoint + return func(ctx context.Context) error { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return err + } + resp, err := client.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + if resp.StatusCode >= 200 && resp.StatusCode < 300 { + return nil + } + return fmt.Errorf("HTTP %s returned %d", url, resp.StatusCode) + } + case rc.TCPPort > 0: + addr := fmt.Sprintf("127.0.0.1:%d", rc.TCPPort) + return func(ctx context.Context) error { + d := net.Dialer{Timeout: 500 * time.Millisecond} + c, err := d.DialContext(ctx, "tcp", addr) + if err != nil { + return err + } + c.Close() + return nil + } + default: + // Degenerate case: no probe specified. Treat as instantly ready. + return func(context.Context) error { return nil } + } +} + +// writeDaemonStatus serializes the current supervisor state to +// ~/.pv/daemon-status.json. Safe to call from the reconcile path. +func writeDaemonStatus(sup *supervisor.Supervisor) error { + snap := DaemonStatus{ + PID: os.Getpid(), + StartedAt: daemonStartedAt, + Supervised: map[string]SupervisedStatus{}, + } + if sup != nil { + for _, name := range sup.SupervisedNames() { + snap.Supervised[name] = SupervisedStatus{ + PID: sup.Pid(name), + Running: sup.IsRunning(name), + } + } + } + data, err := json.MarshalIndent(snap, "", " ") + if err != nil { + return err + } + path := filepath.Join(config.PvDir(), "daemon-status.json") + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return err + } + return os.WriteFile(path, data, 0o644) +} + +// ReadDaemonStatus returns the parsed ~/.pv/daemon-status.json, or nil+error if +// the file is missing or corrupt or if the recorded PID is not alive. +func ReadDaemonStatus() (*DaemonStatus, error) { + path := filepath.Join(config.PvDir(), "daemon-status.json") + data, err := os.ReadFile(path) + if err != nil { + return nil, err + } + var snap DaemonStatus + if err := json.Unmarshal(data, &snap); err != nil { + return nil, err + } + // Liveness check. + if proc, err := os.FindProcess(snap.PID); err == nil { + if err := proc.Signal(syscall.Signal(0)); err != nil { + return nil, fmt.Errorf("daemon-status.json is stale (pid %d not alive)", snap.PID) + } + } + return &snap, nil +} diff --git a/internal/server/binary_service_test.go b/internal/server/binary_service_test.go new file mode 100644 index 0000000..e6a8d28 --- /dev/null +++ b/internal/server/binary_service_test.go @@ -0,0 +1,75 @@ +package server + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/prvious/pv/internal/services" + "github.com/prvious/pv/internal/supervisor" +) + +func TestBuildSupervisorProcess_RustFS(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + svc := &services.RustFS{} + p, err := buildSupervisorProcess(svc) + if err != nil { + t.Fatalf("buildSupervisorProcess: %v", err) + } + if p.Name != "rustfs" { + t.Errorf("Name = %q, want rustfs", p.Name) + } + if !strings.HasSuffix(p.Binary, "/internal/bin/rustfs") { + t.Errorf("Binary = %q; should end with /internal/bin/rustfs", p.Binary) + } + if !strings.Contains(p.LogFile, "logs") || !strings.HasSuffix(p.LogFile, "/rustfs.log") { + t.Errorf("LogFile = %q; expected ~/.pv/logs/rustfs.log", p.LogFile) + } + // Data dir should be created on the fly. + dataDir := "" + for i, a := range p.Args { + if a == "server" && i+1 < len(p.Args) { + dataDir = p.Args[i+1] + break + } + } + if dataDir == "" { + t.Fatal("could not find data dir in Args") + } + if _, err := os.Stat(dataDir); err != nil { + t.Errorf("data dir %s should exist: %v", dataDir, err) + } + if p.Ready == nil { + t.Error("Ready closure must be set") + } + if p.ReadyTimeout == 0 { + t.Error("ReadyTimeout must be non-zero") + } +} + +func TestWriteDaemonStatus_RoundTrip(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + s := supervisor.New() + // No live processes — we just test the file write path. + if err := writeDaemonStatus(s); err != nil { + t.Fatalf("writeDaemonStatus: %v", err) + } + path := filepath.Join(os.Getenv("HOME"), ".pv", "daemon-status.json") + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read daemon-status.json: %v", err) + } + var snap DaemonStatus + if err := json.Unmarshal(data, &snap); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if snap.PID != os.Getpid() { + t.Errorf("PID = %d, want %d", snap.PID, os.Getpid()) + } + if snap.Supervised == nil { + t.Error("Supervised map should be initialized") + } +} From e8509f0014531a36b584bcdcdbee34a6263b06e1 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:53:34 -0400 Subject: [PATCH 09/21] Extend ServerManager.Reconcile with binary-service phase ServerManager now owns a *supervisor.Supervisor alongside the FrankenPHP instances. Reconcile runs the existing FrankenPHP phase and then a new phase that diffs the binary registry against the supervisor state, starting/stopping processes as needed. NewServerManager signature changes to accept the supervisor. Call site in process.go is updated with nil supervisor temporarily; Task 9 wires in a real supervisor. --- internal/server/manager.go | 89 +++++++++++++++++++++++++++++- internal/server/manager_test.go | 98 +++++++++++++++++++++++++++++++++ internal/server/process.go | 4 +- 3 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 internal/server/manager_test.go diff --git a/internal/server/manager.go b/internal/server/manager.go index da71659..40ad642 100644 --- a/internal/server/manager.go +++ b/internal/server/manager.go @@ -1,14 +1,18 @@ package server import ( + "context" "fmt" "os" "strings" "sync" + "time" "github.com/prvious/pv/internal/caddy" "github.com/prvious/pv/internal/config" "github.com/prvious/pv/internal/registry" + "github.com/prvious/pv/internal/services" + "github.com/prvious/pv/internal/supervisor" ) // ServerManager owns the main and secondary FrankenPHP instances. @@ -18,13 +22,16 @@ type ServerManager struct { mu sync.Mutex main *FrankenPHP secondaries map[string]*FrankenPHP // version -> instance + supervisor *supervisor.Supervisor // binary services; may be nil in tests } -// NewServerManager creates a manager with the given main FrankenPHP instance. -func NewServerManager(main *FrankenPHP) *ServerManager { +// NewServerManager creates a manager with the given main FrankenPHP instance +// and an optional supervisor for native binary services. +func NewServerManager(main *FrankenPHP, sup *supervisor.Supervisor) *ServerManager { return &ServerManager{ main: main, secondaries: make(map[string]*FrankenPHP), + supervisor: sup, } } @@ -99,6 +106,17 @@ func (m *ServerManager) Reconcile() error { return fmt.Errorf("reconcile: reload main FrankenPHP: %w", err) } + // Phase 2: binary services. + if err := m.reconcileBinaryServices(context.Background()); err != nil { + fmt.Fprintf(os.Stderr, "Reconcile: %v\n", err) + // non-fatal — FrankenPHP-side reconcile results still returned below + } + + // Phase 3: refresh daemon-status snapshot. + if err := writeDaemonStatus(m.supervisor); err != nil { + fmt.Fprintf(os.Stderr, "Reconcile: write daemon-status: %v\n", err) + } + if len(startErrors) > 0 { return fmt.Errorf("reconcile: %d secondary instance(s) failed: %s", len(startErrors), strings.Join(startErrors, "; ")) } @@ -108,6 +126,10 @@ func (m *ServerManager) Reconcile() error { // Shutdown stops all secondary FrankenPHP instances. // The main instance is stopped separately via its own defer in Start(). func (m *ServerManager) Shutdown() { + if m.supervisor != nil { + m.supervisor.StopAll(10 * time.Second) + } + m.mu.Lock() defer m.mu.Unlock() @@ -129,3 +151,66 @@ func (m *ServerManager) RunningVersions() []string { } return versions } + +// reconcileBinaryServices brings supervisor state in line with the binary +// entries in the registry. Enabled entries are started if not yet running; +// disabled or removed entries are stopped. +// Errors from individual services are collected and logged but do not abort +// the overall reconcile. +func (m *ServerManager) reconcileBinaryServices(ctx context.Context) error { + if m.supervisor == nil { + return nil + } + + reg, err := registry.Load() + if err != nil { + return fmt.Errorf("reconcile binary: load registry: %w", err) + } + + // Compute which binary services the registry wants running. + // Map key is the binary name (e.g. "rustfs"), not the service name ("s3"), + // because the supervisor keys by binary name. + needed := map[string]services.BinaryService{} + for name, svc := range services.AllBinary() { + entry := reg.Services[name] + if entry == nil || entry.Kind != "binary" { + continue + } + // nil Enabled means enabled (back-compat). + if entry.Enabled != nil && !*entry.Enabled { + continue + } + needed[svc.Binary().Name] = svc + } + + // Stop supervised processes no longer needed. + for _, binName := range m.supervisor.SupervisedNames() { + if _, ok := needed[binName]; !ok { + if err := m.supervisor.Stop(binName, 10*time.Second); err != nil { + fmt.Fprintf(os.Stderr, "reconcile binary: stop %s: %v\n", binName, err) + } + } + } + + // Start needed processes not currently supervised. + var startErrors []string + for binName, svc := range needed { + if m.supervisor.IsRunning(binName) { + continue + } + proc, err := buildSupervisorProcess(svc) + if err != nil { + startErrors = append(startErrors, fmt.Sprintf("%s: build: %v", binName, err)) + continue + } + if err := m.supervisor.Start(ctx, proc); err != nil { + startErrors = append(startErrors, fmt.Sprintf("%s: start: %v", binName, err)) + continue + } + } + + if len(startErrors) > 0 { + return fmt.Errorf("binary reconcile: %d service(s) failed: %s", len(startErrors), strings.Join(startErrors, "; ")) + } + return nil +} diff --git a/internal/server/manager_test.go b/internal/server/manager_test.go new file mode 100644 index 0000000..8e4f890 --- /dev/null +++ b/internal/server/manager_test.go @@ -0,0 +1,98 @@ +package server + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/prvious/pv/internal/config" + "github.com/prvious/pv/internal/registry" + "github.com/prvious/pv/internal/supervisor" +) + +func TestReconcile_SpawnsBinaryServices(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + // Seed a registry with s3 as a binary service. + enabled := true + reg := ®istry.Registry{ + Services: map[string]*registry.ServiceInstance{ + "s3": {Kind: "binary", Port: 9000, ConsolePort: 9001, Enabled: &enabled}, + }, + } + if err := reg.Save(); err != nil { + t.Fatal(err) + } + + // Put a fake "rustfs" binary in place so supervisor spawn doesn't ENOENT. + binDir := config.InternalBinDir() + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + fakeBin := filepath.Join(binDir, "rustfs") + // The fake binary must bind port 9000 so the supervisor ready-check succeeds. + script := "#!/usr/bin/env python3\nimport socket, time\ns=socket.socket()\ns.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)\ns.bind(('127.0.0.1', 9000))\ns.listen(1)\ntime.sleep(60)\n" + if err := os.WriteFile(fakeBin, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + + sup := supervisor.New() + m := &ServerManager{supervisor: sup, secondaries: map[string]*FrankenPHP{}} + defer m.supervisor.StopAll(2 * time.Second) + + if err := m.reconcileBinaryServices(context.Background()); err != nil { + t.Fatalf("reconcileBinaryServices: %v", err) + } + if !sup.IsRunning("rustfs") { + t.Error("expected rustfs to be supervised after reconcile") + } +} + +func TestReconcile_StopsDisabledBinaryServices(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + binDir := config.InternalBinDir() + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + // The fake binary must bind port 9000 so the supervisor ready-check succeeds. + script := "#!/usr/bin/env python3\nimport socket, time\ns=socket.socket()\ns.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)\ns.bind(('127.0.0.1', 9000))\ns.listen(1)\ntime.sleep(60)\n" + if err := os.WriteFile(filepath.Join(binDir, "rustfs"), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + + sup := supervisor.New() + m := &ServerManager{supervisor: sup, secondaries: map[string]*FrankenPHP{}} + defer sup.StopAll(2 * time.Second) + + // Phase 1: enabled, should start. + enabled := true + reg1 := ®istry.Registry{Services: map[string]*registry.ServiceInstance{ + "s3": {Kind: "binary", Port: 9000, Enabled: &enabled}, + }} + if err := reg1.Save(); err != nil { + t.Fatal(err) + } + if err := m.reconcileBinaryServices(context.Background()); err != nil { + t.Fatal(err) + } + if !sup.IsRunning("rustfs") { + t.Fatal("expected rustfs running after first reconcile") + } + + // Phase 2: disabled, should stop. + disabled := false + reg2 := ®istry.Registry{Services: map[string]*registry.ServiceInstance{ + "s3": {Kind: "binary", Port: 9000, Enabled: &disabled}, + }} + if err := reg2.Save(); err != nil { + t.Fatal(err) + } + if err := m.reconcileBinaryServices(context.Background()); err != nil { + t.Fatal(err) + } + if sup.IsRunning("rustfs") { + t.Error("expected rustfs stopped after disabling via reconcile") + } +} diff --git a/internal/server/process.go b/internal/server/process.go index ead3cb7..d9408a4 100644 --- a/internal/server/process.go +++ b/internal/server/process.go @@ -90,7 +90,8 @@ func Start(tld string) error { fmt.Fprintf(os.Stderr, "Serving .%s domains on https (port 443) and http (port 80)\n", tld) // Create the server manager and reconcile secondary instances. - manager = NewServerManager(mainFP) + // TODO(Task 9): pass a real supervisor once wired into Start(). + manager = NewServerManager(mainFP, nil) defer func() { manager.Shutdown() manager = nil @@ -178,7 +179,6 @@ func waitForEvent(sigCh chan os.Signal, dnsErr chan error, mainFP *FrankenPHP) e } } - // IsRunning checks if a pv supervisor process is currently running. func IsRunning() bool { pid, err := ReadPID() From 60a041d17cb61beaa36b5226dd6047fffac2665d Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:55:32 -0400 Subject: [PATCH 10/21] Wire supervisor into daemon start and filter Colima boot server.Start() creates a supervisor and hands it to NewServerManager. Colima boot is now gated on the existence of Docker-kind services so a registry with only binary services does not trigger an unnecessary VM boot. ServicesToRecover filters out Kind="binary" entries so the colima-recovery code never tries to "dockerize" a binary service. --- internal/colima/recovery.go | 15 ++++++++++----- internal/server/process.go | 13 ++++++++++--- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/internal/colima/recovery.go b/internal/colima/recovery.go index 4dd3e23..583897f 100644 --- a/internal/colima/recovery.go +++ b/internal/colima/recovery.go @@ -1,17 +1,22 @@ package colima import ( + "sort" + "github.com/prvious/pv/internal/registry" ) -// RecoverServices checks all registered services and ensures their containers -// are running. This is called on daemon start after Colima is verified running. -// The actual Docker operations are performed by the caller since we don't import -// the container package here to avoid circular dependencies. +// ServicesToRecover returns the registry keys of docker-backed services that +// the caller should ensure are running. Binary-backed services are handled +// by the supervisor inside the daemon and must not be recovered here. func ServicesToRecover(reg *registry.Registry) []string { var keys []string - for key := range reg.Services { + for key, inst := range reg.Services { + if inst.Kind == "binary" { + continue + } keys = append(keys, key) } + sort.Strings(keys) return keys } diff --git a/internal/server/process.go b/internal/server/process.go index d9408a4..a24ec60 100644 --- a/internal/server/process.go +++ b/internal/server/process.go @@ -19,6 +19,7 @@ import ( "github.com/prvious/pv/internal/phpenv" "github.com/prvious/pv/internal/registry" "github.com/prvious/pv/internal/services" + "github.com/prvious/pv/internal/supervisor" "github.com/prvious/pv/internal/watcher" ) @@ -90,8 +91,8 @@ func Start(tld string) error { fmt.Fprintf(os.Stderr, "Serving .%s domains on https (port 443) and http (port 80)\n", tld) // Create the server manager and reconcile secondary instances. - // TODO(Task 9): pass a real supervisor once wired into Start(). - manager = NewServerManager(mainFP, nil) + sup := supervisor.New() + manager = NewServerManager(mainFP, sup) defer func() { manager.Shutdown() manager = nil @@ -124,7 +125,13 @@ func Start(tld string) error { // This avoids blocking DNS + FrankenPHP startup on the ~15s VM boot. colimaCtx, colimaCancel := context.WithCancel(context.Background()) defer colimaCancel() - if colima.IsInstalled() && len(reg.ListServices()) > 0 { + dockerCount := 0 + for _, inst := range reg.ListServices() { + if inst.Kind != "binary" { + dockerCount++ + } + } + if colima.IsInstalled() && dockerCount > 0 { go bootColimaAndRecover(colimaCtx, settings.Defaults.VM) } From 72e40a8fe4383d9828608b9973d527efc5201142 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:57:06 -0400 Subject: [PATCH 11/21] Add service command kind dispatcher resolveKind is the single place that decides whether a given service name is docker- or binary-backed. It also rejects pre-existing docker-shaped registry entries for names that are now binary services so migrations do not silently overwrite state. --- internal/commands/service/dispatch.go | 43 ++++++++++++++ internal/commands/service/dispatch_test.go | 67 ++++++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 internal/commands/service/dispatch.go create mode 100644 internal/commands/service/dispatch_test.go diff --git a/internal/commands/service/dispatch.go b/internal/commands/service/dispatch.go new file mode 100644 index 0000000..1a9790f --- /dev/null +++ b/internal/commands/service/dispatch.go @@ -0,0 +1,43 @@ +package service + +import ( + "fmt" + + "github.com/prvious/pv/internal/registry" + "github.com/prvious/pv/internal/services" +) + +type serviceKind int + +const ( + kindUnknown serviceKind = iota + kindDocker + kindBinary +) + +// resolveKind determines whether the named service is a binary or docker +// service, returning at most one of the concrete service values. +// If the name matches a binary service but the registry already holds a +// docker-shaped entry for that name, an error is returned: no silent +// auto-migration. The user's remedy is `pv uninstall && pv setup`. +func resolveKind(reg *registry.Registry, name string) (serviceKind, services.BinaryService, services.Service, error) { + binSvc, binOK := services.LookupBinary(name) + docSvc, docErr := services.Lookup(name) + + if binOK { + // Guard against a pre-existing docker-shaped entry for what is now + // a binary service. + if existing, ok := reg.Services[name]; ok { + if existing.Kind != "binary" { + return kindUnknown, nil, nil, fmt.Errorf( + "%s is already registered (as docker) from a previous pv version. "+ + "Run `pv uninstall && pv setup` to reset", name) + } + } + return kindBinary, binSvc, nil, nil + } + if docErr == nil { + return kindDocker, nil, docSvc, nil + } + return kindUnknown, nil, nil, docErr +} diff --git a/internal/commands/service/dispatch_test.go b/internal/commands/service/dispatch_test.go new file mode 100644 index 0000000..bf481e7 --- /dev/null +++ b/internal/commands/service/dispatch_test.go @@ -0,0 +1,67 @@ +package service + +import ( + "testing" + + "github.com/prvious/pv/internal/registry" + "github.com/prvious/pv/internal/services" +) + +func TestResolveKind_BinaryServiceByName(t *testing.T) { + reg := ®istry.Registry{Services: map[string]*registry.ServiceInstance{}} + kind, bin, doc, err := resolveKind(reg, "s3") + if err != nil { + t.Fatalf("resolveKind: %v", err) + } + if kind != kindBinary { + t.Errorf("kind = %v, want kindBinary", kind) + } + if bin == nil { + t.Error("binary service should be non-nil") + } + if doc != nil { + t.Error("docker service should be nil") + } + if _, ok := bin.(*services.RustFS); !ok { + t.Errorf("expected *RustFS, got %T", bin) + } +} + +func TestResolveKind_DockerServiceByName(t *testing.T) { + reg := ®istry.Registry{Services: map[string]*registry.ServiceInstance{}} + kind, bin, doc, err := resolveKind(reg, "mysql") + if err != nil { + t.Fatalf("resolveKind: %v", err) + } + if kind != kindDocker { + t.Errorf("kind = %v, want kindDocker", kind) + } + if doc == nil { + t.Error("docker service should be non-nil") + } + if bin != nil { + t.Error("binary service should be nil") + } +} + +func TestResolveKind_Unknown_ReturnsError(t *testing.T) { + reg := ®istry.Registry{Services: map[string]*registry.ServiceInstance{}} + _, _, _, err := resolveKind(reg, "bogus") + if err == nil { + t.Fatal("expected error for unknown service") + } +} + +func TestResolveKind_DockerEntryBlocksBinaryRegistration(t *testing.T) { + // Pre-existing Docker "s3" entry (from older pv) should error on a + // service:add for the now-binary "s3" — no silent auto-migration. + reg := ®istry.Registry{ + Services: map[string]*registry.ServiceInstance{ + "s3": {Kind: "", Image: "rustfs/rustfs:latest", Port: 9000}, + }, + } + _, _, _, err := resolveKind(reg, "s3") + if err == nil { + t.Fatal("expected error for pre-existing docker s3 entry") + } +} From 949f84c580e137a2021756c0e28cb5d4a9ae0fb3 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 03:59:37 -0400 Subject: [PATCH 12/21] Implement service:add binary path New addBinary function downloads the service's binary, registers it with Kind=binary and Enabled=true, and signals the running daemon to reconcile. When no daemon is running, the entry is persisted and will be picked up by the next pv start. Docker path extracted into addDocker so the RunE closure is now a thin dispatcher keyed off resolveKind. --- internal/commands/service/add.go | 336 ++++++++++++++++++++----------- 1 file changed, 219 insertions(+), 117 deletions(-) diff --git a/internal/commands/service/add.go b/internal/commands/service/add.go index 744ff65..875a801 100644 --- a/internal/commands/service/add.go +++ b/internal/commands/service/add.go @@ -1,9 +1,13 @@ package service import ( + "context" "fmt" + "net/http" "os" + "time" + "github.com/prvious/pv/internal/binaries" "github.com/prvious/pv/internal/caddy" "github.com/prvious/pv/internal/colima" colimacmds "github.com/prvious/pv/internal/commands/colima" @@ -24,156 +28,254 @@ var addCmd = &cobra.Command{ Example: `# Add MySQL with default version pv service:add mysql -# Add a specific Redis version -pv service:add redis 7 +# Add S3 (RustFS binary) +pv service:add s3 -# Add PostgreSQL -pv service:add postgres 16`, +# Add a specific Redis version +pv service:add redis 7`, Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) error { - svcName := args[0] - svc, err := services.Lookup(svcName) - if err != nil { - return err - } - - version := svc.DefaultVersion() - if len(args) > 1 { - version = args[1] - } - - key := services.ServiceKey(svcName, version) + name := args[0] reg, err := registry.Load() if err != nil { return fmt.Errorf("cannot load registry: %w", err) } - existing, findErr := reg.FindService(key) - if findErr != nil { - return findErr + kind, binSvc, dockerSvc, err := resolveKind(reg, name) + if err != nil { + return err } - if existing != nil { - fmt.Fprintln(os.Stderr) - ui.Success(fmt.Sprintf("%s is already added", ui.Accent.Bold(true).Render(svc.DisplayName()+" "+version))) - fmt.Fprintln(os.Stderr) - return nil + + switch kind { + case kindBinary: + return addBinary(cmd.Context(), reg, binSvc) + case kindDocker: + version := dockerSvc.DefaultVersion() + if len(args) > 1 { + version = args[1] + } + return addDocker(cmd, reg, dockerSvc, name, version) } + return fmt.Errorf("unknown service %q", name) + }, +} + +func addDocker(cmd *cobra.Command, reg *registry.Registry, svc services.Service, svcName, version string) error { + key := services.ServiceKey(svcName, version) + existing, findErr := reg.FindService(key) + if findErr != nil { + return findErr + } + if existing != nil { fmt.Fprintln(os.Stderr) + ui.Success(fmt.Sprintf("%s is already added", ui.Accent.Bold(true).Render(svc.DisplayName()+" "+version))) + fmt.Fprintln(os.Stderr) + return nil + } - opts := svc.CreateOpts(version) + fmt.Fprintln(os.Stderr) - // Create data directory before container creation (bind mounts require it to exist). - dataDir := config.ServiceDataDir(svcName, version) - if err := os.MkdirAll(dataDir, 0755); err != nil { - return fmt.Errorf("cannot create data directory: %w", err) - } + opts := svc.CreateOpts(version) - // Ensure Colima is installed (lazy install on first service:add). - containerReady := false - if !colima.IsInstalled() { - if err := colimacmds.RunInstall(); err != nil { - return fmt.Errorf("cannot install Colima (required for services): %w", err) - } - } + // Create data directory before container creation (bind mounts require it to exist). + dataDir := config.ServiceDataDir(svcName, version) + if err := os.MkdirAll(dataDir, 0755); err != nil { + return fmt.Errorf("cannot create data directory: %w", err) + } - settings, err := config.LoadSettings() - if err != nil { - return fmt.Errorf("cannot load settings: %w", err) + // Ensure Colima is installed (lazy install on first service:add). + containerReady := false + if !colima.IsInstalled() { + if err := colimacmds.RunInstall(); err != nil { + return fmt.Errorf("cannot install Colima (required for services): %w", err) } + } - if err := colima.EnsureRunning(settings.Defaults.VM); err != nil { - ui.Subtle(fmt.Sprintf("Container runtime unavailable: %v", err)) - ui.Subtle("Service registered — container will start when runtime is available.") - } else { - engine, engineErr := container.NewEngine(config.ColimaSocketPath()) - if engineErr != nil { - return fmt.Errorf("cannot connect to Docker: %w", engineErr) - } - defer engine.Close() - - // Pull image. - if err := ui.Step(fmt.Sprintf("Pulling %s...", opts.Image), func() (string, error) { - if err := engine.Pull(cmd.Context(), opts.Image); err != nil { - return "", fmt.Errorf("cannot pull %s: %w", opts.Image, err) - } - return fmt.Sprintf("Pulled %s", opts.Image), nil - }); err != nil { - return err - } + settings, err := config.LoadSettings() + if err != nil { + return fmt.Errorf("cannot load settings: %w", err) + } - // Create and start container. - if err := ui.Step(fmt.Sprintf("Starting %s %s...", svc.DisplayName(), version), func() (string, error) { - if _, err := engine.CreateAndStart(cmd.Context(), opts); err != nil { - return "", err - } - port := svc.Port(version) - return fmt.Sprintf("%s %s running on :%d", svc.DisplayName(), version, port), nil - }); err != nil { - return err - } - containerReady = true + if err := colima.EnsureRunning(settings.Defaults.VM); err != nil { + ui.Subtle(fmt.Sprintf("Container runtime unavailable: %v", err)) + ui.Subtle("Service registered — container will start when runtime is available.") + } else { + engine, engineErr := container.NewEngine(config.ColimaSocketPath()) + if engineErr != nil { + return fmt.Errorf("cannot connect to Docker: %w", engineErr) } + defer engine.Close() - // Update registry. - instance := ®istry.ServiceInstance{ - Image: opts.Image, - Port: svc.Port(version), - ConsolePort: svc.ConsolePort(version), + // Pull image. + if err := ui.Step(fmt.Sprintf("Pulling %s...", opts.Image), func() (string, error) { + if err := engine.Pull(cmd.Context(), opts.Image); err != nil { + return "", fmt.Errorf("cannot pull %s: %w", opts.Image, err) + } + return fmt.Sprintf("Pulled %s", opts.Image), nil + }); err != nil { + return err } - if err := reg.AddService(key, instance); err != nil { + + // Create and start container. + if err := ui.Step(fmt.Sprintf("Starting %s %s...", svc.DisplayName(), version), func() (string, error) { + if _, err := engine.CreateAndStart(cmd.Context(), opts); err != nil { + return "", err + } + port := svc.Port(version) + return fmt.Sprintf("%s %s running on :%d", svc.DisplayName(), version, port), nil + }); err != nil { return err } - if err := reg.Save(); err != nil { - return fmt.Errorf("cannot save registry: %w", err) + containerReady = true + } + + // Update registry. + instance := ®istry.ServiceInstance{ + Image: opts.Image, + Port: svc.Port(version), + ConsolePort: svc.ConsolePort(version), + } + if err := reg.AddService(key, instance); err != nil { + return err + } + if err := reg.Save(); err != nil { + return fmt.Errorf("cannot save registry: %w", err) + } + + // Update .env for linked Laravel projects. + updateLinkedProjectsEnv(reg, svcName, svc, version) + + // Regenerate Caddy configs for service consoles (*.pv.{tld}). + if err := caddy.GenerateServiceSiteConfigs(reg); err != nil { + ui.Subtle(fmt.Sprintf("Could not generate service site config: %v", err)) + } + if server.IsRunning() { + if err := server.SignalDaemon(); err != nil { + ui.Subtle(fmt.Sprintf("Could not signal daemon: %v", err)) } + } - // Update .env for linked Laravel projects. - updateLinkedProjectsEnv(reg, svcName, svc, version) + // Print connection details. + port := svc.Port(version) + if containerReady { + ui.Success(fmt.Sprintf("%s %s running on :%d", svc.DisplayName(), version, port)) + } else { + ui.Success(fmt.Sprintf("%s %s registered on :%d", svc.DisplayName(), version, port)) + } + fmt.Fprintln(os.Stderr) + fmt.Fprintf(os.Stderr, " %s %s\n", ui.Muted.Render("Host"), "127.0.0.1") + fmt.Fprintf(os.Stderr, " %s %d\n", ui.Muted.Render("Port"), port) - // Regenerate Caddy configs for service consoles (*.pv.{tld}). - if err := caddy.GenerateServiceSiteConfigs(reg); err != nil { - ui.Subtle(fmt.Sprintf("Could not generate service site config: %v", err)) + envVars := svc.EnvVars("", port) + if user, ok := envVars["DB_USERNAME"]; ok { + fmt.Fprintf(os.Stderr, " %s %s\n", ui.Muted.Render("User"), user) + pw := envVars["DB_PASSWORD"] + if pw == "" { + pw = "(none)" } - if server.IsRunning() { - if err := server.SignalDaemon(); err != nil { - ui.Subtle(fmt.Sprintf("Could not signal daemon: %v", err)) + fmt.Fprintf(os.Stderr, " %s %s\n", ui.Muted.Render("Pass"), pw) + } + if routes := svc.WebRoutes(); len(routes) > 0 { + settings, _ := config.LoadSettings() + if settings != nil { + for _, route := range routes { + fmt.Fprintf(os.Stderr, " %s https://%s.pv.%s\n", ui.Muted.Render(route.Subdomain), route.Subdomain, settings.Defaults.TLD) } } + } else if consolePt := svc.ConsolePort(version); consolePt > 0 { + fmt.Fprintf(os.Stderr, " %s :%d\n", ui.Muted.Render("Console"), consolePt) + } + fmt.Fprintln(os.Stderr) - // Print connection details. - port := svc.Port(version) - if containerReady { - ui.Success(fmt.Sprintf("%s %s running on :%d", svc.DisplayName(), version, port)) - } else { - ui.Success(fmt.Sprintf("%s %s registered on :%d", svc.DisplayName(), version, port)) - } - fmt.Fprintln(os.Stderr) - fmt.Fprintf(os.Stderr, " %s %s\n", ui.Muted.Render("Host"), "127.0.0.1") - fmt.Fprintf(os.Stderr, " %s %d\n", ui.Muted.Render("Port"), port) - - envVars := svc.EnvVars("", port) - if user, ok := envVars["DB_USERNAME"]; ok { - fmt.Fprintf(os.Stderr, " %s %s\n", ui.Muted.Render("User"), user) - pw := envVars["DB_PASSWORD"] - if pw == "" { - pw = "(none)" - } - fmt.Fprintf(os.Stderr, " %s %s\n", ui.Muted.Render("Pass"), pw) + return nil +} + +// addBinary downloads the service's binary (if not yet present), persists its +// version, registers the service in the registry, then signals the daemon. +func addBinary(ctx context.Context, reg *registry.Registry, svc services.BinaryService) error { + name := svc.Name() + if _, exists := reg.Services[name]; exists { + ui.Success(fmt.Sprintf("%s is already added", svc.DisplayName())) + return nil + } + + client := &http.Client{Timeout: 60 * time.Second} + + // Resolve latest upstream version. + latest, err := binaries.FetchLatestVersion(client, svc.Binary()) + if err != nil { + return fmt.Errorf("cannot resolve latest %s version: %w", svc.Binary().DisplayName, err) + } + + // Download + extract into ~/.pv/internal/bin/. + if err := ui.Step(fmt.Sprintf("Downloading %s %s...", svc.Binary().DisplayName, latest), func() (string, error) { + if err := binaries.InstallBinary(client, svc.Binary(), latest); err != nil { + return "", err } - if routes := svc.WebRoutes(); len(routes) > 0 { - settings, _ := config.LoadSettings() - if settings != nil { - for _, route := range routes { - fmt.Fprintf(os.Stderr, " %s https://%s.pv.%s\n", ui.Muted.Render(route.Subdomain), route.Subdomain, settings.Defaults.TLD) - } - } - } else if consolePt := svc.ConsolePort(version); consolePt > 0 { - fmt.Fprintf(os.Stderr, " %s :%d\n", ui.Muted.Render("Console"), consolePt) + return fmt.Sprintf("Installed %s %s", svc.Binary().DisplayName, latest), nil + }); err != nil { + return err + } + + // Record version for later pv-update comparisons. + vs, err := binaries.LoadVersions() + if err != nil { + return fmt.Errorf("cannot load versions state: %w", err) + } + vs.Set(svc.Binary().Name, latest) + if err := vs.Save(); err != nil { + return fmt.Errorf("cannot save versions state: %w", err) + } + + // Register service. + enabled := true + inst := ®istry.ServiceInstance{ + Port: svc.Port(), + ConsolePort: svc.ConsolePort(), + Kind: "binary", + Enabled: &enabled, + } + if err := reg.AddService(name, inst); err != nil { + return err + } + if err := reg.Save(); err != nil { + return fmt.Errorf("cannot save registry: %w", err) + } + + // Regenerate Caddy configs for service consoles (*.pv.{tld}). + if err := caddy.GenerateServiceSiteConfigs(reg); err != nil { + ui.Subtle(fmt.Sprintf("Could not generate service site config: %v", err)) + } + + // Signal daemon to reconcile. + if server.IsRunning() { + if err := server.SignalDaemon(); err != nil { + ui.Subtle(fmt.Sprintf("Could not signal daemon: %v", err)) } - fmt.Fprintln(os.Stderr) + ui.Success(fmt.Sprintf("%s registered and running on :%d", svc.DisplayName(), svc.Port())) + } else { + ui.Success(fmt.Sprintf("%s registered on :%d", svc.DisplayName(), svc.Port())) + ui.Subtle("daemon not running — service will start on next `pv start`") + } - return nil - }, + printBinaryConnectionDetails(svc) + return nil +} + +// printBinaryConnectionDetails mirrors the verbose "Host / Port / web routes" +// footer that the docker path prints, scoped to the binary-service shape. +func printBinaryConnectionDetails(svc services.BinaryService) { + fmt.Fprintln(os.Stderr) + fmt.Fprintf(os.Stderr, " %s 127.0.0.1\n", ui.Muted.Render("Host")) + fmt.Fprintf(os.Stderr, " %s %d\n", ui.Muted.Render("Port"), svc.Port()) + settings, _ := config.LoadSettings() + if settings != nil { + for _, route := range svc.WebRoutes() { + fmt.Fprintf(os.Stderr, " %s https://%s.pv.%s\n", + ui.Muted.Render(route.Subdomain), route.Subdomain, settings.Defaults.TLD) + } + } + fmt.Fprintln(os.Stderr) } From 927893f136a36af496b5b5e3ab36946471b8d3ae Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 04:02:13 -0400 Subject: [PATCH 13/21] Implement service:start and service:stop binary paths Set Enabled on the registry entry and SignalDaemon so the daemon's reconcile loop spawns or stops the supervised process without restarting FrankenPHP. The 'all services' branch of each command now skips binary-kind entries (the daemon owns their lifecycle via Reconcile). --- internal/commands/service/start.go | 37 +++++++++++++++++++++++++++++- internal/commands/service/stop.go | 36 ++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/internal/commands/service/start.go b/internal/commands/service/start.go index 93f10bc..d028e13 100644 --- a/internal/commands/service/start.go +++ b/internal/commands/service/start.go @@ -8,6 +8,7 @@ import ( "github.com/prvious/pv/internal/config" "github.com/prvious/pv/internal/container" "github.com/prvious/pv/internal/registry" + "github.com/prvious/pv/internal/server" "github.com/prvious/pv/internal/services" "github.com/prvious/pv/internal/ui" "github.com/spf13/cobra" @@ -87,7 +88,12 @@ var startCmd = &cobra.Command{ fmt.Fprintln(os.Stderr) return nil } - for key := range svcs { + for key, inst := range svcs { + if inst.Kind == "binary" { + // Binary services are managed by the daemon; nothing to do here. + // A subsequent SignalDaemon call will reconcile them. + continue + } if err := ui.Step(fmt.Sprintf("Starting %s...", key), func() (string, error) { return startService(cmd, reg, key) }); err != nil { @@ -102,6 +108,35 @@ var startCmd = &cobra.Command{ } } } else { + // Dispatch on service kind. Binary services don't use the versioned-key + // machinery below; they flip a registry flag and let the daemon reconcile. + kind, binSvc, _, resErr := resolveKind(reg, args[0]) + if resErr != nil { + return resErr + } + if kind == kindBinary { + name := binSvc.Name() + inst, ok := reg.Services[name] + if !ok { + return fmt.Errorf("%s not registered; run `pv service:add %s` first", name, name) + } + tru := true + inst.Enabled = &tru + if err := reg.Save(); err != nil { + return fmt.Errorf("cannot save registry: %w", err) + } + if server.IsRunning() { + if err := server.SignalDaemon(); err != nil { + ui.Subtle(fmt.Sprintf("Could not signal daemon: %v", err)) + } + ui.Success(fmt.Sprintf("%s enabled; daemon reconciled", binSvc.DisplayName())) + } else { + ui.Success(fmt.Sprintf("%s enabled", binSvc.DisplayName())) + ui.Subtle("daemon not running — service will start on next `pv start`") + } + return nil + } + key := args[0] var resolveErr error key, resolveErr = reg.ResolveServiceKey(key) diff --git a/internal/commands/service/stop.go b/internal/commands/service/stop.go index 73ee6da..8cd5f27 100644 --- a/internal/commands/service/stop.go +++ b/internal/commands/service/stop.go @@ -7,6 +7,7 @@ import ( "github.com/prvious/pv/internal/config" "github.com/prvious/pv/internal/container" "github.com/prvious/pv/internal/registry" + "github.com/prvious/pv/internal/server" "github.com/prvious/pv/internal/services" "github.com/prvious/pv/internal/ui" "github.com/spf13/cobra" @@ -33,7 +34,12 @@ var stopCmd = &cobra.Command{ fmt.Fprintln(os.Stderr) return nil } - for key := range svcs { + for key, inst := range svcs { + if inst.Kind == "binary" { + // Binary services are managed by the daemon; nothing to do here. + // A subsequent SignalDaemon call will reconcile them. + continue + } if err := ui.Step(fmt.Sprintf("Stopping %s...", key), func() (string, error) { svcName, version := services.ParseServiceKey(key) svcDef, lookupErr := services.Lookup(svcName) @@ -61,6 +67,34 @@ var stopCmd = &cobra.Command{ applyFallbacksToLinkedProjects(reg, svcName) } } else { + // Dispatch on service kind. Binary services don't use the versioned-key + // machinery below; they flip a registry flag and let the daemon reconcile. + kind, binSvc, _, resErr := resolveKind(reg, args[0]) + if resErr != nil { + return resErr + } + if kind == kindBinary { + name := binSvc.Name() + inst, ok := reg.Services[name] + if !ok { + return fmt.Errorf("%s not registered", name) + } + fls := false + inst.Enabled = &fls + if err := reg.Save(); err != nil { + return fmt.Errorf("cannot save registry: %w", err) + } + if server.IsRunning() { + if err := server.SignalDaemon(); err != nil { + ui.Subtle(fmt.Sprintf("Could not signal daemon: %v", err)) + } + ui.Success(fmt.Sprintf("%s disabled; daemon reconciled", binSvc.DisplayName())) + } else { + ui.Success(fmt.Sprintf("%s disabled", binSvc.DisplayName())) + } + return nil + } + key := args[0] var resolveErr error key, resolveErr = reg.ResolveServiceKey(key) From 86d226ae8a7545d300afdadd596fcae1b1176ed6 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 04:03:58 -0400 Subject: [PATCH 14/21] Implement service:remove and service:destroy binary paths remove unregisters and deletes the binary but keeps data. destroy also removes config.ServiceDataDir. Both signal the daemon so the supervised process is stopped. --- internal/commands/service/destroy.go | 40 ++++++++++++++++++++++-- internal/commands/service/remove.go | 46 ++++++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/internal/commands/service/destroy.go b/internal/commands/service/destroy.go index e2ed904..25b43f9 100644 --- a/internal/commands/service/destroy.go +++ b/internal/commands/service/destroy.go @@ -3,8 +3,10 @@ package service import ( "fmt" "os" + "path/filepath" "strings" + "github.com/prvious/pv/internal/binaries" "github.com/prvious/pv/internal/caddy" "github.com/prvious/pv/internal/config" "github.com/prvious/pv/internal/container" @@ -27,11 +29,45 @@ var destroyCmd = &cobra.Command{ if err != nil { return fmt.Errorf("cannot load registry: %w", err) } - var resolveErr error - key, resolveErr = reg.ResolveServiceKey(key) + + kind, binSvc, _, resolveErr := resolveKind(reg, args[0]) if resolveErr != nil { return resolveErr } + if kind == kindBinary { + name := binSvc.Name() + _ = reg.RemoveService(name) + if err := reg.Save(); err != nil { + return fmt.Errorf("cannot save registry: %w", err) + } + + binPath := filepath.Join(config.InternalBinDir(), binSvc.Binary().Name) + _ = os.Remove(binPath) + if vs, vsErr := binaries.LoadVersions(); vsErr == nil { + vs.Set(binSvc.Binary().Name, "") + _ = vs.Save() + } + + dataDir := config.ServiceDataDir(name, "latest") + if err := os.RemoveAll(dataDir); err != nil { + return fmt.Errorf("cannot delete data: %w", err) + } + + if err := caddy.GenerateServiceSiteConfigs(reg); err != nil { + ui.Subtle(fmt.Sprintf("Could not regenerate service site config: %v", err)) + } + if server.IsRunning() { + _ = server.SignalDaemon() + } + ui.Success(fmt.Sprintf("%s destroyed (binary + data gone)", binSvc.DisplayName())) + return nil + } + + var resolveKeyErr error + key, resolveKeyErr = reg.ResolveServiceKey(key) + if resolveKeyErr != nil { + return resolveKeyErr + } svc, findErr := reg.FindService(key) if findErr != nil { diff --git a/internal/commands/service/remove.go b/internal/commands/service/remove.go index 1c46a9a..4db4934 100644 --- a/internal/commands/service/remove.go +++ b/internal/commands/service/remove.go @@ -2,7 +2,10 @@ package service import ( "fmt" + "os" + "path/filepath" + "github.com/prvious/pv/internal/binaries" "github.com/prvious/pv/internal/caddy" "github.com/prvious/pv/internal/config" "github.com/prvious/pv/internal/container" @@ -25,11 +28,50 @@ var removeCmd = &cobra.Command{ if err != nil { return fmt.Errorf("cannot load registry: %w", err) } - var resolveErr error - key, resolveErr = reg.ResolveServiceKey(key) + + kind, binSvc, _, resolveErr := resolveKind(reg, args[0]) if resolveErr != nil { return resolveErr } + if kind == kindBinary { + name := binSvc.Name() + if _, ok := reg.Services[name]; !ok { + return fmt.Errorf("%s not registered", name) + } + if err := reg.RemoveService(name); err != nil { + return err + } + if err := reg.Save(); err != nil { + return fmt.Errorf("cannot save registry: %w", err) + } + // Delete the binary. + binPath := filepath.Join(config.InternalBinDir(), binSvc.Binary().Name) + _ = os.Remove(binPath) + // Clear the tracked version so a future `service:add` redownloads. + if vs, vsErr := binaries.LoadVersions(); vsErr == nil { + vs.Set(binSvc.Binary().Name, "") + _ = vs.Save() + } + + // Regenerate Caddy configs (remove s3.pv.test / s3-api.pv.test routes). + if err := caddy.GenerateServiceSiteConfigs(reg); err != nil { + ui.Subtle(fmt.Sprintf("Could not regenerate service site config: %v", err)) + } + + if server.IsRunning() { + if err := server.SignalDaemon(); err != nil { + ui.Subtle(fmt.Sprintf("Could not signal daemon: %v", err)) + } + } + ui.Success(fmt.Sprintf("%s removed (data preserved)", binSvc.DisplayName())) + return nil + } + + var resolveKeyErr error + key, resolveKeyErr = reg.ResolveServiceKey(key) + if resolveKeyErr != nil { + return resolveKeyErr + } svc, findErr := reg.FindService(key) if findErr != nil { From 44364db876aa52272a69b67599f1d9d01beb6c10 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 04:05:50 -0400 Subject: [PATCH 15/21] Read daemon-status.json for binary service observability service:status shows Kind/Registered/Enabled/Running/PID for binary services. service:list merges docker and binary rows so users see a unified view. --- internal/commands/service/list.go | 37 +++++++++++++++++++++++++ internal/commands/service/status.go | 42 +++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/internal/commands/service/list.go b/internal/commands/service/list.go index ce20ea6..a7fcbef 100644 --- a/internal/commands/service/list.go +++ b/internal/commands/service/list.go @@ -8,6 +8,7 @@ import ( "github.com/prvious/pv/internal/config" "github.com/prvious/pv/internal/container" "github.com/prvious/pv/internal/registry" + "github.com/prvious/pv/internal/server" "github.com/prvious/pv/internal/services" "github.com/prvious/pv/internal/ui" "github.com/spf13/cobra" @@ -40,8 +41,44 @@ var listCmd = &cobra.Command{ ui.Subtle(fmt.Sprintf("Cannot connect to Docker: %v", engineErr)) } + snap, _ := server.ReadDaemonStatus() var rows [][]string for key, svc := range svcs { + if svc.Kind == "binary" { + binDef, ok := services.LookupBinary(key) + if !ok { + continue + } + enabled := true + if svc.Enabled != nil { + enabled = *svc.Enabled + } + running := false + if snap != nil { + if st, exists := snap.Supervised[binDef.Binary().Name]; exists { + running = st.Running + } + } + status := "stopped" + if running { + status = "running" + } else if !enabled { + status = "disabled" + } + portStr := fmt.Sprintf(":%d", svc.Port) + if svc.ConsolePort > 0 { + portStr += fmt.Sprintf(", :%d", svc.ConsolePort) + } + projects := reg.ProjectsUsingService(key) + projectStr := "-" + if len(projects) > 0 { + projectStr = strings.Join(projects, ", ") + } + rows = append(rows, []string{key, status, portStr, projectStr}) + continue + } + + // Docker branch. svcName, version := services.ParseServiceKey(key) status := "added" diff --git a/internal/commands/service/status.go b/internal/commands/service/status.go index 0cb49a8..67ae611 100644 --- a/internal/commands/service/status.go +++ b/internal/commands/service/status.go @@ -2,11 +2,13 @@ package service import ( "fmt" + "os" "strings" "github.com/prvious/pv/internal/config" "github.com/prvious/pv/internal/container" "github.com/prvious/pv/internal/registry" + "github.com/prvious/pv/internal/server" "github.com/prvious/pv/internal/services" "github.com/prvious/pv/internal/ui" "github.com/spf13/cobra" @@ -24,11 +26,47 @@ var statusCmd = &cobra.Command{ if err != nil { return fmt.Errorf("cannot load registry: %w", err) } - var resolveErr error - key, resolveErr = reg.ResolveServiceKey(key) + + kind, binSvc, _, resolveErr := resolveKind(reg, args[0]) if resolveErr != nil { return resolveErr } + if kind == kindBinary { + name := binSvc.Name() + inst, ok := reg.Services[name] + enabled := true + registered := ok + if ok && inst.Enabled != nil { + enabled = *inst.Enabled + } + + running := false + pid := 0 + if snap, err := server.ReadDaemonStatus(); err == nil { + if st, exists := snap.Supervised[binSvc.Binary().Name]; exists { + running = st.Running + pid = st.PID + } + } + + fmt.Fprintln(os.Stderr) + fmt.Fprintf(os.Stderr, " %s %s\n", ui.Muted.Render("Service"), binSvc.DisplayName()) + fmt.Fprintf(os.Stderr, " %s %s\n", ui.Muted.Render("Kind"), "binary") + fmt.Fprintf(os.Stderr, " %s %v\n", ui.Muted.Render("Registered"), registered) + fmt.Fprintf(os.Stderr, " %s %v\n", ui.Muted.Render("Enabled"), enabled) + fmt.Fprintf(os.Stderr, " %s %v\n", ui.Muted.Render("Running"), running) + if pid > 0 { + fmt.Fprintf(os.Stderr, " %s %d\n", ui.Muted.Render("PID"), pid) + } + fmt.Fprintln(os.Stderr) + return nil + } + + var resolveKeyErr error + key, resolveKeyErr = reg.ResolveServiceKey(key) + if resolveKeyErr != nil { + return resolveKeyErr + } instance, findErr := reg.FindService(key) if findErr != nil { From f96bca99c0e64829580d0dadff4c8c8c4dc72c6b Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 04:07:02 -0400 Subject: [PATCH 16/21] Implement service:logs binary path via tail-f of log file Binary services write stdout+stderr to ~/.pv/logs/.log via the supervisor. service:logs dumps existing content and follows appends, exiting on context cancellation. --- internal/commands/service/logs.go | 38 +++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/internal/commands/service/logs.go b/internal/commands/service/logs.go index 35a98d6..6b1f665 100644 --- a/internal/commands/service/logs.go +++ b/internal/commands/service/logs.go @@ -2,7 +2,10 @@ package service import ( "fmt" + "io" "os" + "path/filepath" + "time" "github.com/prvious/pv/internal/config" "github.com/prvious/pv/internal/container" @@ -23,6 +26,41 @@ var logsCmd = &cobra.Command{ if err != nil { return fmt.Errorf("cannot load registry: %w", err) } + + kind, binSvc, _, kindErr := resolveKind(reg, args[0]) + if kindErr != nil { + return kindErr + } + if kind == kindBinary { + logPath := filepath.Join(config.PvDir(), "logs", binSvc.Binary().Name+".log") + f, err := os.Open(logPath) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("no log file yet (%s). Has the service run?", logPath) + } + return err + } + defer f.Close() + // Dump existing content. + if _, err := io.Copy(os.Stdout, f); err != nil { + return err + } + // Follow mode (like tail -f). Poll every 250ms for new data; exit on Ctrl-C. + for { + select { + case <-cmd.Context().Done(): + return nil + case <-time.After(250 * time.Millisecond): + } + if _, err := io.Copy(os.Stdout, f); err != nil { + if err == io.EOF { + continue + } + return err + } + } + } + key, resolveErr := reg.ResolveServiceKey(key) if resolveErr != nil { return resolveErr From 271cc29b1f1c134745f9a6f1c3dee2f86d887d90 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 04:08:50 -0400 Subject: [PATCH 17/21] Refresh binary-service binaries in pv update After the tool loop, iterate registered binary services and compare installed vs latest upstream version. Newer binaries are downloaded; user is advised to cycle the service (or pv restart) to load them since the running process keeps the old binary via its open file descriptor. --- cmd/update.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/cmd/update.go b/cmd/update.go index 3076bd1..c9ec838 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -9,13 +9,17 @@ import ( "syscall" "time" + "github.com/prvious/pv/internal/binaries" "github.com/prvious/pv/internal/colima" colimacmd "github.com/prvious/pv/internal/commands/colima" "github.com/prvious/pv/internal/commands/composer" "github.com/prvious/pv/internal/commands/mago" "github.com/prvious/pv/internal/commands/php" "github.com/prvious/pv/internal/packages" + "github.com/prvious/pv/internal/registry" "github.com/prvious/pv/internal/selfupdate" + "github.com/prvious/pv/internal/server" + "github.com/prvious/pv/internal/services" "github.com/prvious/pv/internal/ui" "github.com/spf13/cobra" ) @@ -99,6 +103,46 @@ var updateCmd = &cobra.Command{ } } + // Step 4: Update binary-service binaries. + reg, _ := registry.Load() + vs, _ := binaries.LoadVersions() + + var binaryUpdated []string + for name, svc := range services.AllBinary() { + if _, registered := reg.Services[name]; !registered { + continue + } + latest, err := binaries.FetchLatestVersion(client, svc.Binary()) + if err != nil { + ui.Subtle(fmt.Sprintf("Skipping %s: %v", svc.DisplayName(), err)) + continue + } + if !binaries.NeedsUpdate(vs, svc.Binary(), latest) { + continue + } + current := vs.Get(svc.Binary().Name) + if err := ui.Step(fmt.Sprintf("Updating %s %s -> %s", svc.Binary().DisplayName, current, latest), func() (string, error) { + if err := binaries.InstallBinary(client, svc.Binary(), latest); err != nil { + return "", err + } + return fmt.Sprintf("Updated %s to %s", svc.Binary().DisplayName, latest), nil + }); err != nil { + ui.Subtle(fmt.Sprintf("Could not update %s: %v", svc.DisplayName(), err)) + continue + } + vs.Set(svc.Binary().Name, latest) + binaryUpdated = append(binaryUpdated, name) + } + if len(binaryUpdated) > 0 { + if err := vs.Save(); err != nil { + ui.Subtle(fmt.Sprintf("Could not save versions state: %v", err)) + } + if server.IsRunning() { + ui.Subtle(fmt.Sprintf("Updated binaries: %s. Run `pv service:stop %s && pv service:start %s` (or `pv restart`) to load them.", + strings.Join(binaryUpdated, ", "), binaryUpdated[0], binaryUpdated[0])) + } + } + ui.Footer(start, "") if len(failures) > 0 { From 2ce7853f0b71ce4ae826f78e8db7999086cf9f9c Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 04:10:44 -0400 Subject: [PATCH 18/21] Add E2E phase for S3 binary service lifecycle Exercises service:add, service:stop, service:start, service:destroy against a real RustFS download. Verifies the binary is written, the daemon-status file advertises the supervised process, and port 9000 is reachable / silent at the expected moments. --- .github/workflows/e2e.yml | 9 +++-- scripts/e2e/s3-binary.sh | 70 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) create mode 100755 scripts/e2e/s3-binary.sh diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index eb43059..a17858e 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -112,13 +112,18 @@ jobs: timeout-minutes: 2 run: scripts/e2e/verify-final.sh - # ── Phase 20: Uninstall ─────────────────────────────────────── + # ── Phase 20: S3 binary service lifecycle ────────────────────── + - name: E2E — S3 binary service lifecycle + timeout-minutes: 3 + run: scripts/e2e/s3-binary.sh + + # ── Phase 21: Uninstall ─────────────────────────────────────── # TODO: frankenphp untrust hangs in CI (internal sudo prompt, no terminal) # - name: Test pv uninstall # timeout-minutes: 1 # run: scripts/e2e/uninstall.sh - # ── Phase 21: Failure Diagnostics & Cleanup ──────────────────── + # ── Phase 22: Failure Diagnostics & Cleanup ──────────────────── - name: Dump logs on failure if: failure() run: scripts/e2e/diagnostics.sh diff --git a/scripts/e2e/s3-binary.sh b/scripts/e2e/s3-binary.sh new file mode 100755 index 0000000..91751ce --- /dev/null +++ b/scripts/e2e/s3-binary.sh @@ -0,0 +1,70 @@ +#!/usr/bin/env bash +set -euo pipefail +source "$(dirname "$0")/helpers.sh" + +echo "==> Phase: S3 binary service (RustFS) lifecycle" + +# Start pv in the background so we have a live daemon for service: commands. +pv start >/tmp/pv-s3-e2e.log 2>&1 & +START_PID=$! +sleep 3 + +cleanup() { + kill "$START_PID" 2>/dev/null || true + pv stop >/dev/null 2>&1 || true +} +trap cleanup EXIT + +echo "==> service:add s3" +pv service:add s3 || { echo "FAIL: pv service:add s3 failed"; exit 1; } + +echo "==> Verify rustfs binary exists" +test -x "$HOME/.pv/internal/bin/rustfs" || { echo "FAIL: rustfs binary not installed"; exit 1; } +echo "OK: rustfs binary at ~/.pv/internal/bin/rustfs" + +echo "==> Verify daemon-status.json lists rustfs" +test -f "$HOME/.pv/daemon-status.json" || { echo "FAIL: daemon-status.json missing"; exit 1; } +grep -q '"rustfs"' "$HOME/.pv/daemon-status.json" || { + echo "FAIL: daemon-status.json does not contain rustfs entry"; + cat "$HOME/.pv/daemon-status.json"; + exit 1; +} +echo "OK: daemon-status.json advertises rustfs" + +echo "==> Verify port 9000 is reachable" +for i in $(seq 1 20); do + if nc -z 127.0.0.1 9000 2>/dev/null; then break; fi + sleep 1 +done +nc -z 127.0.0.1 9000 || { echo "FAIL: port 9000 not reachable after service:add"; exit 1; } +echo "OK: port 9000 reachable" + +echo "==> service:stop s3" +pv service:stop s3 +sleep 2 +if nc -z 127.0.0.1 9000 2>/dev/null; then + echo "FAIL: port 9000 still answering after service:stop" + exit 1 +fi +echo "OK: port 9000 silent after service:stop" + +echo "==> service:start s3" +pv service:start s3 +for i in $(seq 1 20); do + if nc -z 127.0.0.1 9000 2>/dev/null; then break; fi + sleep 1 +done +nc -z 127.0.0.1 9000 || { echo "FAIL: port 9000 not reachable after service:start"; exit 1; } +echo "OK: port 9000 reachable after service:start" + +echo "==> service:destroy s3" +pv service:destroy s3 +test ! -f "$HOME/.pv/internal/bin/rustfs" || { echo "FAIL: rustfs binary not deleted after destroy"; exit 1; } +test ! -d "$HOME/.pv/services/s3/latest/data" || { echo "FAIL: data dir not deleted after destroy"; exit 1; } +echo "OK: binary and data removed" + +echo "==> pv stop" +pv stop || true +trap - EXIT + +echo "OK: S3 binary service lifecycle passed" From 3f5710be3e40333f74496831e991df561e38f55b Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 04:15:18 -0400 Subject: [PATCH 19/21] Fix supervisor Stop/watch race on cmd.Wait exec.Cmd.Wait is documented as safe to call only once; calling it from both Stop and the watcher goroutine tripped the race detector on every run. Each managed process now carries a done channel that the watcher closes when it returns, and Stop awaits that channel instead of spawning a second Wait goroutine. Also absorbs a cosmetic gofmt realignment in internal/server/dns.go picked up earlier in this branch (no functional change). --- internal/server/dns.go | 6 ++--- internal/supervisor/supervisor.go | 40 ++++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/internal/server/dns.go b/internal/server/dns.go index 99b486b..d8ed67b 100644 --- a/internal/server/dns.go +++ b/internal/server/dns.go @@ -37,9 +37,9 @@ func (d *DNSServer) Start() error { mux.HandleFunc(d.tld+".", d.handleQuery) d.server = &dns.Server{ - Addr: d.Addr, - Net: "udp", - Handler: mux, + Addr: d.Addr, + Net: "udp", + Handler: mux, NotifyStartedFunc: func() { close(d.ready) }, } return d.server.ListenAndServe() diff --git a/internal/supervisor/supervisor.go b/internal/supervisor/supervisor.go index 216137e..ceb66f7 100644 --- a/internal/supervisor/supervisor.go +++ b/internal/supervisor/supervisor.go @@ -38,6 +38,10 @@ type managed struct { cancel context.CancelFunc // cancels the watcher goroutine stopped bool // set true when Stop was called explicitly restarts []time.Time // rolling window of restart timestamps + // done is closed by the watcher goroutine when it returns (after + // cmd.Wait has finished). Stop awaits this instead of calling + // cmd.Wait itself — exec.Cmd.Wait is not safe for concurrent callers. + done chan struct{} } // Supervisor manages a set of child processes. @@ -122,7 +126,7 @@ func (s *Supervisor) spawn(p Process) (*managed, error) { } return nil, fmt.Errorf("supervisor: spawn %s: %w", p.Name, err) } - return &managed{proc: p, cmd: cmd}, nil + return &managed{proc: p, cmd: cmd, done: make(chan struct{})}, nil } // waitReady polls p.Ready every 250ms until success or timeout. @@ -146,13 +150,29 @@ func (s *Supervisor) waitReady(ctx context.Context, p Process) error { } } -// watch blocks on cmd.Wait and handles crash restarts. +// watch blocks on cmd.Wait and handles crash restarts. It is the sole caller +// of cmd.Wait for a given cmd — Stop does not call Wait directly (exec.Cmd +// does not support concurrent Wait callers); Stop awaits watch's exit via the +// managed.done channel instead. func (s *Supervisor) watch(ctx context.Context, name string) { + // Capture the initial done channel so we can close it reliably even if + // the managed record is replaced via respawn — callers holding a + // reference to the original managed.done need to be unblocked when the + // original process exits. + s.mu.Lock() + initial, ok := s.processes[name] + s.mu.Unlock() + if !ok { + return + } + currentDone := initial.done + for { s.mu.Lock() m, ok := s.processes[name] s.mu.Unlock() if !ok { + close(currentDone) return } @@ -162,6 +182,7 @@ func (s *Supervisor) watch(ctx context.Context, name string) { if m.stopped { delete(s.processes, name) s.mu.Unlock() + close(currentDone) return } @@ -179,6 +200,7 @@ func (s *Supervisor) watch(ctx context.Context, name string) { fmt.Fprintf(os.Stderr, "supervisor: %s exceeded restart budget (5/60s); giving up (last error: %v)\n", name, waitErr) delete(s.processes, name) s.mu.Unlock() + close(currentDone) return } m.restarts = append(m.restarts, now) @@ -187,6 +209,7 @@ func (s *Supervisor) watch(ctx context.Context, name string) { // Pause briefly and respawn — no ready-wait on recovery. select { case <-ctx.Done(): + close(currentDone) return case <-time.After(2 * time.Second): } @@ -196,6 +219,7 @@ func (s *Supervisor) watch(ctx context.Context, name string) { s.mu.Lock() delete(s.processes, name) s.mu.Unlock() + close(currentDone) return } s.mu.Lock() @@ -204,11 +228,17 @@ func (s *Supervisor) watch(ctx context.Context, name string) { newM.restarts = m.restarts s.processes[name] = newM s.mu.Unlock() + // Signal that the original cmd.Wait has completed; Stop callers + // holding the old done channel can proceed. + close(currentDone) + currentDone = newM.done } } // Stop sends SIGTERM, waits up to timeout, then SIGKILL. -// After Stop returns, IsRunning(name) is false. +// After Stop returns, IsRunning(name) is false. Stop awaits the watcher +// goroutine's exit (via managed.done) rather than calling cmd.Wait itself — +// exec.Cmd.Wait is not safe to call from multiple goroutines. func (s *Supervisor) Stop(name string, timeout time.Duration) error { s.mu.Lock() m, ok := s.processes[name] @@ -222,15 +252,13 @@ func (s *Supervisor) Stop(name string, timeout time.Duration) error { } pid := m.cmd.Process.Pid cmd := m.cmd + done := m.done s.mu.Unlock() if err := cmd.Process.Signal(syscall.SIGTERM); err != nil && !errors.Is(err, os.ErrProcessDone) { return fmt.Errorf("supervisor: SIGTERM %s (pid %d): %w", name, pid, err) } - done := make(chan error, 1) - go func() { done <- cmd.Wait() }() - select { case <-done: case <-time.After(timeout): From fd3acb6919f8713fad6e96cde838137830614a5e Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 11:25:44 -0400 Subject: [PATCH 20/21] Address PR review: fix 6 critical + 2 important issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical: - Supervisor race: Stop + watch both call cmd.Wait on the same exec.Cmd only one owner at a time, but concurrent Stop during respawn can orphan the newly-spawned child. Recheck m.stopped after spawn under the lock; SIGKILL newM and bail out if Stop was called while we were spawning. Added TestSupervisor_RestartsCrashedProcess and TestSupervisor_GivesUpAfterBudget, both pass under -race. - cmd/update.go nil-panic: registry.Load and binaries.LoadVersions can return (nil, err); the old code discarded the error and would panic on reg.Services[name]. Handle errors explicitly and print a subtle warning instead of crashing. - service:destroy swallowed "not registered": check reg.Services[name] up front and return a proper error instead of silently deleting the binary and data when the entry never existed. - Reconcile Phase 2 error swallowed: binary-reconcile failures are now folded into the Reconcile return value alongside secondary-instance errors so service:add surfaces "RustFS failed to start" instead of reporting a false success. - service:start/stop dishonest messaging: when SignalDaemon fails, the old code printed both "Could not signal daemon" and "daemon reconciled" on the same path. Now prints an accurate failure with a "run pv restart" hint. - ReadyCheck invariant unenforced: buildReadyFunc silently treated a zero-value as "instantly ready". Now returns an error on zero-value and on both-set config; error propagates through buildSupervisorProcess. Added TestBuildReadyFunc_RejectsZeroValue / RejectsBothSet. Important: - Replaced Python3 dependency in manager_test.go with a compiled Go helper at internal/server/testdata/fakebinary/main.go. The helper is built once per test run and staged as the fake rustfs binary — no more hidden python3 requirement on CI runners or dev machines. - addBinary now calls updateLinkedProjectsEnvBinary so adding s3 writes AWS_* keys into linked Laravel projects' .env files, matching what the docker path has always done. Added UpdateProjectEnvForBinaryService in internal/laravel/env.go and two tests. Also strengthened TestBuildSupervisorProcess_RustFS to assert --console-enable, --console-address :9001, and --address :9000 all end up in the final supervisor.Process Args — catches regressions at the wrapper layer even if the service's Args() stays correct. --- cmd/update.go | 80 ++++++++++--------- internal/commands/service/add.go | 5 ++ internal/commands/service/destroy.go | 7 +- internal/commands/service/hooks.go | 57 ++++++++++++++ internal/commands/service/start.go | 7 +- internal/commands/service/stop.go | 7 +- internal/laravel/env.go | 19 +++++ internal/laravel/env_test.go | 44 +++++++++++ internal/server/binary_service.go | 26 ++++-- internal/server/binary_service_test.go | 69 ++++++++++++++++ internal/server/manager.go | 20 +++-- internal/server/manager_test.go | 87 ++++++++++++++++----- internal/server/testdata/fakebinary/main.go | 42 ++++++++++ internal/services/binary.go | 5 +- internal/supervisor/supervisor.go | 13 +++ internal/supervisor/supervisor_test.go | 71 +++++++++++++++++ 16 files changed, 487 insertions(+), 72 deletions(-) create mode 100644 internal/server/testdata/fakebinary/main.go diff --git a/cmd/update.go b/cmd/update.go index c9ec838..e8e347f 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -104,42 +104,52 @@ var updateCmd = &cobra.Command{ } // Step 4: Update binary-service binaries. - reg, _ := registry.Load() - vs, _ := binaries.LoadVersions() - - var binaryUpdated []string - for name, svc := range services.AllBinary() { - if _, registered := reg.Services[name]; !registered { - continue - } - latest, err := binaries.FetchLatestVersion(client, svc.Binary()) - if err != nil { - ui.Subtle(fmt.Sprintf("Skipping %s: %v", svc.DisplayName(), err)) - continue - } - if !binaries.NeedsUpdate(vs, svc.Binary(), latest) { - continue - } - current := vs.Get(svc.Binary().Name) - if err := ui.Step(fmt.Sprintf("Updating %s %s -> %s", svc.Binary().DisplayName, current, latest), func() (string, error) { - if err := binaries.InstallBinary(client, svc.Binary(), latest); err != nil { - return "", err + // registry.Load / LoadVersions return nil on a non-IsNotExist error + // (corrupt file, permissions), so both pointers must be checked + // before we dereference them. + reg, regErr := registry.Load() + if regErr != nil { + ui.Subtle(fmt.Sprintf("Skipping binary-service updates: cannot load registry: %v", regErr)) + } else { + vs, vsErr := binaries.LoadVersions() + if vsErr != nil { + ui.Subtle(fmt.Sprintf("Skipping binary-service updates: cannot load versions state: %v", vsErr)) + } else { + var binaryUpdated []string + for name, svc := range services.AllBinary() { + if _, registered := reg.Services[name]; !registered { + continue + } + latest, err := binaries.FetchLatestVersion(client, svc.Binary()) + if err != nil { + ui.Subtle(fmt.Sprintf("Skipping %s: %v", svc.DisplayName(), err)) + continue + } + if !binaries.NeedsUpdate(vs, svc.Binary(), latest) { + continue + } + current := vs.Get(svc.Binary().Name) + if err := ui.Step(fmt.Sprintf("Updating %s %s -> %s", svc.Binary().DisplayName, current, latest), func() (string, error) { + if err := binaries.InstallBinary(client, svc.Binary(), latest); err != nil { + return "", err + } + return fmt.Sprintf("Updated %s to %s", svc.Binary().DisplayName, latest), nil + }); err != nil { + ui.Subtle(fmt.Sprintf("Could not update %s: %v", svc.DisplayName(), err)) + continue + } + vs.Set(svc.Binary().Name, latest) + binaryUpdated = append(binaryUpdated, name) + } + if len(binaryUpdated) > 0 { + if err := vs.Save(); err != nil { + ui.Subtle(fmt.Sprintf("Could not save versions state: %v", err)) + } + if server.IsRunning() { + ui.Subtle(fmt.Sprintf("Updated binaries: %s. Run `pv service:stop %s && pv service:start %s` (or `pv restart`) to load them.", + strings.Join(binaryUpdated, ", "), binaryUpdated[0], binaryUpdated[0])) + } } - return fmt.Sprintf("Updated %s to %s", svc.Binary().DisplayName, latest), nil - }); err != nil { - ui.Subtle(fmt.Sprintf("Could not update %s: %v", svc.DisplayName(), err)) - continue - } - vs.Set(svc.Binary().Name, latest) - binaryUpdated = append(binaryUpdated, name) - } - if len(binaryUpdated) > 0 { - if err := vs.Save(); err != nil { - ui.Subtle(fmt.Sprintf("Could not save versions state: %v", err)) - } - if server.IsRunning() { - ui.Subtle(fmt.Sprintf("Updated binaries: %s. Run `pv service:stop %s && pv service:start %s` (or `pv restart`) to load them.", - strings.Join(binaryUpdated, ", "), binaryUpdated[0], binaryUpdated[0])) } } diff --git a/internal/commands/service/add.go b/internal/commands/service/add.go index 875a801..7b99b68 100644 --- a/internal/commands/service/add.go +++ b/internal/commands/service/add.go @@ -244,6 +244,11 @@ func addBinary(ctx context.Context, reg *registry.Registry, svc services.BinaryS return fmt.Errorf("cannot save registry: %w", err) } + // Update .env for linked Laravel projects — parity with the docker path + // (updateLinkedProjectsEnv at the end of addDocker). Without this the + // user adds s3 but linked projects never get AWS_* keys written. + updateLinkedProjectsEnvBinary(reg, name, svc) + // Regenerate Caddy configs for service consoles (*.pv.{tld}). if err := caddy.GenerateServiceSiteConfigs(reg); err != nil { ui.Subtle(fmt.Sprintf("Could not generate service site config: %v", err)) diff --git a/internal/commands/service/destroy.go b/internal/commands/service/destroy.go index 25b43f9..8ee93ff 100644 --- a/internal/commands/service/destroy.go +++ b/internal/commands/service/destroy.go @@ -36,7 +36,12 @@ var destroyCmd = &cobra.Command{ } if kind == kindBinary { name := binSvc.Name() - _ = reg.RemoveService(name) + if _, ok := reg.Services[name]; !ok { + return fmt.Errorf("%s not registered", name) + } + if err := reg.RemoveService(name); err != nil { + return err + } if err := reg.Save(); err != nil { return fmt.Errorf("cannot save registry: %w", err) } diff --git a/internal/commands/service/hooks.go b/internal/commands/service/hooks.go index 9e34d03..095d2f7 100644 --- a/internal/commands/service/hooks.go +++ b/internal/commands/service/hooks.go @@ -66,6 +66,63 @@ func updateLinkedProjectsEnv(reg *registry.Registry, svcName string, svc service } } +// updateLinkedProjectsEnvBinary mirrors updateLinkedProjectsEnv for binary +// services. It shares the same automation gate and interactive-confirm +// behavior; the only difference is which laravel helper it calls (the +// binary variant doesn't need a port argument because BinaryService.Port() +// is fixed at the struct level). +func updateLinkedProjectsEnvBinary(reg *registry.Registry, svcName string, svc services.BinaryService) { + settings, err := config.LoadSettings() + if err != nil { + ui.Subtle(fmt.Sprintf("Could not load settings for service env hooks: %v", err)) + return + } + if settings.Automation.ServiceEnvUpdate == config.AutoOff { + return + } + + var laravelProjects []registry.Project + for _, p := range reg.List() { + if p.Type == "laravel" || p.Type == "laravel-octane" { + laravelProjects = append(laravelProjects, p) + } + } + if len(laravelProjects) == 0 { + return + } + + shouldUpdate := settings.Automation.ServiceEnvUpdate == config.AutoOn + if settings.Automation.ServiceEnvUpdate == config.AutoAsk { + if !automation.IsInteractive() { + return + } + confirmed, err := automation.ConfirmFunc( + fmt.Sprintf("Update .env for %d linked Laravel project(s)", len(laravelProjects)), + ) + if err != nil { + return + } + shouldUpdate = confirmed + } + if !shouldUpdate { + return + } + + for _, p := range laravelProjects { + project := reg.Find(p.Name) + if project == nil || project.Services == nil { + continue + } + if err := laravel.UpdateProjectEnvForBinaryService( + p.Path, p.Name, svcName, svc, project.Services, + ); err != nil { + ui.Subtle(fmt.Sprintf("Could not update .env for %s: %v", p.Name, err)) + } else { + ui.Success(fmt.Sprintf("Updated .env for %s", p.Name)) + } + } +} + // applyFallbacksToLinkedProjects applies safe env fallbacks when a service // is stopped or removed. func applyFallbacksToLinkedProjects(reg *registry.Registry, svcName string) { diff --git a/internal/commands/service/start.go b/internal/commands/service/start.go index d028e13..81d6838 100644 --- a/internal/commands/service/start.go +++ b/internal/commands/service/start.go @@ -127,7 +127,12 @@ var startCmd = &cobra.Command{ } if server.IsRunning() { if err := server.SignalDaemon(); err != nil { - ui.Subtle(fmt.Sprintf("Could not signal daemon: %v", err)) + // Registry is already updated but the running daemon did + // not pick it up. Don't claim "reconciled" — that would + // lie to the user about the supervisor state. + ui.Fail(fmt.Sprintf("%s enabled in registry, but could not signal daemon: %v", binSvc.DisplayName(), err)) + ui.Subtle("Run `pv restart` to load the change.") + return nil } ui.Success(fmt.Sprintf("%s enabled; daemon reconciled", binSvc.DisplayName())) } else { diff --git a/internal/commands/service/stop.go b/internal/commands/service/stop.go index 8cd5f27..78a159f 100644 --- a/internal/commands/service/stop.go +++ b/internal/commands/service/stop.go @@ -86,7 +86,12 @@ var stopCmd = &cobra.Command{ } if server.IsRunning() { if err := server.SignalDaemon(); err != nil { - ui.Subtle(fmt.Sprintf("Could not signal daemon: %v", err)) + // Registry updated but daemon didn't pick up the change. + // Don't print "reconciled" — the supervised process is + // still running. + ui.Fail(fmt.Sprintf("%s disabled in registry, but could not signal daemon: %v", binSvc.DisplayName(), err)) + ui.Subtle("Run `pv restart` to stop the supervised process.") + return nil } ui.Success(fmt.Sprintf("%s disabled; daemon reconciled", binSvc.DisplayName())) } else { diff --git a/internal/laravel/env.go b/internal/laravel/env.go index 180b7df..9d8c953 100644 --- a/internal/laravel/env.go +++ b/internal/laravel/env.go @@ -94,3 +94,22 @@ func UpdateProjectEnvForService(projectPath, projectName, serviceName string, sv backupPath := envPath + ".pv-backup" return services.MergeDotEnv(envPath, backupPath, allVars) } + +// UpdateProjectEnvForBinaryService mirrors UpdateProjectEnvForService for +// services that run as native binaries (implementing services.BinaryService +// rather than services.Service). The difference is the EnvVars signature: +// binary services don't take a port argument because their port is fixed +// at the struct level. +func UpdateProjectEnvForBinaryService(projectPath, projectName, serviceName string, svc services.BinaryService, bound *registry.ProjectServices) error { + envPath := filepath.Join(projectPath, ".env") + if _, err := os.Stat(envPath); os.IsNotExist(err) { + return nil + } + allVars := svc.EnvVars(projectName) + smartVars := SmartEnvVars(bound) + for k, v := range smartVars { + allVars[k] = v + } + backupPath := envPath + ".pv-backup" + return services.MergeDotEnv(envPath, backupPath, allVars) +} diff --git a/internal/laravel/env_test.go b/internal/laravel/env_test.go index f9675b4..c447a74 100644 --- a/internal/laravel/env_test.go +++ b/internal/laravel/env_test.go @@ -269,3 +269,47 @@ func TestUpdateProjectEnvForService_NoEnvFile(t *testing.T) { t.Fatalf("UpdateProjectEnvForService should not error for missing .env: %v", err) } } + +func TestUpdateProjectEnvForBinaryService(t *testing.T) { + dir := t.TempDir() + envPath := filepath.Join(dir, ".env") + if err := os.WriteFile(envPath, []byte("APP_NAME=test\n"), 0644); err != nil { + t.Fatal(err) + } + + svc, ok := services.LookupBinary("s3") + if !ok { + t.Fatal("s3 binary service not registered") + } + bound := ®istry.ProjectServices{S3: true} + + if err := UpdateProjectEnvForBinaryService(dir, "my-app", "s3", svc, bound); err != nil { + t.Fatalf("UpdateProjectEnvForBinaryService: %v", err) + } + + env, _ := services.ReadDotEnv(envPath) + // Connection vars from RustFS.EnvVars + if env["AWS_BUCKET"] != "my-app" { + t.Errorf("AWS_BUCKET = %q, want my-app", env["AWS_BUCKET"]) + } + if env["AWS_ENDPOINT"] != "http://127.0.0.1:9000" { + t.Errorf("AWS_ENDPOINT = %q, want http://127.0.0.1:9000", env["AWS_ENDPOINT"]) + } + // Smart var from SmartEnvVars — only added when project is bound to s3. + if env["FILESYSTEM_DISK"] != "s3" { + t.Errorf("FILESYSTEM_DISK = %q, want s3", env["FILESYSTEM_DISK"]) + } +} + +func TestUpdateProjectEnvForBinaryService_NoEnvFile(t *testing.T) { + dir := t.TempDir() + svc, ok := services.LookupBinary("s3") + if !ok { + t.Fatal("s3 binary service not registered") + } + bound := ®istry.ProjectServices{S3: true} + + if err := UpdateProjectEnvForBinaryService(dir, "my-app", "s3", svc, bound); err != nil { + t.Fatalf("should not error for missing .env: %v", err) + } +} diff --git a/internal/server/binary_service.go b/internal/server/binary_service.go index 0cfa013..3936425 100644 --- a/internal/server/binary_service.go +++ b/internal/server/binary_service.go @@ -50,7 +50,10 @@ func buildSupervisorProcess(svc services.BinaryService) (supervisor.Process, err } rc := svc.ReadyCheck() - ready := buildReadyFunc(rc) + ready, err := buildReadyFunc(rc) + if err != nil { + return supervisor.Process{}, fmt.Errorf("%s: %w", svc.Name(), err) + } return supervisor.Process{ Name: binaryName, @@ -64,9 +67,17 @@ func buildSupervisorProcess(svc services.BinaryService) (supervisor.Process, err } // buildReadyFunc returns a ReadyFunc appropriate to the ReadyCheck variant. -func buildReadyFunc(rc services.ReadyCheck) func(context.Context) error { +// The ReadyCheck must specify exactly one of TCPPort or HTTPEndpoint — the +// doc comment on the type promises this; we enforce it here rather than +// silently treating a zero-value as "instantly ready" (which would let a +// misconfigured BinaryService bypass the probe entirely). +func buildReadyFunc(rc services.ReadyCheck) (func(context.Context) error, error) { + httpSet := rc.HTTPEndpoint != "" + tcpSet := rc.TCPPort > 0 switch { - case rc.HTTPEndpoint != "": + case httpSet && tcpSet: + return nil, fmt.Errorf("invalid ReadyCheck: both TCPPort and HTTPEndpoint set; specify exactly one") + case httpSet: client := &http.Client{Timeout: 2 * time.Second} url := rc.HTTPEndpoint return func(ctx context.Context) error { @@ -83,8 +94,8 @@ func buildReadyFunc(rc services.ReadyCheck) func(context.Context) error { return nil } return fmt.Errorf("HTTP %s returned %d", url, resp.StatusCode) - } - case rc.TCPPort > 0: + }, nil + case tcpSet: addr := fmt.Sprintf("127.0.0.1:%d", rc.TCPPort) return func(ctx context.Context) error { d := net.Dialer{Timeout: 500 * time.Millisecond} @@ -94,10 +105,9 @@ func buildReadyFunc(rc services.ReadyCheck) func(context.Context) error { } c.Close() return nil - } + }, nil default: - // Degenerate case: no probe specified. Treat as instantly ready. - return func(context.Context) error { return nil } + return nil, fmt.Errorf("invalid ReadyCheck: must set exactly one of TCPPort or HTTPEndpoint") } } diff --git a/internal/server/binary_service_test.go b/internal/server/binary_service_test.go index e6a8d28..5e3e239 100644 --- a/internal/server/binary_service_test.go +++ b/internal/server/binary_service_test.go @@ -47,6 +47,75 @@ func TestBuildSupervisorProcess_RustFS(t *testing.T) { if p.ReadyTimeout == 0 { t.Error("ReadyTimeout must be non-zero") } + + // The full command line must include --console-enable. Without it, RustFS + // does not bind port 9001 even though --console-address is set — this is + // the exact regression we verified during Task 1. Assert it at the + // supervisor-process layer, not just at the service layer. + var sawConsoleEnable, sawConsoleAddress, sawAddress bool + for i, a := range p.Args { + switch a { + case "--console-enable": + sawConsoleEnable = true + case "--console-address": + if i+1 < len(p.Args) && p.Args[i+1] == ":9001" { + sawConsoleAddress = true + } + case "--address": + if i+1 < len(p.Args) && p.Args[i+1] == ":9000" { + sawAddress = true + } + } + } + if !sawConsoleEnable { + t.Errorf("Args missing --console-enable; got %v", p.Args) + } + if !sawConsoleAddress { + t.Errorf("Args missing --console-address :9001; got %v", p.Args) + } + if !sawAddress { + t.Errorf("Args missing --address :9000; got %v", p.Args) + } +} + +func TestBuildReadyFunc_RejectsZeroValue(t *testing.T) { + _, err := buildReadyFunc(services.ReadyCheck{}) + if err == nil { + t.Fatal("expected error for zero-value ReadyCheck") + } + if !strings.Contains(err.Error(), "exactly one") { + t.Errorf("error should mention 'exactly one'; got %v", err) + } +} + +func TestBuildReadyFunc_RejectsBothSet(t *testing.T) { + _, err := buildReadyFunc(services.ReadyCheck{ + TCPPort: 9000, + HTTPEndpoint: "http://127.0.0.1:9000/health", + }) + if err == nil { + t.Fatal("expected error when both TCPPort and HTTPEndpoint are set") + } +} + +func TestBuildReadyFunc_TCPOnly(t *testing.T) { + fn, err := buildReadyFunc(services.ReadyCheck{TCPPort: 9000}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if fn == nil { + t.Fatal("expected non-nil ready func") + } +} + +func TestBuildReadyFunc_HTTPOnly(t *testing.T) { + fn, err := buildReadyFunc(services.ReadyCheck{HTTPEndpoint: "http://127.0.0.1:9000/health"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if fn == nil { + t.Fatal("expected non-nil ready func") + } } func TestWriteDaemonStatus_RoundTrip(t *testing.T) { diff --git a/internal/server/manager.go b/internal/server/manager.go index 40ad642..9cfe97f 100644 --- a/internal/server/manager.go +++ b/internal/server/manager.go @@ -106,10 +106,12 @@ func (m *ServerManager) Reconcile() error { return fmt.Errorf("reconcile: reload main FrankenPHP: %w", err) } - // Phase 2: binary services. - if err := m.reconcileBinaryServices(context.Background()); err != nil { - fmt.Fprintf(os.Stderr, "Reconcile: %v\n", err) - // non-fatal — FrankenPHP-side reconcile results still returned below + // Phase 2: binary services. Errors are logged and also folded into the + // return value so callers (service:add, etc.) can see that a binary + // failed to come up rather than getting a false "reconciled" signal. + binaryErr := m.reconcileBinaryServices(context.Background()) + if binaryErr != nil { + fmt.Fprintf(os.Stderr, "Reconcile: %v\n", binaryErr) } // Phase 3: refresh daemon-status snapshot. @@ -117,8 +119,16 @@ func (m *ServerManager) Reconcile() error { fmt.Fprintf(os.Stderr, "Reconcile: write daemon-status: %v\n", err) } + // Combine secondary-instance and binary-service errors for the caller. + var parts []string if len(startErrors) > 0 { - return fmt.Errorf("reconcile: %d secondary instance(s) failed: %s", len(startErrors), strings.Join(startErrors, "; ")) + parts = append(parts, fmt.Sprintf("secondary instances: %s", strings.Join(startErrors, "; "))) + } + if binaryErr != nil { + parts = append(parts, fmt.Sprintf("binary services: %v", binaryErr)) + } + if len(parts) > 0 { + return fmt.Errorf("reconcile failed: %s", strings.Join(parts, "; ")) } return nil } diff --git a/internal/server/manager_test.go b/internal/server/manager_test.go index 8e4f890..e48b786 100644 --- a/internal/server/manager_test.go +++ b/internal/server/manager_test.go @@ -3,7 +3,10 @@ package server import ( "context" "os" + "os/exec" "path/filepath" + "runtime" + "sync" "testing" "time" @@ -12,7 +15,66 @@ import ( "github.com/prvious/pv/internal/supervisor" ) +// fakeBinaryBuildOnce caches the compiled fake-binary path across tests in +// this package — go build is expensive and the helper is stateless. +var ( + fakeBinaryPathOnce sync.Once + fakeBinaryPath string + fakeBinaryErr error +) + +// compiledFakeBinary compiles testdata/fakebinary/main.go once per test run +// and returns the absolute path to the resulting executable. The binary +// binds a TCP port on 127.0.0.1 and sleeps — matching what the RustFS +// supervisor expects for its TCP ready-check, without needing python3. +func compiledFakeBinary(t *testing.T) string { + t.Helper() + fakeBinaryPathOnce.Do(func() { + dir, err := os.MkdirTemp("", "pv-fake-binary-*") + if err != nil { + fakeBinaryErr = err + return + } + // Keep this around for the life of the test process; the OS will + // clean it up or leave it in the user's tmp — either is fine. + out := filepath.Join(dir, "fakebinary") + src := filepath.Join("testdata", "fakebinary", "main.go") + cmd := exec.Command("go", "build", "-o", out, src) + if output, err := cmd.CombinedOutput(); err != nil { + fakeBinaryErr = err + t.Logf("go build output: %s", output) + return + } + fakeBinaryPath = out + }) + if fakeBinaryErr != nil { + t.Fatalf("compile fake binary: %v", fakeBinaryErr) + } + return fakeBinaryPath +} + +// stageFakeBinaryAsRustfs copies the compiled fake binary into +// ~/.pv/internal/bin/rustfs so the supervisor finds it via the normal path. +func stageFakeBinaryAsRustfs(t *testing.T) { + t.Helper() + src := compiledFakeBinary(t) + binDir := config.InternalBinDir() + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(src) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(binDir, "rustfs"), data, 0o755); err != nil { + t.Fatal(err) + } +} + func TestReconcile_SpawnsBinaryServices(t *testing.T) { + if runtime.GOOS != "darwin" && runtime.GOOS != "linux" { + t.Skipf("fake binary helper not supported on %s", runtime.GOOS) + } t.Setenv("HOME", t.TempDir()) // Seed a registry with s3 as a binary service. @@ -26,17 +88,7 @@ func TestReconcile_SpawnsBinaryServices(t *testing.T) { t.Fatal(err) } - // Put a fake "rustfs" binary in place so supervisor spawn doesn't ENOENT. - binDir := config.InternalBinDir() - if err := os.MkdirAll(binDir, 0o755); err != nil { - t.Fatal(err) - } - fakeBin := filepath.Join(binDir, "rustfs") - // The fake binary must bind port 9000 so the supervisor ready-check succeeds. - script := "#!/usr/bin/env python3\nimport socket, time\ns=socket.socket()\ns.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)\ns.bind(('127.0.0.1', 9000))\ns.listen(1)\ntime.sleep(60)\n" - if err := os.WriteFile(fakeBin, []byte(script), 0o755); err != nil { - t.Fatal(err) - } + stageFakeBinaryAsRustfs(t) sup := supervisor.New() m := &ServerManager{supervisor: sup, secondaries: map[string]*FrankenPHP{}} @@ -51,16 +103,11 @@ func TestReconcile_SpawnsBinaryServices(t *testing.T) { } func TestReconcile_StopsDisabledBinaryServices(t *testing.T) { - t.Setenv("HOME", t.TempDir()) - binDir := config.InternalBinDir() - if err := os.MkdirAll(binDir, 0o755); err != nil { - t.Fatal(err) - } - // The fake binary must bind port 9000 so the supervisor ready-check succeeds. - script := "#!/usr/bin/env python3\nimport socket, time\ns=socket.socket()\ns.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)\ns.bind(('127.0.0.1', 9000))\ns.listen(1)\ntime.sleep(60)\n" - if err := os.WriteFile(filepath.Join(binDir, "rustfs"), []byte(script), 0o755); err != nil { - t.Fatal(err) + if runtime.GOOS != "darwin" && runtime.GOOS != "linux" { + t.Skipf("fake binary helper not supported on %s", runtime.GOOS) } + t.Setenv("HOME", t.TempDir()) + stageFakeBinaryAsRustfs(t) sup := supervisor.New() m := &ServerManager{supervisor: sup, secondaries: map[string]*FrankenPHP{}} diff --git a/internal/server/testdata/fakebinary/main.go b/internal/server/testdata/fakebinary/main.go new file mode 100644 index 0000000..bb35db1 --- /dev/null +++ b/internal/server/testdata/fakebinary/main.go @@ -0,0 +1,42 @@ +// Package main is a test-only fake binary compiled by manager_test.go. +// It binds 127.0.0.1: (default 9000) so the supervisor's TCP +// ready-check succeeds, then sleeps indefinitely until killed. +// +// This exists to remove a hidden dependency on python3 from the test suite. +package main + +import ( + "flag" + "fmt" + "net" + "os" + "time" +) + +func main() { + port := flag.Int("port", 9000, "port to bind on 127.0.0.1") + flag.Parse() + + ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", *port)) + if err != nil { + fmt.Fprintf(os.Stderr, "fakebinary: listen %d: %v\n", *port, err) + os.Exit(1) + } + defer ln.Close() + + // Accept+close connections forever so the listener is live for the + // entire test, but never block waiting for a specific client. + go func() { + for { + c, err := ln.Accept() + if err != nil { + return + } + _ = c.Close() + } + }() + + // Stay alive until the supervisor kills us. An hour is plenty of time + // for any test that uses this helper. + time.Sleep(1 * time.Hour) +} diff --git a/internal/services/binary.go b/internal/services/binary.go index ccd2770..1d78782 100644 --- a/internal/services/binary.go +++ b/internal/services/binary.go @@ -8,7 +8,10 @@ import ( // ReadyCheck describes how a supervisor verifies that a binary service has // finished starting and is ready to accept requests. Exactly one of TCPPort -// or HTTPEndpoint must be set. +// or HTTPEndpoint must be set — a zero value or both-set configuration is +// rejected by the supervisor wiring (see internal/server/binary_service.go) +// so a misconfigured service fails loudly at start time instead of silently +// skipping the probe. type ReadyCheck struct { TCPPort int // probe 127.0.0.1:port until Dial succeeds HTTPEndpoint string // GET this URL, expect a 2xx response diff --git a/internal/supervisor/supervisor.go b/internal/supervisor/supervisor.go index ceb66f7..939ee4c 100644 --- a/internal/supervisor/supervisor.go +++ b/internal/supervisor/supervisor.go @@ -223,6 +223,19 @@ func (s *Supervisor) watch(ctx context.Context, name string) { return } s.mu.Lock() + // Stop may have been called between the spawn above (outside the + // lock) and re-taking the lock here. If so, the freshly spawned + // process is unwanted — kill it, drop it on the floor, and release + // Stop's waiter via close(currentDone). Without this check, newM is + // installed into the map just as Stop deletes the entry, orphaning + // the live child with no supervisor and no kill path. + if m.stopped { + s.mu.Unlock() + _ = newM.cmd.Process.Kill() + _ = newM.cmd.Wait() // reap the zombie we just killed + close(currentDone) + return + } newM.stopped = m.stopped newM.cancel = m.cancel newM.restarts = m.restarts diff --git a/internal/supervisor/supervisor_test.go b/internal/supervisor/supervisor_test.go index 35e6020..8eddb9f 100644 --- a/internal/supervisor/supervisor_test.go +++ b/internal/supervisor/supervisor_test.go @@ -145,6 +145,77 @@ func TestSupervisor_TCPReadyCheck(t *testing.T) { } } +func TestSupervisor_RestartsCrashedProcess(t *testing.T) { + s := New() + // A process that writes its pid to a file and exits with non-zero. + // After crash the supervisor should respawn it — we detect respawn by + // observing a *different* pid appear in the file. + dir := t.TempDir() + pidFile := filepath.Join(dir, "pid") + script := "echo $$ > " + pidFile + "; exit 1" + p := newTestProcess(t, "crasher", script) + p.Ready = func(ctx context.Context) error { return nil } + if err := s.Start(context.Background(), p); err != nil { + t.Fatalf("Start: %v", err) + } + defer s.Stop("crasher", 2*time.Second) + + // Wait for the first pid, then for a different pid (proof of respawn). + firstPid := waitForPidFile(t, pidFile, 3*time.Second, "") + respawnDeadline := time.Now().Add(10 * time.Second) // 2s sleep between restarts + for time.Now().Before(respawnDeadline) { + nextPid := waitForPidFile(t, pidFile, 1*time.Second, firstPid) + if nextPid != "" && nextPid != firstPid { + return // success: respawn observed + } + time.Sleep(200 * time.Millisecond) + } + t.Fatalf("process was not respawned within 10s; first pid=%s", firstPid) +} + +func TestSupervisor_GivesUpAfterBudget(t *testing.T) { + if testing.Short() { + t.Skip("slow: exercises 5×2s restart budget") + } + s := New() + // Immediately-exiting process. Supervisor respawns, each crash records + // a restart timestamp; the 5th crash triggers budget exhaustion. + p := newTestProcess(t, "fast-crasher", "exit 1") + p.Ready = func(ctx context.Context) error { return nil } + if err := s.Start(context.Background(), p); err != nil { + t.Fatalf("Start: %v", err) + } + // The budget is 5 restarts within 60s with a 2s sleep between each. + // After ~5 iterations (~10s) the supervisor should give up and delete + // the process from its tracking map. + deadline := time.Now().Add(15 * time.Second) + for time.Now().Before(deadline) { + if !s.IsRunning("fast-crasher") && len(s.SupervisedNames()) == 0 { + return // success + } + time.Sleep(250 * time.Millisecond) + } + t.Errorf("supervisor did not give up within 15s; SupervisedNames=%v", s.SupervisedNames()) +} + +// waitForPidFile polls the file until it contains a pid that differs from +// excluded (or any pid if excluded is ""). Returns empty string on timeout. +func waitForPidFile(t *testing.T, path string, timeout time.Duration, excluded string) string { + t.Helper() + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + data, err := os.ReadFile(path) + if err == nil { + pid := strings.TrimSpace(string(data)) + if pid != "" && pid != excluded { + return pid + } + } + time.Sleep(50 * time.Millisecond) + } + return "" +} + func TestSupervisor_LogFileIsWritten(t *testing.T) { s := New() p := newTestProcess(t, "logger", "echo hello-from-supervisor; sleep 30") From 7a7c87f2451dd96947cbb79a88964eb53267e7dd Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Tue, 14 Apr 2026 12:02:25 -0400 Subject: [PATCH 21/21] wip --- internal/server/manager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/server/manager_test.go b/internal/server/manager_test.go index e48b786..3141f0f 100644 --- a/internal/server/manager_test.go +++ b/internal/server/manager_test.go @@ -26,7 +26,7 @@ var ( // compiledFakeBinary compiles testdata/fakebinary/main.go once per test run // and returns the absolute path to the resulting executable. The binary // binds a TCP port on 127.0.0.1 and sleeps — matching what the RustFS -// supervisor expects for its TCP ready-check, without needing python3. +// supervisor expects for its TCP ready-check. func compiledFakeBinary(t *testing.T) string { t.Helper() fakeBinaryPathOnce.Do(func() {