From: Arran Cudbard-Bell Date: Sun, 26 Jul 2026 01:19:53 +0000 (-0600) Subject: ci: collect core dumps as artifacts, keep them out of the profiling publish X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f095dcff55564e4e97c74fdc5c67b7be8d4b4bff;p=thirdparty%2Ffreeradius-server.git ci: collect core dumps as artifacts, keep them out of the profiling publish Most suites run with allow_core_dumps = yes, but nothing in CI ever collected a core, so the only record of a crash the logs cannot reconstruct was thrown away with the runner. scripts/ci/collect-core-dumps.sh stages cores into ci-core-dumps/, preserving each path below the searched root so the suite that crashed stays identifiable. The new .github/actions/collect-core-dumps composite action calls the script and uploads the result, and ci, ci-sanitizers, ci-macos, ci-deb, ci-rpm and ci-multi-server-tests all use it. Collection runs on success as well as failure: FreeRADIUS crashing during shutdown does not necessarily fail the suite that provoked the crash, which is how run 30180044334 reported success while dumping two 1.5G cores. publish-profiling-results.sh now drops cores from the store tarball. Those two cores turned a 6M publish into 35M of mostly-zero pages, the store's synchronous ingest then ran ~64s, and wproxy's 60s proxy_read_timeout returned a 504 for a publish that had in fact landed. The multi-server leg passes move: true so a core leaves prof-results/ and rides one artifact rather than inflating the prof-results artifact too. Exclusions are always logged with their sizes; a silently shrunk publish would be worse than a noisy one. The shared name patterns live in scripts/ci/core-dump-names.inc so the script that keeps cores and the script that drops them cannot disagree. Not covered: ci-freebsd, whose VM syncs runner-to-vm only, and the fuzzing leg, which has its own crash artifacts. Whether a core is written at all still depends on the ambient RLIMIT_CORE and kernel.core_pattern, neither of which CI sets; allow_core_dumps only restores the limit FreeRADIUS inherited. --- diff --git a/.github/actions/collect-core-dumps/action.yml b/.github/actions/collect-core-dumps/action.yml new file mode 100644 index 00000000000..d553443ea32 --- /dev/null +++ b/.github/actions/collect-core-dumps/action.yml @@ -0,0 +1,80 @@ +name: collect-core-dumps + +# +# Stage any core dump the job produced and upload it as an artifact. +# +# Most of our suites run with allow_core_dumps = yes, and a core is the only +# record of a crash the logs cannot reconstruct. Nothing collected them +# before, so they were discarded with the runner. +# +# Call with `if: ${{ always() }}`, for two reasons: a crashing suite has +# usually failed by the time this runs, and a crash on shutdown does not +# necessarily fail the suite that provoked it (multi-server profiling reported +# success on 2026-07-25 while dumping two 1.5G cores). +# +# Whether a core is written at all still depends on the ambient RLIMIT_CORE +# and kernel.core_pattern, neither of which CI sets. allow_core_dumps = yes +# only restores the limit FreeRADIUS inherited (fr_set_dumpable() in +# src/lib/util/debug.c), it does not raise it. So this collects whatever was +# produced rather than guaranteeing a core exists. +# + +inputs: + name: + description: > + Artifact name suffix, giving "core-dumps-". Pass whatever + distinguishes this matrix leg, since names must be unique within a run. + required: true + roots: + description: > + Directories to search, space separated, relative to working-directory. + Pass ". /cores" on macOS, which writes cores there. + default: "." + move: + description: > + Move cores out of the searched tree rather than copying them, for when + the tree is itself uploaded or published. Action inputs are strings, so + this is "true"/"false" text however the caller writes it. + default: "false" + working-directory: + description: > + Where to run the collection, relative to the workspace root. Use + "freeradius" for the jobs that check out into a subdirectory. + default: "." + retention-days: + description: How long to keep the artifact. + default: "30" + +runs: + using: composite + + steps: + + # collect-core-dumps.sh always exits 0: a leg's result is decided by its + # tests, not by whether core collection found anything. + - name: Collect core dumps + shell: bash + working-directory: ${{ inputs.working-directory }} + run: | + set -eu + # Accept either spelling rather than betting on how YAML stringifies a + # bare true/false, and reject anything else instead of silently not + # moving. + case "${{ inputs.move }}" in + true|True|TRUE) args="-m" ;; + false|False|FALSE) args="" ;; + *) + echo "collect-core-dumps: move must be true or false, got '${{ inputs.move }}'" >&2 + exit 1 + ;; + esac + # inputs.roots is deliberately unquoted: it is a space separated list. + ./scripts/ci/collect-core-dumps.sh $args ${{ inputs.roots }} + + - name: Upload core dumps + uses: actions/upload-artifact@v6 + with: + name: core-dumps-${{ inputs.name }} + path: ${{ inputs.working-directory }}/ci-core-dumps/ + if-no-files-found: ignore + retention-days: ${{ inputs.retention-days }} diff --git a/.github/workflows/ci-deb.yml b/.github/workflows/ci-deb.yml index 286e25248fe..92b18ae62a8 100644 --- a/.github/workflows/ci-deb.yml +++ b/.github/workflows/ci-deb.yml @@ -253,6 +253,15 @@ jobs: make -f scripts/ci/package-test.mk package-test working-directory: freeradius + # This job checks out into freeradius/, so the action lives below that path + # and the collection runs from there. + - name: Collect core dumps + if: ${{ always() }} + uses: ./freeradius/.github/actions/collect-core-dumps + with: + name: ${{ matrix.env.NAME }} + working-directory: freeradius + - name: Upload radius logs on failure if: ${{ failure() }} uses: actions/upload-artifact@v6 diff --git a/.github/workflows/ci-macos.yml b/.github/workflows/ci-macos.yml index 03f234e9e5a..fd00c3ba22d 100644 --- a/.github/workflows/ci-macos.yml +++ b/.github/workflows/ci-macos.yml @@ -208,6 +208,15 @@ jobs: env: ASAN_OPTIONS: symbolize=1 detect_stack_use_after_return=1 + # macOS writes cores to /cores rather than next to the crashing process, + # so search that directory as well as the checkout. + - name: Collect core dumps + if: ${{ always() }} + uses: ./.github/actions/collect-core-dumps + with: + name: ${{ matrix.env.NAME }} + roots: ". /cores" + # # If the CI has failed and the branch is ci-debug then we start a tmate # session to provide interactive shell access to the session. diff --git a/.github/workflows/ci-multi-server-tests.yml b/.github/workflows/ci-multi-server-tests.yml index c4c639928c0..c525d580e53 100644 --- a/.github/workflows/ci-multi-server-tests.yml +++ b/.github/workflows/ci-multi-server-tests.yml @@ -205,6 +205,16 @@ jobs: ;; esac + # Runs before both uploads below, with move: true so a core leaves + # prof-results/ and rides one artifact instead of inflating the + # prof-results artifact and the store publish as well. + - name: Collect core dumps + if: ${{ always() }} + uses: ./.github/actions/collect-core-dumps + with: + name: ${{ matrix.mode }}-${{ github.sha }} + move: "true" + # # Stash callgrind output as a workflow artifact in profiling # mode. Skipped in service mode (nothing to upload). diff --git a/.github/workflows/ci-rpm.yml b/.github/workflows/ci-rpm.yml index f0608094832..172a2a5a5c1 100644 --- a/.github/workflows/ci-rpm.yml +++ b/.github/workflows/ci-rpm.yml @@ -235,6 +235,15 @@ jobs: make -f scripts/ci/package-test.mk package-test working-directory: freeradius + # This job checks out into freeradius/, so the action lives below that path + # and the collection runs from there. + - name: Collect core dumps + if: ${{ always() }} + uses: ./freeradius/.github/actions/collect-core-dumps + with: + name: ${{ matrix.env.NAME }} + working-directory: freeradius + - name: Upload radius logs on failure if: ${{ failure() }} uses: actions/upload-artifact@v6 diff --git a/.github/workflows/ci-sanitizers.yml b/.github/workflows/ci-sanitizers.yml index e8f0de90d0f..13b29104563 100644 --- a/.github/workflows/ci-sanitizers.yml +++ b/.github/workflows/ci-sanitizers.yml @@ -203,6 +203,12 @@ jobs: smtp_test_server: 127.0.0.1 smtp_test_server_port: 2525 + - name: Collect core dumps + if: ${{ always() }} + uses: ./.github/actions/collect-core-dumps + with: + name: ${{ matrix.os.name }}-${{ matrix.env.NAME }} + # # If the CI has failed and the branch is ci-debug then we start a tmate # session to provide interactive shell access to the session. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 656b32e4a91..11d82994d43 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -212,6 +212,12 @@ jobs: smtp_test_server: 127.0.0.1 smtp_test_server_port: 2525 + - name: Collect core dumps + if: ${{ always() }} + uses: ./.github/actions/collect-core-dumps + with: + name: ${{ matrix.os.name }}-${{ matrix.env.NAME }} + - name: Run fuzzer uses: ./.github/actions/fuzzer diff --git a/.gitignore b/.gitignore index 42f4e66905a..0921e95c11a 100644 --- a/.gitignore +++ b/.gitignore @@ -101,3 +101,6 @@ build-2f* # Profiling results prof-results/ + +# Core dumps staged for upload by scripts/ci/collect-core-dumps.sh +ci-core-dumps/ diff --git a/scripts/ci/collect-core-dumps.sh b/scripts/ci/collect-core-dumps.sh new file mode 100755 index 00000000000..29bd2d3be2d --- /dev/null +++ b/scripts/ci/collect-core-dumps.sh @@ -0,0 +1,142 @@ +#!/bin/sh +# +# Stage any core dumps found under the given roots into one directory, so a +# workflow can upload them as an artifact. +# +# Most of our test suites run with "allow_core_dumps = yes", so a crash can +# leave a core behind. A core is the only record of a crash that the logs +# cannot reconstruct, and nothing in CI collected them before, so they were +# discarded with the runner. Collection runs regardless of whether the suite +# passed: FreeRADIUS crashing during shutdown does not necessarily fail the +# suite that provoked the crash (multi-server profiling did exactly that on +# 2026-07-25, reporting success while dumping two 1.5G cores). +# +# Whether a core is written at all still depends on the ambient RLIMIT_CORE +# and kernel.core_pattern, which CI does not currently set; +# "allow_core_dumps = yes" only restores the limit FreeRADIUS inherited +# (fr_set_dumpable() in src/lib/util/debug.c), it does not raise it. So this +# script collects what happens to be produced rather than guaranteeing a core +# exists. +# +# Never fails: a CI leg's result is decided by its tests, not by whether core +# collection found anything. Always reports what it staged, so an empty result +# reads as genuinely empty rather than as a step that quietly did nothing. +# + +set -u + +self_dir=$(dirname "$0") +outdir=ci-core-dumps +move=no + +usage() +{ + cat <] [root ...] + +Stage core dumps found under each into , preserving their path +below the root so the suite that crashed stays identifiable. Always exits 0. + + -o Where to stage cores. Default "ci-core-dumps". + -m Move cores rather than copying them. Use when a root is itself + uploaded or published, so a core lands in one place only. + -h Show this help. + root ... Directories to search. Default the current directory. + +Examples: + ${0##*/} # search the working tree + ${0##*/} . /cores # also search /cores, as macOS uses + ${0##*/} -m prof-results # empty cores out of a tree due to be uploaded +EOF +} + +while [ $# -gt 0 ]; do + case $1 in + -o) + [ $# -ge 2 ] || { echo "${0##*/}: -o needs a directory" >&2; exit 2; } + outdir=$2 + shift 2 + ;; + -m) + move=yes + shift + ;; + -h|--help) + usage + exit 0 + ;; + --) + shift + break + ;; + -*) + echo "${0##*/}: unknown option $1" >&2 + usage >&2 + exit 2 + ;; + *) + break + ;; + esac +done + +# Default to the working tree. Tests run with their cwd inside the checkout, +# and a relative kernel.core_pattern writes the core next to the crashing +# process, so the checkout is where a core normally lands. +[ $# -gt 0 ] || set -- . + +. "$self_dir/core-dump-names.inc" + +found=0 +staged=0 + +for root in "$@"; do + [ -d "$root" ] || continue + + # A core can be big enough that a second copy matters on a runner disk, + # so link it into place where the filesystem allows and only copy as a + # fallback. + for core in $(find "$root" -type f 2>/dev/null | grep -E "$core_dump_re" | sort); do + found=$((found + 1)) + + # Guard against a stray text file called "core": upload only what + # really is a dump. Without file(1) the name is all we have, so + # accept the candidate rather than discard possible evidence. + if command -v file >/dev/null 2>&1; then + case $(file -b "$core" 2>/dev/null) in + *core*) ;; + *) + echo "skipping $core (not a core dump)" + continue + ;; + esac + fi + + # Keep the path below the root, so which suite crashed stays + # readable in the artifact. + rel=$(printf '%s\n' "$core" | sed -e 's#^\./##' -e 's#^/##') + dest=$outdir/$rel + mkdir -p "$(dirname "$dest")" || continue + + if [ "$move" = yes ]; then + mv "$core" "$dest" || continue + else + ln "$core" "$dest" 2>/dev/null || cp "$core" "$dest" || continue + fi + + staged=$((staged + 1)) + echo "staged $(du -h "$dest" | cut -f1 | tr -d ' ') $rel" + done +done + +if [ "$staged" -gt 0 ]; then + echo "collected $staged core dump(s) into $outdir ($(du -sh "$outdir" | cut -f1 | tr -d ' ') total)" +else + echo "no core dumps found under: $*" +fi + +if [ "$found" -ne "$staged" ]; then + echo "note: $((found - staged)) candidate(s) were skipped, see above" +fi + +exit 0 diff --git a/scripts/ci/core-dump-names.inc b/scripts/ci/core-dump-names.inc new file mode 100644 index 00000000000..0922159fa29 --- /dev/null +++ b/scripts/ci/core-dump-names.inc @@ -0,0 +1,22 @@ +# +# Core dump name patterns. Sourced, not run: no shebang, not executable. +# +# Shared by collect-core-dumps.sh, which keeps cores, and +# publish-profiling-results.sh, which drops them, so the two can never +# disagree about what a core file looks like. +# +# The patterns expect a "/" before the basename, which every "find" result +# under a root has. Basenames covered: "core", "core.", ".core", +# ".core.", and valgrind's "vgcore.". +# +# One pattern per line rather than the equivalent single expression +# "/([^/]*\.)?(vg)?core(\.[^/]+)?$". That compact form is valid ERE and works +# under GNU and BSD grep, but ugrep matches nothing against it and says +# nothing about why. A silent miss here either leaks gigabyte cores into a +# publish or throws away the only evidence of a crash. +# +core_dump_re='/core$ +/core\.[^/]+$ +/[^/]*\.core$ +/[^/]*\.core\.[^/]+$ +/vgcore\.[^/]+$' diff --git a/scripts/ci/publish-profiling-results.sh b/scripts/ci/publish-profiling-results.sh index e81c52c4dab..8d2e9039465 100755 --- a/scripts/ci/publish-profiling-results.sh +++ b/scripts/ci/publish-profiling-results.sh @@ -9,11 +9,38 @@ # automatically when the job has "id-token: write"; the script uses them to # mint an OIDC token whose audience is the target URL's origin. # -# Usage: publish-profiling-results.sh set -eu -[ $# -eq 1 ] || { echo "usage: $0 " >&2; exit 2; } +usage() +{ + cat < + +Tar prof-results/ and POST the tarball to with a GitHub OIDC token. +Core dumps are excluded; collect-core-dumps.sh keeps those instead. Run from +the directory holding prof-results/. A missing prof-results/ tree, or one +holding nothing publishable, is a quiet success. + + Where to POST. Its origin becomes the OIDC audience. + -h Show this help. + +Needs ACTIONS_ID_TOKEN_REQUEST_TOKEN and ACTIONS_ID_TOKEN_REQUEST_URL, which +the runner sets when the job has "id-token: write". + +Example: + ${0##*/} https://cinfra-ca.inkbridge.io/profiling/data +EOF +} + +case ${1:-} in +-h|--help) + usage + exit 0 + ;; +esac + +[ $# -eq 1 ] || { usage >&2; exit 2; } url=$1 # The OIDC audience is the URL's origin (scheme://host). @@ -27,11 +54,38 @@ trap 'rm -rf "$tmpdir"' EXIT # Tar an explicit file list (not the directory) so empty directories and # editor droppings never travel. +all_list="$tmpdir/all" +core_list="$tmpdir/cores" file_list="$tmpdir/files" tarball="$tmpdir/prof-results.tar.gz" -find prof-results -type f ! -name '.DS_Store' | sed 's#^prof-results/##' >"$file_list" +# Paths are kept with a leading "/" while filtering, so a basename is always +# preceded by "/" and the pattern below needs no "(^|/)" alternation, which +# not every grep implementation accepts. The slash comes off again when the +# tar file list is written. +find prof-results -type f ! -name '.DS_Store' | sed 's#^prof-results##' | sort >"$all_list" + +# Drop core dumps. A crash under valgrind dumps the whole address space, so +# one core is ~1.5G; two of them turned a 6M publish into 35M of mostly-zero +# pages and pushed the store's synchronous ingest past the 60s gateway read +# timeout, failing the CI leg on a publish that had in fact landed. Cores are +# collected by collect-core-dumps.sh and uploaded as a workflow artifact, so +# nothing is lost by keeping them out of the store, which only wants +# profiling data. core_dump_re is shared with that script. +. "$(dirname "$0")/core-dump-names.inc" +grep -E "$core_dump_re" "$all_list" >"$core_list" || true +grep -Ev "$core_dump_re" "$all_list" | sed 's#^/##' >"$file_list" || true + +# Never drop files silently: an unexplained gap in a run's tree is worse than +# a noisy publish log. +if [ -s "$core_list" ]; then + echo "excluding $(wc -l <"$core_list" | tr -d ' ') core dump(s) from the store publish:" + while IFS= read -r core; do + echo " $(du -h "prof-results$core" | cut -f1 | tr -d ' ') ${core#/}" + done <"$core_list" +fi + if ! [ -s "$file_list" ]; then - echo "prof-results/ holds no files; skipping" + echo "prof-results/ holds no publishable files; skipping" exit 0 fi tar -czf "$tarball" -C prof-results -T "$file_list"