Skip to content

Commit 444acbf

Browse files
anth-volkclaude
andcommitted
Pause the safety check rather than run an image without it
From the extra review of the reworked 1.06: - Pointing the job at a revision built from 1.04 or 1.05 deployed it: such a revision mounts the database URL and, with Stage 12, the Modal token, but its image has no safety_check module, so every execution would have failed. A revision now needs SIMULATION_CALL_REPORT_MODAL_WORKSPACE_ID, which every deploy has required since the safety check exists; otherwise the schedule is paused. - A failed step in the rollback workflow left the job on the image traffic had just moved away from. The script now takes `pause`, which only pauses the schedule; the rollback workflow runs it when pointing the job at the restored revision fails, and the promotion job when pointing it back at the previous revision fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 034705b commit 444acbf

5 files changed

Lines changed: 127 additions & 19 deletions

File tree

‎.github/scripts/deploy-cloud-run-simulation-safety-check.sh‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,50 +7,74 @@
77
# promoted (or restored by a rollback), so a rejected candidate never records
88
# anything. It copies the revision's image digest, service account, Modal
99
# token and database URL (at the secret versions the revision pins) and its
10-
# plain telemetry settings; nothing else. A revision without the simulation
11-
# database URL predates the safety check, so its schedule is paused.
10+
# plain telemetry settings; nothing else. A revision deployed before the
11+
# safety check existed has no job code, so its schedule is paused instead.
1212
#
13-
# Environment: PROJECT_ID, REGION, SERVICE, SAFETY_CHECK_REVISION,
14-
# SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT, RUNNER_TEMP.
13+
# With the argument `pause`, only pause the schedule: a workflow does this
14+
# when pointing the job at a revision failed part way, so the job never keeps
15+
# running the image traffic just moved away from.
16+
#
17+
# Environment: PROJECT_ID, REGION, SERVICE, and unless pausing,
18+
# SAFETY_CHECK_REVISION, SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT, RUNNER_TEMP.
1519
set -euo pipefail
1620

1721
fail() {
1822
printf '%s\n' "$*" >&2
1923
exit 1
2024
}
2125

26+
pause_schedule() {
27+
if gcloud scheduler jobs describe "${job}" "${scheduler_location[@]}" >/dev/null 2>&1; then
28+
gcloud scheduler jobs pause "${job}" "${scheduler_location[@]}" --quiet
29+
fi
30+
}
31+
2232
# Every five minutes: SAFETY_CHECK_INTERVAL in
2333
# policyengine_simulation_entry.run_recording.
2434
SCHEDULE="*/5 * * * *"
2535
SECRETS=(SIMULATION_DATABASE_URL MODAL_TOKEN_ID MODAL_TOKEN_SECRET)
2636
PLAIN_VARIABLES_FILTER='^(APP_ENVIRONMENT|CLOUD_RUN_REGION|OBSERVABILITY_[A-Z_]+|OTEL_[A-Z_]+|POLICYENGINE_OTEL_GOOGLE_AUDIENCE)$'
2737

28-
for variable_name in PROJECT_ID REGION SERVICE SAFETY_CHECK_REVISION \
29-
SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT RUNNER_TEMP; do
38+
for variable_name in PROJECT_ID REGION SERVICE; do
3039
[[ -n "${!variable_name:-}" ]] || fail "${variable_name} is required"
3140
done
3241
[[ "${PROJECT_ID}" =~ ^[a-z][a-z0-9-]{4,28}[a-z0-9]$ ]] || fail "PROJECT_ID is invalid"
3342
[[ "${REGION}" =~ ^[a-z]+-[a-z]+[0-9]+$ ]] || fail "REGION is invalid"
3443
[[ "${SERVICE}" =~ ^[a-z]([a-z0-9-]{0,47}[a-z0-9])?$ ]] || fail "SERVICE is invalid"
35-
[[ "${SAFETY_CHECK_REVISION}" =~ ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$ ]] \
36-
|| fail "SAFETY_CHECK_REVISION is invalid"
37-
[[ "${SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT}" =~ ^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.gserviceaccount\.com$ ]] \
38-
|| fail "SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT is invalid"
3944

4045
job="${SERVICE}-safety-check"
4146
location=(--project "${PROJECT_ID}" --region "${REGION}")
4247
scheduler_location=(--project "${PROJECT_ID}" --location "${REGION}")
4348

49+
if [[ "${1:-}" == "pause" ]]; then
50+
pause_schedule
51+
printf 'The safety check schedule for %s is paused.\n' "${SERVICE}"
52+
exit 0
53+
fi
54+
55+
for variable_name in SAFETY_CHECK_REVISION SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT RUNNER_TEMP; do
56+
[[ -n "${!variable_name:-}" ]] || fail "${variable_name} is required"
57+
done
58+
[[ "${SAFETY_CHECK_REVISION}" =~ ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$ ]] \
59+
|| fail "SAFETY_CHECK_REVISION is invalid"
60+
[[ "${SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT}" =~ ^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.gserviceaccount\.com$ ]] \
61+
|| fail "SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT is invalid"
62+
4463
revision_json="$(gcloud run revisions describe "${SAFETY_CHECK_REVISION}" "${location[@]}" --format=json)"
4564
revision_service="$(jq -r '.metadata.labels["serving.knative.dev/service"] // ""' <<<"${revision_json}")"
4665
[[ "${revision_service}" == "${SERVICE}" ]] \
4766
|| fail "${SAFETY_CHECK_REVISION} belongs to ${revision_service:-no service}, not ${SERVICE}"
4867

49-
if ! jq -e '.spec.containers[0].env // [] | any(.name == "SIMULATION_DATABASE_URL")' >/dev/null <<<"${revision_json}"; then
50-
if gcloud scheduler jobs describe "${job}" "${scheduler_location[@]}" >/dev/null 2>&1; then
51-
gcloud scheduler jobs pause "${job}" "${scheduler_location[@]}" --quiet
52-
fi
53-
printf '%s has no simulation database, so the safety check is paused.\n' "${SAFETY_CHECK_REVISION}"
68+
# Every revision deployed with the safety check has the simulation database
69+
# and the expected Modal caller (SIMULATION_CALL_REPORT_MODAL_WORKSPACE_ID,
70+
# which the deploy has required since then); an older one has no job code.
71+
if ! jq -e '
72+
.spec.containers[0].env // []
73+
| any(.name == "SIMULATION_DATABASE_URL")
74+
and any(.name == "SIMULATION_CALL_REPORT_MODAL_WORKSPACE_ID")
75+
' >/dev/null <<<"${revision_json}"; then
76+
pause_schedule
77+
printf '%s predates the safety check, so its schedule is paused.\n' "${SAFETY_CHECK_REVISION}"
5478
exit 0
5579
fi
5680

‎.github/workflows/simulation-deploy.reusable.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,12 +1016,17 @@ jobs:
10161016
run: .github/scripts/set-cloud-run-simulation-entry-revision.sh
10171017

10181018
- name: Point the safety check back at the previous revision
1019+
id: safety_check_restore
10191020
if: ${{ failure() && steps.safety_check.outcome == 'failure' && env.PREVIOUS_REVISION != '' }}
10201021
env:
10211022
SAFETY_CHECK_REVISION: ${{ env.PREVIOUS_REVISION }}
10221023
SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT: ${{ vars.SIMULATION_SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT }}
10231024
run: .github/scripts/deploy-cloud-run-simulation-safety-check.sh
10241025

1026+
- name: Pause the safety check after a failed restore
1027+
if: ${{ failure() && steps.safety_check_restore.outcome == 'failure' }}
1028+
run: .github/scripts/deploy-cloud-run-simulation-safety-check.sh pause
1029+
10251030
finalize_routing:
10261031
name: Finalize the Modal routing publication
10271032
# Finish the durable journal after every gate, including failed uploads

‎.github/workflows/simulation-rollback.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,12 +217,18 @@ jobs:
217217
fi
218218
219219
- name: Point the safety check at the restored revision
220+
id: safety_check
220221
if: ${{ inputs.entrypoint_revision != '' }}
221222
env:
222223
SAFETY_CHECK_REVISION: ${{ inputs.entrypoint_revision }}
223224
SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT: ${{ vars.SIMULATION_SAFETY_CHECK_SCHEDULER_SERVICE_ACCOUNT }}
224225
run: .github/scripts/deploy-cloud-run-simulation-safety-check.sh
225226

227+
# The job must not keep running the image traffic just moved away from.
228+
- name: Pause the safety check after a failed update
229+
if: ${{ failure() && steps.safety_check.outcome == 'failure' }}
230+
run: .github/scripts/deploy-cloud-run-simulation-safety-check.sh pause
231+
226232
- name: Verify the stable service serves the restored release
227233
if: ${{ !inputs.resolve_pending_publication }}
228234
env:

‎projects/policyengine-simulation-entry/DEPLOYMENT.md‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,14 @@ records anything:
112112

113113
- the promotion job, after the promoted revision passes every check; if this
114114
step fails, the previous revision is restored and the job is pointed back
115-
at it;
116-
- the rollback workflow, after it restores entrypoint traffic. A restored
117-
revision without the simulation database predates the safety check, so the
118-
schedule is paused.
115+
at it, and if that fails too, the schedule is paused;
116+
- the rollback workflow, after it restores entrypoint traffic; if this step
117+
fails, the schedule is paused, so the job never keeps running the image
118+
traffic just moved away from.
119+
120+
A revision deployed before the safety check existed (without
121+
`SIMULATION_CALL_REPORT_MODAL_WORKSPACE_ID`) has no job code, so pointing the
122+
job at it pauses the schedule instead.
119123

120124
`beta` does not promote its candidate yet, so staging has no safety check
121125
until it does (plan item 1.39).

‎projects/policyengine-simulation-entry/tests/test_safety_check_deployment.py‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ def _revision(*, service=SERVICE, environment=None):
5656
if environment is not None
5757
else [
5858
{"name": "APP_ENVIRONMENT", "value": "production"},
59+
{
60+
"name": "SIMULATION_CALL_REPORT_MODAL_WORKSPACE_ID",
61+
"value": "ac-workspace0001",
62+
},
5963
{"name": "OLD_GATEWAY_URL", "value": "https://legacy.example"},
6064
{"name": "OBSERVABILITY_SERVICE_NAMESPACE", "value": "sim"},
6165
{"name": "CLOUD_RUN_REGION", "value": "us-central1"},
@@ -286,3 +290,68 @@ def test_a_rollback_points_the_job_at_the_restored_revision() -> None:
286290
assert steps[safety]["env"]["SAFETY_CHECK_REVISION"] == (
287291
"${{ inputs.entrypoint_revision }}"
288292
)
293+
294+
295+
def test_a_revision_from_before_the_safety_check_pauses_the_schedule(tmp_path) -> None:
296+
"""A 1.04 or 1.05 revision mounts the database and token but has no job code."""
297+
environment = [
298+
entry
299+
for entry in _revision()["spec"]["containers"][0]["env"]
300+
if entry["name"] != "SIMULATION_CALL_REPORT_MODAL_WORKSPACE_ID"
301+
]
302+
303+
result, fakes, _ = _deploy(
304+
tmp_path, _revision(environment=environment), schedule_exists=True
305+
)
306+
307+
assert result.returncode == 0, result.stderr
308+
assert [call[1:4] for call in fakes.calls("gcloud")] == [
309+
["run", "revisions", "describe"],
310+
["scheduler", "jobs", "describe"],
311+
["scheduler", "jobs", "pause"],
312+
]
313+
314+
315+
@pytest.mark.parametrize("schedule_exists", [True, False])
316+
def test_pausing_stops_the_schedule_without_a_revision(
317+
tmp_path, schedule_exists
318+
) -> None:
319+
fakes, _ = _fakes(tmp_path, _revision(), schedule_exists=schedule_exists)
320+
settings = {
321+
"PROJECT_ID": "policyengine",
322+
"REGION": "us-central1",
323+
"SERVICE": SERVICE,
324+
}
325+
326+
result = run_script(SCRIPT, "pause", env=fakes.env(**settings))
327+
328+
assert result.returncode == 0, result.stderr
329+
expected = [["scheduler", "jobs", "describe"]]
330+
if schedule_exists:
331+
expected.append(["scheduler", "jobs", "pause"])
332+
assert [call[1:4] for call in fakes.calls("gcloud")] == expected
333+
334+
335+
def test_a_failed_update_in_a_rollback_pauses_the_safety_check() -> None:
336+
"""The job must not keep running the image traffic just moved away from."""
337+
steps = _steps(_workflow_jobs(ROLLBACK_WORKFLOW)["rollback"])
338+
safety = _step_named(steps, "Point the safety check at the restored revision")
339+
pause = _step_named(steps, "Pause the safety check after a failed update")
340+
341+
assert safety < pause
342+
assert steps[safety]["id"] == "safety_check"
343+
assert steps[pause]["if"] == (
344+
"${{ failure() && steps.safety_check.outcome == 'failure' }}"
345+
)
346+
assert steps[pause]["run"] == f".github/scripts/{SCRIPT} pause"
347+
348+
349+
def test_a_failed_restore_in_a_promotion_pauses_the_safety_check() -> None:
350+
steps = _steps(_workflow_jobs(REUSABLE_DEPLOY_WORKFLOW)["promote_entrypoint"])
351+
restore = _step_named(steps, "Point the safety check back at the previous revision")
352+
pause = _step_named(steps, "Pause the safety check after a failed restore")
353+
354+
assert restore < pause
355+
assert steps[restore]["id"] == "safety_check_restore"
356+
assert "steps.safety_check_restore.outcome == 'failure'" in steps[pause]["if"]
357+
assert steps[pause]["run"] == f".github/scripts/{SCRIPT} pause"

0 commit comments

Comments
 (0)