From 25f68874a2179f6a02bbdf9072637c390a1d2dce Mon Sep 17 00:00:00 2001 From: Longwt123 <1075015495@qq.com> Date: Thu, 15 Oct 2026 17:46:35 +0800 Subject: [PATCH] feat(k8s): retry transient k8s API failures in auth preflight; bump runner to 2.338.0 Runner pods reach the in-cluster API (10.247.0.1) through node-local ClusterIP rules; every control-plane endpoint rotation opens a sub-second window with three random shapes: ECONNREFUSED (no backend), request hang (stale backend), and transient 403 on unwarmed apiserver replicas. The hook entrypoint isAuthPermissionsOK() runs 13 parallel, untimed, un-retried SelfSubjectAccessReviews before each step; any single failure aborts the whole step (process.exit(1)). - Add retryK8sApiCall: bounded retries with exponential backoff (3s/6s/12s, 4 attempts) + per-request timeout (15s); retries only transient errors (connection-layer codes, hang signatures, HTTP 401/403/408/429/5xx); deterministic errors throw immediately - Wrap the SAR preflight checks with retryK8sApiCall, and fix the shared mutable V1SelfSubjectAccessReview instance (now a fresh SAR per request) - Register actions-runner 2.338.0 in .github/runner-versions.yaml - Add tests/retry-utils-test.ts (classifier + retry behavior) --- .github/runner-versions.yaml | 1 + packages/k8s/package-lock.json | 14 +-- packages/k8s/package.json | 2 +- packages/k8s/src/k8s/index.ts | 13 ++- packages/k8s/src/k8s/utils/index.ts | 131 +++++++++++++++++++++++++ packages/k8s/tests/retry-utils-test.ts | 97 ++++++++++++++++++ 6 files changed, 248 insertions(+), 10 deletions(-) create mode 100644 packages/k8s/tests/retry-utils-test.ts diff --git a/.github/runner-versions.yaml b/.github/runner-versions.yaml index 0e66603f..ea5734e1 100644 --- a/.github/runner-versions.yaml +++ b/.github/runner-versions.yaml @@ -4,3 +4,4 @@ runner_versions: - "2.336.0" - "2.337.0" + - "2.338.0" diff --git a/packages/k8s/package-lock.json b/packages/k8s/package-lock.json index c352db67..5bebdcec 100644 --- a/packages/k8s/package-lock.json +++ b/packages/k8s/package-lock.json @@ -14,7 +14,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" @@ -1914,9 +1914,9 @@ } }, "node_modules/@istanbuljs/load-nyc-config/node_modules/js-yaml": { - "version": "3.14.1", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.14.1.tgz", - "integrity": "sha512-okMH7OXXJ7YrN9Ok3/SXrnu4iX9yOk+25nqX4imS2npuvTYDmo/QEZoqwZkYaIDk3jVvBOTOIEgEhaLOynBS9g==", + "version": "3.15.2", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.15.2.tgz", + "integrity": "sha512-6EuL879VkRA+1Cz578mKMiKvjPNEuk6+r1JaFzoSWejZmtf7xWbIyw1e3KkxlkzTIt9Taw6JBhEppG7utc1P+w==", "dev": true, "license": "MIT", "dependencies": { @@ -5074,9 +5074,9 @@ "license": "MIT" }, "node_modules/js-yaml": { - "version": "4.3.1", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.1.tgz", - "integrity": "sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ==", + "version": "4.3.2", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.2.tgz", + "integrity": "sha512-SFNOvSJ+Dgf/9An904Yx+CgSlIPCkIpao4qo51lpee25TIRejdH3rhR4EZMGoNx3/TP3O+wzWuiTFl4sqbltzA==", "funding": [ { "type": "github", diff --git a/packages/k8s/package.json b/packages/k8s/package.json index 5e82505a..03af76fb 100644 --- a/packages/k8s/package.json +++ b/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" diff --git a/packages/k8s/src/k8s/index.ts b/packages/k8s/src/k8s/index.ts index d5da2256..30b88042 100644 --- a/packages/k8s/src/k8s/index.ts +++ b/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 { - const sar = new k8s.V1SelfSubjectAccessReview() const asyncs: Promise[] = [] 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 { 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})` + ) ) } } diff --git a/packages/k8s/src/k8s/utils/index.ts b/packages/k8s/src/k8s/utils/index.ts index 4ad1d93b..97622488 100644 --- a/packages/k8s/src/k8s/utils/index.ts +++ b/packages/k8s/src/k8s/utils/index.ts @@ -298,3 +298,134 @@ export async function sleep(ms: number): Promise { 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( + promise: Promise, + timeoutMs: number | undefined, + description: string +): Promise { + if (!timeoutMs) { + return promise + } + return new Promise((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( + operation: () => Promise, + description: string, + options: RetryK8sApiCallOptions = {} +): Promise { + 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 +} diff --git a/packages/k8s/tests/retry-utils-test.ts b/packages/k8s/tests/retry-utils-test.ts new file mode 100644 index 00000000..7dba7461 --- /dev/null +++ b/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(() => 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) + }) +})