From f2b1ca5ea28c5968f8a332894e991029c0c2c0a5 Mon Sep 17 00:00:00 2001 From: mnoah1 Date: Mon, 10 Aug 2026 23:39:55 +0000 Subject: [PATCH 1/2] feat(stovepipe): make the fake build runner's outcome and duration configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fake previously succeeded immediately for every build unless a request carried a marker, so a local stack could not be made to look like a real one. Params now set a failure rate and a build duration for every build, and New validates them so a misconfigured stack fails at wiring time. Duration is configured as a typical value plus DurationJitterPercent, a symmetric spread around it: 60s at 25% draws uniformly from 45s to 75s. A percentage of the duration rather than a min/max pair keeps the bounds stateable in one line, cannot be inverted, and — capped at 100 — cannot go negative. Zero jitter reproduces the previous fixed-duration behavior. Markers still win for the build that carries them, so a stack running a failure rate can still ask for a specific outcome per request. The knobs are wired through BUILD_RUNNER_* environment variables and Compose. Also records the leading-underscore convention for unexported globals in AGENTS.md, which this code follows. --- AGENTS.md | 1 + service/stovepipe/README.md | 24 ++ service/stovepipe/docker-compose.yml | 6 + service/stovepipe/server/main.go | 95 +++++- stovepipe/extension/buildrunner/README.md | 4 +- stovepipe/extension/buildrunner/fake/fake.go | 319 +++++++++++++----- .../extension/buildrunner/fake/fake_test.go | 268 +++++++++++++-- 7 files changed, 606 insertions(+), 111 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 71ee93d2..fc0aafd6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -333,6 +333,7 @@ CI runs on every PR and enforces all checks via a `required-checks` gate. **Befo 2. **Interfaces for behavior, structs for data** — use interfaces for behavioral contracts (Consumer, Controller, Storage). Use structs for data containers, configs, and registries (TopicRegistry, SubscriptionConfig). 3. **Value types over pointers** — prefer value types for structs, configs, and return values. Use `(T, bool)` to signal absence instead of `*T`. Pointers only when mutation or shared ownership is needed. 4. **Errors for failures, not control flow** — reserve `error` returns for unexpected or infrastructure failures. Use result types (structs, bools) for expected outcomes like `(Result, error)` or `(T, bool)`. Avoid sentinel errors that represent non-failure states. +5. **Prefix unexported globals with `_`** — package-level `const` and `var` names that are unexported take a leading underscore (`_defaultSlowBuildDuration`, `_tokenFail`), per the [Uber Go style guide](https://github.com/uber-go/guide/blob/master/style.md#prefix-unexported-globals-with-_). The prefix makes a global unmistakable at its use site and makes accidental shadowing by a local obvious. Exported identifiers keep their plain names, and function-local constants are not globals — neither takes the prefix. `var _ Iface = (*impl)(nil)` interface assertions and generated code (`protopb/`, `mock/`) are exempt. Most of the existing tree predates this rule; new and edited code follows it, and older packages are migrated as they are touched rather than in one sweep. ### Error Classification (`platform/errs`) diff --git a/service/stovepipe/README.md b/service/stovepipe/README.md index 426049db..55075dc7 100644 --- a/service/stovepipe/README.md +++ b/service/stovepipe/README.md @@ -16,6 +16,7 @@ Stovepipe therefore needs two MySQL databases: a **storage** database (the `requ - **`inMemoryCounter`** — a process-local `counter.Counter` for sequence numbers; not durable. A real deployment uses a persistent implementation (e.g. `platform/extension/counter/mysql`). - **`fakeSourceControlFactory`** — seeds each queue with a deterministic single-commit history so ingest resolves a stable head URI (and re-ingesting the same queue exercises the dedup path). A real deployment supplies a VCS-backed `sourcecontrol.Factory`. +- **`newBuildRunnerFactory`** — builds the `buildrunner.Factory` from the environment. Each queue gets its own fake runner, bound to that queue's `Config` and sharing the profile the `BUILD_RUNNER_*` knobs describe (failure rate, build duration, and its spread). A real deployment keeps this shape and swaps the constructed runner for a Buildkite or GitHub Actions one (see [`stovepipe/extension/buildrunner`](../../stovepipe/extension/buildrunner)); a deployment that gives queues *different* backends routes on `Config.QueueName` in `For`, as the SubmitQueue orchestrator does in its `profiles.go`. ## Layout @@ -40,6 +41,29 @@ The Stovepipe controllers live under [`stovepipe/controller/`](../../stovepipe/c | `PORT` | no | gRPC listen address | `:8083` | | `HOSTNAME` | no | Subscriber name for the process consumer | `stovepipe-` | +### Build runner + +These knobs configure the fake runner every queue shares. A malformed value fails startup rather than silently falling back to the default. + +| Variable | Required | Description | Default | +|------------------------------------------|----------|--------------------------------------------------------------|---------| +| `BUILD_RUNNER_FAILURE_PERCENT` | no | Share of builds (0-100) the runner reports as failed | `0` | +| `BUILD_RUNNER_DURATION_MS` | no | How long each build reports running before turning terminal | `0` | +| `BUILD_RUNNER_DURATION_JITTER_PERCENT` | no | Spread (0-100) applied to that duration per build | `0` | + +The defaults reproduce the original behavior: a fake runner that succeeds immediately unless a request's head URI carries a `buildrunner-fake=` marker. Markers keep working under a configured rate — they pin the outcome for the build that carries them. + +Durations are configured as a typical value plus a spread, so the bounds stay easy to state: `BUILD_RUNNER_DURATION_MS=60000` with `BUILD_RUNNER_DURATION_JITTER_PERCENT=25` means every build takes between 45s and 75s, drawn uniformly. A jitter of `0` pins every build to exactly the configured duration; `100` is the widest setting, spanning "terminal immediately" to twice the duration. + +Under Compose these are set through `SQ_`-prefixed variables, so a stack whose builds are slow and flaky starts with: + +```bash +SQ_BUILD_RUNNER_FAILURE_PERCENT=20 \ +SQ_BUILD_RUNNER_DURATION_MS=5000 \ +SQ_BUILD_RUNNER_DURATION_JITTER_PERCENT=40 \ +make local-stovepipe-start +``` + ## Running ### Docker Compose (recommended) diff --git a/service/stovepipe/docker-compose.yml b/service/stovepipe/docker-compose.yml index 3c816c21..6e7216cf 100644 --- a/service/stovepipe/docker-compose.yml +++ b/service/stovepipe/docker-compose.yml @@ -68,6 +68,12 @@ services: - STORAGE_MYSQL_DSN=root:root@tcp(mysql-app:3306)/submitqueue?parseTime=true - QUEUE_MYSQL_DSN=root:root@tcp(mysql-queue:3306)/submitqueue?parseTime=true - HOSTNAME=stovepipe-dev + # Fake build runner behavior. Unset means the default: every build + # succeeds immediately unless a request's head URI carries a + # buildrunner-fake marker — see the README. + - BUILD_RUNNER_FAILURE_PERCENT=${SQ_BUILD_RUNNER_FAILURE_PERCENT:-} + - BUILD_RUNNER_DURATION_MS=${SQ_BUILD_RUNNER_DURATION_MS:-} + - BUILD_RUNNER_DURATION_JITTER_PERCENT=${SQ_BUILD_RUNNER_DURATION_JITTER_PERCENT:-} depends_on: mysql-app: condition: service_healthy diff --git a/service/stovepipe/server/main.go b/service/stovepipe/server/main.go index 3b44500b..05c5a1a5 100644 --- a/service/stovepipe/server/main.go +++ b/service/stovepipe/server/main.go @@ -22,6 +22,8 @@ import ( "net" "os" "os/signal" + "strconv" + "strings" "sync" "syscall" "time" @@ -136,13 +138,91 @@ func (fakeSourceControlFactory) For(cfg sourcecontrol.Config) (sourcecontrol.Sou return sourcecontrolfake.New(cfg, []string{fmt.Sprintf("git://%s/HEAD", cfg.QueueName)}), nil } +// Environment variables configuring the fake BuildRunner. +const ( + _envFailurePercent = "BUILD_RUNNER_FAILURE_PERCENT" + _envBuildDurationMs = "BUILD_RUNNER_DURATION_MS" + _envDurationJitterPercent = "BUILD_RUNNER_DURATION_JITTER_PERCENT" +) + // fakeBuildRunnerFactory is the example BuildRunner factory: every queue gets a stateless fake -// runner bound to its own config, which succeeds unless a caller embeds a failure marker in the -// head URI. A real deployment supplies a backend-specific factory (e.g. Buildkite, per queue). -type fakeBuildRunnerFactory struct{} +// runner bound to its own config, carrying the process-wide profile read from the environment. +// A real deployment supplies a backend-specific factory (e.g. Buildkite, per queue). +type fakeBuildRunnerFactory struct { + params buildrunnerfake.Params +} + +func (f fakeBuildRunnerFactory) For(cfg buildrunner.Config) (buildrunner.BuildRunner, error) { + params := f.params + params.Config = cfg + return buildrunnerfake.New(params) +} + +// newBuildRunnerFactory builds the BuildRunner factory from the environment, so +// every queue's fake runner shares the profile set by the BUILD_RUNNER_* knobs. +// The fake is demo-only; a real deployment keeps this shape and swaps the +// constructed runner for a Buildkite or GitHub Actions one. +func newBuildRunnerFactory(logger *zap.SugaredLogger) (buildrunner.Factory, error) { + params, err := fakeBuildRunnerParams() + if err != nil { + return nil, err + } + // Runners are built per resolution, so a profile the extension rejects + // would not surface until the first build. Build one here and discard it + // to keep that failure at startup. + if _, err := buildrunnerfake.New(params); err != nil { + return nil, err + } + logger.Infow("build runner configured", "impl", "fake") + return fakeBuildRunnerFactory{params: params}, nil +} + +// fakeBuildRunnerParams reads the fake runner's profile from the environment, +// leaving Config for the factory to fill in per queue. +func fakeBuildRunnerParams() (buildrunnerfake.Params, error) { + failurePercent, err := envInt(_envFailurePercent) + if err != nil { + return buildrunnerfake.Params{}, err + } + buildDuration, err := envDurationMs(_envBuildDurationMs) + if err != nil { + return buildrunnerfake.Params{}, err + } + durationJitterPercent, err := envInt(_envDurationJitterPercent) + if err != nil { + return buildrunnerfake.Params{}, err + } + return buildrunnerfake.Params{ + FailurePercent: failurePercent, + BuildDuration: buildDuration, + DurationJitterPercent: durationJitterPercent, + }, nil +} -func (fakeBuildRunnerFactory) For(cfg buildrunner.Config) (buildrunner.BuildRunner, error) { - return buildrunnerfake.New(cfg), nil +// envInt reads an integer from the environment, treating unset and empty as 0. A +// malformed value fails startup rather than silently falling back to the default, +// which would leave the stack running a profile nobody asked for. Range checks +// belong to the extension constructors that own the valid range. +func envInt(name string) (int, error) { + raw := strings.TrimSpace(os.Getenv(name)) + if raw == "" { + return 0, nil + } + value, err := strconv.Atoi(raw) + if err != nil { + return 0, fmt.Errorf("invalid %s %q: %w", name, raw, err) + } + return value, nil +} + +// envDurationMs reads a millisecond count from the environment as a duration, +// treating unset and empty as zero. +func envDurationMs(name string) (time.Duration, error) { + ms, err := envInt(name) + if err != nil { + return 0, err + } + return time.Duration(ms) * time.Millisecond, nil } func main() { @@ -277,7 +357,10 @@ func run() error { // it, so a real (stateful) backend introduced later is shared rather than // silently duplicated across controllers. scf := fakeSourceControlFactory{} - brf := fakeBuildRunnerFactory{} + brf, err := newBuildRunnerFactory(logger.Sugar()) + if err != nil { + return err + } storageFty := storageFactory{backend: store} primaryCount, err := registerPrimaryControllers(primaryConsumer, logger.Sugar(), scope, storageFty, registry, scf, brf) diff --git a/stovepipe/extension/buildrunner/README.md b/stovepipe/extension/buildrunner/README.md index b25d64fa..ae78c491 100644 --- a/stovepipe/extension/buildrunner/README.md +++ b/stovepipe/extension/buildrunner/README.md @@ -10,4 +10,6 @@ Implementations return plain, unclassified errors — the calling controller dec Real backends (`buildkite`, `githubactions`) are thin adapters over a shared platform client (`platform/extension/buildrunner/{backend}`) — see that package's README for the HTTP client and vendor-specific details. -See [doc/rfc/stovepipe/steps/build.md](../../../doc/rfc/stovepipe/steps/build.md#why-separate-contracts) for why this is a separate contract from SubmitQueue's own `buildrunner` rather than a shared one. To add a backend, create `buildrunner/{backend}/`, implement `BuildRunner`, and return it from a `New(...)` constructor. +`fake` is the stub for local stacks and tests. By default every build succeeds immediately; its `Params` set a failure rate and a build duration — optionally spread by a percentage so durations vary within bounds the configuration states outright — for every build, and a `buildrunner-fake=` marker in a request's head URI pins the outcome or the running window for one build. It keeps no per-build state — the outcome and the instant the build turns terminal are encoded in the build id — so `Status` can be answered by any instance in any process. Never production. + +See [doc/rfc/stovepipe/steps/build.md](../../../doc/rfc/stovepipe/steps/build.md#why-separate-contracts) for why this is a separate contract from SubmitQueue's own `buildrunner` rather than a shared one. To add a backend, create `buildrunner/{backend}/`, implement `BuildRunner`, and return it from a `New(...)` constructor. Which backend serves which queue is decided in the wiring layer ([`service/stovepipe/server`](../../../service/stovepipe/server)), not here. diff --git a/stovepipe/extension/buildrunner/fake/fake.go b/stovepipe/extension/buildrunner/fake/fake.go index 4c568345..9bcfede3 100644 --- a/stovepipe/extension/buildrunner/fake/fake.go +++ b/stovepipe/extension/buildrunner/fake/fake.go @@ -12,24 +12,36 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package fake provides a buildrunner.BuildRunner whose outcome is driven by the -// triggered head URI. With no marker every build immediately succeeds, behaving -// as a best-case stub for local-stack/e2e wiring. Other behaviors are injected by -// embedding a marker token in headURI of the form "buildrunner-fake=": +// Package fake provides a buildrunner.BuildRunner whose outcome is driven by +// construction-time Params and by the triggered head URI. With zero-value Params +// and no marker every build immediately succeeds, behaving as a best-case stub for +// local-stack/e2e wiring. +// +// Params set the behavior for every build: FailurePercent makes a share of builds +// report BuildStatusFailed, BuildDuration makes builds report BuildStatusRunning +// for a while before turning terminal, and DurationJitterPercent spreads that +// duration per build within stated bounds. Per-build behavior is +// injected instead by embedding a marker token in headURI of the form +// "buildrunner-fake=": // // buildrunner-fake=trigger-error -> Trigger returns a non-nil error // buildrunner-fake=build-fail -> Status reports BuildStatusFailed // buildrunner-fake=build-error -> Status returns a non-nil error // buildrunner-fake=build-slow -> Status reports BuildStatusRunning for a -// short window after Trigger, then succeeds +// short window after Trigger, then reports +// the terminal outcome +// +// A marker overrides the configured behavior for the build that carries it: the +// outcome markers pin the outcome the configured rate would otherwise draw, and +// build-slow pins the running window regardless of BuildDuration. // -// The runner is stateless: Trigger encodes the desired terminal outcome into the -// returned BuildID, and Status decides the result purely from the BuildID it is -// given — no per-build bookkeeping. This means any runner instance can answer -// Status for an id minted by any other (Trigger and Status can even live in -// different processes), and a single running stack can exercise the negative -// paths purely by varying request payloads. It is intended for examples and -// tests only, never production. +// The runner is stateless: Trigger encodes the desired terminal outcome and the +// instant the build reaches it into the returned BuildID, and Status decides the +// result purely from the BuildID it is given — no per-build bookkeeping. This +// means any runner instance can answer Status for an id minted by any other +// (Trigger and Status can even live in different processes), and a single running +// stack can exercise the negative paths purely by varying request payloads. It is +// intended for examples and tests only, never production. package fake import ( @@ -37,6 +49,7 @@ import ( "crypto/rand" "encoding/hex" "fmt" + mathrand "math/rand/v2" "strconv" "strings" "time" @@ -45,93 +58,181 @@ import ( "github.com/uber/submitqueue/stovepipe/extension/buildrunner" ) -// defaultSlowBuildDuration is how long a build-slow build reports -// BuildStatusRunning before succeeding. It must be long enough for the caller's -// poll loop to observe at least one non-terminal status; tests that need a -// different window construct a runner with their own value. -const defaultSlowBuildDuration = 3 * time.Second +// _defaultSlowBuildDuration is how long a build-slow build reports +// BuildStatusRunning before turning terminal when Params leaves BuildDuration +// unset. It must be long enough for the caller's poll loop to observe at least +// one non-terminal status. +const _defaultSlowBuildDuration = 3 * time.Second -// markerPrefix introduces a marker token in headURI: "buildrunner-fake=". -const markerPrefix = "buildrunner-fake=" +// _markerPrefix introduces a marker token in headURI: "buildrunner-fake=". +const _markerPrefix = "buildrunner-fake=" // Recognized marker tokens. See the package doc for the convention. const ( - tokenTriggerError = "trigger-error" - tokenFail = "build-fail" - tokenError = "build-error" - tokenSlow = "build-slow" + _tokenTriggerError = "trigger-error" + _tokenFail = "build-fail" + _tokenError = "build-error" + _tokenSlow = "build-slow" ) -// outcomeOK is the BuildID outcome segment for a build that should succeed. -const outcomeOK = "ok" +// _outcomeOK is the BuildID outcome segment for a build that should succeed. +const _outcomeOK = "ok" + +// _idPrefix introduces every BuildID this fake mints. +const _idPrefix = "fake-" + +// _percentScale is the draw range for a percentage sample: a draw in [0,100) +// compared against a percentage in [0,100] yields that percentage of hits. +const _percentScale = 100 + +// Params configures how a fake runner behaves for builds whose head URI carries +// no marker. The zero value is the best-case stub: every build succeeds +// immediately. +type Params struct { + // Config holds the per-queue identity for this BuildRunner. + Config buildrunner.Config + + // FailurePercent is the share of builds, in percent, whose Status reports + // BuildStatusFailed rather than BuildStatusSucceeded. Valid range is 0-100. + // The outcome is drawn independently per Trigger, so this is a rate over + // many builds rather than a guarantee about any fixed number of them. The + // zero value never fails. + FailurePercent int -// runner is a buildrunner.BuildRunner that reports every build as succeeded -// unless a marker token in headURI requests otherwise. It holds no per-build -// state: the outcome is encoded in the BuildID at Trigger and read back out at -// Status. Uniqueness comes from a random suffix per id, so it needs no shared -// counter and never collides across instances or processes. + // BuildDuration is how long a build reports BuildStatusRunning, measured + // from Trigger, before it reports its terminal outcome. The zero value makes + // builds terminal on the first Status call. It also becomes the window for + // the build-slow marker, which otherwise uses its own default. + BuildDuration time.Duration + + // DurationJitterPercent spreads each build's duration around BuildDuration, + // as a percentage of it: 25 draws uniformly from [0.75×BuildDuration, + // 1.25×BuildDuration], so builds vary but stay within bounds the integrator + // can state in one line. Valid range is 0-100, which keeps the lower bound at + // or above zero; at 100 a build may come back terminal immediately or take + // twice BuildDuration. The spread is drawn independently per Trigger and + // applies to the build-slow marker's window too. The zero value makes every + // build take exactly its window. + DurationJitterPercent int +} + +// runner is a buildrunner.BuildRunner that reports builds as succeeded unless +// its configured failure rate or a marker token in headURI requests otherwise. +// It holds no per-build state: the outcome and the instant it becomes terminal +// are encoded in the BuildID at Trigger and read back out at Status. Uniqueness +// comes from a random suffix per id, so it needs no shared counter and never +// collides across instances or processes. type runner struct { // cfg is the per-queue identity this runner was built for. cfg buildrunner.Config - // slowBuildDuration is how long a build-slow build reports running before - // it succeeds. Configuration, not per-build state: it is read at Trigger to - // compute the deadline baked into the id, never mutated. + + // failurePercent is the share of builds, in percent, that report failed. + // Configuration, not per-build state: it is read at Trigger to draw the + // outcome baked into the id, never mutated. + failurePercent int + + // buildDuration is how long every build reports running before it turns + // terminal. Zero leaves builds terminal immediately. + buildDuration time.Duration + + // slowBuildDuration is the running window for a build-slow build, which + // applies even when buildDuration leaves other builds terminal immediately. slowBuildDuration time.Duration + + // durationJitterPercent is how far, in percent, a build's duration may fall + // either side of its window. Zero pins every build to its window exactly. + durationJitterPercent int + + // intn draws the outcome and duration samples from [0,n). A field rather than + // a direct call to math/rand so tests can pin the draws. + intn func(n int) int } -// New returns a buildrunner.BuildRunner bound to the queue named in cfg that -// defaults to succeeding and honors marker tokens embedded in the triggered -// headURI. -func New(cfg buildrunner.Config) buildrunner.BuildRunner { - return runner{cfg: cfg, slowBuildDuration: defaultSlowBuildDuration} +// New returns a buildrunner.BuildRunner bound to the queue named in +// params.Config, whose failure rate and build duration come from params, and +// which honors marker tokens embedded in the triggered headURI. The zero-value +// Params ask for the best-case stub: every build succeeds immediately. It rejects a percentage outside 0-100 and a negative +// BuildDuration so a misconfigured stack fails at wiring time rather than +// silently running different builds than the integrator asked for. +func New(params Params) (buildrunner.BuildRunner, error) { + if params.FailurePercent < 0 || params.FailurePercent > _percentScale { + return nil, fmt.Errorf("fake: failure percent %d outside 0-100", params.FailurePercent) + } + if params.BuildDuration < 0 { + return nil, fmt.Errorf("fake: build duration %s is negative", params.BuildDuration) + } + if params.DurationJitterPercent < 0 || params.DurationJitterPercent > _percentScale { + return nil, fmt.Errorf("fake: duration jitter percent %d outside 0-100", params.DurationJitterPercent) + } + return newRunner(params), nil +} + +// newRunner constructs a runner from validated params. Used by New and by tests +// that need to override a field the Params do not expose. +func newRunner(params Params) runner { + slowBuildDuration := params.BuildDuration + if slowBuildDuration <= 0 { + slowBuildDuration = _defaultSlowBuildDuration + } + return runner{ + cfg: params.Config, + failurePercent: params.FailurePercent, + buildDuration: params.BuildDuration, + slowBuildDuration: slowBuildDuration, + durationJitterPercent: params.DurationJitterPercent, + intn: mathrand.IntN, + } } // Trigger fails when headURI carries the trigger-error marker; otherwise it -// returns a unique BuildID that encodes the terminal outcome the build should -// report at Status time (decided from the headURI marker). baseURI and metadata -// are ignored. +// returns a unique BuildID that encodes both the terminal outcome the build +// should report at Status time and the instant it reaches it. The outcome comes +// from the configured failure rate unless a marker in headURI pins it. baseURI +// and metadata are ignored. func (r runner) Trigger(_ context.Context, _, headURI string, _ entity.BuildMetadata) (entity.BuildID, error) { - outcome := outcomeOK + outcome := r.drawOutcome() + readyAt := terminalAt(r.drawDuration(r.buildDuration)) switch marker(headURI) { - case tokenTriggerError: + case _tokenTriggerError: return entity.BuildID{}, fmt.Errorf("fake: marked trigger error") - case tokenFail: - outcome = tokenFail - case tokenError: - outcome = tokenError - case tokenSlow: - // A slow build carries the wall-clock instant it becomes terminal, so Status - // stays stateless: any instance, in any process, decodes the same deadline. - outcome = fmt.Sprintf("%s-%d", tokenSlow, time.Now().Add(r.slowBuildDuration).UnixMilli()) - } - - // Encode the outcome in the id (e.g. "fake-build-fail-a1b2c3d4") so Status is - // stateless. The random suffix keeps ids globally unique across instances and - // processes without any shared state. + case _tokenFail: + outcome = _tokenFail + case _tokenError: + outcome = _tokenError + case _tokenSlow: + // Only the timing is pinned; the outcome still comes from the draw + // above, so a slow build fails at the configured rate like any other. + readyAt = terminalAt(r.drawDuration(r.slowBuildDuration)) + } + + // Encode the outcome and terminal instant in the id (e.g. + // "fake-build-fail-0-a1b2c3d4") so Status is stateless. The random suffix + // keeps ids globally unique across instances and processes without any + // shared state. suffix, err := randomSuffix() if err != nil { return entity.BuildID{}, fmt.Errorf("fake: generating build id: %w", err) } - return entity.BuildID{ID: fmt.Sprintf("fake-%s-%s", outcome, suffix)}, nil + return entity.BuildID{ID: fmt.Sprintf("%s%s-%d-%s", _idPrefix, outcome, readyAt, suffix)}, nil } -// Status decides the result purely from the BuildID's encoded outcome. Ids that -// carry no recognized outcome (including those not minted by this fake) default -// to succeeded, keeping the runner best-case. +// Status decides the result purely from the BuildID's encoded outcome and +// terminal instant. Ids that carry no recognized outcome (including those not +// minted by this fake) default to succeeded, keeping the runner best-case. func (r runner) Status(_ context.Context, buildID entity.BuildID) (entity.BuildStatus, entity.BuildMetadata, error) { - switch { - case strings.Contains(buildID.ID, tokenError): + outcome, readyAt := decodeID(buildID.ID) + // A marked error is a failure of the Status call itself, so it surfaces + // whether or not the build would still be running. + if outcome == _tokenError { return entity.BuildStatusUnknown, nil, fmt.Errorf("fake: marked build error") - case strings.Contains(buildID.ID, tokenFail): + } + if readyAt > time.Now().UnixMilli() { + return entity.BuildStatusRunning, nil, nil + } + if outcome == _tokenFail { return entity.BuildStatusFailed, nil, nil - case strings.Contains(buildID.ID, tokenSlow): - if readyAt, ok := slowReadyAt(buildID.ID); ok && time.Now().UnixMilli() < readyAt { - return entity.BuildStatusRunning, nil, nil - } - return entity.BuildStatusSucceeded, nil, nil - default: - return entity.BuildStatusSucceeded, nil, nil } + return entity.BuildStatusSucceeded, nil, nil } // Cancel is a no-op and always succeeds. @@ -139,13 +240,38 @@ func (r runner) Cancel(_ context.Context, _ entity.BuildID) error { return nil } +// drawOutcome samples the terminal outcome for a build from the configured +// failure rate. +func (r runner) drawOutcome() string { + if r.failurePercent <= 0 { + return _outcomeOK + } + if r.intn(_percentScale) < r.failurePercent { + return _tokenFail + } + return _outcomeOK +} + +// drawDuration samples how long one build runs: window spread uniformly by up to +// ±durationJitterPercent of itself, leaving window untouched when there is no +// window or no jitter configured. The draw covers the 2j+1 whole percentage +// points from -j to +j inclusive, so it is centered on window and, with the +// percentage capped at 100, never yields a negative duration. +func (r runner) drawDuration(window time.Duration) time.Duration { + if window <= 0 || r.durationJitterPercent <= 0 { + return window + } + offset := r.intn(2*r.durationJitterPercent+1) - r.durationJitterPercent + return window + window*time.Duration(offset)/_percentScale +} + // marker returns the marker token embedded in uri, or "" if none is present. // The token ends at the first "&", "#", or "/" delimiter, so a marker may sit // among other query parameters, before a fragment, or ahead of a further path // segment (as when a URI is built as "git:///HEAD" and the marker rides // in on the queue name). func marker(uri string) string { - _, rest, found := strings.Cut(uri, markerPrefix) + _, rest, found := strings.Cut(uri, _markerPrefix) if !found { return "" } @@ -155,21 +281,46 @@ func marker(uri string) string { return rest } -// slowReadyAt extracts the epoch-millisecond instant at which a build-slow build -// becomes terminal, which Trigger encodes into the id as -// "fake-build-slow--". It reports false when the id carries no -// parsable deadline, in which case the caller treats the build as already terminal. -func slowReadyAt(id string) (int64, bool) { - _, rest, found := strings.Cut(id, tokenSlow+"-") - if !found { - return 0, false +// terminalAt converts a running window into the epoch-millisecond instant at +// which the build turns terminal, or 0 when it is terminal immediately. Baking +// the instant into the id is what keeps Status stateless: any instance, in any +// process, decodes the same deadline. +func terminalAt(window time.Duration) int64 { + if window <= 0 { + return 0 } - digits, _, _ := strings.Cut(rest, "-") - readyAt, err := strconv.ParseInt(digits, 10, 64) - if err != nil { - return 0, false + return time.Now().Add(window).UnixMilli() +} + +// decodeID recovers the outcome and terminal instant that Trigger encoded into +// an id of the form "fake---". An id carrying no +// parsable instant — one minted before the instant became part of the id, or not +// minted by this fake at all — falls back to reading the outcome as a substring +// and to being terminal immediately. +func decodeID(id string) (string, int64) { + // Trim the random suffix, then the instant, leaving "fake-". + if suffixAt := strings.LastIndex(id, "-"); suffixAt > 0 { + head := id[:suffixAt] + if instantAt := strings.LastIndex(head, "-"); instantAt > 0 { + if readyAt, err := strconv.ParseInt(head[instantAt+1:], 10, 64); err == nil { + return strings.TrimPrefix(head[:instantAt], _idPrefix), readyAt + } + } + } + return substringOutcome(id), 0 +} + +// substringOutcome reads the outcome out of an id that carries no encoded +// instant, matching the outcome tokens anywhere in the id. +func substringOutcome(id string) string { + switch { + case strings.Contains(id, _tokenError): + return _tokenError + case strings.Contains(id, _tokenFail): + return _tokenFail + default: + return _outcomeOK } - return readyAt, true } // randomSuffix returns a short random hex string used to keep fake BuildIDs diff --git a/stovepipe/extension/buildrunner/fake/fake_test.go b/stovepipe/extension/buildrunner/fake/fake_test.go index e5f8e27d..bac43d0c 100644 --- a/stovepipe/extension/buildrunner/fake/fake_test.go +++ b/stovepipe/extension/buildrunner/fake/fake_test.go @@ -29,8 +29,19 @@ import ( // testCfg is the per-queue identity used by every case in this file. var testCfg = buildrunner.Config{QueueName: "test-queue"} +// newFake constructs a fake for the test queue from params, failing the test if +// they are invalid. Most tests want default behavior, for which the zero-value +// Params suffice. +func newFake(t *testing.T, params Params) buildrunner.BuildRunner { + t.Helper() + params.Config = testCfg + runner, err := New(params) + require.NoError(t, err) + return runner +} + func TestNew_ImplementsInterface(t *testing.T) { - var _ buildrunner.BuildRunner = New(testCfg) + var _ buildrunner.BuildRunner = newFake(t, Params{}) } func TestTrigger(t *testing.T) { @@ -47,7 +58,7 @@ func TestTrigger(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - id, err := New(testCfg).Trigger(context.Background(), "", tt.headURI, nil) + id, err := newFake(t, Params{}).Trigger(context.Background(), "", tt.headURI, nil) if tt.wantErr { require.Error(t, err) assert.Empty(t, id.ID) @@ -60,9 +71,9 @@ func TestTrigger(t *testing.T) { } func TestTrigger_UniqueIDs(t *testing.T) { - a, err := New(testCfg).Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) + a, err := newFake(t, Params{}).Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) require.NoError(t, err) - b, err := New(testCfg).Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) + b, err := newFake(t, Params{}).Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) require.NoError(t, err) assert.NotEqual(t, a.ID, b.ID) } @@ -80,10 +91,10 @@ func TestStatus(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - id, err := New(testCfg).Trigger(context.Background(), "", tt.headURI, nil) + id, err := newFake(t, Params{}).Trigger(context.Background(), "", tt.headURI, nil) require.NoError(t, err) - status, metadata, err := New(testCfg).Status(context.Background(), id) + status, metadata, err := newFake(t, Params{}).Status(context.Background(), id) if tt.wantErr { require.Error(t, err) return @@ -96,23 +107,23 @@ func TestStatus(t *testing.T) { } func TestStatus_UnrecognizedIDSucceeds(t *testing.T) { - status, metadata, err := New(testCfg).Status(context.Background(), entity.BuildID{ID: "not-minted-by-this-fake"}) + status, metadata, err := newFake(t, Params{}).Status(context.Background(), entity.BuildID{ID: "not-minted-by-this-fake"}) require.NoError(t, err) assert.Equal(t, entity.BuildStatusSucceeded, status) assert.Nil(t, metadata) } func TestStatus_StatelessAcrossInstances(t *testing.T) { - id, err := New(testCfg).Trigger(context.Background(), "", "git://repo/ref/deadbeef?buildrunner-fake=build-fail", nil) + id, err := newFake(t, Params{}).Trigger(context.Background(), "", "git://repo/ref/deadbeef?buildrunner-fake=build-fail", nil) require.NoError(t, err) - status, _, err := New(testCfg).Status(context.Background(), id) + status, _, err := newFake(t, Params{}).Status(context.Background(), id) require.NoError(t, err) assert.Equal(t, entity.BuildStatusFailed, status) } func TestCancel_NoOp(t *testing.T) { - err := New(testCfg).Cancel(context.Background(), entity.BuildID{ID: "anything"}) + err := newFake(t, Params{}).Cancel(context.Background(), entity.BuildID{ID: "anything"}) assert.NoError(t, err) } @@ -121,30 +132,247 @@ func TestCancel_NoOp(t *testing.T) { // integration or e2e stack. func TestStatus_BuildSlowReportsRunningThenSucceeds(t *testing.T) { // A window long enough that the build is still running when Status is called. - slow := runner{slowBuildDuration: 30 * time.Second} + slow := newRunner(Params{}) + slow.slowBuildDuration = 30 * time.Second id, err := slow.Trigger(context.Background(), "", "git://repo/ref/deadbeef?buildrunner-fake=build-slow", nil) require.NoError(t, err) - status, _, err := New(testCfg).Status(context.Background(), id) + status, _, err := newFake(t, Params{}).Status(context.Background(), id) require.NoError(t, err) assert.Equal(t, entity.BuildStatusRunning, status) // An id whose deadline has already passed reports the terminal outcome. Encoding // the deadline in the id is what keeps Status stateless across instances. - elapsed := entity.BuildID{ID: fmt.Sprintf("fake-build-slow-%d-abcd1234", time.Now().UnixMilli()-1)} - status, _, err = New(testCfg).Status(context.Background(), elapsed) + elapsed := entity.BuildID{ID: fmt.Sprintf("fake-ok-%d-abcd1234", time.Now().UnixMilli()-1)} + status, _, err = newFake(t, Params{}).Status(context.Background(), elapsed) require.NoError(t, err) assert.Equal(t, entity.BuildStatusSucceeded, status) } -// TestStatus_BuildSlowWithoutDeadlineSucceeds pins the fallback: an id carrying the -// marker but no parsable deadline is treated as already terminal rather than polling -// forever. -func TestStatus_BuildSlowWithoutDeadlineSucceeds(t *testing.T) { - status, _, err := New(testCfg).Status(context.Background(), entity.BuildID{ID: "fake-build-slow-nodeadline"}) +// TestStatus_WithoutDeadlineIsTerminal pins the fallback: an id carrying no parsable +// deadline is treated as already terminal rather than polling forever. Ids minted +// before the deadline became part of every id take this path. +func TestStatus_WithoutDeadlineIsTerminal(t *testing.T) { + tests := []struct { + name string + buildID string + wantStatus entity.BuildStatus + }{ + {name: "no deadline segment", buildID: "fake-build-slow-nodeadline", wantStatus: entity.BuildStatusSucceeded}, + {name: "legacy succeeded id", buildID: "fake-ok-abcd1234", wantStatus: entity.BuildStatusSucceeded}, + {name: "legacy failed id", buildID: "fake-build-fail-abcd1234", wantStatus: entity.BuildStatusFailed}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + status, _, err := newFake(t, Params{}).Status(context.Background(), entity.BuildID{ID: tt.buildID}) + require.NoError(t, err) + assert.Equal(t, tt.wantStatus, status) + }) + } +} + +func TestNew(t *testing.T) { + tests := []struct { + name string + params Params + wantErr bool + }{ + {name: "zero value is valid", params: Params{}}, + {name: "rate and duration", params: Params{FailurePercent: 50, BuildDuration: time.Second}}, + {name: "always fails", params: Params{FailurePercent: 100}}, + {name: "negative percent", params: Params{FailurePercent: -1}, wantErr: true}, + {name: "percent above 100", params: Params{FailurePercent: 101}, wantErr: true}, + {name: "negative duration", params: Params{BuildDuration: -time.Second}, wantErr: true}, + {name: "duration with jitter", params: Params{BuildDuration: time.Minute, DurationJitterPercent: 25}}, + {name: "full jitter", params: Params{BuildDuration: time.Minute, DurationJitterPercent: 100}}, + {name: "negative jitter", params: Params{DurationJitterPercent: -1}, wantErr: true}, + {name: "jitter above 100", params: Params{DurationJitterPercent: 101}, wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := New(tt.params) + if tt.wantErr { + require.Error(t, err) + assert.Nil(t, got) + return + } + require.NoError(t, err) + }) + } +} + +// TestStatus_FailurePercent covers the configured outcome rate at both extremes, +// where the drawn outcome is fully determined. +func TestStatus_FailurePercent(t *testing.T) { + tests := []struct { + name string + failurePercent int + wantStatus entity.BuildStatus + }{ + {name: "never fails", failurePercent: 0, wantStatus: entity.BuildStatusSucceeded}, + {name: "always fails", failurePercent: 100, wantStatus: entity.BuildStatusFailed}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := newFake(t, Params{FailurePercent: tt.failurePercent}) + + for range 20 { + id, err := r.Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) + require.NoError(t, err) + + status, _, err := r.Status(context.Background(), id) + require.NoError(t, err) + assert.Equal(t, tt.wantStatus, status) + } + }) + } +} + +// TestDrawOutcome_Boundary pins how a partial rate maps a draw onto an outcome, +// which the extremes above cannot distinguish. +func TestDrawOutcome_Boundary(t *testing.T) { + tests := []struct { + name string + failurePercent int + draw int + wantOutcome string + }{ + {name: "draw below rate fails", failurePercent: 25, draw: 24, wantOutcome: _tokenFail}, + {name: "draw at rate succeeds", failurePercent: 25, draw: 25, wantOutcome: _outcomeOK}, + {name: "zero rate never draws", failurePercent: 0, draw: 0, wantOutcome: _outcomeOK}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := newRunner(Params{FailurePercent: tt.failurePercent}) + r.intn = func(int) int { return tt.draw } + assert.Equal(t, tt.wantOutcome, r.drawOutcome()) + }) + } +} + +// TestDrawDuration_Bounds pins the bounds a configured jitter promises, at both +// ends of the draw: the duration never leaves ±jitter percent of the window. +func TestDrawDuration_Bounds(t *testing.T) { + tests := []struct { + name string + window time.Duration + jitter int + draw int + wantDuration time.Duration + }{ + {name: "no jitter pins the window", window: time.Minute, jitter: 0, draw: 0, wantDuration: time.Minute}, + {name: "lowest draw is the lower bound", window: time.Minute, jitter: 25, draw: 0, wantDuration: 45 * time.Second}, + {name: "midpoint draw is the window", window: time.Minute, jitter: 25, draw: 25, wantDuration: time.Minute}, + {name: "highest draw is the upper bound", window: time.Minute, jitter: 25, draw: 50, wantDuration: 75 * time.Second}, + {name: "full jitter can erase the window", window: time.Minute, jitter: 100, draw: 0, wantDuration: 0}, + {name: "full jitter can double the window", window: time.Minute, jitter: 100, draw: 200, wantDuration: 2 * time.Minute}, + {name: "no window has nothing to spread", window: 0, jitter: 100, draw: 0, wantDuration: 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := newRunner(Params{BuildDuration: tt.window, DurationJitterPercent: tt.jitter}) + r.intn = func(int) int { return tt.draw } + assert.Equal(t, tt.wantDuration, r.drawDuration(tt.window)) + }) + } +} + +// TestTrigger_DurationJitterStaysWithinBounds covers the jitter as Trigger applies +// it, over real draws: every deadline baked into an id falls inside the window's +// stated bounds, so an integrator can size a poll loop from the configuration +// alone. +func TestTrigger_DurationJitterStaysWithinBounds(t *testing.T) { + const ( + window = 30 * time.Second + jitter = 20 + ) + r := newFake(t, Params{BuildDuration: window, DurationJitterPercent: jitter}) + + varied := false + var first int64 + for i := range 50 { + before := time.Now() + id, err := r.Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) + require.NoError(t, err) + after := time.Now() + + _, readyAt := decodeID(id.ID) + + // Trigger measured the deadline from an instant inside [before, after], + // so the loosest true bounds shift each end by the call's own duration. + low := before.Add(window * (100 - jitter) / 100).UnixMilli() + high := after.Add(window * (100 + jitter) / 100).UnixMilli() + assert.GreaterOrEqual(t, readyAt, low) + assert.LessOrEqual(t, readyAt, high) + + if i == 0 { + first = readyAt + } else if readyAt != first { + varied = true + } + } + // The draws are random, so this asserts only that they are not all identical — + // 50 draws over a 12s span collide only if the jitter is not applied at all. + assert.True(t, varied, "expected jittered deadlines to vary across triggers") +} + +// TestStatus_BuildDurationReportsRunningFirst covers a configured build duration +// applying to every build, not just marked ones, and holding the build +// non-terminal until the window elapses. +func TestStatus_BuildDurationReportsRunningFirst(t *testing.T) { + // A window long enough that the build is still running when Status is called. + r := newFake(t, Params{FailurePercent: 100, BuildDuration: 30 * time.Second}) + + id, err := r.Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) require.NoError(t, err) - assert.Equal(t, entity.BuildStatusSucceeded, status) + + status, _, err := r.Status(context.Background(), id) + require.NoError(t, err) + assert.Equal(t, entity.BuildStatusRunning, status) + + // The same build reports its configured outcome once the window has elapsed. + elapsed := entity.BuildID{ID: fmt.Sprintf("fake-%s-%d-abcd1234", _tokenFail, time.Now().UnixMilli()-1)} + status, _, err = r.Status(context.Background(), elapsed) + require.NoError(t, err) + assert.Equal(t, entity.BuildStatusFailed, status) +} + +// TestStatus_MarkerOverridesConfiguredRate covers a marker pinning the outcome for +// its own build, so a stack running a failure rate can still ask for a specific +// outcome per request. +func TestStatus_MarkerOverridesConfiguredRate(t *testing.T) { + tests := []struct { + name string + failurePercent int + headURI string + wantStatus entity.BuildStatus + }{ + { + name: "fail marker under a never-fail rate", + failurePercent: 0, + headURI: "git://repo/ref/deadbeef?buildrunner-fake=build-fail", + wantStatus: entity.BuildStatusFailed, + }, + { + name: "unmarked build under an always-fail rate", + failurePercent: 100, + headURI: "git://repo/ref/deadbeef", + wantStatus: entity.BuildStatusFailed, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := newFake(t, Params{FailurePercent: tt.failurePercent}) + + id, err := r.Trigger(context.Background(), "", tt.headURI, nil) + require.NoError(t, err) + + status, _, err := r.Status(context.Background(), id) + require.NoError(t, err) + assert.Equal(t, tt.wantStatus, status) + }) + } } // TestMarker_StopsAtPathSegment covers a marker that arrives mid-URI rather than at the From 338e5a681da2999f31c8a050bec1972d92b81c47 Mon Sep 17 00:00:00 2001 From: mnoah1 Date: Mon, 10 Aug 2026 23:40:26 +0000 Subject: [PATCH 2/2] feat(stovepipe): add sampler build runner splitting traffic by percentage The sampler wraps two BuildRunners and routes a configured share of builds to the candidate, so a backend rollout or comparison can be exercised on live traffic without a real CI system. The sample is drawn per Trigger, so the percentage is a rate over many builds rather than a guarantee. Status and Cancel have to reach whichever runner minted a build's opaque id, so the sampler tags each id with the slot behind it and strips the tag before delegating; untagged ids route to the baseline. Tagging rather than bookkeeping keeps it stateless across redeliveries and replicas, and lets samplers nest. BUILD_RUNNER=sampler selects it in the wiring layer, which grows a second set of BUILD_RUNNER_CANDIDATE_* knobs so the two runners can be given different profiles. --- service/stovepipe/README.md | 31 +- service/stovepipe/docker-compose.yml | 12 +- service/stovepipe/server/BUILD.bazel | 1 + service/stovepipe/server/main.go | 122 ++++++-- stovepipe/extension/buildrunner/README.md | 4 +- .../extension/buildrunner/sampler/BUILD.bazel | 28 ++ .../extension/buildrunner/sampler/sampler.go | 196 ++++++++++++ .../buildrunner/sampler/sampler_test.go | 291 ++++++++++++++++++ 8 files changed, 649 insertions(+), 36 deletions(-) create mode 100644 stovepipe/extension/buildrunner/sampler/BUILD.bazel create mode 100644 stovepipe/extension/buildrunner/sampler/sampler.go create mode 100644 stovepipe/extension/buildrunner/sampler/sampler_test.go diff --git a/service/stovepipe/README.md b/service/stovepipe/README.md index 55075dc7..4a96d146 100644 --- a/service/stovepipe/README.md +++ b/service/stovepipe/README.md @@ -16,7 +16,7 @@ Stovepipe therefore needs two MySQL databases: a **storage** database (the `requ - **`inMemoryCounter`** — a process-local `counter.Counter` for sequence numbers; not durable. A real deployment uses a persistent implementation (e.g. `platform/extension/counter/mysql`). - **`fakeSourceControlFactory`** — seeds each queue with a deterministic single-commit history so ingest resolves a stable head URI (and re-ingesting the same queue exercises the dedup path). A real deployment supplies a VCS-backed `sourcecontrol.Factory`. -- **`newBuildRunnerFactory`** — builds the `buildrunner.Factory` from the environment. Each queue gets its own fake runner, bound to that queue's `Config` and sharing the profile the `BUILD_RUNNER_*` knobs describe (failure rate, build duration, and its spread). A real deployment keeps this shape and swaps the constructed runner for a Buildkite or GitHub Actions one (see [`stovepipe/extension/buildrunner`](../../stovepipe/extension/buildrunner)); a deployment that gives queues *different* backends routes on `Config.QueueName` in `For`, as the SubmitQueue orchestrator does in its `profiles.go`. +- **`newBuildRunnerFactory`** — builds the `buildrunner.Factory` from the environment. Every runner it hands out is bound to the resolving queue's `Config` and shares the profile the `BUILD_RUNNER_*` knobs describe (failure rate, build duration, and its spread). `BUILD_RUNNER=fake` (the default) gives each queue one fake runner; `BUILD_RUNNER=sampler` gives it two and splits builds between them by percentage, which is how a backend rollout or comparison is exercised without a real CI system. A real deployment keeps this shape and swaps the constructed runners for Buildkite or GitHub Actions ones (see [`stovepipe/extension/buildrunner`](../../stovepipe/extension/buildrunner)); a deployment that gives queues *different* backends routes on `Config.QueueName` in `For`, as the SubmitQueue orchestrator does in its `profiles.go`. ## Layout @@ -43,24 +43,31 @@ The Stovepipe controllers live under [`stovepipe/controller/`](../../stovepipe/c ### Build runner -These knobs configure the fake runner every queue shares. A malformed value fails startup rather than silently falling back to the default. +`BUILD_RUNNER` selects the implementation; the bare `BUILD_RUNNER_*` knobs configure the runner that takes unsampled builds, and the `_CANDIDATE_` ones configure the runner that `sampler` splits traffic with. A malformed value fails startup rather than silently falling back to the default. -| Variable | Required | Description | Default | -|------------------------------------------|----------|--------------------------------------------------------------|---------| -| `BUILD_RUNNER_FAILURE_PERCENT` | no | Share of builds (0-100) the runner reports as failed | `0` | -| `BUILD_RUNNER_DURATION_MS` | no | How long each build reports running before turning terminal | `0` | -| `BUILD_RUNNER_DURATION_JITTER_PERCENT` | no | Spread (0-100) applied to that duration per build | `0` | +| Variable | Required | Description | Default | +|---------------------------------------------------|----------|--------------------------------------------------------------------------|---------| +| `BUILD_RUNNER` | no | `fake` or `sampler` | `fake` | +| `BUILD_RUNNER_FAILURE_PERCENT` | no | Share of builds (0-100) the runner reports as failed | `0` | +| `BUILD_RUNNER_DURATION_MS` | no | How long each build reports running before turning terminal | `0` | +| `BUILD_RUNNER_DURATION_JITTER_PERCENT` | no | Spread (0-100) applied to that duration per build | `0` | +| `BUILD_RUNNER_SAMPLE_PERCENT` | no | Share of builds (0-100) routed to the candidate runner | `0` | +| `BUILD_RUNNER_CANDIDATE_FAILURE_PERCENT` | no | Failure share for the candidate runner | `0` | +| `BUILD_RUNNER_CANDIDATE_DURATION_MS` | no | Build duration for the candidate runner | `0` | +| `BUILD_RUNNER_CANDIDATE_DURATION_JITTER_PERCENT` | no | Duration spread for the candidate runner | `0` | -The defaults reproduce the original behavior: a fake runner that succeeds immediately unless a request's head URI carries a `buildrunner-fake=` marker. Markers keep working under a configured rate — they pin the outcome for the build that carries them. +The defaults reproduce the original behavior: one fake runner that succeeds immediately unless a request's head URI carries a `buildrunner-fake=` marker. Markers keep working under a configured rate — they pin the outcome for the build that carries them. Durations are configured as a typical value plus a spread, so the bounds stay easy to state: `BUILD_RUNNER_DURATION_MS=60000` with `BUILD_RUNNER_DURATION_JITTER_PERCENT=25` means every build takes between 45s and 75s, drawn uniformly. A jitter of `0` pins every build to exactly the configured duration; `100` is the widest setting, spanning "terminal immediately" to twice the duration. -Under Compose these are set through `SQ_`-prefixed variables, so a stack whose builds are slow and flaky starts with: +Under Compose these are set through `SQ_`-prefixed variables, so a sampled stack that sends a tenth of its builds to a slow, flaky runner starts with: ```bash -SQ_BUILD_RUNNER_FAILURE_PERCENT=20 \ -SQ_BUILD_RUNNER_DURATION_MS=5000 \ -SQ_BUILD_RUNNER_DURATION_JITTER_PERCENT=40 \ +SQ_BUILD_RUNNER=sampler \ +SQ_BUILD_RUNNER_SAMPLE_PERCENT=10 \ +SQ_BUILD_RUNNER_CANDIDATE_FAILURE_PERCENT=50 \ +SQ_BUILD_RUNNER_CANDIDATE_DURATION_MS=5000 \ +SQ_BUILD_RUNNER_CANDIDATE_DURATION_JITTER_PERCENT=40 \ make local-stovepipe-start ``` diff --git a/service/stovepipe/docker-compose.yml b/service/stovepipe/docker-compose.yml index 6e7216cf..e3a238db 100644 --- a/service/stovepipe/docker-compose.yml +++ b/service/stovepipe/docker-compose.yml @@ -68,12 +68,18 @@ services: - STORAGE_MYSQL_DSN=root:root@tcp(mysql-app:3306)/submitqueue?parseTime=true - QUEUE_MYSQL_DSN=root:root@tcp(mysql-queue:3306)/submitqueue?parseTime=true - HOSTNAME=stovepipe-dev - # Fake build runner behavior. Unset means the default: every build - # succeeds immediately unless a request's head URI carries a - # buildrunner-fake marker — see the README. + # Build runner selection and behavior. Unset means the default: a single + # fake runner that succeeds immediately unless a request's head URI + # carries a buildrunner-fake marker. Set SQ_BUILD_RUNNER=sampler to split + # builds between two fakes with different profiles — see the README. + - BUILD_RUNNER=${SQ_BUILD_RUNNER:-} - BUILD_RUNNER_FAILURE_PERCENT=${SQ_BUILD_RUNNER_FAILURE_PERCENT:-} - BUILD_RUNNER_DURATION_MS=${SQ_BUILD_RUNNER_DURATION_MS:-} - BUILD_RUNNER_DURATION_JITTER_PERCENT=${SQ_BUILD_RUNNER_DURATION_JITTER_PERCENT:-} + - BUILD_RUNNER_SAMPLE_PERCENT=${SQ_BUILD_RUNNER_SAMPLE_PERCENT:-} + - BUILD_RUNNER_CANDIDATE_FAILURE_PERCENT=${SQ_BUILD_RUNNER_CANDIDATE_FAILURE_PERCENT:-} + - BUILD_RUNNER_CANDIDATE_DURATION_MS=${SQ_BUILD_RUNNER_CANDIDATE_DURATION_MS:-} + - BUILD_RUNNER_CANDIDATE_DURATION_JITTER_PERCENT=${SQ_BUILD_RUNNER_CANDIDATE_DURATION_JITTER_PERCENT:-} depends_on: mysql-app: condition: service_healthy diff --git a/service/stovepipe/server/BUILD.bazel b/service/stovepipe/server/BUILD.bazel index c7663829..1244f68e 100644 --- a/service/stovepipe/server/BUILD.bazel +++ b/service/stovepipe/server/BUILD.bazel @@ -25,6 +25,7 @@ go_library( "//stovepipe/core/messagequeue:go_default_library", "//stovepipe/extension/buildrunner:go_default_library", "//stovepipe/extension/buildrunner/fake:go_default_library", + "//stovepipe/extension/buildrunner/sampler:go_default_library", "//stovepipe/extension/queueconfig/default:go_default_library", "//stovepipe/extension/sourcecontrol:go_default_library", "//stovepipe/extension/sourcecontrol/fake:go_default_library", diff --git a/service/stovepipe/server/main.go b/service/stovepipe/server/main.go index 05c5a1a5..96542659 100644 --- a/service/stovepipe/server/main.go +++ b/service/stovepipe/server/main.go @@ -49,6 +49,7 @@ import ( stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue" "github.com/uber/submitqueue/stovepipe/extension/buildrunner" buildrunnerfake "github.com/uber/submitqueue/stovepipe/extension/buildrunner/fake" + buildrunnersampler "github.com/uber/submitqueue/stovepipe/extension/buildrunner/sampler" queueconfigdefault "github.com/uber/submitqueue/stovepipe/extension/queueconfig/default" "github.com/uber/submitqueue/stovepipe/extension/sourcecontrol" sourcecontrolfake "github.com/uber/submitqueue/stovepipe/extension/sourcecontrol/fake" @@ -138,11 +139,34 @@ func (fakeSourceControlFactory) For(cfg sourcecontrol.Config) (sourcecontrol.Sou return sourcecontrolfake.New(cfg, []string{fmt.Sprintf("git://%s/HEAD", cfg.QueueName)}), nil } -// Environment variables configuring the fake BuildRunner. +// Environment variables selecting and configuring the BuildRunner. The bare +// BUILD_RUNNER_* knobs configure the runner that takes unsampled builds; the +// _CANDIDATE_ knobs configure the one BUILD_RUNNER=sampler splits traffic with. const ( - _envFailurePercent = "BUILD_RUNNER_FAILURE_PERCENT" - _envBuildDurationMs = "BUILD_RUNNER_DURATION_MS" - _envDurationJitterPercent = "BUILD_RUNNER_DURATION_JITTER_PERCENT" + _envBuildRunner = "BUILD_RUNNER" + _envSamplePercent = "BUILD_RUNNER_SAMPLE_PERCENT" +) + +// fakeEnv names the environment variables configuring one fake build runner. +// Two sets exist so the sampler's two runners can be given different profiles +// from the same code path. +type fakeEnv struct { + failurePercent string + buildDurationMs string + durationJitterPercent string +} + +var ( + _baselineFakeEnv = fakeEnv{ + failurePercent: "BUILD_RUNNER_FAILURE_PERCENT", + buildDurationMs: "BUILD_RUNNER_DURATION_MS", + durationJitterPercent: "BUILD_RUNNER_DURATION_JITTER_PERCENT", + } + _candidateFakeEnv = fakeEnv{ + failurePercent: "BUILD_RUNNER_CANDIDATE_FAILURE_PERCENT", + buildDurationMs: "BUILD_RUNNER_CANDIDATE_DURATION_MS", + durationJitterPercent: "BUILD_RUNNER_CANDIDATE_DURATION_JITTER_PERCENT", + } ) // fakeBuildRunnerFactory is the example BuildRunner factory: every queue gets a stateless fake @@ -158,37 +182,95 @@ func (f fakeBuildRunnerFactory) For(cfg buildrunner.Config) (buildrunner.BuildRu return buildrunnerfake.New(params) } -// newBuildRunnerFactory builds the BuildRunner factory from the environment, so -// every queue's fake runner shares the profile set by the BUILD_RUNNER_* knobs. -// The fake is demo-only; a real deployment keeps this shape and swaps the -// constructed runner for a Buildkite or GitHub Actions one. +// samplerBuildRunnerFactory gives each queue a sampler over two fake runners +// bound to that queue, so the split applies per queue rather than through one +// process-wide instance. +type samplerBuildRunnerFactory struct { + baseline buildrunnerfake.Params + candidate buildrunnerfake.Params + candidatePercent int + logger *zap.SugaredLogger +} + +func (f samplerBuildRunnerFactory) For(cfg buildrunner.Config) (buildrunner.BuildRunner, error) { + baseline, err := fakeBuildRunnerFactory{params: f.baseline}.For(cfg) + if err != nil { + return nil, err + } + candidate, err := fakeBuildRunnerFactory{params: f.candidate}.For(cfg) + if err != nil { + return nil, err + } + return buildrunnersampler.New(buildrunnersampler.Params{ + Config: cfg, + Baseline: baseline, + Candidate: candidate, + CandidatePercent: f.candidatePercent, + Logger: f.logger, + }) +} + +// newBuildRunnerFactory builds the BuildRunner factory from the environment. +// BUILD_RUNNER selects the implementation: "fake" (the default) gives every +// queue one fake runner, and "sampler" gives it two and splits builds between +// them by percentage, which is how a rollout or a backend comparison is +// exercised locally. Both are demo-only; a real deployment keeps this shape and +// swaps the constructed runners for Buildkite or GitHub Actions ones. func newBuildRunnerFactory(logger *zap.SugaredLogger) (buildrunner.Factory, error) { - params, err := fakeBuildRunnerParams() + baseline, err := fakeBuildRunnerParams(_baselineFakeEnv) if err != nil { return nil, err } - // Runners are built per resolution, so a profile the extension rejects - // would not surface until the first build. Build one here and discard it + + var factory buildrunner.Factory + switch impl := strings.ToLower(strings.TrimSpace(os.Getenv(_envBuildRunner))); impl { + case "", "fake": + factory = fakeBuildRunnerFactory{params: baseline} + logger.Infow("build runner configured", "impl", "fake") + + case "sampler": + candidate, err := fakeBuildRunnerParams(_candidateFakeEnv) + if err != nil { + return nil, err + } + candidatePercent, err := envInt(_envSamplePercent) + if err != nil { + return nil, err + } + factory = samplerBuildRunnerFactory{ + baseline: baseline, + candidate: candidate, + candidatePercent: candidatePercent, + logger: logger, + } + logger.Infow("build runner configured", "impl", "sampler", "candidate_percent", candidatePercent) + + default: + return nil, fmt.Errorf("invalid %s %q", _envBuildRunner, impl) + } + + // Runners are built per resolution, so a profile the extensions reject + // would not surface until the first build. Resolve one here and discard it // to keep that failure at startup. - if _, err := buildrunnerfake.New(params); err != nil { + if _, err := factory.For(buildrunner.Config{}); err != nil { return nil, err } - logger.Infow("build runner configured", "impl", "fake") - return fakeBuildRunnerFactory{params: params}, nil + return factory, nil } -// fakeBuildRunnerParams reads the fake runner's profile from the environment, -// leaving Config for the factory to fill in per queue. -func fakeBuildRunnerParams() (buildrunnerfake.Params, error) { - failurePercent, err := envInt(_envFailurePercent) +// fakeBuildRunnerParams reads one fake runner's profile from the named +// environment variables, leaving Config for the factory to fill in per queue. +// The caller varies the names to give several runners different profiles. +func fakeBuildRunnerParams(env fakeEnv) (buildrunnerfake.Params, error) { + failurePercent, err := envInt(env.failurePercent) if err != nil { return buildrunnerfake.Params{}, err } - buildDuration, err := envDurationMs(_envBuildDurationMs) + buildDuration, err := envDurationMs(env.buildDurationMs) if err != nil { return buildrunnerfake.Params{}, err } - durationJitterPercent, err := envInt(_envDurationJitterPercent) + durationJitterPercent, err := envInt(env.durationJitterPercent) if err != nil { return buildrunnerfake.Params{}, err } diff --git a/stovepipe/extension/buildrunner/README.md b/stovepipe/extension/buildrunner/README.md index ae78c491..748b100c 100644 --- a/stovepipe/extension/buildrunner/README.md +++ b/stovepipe/extension/buildrunner/README.md @@ -12,4 +12,6 @@ Real backends (`buildkite`, `githubactions`) are thin adapters over a shared pla `fake` is the stub for local stacks and tests. By default every build succeeds immediately; its `Params` set a failure rate and a build duration — optionally spread by a percentage so durations vary within bounds the configuration states outright — for every build, and a `buildrunner-fake=` marker in a request's head URI pins the outcome or the running window for one build. It keeps no per-build state — the outcome and the instant the build turns terminal are encoded in the build id — so `Status` can be answered by any instance in any process. Never production. -See [doc/rfc/stovepipe/steps/build.md](../../../doc/rfc/stovepipe/steps/build.md#why-separate-contracts) for why this is a separate contract from SubmitQueue's own `buildrunner` rather than a shared one. To add a backend, create `buildrunner/{backend}/`, implement `BuildRunner`, and return it from a `New(...)` constructor. Which backend serves which queue is decided in the wiring layer ([`service/stovepipe/server`](../../../service/stovepipe/server)), not here. +`sampler` is a composite rather than a backend: it wraps two other `BuildRunner`s and sends a configured percentage of builds to the second one, which is how a new backend is rolled out gradually or compared against the incumbent on live traffic. The sample is drawn per `Trigger`, so the percentage is a rate over many builds. Because `Status` and `Cancel` have to reach whichever runner minted a build's opaque id, the sampler tags each id with the runner behind it and strips the tag before delegating; untagged ids route to the baseline. That tagging is what keeps it stateless across redeliveries and replicas, and it lets samplers nest. + +See [doc/rfc/stovepipe/steps/build.md](../../../doc/rfc/stovepipe/steps/build.md#why-separate-contracts) for why this is a separate contract from SubmitQueue's own `buildrunner` rather than a shared one. To add a backend, create `buildrunner/{backend}/`, implement `BuildRunner`, and return it from a `New(...)` constructor. Which backend serves which queue — and whether a queue gets a sampler at all — is decided in the wiring layer ([`service/stovepipe/server`](../../../service/stovepipe/server)), not here. diff --git a/stovepipe/extension/buildrunner/sampler/BUILD.bazel b/stovepipe/extension/buildrunner/sampler/BUILD.bazel new file mode 100644 index 00000000..1532cd57 --- /dev/null +++ b/stovepipe/extension/buildrunner/sampler/BUILD.bazel @@ -0,0 +1,28 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["sampler.go"], + importpath = "github.com/uber/submitqueue/stovepipe/extension/buildrunner/sampler", + visibility = ["//visibility:public"], + deps = [ + "//stovepipe/entity:go_default_library", + "//stovepipe/extension/buildrunner:go_default_library", + "@org_uber_go_zap//:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["sampler_test.go"], + embed = [":go_default_library"], + deps = [ + "//stovepipe/entity:go_default_library", + "//stovepipe/extension/buildrunner:go_default_library", + "//stovepipe/extension/buildrunner/mock:go_default_library", + "@com_github_stretchr_testify//assert:go_default_library", + "@com_github_stretchr_testify//require:go_default_library", + "@org_uber_go_mock//gomock:go_default_library", + "@org_uber_go_zap//:go_default_library", + ], +) diff --git a/stovepipe/extension/buildrunner/sampler/sampler.go b/stovepipe/extension/buildrunner/sampler/sampler.go new file mode 100644 index 00000000..196c4104 --- /dev/null +++ b/stovepipe/extension/buildrunner/sampler/sampler.go @@ -0,0 +1,196 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package sampler implements buildrunner.BuildRunner by splitting builds across +// two other BuildRunners: a baseline that takes most of the traffic and a +// candidate that takes a configured percentage of it. It is the mechanism for +// rolling a new build backend out gradually, or for comparing two backends on +// live traffic, without deploying separate queues. +// +// The sample is drawn per Trigger, so the percentage is a rate over many builds +// rather than a guarantee about any fixed number of them. Because the split is +// per build and not per queue, a sampler is composed rather than routed to: the +// wiring layer decides which queues get a sampler, and the sampler decides which +// builds within those queues get the candidate. +// +// Status and Cancel must reach the same runner that minted a build's id, and a +// BuildRunner id is opaque — nothing else can tell whose it is. The sampler +// therefore tags each id with the runner that produced it and strips the tag +// before delegating. Tagging keeps the sampler stateless the same way the fake +// runner's encoded outcome does: any sampler instance, in any process, routes an +// id the same way, so redelivery and multi-replica deployments need no shared +// bookkeeping. Ids that carry no tag — minted before the sampler was wired, or by +// a runner used directly — route to the baseline. +package sampler + +import ( + "context" + "fmt" + mathrand "math/rand/v2" + "strings" + + "go.uber.org/zap" + + "github.com/uber/submitqueue/stovepipe/entity" + "github.com/uber/submitqueue/stovepipe/extension/buildrunner" +) + +// _idPrefix introduces the routing tag on every BuildID the sampler mints: +// "sampler--". The delegate's id keeps its own shape +// inside the tag, so a delegate that is itself a sampler nests cleanly. +const _idPrefix = "sampler-" + +// Slot names identifying which delegate minted a build, carried in the id. +const ( + _slotBaseline = "baseline" + _slotCandidate = "candidate" +) + +// _percentScale is the draw range for a percentage sample: a draw in [0,100) +// compared against a percentage in [0,100] yields that percentage of hits. +const _percentScale = 100 + +// Params holds the dependencies and split for a sampling BuildRunner. +type Params struct { + // Config holds the per-queue identity for this BuildRunner. + Config buildrunner.Config + + // Baseline runs every build not sampled into the candidate. Required. + Baseline buildrunner.BuildRunner + + // Candidate runs the sampled share of builds. Required even at a + // CandidatePercent of 0, because Status and Cancel still have to reach it + // for builds a previous, higher percentage sent its way. + Candidate buildrunner.BuildRunner + + // CandidatePercent is the share of builds, in percent, routed to Candidate. + // Valid range is 0-100; the zero value sends everything to Baseline. + CandidatePercent int + + // Logger is the structured logger. Required. + Logger *zap.SugaredLogger +} + +// runner implements buildrunner.BuildRunner. +type runner struct { + // cfg is the per-queue identity this runner was built for. + cfg buildrunner.Config + baseline buildrunner.BuildRunner + candidate buildrunner.BuildRunner + candidatePercent int + logger *zap.SugaredLogger + + // intn draws the routing sample from [0,n). A field rather than a direct + // call to math/rand so tests can pin the draw. + intn func(n int) int +} + +var _ buildrunner.BuildRunner = (*runner)(nil) + +// New constructs a BuildRunner that routes params.CandidatePercent of builds to +// the candidate and the rest to the baseline. It rejects a missing delegate or a +// percentage outside 0-100 so a misconfigured split fails at wiring time rather +// than at the first build. +func New(params Params) (buildrunner.BuildRunner, error) { + if params.Baseline == nil { + return nil, fmt.Errorf("sampler: baseline build runner is required") + } + if params.Candidate == nil { + return nil, fmt.Errorf("sampler: candidate build runner is required") + } + if params.CandidatePercent < 0 || params.CandidatePercent > _percentScale { + return nil, fmt.Errorf("sampler: candidate percent %d outside 0-100", params.CandidatePercent) + } + if params.Logger == nil { + return nil, fmt.Errorf("sampler: logger is required") + } + return &runner{ + cfg: params.Config, + baseline: params.Baseline, + candidate: params.Candidate, + candidatePercent: params.CandidatePercent, + logger: params.Logger.Named("sampler_buildrunner"), + intn: mathrand.IntN, + }, nil +} + +// Trigger draws a delegate for this build and returns its id tagged with the +// delegate that minted it. A delegate's error is propagated rather than retried +// against the other delegate: a sampled rollout exists to surface the sampled +// backend's failures, and silently covering for it would hide the very signal +// the split was set up to collect. +func (r *runner) Trigger(ctx context.Context, baseURI, headURI string, metadata entity.BuildMetadata) (entity.BuildID, error) { + delegate, slot := r.draw() + buildID, err := delegate.Trigger(ctx, baseURI, headURI, metadata) + if err != nil { + return entity.BuildID{}, fmt.Errorf("sampler: %s trigger: %w", slot, err) + } + + tagged := entity.BuildID{ID: _idPrefix + slot + "-" + buildID.ID} + r.logger.Debugw("routed build", + "queue", r.cfg.QueueName, + "slot", slot, + "candidate_percent", r.candidatePercent, + "build_id", tagged.ID, + ) + return tagged, nil +} + +// Status delegates to the runner that minted buildID, with the routing tag +// stripped so the delegate sees the id it issued. +func (r *runner) Status(ctx context.Context, buildID entity.BuildID) (entity.BuildStatus, entity.BuildMetadata, error) { + delegate, slot, delegateID := r.route(buildID) + status, metadata, err := delegate.Status(ctx, delegateID) + if err != nil { + return entity.BuildStatusUnknown, nil, fmt.Errorf("sampler: %s status: %w", slot, err) + } + return status, metadata, nil +} + +// Cancel delegates to the runner that minted buildID, with the routing tag +// stripped so the delegate sees the id it issued. +func (r *runner) Cancel(ctx context.Context, buildID entity.BuildID) error { + delegate, slot, delegateID := r.route(buildID) + if err := delegate.Cancel(ctx, delegateID); err != nil { + return fmt.Errorf("sampler: %s cancel: %w", slot, err) + } + return nil +} + +// draw samples the delegate for a new build. +func (r *runner) draw() (buildrunner.BuildRunner, string) { + if r.candidatePercent > 0 && r.intn(_percentScale) < r.candidatePercent { + return r.candidate, _slotCandidate + } + return r.baseline, _slotBaseline +} + +// route resolves the delegate that minted buildID from the tag Trigger added, +// returning it alongside the delegate's own untagged id. An untagged or +// unrecognized id routes to the baseline unchanged: the baseline is the runner +// that handled traffic before the sampler was wired, so it is the only delegate +// that can plausibly own an id the sampler never minted. +func (r *runner) route(buildID entity.BuildID) (buildrunner.BuildRunner, string, entity.BuildID) { + if tag, found := strings.CutPrefix(buildID.ID, _idPrefix); found { + if slot, delegateID, split := strings.Cut(tag, "-"); split { + switch slot { + case _slotBaseline: + return r.baseline, _slotBaseline, entity.BuildID{ID: delegateID} + case _slotCandidate: + return r.candidate, _slotCandidate, entity.BuildID{ID: delegateID} + } + } + } + return r.baseline, _slotBaseline, buildID +} diff --git a/stovepipe/extension/buildrunner/sampler/sampler_test.go b/stovepipe/extension/buildrunner/sampler/sampler_test.go new file mode 100644 index 00000000..7f7f8610 --- /dev/null +++ b/stovepipe/extension/buildrunner/sampler/sampler_test.go @@ -0,0 +1,291 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sampler + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + "go.uber.org/zap" + + "github.com/uber/submitqueue/stovepipe/entity" + "github.com/uber/submitqueue/stovepipe/extension/buildrunner" + buildrunnermock "github.com/uber/submitqueue/stovepipe/extension/buildrunner/mock" +) + +// fixedDraw returns a sampling function that always draws value, pinning which +// delegate a Trigger routes to. +func fixedDraw(value int) func(int) int { + return func(int) int { return value } +} + +// newTestRunner builds a sampler over two mock delegates with a pinned draw, so +// routing is asserted without depending on the random source. +func newTestRunner(t *testing.T, candidatePercent, draw int) (*runner, *buildrunnermock.MockBuildRunner, *buildrunnermock.MockBuildRunner) { + t.Helper() + ctrl := gomock.NewController(t) + baseline := buildrunnermock.NewMockBuildRunner(ctrl) + candidate := buildrunnermock.NewMockBuildRunner(ctrl) + return &runner{ + baseline: baseline, + candidate: candidate, + candidatePercent: candidatePercent, + logger: zap.NewNop().Sugar(), + intn: fixedDraw(draw), + }, baseline, candidate +} + +func TestNew(t *testing.T) { + ctrl := gomock.NewController(t) + delegate := buildrunnermock.NewMockBuildRunner(ctrl) + + tests := []struct { + name string + params Params + wantErr bool + }{ + { + name: "valid params", + params: Params{Baseline: delegate, Candidate: delegate, CandidatePercent: 10, Logger: zap.NewNop().Sugar()}, + }, + { + name: "zero percent is valid", + params: Params{Baseline: delegate, Candidate: delegate, Logger: zap.NewNop().Sugar()}, + }, + { + name: "full percent is valid", + params: Params{Baseline: delegate, Candidate: delegate, CandidatePercent: 100, Logger: zap.NewNop().Sugar()}, + }, + { + name: "missing baseline", + params: Params{Candidate: delegate, Logger: zap.NewNop().Sugar()}, + wantErr: true, + }, + { + name: "missing candidate", + params: Params{Baseline: delegate, Logger: zap.NewNop().Sugar()}, + wantErr: true, + }, + { + name: "negative percent", + params: Params{Baseline: delegate, Candidate: delegate, CandidatePercent: -1, Logger: zap.NewNop().Sugar()}, + wantErr: true, + }, + { + name: "percent above 100", + params: Params{Baseline: delegate, Candidate: delegate, CandidatePercent: 101, Logger: zap.NewNop().Sugar()}, + wantErr: true, + }, + { + name: "missing logger", + params: Params{Baseline: delegate, Candidate: delegate}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := New(tt.params) + if tt.wantErr { + require.Error(t, err) + assert.Nil(t, got) + return + } + require.NoError(t, err) + var _ buildrunner.BuildRunner = got + }) + } +} + +func TestTrigger_RoutesByDraw(t *testing.T) { + tests := []struct { + name string + candidatePercent int + draw int + wantSlot string + }{ + {name: "zero percent never samples", candidatePercent: 0, draw: 0, wantSlot: _slotBaseline}, + {name: "full percent always samples", candidatePercent: 100, draw: 99, wantSlot: _slotCandidate}, + {name: "draw below percent samples", candidatePercent: 25, draw: 24, wantSlot: _slotCandidate}, + {name: "draw at percent does not sample", candidatePercent: 25, draw: 25, wantSlot: _slotBaseline}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r, baseline, candidate := newTestRunner(t, tt.candidatePercent, tt.draw) + routed := baseline + if tt.wantSlot == _slotCandidate { + routed = candidate + } + routed.EXPECT(). + Trigger(gomock.Any(), "base", "head", entity.BuildMetadata{"k": "v"}). + Return(entity.BuildID{ID: "delegate-id"}, nil) + + got, err := r.Trigger(context.Background(), "base", "head", entity.BuildMetadata{"k": "v"}) + require.NoError(t, err) + assert.Equal(t, fmt.Sprintf("sampler-%s-delegate-id", tt.wantSlot), got.ID) + }) + } +} + +// TestTrigger_SplitsTrafficWithRealDraws covers the actual promise of the sampler +// — that a middling percentage reaches both delegates — against the random source +// the wiring layer uses, which the pinned-draw tests deliberately bypass. The +// MinTimes(1) expectations on both delegates are the assertion. +func TestTrigger_SplitsTrafficWithRealDraws(t *testing.T) { + ctrl := gomock.NewController(t) + baseline := buildrunnermock.NewMockBuildRunner(ctrl) + candidate := buildrunnermock.NewMockBuildRunner(ctrl) + baseline.EXPECT().Trigger(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(entity.BuildID{ID: "baseline-id"}, nil).MinTimes(1) + candidate.EXPECT().Trigger(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(entity.BuildID{ID: "candidate-id"}, nil).MinTimes(1) + + r, err := New(Params{ + Baseline: baseline, + Candidate: candidate, + CandidatePercent: 50, + Logger: zap.NewNop().Sugar(), + }) + require.NoError(t, err) + + for range 100 { + _, err := r.Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) + require.NoError(t, err) + } +} + +// TestTrigger_DelegateErrorIsNotRetriedElsewhere pins the deliberate absence of a +// fallback: the sampled delegate's failure surfaces instead of being covered by +// the other delegate, which is what makes the split useful as a signal. +func TestTrigger_DelegateErrorIsNotRetriedElsewhere(t *testing.T) { + r, _, candidate := newTestRunner(t, 100, 0) + candidate.EXPECT(). + Trigger(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(entity.BuildID{}, fmt.Errorf("candidate unavailable")) + + got, err := r.Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) + require.Error(t, err) + assert.Empty(t, got.ID) +} + +func TestStatus_RoutesToMintingDelegate(t *testing.T) { + tests := []struct { + name string + buildID string + wantSlot string + wantDelegateID string + }{ + { + name: "baseline tag", + buildID: "sampler-baseline-fake-ok-0-a1b2c3d4", + wantSlot: _slotBaseline, + wantDelegateID: "fake-ok-0-a1b2c3d4", + }, + { + name: "candidate tag", + buildID: "sampler-candidate-12345", + wantSlot: _slotCandidate, + wantDelegateID: "12345", + }, + { + // A delegate that is itself a sampler keeps its own tag intact, so + // samplers nest without either layer misrouting. + name: "nested sampler tag", + buildID: "sampler-candidate-sampler-baseline-fake-ok-0-a1b2c3d4", + wantSlot: _slotCandidate, + wantDelegateID: "sampler-baseline-fake-ok-0-a1b2c3d4", + }, + { + // Ids minted before the sampler was wired carry no tag and belong to + // the runner that was serving traffic then. + name: "untagged id falls back to baseline", + buildID: "fake-ok-0-a1b2c3d4", + wantSlot: _slotBaseline, + wantDelegateID: "fake-ok-0-a1b2c3d4", + }, + { + name: "unknown slot falls back to baseline", + buildID: "sampler-mystery-a1b2c3d4", + wantSlot: _slotBaseline, + wantDelegateID: "sampler-mystery-a1b2c3d4", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r, baseline, candidate := newTestRunner(t, 50, 0) + routed := baseline + if tt.wantSlot == _slotCandidate { + routed = candidate + } + want := entity.BuildID{ID: tt.wantDelegateID} + routed.EXPECT(). + Status(gomock.Any(), want). + Return(entity.BuildStatusRunning, entity.BuildMetadata{"url": "http://build"}, nil) + + status, metadata, err := r.Status(context.Background(), entity.BuildID{ID: tt.buildID}) + require.NoError(t, err) + assert.Equal(t, entity.BuildStatusRunning, status) + assert.Equal(t, entity.BuildMetadata{"url": "http://build"}, metadata) + }) + } +} + +func TestStatus_DelegateError(t *testing.T) { + r, baseline, _ := newTestRunner(t, 0, 0) + baseline.EXPECT(). + Status(gomock.Any(), entity.BuildID{ID: "delegate-id"}). + Return(entity.BuildStatusUnknown, nil, fmt.Errorf("backend down")) + + status, metadata, err := r.Status(context.Background(), entity.BuildID{ID: "sampler-baseline-delegate-id"}) + require.Error(t, err) + assert.Equal(t, entity.BuildStatusUnknown, status) + assert.Nil(t, metadata) +} + +func TestCancel_RoutesToMintingDelegate(t *testing.T) { + r, _, candidate := newTestRunner(t, 0, 0) + candidate.EXPECT().Cancel(gomock.Any(), entity.BuildID{ID: "12345"}).Return(nil) + + require.NoError(t, r.Cancel(context.Background(), entity.BuildID{ID: "sampler-candidate-12345"})) +} + +func TestCancel_DelegateError(t *testing.T) { + r, baseline, _ := newTestRunner(t, 0, 0) + baseline.EXPECT().Cancel(gomock.Any(), gomock.Any()).Return(fmt.Errorf("backend down")) + + require.Error(t, r.Cancel(context.Background(), entity.BuildID{ID: "sampler-baseline-12345"})) +} + +// TestTriggerThenStatus_RoundTrip covers the whole point of tagging: a build's +// status reaches the delegate that minted it, with the delegate's own id restored. +func TestTriggerThenStatus_RoundTrip(t *testing.T) { + r, _, candidate := newTestRunner(t, 100, 0) + candidate.EXPECT(). + Trigger(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(entity.BuildID{ID: "fake-ok-0-a1b2c3d4"}, nil) + candidate.EXPECT(). + Status(gomock.Any(), entity.BuildID{ID: "fake-ok-0-a1b2c3d4"}). + Return(entity.BuildStatusSucceeded, nil, nil) + + buildID, err := r.Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) + require.NoError(t, err) + + status, _, err := r.Status(context.Background(), buildID) + require.NoError(t, err) + assert.Equal(t, entity.BuildStatusSucceeded, status) +}