From ac348e69776e5a33a0f0c265426e7283bf1cbde6 Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Thu, 8 Aug 2024 14:41:30 -0700 Subject: [PATCH 01/12] Add the environment helper scripts for testing In order to run test scripts outside of a testing framework, the handling of environment setup should not be solely dependent on running within a dedicated test framework. This has the added benefit of compartmentalizing the duties of environment and dependency solving from running the tests. These environment scripts allow for the selection of a particular environment with the default being the fqdn of the current host. From there, arguments are routed using standard POSIX-sh to a respective script. In the case of Derecho (applicable to any system using lmod) all subsequent argument are treated as modules to load into the current session. The hostenv.sh script relies on one "argument" $AS_HOST being passed in via variable setting to facilitate selection. The helpers.sh script provides convenience features for substing checking in sh, delayed environment variable expansion via eval, and quick banner creation. The derecho.sh script is included as the first supported environment. --- .ci/env/derecho.sh | 22 ++++++++++++++++++++++ .ci/env/helpers.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++++ .ci/env/hostenv.sh | 16 ++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100755 .ci/env/derecho.sh create mode 100644 .ci/env/helpers.sh create mode 100644 .ci/env/hostenv.sh diff --git a/.ci/env/derecho.sh b/.ci/env/derecho.sh new file mode 100755 index 0000000000..ded0fd7a13 --- /dev/null +++ b/.ci/env/derecho.sh @@ -0,0 +1,22 @@ +#!/bin/sh + +echo "Setting up derecho environment" +workingDirectory=$PWD +. /etc/profile.d/z00_modules.sh +echo "Loading modules : $*" +cmd="module purge" +echo $cmd && eval "${cmd}" + +# We should be handed in the modules to load +while [ $# -gt 0 ]; do + cmd="module load $1" + echo $cmd && eval "${cmd}" + shift +done + +# Go back for asinine reasons of HPC config changing your directory on you +if [ "$workingDirectory" != "$PWD" ]; then + echo "derecho module loading changed working directory" + echo " Moving back to $workingDirectory" + cd $workingDirectory +fi diff --git a/.ci/env/helpers.sh b/.ci/env/helpers.sh new file mode 100644 index 0000000000..2ddd560893 --- /dev/null +++ b/.ci/env/helpers.sh @@ -0,0 +1,46 @@ +#!/bin/sh + +# Useful string manipulation functions, leaving in for posterity +# https://stackoverflow.com/a/8811800 +# contains(string, substring) +# +# Returns 0 if the specified string contains the specified substring, +# otherwise returns 1. +contains() +{ + string="$1" + substring="$2" + + if [ "${string#*"$substring"}" != "$string" ]; then + echo 0 # $substring is in $string + else + echo 1 # $substring is not in $string + fi +} + +setenvStr() +{ + # Changing IFS produces the most consistent results + tmpIFS=$IFS + IFS="," + string="$1" + for s in $string; do + if [ ! -z $s ]; then + eval "echo export \"$s\"" + eval "export \"$s\"" + fi + done + IFS=$tmpIFS +} + +banner() +{ + lengthBanner=$1 + shift + # https://www.shellscript.sh/examples/banner/ + printf "#%${lengthBanner}s#\n" | tr " " "=" + printf "# %-$(( ${lengthBanner} - 2 ))s #\n" "`date`" + printf "# %-$(( ${lengthBanner} - 2 ))s #\n" " " + printf "# %-$(( ${lengthBanner} - 2 ))s #\n" "$*" + printf "#%${lengthBanner}s#\n" | tr " " "=" +} diff --git a/.ci/env/hostenv.sh b/.ci/env/hostenv.sh new file mode 100644 index 0000000000..208d1e57f3 --- /dev/null +++ b/.ci/env/hostenv.sh @@ -0,0 +1,16 @@ +#!/bin/sh + +# Allow selection of hostname, and if none is provided use the current machine +# While this may seem unintuitive at first, it provides the flexibility of using +# "named" configurations without being explicitly tied to fqdn +hostname=$AS_HOST +if [ -z "$hostname" ]; then + hostname=$( python3 -c "import socket; print( socket.getfqdn() )" ) +fi + +if [ $( contains ${hostname} hsn.de.hpc ) -eq 0 ]; then + # Derecho HPC SuSE PBS + . .ci/env/derecho.sh +else + echo "No known environment for '${hostname}', using current" +fi From d2192dc66f924e004932cccdea72cbae23c6aece Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Thu, 8 Aug 2024 15:02:24 -0700 Subject: [PATCH 02/12] Add a compilation test script This script will facilitate the first tests. There are only three requirements of any given test script with the planned testing framework. If a different testing framework is used in the future, these requirements of the test scripts can and should be re-evaluated. The test script should : 1. Take the intended host / configuration environment as the first argument 2. Take the working directory to immediately change to as the second argument 3. Output some key phrase at the end of the test to denote success, anything else (non-zero exit code, no phrase but return zero) is a failure This particular compilation test script satisfies the above while also providing enough flexibility to select compile target, stanza configuration, parallel jobs, and other command-line options into the make build. Additionally, for convenience environment variables can be passed in as command-line options to the test script to modularize certain inputs. --- .ci/tests/build.sh | 108 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100755 .ci/tests/build.sh diff --git a/.ci/tests/build.sh b/.ci/tests/build.sh new file mode 100755 index 0000000000..a0598fd499 --- /dev/null +++ b/.ci/tests/build.sh @@ -0,0 +1,108 @@ +#!/bin/sh +help() +{ + echo "./build.sh as_host workingdir [options] [-- ]" + echo " as_host First argument must be the host configuration to use for environment loading" + echo " workingdir First argument must be the working dir to immediate cd to" + echo " -c Configuration build type, piped directly into configure" + echo " -n Configuration nesting type, piped directly into configure" + echo " -o Configuration optstring passed into configure" + echo " -b Build command passed into compile" + echo " -e environment variables in comma-delimited list, e.g. var=1,foo,bar=0" + echo " -- Directly pass options to hostenv.sh, equivalent to hostenv.sh " + echo " -h Print this message" + echo "" + echo "If you wish to use an env var in your arg such as '-c \$SERIAL -e SERIAL=32', you must" + echo "you will need to do '-c \\\$SERIAL -e SERIAL=32' to delay shell expansion" +} + +echo "Input arguments:" +echo "$*" + +AS_HOST=$1 +shift +if [ $AS_HOST = "-h" ]; then + help + exit 0 +fi + +workingDirectory=$1 +shift + +cd $workingDirectory + +# Get some helper functions +. .ci/env/helpers.sh + +while getopts c:n:o:b:e:h opt; do + case $opt in + c) + configuration="$OPTARG" + ;; + n) + nesting="$OPTARG" + ;; + o) + configOpt="$OPTARG" + ;; + b) + buildCommand="$OPTARG" + ;; + e) + envVars="$envVars,$OPTARG" + ;; + h) help; exit 0 ;; + *) help; exit 1 ;; + :) help; exit 1 ;; + \?) help; exit 1 ;; + esac +done + +shift "$((OPTIND - 1))" + +# Everything else goes to our env setup +. .ci/env/hostenv.sh $* + +# Now evaluate env vars in case it pulls from hostenv.sh +if [ ! -z "$envVars" ]; then + setenvStr "$envVars" +fi + +# Re-evaluate input values for delayed expansion +eval "configuration=\"$configuration\"" +eval "nesting=\"$nesting\"" +eval "configOpt=\"$configOpt\"" +eval "buildCommand=\"$buildCommand\"" + +./clean -a + +echo "Compiling with option $configuration nesting=$nesting and additional flags '$configOpt'" +./configure $configOpt << EOF +$configuration +$nesting +EOF + +if [ ! -f configure.wrf ]; then + echo "Failed to configure" + exit 1 +fi + +echo "./compile $buildCommand" +./compile $buildCommand + +result=$? + +if [ $result -ne 0 ]; then + echo "Failed to compile" + exit 1 +fi + +# And a *very* special check because WRF compiles the WRF way and force-ignores all make errors +# putting the onus on US to check for things +if [ ! -x ./main/wrf.exe ]; then # There's a bunch of other execs but this is the most important and + # doing more checks to accomodate just reinforces this bad design + echo "Failed to compile" + exit 1 +fi + +echo "TEST $(basename $0) PASS" From 5340db7101fbc78770656c667dbd343064f5b758 Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Thu, 8 Aug 2024 15:06:34 -0700 Subject: [PATCH 03/12] Add a framework to easily facilitate testing --- .ci/hpc-workflows | 1 + .gitmodules | 3 +++ 2 files changed, 4 insertions(+) create mode 160000 .ci/hpc-workflows diff --git a/.ci/hpc-workflows b/.ci/hpc-workflows new file mode 160000 index 0000000000..8b5fb39726 --- /dev/null +++ b/.ci/hpc-workflows @@ -0,0 +1 @@ +Subproject commit 8b5fb39726f3800ed5dcc3e4d2ad0e82de5a6497 diff --git a/.gitmodules b/.gitmodules index 4e323edda7..5d206bcbf5 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,6 @@ [submodule "phys/noahmp"] path = phys/noahmp url = https://github.com/NCAR/noahmp +[submodule ".ci/hpc-workflows"] + path = .ci/hpc-workflows + url = https://github.com/islas/hpc-workflows From 6870c60ce5b832838004f5c0820c4545d5e313df Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Thu, 8 Aug 2024 15:28:26 -0700 Subject: [PATCH 04/12] Add a simple test definition for testing GNU make builds Following the documentation of the hpc-workflows testing framework and the testing structure found in .ci/, a JSON file for a GNU compilation test was added. This test will compile the em_real core using the GNU Linux x86 stanza configuration. All other options are left as default. If this test is run using the derecho configuration the appropriate modules will attempt to be loaded. For non-derecho environments, per the testing structure under .ci/, if no configuration exists in .ci/hostenv.sh then the current environment wil be used verbatim. --- .ci/wrf_compilation_tests-make.json | 69 +++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 .ci/wrf_compilation_tests-make.json diff --git a/.ci/wrf_compilation_tests-make.json b/.ci/wrf_compilation_tests-make.json new file mode 100644 index 0000000000..b80a2b8b53 --- /dev/null +++ b/.ci/wrf_compilation_tests-make.json @@ -0,0 +1,69 @@ +{ + "submit_options" : + { + "timelimit" : "00:20:00", + "working_directory" : "..", + "arguments" : + { + "base_env_numprocs" : [ "-e", "NUM_PROCS=4" ], + + ".*make.*::args_nesting" : [ "-n", "1" ], + ".*make.*::args_configopt" : [ "-o", "-d" ], + ".*make.*::args_build_tgt" : [ "-b", "em_real -j $NUM_PROCS" ] + }, + "hsn.de.hpc" : + { + "submission" : "PBS", + "queue" : "main", + "hpc_arguments" : + { + "node_select" : { "-l " : { "select" : 1, "ncpus" : 16 } }, + "priority" : { "-l " : { "job_priority" : "economy" } } + }, + "arguments" : + { + "base_env_numprocs" : [ "-e", "NUM_PROCS=16" ], + "very_last_modules" : [ "netcdf" ], + ".*gnu.*::test_modules" : [ "gcc" ], + ".*intel(?!-llvm).*::test_modules" : [ "intel-classic" ], + ".*intel-llvm.*::test_modules" : [ "intel-oneapi" ], + ".*pgi.*::test_modules" : [ "nvhpc" ], + ".*dm.*::test_mpi_module" : [ "cray-mpich" ] + } + } + }, + "make-gnu" : + { + "steps" : + { + "serial" : + { + "command" : ".ci/tests/build.sh", + "arguments" : [ "-c", "32" ] + }, + "sm" : + { + "command" : ".ci/tests/build.sh", + "arguments" : [ "-c", "33" ], + "dependencies" : { "serial" : "afterany" } + } + } + }, + "make-gnu-mpi" : + { + "steps" : + { + "dm" : + { + "command" : ".ci/tests/build.sh", + "arguments" : [ "-c", "34" ] + }, + "dm+sm" : + { + "command" : ".ci/tests/build.sh", + "arguments" : [ "-c", "35" ], + "dependencies" : { "dm" : "afterany" } + } + } + } +} From 8adbae205bd8df0175233ab8cf9e9bed26ee6b0d Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Thu, 8 Aug 2024 15:36:44 -0700 Subject: [PATCH 05/12] Add a reusable workflow to quickly setup CI/CD tests using hpc-workflows This reusable workflow balances quick setup with github actions-specific features. It assumes that the tests can be controlled via a label being set in a PR. To coordinate PR vs primary branch testing, a suffix is generated using either the PR number or the branch name. This suffix is then used to relocate log files to an archival location in an organized fashion. Github artifacts are still used for failed test capture, but logs will also be moved to the archive location for quicker access if one has access to where these tests execute. To allow for parallelized testing available from hpc-workflows, the workflow can make duplicate directories of the repository that can each run their own test instance without clobbering files. Once tests are run, results are gathered, relocated to archival location, reported and printed to the screen, summarized into the actions summary page, and then packaged into an artifact if failure occured. Finally, the test label is removed if the named tests and label match. --- .github/workflows/test_workflow.yml | 149 ++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 .github/workflows/test_workflow.yml diff --git a/.github/workflows/test_workflow.yml b/.github/workflows/test_workflow.yml new file mode 100644 index 0000000000..20a987e73b --- /dev/null +++ b/.github/workflows/test_workflow.yml @@ -0,0 +1,149 @@ + + +on : + workflow_call : + inputs : + label : + required : true + type : string + hpc-workflows_path : + required : true + type : string + archive : + required : true + type : string + + name : + required : true + type : string + id : + required : true + type : string + host : + required : true + type : string + fileroot : + required : true + type : string + account : + required : true + type : string + tests : + required : true + type : string + mkdirs : + required : true + type : boolean + args : + required : false + type : string + default : "" + pool : + required : false + type : number + default : 1 + tpool : + required : false + type : number + default : 1 + + + +jobs: + test_workflow : + + # Is 5 days a reasonable wait time for testing? + timeout-minutes: 7200 + name: Test ${{ inputs.name }} on ${{ inputs.host }} + runs-on: ${{ inputs.host }} + env : + LOG_SUFFIX : ${{ github.event_name == 'push' && 'master' || github.event.number }} + steps: + - uses: actions/checkout@v4 + with: + path : main + submodules: true + + # Immediately copy out to # of tests to do + - name: Create testing directories + if : ${{ inputs.mkdirs }} + id : cpTestDirs + run : | + for testDir in ${{ join( fromJson( inputs.tests ), ' ' ) }}; do + echo "Creating duplicate directory for $testDir" + # Remove if it exists to get a fresh start + rm -rf $testDir + cp -Rp main/ $testDir + done + + - name: Test ${{ inputs.name }} + id : runTest + run: | + if [ "${{ inputs.mkdirs }}" = "true" ]; then + ALT_DIRS="-alt ../${{ join( fromJson( inputs.tests ), '/.ci ../' ) }}/.ci" + fi + ./main/${{ inputs.hpc-workflows_path }}/.ci/runner.py \ + ./main/.ci/${{ inputs.fileroot }}.json \ + -t ${{ join( fromJson( inputs.tests ), ' ' ) }} \ + -a "${{ inputs.account }}" \ + -p ${{ inputs.pool}} -tp ${{ inputs.tpool }} \ + ${{ inputs.args }} $ALT_DIRS + + + - name: Report failed tests and steps + if : ${{ failure() }} + run : | + # move log files to safe location + ./main/${{ inputs.hpc-workflows_path }}/.ci/relocator.py ./main/.ci/${{ inputs.fileroot }}.log ${{ inputs.archive }}/$LOG_SUFFIX/${{ inputs.id }} + + # report on them - alt dirs need extra help + if [ "${{ inputs.mkdirs }}" = "true" ]; then + 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 + + # report on them + echo "# Summary for ${{ join( fromJson( inputs.tests ), ' ' ) }}" >> $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 + echo "\`\`\`" >> $GITHUB_STEP_SUMMARY + + - name: Clean up testing directories + if : ${{ success() }} + id : rmTestDirs + run : | + for testDir in ${{ join( fromJson( inputs.tests ), ' ' ) }}; do + echo "Removing duplicate directory for $testDir" + rm -rf $testDir + done + + - name: Upload test logs + if : ${{ failure() }} + uses : actions/upload-artifact@v4 + with: + # as per usual with ci/cd stuff I am shocked but not surprised when the advertised + # *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' && 'master' || github.event.number }}-${{ inputs.id }}_logfiles + path: ${{ inputs.archive }}/${{ github.event_name == 'push' && 'master' || github.event.number }}/${{ inputs.id }}/ + + + - 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 }} + + + + From 5b188709b2173a8a966eba86880b2aea6f4e86a6 Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Thu, 8 Aug 2024 15:52:20 -0700 Subject: [PATCH 06/12] Add CI/CD tests to the repository using github actions This pipeline is triggered if any pushes occur on master or develop OR if a PR is labeled with an appropriate tag as specified by the tests within this workflow. Additionally, a specific label to trigger all tests can be used that will be removed from the PR when all tests finish, regardless of exit status. The pipeline makes extensive use of the reusable test_workflow.yml to instantiate tests on runners. This pipeline currently only includes the definition for one test to be run on a github runner with tags that satisfy "derecho". Likewise, other hard-coded values appearing in here assume a particular runner setup and environment. --- .github/workflows/ci.yml | 85 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000000..a8d4c6675a --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,85 @@ +name: Regression Suite +run-name : ${{ github.event_name == 'push' && 'CI' || github.event.label.name }} (${{ github.event_name }}) + +on: + push: + branches: [ master, develop ] + 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: + buildtests: + if : ${{ github.event.label.name == 'compile-tests' || github.event.label.name == 'all-tests' || github.event_name == 'push' }} + strategy: + max-parallel: 4 + fail-fast: false + matrix: + + testSet : + - host : derecho + hpc-workflows_path : .ci/hpc-workflows + archive : /glade/work/aislas/github/runners/wrf/derecho/logs/ + account : NMMM0012 + name : "Make Compilation Tests" + id : make-tests + fileroot : wrf_compilation_tests-make + args : -j='{"node_select":{"-l ":{"select":1}}}' + pool : 8 + tpool : 1 + mkdirs : true + tests : + - make-gnu + - make-gnu-mpi + # add new compilation tests here + + uses : ./.github/workflows/test_workflow.yml + with : + # This should be the only hard-coded value, we don't use ${{ github.event.label.name }} + # to avoid 'all-tests' to be used in this workflow + label : compile-tests + + # Everything below this should remain the same and comes from the testSet matrix + hpc-workflows_path : ${{ matrix.testSet.hpc-workflows_path }} + archive : ${{ matrix.testSet.archive }} + name : ${{ matrix.testSet.name }} + id : ${{ matrix.testSet.id }} + host : ${{ matrix.testSet.host }} + fileroot : ${{ matrix.testSet.fileroot }} + account : ${{ matrix.testSet.account }} + tests : ${{ toJson( matrix.testSet.tests ) }} + mkdirs : ${{ matrix.testSet.mkdirs }} + args : ${{ matrix.testSet.args }} + pool : ${{ matrix.testSet.pool }} + tpool : ${{ matrix.testSet.tpool }} + 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' }} + name : Remove 'all-tests' label + runs-on: ubuntu-latest + needs : [ buildtests ] # Put tests here to make this wait for the tests to complete + steps: + - name : Remove '${{ github.event.label.name }}' 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/${{ github.event.label.name }} From 8901c506bac28fd92bef795f8c8170e4edc1a78a Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Thu, 8 Aug 2024 16:12:50 -0700 Subject: [PATCH 07/12] Add .log files to .gitignore for testing output --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 876fb30491..09e62c68a5 100644 --- a/.gitignore +++ b/.gitignore @@ -36,3 +36,4 @@ wrfout_d* *.nc rsl.out.* rsl.error.* +*.log From 9ff08005c5a03801ce6bc95d542b34aea04a748e Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Thu, 12 Sep 2024 12:40:34 -0700 Subject: [PATCH 08/12] Adding a note about permissions for PR label modification --- .github/workflows/ci.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a8d4c6675a..7de20882f2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,12 @@ on: pull_request: types: [ labeled ] - +# https://docs.github.com/en/actions/sharing-automations/reusing-workflows#supported-keywords-for-jobs-that-call-a-reusable-workflow +# We must set the highest set of permissions here first +permissions : + contents : read + pull-requests : write + # Write our tests out this way for easier legibility # testsSet : # - key : value From 73e70675f0ae87592aa79fe771159079151a9434 Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Thu, 12 Sep 2024 18:20:13 -0700 Subject: [PATCH 09/12] Adjust triggers based on buried documentation to get proper permissions --- .github/workflows/ci.yml | 6 +++++- .github/workflows/test_workflow.yml | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7de20882f2..a00f29e3c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,11 @@ run-name : ${{ github.event_name == 'push' && 'CI' || github.event.label.name }} on: push: branches: [ master, develop ] - pull_request: +# 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_target: types: [ labeled ] # https://docs.github.com/en/actions/sharing-automations/reusing-workflows#supported-keywords-for-jobs-that-call-a-reusable-workflow diff --git a/.github/workflows/test_workflow.yml b/.github/workflows/test_workflow.yml index 20a987e73b..b2bd988290 100644 --- a/.github/workflows/test_workflow.yml +++ b/.github/workflows/test_workflow.yml @@ -62,6 +62,8 @@ jobs: - uses: actions/checkout@v4 with: path : main + ref : ${{ github.event.pull_request.head.ref }} + repository: ${{github.event.pull_request.head.repo.full_name}} submodules: true # Immediately copy out to # of tests to do From 87190fe15ba557fdfc8113393772549df9abed95 Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Thu, 12 Sep 2024 19:31:51 -0700 Subject: [PATCH 10/12] Removing the 'remove label' feature until a secure workflow can be established --- .github/workflows/ci.yml | 13 ++++++++----- .github/workflows/test_workflow.yml | 25 ++++++++++++------------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a00f29e3c1..ec396e2ce2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,14 +8,16 @@ on: # 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_target: + pull_request: types: [ labeled ] # https://docs.github.com/en/actions/sharing-automations/reusing-workflows#supported-keywords-for-jobs-that-call-a-reusable-workflow -# We must set the highest set of permissions here first -permissions : - contents : read - pull-requests : write +# Also https://stackoverflow.com/a/74959635 +# TL;DR - For public repositories the safest approach will be to use the default read permissions, but at the cost +# of not being able to modify the labels. That will need to be a separate [trusted] workflow that runs from the base repo +# permissions : +# contents : read +# pull-requests : write # Write our tests out this way for easier legibility # testsSet : @@ -70,6 +72,7 @@ jobs: args : ${{ matrix.testSet.args }} pool : ${{ matrix.testSet.pool }} tpool : ${{ matrix.testSet.tpool }} + # I am leaving this here for posterity if this is to be replicated in private repositories for testing permissions: contents: read pull-requests: write diff --git a/.github/workflows/test_workflow.yml b/.github/workflows/test_workflow.yml index b2bd988290..abcb901c0a 100644 --- a/.github/workflows/test_workflow.yml +++ b/.github/workflows/test_workflow.yml @@ -62,8 +62,6 @@ jobs: - uses: actions/checkout@v4 with: path : main - ref : ${{ github.event.pull_request.head.ref }} - repository: ${{github.event.pull_request.head.repo.full_name}} submodules: true # Immediately copy out to # of tests to do @@ -134,17 +132,18 @@ jobs: name: ${{ github.event_name == 'push' && 'master' || github.event.number }}-${{ inputs.id }}_logfiles path: ${{ inputs.archive }}/${{ github.event_name == 'push' && 'master' || github.event.number }}/${{ inputs.id }}/ - - - 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 }} + # 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 }} From 2b12cf237a6bf9216966c052b0270e0220800bf6 Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Fri, 13 Sep 2024 15:28:15 -0700 Subject: [PATCH 11/12] Use updated internal check on filenames for more tolerant regex --- .ci/hpc-workflows | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/hpc-workflows b/.ci/hpc-workflows index 8b5fb39726..ba8393447c 160000 --- a/.ci/hpc-workflows +++ b/.ci/hpc-workflows @@ -1 +1 @@ -Subproject commit 8b5fb39726f3800ed5dcc3e4d2ad0e82de5a6497 +Subproject commit ba8393447c8a2cef23952c01425154ceb34d64e4 From b14a5360a1ce788078dc292db38380e1d8e4dd65 Mon Sep 17 00:00:00 2001 From: Anthony Islas Date: Mon, 16 Sep 2024 16:20:18 -0700 Subject: [PATCH 12/12] Adjust comments to a neutral tone --- .ci/env/derecho.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/env/derecho.sh b/.ci/env/derecho.sh index ded0fd7a13..2feea5c9ee 100755 --- a/.ci/env/derecho.sh +++ b/.ci/env/derecho.sh @@ -14,7 +14,7 @@ while [ $# -gt 0 ]; do shift done -# Go back for asinine reasons of HPC config changing your directory on you +# Go back to working directory if for unknown reason HPC config changing your directory on you if [ "$workingDirectory" != "$PWD" ]; then echo "derecho module loading changed working directory" echo " Moving back to $workingDirectory"