diff --git a/.github/workflows/ci-pipeline.yml b/.github/workflows/ci-pipeline.yml index af27aac63983..e1ba8d077205 100644 --- a/.github/workflows/ci-pipeline.yml +++ b/.github/workflows/ci-pipeline.yml @@ -22,76 +22,13 @@ concurrency: group: ${{ github.workflow }}-${{ github.head_ref || github.ref_name }} cancel-in-progress: true +# +# BRANCH-LOCAL: every leg except the multi-server tests is removed while the +# profiling result check is developed on this branch. Restore the full job +# list from master before merging. +# jobs: - ci: - name: CI - uses: ./.github/workflows/ci.yml - with: - selfhosted: ${{ github.repository_owner == 'FreeRADIUS' && '1' || '0' }} - docker_prefix: ${{ github.repository_owner == 'FreeRADIUS' && 'docker.internal.networkradius.com/' || '' }} - secrets: inherit - - ci-deb: - name: CI DEB - uses: ./.github/workflows/ci-deb.yml - secrets: inherit - - ci-rpm: - name: CI RPM - uses: ./.github/workflows/ci-rpm.yml - secrets: inherit - - ci-sanitizers: - name: CI-Sanitizers - uses: ./.github/workflows/ci-sanitizers.yml - with: - selfhosted: ${{ github.repository_owner == 'FreeRADIUS' && '1' || '0' }} - docker_prefix: ${{ github.repository_owner == 'FreeRADIUS' && 'docker.internal.networkradius.com/' || '' }} - secrets: inherit - - # - # Run on every commit but absent from merge's needs:, so a failure here is - # reported and diagnosed without holding up the merge. - # - ci-freebsd: - name: CI FreeBSD - uses: ./.github/workflows/ci-freebsd.yml - secrets: inherit - - ci-macos: - name: CI macOS - uses: ./.github/workflows/ci-macos.yml - secrets: inherit - ci-multi-server: name: Multi-Server CI Tests uses: ./.github/workflows/ci-multi-server-tests.yml secrets: inherit - - # - # Image builds take about as long as CI takes to fail, so ungated they were - # finishing in full on every broken commit. needs: ci skips them instead. - # They are per-commit smoke builds - docker-refresh.yml does the publishing - # on its own schedule - so starting them later costs nothing. - # - docker-crossbuild: - name: Docker crossbuild images - needs: [ci] - uses: ./.github/workflows/docker-crossbuild.yml - secrets: inherit - - docker-service: - name: Docker service images - needs: [ci] - uses: ./.github/workflows/docker-service.yml - secrets: inherit - - # - # No if:, so the default applies: runs only when every leg it needs - # succeeded, and is skipped otherwise. Only the gating legs are listed. - # - merge: - needs: [ci, ci-deb, ci-rpm, ci-sanitizers] - uses: ./.github/workflows/merge-upstream.yml - secrets: - GH_APP_PRIVATE_KEY: ${{ secrets.GH_APP_PRIVATE_KEY }} diff --git a/scripts/ci/publish-profiling-results.sh b/scripts/ci/publish-profiling-results.sh index 3512749d5a79..d30c0b50e779 100755 --- a/scripts/ci/publish-profiling-results.sh +++ b/scripts/ci/publish-profiling-results.sh @@ -22,9 +22,11 @@ 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. -Fails without publishing when any test's valgrind-exit-status is non-zero, -because a run valgrind killed has truncated callgrind output whose numbers are -not comparable with previous runs. +Prunes any test whose valgrind-exit-status is non-zero or missing, because a +run valgrind killed has truncated callgrind output whose numbers are not +comparable with previous runs. The clean remainder is published, so the store +receives only comparable data. A run where every test is unclean has nothing +comparable to publish and exits non-zero so the CI leg goes red. Where to POST. Its origin becomes the OIDC audience. -h Show this help. @@ -53,13 +55,19 @@ audience="${url%%://*}://${host_path%%/*}" [ -d prof-results ] || { echo "no prof-results/ tree; skipping"; exit 0; } -# Refuse to publish a run valgrind did not finish cleanly. start_valgrind_- +# Never publish a test valgrind did not finish cleanly. start_valgrind_- # profiling.sh drops a valgrind-exit-status file in each test's results dir; a # non-zero status means valgrind was killed, which leaves callgrind output # truncated at whatever point it died. Numbers from a truncated run are not # comparable with a clean one, and publishing them silently poisons the -# per-suite history the regression gate compares against. Exits non-zero so -# the leg goes red rather than passing with nothing uploaded. +# per-suite history the regression gate compares against, so unclean tests +# are pruned (with a warning naming each one) and the rest publish normally. +# +# A results dir holding the wrapper's log but no valgrind-exit-status is just +# as unclean: the wrapper writes valgrind_profiling.log first and the status +# file only after valgrind exits, so a missing status file means the wrapper +# was killed mid-run (e.g. the container was torn down around a hung +# shutdown) and never saw valgrind finish. unclean="" for status_file in $(find prof-results -type f -name valgrind-exit-status | sort); do read -r status <"$status_file" || status="unreadable" @@ -68,13 +76,55 @@ for status_file in $(find prof-results -type f -name valgrind-exit-status | sort esac unclean="${unclean} ${status_file%/valgrind-exit-status}:${status}" done +for wrapper_log in $(find prof-results -type f -name valgrind_profiling.log | sort); do + dir=${wrapper_log%/valgrind_profiling.log} + [ -f "$dir/valgrind-exit-status" ] || unclean="${unclean} ${dir}:missing" +done +# Exit statuses above 128 are 128 + signal number; name the common ones so +# the CI log reads as a cause, not a bare number. +explain_status() +{ + case $1 in + missing) + echo "no exit status recorded: the profiling wrapper was killed mid-run" ;; + 134) echo "status 134: SIGABRT, usually a freeradius assert (see the freeradius.log line below)" ;; + 137) echo "status 137: SIGKILL (out-of-memory killer, or forced container teardown)" ;; + 139) echo "status 139: SIGSEGV (crash)" ;; + 143) echo "status 143: SIGTERM (asked to shut down mid-run)" ;; + *) if [ "$1" -gt 128 ] 2>/dev/null; then + echo "status $1: killed by signal $(($1 - 128))" + else + echo "valgrind exited with status $1" + fi ;; + esac +} + if [ -n "$unclean" ]; then - echo "ERROR: refusing to publish, valgrind exited uncleanly in:" >&2 + echo "WARNING: pruning tests where valgrind did not finish cleanly:" >&2 for entry in $unclean; do - echo " ${entry%:*} (status ${entry##*:})" >&2 + dir=${entry%:*} + echo " ${dir}" >&2 + echo " $(explain_status "${entry##*:}")" >&2 + # Valgrind passes the profiled server's exit status through, so the + # cause usually lives in freeradius.log (asserts, caught signals), + # not valgrind.log. Quote the first such line so the cause is + # visible without downloading the artifact. NOTE: valgrind.log's + # "brk segment overflow" warning appears in clean runs too (glibc + # falls back to mmap); do not treat it as the failure reason. + diag=$(grep -E -m1 "ASSERT FAILED|CAUGHT SIGNAL|_EXIT\(|PANIC" "$dir/freeradius.log" 2>/dev/null || true) + if [ -n "$diag" ]; then + echo " freeradius.log: ${diag}" >&2 + else + diag=$(grep -E -m1 "Assertion|FATAL|out of memory|'impossible' happened|Fatal error" "$dir/valgrind.log" 2>/dev/null || true) + [ -n "$diag" ] && echo " valgrind.log: ${diag}" >&2 + fi + done + # Drop each unclean test's directory so only comparable data travels. An + # all-unclean run leaves nothing publishable and errors out at the + # empty-file-list check below. + for entry in $unclean; do + rm -rf "${entry%:*}" done - echo "ERROR: truncated profiling data is not comparable with previous runs" >&2 - exit 1 fi tmpdir=$(mktemp -d) @@ -113,6 +163,13 @@ if [ -s "$core_list" ]; then fi if ! [ -s "$file_list" ]; then + # An all-unclean run is a failure, not a skip: every test was pruned, so + # the run produced no comparable data at all and the leg must go red + # rather than quietly publishing nothing. + if [ -n "$unclean" ]; then + echo "ERROR: every test was pruned as unclean; no comparable data to publish" >&2 + exit 1 + fi echo "prof-results/ holds no publishable files; skipping" exit 0 fi diff --git a/src/tests/multi-server/scripts/profiling/start_valgrind_profiling.sh b/src/tests/multi-server/scripts/profiling/start_valgrind_profiling.sh index 92f81e757c0f..fe6fc2296d8f 100755 --- a/src/tests/multi-server/scripts/profiling/start_valgrind_profiling.sh +++ b/src/tests/multi-server/scripts/profiling/start_valgrind_profiling.sh @@ -128,15 +128,27 @@ wait ${VALGRIND_PID} 2>/dev/null || VALGRIND_STATUS=$? echo "${VALGRIND_STATUS}" > /etc/prof-results/valgrind-exit-status if [ "${VALGRIND_STATUS}" -ne 0 ]; then - # Over 128 means a signal. 139 is SIGSEGV, which is how valgrind exiting on - # its 8 MB brk segment ceiling presents; valgrind.log names the real reason - # on the line above its backtrace. + # Over 128 means a signal. Valgrind passes the profiled server's exit + # status through (verified: freeradius's own _EXIT(134) matched the 134 + # recorded here on the e26e348 ldap run), so a signal status usually + # means FREERADIUS died of that signal - an assert or crash logged in + # freeradius.log - rather than valgrind itself being killed. + # valgrind.log's "brk segment overflow" warning is NOT the reason: clean + # runs carry it too (glibc falls back to mmap when brk cannot grow). if [ "${VALGRIND_STATUS}" -gt 128 ]; then - echo "ERROR: valgrind was killed by signal $((VALGRIND_STATUS - 128)); profiling data is truncated" >&2 + SIG=$((VALGRIND_STATUS - 128)) + case ${SIG} in + 6) SIGNAME="SIGABRT (abort/assertion)" ;; + 9) SIGNAME="SIGKILL (OOM killer or forced teardown)" ;; + 11) SIGNAME="SIGSEGV (crash)" ;; + 15) SIGNAME="SIGTERM" ;; + *) SIGNAME="signal ${SIG}" ;; + esac + echo "ERROR: exit status ${VALGRIND_STATUS}: ${SIGNAME}; freeradius likely died of that signal (see freeradius.log for asserts/backtraces)" >&2 else echo "ERROR: valgrind exited ${VALGRIND_STATUS}; profiling data may be truncated" >&2 fi - echo "ERROR: see valgrind.log for the reason; these results will not be published" >&2 + echo "ERROR: these results will not be published" >&2 fi # Signal that valgrind has finished writing all profiling data