Skip to content

Commit 2c32bbd

Browse files
prathshenoymnoah1
andauthored
fix(stovepipe): Promote before project status (#873)
## Summary **What**: - Advance the green baseline and promote the repository before resolving or recording project statuses. **Why**: - Let subsequent validations use a recorded green commit without waiting for project discovery or project-fact writes. ## Test Plan - [x] Add unit tests. ## Revert Plan - Revert this PR to restore the previous recording order. No schema or data rollback is needed. ## Issue Links - [CODEM-589](https://linear.app/uber/issue/CODEM-589/promo-race) Co-authored-by: Michael Noah <92764374+mnoah1@users.noreply.github.com>
1 parent c41f602 commit 2c32bbd

3 files changed

Lines changed: 35 additions & 4 deletions

File tree

‎doc/rfc/stovepipe/steps/record.md‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,20 @@ For a delivery carrying a `Record` payload:
6262
ref, and no retry can promote it.
6363
- other error -> return raw.
6464
65-
10. Publish a validation.repository.recorded HookEvent naming the request whose fact
65+
10. Resolve and record optional project results.
66+
- failure -> return raw; the repository fact, bookmark, and promotion are not undone.
67+
68+
11. Publish a validation.repository.recorded HookEvent naming the request whose fact
6669
was persisted.
6770
- publish failure -> return raw; non-retryable, so it dead-letters.
6871
69-
11. ack.
72+
12. ack.
7073
```
7174

7275
Every decision after step 6 uses the persisted fact, not the outcome read from this delivery's Request. The first immutable fact controls the bookmark, the ref, and the hook event.
7376

77+
Project-status processing does not delay baseline advancement or promotion. It still completes before the recorded notification. Bookmark advancement and ref promotion remain separate operations; this ordering does not make them atomic.
78+
7479
## Validation facts
7580

7681
A fact answers "how broken was this scope at this Queue URI?" Its identity is `(queue, uri, project)`, where `project` is empty for whole-repository greenness and a stable project id in Phase 2.

‎stovepipe/controller/record/record.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,10 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
154154
if err := c.persistValidationFactRecordedLog(ctx, store, request, fact); err != nil {
155155
return err
156156
}
157-
if err := c.recordProjectFacts(ctx, store, request); err != nil {
157+
if err := c.applyFactToDerivedCaches(ctx, store, request, fact, created); err != nil {
158158
return err
159159
}
160-
if err := c.applyFactToDerivedCaches(ctx, store, request, fact, created); err != nil {
160+
if err := c.recordProjectFacts(ctx, store, request); err != nil {
161161
return err
162162
}
163163
return c.publishHookEvent(ctx, request, hookevent.NewValidationRepositoryRecorded(request))

‎stovepipe/controller/record/record_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,32 @@ func TestProcess_AdvancesBookmarkOnSuccess(t *testing.T) {
334334
}
335335
}
336336

337+
func TestProcess_PromotesGreenBeforeProjectResolution(t *testing.T) {
338+
ctrl := gomock.NewController(t)
339+
c, m := newController(t, ctrl)
340+
request := requestWithState(entity.RequestStateSucceeded)
341+
m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(request, nil)
342+
m.factStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil)
343+
m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil)
344+
m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil)
345+
m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI).
346+
Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil)
347+
promote := m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil)
348+
349+
resolverFactory := projectstatusmock.NewMockResolverFactory(ctrl)
350+
resolver := projectstatusmock.NewMockResolver(ctrl)
351+
c.projectStatusResolverFactory = resolverFactory
352+
resolverFactory.EXPECT().For(projectstatus.ResolverConfig{QueueName: testQueue}).
353+
After(promote).
354+
Return(resolver, nil)
355+
sentinel := errors.New("resolution unavailable")
356+
resolver.EXPECT().Resolve(gomock.Any(), request, request.TerminalBuildID).Return(nil, sentinel)
357+
358+
err := c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID)))
359+
require.ErrorIs(t, err, sentinel)
360+
assert.Empty(t, m.hooks.events)
361+
}
362+
337363
func TestProcess_RejectsLegacyBookmark(t *testing.T) {
338364
ctrl := gomock.NewController(t)
339365
c, m := newController(t, ctrl)

0 commit comments

Comments
 (0)