From 6b1397531c290367326fa6e98ee990a0174e18ae Mon Sep 17 00:00:00 2001 From: mnoah1 Date: Fri, 6 Nov 2026 22:09:07 +0000 Subject: [PATCH] feat(stovepipe): define named batch repair executor contracts Summary: Let queues register multiple repair backends and let each backend group its own issue openings. Submission receipts identify accepted work or a terminal skip for each input item; metadata stays in versioned storage. This contract change builds on repair-state persistence. Controller and reference-server callers migrate in the next PR in the stack. Test Plan: `./tool/bazel build //stovepipe/extension/fixer/...` builds the contract, generated mock, and no-op implementation. `make mocks` regenerated the mock, and lint, tidy, and Gazelle checks verify the generated sources and build metadata. Caller compilation requires the next PR. Revert Plan: Revert the dependent dispatch change before reverting this contract change. --- stovepipe/extension/fixer/fixer.go | 40 +++++++++++++++----- stovepipe/extension/fixer/mock/fixer_mock.go | 8 ++-- stovepipe/extension/fixer/noop/noop.go | 8 +++- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/stovepipe/extension/fixer/fixer.go b/stovepipe/extension/fixer/fixer.go index 533849367..cf4a79a77 100644 --- a/stovepipe/extension/fixer/fixer.go +++ b/stovepipe/extension/fixer/fixer.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package fixer defines repair submission for an open issue. +// Package fixer defines repair submission for open issues. package fixer //go:generate mockgen -source=fixer.go -destination=mock/fixer_mock.go -package=mock @@ -23,22 +23,42 @@ import ( "github.com/uber/submitqueue/stovepipe/entity" ) -// RepairExecutor submits repair work for one open issue within a configured queue. -// Implementations must be concurrency-safe and return promptly after submission. +// RepairExecutor chooses how to group repair work. Implementations must be concurrency-safe. type RepairExecutor interface { - // Submit returns a nonempty opaque fix ID on acceptance, without establishing recovery. - // An ID returned alongside an error still identifies the repair attempt. - // Errors without an ID may follow acceptance; repeated calls can start additional work. - Submit(context.Context, entity.OpenIssue) (string, error) + // Submit returns per-item backend acceptance or terminal skips, not repair completion. + // Valid submissions returned alongside errors are persisted; omitted items remain pending. + // Retries may include subsets or additional items; implementations deduplicate by opening. + // Pending rows exist before Submit. Executors manage metadata through injected storage, + // preserving submission fields and reloading on version conflicts. + Submit(context.Context, []entity.OpenIssue) ([]Submission, error) } -// Config identifies the queue whose repair executor is requested. +// Submission associates one input item with accepted repair work or a deliberate skip. +// Acceptance does not indicate repair completion or success. +type Submission struct { + // Issue identifies the input opening, including its first-seen build. + Issue entity.OpenIssue + // FixID is the backend request or run ID for accepted work; multiple items may share it. + FixID string + // SkipReason is nonempty for a terminal skip. Exactly one of FixID and SkipReason is required. + SkipReason string +} + +// Registration binds a stable queue-local identity to an executor. +type Registration struct { + // ID is durable across configuration changes and must not be reused for another backend. + ID string + // Executor implements the registered repair policy. + Executor RepairExecutor +} + +// Config identifies a validation queue. type Config struct { // QueueName identifies the validation queue. QueueName string } -// Factory resolves queue-bound executors; routing belongs to service wiring. +// Factory resolves all registered executors; an empty slice disables dispatch. type Factory interface { - For(Config) (RepairExecutor, error) + For(Config) ([]Registration, error) } diff --git a/stovepipe/extension/fixer/mock/fixer_mock.go b/stovepipe/extension/fixer/mock/fixer_mock.go index 25e4779f5..9e9912274 100644 --- a/stovepipe/extension/fixer/mock/fixer_mock.go +++ b/stovepipe/extension/fixer/mock/fixer_mock.go @@ -43,10 +43,10 @@ func (m *MockRepairExecutor) EXPECT() *MockRepairExecutorMockRecorder { } // Submit mocks base method. -func (m *MockRepairExecutor) Submit(arg0 context.Context, arg1 entity.OpenIssue) (string, error) { +func (m *MockRepairExecutor) Submit(arg0 context.Context, arg1 []entity.OpenIssue) ([]fixer.Submission, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Submit", arg0, arg1) - ret0, _ := ret[0].(string) + ret0, _ := ret[0].([]fixer.Submission) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -82,10 +82,10 @@ func (m *MockFactory) EXPECT() *MockFactoryMockRecorder { } // For mocks base method. -func (m *MockFactory) For(arg0 fixer.Config) (fixer.RepairExecutor, error) { +func (m *MockFactory) For(arg0 fixer.Config) ([]fixer.Registration, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "For", arg0) - ret0, _ := ret[0].(fixer.RepairExecutor) + ret0, _ := ret[0].([]fixer.Registration) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/stovepipe/extension/fixer/noop/noop.go b/stovepipe/extension/fixer/noop/noop.go index 7ab458c96..88e128c06 100644 --- a/stovepipe/extension/fixer/noop/noop.go +++ b/stovepipe/extension/fixer/noop/noop.go @@ -29,6 +29,10 @@ func New() fixer.RepairExecutor { return executor{} } -func (executor) Submit(_ context.Context, issue entity.OpenIssue) (string, error) { - return "noop/" + issue.FirstSeenBuildID, nil +func (executor) Submit(_ context.Context, issues []entity.OpenIssue) ([]fixer.Submission, error) { + submissions := make([]fixer.Submission, 0, len(issues)) + for _, issue := range issues { + submissions = append(submissions, fixer.Submission{Issue: issue, FixID: "noop/" + issue.FirstSeenBuildID}) + } + return submissions, nil }