diff --git a/doc/rfc/stovepipe/steps/build.md b/doc/rfc/stovepipe/steps/build.md index 886fdba3f..c279238e9 100644 --- a/doc/rfc/stovepipe/steps/build.md +++ b/doc/rfc/stovepipe/steps/build.md @@ -54,8 +54,9 @@ For a delivery carrying request id `R`: 6. Persist Build{ID: buildID.ID, RequestID: R.ID, Status: accepted, Version: 1} via BuildStore.Create. - - the row carries no scope; it is recoverable from the Request's immutable fields - (see the entity table). + - the row carries no validation scope; that is recoverable from the Request's immutable + fields (see the entity table). When a build wins the terminal-outcome race, buildsignal + records its id as `Request.TerminalBuildID`. - a crash between step 5 and this write orphans the triggered build (see Idempotency). - ErrAlreadyExists -> benign (reachable only with a backend that returns deterministic ids for retried triggers); continue to step 7. @@ -69,7 +70,7 @@ For a delivery carrying request id `R`: 8. ack. ``` -`Build.ID` is **minted by the runner at `Trigger`**, exactly as in SubmitQueue's build controller: the runner returns its native id (a Buildkite build number, a CI-gateway job id), `build` adopts it as the `Build`'s key, and that same id travels on every hop that needs a build — `build` → `buildsignal` carries the build id in the message, so the poll loop reaches the `Build` by a direct get on identity it was handed. `buildsignal` → `record` carries the **request id** instead: `record`'s unit of work is a Request, and the build's terminal status is projected onto `Request.State` before the publish, so `record` never reaches a `Build` at all. No reader ever *derives* a build id or needs a reverse index; `Build.RequestID` covers the one navigation the pipeline needs in the other direction (`Build` → `Request`). Another approach is deriving the key from the Request (`buildKey(R)`) and/or passing a caller-supplied idempotency key to `Trigger`; see [Alternatives considered](#alternatives-considered-for-the-build-identity) for what each would buy and cost. +`Build.ID` is **minted by the runner at `Trigger`**, exactly as in SubmitQueue's build controller: the runner returns its native id (a Buildkite build number, a CI-gateway job id), `build` adopts it as the `Build`'s key, and that same id travels on every hop that needs a build — `build` → `buildsignal` carries the build id in the message, so the poll loop reaches the `Build` by a direct get on identity it was handed. `buildsignal` → `record` carries the **request id** instead: `record`'s unit of work is a Request, and buildsignal records the winning build id on that Request before publishing. A later artifact reader reads `Request.TerminalBuildID`; it does not query builds by RequestID. Another approach is deriving the key from the Request (`buildKey(R)`) and/or passing a caller-supplied idempotency key to `Trigger`; see [Alternatives considered](#alternatives-considered-for-the-build-identity) for what each would buy and cost. `build` writes only the `Build`, and only at creation; it never mutates `Request.State`. The Request stays `processing` (set by `process`) through `build` until `buildsignal` moves it terminal by recording the build's outcome. `Build.Status` is the fine-grained build lifecycle; `Request.State` is the coarse pipeline lifecycle. This is also what keeps `process.md` step 3 correct: because `build` leaves the Request at `processing`, a redelivered `process` message still matches its "if processing, re-publish to build" guard. @@ -200,7 +201,7 @@ Trigger(ctx context.Context, baseURI, headURI string, projectScope entity.Projec Both `Trigger` and `Status`/`Cancel` differ *in contract* between domains, even though `Status`/`Cancel` happen to be identical in shape: both domains poll and cancel by the same opaque, runner-minted id with the same async semantics. Rather than promoting that shape parity into a shared `platform/base`/`platform/extension/buildrunner` type and interface — which would force a one-time migration of SubmitQueue's already-shipped controllers, storage, and protobuf mappings onto the shared type — each domain keeps its own `BuildRunner` interface and its own local `BuildID`/`BuildStatus`/`BuildMetadata`, and real code reuse happens one layer down, in a shared backend implementation (e.g. a Buildkite client) that both domains' concrete runners wrap. [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract) below records the shapes weighed against this one, including the shared-interface alternative that was set aside. -There is exactly one build id: the runner mints it at `Trigger`, `build` adopts it as `Build.ID`, and every later call and message carries it verbatim — `Status`/`Cancel` take the same value `Trigger` returned, the queue payload is the same value, the store key is the same value. This is SubmitQueue's convention end to end. The id is opaque: no stovepipe reader parses it, derives it, or equates it with another entity's id — the trap SubmitQueue's speculate/cancel path falls into. And per the extension rules a runner keeps only transient local state, so the durable `Request` ↔ `Build` linkage lives in **our** store as `Build.RequestID`, never in the runner. +There is exactly one build id: the runner mints it at `Trigger`, `build` adopts it as `Build.ID`, and every later call and message carries it verbatim — `Status`/`Cancel` take the same value `Trigger` returned, the queue payload is the same value, the store key is the same value. This is SubmitQueue's convention end to end. The id is opaque: no stovepipe reader parses it, derives it, or equates it with another entity's id. The terminal request-history state entry retains the winning id alongside the terminal Request state; it is selected by buildsignal's outcome CAS, not a derived key or a reverse index. Per the extension rules a runner keeps only transient local state, so that entry and `Build.RequestID` live in **our** store, never in the runner. Supporting entity types: `BuildStatus`, `BuildMetadata`, and `BuildID` live in `stovepipe/entity`, shaped the same as SubmitQueue's `submitqueue/entity` equivalents but defined and duplicated locally rather than shared — `BuildStatus` is the narrow lowercase enum `"" (unknown) / accepted / running / succeeded / failed / cancelled` with an `IsTerminal()` predicate covering the last three, `BuildMetadata` is the free-form `map[string]string`, and `BuildID` is a `{ID string}` wire struct wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, `Status`/`Cancel`'s parameter, the queue payload. `stovepipe/entity/build.go` keeps what's stovepipe-specific: the `Build` entity itself (`RequestID` alongside `ID`/`Status`/`Version`). How a target graph reaches `analyze` is out of scope for this doc — left to the `analyze` design. @@ -264,12 +265,12 @@ Key the `Build` by identity derived from the Request — `buildKey(R) = R.ID` fo | Pros | Cons | |---|---| | Redelivery dedup by direct get: checking `BuildStore.Get(buildKey(R))` before triggering means at-least-once delivery never starts a second build | A second id concept (`Build.ID` beside `Build.RunnerBuildID`) carried by every entity, signature, and reader forever | -| `Request` → `Build` navigation with no reverse index, per the KV key-derivation rule in [AGENTS.md](../../../../AGENTS.md) | No current reader needs to *derive* a build id — the id travels in every message hop, so each consumer already holds the key it needs | +| `Request` → `Build` navigation with no reverse index, per the KV key-derivation rule in [AGENTS.md](../../../../AGENTS.md) | `record` needs the winning build's id, which `Request.TerminalBuildID` supplies directly; changing the Build key would still add a second identity | | Enforces (rather than assumes) the direct-navigation property SubmitQueue's speculate takes on faith | Diverges entity shape and controller flow from SubmitQueue, weakening the "structurally the same controller" claim and dual-implementing-backend symmetry | Trade-offs: the dedup guards a rare event at a permanent modeling cost. The duplicate it prevents arises only from a redelivery inside the trigger window — rare, and already harmless (identical scope; `buildsignal`'s superseded short-circuit and its first-writer-wins outcome CAS make the loser a no-op — see [Idempotency](#idempotency)). The prospective key-derivers — a future canceller, or `analyze` reaching back to the Phase-1 target graph — would need to be handed the id by their producing stage instead, if those designs land. -Note that moving `record`'s input from the build id to the request id does *not* trigger this alternative, even though it removes the last hop that carried a build id to a Request-scoped consumer. The trigger condition is a stage that must **derive a build's key from a Request**, and `record` does not: the build's terminal status is projected onto `Request.State` before the publish, so `record` reads the Request and never reaches a `Build`. +Moving `record`'s input from the build id to the request id now requires the winning identity to survive that handoff. The trigger condition for this alternative is still a stage that must **derive a build's key from a Request**. `record` does not: buildsignal retains the terminal state and its winning build id before publishing, so `record` can resolve that state entry without a reverse Build index. #### Alternative B: caller-supplied idempotency key on `Trigger` @@ -318,7 +319,7 @@ The row deliberately carries no scope: `R.URI`, `R.BaseURI`, and `R.BuildStrateg `IsTerminal()` on `entity.BuildStatus` covers exactly the three terminal rows. Once `buildsignal` persists one of them, that status is **write-once** — a later poll reporting a different terminal value never overwrites it (see [buildsignal.md](buildsignal.md#algorithm), step 6). -Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only travels" convention as `RequestID`, shaped like SubmitQueue's own `entity.BuildID` but not the same Go type — see the [contract](#stovepipe-buildrunner-contract)), wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, the queue payload, `Status`/`Cancel`'s parameter. `buildsignal` reaches a build by the id carried in its message, and `record` reads the `Request` (whose state carries the build's outcome) rather than a `Build`, so no reverse index from `Request` to its builds is ever needed. +Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only travels" convention as `RequestID`, shaped like SubmitQueue's own `entity.BuildID` but not the same Go type — see the [contract](#stovepipe-buildrunner-contract)), wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, the queue payload, `Status`/`Cancel`'s parameter. `buildsignal` reaches a build by the id carried in its message, then records the winning id as `Request.TerminalBuildID`. An artifact reader reads that field instead of querying a Build by request. **`BuildStore`** (new, added to the `Storage` aggregator via `GetBuildStore()`), matching stovepipe's existing `RequestStore` conventions — **generic `Update` with caller-owned version arithmetic**: diff --git a/doc/rfc/stovepipe/steps/buildsignal.md b/doc/rfc/stovepipe/steps/buildsignal.md index 2d5d7f5b4..caed8c3e0 100644 --- a/doc/rfc/stovepipe/steps/buildsignal.md +++ b/doc/rfc/stovepipe/steps/buildsignal.md @@ -12,7 +12,7 @@ It handles only the poll loop: it does not decide build strategy, write greennes Its logic does not branch on phase: it loads the `Build`, polls it toward terminal, persists the result, and publishes the request id onward to `record`. What differs between phases is what `record` does with that publish (whole-repo vs. per-project greenness) — not anything `buildsignal` decides. -`buildsignal` is the sole writer of `Build.Status`/`Build.Version` after `build` creates the row (see [build.md](build.md#input-partitioning-and-the-single-writer-property)). It reads `Request` via `RequestStore.Get` (for `R.Queue`, to resolve the build-runner) and writes it exactly once, at the terminal transition, to record the build's outcome — the one `Request.State` write outside `process` and the DLQ reconciler. +`buildsignal` is the sole writer of `Build.Status`/`Build.Version` after `build` creates the row (see [build.md](build.md#input-partitioning-and-the-single-writer-property)). It reads `Request` via `RequestStore.Get` (for `R.Queue`, to resolve the build-runner) and writes it exactly once at the terminal transition, recording the build's outcome and `Request.TerminalBuildID` — the one `Request.State` write outside `process` and the DLQ reconciler. Its early-exit guard is deliberately narrower than `State.IsTerminal()`: it proceeds when the request is `processing` **or** already carries a build outcome. The second case matters because a redelivery after the outcome was stamped but before the `record` publish landed must re-publish rather than drop the signal; everything it re-runs is a no-op (the status is unchanged, the outcome is already recorded, the slot is not released twice) and the `record` publish is idempotent. @@ -63,12 +63,14 @@ For a delivery carrying build id `B`: 7. If the stored status is terminal, and R does not already carry an outcome: a. Release the queue's build slot: CAS-decrement Queue.in_flight_count, clamped at zero. - failure here aborts the step: R must not go terminal while still holding a slot. - b. CAS R from processing to the outcome the stored status projects onto it: + b. CAS R from processing to the outcome the stored status projects onto it and set + R.TerminalBuildID to B: succeeded -> succeeded, failed -> failed, cancelled -> cancelled. First writer wins. Then publish R.ID to the record topic, partitioned by request id; ack, return. No re-publish to buildsignal. - - record loads the Request directly by this key and derives greenness from its outcome, - so it never reaches a Build and no reverse lookup from Request to its builds is needed. + - record loads the Request directly by this key and derives greenness from its outcome. + A later artifact reader reads R.TerminalBuildID; no reverse lookup from Request to its + builds is needed. - the message id is the request id, so a redelivery republishing the same terminal signal dedups into the original message; record is idempotent regardless. - publish failure -> return raw (non-retryable); the outcome is persisted, operational diff --git a/stovepipe/controller/buildsignal/buildsignal.go b/stovepipe/controller/buildsignal/buildsignal.go index 2c56a04b8..d8f40cad5 100644 --- a/stovepipe/controller/buildsignal/buildsignal.go +++ b/stovepipe/controller/buildsignal/buildsignal.go @@ -177,7 +177,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er if err := c.persistBuildFinishedLog(ctx, store, request, build.ID); err != nil { return err } - if err := c.finishRequest(ctx, store, &request, effective); err != nil { + if err := c.finishRequest(ctx, store, &request, effective, build.ID); err != nil { return err } if err := c.persistOutcomeLog(ctx, store, request); err != nil { @@ -234,7 +234,7 @@ func (c *Controller) persistBuildFinishedLog(ctx context.Context, store storage. // the request non-terminal, so redelivery re-runs both steps and decrements again // — transiently over-admitting by one until releaseBuildSlot's zero clamp // reconverges, which is the failure mode this pipeline prefers. -func (c *Controller) finishRequest(ctx context.Context, store storage.Storage, request *entity.Request, status entity.BuildStatus) error { +func (c *Controller) finishRequest(ctx context.Context, store storage.Storage, request *entity.Request, status entity.BuildStatus, buildID string) error { if request.State.HasBuildOutcome() { return nil } @@ -244,7 +244,7 @@ func (c *Controller) finishRequest(ctx context.Context, store storage.Storage, r return err } - if err := c.markOutcome(ctx, store, request, outcomeState(status)); err != nil { + if err := c.markOutcome(ctx, store, request, outcomeState(status), buildID); err != nil { metrics.NamedCounter(c.metricsScope, _opName, "storage_errors", 1, metrics.TagsFromContext(ctx)...) return err } @@ -290,7 +290,7 @@ func outcomeState(status entity.BuildStatus) entity.RequestState { // conflicts. First writer wins: once any outcome is recorded a later caller leaves it // alone, so duplicate builds for one request (which build.md accepts) cannot flip the // verdict back and forth. -func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, request *entity.Request, state entity.RequestState) error { +func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, request *entity.Request, state entity.RequestState, buildID string) error { reqStore := store.GetRequestStore() for { @@ -300,6 +300,7 @@ func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, req updated := *request updated.State = state + updated.TerminalBuildID = buildID newVersion := request.Version + 1 if err := reqStore.Update(ctx, updated, request.Version, newVersion); err != nil { if errors.Is(err, storage.ErrVersionMismatch) { diff --git a/stovepipe/controller/buildsignal/buildsignal_test.go b/stovepipe/controller/buildsignal/buildsignal_test.go index 0bfb76820..f8a408b75 100644 --- a/stovepipe/controller/buildsignal/buildsignal_test.go +++ b/stovepipe/controller/buildsignal/buildsignal_test.go @@ -178,7 +178,9 @@ func expectFinishWrites(m buildsignalMocks, state entity.RequestState) *gomock.C eventCall := expectBuildFinished(m) m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(1, 4), nil).After(eventCall) m.queueStore.EXPECT().Update(gomock.Any(), queueRow(0, 4), int32(4), int32(5)).Return(nil) - return m.reqStore.EXPECT().Update(gomock.Any(), requestWithState(state), int32(1), int32(2)).Return(nil) + request := requestWithState(state) + request.TerminalBuildID = testBuildID + return m.reqStore.EXPECT().Update(gomock.Any(), request, int32(1), int32(2)).Return(nil) } func expectBuildFinished(m buildsignalMocks) *gomock.Call { @@ -214,6 +216,22 @@ func expectOutcomeLog(m buildsignalMocks, state entity.RequestState, version int ).Return(nil) } +func TestMarkOutcomePreservesFirstTerminalBuild(t *testing.T) { + ctrl := gomock.NewController(t) + c, m := newController(t, ctrl) + request := requestWithState(entity.RequestStateProcessing) + winner := requestWithState(entity.RequestStateSucceeded) + winner.Version = 2 + winner.TerminalBuildID = "winning-build" + + m.reqStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(storage.ErrVersionMismatch) + m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(winner, nil) + + err := c.markOutcome(context.Background(), m.store, &request, entity.RequestStateFailed, "losing-build") + require.NoError(t, err) + assert.Equal(t, winner, request) +} + func TestProcess(t *testing.T) { tests := []struct { name string diff --git a/stovepipe/entity/request.go b/stovepipe/entity/request.go index efd5d64fa..0da57752a 100644 --- a/stovepipe/entity/request.go +++ b/stovepipe/entity/request.go @@ -116,6 +116,9 @@ type Request struct { // State is the current state of the request in the pipeline. State RequestState `json:"state"` + // TerminalBuildID identifies the build that established the terminal state. + // It is empty until a build reaches a terminal state. + TerminalBuildID string `json:"terminal_build_id"` // Version is the version of the object. It is used for optimistic locking. // Versioning starts at 1 and is incremented for each change to the object. Version int32 `json:"version"` diff --git a/stovepipe/extension/storage/mysql/request_store.go b/stovepipe/extension/storage/mysql/request_store.go index 0c2492b13..54417160c 100644 --- a/stovepipe/extension/storage/mysql/request_store.go +++ b/stovepipe/extension/storage/mysql/request_store.go @@ -55,14 +55,15 @@ func (r *requestStore) Create(ctx context.Context, request entity.Request) (retE } _, err := r.db.ExecContext(ctx, - `INSERT INTO request (id, queue, uri, state, build_strategy, base_uri, version) - VALUES (?, ?, ?, ?, ?, ?, ?)`, + `INSERT INTO request (id, queue, uri, state, build_strategy, base_uri, terminal_build_id, version) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, + request.TerminalBuildID, request.Version, ) if err != nil { @@ -82,7 +83,7 @@ func (r *requestStore) Get(ctx context.Context, id string) (ret entity.Request, var req entity.Request err := r.db.QueryRowContext(ctx, - `SELECT id, queue, uri, state, build_strategy, base_uri, version + `SELECT id, queue, uri, state, build_strategy, base_uri, terminal_build_id, version FROM request WHERE queue = ? AND id = ?`, r.queue, id, ).Scan( @@ -92,6 +93,7 @@ func (r *requestStore) Get(ctx context.Context, id string) (ret entity.Request, &req.State, &req.BuildStrategy, &req.BaseURI, + &req.TerminalBuildID, &req.Version, ) @@ -105,7 +107,8 @@ func (r *requestStore) Get(ctx context.Context, id string) (ret entity.Request, return req, nil } -// Update persists the mutable fields of request (uri, state, build_strategy, base_uri) if the +// Update persists the mutable fields of request (uri, state, build_strategy, base_uri, +// terminal_build_id) if the // oldVersion, writing newVersion. Returns ErrVersionMismatch if the stored version does not match // (including when the request does not exist). This is a pure conditional write; the caller owns // version arithmetic. @@ -119,12 +122,13 @@ func (r *requestStore) Update(ctx context.Context, request entity.Request, oldVe result, err := r.db.ExecContext(ctx, `UPDATE request - SET uri = ?, state = ?, build_strategy = ?, base_uri = ?, version = ? + SET uri = ?, state = ?, build_strategy = ?, base_uri = ?, terminal_build_id = ?, version = ? WHERE queue = ? AND id = ? AND version = ?`, request.URI, request.State, request.BuildStrategy, request.BaseURI, + request.TerminalBuildID, newVersion, request.Queue, request.ID, diff --git a/stovepipe/extension/storage/mysql/request_store_test.go b/stovepipe/extension/storage/mysql/request_store_test.go index 0fc657cfa..3dd00ae0f 100644 --- a/stovepipe/extension/storage/mysql/request_store_test.go +++ b/stovepipe/extension/storage/mysql/request_store_test.go @@ -62,7 +62,7 @@ func TestRequestStore_Create(t *testing.T) { name: "success", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("INSERT INTO request"). - WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.Version). + WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.TerminalBuildID, request.Version). WillReturnResult(sqlmock.NewResult(0, 1)) }, }, @@ -70,7 +70,7 @@ func TestRequestStore_Create(t *testing.T) { name: "duplicate id returns ErrAlreadyExists", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("INSERT INTO request"). - WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.Version). + WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.TerminalBuildID, request.Version). WillReturnError(&mysql.MySQLError{Number: mysqlErrDuplicateEntry}) }, wantErr: true, @@ -80,7 +80,7 @@ func TestRequestStore_Create(t *testing.T) { name: "other exec error", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("INSERT INTO request"). - WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.Version). + WithArgs(request.ID, request.Queue, request.URI, request.State, request.BuildStrategy, request.BaseURI, request.TerminalBuildID, request.Version). WillReturnError(fmt.Errorf("connection reset")) }, wantErr: true, @@ -131,9 +131,9 @@ func TestRequestStore_Get(t *testing.T) { name: "found", id: want.ID, setup: func(mock sqlmock.Sqlmock) { - rows := sqlmock.NewRows([]string{"id", "queue", "uri", "state", "build_strategy", "base_uri", "version"}). - AddRow(want.ID, want.Queue, want.URI, string(want.State), string(want.BuildStrategy), want.BaseURI, want.Version) - mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, version"). + rows := sqlmock.NewRows([]string{"id", "queue", "uri", "state", "build_strategy", "base_uri", "terminal_build_id", "version"}). + AddRow(want.ID, want.Queue, want.URI, string(want.State), string(want.BuildStrategy), want.BaseURI, want.TerminalBuildID, want.Version) + mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, terminal_build_id, version"). WithArgs("monorepo/main", want.ID). WillReturnRows(rows) }, @@ -143,7 +143,7 @@ func TestRequestStore_Get(t *testing.T) { name: "not found", id: "missing", setup: func(mock sqlmock.Sqlmock) { - mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, version"). + mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, terminal_build_id, version"). WithArgs("monorepo/main", "missing"). WillReturnError(sql.ErrNoRows) }, @@ -154,7 +154,7 @@ func TestRequestStore_Get(t *testing.T) { name: "query error", id: "bad", setup: func(mock sqlmock.Sqlmock) { - mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, version"). + mock.ExpectQuery("SELECT id, queue, uri, state, build_strategy, base_uri, terminal_build_id, version"). WithArgs("monorepo/main", "bad"). WillReturnError(fmt.Errorf("connection reset")) }, @@ -205,7 +205,7 @@ func TestRequestStore_Update(t *testing.T) { name: "success", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("UPDATE request"). - WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, newVersion, request.Queue, request.ID, oldVersion). + WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.TerminalBuildID, newVersion, request.Queue, request.ID, oldVersion). WillReturnResult(sqlmock.NewResult(0, 1)) }, }, @@ -213,7 +213,7 @@ func TestRequestStore_Update(t *testing.T) { name: "version mismatch", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("UPDATE request"). - WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, newVersion, request.Queue, request.ID, oldVersion). + WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.TerminalBuildID, newVersion, request.Queue, request.ID, oldVersion). WillReturnResult(sqlmock.NewResult(0, 0)) }, wantErr: true, @@ -223,7 +223,7 @@ func TestRequestStore_Update(t *testing.T) { name: "exec error", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("UPDATE request"). - WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, newVersion, request.Queue, request.ID, oldVersion). + WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.TerminalBuildID, newVersion, request.Queue, request.ID, oldVersion). WillReturnError(fmt.Errorf("connection reset")) }, wantErr: true, @@ -232,7 +232,7 @@ func TestRequestStore_Update(t *testing.T) { name: "rows affected error", setup: func(mock sqlmock.Sqlmock) { mock.ExpectExec("UPDATE request"). - WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, newVersion, request.Queue, request.ID, oldVersion). + WithArgs(request.URI, request.State, request.BuildStrategy, request.BaseURI, request.TerminalBuildID, newVersion, request.Queue, request.ID, oldVersion). WillReturnResult(sqlmock.NewErrorResult(fmt.Errorf("driver error"))) }, wantErr: true, diff --git a/stovepipe/extension/storage/mysql/schema/request.sql b/stovepipe/extension/storage/mysql/schema/request.sql index 14c766878..de953e62c 100644 --- a/stovepipe/extension/storage/mysql/schema/request.sql +++ b/stovepipe/extension/storage/mysql/schema/request.sql @@ -7,7 +7,8 @@ CREATE TABLE IF NOT EXISTS request ( uri VARCHAR(255) NOT NULL, state VARCHAR(64) NOT NULL, build_strategy VARCHAR(64) NOT NULL DEFAULT '', - base_uri VARCHAR(255) NOT NULL DEFAULT '', - version INT NOT NULL, + base_uri VARCHAR(255) NOT NULL DEFAULT '', + version INT NOT NULL, + terminal_build_id VARCHAR(255) NOT NULL DEFAULT '', PRIMARY KEY (queue, id) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;