diff --git a/Makefile b/Makefile index 9d8cd31d..c3b1de69 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,6 @@ COMPOSE = docker-compose # SubmitQueue compose files COMPOSE_FILE = service/submitqueue/docker-compose.yml -PROVIDER_COMPOSE_FILE = service/submitqueue/docker-compose.provider.yml GATEWAY_COMPOSE_FILE = service/submitqueue/gateway/server/docker-compose.yml ORCHESTRATOR_COMPOSE_FILE = service/submitqueue/orchestrator/server/docker-compose.yml @@ -46,15 +45,40 @@ PROTO_PACKAGES = api/base/change api/base/mergestrategy api/base/messagequeue ap # Set REPO_ROOT for docker-compose export REPO_ROOT := $(shell pwd) -# Which provider the demo stack targets. Selects a configuration directory rather -# than a code path, so adding a provider means adding a directory — see +# Which provider the demo stack targets, and the only difference between a free +# local run and a live one. Selects a configuration directory rather than a code +# path, so adding a provider is mostly adding a directory — see # service/submitqueue/demo/provider/README.md. -PROVIDER ?= github +# +# fake a change is a URI; nothing merges anywhere. Needs nothing. +# git branches in a bare repository on disk; real fetch, cherry-pick, push. +# github real pull requests. Needs a repository and GITHUB_TOKEN. +PROVIDER ?= fake export SQ_PROVIDER_CONFIG_DIR ?= $(REPO_ROOT)/service/submitqueue/demo/provider/$(PROVIDER) -# Defaults for `make land` / `make demo-pr` against the provider demo stack. +# Which compose overlay each mode needs. This cannot live in the provider +# directory: the two config files say how the services are configured, not what +# has to be mounted or which credential has to be present for them to start. +PROVIDER_COMPOSE_FILE_fake = service/submitqueue/docker-compose.fake.yml +PROVIDER_COMPOSE_FILE_git = service/submitqueue/docker-compose.git.yml +PROVIDER_COMPOSE_FILE_github = service/submitqueue/docker-compose.provider.yml +PROVIDER_COMPOSE_FILE = $(PROVIDER_COMPOSE_FILE_$(PROVIDER)) + +# Where PROVIDER=git keeps the bare repository it merges into. Outside the +# repository, so a demo leaves nothing in a checkout, and bind-mounted rather +# than kept in a volume so `git log` on the host can show what landed. +# +# SQ_RUNWAY_CHECKOUT_DIR is deliberately not set: unset, Runway's working trees +# live in a named volume instead of on the host. See docker-compose.git.yml. +export SQ_GIT_SANDBOX_DIR ?= /tmp/sq-sandbox + +# Defaults for `make land` / `make demo-requests` against the provider demo stack. DEMO_REPO ?= behinddwalls/sq-demo COUNT ?= 3 +# How many folders demo-requests spreads its changes across, which decides how +# much they conflict: changes sharing a folder are batched in order, changes in +# different folders go out together. 0 picks one per run. +FOLDERS ?= 0 FILES ?= 3 CONCURRENCY ?= 5 STACKED ?= false @@ -77,7 +101,7 @@ define assert_clean fi endef -.PHONY: build build-all-linux build-runway-linux build-submitqueue-gateway-client build-submitqueue-gateway-linux build-submitqueue-gateway-server build-submitqueue-orchestrator-linux build-stovepipe-linux build-stovepipe-linux-debug check-gazelle check-mocks check-tidy clean clean-proto deps e2e-test fmt gazelle integration-test integration-test-submitqueue-consumer integration-test-extensions integration-test-submitqueue-gateway integration-test-submitqueue-orchestrator license-fix lint lint-binary lint-fmt lint-license local-init-runway-queue-schema local-init-stovepipe-schemas local-runway-start local-runway-stop local-submitqueue-clean local-submitqueue-gateway-start local-submitqueue-gateway-stop local-init-submitqueue-schemas local-submitqueue-logs local-submitqueue-orchestrator-start local-submitqueue-orchestrator-stop local-submitqueue-ps local-submitqueue-restart local-submitqueue-start local-stop local-stovepipe-debug-start local-stovepipe-logs local-stovepipe-start local-stovepipe-stop mocks proto query-deps query-targets run-client-runway run-client-submitqueue-gateway run-client-submitqueue-orchestrator run-client-stovepipe run-queue-admin test test-no-cache tidy tidy-bazel tidy-go help +.PHONY: build build-all-linux build-runway-linux build-submitqueue-gateway-client build-submitqueue-gateway-linux build-submitqueue-gateway-server build-submitqueue-orchestrator-linux build-stovepipe-linux build-stovepipe-linux-debug check-gazelle check-mocks check-tidy clean clean-proto demo-requests deps e2e-test fmt gazelle integration-test integration-test-submitqueue-consumer integration-test-extensions integration-test-submitqueue-gateway integration-test-submitqueue-orchestrator license-fix lint lint-binary lint-fmt lint-license local-init-runway-queue-schema local-init-stovepipe-schemas local-runway-start local-runway-stop local-submitqueue-stop local-submitqueue-clean local-submitqueue-gateway-start local-submitqueue-gateway-stop local-init-submitqueue-schemas local-submitqueue-logs local-submitqueue-orchestrator-start local-submitqueue-orchestrator-stop local-submitqueue-ps local-submitqueue-restart local-submitqueue-start local-stop local-stovepipe-debug-start local-stovepipe-logs local-stovepipe-start local-stovepipe-stop mocks proto query-deps query-targets run-client-runway run-client-submitqueue-gateway run-client-submitqueue-orchestrator run-client-stovepipe run-queue-admin test test-no-cache tidy tidy-bazel tidy-go help build: ## Build all services and examples @@ -172,10 +196,13 @@ clean-proto: ## Clean generated proto files @rm -f $(foreach p,$(PROTO_PACKAGES),$(p)/protopb/*.pb.go $(p)/protopb/*.pb.yarpc.go) @echo "Proto clean complete!" -demo-pr: ## Create N PRs in the demo repo, enqueue each as it is created, and watch (COUNT=3 FILES=3 CONCURRENCY=5; needs GITHUB_TOKEN) - @$(BAZEL) run //service/submitqueue/demo/pr -- \ +demo-requests: ## Create N changes, enqueue each as it is created, and watch (PROVIDER=fake|git|github COUNT=3 FOLDERS=0 FILES=3 CONCURRENCY=5) + @$(BAZEL) run //service/submitqueue/demo/requests -- \ + -provider $(PROVIDER) \ -repo $(DEMO_REPO) \ + -sandbox-dir $(SQ_GIT_SANDBOX_DIR) \ -count $(COUNT) \ + -folders $(FOLDERS) \ -files $(FILES) \ -concurrency $(CONCURRENCY) \ -stacked=$(STACKED) \ @@ -277,10 +304,13 @@ lint-message-id: ## Check queue messages are only constructed through platform/p lint-queue-shard: ## Check every table's primary key leads with the queue column @$(BAZEL) run //tool/linter/queueshard -local-submitqueue-clean: ## Stop and remove all local services, volumes, and images +local-submitqueue-clean: ## Stop the stack and remove its volumes, images, and PROVIDER=git's sandbox @echo "Cleaning all services and data..." - @$(COMPOSE) -f $(COMPOSE_FILE) -p $(SUBMITQUEUE_LOCAL_PROJECT) down -v --rmi local - @echo "All services, volumes, and images removed." + @# The overlay is named so that volumes it declares — Runway's checkouts — + @# are removed too, rather than surviving as an orphan. + @$(COMPOSE) -f $(COMPOSE_FILE) -f $(PROVIDER_COMPOSE_FILE) -p $(SUBMITQUEUE_LOCAL_PROJECT) down -v --rmi local + @rm -rf "$(SQ_GIT_SANDBOX_DIR)" + @echo "All services, volumes, images, and $(SQ_GIT_SANDBOX_DIR) removed." local-submitqueue-gateway-start: build-submitqueue-gateway-linux ## Start Gateway service locally (Gateway + 2 MySQL databases) @echo "Starting Gateway with docker-compose..." @@ -301,26 +331,6 @@ local-submitqueue-gateway-stop: ## Stop Gateway service @$(COMPOSE) -f $(GATEWAY_COMPOSE_FILE) -p $(SUBMITQUEUE_LOCAL_PROJECT) down @echo "Gateway services stopped." -local-provider-start: build-all-linux ## Start the full stack against a real provider (PROVIDER=github; needs GITHUB_TOKEN) - @echo "Starting full stack against provider '$(PROVIDER)' ($(SQ_PROVIDER_CONFIG_DIR))..." - @test -f "$(SQ_PROVIDER_CONFIG_DIR)/merge.yaml" \ - || { echo "No such provider '$(PROVIDER)': $(SQ_PROVIDER_CONFIG_DIR)/merge.yaml not found"; exit 2; } - @$(COMPOSE) -f $(COMPOSE_FILE) -f $(PROVIDER_COMPOSE_FILE) -p $(PROVIDER_LOCAL_PROJECT) up -d --build --wait - @echo "Applying database schemas..." - @$(MAKE) -s local-init-submitqueue-schemas SUBMITQUEUE_LOCAL_PROJECT=$(PROVIDER_LOCAL_PROJECT) - @echo "" - @echo "✅ Stack is running against provider '$(PROVIDER)'." - @echo "" - @echo "Gateway gRPC port: $$(docker port $(PROVIDER_LOCAL_PROJECT)-gateway-service-1 8080 2>/dev/null | cut -d: -f2 || echo 'unknown')" - @echo "" - @echo "Land a change with:" - @echo " make land PR=https://github.com/owner/repo/pull/7 GATEWAY_ADDR=localhost:" - -local-provider-stop: ## Stop the provider demo stack - @echo "Stopping provider stack..." - @$(COMPOSE) -f $(COMPOSE_FILE) -f $(PROVIDER_COMPOSE_FILE) -p $(PROVIDER_LOCAL_PROJECT) down - @echo "Provider stack stopped." - local-init-submitqueue-schemas: ## Manually apply all database schemas @echo "Applying storage schema to mysql-app..." @for file in submitqueue/extension/storage/mysql/schema/*.sql; do \ @@ -429,17 +439,48 @@ local-submitqueue-restart: build-all-linux ## Restart all services (rebuild and @echo "Services restarted!" @make local-submitqueue-ps -local-submitqueue-start: build-all-linux ## Start full stack (Gateway + Orchestrator + MySQL) - @echo "Starting full stack with docker-compose..." - @$(COMPOSE) -f $(COMPOSE_FILE) -p $(SUBMITQUEUE_LOCAL_PROJECT) up -d --build --wait +local-submitqueue-start: build-all-linux ## Start full stack (PROVIDER=fake|git|github; github needs GITHUB_TOKEN) + @echo "Starting full stack against provider '$(PROVIDER)' ($(SQ_PROVIDER_CONFIG_DIR))..." + @test -f "$(SQ_PROVIDER_CONFIG_DIR)/merge.yaml" \ + || { echo "No such provider '$(PROVIDER)': $(SQ_PROVIDER_CONFIG_DIR)/merge.yaml not found"; exit 2; } + @test -n "$(PROVIDER_COMPOSE_FILE)" \ + || { echo "Provider '$(PROVIDER)' has no compose overlay; add PROVIDER_COMPOSE_FILE_$(PROVIDER) to the Makefile"; exit 2; } + @if [ "$(PROVIDER)" = "git" ]; then \ + $(BAZEL) run //tool/gitsandbox -- -sandbox-dir "$(SQ_GIT_SANDBOX_DIR)" || exit 1; \ + fi + @# Rootless Docker maps container root to the host user; rootful Docker needs + @# the host UID:GID explicitly, or the services write files into the sandbox + @# bind mount that the host user cannot then read or remove. Resolved here + @# rather than at parse time, so `make help` does not shell out to Docker. + @set -e; \ + if docker info --format '{{json .SecurityOptions}}' 2>/dev/null | grep -q 'name=rootless'; then \ + export SQ_CONTAINER_USER=0:0; \ + else \ + export SQ_CONTAINER_USER=$$(id -u):$$(id -g); \ + fi; \ + $(COMPOSE) -f $(COMPOSE_FILE) -f $(PROVIDER_COMPOSE_FILE) -p $(SUBMITQUEUE_LOCAL_PROJECT) up -d --build --wait @echo "Applying database schemas..." @$(MAKE) -s local-init-submitqueue-schemas @echo "" - @echo "✅ Full stack is running!" + @echo "✅ Stack is running against provider '$(PROVIDER)'." + @echo "" + @echo "Gateway gRPC port: $$(docker port $(SUBMITQUEUE_LOCAL_PROJECT)-gateway-service-1 8080 2>/dev/null | cut -d: -f2 || echo 'unknown')" + @if [ "$(PROVIDER)" = "git" ]; then \ + echo "Merge target: $(SQ_GIT_SANDBOX_DIR)/sandbox.git"; \ + fi @echo "" - @make local-submitqueue-ps + @echo "Generate traffic with:" + @echo " make demo-requests GATEWAY_ADDR=localhost:" + +local-submitqueue-stop: ## Stop the SubmitQueue stack (keeps data and PROVIDER=git's sandbox) + @echo "Stopping SubmitQueue services..." + @$(COMPOSE) -f $(COMPOSE_FILE) -f $(PROVIDER_COMPOSE_FILE) -p $(SUBMITQUEUE_LOCAL_PROJECT) down + @echo "SubmitQueue services stopped. Data volumes preserved." + @if [ -d "$(SQ_GIT_SANDBOX_DIR)" ]; then \ + echo "Sandbox repository left at $(SQ_GIT_SANDBOX_DIR); remove it with 'make local-submitqueue-clean'."; \ + fi -local-stop: ## Stop all services (keep data) +local-stop: ## Stop every local stack — SubmitQueue, Stovepipe, and Runway (keep data) @echo "Stopping all services..." @$(COMPOSE) -f $(COMPOSE_FILE) -p $(SUBMITQUEUE_LOCAL_PROJECT) down @$(COMPOSE) -f $(STOVEPIPE_COMPOSE_FILE) -p $(STOVEPIPE_LOCAL_PROJECT) down diff --git a/README.md b/README.md index c241ea57..9c3b5ae1 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ Cross-domain Go code (errors, metrics, consumer framework, HTTP helpers, shared ## Quick Start -Land a change and watch it reach `landed`. Requires Docker and Docker Compose, and nothing else — no repository, no account, no token. See [Development Setup](doc/howto/DEVELOPMENT.md) for full prerequisites. +Put traffic through the queue and watch it land. Requires Docker and Docker Compose, and nothing else — no repository, no account, no token. See [Development Setup](doc/howto/DEVELOPMENT.md) for full prerequisites. ```bash # Start the full stack (Gateway + Orchestrator + Runway + MySQL) @@ -25,28 +25,33 @@ make local-submitqueue-start make local-submitqueue-ps export GATEWAY_ADDR=localhost: -# Submit a change, and follow the receipt it returns -make land QUEUE=test-queue \ - URI='git://git.example.com/demo/refs%2Fheads%2Ffeature-a/1111111111111111111111111111111111111111' -make land-status QUEUE=test-queue SQID=test-queue/1 +# Create changes, enqueue each as it is created, and watch them settle +make demo-requests # Stop services -make local-stop +make local-submitqueue-stop ``` -Every integration at the edges is faked — the change provider, CI, and the merge itself — so the run is free and finishes in seconds. The queue's own logic is real: validation, batching, conflict analysis, and speculation all run, and the request log records the full trail from `accepted` to `landed`. Nothing is pushed to any repository. +`PROVIDER` decides where changes come from and what landing them does, and it is the only thing that changes between them: -[Quickstart](doc/howto/QUICKSTART.md) explains the change URI, how to make a change fail on demand, and what this does and does not prove. From there, `make e2e-git-test` adds a real git merge (still no credentials), and [PROVIDER-E2E.md](doc/howto/PROVIDER-E2E.md) adds a live provider. See [service/README.md](service/README.md) for running individual services and clients. +| `PROVIDER` | A change is | Landing it | Needs | +|---|---|---|---| +| **`fake`** (default) | a URI, and nothing else | reports success without touching a repository | nothing | +| **`git`** | a branch in a bare repository on disk | a real fetch, cherry-pick and push | nothing | +| **`github`** | a real pull request | a real push to a real repository | a repository and a token | + +The queue's own logic is real in all three: validation, batching, conflict analysis, speculation, and a request log recording the full trail from `accepted` to `landed`. `PROVIDER=git make local-submitqueue-start` is the first rung where a commit actually reaches a branch, and it still needs no credential. + +[Quickstart](doc/howto/QUICKSTART.md) walks all three rungs — proving a change landed with `git log`, making one fail on demand, and what a live provider needs. See [service/README.md](service/README.md) for running individual services and clients. ## Documentation | Document | Description | |----------|-------------| -| [Quickstart](doc/howto/QUICKSTART.md) | Land a change locally with no credentials | +| [Quickstart](doc/howto/QUICKSTART.md) | Run the stack and land changes — fake, local git, or GitHub | | [Development Setup](doc/howto/DEVELOPMENT.md) | Prerequisites, build, environment, IDE setup | | [Contributing](CONTRIBUTING.md) | How to contribute, workflow, guidelines | | [Testing Guide](doc/howto/TESTING.md) | Unit, integration, and E2E testing patterns | -| [Landing real changes](doc/howto/PROVIDER-E2E.md) | Running the pipeline against a live provider | | [Architecture Guide](CLAUDE.md) | Project layout, patterns, conventions | | [Examples](service/README.md) | Running services, clients, API reference | | [RFCs](doc/rfc/index.md) | Design documents and proposals | diff --git a/doc/howto/DEVELOPMENT.md b/doc/howto/DEVELOPMENT.md index d88a3af0..eeddb818 100644 --- a/doc/howto/DEVELOPMENT.md +++ b/doc/howto/DEVELOPMENT.md @@ -64,16 +64,14 @@ make local-submitqueue-start make local-submitqueue-ps export GATEWAY_ADDR=localhost: -# 4. Land a change and follow it to a terminal status -make land QUEUE=test-queue \ - URI='git://git.example.com/demo/refs%2Fheads%2Ffeature-a/1111111111111111111111111111111111111111' -make land-status QUEUE=test-queue SQID=test-queue/1 +# 4. Create changes, enqueue them, and watch them land +make demo-requests # 5. Stop services make local-stop ``` -[QUICKSTART.md](QUICKSTART.md) walks through the same run in detail — what the change URI has to look like, how to make a change fail on demand, and which parts of the pipeline are faked. +[QUICKSTART.md](QUICKSTART.md) walks through the same run in detail, and on to `PROVIDER=git`, which lands real commits into a repository on disk — still with no credential. If any step fails, see [Troubleshooting](#troubleshooting) below. diff --git a/doc/howto/PROVIDER-E2E.md b/doc/howto/PROVIDER-E2E.md deleted file mode 100644 index 72e1bb77..00000000 --- a/doc/howto/PROVIDER-E2E.md +++ /dev/null @@ -1,254 +0,0 @@ -# Landing real changes against a provider - -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. - -Two tiers below it run with no credentials at all and cover most of what can break: - -| | Command | Covers | Secrets | -|---|---|---|---| -| Tier 1 | `make e2e-test` | pipeline choreography, on the noop merger | none | -| Tier 2 | `make e2e-git-test` | real git: provisioning, cherry-pick, atomic push, head-branch updates | none | -| Tier 3 | this document | the change provider: reading metadata, its CI, changes marked merged | a token | - -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. - -## What you need - -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. - -A **token** for it, scoped to that one repository. - -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: - -| Permission | Access | Needed by | -|---|---|---| -| Metadata | Read | mandatory on every fine-grained token; GitHub adds it for you | -| Contents | Read and write | the git merger — clone, fetch, push to the target branch, and force-move each landed change's head branch | -| Pull requests | Read | the change provider reads pull request metadata, and `land -pr` reads the head commit | -| Pull requests | Read **and write** | only for `make demo-pr`, which opens pull requests | -| Actions | Read and write | only if you switch the build runner to GitHub Actions — dispatch a run, poll it, cancel it | - -A **classic** PAT needs `repo`, plus `workflow` if you use the GitHub Actions build runner. - -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. - -## Configure - -Everything provider-specific is one directory: [`service/submitqueue/demo/provider/github/`](../../service/submitqueue/demo/provider/github). Edit the three marked lines in `merge.yaml`: - -```yaml -remoteUrl: https://github.com//.git -target: main -checkoutPath: /var/runway/checkouts/ -``` - -Neither file holds a secret — `tokenEnv: GITHUB_TOKEN` names the variable, and the value comes from your environment. - -## Run - -```bash -export GITHUB_TOKEN=ghp_... -make local-provider-start PROVIDER=github -``` - -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. - -`local-provider-start` prints the gateway's port. Export it so the commands below are shorter: - -```bash -export GATEWAY_ADDR=localhost: -``` - -## Land a single change - -Open a pull request against `main` in the scratch repo, then: - -```bash -make land PR=https://github.com///pull/1 -``` - -`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: - -```bash -make land-status SQID= GATEWAY_ADDR=$GATEWAY_ADDR -``` - -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`. - -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. - -## Land a stack - -Open a chain of pull requests where each targets the previous one's branch, then submit them in order: - -```bash -make land PRS="https://github.com///pull/1 \ - https://github.com///pull/2 \ - https://github.com///pull/3" -``` - -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. - -## Simulating traffic - -Opening pull requests by hand gets old fast. `demo-pr` creates them, enqueues them, and shows you where each one is: - -```bash -make demo-pr # 3 independent PRs, each enqueued as it is created -make demo-pr COUNT=8 # more traffic -make demo-pr FILES=8 # wider changes, more files per PR -make demo-pr CONCURRENCY=1 # create them one at a time -make demo-pr STACKED=true # one stack, enqueued as a single request -make demo-pr LAND=false # create only, print the land command -``` - -Each pull request is enqueued the moment it exists, so the queue is already working on the first while the last is still being opened. That overlap is the point: a queue holding one request at a time never batches, never analyzes a conflict against another batch, and never speculates. Nothing is awaited until every request is in. - -Independent pull requests are created **five at a time** by default (`CONCURRENCY`). Opening one is several round trips — a branch, a commit per file, the pull request itself — so creating them serially was most of what a large run spent its time on, and it delayed the overlap the demo exists to show. A stack ignores the setting: each of its changes is based on the branch before it, so the next cannot be cut until the previous head exists. Lower it if the provider starts refusing bursts. - -The table is there from the start — one row per land request, drawn before the first pull request exists and filled in as the run proceeds. Whatever is happening right now is a single line underneath it, so creating and enqueuing does not scroll the table away: - -``` - REQUEST CHANGES ELAPSED STAGE - ───────────── ─────── ─────── ──────────────────────────────────────────────── - demo-queue/12 #31 34s accepted → started → validating → validated → - batched → speculating → speculated → landing → - landed - demo-queue/13 #32 31s accepted → started → validating → validated - demo-queue/14 #33 28s accepted → started - - ▸ 1 of 3 settled -``` - -Each row shows the states its request passed through, not just the one it is in. That comes from the gateway's history API rather than from sampling the current status, so a transition between two polls is not missed. `CHANGES` links to the pull request: on a terminal `#31` is clickable, and in a redirected run it is written out as a full URL instead. `ELAPSED` runs from the moment the gateway accepted the request and stops when it settles, so a finished row keeps the time it took rather than counting on. - -The trail is as detailed as what the pipeline reports, which is the full walk: `accepted`, `started`, `validating`, `validated`, `batched`, `speculating`, `speculated`, `landing`, and then a terminal `landed`, `error` or `cancelled`. `building` and `built` are recorded alongside as events rather than statuses. A long pause on `speculating` is the batch waiting on its build, not a stuck request. - -`STACKED=true` is the exception to the overlap: one request carries the whole chain, so it can only go in once every pull request in it exists. That is the atomic-stack path — the whole set reaches `main` in a single push, and the table shows it as the single row it is. - -It talks to GitHub over the REST API with the same `GITHUB_TOKEN`, so it needs no clone and no git binary. Each run tags its branches with a timestamp so repeated runs do not collide, and every file a change writes is at a path no other change uses, so independent changes do not conflict by accident. - -A change touches several files rather than one, each committed separately, so it arrives as a multi-file, multi-commit pull request — closer to a real change, and enough to exercise replaying a range of commits. `FILES` sets the floor (default 3); the actual count varies a little above it, derived from the run tag so replaying a tag reproduces the same run. Paths are sharded into two levels of hex buckets under `demo/` (`demo/c2/91/--.txt`), which keeps the tree from degenerating into one enormous directory as runs accumulate. - -The command exits non-zero if any request settles anywhere other than `landed`, so it works in a script. Piped to a file it prints a fresh table whenever a request moves — and not when only the clock did — instead of redrawing in place. - -## Watching it work - -The queue itself is readable without creating any traffic: - -```bash -make land-list # a table of recent requests -make land-list SINCE=24h LIMIT=200 # a wider window -make land-watch # follow them until they settle -``` - -Both draw the same table `make demo-pr` does — the demo tool and the CLI share it — but against whatever the queue already holds, so watching a queue no longer means adding to it. `land-watch` fixes its set when it starts and exits non-zero if any request in that set finishes anywhere other than `landed`, which makes it usable from a script. A request accepted after the watch begins is not picked up: a watch that grew as the queue did would never finish. - -Under the hood these are `client list` and `client watch`, which take a queue and reach any gateway: - -```bash -bazel run //service/submitqueue/gateway/client:gateway -- \ - -addr sq.example.com:443 -tls list -queue my-queue -since 1h -``` - -`-addr` is passed to the dialler untouched, so `dns:///host:port` and `unix:///path.sock` work as well as a plain `host:port`. Transport security is a separate flag rather than part of the address, because gRPC keeps target resolution and credentials apart — there is no `grpcs://` to write. - -A listing of a busy queue is mostly `speculating` rows, since that is where a request spends most of its active life — waiting on the build its batch was admitted for. - -### Authentication - -The gateway admits every caller. It is a sandbox stack, and nothing in it checks a credential. - -The client can still present one, for a gateway reached through something that does — a proxy, a mesh sidecar, an ingress that terminates auth ahead of the service. It reads `SQ_TOKEN` by default and sends it as `Authorization: Bearer …`; `-token-env` names a different variable, and an unset one sends nothing rather than failing, which is how it stays usable against a stack that wants no credential. - -```bash -SQ_TOKEN=$(cat ~/.sq-token) bazel run //service/submitqueue/gateway/client:gateway -- \ - -addr sq.example.com:443 -tls list -queue my-queue -``` - -### Service logs - -```bash -docker compose -p submitqueue-provider logs -f runway-service -``` - -Runway logs each merge and each head-branch move: - -``` -moved change head branch to its landed commit {"change": "you/repo#1", "branch": "refs/heads/feature-a", ...} -``` - -The message queue logs a line per message published, fetched, leased and acked, which at debug level buries everything else a service says. It is levelled separately from the rest of the service, at info by default. To follow the queue itself — chasing a message that never arrived, or a partition that never got leased — turn it back up for the services you care about: - -```bash -QUEUE_LOG_LEVEL=debug make local-submitqueue-start -``` - -`QUEUE_LOG_LEVEL` takes any zap level name. It can only raise the queue's level above the one the service logger was built with, never lower it, so it cannot be used to make a quiet service verbose. - -## When it does not work - -**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. - -**The change lands but the pull request stays open.** Two causes, distinguishable in Runway's logs. - -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. - -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. - -**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. - -**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. - -## Using real CI - -The demo keeps the build runner fake so a land finishes in seconds. Switching to real GitHub Actions takes three things. - -**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: - -```yaml -on: - pull_request: - merge_group: - workflow_dispatch: - inputs: - sq_head_uris: - description: "JSON array of change URIs in the batch under test" - required: false - sq_base_uris: - description: "JSON array of in-flight change URIs this batch speculates on top of" - required: false - sq_queue: - description: "SubmitQueue queue name" - required: false - sq_metadata: - description: "Caller-supplied build metadata, as JSON" - required: false -``` - -**2. Point the runner at it.** Replace the `buildRunner` line for the queue in `profiles.yaml`: - -```yaml -buildRunner: - type: githubactions - owner: behinddwalls - repo: sq-demo - workflow: ci.yml # file name or numeric workflow id - ref: main # the branch the workflow definition is read from -``` - -**3. Grant Actions: Read and write** on the token (see the permissions table above). - -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. - -## Clean up - -```bash -make local-provider-stop -``` - -The scratch repository keeps whatever landed; reset it with `git push --force` from a known-good commit. - -## Another provider - -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). diff --git a/doc/howto/QUICKSTART.md b/doc/howto/QUICKSTART.md index 866de32c..d22099a1 100644 --- a/doc/howto/QUICKSTART.md +++ b/doc/howto/QUICKSTART.md @@ -1,16 +1,16 @@ -# Landing a change with no credentials +# Quickstart -The shortest path from a clone to watching the queue actually do its job: start the stack, submit a change, watch it reach `landed`. It needs Docker and nothing else — no repository, no account, no token — because every integration at the edges is faked. +Start the stack, put traffic through it, and watch changes land — beginning with nothing installed but Docker. -That is also its limit, and worth being clear about before you read anything into a green result. Faking the edges is what makes the run free; it also means `landed` here proves the queue's own logic ran, not that a commit reached a branch anywhere. +The stack always runs the same way. What changes is where the changes come from and what landing them does, chosen with `PROVIDER`: -| | Command | What is real | What you need | +| `PROVIDER` | A change is | Landing it | Needs | |---|---|---|---| -| **Tier 1, here** | `make local-submitqueue-start` | the pipeline: validation, batching, conflict analysis, speculation, the request log | Docker | -| Tier 2 | `make e2e-git-test` | the above, plus a real git merge into a bare repository | Docker | -| Tier 3 | [PROVIDER-E2E.md](PROVIDER-E2E.md) | the above, plus a provider: change metadata, its CI, changes marked merged | a repository and a token | +| **`fake`** (default) | a URI, and nothing else | reports success without touching a repository | nothing | +| **`git`** | a branch in a bare repository on disk | a real fetch, cherry-pick and push | nothing | +| **`github`** | a real pull request | a real push to a real repository | a repository and a token | -The tiers are the ones [PROVIDER-E2E.md](PROVIDER-E2E.md) names. This guide is the manual form of tier 1; `make e2e-test` is the same ground covered automatically, and is what CI runs. +They are a ladder, not alternatives: the same commands work on each rung, so you can start with the one that needs nothing and only pay for what you want to see next. Each is a directory of configuration under [`service/submitqueue/demo/provider/`](../../service/submitqueue/demo/provider) — the difference between rungs is two YAML files, not a code path. ## Start the stack @@ -20,126 +20,344 @@ make local-submitqueue-start This builds the Linux binaries, brings up Gateway, Orchestrator, Runway and two MySQL databases, and applies their schemas. The first run spends most of its time in the Bazel build; later ones start in seconds. -Compose publishes each service on a **random** host port so several stacks can run side by side, which means there is no fixed address to hard-code. The start-up output ends with the ports; `make local-submitqueue-ps` prints them again at any time: +Compose publishes each service on a **random** host port so several stacks can run side by side, which means there is no fixed address to hard-code. The start-up output ends with the ports, and `make local-submitqueue-ps` prints them again at any time: ``` -📡 Service Endpoints: - Gateway gRPC: localhost:55343 +✅ Stack is running against provider 'fake'. + +Gateway gRPC port: 58537 ``` Export it, because every command below needs it: ```bash -export GATEWAY_ADDR=localhost:55343 +export GATEWAY_ADDR=localhost:58537 ``` -Leaving it unset does not fall back to anything useful — the client's default is `localhost:8081`, which is the `go run` port, not the compose one. +Leaving it unset does not fall back to anything useful — the client's default is `localhost:8081`, the `go run` port rather than the compose one. -## Land a change +## Put traffic through it ```bash -make land QUEUE=test-queue \ - URI='git://git.example.com/demo/refs%2Fheads%2Ffeature-a/1111111111111111111111111111111111111111' +make demo-requests +``` + +`demo-requests` creates changes, enqueues each the moment it exists, and watches them all until they settle: + +``` +Creating 3 change(s) across 8 folder(s) via fake changes (no repository) — independent, 5 at a time, each enqueued as soon as it is created + + REQUEST CHANGES ELAPSED STAGE + ──────────── ────────────────── ─────── ───────────────────────────────────────────── + demo-queue/1 demo/0814-135021/1 13s accepted → started → validating → validated → + batching → batched → speculating → speculated → + landing → landed ``` +The overlap is the point. A queue holding one request at a time never batches, never analyzes a conflict, and never speculates — so nothing is awaited until every change is in, and the queue is already working on the first while the last is still being created. + +Each row shows the states its request passed through, not just the one it is in, read from the gateway's history API rather than sampled — a transition between two polls is not missed. `ELAPSED` runs from the moment the gateway accepted the request and stops when it settles, so a finished row keeps the time it took rather than counting on. The command exits non-zero if any request settles anywhere other than `landed`, so it works in a script. + +```bash +make demo-requests COUNT=8 # more traffic +make demo-requests FOLDERS=1 # every change in one folder: all of them conflict +make demo-requests FOLDERS=50 # a folder each: none of them conflict +make demo-requests FILES=8 # wider changes, more files each +make demo-requests CONCURRENCY=1 # create them one at a time +make demo-requests STACKED=true # one stack, enqueued as a single request +make demo-requests LAND=false # create only, print the command to enqueue them ``` -Change 1: git://git.example.com/demo/refs%2Fheads%2Ffeature-a/1111111111111111111111111111111111111111 -Landed request submitted. - sqid: test-queue/1 + +Independent changes are created **five at a time** by default (`CONCURRENCY`), because creating them serially is most of what a large run spends its time on and it delays the overlap the demo exists to show. A stack ignores the setting: each of its changes is based on the branch before it, so the next cannot be cut until the previous head exists. + +A change touches several files rather than one, each committed separately, so it arrives as a multi-file, multi-commit change — closer to a real one, and enough to exercise replaying a range of commits. `FILES` sets the floor (default 3); the actual count varies a little above it, derived from the run tag so replaying a tag reproduces the same run. + +Every change writes all of its files into one folder under `demo/`, and `FOLDERS` decides how many folders there are to land in — by default a number between five and ten, picked per run. That is what makes a run interesting rather than uniform, because `demo-queue` uses the `pathoverlap` analyzer keyed on the directory: two changes landing in the same folder are batched in order and the second speculates on the first, while changes in different folders go out beside each other. A run prints the number it picked, and repeating a run tag reproduces the same collisions. + +Set it deliberately when you want a run to show one thing. `FOLDERS=1` puts every change in the same place, so the queue serializes the lot and each change speculates on the one before it. A number well above `COUNT` keeps them all apart, so they go out together. + +You can watch the queue reach that conclusion: + +```bash +docker exec submitqueue-mysql-app-1 mysql -uroot -proot submitqueue \ + -N -e "select batch_id, dependents from batch_dependent where dependents != '[]'" ``` -The `sqid` is the receipt. Numbering is per queue and starts from a counter that lives in the application database, which the stack does not carry across a restart (see Clean up), so on a freshly started stack the first request really is `test-queue/1` and the commands below can be copied as they are. +Every dependency listed is between changes sharing a folder; a batch that shares its folder with nothing in flight has none. -### Why the URI looks like that +Landing one change by hand instead: -A change URI is `git://{remote}/{repo}/{ref}/{commit_sha}`, and the fake change provider echoes back whatever it is handed — so the remote, repository and branch above need not exist. Two parts are still checked before anything is echoed, and both reject a request outright: +```bash +make land QUEUE=demo-queue \ + URI='git://demo.example.com/demo/refs%2Fheads%2Fmy-change/1111111111111111111111111111111111111111' +make land-status QUEUE=demo-queue SQID=demo-queue/1 +``` -- the **commit SHA must be 40 lowercase hex characters**, so a placeholder like `abc123` fails; -- the **ref must be fully qualified and percent-encoded** — `refs%2Fheads%2Ffeature-a`, not `feature-a`. Encoding is what keeps a branch name containing slashes inside a single path segment. +A change URI is `git://{remote}/{repo}/{ref}/{commit_sha}`. Two parts are checked before anything else happens: the **commit SHA must be 40 lowercase hex characters**, and the **ref must be fully qualified and percent-encoded** — `refs%2Fheads%2Fmy-change`, not `my-change`. Encoding is what keeps a branch name containing slashes inside a single path segment. -Quote the URI in your shell. It contains a `%`, and in the failure case below a `?` as well. +## Watching the queue -## Follow it +The queue is readable without creating any traffic: ```bash -make land-status QUEUE=test-queue SQID=test-queue/1 +make land-list # a table of recent requests +make land-list SINCE=24h LIMIT=200 # a wider window +make land-watch # follow them until they settle ``` +Both draw the same table `make demo-requests` does — the demo tool and the CLI share it — but against whatever the queue already holds, so watching a queue no longer means adding to it. They do not carry the same information, though: `list` is a one-shot read of the queue's receipts and does not fetch histories, so its `STAGE` column is always `…`, while `watch` follows the history API and fills the trail in as each request moves. + +`land-watch` fixes its set when it starts and exits non-zero if any request in that set finishes anywhere other than `landed`, which makes it usable from a script. A request accepted after the watch begins is not picked up: a watch that grew as the queue did would never finish. + +A listing of a busy queue is mostly `speculating` rows, since that is where a request spends most of its active life — waiting on the build its batch was admitted for. + +Under the hood these are `client list` and `client watch`, which take a queue and reach any gateway: + +```bash +bazel run //service/submitqueue/gateway/client:gateway -- \ + -addr sq.example.com:443 -tls list -queue my-queue -since 1h ``` -sqid: test-queue/1 -queue: test-queue -status: landed + +`-addr` is passed to the dialler untouched, so `dns:///host:port` and `unix:///path.sock` work as well as a plain `host:port`. Transport security is a separate flag rather than part of the address, because gRPC keeps target resolution and credentials apart — there is no `grpcs://` to write. + +### Authentication + +The gateway admits every caller. It is a sandbox stack, and nothing in it checks a credential. + +The client can still present one, for a gateway reached through something that does — a proxy, a mesh sidecar, an ingress that terminates auth ahead of the service. It reads `SQ_TOKEN` by default and sends it as `Authorization: Bearer …`; `-token-env` names a different variable, and an unset one sends nothing rather than failing, which is how it stays usable against a stack that wants no credential. + +```bash +SQ_TOKEN=$(cat ~/.sq-token) bazel run //service/submitqueue/gateway/client:gateway -- \ + -addr sq.example.com:443 -tls list -queue my-queue ``` -A request settles in a few seconds here, since the fake build runner passes instantly. Run the command while it is still moving and you will catch it mid-pipeline — `speculating` is the usual one to see. The full trail a successful request records is: +### Service logs +```bash +docker compose -p submitqueue-provider logs -f runway-service ``` -accepted → started → validating → validated → batched → speculating → speculated → landing → landed + +The message queue logs a line per message published, fetched, leased and acked, which at debug level buries everything else a service says. It is levelled separately from the rest of the service, at info by default. To follow the queue itself — chasing a message that never arrived, or a partition that never got leased — turn it back up: + +```bash +QUEUE_LOG_LEVEL=debug make local-submitqueue-start ``` -with `building` and `built` recorded alongside as events rather than statuses. +`QUEUE_LOG_LEVEL` takes any zap level name. It can only raise the queue's level above the one the service logger was built with, never lower it, so it cannot be used to make a quiet service verbose. + +## What this proves, and what it does not + +The queue's own logic is real in every mode: validation, batching, conflict analysis, speculation, the request log, and the whole trail from `accepted` to `landed`. + +In `fake` mode, everything at the edges is not. There is no repository, so a change is a URI that points at nothing; the build runner passes instantly; and Runway uses the **noop merger**, which reports success without touching anything. `landed` here means the pipeline ran to completion — not that a commit exists anywhere. -To watch a whole queue rather than one request: +For that, take the next rung. + +## Land into a real repository ```bash -make land-list QUEUE=test-queue # a table of recent requests -make land-watch QUEUE=test-queue # follow them until they settle +make local-submitqueue-stop +PROVIDER=git make local-submitqueue-start ``` -Both draw the table `make demo-pr` uses, but they do not carry the same information. `list` is a one-shot read of the queue's receipts and does not fetch histories, so its `STAGE` column is always `…`; `watch` follows the history API and fills the trail in as each request moves. Use `land-list` to see what is in the queue and `land-watch` to see where it is going. +Still no credential. `PROVIDER=git` provisions a bare repository at `/tmp/sq-sandbox/sandbox.git`, points Runway at it by path, and prints it at startup: -## Make one fail +``` +✅ Stack is running against provider 'git'. -The fakes take instructions through the change URI itself, so a failure needs no configuration change and no restart. Append `?sq-fake=build-fail`: +Gateway gRPC port: 55295 +Merge target: /tmp/sq-sandbox/sandbox.git +``` + +Then the same command as before, with the same `PROVIDER`: + +```bash +export GATEWAY_ADDR=localhost:55295 +PROVIDER=git make demo-requests +``` + +Now `demo-requests` pushes real branches with real commits, and landing them is a real cherry-pick and push. Look at the repository itself: + +```bash +git -C /tmp/sq-sandbox/sandbox.git log --oneline main +``` + +``` +b517508 squash: demo-queue/5 (sandbox@refs/heads/demo/0814-134848/2) +25c86c5 squash: demo-queue/4 (sandbox@refs/heads/demo/0814-134848/1) +9f72dcf squash: demo-queue/3 (sandbox@refs/heads/demo/0814-134848/3) +b5d86d6 seed the sandbox +``` + +The commits are there, and they are not the ones that were pushed: `SQUASH_REBASE` replays each change onto the target rather than merging it, which is why the queue can keep the trunk linear. + +**`PROVIDER` has to match on both commands.** It selects what the stack merges with *and* what `demo-requests` creates; pointing fake changes at a stack wired to git means asking the merger to fetch a ref that was never pushed. + +One property worth seeing, because it is the thing a submit queue exists for. A stack lands as a single push, so no reader ever observes it half-applied: ```bash -make land QUEUE=test-queue \ - URI='git://git.example.com/demo/refs%2Fheads%2Ffeature-b/2222222222222222222222222222222222222222?sq-fake=build-fail' -make land-status QUEUE=test-queue SQID=test-queue/2 +git -C /tmp/sq-sandbox/sandbox.git reflog show refs/heads/main | wc -l +PROVIDER=git make demo-requests COUNT=3 STACKED=true +git -C /tmp/sq-sandbox/sandbox.git reflog show refs/heads/main | wc -l +``` + +Three changes, one more ref update than before. + +The sandbox survives `make local-submitqueue-stop`, so what landed is still there to look at. `make local-submitqueue-clean` removes it. + +## Land against GitHub + +The last rung is the only one that needs credentials, and the only one where a change is a pull request that a person can review. + +### What you need + +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. + +A **token** for it, scoped to that one repository. + +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: + +| Permission | Access | Needed by | +|---|---|---| +| Metadata | Read | mandatory on every fine-grained token; GitHub adds it for you | +| Contents | Read and write | the git merger — clone, fetch, push to the target branch, and force-move each landed change's head branch | +| Pull requests | Read | the change provider reads pull request metadata, and `land -pr` reads the head commit | +| Pull requests | Read **and write** | only for `make demo-requests`, which opens pull requests | +| Actions | Read and write | only if you switch the build runner to GitHub Actions — dispatch a run, poll it, cancel it | + +A **classic** PAT needs `repo`, plus `workflow` if you use the GitHub Actions build runner. + +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. + +### Configure and run + +Everything provider-specific is one directory: [`service/submitqueue/demo/provider/github/`](../../service/submitqueue/demo/provider/github). Edit the three marked lines in `merge.yaml`: + +```yaml +remoteUrl: https://github.com//.git +target: main +checkoutPath: /var/runway/checkouts/ ``` +Neither file holds a secret — `tokenEnv: GITHUB_TOKEN` names the variable, and the value comes from your environment. + +```bash +export GITHUB_TOKEN=ghp_... +PROVIDER=github make local-submitqueue-start ``` -status: error + +The token is required rather than defaulted: a stack that silently falls back to the fake integrations reports changes as landed without having gone near the provider, which is a much worse way to find out. + +From here everything is as before — `PROVIDER=github make demo-requests` opens real pull requests, enqueues them and watches them land. + +### Land a pull request + +Open a pull request against `main` in the scratch repo, then: + +```bash +make land PR=https://github.com///pull/1 ``` -The request walks the same path as far as `speculating`, records `building`, and then goes terminal at `error` instead of landing. Other tokens follow the same `sq-fake=` convention and are documented on the fake they drive — `provider-error` on the change provider, `unmergeable` and `mergecheck-error` on the merge checker, `trigger-error` and `build-error` on the build runner. +`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` to follow with `make land-status`. When the request reaches `landed`, the pull request shows **Merged** and its commit is on `main`. + +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. -Submit a second change immediately behind the failing one and you can watch the property that makes a queue worth having: +A stack is a chain of pull requests where each targets the previous one's branch, submitted in order: ```bash -make land QUEUE=test-queue \ - URI='git://git.example.com/demo/refs%2Fheads%2Fbad/4444444444444444444444444444444444444444?sq-fake=build-fail' -make land QUEUE=test-queue \ - URI='git://git.example.com/demo/refs%2Fheads%2Fgood/5555555555555555555555555555555555555555' -make land-watch QUEUE=test-queue +make land PRS="https://github.com///pull/1 \ + https://github.com///pull/2 \ + https://github.com///pull/3" ``` -Both are in flight together, and `test-queue` serializes them — its conflict analyzer is the conservative `all`, so the second is batched behind the first and speculates on it succeeding. When the first fails that guess is contradicted, and the second re-plans and lands anyway. One bad change goes to `error`; the good one behind it still reaches `landed` without a human touching it. +The order of `PRS` is the stack order. All three land as one push to `main`, and all three show as merged. + +### Using real CI + +The demo keeps the build runner fake so a land finishes in seconds. Switching to real GitHub Actions takes three things. + +**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: + +```yaml +on: + pull_request: + merge_group: + workflow_dispatch: + inputs: + sq_head_uris: + description: "JSON array of change URIs in the batch under test" + required: false + sq_base_uris: + description: "JSON array of in-flight change URIs this batch speculates on top of" + required: false + sq_queue: + description: "SubmitQueue queue name" + required: false + sq_metadata: + description: "Caller-supplied build metadata, as JSON" + required: false +``` + +**2. Point the runner at it.** Replace the `buildRunner` line for the queue in `profiles.yaml`: + +```yaml +buildRunner: + type: githubactions + owner: behinddwalls + repo: sq-demo + workflow: ci.yml # file name or numeric workflow id + ref: main # the branch the workflow definition is read from +``` + +**3. Grant Actions: Read and write** on the token (see the permissions table above). + +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. -## What this does not test +## Make a change fail -Everything that touches the outside world, which is exactly what the fakes replaced. No commit reaches a branch — Runway falls back to the **noop merger** whenever no `MERGE_CONFIG_PATH` is configured, and the local stack configures none. No CI runs. No change is marked merged anywhere. +The fakes take instructions through the change URI itself, so a failure needs no configuration change and no restart. Append `?sq-fake=build-fail`: + +```bash +make land QUEUE=demo-queue \ + URI='git://demo.example.com/demo/refs%2Fheads%2Fbad/2222222222222222222222222222222222222222?sq-fake=build-fail' +``` + +That request walks the same path as far as `speculating`, records `building`, and then goes terminal at `error` instead of landing. Other tokens follow the same `sq-fake=` convention and are documented on the fake they drive — `provider-error` on the change provider, `unmergeable` and `mergecheck-error` on the merge checker, `trigger-error` and `build-error` on the build runner. -Add the merge back with `make e2e-git-test`, which points Runway at a bare repository on a shared volume and asserts against the repository itself — still with no credential. Add the provider on top of that by following [PROVIDER-E2E.md](PROVIDER-E2E.md), which is the first tier that needs a token. +Submit a good change into the **same folder** as a failing one and you can watch what makes a queue worth having: the two are batched in order, and the second speculates on the first landing. When the first fails, that guess is contradicted, the second re-plans, and it lands anyway. ## Clean up ```bash -make local-stop # stop the services -make local-submitqueue-clean # also remove volumes and images +make local-submitqueue-stop # stop the services; PROVIDER=git's repository stays +make local-submitqueue-clean # also delete the sandbox repository ``` -`make local-stop` reports that data volumes are preserved, which is true of the volumes themselves but not of the data you can reach: both MySQL services mount **anonymous** volumes, so stopping detaches them and the next start creates a new, empty pair. Expect a restarted stack to have forgotten every request — and expect each cycle to leave a few hundred megabytes of orphaned volume behind, which is what eventually fills Docker up. `make local-submitqueue-clean` removes them as it goes. +A GitHub scratch repository keeps whatever landed; reset it with `git push --force` from a known-good commit. + +Both MySQL services mount **anonymous** volumes, so a stop/start cycle orphans a pair and comes back to an empty database. Expect a restarted stack to have forgotten every request, and expect each cycle to leave a few hundred megabytes of orphaned volume behind — which is what eventually fills Docker up. ## Troubleshooting -**MySQL exits immediately, and the stack fails with `dependency failed to start`.** Check `docker logs submitqueue-mysql-queue-1`. Two causes look similar: +**MySQL exits immediately, and the stack fails with `dependency failed to start`.** Check `docker logs submitqueue-provider-mysql-queue-1`. Two causes look similar: -- `No space left on device` — Docker is full. Every stop/start cycle orphans the two anonymous MySQL volumes (see Clean up), and integration and E2E runs add their own at a few hundred megabytes apiece; `docker system df` shows the total. `docker volume prune` reclaims every detached volume on the machine, so check `docker volume ls -f dangling=true` first if anything else of yours might be in there. -- `--initialize specified but the data directory has files in it` — a previous run died partway through initializing, leaving a half-written data directory that the next start cannot use. Remove that stack's volumes with `docker compose -f service/submitqueue/docker-compose.yml -p submitqueue down -v` and start again. +- `No space left on device` — Docker is full, usually of the orphaned volumes above; `docker system df` shows the total. `docker volume prune` reclaims every detached volume on the machine, so check `docker volume ls -f dangling=true` first if anything else of yours might be in there. +- `--initialize specified but the data directory has files in it` — a previous run died partway through initializing. Remove that stack's volumes with `docker compose -f service/submitqueue/docker-compose.yml -p submitqueue-provider down -v` and start again. -**A land is rejected before it returns an sqid.** The URI failed validation. Re-read the two rules above: 40 hex characters of SHA, and a percent-encoded `refs/…` ref. +**A land is rejected before it returns an sqid.** The URI failed validation: 40 hex characters of SHA, and a percent-encoded `refs/…` ref. + +**Everything reports `error` immediately.** Check the queue name exists in [`queues.yaml`](../../service/submitqueue/gateway/server/queues.yaml) and is configured in the provider directory you are running. A queue with no entry in `merge.yaml` gets the noop merger by design, so it will appear to land without pushing anything. + +**`PROVIDER=git` lands report `error`.** Read Runway's log and look for the git command that failed. A change whose branch was never pushed to the sandbox, or a `demo-requests` run whose `PROVIDER` did not match the stack's, both surface here as a failed fetch or cherry-pick. + +**`PROVIDER=github`: 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. + +**`PROVIDER=github`: the change lands but the pull request stays open.** Two causes, distinguishable in Runway's logs. 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, and the log says `no head branch on this remote for change`. Otherwise it is **protection on the head branch** blocking the force update, logged as `could not move change head branch`. The land itself succeeded either way — the failure is reported and deliberately not retried, because the push already happened and cannot be undone. + +**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. **`grpcurl` reports `target server does not expose service`.** The server registers reflection, but its descriptor references `api/base/change/proto/change.proto` while the generated code registers that file as `change.proto`, so reflection cannot resolve the gateway's descriptor. Use the client CLI, which is what every command here does. -**Everything reports `error` immediately.** Check the queue exists in [`queues.yaml`](../../service/submitqueue/gateway/server/queues.yaml). `test-queue` is there by default. +## Adding another provider + +Nothing above is GitHub-specific except the contents of that configuration directory and the URI parser behind it. Adding GitLab or another provider is a new directory 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). diff --git a/platform/fakemarker/fakemarker.go b/platform/fakemarker/fakemarker.go index 741c5976..c86f9a9c 100644 --- a/platform/fakemarker/fakemarker.go +++ b/platform/fakemarker/fakemarker.go @@ -21,6 +21,7 @@ package fakemarker import ( + "net/url" "strings" "github.com/uber/submitqueue/platform/base/change" @@ -56,3 +57,42 @@ func TokenInChanges(changes []change.Change) string { } return "" } + +// FilesPrefix introduces the paths a change touches: "sq-files=a.txt,b/c.txt". +// +// A real provider is asked what a change changed. A fake one has no repository +// to ask, so the caller states it — which is what lets a conflict analyzer that +// keys on paths do its actual job against changes that were never pushed +// anywhere. +const FilesPrefix = "sq-files=" + +// Files returns the paths listed by the first URI carrying a file marker, or nil +// if none do. Paths are comma-separated and percent-decoded, and the list ends +// at the first "&" or "#" so it can sit among other query parameters. +func Files(uris []string) []string { + for _, u := range uris { + i := strings.Index(u, FilesPrefix) + if i < 0 { + continue + } + rest := u[i+len(FilesPrefix):] + if j := strings.IndexAny(rest, "&#"); j >= 0 { + rest = rest[:j] + } + + var paths []string + for _, raw := range strings.Split(rest, ",") { + decoded, err := url.QueryUnescape(raw) + if err != nil { + // A path that will not decode is not worth failing a demo over; + // the marker is a convenience, not a contract. + decoded = raw + } + if trimmed := strings.TrimSpace(decoded); trimmed != "" { + paths = append(paths, trimmed) + } + } + return paths + } + return nil +} diff --git a/platform/fakemarker/fakemarker_test.go b/platform/fakemarker/fakemarker_test.go index 90e35478..1fd4bc46 100644 --- a/platform/fakemarker/fakemarker_test.go +++ b/platform/fakemarker/fakemarker_test.go @@ -21,6 +21,70 @@ import ( "github.com/uber/submitqueue/platform/base/change" ) +func TestFiles(t *testing.T) { + tests := []struct { + name string + uris []string + want []string + }{ + { + name: "no uris", + uris: nil, + want: nil, + }, + { + name: "no marker", + uris: []string{"git://git.example.com/r/refs%2Fheads%2Fa/abc"}, + want: nil, + }, + { + name: "one path", + uris: []string{"git://git.example.com/r/x/y?sq-files=demo/alpha/one.txt"}, + want: []string{"demo/alpha/one.txt"}, + }, + { + name: "several paths", + uris: []string{"git://git.example.com/r/x/y?sq-files=demo/alpha/one.txt,demo/beta/two.txt"}, + want: []string{"demo/alpha/one.txt", "demo/beta/two.txt"}, + }, + { + name: "percent-encoded path", + uris: []string{"git://git.example.com/r/x/y?sq-files=demo%2Falpha%2Fone.txt"}, + want: []string{"demo/alpha/one.txt"}, + }, + { + name: "trimmed at the next parameter", + uris: []string{"git://git.example.com/r/x/y?sq-files=demo/alpha/one.txt&sq-fake=build-fail"}, + want: []string{"demo/alpha/one.txt"}, + }, + { + // The two markers are independent, and one change may carry both. + name: "found after another parameter", + uris: []string{"git://git.example.com/r/x/y?sq-fake=build-fail&sq-files=demo/alpha/one.txt"}, + want: []string{"demo/alpha/one.txt"}, + }, + { + name: "empty entries are dropped", + uris: []string{"git://git.example.com/r/x/y?sq-files=demo/alpha/one.txt,,"}, + want: []string{"demo/alpha/one.txt"}, + }, + { + name: "marker on a later uri", + uris: []string{ + "git://git.example.com/r/x/y", + "git://git.example.com/r/x/z?sq-files=demo/gamma/three.txt", + }, + want: []string{"demo/gamma/three.txt"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, Files(tt.uris)) + }) + } +} + func TestToken(t *testing.T) { tests := []struct { name string diff --git a/platform/gitexec/BUILD.bazel b/platform/gitexec/BUILD.bazel new file mode 100644 index 00000000..0f88bbca --- /dev/null +++ b/platform/gitexec/BUILD.bazel @@ -0,0 +1,8 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["gitexec.go"], + importpath = "github.com/uber/submitqueue/platform/gitexec", + visibility = ["//visibility:public"], +) diff --git a/platform/gitexec/gitexec.go b/platform/gitexec/gitexec.go new file mode 100644 index 00000000..519e78cd --- /dev/null +++ b/platform/gitexec/gitexec.go @@ -0,0 +1,111 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package gitexec locates a git binary and runs it with the ambient +// environment stripped out. +// +// Demo and development tooling drives git on a developer's own machine, where +// hooks, a signing key, or a commit template configured globally would each +// break a run in a way that has nothing to do with SubmitQueue. Every command +// built here therefore carries the same scrubbed environment the git merger +// uses (see runway/extension/merger/git), so tooling behaves the same on every +// machine. +// +// This resolves only the executable, because tooling runs porcelain +// (init, clone, commit, push) rather than constructing a merger's GitRuntime, +// which additionally pins the exec path and template directory. +package gitexec + +import ( + "bytes" + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" +) + +// Resolve returns an absolute path to a git binary. +// +// Preference order is the supplied path, then GIT_EXECUTABLE, then PATH — the +// same convention the Runway server's runtime resolution follows, so a +// deployment or a Bazel target can pin git without the caller knowing which +// did. +func Resolve(path string) (string, error) { + candidate := strings.TrimSpace(path) + source := "-git" + if candidate == "" { + candidate, source = strings.TrimSpace(os.Getenv("GIT_EXECUTABLE")), "GIT_EXECUTABLE" + } + if candidate == "" { + found, err := exec.LookPath("git") + if err != nil { + return "", fmt.Errorf("no git on PATH, and neither -git nor GIT_EXECUTABLE is set: %w", err) + } + candidate, source = found, "git resolved from PATH" + } + + absolute, err := filepath.Abs(candidate) + if err != nil { + return "", fmt.Errorf("%s: could not resolve %q to an absolute path: %w", source, candidate, err) + } + if info, err := os.Stat(absolute); err != nil || info.IsDir() { + return "", fmt.Errorf("%s: %q is not an executable file", source, absolute) + } + return absolute, nil +} + +// Command builds a git invocation in dir with the ambient environment removed. +// An empty dir runs in the current working directory. +func Command(ctx context.Context, git, dir string, args ...string) *exec.Cmd { + cmd := exec.CommandContext(ctx, git, args...) + cmd.Dir = dir + // A developer's global config is the usual reason a scripted git run fails + // on one machine and not another: a hooks path, a signing requirement, or a + // commit template. None of it is relevant to seeding a sandbox. + cmd.Env = []string{ + "GIT_CONFIG_NOSYSTEM=1", + "GIT_CONFIG_GLOBAL=" + os.DevNull, + "GIT_ATTR_NOSYSTEM=1", + "GIT_TERMINAL_PROMPT=0", + "GIT_PAGER=cat", + "GIT_EDITOR=:", + "PATH=" + os.Getenv("PATH"), + } + return cmd +} + +// Run executes a git command and discards its output, reporting stderr in the +// error so a failure says what git said rather than only that it exited. +func Run(ctx context.Context, git, dir string, args ...string) error { + _, err := Output(ctx, git, dir, args...) + return err +} + +// Output executes a git command and returns its trimmed stdout. +func Output(ctx context.Context, git, dir string, args ...string) (string, error) { + cmd := Command(ctx, git, dir, args...) + var stderr bytes.Buffer + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + message := strings.TrimSpace(stderr.String()) + if message == "" { + message = err.Error() + } + return "", fmt.Errorf("git %s: %s", strings.Join(args, " "), message) + } + return strings.TrimSpace(string(out)), nil +} diff --git a/service/runway/server/Dockerfile b/service/runway/server/Dockerfile index c823de6e..a4dd19fb 100644 --- a/service/runway/server/Dockerfile +++ b/service/runway/server/Dockerfile @@ -4,7 +4,15 @@ FROM debian:bookworm-slim # to fetch, apply, and push. Without it the service still starts, but only the # noop merger works — a git merge target fails at startup instead. RUN apt-get update && apt-get install -y ca-certificates git && rm -rf /var/lib/apt/lists/* \ - && mkdir -p /app && chmod 0755 /app + && mkdir -p /app && chmod 0755 /app \ + # The working trees the git merger owns. Created here so that a named volume + # mounted over it starts with a mode the service can write: Docker seeds an + # empty volume from the image, and the container's user is deployment- + # configurable (SQ_CONTAINER_USER), so a root-owned directory would leave a + # non-root service unable to provision its checkout. Group- and + # world-writable for that reason, which costs nothing on a path that exists + # to be mounted over. + && mkdir -p /var/runway/checkouts && chmod 0777 /var/runway/checkouts WORKDIR /app # Built via: make build-runway-linux diff --git a/service/submitqueue/demo/pr/BUILD.bazel b/service/submitqueue/demo/pr/BUILD.bazel deleted file mode 100644 index 0917fa7a..00000000 --- a/service/submitqueue/demo/pr/BUILD.bazel +++ /dev/null @@ -1,33 +0,0 @@ -load("@rules_go//go:def.bzl", "go_binary", "go_library", "go_test") - -go_library( - name = "go_default_library", - srcs = [ - "github.go", - "main.go", - ], - importpath = "github.com/uber/submitqueue/service/submitqueue/demo/pr", - visibility = ["//visibility:private"], - deps = [ - "//api/base/mergestrategy/protopb:go_default_library", - "//platform/base/change/github:go_default_library", - "//submitqueue/client:go_default_library", - "@org_golang_x_sync//errgroup:go_default_library", - ], -) - -go_binary( - name = "pr", - embed = [":go_default_library"], - visibility = ["//visibility:public"], -) - -go_test( - name = "go_default_test", - srcs = ["main_test.go"], - embed = [":go_default_library"], - deps = [ - "@com_github_stretchr_testify//assert:go_default_library", - "@com_github_stretchr_testify//require:go_default_library", - ], -) diff --git a/service/submitqueue/demo/pr/main.go b/service/submitqueue/demo/pr/main.go deleted file mode 100644 index e898e3f9..00000000 --- a/service/submitqueue/demo/pr/main.go +++ /dev/null @@ -1,492 +0,0 @@ -// Copyright (c) 2025 Uber Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Command pr populates a scratch repository with pull requests, enqueues them, -// and watches them move through the pipeline — so the demo stack can be -// exercised repeatedly without opening pull requests by hand. -// -// Nothing is awaited until the end, which is the point. Each pull request is -// enqueued the moment it is created, so the queue is already working on the -// first while the last is still being opened. A queue that only ever holds one -// request in flight never batches, never analyzes a conflict against another -// batch, and never speculates; those behaviors only appear when requests -// overlap. The table watches all of them at once. -// -// Independent pull requests are opened several at a time (-concurrency), since -// each is several round trips to the provider and nothing about them depends on -// the others. A stack cannot be: every change in it is based on the branch -// before it, so the next cannot be cut until the previous head exists. -// -// Two shapes of change, because the pipeline treats them differently: -// -// - independent (default): each pull request targets the base branch and is -// enqueued as its own request, immediately after it is created. This is -// what puts requests in flight against each other. -// - stacked (-stacked): each pull request is based on the one before it, and -// all of them go in as a single request once the chain exists — the -// atomic-stack path, where the whole set reaches the target in one push. -// -// The table is drawn before the first pull request exists and refreshed for the -// whole run, so there is never a stretch with nothing to look at. Each row is -// one land request and shows the states it has passed through, read from the -// gateway's history API rather than sampled — polling only the current status -// would miss any transition that happens between two ticks, which for a fast -// queue is most of them. -// -// Everything goes through GitHub's REST API rather than a local clone, so the -// tool needs no checkout and no git binary — only GITHUB_TOKEN, the same -// credential the stack itself uses. -package main - -import ( - "context" - "crypto/sha256" - "flag" - "fmt" - "os" - "strings" - "time" - - mergestrategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb" - githubchange "github.com/uber/submitqueue/platform/base/change/github" - "github.com/uber/submitqueue/submitqueue/client" - "golang.org/x/sync/errgroup" -) - -func main() { - cfg := parseFlags() - if err := run(context.Background(), cfg); err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(1) - } -} - -// config is everything the run needs, resolved from flags and the environment. -type config struct { - repo string - base string - count int - files int - concurrency int - stacked bool - prefix string - land bool - watch bool - addr string - tls bool - tokenEnv string - queue string - strategy string - token string - apiRoot string - host string -} - -func parseFlags() config { - var c config - flag.StringVar(&c.repo, "repo", "behinddwalls/sq-demo", "scratch repository as owner/name") - flag.StringVar(&c.base, "base", "main", "branch the changes target") - flag.IntVar(&c.count, "count", 3, "how many pull requests to create") - flag.IntVar(&c.files, "files", 3, "fewest files each pull request touches; the actual count varies a little above it") - flag.IntVar(&c.concurrency, "concurrency", 5, - "how many pull requests to create at once; a stack ignores it, being sequential by nature") - flag.BoolVar(&c.stacked, "stacked", false, "chain the pull requests and enqueue them as one stack") - flag.StringVar(&c.prefix, "prefix", "demo", "branch name prefix") - flag.BoolVar(&c.land, "land", true, "enqueue each pull request as it is created") - flag.BoolVar(&c.watch, "watch", true, "watch the requests until they all settle") - flag.StringVar(&c.addr, "addr", "localhost:8081", "gateway address") - flag.BoolVar(&c.tls, "tls", false, "dial the gateway with transport security") - flag.StringVar(&c.tokenEnv, "token-env", client.DefaultTokenEnv, "environment variable holding the gateway bearer token") - flag.StringVar(&c.queue, "queue", "demo-queue", "queue to land on") - flag.StringVar(&c.strategy, "strategy", "SQUASH_REBASE", "merge strategy") - flag.Parse() - - c.token = os.Getenv("GITHUB_TOKEN") - c.apiRoot = "https://api.github.com" - c.host = "github.com" - return c -} - -func run(ctx context.Context, cfg config) error { - if err := cfg.validate(); err != nil { - return err - } - owner, repo, _ := strings.Cut(cfg.repo, "/") - if owner == "" || repo == "" { - return fmt.Errorf("-repo %q must be owner/name", cfg.repo) - } - strategy, err := client.ParseStrategy(cfg.strategy) - if err != nil { - return err - } - - gh := &githubClient{root: cfg.apiRoot, token: cfg.token, owner: owner, repo: repo} - baseSHA, err := gh.branchSHA(ctx, cfg.base) - if err != nil { - return fmt.Errorf("read %s: %w", cfg.base, err) - } - - var sq *client.Client - if cfg.land { - sq, err = client.New(client.Options{Addr: cfg.addr, TLS: cfg.tls, TokenEnv: cfg.tokenEnv}) - if err != nil { - return err - } - defer sq.Close() - } - - // A run tag keeps repeated invocations from colliding on branch names, and - // makes it obvious in the repository which changes came from one run. - tag := time.Now().Format("0102-150405") - fmt.Printf("Creating %d pull request(s) in %s — %s\n\n", cfg.count, cfg.repo, shape(cfg)) - - // Every row is known before anything is created: one per pull request, or a - // single one for a stack, since the whole chain lands as one request. The - // table is therefore complete from the first draw and only ever fills in. - t := client.NewTracker(client.NewRows(rowCount(cfg))) - t.Note("starting") - - // Statuses are read on their own clock, concurrently with creation. A run - // that only started polling once every pull request existed would show an - // empty trail for the whole creation phase — which for a large -count is - // most of the run, and is exactly the stretch worth watching, since the - // early requests are already moving through the queue by then. - if cfg.land { - polling, stop := context.WithCancel(ctx) - defer stop() - go t.Poll(polling, sq.Gateway(), cfg.queue) - } - - created, err := createAndEnqueue(ctx, gh, sq, cfg, strategy, tag, baseSHA, t) - if err != nil { - return err - } - t.Seal() - - if !cfg.land { - t.Note("created %d pull request(s), not enqueued", len(created)) - fmt.Printf("\nEnqueue them with:\n make land PRS=\"%s\"\n", strings.Join(urlsOf(created), " ")) - return nil - } - if !cfg.watch { - t.Note("enqueued, not watching") - return nil - } - - select { - case <-ctx.Done(): - return ctx.Err() - case <-t.Settled(): - } - return t.Conclude() -} - -// rowCount is how many rows the table needs: one per pull request, or a single -// one for a stack, which lands as one request however many changes it carries. -func rowCount(cfg config) int { - if cfg.stacked { - return 1 - } - return cfg.count -} - -func shape(cfg config) string { - if cfg.stacked { - return "stacked, enqueued as one request once the chain exists" - } - if cfg.concurrency > 1 { - return fmt.Sprintf("independent, %d at a time, each enqueued as soon as it is created", cfg.concurrency) - } - return "independent, each enqueued as soon as it is created" -} - -// validate rejects a configuration the run cannot proceed with. -func (c config) validate() error { - if c.token == "" { - return fmt.Errorf("GITHUB_TOKEN is not set; it is the same credential the stack uses") - } - if c.count < 1 { - return fmt.Errorf("-count must be at least 1") - } - if c.concurrency < 1 { - return fmt.Errorf("-concurrency must be at least 1") - } - if c.files < 1 { - return fmt.Errorf("-files must be at least 1") - } - if _, _, ok := strings.Cut(c.repo, "/"); !ok { - return fmt.Errorf("-repo %q must be owner/name", c.repo) - } - return nil -} - -// change is one pull request this run created. -type change struct { - number int - url string - branch string - // headSHA is the commit the pull request now points at, which the next - // change in a stack branches from. - headSHA string - // uri is the SubmitQueue change URI pinning the pull request to its head. - uri string -} - -func urlsOf(cs []change) []string { - out := make([]string, 0, len(cs)) - for _, c := range cs { - out = append(out, c.url) - } - return out -} - -// shardDirs is how many nested bucket directories a path carries under the demo -// root. Two levels of 256 buckets spread a run's files widely enough that no -// directory becomes a dumping ground, while staying shallow enough to read in a -// diff. -const shardDirs = 2 - -// changeFilePath returns the repository path for one file of one change. -// -// The leaf name carries the run tag, the change index and the file index, which -// is what makes it unique: no two files in a run, and no two runs against the -// same repository, can ever name the same path. That uniqueness is load-bearing -// — see createAndEnqueue. -// -// The directories are the leading bytes of the leaf's SHA-256, so files land in -// buckets that are uniform without any coordination and stable across runs. Two -// unrelated changes sharing a bucket is expected and harmless: the bucket is -// only a directory, and it is the leaf that has to be distinct. -func changeFilePath(tag string, change, file int) string { - leaf := fmt.Sprintf("%s-%d-%d.txt", tag, change, file) - sum := sha256.Sum256([]byte(leaf)) - - parts := make([]string, 0, shardDirs+2) - parts = append(parts, "demo") - for i := 0; i < shardDirs; i++ { - parts = append(parts, fmt.Sprintf("%02x", sum[i])) - } - parts = append(parts, leaf) - return strings.Join(parts, "/") -} - -// changeFileCount returns how many files a change touches: at least min, varied -// a little so a run does not produce a row of identically shaped pull requests. -// -// The variation is derived from the run tag and the change index rather than -// from a clock or a global source of randomness, so replaying a tag reproduces -// the same run. A demo that cannot be reproduced is hard to talk about when -// something in it goes wrong. -func changeFileCount(tag string, change, min int) int { - if min < 1 { - min = 1 - } - sum := sha256.Sum256([]byte(fmt.Sprintf("%s#%d", tag, change))) - return min + int(sum[0]%4) -} - -// createAndEnqueue opens the pull requests and puts them on the queue, filling -// in the tracker's rows as it goes and reporting each step beneath the table. -// -// For independent changes the two steps interleave: each pull request is -// enqueued the moment it exists, so the queue is already working on it while -// the next is being opened. Stacked changes cannot interleave — one request -// carries the whole chain, so it can only be submitted once the chain is -// complete. -// -// Every file a change writes is its own, at a path no other change uses. -// Independent changes would otherwise collide on content and the run would -// measure conflict handling rather than the throughput it is trying to show; a -// caller wanting a conflict can make one deliberately. Each change spreads -// several files across the sharded tree, so it arrives as a multi-file, multi- -// commit pull request rather than a single-line edit — which is both closer to -// a real change and enough to exercise replaying a range of commits. -func createAndEnqueue( - ctx context.Context, - gh *githubClient, - sq *client.Client, - cfg config, - strategy mergestrategypb.Strategy, - tag, baseSHA string, - t *client.Tracker, -) ([]change, error) { - if cfg.stacked { - return createStack(ctx, gh, sq, cfg, strategy, tag, baseSHA, t) - } - return createIndependent(ctx, gh, sq, cfg, strategy, tag, baseSHA, t) -} - -// createIndependent opens the pull requests concurrently, up to the configured -// limit, enqueuing each the moment it exists. -// -// Independent changes have nothing to say to each other: each branches from the -// same base and writes files no other change touches, so the only reason to -// create them one at a time was that the loop did. Creating a pull request is -// several round trips to the provider — a branch, a commit per file, the pull -// request itself — and doing that serially is most of what a large run spends -// its time on. It also delays the overlap the demo exists to show, since the -// queue cannot work on requests that have not been submitted yet. -// -// The limit is there because the provider is a shared service with its own -// opinion about burst rates, and because the point is to feed the queue, not to -// find out how fast a repository can be hammered. -func createIndependent( - ctx context.Context, - gh *githubClient, - sq *client.Client, - cfg config, - strategy mergestrategypb.Strategy, - tag, baseSHA string, - t *client.Tracker, -) ([]change, error) { - rows := t.Rows() - // Indexed rather than appended: the workers finish in whatever order the - // provider answers them, and the caller still wants the run's own order. - created := make([]change, cfg.count) - - group, groupCtx := errgroup.WithContext(ctx) - group.SetLimit(cfg.concurrency) - - for i := 1; i <= cfg.count; i++ { - group.Go(func() error { - c, err := createOne(groupCtx, gh, cfg, tag, baseSHA, cfg.base, i, t, rows[i-1]) - if err != nil { - return err - } - created[i-1] = c - - if !cfg.land { - return nil - } - t.Note("enqueuing #%d", c.number) - sqid, err := sq.Land(groupCtx, cfg.queue, urisOf([]change{c}), strategy) - if err != nil { - return err - } - t.Update(func() { rows[i-1].SQID, rows[i-1].Submitted = sqid, time.Now() }) - return nil - }) - } - - if err := group.Wait(); err != nil { - return nil, err - } - return created, nil -} - -// createStack opens the pull requests one after another, each based on the one -// before it, and submits the whole chain as a single request. -// -// This one cannot be parallelized, and not for want of trying: a change is -// based on the branch of the change before it and must see its content, so the -// next branch cannot be cut until the previous head exists. -func createStack( - ctx context.Context, - gh *githubClient, - sq *client.Client, - cfg config, - strategy mergestrategypb.Strategy, - tag, baseSHA string, - t *client.Tracker, -) ([]change, error) { - rows := t.Rows() - created := make([]change, 0, cfg.count) - - parentBranch, parentSHA := cfg.base, baseSHA - for i := 1; i <= cfg.count; i++ { - // A stack is one request, so every change lands on the single row. - c, err := createOne(ctx, gh, cfg, tag, parentSHA, parentBranch, i, t, rows[0]) - if err != nil { - return nil, err - } - created = append(created, c) - parentBranch, parentSHA = c.branch, c.headSHA - } - - // The stack goes in as one request, which is only possible now that every - // change in it exists. - if cfg.land { - t.Note("enqueuing the stack") - sqid, err := sq.Land(ctx, cfg.queue, urisOf(created), strategy) - if err != nil { - return nil, err - } - t.Update(func() { rows[0].SQID, rows[0].Submitted = sqid, time.Now() }) - } - return created, nil -} - -// createOne cuts a branch from parentSHA, writes the change's files to it, and -// opens a pull request against parentBranch, recording it on the given row. -func createOne( - ctx context.Context, - gh *githubClient, - cfg config, - tag, parentSHA, parentBranch string, - i int, - t *client.Tracker, - target *client.Row, -) (change, error) { - branch := fmt.Sprintf("%s/%s/%d", cfg.prefix, tag, i) - t.Note("creating branch %s", branch) - if err := gh.createBranch(ctx, branch, parentSHA); err != nil { - return change{}, fmt.Errorf("create branch %s: %w", branch, err) - } - - // Each file is its own commit, so the pull request arrives as a range of - // commits rather than a single edit. The last one is the head the change - // URI pins. - var headSHA string - fileCount := changeFileCount(tag, i, cfg.files) - for k := 1; k <= fileCount; k++ { - path := changeFilePath(tag, i, k) - body := fmt.Sprintf("change %d of run %s\nfile %d of %d\n", i, tag, k, fileCount) - t.Note("committing %s (%d/%d)", path, k, fileCount) - - message := fmt.Sprintf("demo change %d (run %s): file %d of %d", i, tag, k, fileCount) - sha, err := gh.commitFile(ctx, branch, path, body, message) - if err != nil { - return change{}, fmt.Errorf("commit %s to %s: %w", path, branch, err) - } - headSHA = sha - } - - t.Note("opening pull request for %s", branch) - number, url, err := gh.openPR(ctx, fmt.Sprintf("demo change %d (run %s)", i, tag), branch, parentBranch) - if err != nil { - return change{}, fmt.Errorf("open pull request for %s: %w", branch, err) - } - - c := change{ - number: number, url: url, branch: branch, headSHA: headSHA, - uri: githubchange.ChangeID{ - Scheme: "github", Host: cfg.host, Org: gh.owner, Repo: gh.repo, - PRNumber: number, HeadCommitSHA: headSHA, - }.String(), - } - // The cell is what the table shows for this change: the pull request - // number, clickable where the terminal allows it. - cell := client.Cell{Text: fmt.Sprintf("#%d", number), URL: url} - t.Update(func() { target.Cells = append(target.Cells, cell) }) - return c, nil -} - -// urisOf is the change URIs the run pinned, in caller order. -func urisOf(cs []change) []string { - out := make([]string, 0, len(cs)) - for _, c := range cs { - out = append(out, c.uri) - } - return out -} diff --git a/service/submitqueue/demo/provider/README.md b/service/submitqueue/demo/provider/README.md index ecb1f525..cf8a95fc 100644 --- a/service/submitqueue/demo/provider/README.md +++ b/service/submitqueue/demo/provider/README.md @@ -9,14 +9,15 @@ Each directory here is one **provider** — a code-hosting system SubmitQueue la 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. -Pick one with `make local-provider-start PROVIDER=`, 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. +Pick one with `make local-submitqueue-start PROVIDER=`, 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. -| Directory | What it demonstrates | -|---|---| -| [`github/`](github) | a live provider: GitHub change metadata, a real repository, pull requests marked merged | -| [`local/`](local) | a plain git remote with no provider at all — used by the hermetic git E2E (`make e2e-git-test`) | +| Directory | What it demonstrates | Needs | +|---|---|---| +| [`fake/`](fake) | the queue alone: a change is a URI, nothing merges anywhere — the default, and what the quickstart runs | nothing | +| [`git/`](git) | a plain git remote with no provider at all: real fetch, cherry-pick and push against a bare repository | nothing | +| [`github/`](github) | a live provider: GitHub change metadata, a real repository, pull requests marked merged | a repository and a token | -`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. +The three are a ladder, and the rung is the only thing that changes: the same commands land against all of them. `git/` is worth reading first of the two real ones. 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 — and it is what the hermetic git E2E (`make e2e-git-test`) runs against. ## Adding a provider @@ -30,8 +31,11 @@ Everything provider-specific is reached through an existing seam, so a new provi | `submitqueue/extension/buildrunner/{provider}/` | only if the provider's CI is not already covered by the Buildkite or GitHub Actions runners | | `service/submitqueue/gateway/client/main.go` | one case in `resolvePullRequest`, so `land -pr ` accepts the provider's change URLs | | this directory | a `{provider}/` with the two files above | +| `Makefile` | one line in the `PROVIDER_COMPOSE_FILE_{provider}` map, naming which compose overlay the mode needs | + +The Makefile line is there because a mode's *mounts* are not something the two config files can express: `github` needs a credential in the environment, `git` needs the sandbox and checkout directories bind-mounted, and `fake` needs neither. A provider reaching a remote API over a token is the common case and can reuse `docker-compose.provider.yml` verbatim, so for most new providers that line names an existing file rather than a new one. -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. +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, and the hermetic git E2E are all provider-independent and need no change. Two of those deserve explanation. @@ -39,4 +43,4 @@ Two of those deserve explanation. **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. -See [doc/howto/PROVIDER-E2E.md](../../../../doc/howto/PROVIDER-E2E.md) for running a real land end to end. +See [doc/howto/QUICKSTART.md](../../../../doc/howto/QUICKSTART.md) for running each of these by hand, from the credential-free modes to a real land against GitHub. diff --git a/service/submitqueue/demo/provider/fake/merge.yaml b/service/submitqueue/demo/provider/fake/merge.yaml new file mode 100644 index 00000000..19f3eb35 --- /dev/null +++ b/service/submitqueue/demo/provider/fake/merge.yaml @@ -0,0 +1,13 @@ +# Merge targets for the "fake" example: there are none. +# +# Nothing here merges. The noop merger reports success without touching a +# repository, which is what lets a request reach `landed` with no repository to +# land into. +# +# That is the mode's limit, and worth knowing before reading anything into a +# green run: `landed` here means the pipeline ran to completion, not that a +# commit exists anywhere. Switch to PROVIDER=git for a real merge into a local +# repository, still without a credential — see ../git/merge.yaml. + +defaults: + merger: {type: noop} diff --git a/service/submitqueue/demo/provider/fake/profiles.yaml b/service/submitqueue/demo/provider/fake/profiles.yaml new file mode 100644 index 00000000..a0e51383 --- /dev/null +++ b/service/submitqueue/demo/provider/fake/profiles.yaml @@ -0,0 +1,35 @@ +# Extension profiles for the "fake" example: nothing outside the queue is real. +# +# The default provider, and the one the quickstart runs. There is no repository, +# no CI, and no credential anywhere — a change is a URI and nothing more. What +# it exercises is the queue itself: validation, batching, conflict analysis, +# speculation, and the request log. +# +# Everything here is what the orchestrator would fall back to on its own with no +# configuration at all. It is written out because a mode you can read in a file +# is easier to trust than one implied by an absent one, and because it makes the +# difference from ../git and ../github a diff rather than an explanation. + +defaults: + # No provider to ask, so the fake echoes back each URI it is given. + changeProvider: {type: fake} + # Every build succeeds immediately, so a land completes in seconds. + buildRunner: {type: fake} + # Serialize conservatively unless a queue says otherwise. + analyzer: {type: all} + +queues: + # The queue `make demo-requests` and `make land` use by default. + # + # Serializes batches that touch a shared directory, so changes land in + # parallel unless they land in the same folder — the same setting the git and + # github modes use, so what a run shows does not depend on which one it ran + # against. + # + # This keys on the files a change reports, and no provider can be asked about + # a change that exists nowhere. `make demo-requests` therefore states the + # paths on the change URI (`sq-files=`) and the fake change provider reports + # them back. A change submitted by hand carries no such marker, touches + # nothing as far as the analyzer can tell, and so never conflicts. + - name: demo-queue + analyzer: {type: pathoverlap, by: directory} diff --git a/service/submitqueue/demo/provider/local/BUILD.bazel b/service/submitqueue/demo/provider/git/BUILD.bazel similarity index 100% rename from service/submitqueue/demo/provider/local/BUILD.bazel rename to service/submitqueue/demo/provider/git/BUILD.bazel diff --git a/service/submitqueue/demo/provider/git/merge.yaml b/service/submitqueue/demo/provider/git/merge.yaml new file mode 100644 index 00000000..fcba9da9 --- /dev/null +++ b/service/submitqueue/demo/provider/git/merge.yaml @@ -0,0 +1,51 @@ +# Merge targets for the "git" example: a plain git remote with no provider. +# +# The target is a bare repository on a shared volume, addressed by path. That +# exercises the whole merge machinery (real fetch, cherry-pick, push, +# head-branch update) with no credential, no network, and no provider account, +# which is what lets the hermetic git E2E (`make e2e-git-test`) gate PRs in CI. +# +# It doubles as the worked example of a non-GitHub target: nothing below names a +# provider, because the merger does not have one. See ../README.md. + +defaults: + # Any queue without an entry below does not merge for real. + merger: {type: noop} + +queues: + # The queue `make demo-requests` and `make land` use by default. Squash-rebases to + # match the GitHub demo, so switching providers changes where changes come + # from and not what landing does to them. + - name: demo-queue + merger: + type: git + # A local path needs no credential, so no tokenEnv is named. + remoteUrl: file:///srv/git/sandbox.git + remote: origin + target: main + # Deliberately not e2e-git-queue's working tree. Queues sharing a checkout + # share a single merger instance, so two differing configurations pointing + # at one checkout is refused at startup. + checkoutPath: /var/runway/checkouts/demo + defaultStrategy: SQUASH_REBASE + checkStaleness: true + # Rewriting strategies leave the change's original head unreachable from + # the target, so its branch is moved to the commit it landed as. On a + # provider this is what makes the change show as merged; here it is simply + # observable as the branch having moved. + updateHeadBranch: true + + # Driven by the E2E, which asserts against the repository itself — what + # reached the target branch, in what order, and in how many ref updates. These + # settings are what those assertions expect. + - name: e2e-git-queue + merger: + type: git + remoteUrl: file:///srv/git/sandbox.git + remote: origin + target: main + # Provisioned at startup: cloned, remote configured, target checked out. + checkoutPath: /var/runway/checkouts/sandbox + defaultStrategy: REBASE + checkStaleness: true + updateHeadBranch: true diff --git a/service/submitqueue/demo/provider/local/profiles.yaml b/service/submitqueue/demo/provider/git/profiles.yaml similarity index 52% rename from service/submitqueue/demo/provider/local/profiles.yaml rename to service/submitqueue/demo/provider/git/profiles.yaml index 05d784c4..1b4bf384 100644 --- a/service/submitqueue/demo/provider/local/profiles.yaml +++ b/service/submitqueue/demo/provider/git/profiles.yaml @@ -1,4 +1,4 @@ -# Extension profiles for the "local" example: a plain git remote with no provider. +# Extension profiles for the "git" example: a plain git remote with no provider. # # There is no provider to ask about a change, and no CI to run, so both edge # integrations stay fake. What this example exercises is the merge itself — see @@ -14,6 +14,17 @@ defaults: analyzer: {type: all} queues: + # The queue `make demo-requests` and `make land` use by default. Serializes + # batches that touch a shared directory, matching the github mode. + # + # The commits here are real, but the change provider above is not, and it + # cannot read a repository to find out what they touched. `make demo-requests` + # states the paths it committed on the change URI (`sq-files=`) for the fake + # provider to report back. A change submitted by hand carries no such marker + # and so conflicts with nothing. + - name: demo-queue + analyzer: {type: pathoverlap, by: directory} + - name: e2e-git-queue # Maximum parallelism: batches never conflict, so the test controls # ordering through what it lands rather than through the analyzer. diff --git a/service/submitqueue/demo/provider/github/merge.yaml b/service/submitqueue/demo/provider/github/merge.yaml index a407971f..9cacb6ad 100644 --- a/service/submitqueue/demo/provider/github/merge.yaml +++ b/service/submitqueue/demo/provider/github/merge.yaml @@ -1,7 +1,7 @@ # Merge targets for the GitHub demo. # # Edit the three placeholders below to point at your own scratch repository, -# then: make local-provider-start PROVIDER=github +# then: make local-submitqueue-start PROVIDER=github # # This file holds no secret. `tokenEnv` names the environment variable carrying # the credential, so the file stays committable and rotating the token needs no diff --git a/service/submitqueue/demo/provider/local/merge.yaml b/service/submitqueue/demo/provider/local/merge.yaml deleted file mode 100644 index 66536f5e..00000000 --- a/service/submitqueue/demo/provider/local/merge.yaml +++ /dev/null @@ -1,32 +0,0 @@ -# Merge targets for the "local" example: a plain git remote with no provider. -# -# This is what the hermetic git E2E (`make e2e-git-test`) runs against — a bare -# repository on a shared volume, addressed by path. It exercises the whole merge -# machinery (real fetch, cherry-pick, push, head-branch update) with no -# credential, no network, and no provider account, which is what lets it gate PRs -# in CI. -# -# It doubles as the worked example of a non-GitHub target: nothing below names a -# provider, because the merger does not have one. See ../README.md. - -defaults: - # Any queue without an entry below does not merge for real. - merger: {type: noop} - -queues: - - name: e2e-git-queue - merger: - type: git - # A local path needs no credential, so no tokenEnv is named. - remoteUrl: file:///srv/git/sandbox.git - remote: origin - target: main - # Provisioned at startup: cloned, remote configured, target checked out. - checkoutPath: /var/runway/checkouts/sandbox - defaultStrategy: REBASE - checkStaleness: true - # Rewriting strategies leave the change's original head unreachable from - # the target, so its branch is moved to the commit it landed as. On a - # provider this is what makes the change show as merged; here it is simply - # observable as the branch having moved. - updateHeadBranch: true diff --git a/service/submitqueue/demo/requests/BUILD.bazel b/service/submitqueue/demo/requests/BUILD.bazel new file mode 100644 index 00000000..6189c153 --- /dev/null +++ b/service/submitqueue/demo/requests/BUILD.bazel @@ -0,0 +1,55 @@ +load("@rules_go//go:def.bzl", "go_binary", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = [ + "fake.go", + "git.go", + "github.go", + "main.go", + "source.go", + ], + importpath = "github.com/uber/submitqueue/service/submitqueue/demo/requests", + visibility = ["//visibility:private"], + deps = [ + "//api/base/mergestrategy/protopb:go_default_library", + "//platform/base/change/git:go_default_library", + "//platform/base/change/github:go_default_library", + "//platform/fakemarker:go_default_library", + "//platform/gitexec:go_default_library", + "//submitqueue/client:go_default_library", + "@org_golang_x_sync//errgroup:go_default_library", + ], +) + +go_binary( + name = "requests", + # -provider git authors changes with the same pinned git the merger uses. + # Unused by the other two providers, which touch no repository. + args = ["-git=$(location @git//:git)"], + data = ["@git"], + embed = [":go_default_library"], + visibility = ["//visibility:public"], +) + +go_test( + name = "go_default_test", + srcs = [ + "main_test.go", + "source_test.go", + ], + data = ["@git"], + embed = [":go_default_library"], + # The git source drives a real repository, with the same pinned git the + # merger and the sandbox provisioner use. + env = { + "SUBMITQUEUE_TEST_GIT": "$(location @git//:git)", + }, + deps = [ + "//platform/base/change/git:go_default_library", + "//platform/fakemarker:go_default_library", + "//platform/gitexec:go_default_library", + "@com_github_stretchr_testify//assert:go_default_library", + "@com_github_stretchr_testify//require:go_default_library", + ], +) diff --git a/service/submitqueue/demo/requests/fake.go b/service/submitqueue/demo/requests/fake.go new file mode 100644 index 00000000..ee4c6a00 --- /dev/null +++ b/service/submitqueue/demo/requests/fake.go @@ -0,0 +1,74 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "context" + "crypto/sha256" + "encoding/hex" + + gitchange "github.com/uber/submitqueue/platform/base/change/git" + "github.com/uber/submitqueue/submitqueue/client" +) + +// fakeRemote is the authority in the change URIs this source mints. Nothing +// resolves it — with no repository behind a change, the URI is an identifier +// and not an address. +const fakeRemote = "demo.example.com" + +// fakeRepo is the repository segment of those URIs. +const fakeRepo = "demo" + +// fakeSource invents changes. It performs no I/O at all: there is no repository +// to create a branch in, so a change is a URI and nothing more. +// +// This is what the default provider submits. It is the fastest way to watch the +// queue work, and the reason the quickstart needs neither a repository nor a +// credential — at the cost of the URIs pointing at nothing, which is only +// sound because the fake change provider echoes back whatever it is handed and +// the noop merger never tries to fetch it. +type fakeSource struct{} + +func (fakeSource) baseSHA(_ context.Context, branch string) (string, error) { + return syntheticSHA("base", branch), nil +} + +func (fakeSource) open(_ context.Context, spec changeSpec) (openedChange, error) { + // Derived from the branch, so a change is distinct from every other change + // and stable across runs of the same tag — a run can be replayed and the + // URIs it submits will match. + headSHA := syntheticSHA("head", spec.branch) + return openedChange{ + headSHA: headSHA, + // No files are written anywhere, but the change still says which paths + // it would have touched, so the conflict analyzer has something to key + // on. It is the only claim in this mode that is not backed by anything. + uri: withFiles(gitchange.ChangeID{ + Scheme: "git", Remote: fakeRemote, Repo: fakeRepo, + Ref: "refs/heads/" + spec.branch, CommitSHA: headSHA, + }.String(), spec.files), + // There is no pull request to number and nothing to link to, so the + // branch name is what identifies the change. An empty URL renders as + // plain text rather than as a link that goes nowhere. + cell: client.Cell{Text: spec.branch}, + }, nil +} + +// syntheticSHA is a stand-in for a commit SHA: 40 lowercase hex characters, +// which is what a change URI requires and what the gateway validates. +func syntheticSHA(kind, seed string) string { + sum := sha256.Sum256([]byte(kind + "\x00" + seed)) + return hex.EncodeToString(sum[:])[:40] +} diff --git a/service/submitqueue/demo/requests/git.go b/service/submitqueue/demo/requests/git.go new file mode 100644 index 00000000..d8f76546 --- /dev/null +++ b/service/submitqueue/demo/requests/git.go @@ -0,0 +1,150 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "context" + "fmt" + "os" + "path/filepath" + "sync" + + gitchange "github.com/uber/submitqueue/platform/base/change/git" + "github.com/uber/submitqueue/platform/gitexec" + "github.com/uber/submitqueue/submitqueue/client" +) + +// gitRemote is the authority in the change URIs this source mints. The merger +// reads the ref and commit out of a URI and reaches the repository through its +// own configured remote, so this identifies a change rather than routing to it +// — which is why a local sandbox can carry a hostname it does not answer to. +const gitRemote = "git.example.com" + +// gitSource opens changes as branches in a bare repository on disk. There is no +// provider and no pull request: a change is a branch, and landing it is a real +// fetch, cherry-pick and push. +type gitSource struct { + git string + repo string + // work is the clone changes are authored in, owned by this source and + // removed when the run ends. + work string + // mu serializes git commands. A single working tree cannot take concurrent + // checkouts and commits — the index is one file — so -concurrency stops at + // this boundary. That costs nothing: the flag exists because opening a pull + // request is several round trips to a provider, and there is no provider + // here. + mu sync.Mutex +} + +// newGitSource clones the sandbox repository into a scratch directory the +// caller must close. +func newGitSource(ctx context.Context, git, bare, repo string) (*gitSource, error) { + if _, err := os.Stat(bare); err != nil { + return nil, fmt.Errorf( + "no sandbox repository at %s; start the stack with `make local-submitqueue-start PROVIDER=git`: %w", bare, err) + } + + work, err := os.MkdirTemp("", "sq-demo-work-") + if err != nil { + return nil, fmt.Errorf("could not create a working clone: %w", err) + } + if err := gitexec.Run(ctx, git, "", "clone", bare, work); err != nil { + os.RemoveAll(work) + return nil, err + } + // An identity is needed to commit, and gitexec strips the ambient config so + // a developer's own settings cannot fail the run. + for _, kv := range [][2]string{ + {"user.name", "SubmitQueue Demo"}, + {"user.email", "demo@submitqueue.invalid"}, + } { + if err := gitexec.Run(ctx, git, work, "config", kv[0], kv[1]); err != nil { + os.RemoveAll(work) + return nil, err + } + } + return &gitSource{git: git, repo: repo, work: work}, nil +} + +func (s *gitSource) close() { + os.RemoveAll(s.work) +} + +func (s *gitSource) baseSHA(ctx context.Context, branch string) (string, error) { + s.mu.Lock() + defer s.mu.Unlock() + + if err := gitexec.Run(ctx, s.git, s.work, "fetch", "origin"); err != nil { + return "", err + } + return gitexec.Output(ctx, s.git, s.work, "rev-parse", "origin/"+branch) +} + +func (s *gitSource) open(ctx context.Context, spec changeSpec) (openedChange, error) { + s.mu.Lock() + defer s.mu.Unlock() + + spec.note("creating branch %s", spec.branch) + if err := gitexec.Run(ctx, s.git, s.work, "fetch", "origin"); err != nil { + return openedChange{}, err + } + // Cut from the parent commit rather than the parent branch: in a stack that + // commit was pushed a moment ago, and naming it exactly is what keeps the + // chain in the order the run intended. + if err := gitexec.Run(ctx, s.git, s.work, "checkout", "-B", spec.branch, spec.parentSHA); err != nil { + return openedChange{}, fmt.Errorf("branch %s from %s: %w", spec.branch, spec.parentSHA, err) + } + + for k, file := range spec.files { + spec.note("committing %s (%d/%d)", file.path, k+1, len(spec.files)) + full := filepath.Join(s.work, file.path) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + return openedChange{}, fmt.Errorf("create %s: %w", filepath.Dir(file.path), err) + } + if err := os.WriteFile(full, []byte(file.body), 0o644); err != nil { + return openedChange{}, fmt.Errorf("write %s: %w", file.path, err) + } + if err := gitexec.Run(ctx, s.git, s.work, "add", "--", file.path); err != nil { + return openedChange{}, err + } + if err := gitexec.Run(ctx, s.git, s.work, "commit", "-m", file.message); err != nil { + return openedChange{}, err + } + } + + spec.note("pushing %s", spec.branch) + if err := gitexec.Run(ctx, s.git, s.work, "push", "-f", "origin", spec.branch); err != nil { + return openedChange{}, fmt.Errorf("push %s: %w", spec.branch, err) + } + headSHA, err := gitexec.Output(ctx, s.git, s.work, "rev-parse", "HEAD") + if err != nil { + return openedChange{}, err + } + + return openedChange{ + headSHA: headSHA, + // The commits are real, but the fake change provider is what the + // orchestrator asks about them, and it cannot read a repository — so the + // paths just committed are stated on the URI for it to report back. + uri: withFiles(gitchange.ChangeID{ + Scheme: "git", Remote: gitRemote, Repo: s.repo, + Ref: "refs/heads/" + spec.branch, CommitSHA: headSHA, + }.String(), spec.files), + // No pull request to number, so the branch names the change. Empty URL: + // a branch in a bare repository has nothing to open. + cell: client.Cell{Text: spec.branch}, + }, nil +} diff --git a/service/submitqueue/demo/pr/github.go b/service/submitqueue/demo/requests/github.go similarity index 64% rename from service/submitqueue/demo/pr/github.go rename to service/submitqueue/demo/requests/github.go index 53e78039..47073cc4 100644 --- a/service/submitqueue/demo/pr/github.go +++ b/service/submitqueue/demo/requests/github.go @@ -22,8 +22,64 @@ import ( "fmt" "net/http" "strings" + + githubchange "github.com/uber/submitqueue/platform/base/change/github" + "github.com/uber/submitqueue/submitqueue/client" ) +// githubSource opens changes as real pull requests, entirely over GitHub's REST +// API — so it needs no clone and no git binary, only a token. +type githubSource struct { + client *githubClient + // host is the authority in the change URIs this source mints, which for + // GitHub Enterprise differs from the API root. + host string +} + +func newGitHubSource(cfg config, owner, repo string) *githubSource { + return &githubSource{ + client: &githubClient{root: cfg.apiRoot, token: cfg.token, owner: owner, repo: repo}, + host: cfg.host, + } +} + +func (s *githubSource) baseSHA(ctx context.Context, branch string) (string, error) { + return s.client.branchSHA(ctx, branch) +} + +func (s *githubSource) open(ctx context.Context, spec changeSpec) (openedChange, error) { + spec.note("creating branch %s", spec.branch) + if err := s.client.createBranch(ctx, spec.branch, spec.parentSHA); err != nil { + return openedChange{}, fmt.Errorf("create branch %s: %w", spec.branch, err) + } + + var headSHA string + for k, file := range spec.files { + spec.note("committing %s (%d/%d)", file.path, k+1, len(spec.files)) + sha, err := s.client.commitFile(ctx, spec.branch, file.path, file.body, file.message) + if err != nil { + return openedChange{}, fmt.Errorf("commit %s to %s: %w", file.path, spec.branch, err) + } + headSHA = sha + } + + spec.note("opening pull request for %s", spec.branch) + number, url, err := s.client.openPR(ctx, spec.title, spec.branch, spec.parentBranch) + if err != nil { + return openedChange{}, fmt.Errorf("open pull request for %s: %w", spec.branch, err) + } + + return openedChange{ + headSHA: headSHA, + uri: githubchange.ChangeID{ + Scheme: "github", Host: s.host, Org: s.client.owner, Repo: s.client.repo, + PRNumber: number, HeadCommitSHA: headSHA, + }.String(), + // The pull request number, clickable where the terminal allows it. + cell: client.Cell{Text: fmt.Sprintf("#%d", number), URL: url}, + }, nil +} + // githubClient is the slice of GitHub's REST API this tool needs: read a // branch, create a branch, commit a file, open a pull request. type githubClient struct { @@ -70,7 +126,7 @@ func (g *githubClient) commitFile(ctx context.Context, branch, path, content, me } func (g *githubClient) openPR(ctx context.Context, title, head, base string) (int, string, error) { - body := map[string]string{"title": title, "head": head, "base": base, "body": "Opened by service/submitqueue/demo/pr."} + body := map[string]string{"title": title, "head": head, "base": base, "body": "Opened by service/submitqueue/demo/requests."} var out struct { Number int `json:"number"` HTMLURL string `json:"html_url"` diff --git a/service/submitqueue/demo/requests/main.go b/service/submitqueue/demo/requests/main.go new file mode 100644 index 00000000..d371b7f9 --- /dev/null +++ b/service/submitqueue/demo/requests/main.go @@ -0,0 +1,614 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Command requests creates changes, enqueues each as a land request, and +// watches them move through the pipeline — so the demo stack can be exercised +// repeatedly without authoring changes by hand. +// +// Nothing is awaited until the end, which is the point. Each change is enqueued +// the moment it is created, so the queue is already working on the first while +// the last is still being made. A queue that only ever holds one request in +// flight never batches, never analyzes a conflict against another batch, and +// never speculates; those behaviors only appear when requests overlap. The +// table watches all of them at once. +// +// Independent changes are created several at a time (-concurrency), since +// nothing about them depends on the others — which matters most against a +// provider, where each is several round trips. A stack cannot be: every change +// in it is based on the branch before it, so the next cannot be cut until the +// previous head exists. +// +// Two shapes of change, because the pipeline treats them differently: +// +// - independent (default): each change targets the base branch and is +// enqueued as its own request, immediately after it is created. This is +// what puts requests in flight against each other. +// - stacked (-stacked): each change is based on the one before it, and all of +// them go in as a single request once the chain exists — the atomic-stack +// path, where the whole set reaches the target in one push. +// +// The table is drawn before the first change exists and refreshed for the whole +// run, so there is never a stretch with nothing to look at. Each row is one land +// request and shows the states it has passed through, read from the gateway's +// history API rather than sampled — polling only the current status would miss +// any transition that happens between two ticks, which for a fast queue is most +// of them. +// +// How a change is made depends on -provider, matching the stack the run is +// pointed at: +// +// - fake (default): a change is a URI and nothing else. No repository, no +// credential, no I/O — the fastest way to put traffic through the queue. +// - git: a branch pushed to the sandbox repository the stack merges into. +// Real commits, still no credential. +// - github: a real pull request over the REST API, which needs no clone and +// no git binary, only GITHUB_TOKEN — the same credential the stack uses. +package main + +import ( + "context" + "crypto/sha256" + "flag" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + mergestrategypb "github.com/uber/submitqueue/api/base/mergestrategy/protopb" + "github.com/uber/submitqueue/platform/gitexec" + "github.com/uber/submitqueue/submitqueue/client" + "golang.org/x/sync/errgroup" +) + +func main() { + cfg := parseFlags() + if err := run(context.Background(), cfg); err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(1) + } +} + +// config is everything the run needs, resolved from flags and the environment. +type config struct { + provider string + repo string + sandboxDir string + git string + base string + count int + folders int + files int + concurrency int + stacked bool + prefix string + land bool + watch bool + addr string + tls bool + tokenEnv string + queue string + strategy string + token string + apiRoot string + host string +} + +func parseFlags() config { + var c config + flag.StringVar(&c.provider, "provider", providerFake, "how changes are created: fake, git, or github") + flag.StringVar(&c.repo, "repo", "behinddwalls/sq-demo", "github: scratch repository as owner/name") + flag.StringVar(&c.sandboxDir, "sandbox-dir", "/tmp/sq-sandbox", "git: directory holding the sandbox repository") + flag.StringVar(&c.git, "git", "", "git: path to the git binary; defaults to GIT_EXECUTABLE, then PATH") + flag.StringVar(&c.base, "base", "main", "branch the changes target") + flag.IntVar(&c.count, "count", 3, "how many changes to create") + flag.IntVar(&c.folders, "folders", 0, + "how many folders to spread the changes across; 0 picks one per run. Changes sharing a folder are batched in order, changes in different folders go out together") + flag.IntVar(&c.files, "files", 3, + "fewest files each change touches; the actual count varies a little above it. Ignored by -provider fake, which writes none") + flag.IntVar(&c.concurrency, "concurrency", 5, + "how many changes to create at once; a stack ignores it, being sequential by nature, and -provider git serializes its git commands") + flag.BoolVar(&c.stacked, "stacked", false, "chain the changes and enqueue them as one stack") + flag.StringVar(&c.prefix, "prefix", "demo", "branch name prefix") + flag.BoolVar(&c.land, "land", true, "enqueue each change as it is created") + flag.BoolVar(&c.watch, "watch", true, "watch the requests until they all settle") + flag.StringVar(&c.addr, "addr", "localhost:8081", "gateway address") + flag.BoolVar(&c.tls, "tls", false, "dial the gateway with transport security") + flag.StringVar(&c.tokenEnv, "token-env", client.DefaultTokenEnv, "environment variable holding the gateway bearer token") + flag.StringVar(&c.queue, "queue", "demo-queue", "queue to land on") + flag.StringVar(&c.strategy, "strategy", "SQUASH_REBASE", "merge strategy") + flag.Parse() + + // Only the GitHub source reads a credential; the other two must not fail, + // or even appear to depend on one, when none is set. + if c.provider == providerGitHub { + c.token = os.Getenv("GITHUB_TOKEN") + c.apiRoot = "https://api.github.com" + c.host = "github.com" + } + return c +} + +func run(ctx context.Context, cfg config) error { + if err := cfg.validate(); err != nil { + return err + } + strategy, err := client.ParseStrategy(cfg.strategy) + if err != nil { + return err + } + + src, cleanup, err := newChangeSource(ctx, cfg) + if err != nil { + return err + } + defer cleanup() + + baseSHA, err := src.baseSHA(ctx, cfg.base) + if err != nil { + return fmt.Errorf("read %s: %w", cfg.base, err) + } + + var sq *client.Client + if cfg.land { + sq, err = client.New(client.Options{Addr: cfg.addr, TLS: cfg.tls, TokenEnv: cfg.tokenEnv}) + if err != nil { + return err + } + defer sq.Close() + } + + // A run tag keeps repeated invocations from colliding on branch names, and + // makes it obvious in the repository which changes came from one run. + tag := time.Now().Format("0102-150405") + // Resolved once, so every change in the run is dealt into the same tree and + // the number can be reported rather than inferred from the paths. + cfg.folders = resolveFolders(tag, cfg.folders) + fmt.Printf("Creating %d change(s) across %d folder(s) via %s — %s\n\n", + cfg.count, cfg.folders, target(cfg), shape(cfg)) + + // Every row is known before anything is created: one per change, or a + // single one for a stack, since the whole chain lands as one request. The + // table is therefore complete from the first draw and only ever fills in. + t := client.NewTracker(client.NewRows(rowCount(cfg))) + t.Note("starting") + + // Statuses are read on their own clock, concurrently with creation. A run + // that only started polling once every change existed would show an + // empty trail for the whole creation phase — which for a large -count is + // most of the run, and is exactly the stretch worth watching, since the + // early requests are already moving through the queue by then. + if cfg.land { + polling, stop := context.WithCancel(ctx) + defer stop() + go t.Poll(polling, sq.Gateway(), cfg.queue) + } + + created, err := createAndEnqueue(ctx, src, sq, cfg, strategy, tag, baseSHA, t) + if err != nil { + return err + } + t.Seal() + + if !cfg.land { + t.Note("created %d change(s), not enqueued", len(created)) + fmt.Printf("\nEnqueue them with:\n %s\n", enqueueHint(created)) + return nil + } + if !cfg.watch { + t.Note("enqueued, not watching") + return nil + } + + select { + case <-ctx.Done(): + return ctx.Err() + case <-t.Settled(): + } + return t.Conclude() +} + +// rowCount is how many rows the table needs: one per change, or a single +// one for a stack, which lands as one request however many changes it carries. +func rowCount(cfg config) int { + if cfg.stacked { + return 1 + } + return cfg.count +} + +func shape(cfg config) string { + if cfg.stacked { + return "stacked, enqueued as one request once the chain exists" + } + if cfg.concurrency > 1 { + return fmt.Sprintf("independent, %d at a time, each enqueued as soon as it is created", cfg.concurrency) + } + return "independent, each enqueued as soon as it is created" +} + +// validate rejects a configuration the run cannot proceed with. +func (c config) validate() error { + switch c.provider { + case providerFake, providerGit: + case providerGitHub: + if c.token == "" { + return fmt.Errorf("GITHUB_TOKEN is not set; it is the same credential the stack uses") + } + if _, _, ok := strings.Cut(c.repo, "/"); !ok { + return fmt.Errorf("-repo %q must be owner/name", c.repo) + } + default: + return fmt.Errorf("-provider %q must be one of %s, %s, or %s", + c.provider, providerFake, providerGit, providerGitHub) + } + if c.count < 1 { + return fmt.Errorf("-count must be at least 1") + } + if c.concurrency < 1 { + return fmt.Errorf("-concurrency must be at least 1") + } + if c.files < 1 { + return fmt.Errorf("-files must be at least 1") + } + if c.folders < 0 { + return fmt.Errorf("-folders cannot be negative; 0 picks one per run") + } + return nil +} + +// newChangeSource builds the source for the configured provider, and a cleanup +// to run when the run ends. +func newChangeSource(ctx context.Context, cfg config) (changeSource, func(), error) { + switch cfg.provider { + case providerFake: + return fakeSource{}, func() {}, nil + + case providerGit: + git, err := gitexec.Resolve(cfg.git) + if err != nil { + return nil, nil, err + } + src, err := newGitSource(ctx, git, filepath.Join(cfg.sandboxDir, sandboxRepo+".git"), sandboxRepo) + if err != nil { + return nil, nil, err + } + return src, src.close, nil + + case providerGitHub: + owner, repo, _ := strings.Cut(cfg.repo, "/") + return newGitHubSource(cfg, owner, repo), func() {}, nil + } + // validate has already rejected anything else. + return nil, nil, fmt.Errorf("unknown provider %q", cfg.provider) +} + +// sandboxRepo is the repository the git provider's sandbox holds, matching what +// tool/gitsandbox creates and what demo/provider/git/merge.yaml merges into. +const sandboxRepo = "sandbox" + +// target describes where the run is creating changes, for the opening line. +func target(cfg config) string { + switch cfg.provider { + case providerGit: + return fmt.Sprintf("git (%s)", filepath.Join(cfg.sandboxDir, sandboxRepo+".git")) + case providerGitHub: + return fmt.Sprintf("github (%s)", cfg.repo) + default: + return "fake changes (no repository)" + } +} + +// change is one change this run created. +type change struct { + // label identifies the change in the table: a pull request number where + // there is one, and the branch otherwise. + label string + // url is where the change can be opened, empty when it lives nowhere a + // browser can reach. + url string + branch string + // headSHA is the commit the change now points at, which the next change in + // a stack branches from. + headSHA string + // uri is the SubmitQueue change URI pinning the change to its head. + uri string +} + +// enqueueHint is the command that lands what a -land=false run created. +// +// Pull request URLs where there are any, since that is what a reader recognizes +// and can open; change URIs otherwise, which is the only handle a branch in a +// local repository has. +func enqueueHint(cs []change) string { + if len(cs) > 0 && cs[0].url != "" { + return fmt.Sprintf("make land PRS=%q", strings.Join(urlsOf(cs), " ")) + } + return fmt.Sprintf("make land URIS=%q", strings.Join(urisOf(cs), " ")) +} + +func urlsOf(cs []change) []string { + out := make([]string, 0, len(cs)) + for _, c := range cs { + out = append(out, c.url) + } + return out +} + +// How many directories a run spreads its changes across, as a range rather than +// a number: each run picks one, so repeated runs against the same queue do not +// all collide in the same shape. +// +// This range is the whole dial on what a run demonstrates. The default analyzer +// keys on the directory, so a large number means nothing ever collides and a +// run shows only the parallel case, while one means everything serializes and +// it shows only the other. Five to ten is wide enough to look like a real tree +// and small enough that a run of a handful of changes still puts two of them in +// the same place. +// +// It used to be two nested levels of 256, keyed on each file's own name, which +// put a twelve-file run's odds of any collision at roughly one in a thousand +// and scattered a single change across as many directories as it had files. +const ( + minShardDirs = 5 + maxShardDirs = 10 +) + +// resolveFolders is how many directories a run spreads its changes across: +// what -folders asked for, or a number picked from the range above when it +// asked for nothing. +// +// Worth setting deliberately when the run is meant to show one thing. -folders 1 +// puts every change in the same place, so the queue serializes the lot and each +// change speculates on the one before it; a number well above -count keeps them +// apart, so they all go out together. +func resolveFolders(tag string, configured int) int { + if configured > 0 { + return configured + } + sum := sha256.Sum256([]byte("folders#" + tag)) + return minShardDirs + int(sum[0])%(maxShardDirs-minShardDirs+1) +} + +// changeShard is the directory one change writes into, as a fixed-width number +// so a run's collisions are visible at a glance in a table or a diff. +// +// Keyed on the change rather than on each file, so a change occupies exactly +// one directory: a change spread over several would overlap with everything and +// report a conflict that says nothing about it. Derived from the run tag, so +// replaying a tag reproduces the same collisions. +func changeShard(tag string, folders, change int) string { + sum := sha256.Sum256([]byte(fmt.Sprintf("shard#%s#%d", tag, change))) + return fmt.Sprintf("%02d", int(sum[0])%folders) +} + +// changeFilePath returns the repository path for one file of one change. +// +// The leaf name carries the run tag, the change index and the file index, which +// is what makes it unique: no two files in a run, and no two runs against the +// same repository, can ever name the same path. That uniqueness is load-bearing +// — see createAndEnqueue — and it is the leaf that carries it, not the +// directory. Two changes deliberately share a directory (see shardDirs) so the +// conflict analyzer has something to find, and they still never write the same +// file, so a shared directory serializes them rather than making them collide. +func changeFilePath(tag string, folders, change, file int) string { + leaf := fmt.Sprintf("%s-%d-%d.txt", tag, change, file) + return strings.Join([]string{"demo", changeShard(tag, folders, change), leaf}, "/") +} + +// changeFileCount returns how many files a change touches: at least min, varied +// a little so a run does not produce a row of identically shaped changes. +// +// The variation is derived from the run tag and the change index rather than +// from a clock or a global source of randomness, so replaying a tag reproduces +// the same run. A demo that cannot be reproduced is hard to talk about when +// something in it goes wrong. +func changeFileCount(tag string, change, min int) int { + if min < 1 { + min = 1 + } + sum := sha256.Sum256([]byte(fmt.Sprintf("%s#%d", tag, change))) + return min + int(sum[0]%4) +} + +// createAndEnqueue creates the changes and puts them on the queue, filling +// in the tracker's rows as it goes and reporting each step beneath the table. +// +// For independent changes the two steps interleave: each change is +// enqueued the moment it exists, so the queue is already working on it while +// the next is being opened. Stacked changes cannot interleave — one request +// carries the whole chain, so it can only be submitted once the chain is +// complete. +// +// Every file a change writes is its own, at a path no other change uses. +// Independent changes would otherwise collide on content and the run would +// measure conflict handling rather than the throughput it is trying to show; a +// caller wanting a conflict can make one deliberately. Each change spreads +// several files across the sharded tree, so it arrives as a multi-file, multi- +// commit change rather than a single-line edit — which is both closer to +// a real change and enough to exercise replaying a range of commits. +func createAndEnqueue( + ctx context.Context, + src changeSource, + sq *client.Client, + cfg config, + strategy mergestrategypb.Strategy, + tag, baseSHA string, + t *client.Tracker, +) ([]change, error) { + if cfg.stacked { + return createStack(ctx, src, sq, cfg, strategy, tag, baseSHA, t) + } + return createIndependent(ctx, src, sq, cfg, strategy, tag, baseSHA, t) +} + +// createIndependent creates the changes concurrently, up to the configured +// limit, enqueuing each the moment it exists. +// +// Independent changes have nothing to say to each other: each branches from the +// same base and writes files no other change touches, so the only reason to +// create them one at a time was that the loop did. Creating a change is +// several round trips to the provider — a branch, a commit per file, the pull +// request itself — and doing that serially is most of what a large run spends +// its time on. It also delays the overlap the demo exists to show, since the +// queue cannot work on requests that have not been submitted yet. +// +// The limit is there because the provider is a shared service with its own +// opinion about burst rates, and because the point is to feed the queue, not to +// find out how fast a repository can be hammered. +func createIndependent( + ctx context.Context, + src changeSource, + sq *client.Client, + cfg config, + strategy mergestrategypb.Strategy, + tag, baseSHA string, + t *client.Tracker, +) ([]change, error) { + rows := t.Rows() + // Indexed rather than appended: the workers finish in whatever order the + // provider answers them, and the caller still wants the run's own order. + created := make([]change, cfg.count) + + group, groupCtx := errgroup.WithContext(ctx) + group.SetLimit(cfg.concurrency) + + for i := 1; i <= cfg.count; i++ { + group.Go(func() error { + c, err := createOne(groupCtx, src, cfg, tag, baseSHA, cfg.base, i, t, rows[i-1]) + if err != nil { + return err + } + created[i-1] = c + + if !cfg.land { + return nil + } + t.Note("enqueuing %s", c.label) + sqid, err := sq.Land(groupCtx, cfg.queue, urisOf([]change{c}), strategy) + if err != nil { + return err + } + t.Update(func() { rows[i-1].SQID, rows[i-1].Submitted = sqid, time.Now() }) + return nil + }) + } + + if err := group.Wait(); err != nil { + return nil, err + } + return created, nil +} + +// createStack creates the changes one after another, each based on the one +// before it, and submits the whole chain as a single request. +// +// This one cannot be parallelized, and not for want of trying: a change is +// based on the branch of the change before it and must see its content, so the +// next branch cannot be cut until the previous head exists. +func createStack( + ctx context.Context, + src changeSource, + sq *client.Client, + cfg config, + strategy mergestrategypb.Strategy, + tag, baseSHA string, + t *client.Tracker, +) ([]change, error) { + rows := t.Rows() + created := make([]change, 0, cfg.count) + + parentBranch, parentSHA := cfg.base, baseSHA + for i := 1; i <= cfg.count; i++ { + // A stack is one request, so every change lands on the single row. + c, err := createOne(ctx, src, cfg, tag, parentSHA, parentBranch, i, t, rows[0]) + if err != nil { + return nil, err + } + created = append(created, c) + parentBranch, parentSHA = c.branch, c.headSHA + } + + // The stack goes in as one request, which is only possible now that every + // change in it exists. + if cfg.land { + t.Note("enqueuing the stack") + sqid, err := sq.Land(ctx, cfg.queue, urisOf(created), strategy) + if err != nil { + return nil, err + } + t.Update(func() { rows[0].SQID, rows[0].Submitted = sqid, time.Now() }) + } + return created, nil +} + +// createOne describes one change — its branch, and the files it writes — hands +// it to the source to be made real, and records it on the given row. +// +// What a change is made of is decided here rather than by the source, so the +// three providers put the same shape of change through the queue and differ +// only in how it comes to exist. +func createOne( + ctx context.Context, + src changeSource, + cfg config, + tag, parentSHA, parentBranch string, + i int, + t *client.Tracker, + target *client.Row, +) (change, error) { + branch := fmt.Sprintf("%s/%s/%d", cfg.prefix, tag, i) + + // Each file is its own commit, so a change arrives as a range of commits + // rather than a single edit. + fileCount := changeFileCount(tag, i, cfg.files) + files := make([]changeFile, 0, fileCount) + for k := 1; k <= fileCount; k++ { + files = append(files, changeFile{ + path: changeFilePath(tag, cfg.folders, i, k), + body: fmt.Sprintf("change %d of run %s\nfile %d of %d\n", i, tag, k, fileCount), + message: fmt.Sprintf("demo change %d (run %s): file %d of %d", i, tag, k, fileCount), + }) + } + + opened, err := src.open(ctx, changeSpec{ + branch: branch, + parentBranch: parentBranch, + parentSHA: parentSHA, + title: fmt.Sprintf("demo change %d (run %s)", i, tag), + files: files, + note: t.Note, + }) + if err != nil { + return change{}, err + } + + t.Update(func() { target.Cells = append(target.Cells, opened.cell) }) + return change{ + label: opened.cell.Text, + url: opened.cell.URL, + branch: branch, + headSHA: opened.headSHA, + uri: opened.uri, + }, nil +} + +// urisOf is the change URIs the run pinned, in caller order. +func urisOf(cs []change) []string { + out := make([]string, 0, len(cs)) + for _, c := range cs { + out = append(out, c.uri) + } + return out +} diff --git a/service/submitqueue/demo/pr/main_test.go b/service/submitqueue/demo/requests/main_test.go similarity index 55% rename from service/submitqueue/demo/pr/main_test.go rename to service/submitqueue/demo/requests/main_test.go index cd13684d..48a85615 100644 --- a/service/submitqueue/demo/pr/main_test.go +++ b/service/submitqueue/demo/requests/main_test.go @@ -31,7 +31,7 @@ func TestChangeFilePath_IsUniquePerFileAcrossChangesAndRuns(t *testing.T) { for _, tag := range []string{"0810-1203", "0810-1204"} { for change := 1; change <= 20; change++ { for file := 1; file <= 8; file++ { - path := changeFilePath(tag, change, file) + path := changeFilePath(tag, resolveFolders(tag, 0), change, file) owner := fmt.Sprintf("%s/%d/%d", tag, change, file) if prev, ok := seen[path]; ok { t.Fatalf("path %s produced for both %s and %s", path, prev, owner) @@ -42,30 +42,67 @@ func TestChangeFilePath_IsUniquePerFileAcrossChangesAndRuns(t *testing.T) { } } -func TestChangeFilePath_ShardsUnderTheDemoRoot(t *testing.T) { - path := changeFilePath("0810-1203", 1, 1) +func TestChangeFilePath_PutsEveryFileOfAChangeInOneDirectory(t *testing.T) { + // The conflict analyzer keys on the directory, so a change spread across + // several would overlap with everything and report a conflict that says + // nothing about the change. + dirs := make(map[string]struct{}) + for file := 1; file <= 8; file++ { + parts := strings.Split(changeFilePath("0810-1203", 5, 1, file), "/") + require.Len(t, parts, 3, "demo root, one folder, and the leaf") + assert.Equal(t, "demo", parts[0]) + assert.Regexp(t, `^\d{2}$`, parts[1]) + dirs[strings.Join(parts[:2], "/")] = struct{}{} + } + assert.Len(t, dirs, 1, "one change writes into one directory") +} - parts := strings.Split(path, "/") - require.Len(t, parts, shardDirs+2, "demo root, %d bucket dirs, and the leaf", shardDirs) - assert.Equal(t, "demo", parts[0]) - for _, bucket := range parts[1 : len(parts)-1] { - assert.Len(t, bucket, 2, "each bucket is one hex byte") - assert.Regexp(t, "^[0-9a-f]{2}$", bucket) +// -folders is the dial on what a run demonstrates, so both ends of it have to +// do what they say. +func TestChangeFilePath_FollowsTheFolderCount(t *testing.T) { + folderOf := func(folders, change int) string { + parts := strings.Split(changeFilePath("0810-1203", folders, change, 1), "/") + return strings.Join(parts[:2], "/") } - assert.Equal(t, "0810-1203-1-1.txt", parts[len(parts)-1]) + + t.Run("one folder puts every change together", func(t *testing.T) { + dirs := make(map[string]struct{}) + for change := 1; change <= 10; change++ { + dirs[folderOf(1, change)] = struct{}{} + } + assert.Len(t, dirs, 1, "every change must conflict with every other") + }) + + t.Run("many folders keep changes apart", func(t *testing.T) { + dirs := make(map[string]struct{}) + for change := 1; change <= 3; change++ { + dirs[folderOf(64, change)] = struct{}{} + } + assert.Len(t, dirs, 3, "three changes in 64 folders should not collide") + }) } -func TestChangeFilePath_SpreadsAcrossManyBuckets(t *testing.T) { - // A layout that puts everything in one directory would satisfy the - // uniqueness test above while defeating the point of sharding. - buckets := make(map[string]struct{}) - for change := 1; change <= 20; change++ { - for file := 1; file <= 4; file++ { - parts := strings.Split(changeFilePath("0810-1203", change, file), "/") - buckets[strings.Join(parts[1:len(parts)-1], "/")] = struct{}{} +func TestResolveFolders(t *testing.T) { + t.Run("honors an explicit count", func(t *testing.T) { + assert.Equal(t, 1, resolveFolders("0810-1203", 1)) + assert.Equal(t, 42, resolveFolders("0810-1203", 42)) + }) + + t.Run("picks within the range when unset", func(t *testing.T) { + // A demo that cannot be replayed is hard to talk about when something in + // it goes wrong, and how the changes were spread is part of what + // happened — so the pick follows the run tag. + assert.Equal(t, resolveFolders("0810-1203", 0), resolveFolders("0810-1203", 0)) + + seen := make(map[int]struct{}) + for minute := range 60 { + folders := resolveFolders(fmt.Sprintf("0810-12%02d", minute), 0) + assert.GreaterOrEqual(t, folders, minShardDirs) + assert.LessOrEqual(t, folders, maxShardDirs) + seen[folders] = struct{}{} } - } - assert.Greater(t, len(buckets), 50, "80 files should land in many distinct buckets") + assert.Greater(t, len(seen), 1, "runs must not all pick the same number") + }) } func TestChangeFileCount(t *testing.T) { @@ -112,7 +149,10 @@ func TestRowCount(t *testing.T) { } func TestConfigValidate(t *testing.T) { - valid := config{token: "t", repo: "owner/name", count: 3, files: 3, concurrency: 5} + valid := config{ + provider: providerGitHub, token: "t", repo: "owner/name", + count: 3, files: 3, concurrency: 5, + } tests := []struct { name string @@ -127,6 +167,23 @@ func TestConfigValidate(t *testing.T) { {name: "zero concurrency would never start", mutate: func(c *config) { c.concurrency = 0 }, wantErr: true}, {name: "negative concurrency", mutate: func(c *config) { c.concurrency = -1 }, wantErr: true}, {name: "repo without an owner", mutate: func(c *config) { c.repo = "name" }, wantErr: true}, + + // The credential and the repository are GitHub's alone. Requiring + // either of the other two modes to carry them is what would make the + // quickstart need a token it has no use for. + {name: "fake needs no token", mutate: func(c *config) { + c.provider, c.token, c.repo = providerFake, "", "" + }}, + {name: "git needs no token", mutate: func(c *config) { + c.provider, c.token, c.repo = providerGit, "", "" + }}, + {name: "an unknown provider", mutate: func(c *config) { c.provider = "gitlab" }, wantErr: true}, + {name: "no provider at all", mutate: func(c *config) { c.provider = "" }, wantErr: true}, + + // Counts are checked for every mode, not just the ones that do I/O. + {name: "fake with no changes to make", mutate: func(c *config) { + c.provider, c.token, c.repo, c.count = providerFake, "", "", 0 + }, wantErr: true}, } for _, tt := range tests { diff --git a/service/submitqueue/demo/requests/source.go b/service/submitqueue/demo/requests/source.go new file mode 100644 index 00000000..b51d109d --- /dev/null +++ b/service/submitqueue/demo/requests/source.go @@ -0,0 +1,132 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "context" + "net/url" + "path" + + "github.com/uber/submitqueue/platform/fakemarker" + "github.com/uber/submitqueue/submitqueue/client" +) + +// Provider names, matching the demo provider directories and the Makefile's +// PROVIDER variable. +const ( + providerFake = "fake" + providerGit = "git" + providerGitHub = "github" +) + +// changeSource creates the changes a run submits, in whatever way the provider +// it stands for makes a change exist. +// +// Everything above this interface — how many changes to create, whether they +// stack, when each is enqueued, what the table shows — is the same for all +// three. What differs is only whether a change is a pull request, a branch in a +// repository on disk, or nothing at all but a URI. +type changeSource interface { + // baseSHA resolves the commit the first change branches from. + baseSHA(ctx context.Context, branch string) (string, error) + + // open makes one change exist and reports what the run needs to track it. + open(ctx context.Context, spec changeSpec) (openedChange, error) +} + +// changeSpec is one change to create: a branch cut from a parent, carrying +// files. The caller decides what a change is made of, so every provider +// produces the same shape of change. +type changeSpec struct { + // branch is the ref to create. + branch string + // parentBranch is what the change targets — the base branch, or the branch + // of the change before it in a stack. + parentBranch string + // parentSHA is the commit the branch is cut from. + parentSHA string + // title describes the change where a provider shows one. + title string + // files are committed one at a time, so a change arrives as a range of + // commits rather than a single edit. + files []changeFile + // note reports progress to the run's table. Sources call it for the steps + // slow enough to be worth watching. + note func(format string, args ...any) +} + +// changeFile is one file a change writes, and the commit that carries it. +type changeFile struct { + path string + body string + message string +} + +// maxChangeURIBytes is the longest change URI the gateway accepts, because a +// URI is also a storage key. +const maxChangeURIBytes = 255 + +// withFiles appends to a change URI the paths it touches, for sources whose +// changes no provider can be asked about. +// +// The orchestrator's conflict analyzer keys on the files a change reports, and +// gets them from the change provider. With no provider behind fake and git +// changes, the run that authored them is the only thing that knows — so it says +// so on the URI, and the fake provider reads it back. Without this the analyzer +// sees a change that touches nothing, and a batch that touches nothing conflicts +// with nothing. +// +// One path per directory, not all of them. The demo's analyzer keys on the +// directory, so a second file in a directory already named adds a key that is +// already there — while the URI has a fixed byte budget that a change touching +// eight files would blow straight through. Paths that do not fit are dropped +// rather than truncated: a shortened path is a different directory, which would +// be worse than an unreported one. +func withFiles(base string, files []changeFile) string { + seen := make(map[string]struct{}, len(files)) + marker := "?" + fakemarker.FilesPrefix + + for _, f := range files { + dir := path.Dir(f.path) + if _, ok := seen[dir]; ok { + continue + } + entry := url.QueryEscape(f.path) + if len(seen) > 0 { + entry = "," + entry + } + if len(base)+len(marker)+len(entry) > maxChangeURIBytes { + break + } + seen[dir] = struct{}{} + marker += entry + } + + if len(seen) == 0 { + return base + } + return base + marker +} + +// openedChange is what a run needs back about a change that now exists. +type openedChange struct { + // headSHA is the commit the change's URI pins, and what the next change in + // a stack is cut from. + headSHA string + // uri is the change URI submitted to the gateway. + uri string + // cell is what the table shows for this change. + cell client.Cell +} diff --git a/service/submitqueue/demo/requests/source_test.go b/service/submitqueue/demo/requests/source_test.go new file mode 100644 index 00000000..8f45cde2 --- /dev/null +++ b/service/submitqueue/demo/requests/source_test.go @@ -0,0 +1,300 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + gitchange "github.com/uber/submitqueue/platform/base/change/git" + "github.com/uber/submitqueue/platform/fakemarker" + "github.com/uber/submitqueue/platform/gitexec" +) + +// quietSpec is a changeSpec whose progress notes go nowhere, for tests that +// care about what was created rather than what was reported. +func quietSpec(branch, parentBranch, parentSHA string, files ...changeFile) changeSpec { + return changeSpec{ + branch: branch, + parentBranch: parentBranch, + parentSHA: parentSHA, + title: "demo change", + files: files, + note: func(string, ...any) {}, + } +} + +func TestWithFiles_NamesOnePathPerDirectory(t *testing.T) { + uri := withFiles("git://git.example.com/r/x/y", []changeFile{ + {path: "demo/alpha/one.txt"}, + {path: "demo/alpha/two.txt"}, + {path: "demo/beta/three.txt"}, + }) + + assert.Equal(t, []string{"demo/alpha/one.txt", "demo/beta/three.txt"}, fakemarker.Files([]string{uri}), + "a second file in a directory already named adds no key the analyzer does not have") +} + +// The gateway rejects a change URI over 255 bytes, so a change touching many +// directories must lose paths rather than produce a request that cannot be +// submitted at all. +func TestWithFiles_StaysWithinTheURIBudget(t *testing.T) { + base := "git://git.example.com/sandbox/refs%2Fheads%2Fdemo%2F0814-154238%2F1/" + strings.Repeat("a", 40) + files := make([]changeFile, 0, 20) + for i := range 20 { + files = append(files, changeFile{path: fmt.Sprintf("demo/area-%02d/0814-154238-1-%d.txt", i, i)}) + } + + uri := withFiles(base, files) + assert.LessOrEqual(t, len(uri), maxChangeURIBytes) + assert.NotEmpty(t, fakemarker.Files([]string{uri}), "some paths must still be reported") +} + +func TestWithFiles_LeavesTheURIAloneWithNoFiles(t *testing.T) { + assert.Equal(t, "git://git.example.com/r/x/y", withFiles("git://git.example.com/r/x/y", nil)) +} + +// Every URI a run submits has to survive the gateway's validation, marker and +// all — which is what the first attempt at this got wrong. +func TestSources_ProduceSubmittableURIs(t *testing.T) { + files := make([]changeFile, 0, 8) + for k := 1; k <= 8; k++ { + files = append(files, changeFile{path: changeFilePath("0814-154238", 5, 1, k)}) + } + spec := quietSpec("demo/0814-154238/1", "main", strings.Repeat("b", 40), files...) + + opened, err := fakeSource{}.open(context.Background(), spec) + require.NoError(t, err) + assert.LessOrEqual(t, len(opened.uri), maxChangeURIBytes) + _, err = gitchange.ParseChangeID(opened.uri) + assert.NoError(t, err) +} + +func TestFakeSource_MintsAParsableChangeURI(t *testing.T) { + opened, err := fakeSource{}.open(context.Background(), quietSpec("demo/0102-030405/1", "main", "abc")) + require.NoError(t, err) + + // The gateway parses what this mints, so a URI it would reject is a bug + // here rather than a surprise at land time. + id, err := gitchange.ParseChangeID(opened.uri) + require.NoError(t, err) + assert.Equal(t, "refs/heads/demo/0102-030405/1", id.Ref, + "the ref must survive percent-encoding, slashes and all") + assert.Equal(t, opened.headSHA, id.CommitSHA) + assert.Len(t, opened.headSHA, 40) +} + +func TestFakeSource_GivesEachChangeItsOwnCommit(t *testing.T) { + ctx := context.Background() + first, err := fakeSource{}.open(ctx, quietSpec("demo/run/1", "main", "abc")) + require.NoError(t, err) + second, err := fakeSource{}.open(ctx, quietSpec("demo/run/2", "main", "abc")) + require.NoError(t, err) + + assert.NotEqual(t, first.headSHA, second.headSHA, + "two changes sharing a head would land as one") +} + +// A change with no repository behind it has nothing to open, and a cell with a +// URL renders as a link — so it must not carry one. +func TestFakeSource_LabelsChangesWithTheBranchAndNoLink(t *testing.T) { + opened, err := fakeSource{}.open(context.Background(), quietSpec("demo/run/1", "main", "abc")) + require.NoError(t, err) + + assert.Equal(t, "demo/run/1", opened.cell.Text) + assert.Empty(t, opened.cell.URL) +} + +func TestFakeSource_IsReproducible(t *testing.T) { + ctx := context.Background() + first, err := fakeSource{}.open(ctx, quietSpec("demo/run/1", "main", "abc")) + require.NoError(t, err) + again, err := fakeSource{}.open(ctx, quietSpec("demo/run/1", "main", "abc")) + require.NoError(t, err) + + assert.Equal(t, first.uri, again.uri, "replaying a run must submit the same change") +} + +// testGit resolves the pinned git the test target supplies. See the identical +// helper in tool/gitsandbox for why the runfile has to be re-rooted. +func testGit(t *testing.T) string { + t.Helper() + + supplied := os.Getenv("SUBMITQUEUE_TEST_GIT") + require.NotEmpty(t, supplied, "the test target must supply SUBMITQUEUE_TEST_GIT") + if git, err := gitexec.Resolve(supplied); err == nil { + return git + } + + slashed := filepath.ToSlash(supplied) + index := strings.Index(slashed, "/external/") + require.GreaterOrEqual(t, index, 0, "SUBMITQUEUE_TEST_GIT=%q is not a runfile", supplied) + root := os.Getenv("TEST_SRCDIR") + require.NotEmpty(t, root) + + git, err := gitexec.Resolve(filepath.Join(root, filepath.FromSlash(slashed[index+len("/external/"):]))) + require.NoError(t, err) + return git +} + +// sandbox creates a bare repository with one commit on main, standing in for +// what tool/gitsandbox provisions. +func sandbox(t *testing.T, git string) string { + t.Helper() + ctx := context.Background() + bare := filepath.Join(t.TempDir(), "sandbox.git") + + require.NoError(t, gitexec.Run(ctx, git, "", "init", "--bare", "-b", "main", bare)) + seed := t.TempDir() + require.NoError(t, gitexec.Run(ctx, git, "", "clone", bare, seed)) + require.NoError(t, gitexec.Run(ctx, git, seed, "config", "user.name", "Test")) + require.NoError(t, gitexec.Run(ctx, git, seed, "config", "user.email", "test@example.invalid")) + require.NoError(t, os.WriteFile(filepath.Join(seed, "seed.txt"), []byte("seed\n"), 0o644)) + require.NoError(t, gitexec.Run(ctx, git, seed, "add", ".")) + require.NoError(t, gitexec.Run(ctx, git, seed, "commit", "-m", "seed")) + require.NoError(t, gitexec.Run(ctx, git, seed, "push", "origin", "main")) + return bare +} + +func TestGitSource_PushesABranchWithACommitPerFile(t *testing.T) { + git := testGit(t) + ctx := context.Background() + bare := sandbox(t, git) + + src, err := newGitSource(ctx, git, bare, "sandbox") + require.NoError(t, err) + defer src.close() + + base, err := src.baseSHA(ctx, "main") + require.NoError(t, err) + + opened, err := src.open(ctx, quietSpec("demo/run/1", "main", base, + changeFile{path: "a/one.txt", body: "one\n", message: "add one"}, + changeFile{path: "b/two.txt", body: "two\n", message: "add two"}, + )) + require.NoError(t, err) + + subjects, err := gitexec.Output(ctx, git, bare, "log", "--reverse", "--format=%s", "main..refs/heads/demo/run/1") + require.NoError(t, err) + assert.Equal(t, []string{"add one", "add two"}, strings.Split(subjects, "\n"), + "each file must arrive as its own commit, in order") + + head, err := gitexec.Output(ctx, git, bare, "rev-parse", "refs/heads/demo/run/1") + require.NoError(t, err) + assert.Equal(t, head, opened.headSHA, "the URI must pin the commit that was pushed") + + contents, err := gitexec.Output(ctx, git, bare, "show", "refs/heads/demo/run/1:a/one.txt") + require.NoError(t, err) + assert.Equal(t, "one", contents) +} + +func TestGitSource_MintsAURIPinningTheHead(t *testing.T) { + git := testGit(t) + ctx := context.Background() + src, err := newGitSource(ctx, git, sandbox(t, git), "sandbox") + require.NoError(t, err) + defer src.close() + + base, err := src.baseSHA(ctx, "main") + require.NoError(t, err) + opened, err := src.open(ctx, quietSpec("demo/run/1", "main", base, + changeFile{path: "one.txt", body: "one\n", message: "add one"})) + require.NoError(t, err) + + id, err := gitchange.ParseChangeID(opened.uri) + require.NoError(t, err) + assert.Equal(t, "sandbox", id.Repo) + assert.Equal(t, "refs/heads/demo/run/1", id.Ref) + assert.Equal(t, opened.headSHA, id.CommitSHA) + assert.Equal(t, "demo/run/1", opened.cell.Text) + assert.Empty(t, opened.cell.URL, "a branch in a bare repository has nothing to open") +} + +// A stack is cut from the previous change's head rather than the base, which is +// what makes the chain land in the order it was built. +func TestGitSource_StacksOnAPreviousChange(t *testing.T) { + git := testGit(t) + ctx := context.Background() + bare := sandbox(t, git) + src, err := newGitSource(ctx, git, bare, "sandbox") + require.NoError(t, err) + defer src.close() + + base, err := src.baseSHA(ctx, "main") + require.NoError(t, err) + first, err := src.open(ctx, quietSpec("demo/run/1", "main", base, + changeFile{path: "one.txt", body: "one\n", message: "add one"})) + require.NoError(t, err) + second, err := src.open(ctx, quietSpec("demo/run/2", "demo/run/1", first.headSHA, + changeFile{path: "two.txt", body: "two\n", message: "add two"})) + require.NoError(t, err) + + require.NoError(t, gitexec.Run(ctx, git, bare, "merge-base", "--is-ancestor", first.headSHA, second.headSHA), + "the second change must build on the first") +} + +// One working tree cannot take concurrent checkouts and commits. createIndependent +// runs up to -concurrency of these at once, so the source has to serialize them +// itself; without the lock this corrupts the index or races the branch. +func TestGitSource_SerializesConcurrentChanges(t *testing.T) { + git := testGit(t) + ctx := context.Background() + bare := sandbox(t, git) + src, err := newGitSource(ctx, git, bare, "sandbox") + require.NoError(t, err) + defer src.close() + + base, err := src.baseSHA(ctx, "main") + require.NoError(t, err) + + const changes = 5 + var wg sync.WaitGroup + errs := make([]error, changes) + for i := range changes { + wg.Add(1) + go func() { + defer wg.Done() + branch := fmt.Sprintf("demo/run/%d", i+1) + _, errs[i] = src.open(ctx, quietSpec(branch, "main", base, + changeFile{path: fmt.Sprintf("file-%d.txt", i+1), body: "x\n", message: "add " + branch})) + }() + } + wg.Wait() + + for i, err := range errs { + require.NoError(t, err, "change %d failed", i+1) + } + for i := range changes { + branch := fmt.Sprintf("refs/heads/demo/run/%d", i+1) + _, err := gitexec.Output(ctx, git, bare, "rev-parse", branch) + assert.NoError(t, err, "%s must exist", branch) + } +} + +func TestNewChangeSource_GitRejectsAMissingSandbox(t *testing.T) { + _, _, err := newChangeSource(context.Background(), config{ + provider: providerGit, + git: testGit(t), + sandboxDir: filepath.Join(t.TempDir(), "nothing-here"), + }) + require.Error(t, err, "a missing sandbox must say so rather than fail later mid-run") +} diff --git a/service/submitqueue/docker-compose.fake.yml b/service/submitqueue/docker-compose.fake.yml new file mode 100644 index 00000000..d078c564 --- /dev/null +++ b/service/submitqueue/docker-compose.fake.yml @@ -0,0 +1,37 @@ +# Compose overlay: run the stack with nothing outside the queue. +# +# Layered over docker-compose.yml, which it does not modify: +# +# docker compose -f docker-compose.yml -f docker-compose.fake.yml up +# +# or, more simply, `make local-submitqueue-start` — this is the default provider. +# +# The lightest of the three overlays, and the only one that mounts nothing but +# configuration: there is no repository to reach and no credential to carry, so +# a change is a URI and the merge is a no-op. See demo/provider/fake. +# +# Required in the environment: +# SQ_PROVIDER_CONFIG_DIR directory holding profiles.yaml and merge.yaml +# +# The stack would run without this overlay at all — the orchestrator falls back +# to fake integrations when no profiles are configured, and Runway to the noop +# merger. Mounting the configuration anyway is what makes the mode a file +# someone can read and diff against ../git and ../github, rather than a +# behaviour they have to know about. + +services: + orchestrator-service: + environment: + # Which change provider, build runner, and conflict analyzer each queue + # resolves to. + - PROFILES_CONFIG_PATH=/etc/submitqueue/profiles.yaml + volumes: + - ${SQ_PROVIDER_CONFIG_DIR:?set SQ_PROVIDER_CONFIG_DIR, e.g. ./service/submitqueue/demo/provider/fake}:/etc/submitqueue:ro + + runway-service: + environment: + # Per-queue merge targets. Every queue here resolves to the noop merger, + # which reports success without touching a repository. + - MERGE_CONFIG_PATH=/etc/submitqueue/merge.yaml + volumes: + - ${SQ_PROVIDER_CONFIG_DIR:?set SQ_PROVIDER_CONFIG_DIR, e.g. ./service/submitqueue/demo/provider/fake}:/etc/submitqueue:ro diff --git a/service/submitqueue/docker-compose.git.yml b/service/submitqueue/docker-compose.git.yml index 4c09bd98..9371f01f 100644 --- a/service/submitqueue/docker-compose.git.yml +++ b/service/submitqueue/docker-compose.git.yml @@ -16,7 +16,7 @@ # # Required in the environment, all owned by the test: # SQ_PROVIDER_CONFIG_DIR profiles.yaml / merge.yaml selecting each queue's -# extensions (service/submitqueue/demo/provider/local) +# extensions (service/submitqueue/demo/provider/git) # SQ_GIT_SANDBOX_DIR the bare repository the merger fetches and pushes # SQ_RUNWAY_CHECKOUT_DIR storage for the working trees the merger owns @@ -39,7 +39,19 @@ services: - ${SQ_PROVIDER_CONFIG_DIR}:/etc/submitqueue:ro - ${SQ_GIT_SANDBOX_DIR}:/srv/git # A checkout is state, not image content, so it is mounted rather than - # baked in — and mounting it is also what makes the path writable by a - # service running as a non-root user, which the E2E does so its bind - # mounts stay readable from the host. - - ${SQ_RUNWAY_CHECKOUT_DIR}:/var/runway/checkouts + # baked in. + # + # Where it is mounted from depends on who is running. The E2E points + # SQ_RUNWAY_CHECKOUT_DIR at a host directory, because it runs the services + # as the host user and a named volume would arrive owned by root. Left + # unset — which is how `make local-submitqueue-start PROVIDER=git` runs it — + # it is a named volume instead, keeping the merger's object writes off a + # bind mount: this is the directory git clones, cherry-picks and commits + # in, and on macOS a freshly written loose object can read back as corrupt + # over the host filesystem bridge, failing a merge that has nothing wrong + # with it. The bare repository above only receives finished pushes, so it + # stays a bind mount and keeps `git log` on the host working. + - ${SQ_RUNWAY_CHECKOUT_DIR:-runway-checkouts}:/var/runway/checkouts + +volumes: + runway-checkouts: diff --git a/service/submitqueue/docker-compose.provider.yml b/service/submitqueue/docker-compose.provider.yml index ed226986..fbd22d9a 100644 --- a/service/submitqueue/docker-compose.provider.yml +++ b/service/submitqueue/docker-compose.provider.yml @@ -4,7 +4,7 @@ # # docker compose -f docker-compose.yml -f docker-compose.provider.yml up # -# or, more simply, `make local-provider-start PROVIDER=github`. +# or, more simply, `make local-submitqueue-start PROVIDER=github`. # # Nothing here names a provider. Which one is reached is decided entirely by the # configuration directory bind-mounted at /etc/submitqueue — see diff --git a/service/submitqueue/gateway/server/queues.yaml b/service/submitqueue/gateway/server/queues.yaml index 5e7434d3..693a3edb 100644 --- a/service/submitqueue/gateway/server/queues.yaml +++ b/service/submitqueue/gateway/server/queues.yaml @@ -22,11 +22,11 @@ queues: # orchestrator example server. - name: e2e-conflict-error-queue # Used by the hermetic git E2E, where Runway is wired to a real git merger - # against a bare repository. See service/submitqueue/demo/provider/local. + # against a bare repository. See service/submitqueue/demo/provider/git. - name: e2e-git-queue - # Used by the provider demo stack (make local-provider-start), where the whole - # pipeline runs against a real repository. See - # service/submitqueue/demo/provider and doc/howto/PROVIDER-E2E.md. + # Used by the provider demo stack (make local-submitqueue-start) in every mode — + # fake, git, and github. See service/submitqueue/demo/provider and + # doc/howto/QUICKSTART.md. - name: demo-queue # Inherits the baseline "all" analyzer, which serializes the queue, so a # second request lands as a batch depending on the first. e2e uses that to diff --git a/submitqueue/extension/changeprovider/fake/fake.go b/submitqueue/extension/changeprovider/fake/fake.go index c6c1b4b7..b98c0779 100644 --- a/submitqueue/extension/changeprovider/fake/fake.go +++ b/submitqueue/extension/changeprovider/fake/fake.go @@ -20,6 +20,11 @@ // // sq-fake=provider-error -> non-nil error // +// A URI may also carry the paths its change touches, which a real provider would +// have reported from the repository: +// +// sq-files=pkg/a/one.go,pkg/b/two.go +// // This lets a single running stack exercise negative paths purely by varying // request payloads. It is intended for examples and tests only, never // production. @@ -53,6 +58,11 @@ func New(cfg changeprovider.Config) changeprovider.ChangeProvider { // Get returns one ChangeInfo per URI in the request's change, unless a recognized // marker token requests a failure. The "one ChangeInfo per URI" contract is preserved. +// +// A URI may also state the paths it touches, which is what lets a path-keyed +// conflict analyzer work against changes no provider knows about. Line counts +// are reported as a single added line each: the analyzers that read paths do not +// weigh them, and inventing a number would only look like data. func (provider) Get(_ context.Context, request entity.Request) ([]entity.ChangeInfo, error) { change := request.Change if fakemarker.Token(change.URIs) == tokenError { @@ -61,7 +71,12 @@ func (provider) Get(_ context.Context, request entity.Request) ([]entity.ChangeI infos := make([]entity.ChangeInfo, 0, len(change.URIs)) for _, uri := range change.URIs { - infos = append(infos, entity.ChangeInfo{URI: uri}) + info := entity.ChangeInfo{URI: uri} + for _, path := range fakemarker.Files([]string{uri}) { + info.Details.ChangedFiles = append(info.Details.ChangedFiles, + entity.ChangedFile{Path: path, LinesAdded: 1}) + } + infos = append(infos, info) } return infos, nil } diff --git a/submitqueue/extension/changeprovider/fake/fake_test.go b/submitqueue/extension/changeprovider/fake/fake_test.go index 6a420783..1bb04b54 100644 --- a/submitqueue/extension/changeprovider/fake/fake_test.go +++ b/submitqueue/extension/changeprovider/fake/fake_test.go @@ -68,3 +68,33 @@ func TestProvider_Get_ErrorMarker(t *testing.T) { }}) require.Error(t, err) } + +// Without this the path-keyed conflict analyzers see a change that touches +// nothing, and a batch that touches nothing conflicts with nothing — so a queue +// configured to serialize on overlap silently runs everything in parallel. +func TestProvider_Get_ReportsFilesFromTheURI(t *testing.T) { + p := New(testCfg) + + infos, err := p.Get(context.Background(), entity.Request{Change: change.Change{ + URIs: []string{"git://git.example.com/sandbox/refs%2Fheads%2Fa/abc?sq-files=demo/alpha/one.txt,demo/alpha/two.txt"}, + }}) + require.NoError(t, err) + require.Len(t, infos, 1) + + paths := make([]string, 0, len(infos[0].Details.ChangedFiles)) + for _, f := range infos[0].Details.ChangedFiles { + paths = append(paths, f.Path) + } + assert.Equal(t, []string{"demo/alpha/one.txt", "demo/alpha/two.txt"}, paths) +} + +func TestProvider_Get_ReportsNoFilesWithoutTheMarker(t *testing.T) { + p := New(testCfg) + + infos, err := p.Get(context.Background(), entity.Request{Change: change.Change{ + URIs: []string{"git://git.example.com/sandbox/refs%2Fheads%2Fa/abc"}, + }}) + require.NoError(t, err) + require.Len(t, infos, 1) + assert.Empty(t, infos[0].Details.ChangedFiles) +} diff --git a/test/e2e/submitqueue/BUILD.bazel b/test/e2e/submitqueue/BUILD.bazel index a2f1c713..73bed50c 100644 --- a/test/e2e/submitqueue/BUILD.bazel +++ b/test/e2e/submitqueue/BUILD.bazel @@ -13,7 +13,7 @@ go_test( "//service/runway/server:docker_test_context", "//service/submitqueue:docker-compose.git.yml", "//service/submitqueue:docker-compose.yml", - "//service/submitqueue/demo/provider/local:config", + "//service/submitqueue/demo/provider/git:config", "//service/submitqueue/gateway/server:docker_test_context", "//service/submitqueue/orchestrator/server:docker_test_context", "//submitqueue/extension/storage/mysql/schema", diff --git a/test/e2e/submitqueue/git_suite_test.go b/test/e2e/submitqueue/git_suite_test.go index 681fbc01..dd508476 100644 --- a/test/e2e/submitqueue/git_suite_test.go +++ b/test/e2e/submitqueue/git_suite_test.go @@ -24,8 +24,8 @@ // that is what the merge machinery depends on — which is what lets these // assertions gate a pull request. What it deliberately cannot cover is the half // that is specific to a change provider: reading change metadata, that -// provider's CI, and a real change being marked merged. That is the manual -// tier; see doc/howto/PROVIDER-E2E.md. +// provider's CI, and a real change being marked merged. Those need a repository +// and a credential, so they are exercised by hand — see doc/howto/QUICKSTART.md. package e2e_test import ( @@ -50,7 +50,7 @@ import ( ) // gitQueue is the queue wired to the git merger in -// service/submitqueue/demo/provider/local/merge.yaml. +// service/submitqueue/demo/provider/git/merge.yaml. const gitQueue = "e2e-git-queue" // sandboxRemote is the host name used in git:// change URIs. The merger reads @@ -300,7 +300,7 @@ func (s *GitMergeSuite) stageProviderConfig() string { t := s.T() staged := t.TempDir() for _, name := range []string{"merge.yaml", "profiles.yaml"} { - contents, err := os.ReadFile(testutil.Runfile("service/submitqueue/demo/provider/local/" + name)) + contents, err := os.ReadFile(testutil.Runfile("service/submitqueue/demo/provider/git/" + name)) require.NoError(t, err, "reading example config %s", name) require.NoError(t, os.WriteFile(filepath.Join(staged, name), contents, 0o644)) } diff --git a/tool/gitsandbox/BUILD.bazel b/tool/gitsandbox/BUILD.bazel new file mode 100644 index 00000000..d01f72c7 --- /dev/null +++ b/tool/gitsandbox/BUILD.bazel @@ -0,0 +1,38 @@ +load("@rules_go//go:def.bzl", "go_binary", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["main.go"], + importpath = "github.com/uber/submitqueue/tool/gitsandbox", + visibility = ["//visibility:private"], + deps = ["//platform/gitexec:go_default_library"], +) + +go_binary( + name = "gitsandbox", + # The pinned git the merger uses, rather than whatever the host has, so a + # sandbox is seeded by the same build that later merges into it. `args` is + # honored by `bazel run`, and flags the Makefile appends after `--` still + # win, since Go's flag package takes the last occurrence. + args = ["-git=$(location @git//:git)"], + data = ["@git"], + embed = [":go_default_library"], + visibility = ["//visibility:public"], +) + +go_test( + name = "go_default_test", + srcs = ["main_test.go"], + data = ["@git"], + embed = [":go_default_library"], + # The same pinned git the binary seeds with, so these assertions cannot + # pass against a host git that behaves differently. + env = { + "SUBMITQUEUE_TEST_GIT": "$(location @git//:git)", + }, + deps = [ + "//platform/gitexec:go_default_library", + "@com_github_stretchr_testify//assert:go_default_library", + "@com_github_stretchr_testify//require:go_default_library", + ], +) diff --git a/tool/gitsandbox/main.go b/tool/gitsandbox/main.go new file mode 100644 index 00000000..4c3d05cb --- /dev/null +++ b/tool/gitsandbox/main.go @@ -0,0 +1,168 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Command gitsandbox provisions the bare repository the git provider merges +// into (`make local-submitqueue-start PROVIDER=git`). +// +// A merge target has to exist before Runway starts: the merger clones it at +// boot and fails outright if it cannot. So this runs ahead of the stack, +// creating a bare repository with one commit on the target branch — the +// smallest thing a merge can be performed against. +// +// It is idempotent. An already-initialized repository is left exactly as it is, +// so restarting the stack keeps whatever previous runs landed rather than +// resetting the history someone may be looking at. +package main + +import ( + "context" + "flag" + "fmt" + "os" + "path/filepath" + + "github.com/uber/submitqueue/platform/gitexec" +) + +// seedFile is committed so the target branch exists with something on it. A +// branch with no commits cannot be cloned or merged into. +const seedFile = "README.md" + +const seedContents = `# SubmitQueue sandbox + +Created by ` + "`make local-submitqueue-start PROVIDER=git`" + `. Everything below the +seed commit was landed by SubmitQueue. +` + +func main() { + git := flag.String("git", "", "path to the git binary; defaults to GIT_EXECUTABLE, then PATH") + sandboxDir := flag.String("sandbox-dir", "", "directory holding the bare repository (required)") + checkoutDir := flag.String("checkout-dir", "", "directory Runway provisions its working trees in") + repoName := flag.String("repo-name", "sandbox", "bare repository name, without the .git suffix") + branch := flag.String("branch", "main", "target branch created by the seed commit") + flag.Parse() + + if err := run(context.Background(), *git, *sandboxDir, *checkoutDir, *repoName, *branch); err != nil { + fmt.Fprintln(os.Stderr, "Error:", err) + os.Exit(1) + } +} + +func run(ctx context.Context, git, sandboxDir, checkoutDir, repoName, branch string) error { + if sandboxDir == "" { + return fmt.Errorf("-sandbox-dir is required") + } + + resolved, err := gitexec.Resolve(git) + if err != nil { + return err + } + + bare := filepath.Join(sandboxDir, repoName+".git") + created, err := provision(ctx, resolved, bare, branch) + if err != nil { + return err + } + + // Docker creates a missing bind-mount source itself, owned by root, which + // on rootful Docker leaves a directory the host user cannot clean up. + if checkoutDir != "" { + if err := os.MkdirAll(checkoutDir, 0o755); err != nil { + return fmt.Errorf("could not create checkout directory %q: %w", checkoutDir, err) + } + } + + if created { + fmt.Printf("Initialized sandbox repository at %s (branch %s)\n", bare, branch) + } else { + fmt.Printf("Reusing sandbox repository at %s\n", bare) + } + return nil +} + +// provision creates and seeds the bare repository, reporting whether it did any +// work. An existing repository is left alone. +func provision(ctx context.Context, git, bare, branch string) (bool, error) { + // HEAD is written by `git init` before anything else, so its presence marks + // a repository that has at least been initialized. + if _, err := os.Stat(filepath.Join(bare, "HEAD")); err == nil { + return false, nil + } + + if err := os.MkdirAll(filepath.Dir(bare), 0o755); err != nil { + return false, fmt.Errorf("could not create sandbox directory: %w", err) + } + + if err := create(ctx, git, bare, branch); err != nil { + // A repository that is initialized but has no commit cannot be merged + // into, and the next run would skip it as already provisioned — so the + // stack would fail at boot instead of being repaired. Leave nothing + // behind rather than something unusable. + if removeErr := os.RemoveAll(bare); removeErr != nil { + return false, fmt.Errorf("%w (and the partial repository at %s could not be removed: %v)", err, bare, removeErr) + } + return false, err + } + return true, nil +} + +// create initializes the bare repository and puts the first commit on its +// target branch. +func create(ctx context.Context, git, bare, branch string) error { + if err := gitexec.Run(ctx, git, "", "init", "--bare", "-b", branch, bare); err != nil { + return err + } + // Bare repositories do not log ref updates by default, and the reflog is + // how a reader confirms a whole stack landed in a single push. + if err := gitexec.Run(ctx, git, bare, "config", "core.logAllRefUpdates", "true"); err != nil { + return err + } + return seed(ctx, git, bare, branch) +} + +// seed commits an initial file on the target branch, through a throwaway clone +// because a bare repository has no working tree to commit from. +func seed(ctx context.Context, git, bare, branch string) error { + work, err := os.MkdirTemp("", "sq-sandbox-seed-") + if err != nil { + return fmt.Errorf("could not create a working clone: %w", err) + } + defer os.RemoveAll(work) + + if err := gitexec.Run(ctx, git, "", "clone", bare, work); err != nil { + return err + } + // An identity is required to commit, and the ambient one is unavailable: + // gitexec strips the global config precisely so a developer's hooks and + // signing settings cannot fail this. + for _, kv := range [][2]string{ + {"user.name", "SubmitQueue Sandbox"}, + {"user.email", "sandbox@submitqueue.invalid"}, + } { + if err := gitexec.Run(ctx, git, work, "config", kv[0], kv[1]); err != nil { + return err + } + } + + if err := os.WriteFile(filepath.Join(work, seedFile), []byte(seedContents), 0o644); err != nil { + return fmt.Errorf("could not write the seed file: %w", err) + } + if err := gitexec.Run(ctx, git, work, "add", seedFile); err != nil { + return err + } + if err := gitexec.Run(ctx, git, work, "commit", "-m", "seed the sandbox"); err != nil { + return err + } + return gitexec.Run(ctx, git, work, "push", "origin", branch) +} diff --git a/tool/gitsandbox/main_test.go b/tool/gitsandbox/main_test.go new file mode 100644 index 00000000..00ae9b54 --- /dev/null +++ b/tool/gitsandbox/main_test.go @@ -0,0 +1,201 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/uber/submitqueue/platform/gitexec" +) + +// testGit resolves the pinned git the test target supplies, so these assertions +// exercise the same build the sandbox is seeded with rather than the host's. +// +// rules_go expands $(location) to an execroot-relative path, which for an +// external output has to be re-rooted under the runfiles directory a test runs +// from — the same re-rooting the git E2E does. Under `bazel run` the binary +// needs none of this, because it already runs from the runfiles tree. +func testGit(t *testing.T) string { + t.Helper() + + supplied := os.Getenv("SUBMITQUEUE_TEST_GIT") + require.NotEmpty(t, supplied, "the test target must supply SUBMITQUEUE_TEST_GIT") + + if git, err := gitexec.Resolve(supplied); err == nil { + return git + } + + slashed := filepath.ToSlash(supplied) + index := strings.Index(slashed, "/external/") + require.GreaterOrEqual(t, index, 0, "SUBMITQUEUE_TEST_GIT=%q is not a runfile", supplied) + external := slashed[index+len("/external/"):] + + root := os.Getenv("TEST_SRCDIR") + require.NotEmpty(t, root) + + git, err := gitexec.Resolve(filepath.Join(root, filepath.FromSlash(external))) + require.NoError(t, err, "the test target must supply a git binary") + return git +} + +func TestProvision_SeedsABareRepositoryOnTheTargetBranch(t *testing.T) { + git := testGit(t) + ctx := context.Background() + bare := filepath.Join(t.TempDir(), "sandbox.git") + + created, err := provision(ctx, git, bare, "main") + require.NoError(t, err) + assert.True(t, created, "a fresh directory must report that it was created") + + head, err := gitexec.Output(ctx, git, bare, "rev-parse", "refs/heads/main") + require.NoError(t, err, "the target branch must exist after seeding") + assert.Len(t, head, 40) + + subject, err := gitexec.Output(ctx, git, bare, "log", "-1", "--format=%s", "refs/heads/main") + require.NoError(t, err) + assert.Equal(t, "seed the sandbox", subject) + + contents, err := gitexec.Output(ctx, git, bare, "show", "refs/heads/main:"+seedFile) + require.NoError(t, err) + assert.Contains(t, contents, "SubmitQueue sandbox") +} + +func TestProvision_LeavesAnExistingRepositoryAlone(t *testing.T) { + git := testGit(t) + ctx := context.Background() + bare := filepath.Join(t.TempDir(), "sandbox.git") + + _, err := provision(ctx, git, bare, "main") + require.NoError(t, err) + before, err := gitexec.Output(ctx, git, bare, "rev-parse", "refs/heads/main") + require.NoError(t, err) + + created, err := provision(ctx, git, bare, "main") + require.NoError(t, err) + assert.False(t, created, "an initialized repository must not be re-created") + + after, err := gitexec.Output(ctx, git, bare, "rev-parse", "refs/heads/main") + require.NoError(t, err) + assert.Equal(t, before, after, + "re-running must preserve whatever landed since the first run") +} + +// A restarted stack must not lose the history someone is looking at, which is +// the whole reason provisioning is idempotent rather than a reset. +func TestProvision_PreservesCommitsLandedAfterSeeding(t *testing.T) { + git := testGit(t) + ctx := context.Background() + bare := filepath.Join(t.TempDir(), "sandbox.git") + require.NoError(t, func() error { _, err := provision(ctx, git, bare, "main"); return err }()) + + work := t.TempDir() + require.NoError(t, gitexec.Run(ctx, git, "", "clone", bare, work)) + require.NoError(t, gitexec.Run(ctx, git, work, "config", "user.name", "Test")) + require.NoError(t, gitexec.Run(ctx, git, work, "config", "user.email", "test@example.invalid")) + require.NoError(t, os.WriteFile(filepath.Join(work, "landed.txt"), []byte("landed\n"), 0o644)) + require.NoError(t, gitexec.Run(ctx, git, work, "add", ".")) + require.NoError(t, gitexec.Run(ctx, git, work, "commit", "-m", "a landed change")) + require.NoError(t, gitexec.Run(ctx, git, work, "push", "origin", "main")) + + _, err := provision(ctx, git, bare, "main") + require.NoError(t, err) + + subject, err := gitexec.Output(ctx, git, bare, "log", "-1", "--format=%s", "refs/heads/main") + require.NoError(t, err) + assert.Equal(t, "a landed change", subject) +} + +func TestProvision_HonorsTheBranchName(t *testing.T) { + git := testGit(t) + ctx := context.Background() + bare := filepath.Join(t.TempDir(), "sandbox.git") + + _, err := provision(ctx, git, bare, "trunk") + require.NoError(t, err) + + _, err = gitexec.Output(ctx, git, bare, "rev-parse", "refs/heads/trunk") + assert.NoError(t, err) +} + +// The reflog is how a reader confirms a stack landed in one push, and bare +// repositories do not keep one unless asked. +func TestProvision_EnablesTheReflog(t *testing.T) { + git := testGit(t) + ctx := context.Background() + bare := filepath.Join(t.TempDir(), "sandbox.git") + + _, err := provision(ctx, git, bare, "main") + require.NoError(t, err) + + value, err := gitexec.Output(ctx, git, bare, "config", "core.logAllRefUpdates") + require.NoError(t, err) + assert.Equal(t, "true", value) +} + +func TestRun_RequiresASandboxDirectory(t *testing.T) { + err := run(context.Background(), testGit(t), "", "", "sandbox", "main") + require.Error(t, err) +} + +func TestRun_CreatesTheCheckoutDirectory(t *testing.T) { + checkout := filepath.Join(t.TempDir(), "checkouts") + + err := run(context.Background(), testGit(t), t.TempDir(), checkout, "sandbox", "main") + require.NoError(t, err) + + info, err := os.Stat(checkout) + require.NoError(t, err) + assert.True(t, info.IsDir()) +} + +// A repository that exists but carries no commit cannot be merged into, and the +// next run would skip it as already provisioned rather than repair it. +func TestProvision_LeavesNothingBehindWhenCreationFails(t *testing.T) { + ctx := context.Background() + bare := filepath.Join(t.TempDir(), "sandbox.git") + + // A branch name git refuses, so creation fails partway. + _, err := provision(ctx, testGit(t), bare, "refs/heads/") + require.Error(t, err) + + _, statErr := os.Stat(bare) + assert.True(t, os.IsNotExist(statErr), + "a repository that could not be created must not be left behind") + assert.NotContains(t, strings.ToLower(err.Error()), "could not be removed") +} + +// The failure above must be recoverable: a corrected run provisions cleanly +// rather than tripping over remnants of the one before it. +func TestProvision_SucceedsAfterAFailedAttempt(t *testing.T) { + git := testGit(t) + ctx := context.Background() + bare := filepath.Join(t.TempDir(), "sandbox.git") + + _, err := provision(ctx, git, bare, "refs/heads/") + require.Error(t, err) + + created, err := provision(ctx, git, bare, "main") + require.NoError(t, err) + assert.True(t, created) + + _, err = gitexec.Output(ctx, git, bare, "rev-parse", "refs/heads/main") + assert.NoError(t, err) +}