mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
fix(ci): probe the host egress proxy so mainland release hosts can build the sandbox image (#16281)
The docker-container driver's buildkitd inherits neither the runner's proxy env nor the host network namespace. On ecs-qwen-hz1-17/18 the DNS answer for registry-1.docker.io is poisoned, so the pinned node:22-slim resolution dies with a direct-dial timeout before any application step — five consecutive image-build failures (#16039, #16162) that forced ECS_RELEASE_HOST_RUNNER_ENABLED=false. Probe the fleet-managed proxy marker and inject network=host plus env.*_PROXY driver opts only when the proxy answers a live registry probe. Hosted and HK runners have no marker file, so the probe emits nothing and their builds keep the default direct route. Verified live on hz1: unopted builder reproduces the incident, opted builder resolves the pinned digest for both platforms. Co-authored-by: Qwen-Coder <[email protected]>
This commit is contained in:
1 parent
db9478db8b
commit
5890329390
3 files changed
+385
No files matched your search
@@ -0,0 +1,88 @@
|
||||
#!/usr/bin/env bash
|
||||
# Emit docker-container driver opts that route buildkitd's own registry
|
||||
# traffic through the host's egress proxy — but only where that proxy
|
||||
# exists and answers. Prints nothing otherwise, so hosted runners and the
|
||||
# HK pool keep their default direct route.
|
||||
#
|
||||
# Why this exists (#16039, #16162): the docker-container driver runs
|
||||
# buildkitd in a container that inherits NEITHER the runner process's
|
||||
# proxy env NOR the host network namespace. On the mainland release hosts
|
||||
# the DNS answer for registry-1.docker.io is poisoned, so buildkitd's
|
||||
# direct dial times out while resolving the pinned node:22-slim base —
|
||||
# before any application build step. The host-level proxies (dockerd
|
||||
# systemd override, runner service env) cannot reach it:
|
||||
# env.* puts the proxy into buildkitd's own process environment,
|
||||
# which its registry resolver reads;
|
||||
# network=host makes 127.0.0.1:7890 inside the container reach the host
|
||||
# proxy instead of the container's own loopback.
|
||||
# Both opts are load-bearing; verified on ecs-qwen-hz1-17/18 where the
|
||||
# unopted probe fails with `dial tcp <poisoned IP>:443: i/o timeout` and
|
||||
# the opted build resolves the pinned digest for both platforms.
|
||||
#
|
||||
# The decision is fail-closed: a configured-but-broken proxy must not
|
||||
# capture buildkitd, and a host without the marker file must not receive
|
||||
# opts pointing at a dead port. The probe log goes to stderr so the next
|
||||
# broken build has the measurement in its step log (#16218 precedent).
|
||||
set -euo pipefail
|
||||
|
||||
# Overridable for the test replay; production reads the fleet-managed file.
|
||||
PROXY_ENV_FILE="${QWEN_PROXY_ENV_FILE:-/etc/qwen-proxy-client/proxy.env}"
|
||||
|
||||
note() { echo "buildkitd proxy: $*" >&2; }
|
||||
|
||||
# Unreadable counts as absent: under set -e a failing read would kill the
|
||||
# whole build instead of falling back to the default direct route.
|
||||
if [ ! -f "${PROXY_ENV_FILE}" ] || [ ! -r "${PROXY_ENV_FILE}" ]; then
|
||||
note "none (no readable ${PROXY_ENV_FILE})"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
proxy_url="$(sed -n 's/^https_proxy=//p' "${PROXY_ENV_FILE}" 2> /dev/null | head -n 1 || true)"
|
||||
# Single-line http(s) URL only, with no CSV metacharacters: buildx parses
|
||||
# each driver-opt as CSV (csvToMap), so a comma or double quote in the
|
||||
# value could split one emitted line into injected options — or fail
|
||||
# builder creation outright. Whitespace is excluded for the same reason.
|
||||
url_re='^https?://[^[:space:],"]+$'
|
||||
if ! [[ "${proxy_url}" =~ ${url_re} ]]; then
|
||||
note "none (no usable https_proxy in ${PROXY_ENV_FILE})"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
if ! command -v curl > /dev/null 2>&1; then
|
||||
note "none (curl unavailable)"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# 401 is the healthy unauthenticated answer from the real registry; 200
|
||||
# would prove reachability too. Anything else (000 timeout, 5xx) leaves
|
||||
# buildkitd on its default direct route rather than aiming it at a proxy
|
||||
# that cannot carry the traffic.
|
||||
# `--noproxy ''` is load-bearing: curl consults the no-proxy list after
|
||||
# picking a proxy, so an inherited NO_PROXY/no_proxy matching the registry
|
||||
# (including `*`) would send this probe direct and let a dead proxy pass as
|
||||
# healthy. Clearing the list for this one invocation is the only lever here;
|
||||
# a NO_PROXY driver-opt is not an option (see the note below).
|
||||
code="$(curl -s -o /dev/null --max-time 5 --noproxy '' -x "${proxy_url}" \
|
||||
-w '%{http_code}' 'https://registry-1.docker.io/v2/' 2> /dev/null || true)"
|
||||
case "${code}" in
|
||||
200 | 401) ;;
|
||||
*)
|
||||
note "none (registry probe via ${proxy_url} returned ${code:-000})"
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
|
||||
note "${proxy_url} (registry probe ${code})"
|
||||
# No NO_PROXY line on purpose: buildx CSV-parses each driver-opt, and a
|
||||
# comma list breaks builder creation both unquoted ("expecting k=v") and
|
||||
# value-quoted ("bare \" in non-quoted-field") — both forms rejected by
|
||||
# buildx v0.38.1 live. buildkitd only dials external registries, so a
|
||||
# loopback exclusion list buys nothing. Every emitted line is free of
|
||||
# CSV metacharacters: the URL validator above excludes commas/quotes.
|
||||
cat << EOF
|
||||
network=host
|
||||
env.HTTP_PROXY=${proxy_url}
|
||||
env.HTTPS_PROXY=${proxy_url}
|
||||
env.http_proxy=${proxy_url}
|
||||
env.https_proxy=${proxy_url}
|
||||
EOF
|
||||
@@ -96,11 +96,28 @@ jobs:
|
||||
echo "Publish input: ${{ github.event.inputs.publish }}"
|
||||
echo "GitHub ref: ${{ github.ref }}"
|
||||
|
||||
# buildkitd (docker-container driver) inherits neither the runner's
|
||||
# proxy env nor the host network; on the mainland release hosts its
|
||||
# direct registry dial hits poisoned DNS and the image build dies
|
||||
# before any application step (#16039, #16162). The probe emits
|
||||
# driver opts only where a working local proxy exists, so hosted and
|
||||
# HK runners keep the default direct route.
|
||||
- name: 'Detect egress proxy for buildkitd'
|
||||
id: 'buildkit_proxy'
|
||||
run: |
|
||||
{
|
||||
echo 'driver_opts<<EOF'
|
||||
bash .github/scripts/buildkitd-proxy-opts.sh
|
||||
echo 'EOF'
|
||||
} >> "$GITHUB_OUTPUT"
|
||||
|
||||
- name: 'Set up QEMU'
|
||||
uses: 'docker/setup-qemu-action@v4' # ratchet:exclude
|
||||
|
||||
- name: 'Set up Docker Buildx'
|
||||
uses: 'docker/setup-buildx-action@v4' # ratchet:exclude
|
||||
with:
|
||||
driver-opts: '${{ steps.buildkit_proxy.outputs.driver_opts }}'
|
||||
|
||||
- name: 'Extract metadata (tags, labels) for Docker'
|
||||
id: 'meta'
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
existsSync,
|
||||
mkdtempSync,
|
||||
readFileSync,
|
||||
symlinkSync,
|
||||
writeFileSync,
|
||||
} from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
@@ -36,6 +37,14 @@ const retryStep =
|
||||
workflow.match(
|
||||
/- name: 'Build and push Docker image \(retry\)'[\s\S]*?(?=\n[ ]{2}# One issue per version)/,
|
||||
)?.[0] ?? '';
|
||||
const proxyDetectStep =
|
||||
workflow.match(
|
||||
/- name: 'Detect egress proxy for buildkitd'[\s\S]*?(?=\n[ ]{6}- name: 'Set up QEMU')/,
|
||||
)?.[0] ?? '';
|
||||
const buildxStep =
|
||||
workflow.match(
|
||||
/- name: 'Set up Docker Buildx'[\s\S]*?(?=\n[ ]{6}- name: 'Extract metadata)/,
|
||||
)?.[0] ?? '';
|
||||
const failureIssueJob =
|
||||
workflow.match(/file-failure-issue:[\s\S]*$/)?.[0] ?? '';
|
||||
const buildJob =
|
||||
@@ -185,6 +194,40 @@ describe('build-and-publish-image workflow', () => {
|
||||
expect(lookupScript).toContain('jq -r --arg marker_html "${MARKER_HTML}"');
|
||||
expect(lookupScript).toContain('contains($marker_html)');
|
||||
});
|
||||
|
||||
it('feeds buildx a probed proxy decision, never a pinned proxy', () => {
|
||||
// #16039/#16162: the docker-container driver's buildkitd inherits
|
||||
// neither the runner's proxy env nor the host network, so on mainland
|
||||
// release hosts its registry dial hits poisoned DNS and the image
|
||||
// build dies before any application step. The fix must stay a probe:
|
||||
// hosted and HK runners have clean direct egress and NO local proxy,
|
||||
// so a hardcoded 127.0.0.1:7890 in the YAML would aim their buildkitd
|
||||
// at a dead port and break the paths that work today.
|
||||
expect(proxyDetectStep).toContain("id: 'buildkit_proxy'");
|
||||
// One ordered pin instead of four independent substrings: hoisting the
|
||||
// probe call out of the `{ ... } >> "$GITHUB_OUTPUT"` group leaves
|
||||
// driver_opts empty while each substring pin stays green. The script's
|
||||
// stdout must stay inside the group; its probe log goes to stderr and
|
||||
// stays in the step log.
|
||||
expect(proxyDetectStep).toContain(
|
||||
[
|
||||
'run: |',
|
||||
' {',
|
||||
" echo 'driver_opts<<EOF'",
|
||||
' bash .github/scripts/buildkitd-proxy-opts.sh',
|
||||
" echo 'EOF'",
|
||||
' } >> "$GITHUB_OUTPUT"',
|
||||
].join('\n'),
|
||||
);
|
||||
expect(buildxStep).toContain(
|
||||
"driver-opts: '${{ steps.buildkit_proxy.outputs.driver_opts }}'",
|
||||
);
|
||||
// The decision must exist before the builder that consumes it.
|
||||
expect(
|
||||
workflow.indexOf("'Detect egress proxy for buildkitd'"),
|
||||
).toBeLessThan(workflow.indexOf("'Set up Docker Buildx'"));
|
||||
expect(workflow).not.toContain('127.0.0.1:7890');
|
||||
});
|
||||
});
|
||||
|
||||
// Replay the script under a recording gh stub instead of pinning only the
|
||||
@@ -720,3 +763,240 @@ describe.skipIf(!replayable)(
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
// Replay the proxy probe under a recording curl stub. The decision is
|
||||
// fail-closed: anything short of a reachable registry through a usable
|
||||
// local proxy must emit NOTHING, because an injected-but-dead proxy breaks
|
||||
// the hosted/HK builds that work today (#16039/#16162). Text pins cannot
|
||||
// see that contract; the replay can. POSIX paths and an extensionless curl
|
||||
// stub keep this off the Windows lane, same as the gh-stub replay above.
|
||||
describe.skipIf(process.platform === 'win32')(
|
||||
'buildkitd-proxy-opts script behavior',
|
||||
() => {
|
||||
const runProbe = ({
|
||||
proxyEnv,
|
||||
curlCode = '401',
|
||||
curlExit = 0,
|
||||
noCurl = false,
|
||||
}) => {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'buildkitd-proxy-'));
|
||||
const callsLog = join(dir, 'curl-calls.log');
|
||||
if (!noCurl) {
|
||||
writeFileSync(
|
||||
join(dir, 'curl'),
|
||||
[
|
||||
'#!/bin/bash',
|
||||
// One token per NUL: `$*` joins argv with single spaces, which
|
||||
// hides a deleted empty argument (`--noproxy ''`) from every
|
||||
// assertion written against the joined line.
|
||||
'printf "%s\\0" "$@" >> "' + callsLog + '"',
|
||||
`printf '%s' '${curlCode}'`,
|
||||
`exit ${curlExit}`,
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
chmodSync(join(dir, 'curl'), 0o755);
|
||||
} else {
|
||||
// A PATH carrying the script's other tools but no curl: the
|
||||
// curl-absent branch must fail closed, not crash under set -e.
|
||||
for (const tool of ['bash', 'sed', 'head', 'cat']) {
|
||||
const located = spawnSync('which', [tool], {
|
||||
encoding: 'utf8',
|
||||
}).stdout.trim();
|
||||
symlinkSync(located, join(dir, tool));
|
||||
}
|
||||
}
|
||||
const envFile = join(dir, 'proxy.env');
|
||||
if (proxyEnv !== undefined) writeFileSync(envFile, proxyEnv);
|
||||
const result = spawnSync(
|
||||
'bash',
|
||||
['.github/scripts/buildkitd-proxy-opts.sh'],
|
||||
{
|
||||
encoding: 'utf8',
|
||||
env: {
|
||||
PATH: noCurl ? dir : dir + ':' + (process.env.PATH ?? ''),
|
||||
QWEN_PROXY_ENV_FILE: envFile,
|
||||
},
|
||||
},
|
||||
);
|
||||
const calls = existsSync(callsLog) ? readFileSync(callsLog, 'utf8') : '';
|
||||
return {
|
||||
status: result.status,
|
||||
out: result.stdout,
|
||||
err: result.stderr,
|
||||
// Raw NUL-separated log, kept as a string so the "no probe ran"
|
||||
// cases can still assert ''.
|
||||
calls,
|
||||
// One token per entry: an empty argument survives the round trip, so
|
||||
// deleting the `''` value of `--noproxy` cannot pass unnoticed.
|
||||
argv: calls === '' ? [] : calls.split('\0').slice(0, -1),
|
||||
};
|
||||
};
|
||||
|
||||
it('emits host-network proxy opts when the probe reaches the registry', () => {
|
||||
const result = runProbe({
|
||||
proxyEnv:
|
||||
'http_proxy=http://127.0.0.1:7890\nhttps_proxy=http://127.0.0.1:7890\n',
|
||||
});
|
||||
expect(result.status).toBe(0);
|
||||
// network=host is load-bearing: on the default bridge the injected
|
||||
// 127.0.0.1:7890 would point at the container's own loopback, not the
|
||||
// host proxy. Both spellings of each variable ride along because
|
||||
// resolvers disagree about which case they read. No NO_PROXY line:
|
||||
// buildx CSV-parses driver-opts and rejects comma lists both unquoted
|
||||
// and value-quoted (both verified live); buildkitd only dials
|
||||
// external registries, so the exclusion list buys nothing.
|
||||
expect(result.out).toBe(
|
||||
[
|
||||
'network=host',
|
||||
'env.HTTP_PROXY=http://127.0.0.1:7890',
|
||||
'env.HTTPS_PROXY=http://127.0.0.1:7890',
|
||||
'env.http_proxy=http://127.0.0.1:7890',
|
||||
'env.https_proxy=http://127.0.0.1:7890',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
// Exact argv, token by token. The probe must aim at the exact endpoint
|
||||
// whose poisoned resolution caused the incident, through the exact
|
||||
// proxy it will inject, and `--noproxy ''` must stay adjacent to `-x`:
|
||||
// an inherited NO_PROXY/no_proxy matching the registry would make curl
|
||||
// dial direct while still printing 401, so a dead proxy would pass.
|
||||
// `-o /dev/null` and `--max-time 5` are held here for the same reason:
|
||||
// deleting either lets a chatty or hung probe report a healthy code.
|
||||
// The empty string also proves the log kept argv shape instead of
|
||||
// `$*`-joining that value away.
|
||||
expect(result.argv).toEqual([
|
||||
'-s',
|
||||
'-o',
|
||||
'/dev/null',
|
||||
'--max-time',
|
||||
'5',
|
||||
'--noproxy',
|
||||
'',
|
||||
'-x',
|
||||
'http://127.0.0.1:7890',
|
||||
'-w',
|
||||
'%{http_code}',
|
||||
'https://registry-1.docker.io/v2/',
|
||||
]);
|
||||
// The measurement reaches the step log, or the next broken build is
|
||||
// another manual SSH investigation (#16218 precedent).
|
||||
expect(result.err).toContain('buildkitd proxy:');
|
||||
expect(result.err).toContain('401');
|
||||
});
|
||||
|
||||
it('keeps the fleet path as the default when the override is absent', () => {
|
||||
// Every replay above injects QWEN_PROXY_ENV_FILE, so nothing read the
|
||||
// production default. It cannot be replayed: the fleet manages that
|
||||
// absolute path and a self-hosted host may legitimately have it, so a
|
||||
// replay asserting "missing" would misread such a host as a defect.
|
||||
// The literal is therefore the deterministic half — the override stays
|
||||
// optional and points at the fleet file when unset.
|
||||
const source = readFileSync(
|
||||
'.github/scripts/buildkitd-proxy-opts.sh',
|
||||
'utf8',
|
||||
);
|
||||
expect(source).toContain(
|
||||
'PROXY_ENV_FILE="${QWEN_PROXY_ENV_FILE:-/etc/qwen-proxy-client/proxy.env}"',
|
||||
);
|
||||
});
|
||||
|
||||
it('accepts a 200 probe answer as well', () => {
|
||||
const result = runProbe({
|
||||
proxyEnv: 'https_proxy=http://127.0.0.1:7890\n',
|
||||
curlCode: '200',
|
||||
});
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.out).toContain('network=host');
|
||||
});
|
||||
|
||||
it('stays silent when the marker file does not exist', () => {
|
||||
// Hosted runners and the HK pool: no proxy configured, direct egress
|
||||
// is clean. Emitting opts here would break them.
|
||||
const result = runProbe({ proxyEnv: undefined });
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.out).toBe('');
|
||||
expect(result.calls).toBe('');
|
||||
});
|
||||
|
||||
it('stays silent when the proxy cannot reach the registry', () => {
|
||||
// A configured-but-broken proxy must not capture buildkitd: fail
|
||||
// closed to the default direct route (the pre-fix behavior), not
|
||||
// into a dead proxy.
|
||||
const result = runProbe({
|
||||
proxyEnv: 'https_proxy=http://127.0.0.1:7890\n',
|
||||
curlCode: '000',
|
||||
curlExit: 28,
|
||||
});
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.out).toBe('');
|
||||
expect(result.err).toContain('buildkitd proxy: none');
|
||||
});
|
||||
|
||||
it('stays silent on a 5xx probe answer', () => {
|
||||
// A proxy that answers but cannot carry registry traffic is no
|
||||
// better than a dead one for this decision.
|
||||
const result = runProbe({
|
||||
proxyEnv: 'https_proxy=http://127.0.0.1:7890\n',
|
||||
curlCode: '502',
|
||||
});
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.out).toBe('');
|
||||
expect(result.err).toContain('502');
|
||||
});
|
||||
|
||||
it('stays silent when curl is unavailable', () => {
|
||||
const result = runProbe({
|
||||
proxyEnv: 'https_proxy=http://127.0.0.1:7890\n',
|
||||
noCurl: true,
|
||||
});
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.out).toBe('');
|
||||
expect(result.err).toContain('curl unavailable');
|
||||
});
|
||||
|
||||
it('stays silent when the file has no usable https_proxy value', () => {
|
||||
const result = runProbe({
|
||||
proxyEnv: 'HTTP_PROXY=http://127.0.0.1:7890\n',
|
||||
});
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.out).toBe('');
|
||||
expect(result.calls).toBe('');
|
||||
});
|
||||
|
||||
it('rejects a value that could inject extra driver opts', () => {
|
||||
// The value lands verbatim in driver-opts lines; whitespace or a
|
||||
// non-URL shape must disqualify it instead of emitting more options.
|
||||
const result = runProbe({
|
||||
proxyEnv: 'https_proxy=http://127.0.0.1:7890 env.evil=1\n',
|
||||
});
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.out).toBe('');
|
||||
expect(result.calls).toBe('');
|
||||
});
|
||||
|
||||
it('rejects a comma that would CSV-split the emitted opt', () => {
|
||||
// buildx parses each driver-opt as CSV. A comma riding in the URL
|
||||
// userinfo still probes fine (the host is after the @), but the
|
||||
// emitted env line would split into an injected env.EVIL opt plus a
|
||||
// truncated proxy value — so the validator must exclude commas.
|
||||
const result = runProbe({
|
||||
proxyEnv: 'https_proxy=http://u,[email protected]:7890\n',
|
||||
});
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.out).toBe('');
|
||||
expect(result.calls).toBe('');
|
||||
});
|
||||
|
||||
it('rejects a double quote that would break CSV parsing', () => {
|
||||
// A bare quote in the value makes buildx's CSV reader error out and
|
||||
// fails builder creation; disqualify it at the probe instead.
|
||||
const result = runProbe({
|
||||
proxyEnv: 'https_proxy=http://127.0.0.1:7890"\n',
|
||||
});
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.out).toBe('');
|
||||
expect(result.calls).toBe('');
|
||||
});
|
||||
},
|
||||
);
|
||||
Reference in new issue
Block a user