diff --git a/cloud/dev/scripts/deploy-relay-blue-green.mjs b/cloud/dev/scripts/deploy-relay-blue-green.mjs index f9dfe213daf..a9ed8ca941e 100644 --- a/cloud/dev/scripts/deploy-relay-blue-green.mjs +++ b/cloud/dev/scripts/deploy-relay-blue-green.mjs @@ -601,6 +601,29 @@ function updateTraffic(config, args) { ]) } +export function serviceMaximumInstances(service) { + const value = service.metadata?.annotations?.['run.googleapis.com/maxScale'] + return value === undefined ? undefined : Number(value) +} + +function clearServiceMaximumInstances(config) { + // `--max=0` resets the service cap to Cloud Run's default without a new revision. + commandText(['run', 'services', 'update', config.service, ...serviceArguments(config), '--max=0', '--quiet']) +} + +// Why: a service cap is divided among the revisions taking traffic, so one below two full +// revisions leaves a 20% candidate a fifth of it while the serving floor holds the rest. +function ensureServiceRoomForOverlap(config, operations, service, revisionMaximum) { + const cap = serviceMaximumInstances(service) + if (cap === undefined || cap >= 2 * revisionMaximum) return + operations.clearServiceMaximumInstances(config) + const cleared = serviceMaximumInstances(operations.describeService(config)) + if (cleared !== undefined && cleared < 2 * revisionMaximum) { + throw new Error(`director service still caps instances at ${cleared}, below two revisions of ${revisionMaximum}`) + } + console.warn(JSON.stringify({ event: 'director_service_cap_cleared', from: cap, to: cleared ?? null })) +} + function removeDirectorTrafficTags(config, operations, retained = new Set()) { const tags = trafficTags(operations.describeService(config)).filter( (tag) => !retained.has(tag) @@ -636,7 +659,11 @@ function pruneDirectorRevisions(config, operations) { } } -function deployCandidate( +function deployCandidate(...candidate) { + commandText(candidateUpdateArguments(...candidate)) +} + +export function candidateUpdateArguments( config, tag, env = {}, @@ -657,7 +684,8 @@ function deployCandidate( tag, '--no-traffic' ] - if (maxInstances !== undefined) args.push('--max', String(maxInstances)) + // Per revision: gcloud's `--max` is the service cap, which Cloud Run splits by traffic share. + if (maxInstances !== undefined) args.push('--max-instances', String(maxInstances)) if (config.role === 'director') { args.push( '--update-secrets', @@ -676,7 +704,7 @@ function deployCandidate( args.push('--update-env-vars', environmentUpdateValue(env)) } args.push('--quiet') - commandText(args) + return args } function revisionShape(revision, mutableEnvironment, allowServiceAccountChange = false) { @@ -1014,6 +1042,7 @@ export async function deployDirector(config, tag, overrides = {}) { deployCandidate, describeService, describeRevision, + clearServiceMaximumInstances, listRevisions, deleteRevision, updateTraffic, @@ -1124,6 +1153,7 @@ export async function deployDirector(config, tag, overrides = {}) { if (!bootstrapRuntimeIdentity) { await verifyRehomeDisabled(config['rehome-control-origin']) } + ensureServiceRoomForOverlap(config, operations, initialService, requiredMaximumInstances) removeDirectorTrafficTags(config, operations) let deployed let staged = false diff --git a/cloud/dev/scripts/deploy-relay-blue-green.test.mjs b/cloud/dev/scripts/deploy-relay-blue-green.test.mjs index fac960c12e9..f5b4e87997f 100644 --- a/cloud/dev/scripts/deploy-relay-blue-green.test.mjs +++ b/cloud/dev/scripts/deploy-relay-blue-green.test.mjs @@ -32,6 +32,7 @@ import { taggedRevisionOrigin, taggedTraffic, trafficTags, + candidateUpdateArguments, waitForEvacuationCapacity, waitForServingInstances } from './deploy-relay-blue-green.mjs' @@ -445,7 +446,10 @@ function directorHarness({ callerEmail, // The revision template's Cloud SQL transport; a Terraform apply changes it under the serving one. templateTransport, - spreadFailure + spreadFailure, + // The service-level cap gcloud's `--max` sets; Cloud Run divides it by traffic share. + serviceMaximum, + serviceCapSticks = false } = {}) { const state = { activeRevision: 'relay-00001-old', @@ -474,6 +478,8 @@ function directorHarness({ ]), nextRevision: 2, latestCreated: 'relay-00001-old', + serviceMaximum, + serviceCapClears: 0, templateTransport } const removed = [] @@ -483,6 +489,11 @@ function directorHarness({ let pendingCleanupFailure = cleanupFailsBeforeRemoval const operations = { describeService: () => ({ + metadata: { + annotations: state.serviceMaximum === undefined + ? {} + : { 'run.googleapis.com/maxScale': String(state.serviceMaximum) } + }, status: { url: 'https://relay-hash-uc.a.run.app', latestCreatedRevisionName: state.latestCreated, @@ -548,6 +559,10 @@ function directorHarness({ state.tags.set(tag, revision) if (deployFailure) throw deployFailure }, + clearServiceMaximumInstances: () => { + state.serviceCapClears += 1 + if (!serviceCapSticks) state.serviceMaximum = 100 + }, listRevisions: () => [...state.revisions].map(([name]) => ({ metadata: { name } })), deleteRevision: (_config, revision) => { @@ -719,6 +734,46 @@ test('a multi-instance floor moves part of the traffic until every floor instanc assert.equal(harness.state.activeRevision, 'relay-00003-new') }) +test('a candidate pins its own maximum, never the service-wide cap', () => { + const args = candidateUpdateArguments( + { service: 'relay', project: 'p', region: 'r', role: 'director' }, + 'candidate-new', + {}, + 'relay@sha256:abc', + 5, + 5, + '1' + ) + assert.equal(args[args.indexOf('--max-instances') + 1], '5') + assert.ok(!args.includes('--max')) +}) + +test('a service cap below two full revisions is cleared before any candidate exists', async () => { + const harness = directorHarness({ servingMinimum: 5, serviceMaximum: 5 }) + await deployDirector({ 'max-instances': '5' }, 'candidate-new', harness.operations) + assert.equal(harness.state.serviceCapClears, 1) + assert.equal(harness.state.serviceMaximum, 100) + assert.equal(harness.state.activeRevision, 'relay-00003-new') +}) + +test('a service cap with room for two revisions, or none, is left alone', async () => { + for (const serviceMaximum of [undefined, 10, 100]) { + const harness = directorHarness({ servingMinimum: 5, serviceMaximum }) + await deployDirector({ 'max-instances': '5' }, 'candidate-new', harness.operations) + assert.equal(harness.state.serviceCapClears, 0) + } +}) + +test('a service cap that will not clear stops the deploy before traffic moves', async () => { + const harness = directorHarness({ servingMinimum: 5, serviceMaximum: 5, serviceCapSticks: true }) + await assert.rejects( + deployDirector({ 'max-instances': '5' }, 'candidate-new', harness.operations), + /still caps instances at 5, below two revisions of 5/ + ) + assert.equal(harness.state.activeRevision, 'relay-00001-old') + assert.deepEqual(harness.trafficMoves, []) +}) + test('a floor of one moves all traffic at once', async () => { const harness = directorHarness() await deployDirector({}, 'candidate-new', harness.operations) diff --git a/cloud/docs/orca-relay-operations.md b/cloud/docs/orca-relay-operations.md index 2e23041dc36..7f1c0b3360b 100644 --- a/cloud/docs/orca-relay-operations.md +++ b/cloud/docs/orca-relay-operations.md @@ -244,7 +244,10 @@ promotes a second compatible revision, and retains older revisions through the stabilization window. With a floor above one, promotion first moves 20% of traffic and waits (up to six minutes) until every floor instance has served a request, because Cloud Run routes new floor instances only ~100 s after they -start; a timeout returns all traffic to the previous serving revision. +start; a timeout returns all traffic to the previous serving revision. Each +revision carries its own instance maximum. A service-level cap would be split by +traffic share (a 20% candidate under a cap of 5 gets one instance), so the deploy +resets any service cap below two full revisions to Cloud Run's default first. If the deploy job is cancelled or loses its runner during that wait, the 20/80 split stays in place, and both the next deploy and the rehome workflow refuse a