feat(k8s): retry transient k8s API failures in auth preflight; bump runner to 2.338.0

.github/runner-versions.yaml

@@ -4,3 +4,4 @@
 runner_versions:
   - "2.336.0"
   - "2.337.0"
+  - "2.338.0"

packages/k8s/package.json

@@ -18,7 +18,7 @@
     "@actions/io": "^1.1.3",
     "@kubernetes/client-node": "^1.3.0",
     "hooklib": "file:../hooklib",
-    "js-yaml": "^4.3.1",
+    "js-yaml": "^4.3.2",
     "shlex": "^3.0.0",
     "tar-fs": "^3.1.0",
     "uuid": "^11.1.1"

packages/k8s/src/k8s/index.ts

@@ -20,6 +20,7 @@ import {
   fixArgs,
   listDirAllCommand,
   sleep,
+  retryK8sApiCall,
   EXTERNALS_VOLUME_NAME,
   GITHUB_VOLUME_NAME,
   WORK_VOLUME
@@ -1725,10 +1726,12 @@ export async function getPodStatus(
 }
 
 export async function isAuthPermissionsOK(): Promise<boolean> {
-  const sar = new k8s.V1SelfSubjectAccessReview()
   const asyncs: Promise<k8s.V1SelfSubjectAccessReview>[] = []
   for (const resource of requiredPermissions) {
     for (const verb of resource.verbs) {
+      // A fresh SAR per request: reusing a single mutable instance would let a
+      // retry re-submit the body of whichever verb the outer loop reached last.
+      const sar = new k8s.V1SelfSubjectAccessReview()
       sar.spec = new k8s.V1SelfSubjectAccessReviewSpec()
       sar.spec.resourceAttributes = new k8s.V1ResourceAttributes()
       sar.spec.resourceAttributes.verb = verb
@@ -1737,7 +1740,13 @@ export async function isAuthPermissionsOK(): Promise<boolean> {
       sar.spec.resourceAttributes.resource = resource.resource
       sar.spec.resourceAttributes.subresource = resource.subresource
       asyncs.push(
-        k8sAuthorizationV1Api.createSelfSubjectAccessReview({ body: sar })
+        retryK8sApiCall(
+          async () =>
+            k8sAuthorizationV1Api.createSelfSubjectAccessReview({ body: sar }),
+          `createSelfSubjectAccessReview(${resource.resource}${
+            resource.subresource ? '/' + resource.subresource : ''
+          },${verb})`
+        )
       )
     }
   }

packages/k8s/src/k8s/utils/index.ts

@@ -298,3 +298,134 @@ export async function sleep(ms: number): Promise<void> {
 export function listDirAllCommand(dir: string): string {
   return `cd ${shlex.quote(dir)} && find . -type f -not -path '*/_runner_hook_responses*' -exec stat -c '%s %n' {} \\;`
 }
+
+const RETRYABLE_NETWORK_ERROR_CODES = new Set([
+  'ECONNREFUSED',
+  'ECONNRESET',
+  'EPIPE',
+  'ENOTFOUND',
+  'EAI_AGAIN',
+  'ETIMEDOUT',
+  'EHOSTUNREACH',
+  'ENETUNREACH',
+  'UND_ERR_CONNECT',
+  'UND_ERR_CONNECT_TIMEOUT',
+  'UND_ERR_HEADERS_TIMEOUT',
+  'UND_ERR_BODY_TIMEOUT',
+  'UND_ERR_SOCKET'
+])
+
+const RETRYABLE_MESSAGE_PATTERN =
+  /ECONNREFUSED|ECONNRESET|ETIMEDOUT|EAI_AGAIN|socket hang up|network timeout|timed out after/i
+
+// Transient HTTP statuses worth retrying:
+// - 401/403: an apiserver replica may briefly answer before its
+//   auth/RBAC caches are warm (observed on self-hosted k8s runners as
+//   "selfsubjectaccessreviews ... is forbidden" that succeeds on retry)
+// - 408/429/5xx: classic transient server-side conditions
+const RETRYABLE_HTTP_STATUS_CODES = new Set([
+  401, 403, 408, 429, 500, 502, 503, 504
+])
+
+export function isRetryableK8sApiError(err: unknown): boolean {
+  if (err === null || err === undefined || typeof err !== 'object') {
+    return false
+  }
+  const e = err as {
+    code?: unknown
+    cause?: { code?: unknown }
+    message?: unknown
+  }
+  if (typeof e.code === 'number') {
+    return RETRYABLE_HTTP_STATUS_CODES.has(e.code)
+  }
+  if (typeof e.code === 'string' && RETRYABLE_NETWORK_ERROR_CODES.has(e.code)) {
+    return true
+  }
+  if (e.cause && typeof e.cause.code === 'string') {
+    return RETRYABLE_NETWORK_ERROR_CODES.has(e.cause.code)
+  }
+  const message = typeof e.message === 'string' ? e.message : String(err)
+  return RETRYABLE_MESSAGE_PATTERN.test(message)
+}
+
+export interface RetryK8sApiCallOptions {
+  attempts?: number
+  baseDelayMs?: number
+  maxDelayMs?: number
+  timeoutMs?: number
+}
+
+async function withTimeout<T>(
+  promise: Promise<T>,
+  timeoutMs: number | undefined,
+  description: string
+): Promise<T> {
+  if (!timeoutMs) {
+    return promise
+  }
+  return new Promise<T>((resolve, reject) => {
+    const timer = setTimeout(() => {
+      reject(
+        Object.assign(
+          new Error(
+            `k8s API call "${description}" timed out after ${timeoutMs} ms`
+          ),
+          {
+            code: 'ETIMEDOUT'
+          }
+        )
+      )
+    }, timeoutMs)
+    promise
+      .then(value => {
+        clearTimeout(timer)
+        resolve(value)
+      })
+      .catch(err => {
+        clearTimeout(timer)
+        reject(err)
+      })
+  })
+}
+
+/**
+ * Executes a Kubernetes API call with bounded retries and exponential backoff
+ * on transient failures (connection refused/reset/hang, or a momentary
+ * auth/RBAC 401/403 from a warming-up apiserver replica).
+ *
+ * The kubelet-to-apiserver ClusterIP path on self-hosted runner nodes opens
+ * sub-second windows whenever managed control-plane endpoints rotate; a single
+ * untimed request hitting such a window used to abort the whole workflow step.
+ * Retrying a handful of times rides through the window instead of failing.
+ */
+export async function retryK8sApiCall<T>(
+  operation: () => Promise<T>,
+  description: string,
+  options: RetryK8sApiCallOptions = {}
+): Promise<T> {
+  const attempts = options.attempts ?? 4
+  const baseDelayMs = options.baseDelayMs ?? 3000
+  const maxDelayMs = options.maxDelayMs ?? 15000
+  const timeoutMs = options.timeoutMs ?? 15000
+
+  let lastError: unknown
+  for (let attempt = 1; attempt <= attempts; attempt++) {
+    try {
+      return await withTimeout(operation(), timeoutMs, description)
+    } catch (err) {
+      lastError = err
+      if (attempt === attempts || !isRetryableK8sApiError(err)) {
+        throw err
+      }
+      const delayMs = Math.min(maxDelayMs, baseDelayMs * 2 ** (attempt - 1))
+      core.warning(
+        `k8s API call "${description}" failed on attempt ${attempt}/${attempts}: ${
+          err instanceof Error ? err.message : String(err)
+        }. Retrying in ${delayMs} ms.`
+      )
+      await sleep(delayMs)
+    }
+  }
+  throw lastError
+}

packages/k8s/tests/retry-utils-test.ts

@@ -0,0 +1,97 @@
+import { isRetryableK8sApiError, retryK8sApiCall } from '../src/k8s/utils'
+
+describe('isRetryableK8sApiError', () => {
+  it('treats transient HTTP statuses as retryable', () => {
+    // ApiException thrown by @kubernetes/client-node carries a numeric `code`
+    expect(isRetryableK8sApiError({ code: 403 })).toBe(true)
+    expect(isRetryableK8sApiError({ code: 401 })).toBe(true)
+    expect(isRetryableK8sApiError({ code: 429 })).toBe(true)
+    expect(isRetryableK8sApiError({ code: 500 })).toBe(true)
+    expect(isRetryableK8sApiError({ code: 503 })).toBe(true)
+  })
+
+  it('treats non-transient HTTP statuses as fatal', () => {
+    expect(isRetryableK8sApiError({ code: 400 })).toBe(false)
+    expect(isRetryableK8sApiError({ code: 404 })).toBe(false)
+    expect(isRetryableK8sApiError({ code: 409 })).toBe(false)
+  })
+
+  it('treats connection-level fetch failures as retryable', () => {
+    // FetchError from node-fetch carries a system error `code`
+    expect(isRetryableK8sApiError({ code: 'ECONNREFUSED' })).toBe(true)
+    expect(isRetryableK8sApiError({ code: 'ECONNRESET' })).toBe(true)
+    expect(isRetryableK8sApiError({ code: 'ETIMEDOUT' })).toBe(true)
+    // undici-style wrapped causes
+    expect(
+      isRetryableK8sApiError({ cause: { code: 'UND_ERR_CONNECT_TIMEOUT' } })
+    ).toBe(true)
+  })
+
+  it('falls back to the error message', () => {
+    expect(
+      isRetryableK8sApiError(
+        new Error(
+          'request to https://10.247.0.1/apis/authorization.k8s.io/v1/selfsubjectaccessreviews failed, reason: connect ECONNREFUSED 10.247.0.1:443'
+        )
+      )
+    ).toBe(true)
+    expect(isRetryableK8sApiError(new Error('plain bug'))).toBe(false)
+    expect(isRetryableK8sApiError(undefined)).toBe(false)
+    expect(isRetryableK8sApiError('ECONNREFUSED')).toBe(false)
+  })
+})
+
+describe('retryK8sApiCall', () => {
+  const fast = { attempts: 3, baseDelayMs: 1, maxDelayMs: 2 }
+
+  it('returns the first successful result without retrying', async () => {
+    const op = jest.fn().mockResolvedValue('ok')
+    await expect(retryK8sApiCall(op, 'test', fast)).resolves.toBe('ok')
+    expect(op).toHaveBeenCalledTimes(1)
+  })
+
+  it('rides through transient failures and succeeds', async () => {
+    const op = jest
+      .fn()
+      .mockRejectedValueOnce({ code: 'ECONNREFUSED' })
+      .mockRejectedValueOnce({ code: 403 })
+      .mockResolvedValue('recovered')
+    await expect(retryK8sApiCall(op, 'test', fast)).resolves.toBe('recovered')
+    expect(op).toHaveBeenCalledTimes(3)
+  })
+
+  it('does not retry fatal (non-transient) errors', async () => {
+    const op = jest
+      .fn()
+      .mockRejectedValue({ code: 400, message: 'bad request' })
+    await expect(retryK8sApiCall(op, 'test', fast)).rejects.toMatchObject({
+      code: 400
+    })
+    expect(op).toHaveBeenCalledTimes(1)
+  })
+
+  it('throws the last error once attempts are exhausted', async () => {
+    const op = jest.fn().mockRejectedValue({ code: 503 })
+    await expect(
+      retryK8sApiCall(op, 'test', { ...fast, attempts: 2 })
+    ).rejects.toMatchObject({ code: 503 })
+    expect(op).toHaveBeenCalledTimes(2)
+  })
+
+  it('abandons a hung request via timeout and retries', async () => {
+    const neverResolves = new Promise<string>(() => undefined)
+    const op = jest
+      .fn()
+      .mockReturnValueOnce(neverResolves)
+      .mockResolvedValueOnce('late-success')
+    await expect(
+      retryK8sApiCall(op, 'test', {
+        attempts: 2,
+        baseDelayMs: 1,
+        maxDelayMs: 2,
+        timeoutMs: 20
+      })
+    ).resolves.toBe('late-success')
+    expect(op).toHaveBeenCalledTimes(2)
+  })
+})