From 72fb7c3e6205831ab86d02ab9cde0dd43f6a8383 Mon Sep 17 00:00:00 2001 From: Rafael Matias Date: Thu, 2 Jul 2026 09:01:28 +0200 Subject: [PATCH] feat(builder): auto-mount + promote schelk datadirs on state-actor build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a builder.state_actor target's output_dir lives under a schelk mount, `benchmarkoor build` now: 1. ensures the schelk scratch is mounted before it inspects or writes the datadir (so the skip/diff checks read the mounted content, not an empty mount point), and 2. runs `schelk promote` after a build that actually ran, persisting the new datadir as the virgin baseline. A skipped, unchanged target is not promoted; a failed promote is fatal (a stale baseline would let a later `schelk recover` silently revert). Detection is automatic from the output_dir path — no config needed, and non-schelk builds are untouched. New datadir helpers: SchelkDir, EnsureSchelkMounted, SchelkPromote. The runner's schelk mount-preflight is refactored to share EnsureSchelkMounted (removing the duplicated logic while preserving its errors and `mount -y`). --- docs/configuration.md | 2 + pkg/builder/state_actor.go | 31 ++++++++++++++ pkg/config/config.go | 35 +++------------- pkg/datadir/schelk.go | 82 ++++++++++++++++++++++++++++++++++++++ pkg/datadir/schelk_test.go | 66 ++++++++++++++++++++++++++++++ 5 files changed, 186 insertions(+), 30 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 09b84cfa..a332364d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1119,6 +1119,8 @@ What benchmarkoor does at runtime: - **Per container lifecycle**, `Prepare` runs `schelk restore` (recover + mount) so each iteration starts from the virgin baseline. `Cleanup` runs `schelk recover` to unmount and restore baseline. - **Graceful shutdown**: schelk commands run in their own process group, so a SIGTERM to benchmarkoor does not propagate to schelk. An in-flight schelk command is given up to 60 seconds to finish before being killed, so a recover mid-flight is not interrupted. +**Building onto a schelk mount:** when a `builder.state_actor` target's `output_dir` is under the schelk mount, `benchmarkoor build` mounts the scratch first (the same `schelk mount` preflight as above), materialises the datadir onto it, and — only when a build actually ran (fresh / `--force` / a `--rebuild-on-diff` change) — runs `schelk promote` to persist the new datadir as the virgin baseline. A skipped, unchanged target is not promoted. This is how the built datadir becomes the baseline the runner's per-iteration `schelk restore` resets to. No configuration is needed — it is detected from the output_dir path. + Notes: - `rollback_strategy: container-checkpoint-restore` is **not** compatible with `method: schelk` (it requires `method: zfs`). - All operational schelk commands require root. diff --git a/pkg/builder/state_actor.go b/pkg/builder/state_actor.go index 0a04ab71..af405477 100644 --- a/pkg/builder/state_actor.go +++ b/pkg/builder/state_actor.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/ethpandaops/benchmarkoor/pkg/config" + "github.com/ethpandaops/benchmarkoor/pkg/datadir" "github.com/ethpandaops/benchmarkoor/pkg/docker" "github.com/sirupsen/logrus" ) @@ -92,6 +93,23 @@ func (b *StateActorBuilder) Build(ctx context.Context, name string, opts BuildOp // "Running state-actor" line, so they don't repeat on every streamed line. log := b.log.WithField("target", target.EffectiveName()) + // When output_dir lives under a schelk mount, make sure the scratch is + // mounted before anything inspects or writes it: the datadir is materialised + // onto the mount, and the skip/diff checks below must read the mounted + // content, not an empty mount point. Non-schelk output_dirs are untouched. + schelkMount, isSchelk, err := datadir.SchelkDir(target.OutputDir) + if err != nil { + return false, fmt.Errorf("checking schelk state for %q: %w", target.OutputDir, err) + } + + if isSchelk { + log.WithField("mount_point", schelkMount).Info("output_dir is under a schelk mount; ensuring mounted") + + if err := datadir.EnsureSchelkMounted(ctx, log); err != nil { + return false, fmt.Errorf("ensuring schelk mount: %w", err) + } + } + // The CLI `--force` flag and per-target `force: true` both bypass the // skip-on-populated check, wipe the existing output_dir, and forward // `--force` to state-actor. @@ -214,6 +232,19 @@ func (b *StateActorBuilder) Build(ctx context.Context, name string, opts BuildOp log.WithError(err).Warn("Failed to write build fingerprint sidecar") } + // The datadir just changed (a build ran), so persist it as the new schelk + // baseline. This reaches here only when a build actually happened — the skip + // paths return earlier — so an unchanged, skipped target is never promoted. + // Failure is fatal: leaving the mount ahead of the baseline would let a + // later `schelk recover` silently revert to the stale datadir. + if isSchelk { + log.Info("Persisting datadir as the new schelk baseline (`schelk promote`)") + + if err := datadir.SchelkPromote(ctx, log); err != nil { + return false, fmt.Errorf("schelk promote: %w", err) + } + } + log.Info("Build completed") return false, nil diff --git a/pkg/config/config.go b/pkg/config/config.go index 1ca18f02..782f4a72 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -3078,37 +3078,12 @@ func (c *Config) validateDataDirMethods(opt ValidateOpts) error { return fmt.Errorf("datadir.method \"schelk\": schelk state has no mount_point — run `schelk init-new` or `schelk init-from` first") } - mountedActual, err := datadir.IsMountedAt(state.MountPoint) - if err != nil { - return fmt.Errorf("datadir.method \"schelk\": checking mount status: %w", err) - } - - switch { - case state.IsMounted && !mountedActual: - // schelk state says mounted but the kernel disagrees — typically - // a crash or interrupted recover left dm-era and state out of sync. - // schelk mount will refuse ("Volume is already mounted") and only - // `schelk full-recover` can re-establish a consistent baseline. - return fmt.Errorf( - "datadir.method \"schelk\": state at %q says is_mounted=true but %q is not in /proc/mounts "+ - "(inconsistent, likely from a prior crash) — run `%s full-recover` to reset, then retry", - datadir.SchelkStatePath(), state.MountPoint, bin, - ) - case !state.IsMounted && !mountedActual: - // Use datadir.RunSchelk so a SIGTERM during validation does not - // kill the in-flight `schelk mount` mid-operation. - output, runErr := datadir.RunSchelk(context.Background(), nil, "mount", "-y") - if runErr != nil { - return fmt.Errorf( - "datadir.method \"schelk\": `%s mount` failed: %w (output: %s)", - bin, runErr, strings.TrimSpace(string(output)), - ) - } + // Ensure the scratch is mounted (shared with the state-actor builder's + // preflight). Errors on the crash-inconsistent state (pointing the user at + // `schelk full-recover`) or a failed `schelk mount`. + if err := datadir.EnsureSchelkMounted(context.Background(), nil); err != nil { + return fmt.Errorf("datadir.method %q: %w", "schelk", err) } - // state.IsMounted && mountedActual: nothing to do — Prepare will run - // schelk restore to re-establish baseline at run start. - // !state.IsMounted && mountedActual: unusual but harmless; let Prepare - // handle it via restore. for _, si := range schelkInstances { info, statErr := os.Stat(si.sourceDir) diff --git a/pkg/datadir/schelk.go b/pkg/datadir/schelk.go index c06c4d07..ac218d49 100644 --- a/pkg/datadir/schelk.go +++ b/pkg/datadir/schelk.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "os" "os/exec" @@ -286,6 +287,87 @@ func ReadSchelkState(path string) (*SchelkState, error) { return &s, nil } +// SchelkDir reports whether outputDir lives under a configured schelk mount, +// returning the mount point when it does. When schelk is not set up (no state +// file) it returns isSchelk=false with no error, so callers that don't use +// schelk are unaffected. A malformed/unreadable existing state file is an error. +func SchelkDir(outputDir string) (mountPoint string, isSchelk bool, err error) { + statePath := SchelkStatePath() + if _, statErr := os.Stat(statePath); errors.Is(statErr, os.ErrNotExist) { + return "", false, nil + } + + state, err := ReadSchelkState(statePath) + if err != nil { + return "", false, err + } + + if state.MountPoint == "" { + return "", false, nil + } + + abs, err := filepath.Abs(outputDir) + if err != nil { + return "", false, fmt.Errorf("resolving output_dir %q: %w", outputDir, err) + } + + return state.MountPoint, pathIsUnderMountpoint(abs, state.MountPoint), nil +} + +// EnsureSchelkMounted makes sure the schelk scratch volume is mounted, running +// `schelk mount` when it is not. It errors when schelk's recorded state and the +// kernel disagree (a crash artefact that only `schelk full-recover` resolves). +func EnsureSchelkMounted(ctx context.Context, log logrus.FieldLogger) error { + bin := SchelkBinary() + + state, err := ReadSchelkState(SchelkStatePath()) + if err != nil { + return err + } + + if state.MountPoint == "" { + return fmt.Errorf("schelk state has no mount_point — run `schelk init-new` or `schelk init-from` first") + } + + mounted, err := IsMountedAt(state.MountPoint) + if err != nil { + return fmt.Errorf("checking mount status: %w", err) + } + + switch { + case state.IsMounted && !mounted: + // schelk state says mounted but the kernel disagrees — typically a + // crash or interrupted recover left dm-era and state out of sync. + // `schelk mount` will refuse ("Volume is already mounted"); only + // `schelk full-recover` can re-establish a consistent baseline. + return fmt.Errorf( + "schelk state at %q says is_mounted=true but %q is not in /proc/mounts "+ + "(inconsistent, likely from a prior crash) — run `%s full-recover` to reset, then retry", + SchelkStatePath(), state.MountPoint, bin, + ) + case !mounted: + output, runErr := RunSchelk(ctx, log, "mount", "-y") + if runErr != nil { + return fmt.Errorf("`%s mount` failed: %w (output: %s)", + bin, runErr, strings.TrimSpace(string(output))) + } + } + + return nil +} + +// SchelkPromote persists the current scratch contents as the new virgin +// baseline via `schelk promote`, so subsequent recover/restore reset to it. +func SchelkPromote(ctx context.Context, log logrus.FieldLogger) error { + output, err := RunSchelk(ctx, log, "promote", "-y") + if err != nil { + return fmt.Errorf("`%s promote` failed: %w (output: %s)", + SchelkBinary(), err, strings.TrimSpace(string(output))) + } + + return nil +} + // IsMountedAt reports whether the given mount point appears as a mount // in /proc/mounts. The schelk state's is_mounted flag is unreliable // after a crash, so we consult the kernel directly. diff --git a/pkg/datadir/schelk_test.go b/pkg/datadir/schelk_test.go index eee417a9..08b1af46 100644 --- a/pkg/datadir/schelk_test.go +++ b/pkg/datadir/schelk_test.go @@ -27,6 +27,72 @@ func installFakeSchelk(t *testing.T, body string) { t.Setenv("BENCHMARKOOR_SCHELK_BIN", binPath) } +// writeSchelkState writes a state.json with the given mount_point and points +// SCHELK_STATE at it. +func writeSchelkState(t *testing.T, mountPoint string) { + t.Helper() + + statePath := filepath.Join(t.TempDir(), "state.json") + data := fmt.Sprintf(`{"mount_point":%q,"is_mounted":true}`, mountPoint) + require.NoError(t, os.WriteFile(statePath, []byte(data), 0o600)) + t.Setenv("SCHELK_STATE", statePath) +} + +func TestSchelkDir(t *testing.T) { + t.Run("no state file is not a schelk dir", func(t *testing.T) { + t.Setenv("SCHELK_STATE", filepath.Join(t.TempDir(), "absent.json")) + + mp, isSchelk, err := SchelkDir("/some/dir") + require.NoError(t, err) + assert.False(t, isSchelk) + assert.Empty(t, mp) + }) + + t.Run("output_dir under the mount is a schelk dir", func(t *testing.T) { + mount := t.TempDir() + writeSchelkState(t, mount) + + mp, isSchelk, err := SchelkDir(filepath.Join(mount, "eth", "geth")) + require.NoError(t, err) + assert.True(t, isSchelk) + assert.Equal(t, mount, mp) + }) + + t.Run("output_dir outside the mount is not a schelk dir", func(t *testing.T) { + mount := t.TempDir() + writeSchelkState(t, mount) + + mp, isSchelk, err := SchelkDir(filepath.Join(t.TempDir(), "geth")) + require.NoError(t, err) + assert.False(t, isSchelk) + assert.Equal(t, mount, mp) + }) +} + +func TestEnsureSchelkMounted_NoMountPoint(t *testing.T) { + writeSchelkState(t, "") + + err := EnsureSchelkMounted(context.Background(), nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "no mount_point") +} + +func TestSchelkPromote(t *testing.T) { + t.Run("invokes promote -y", func(t *testing.T) { + installFakeSchelk(t, `[ "$1" = promote ] && [ "$2" = "-y" ] && exit 0 || exit 3`) + require.NoError(t, SchelkPromote(context.Background(), nil)) + }) + + t.Run("surfaces failure with output", func(t *testing.T) { + installFakeSchelk(t, "echo nope >&2\nexit 1") + + err := SchelkPromote(context.Background(), nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "promote") + assert.Contains(t, err.Error(), "nope") + }) +} + func TestRunSchelk_HappyPath(t *testing.T) { installFakeSchelk(t, "echo hello\nexit 0")