From 64fd97ddcfe4b8136ae14d2da7e533ee4619dd7c Mon Sep 17 00:00:00 2001 From: kurok <22548029+kurok@users.noreply.github.com> Date: Sun, 5 Jul 2026 14:01:11 +0100 Subject: [PATCH] fix: restore phone-home diagnostics in warm-pool download step buildReusableUserData()'s RUNNER_DOWNLOAD heredoc (the reuse: stop cold-boot path) dropped the ERR trap before entering the sudo -u runner shell and never re-included PHONE_HOME_HELPERS or re-armed the trap inside it, unlike buildUserData()'s plain-path RUNNER_BOOTSTRAP equivalent. Functions and traps don't survive the sudo -u boundary, so any curl/sha256sum/tar failure during the warm-pool download step was silent: no failed:downloading bootstrap tag, just a stall until the external GitHub registration timeout. Re-include PHONE_HOME_HELPERS inside the RUNNER_DOWNLOAD heredoc and re-arm the ERR trap for the downloading phase before the download/ verify sequence, mirroring the plain path line for line. Adds regression tests scoped to the RUNNER_DOWNLOAD heredoc body so they can't be satisfied by the outer shell's or register script's traps. Fixes #61 Signed-off-by: kurok <22548029+kurok@users.noreply.github.com> --- dist/index.js | 8 ++++++-- src/aws.js | 8 ++++++-- tests/warmpool.test.js | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/dist/index.js b/dist/index.js index 9fb4e0cf..94773fa0 100644 --- a/dist/index.js +++ b/dist/index.js @@ -105012,6 +105012,8 @@ function buildReusableUserData({ runnerVersion, owner, repo, label, githubRegist '#!/bin/bash', 'set -euo pipefail', ...PHONE_HOME_HELPERS, + 'GH_RUNNER_STEP=configuring', + 'trap \'gh_runner_phone_home "failed:${GH_RUNNER_STEP}"\' ERR', 'IMDS_TOKEN=$(curl -fsS -X PUT "http://169.254.169.254/latest/api/token" -H "X-aws-ec2-metadata-token-ttl-seconds: 120" 2>/dev/null || true)', 'UD=$(curl -fsS -H "X-aws-ec2-metadata-token: $IMDS_TOKEN" "http://169.254.169.254/latest/user-data" 2>/dev/null || true)', 'GH_TOKEN=$(printf \'%s\\n\' "$UD" | sed -n "s/^GH_TOKEN=\'\\(.*\\)\'$/\\1/p" | head -n1)', @@ -105019,8 +105021,6 @@ function buildReusableUserData({ runnerVersion, owner, repo, label, githubRegist 'GH_REPO_URL=$(printf \'%s\\n\' "$UD" | sed -n "s#^GH_REPO_URL=\'\\(.*\\)\'$#\\1#p" | head -n1)', 'cd /home/runner/actions-runner', 'rm -f .runner .credentials .credentials_rsaparams', - 'GH_RUNNER_STEP=configuring', - 'trap \'gh_runner_phone_home "failed:${GH_RUNNER_STEP}"\' ERR', 'gh_runner_phone_home configuring', 'sudo -u runner -H env DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1 ./config.sh --url "$GH_REPO_URL" --token "$GH_TOKEN" --labels "$GH_LABEL" --ephemeral --unattended --disableupdate', 'GH_RUNNER_STEP=registered', @@ -105070,6 +105070,10 @@ function buildReusableUserData({ runnerVersion, owner, repo, label, githubRegist 'trap - ERR', "sudo -u runner -H bash <<'RUNNER_DOWNLOAD'", 'set -euo pipefail', + ...PHONE_HOME_HELPERS, + 'GH_RUNNER_STEP=downloading', + "trap 'gh_runner_phone_home \"failed:${GH_RUNNER_STEP}\"' ERR", + 'gh_runner_phone_home downloading', 'cd "$HOME"', 'mkdir -p actions-runner && cd actions-runner', 'if [ ! -x ./run.sh ]; then', diff --git a/src/aws.js b/src/aws.js index 8aa1c4da..47646298 100644 --- a/src/aws.js +++ b/src/aws.js @@ -362,6 +362,8 @@ function buildReusableUserData({ runnerVersion, owner, repo, label, githubRegist '#!/bin/bash', 'set -euo pipefail', ...PHONE_HOME_HELPERS, + 'GH_RUNNER_STEP=configuring', + 'trap \'gh_runner_phone_home "failed:${GH_RUNNER_STEP}"\' ERR', 'IMDS_TOKEN=$(curl -fsS -X PUT "http://169.254.169.254/latest/api/token" -H "X-aws-ec2-metadata-token-ttl-seconds: 120" 2>/dev/null || true)', 'UD=$(curl -fsS -H "X-aws-ec2-metadata-token: $IMDS_TOKEN" "http://169.254.169.254/latest/user-data" 2>/dev/null || true)', 'GH_TOKEN=$(printf \'%s\\n\' "$UD" | sed -n "s/^GH_TOKEN=\'\\(.*\\)\'$/\\1/p" | head -n1)', @@ -369,8 +371,6 @@ function buildReusableUserData({ runnerVersion, owner, repo, label, githubRegist 'GH_REPO_URL=$(printf \'%s\\n\' "$UD" | sed -n "s#^GH_REPO_URL=\'\\(.*\\)\'$#\\1#p" | head -n1)', 'cd /home/runner/actions-runner', 'rm -f .runner .credentials .credentials_rsaparams', - 'GH_RUNNER_STEP=configuring', - 'trap \'gh_runner_phone_home "failed:${GH_RUNNER_STEP}"\' ERR', 'gh_runner_phone_home configuring', 'sudo -u runner -H env DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1 ./config.sh --url "$GH_REPO_URL" --token "$GH_TOKEN" --labels "$GH_LABEL" --ephemeral --unattended --disableupdate', 'GH_RUNNER_STEP=registered', @@ -420,6 +420,10 @@ function buildReusableUserData({ runnerVersion, owner, repo, label, githubRegist 'trap - ERR', "sudo -u runner -H bash <<'RUNNER_DOWNLOAD'", 'set -euo pipefail', + ...PHONE_HOME_HELPERS, + 'GH_RUNNER_STEP=downloading', + "trap 'gh_runner_phone_home \"failed:${GH_RUNNER_STEP}\"' ERR", + 'gh_runner_phone_home downloading', 'cd "$HOME"', 'mkdir -p actions-runner && cd actions-runner', 'if [ ! -x ./run.sh ]; then', diff --git a/tests/warmpool.test.js b/tests/warmpool.test.js index e260ff22..83ef7da5 100644 --- a/tests/warmpool.test.js +++ b/tests/warmpool.test.js @@ -127,6 +127,47 @@ describe('getInstanceCycles / setInstanceCycles', () => { describe('buildUserData — reuse: stop variant', () => { const args = { runnerVersion: '2.335.1', owner: 'o', repo: 'r', label: 'l', githubRegistrationToken: 'TOK', shaX64: 'x', shaArm64: 'a' }; + // Isolates the body of the sudo -u runner RUNNER_DOWNLOAD heredoc from the + // rest of the generated script, so assertions about what runs inside that + // runner-user shell can't be satisfied by matches elsewhere in the script + // (the outer root shell's own preparing trap, or the register script's + // configuring trap). + const extractRunnerDownloadHeredoc = (ud) => { + const startMarker = "sudo -u runner -H bash <<'RUNNER_DOWNLOAD'"; + const start = ud.indexOf(startMarker); + expect(start).toBeGreaterThan(-1); + const bodyStart = start + startMarker.length; + const end = ud.indexOf('\nRUNNER_DOWNLOAD', bodyStart); + expect(end).toBeGreaterThan(bodyStart); + return ud.slice(bodyStart, end); + }; + + test('re-includes PHONE_HOME_HELPERS inside the RUNNER_DOWNLOAD heredoc (regression, #61)', () => { + const ud = aws.buildUserData({ ...args, reuse: 'stop' }); + const heredoc = extractRunnerDownloadHeredoc(ud); + // Functions/traps set in the outer (root) shell do not survive the + // `sudo -u runner` boundary — the helpers must be redefined inside this + // heredoc, exactly as the plain-path RUNNER_BOOTSTRAP shell does. + expect(heredoc).toContain('gh_runner_imds() {'); + expect(heredoc).toContain('gh_runner_phone_home() {'); + expect(heredoc).toContain(`Key=${aws.BOOTSTRAP_TAG_KEY},Value=$1`); + }); + + test('arms an ERR trap for the downloading phase inside the RUNNER_DOWNLOAD heredoc (regression, #61)', () => { + const ud = aws.buildUserData({ ...args, reuse: 'stop' }); + const heredoc = extractRunnerDownloadHeredoc(ud); + const trapLine = "trap 'gh_runner_phone_home \"failed:${GH_RUNNER_STEP}\"' ERR"; + // Scoped to this heredoc alone: the outer shell's 'preparing' trap and + // the register script's 'configuring' trap live elsewhere in the script + // and must not be able to satisfy this assertion. + expect(heredoc).toContain(trapLine); + expect(heredoc).toContain('GH_RUNNER_STEP=downloading'); + // The step must be (re)tagged, and the trap (re)armed, before any of the + // curl/sha256sum/tar commands that can actually fail inside this shell. + expect(heredoc.indexOf('GH_RUNNER_STEP=downloading')).toBeLessThan(heredoc.indexOf(trapLine)); + expect(heredoc.indexOf(trapLine)).toBeLessThan(heredoc.indexOf('curl -fsSLo')); + }); + test('installs a per-boot systemd service + IMDS-read re-registration hook', () => { const ud = aws.buildUserData({ ...args, reuse: 'stop' }); expect(ud).toContain('/opt/gh-runner-register.sh');