diff --git a/bgworker.go b/bgworker.go new file mode 100644 index 0000000000..1254e5ab1a --- /dev/null +++ b/bgworker.go @@ -0,0 +1,116 @@ +package frankenphp + +import ( + "fmt" + "strings" + "sync" + "sync/atomic" +) + +// Scope isolates background workers between php_server blocks; the +// zero value is the global/embed scope. Obtain values via NextScope. +type Scope uint64 + +var scopeCounter atomic.Uint64 + +// NextScope returns a fresh scope value. Each php_server block should +// call this once during provisioning. +func NextScope() Scope { + return Scope(scopeCounter.Add(1)) +} + +// scopeLabels maps Scope -> human-readable label registered by the +// embedder (e.g. the Caddy module). +var scopeLabels sync.Map + +// SetScopeLabel attaches a human-readable label to a scope; the bg-worker +// metric emitter renders it as e.g. server="api.example.com" instead of +// an opaque numeric id. Empty labels are ignored. +func SetScopeLabel(s Scope, label string) { + if label == "" { + return + } + scopeLabels.Store(s, label) +} + +// lookupScopeLabel reports whether a label has been registered for s, +// returning ("", false) when none has. Distinguishes "unset" from +// "explicitly empty" without the numeric fallback. +func lookupScopeLabel(s Scope) (string, bool) { + v, ok := scopeLabels.Load(s) + if !ok { + return "", false + } + return v.(string), true +} + +// bgWorkerMetricName formats the metric label for a background worker: +// "m#:". scopeLabel is empty when the scope +// has no registered label (embed/global, or before the embedder calls +// SetScopeLabel). The "m#" prefix mirrors the m# convention used for +// module workers; the colon keeps the format uniform so a single regex +// (m#([^:]*):(.+)) parses both labelled and unlabelled forms. +func bgWorkerMetricName(scope Scope, runtimeName string) string { + label, _ := lookupScopeLabel(scope) + return "m#" + label + ":" + runtimeName +} + +// backgroundLookups maps scope -> name -> *worker. Scope 0 is the +// global/embed scope. nil when no background worker is declared. +var backgroundLookups map[Scope]map[string]*worker + +// buildBackgroundWorkerLookups maps each declared bg worker into its scope's +// lookup. Per-scope name collisions are caught here because bg workers +// intentionally skip the global workersByName map (so two scopes can share +// a user-facing name). Names are not allowed to be empty in this minimal +// build; catch-all bg workers are deferred to a follow-up PR. +func buildBackgroundWorkerLookups(workers []*worker, opts []workerOpt) (map[Scope]map[string]*worker, error) { + lookups := make(map[Scope]map[string]*worker) + + for i, o := range opts { + if !o.isBackgroundWorker { + continue + } + w := workers[i] + w.scope = o.scope + + phpName := strings.TrimPrefix(w.name, "m#") + if phpName == "" || phpName == w.fileName { + return nil, fmt.Errorf("background worker must have an explicit name (got %q)", w.name) + } + + byName := lookups[o.scope] + if byName == nil { + byName = make(map[string]*worker) + lookups[o.scope] = byName + } + if _, exists := byName[phpName]; exists { + return nil, fmt.Errorf("duplicate background worker name %q in the same scope", phpName) + } + byName[phpName] = w + } + + if len(lookups) == 0 { + return nil, nil + } + return lookups, nil +} + +// reserveBackgroundWorkerThreads returns the thread budget to add to the +// pool for declared bg workers, and pre-registers totalWorkers so a bg-only +// deployment has the metric initialised. num must be >= 1 for bg workers +// in this build. +func reserveBackgroundWorkerThreads(opt *opt) (int, error) { + reserved := 0 + for _, w := range opt.workers { + if !w.isBackgroundWorker { + continue + } + if w.num < 1 { + return 0, fmt.Errorf("background worker %q must declare num >= 1 (lazy/ensure() machinery is not in this build)", w.name) + } + reserved += w.num + metrics.TotalWorkers(bgWorkerMetricName(w.scope, w.name), w.num) + } + return reserved, nil +} diff --git a/bgworker_test.go b/bgworker_test.go new file mode 100644 index 0000000000..5ef6ec6e42 --- /dev/null +++ b/bgworker_test.go @@ -0,0 +1,86 @@ +package frankenphp_test + +import ( + "path/filepath" + "testing" + "time" + + "github.com/dunglas/frankenphp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestBackgroundWorkerLifecycle boots a background worker that touches a +// sentinel file then parks on the stop pipe. It proves the bg worker runs +// (sentinel appears) and that Shutdown returns within a reasonable time. +func TestBackgroundWorkerLifecycle(t *testing.T) { + tmp := t.TempDir() + sentinel := filepath.Join(tmp, "bg-lifecycle.sentinel") + + require.NoError(t, frankenphp.Init( + frankenphp.WithWorkers("bg-lifecycle", "testdata/bgworker/basic.php", 1, + frankenphp.WithWorkerBackground(), + frankenphp.WithWorkerEnv(map[string]string{"BG_SENTINEL": sentinel}), + ), + frankenphp.WithNumThreads(2), + )) + // Note: this test asserts on Shutdown timing, so it manages Shutdown + // itself instead of using setupFrankenPHP's t.Cleanup hook. + + requireFileEventually(t, sentinel, "background worker did not touch sentinel") + + done := make(chan struct{}) + go func() { + frankenphp.Shutdown() + close(done) + }() + + select { + case <-done: + case <-time.After(10 * time.Second): + t.Fatalf("Shutdown did not return within 10s") + } +} + +// TestBackgroundWorkerCrashRestarts boots a worker that exit(1)s on its +// first run and touches a "restarted" sentinel on its second run. The +// sentinel proves the crash-restart loop fired. +func TestBackgroundWorkerCrashRestarts(t *testing.T) { + tmp := t.TempDir() + crashMarker := filepath.Join(tmp, "bg-crash.marker") + restarted := filepath.Join(tmp, "bg-crash.restarted") + + setupFrankenPHP(t, + frankenphp.WithWorkers("bg-crash", "testdata/bgworker/crash.php", 1, + frankenphp.WithWorkerBackground(), + frankenphp.WithWorkerEnv(map[string]string{ + "BG_CRASH_MARKER": crashMarker, + "BG_RESTARTED_SENTINEL": restarted, + }), + ), + frankenphp.WithNumThreads(2), + ) + + requireFileEventually(t, restarted, "background worker did not restart after crash") +} + +// TestBackgroundWorkerWithoutHTTP confirms that a request to a script +// unrelated to the bg worker still works: the bg worker doesn't intercept +// HTTP traffic. +func TestBackgroundWorkerWithoutHTTP(t *testing.T) { + tmp := t.TempDir() + sentinel := filepath.Join(tmp, "bg-nohttp.sentinel") + + testDataDir := setupFrankenPHP(t, + frankenphp.WithWorkers("bg-nohttp", "testdata/bgworker/basic.php", 1, + frankenphp.WithWorkerBackground(), + frankenphp.WithWorkerEnv(map[string]string{"BG_SENTINEL": sentinel}), + ), + frankenphp.WithNumThreads(2), + ) + + requireFileEventually(t, sentinel, "background worker did not touch sentinel") + + body := serveBody(t, testDataDir, "index.php") + assert.NotEmpty(t, body, "expected non-empty body from index.php") +} diff --git a/bgworkerhelpers_test.go b/bgworkerhelpers_test.go new file mode 100644 index 0000000000..a51a0042f8 --- /dev/null +++ b/bgworkerhelpers_test.go @@ -0,0 +1,57 @@ +package frankenphp_test + +import ( + "io" + "net/http/httptest" + "os" + "testing" + "time" + + "github.com/dunglas/frankenphp" + "github.com/stretchr/testify/require" +) + +// requireFileEventually asserts that `path` appears on disk before the +// deadline. Wraps require.Eventually so call sites stay short. +func requireFileEventually(t testing.TB, path string, msgAndArgs ...any) { + t.Helper() + require.Eventually(t, func() bool { + _, err := os.Stat(path) + return err == nil + }, 5*time.Second, 25*time.Millisecond, msgAndArgs...) +} + +// setupFrankenPHP boots FrankenPHP with the given options, registers +// Shutdown as a t.Cleanup, and returns the absolute path to the testdata +// directory. Saves the boilerplate every bg-worker test repeats. +func setupFrankenPHP(t *testing.T, opts ...frankenphp.Option) (testDataDir string) { + t.Helper() + cwd, err := os.Getwd() + require.NoError(t, err) + testDataDir = cwd + "/testdata/" + require.NoError(t, frankenphp.Init(opts...)) + t.Cleanup(frankenphp.Shutdown) + return +} + +// serveBody runs `script` (relative to testDataDir, may include a query +// string) through FrankenPHP and returns the response body. ErrRejected is +// treated as a non-fatal outcome so worker-mode quirks don't fail tests +// that only care about the script's stdout. +func serveBody(t *testing.T, testDataDir, scriptAndQuery string, opts ...frankenphp.RequestOption) string { + t.Helper() + req := httptest.NewRequest("GET", "http://example.com/"+scriptAndQuery, nil) + reqOpts := append([]frankenphp.RequestOption{ + frankenphp.WithRequestDocumentRoot(testDataDir, false), + }, opts...) + fr, err := frankenphp.NewRequestWithContext(req, reqOpts...) + require.NoError(t, err) + + w := httptest.NewRecorder() + if err := frankenphp.ServeHTTP(w, fr); err != nil { + require.ErrorAs(t, err, &frankenphp.ErrRejected{}) + } + body, err := io.ReadAll(w.Result().Body) + require.NoError(t, err) + return string(body) +} diff --git a/bgworkerscope_test.go b/bgworkerscope_test.go new file mode 100644 index 0000000000..844d9de787 --- /dev/null +++ b/bgworkerscope_test.go @@ -0,0 +1,89 @@ +package frankenphp + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// setupBgWorker boots FrankenPHP with the given options (internal-package +// variant), registers Shutdown as a t.Cleanup, and returns the absolute +// path to the testdata directory. +func setupBgWorker(t *testing.T, opts ...Option) (testDataDir string) { + t.Helper() + cwd, err := os.Getwd() + require.NoError(t, err) + testDataDir = cwd + "/testdata/" + require.NoError(t, Init(opts...)) + t.Cleanup(Shutdown) + return +} + +// requireSentinelEventually asserts that `path` appears on disk before the +// deadline. Wraps require.Eventually so call sites stay short. +func requireSentinelEventually(t testing.TB, path string, msgAndArgs ...any) { + t.Helper() + require.Eventually(t, func() bool { + _, err := os.Stat(path) + return err == nil + }, 5*time.Second, 10*time.Millisecond, msgAndArgs...) +} + +// TestNextScopeIsDistinct verifies the scope counter +// hands out unique values on consecutive calls. +func TestNextScopeIsDistinct(t *testing.T) { + a := NextScope() + b := NextScope() + assert.NotEqual(t, a, b, "consecutive scopes must differ") + assert.NotZero(t, a, "scopes must be non-zero (zero is the global scope)") + assert.NotZero(t, b, "scopes must be non-zero (zero is the global scope)") +} + +// TestBackgroundWorkerSameNameDifferentScope declares two named bg +// workers with the same user-facing name in distinct scopes. Both must +// Init successfully (the global workersByName collision check must +// recognize bg workers as scope-isolated), each must produce its own +// sentinel under its scope-specific directory. +func TestBackgroundWorkerSameNameDifferentScope(t *testing.T) { + scopeA := NextScope() + scopeB := NextScope() + + tmp := t.TempDir() + dirA := filepath.Join(tmp, "a") + dirB := filepath.Join(tmp, "b") + require.NoError(t, os.MkdirAll(dirA, 0o755)) + require.NoError(t, os.MkdirAll(dirB, 0o755)) + + setupBgWorker(t, + WithWorkers("shared", "testdata/bgworker/named.php", 1, + WithWorkerBackground(), + WithWorkerScope(scopeA), + WithWorkerEnv(map[string]string{"BG_SENTINEL_DIR": dirA}), + ), + WithWorkers("shared", "testdata/bgworker/named.php", 1, + WithWorkerBackground(), + WithWorkerScope(scopeB), + WithWorkerEnv(map[string]string{"BG_SENTINEL_DIR": dirB}), + ), + WithNumThreads(4), + ) + + // Both lookups must exist and resolve "shared" to a *worker. + require.NotNil(t, backgroundLookups[scopeA], "scope A lookup missing") + require.NotNil(t, backgroundLookups[scopeB], "scope B lookup missing") + assert.NotNil(t, backgroundLookups[scopeA]["shared"], "scope A must resolve 'shared'") + assert.NotNil(t, backgroundLookups[scopeB]["shared"], "scope B must resolve 'shared'") + // And they must be distinct *worker instances (not the same pointer). + assert.NotSame(t, + backgroundLookups[scopeA]["shared"], + backgroundLookups[scopeB]["shared"], + "each scope must own a distinct *worker for the same name") + + // Each scope's worker writes its sentinel under its own dir. + requireSentinelEventually(t, filepath.Join(dirA, "shared"), "scope A worker did not touch sentinel") + requireSentinelEventually(t, filepath.Join(dirB, "shared"), "scope B worker did not touch sentinel") +} diff --git a/caddy/app.go b/caddy/app.go index f10bf965bf..8724be6dda 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -1,7 +1,6 @@ package caddy import ( - "context" "errors" "fmt" "log/slog" @@ -62,17 +61,24 @@ type FrankenPHPApp struct { opts []frankenphp.Option metrics frankenphp.Metrics - ctx context.Context + ctx caddy.Context logger *slog.Logger + + // scopeOwners is the per-php_server module registry used by Start + // to resolve a human-readable scope label (via the cascade in + // scopelabel.go) before frankenphp.Init starts any bg worker, so + // the very first metric call already carries the right prefix. + scopeOwnersMu sync.Mutex + scopeOwners map[frankenphp.Scope]*FrankenPHPModule } var errIni = errors.New(`"php_ini" must be in the format: php_ini "" ""`) // CaddyModule returns the Caddy module information. -func (f FrankenPHPApp) CaddyModule() caddy.ModuleInfo { +func (*FrankenPHPApp) CaddyModule() caddy.ModuleInfo { return caddy.ModuleInfo{ ID: "frankenphp", - New: func() caddy.Module { return &f }, + New: func() caddy.Module { return new(FrankenPHPApp) }, } } @@ -139,6 +145,41 @@ func (f *FrankenPHPApp) addModuleWorkers(workers ...workerConfig) ([]workerConfi return workers, nil } +// registerScopeOwner records the FrankenPHPModule that allocated the +// given scope so Start() can resolve its label before frankenphp.Init. +func (f *FrankenPHPApp) registerScopeOwner(scope frankenphp.Scope, mod *FrankenPHPModule) { + f.scopeOwnersMu.Lock() + defer f.scopeOwnersMu.Unlock() + if f.scopeOwners == nil { + f.scopeOwners = make(map[frankenphp.Scope]*FrankenPHPModule) + } + f.scopeOwners[scope] = mod +} + +// resolveScopeLabels walks the http app's servers once per registered +// module, picks the one whose route tree contains the module, runs the +// scopelabel.go cascade, and stores the result via SetScopeLabel. Runs +// before frankenphp.Init so the first metric emit on every bg worker +// already carries the right "m#