fix(relay): give each director revision its own instance maximum instead of a split service cap (#31178)

The deploy passed `--max`, which is gcloud's service-wide cap. Cloud Run divides
that cap among the revisions taking traffic, so during the 20/80 stage a
candidate under a cap of 5 got one instance while the serving floor kept four,
and the spread check could never see the full floor (deploy M, 1 of 5 for six
minutes). Candidates now pin `--max-instances` per revision, and the deploy
resets a service cap below two full revisions to Cloud Run's default before it
creates any candidate. The rollout overlap of two full directors is already in
the checked Cloud SQL connection budget.

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

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)

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