From 7ff4b98647f4347e6d174e8175a087524ccb3e13 Mon Sep 17 00:00:00 2001 From: Anthony Volk Date: Thu, 2 Jul 2026 21:18:51 +0200 Subject: [PATCH 1/6] Isolate staging Cloud Run service and add backend response header Migration PR 4 stage 1. The staging Cloud Run track previously promoted staging-configured revisions to 100% of the production policyengine-api service URL on every push; once a load balancer fronts that service this becomes a public incident per deploy. Staging now deploys and promotes its own policyengine-api-staging service, with the image name decoupled from the service name because production reuses the staging-built image. Every API response now carries X-PolicyEngine-Backend (app_engine or cloud_run) so traffic-weight changes and rollbacks during the host cutover can be verified in-band with sampled requests. Co-Authored-By: Claude Fable 5 --- .github/scripts/cloud_run_env.sh | 6 +- .github/workflows/push.yml | 4 ++ ...on-pr4-stage1-staging-isolation.changed.md | 1 + policyengine_api/asgi_factory.py | 6 ++ policyengine_api/migration_flags.py | 14 +++++ policyengine_api/migration_logging.py | 6 ++ .../routes/test_migration_context_logging.py | 47 ++++++++++++++ tests/unit/test_cloud_run_deploy_scripts.py | 62 +++++++++++++++++++ 8 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 changelog.d/migration-pr4-stage1-staging-isolation.changed.md diff --git a/.github/scripts/cloud_run_env.sh b/.github/scripts/cloud_run_env.sh index 66ca3c5f1..9f8c0282a 100755 --- a/.github/scripts/cloud_run_env.sh +++ b/.github/scripts/cloud_run_env.sh @@ -5,6 +5,9 @@ cloud_run_set_defaults() { CLOUD_RUN_REGION="${CLOUD_RUN_REGION:-us-central1}" CLOUD_RUN_SERVICE="${CLOUD_RUN_SERVICE:-policyengine-api}" CLOUD_RUN_ARTIFACT_REPOSITORY="${CLOUD_RUN_ARTIFACT_REPOSITORY:-policyengine-api}" + # Image name stays fixed across services: the production deploy reuses the + # image built by the staging track, so it must not embed the service name. + CLOUD_RUN_IMAGE_NAME="${CLOUD_RUN_IMAGE_NAME:-policyengine-api}" CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT="${CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT:-policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com}" CLOUD_RUN_CLOUD_SQL_INSTANCE="${CLOUD_RUN_CLOUD_SQL_INSTANCE:-policyengine-api:us-central1:policyengine-api-data}" CLOUD_RUN_CPU="${CLOUD_RUN_CPU:-4}" @@ -22,7 +25,7 @@ cloud_run_set_defaults() { local sha sha="${GITHUB_SHA:-local}" CLOUD_RUN_IMAGE_TAG="${CLOUD_RUN_IMAGE_TAG:-${sha}}" - CLOUD_RUN_IMAGE_URI="${CLOUD_RUN_IMAGE_URI:-${CLOUD_RUN_REGION}-docker.pkg.dev/${CLOUD_RUN_PROJECT}/${CLOUD_RUN_ARTIFACT_REPOSITORY}/${CLOUD_RUN_SERVICE}:${CLOUD_RUN_IMAGE_TAG}}" + CLOUD_RUN_IMAGE_URI="${CLOUD_RUN_IMAGE_URI:-${CLOUD_RUN_REGION}-docker.pkg.dev/${CLOUD_RUN_PROJECT}/${CLOUD_RUN_ARTIFACT_REPOSITORY}/${CLOUD_RUN_IMAGE_NAME}:${CLOUD_RUN_IMAGE_TAG}}" local short_sha short_sha="${sha:0:7}" @@ -32,6 +35,7 @@ cloud_run_set_defaults() { export CLOUD_RUN_REGION export CLOUD_RUN_SERVICE export CLOUD_RUN_ARTIFACT_REPOSITORY + export CLOUD_RUN_IMAGE_NAME export CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT export CLOUD_RUN_CLOUD_SQL_INSTANCE export CLOUD_RUN_CPU diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 501fbd26e..83308d0ce 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -211,6 +211,8 @@ jobs: (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') environment: staging + env: + CLOUD_RUN_SERVICE: policyengine-api-staging permissions: contents: read id-token: write @@ -316,6 +318,8 @@ jobs: (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') environment: staging + env: + CLOUD_RUN_SERVICE: policyengine-api-staging permissions: contents: read id-token: write diff --git a/changelog.d/migration-pr4-stage1-staging-isolation.changed.md b/changelog.d/migration-pr4-stage1-staging-isolation.changed.md new file mode 100644 index 000000000..c8d37b662 --- /dev/null +++ b/changelog.d/migration-pr4-stage1-staging-isolation.changed.md @@ -0,0 +1 @@ +Isolated staging Cloud Run deploys onto a dedicated policyengine-api-staging service and tagged every API response with an X-PolicyEngine-Backend header. diff --git a/policyengine_api/asgi_factory.py b/policyengine_api/asgi_factory.py index 2456ccccd..e0fd10036 100644 --- a/policyengine_api/asgi_factory.py +++ b/policyengine_api/asgi_factory.py @@ -9,6 +9,10 @@ from a2wsgi import WSGIMiddleware from fastapi import FastAPI, HTTPException from policyengine_api.constants import VERSION +from policyengine_api.migration_flags import ( + BACKEND_RESPONSE_HEADER, + get_api_host_backend, +) from policyengine_api.migration_logging import log_migration_request from pydantic import BaseModel from starlette.middleware.gzip import GZipMiddleware @@ -56,6 +60,8 @@ async def add_cors_for_native_routes(request, call_next): started_at = time.time() request_id = request.headers.get("X-Request-ID") or uuid.uuid4().hex response = await call_next(request) + if BACKEND_RESPONSE_HEADER not in response.headers: + response.headers[BACKEND_RESPONSE_HEADER] = get_api_host_backend() origin = request.headers.get("origin") if origin and "access-control-allow-origin" not in response.headers: response.headers["Access-Control-Allow-Origin"] = origin diff --git a/policyengine_api/migration_flags.py b/policyengine_api/migration_flags.py index 0d659eb36..c270fa0e8 100644 --- a/policyengine_api/migration_flags.py +++ b/policyengine_api/migration_flags.py @@ -30,6 +30,8 @@ DEFAULT_SIM_FRONT_DOOR = "old_gateway_direct" DEFAULT_SIM_COMPUTE_BACKEND = "old_gateway" +BACKEND_RESPONSE_HEADER = "X-PolicyEngine-Backend" + @dataclass(frozen=True) class MigrationContext: @@ -112,6 +114,18 @@ def get_db_read(entity: str) -> str: return _read_choice(env_name, DEFAULT_DB_SOURCE, DB_READ_SOURCES) +def get_api_host_backend() -> str: + """Backend value for response tagging; must never break a response.""" + try: + return _read_choice( + "API_HOST_BACKEND", + DEFAULT_API_HOST_BACKEND, + API_HOST_BACKENDS, + ) + except ValueError: + return DEFAULT_API_HOST_BACKEND + + def get_sim_compute(flow: str) -> str: env_name = f"SIM_COMPUTE_{flow.upper()}" return _read_choice( diff --git a/policyengine_api/migration_logging.py b/policyengine_api/migration_logging.py index 5230502d8..86e0b1be8 100644 --- a/policyengine_api/migration_logging.py +++ b/policyengine_api/migration_logging.py @@ -9,6 +9,8 @@ from policyengine_api.gcp_logging import logger from policyengine_api.migration_flags import ( + BACKEND_RESPONSE_HEADER, + get_api_host_backend, get_migration_log_context, infer_route_group, ) @@ -26,6 +28,10 @@ def set_request_migration_context(): @app.after_request def log_request_migration_context(response): + try: + response.headers[BACKEND_RESPONSE_HEADER] = get_api_host_backend() + except Exception: + pass try: log_migration_request( request_id=getattr(flask.g, "request_id", None), diff --git a/tests/unit/routes/test_migration_context_logging.py b/tests/unit/routes/test_migration_context_logging.py index 29f32a72d..9e3b9939b 100644 --- a/tests/unit/routes/test_migration_context_logging.py +++ b/tests/unit/routes/test_migration_context_logging.py @@ -4,6 +4,7 @@ from flask import Flask, Response from policyengine_api.asgi_factory import create_asgi_app +from policyengine_api.migration_flags import BACKEND_RESPONSE_HEADER from policyengine_api.migration_logging import register_migration_request_logging @@ -108,3 +109,49 @@ def test_asgi_shell_does_not_log_unregistered_flask_fallback_routes(): assert response.status_code == 200 assert response.content == b"fallback" mock_logger.log_struct.assert_not_called() + + +def test_flask_responses_include_backend_header_default(): + with patch("policyengine_api.migration_logging.logger"): + response = _app().test_client().get("/readiness-check") + + assert response.status_code == 200 + assert response.headers[BACKEND_RESPONSE_HEADER] == "app_engine" + + +def test_flask_responses_include_backend_header_for_cloud_run(monkeypatch): + monkeypatch.setenv("API_HOST_BACKEND", "cloud_run") + + with patch("policyengine_api.migration_logging.logger"): + response = _app().test_client().get("/readiness-check") + + assert response.headers[BACKEND_RESPONSE_HEADER] == "cloud_run" + + +def test_backend_header_falls_back_to_default_on_invalid_flag(monkeypatch): + monkeypatch.setenv("API_HOST_BACKEND", "not-a-backend") + + with patch("policyengine_api.migration_logging.logger"): + response = _app().test_client().get("/readiness-check") + + assert response.status_code == 200 + assert response.headers[BACKEND_RESPONSE_HEADER] == "app_engine" + + +def test_fastapi_native_routes_include_backend_header(monkeypatch): + monkeypatch.setenv("API_HOST_BACKEND", "cloud_run") + + with patch("policyengine_api.migration_logging.logger"): + response = TestClient(create_asgi_app(_app())).get("/health") + + assert response.status_code == 200 + assert response.headers[BACKEND_RESPONSE_HEADER] == "cloud_run" + + +def test_asgi_shell_adds_backend_header_when_flask_hook_is_absent(): + response = TestClient(create_asgi_app(_app_without_migration_logging())).get( + "/fallback" + ) + + assert response.status_code == 200 + assert response.headers[BACKEND_RESPONSE_HEADER] == "app_engine" diff --git a/tests/unit/test_cloud_run_deploy_scripts.py b/tests/unit/test_cloud_run_deploy_scripts.py index 73f41f81f..45a4bbc1a 100644 --- a/tests/unit/test_cloud_run_deploy_scripts.py +++ b/tests/unit/test_cloud_run_deploy_scripts.py @@ -9,6 +9,7 @@ REPO = Path(__file__).resolve().parents[2] PRODUCTION_CLOUD_SQL_INSTANCE = "policyengine-api:us-central1:policyengine-api-data" +STAGING_CLOUD_RUN_SERVICE = "policyengine-api-staging" DEDICATED_CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT = ( "policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com" ) @@ -384,6 +385,67 @@ def test_promote_cloud_run_tag_dry_run_shifts_service_traffic_to_tag(): assert "--to-latest" not in result.stdout +def test_push_workflow_isolates_staging_cloud_run_service(): + workflow = _push_workflow() + staging_deploy = _workflow_job_block(workflow, "deploy-cloud-run-staging") + staging_promote = _workflow_job_block(workflow, "promote-cloud-run-staging") + production_deploy = _workflow_job_block(workflow, "deploy-cloud-run-candidate") + + staging_service_env = f"CLOUD_RUN_SERVICE: {STAGING_CLOUD_RUN_SERVICE}" + assert staging_service_env in staging_deploy + assert staging_service_env in staging_promote + assert STAGING_CLOUD_RUN_SERVICE not in production_deploy + + +def test_build_cloud_run_image_uri_is_independent_of_service_override(): + result = _run_script( + ".github/scripts/build_cloud_run_image.sh", + _script_env( + GITHUB_SHA="1234567890abcdef", + GITHUB_RUN_NUMBER="42", + CLOUD_RUN_SERVICE=STAGING_CLOUD_RUN_SERVICE, + ), + ) + + assert result.returncode == 0, result.stderr + assert ( + "us-central1-docker.pkg.dev/policyengine-api/policyengine-api/" + "policyengine-api:1234567890abcdef" + ) in result.stdout + assert f"{STAGING_CLOUD_RUN_SERVICE}:" not in result.stdout + + +def test_deploy_cloud_run_candidate_dry_run_targets_service_override(): + result = _run_script( + ".github/scripts/deploy_cloud_run_candidate.sh", + _script_env( + **_required_runtime_env(), + CLOUD_RUN_IMAGE_URI="us-central1-docker.pkg.dev/project/repo/api:sha", + CLOUD_RUN_TAG="stage3-test", + CLOUD_RUN_SERVICE=STAGING_CLOUD_RUN_SERVICE, + ), + ) + + assert result.returncode == 0, result.stderr + assert f"gcloud run deploy {STAGING_CLOUD_RUN_SERVICE}" in result.stdout + + +def test_promote_cloud_run_tag_dry_run_targets_service_override(): + result = _run_script( + ".github/scripts/promote_cloud_run_tag.sh", + _script_env( + CLOUD_RUN_TAG="stage3-test", + CLOUD_RUN_SERVICE=STAGING_CLOUD_RUN_SERVICE, + ), + ) + + assert result.returncode == 0, result.stderr + assert ( + f"gcloud run services update-traffic {STAGING_CLOUD_RUN_SERVICE}" + in result.stdout + ) + + def test_push_workflow_tests_app_engine_and_cloud_run_staging_tracks(): workflow = _push_workflow() app_engine_tests = _workflow_job_block(workflow, "integration-tests-staging") From 7e726d5752b05b03478d3e1e0dc9008688472c8a Mon Sep 17 00:00:00 2001 From: Anthony Volk Date: Thu, 2 Jul 2026 22:24:29 +0200 Subject: [PATCH 2/6] Address Stage 1 review findings Pin CLOUD_RUN_SERVICE explicitly on the production Cloud Run job and add workflow invariants: every service-targeting Cloud Run job must pin a service, and only the production job may promote the production service. The cloud_run_env.sh default silently targeting production was the mechanism behind the original staging-promotes-to-prod bug. Drop the dead try/except around the backend header assignment; get_api_host_backend() is fail-safe by design, so the except could only mask real header-assignment bugs, and the ASGI path never had one. Document the one-time staging service bootstrap (gcloud rejects --no-traffic when creating a new service, and the deploy service account cannot set the public invoker binding) in a Stage 1 runbook. Co-Authored-By: Claude Fable 5 --- .github/workflows/push.yml | 2 + docs/migration-pr4-staging-service-runbook.md | 79 +++++++++++++++++++ policyengine_api/migration_logging.py | 5 +- tests/unit/test_cloud_run_deploy_scripts.py | 43 ++++++++++ 4 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 docs/migration-pr4-staging-service-runbook.md diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 83308d0ce..0ed07c284 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -475,6 +475,8 @@ jobs: (github.repository == 'PolicyEngine/policyengine-api') && (github.event.head_commit.message == 'Update PolicyEngine API') environment: production + env: + CLOUD_RUN_SERVICE: policyengine-api permissions: contents: read id-token: write diff --git a/docs/migration-pr4-staging-service-runbook.md b/docs/migration-pr4-staging-service-runbook.md new file mode 100644 index 000000000..d64c54ed1 --- /dev/null +++ b/docs/migration-pr4-staging-service-runbook.md @@ -0,0 +1,79 @@ +# PR 4 Stage 1 Staging Cloud Run Service Runbook + +Stage 1 of the public host cutover splits staging Cloud Run deploys onto a dedicated +`policyengine-api-staging` service. Previously, both the staging and production CI tracks +targeted the single `policyengine-api` service, and the staging promote step put a +staging-configured revision on 100% of the production service URL during every push. That +is harmless while nothing public routes to the service URL, and unacceptable once a load +balancer fronts it. + +## One-Time Bootstrap (completed 2026-07-02) + +The CI pipeline cannot create this service itself, for two reasons: + +1. `deploy_cloud_run_candidate.sh` always deploys with `--no-traffic` (the PR 3 + candidate-then-promote pattern). gcloud rejects `--no-traffic` when creating a new + service, because a new service must route 100% of traffic to its first revision. +2. The GitHub deploy service account holds `roles/run.developer`, which can create and + deploy services but cannot set IAM policy. The `allUsers` -> `roles/run.invoker` + binding that `--allow-unauthenticated` needs on a new service must be created by a + project owner or `roles/run.admin` principal. (The production service's binding was + created manually during PR 3.) + +Bootstrap command, run once by a project owner before merging the Stage 1 PR: + +```bash +gcloud run deploy policyengine-api-staging \ + --project policyengine-api \ + --region us-central1 \ + --image us-docker.pkg.dev/cloudrun/container/hello \ + --allow-unauthenticated \ + --service-account policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com +``` + +The placeholder image is intentional: the first CI staging deploy replaces the revision +entirely, since `deploy_cloud_run_candidate.sh` sets every flag (image, env vars, secrets, +resources, service account, Cloud SQL attachment) explicitly on each deploy. + +## Verification + +```bash +gcloud run services describe policyengine-api-staging \ + --project policyengine-api --region us-central1 \ + --format "value(status.url, spec.template.spec.serviceAccountName)" +gcloud run services get-iam-policy policyengine-api-staging \ + --project policyengine-api --region us-central1 +curl -s -o /dev/null -w '%{http_code}\n' +``` + +Expected: service URL resolves, runtime service account is +`policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com`, IAM policy contains +`allUsers` with `roles/run.invoker`, and the curl returns 200. + +## Steady State After Stage 1 + +- Staging jobs (`deploy-cloud-run-staging`, `promote-cloud-run-staging`) pin + `CLOUD_RUN_SERVICE: policyengine-api-staging`; the production job + (`deploy-cloud-run-candidate`) pins `CLOUD_RUN_SERVICE: policyengine-api`. Unit tests + enforce that every service-targeting Cloud Run job carries an explicit pin. +- The Docker image name is decoupled from the service name (`CLOUD_RUN_IMAGE_NAME`): + production reuses the image built by the staging track, so both tracks push and pull + `policyengine-api:` regardless of service. +- Every API response carries `X-PolicyEngine-Backend` (`app_engine` or `cloud_run`) for + in-band backend verification during the host cutover traffic ramps. +- Known follow-up (out of Stage 1 scope): the staging service still uses the prod-named + Secret Manager secrets and the production Cloud SQL instance with staging DB user/name + values, as the shared-service track always did. The service split makes a per-service + secret set possible; migrate in a later stage. + +## Exit Gates + +Evaluated on the first post-merge push cycle (see the Stage 1 section of +`api-v1-pr4-cloud-run-host-cutover-execution-plan.md` in the planning folder): + +- `gcloud run services describe policyengine-api --region us-central1` shows only + `stage3-prod-*` tags at 100% at all times, including after the staging promote step. +- `policyengine-api-staging` is healthy and passes the staging integration suite via its + own URL. +- `X-PolicyEngine-Backend` is present and correct on the App Engine production URL, the + Cloud Run staging service URL, and the Cloud Run production service URL. diff --git a/policyengine_api/migration_logging.py b/policyengine_api/migration_logging.py index 86e0b1be8..aa3c575aa 100644 --- a/policyengine_api/migration_logging.py +++ b/policyengine_api/migration_logging.py @@ -28,10 +28,7 @@ def set_request_migration_context(): @app.after_request def log_request_migration_context(response): - try: - response.headers[BACKEND_RESPONSE_HEADER] = get_api_host_backend() - except Exception: - pass + response.headers[BACKEND_RESPONSE_HEADER] = get_api_host_backend() try: log_migration_request( request_id=getattr(flask.g, "request_id", None), diff --git a/tests/unit/test_cloud_run_deploy_scripts.py b/tests/unit/test_cloud_run_deploy_scripts.py index 45a4bbc1a..dc5f0c73e 100644 --- a/tests/unit/test_cloud_run_deploy_scripts.py +++ b/tests/unit/test_cloud_run_deploy_scripts.py @@ -9,7 +9,14 @@ REPO = Path(__file__).resolve().parents[2] PRODUCTION_CLOUD_SQL_INSTANCE = "policyengine-api:us-central1:policyengine-api-data" +PRODUCTION_CLOUD_RUN_SERVICE = "policyengine-api" STAGING_CLOUD_RUN_SERVICE = "policyengine-api-staging" +CLOUD_RUN_SERVICE_SCRIPTS = ( + "scripts/deploy_cloud_run_candidate.sh", + "scripts/promote_cloud_run_tag.sh", + "scripts/get_cloud_run_tag_url.sh", + "scripts/get_cloud_run_service_url.sh", +) DEDICATED_CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT = ( "policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com" ) @@ -385,6 +392,10 @@ def test_promote_cloud_run_tag_dry_run_shifts_service_traffic_to_tag(): assert "--to-latest" not in result.stdout +def _workflow_job_names(workflow: str) -> list[str]: + return re.findall(r"^ ([a-zA-Z0-9_-]+):$", workflow, flags=re.MULTILINE) + + def test_push_workflow_isolates_staging_cloud_run_service(): workflow = _push_workflow() staging_deploy = _workflow_job_block(workflow, "deploy-cloud-run-staging") @@ -395,6 +406,38 @@ def test_push_workflow_isolates_staging_cloud_run_service(): assert staging_service_env in staging_deploy assert staging_service_env in staging_promote assert STAGING_CLOUD_RUN_SERVICE not in production_deploy + assert f"CLOUD_RUN_SERVICE: {PRODUCTION_CLOUD_RUN_SERVICE}\n" in production_deploy + + +def test_every_cloud_run_job_pins_a_service(): + workflow = _push_workflow() + + for job_name in _workflow_job_names(workflow): + block = _workflow_job_block(workflow, job_name) + if not any(script in block for script in CLOUD_RUN_SERVICE_SCRIPTS): + continue + assert re.search(r"CLOUD_RUN_SERVICE: \S+", block), ( + f"Job {job_name} uses a service-targeting Cloud Run script without " + "pinning CLOUD_RUN_SERVICE; the cloud_run_env.sh default silently " + "targets production" + ) + + +def test_only_production_job_promotes_the_production_cloud_run_service(): + workflow = _push_workflow() + + for job_name in _workflow_job_names(workflow): + block = _workflow_job_block(workflow, job_name) + if "scripts/promote_cloud_run_tag.sh" not in block: + continue + if job_name == "deploy-cloud-run-candidate": + expected = f"CLOUD_RUN_SERVICE: {PRODUCTION_CLOUD_RUN_SERVICE}\n" + else: + expected = f"CLOUD_RUN_SERVICE: {STAGING_CLOUD_RUN_SERVICE}" + assert expected in block, ( + f"Job {job_name} promotes Cloud Run traffic without pinning the " + "expected service" + ) def test_build_cloud_run_image_uri_is_independent_of_service_override(): From c13f0be6b6726a7cdca322bd69fd44c858b9f10b Mon Sep 17 00:00:00 2001 From: Anthony Volk Date: Mon, 6 Jul 2026 16:00:45 +0200 Subject: [PATCH 3/6] Enable startup CPU boost on Cloud Run candidate deploys Cold-start imports (~232s, dominated by country package construction) sit on the edge of Cloud Run's hard 240s startup probe cap, which cannot be extended by any probe configuration. Startup CPU boost shortens the import within that cap; the structural fix (binding the port before app import) follows in the next migration stage. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/scripts/deploy_cloud_run_candidate.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/scripts/deploy_cloud_run_candidate.sh b/.github/scripts/deploy_cloud_run_candidate.sh index f97dd9a29..c76388f1a 100755 --- a/.github/scripts/deploy_cloud_run_candidate.sh +++ b/.github/scripts/deploy_cloud_run_candidate.sh @@ -47,6 +47,7 @@ cloud_run_run gcloud run deploy "${CLOUD_RUN_SERVICE}" \ --add-cloudsql-instances "${CLOUD_RUN_CLOUD_SQL_INSTANCE}" \ --port "${CLOUD_RUN_PORT}" \ --cpu "${CLOUD_RUN_CPU}" \ + --cpu-boost \ --memory "${CLOUD_RUN_MEMORY}" \ --timeout "${CLOUD_RUN_TIMEOUT}" \ --min-instances "${CLOUD_RUN_MIN_INSTANCES}" \ From edaa75ce868f493a615729f71b9d87ab7d5a0c09 Mon Sep 17 00:00:00 2001 From: Anthony Volk Date: Mon, 6 Jul 2026 16:20:27 +0200 Subject: [PATCH 4/6] Bind the Cloud Run port before app import via gunicorn Cloud Run's startup probe hard-caps total probe time at 240s, and the app import (~232s, dominated by country package construction) races it on every instance start, failing roughly half of first-attempt deploys. gunicorn's master binds the listen socket before forking workers, and without --preload the import happens in the worker post-fork, so the probe passes in milliseconds and readiness is governed by the pipeline's health_check.sh /readiness-check poll (900s budget) instead of the platform cap. --timeout 0 disables the worker heartbeat watchdog that would otherwise kill the worker mid-import; --keep-alive 5 and --forwarded-allow-ips preserve today's uvicorn keep-alive and proxy header behavior (UvicornWorker leaves proxy_headers at uvicorn's default of enabled). health_check.sh curls now carry --max-time (default 10s, overridable via HEALTH_CHECK_CURL_MAX_TIME_SECONDS): with the port open before the app can answer, an unanswered poll would otherwise block until Cloud Run's 300s request timeout. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/scripts/health_check.sh | 5 +++- gcp/cloud_run/start.sh | 32 ++++++++++++--------- tests/unit/test_cloud_run_deploy_scripts.py | 10 +++---- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/.github/scripts/health_check.sh b/.github/scripts/health_check.sh index b61e9f470..2c21059e1 100644 --- a/.github/scripts/health_check.sh +++ b/.github/scripts/health_check.sh @@ -5,10 +5,13 @@ set -euo pipefail health_url="${1:?health check URL is required}" timeout_seconds="${HEALTH_CHECK_TIMEOUT_SECONDS:-900}" interval_seconds="${HEALTH_CHECK_INTERVAL_SECONDS:-10}" +curl_max_time_seconds="${HEALTH_CHECK_CURL_MAX_TIME_SECONDS:-10}" deadline=$((SECONDS + timeout_seconds)) while (( SECONDS < deadline )); do - if curl --silent --show-error --fail "${health_url}" >/dev/null; then + if curl --silent --show-error --fail \ + --max-time "${curl_max_time_seconds}" \ + "${health_url}" >/dev/null; then exit 0 fi sleep "${interval_seconds}" diff --git a/gcp/cloud_run/start.sh b/gcp/cloud_run/start.sh index 35a27a455..31e2b188b 100755 --- a/gcp/cloud_run/start.sh +++ b/gcp/cloud_run/start.sh @@ -10,21 +10,21 @@ REDIS_READY_MAX_ATTEMPTS="${REDIS_READY_MAX_ATTEMPTS:-30}" export CACHE_REDIS_HOST CACHE_REDIS_PORT CACHE_REDIS_DB redis_pid="" -uvicorn_pid="" +server_pid="" shutdown() { trap - INT TERM - if [ -n "$uvicorn_pid" ] && kill -0 "$uvicorn_pid" 2>/dev/null; then - kill "$uvicorn_pid" 2>/dev/null || true + if [ -n "$server_pid" ] && kill -0 "$server_pid" 2>/dev/null; then + kill "$server_pid" 2>/dev/null || true fi if [ -n "$redis_pid" ] && kill -0 "$redis_pid" 2>/dev/null; then kill "$redis_pid" 2>/dev/null || true fi - if [ -n "$uvicorn_pid" ]; then - wait "$uvicorn_pid" 2>/dev/null || true + if [ -n "$server_pid" ]; then + wait "$server_pid" 2>/dev/null || true fi if [ -n "$redis_pid" ]; then @@ -58,23 +58,29 @@ until redis-cli -h "$CACHE_REDIS_HOST" -p "$CACHE_REDIS_PORT" ping >/dev/null 2> sleep 1 done -uvicorn policyengine_api.asgi:app \ - --host 0.0.0.0 \ - --port "$PORT" \ +# gunicorn's master binds the listen socket before forking workers, so the +# Cloud Run TCP startup probe passes immediately instead of racing the +# multi-minute app import (which happens in the worker, post-fork, because +# --preload is NOT set). --timeout 0 is required: a worker mid-import does +# not heartbeat, and the default 30s watchdog would kill it before boot. +gunicorn policyengine_api.asgi:app \ + --worker-class uvicorn.workers.UvicornWorker \ --workers "$WEB_CONCURRENCY" \ - --proxy-headers \ + --bind "0.0.0.0:${PORT}" \ + --timeout 0 \ + --keep-alive 5 \ --forwarded-allow-ips '*' & -uvicorn_pid="$!" +server_pid="$!" set +e -wait -n "$redis_pid" "$uvicorn_pid" +wait -n "$redis_pid" "$server_pid" status="$?" set -e if ! kill -0 "$redis_pid" 2>/dev/null; then echo "Redis exited; stopping Cloud Run container" >&2 -elif ! kill -0 "$uvicorn_pid" 2>/dev/null; then - echo "Uvicorn exited; stopping Cloud Run container" >&2 +elif ! kill -0 "$server_pid" 2>/dev/null; then + echo "API server exited; stopping Cloud Run container" >&2 else echo "A supervised Cloud Run process exited; stopping container" >&2 fi diff --git a/tests/unit/test_cloud_run_deploy_scripts.py b/tests/unit/test_cloud_run_deploy_scripts.py index dc5f0c73e..2fe65f318 100644 --- a/tests/unit/test_cloud_run_deploy_scripts.py +++ b/tests/unit/test_cloud_run_deploy_scripts.py @@ -270,20 +270,20 @@ def test_cloud_run_dockerfile_runs_startup_with_bash(): assert 'CMD ["/bin/sh", "/app/start.sh"]' not in dockerfile -def test_cloud_run_startup_supervises_redis_and_uvicorn_children(): +def test_cloud_run_startup_supervises_redis_and_server_children(): start_script = (REPO / "gcp/cloud_run/start.sh").read_text(encoding="utf-8") assert "#!/usr/bin/env bash" in start_script assert 'redis_pid="$!"' in start_script - assert 'uvicorn_pid="$!"' in start_script + assert 'server_pid="$!"' in start_script assert "REDIS_READY_MAX_ATTEMPTS" in start_script assert "Redis exited before becoming ready" in start_script assert "Redis did not become ready" in start_script assert "Redis exited; stopping Cloud Run container" in start_script - assert "Uvicorn exited; stopping Cloud Run container" in start_script - assert 'wait -n "$redis_pid" "$uvicorn_pid"' in start_script + assert "API server exited; stopping Cloud Run container" in start_script + assert 'wait -n "$redis_pid" "$server_pid"' in start_script assert 'kill -0 "$redis_pid"' in start_script - assert 'kill -0 "$uvicorn_pid"' in start_script + assert 'kill -0 "$server_pid"' in start_script assert "trap 'shutdown; exit 143' INT TERM" in start_script assert "pkill" not in start_script assert re.search(r"(?m)^ *wait 2>/dev/null", start_script) is None From f0618237b18c75783f5bb67c82d6a851affbac06 Mon Sep 17 00:00:00 2001 From: Anthony Volk Date: Mon, 6 Jul 2026 16:27:13 +0200 Subject: [PATCH 5/6] Tune health check cadence: 15s request timeout, 5s poll interval Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/scripts/health_check.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/scripts/health_check.sh b/.github/scripts/health_check.sh index 2c21059e1..41411b365 100644 --- a/.github/scripts/health_check.sh +++ b/.github/scripts/health_check.sh @@ -4,8 +4,8 @@ set -euo pipefail health_url="${1:?health check URL is required}" timeout_seconds="${HEALTH_CHECK_TIMEOUT_SECONDS:-900}" -interval_seconds="${HEALTH_CHECK_INTERVAL_SECONDS:-10}" -curl_max_time_seconds="${HEALTH_CHECK_CURL_MAX_TIME_SECONDS:-10}" +interval_seconds="${HEALTH_CHECK_INTERVAL_SECONDS:-5}" +curl_max_time_seconds="${HEALTH_CHECK_CURL_MAX_TIME_SECONDS:-15}" deadline=$((SECONDS + timeout_seconds)) while (( SECONDS < deadline )); do From 0839a968b81c03fbe3a2a545d94977a591a50591 Mon Sep 17 00:00:00 2001 From: Anthony Volk Date: Mon, 6 Jul 2026 18:56:11 +0200 Subject: [PATCH 6/6] Restructure migration runbooks: living ops doc + append-only history docs/ was accumulating per-PR runbooks that interleave one-time records with operational reference, and the PR 3 runbook described the shared single-service behavior this PR removes. Split the two concerns: docs/migration/cloud-run-operations.md is the single living reference (topology, startup behavior, IAM constraints, verification commands), edited in place as behavior changes; per-stage records move to docs/migration/history/ as append-only documents with banners pointing at the living doc. The Stage 1 record now covers the startup de-flake items and carries the full amended exit-gate list. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/migration/cloud-run-operations.md | 98 +++++++++++++++++++ .../migration-pr1-baseline-runbook.md | 3 + .../migration-pr2-fastapi-shell-runbook.md | 3 + ...gration-pr3-cloud-run-candidate-runbook.md | 3 + .../pr4-stage1-staging-service-runbook.md} | 36 ++++--- 5 files changed, 128 insertions(+), 15 deletions(-) create mode 100644 docs/migration/cloud-run-operations.md rename docs/{ => migration/history}/migration-pr1-baseline-runbook.md (91%) rename docs/{ => migration/history}/migration-pr2-fastapi-shell-runbook.md (86%) rename docs/{ => migration/history}/migration-pr3-cloud-run-candidate-runbook.md (97%) rename docs/{migration-pr4-staging-service-runbook.md => migration/history/pr4-stage1-staging-service-runbook.md} (67%) diff --git a/docs/migration/cloud-run-operations.md b/docs/migration/cloud-run-operations.md new file mode 100644 index 000000000..5472e08c0 --- /dev/null +++ b/docs/migration/cloud-run-operations.md @@ -0,0 +1,98 @@ +# Cloud Run Operations (living reference) + +This is the single living reference for how API v1's Cloud Run deployments work. It is +edited in place whenever behavior changes. Per-stage records (bootstrap commands, +exit-gate evidence, one-off measurements) are append-only documents under `history/` and +are never updated to match later reality. + +Configuration **values** (CPU, memory, scaling, secrets, service names) live in +`.github/scripts/cloud_run_env.sh` and the deploy scripts — they are the source of truth +and are intentionally not duplicated here. This document explains the **semantics**. + +## Topology + +- The public host `api.policyengine.org` is served by **App Engine** (bound via + `gcp/dispatch.yaml`). No public traffic reaches Cloud Run until the load-balancer + stages of the host cutover plan (`api-v1-pr4-cloud-run-host-cutover-execution-plan.md` + in the planning folder). +- Two Cloud Run services in `policyengine-api` / `us-central1`: + - **`policyengine-api`** — the production candidate. Every push to master deploys a + tagged `--no-traffic` revision (`stage3-prod-*`), which is promoted to the service + URL after integration tests. Only CI and its own health checks call this URL today. + - **`policyengine-api-staging`** — the staging track's service (split from the + production service in migration PR 4, Stage 1). Staging jobs pin + `CLOUD_RUN_SERVICE: policyengine-api-staging`; unit tests enforce that every + service-targeting Cloud Run job carries an explicit pin and that no staging job can + touch production traffic. +- The Docker image name is decoupled from the service name (`CLOUD_RUN_IMAGE_NAME`): + production reuses the image built by the staging track, so both tracks push and pull + `policyengine-api:` regardless of service. +- Every API response carries `X-PolicyEngine-Backend` (`app_engine` or `cloud_run`) for + in-band backend verification during the host-cutover traffic ramps. + +## Startup behavior + +The app import is slow (~230s cold: `policyengine_api/country.py` instantiates five +country packages, ~210s of it) and Cloud Run hard-caps total startup-probe time at 240s — +the default TCP probe already sits at that cap, and **no probe configuration can extend +it**. Two mitigations are therefore built in: + +- `gcp/cloud_run/start.sh` runs **gunicorn with `uvicorn.workers.UvicornWorker` and no + `--preload`**: the gunicorn master binds the listen socket in milliseconds (satisfying + the startup probe immediately), and the import happens in the worker after fork. + `--timeout 0` is required — a worker mid-import does not heartbeat, and gunicorn's + default 30s watchdog would kill it. `--keep-alive 5` and `--forwarded-allow-ips '*'` + preserve the previous uvicorn keep-alive and proxy-header behavior. +- Candidate deploys set `--cpu-boost` to shorten the import. + +Consequences to keep in mind: + +- An instance is "started" (port bound) before the app can answer. Requests arriving in + that window queue until the worker is ready or Cloud Run's request timeout fires. This + is acceptable while the services carry no public traffic; the min-instances decision in + the cutover plan's Stage 3 keeps cold imports off the request path before traffic ramps. +- **Cold-start and readiness measurements must be taken from `/readiness-check` turning + healthy**, not from instance start or port bind — the bind time is no longer meaningful. +- Readiness in CI is governed by `.github/scripts/health_check.sh` polling + `/readiness-check` (defaults: 900s budget, 5s interval, 15s per-request `--max-time`; + all env-overridable). + +## IAM and bootstrap constraints + +- The GitHub deploy service account holds `roles/run.developer`: it can deploy to + existing services but cannot create the `allUsers` → `roles/run.invoker` binding a new + service needs, and `deploy_cloud_run_candidate.sh` always deploys `--no-traffic`, which + gcloud rejects on service creation. **New Cloud Run services are therefore bootstrapped + manually by a project owner** (placeholder image + IAM binding + runtime service + account); the first CI deploy replaces the revision entirely. See + `history/pr4-stage1-staging-service-runbook.md` for the pattern. +- Both services currently run as the dedicated runtime service account and share the + prod-named Secret Manager secrets and the production Cloud SQL instance. Known + follow-up: per-service secrets became possible once the services split; migrate in a + later stage. + +## Verification commands + +```bash +# Service state, runtime SA, traffic tags +gcloud run services describe policyengine-api \ + --project policyengine-api --region us-central1 \ + --format 'value(status.url, spec.template.spec.serviceAccountName, status.traffic)' + +# IAM (expect allUsers -> roles/run.invoker) +gcloud run services get-iam-policy policyengine-api-staging \ + --project policyengine-api --region us-central1 + +# Readiness and backend identification +curl -s -o /dev/null -w '%{http_code}\n' "$SERVICE_URL/readiness-check" +curl -sI "$SERVICE_URL/readiness-check" | grep -i x-policyengine-backend +``` + +## History index + +- `history/migration-pr1-baseline-runbook.md` — PR 1 baseline capture +- `history/migration-pr2-fastapi-shell-runbook.md` — PR 2 FastAPI/ASGI shell +- `history/migration-pr3-cloud-run-candidate-runbook.md` — PR 3 candidate/promote + pipeline (documents the pre-Stage-1 single-service behavior) +- `history/pr4-stage1-staging-service-runbook.md` — PR 4 Stage 1 staging service split, + startup de-flake, and exit gates diff --git a/docs/migration-pr1-baseline-runbook.md b/docs/migration/history/migration-pr1-baseline-runbook.md similarity index 91% rename from docs/migration-pr1-baseline-runbook.md rename to docs/migration/history/migration-pr1-baseline-runbook.md index d9255a221..11dfa6674 100644 --- a/docs/migration-pr1-baseline-runbook.md +++ b/docs/migration/history/migration-pr1-baseline-runbook.md @@ -1,5 +1,8 @@ # PR 1 Migration Baseline Runbook +> Historical record — describes the pipeline as it was when this PR landed. +> Current behavior: see [`../cloud-run-operations.md`](../cloud-run-operations.md). + PR 1 does not shift traffic or change API behavior. Before PR 2 starts, capture baseline production or staging metrics so later cutovers can compare against the same surface. diff --git a/docs/migration-pr2-fastapi-shell-runbook.md b/docs/migration/history/migration-pr2-fastapi-shell-runbook.md similarity index 86% rename from docs/migration-pr2-fastapi-shell-runbook.md rename to docs/migration/history/migration-pr2-fastapi-shell-runbook.md index 8a5a8d6ed..18ed2979f 100644 --- a/docs/migration-pr2-fastapi-shell-runbook.md +++ b/docs/migration/history/migration-pr2-fastapi-shell-runbook.md @@ -1,5 +1,8 @@ # PR 2 FastAPI Shell Runbook +> Historical record — describes the pipeline as it was when this PR landed. +> Current behavior: see [`../cloud-run-operations.md`](../cloud-run-operations.md). + PR 2 adds an ASGI FastAPI shell around the existing Flask API. It is a compatibility step only. diff --git a/docs/migration-pr3-cloud-run-candidate-runbook.md b/docs/migration/history/migration-pr3-cloud-run-candidate-runbook.md similarity index 97% rename from docs/migration-pr3-cloud-run-candidate-runbook.md rename to docs/migration/history/migration-pr3-cloud-run-candidate-runbook.md index 9aac27f3c..5bc8f7472 100644 --- a/docs/migration-pr3-cloud-run-candidate-runbook.md +++ b/docs/migration/history/migration-pr3-cloud-run-candidate-runbook.md @@ -1,5 +1,8 @@ # PR 3 Cloud Run Candidate Runbook +> Historical record — describes the pipeline as it was when this PR landed. +> Current behavior: see [`../cloud-run-operations.md`](../cloud-run-operations.md). + PR 3 adds a production-configured Cloud Run candidate for the FastAPI ASGI shell. It makes the Cloud Run service URL live after staged validation, but it does not migrate the public App Engine/custom API URL. diff --git a/docs/migration-pr4-staging-service-runbook.md b/docs/migration/history/pr4-stage1-staging-service-runbook.md similarity index 67% rename from docs/migration-pr4-staging-service-runbook.md rename to docs/migration/history/pr4-stage1-staging-service-runbook.md index d64c54ed1..1bd35e575 100644 --- a/docs/migration-pr4-staging-service-runbook.md +++ b/docs/migration/history/pr4-stage1-staging-service-runbook.md @@ -1,4 +1,7 @@ -# PR 4 Stage 1 Staging Cloud Run Service Runbook +# PR 4 Stage 1 Runbook: Staging Service Split + Startup De-flake + +> Per-stage record. Current behavior: +> see [`../cloud-run-operations.md`](../cloud-run-operations.md). Stage 1 of the public host cutover splits staging Cloud Run deploys onto a dedicated `policyengine-api-staging` service. Previously, both the staging and production CI tracks @@ -7,6 +10,14 @@ staging-configured revision on 100% of the production service URL during every p is harmless while nothing public routes to the service URL, and unacceptable once a load balancer fronts it. +Stage 1 also de-flakes candidate startup (plan items 3–5): `--cpu-boost` on candidate +deploys, a gunicorn entrypoint that binds the port before the multi-minute app import +(Cloud Run hard-caps startup-probe time at 240s, which the import raced and lost on +roughly half of first attempts), and `--max-time`/interval tuning on `health_check.sh`. +Mechanics and rationale live in the operations doc; the evidence (failing runs, revision +logs, the 240s platform cap) is recorded in the Stage 1 section of +`api-v1-pr4-cloud-run-host-cutover-execution-plan.md` in the planning folder. + ## One-Time Bootstrap (completed 2026-07-02) The CI pipeline cannot create this service itself, for two reasons: @@ -52,23 +63,13 @@ Expected: service URL resolves, runtime service account is ## Steady State After Stage 1 -- Staging jobs (`deploy-cloud-run-staging`, `promote-cloud-run-staging`) pin - `CLOUD_RUN_SERVICE: policyengine-api-staging`; the production job - (`deploy-cloud-run-candidate`) pins `CLOUD_RUN_SERVICE: policyengine-api`. Unit tests - enforce that every service-targeting Cloud Run job carries an explicit pin. -- The Docker image name is decoupled from the service name (`CLOUD_RUN_IMAGE_NAME`): - production reuses the image built by the staging track, so both tracks push and pull - `policyengine-api:` regardless of service. -- Every API response carries `X-PolicyEngine-Backend` (`app_engine` or `cloud_run`) for - in-band backend verification during the host cutover traffic ramps. -- Known follow-up (out of Stage 1 scope): the staging service still uses the prod-named - Secret Manager secrets and the production Cloud SQL instance with staging DB user/name - values, as the shared-service track always did. The service split makes a per-service - secret set possible; migrate in a later stage. +Described in [`../cloud-run-operations.md`](../cloud-run-operations.md) (topology, +startup behavior, IAM/secrets follow-ups) — that document, not this record, tracks +current reality. ## Exit Gates -Evaluated on the first post-merge push cycle (see the Stage 1 section of +Evaluated on the first post-merge push cycle (source of truth: the Stage 1 section of `api-v1-pr4-cloud-run-host-cutover-execution-plan.md` in the planning folder): - `gcloud run services describe policyengine-api --region us-central1` shows only @@ -77,3 +78,8 @@ Evaluated on the first post-merge push cycle (see the Stage 1 section of own URL. - `X-PolicyEngine-Backend` is present and correct on the App Engine production URL, the Cloud Run staging service URL, and the Cloud Run production service URL. +- New revisions show startup CPU boost in `gcloud run services describe`; every Cloud Run + deploy job (staging + prod candidate) green on first attempt. +- Revision logs show the port bound within ~15s of instance start and `/readiness-check` + healthy within the `health_check.sh` budget; the container-start → ready duration is + recorded as an input to Stage 2's timing qualification.