Skip to content

Commit c5e5759

Browse files
committed
feat(buildrunner): noop impl, poll-driven buildsignal, pipeline wiring
### Why? The `BuildRunner` interface needs an in-tree noop implementation to unblock wiring tests, and the orchestrator's build stage needs to drive the new contract end-to-end: trigger the runner, persist the result, and poll `Status` until terminal so the batch state machine can react. ### What? Stacks on top of the BuildRunner interface and `PublishAfter` PRs. Implements the contract and wires it into the orchestrator pipeline. The build poll loop runs as queue traffic inside the existing `buildsignal` consumer (no separate stage). On each delivery it calls `BuildRunner.Status`, persists the result via `BuildStore.UpdateStatus`, publishes the batch ID to `speculate` so the state machine re-evaluates, and re-publishes itself via `Publisher.PublishAfter` until the build reaches a terminal state. A webhook-capable backend can publish into the same topic — the consumer cannot tell a poll-driven message from a push. Pieces: - `extension/buildrunner/noop`: a `BuildRunner` stub that returns `BuildStatusSucceeded` immediately. Useful as a wiring backstop and a best-case baseline. - `orchestrator/controller/build`: assembles `base` from `batch.Dependencies` and `head` from `batch.Contains`, calls `Trigger`, persists the initial `Build{Accepted}` via `BuildStore.Create` (`ErrAlreadyExists` is swallowed for redelivery), publishes to `buildsignal`. - `orchestrator/controller/buildsignal`: the polling consumer described above. `PollDelayAcceptedMs=5000`, `PollDelayRunningMs=2000` by default (vars so tests can override). - `example/server/orchestrator/main.go`: passes the `BuildRunner` to both `build.NewController` and `buildsignal.NewController`; pipeline diagram updated.
1 parent 2dd6d25 commit c5e5759

12 files changed

Lines changed: 691 additions & 135 deletions

File tree

core/consumer/registry.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ const (
4040
TopicKeySpeculate TopicKey = "speculate"
4141
// TopicKeyBuild is the pipeline stage where speculated batches are published for builds.
4242
TopicKeyBuild TopicKey = "build"
43-
// TopicKeyBuildSignal is the pipeline stage where builds are published for build signal processing.
43+
// TopicKeyBuildSignal is the polling stage for triggered builds. Each
44+
// message carries a Build; the consumer calls BuildRunner.Status,
45+
// persists the latest status, publishes the batch ID to TopicKeySpeculate
46+
// so the state machine re-evaluates, and re-publishes itself via
47+
// PublishAfter when the build has not yet reached a terminal state.
4448
TopicKeyBuildSignal TopicKey = "buildsignal"
4549
// TopicKeyMerge is the pipeline stage where speculated batches are published for merging.
4650
TopicKeyMerge TopicKey = "merge"

example/server/orchestrator/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ go_library(
1414
"//core/consumer",
1515
"//core/httpclient",
1616
"//entity",
17+
"//extension/buildrunner",
18+
"//extension/buildrunner/noop",
1719
"//extension/changeprovider",
1820
"//extension/changeprovider/github",
1921
"//extension/changestore",

example/server/orchestrator/main.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ import (
3333
"github.com/uber/submitqueue/core/consumer"
3434
"github.com/uber/submitqueue/core/httpclient"
3535
"github.com/uber/submitqueue/entity"
36+
"github.com/uber/submitqueue/extension/buildrunner"
37+
buildnoop "github.com/uber/submitqueue/extension/buildrunner/noop"
3638
"github.com/uber/submitqueue/extension/changeprovider"
3739
githubprovider "github.com/uber/submitqueue/extension/changeprovider/github"
3840
"github.com/uber/submitqueue/extension/changestore"
@@ -216,8 +218,12 @@ func run() error {
216218
return fmt.Errorf("failed to create pusher: %w", err)
217219
}
218220

221+
// Create build runner. The noop runner is the pass-through default
222+
// (every build immediately succeeds) until a real backend is wired in.
223+
br := buildnoop.New()
224+
219225
// Register controllers
220-
if err := registerControllers(c, logger.Sugar(), scope, registry, mc, cp, psh, cnt, store, changeStore); err != nil {
226+
if err := registerControllers(c, logger.Sugar(), scope, registry, mc, cp, psh, br, cnt, store, changeStore); err != nil {
221227
return err
222228
}
223229

@@ -397,12 +403,12 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
397403
// Pipeline:
398404
//
399405
// request → validate → batch → score → speculate → build → buildsignal ─┐
400-
// ↑ ↘
401-
// │ merge → conclude
402-
// │ │
403-
// └────────┴───────────────────────
406+
// ↑ ↘ ↻ poll
407+
// │ merge → conclude │
408+
// │ │ │
409+
// └────────┴───────────────────────┘
404410

405-
func registerControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope tally.Scope, registry consumer.TopicRegistry, mc mergechecker.MergeChecker, cp changeprovider.ChangeProvider, psh pusher.Pusher, cnt counter.Counter, store storage.Storage, changeStore changestore.ChangeStore) error {
411+
func registerControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope tally.Scope, registry consumer.TopicRegistry, mc mergechecker.MergeChecker, cp changeprovider.ChangeProvider, psh pusher.Pusher, br buildrunner.BuildRunner, cnt counter.Counter, store storage.Storage, changeStore changestore.ChangeStore) error {
406412
requestController := start.NewController(
407413
logger,
408414
scope,
@@ -488,6 +494,7 @@ func registerControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope t
488494
logger,
489495
scope,
490496
store,
497+
br,
491498
registry,
492499
consumer.TopicKeyBuild,
493500
"orchestrator-build",
@@ -500,6 +507,7 @@ func registerControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope t
500507
logger,
501508
scope,
502509
store,
510+
br,
503511
registry,
504512
consumer.TopicKeyBuildSignal,
505513
"orchestrator-buildsignal",
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
load("@rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "noop",
5+
srcs = ["noop.go"],
6+
importpath = "github.com/uber/submitqueue/extension/buildrunner/noop",
7+
visibility = ["//visibility:public"],
8+
deps = [
9+
"//entity",
10+
"//extension/buildrunner",
11+
],
12+
)
13+
14+
go_test(
15+
name = "noop_test",
16+
srcs = ["noop_test.go"],
17+
embed = [":noop"],
18+
deps = [
19+
"//entity",
20+
"//extension/buildrunner",
21+
"@com_github_stretchr_testify//assert",
22+
"@com_github_stretchr_testify//require",
23+
],
24+
)

extension/buildrunner/noop/noop.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// Package noop provides a buildrunner.BuildRunner that performs no real
16+
// work: every triggered build immediately succeeds. It is intended as a
17+
// stub for wiring tests and as a best-case baseline where every build
18+
// passes.
19+
package noop
20+
21+
import (
22+
"context"
23+
"fmt"
24+
"sync/atomic"
25+
26+
"github.com/uber/submitqueue/entity"
27+
"github.com/uber/submitqueue/extension/buildrunner"
28+
)
29+
30+
// runner is a buildrunner.BuildRunner that does no real work and reports
31+
// every build as immediately succeeded. The atomic counter hands out
32+
// unique build IDs and makes the type safe for concurrent use.
33+
type runner struct {
34+
counter atomic.Uint64
35+
}
36+
37+
// New returns a buildrunner.BuildRunner that performs no real work.
38+
func New() buildrunner.BuildRunner {
39+
return &runner{}
40+
}
41+
42+
// Trigger returns a unique build ID without contacting any runner.
43+
// Inputs are ignored.
44+
func (r *runner) Trigger(_ context.Context, _ string, _ []entity.Change, _ []entity.Change, _ entity.BuildMetadata) (string, error) {
45+
return fmt.Sprintf("noop-%d", r.counter.Add(1)), nil
46+
}
47+
48+
// Status always reports BuildStatusSucceeded with no metadata.
49+
func (r *runner) Status(_ context.Context, _ string) (entity.BuildStatus, entity.BuildMetadata, error) {
50+
return entity.BuildStatusSucceeded, nil, nil
51+
}
52+
53+
// Cancel is a no-op.
54+
func (r *runner) Cancel(_ context.Context, _ string) error {
55+
return nil
56+
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package noop
16+
17+
import (
18+
"context"
19+
"testing"
20+
21+
"github.com/stretchr/testify/assert"
22+
"github.com/stretchr/testify/require"
23+
"github.com/uber/submitqueue/entity"
24+
"github.com/uber/submitqueue/extension/buildrunner"
25+
)
26+
27+
func TestNew_ImplementsInterface(t *testing.T) {
28+
var _ buildrunner.BuildRunner = New()
29+
}
30+
31+
func TestRunner_Trigger(t *testing.T) {
32+
r := New()
33+
ctx := context.Background()
34+
35+
id1, err := r.Trigger(ctx, "queueA",
36+
[]entity.Change{{URIs: []string{"github://owner/repo/pull/1"}}},
37+
[]entity.Change{{URIs: []string{"github://owner/repo/pull/2"}}},
38+
entity.BuildMetadata{"requester": "alice"},
39+
)
40+
require.NoError(t, err)
41+
assert.NotEmpty(t, id1)
42+
43+
// IDs are unique across calls, even with empty inputs.
44+
id2, err := r.Trigger(ctx, "queueA", nil, nil, nil)
45+
require.NoError(t, err)
46+
assert.NotEqual(t, id1, id2)
47+
}
48+
49+
func TestRunner_Status(t *testing.T) {
50+
r := New()
51+
52+
status, meta, err := r.Status(context.Background(), "any-id")
53+
require.NoError(t, err)
54+
assert.Equal(t, entity.BuildStatusSucceeded, status)
55+
assert.Empty(t, meta)
56+
}
57+
58+
func TestRunner_Cancel(t *testing.T) {
59+
r := New()
60+
assert.NoError(t, r.Cancel(context.Background(), "any-id"))
61+
}

orchestrator/controller/build/BUILD.bazel

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ go_library(
1010
"//core/metrics",
1111
"//entity",
1212
"//entity/queue",
13+
"//extension/buildrunner",
1314
"//extension/storage",
1415
"@com_github_uber_go_tally_v4//:tally",
1516
"@org_uber_go_zap//:zap",
@@ -25,7 +26,11 @@ go_test(
2526
"//core/errs",
2627
"//entity",
2728
"//entity/queue",
29+
"//extension/buildrunner",
30+
"//extension/buildrunner/mock",
31+
"//extension/buildrunner/noop",
2832
"//extension/queue/mock",
33+
"//extension/storage",
2934
"//extension/storage/mock",
3035
"@com_github_stretchr_testify//assert",
3136
"@com_github_stretchr_testify//require",

orchestrator/controller/build/build.go

Lines changed: 65 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@ package build
1616

1717
import (
1818
"context"
19+
"errors"
1920
"fmt"
2021

2122
"github.com/uber-go/tally/v4"
2223
"github.com/uber/submitqueue/core/consumer"
2324
"github.com/uber/submitqueue/core/metrics"
2425
"github.com/uber/submitqueue/entity"
2526
entityqueue "github.com/uber/submitqueue/entity/queue"
27+
"github.com/uber/submitqueue/extension/buildrunner"
2628
"github.com/uber/submitqueue/extension/storage"
2729
"go.uber.org/zap"
2830
)
@@ -34,6 +36,7 @@ type Controller struct {
3436
logger *zap.SugaredLogger
3537
metricsScope tally.Scope
3638
store storage.Storage
39+
buildRunner buildrunner.BuildRunner
3740
registry consumer.TopicRegistry
3841
topicKey consumer.TopicKey
3942
consumerGroup string
@@ -47,6 +50,7 @@ func NewController(
4750
logger *zap.SugaredLogger,
4851
scope tally.Scope,
4952
store storage.Storage,
53+
buildRunner buildrunner.BuildRunner,
5054
registry consumer.TopicRegistry,
5155
topicKey consumer.TopicKey,
5256
consumerGroup string,
@@ -55,6 +59,7 @@ func NewController(
5559
logger: logger.Named("build_controller"),
5660
metricsScope: scope.SubScope("build_controller"),
5761
store: store,
62+
buildRunner: buildRunner,
5863
registry: registry,
5964
topicKey: topicKey,
6065
consumerGroup: consumerGroup,
@@ -95,17 +100,45 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
95100
"partition_key", msg.PartitionKey,
96101
)
97102

98-
// TODO: Add build logic
99-
// - Trigger CI build
100-
// - Track build status
103+
// Assemble base (dependency batches in order) and head (this batch).
104+
base, err := c.collectChanges(ctx, batch.Dependencies)
105+
if err != nil {
106+
metrics.NamedCounter(c.metricsScope, opName, "storage_errors", 1)
107+
return fmt.Errorf("failed to assemble base changes for batch %s: %w", batch.ID, err)
108+
}
109+
head, err := c.collectChanges(ctx, []string{batch.ID})
110+
if err != nil {
111+
metrics.NamedCounter(c.metricsScope, opName, "storage_errors", 1)
112+
return fmt.Errorf("failed to assemble head changes for batch %s: %w", batch.ID, err)
113+
}
114+
115+
// Trigger the build with the configured build manager. metadata is nil
116+
// until a caller-supplied source materializes (e.g. requester / ticket
117+
// pulled off the originating LandRequest).
118+
buildID, err := c.buildRunner.Trigger(ctx, batch.Queue, base, head, nil)
119+
if err != nil {
120+
metrics.NamedCounter(c.metricsScope, opName, "trigger_errors", 1)
121+
return fmt.Errorf("failed to trigger build for batch %s: %w", batch.ID, err)
122+
}
101123

102124
build := entity.Build{
103-
ID: batch.ID,
104-
BatchID: batch.ID,
105-
Status: entity.BuildStatusAccepted,
125+
ID: buildID,
126+
BatchID: batch.ID,
127+
SpeculationPath: entity.SpeculationPathInfo{Base: append([]string{}, batch.Dependencies...)},
128+
Status: entity.BuildStatusAccepted,
129+
}
130+
131+
// Persist the initial Build snapshot so the buildsignal poll loop has a
132+
// row to UpdateStatus against. ErrAlreadyExists is benign — a redelivery
133+
// of this message after a previous successful Create.
134+
if err := c.store.GetBuildStore().Create(ctx, build); err != nil && !errors.Is(err, storage.ErrAlreadyExists) {
135+
metrics.NamedCounter(c.metricsScope, opName, "storage_errors", 1)
136+
return fmt.Errorf("failed to persist build %s: %w", build.ID, err)
106137
}
107138

108-
// Publish build to build signal topic
139+
// Hand off to the buildsignal poll loop; it calls Status, updates the
140+
// persisted Build, publishes to speculate, and re-publishes itself via
141+
// PublishAfter until terminal.
109142
if err := c.publish(ctx, consumer.TopicKeyBuildSignal, build); err != nil {
110143
metrics.NamedCounter(c.metricsScope, opName, "publish_errors", 1)
111144
return fmt.Errorf("failed to publish to buildsignal: %w", err)
@@ -114,12 +147,37 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
114147
c.logger.Infow("published build to buildsignal",
115148
"batch_id", batch.ID,
116149
"build_id", build.ID,
150+
"status", string(build.Status),
117151
"topic_key", consumer.TopicKeyBuildSignal,
118152
)
119153

120154
return nil // Success - message will be acked
121155
}
122156

157+
// collectChanges loads each batch by ID and concatenates the Change values
158+
// from its contained requests in batch order. Used to build the base
159+
// (dependency batches) and head (this batch) inputs to BuildRunner.Trigger.
160+
func (c *Controller) collectChanges(ctx context.Context, batchIDs []string) ([]entity.Change, error) {
161+
if len(batchIDs) == 0 {
162+
return nil, nil
163+
}
164+
var changes []entity.Change
165+
for _, bID := range batchIDs {
166+
b, err := c.store.GetBatchStore().Get(ctx, bID)
167+
if err != nil {
168+
return nil, fmt.Errorf("failed to get batch %s: %w", bID, err)
169+
}
170+
for _, reqID := range b.Contains {
171+
req, err := c.store.GetRequestStore().Get(ctx, reqID)
172+
if err != nil {
173+
return nil, fmt.Errorf("failed to get request %s for batch %s: %w", reqID, bID, err)
174+
}
175+
changes = append(changes, req.Change)
176+
}
177+
}
178+
return changes, nil
179+
}
180+
123181
// publish publishes a build to the specified topic key.
124182
func (c *Controller) publish(ctx context.Context, key consumer.TopicKey, build entity.Build) error {
125183
payload, err := build.ToBytes()

0 commit comments

Comments
 (0)