fix(relay): keep the backend service out of the same-cap wave; the capacity role cannot update it (#22140)

The capacity role the same-cap wave authenticates as, orcaRelayProductionCapacity,
has no compute.backendServices.update. Since #21860 added
`google_compute_backend_service.relay_gce_cell["${TARGET_CELL_ID}"]` to both of the
job's plan invocations, every wave has therefore created the new instance template,
modified the MIG, and then failed 403 on the backend, leaving the cell isolated with
its trust probe, admission restore, and shadow gate all skipped. Run 35684694704 on
production-gce-c7 is the first one that hit it in production.

Drop the backend target from both plans and restore the resume gate to exactly
`.changes == 2` (the template-and-MIG rollback-image drift) or a converged plan,
removing the backend-only resume apply #21865 added on top. A resume applies nothing
again, which is what a resume means.

The validator keeps its bound on a cell backend update, so it still reports one and
refuses anything wider, but a wave plan can no longer contain one. The drain timeout
from #21848 and the log_config from #21860 need a root apply by a principal that holds
the permission; granting the capacity role that permission is itself a root apply, so
it can follow as its own change rather than blocking every wave in the meantime.

Claude-Session: ced32ebb-7155-4413-adad-1eccd14c2010
This commit is contained in:
Jinwoo Hong
2026-09-22 02:50:32 -04:00
committed by GitHub
parent 895f2cf477
commit bf8d63bc67
4 changed files with 41 additions and 75 deletions
@@ -598,16 +598,15 @@ jobs:
# Zero resource changes prove the prior run's apply completed and no
# restart will follow, keeping the incarnation check honest. Root
# outputs may lag a targeted apply, so judge resource_changes only.
# The backend service is targeted too, so its reviewed drain timeout
# and request logging can be the only thing left here; neither
# restarts an instance, so the validator below clears that on its
# own, without the template-and-MIG pair.
# The cell backend service is deliberately not targeted: the capacity role has no
# compute.backendServices.update, so naming it fails the apply after the MIG has
# rolled. Its drain and log_config settings ship in a root apply by a principal
# that holds the permission.
terraform -chdir=infra/terraform plan \
-var-file=environments/production.tfvars \
-var-file="${RUNNER_TEMP}/relay-same-cap.tfvars.json" \
"-target=google_compute_instance_template.relay_gce_cell[\"${TARGET_CELL_ID}\"]" \
"-target=google_compute_instance_group_manager.relay_gce_cell[\"${TARGET_CELL_ID}\"]" \
"-target=google_compute_backend_service.relay_gce_cell[\"${TARGET_CELL_ID}\"]" \
-out="${RUNNER_TEMP}/relay-same-cap-resume.tfplan"
if ! terraform -chdir=infra/terraform show -json \
"${RUNNER_TEMP}/relay-same-cap-resume.tfplan" \
@@ -644,21 +643,7 @@ jobs:
--regional-rehome-protocol "${DESIRED_REHOME_PROTOCOL}" \
"${POOL_ARGUMENTS[@]}")"
echo "${RESUME_REVIEW}"
jq -e '.changes == 2
or (.changes == 0 and ((.backendUpdate // []) | length) > 0)' \
<<< "${RESUME_REVIEW}" >/dev/null
# changes == 0 here means the template and MIG are converged and this cell's
# reviewed backend update is the only thing left, so the resume is not complete:
# apply it, or the cell silently keeps the 300-second drain and no request
# logging and the operator reads that as a finished roll. The plan holds nothing
# else (the validator bounded it to this cell's backend, and the template and MIG
# are no-ops in it), and neither attribute restarts an instance, so the
# incarnation check downstream stays honest. Template-and-MIG drift still applies
# nothing, which is what a resume means.
if test "$(jq -er '.changes' <<< "${RESUME_REVIEW}")" = 0; then
terraform -chdir=infra/terraform apply -auto-approve \
"${RUNNER_TEMP}/relay-same-cap-resume.tfplan"
fi
jq -e '.changes == 2' <<< "${RESUME_REVIEW}" >/dev/null
fi
gcloud compute instance-groups managed wait-until "${MIG_NAME}" --stable \
--project "${GCP_PROJECT_ID}" --zone "${TARGET_ZONE}" --timeout 900
@@ -681,7 +666,6 @@ jobs:
-var-file="${RUNNER_TEMP}/relay-same-cap.tfvars.json" \
"-target=google_compute_instance_template.relay_gce_cell[\"${TARGET_CELL_ID}\"]" \
"-target=google_compute_instance_group_manager.relay_gce_cell[\"${TARGET_CELL_ID}\"]" \
"-target=google_compute_backend_service.relay_gce_cell[\"${TARGET_CELL_ID}\"]" \
-out="${RUNNER_TEMP}/relay-same-cap.tfplan"
PLAN_REVIEW="$(terraform -chdir=infra/terraform show -json \
"${RUNNER_TEMP}/relay-same-cap.tfplan" \
@@ -56,9 +56,7 @@ test('same-cap wrapper is reusable, canary-bound, and sequential', () => {
)
// The relaxation is only safe if the reviewed validator actually runs on
// the NON-converged branch, in same-cap-cell mode, with the trust config
// the validator requires, restricted to the template-and-MIG change pair or,
// when only the reviewed backend attributes are left, to those alone — and
// that last case then has to be applied, not waved through as converged.
// the validator requires, restricted to the template-and-MIG change pair.
assert.match(
job,
/if ! terraform -chdir=infra\/terraform show -json[\s\S]{0,220}\| length == 0' >\/dev\/null\n then\n/
@@ -77,20 +75,15 @@ test('same-cap wrapper is reusable, canary-bound, and sequential', () => {
)
assert.match(
job,
/host-drain \\\n {16}--regional-rehome-protocol "\$\{DESIRED_REHOME_PROTOCOL\}" \\\n {16}"\$\{POOL_ARGUMENTS\[@\]\}"\)"\n {12}echo "\$\{RESUME_REVIEW\}"\n {12}jq -e '\.changes == 2\n {16}or \(\.changes == 0 and \(\(\.backendUpdate \/\/ \[\]\) \| length\) > 0\)' \\\n {14}<<< "\$\{RESUME_REVIEW\}" >\/dev\/null/
/host-drain \\\n {16}--regional-rehome-protocol "\$\{DESIRED_REHOME_PROTOCOL\}" \\\n {16}"\$\{POOL_ARGUMENTS\[@\]\}"\)"\n {12}echo "\$\{RESUME_REVIEW\}"\n {12}jq -e '\.changes == 2' <<< "\$\{RESUME_REVIEW\}" >\/dev\/null/
)
// A resume whose only unapplied change is the reviewed backend update must apply it. Leaving
// it is how a cell keeps the 300-second drain and no request logging behind a green resume.
assert.match(
job,
/if test "\$\(jq -er '\.changes' <<< "\$\{RESUME_REVIEW\}"\)" = 0; then\n {14}terraform -chdir=infra\/terraform apply -auto-approve \\\n {16}"\$\{RUNNER_TEMP\}\/relay-same-cap-resume\.tfplan"\n {12}fi\n/
)
// Template-and-MIG drift still applies nothing on resume, which is what a resume means.
// A resume applies nothing at all, which is what a resume means: the only accepted
// unconverged plan is the template-and-MIG rollback-image drift, and it is left pending.
const resumeStep = job.slice(
job.indexOf('- name: Require converged Terraform state and a stable MIG on resume'),
job.indexOf('- name: Apply only the selected same-cap template and MIG')
)
assert.equal(resumeStep.split('terraform -chdir=infra/terraform apply').length, 2)
assert.equal(resumeStep.split('terraform -chdir=infra/terraform apply').length, 1)
assert.match(job, /resume requires the isolated migration-only cell/)
assert.match(job, /test "\$\{TARGET_INCARNATION\}" = "\$\{SOURCE_INCARNATION\}"/)
assert.match(job, /\(.regionalRehomeProtocol \/\/ 0\) == \$protocol/)
@@ -336,9 +336,10 @@ describe('same-cap roll scripts accept every same-cap cell', () => {
assert.equal(workflow.split('POOL_ARGUMENTS=()').length, 3)
})
// One cell's whole serving path and nothing else: the template, the MIG bound to it, and the
// backend service, whose reviewed drain timeout would otherwise need a fleet-wide root apply.
it('targets exactly this cell template, MIG, and backend on every plan the job runs', () => {
// One cell's compute path and nothing else: the template and the MIG bound to it. The cell
// backend service stays out because the capacity role has no compute.backendServices.update,
// so naming it fails the apply after the MIG has already rolled.
it('targets exactly this cell template and MIG on every plan the job runs', () => {
const plans = workflow.split('terraform -chdir=infra/terraform plan').slice(1)
assert.equal(plans.length, 2)
for (const plan of plans) {
@@ -350,15 +351,18 @@ describe('same-cap roll scripts accept every same-cap cell', () => {
.map(([, resource]) => resource),
[
'google_compute_instance_template.relay_gce_cell',
'google_compute_instance_group_manager.relay_gce_cell',
'google_compute_backend_service.relay_gce_cell'
'google_compute_instance_group_manager.relay_gce_cell'
]
)
// Any target that is not one of those three, or not scoped to this cell, fails here.
assert.equal(call.split('-target=').length, 4)
// Any target that is not one of those two, or not scoped to this cell, fails here.
assert.equal(call.split('-target=').length, 3)
}
})
it('never names a backend service on any plan or apply in the job', () => {
assert.equal(workflow.includes('google_compute_backend_service'), false)
})
it('validates a correct plan for every wave cell at that cell\'s rehome protocol', () => {
const trusted = SAME_CAP_CELLS.filter((cell) => REHOME_SOURCE_CELLS.has(cell))
// Only a declared rehome source may roll at a trusted protocol at all; the job refuses
@@ -718,49 +722,37 @@ describe('same-cap roll scripts accept every same-cap cell', () => {
assert.equal(apply.split('wait-until "${MIG_NAME}" --stable').length, 3)
})
// One validator verdict decides three different outcomes. Run the predicates the job ships
// rather than restating them, because restating them is how the two drift apart.
it('decides refuse, apply, or skip on resume from the shipped predicate', () => {
// Run the predicate the job ships rather than restating it, because restating it is how the
// two drift apart. An unconverged resume accepts the template-and-MIG pair and nothing else,
// and it applies nothing: a backend change cannot reach this plan, which no longer targets one.
it('accepts only the template-and-MIG pair on an unconverged resume', () => {
const step = workflow.slice(
workflow.indexOf('- name: Require converged Terraform state and a stable MIG on resume'),
workflow.indexOf('- name: Apply only the selected same-cap template and MIG')
)
const accept = /jq -e '(\.changes == 2\n[\s\S]*?)' \\\n\s+<<< "\$\{RESUME_REVIEW\}"/.exec(step)
const accept = /jq -e '(\.changes == 2)' <<< "\$\{RESUME_REVIEW\}"/.exec(step)
assert.notEqual(accept, null, 'the resume step no longer gates on a validator verdict')
assert.match(step, /if test "\$\(jq -er '\.changes' <<< "\$\{RESUME_REVIEW\}"\)" = 0; then/)
assert.equal(step.includes('terraform -chdir=infra/terraform apply'), false)
const outcome = (review) => {
const resolved = spawnSync('bash', ['-euo', 'pipefail', '-c', [
`RESUME_REVIEW=${JSON.stringify(JSON.stringify(review))}`,
`jq -e '${accept[1]}' <<< "\${RESUME_REVIEW}" >/dev/null || { echo refuse; exit 0; }`,
'if test "$(jq -er \'.changes\' <<< "${RESUME_REVIEW}")" = 0',
'then echo apply; else echo skip; fi'
'echo accept'
].join('\n')], { encoding: 'utf8' })
assert.equal(resolved.status, 0, resolved.stderr)
return resolved.stdout.trim()
}
// Template and MIG converged, this cell's reviewed backend not: apply it here, or the cell
// keeps the 300-second drain and no request logging behind a green resume.
assert.equal(
outcome({ changes: 0, backendUpdate: ['connection_draining_timeout_sec', 'log_config.0'] }),
'apply'
)
assert.equal(outcome({ changes: 0, backendUpdate: ['log_config.0'] }), 'apply')
// Template-and-MIG drift still applies nothing, which is what a resume means.
assert.equal(outcome({ changes: 2 }), 'skip')
assert.equal(
outcome({ changes: 2, backendUpdate: ['connection_draining_timeout_sec'] }),
'skip'
)
// Anything the validator did not bound to this cell's reviewed change set fails the step.
assert.equal(outcome({ changes: 2 }), 'accept')
// Anything the validator did not bound to the reviewed rollback-image drift fails the step.
assert.equal(outcome({ changes: 0 }), 'refuse')
assert.equal(outcome({ changes: 0, backendUpdate: [] }), 'refuse')
assert.equal(outcome({ changes: 1, backendUpdate: ['log_config.0'] }), 'refuse')
assert.equal(outcome({ changes: 0, backendUpdate: ['log_config.0'] }), 'refuse')
assert.equal(outcome({ changes: 1 }), 'refuse')
assert.equal(outcome({ changes: 3 }), 'refuse')
})
// The stranded path is the other reader of `changes`, and a pending backend update must not
// suppress the explicit MIG roll that is the only thing clearing a stranded cell's drain flag.
it('rolls a stranded MIG on the shipped predicate, backend update or not', () => {
// The stranded cell's explicit MIG roll is the only thing that clears its drain flag, and
// `changes` is what decides it, so run the shipped predicate rather than restating it.
it('rolls a stranded MIG on the shipped predicate', () => {
const apply = workflow
.split('name: Apply only the selected same-cap template and MIG')[1]
.split('\n - id:')[0]
@@ -779,10 +771,6 @@ describe('same-cap roll scripts accept every same-cap cell', () => {
return resolved.stdout.trim()
}
assert.equal(rolls('stranded', { changes: 0 }), 'replace')
assert.equal(
rolls('stranded', { changes: 0, backendUpdate: ['connection_draining_timeout_sec'] }),
'replace'
)
// A real template replacement already restarts the instance; rolling again would be a second.
assert.equal(rolls('stranded', { changes: 2 }), 'no-replace')
assert.equal(rolls('resume', { changes: 0 }), 'no-replace')
@@ -301,11 +301,12 @@ function requireDesiredStartupScript(script, config) {
}
}
// The same-cap job targets this cell's backend service so the two declared-but-unapplied
// settings land one cell at a time: an unindexed root plan pulls the whole MIG and template
// resources in as dependencies, which standing image drift turns into a 29-cell roll. Each
// attribute is optional because a cell that already has it plans no change for it.
// Splitting the backend out here keeps `changes` the template-and-MIG count both callers read.
// The same-cap job no longer targets this cell's backend service (the capacity role has no
// compute.backendServices.update), so a wave plan carries no backend change and this reports an
// empty list. It stays as the bound on any caller that does target one: exactly this cell's
// backend, exactly the reviewed drain and request-logging attributes, each optional because a
// cell that already has one plans no change for it. Splitting the backend out keeps `changes`
// the template-and-MIG count both callers read.
function takeCellBackendUpdate(changes, config) {
const backends = changes.filter(
({ address }) => typeof address === 'string' && address.startsWith(`${CELL_BACKEND_RESOURCE}[`)