Skip to content

Commit 3c70ced

Browse files
committed
feat(stovepipe): Mark the terminal outcome build
Summary: **What**: - Add Build.IsTerminalOutcome and persist it when buildsignal observes a terminal build status. - Enforce at most one selected build per request with a generated nullable request key and unique MySQL index. **Why**: - Keep the selected terminal build as operational state on Build while preventing duplicate builds from both becoming selected. Test Plan: - [x] `go test ./stovepipe/extension/storage/mysql ./stovepipe/controller/buildsignal` Revert Plan: - Revert this PR to remove the marker and its unique index if the Build-based selection model is not adopted. Jira Issues: None
1 parent b508b3d commit 3c70ced

6 files changed

Lines changed: 48 additions & 23 deletions

File tree

‎stovepipe/controller/buildsignal/buildsignal.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,7 @@ func (c *Controller) reconcile(ctx context.Context, store storage.Storage, build
370370
newVersion := build.Version + 1
371371
updated := build
372372
updated.Status = status
373+
updated.IsTerminalOutcome = status.IsTerminal()
373374
if err := store.GetBuildStore().Update(ctx, updated, build.Version, newVersion); err != nil {
374375
return "", fmt.Errorf("failed to persist status for build %s: %w", build.ID, err)
375376
}

‎stovepipe/controller/buildsignal/buildsignal_test.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,9 @@ func TestProcess(t *testing.T) {
326326
m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateProcessing), nil)
327327
m.runnerFactory.EXPECT().For(buildrunner.Config{QueueName: testQueue}).Return(m.runner, nil)
328328
m.runner.EXPECT().Status(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(entity.BuildStatusSucceeded, nil, nil)
329-
m.buildStore.EXPECT().Update(gomock.Any(), build(entity.BuildStatusSucceeded, 2), int32(2), int32(3)).Return(nil)
329+
updated := build(entity.BuildStatusSucceeded, 2)
330+
updated.IsTerminalOutcome = true
331+
m.buildStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil)
330332
logCall := expectFinish(m, entity.RequestStateSucceeded)
331333
m.publisher.EXPECT().Publish(gomock.Any(), "record", gomock.Any()).Return(nil).After(logCall)
332334
},
@@ -338,7 +340,9 @@ func TestProcess(t *testing.T) {
338340
m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateProcessing), nil)
339341
m.runnerFactory.EXPECT().For(buildrunner.Config{QueueName: testQueue}).Return(m.runner, nil)
340342
m.runner.EXPECT().Status(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(entity.BuildStatusFailed, nil, nil)
341-
m.buildStore.EXPECT().Update(gomock.Any(), build(entity.BuildStatusFailed, 2), int32(2), int32(3)).Return(nil)
343+
updated := build(entity.BuildStatusFailed, 2)
344+
updated.IsTerminalOutcome = true
345+
m.buildStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil)
342346
logCall := expectFinish(m, entity.RequestStateFailed)
343347
m.publisher.EXPECT().Publish(gomock.Any(), "record", gomock.Any()).Return(nil).After(logCall)
344348
},
@@ -350,7 +354,9 @@ func TestProcess(t *testing.T) {
350354
m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateProcessing), nil)
351355
m.runnerFactory.EXPECT().For(buildrunner.Config{QueueName: testQueue}).Return(m.runner, nil)
352356
m.runner.EXPECT().Status(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(entity.BuildStatusCancelled, nil, nil)
353-
m.buildStore.EXPECT().Update(gomock.Any(), build(entity.BuildStatusCancelled, 2), int32(2), int32(3)).Return(nil)
357+
updated := build(entity.BuildStatusCancelled, 2)
358+
updated.IsTerminalOutcome = true
359+
m.buildStore.EXPECT().Update(gomock.Any(), updated, int32(2), int32(3)).Return(nil)
354360
logCall := expectFinish(m, entity.RequestStateCancelled)
355361
m.publisher.EXPECT().Publish(gomock.Any(), "record", gomock.Any()).Return(nil).After(logCall)
356362
},

‎stovepipe/entity/build.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@ func (s BuildStatus) IsTerminal() bool {
5454
}
5555

5656
// Build represents a single build triggered for a Request's commit. All
57-
// fields except Status and Version are immutable after creation — build is
58-
// the sole creator (via BuildStore.Create), and buildsignal is the sole
59-
// writer of Status/Version afterward.
57+
// fields except Status, IsTerminalOutcome, and Version are immutable after
58+
// creation — build is the sole creator (via BuildStore.Create), and
59+
// buildsignal is the sole writer afterward.
6060
type Build struct {
6161
// ID is the build's own key: the runner-assigned id returned by
6262
// Trigger (e.g. a Buildkite build number). Opaque; never parsed or
@@ -67,6 +67,9 @@ type Build struct {
6767
RequestID string `json:"request_id"`
6868
// Status is the build's lifecycle state.
6969
Status BuildStatus `json:"status"`
70+
// IsTerminalOutcome reports whether this build established its request's
71+
// terminal state. At most one build for a request may be true.
72+
IsTerminalOutcome bool `json:"is_terminal_outcome"`
7073
// Version is used for optimistic locking. Versioning starts at 1 and
7174
// is incremented for each change to the object.
7275
Version int32 `json:"version"`

‎stovepipe/extension/storage/mysql/build_store.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,15 @@ func (b *buildStore) Create(ctx context.Context, build entity.Build) (retErr err
4646
defer func() { op.Complete(retErr) }()
4747

4848
_, err := b.db.ExecContext(ctx,
49-
`INSERT INTO build (queue, id, request_id, status, version)
50-
VALUES (?, ?, ?, ?, ?)`,
49+
`INSERT INTO build (queue, id, request_id, status, version, is_terminal_outcome, terminal_outcome_request_id)
50+
VALUES (?, ?, ?, ?, ?, ?, ?)`,
5151
b.queue,
5252
build.ID,
5353
build.RequestID,
5454
build.Status,
5555
build.Version,
56+
build.IsTerminalOutcome,
57+
nil,
5658
)
5759
if err != nil {
5860
if isDuplicateEntry(err) {
@@ -71,13 +73,14 @@ func (b *buildStore) Get(ctx context.Context, id string) (ret entity.Build, retE
7173

7274
var build entity.Build
7375
err := b.db.QueryRowContext(ctx,
74-
`SELECT id, request_id, status, version
76+
`SELECT id, request_id, status, is_terminal_outcome, version
7577
FROM build WHERE queue = ? AND id = ?`,
7678
b.queue, id,
7779
).Scan(
7880
&build.ID,
7981
&build.RequestID,
8082
&build.Status,
83+
&build.IsTerminalOutcome,
8184
&build.Version,
8285
)
8386

@@ -101,9 +104,11 @@ func (b *buildStore) Update(ctx context.Context, build entity.Build, oldVersion,
101104

102105
result, err := b.db.ExecContext(ctx,
103106
`UPDATE build
104-
SET status = ?, version = ?
107+
SET status = ?, is_terminal_outcome = ?, terminal_outcome_request_id = ?, version = ?
105108
WHERE queue = ? AND id = ? AND version = ?`,
106109
build.Status,
110+
build.IsTerminalOutcome,
111+
terminalOutcomeRequestID(build),
107112
newVersion,
108113
b.queue,
109114
build.ID,
@@ -133,3 +138,10 @@ func (b *buildStore) Update(ctx context.Context, build entity.Build, oldVersion,
133138

134139
return nil
135140
}
141+
142+
func terminalOutcomeRequestID(build entity.Build) any {
143+
if build.IsTerminalOutcome {
144+
return build.RequestID
145+
}
146+
return nil
147+
}

‎stovepipe/extension/storage/mysql/build_store_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@ func TestBuildStore_Create(t *testing.T) {
5858
name: "success",
5959
setup: func(mock sqlmock.Sqlmock) {
6060
mock.ExpectExec("INSERT INTO build").
61-
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.Version).
61+
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.Version, build.IsTerminalOutcome, nil).
6262
WillReturnResult(sqlmock.NewResult(0, 1))
6363
},
6464
},
6565
{
6666
name: "duplicate id returns ErrAlreadyExists",
6767
setup: func(mock sqlmock.Sqlmock) {
6868
mock.ExpectExec("INSERT INTO build").
69-
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.Version).
69+
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.Version, build.IsTerminalOutcome, nil).
7070
WillReturnError(&mysql.MySQLError{Number: mysqlErrDuplicateEntry})
7171
},
7272
wantErr: true,
@@ -76,7 +76,7 @@ func TestBuildStore_Create(t *testing.T) {
7676
name: "other exec error",
7777
setup: func(mock sqlmock.Sqlmock) {
7878
mock.ExpectExec("INSERT INTO build").
79-
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.Version).
79+
WithArgs("monorepo/main", build.ID, build.RequestID, build.Status, build.Version, build.IsTerminalOutcome, nil).
8080
WillReturnError(fmt.Errorf("connection reset"))
8181
},
8282
wantErr: true,
@@ -124,9 +124,9 @@ func TestBuildStore_Get(t *testing.T) {
124124
name: "found",
125125
id: want.ID,
126126
setup: func(mock sqlmock.Sqlmock) {
127-
rows := sqlmock.NewRows([]string{"id", "request_id", "status", "version"}).
128-
AddRow(want.ID, want.RequestID, string(want.Status), want.Version)
129-
mock.ExpectQuery("SELECT id, request_id, status, version").
127+
rows := sqlmock.NewRows([]string{"id", "request_id", "status", "is_terminal_outcome", "version"}).
128+
AddRow(want.ID, want.RequestID, string(want.Status), want.IsTerminalOutcome, want.Version)
129+
mock.ExpectQuery("SELECT id, request_id, status, is_terminal_outcome, version").
130130
WithArgs("monorepo/main", want.ID).
131131
WillReturnRows(rows)
132132
},
@@ -136,7 +136,7 @@ func TestBuildStore_Get(t *testing.T) {
136136
name: "not found",
137137
id: "missing",
138138
setup: func(mock sqlmock.Sqlmock) {
139-
mock.ExpectQuery("SELECT id, request_id, status, version").
139+
mock.ExpectQuery("SELECT id, request_id, status, is_terminal_outcome, version").
140140
WithArgs("monorepo/main", "missing").
141141
WillReturnError(sql.ErrNoRows)
142142
},
@@ -147,7 +147,7 @@ func TestBuildStore_Get(t *testing.T) {
147147
name: "query error",
148148
id: "bad",
149149
setup: func(mock sqlmock.Sqlmock) {
150-
mock.ExpectQuery("SELECT id, request_id, status, version").
150+
mock.ExpectQuery("SELECT id, request_id, status, is_terminal_outcome, version").
151151
WithArgs("monorepo/main", "bad").
152152
WillReturnError(fmt.Errorf("connection reset"))
153153
},
@@ -191,15 +191,15 @@ func TestBuildStore_Update(t *testing.T) {
191191
name: "success",
192192
setup: func(mock sqlmock.Sqlmock) {
193193
mock.ExpectExec("UPDATE build").
194-
WithArgs(build.Status, newVersion, "monorepo/main", build.ID, oldVersion).
194+
WithArgs(build.Status, build.IsTerminalOutcome, nil, newVersion, "monorepo/main", build.ID, oldVersion).
195195
WillReturnResult(sqlmock.NewResult(0, 1))
196196
},
197197
},
198198
{
199199
name: "version mismatch",
200200
setup: func(mock sqlmock.Sqlmock) {
201201
mock.ExpectExec("UPDATE build").
202-
WithArgs(build.Status, newVersion, "monorepo/main", build.ID, oldVersion).
202+
WithArgs(build.Status, build.IsTerminalOutcome, nil, newVersion, "monorepo/main", build.ID, oldVersion).
203203
WillReturnResult(sqlmock.NewResult(0, 0))
204204
},
205205
wantErr: true,
@@ -209,7 +209,7 @@ func TestBuildStore_Update(t *testing.T) {
209209
name: "exec error",
210210
setup: func(mock sqlmock.Sqlmock) {
211211
mock.ExpectExec("UPDATE build").
212-
WithArgs(build.Status, newVersion, "monorepo/main", build.ID, oldVersion).
212+
WithArgs(build.Status, build.IsTerminalOutcome, nil, newVersion, "monorepo/main", build.ID, oldVersion).
213213
WillReturnError(fmt.Errorf("connection reset"))
214214
},
215215
wantErr: true,
@@ -218,7 +218,7 @@ func TestBuildStore_Update(t *testing.T) {
218218
name: "rows affected error",
219219
setup: func(mock sqlmock.Sqlmock) {
220220
mock.ExpectExec("UPDATE build").
221-
WithArgs(build.Status, newVersion, "monorepo/main", build.ID, oldVersion).
221+
WithArgs(build.Status, build.IsTerminalOutcome, nil, newVersion, "monorepo/main", build.ID, oldVersion).
222222
WillReturnResult(sqlmock.NewErrorResult(fmt.Errorf("driver error")))
223223
},
224224
wantErr: true,

‎stovepipe/extension/storage/mysql/schema/build.sql‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,8 @@ CREATE TABLE IF NOT EXISTS build (
66
request_id VARCHAR(255) NOT NULL,
77
status VARCHAR(64) NOT NULL,
88
version INT NOT NULL,
9-
PRIMARY KEY (queue, id)
9+
is_terminal_outcome BOOLEAN NOT NULL DEFAULT FALSE,
10+
terminal_outcome_request_id VARCHAR(255) NULL,
11+
PRIMARY KEY (queue, id),
12+
UNIQUE KEY terminal_outcome_request (queue, terminal_outcome_request_id)
1013
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

0 commit comments

Comments
 (0)