diff --git a/doc/rfc/stovepipe/workflow.md b/doc/rfc/stovepipe/workflow.md index e485165d..28bc3662 100644 --- a/doc/rfc/stovepipe/workflow.md +++ b/doc/rfc/stovepipe/workflow.md @@ -43,11 +43,17 @@ Greenness is recorded as a **health degree** where **`0` means green** and **hig A **project** is a caller-defined slice of the repository. Whole-repo greenness answers "is the branch green at this URI"; project greenness answers the question deployments actually need — **"is *this project* green at this URI"**, and its dual, "what is the latest URI at which this project is green". Projects are derived from the build's **target graph**: analysis sees which targets broke and maps them to projects. How targets map to projects is implementer-specific (directory ownership, build metadata, an external service) and lives behind the project-analysis stage, not in the core pipeline. +### Promotion ref — the last green commit, by name + +A Queue may have a **promotion ref**: a stable branch name (say `verified-main` for `monorepo/main`) that Stovepipe advances to each commit it establishes green. A deploy gate or cache warmer then fetches that name and needs to know nothing about Stovepipe, URIs, or greenness degrees. It is the pull-shaped counterpart to Hooks' push: the same fact, available to consumers that would rather resolve a ref than subscribe to an event. + +The ref is a *cache* of the last-green URI, not a second record of greenness. It only ever moves where the bookmark already points, so it inherits the same forward-only rule, and a commit that a history rewrite has dropped from the branch is skipped rather than retried — the next green commit corrects the ref. Which ref a Queue promotes to, and whether it has one at all, is `SourceControl` configuration resolved from the Queue name alongside the repo and credentials; the pipeline names only the commit, never a branch. + ## Extensions | Extension | Responsibility | |---|---| -| **SourceControl** | Resolve a Queue name to its current head URI; answer ancestry/comparison questions between two URIs (is the new head a fast-forward descendant of the last green, or was history rewritten?); enumerate commits in a range. The sole owner of URI semantics. | +| **SourceControl** | Resolve a Queue name to its current head URI; answer ancestry/comparison questions between two URIs (is the new head a fast-forward descendant of the last green, or was history rewritten?); enumerate commits in a range; advance the Queue's **promotion ref** to a commit. The sole owner of URI semantics, including which refs a Queue name resolves to. | | **build-runner** | Build a scope at a URI (optionally relative to a baseline URI), returning pass/fail and the target graph. See [build-runner.md](../submitqueue/build-runner.md). | | **Hooks** | Publish Stovepipe's greenness events to downstream systems — "this URI / this project is now green (or not green)". Fire-and-forget notification, decoupled so Stovepipe does not know or care who consumes the event. | | **Storage** | Persist Queues (incl. last-green URI), Requests, build records, and per-URI / per-project greenness. Key/value-shaped per the extension-design rules in [CLAUDE.md](../../../CLAUDE.md). | @@ -129,7 +135,7 @@ The pipeline runs in two phases against the same Request. **Phase 1** establishe 2. **process** — decides build strategy (incremental since last-green vs full monorepo), gates concurrent work per Queue, coalesces backlog to the latest head, and publishes to `build`. See [process.md](steps/process.md). 3. **build** — runs the build-runner for the chosen scope. A flag derived from `process` decides whether to build relative to the last-green **baseline URI** (incremental) or from scratch (full). It records a build and publishes the BuildID. 4. **buildsignal** — records the build's status and target graph when the build completes, then releases the Queue's `in_flight_count` slot, projects the terminal status onto the Request (`succeeded` / `failed` / `cancelled`), and publishes the RequestID to `record`. -5. **record** — writes the whole-repo greenness for the head URI (`0` green / `1` broken to start), derived from the Request's build outcome. On green it advances the Queue's **last-green URI** so the next `process` can build incrementally from here. It fires the **Hooks** extension with the green/not-green event, then fans out into Phase 2. The Queue's `in_flight_count` was already released by `buildsignal` when the build went terminal. +5. **record** — writes the whole-repo greenness for the head URI (`0` green / `1` broken to start), derived from the Request's build outcome. On green it advances the Queue's **last-green URI** so the next `process` can build incrementally from here, and asks `SourceControl` to advance the Queue's **promotion ref** to the same commit (see [Promotion ref](#promotion-ref)). It fires the **Hooks** extension with the green/not-green event, then fans out into Phase 2. The Queue's `in_flight_count` was already released by `buildsignal` when the build went terminal. ### Phase 2 — project greenness @@ -147,7 +153,7 @@ The pipeline runs in two phases against the same Request. **Phase 1** establishe | **process** | RequestID | build | Build strategy, concurrency gate, backlog coalescing → [process.md](steps/process.md) | | **build** | RequestID | buildsignal | Run the build-runner for the chosen scope; baseline = last-green URI iff incremental | | **buildsignal** | BuildID | record (P1), record (P2) | Record build status + target graph; release `in_flight_count`; project the outcome onto the Request; signal completion | -| **record** | RequestID | analyze (P1→P2), Hooks | Write greenness; advance last-green URI on whole-repo green; fire Hooks | +| **record** | RequestID | analyze (P1→P2), Hooks | Write greenness; on whole-repo green advance last-green URI and the promotion ref; fire Hooks | | **analyze** | RequestID | build | Map broken/at-risk targets → projects; decide project-scoped builds | ## Step RFCs diff --git a/service/stovepipe/README.md b/service/stovepipe/README.md index 426049db..4a378bfe 100644 --- a/service/stovepipe/README.md +++ b/service/stovepipe/README.md @@ -15,7 +15,7 @@ Stovepipe therefore needs two MySQL databases: a **storage** database (the `requ `server/main.go` is the composition root and supplies the concrete extension implementations. Two are deliberately demo-only and must be replaced for any real deployment: - **`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`. +- **`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`, which is also where a queue's promotion ref is resolved. The fake has no ref to move, so a promotion locally shows up only in the record consumer's logs. ## Layout diff --git a/service/stovepipe/server/main.go b/service/stovepipe/server/main.go index 2d3eef57..6c58fabf 100644 --- a/service/stovepipe/server/main.go +++ b/service/stovepipe/server/main.go @@ -130,7 +130,10 @@ func (f *inMemoryCounterFactory) For(config counter.Config) (counter.Counter, er // fakeSourceControlFactory is the example SourceControl factory. It 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 factory. +// the same queue exercises the dedup path). It has no ref to promote onto, so a promotion +// only succeeds or reports the commit as gone, and the local stack shows it in the record +// consumer's log. A real deployment supplies a VCS-backed factory, which is also where the +// promotion ref is resolved from the queue name, alongside the repo and credentials. type fakeSourceControlFactory struct{} func (fakeSourceControlFactory) For(cfg sourcecontrol.Config) (sourcecontrol.SourceControl, error) { diff --git a/stovepipe/controller/record/record.go b/stovepipe/controller/record/record.go index 7780646b..a14120f5 100644 --- a/stovepipe/controller/record/record.go +++ b/stovepipe/controller/record/record.go @@ -18,7 +18,9 @@ // // The durable state is a ValidationFact per validated commit, plus the queue's // last-green bookmark, which process reads to choose an incremental build -// baseline. Downstream hooks are not implemented yet. +// baseline. A green commit is also promoted, moving the queue's promotion ref so +// downstream systems can pull the latest green commit by name. Downstream hooks +// are not implemented yet. package record import ( @@ -39,8 +41,8 @@ import ( ) // Controller consumes Record messages, records the build's validation fact, and -// advances the queue's last-green bookmark when that fact is green. Implements -// consumer.Controller. +// when that fact is green advances the queue's last-green bookmark and promotes +// the commit. Implements consumer.Controller. type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope @@ -80,9 +82,10 @@ func NewController( } } -// Process loads the request referenced by the delivery and, when its build -// succeeded, advances the queue's last-green bookmark. Returns nil to ack -// (success) or an error to nack (retry) / reject (DLQ). +// Process loads the request referenced by the delivery, records its validation +// fact and, when that fact is green, advances the queue's last-green bookmark and +// promotes the commit. Returns nil to ack (success) or an error to nack (retry) / +// reject (DLQ). // // buildsignal stamps the outcome on the request before publishing here, so a // request without a build outcome is a producer invariant violation rather @@ -133,11 +136,18 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er } return nil } - if err := c.advanceLastGreen(ctx, store, request); err != nil { + holdsBookmark, err := c.advanceLastGreen(ctx, store, request) + if err != nil { metrics.NamedCounter(c.metricsScope, _opName, "storage_errors", 1) return err } - return nil + if !holdsBookmark { + // A later green commit already holds the bookmark, so it also owns + // the promotion ref: promoting this older commit would move the ref + // backwards. + return nil + } + return c.promote(ctx, request) case entity.RequestStateCancelled: // A cancelled build decided nothing about the commit, so it establishes @@ -296,30 +306,34 @@ func degreeFor(state entity.RequestState) float64 { } // advanceLastGreen points the queue's bookmark at request, retrying on version -// conflicts. The bookmark only moves forward: a candidate whose id is not newer -// than the stored one is skipped without a write, which also makes a redelivery -// of the same request a no-op. +// conflicts, and reports whether request holds the bookmark afterwards. The +// bookmark only moves forward: an older candidate is skipped without a write and +// does not hold it, while a redelivery of the request that already set it holds it +// without a write, so the promotion that follows is retried. // // The bookmark is a cache of "newest green URI" derived from the facts, so it is // advanced only after the green fact is durable. Losing the advance to a crash is // recoverable — the redelivery reloads the same fact and retries — whereas a // bookmark with no fact behind it would point at greenness nothing recorded. -func (c *Controller) advanceLastGreen(ctx context.Context, store storage.Storage, request entity.Request) error { +func (c *Controller) advanceLastGreen(ctx context.Context, store storage.Storage, request entity.Request) (bool, error) { queueStore := store.GetQueueStore() for { queueRow, err := queueStore.Get(ctx, request.Queue) if err != nil { - return fmt.Errorf("failed to load queue %s to advance last green: %w", request.Queue, err) + return false, fmt.Errorf("failed to load queue %s to advance last green: %w", request.Queue, err) } - newer, err := isNewerRequest(request.Queue, request.ID, queueRow.LastGreenRequestID) + cmp, err := compareToBookmark(request.Queue, request.ID, queueRow.LastGreenRequestID) if err != nil { // Non-retryable: re-parsing the same ids cannot start succeeding. - return err + return false, err } - if !newer { - return nil + if cmp < 0 { + return false, nil + } + if cmp == 0 { + return true, nil } updated := queueRow @@ -330,7 +344,7 @@ func (c *Controller) advanceLastGreen(ctx context.Context, store storage.Storage if errors.Is(err, storage.ErrVersionMismatch) { continue } - return fmt.Errorf("failed to advance last green for queue %s: %w", request.Queue, err) + return false, fmt.Errorf("failed to advance last green for queue %s: %w", request.Queue, err) } metrics.NamedCounter(c.metricsScope, _opName, "last_green_advanced", 1) @@ -340,7 +354,7 @@ func (c *Controller) advanceLastGreen(ctx context.Context, store storage.Storage "last_green_uri", request.URI, ) c.emitLastGreenTimestamp(ctx, request) - return nil + return true, nil } } @@ -396,17 +410,64 @@ func (c *Controller) emitLastGreenTimestamp(ctx context.Context, request entity. ) } -// isNewerRequest reports whether candidate was ingested after current. An empty -// current means the bookmark has never been set, so any candidate is newer. -func isNewerRequest(queue, candidate, current string) (bool, error) { +// promote points the queue's promotion ref at the request's commit so downstream +// systems can pull the latest green commit by name. Which ref that is — and whether +// the queue has one at all — is source-control configuration, so this stage names +// only the commit. +// +// Like the bookmark, the ref is a cache of the facts, so it moves only after the +// green fact is durable. Promotion is idempotent, so a redelivery repeats it +// harmlessly. A commit that a rewritten history dropped from the ref cannot be +// promoted by any retry, so that case is counted and skipped rather than failed. +func (c *Controller) promote(ctx context.Context, request entity.Request) error { + sc, err := c.sourceControls.For(sourcecontrol.Config{QueueName: request.Queue}) + if err != nil { + metrics.NamedCounter(c.metricsScope, _opName, "source_control_errors", 1, + metrics.NewTag("stage", "resolve"), + ) + return fmt.Errorf("failed to resolve source control for queue %s: %w", request.Queue, err) + } + + if err := sc.Promote(ctx, request.URI); err != nil { + if sourcecontrol.IsNotFound(err) { + metrics.NamedCounter(c.metricsScope, _opName, "promotions_skipped", 1, + metrics.NewTag("reason", "unknown_uri"), + ) + c.logger.Warnw("green commit is no longer on the queue's ref; skipping promotion", + "queue", request.Queue, + "request_id", request.ID, + "uri", request.URI, + ) + return nil + } + + metrics.NamedCounter(c.metricsScope, _opName, "source_control_errors", 1, + metrics.NewTag("stage", "promote"), + ) + return fmt.Errorf("failed to promote uri %s of queue %s: %w", request.URI, request.Queue, err) + } + + metrics.NamedCounter(c.metricsScope, _opName, "promotions", 1) + c.logger.Infow("promoted green commit", + "queue", request.Queue, + "request_id", request.ID, + "uri", request.URI, + ) + return nil +} + +// compareToBookmark orders candidate against the request id currently holding the +// bookmark, by ingest order, using the sign convention of entity.CompareRequestID. +// An empty current means the bookmark has never been set, so any candidate is newer. +func compareToBookmark(queue, candidate, current string) (int, error) { if current == "" { - return true, nil + return 1, nil } cmp, err := entity.CompareRequestID(queue, candidate, current) if err != nil { - return false, fmt.Errorf("failed to compare request ids for queue %s: %w", queue, err) + return 0, fmt.Errorf("failed to compare request ids for queue %s: %w", queue, err) } - return cmp > 0, nil + return cmp, nil } // loadRequest loads the request by id. diff --git a/stovepipe/controller/record/record_test.go b/stovepipe/controller/record/record_test.go index 5562c543..404f3fc6 100644 --- a/stovepipe/controller/record/record_test.go +++ b/stovepipe/controller/record/record_test.go @@ -213,6 +213,7 @@ func TestProcess_AdvancesBookmarkOnSuccess(t *testing.T) { }) m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) + m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) require.NoError(t, c.Process(context.Background(), delivery(t, ctrl, recordPayload(t, testID)))) assert.Equal(t, tt.wantURI, written.LastGreenURI) @@ -265,6 +266,7 @@ func TestProcess_TimestampReportingFailureDoesNotFailRecord(t *testing.T) { m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI).Return(tt.info, tt.err) + m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) require.NoError(t, c.Process(context.Background(), delivery(t, ctrl, recordPayload(t, testID)))) assert.Empty(t, m.metricsScope.Snapshot().Gauges()) @@ -275,7 +277,10 @@ func TestProcess_TimestampReportingFailureDoesNotFailRecord(t *testing.T) { } } -func TestProcess_UnresolvableSourceControlDoesNotFailRecord(t *testing.T) { +// A backend that cannot be resolved leaves the timestamp unreported, which is +// counted and swallowed. The promotion that follows needs the same backend, so it +// is what fails the record and sends the message round again. +func TestProcess_UnresolvableSourceControlCountsTimestampFailure(t *testing.T) { ctrl := gomock.NewController(t) c, m := newController(t, ctrl) c.sourceControls = failingSourceControlFactory{} @@ -287,7 +292,7 @@ func TestProcess_UnresolvableSourceControlDoesNotFailRecord(t *testing.T) { m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) - require.NoError(t, c.Process(context.Background(), delivery(t, ctrl, recordPayload(t, testID)))) + require.Error(t, c.Process(context.Background(), delivery(t, ctrl, recordPayload(t, testID)))) assert.Empty(t, m.metricsScope.Snapshot().Gauges()) counter, ok := m.metricsScope.Snapshot().Counters()["record_controller.record.last_green_timestamp_resolve_errors+queue=monorepo/main"] require.True(t, ok) @@ -464,6 +469,7 @@ func TestProcess_AdoptsExistingFactFromSameRequest(t *testing.T) { m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) + m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) } require.NoError(t, c.Process(context.Background(), delivery(t, ctrl, recordPayload(t, testID)))) @@ -487,14 +493,20 @@ func TestProcess_ExistingFactFromDifferentRequestFails(t *testing.T) { func TestProcess_SkipsBookmarkWhenNotNewer(t *testing.T) { tests := []struct { - name string - stored entity.Queue + name string + stored entity.Queue + wantPromote bool }{ { - name: "same request redelivered", - stored: queueRow(testURI, testID, 3), + // The request already holds the bookmark, so it still owns the ref: + // the promotion is retried in case the first attempt never landed. + name: "same request redelivered", + stored: queueRow(testURI, testID, 3), + wantPromote: true, }, { + // A newer green commit owns the bookmark and the ref, so promoting + // this one would move the ref backwards. name: "stored bookmark is newer", stored: queueRow("git://remote/monorepo/main/newer", "request/monorepo/main/9", 5), }, @@ -512,6 +524,10 @@ func TestProcess_SkipsBookmarkWhenNotNewer(t *testing.T) { m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(tt.stored, nil) // No Update: the bookmark only moves forward. + if tt.wantPromote { + m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) + } + require.NoError(t, c.Process(context.Background(), delivery(t, ctrl, recordPayload(t, testID)))) assert.NotContains( t, @@ -522,6 +538,67 @@ func TestProcess_SkipsBookmarkWhenNotNewer(t *testing.T) { } } +func TestProcess_SkipsPromotionWhenCommitLeftTheRef(t *testing.T) { + ctrl := gomock.NewController(t) + c, m := newController(t, ctrl) + + m.reqStore.EXPECT().Get(gomock.Any(), testID). + Return(requestWithState(entity.RequestStateSucceeded), nil) + var fact entity.ValidationFact + m.expectFactCreated(&fact) + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) + m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) + m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). + Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) + + // A rewritten history dropped the commit from the ref: no retry can promote + // it, so the message is acked rather than sent round again. + m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(sourcecontrol.ErrNotFound) + + require.NoError(t, c.Process(context.Background(), delivery(t, ctrl, recordPayload(t, testID)))) +} + +func TestProcess_PromotionErrorsPropagate(t *testing.T) { + tests := []struct { + name string + setup func(c *Controller, m recordMocks) + }{ + { + name: "source control resolve fails", + setup: func(c *Controller, _ recordMocks) { + c.sourceControls = failingSourceControlFactory{} + }, + }, + { + name: "promote fails", + setup: func(_ *Controller, m recordMocks) { + m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). + Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) + m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(errors.New("boom")) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + c, m := newController(t, ctrl) + + m.reqStore.EXPECT().Get(gomock.Any(), testID). + Return(requestWithState(entity.RequestStateSucceeded), nil) + var fact entity.ValidationFact + m.expectFactCreated(&fact) + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) + m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) + tt.setup(c, m) + + // The bookmark already advanced, so the redelivery re-promotes the + // same commit; failing here is what makes that retry happen. + require.Error(t, c.Process(context.Background(), delivery(t, ctrl, recordPayload(t, testID)))) + }) + } +} + func TestProcess_TerminalWithoutFactDoesNotTouchStores(t *testing.T) { tests := []struct { name string @@ -588,6 +665,7 @@ func TestProcess_RetriesBookmarkOnVersionMismatch(t *testing.T) { ) m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) + m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) require.NoError(t, c.Process(context.Background(), delivery(t, ctrl, recordPayload(t, testID)))) } diff --git a/stovepipe/extension/sourcecontrol/README.md b/stovepipe/extension/sourcecontrol/README.md index da7a8212..a30f1673 100644 --- a/stovepipe/extension/sourcecontrol/README.md +++ b/stovepipe/extension/sourcecontrol/README.md @@ -9,13 +9,16 @@ A `SourceControl` is **bound to a single queue** (a repo+ref) when its `Factory` - **Latest** resolves the queue's ref to the URI of its latest commit — the commit a new validation `Request` is minted against during `ingest`. - **IsAncestor** answers whether one URI is an ancestor of another. The `process` stage uses it to choose a build strategy: if the queue's last-green URI is no longer an ancestor of the latest commit, history was rewritten and a full build is required rather than an incremental one. - **History** returns a bounded, newest-first page of commit URIs on the ref, using the shared generic `page.Page[string]` (`platform/base/page`). It is paginated with an opaque cursor: callers pass an empty cursor for the newest page and the page's `NextCursor` to walk further back, stopping when it is empty. Pagination keeps a remote backend cheap; callers join the URIs against the request store to render the greenness of each commit. +- **Promote** lands the queue's promotion ref on a commit the `record` stage has established green, so downstream systems can fetch the latest known-good commit by name. Only the commit is named: which ref is promoted — and whether the queue promotes at all — is integrator configuration injected at construction, exactly like the read ref, the endpoint, and credentials. A backend with no promotion target configured succeeds without doing anything, so promotion can be enabled per environment and rolled out under a name nobody pulls yet. Promotion is idempotent, and lands the ref on the commit even when it is not a descendant of where the ref currently points, since a history rewrite must not wedge the ref. ## Errors Implementations return plain errors and use the package sentinel `ErrNotFound` (with the `IsNotFound` / `WrapNotFound` helpers) when a queue, ref, or URI cannot be resolved. They do not classify errors as user- or infra-caused — the calling controller does that. +`ErrNotFound` from `Promote` specifically means the commit is no longer on the queue's ref, which is a fact about the branch rather than a transient failure: the `record` stage counts and skips it instead of retrying, because no redelivery can put the commit back. + ## Implementations -- **fake** — an in-memory backend seeded with a queue's ref history (newest first), for examples and tests. +- **fake** — an in-memory backend seeded with a queue's ref history (newest first), for examples and tests. It has no ref to move, so `Promote` records nothing and reports only whether the commit is on the seeded history. To add a backend, create `sourcecontrol/{backend}/`, implement the `SourceControl` interface, and return it from a `New(...)` constructor. diff --git a/stovepipe/extension/sourcecontrol/fake/fake.go b/stovepipe/extension/sourcecontrol/fake/fake.go index 5890a906..719fbc76 100644 --- a/stovepipe/extension/sourcecontrol/fake/fake.go +++ b/stovepipe/extension/sourcecontrol/fake/fake.go @@ -63,6 +63,18 @@ func (s sourceControlFake) Latest(_ context.Context) (string, error) { return s.history[0], nil } +// Promote accepts a commit that is on the history and returns ErrNotFound for one +// that is not — the answer a real backend gives once a rewrite drops the commit +// from the ref. Nothing is recorded: the fake holds no ref to move, and a promotion +// is observable only through the caller's own logs and metrics, so keeping it +// stateless lets any instance answer for any other. +func (s sourceControlFake) Promote(_ context.Context, uri string) error { + if s.indexOf(uri) < 0 { + return sourcecontrol.ErrNotFound + } + return nil +} + // IsAncestor reports whether ancestor is an ancestor of descendant. Both URIs // must be on the ref; an unknown URI yields ErrNotFound. Since the history is // newest-first, ancestor is an ancestor of descendant when its index is greater diff --git a/stovepipe/extension/sourcecontrol/fake/fake_test.go b/stovepipe/extension/sourcecontrol/fake/fake_test.go index 2e2af523..d87c9d60 100644 --- a/stovepipe/extension/sourcecontrol/fake/fake_test.go +++ b/stovepipe/extension/sourcecontrol/fake/fake_test.go @@ -84,6 +84,33 @@ func TestIsAncestor(t *testing.T) { } } +func TestPromote(t *testing.T) { + tests := []struct { + name string + uri string + wantErr bool + }{ + {name: "latest commit", uri: "git://repo/ref/c"}, + {name: "older commit on the ref", uri: "git://repo/ref/a"}, + {name: "commit not on the ref", uri: "git://repo/ref/x", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sc := New(testCfg, history) + err := sc.Promote(context.Background(), tt.uri) + if tt.wantErr { + require.ErrorIs(t, err, sourcecontrol.ErrNotFound) + return + } + require.NoError(t, err) + + // Promotion is idempotent, so a caller retrying after a lost + // response gets the same answer. + require.NoError(t, sc.Promote(context.Background(), tt.uri)) + }) + } +} + func TestHistory(t *testing.T) { tests := []struct { name string diff --git a/stovepipe/extension/sourcecontrol/mock/sourcecontrol_mock.go b/stovepipe/extension/sourcecontrol/mock/sourcecontrol_mock.go index a7a3c05f..80fefd95 100644 --- a/stovepipe/extension/sourcecontrol/mock/sourcecontrol_mock.go +++ b/stovepipe/extension/sourcecontrol/mock/sourcecontrol_mock.go @@ -102,6 +102,20 @@ func (mr *MockSourceControlMockRecorder) Latest(ctx any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Latest", reflect.TypeOf((*MockSourceControl)(nil).Latest), ctx) } +// Promote mocks base method. +func (m *MockSourceControl) Promote(ctx context.Context, uri string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Promote", ctx, uri) + ret0, _ := ret[0].(error) + return ret0 +} + +// Promote indicates an expected call of Promote. +func (mr *MockSourceControlMockRecorder) Promote(ctx, uri any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Promote", reflect.TypeOf((*MockSourceControl)(nil).Promote), ctx, uri) +} + // MockFactory is a mock of Factory interface. type MockFactory struct { ctrl *gomock.Controller diff --git a/stovepipe/extension/sourcecontrol/sourcecontrol.go b/stovepipe/extension/sourcecontrol/sourcecontrol.go index 3dccec03..415f81e5 100644 --- a/stovepipe/extension/sourcecontrol/sourcecontrol.go +++ b/stovepipe/extension/sourcecontrol/sourcecontrol.go @@ -18,7 +18,8 @@ // for the reference git backend, but a Mercurial or Perforce backend would mint // its own scheme). Nothing outside an implementation parses a URI — it is a token // handed back to ask questions ("what is the latest commit of this ref?", "is A an -// ancestor of B?"). A SourceControl is bound to a single queue (a repo+ref) at +// ancestor of B?") or to name the commit to act on ("point the promotion ref at +// this one"). A SourceControl is bound to a single queue (a repo+ref) at // construction by its Factory, so its methods take no queue argument. package sourcecontrol @@ -56,8 +57,9 @@ func WrapNotFound(err error) error { return fmt.Errorf("%w: %w", ErrNotFound, err) } -// SourceControl resolves and compares commit URIs for the single queue it is -// bound to. Implementations interpret URIs; callers treat them as opaque tokens. +// SourceControl resolves, compares, and promotes commit URIs for the single +// queue it is bound to. Implementations interpret URIs; callers treat them as +// opaque tokens. type SourceControl interface { // Latest returns the URI of the latest commit on the queue's ref — the // commit a new validation Request is minted against. Returns ErrNotFound if @@ -71,6 +73,18 @@ type SourceControl interface { // incremental one. Returns ErrNotFound if either URI is unknown to the ref. IsAncestor(ctx context.Context, ancestor, descendant string) (bool, error) + // Promote points the queue's promotion ref at uri — a stable name (e.g. a + // "verified" branch) that downstream systems pull to get the latest commit + // the caller has established as green. The ref is integrator configuration + // injected at construction, like the endpoint and credentials, so callers + // name only the commit; a backend with no promotion target configured makes + // this a no-op. It is idempotent: promoting the URI the ref already points + // at changes nothing. Callers promote monotonically, but a history rewrite + // can leave the ref somewhere unrelated to uri, and implementations must + // still land it on uri rather than refusing. Returns ErrNotFound when uri is + // no longer on the queue's ref, which a rewritten history can cause. + Promote(ctx context.Context, uri string) error + // History returns a bounded page of the queue's commit URIs, newest first. // It is paginated with an opaque cursor so a remote backend stays cheap: // callers pass an empty cursor for the first (newest) page and the page's @@ -88,7 +102,8 @@ type SourceControl interface { // Config carries the per-queue identity handed to a Factory. The system knows // only the queue name; everything an implementation needs (the VCS endpoint, -// credentials, the ref it maps to) is injected at construction by the integrator. +// credentials, the ref it maps to, the ref it promotes to) is injected at +// construction by the integrator. type Config struct { // QueueName identifies the queue (a repo+ref) this SourceControl serves. QueueName string