From a7271f9dd78e05befbcd59abfc68729dbc447573 Mon Sep 17 00:00:00 2001 From: Chris Evich Date: Wed, 7 Jun 2023 14:19:22 -0400 Subject: [PATCH] GHA: Fix bad job-names & links in monitoring emails Due to a bad file-format design, if a cirrus-cron job happened to have a name w/ spaces, the generated e-mail text would be broken. For example: ``` Cron build 'VM' Failed: https://cirrus-ci.com/build/Image Maintenance 5630822628196352 ``` Fix this by flipping the field-order in an intermediate file, so the build ID comes first, then the job name. This makes it much easier for `read` to process, since all words will be stored into the final variable (now the job name). Also change all variables that reference this intermediate file such that they continue to reflect the expected field order. Update script tests and add a new test to confirm expected file processing and output. Signed-off-by: Chris Evich --- .../check_cirrus_cron/cron_failures.sh | 25 +++++++++--------- .../check_cirrus_cron/make_email_body.sh | 16 +++++++----- .../check_cirrus_cron/rerun_failed_tasks.sh | 26 ++++++++++--------- .github/actions/check_cirrus_cron/test.sh | 22 ++++++++++++---- .github/workflows/check_cirrus_cron.yml | 4 +-- .github/workflows/rerun_cirrus_cron.yml | 4 +-- 6 files changed, 57 insertions(+), 40 deletions(-) diff --git a/.github/actions/check_cirrus_cron/cron_failures.sh b/.github/actions/check_cirrus_cron/cron_failures.sh index f4d11613ec..e97754d629 100755 --- a/.github/actions/check_cirrus_cron/cron_failures.sh +++ b/.github/actions/check_cirrus_cron/cron_failures.sh @@ -10,8 +10,8 @@ source $(dirname "${BASH_SOURCE[0]}")/lib.sh _errfmt="Expecting %s value to not be empty" if [[ -z "$GITHUB_REPOSITORY" ]]; then # / err $(printf "$_errfmt" "\$GITHUB_REPOSITORY") -elif [[ -z "$NAME_ID_FILEPATH" ]]; then # output filepath - err $(printf "$_errfmt" "\$NAME_ID_FILEPATH") +elif [[ -z "$ID_NAME_FILEPATH" ]]; then # output filepath + err $(printf "$_errfmt" "\$ID_NAME_FILEPATH") fi confirm_gha_environment @@ -74,18 +74,19 @@ gql "$(<./artifacts/query.json)" "$filt_head" > ./artifacts/reply.json # } # ... -filt="$filt_head | map(select(.lastInvocationBuild.status==\"FAILED\") | { name:.name, id:.lastInvocationBuild.id} | join(\" \")) | join(\"\n\")" -jq --raw-output "$filt" ./artifacts/reply.json > "$NAME_ID_FILEPATH" +# Output format: +# Where may contain multiple words +filt="$filt_head | map(select(.lastInvocationBuild.status==\"FAILED\") | {id:.lastInvocationBuild.id, name:.name} | join(\" \")) | join(\"\n\")" +jq --raw-output "$filt" ./artifacts/reply.json > "$ID_NAME_FILEPATH" -echo " " -cat "$NAME_ID_FILEPATH" +# Print out the file to assist in job debugging +echo " " +cat "$ID_NAME_FILEPATH" -# Don't rely on a newline present for zero/one output line, always count words -records=$(wc --words "$NAME_ID_FILEPATH" | cut -d ' ' -f 1) -# Always two words per record -failures=$((records/2)) +# Count non-empty lines (in case there are any) +records=$(awk -r -e '/\w+/{print $0}' "$ID_NAME_FILEPATH" | wc -l) # Set the output of this step. # Ref: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-output-parameter # shellcheck disable=SC2154 -echo "failures=$failures" >> $GITHUB_OUTPUT -echo "Total failed Cirrus-CI cron builds: $failures" +echo "failures=$records" >> $GITHUB_OUTPUT +echo "Total failed Cirrus-CI cron builds: $records" diff --git a/.github/actions/check_cirrus_cron/make_email_body.sh b/.github/actions/check_cirrus_cron/make_email_body.sh index af38566286..ef013a85ba 100755 --- a/.github/actions/check_cirrus_cron/make_email_body.sh +++ b/.github/actions/check_cirrus_cron/make_email_body.sh @@ -9,24 +9,26 @@ set -eo pipefail source $(dirname "${BASH_SOURCE[0]}")/lib.sh _errfmt="Expecting %s value to not be empty" -# NAME_ID_FILEPATH is defined by workflow YAML +# ID_NAME_FILEPATH is defined by workflow YAML # shellcheck disable=SC2154 if [[ -z "$GITHUB_REPOSITORY" ]]; then err $(printf "$_errfmt" "\$GITHUB_REPOSITORY") -elif [[ ! -r "$NAME_ID_FILEPATH" ]]; then - err "Expecting \$NAME_ID_FILEPATH value ($NAME_ID_FILEPATH) to be a readable file" +elif [[ ! -r "$ID_NAME_FILEPATH" ]]; then + err "Expecting \$ID_NAME_FILEPATH value ($ID_NAME_FILEPATH) to be a readable file" fi confirm_gha_environment -mkdir -p artifacts +# GITHUB_WORKSPACE confirmed by confirm_gha_environment() +# shellcheck disable=SC2154 +mkdir -p "$GITHUB_WORKSPACE/artifacts" ( echo "Detected one or more Cirrus-CI cron-triggered jobs have failed recently:" echo "" - while read -r NAME BID; do + while read -r BID NAME; do echo "Cron build '$NAME' Failed: https://cirrus-ci.com/build/$BID" - done < "$NAME_ID_FILEPATH" + done < "$ID_NAME_FILEPATH" echo "" # Defined by github-actions @@ -35,4 +37,4 @@ mkdir -p artifacts # Separate content from sendgrid.com automatic footer. echo "" echo "" -) > ./artifacts/email_body.txt +) > $GITHUB_WORKSPACE/artifacts/email_body.txt diff --git a/.github/actions/check_cirrus_cron/rerun_failed_tasks.sh b/.github/actions/check_cirrus_cron/rerun_failed_tasks.sh index 4dd118e578..3c422b066c 100755 --- a/.github/actions/check_cirrus_cron/rerun_failed_tasks.sh +++ b/.github/actions/check_cirrus_cron/rerun_failed_tasks.sh @@ -14,33 +14,35 @@ set -eo pipefail # For example, from https://cirrus-ci.com/github/containers/podman/main # (pick an old one from the bottom, since re-running it won't affect anybody) # 3. Create a temp. file, like /tmp/fail with a single line, of the form: -# -# 4. export NAME_ID_FILEPATH=/tmp/fail +# +# 4. export ID_NAME_FILEPATH=/tmp/fail # 5. execute this script, and refresh the build in the WebUI, all unsuccessful # tasks should change status to running or scheduled. Note: some later # tasks may remain red as they wait for dependencies to run and pass. -# 6. After each run, cleanup with 'rm -rf ./artifacts' +# 6. After each run, cleanup with 'rm -rf $GITHUB_WORKSPACE/artifacts' # (unless you want to examine them) source $(dirname "${BASH_SOURCE[0]}")/lib.sh _errfmt="Expecting %s value to not be empty" -# NAME_ID_FILEPATH is defined by workflow YAML +# ID_NAME_FILEPATH is defined by workflow YAML # shellcheck disable=SC2154 if [[ -z "$SECRET_CIRRUS_API_KEY" ]]; then err $(printf "$_errfmt" "\$SECRET_CIRRUS_API_KEY") -elif [[ ! -r "$NAME_ID_FILEPATH" ]]; then # output from cron_failures.sh - err $(printf "Expecting %s value to be a readable file" "\$NAME_ID_FILEPATH") +elif [[ ! -r "$ID_NAME_FILEPATH" ]]; then # output from cron_failures.sh + err $(printf "Expecting %s value to be a readable file" "\$ID_NAME_FILEPATH") fi confirm_gha_environment -mkdir -p artifacts +# GITHUB_WORKSPACE confirmed by confirm_gha_environment() +# shellcheck disable=SC2154 +mkdir -p $GITHUB_WORKSPACE/artifacts # If there are no tasks, don't fail reading the file -truncate -s 0 ./artifacts/rerun_tids.txt +truncate -s 0 $GITHUB_WORKSPACE/artifacts/rerun_tids.txt -cat "$NAME_ID_FILEPATH" | \ - while read -r NAME BID; do +cat "$ID_NAME_FILEPATH" | \ + while read -r BID NAME; do if [[ -z "$NAME" ]]; then err $(printf "$_errfmt" "\$NAME") elif [[ -z "$BID" ]]; then @@ -80,11 +82,11 @@ cat "$NAME_ID_FILEPATH" | \ msg "Rerunning build $BID task $TID" # Must send result through a file into rerun_tasks array # because this section is executing in a child-shell - echo "$TID" >> ./artifacts/rerun_tids.txt + echo "$TID" >> $GITHUB_WORKSPACE/artifacts/rerun_tids.txt fi done declare -a rerun_tasks - mapfile rerun_tasks <./artifacts/rerun_tids.txt + mapfile rerun_tasks <$GITHUB_WORKSPACE/artifacts/rerun_tids.txt msg "::endgroup::" if [[ "${#rerun_tasks[*]}" -eq 0 ]]; then diff --git a/.github/actions/check_cirrus_cron/test.sh b/.github/actions/check_cirrus_cron/test.sh index 70671f2acd..19f2e35287 100644 --- a/.github/actions/check_cirrus_cron/test.sh +++ b/.github/actions/check_cirrus_cron/test.sh @@ -42,12 +42,12 @@ export GITHUB_REPOSITORY="$CIRRUS_REPO_FULL_NAME" export GITHUB_WORKSPACE=$(mktemp -d -p '' cron_failures_workspace_XXXX) export GITHUB_WORKFLOW="testing" # shellcheck disable=SC2155 -export NAME_ID_FILEPATH=$(mktemp -p '' cron_failures_data_XXXX) -trap "rm -rf $GITHUB_OUTPUT $GITHUB_WORKSPACE $NAME_ID_FILEPATH" EXIT +export ID_NAME_FILEPATH=$(mktemp -p '' cron_failures_data_XXXX) +trap "rm -rf $GITHUB_OUTPUT $GITHUB_WORKSPACE $ID_NAME_FILEPATH" EXIT ##### -cd /tmp || fail +cd $GITHUB_WORKSPACE || fail # Replace newlines and indentation to make grep easier if ! $base/cron_failures.sh |& \ tr -s '[:space:]' ' ' > $GITHUB_WORKSPACE/output; then @@ -62,7 +62,7 @@ expect_regex \ msg "$header make_email_body.sh" # It's possible no cirrus-cron jobs actually failed -echo '' >> "$NAME_ID_FILEPATH" +echo -e '\n\n \n\t\n' >> "$ID_NAME_FILEPATH" # blank lines should be ignored # Don't need to test stdout/stderr of this if ! $base/make_email_body.sh; then die "make_email_body.sh failed" @@ -74,11 +74,23 @@ expect_regex \ ##### +msg "$header make_email_body.sh name and link" +# Job names may contain spaces, confirm lines are parsed properly +echo -e '1234567890 cirrus-cron test job' >> "$ID_NAME_FILEPATH" # Append to blank lines +$base/make_email_body.sh +expected="Cron build 'cirrus-cron test job' Failed: https://cirrus-ci.com/build/1234567890" +if ! grep -q "$expected" $GITHUB_WORKSPACE/artifacts/email_body.txt; then + die "Expecting to find string '$expected' in generated e-mail body: +$(<$GITHUB_WORKSPACE/artifacts/email_body.txt)" +fi + +##### + msg "$header rerun_failed_tasks.sh" export SECRET_CIRRUS_API_KEY=testing-nottherightkey # test.sh is sensitive to the 'testing' name. Var. defined by cirrus-ci # shellcheck disable=SC2154 -echo "testing $CIRRUS_BUILD_ID" > "$NAME_ID_FILEPATH" +echo "$CIRRUS_BUILD_ID test cron job name" > "$ID_NAME_FILEPATH" if ! $base/rerun_failed_tasks.sh |& \ tr -s '[:space:]' ' ' > $GITHUB_WORKSPACE/rerun_output; then die "rerun_failed_tasks.sh failed" diff --git a/.github/workflows/check_cirrus_cron.yml b/.github/workflows/check_cirrus_cron.yml index 8c0ff890c4..313f28be34 100644 --- a/.github/workflows/check_cirrus_cron.yml +++ b/.github/workflows/check_cirrus_cron.yml @@ -32,9 +32,9 @@ on: env: # CSV listing of e-mail addresses for delivery failure or error notices RCPTCSV: rh.container.bot@gmail.com,podman-monitor@lists.podman.io - # Filename for table of cron-name to build-id data + # Filename for table of build-id to cron-name data # (must be in $GITHUB_WORKSPACE/artifacts/) - NAME_ID_FILEPATH: './artifacts/name_id.txt' + ID_NAME_FILEPATH: './artifacts/id_name.txt' permissions: contents: read diff --git a/.github/workflows/rerun_cirrus_cron.yml b/.github/workflows/rerun_cirrus_cron.yml index bb693f00b6..4867b1571d 100644 --- a/.github/workflows/rerun_cirrus_cron.yml +++ b/.github/workflows/rerun_cirrus_cron.yml @@ -31,9 +31,9 @@ on: env: # CSV listing of e-mail addresses for delivery failure or error notices RCPTCSV: rh.container.bot@gmail.com,podman-monitor@lists.podman.io - # Filename for table of cron-name to build-id data + # Filename for table of build-id to cron-name data # (must be in $GITHUB_WORKSPACE/artifacts/) - NAME_ID_FILEPATH: './artifacts/name_id.txt' + ID_NAME_FILEPATH: './artifacts/id_name.txt' permissions: contents: read