refactor(push): remove redundant cloud storage and rollout coupling

2026-09-30 08:03:12 +00:00 · 2026-09-09 16:03:49 -04:00
parent a4fead9cb4
commit bba1b53447
28 changed files with 360 additions and 621 deletions
@@ -283,8 +283,6 @@ export const LEASED_WORKFLOWS = named([
    'operate-relay-production-rehome.yml',
    production({ leaseFiles: ['operate-relay-production-rehome-job.yml'] })
  ],
-  // The gateway applies its schema at startup, so its deploy revision is the schema step.
-  ['push-deploy.yml', production()],
  ['deploy-relay-asia-topology.yml', eitherEnvironment()],
  ['operate-relay-asia-admission.yml', eitherEnvironment()],
  ['deploy-relay-staging.yml', staging()],
@@ -300,6 +298,10 @@ export const LEASED_WORKFLOWS = named([
 ])

 export const NOT_A_CLOUD_SQL_CANDIDATE = named([
+  [
+    'push-deploy.yml',
+    'Push attaches only to its dedicated database and serializes its own traffic changes under production-push-rollout and the durable push-rollout lease; push-gateway-workflow tests verify both.'
+  ],
  [
    'monitor-relay-production.yml',
    'Read-only. Its identity holds monitoring, logging, Cloud SQL and compute viewer roles only, and it runs `gcloud sql instances describe`, never a mutation. It consumes no connection budget, so the durable lease would only let monitoring block a rollout and a rollout block monitoring.'
@@ -12,7 +12,7 @@ import { readRelayWorkflow, relayWorkflowFile } from './relay-repository.mjs'

 // Why: the push gateway holds the APNs key and is the only thing standing between a paired
 // phone and a silent notification pipeline. Its deploy is a blue/green rollout against the
-// shared Cloud SQL instance, and each of the guarantees below is one careless edit from gone.
+// dedicated Cloud SQL instance, and each of the guarantees below is one careless edit from gone.
 const WORKFLOW = 'push-deploy.yml'
 const workflow = readRelayWorkflow(WORKFLOW)
 const deploy = () => {
@@ -60,15 +60,15 @@ test('Terraform trusts this exact workflow file on the production deploy provide
  assert.equal(relayWorkflowFile(WORKFLOW), 'cloud-push-deploy.yml')
 })

-test('the rollout is serialized and leases the production Cloud SQL rollout lock', () => {
+test('the rollout is serialized and leases its dedicated push rollout lock', () => {
  const blocks = concurrencyBlocks(workflow)
  assert.equal(blocks.length, 1)
-  assert.equal(blocks[0].group, 'production-cloud-sql-rollout')
+  assert.equal(blocks[0].group, 'production-push-rollout')
  assert.equal(blocks[0].cancelInProgress, 'false')
  const steps = leaseSteps(workflow)
  assert.equal(steps.length, 1, 'exactly one lease step, held for the whole run')
  assert.equal(steps[0].bucket, 'onorca-cloud-terraform-state')
-  assert.equal(steps[0].object, 'terraform/state/cloud-sql-rollout/production.lock')
+  assert.equal(steps[0].object, 'terraform/state/push-rollout/production.lock')
  assert.equal(steps[0].release, undefined, 'release stays at its default for a single-job run')
 })

@@ -138,7 +138,7 @@ test('the database pool size is Terraform-owned and bounded at plan time', () =>
  assert.ok(block, 'the push service no longer declares a lifecycle block')
  assert.match(
    block[1],
-    /var\.push_max_instances \* var\.push_database_pool_max <= 4/,
+    /var\.push_max_instances \* var\.push_database_pool_max \* 3 <= 64/,
    'instances x pool must be bounded at plan time'
  )
  assert.match(
@@ -313,3 +313,22 @@ test('push credentials cannot assume the shared Relay deploy identity', () => {
 test('dedicated database admits three simultaneous revision pools', () => {
  assert.match(terraform('push-gateway.tf'), /var\.push_max_instances \* var\.push_database_pool_max \* 3 <= 64/)
 })
+
+test('push has only a dedicated database attachment and a narrowly scoped deployment lease', () => {
+  const service = terraform('push-gateway.tf')
+  const database = terraform('push-dedicated-database.tf')
+  assert.match(service, /instances = \[google_sql_database_instance\.push_dedicated\[0\]\.connection_name\]/)
+  assert.match(service, /secret\s*= google_secret_manager_secret\.push_dedicated_database_url\[0\]\.secret_id/)
+  assert.match(service, /version = google_secret_manager_secret_version\.push_dedicated_database_url\[0\]\.version/)
+  assert.doesNotMatch(service + database, /push_dedicated_database_(?:active|enabled)|local\.relay_database_connection_name|resource "google_sql_database" "push"/)
+  assert.match(database, /tier\s*= "db-custom-2-7680"/)
+  assert.match(database, /availability_type = "REGIONAL"/)
+  assert.match(database, /deletion_protection\s*= true/)
+  assert.match(database, /deletion_protection_enabled = true/)
+  const identity = terraform('push-deploy-identity.tf')
+  const lease = identity.match(/resource "google_storage_bucket_iam_member" "github_push_rollout_lease" \{([\s\S]*?)\n\}/)?.[1]
+  assert.ok(lease)
+  assert.match(lease, /member = local\.push_deploy_member/)
+  assert.match(lease, /role\s*= "roles\/storage.objectAdmin"/)
+  assert.match(lease, /resource.name == 'projects\/_\/buckets\/\$\{var.project_id\}-terraform-state\/objects\/terraform\/state\/push-rollout\/production.lock'/)
+})
@@ -33,13 +33,6 @@ function requiredInteger(source, pattern, label) {
  return value
 }

-// A tfvars file states only what it overrides, so an absent key means the variable default holds.
-// Reading the default as the fallback keeps this honest either way.
-function overriddenInteger(override, overridePattern, source, pattern, label) {
-  if (!overridePattern.test(override)) return requiredInteger(source, pattern, label)
-  return requiredInteger(override, overridePattern, label)
-}
-
 function productionCells(source, defaultPoolMax) {
  const fencedMatch = source.match(/relay_gce_fenced_cells\s*=\s*\[([^\]]*)\]/)
  if (!fencedMatch) throw new Error('could not read fenced Relay cells')
@@ -59,13 +52,11 @@ function productionCells(source, defaultPoolMax) {
 }

 export function calculateRelayCloudSqlConnectionBudget(inputs) {
-  const pushDraw = inputs.pushInstances * inputs.pushPoolMax
  const consumers = {
    cells: inputs.cellPoolTotal + inputs.asiaCellCount * inputs.asiaPoolMax,
    directors: inputs.directorInstances * inputs.directorPoolMax,
    auth: inputs.authInstances * inputs.authPoolMax,
-    api: inputs.apiInstances * inputs.apiPoolMax,
-    push: pushDraw
+    api: inputs.apiInstances * inputs.apiPoolMax
  }
  const configuredMaximum = Object.values(consumers).reduce((total, value) => total + value, 0)
  const retainedDirectorRollback = inputs.directorInstances * inputs.directorPoolMax
@@ -73,8 +64,6 @@ export function calculateRelayCloudSqlConnectionBudget(inputs) {
    relayDirectorCandidate: retainedDirectorRollback * 2,
    apiCandidate: retainedDirectorRollback + inputs.apiInstances * inputs.apiPoolMax,
    authCandidate: retainedDirectorRollback + inputs.authInstances * inputs.authPoolMax,
-    // Serving is already counted; validation/rejected and its successor add two pools.
-    pushCandidate: retainedDirectorRollback + pushDraw * 2,
    relayCells: retainedDirectorRollback
  }
  const rolloutOverlap = Math.max(...Object.values(candidateOverlap))
@@ -142,20 +131,6 @@ export function readRelayCloudSqlConnectionBudget({
      /variable\s+"relay_director_database_pool_max"[\s\S]*?default\s*=\s*(\d+)/,
      'director pool maximum'
    ),
-    // The mobile push gateway shares this instance. Its draw was invisible here until Terraform
-    // declared the pool: docs/push-gateway.md, "Shape".
-    pushInstances: overriddenInteger(
-      productionTfvars,
-      /^\s*push_max_instances\s*=\s*(\d+)/m,
-      terraformVariables,
-      /variable\s+"push_max_instances"[\s\S]*?default\s*=\s*(\d+)/,
-      'push gateway instances'
-    ),
-    pushPoolMax: requiredInteger(
-      terraformVariables,
-      /variable\s+"push_database_pool_max"[\s\S]*?default\s*=\s*(\d+)/,
-      'push gateway pool maximum'
-    ),
    authInstances: apps.authInstances,
    authPoolMax: apps.authPoolMax,
    apiInstances: apps.apiInstances,
@@ -6,91 +6,28 @@ import {
  readRelayCloudSqlConnectionBudget
 } from './relay-cloud-sql-connection-budget.mjs'

-// Why these numbers are this tight: the shared instance's 400 connections were already spoken
-// for, and the relay shape below leaves exactly five. The gateway is sized to fit in four, two
-// instances times a two-connection pool, and its rollout overlap of 23 stays under the API
-// candidate's 65, so the Math.max is the API candidate rather than the gateway.
-//
-// `Deploy Relay Asia Topology` gates on `withinBudget == true`, so the single remaining
-// connection is the whole margin. Anything that raises a pool or an instance count moves it.
-test('production plus the push gateway keeps allowance and reserve below the ceiling', () => {
+test('production shared consumers keep allowance and reserve below the ceiling', () => {
  const report = readRelayCloudSqlConnectionBudget()

-  assert.deepEqual(report.consumers, { cells: 230, directors: 15, auth: 20, api: 50, push: 4 })
+  assert.deepEqual(report.consumers, { cells: 230, directors: 15, auth: 20, api: 50 })
  assert.deepEqual(report.asia, { cells: 3, poolMax: 10 })
-  assert.equal(report.configuredMaximum, 319)
+  assert.equal(report.configuredMaximum, 315)
  assert.equal(report.rolloutOverlap.relayDirectorCandidate, 30)
  assert.equal(report.rolloutOverlap.apiCandidate, 65)
  assert.equal(report.rolloutOverlap.authCandidate, 35)
-  assert.equal(report.rolloutOverlap.pushCandidate, 23)
  assert.equal(report.rolloutOverlap.relayCells, 15)
  assert.equal(report.rolloutOverlap.retainedDirectorRollback, 15)
-  // The gateway does not set the maximum; the API candidate does, as it did before it existed.
  assert.equal(report.rolloutOverlap.maximum, 65)
  assert.equal(report.maintenanceAdminAllowance, 5)
  assert.equal(report.explicitReserve, 10)
  assert.equal(report.usableCeiling, 390)
-  assert.equal(report.operatingMaximum, 389)
-  assert.equal(report.remainingWithinUsableCeiling, 1)
-  assert.equal(report.budgetedTotal, 399)
-  assert.equal(report.unallocated, 1)
-  assert.equal(report.withinBudget, true)
-})
-
-// Why: the same relay shape without a push gateway is the before picture, and it stood at five
-// connections clear. Holding it here keeps the gateway's cost visible as the four it takes,
-// rather than letting drift elsewhere in the budget hide inside the same margin.
-test('the same relay shape without the gateway stays inside the ceiling', () => {
-  const report = calculateRelayCloudSqlConnectionBudget({
-    cellPoolTotal: 200,
-    asiaCellCount: 3,
-    asiaPoolMax: 10,
-    directorInstances: 5,
-    directorPoolMax: 3,
-    authInstances: 2,
-    authPoolMax: 10,
-    apiInstances: 10,
-    apiPoolMax: 5,
-    pushInstances: 0,
-    pushPoolMax: 0,
-    maxConnections: 400,
-    maintenanceAdminAllowance: 5,
-    explicitReserve: 10
-  })
-
-  assert.equal(report.consumers.push, 0)
-  assert.equal(report.rolloutOverlap.maximum, 65)
  assert.equal(report.operatingMaximum, 385)
  assert.equal(report.remainingWithinUsableCeiling, 5)
+  assert.equal(report.budgetedTotal, 395)
+  assert.equal(report.unallocated, 5)
  assert.equal(report.withinBudget, true)
 })

-// Why: a tagged candidate is directly addressable and sits outside the service-wide cap, so all three
-// push revisions can reach the ceiling at once. The API and auth candidates add one copy; this
-// one adds two, like the director candidate.
-test('the push rollout scenario triples the gateway draw over the retained director', () => {
-  const report = calculateRelayCloudSqlConnectionBudget({
-    cellPoolTotal: 0,
-    asiaCellCount: 0,
-    asiaPoolMax: 0,
-    directorInstances: 5,
-    directorPoolMax: 3,
-    authInstances: 0,
-    authPoolMax: 0,
-    apiInstances: 0,
-    apiPoolMax: 0,
-    pushInstances: 2,
-    pushPoolMax: 2,
-    maxConnections: 400,
-    maintenanceAdminAllowance: 5,
-    explicitReserve: 10
-  })
-
-  assert.equal(report.consumers.push, 4)
-  // Serving is in the base; overlap adds 15 retained director plus two 4-connection pools.
-  assert.equal(report.rolloutOverlap.pushCandidate, 23)
-})
-
 test('fails closed when pool growth consumes the explicit reserve', () => {
  const report = calculateRelayCloudSqlConnectionBudget({
    cellPoolTotal: 200,
@@ -102,14 +39,12 @@ test('fails closed when pool growth consumes the explicit reserve', () => {
    authPoolMax: 10,
    apiInstances: 20,
    apiPoolMax: 5,
-    pushInstances: 4,
-    pushPoolMax: 10,
    maxConnections: 400,
    maintenanceAdminAllowance: 5,
    explicitReserve: 10
  })

-  assert.equal(report.operatingMaximum, 555)
+  assert.equal(report.operatingMaximum, 515)
  assert.equal(report.withinBudget, false)
 })

@@ -141,15 +76,11 @@ test('excludes fenced cell pools and reads per-cell pool overrides', () => {
  })

  assert.equal(report.consumers.cells, 14)
-  // No push_max_instances in this tfvars, so the variable default of one instance holds.
-  assert.equal(report.consumers.push, 2)
-  assert.equal(report.operatingMaximum, 48)
-  assert.equal(report.budgetedTotal, 49)
+  assert.equal(report.operatingMaximum, 46)
+  assert.equal(report.budgetedTotal, 47)
 })

-// Why: production.tfvars overrides push_max_instances down to 2 while variables.tf still defaults
-// to 4, so reading the default instead of the override would overstate the live draw by half.
-test('a tfvars push_max_instances override wins over the variable default', () => {
+test('dedicated push scaling does not consume shared capacity', () => {
  const report = readRelayCloudSqlConnectionBudget({
    proposedAsiaCellCount: 1,
    appConsumers: { authInstances: 1, authPoolMax: 10, apiInstances: 1, apiPoolMax: 5, maxConnections: 100 },
@@ -175,8 +106,9 @@ test('a tfvars push_max_instances override wins over the variable default', () =
    explicitReserve: 1
  })

-  assert.equal(report.consumers.push, 6)
-  assert.equal(report.rolloutOverlap.pushCandidate, 15)
+  assert.equal(report.consumers.push, undefined)
+  assert.equal(report.rolloutOverlap.pushCandidate, undefined)
+  assert.equal(report.operatingMaximum, 46)
 })

 test('requires strict headroom below the physical ceiling', () => {
@@ -190,14 +122,12 @@ test('requires strict headroom below the physical ceiling', () => {
    authPoolMax: 10,
    apiInstances: 1,
    apiPoolMax: 5,
-    pushInstances: 1,
-    pushPoolMax: 2,
    maxConnections: 50,
    maintenanceAdminAllowance: 9,
    explicitReserve: 3
  })

-  assert.equal(report.budgetedTotal, 65)
+  assert.equal(report.budgetedTotal, 63)
  assert.equal(report.withinBudget, false)
 })