From cae733256bf53b63d5be9c425fdb5a929aa5b0d3 Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Fri, 13 Dec 2024 17:06:21 -0700 Subject: [PATCH 1/4] Convert the main regression test workflow into a dispatch workflow This will effectively decouple the trigger events of a PR from this PR running, thereby avoiding either superfluous statuses of skipped jobs or overriden statuses from actual jobs from multiple label events. Updates are made to use the inputs provided by the dispatch trigger rather than now non-existent context variables. --- .github/workflows/ci.yml | 64 ++++++++++++++++++++++------- .github/workflows/test_workflow.yml | 26 ++++++++++-- 2 files changed, 71 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 087f9eba34..5e02138c15 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,15 +1,38 @@ name: Regression Suite -run-name : ${{ github.event_name == 'push' && 'CI' || github.event.label.name }} (${{ github.event_name }}) +run-name : Run ${{ inputs.event_name == 'push' && 'CI' || inputs.test }} (${{ inputs.event_name }}) on: - push: - branches: [ master, develop ] -# See https://stackoverflow.com/a/78444521 and -# https://github.com/orgs/community/discussions/26874#discussioncomment-3253755 -# as well as official (but buried) documentation : -# https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#pull-request-events-for-forked-repositories-2 - pull_request: - types: [ labeled ] + workflow_dispatch: + inputs: + event_name: + description: Event name that triggered this dispatch + required: false + default: push + type: string + event_number: + description: Event number that triggered this dispatch + required: false + default: 0 + type: string + ref: + description: Actual ref to use + required: false + default: null + type: string + sha: + description: Actual sha to use + required: false + default: null + type: string + test: + description: Test to run + required: true + default: all-tests + type: choice + options: + - all-tests + - compile-tests + # https://docs.github.com/en/actions/sharing-automations/reusing-workflows#supported-keywords-for-jobs-that-call-a-reusable-workflow # Also https://stackoverflow.com/a/74959635 @@ -30,7 +53,7 @@ on: # https://stackoverflow.com/a/68940067 jobs: buildtests: - if : ${{ github.event.label.name == 'compile-tests' || github.event.label.name == 'all-tests' || github.event_name == 'push' }} + if : ${{ contains( fromJson('["compile-tests","all-tests"]'), inputs.test ) || inputs.event_name == 'push' }} strategy: max-parallel: 4 fail-fast: false @@ -69,7 +92,7 @@ jobs: uses : ./.github/workflows/test_workflow.yml with : - # This should be the only hard-coded value, we don't use ${{ github.event.label.name }} + # This should be the only hard-coded value, we don't use ${{ inputs.test }} # to avoid 'all-tests' to be used in this workflow label : compile-tests @@ -86,26 +109,37 @@ jobs: args : ${{ matrix.testSet.args }} pool : ${{ matrix.testSet.pool }} tpool : ${{ matrix.testSet.tpool }} + + # required to emulate event trigger + event_name : ${{ inputs.event_name }} + event_number : ${{ inputs.event_number }} + event_label : ${{ inputs.test }} + ref : ${{ inputs.ref }} + sha : ${{ inputs.sha }} + # I am leaving this here for posterity if this is to be replicated in private repositories for testing permissions: contents: read pull-requests: write name : Test ${{ matrix.testSet.name }} on ${{ matrix.testSet.host }} + # In the event that 'all-tests' is used, this final job will be the one to remove # the label from the PR removeAllLabel : - if : ${{ !cancelled() && github.event.label.name == 'all-tests' }} + if : ${{ !cancelled() && inputs.test == 'all-tests' }} name : Remove 'all-tests' label runs-on: ubuntu-latest needs : [ buildtests ] # Put tests here to make this wait for the tests to complete + permissions: + pull-requests: write steps: - - name : Remove '${{ github.event.label.name }}' label + - name : Remove '${{ inputs.test }}' label env: - PR_NUMBER: ${{ github.event.number }} + PR_NUMBER: ${{ inputs.event_number }} run: | curl \ -X DELETE \ -H "Accept: application/vnd.github.v3+json" \ -H 'Authorization: token ${{ github.token }}' \ - https://api.github.com/repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/labels/${{ github.event.label.name }} + https://api.github.com/repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/labels/${{ inputs.test }} diff --git a/.github/workflows/test_workflow.yml b/.github/workflows/test_workflow.yml index e80d6f3392..ce08d1da1a 100644 --- a/.github/workflows/test_workflow.yml +++ b/.github/workflows/test_workflow.yml @@ -46,6 +46,22 @@ on : required : false type : number default : 1 + + event_name : + required : true + type : string + event_number: + required : true + type : string + event_label: + required : true + type : string + ref : + required : true + type : string + sha : + required : true + type : string @@ -57,12 +73,14 @@ jobs: name: Test ${{ inputs.name }} on ${{ inputs.host }} runs-on: ${{ inputs.host }} env : - LOG_SUFFIX : ${{ github.event_name == 'push' && 'master' || github.event.number }} + LOG_SUFFIX : ${{ inputs.event_name == 'push' && github.ref_name || inputs.event_number }} steps: - uses: actions/checkout@v4 with: path : main submodules: true + ref: ${{ inputs.event_name == 'push' && github.ref || inputs.ref }} + # Immediately copy out to # of tests to do - name: Create testing directories @@ -90,7 +108,7 @@ jobs: -t ${{ join( fromJson( inputs.tests ), ' ' ) }} \ -a "${{ inputs.account }}" \ -p ${{ inputs.pool}} -tp ${{ inputs.tpool }} \ - -jn ${{ github.event_name == 'push' && github.ref_name || github.event.number }}-${{ inputs.id }} \ + -jn ${{ inputs.event_name == 'push' && github.ref_name || inputs.event_number }}-${{ inputs.id }} \ ${{ inputs.args }} $ALT_DIRS @@ -133,8 +151,8 @@ jobs: # *documented* functionality doesn't work as expected. Wow, bravo # can't use ${{ env. }} as somehow this combination of matrix->reusable workflow->call step is too complex # and expands to nothing - name: ${{ github.event_name == 'push' && github.ref_name || github.event.number }}-${{ inputs.id }}_logfiles - path: ${{ inputs.archive }}/${{ github.event_name == 'push' && github.ref_name || github.event.number }}/${{ inputs.id }}/ + name: ${{ inputs.event_name == 'push' && github.ref_name || inputs.event_number }}-${{ inputs.id }}_logfiles + path: ${{ inputs.archive }}/${{ inputs.event_name == 'push' && github.ref_name || inputs.event_number }}/${{ inputs.id }}/ # As noted in ci.yml, this will need to be moved to a separate workflow with pull_request_target # and strictly controlled usage of the GH token From 25f485237e60b98e9cf7c352a1f4e956d8f18958 Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Fri, 13 Dec 2024 17:11:14 -0700 Subject: [PATCH 2/4] Use commit statuses to reinform PRs of run state The input trigger or default push shall inform which SHA to use, which will effectively reinform the PR of the state of the workflow run. This now allows the workflow to deliberately override or post workflow runs to a PR without implicit events removing previous statuses or skipped jobs polluting the PR state. --- .github/workflows/ci.yml | 1 + .github/workflows/test_workflow.yml | 57 +++++++++++++++++++++++------ 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5e02138c15..79a88db082 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -121,6 +121,7 @@ jobs: permissions: contents: read pull-requests: write + statuses: write name : Test ${{ matrix.testSet.name }} on ${{ matrix.testSet.host }} diff --git a/.github/workflows/test_workflow.yml b/.github/workflows/test_workflow.yml index ce08d1da1a..fb5eadbf23 100644 --- a/.github/workflows/test_workflow.yml +++ b/.github/workflows/test_workflow.yml @@ -75,6 +75,25 @@ jobs: env : LOG_SUFFIX : ${{ inputs.event_name == 'push' && github.ref_name || inputs.event_number }} steps: + # Don't use gh checks as they are woefully underdeveloped as a feature leading + # to confusing UI and misplaced metrics + # https://github.com/orgs/community/discussions/24616 + - name: Set pending status + id: check_run_start + uses: actions/github-script@v7 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + github.rest.repos.createCommitStatus({ + owner: context.repo.owner, + repo: context.repo.repo, + sha: '${{ inputs.event_name == 'push' && github.sha || inputs.sha }}', + target_url: '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}', + description: '${{ inputs.name }}', + context: '${{ inputs.host }}/${{ inputs.id }}', + state: 'pending' + }) + - uses: actions/checkout@v4 with: path : main @@ -154,18 +173,32 @@ jobs: name: ${{ inputs.event_name == 'push' && github.ref_name || inputs.event_number }}-${{ inputs.id }}_logfiles path: ${{ inputs.archive }}/${{ inputs.event_name == 'push' && github.ref_name || inputs.event_number }}/${{ inputs.id }}/ - # As noted in ci.yml, this will need to be moved to a separate workflow with pull_request_target - # and strictly controlled usage of the GH token - # - name : Remove '${{ inputs.label }}' label - # if : ${{ !cancelled() && github.event.label.name == inputs.label }} - # env: - # PR_NUMBER: ${{ github.event.number }} - # run: | - # curl \ - # -X DELETE \ - # -H "Accept: application/vnd.github.v3+json" \ - # -H 'Authorization: token ${{ github.token }}' \ - # https://api.github.com/repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/labels/${{ inputs.label }} + - name: Set completed status + if: ${{ always() }} + uses: actions/github-script@v7 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + github.rest.repos.createCommitStatus({ + owner: context.repo.owner, + repo: context.repo.repo, + sha: '${{ inputs.event_name == 'push' && github.sha || inputs.sha }}', + target_url: '${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}', + description: '${{ inputs.name }}', + context: '${{ inputs.host }}/${{ inputs.id }}', + state: '${{ job.status == 'success' && 'success' || 'failure' }}' + }) + + - name : Remove '${{ inputs.label }}' label + if : ${{ !cancelled() && inputs.event_label == inputs.label }} + env: + PR_NUMBER: ${{ inputs.event_number }} + run: | + curl \ + -X DELETE \ + -H "Accept: application/vnd.github.v3+json" \ + -H 'Authorization: token ${{ github.token }}' \ + https://api.github.com/repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/labels/${{ inputs.label }} From c22e2a1cc6ff33d0eb2c5da03bd698ed83a10249 Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Fri, 13 Dec 2024 17:15:59 -0700 Subject: [PATCH 3/4] Improve logging to provide accurate summary and artifacts --- .ci/hpc-workflows | 2 +- .github/workflows/test_workflow.yml | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/.ci/hpc-workflows b/.ci/hpc-workflows index dfc8e6d823..043a4b6711 160000 --- a/.ci/hpc-workflows +++ b/.ci/hpc-workflows @@ -1 +1 @@ -Subproject commit dfc8e6d823b80497ea41bab94e1fdf3f4594ad18 +Subproject commit 043a4b6711e7f7d6dd41bd932e6e7336334e9a53 diff --git a/.github/workflows/test_workflow.yml b/.github/workflows/test_workflow.yml index fb5eadbf23..a4060dcb6c 100644 --- a/.github/workflows/test_workflow.yml +++ b/.github/workflows/test_workflow.yml @@ -142,16 +142,20 @@ jobs: masterlogLoc=main/.ci fi ./main/${{ inputs.hpc-workflows_path }}/.ci/reporter.py ${{ inputs.archive }}/$LOG_SUFFIX/${{ inputs.id }}/$masterlogLoc/${{ inputs.fileroot }}.log \ - -e ./${{ inputs.hpc-workflows_path }}/.ci/runner.py \ - -o GITHUB -m # only mark fail steps with gh syntax + -e ./${{ inputs.hpc-workflows_path }}/.ci/runner.py -p ./.ci \ + -o GITHUB -m -n # only mark fail steps with gh syntax + + echo "Relaying results to summary..." # report on them - echo "# Summary for ${{ join( fromJson( inputs.tests ), ' ' ) }}" >> $GITHUB_STEP_SUMMARY + echo "# Summary for ${{ inputs.name }}" >> $GITHUB_STEP_SUMMARY echo "\`\`\`" >> $GITHUB_STEP_SUMMARY ./main/${{ inputs.hpc-workflows_path }}/.ci/reporter.py ${{ inputs.archive }}/$LOG_SUFFIX/${{ inputs.id }}/$masterlogLoc/${{ inputs.fileroot }}.log \ - -e ./${{ inputs.hpc-workflows_path }}/.ci/runner.py \ - -s >> $GITHUB_STEP_SUMMARY + -e ./${{ inputs.hpc-workflows_path }}/.ci/runner.py -p ./.ci \ + -s -n >> $GITHUB_STEP_SUMMARY echo "\`\`\`" >> $GITHUB_STEP_SUMMARY + # We know this is a failure + exit 1 - name: Clean up testing directories if : ${{ success() }} @@ -172,6 +176,7 @@ jobs: # and expands to nothing name: ${{ inputs.event_name == 'push' && github.ref_name || inputs.event_number }}-${{ inputs.id }}_logfiles path: ${{ inputs.archive }}/${{ inputs.event_name == 'push' && github.ref_name || inputs.event_number }}/${{ inputs.id }}/ + include-hidden-files: true - name: Set completed status if: ${{ always() }} From 91f918bf210b14bc98a6ffa5e1c1706c84376185 Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Fri, 13 Dec 2024 17:18:04 -0700 Subject: [PATCH 4/4] Provide an entry point for trigger events to facilitate dispatch workflow This simple entry point workflow should funnel event webhooks to the dispatch workflow IF the event is deemed an actual test event. This combined with the decoupled dispatch workflow is how PRs will still be able to trigger testing without needing manual running of the dispatch workflow. Likewise, it will stop unnecessary skipped job statuses from showing up on the PR. The status of this workflow will inevitably be constantly overriden from multiple label events, but the published commit status of the dispatch workflow, if queued, will provide the handle to the run via a target_url. Since the workflow_dispatch webhook event cannot take a pull request merge ref, certain measures must be taken to allow running the workflow in a valid context. For PRs originating from forks, it is impossible to run the workflow in the parent repo using the head ref as that branch exists in a different repo. In this case we will run the base ref of the PR. This has the added benefit of security, ensuring the runners never run workflows from outside sources. Unfortunately, this would limit critical patches to the workflow from being demonstated within a PR. To allow for this use case, when a PR from an internal repo branch is used, the dispatch event will be made using the PR branch head ref, thus running the changes. This paradigm will also guarantee that the workflow_dispatch will only ever use internal refs, increasing security and allowing label modifications. --- .github/workflows/entry_point.yml | 54 +++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 .github/workflows/entry_point.yml diff --git a/.github/workflows/entry_point.yml b/.github/workflows/entry_point.yml new file mode 100644 index 0000000000..ed4ce200cd --- /dev/null +++ b/.github/workflows/entry_point.yml @@ -0,0 +1,54 @@ +name: Regression Suite Entry Point CI/CD +run-name : Queue ${{ github.event_name == 'push' && 'CI' || github.event.label.name }} (${{ github.event_name }}) + +on: + push: + branches: [ master, develop ] +# See https://stackoverflow.com/a/78444521 and +# https://github.com/orgs/community/discussions/26874#discussioncomment-3253755 +# as well as official (but buried) documentation : +# https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#pull-request-events-for-forked-repositories-2 + pull_request: + types: [ labeled ] + +# Write our tests out this way for easier legibility +# testsSet : +# - key : value +# key : value +# tests : +# - value +# - value +# - < next test > +# https://stackoverflow.com/a/68940067 +jobs: + queue_tests: + if : ${{ contains( fromJson('["compile-tests","all-tests"]'), github.event.label.name ) || github.event_name == 'push' }} + name: Queue Test (${{ github.event_name == 'push' && github.ref_name || github.event.label.name }}) + runs-on: ubuntu-latest + permissions: + actions: write + steps: + - name: Dispatch Regression Suite + run : | + curl -L \ + -X POST \ + -H "Accept: application/vnd.github+json" \ + -H "Authorization: Bearer ${{ github.token }}" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + https://api.github.com/repos/${GITHUB_REPOSITORY}/actions/workflows/ci.yml/dispatches \ + --data-binary @- << EOF + { + "ref" : "${{ github.event_name == 'push' && github.ref_name || github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.head.ref || github.event.pull_request.base.ref }}", + "inputs" : + { + "event_name" : "${{ github.event_name }}", + "event_number" : "${{ github.event.number }}", + "test" : "${{ github.event.label.name }}", + "ref" : "${{ github.ref }}", + "sha" : "${{ github.event_name == 'push' && github.sha || github.event.pull_request.head.sha }}" + } + } + EOF + + +