Skip to content

Commit c006239

Browse files
committed
feat(runway): wire the git merger into the server
## Summary ### Why? The git merger exists but nothing constructs it — the server still builds the noop factory, so a deployed Runway acknowledges every merge as an instant success. This is the change that makes Runway actually merge. It is deliberately last in the stack and deliberately opt-in: the switch is the presence of `MERGE_CHECKOUT_PATH`. Unset, the server keeps wiring noop, which is what local development, the compose stack, and the e2e suite depend on — none of them have a git checkout to hand the merger. ### What? `newMergerFactory` now reads the environment and returns either backend. With `MERGE_CHECKOUT_PATH` set it builds a git merger from the `MERGE_*` / `GIT_*` variables — remote, target branch, default strategy, committer identity, and the pinned git runtime — and logs the resolved configuration at startup. Without it, noop, with a log line saying so. A malformed configuration fails startup rather than degrading silently: `parseStrategy` rejects an unrecognized `MERGE_DEFAULT_STRATEGY`, and `DEFAULT` itself is rejected because it cannot be the value a `DEFAULT` step resolves to. Three settings govern the behaviors the merger cannot infer: - `MERGE_CHECK_STALENESS` (default on) verifies each change's provider ref still points at the commit its URI names before applying. - `MERGE_ALLOW_UNRELATED_HISTORIES` (default off) lets a MERGE step import a history that shares no ancestry with the target. It stays off because the refusal it lifts is a safeguard everywhere except a queue whose purpose is such imports. - `MERGE_FETCH_REFSPECS` supplies extra refspecs for a remote that will not serve an unadvertised commit by SHA. Normally empty. `gitMergerFactory` hands the same merger instance to every queue. The merger owns one checkout and serializes its own operations, so a second instance over the same directory would race; a deployment that lands multiple targets wires a per-queue map instead. This is also why the factory is built once at startup rather than per request. ## Test Plan ✅ `bazel build //service/runway/...` — wiring compiles ✅ `bazel test //runway/...` — all targets pass Not covered by automated tests: the git path only engages when `MERGE_CHECKOUT_PATH` points at a real checkout, so the env-to-`Params` mapping is exercised by the merger's own suite rather than through `main`. Watch the `git merger configured` startup log — with checkout, target, and default strategy — on the first deployment that sets the variable; its absence means the server silently fell back to noop.
1 parent 9d0e8b7 commit c006239

4 files changed

Lines changed: 142 additions & 5 deletions

File tree

runway/README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ Runway is a single service (the domain *is* the service); its controllers live d
1313

1414
Each controller deserializes the `MergeRequest`, obtains a `Merger` for the request's queue from the [`merger`](extension/merger) extension, applies the ordered steps, and publishes a `MergeResult` to the corresponding signal queue (`merge-conflict-check-signal` / `merge-signal`). `merge` commits and reports the produced revisions; `merge-conflict-check` is a dry run that reports mergeability with empty outputs.
1515

16+
## Merger extension
17+
18+
[`extension/merger`](extension/merger) is the pluggable merge contract. Implementations:
19+
20+
- [`git`](extension/merger/git) — a git-CLI backend that honors each step's strategy (REBASE, SQUASH_REBASE, MERGE, PROMOTE; DEFAULT resolves to a per-instance default). See its [README](extension/merger/git/README.md).
21+
- `noop` — always-succeeds stub for local development.
22+
1623
## Failure handling
1724

1825
A merge outcome the controller can name is published as a `FAILED` result and acked, not retried: a merge conflict (`merger.ErrConflict`) or an invalid request (`merger.ErrInvalidRequest` — unknown strategy, malformed change URI, invalid PROMOTE composition). The `merger.IsTerminal` helper draws that line. Any other error is an infrastructure fault and is nacked for retry.

service/runway/README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ Each controller applies the request's ordered steps via a `Merger` and publishes
1313

1414
These topic keys and their wire contracts are owned by the queue's producer side and published under `api/runway/messagequeue/` (the external, cross-domain contract).
1515

16+
### Merger backend
17+
18+
The merge work is done by the [`merger`](../../runway/extension/merger) extension. By default the server wires the **noop** merger (always succeeds — for local dev and compose). Set `MERGE_CHECKOUT_PATH` to wire the real **git** merger, built from the `MERGE_*` / `GIT_*` environment (see Configuration); the git merger owns the checkout at that path and pushes to the configured remote/target.
19+
1620
Because Runway only consumes queues and serves `Ping`, it needs a **queue** database but no application/storage database.
1721

1822
## Layout
@@ -36,6 +40,16 @@ The Runway controllers themselves live under [`runway/controller/`](../../runway
3640
| `QUEUE_MYSQL_DSN` | yes | Queue database DSN ||
3741
| `PORT` | no | gRPC listen address | `:8086` |
3842
| `HOSTNAME` | no | Subscriber name for the queue consumer | `runway-<unix_ts>` |
43+
| `MERGE_CHECKOUT_PATH` | no | Absolute path to the git checkout the merger owns. When unset, the noop merger is used. | — (noop) |
44+
| `MERGE_REMOTE` | no | Git remote to fetch/push | `origin` |
45+
| `MERGE_TARGET` | no | Destination branch on the remote | `main` |
46+
| `MERGE_DEFAULT_STRATEGY` | no | Strategy a `DEFAULT` step resolves to: `REBASE`, `SQUASH_REBASE`, `MERGE`, or `PROMOTE` | `REBASE` |
47+
| `MERGE_COMMITTER_NAME` | no | Committer name for service-created commits | `SubmitQueue Runway` |
48+
| `MERGE_COMMITTER_EMAIL` | no | Committer email for service-created commits | `runway@submitqueue.invalid` |
49+
| `GIT_EXECUTABLE` / `GIT_EXEC_PATH` / `GIT_TEMPLATE_DIR` | when `MERGE_CHECKOUT_PATH` set | Absolute paths to the pinned git runtime ||
50+
| `MERGE_CHECK_STALENESS` | no | Verify each change's provider ref still points at the commit its URI names before applying | `true` |
51+
| `MERGE_ALLOW_UNRELATED_HISTORIES` | no | Let a `MERGE` step integrate a change sharing no ancestry with the target (repository imports). Leave off unless the queue exists to perform imports. | `false` |
52+
| `MERGE_FETCH_REFSPECS` | no | Comma-separated extra refspecs fetched each cycle. Only needed for a remote that refuses to serve an unadvertised commit by SHA. ||
3953

4054
## Running
4155

service/runway/server/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ go_library(
1111
importpath = "github.com/uber/submitqueue/service/runway/server",
1212
visibility = ["//visibility:private"],
1313
deps = [
14+
"//api/base/mergestrategy/protopb:go_default_library",
1415
"//api/runway/messagequeue:go_default_library",
1516
"//api/runway/protopb:go_default_library",
1617
"//platform/consumer:go_default_library",
@@ -27,6 +28,7 @@ go_library(
2728
"//runway/controller/merge:go_default_library",
2829
"//runway/controller/mergeconflictcheck:go_default_library",
2930
"//runway/extension/merger:go_default_library",
31+
"//runway/extension/merger/git:go_default_library",
3032
"//runway/extension/merger/noop:go_default_library",
3133
"@com_github_go_sql_driver_mysql//:go_default_library",
3234
"@com_github_uber_go_tally//:go_default_library",

service/runway/server/main.go

Lines changed: 119 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,15 @@ import (
2222
"net"
2323
"os"
2424
"os/signal"
25+
"strconv"
26+
"strings"
2527
"sync"
2628
"syscall"
2729
"time"
2830

2931
_ "github.com/go-sql-driver/mysql"
3032
"github.com/uber-go/tally"
33+
mergestrategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb"
3134
runwaymq "github.com/uber/submitqueue/api/runway/messagequeue"
3235
pb "github.com/uber/submitqueue/api/runway/protopb"
3336
"github.com/uber/submitqueue/platform/consumer"
@@ -44,6 +47,7 @@ import (
4447
"github.com/uber/submitqueue/runway/controller/merge"
4548
"github.com/uber/submitqueue/runway/controller/mergeconflictcheck"
4649
"github.com/uber/submitqueue/runway/extension/merger"
50+
gitmerger "github.com/uber/submitqueue/runway/extension/merger/git"
4751
"github.com/uber/submitqueue/runway/extension/merger/noop"
4852
"go.uber.org/zap"
4953
"google.golang.org/grpc"
@@ -164,7 +168,10 @@ func run() error {
164168
gate,
165169
)
166170

167-
mergerFactory := newMergerFactory()
171+
mergerFactory, err := newMergerFactory(logger, scope.SubScope("merger"))
172+
if err != nil {
173+
return fmt.Errorf("failed to create merger factory: %w", err)
174+
}
168175

169176
mergeConflictCheckController := mergeconflictcheck.NewController(mergeconflictcheck.Params{
170177
Logger: logger.Sugar(),
@@ -295,10 +302,65 @@ func run() error {
295302
return err
296303
}
297304

298-
// newMergerFactory returns a merger.Factory for the example server. The noop
299-
// implementation always succeeds; a real deployment wires a VCS-backed factory.
300-
func newMergerFactory() merger.Factory {
301-
return &noopMergerFactory{}
305+
// newMergerFactory returns a merger.Factory for the server. When
306+
// MERGE_CHECKOUT_PATH is set it wires the git-backed merger built from the
307+
// MERGE_* / GIT_* environment; otherwise it falls back to the noop merger so
308+
// local development and compose runs need no git checkout.
309+
func newMergerFactory(logger *zap.Logger, scope tally.Scope) (merger.Factory, error) {
310+
checkoutPath := os.Getenv("MERGE_CHECKOUT_PATH")
311+
if checkoutPath == "" {
312+
logger.Info("MERGE_CHECKOUT_PATH not set; using noop merger")
313+
return &noopMergerFactory{}, nil
314+
}
315+
316+
defaultStrategy, err := parseStrategy(os.Getenv("MERGE_DEFAULT_STRATEGY"))
317+
if err != nil {
318+
return nil, err
319+
}
320+
321+
target := envOr("MERGE_TARGET", "main")
322+
m, err := gitmerger.NewMerger(gitmerger.Params{
323+
CheckoutPath: checkoutPath,
324+
Remote: envOr("MERGE_REMOTE", "origin"),
325+
Target: target,
326+
DefaultStrategy: defaultStrategy,
327+
Runtime: gitmerger.GitRuntime{
328+
Executable: os.Getenv("GIT_EXECUTABLE"),
329+
ExecPath: os.Getenv("GIT_EXEC_PATH"),
330+
TemplateDir: os.Getenv("GIT_TEMPLATE_DIR"),
331+
},
332+
CommitterName: os.Getenv("MERGE_COMMITTER_NAME"),
333+
CommitterEmail: os.Getenv("MERGE_COMMITTER_EMAIL"),
334+
FetchRefspecs: splitRefspecs(os.Getenv("MERGE_FETCH_REFSPECS")),
335+
CheckStaleness: envBool("MERGE_CHECK_STALENESS", true),
336+
// Off by default: it lifts git's refusal to join two unrelated history
337+
// graphs, which is a safeguard everywhere except a queue whose purpose
338+
// is importing one repository's history into another.
339+
AllowUnrelatedHistories: envBool("MERGE_ALLOW_UNRELATED_HISTORIES", false),
340+
Logger: logger.Sugar(),
341+
MetricsScope: scope,
342+
})
343+
if err != nil {
344+
return nil, fmt.Errorf("failed to build git merger: %w", err)
345+
}
346+
logger.Info("git merger configured",
347+
zap.String("checkout", checkoutPath),
348+
zap.String("target", target),
349+
zap.String("default_strategy", defaultStrategy.String()),
350+
)
351+
return &gitMergerFactory{merger: m}, nil
352+
}
353+
354+
// gitMergerFactory returns a single git-backed merger for every queue. The
355+
// merger owns one checkout and serializes its own operations, so one instance
356+
// is shared across queues. A deployment that lands multiple targets wires a
357+
// factory with a per-queue map instead.
358+
type gitMergerFactory struct {
359+
merger merger.Merger
360+
}
361+
362+
func (f *gitMergerFactory) For(_ merger.Config) (merger.Merger, error) {
363+
return f.merger, nil
302364
}
303365

304366
type noopMergerFactory struct{}
@@ -307,6 +369,58 @@ func (f *noopMergerFactory) For(_ merger.Config) (merger.Merger, error) {
307369
return noop.New(), nil
308370
}
309371

372+
// parseStrategy maps the MERGE_DEFAULT_STRATEGY env value to a concrete merge
373+
// strategy, defaulting to REBASE when unset. DEFAULT is rejected because it
374+
// cannot itself be the default a step resolves to.
375+
func parseStrategy(name string) (mergestrategypb.Strategy, error) {
376+
switch strings.ToUpper(strings.TrimSpace(name)) {
377+
case "", "REBASE":
378+
return mergestrategypb.Strategy_REBASE, nil
379+
case "SQUASH_REBASE":
380+
return mergestrategypb.Strategy_SQUASH_REBASE, nil
381+
case "MERGE":
382+
return mergestrategypb.Strategy_MERGE, nil
383+
case "PROMOTE":
384+
return mergestrategypb.Strategy_PROMOTE, nil
385+
default:
386+
return mergestrategypb.Strategy_DEFAULT, fmt.Errorf("invalid MERGE_DEFAULT_STRATEGY %q", name)
387+
}
388+
}
389+
390+
// splitRefspecs parses the comma-separated MERGE_FETCH_REFSPECS value. Empty
391+
// entries are dropped so a trailing comma is harmless.
392+
func splitRefspecs(v string) []string {
393+
var out []string
394+
for _, part := range strings.Split(v, ",") {
395+
if trimmed := strings.TrimSpace(part); trimmed != "" {
396+
out = append(out, trimmed)
397+
}
398+
}
399+
return out
400+
}
401+
402+
// envBool reads a boolean environment value, returning fallback when unset or
403+
// unparseable.
404+
func envBool(key string, fallback bool) bool {
405+
v := strings.TrimSpace(os.Getenv(key))
406+
if v == "" {
407+
return fallback
408+
}
409+
parsed, err := strconv.ParseBool(v)
410+
if err != nil {
411+
return fallback
412+
}
413+
return parsed
414+
}
415+
416+
// envOr returns the environment value for key, or fallback when unset.
417+
func envOr(key, fallback string) string {
418+
if v := os.Getenv(key); v != "" {
419+
return v
420+
}
421+
return fallback
422+
}
423+
310424
// newTopicRegistry builds the TopicRegistry for Runway's merge queues. Inbound
311425
// topics (merge-conflict-check, merge) have subscriptions; outbound signal topics
312426
// are publish-only.

0 commit comments

Comments
 (0)