From 45af65da819e08c673c96c8cfa33eefa6b3184d1 Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Thu, 20 Jul 2023 09:07:20 -0400 Subject: [PATCH] Backport of Limit number of tests in CI comment into release/1.14.x (#21971) * backport of commit dc104898f700447f7764919445c7559baeb7e987 (#21853) * fix multiline * shellcheck, and success message for builds * add full path * cat the summary * fix and faster * fix if condition * base64 in a separate step * echo * check against empty string * add echo * only use matrix ids * only id * echo matrix * remove wrapping array * tojson * try echo again * use jq to get packages * don't quote * only run binary tests once * only run binary tests once * test what's wrong with the binary * separate file * use matrix file * failed test * update comment on success * correct variable name * bae64 fix * output to file * use multiline * fix * fix formatting * fix newline * fix whitespace * correct body, remove comma * small fixes * shellcheck * another shellcheck fix * fix deprecation checker * only run comments for prs * Update .github/workflows/test-go.yml Co-authored-by: Mike Palmiotto * Update .github/workflows/test-go.yml Co-authored-by: Mike Palmiotto * fixes --------- Co-authored-by: Mike Palmiotto * backport of commit 3b00dde1ba4d479fbd67b1d0767e421e495d8cce (#21936) * limit test comments * remove unecessary tee * fix go test condition * fix * fail test * remove ailways entirely * fix columns * make a bunch of tests fail * separate line * include Failures: * remove test fails * fix whitespace * backport of commit 245430215c00d80a38283020fca114bade022e0f (#21973) * only add binary tests if they exist * shellcheck --------- Co-authored-by: miagilepner Co-authored-by: Mike Palmiotto --- .github/scripts/gh_comment.sh | 38 +++++++++++++ .github/scripts/report_failed_builds.sh | 35 ++++-------- .github/scripts/report_failed_tests.sh | 42 +++++++++++++++ .github/workflows/build.yml | 13 ++--- .github/workflows/ci.yml | 25 +++++++-- .github/workflows/test-go.yml | 71 +++++++++++++++++-------- Makefile | 8 +-- scripts/deprecations-checker.sh | 2 +- 8 files changed, 169 insertions(+), 65 deletions(-) create mode 100644 .github/scripts/gh_comment.sh create mode 100755 .github/scripts/report_failed_tests.sh diff --git a/.github/scripts/gh_comment.sh b/.github/scripts/gh_comment.sh new file mode 100644 index 000000000..b47df541a --- /dev/null +++ b/.github/scripts/gh_comment.sh @@ -0,0 +1,38 @@ +#!/bin/bash + +set -e + + +function update_or_create_comment { + REPO=$1 + PR_NUMBER=$2 + SEARCH_KEY=$3 + BODY=$4 + + # We only want for the GH bot to place one comment to report build failures + # and if we rerun a job, that comment needs to be updated. + # Let's try to find if the GH bot has placed a similar comment + comment_id=$(gh api \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + --paginate \ + /repos/hashicorp/"$REPO"/issues/"$PR_NUMBER"/comments | jq -r --arg SEARCH_KEY "$SEARCH_KEY" '.[] | select (.body | contains($SEARCH_KEY)) | .id') + + if [[ "$comment_id" != "" ]]; then + # update the comment with the new body + gh api \ + --method PATCH \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + /repos/hashicorp/"$REPO"/issues/comments/"$comment_id" \ + -f body="$BODY" + else + # create a comment with the new body + gh api \ + --method POST \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + /repos/hashicorp/"$REPO"/issues/"$PR_NUMBER"/comments \ + -f body="$BODY" + fi +} \ No newline at end of file diff --git a/.github/scripts/report_failed_builds.sh b/.github/scripts/report_failed_builds.sh index d0c8982f1..b4cd3480c 100755 --- a/.github/scripts/report_failed_builds.sh +++ b/.github/scripts/report_failed_builds.sh @@ -30,30 +30,15 @@ done # Create a comment to be posted on the PR # This comment reports failed jobs and the url to the failed workflow -new_body="build failed for these jobs: ${failed_jobs[*]}. Please refer to this workflow to learn more: https://github.com/hashicorp/vault/actions/runs/$RUN_ID" - -# We only want for the GH bot to place one comment to report build failures -# and if we rerun a job, that comment needs to be updated. -# Let's try to find if the GH bot has placed a similar comment -comment_id=$(gh api \ - -H "Accept: application/vnd.github+json" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - /repos/hashicorp/"$REPO"/issues/"$PR_NUMBER"/comments | jq -r '.[] | select (.body | contains("build failed for these job")) | .id') - -if [[ "$comment_id" != "" ]]; then - # update the comment with the new body - gh api \ - --method PATCH \ - -H "Accept: application/vnd.github+json" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - /repos/hashicorp/"$REPO"/issues/comments/"$comment_id" \ - -f body="$new_body" +if [ ${#failed_jobs[@]} -eq 0 ]; then + new_body="Build Results: +All builds succeeded! :white_check_mark:" else - # create a comment with the new body - gh api \ - --method POST \ - -H "Accept: application/vnd.github+json" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - /repos/hashicorp/"$REPO"/issues/"$PR_NUMBER"/comments \ - -f body="$new_body" + new_body="Build Results: +Build failed for these jobs: ${failed_jobs[*]}. Please refer to this workflow to learn more: https://github.com/hashicorp/vault/actions/runs/$RUN_ID" fi + + +source ./.github/scripts/gh_comment.sh + +update_or_create_comment "$REPO" "$PR_NUMBER" "Build Results:" "$new_body" \ No newline at end of file diff --git a/.github/scripts/report_failed_tests.sh b/.github/scripts/report_failed_tests.sh new file mode 100755 index 000000000..d69d43a16 --- /dev/null +++ b/.github/scripts/report_failed_tests.sh @@ -0,0 +1,42 @@ +#!/bin/bash + +set -e +MAX_TESTS=10 +# this script expects the following env vars to be set +# error if these are not set +[ ${GITHUB_TOKEN:?} ] +[ ${RUN_ID:?} ] +[ ${REPO:?} ] +[ ${PR_NUMBER:?} ] +if [ -z "$TABLE_DATA" ]; then + BODY="CI Results: +All Go tests succeeded! :white_check_mark:" +else + # Remove any rows that don't have a test name + # Only keep the test type, test package, test name, and logs column + # Remove the scroll emoji + # Remove "github.com/hashicorp/vault" from the package name + TABLE_DATA=$(echo "$TABLE_DATA" | awk -F\| '{if ($4 != " - ") { print "|" $2 "|" $3 "|" $4 "|" $7 }}' | sed -r 's/ :scroll://' | sed -r 's/github.com\/hashicorp\/vault\///') + NUM_FAILURES=$(wc -l <<< "$TABLE_DATA") + + # Check if the number of failures is greater than the maximum tests to display + # If so, limit the table to MAX_TESTS number of results + if [ "$NUM_FAILURES" -gt "$MAX_TESTS" ]; then + TABLE_DATA=$(echo "$TABLE_DATA" | head -n "$MAX_TESTS") + NUM_OTHER=( $NUM_FAILURES - "$MAX_TESTS" ) + TABLE_DATA="$TABLE_DATA + +and $NUM_OTHER other tests" + fi + + # Add the header for the table + BODY="CI Results: +Failures: +| Test Type | Package | Test | Logs | +| --------- | ------- | ---- | ---- | +${TABLE_DATA}" +fi + +source ./.github/scripts/gh_comment.sh + +update_or_create_comment "$REPO" "$PR_NUMBER" "CI Results:" "$BODY" \ No newline at end of file diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f082901bc..55ae2953d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -292,10 +292,7 @@ jobs: - build-ubi - test - test-docker-k8s - if: | - always() && (cancelled() || - contains(needs.*.result, 'cancelled') || - contains(needs.*.result, 'failure')) + if: (success() || failure()) && github.head_ref != '' runs-on: ubuntu-latest steps: - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 @@ -317,10 +314,7 @@ jobs: completed-successfully: # We force a failure if any of the dependent jobs fail, # this is a workaround for the issue reported https://github.com/actions/runner/issues/2566 - if: | - always() && (cancelled() || - contains(needs.*.result, 'cancelled') || - contains(needs.*.result, 'failure')) + if: always() runs-on: ubuntu-latest needs: - build-other @@ -332,5 +326,4 @@ jobs: - test-docker-k8s steps: - run: | - echo "Some of the required build and test workflows have failed!" - exit 1 + tr -d '\n' <<< '${{ toJSON(needs.*.result) }}' | grep -q -v -E '(failure|cancelled)' \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c1c146722..f1534efcc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -109,7 +109,8 @@ jobs: # The regular Go tests use an extra runner to execute the # binary-dependent tests. We isolate them there so that the # other tests aren't slowed down waiting for a binary build. - total-runners: 17 + binary-tests: true + total-runners: 16 go-arch: amd64 go-tags: '${{ needs.setup.outputs.go-tags }},deadlock' runs-on: ${{ needs.setup.outputs.compute-larger }} @@ -363,14 +364,32 @@ jobs: # Sort all of the summary table rows and push them to a temp file. temp_file_name=temp-$(date +%s) cat failure-summary-*.md | sort >> "$temp_file_name" - + # If there are test failures, present them in a format of a GitHub Markdown table. if [ -s "$temp_file_name" ]; then # shellcheck disable=SC2129 # Here we create the headings for the summary table echo "| Test Type | Package | Test | Elapsed | Runner Index | Logs |" >> "$GITHUB_STEP_SUMMARY" echo "| --------- | ------- | ---- | ------- | ------------ | ---- |" >> "$GITHUB_STEP_SUMMARY" - cat "$temp_file_name" >> "$GITHUB_STEP_SUMMARY" + # shellcheck disable=SC2002 + cat "$temp_file_name" >> "$GITHUB_STEP_SUMMARY" else echo "### All Go tests passed! :white_check_mark:" >> "$GITHUB_STEP_SUMMARY" fi + + # the random EOF is needed for a multiline environment variable + EOF=$(dd if=/dev/urandom bs=15 count=1 status=none | base64) + # shellcheck disable=SC2129 + echo "TABLE_TEST_RESULTS<<$EOF" >> "$GITHUB_ENV" + cat "$temp_file_name" >> "$GITHUB_ENV" + echo "$EOF" >> "$GITHUB_ENV" + - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + - name: Create comment + if: github.head_ref != '' + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + RUN_ID: ${{ github.run_id }} + REPO: ${{ github.event.repository.name }} + TABLE_DATA: ${{ env.TABLE_TEST_RESULTS }} + run: ./.github/scripts/report_failed_tests.sh diff --git a/.github/workflows/test-go.yml b/.github/workflows/test-go.yml index a0a5633ff..ca6c2e0eb 100644 --- a/.github/workflows/test-go.yml +++ b/.github/workflows/test-go.yml @@ -10,9 +10,14 @@ on: required: true type: string total-runners: - description: Number of runners to use for executing the tests on. + description: Number of runners to use for executing non-binary tests. required: true type: string + binary-tests: + description: Whether to run the binary tests. + required: false + default: false + type: boolean env-vars: description: A map of environment variables as JSON. required: false @@ -119,10 +124,11 @@ jobs: run: | ( go list ./... | grep -v "_binary" | grep -v "vault/integ" | gotestsum tool ci-matrix --debug \ - --partitions 16 \ + --partitions "${{ inputs.total-runners }}" \ --timing-files 'test-results/go-test/*.json' > matrix.json ) - name: Capture list of binary tests + if: inputs.binary-tests id: list-binary-tests run: | LIST="$(go list ./... | grep "_binary" | xargs)" @@ -130,20 +136,34 @@ jobs: - name: Build complete matrix id: build run: | - set -exo pipefail - export BINARY_TESTS="${{ steps.list-binary-tests.outputs.list }}" - ( - echo -n "matrix=" - jq -c --arg BINARY "${BINARY_TESTS}" \ - '.include += [{ - "id": 16, - "estimatedRuntime": "N/A", - "packages": $BINARY, - "description": "partition 16 - binary test packages" - }]' matrix.json - ) >> "$GITHUB_OUTPUT" + set -exo pipefail + matrix_file="matrix.json" + if [ "${{ inputs.binary-tests}}" == "true" ] && [ -n "${{ steps.list-binary-tests.outputs.list }}" ]; then + export BINARY_TESTS="${{ steps.list-binary-tests.outputs.list }}" + jq --arg BINARY "${BINARY_TESTS}" --arg BINARY_INDEX "${{ inputs.total-runners }}" \ + '.include += [{ + "id": $BINARY_INDEX, + "estimatedRuntime": "N/A", + "packages": $BINARY, + "description": "partition $BINARY_INDEX - binary test packages" + }]' matrix.json > new-matrix.json + matrix_file="new-matrix.json" + fi + # convert the json to a map keyed by id + ( + echo -n "matrix=" + jq -c \ + '.include | map( { (.id|tostring): . } ) | add' "$matrix_file" + ) >> "$GITHUB_OUTPUT" + # extract an array of ids from the json + ( + echo -n "matrix_ids=" + jq -c \ + '[ .include[].id | tostring ]' "$matrix_file" + ) >> "$GITHUB_OUTPUT" outputs: matrix: ${{ steps.build.outputs.matrix }} + matrix_ids: ${{ steps.build.outputs.matrix_ids }} test-go: needs: test-matrix @@ -154,7 +174,8 @@ jobs: runs-on: ${{ fromJSON(inputs.runs-on) }} strategy: fail-fast: false - matrix: ${{ fromJSON(needs.test-matrix.outputs.matrix) }} + matrix: + id: ${{ fromJSON(needs.test-matrix.outputs.matrix_ids) }} env: GOPRIVATE: github.com/hashicorp/* TIMEOUT_IN_MINUTES: ${{ inputs.timeout-minutes }} @@ -196,7 +217,7 @@ jobs: run: | git config --global url."https://${{ secrets.ELEVATED_GITHUB_TOKEN}}@github.com".insteadOf https://github.com - id: build - if: contains(matrix.packages, '_binary') + if: inputs.binary-tests && matrix.id == inputs.total-runners env: GOPRIVATE: github.com/hashicorp/* run: time make ci-bootstrap dev @@ -207,10 +228,16 @@ jobs: COMMIT_SHA: ${{ github.sha }} run: | set -exo pipefail - + # Build the dynamically generated source files. make prep - + + packages=$(echo "${{ toJSON(needs.test-matrix.outputs.matrix) }}" | jq -c -r --arg id "${{ matrix.id }}" '.[$id] | .packages') + + if [ -z "$packages" ]; then + echo "no test packages to run" + exit 1 + fi # We don't want VAULT_LICENSE set when running Go tests, because that's # not what developers have in their environments and it could break some # tests; it would be like setting VAULT_TOKEN. However some non-Go @@ -253,7 +280,7 @@ jobs: --jsonfile test-results/go-test/results-${{ matrix.id }}.json \ --jsonfile-timing-events failure-summary-${{ matrix.id }}${{ inputs.name != '' && '-' || '' }}${{ inputs.name }}.json \ $RERUN_FAILS \ - --packages "${{ matrix.packages }}" \ + --packages "$packages" \ -- \ -tags "${{ inputs.go-tags }}" \ -timeout=${{ env.TIMEOUT_IN_MINUTES }}m \ @@ -307,13 +334,13 @@ jobs: let prefixToSearchFor; switch ("${{ inputs.name }}") { case "race": - prefixToSearchFor = 'Run Go tests with data race detection / test-go (${{ matrix.id }},' + prefixToSearchFor = 'Run Go tests with data race detection / test-go (${{ matrix.id }})' break case "fips": - prefixToSearchFor = 'Run Go tests with FIPS configuration / test-go (${{ matrix.id }},' + prefixToSearchFor = 'Run Go tests with FIPS configuration / test-go (${{ matrix.id }})' break default: - prefixToSearchFor = 'Run Go tests / test-go (${{ matrix.id }},' + prefixToSearchFor = 'Run Go tests / test-go (${{ matrix.id }})' } const jobData = result.data.jobs.filter( diff --git a/Makefile b/Makefile index fc5dca3ad..e1993f03a 100644 --- a/Makefile +++ b/Makefile @@ -115,12 +115,12 @@ vet: # deprecations runs staticcheck tool to look for deprecations. Checks entire code to see if it # has deprecated function, variable, constant or field -deprecations: bootstrap +deprecations: bootstrap prep @BUILD_TAGS='$(BUILD_TAGS)' ./scripts/deprecations-checker.sh "" # ci-deprecations runs staticcheck tool to look for deprecations. All output gets piped to revgrep # which will only return an error if changes that is not on main has deprecated function, variable, constant or field -ci-deprecations: ci-bootstrap +ci-deprecations: ci-bootstrap prep @BUILD_TAGS='$(BUILD_TAGS)' ./scripts/deprecations-checker.sh main tools/codechecker/.bin/codechecker: @@ -129,13 +129,13 @@ tools/codechecker/.bin/codechecker: # vet-codechecker runs our custom linters on the test functions. All output gets # piped to revgrep which will only return an error if new piece of code violates # the check -vet-codechecker: bootstrap tools/codechecker/.bin/codechecker +vet-codechecker: bootstrap tools/codechecker/.bin/codechecker prep @$(GO_CMD) vet -vettool=./tools/codechecker/.bin/codechecker -tags=$(BUILD_TAGS) ./... 2>&1 | revgrep # vet-codechecker runs our custom linters on the test functions. All output gets # piped to revgrep which will only return an error if new piece of code that is # not on main violates the check -ci-vet-codechecker: ci-bootstrap tools/codechecker/.bin/codechecker +ci-vet-codechecker: ci-bootstrap tools/codechecker/.bin/codechecker prep @$(GO_CMD) vet -vettool=./tools/codechecker/.bin/codechecker -tags=$(BUILD_TAGS) ./... 2>&1 | revgrep origin/main # lint runs vet plus a number of other checkers, it is more comprehensive, but louder diff --git a/scripts/deprecations-checker.sh b/scripts/deprecations-checker.sh index 508464245..586ac13c8 100755 --- a/scripts/deprecations-checker.sh +++ b/scripts/deprecations-checker.sh @@ -34,5 +34,5 @@ if [ -z $1 ] else # GitHub Actions will use this to find only changes wrt PR's base ref branch # revgrep CLI tool will return an exit status of 1 if any issues match, else it will return 0 - staticcheck -checks="SA1019" -tags="$BUILD_TAGS" 2>&1 | revgrep "$(git merge-base HEAD "origin/$1")" + staticcheck -checks="SA1019" -tags="$BUILD_TAGS" 2>&1 | revgrep origin/"$1" fi