Skip to content

Commit 5957263

Browse files
committed
docs(submitqueue): provider example configuration and landing runbook
## Summary ### Why? Every piece needed to land a change against a live provider was in place, but nothing said how to point the stack at one — and the pieces that vary by provider were scattered across environment variables and two config files with no worked example. ### What? Which provider a deployment targets becomes a bind-mounted directory: `make local-provider-start PROVIDER=github` mounts `example/provider/<name>/` into the orchestrator and Runway. Because the choice is a mount rather than a build input, switching providers needs no rebuild, and adding one is a directory plus the extension implementations behind it — no compose, Makefile, or image change. The compose overlay names no provider at all. It requires the token rather than defaulting it: falling back to the fake integrations would let the stack report a change as landed without having gone near the provider, which is a far worse failure than not starting. `example/provider/README.md` carries the complete touchpoint list for adding a provider, and — more usefully — what is *not* on it: the merger's apply and push paths, the head-branch update, the pipeline, the wire contract, and the hermetic git E2E are all provider-independent. The runbook covers the two first-run traps that are easy to misdiagnose. Branch protection on the target rejects the merger's push like anyone else's. And a change that lands but stays open is either a fork (expected and permanent — its head branch lives in another repository) or head-branch protection blocking the update; Runway's logs distinguish them. ## Test Plan ✅ `docker compose -f docker-compose.yml -f docker-compose.provider.yml config` validates, and fails fast with `required variable GITHUB_TOKEN is missing a value` when the token is unset. ✅ `make help` lists `land`, `local-provider-start`, and `local-provider-stop`. Landing against a live provider is manual by nature — it needs a scratch repository and a token, so it is not automated in CI. The runbook is the procedure. # Conflicts: # service/submitqueue/demo/provider/README.md # service/submitqueue/demo/provider/github/merge.yaml # service/submitqueue/demo/provider/github/profiles.yaml
1 parent 93b88dc commit 5957263

7 files changed

Lines changed: 408 additions & 2 deletions

File tree

Makefile

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,17 @@ COMPOSE = docker-compose
66

77
# SubmitQueue compose files
88
COMPOSE_FILE = service/submitqueue/docker-compose.yml
9+
PROVIDER_COMPOSE_FILE = service/submitqueue/docker-compose.provider.yml
910
GATEWAY_COMPOSE_FILE = service/submitqueue/gateway/server/docker-compose.yml
1011
ORCHESTRATOR_COMPOSE_FILE = service/submitqueue/orchestrator/server/docker-compose.yml
1112

1213
# Fixed project name for local manual testing (tests use unique random names)
1314
SUBMITQUEUE_LOCAL_PROJECT = submitqueue
1415

16+
# Separate project for the provider demo stack, so it can run alongside the plain
17+
# local stack without the two sharing containers or volumes.
18+
PROVIDER_LOCAL_PROJECT = submitqueue-provider
19+
1520
# Stovepipe compose file (single Ping-only service)
1621
STOVEPIPE_COMPOSE_FILE = service/stovepipe/docker-compose.yml
1722
STOVEPIPE_DEBUG_COMPOSE_FILE = service/stovepipe/docker-compose.debug.yml
@@ -41,6 +46,17 @@ PROTO_PACKAGES = api/base/change api/base/mergestrategy api/base/messagequeue ap
4146
# Set REPO_ROOT for docker-compose
4247
export REPO_ROOT := $(shell pwd)
4348

49+
# Which provider the demo stack targets. Selects a configuration directory rather
50+
# than a code path, so adding a provider means adding a directory — see
51+
# service/submitqueue/demo/provider/README.md.
52+
PROVIDER ?= github
53+
export SQ_PROVIDER_CONFIG_DIR ?= $(REPO_ROOT)/service/submitqueue/demo/provider/$(PROVIDER)
54+
55+
# Defaults for `make land` against the provider demo stack.
56+
QUEUE ?= demo-queue
57+
STRATEGY ?= SQUASH_REBASE
58+
GATEWAY_ADDR ?= localhost:8081
59+
4460
# Fails if git working tree is dirty. Usage: $(call assert_clean,fix command)
4561
define assert_clean
4662
@if ! git diff --quiet; then \
@@ -136,7 +152,7 @@ deps: tidy-go ## Download and tidy Go dependencies
136152

137153
e2e-git-test: ## Run the hermetic git E2E (real merger against a bare repo; no credentials)
138154
@echo "Running hermetic git end-to-end tests..."
139-
@$(BAZEL) test //test/e2e/submitqueue:go_default_test --test_output=errors \\
155+
@$(BAZEL) test //test/e2e/submitqueue:go_default_test --test_output=errors \
140156
--test_filter='TestGitMergeE2E'
141157

142158
e2e-test: ## Run end-to-end tests (hermetic; Bazel builds all inputs; runs in parallel)
@@ -174,6 +190,26 @@ integration-test-submitqueue-orchestrator: ## Run Orchestrator integration tests
174190
@echo "Running Orchestrator integration tests..."
175191
@$(BAZEL) test //test/integration/submitqueue/orchestrator:go_default_test --test_output=streamed
176192

193+
land: ## Land a change or a stack (PR=<url>, PRS="<url> <url>", or URI=<change-uri>)
194+
@if [ -z "$(PR)$(PRS)$(URI)$(URIS)" ]; then \
195+
echo "Usage: make land PR=https://github.com/owner/repo/pull/7"; \
196+
echo " stack: make land PRS=\"<url-1> <url-2> <url-3>\" (order is the stack order)"; \
197+
echo " by uri: make land URI=github://github.com/owner/repo/pull/7/<sha>"; \
198+
echo " opts: QUEUE=$(QUEUE) STRATEGY=$(STRATEGY) GATEWAY_ADDR=$(GATEWAY_ADDR)"; \
199+
exit 2; \
200+
fi
201+
@$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
202+
-addr $(GATEWAY_ADDR) land \
203+
-queue $(QUEUE) \
204+
-strategy $(STRATEGY) \
205+
$(if $(PR),-pr $(PR)) $(foreach p,$(PRS),-pr $(p)) \
206+
$(if $(URI),-uri $(URI)) $(foreach u,$(URIS),-uri $(u))
207+
208+
land-status: ## Read a landed request's status (SQID=... [QUEUE=demo-queue])
209+
@if [ -z "$(SQID)" ]; then echo "Usage: make land-status SQID=demo-queue/1 [QUEUE=demo-queue]"; exit 2; fi
210+
@$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
211+
-addr $(GATEWAY_ADDR) status -queue $(QUEUE) -sqid $(SQID)
212+
177213
license-fix: ## Add missing license headers to source files
178214
@$(BAZEL) run //tool/linter/licenseheader -- --fix
179215

@@ -217,6 +253,26 @@ local-submitqueue-gateway-stop: ## Stop Gateway service
217253
@$(COMPOSE) -f $(GATEWAY_COMPOSE_FILE) -p $(SUBMITQUEUE_LOCAL_PROJECT) down
218254
@echo "Gateway services stopped."
219255

256+
local-provider-start: build-all-linux ## Start the full stack against a real provider (PROVIDER=github; needs GITHUB_TOKEN)
257+
@echo "Starting full stack against provider '$(PROVIDER)' ($(SQ_PROVIDER_CONFIG_DIR))..."
258+
@test -f "$(SQ_PROVIDER_CONFIG_DIR)/merge.yaml" \
259+
|| { echo "No such provider '$(PROVIDER)': $(SQ_PROVIDER_CONFIG_DIR)/merge.yaml not found"; exit 2; }
260+
@$(COMPOSE) -f $(COMPOSE_FILE) -f $(PROVIDER_COMPOSE_FILE) -p $(PROVIDER_LOCAL_PROJECT) up -d --build --wait
261+
@echo "Applying database schemas..."
262+
@$(MAKE) -s local-init-submitqueue-schemas SUBMITQUEUE_LOCAL_PROJECT=$(PROVIDER_LOCAL_PROJECT)
263+
@echo ""
264+
@echo "✅ Stack is running against provider '$(PROVIDER)'."
265+
@echo ""
266+
@echo "Gateway gRPC port: $$(docker port $(PROVIDER_LOCAL_PROJECT)-gateway-service-1 8080 2>/dev/null | cut -d: -f2 || echo 'unknown')"
267+
@echo ""
268+
@echo "Land a change with:"
269+
@echo " make land PR=https://github.com/owner/repo/pull/7 GATEWAY_ADDR=localhost:<gateway port>"
270+
271+
local-provider-stop: ## Stop the provider demo stack
272+
@echo "Stopping provider stack..."
273+
@$(COMPOSE) -f $(COMPOSE_FILE) -f $(PROVIDER_COMPOSE_FILE) -p $(PROVIDER_LOCAL_PROJECT) down
274+
@echo "Provider stack stopped."
275+
220276
local-init-submitqueue-schemas: ## Manually apply all database schemas
221277
@echo "Applying storage schema to mysql-app..."
222278
@for file in submitqueue/extension/storage/mysql/schema/*.sql; do \

doc/howto/PROVIDER-E2E.md

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
# Landing real changes against a provider
2+
3+
How to run the whole pipeline against a live repository and watch a change actually land. This is the manual tier: it needs a scratch repository and a token, which is why it is not automated in CI.
4+
5+
Two tiers below it run with no credentials at all and cover most of what can break:
6+
7+
| | Command | Covers | Secrets |
8+
|---|---|---|---|
9+
| Tier 1 | `make e2e-test` | pipeline choreography, on the noop merger | none |
10+
| Tier 2 | `make e2e-git-test` | real git: provisioning, cherry-pick, atomic push, head-branch updates | none |
11+
| Tier 3 | this document | the change provider: reading metadata, its CI, changes marked merged | a token |
12+
13+
Run tier 2 first. If the merge machinery is broken, it will say so in under a minute and without a repository to clean up afterwards.
14+
15+
## What you need
16+
17+
A **scratch repository** you are willing to have commits pushed to and branches force-moved on. Do not point this at anything you care about — the merger pushes to the target branch and rewrites the head branch of every change it lands.
18+
19+
A **token** for it, scoped to that one repository.
20+
21+
For a **fine-grained** token, grant these repository permissions. Each is here because a specific component needs it, so you can drop the last two if you are not using those pieces:
22+
23+
| Permission | Access | Needed by |
24+
|---|---|---|
25+
| Metadata | Read | mandatory on every fine-grained token; GitHub adds it for you |
26+
| Contents | Read and write | the git merger — clone, fetch, push to the target branch, and force-move each landed change's head branch |
27+
| Pull requests | Read | the change provider reads pull request metadata, and `land -pr` reads the head commit |
28+
| Pull requests | Read **and write** | only for `make demo-prs`, which opens pull requests |
29+
| Actions | Read and write | only if you switch the build runner to GitHub Actions — dispatch a run, poll it, cancel it |
30+
31+
A **classic** PAT needs `repo`, plus `workflow` if you use the GitHub Actions build runner.
32+
33+
Two things people get caught by. Fine-grained tokens must have the repository explicitly selected under "Repository access" — org-owned repositories also need the org to have approved fine-grained tokens at all. And **Contents: Read and write is the one that cannot be reduced**: landing *is* pushing, so a read-only token fails at the last step, after everything else has appeared to work.
34+
35+
## Configure
36+
37+
Everything provider-specific is one directory: [`service/submitqueue/demo/provider/github/`](../../service/submitqueue/demo/provider/github). Edit the three marked lines in `merge.yaml`:
38+
39+
```yaml
40+
remoteUrl: https://github.com/<you>/<your-scratch-repo>.git
41+
target: main
42+
checkoutPath: /var/runway/checkouts/<your-scratch-repo>
43+
```
44+
45+
Neither file holds a secret — `tokenEnv: GITHUB_TOKEN` names the variable, and the value comes from your environment.
46+
47+
## Run
48+
49+
```bash
50+
export GITHUB_TOKEN=ghp_...
51+
make local-provider-start PROVIDER=github
52+
```
53+
54+
The stack refuses to start without the token rather than falling back to the fake integrations. That is deliberate: a stack that silently runs on fakes reports changes as landed without having gone near the provider, which is a much worse way to find out.
55+
56+
`local-provider-start` prints the gateway's port. Export it so the commands below are shorter:
57+
58+
```bash
59+
export GATEWAY_ADDR=localhost:<port>
60+
```
61+
62+
## Land a single change
63+
64+
Open a pull request against `main` in the scratch repo, then:
65+
66+
```bash
67+
make land PR=https://github.com/<you>/<repo>/pull/1
68+
```
69+
70+
`land` resolves the pull request's head commit and prints the change URI it built, so there is no 40-character SHA to copy. It returns an `sqid`; follow it with:
71+
72+
```bash
73+
make land-status SQID=<sqid> GATEWAY_ADDR=$GATEWAY_ADDR
74+
```
75+
76+
The status walks `accepted → started → validated → batched → landed`. When it reaches `landed`, on GitHub the pull request shows **Merged** and its commit is on `main`.
77+
78+
Worth understanding *why* it shows merged, because nothing called an API to close it. A provider marks a change merged once its head commit is reachable from the target branch. `SQUASH_REBASE` rewrites the commits, so the pull request's original head is nowhere in `main` — and `updateHeadBranch` therefore moves the pull request's branch to the commit it landed as. GitHub draws its own conclusion from that.
79+
80+
## Land a stack
81+
82+
Open a chain of pull requests where each targets the previous one's branch, then submit them in order:
83+
84+
```bash
85+
make land PRS="https://github.com/<you>/<repo>/pull/1 \
86+
https://github.com/<you>/<repo>/pull/2 \
87+
https://github.com/<you>/<repo>/pull/3"
88+
```
89+
90+
The order of `PRS` is the stack order. All three land as **one push** to `main` — there is no window where a reader sees the stack half-applied — and all three show as merged. Tier 2 asserts the single-push property mechanically, by counting ref updates in the target's reflog.
91+
92+
## Watching it work
93+
94+
```bash
95+
docker compose -p submitqueue-provider logs -f runway-service
96+
```
97+
98+
Runway logs each merge and each head-branch move:
99+
100+
```
101+
moved change head branch to its landed commit {"change": "you/repo#1", "branch": "refs/heads/feature-a", ...}
102+
```
103+
104+
## When it does not work
105+
106+
**The push is rejected on the first try.** Branch protection on `main` — required status checks, or a linear-history or no-force-push rule — applies to the merger like anyone else. Either relax it on the scratch repo or add the token's identity to the bypass list.
107+
108+
**The change lands but the pull request stays open.** Two causes, distinguishable in Runway's logs.
109+
110+
If the change came from a **fork**, this is expected and permanent: the head branch lives in the contributor's repository, which this stack has no business writing to. The log says `no head branch on this remote for change`. The change is on `main`; only the pull request's status is wrong.
111+
112+
Otherwise it is **protection on the head branch** blocking the force update. The log says `could not move change head branch`. Note the land itself succeeded — the failure is reported and deliberately not retried, because the push already happened and cannot be undone.
113+
114+
**A change is rejected as stale.** Its head moved after it was submitted, so the commit named is no longer the one under review. Re-submit it. This also happens if you re-land a change that already landed, since landing moved its branch.
115+
116+
**Everything reports `error` immediately.** Check the queue name exists in [`queues.yaml`](../../service/submitqueue/gateway/server/queues.yaml) and matches the one in `profiles.yaml` and `merge.yaml`. A queue with no entry in `merge.yaml` gets the noop merger by design, so it will appear to land without pushing anything.
117+
118+
## Using real CI
119+
120+
The demo keeps the build runner fake so a land finishes in seconds. Switching to real GitHub Actions takes three things.
121+
122+
**1. The workflow must be dispatchable.** The runner triggers builds with `POST /actions/workflows/{id}/dispatches`, which only works if the workflow declares `workflow_dispatch`. A typical scratch-repo `ci.yml` triggered on `pull_request` alone cannot be dispatched at all — GitHub rejects it. Add the trigger and the inputs the runner sends:
123+
124+
```yaml
125+
on:
126+
pull_request:
127+
merge_group:
128+
workflow_dispatch:
129+
inputs:
130+
sq_head_uris:
131+
description: "JSON array of change URIs in the batch under test"
132+
required: false
133+
sq_base_uris:
134+
description: "JSON array of in-flight change URIs this batch speculates on top of"
135+
required: false
136+
sq_queue:
137+
description: "SubmitQueue queue name"
138+
required: false
139+
sq_metadata:
140+
description: "Caller-supplied build metadata, as JSON"
141+
required: false
142+
```
143+
144+
**2. Point the runner at it.** Replace the `buildRunner` line for the queue in `profiles.yaml`:
145+
146+
```yaml
147+
buildRunner:
148+
type: githubactions
149+
owner: behinddwalls
150+
repo: sq-demo
151+
workflow: ci.yml # file name or numeric workflow id
152+
ref: main # the branch the workflow definition is read from
153+
```
154+
155+
**3. Grant Actions: Read and write** on the token (see the permissions table above).
156+
157+
One caveat worth understanding before you rely on the result. A workflow that only checks out the pull request tests *that change alone* — which is not what a submit queue is for. The point of speculation is to test the **combination**: `sq_base_uris` are the in-flight changes assumed to land first, and `sq_head_uris` is the batch under test on top of them. Until the workflow actually applies both, a green run says nothing about whether the batch lands cleanly, and the queue is only exercising its trigger-and-poll loop.
158+
159+
## Clean up
160+
161+
```bash
162+
make local-provider-stop
163+
```
164+
165+
The scratch repository keeps whatever landed; reset it with `git push --force` from a known-good commit.
166+
167+
## Another provider
168+
169+
Nothing above is GitHub-specific except the contents of the config directory and the URI parser behind it. Adding GitLab or another provider is a new directory here plus a handful of new files beside the existing ones — the complete list is in [`service/submitqueue/demo/provider/README.md`](../../service/submitqueue/demo/provider/README.md).
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Provider configurations
2+
3+
Each directory here is one **provider** — a code-hosting system SubmitQueue lands changes on — described entirely as configuration. A directory holds two files:
4+
5+
| File | Selects |
6+
|---|---|
7+
| `profiles.yaml` | the change provider, build runner, and conflict analyzer each queue resolves to (read by the orchestrator) |
8+
| `merge.yaml` | the merge target each queue lands on (read by Runway) |
9+
10+
Neither holds a secret. Each integration names the *environment variable* carrying its credential, so these files stay committable and rotating a token needs no edit.
11+
12+
Pick one with `make local-provider-start PROVIDER=<name>`, which bind-mounts that directory into the orchestrator and Runway. Because the choice is a mount rather than a build input, switching providers needs no rebuild.
13+
14+
| Directory | What it demonstrates |
15+
|---|---|
16+
| [`github/`](github) | a live provider: GitHub change metadata, a real repository, pull requests marked merged |
17+
| [`local/`](local) | a plain git remote with no provider at all — used by the hermetic git E2E (`make e2e-git-test`) |
18+
19+
`local/` is worth reading first. It is proof that the merge machinery has no provider in it: the same Runway code path lands changes against a bare repository addressed by path, with no credential and no API.
20+
21+
## Adding a provider
22+
23+
Everything provider-specific is reached through an existing seam, so a new provider is new code beside the old, not a change to it. The complete list:
24+
25+
| Touchpoint | Why |
26+
|---|---|
27+
| `platform/base/change/{provider}/change_id.go` | parse `{provider}://…` change URIs |
28+
| `runway/extension/merger/git/changeref.go` | one `case` in `resolveChange`, mapping the URI to the ref the provider publishes a change's head under — GitHub `refs/pull/{n}/head`, GitLab `refs/merge-requests/{iid}/head` |
29+
| `submitqueue/extension/changeprovider/{provider}/` and one case in `changeprovider/routing` | fetch change metadata |
30+
| `submitqueue/extension/buildrunner/{provider}/` | only if the provider's CI is not already covered by the Buildkite or GitHub Actions runners |
31+
| `service/submitqueue/gateway/client/main.go` | one case in `resolvePullRequest`, so `land -pr <url>` accepts the provider's change URLs |
32+
| this directory | a `{provider}/` with the two files above |
33+
34+
What is **not** on that list is the point of it: the merger's apply and push paths, the head-branch update, the orchestrator pipeline, the wire contract, the compose overlay, and the hermetic git E2E are all provider-independent and need no change.
35+
36+
Two of those deserve explanation.
37+
38+
**The merger stays provider-neutral** because `resolveChange` reduces every URI to the same three things — the commit to apply, the ref it lives under, and a label — before any git command runs. The apply paths never learn which provider a change came from.
39+
40+
**Marking a change merged needs no provider API.** A provider decides whether a change merged while it processes the push to the target branch, comparing the change's recorded head against what that push makes reachable. `MERGE` and `PROMOTE` satisfy that by construction; the rewriting strategies do not, so `updateHeadBranch` moves the change's head branch to the commit it landed as — as its own push, immediately before the target is pushed. The ordering is the mechanism: a head moved *after* the target has been pushed, or in the same atomic push, is recorded too late, and the provider marks the change closed rather than merged even though its head is demonstrably on the target. That works by matching a SHA against the remote's branch tips — no change number, no API call — so it behaves identically for a GitHub pull request and a GitLab merge request. The one case it cannot serve is a change proposed from a fork, whose head branch lives in another repository: such a change lands and stays open.
41+
42+
See [doc/howto/PROVIDER-E2E.md](../../../../doc/howto/PROVIDER-E2E.md) for running a real land end to end.

0 commit comments

Comments
 (0)