Skip to content

Commit

Permalink
Implement CI job optionality (iree-org#14312)
Browse files Browse the repository at this point in the history
With an increasing number of CI jobs and more resource issues, I think
it's useful for people to be able to specify which jobs they actually
want to run. In particular, I think it would be beneficial to make some
jobs, such as Windows, Mac, and A100, available for opt-in. Personally,
when adding new jobs, I am routinely hacking the ci.yml file to only
run specific jobs and I think I am probably better able to do that: I
see other people running the entire workflow repeatedly just to test
one job.

Now that we re-evaluate the PR description when rerunning the job,
people don't need to push another commit or anything after editing the
description: they can 'just' cancel and rerun the job (still not ideal).

I know people have struggled with the ergonomics of trailers for CI
control in the past. I tried to make the tag names follow a consistent
format and also added additional error checking like looking for
unknown usage of reserved prefixes. Weird tag combinations provide
immediate errors. The lack of correspondence between `skip-ci` and
`ci-skip` is a bit unfortunate. I'm open to other names, but I don't
think we should choose something less natural just to avoid this. We
should probably drop `skip-ci` entirely with this because you would
spell that `ci-skip: all`. That does have the disadvantage of not being
able to provide justification as part of the trailer, but that can be
done free-form in the PR description without losing much IMO.

One thing this doesn't do is any kind of dependency analysis. That is,
if you specify `ci-exactly: test_all`, it's not going to run
`build_all` and then the `test_all` job will just fail. This is
theoretically something it could do, since it's already parsing the
workflow file, but I'm not sure if that actually makes things less
confusing.

This also perhaps prompts cleaning up the job names so they're a bit
pithier and more intuitive. I think `build_test_all_windows` could just
be called `windows` for instance: this is a case where very structured
names actually get in the way IMO.

Fixes iree-org#10042

Tested running locally with various env variable settings. Tested a
few different options in this PR also.

ci-skip: all
  • Loading branch information
GMNGeoffrey authored Jul 7, 2023
1 parent 89919da commit 09f54d8
Show file tree
Hide file tree
Showing 6 changed files with 453 additions and 89 deletions.
54 changes: 27 additions & 27 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ jobs:
##############################################################################
build_all:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'build_all')
uses: ./.github/workflows/build_all.yml
with:
runner-group: ${{ needs.setup.outputs.runner-group }}
Expand All @@ -74,7 +74,7 @@ jobs:

build_test_all_windows:
needs: setup
if: fromJson(needs.setup.outputs.should-run) && ! fromJson(needs.setup.outputs.is-pr)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'build_test_all_windows')
runs-on: windows-2022-64core
defaults:
run:
Expand Down Expand Up @@ -132,7 +132,7 @@ jobs:

build_test_all_macos_arm64:
needs: setup
if: fromJson(needs.setup.outputs.should-run) && ! fromJson(needs.setup.outputs.is-pr)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'build_test_all_macos_arm64')
runs-on:
- ${{ github.repository == 'openxla/iree' && 'self-hosted' || 'macos-11' }} # must come first
- runner-group=postsubmit
Expand Down Expand Up @@ -169,7 +169,7 @@ jobs:
build_test_all_macos_x86_64:
needs: setup
if: fromJson(needs.setup.outputs.should-run) && ! fromJson(needs.setup.outputs.is-pr)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'build_test_all_macos_x86_64')
runs-on: macos-13-xl
env:
BUILD_DIR: build-macos
Expand Down Expand Up @@ -220,7 +220,7 @@ jobs:

build_test_all_bazel:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'build_test_all_bazel')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand All @@ -243,7 +243,7 @@ jobs:
test_all:
needs: [setup, build_all]
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'test_all')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down Expand Up @@ -273,7 +273,7 @@ jobs:
test_gpu:
needs: [setup, build_all]
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'test_gpu')
env:
BUILD_DIR: ${{ needs.build_all.outputs.build-dir }}
BUILD_DIR_ARCHIVE: ${{ needs.build_all.outputs.build-dir-archive }}
Expand Down Expand Up @@ -334,7 +334,7 @@ jobs:
##############################################################################
build_test_runtime:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'build_test_runtime')
runs-on: ubuntu-20.04-64core
env:
BUILD_DIR: build-runtime
Expand Down Expand Up @@ -362,7 +362,7 @@ jobs:
build_test_runtime_windows:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'build_test_runtime_windows')
runs-on: windows-2022-64core
defaults:
run:
Expand Down Expand Up @@ -391,7 +391,7 @@ jobs:
##############################################################################
test_tf_integrations:
needs: [setup, build_all]
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'test_tf_integrations')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down Expand Up @@ -421,7 +421,7 @@ jobs:
test_tf_integrations_gpu:
needs: [setup, build_all]
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'test_tf_integrations_gpu')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down Expand Up @@ -459,7 +459,7 @@ jobs:
##############################################################################
python_release_packages:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'python_release_packages')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down Expand Up @@ -498,7 +498,7 @@ jobs:
asan:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'asan')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down Expand Up @@ -526,7 +526,7 @@ jobs:
tsan:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'tsan')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand All @@ -551,7 +551,7 @@ jobs:
small_runtime:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'small_runtime')
runs-on: ubuntu-20.04-64core
env:
BUILD_DIR: build-runtime
Expand All @@ -578,7 +578,7 @@ jobs:
gcc:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'gcc')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down Expand Up @@ -610,7 +610,7 @@ jobs:
tracing:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'tracing')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down Expand Up @@ -639,7 +639,7 @@ jobs:
debug:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'debug')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down Expand Up @@ -668,7 +668,7 @@ jobs:
byo_llvm:
needs: setup
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'byo_llvm')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down Expand Up @@ -697,7 +697,7 @@ jobs:

build_benchmark_tools:
needs: [setup, build_all]
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'build_benchmark_tools')
uses: ./.github/workflows/build_benchmark_tools.yml
with:
runner-group: ${{ needs.setup.outputs.runner-group }}
Expand All @@ -708,7 +708,7 @@ jobs:

build_e2e_test_artifacts:
needs: [setup, build_all]
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'build_e2e_test_artifacts')
uses: ./.github/workflows/build_e2e_test_artifacts.yml
with:
runner-group: ${{ needs.setup.outputs.runner-group }}
Expand All @@ -720,7 +720,7 @@ jobs:

compilation_benchmarks:
needs: [setup, build_e2e_test_artifacts]
if: fromJson(needs.setup.outputs.should-run) && needs.setup.outputs.benchmark-presets != ''
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'compilation_benchmarks') && needs.setup.outputs.benchmark-presets != ''
uses: ./.github/workflows/benchmark_compilation.yml
with:
runner-group: ${{ needs.setup.outputs.runner-group }}
Expand All @@ -732,7 +732,7 @@ jobs:

execution_benchmarks:
needs: [setup, build_benchmark_tools, build_e2e_test_artifacts]
if: fromJson(needs.setup.outputs.should-run) && needs.setup.outputs.benchmark-presets != ''
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'execution_benchmarks') && needs.setup.outputs.benchmark-presets != ''
uses: ./.github/workflows/benchmark_execution.yml
with:
# env.GCS_DIR is also duplicated in this workflow. See the note there on
Expand All @@ -745,7 +745,7 @@ jobs:

process_benchmark_results:
needs: [setup, compilation_benchmarks, execution_benchmarks]
if: fromJson(needs.setup.outputs.should-run) && needs.setup.outputs.benchmark-presets != ''
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'process_benchmark_results') && needs.setup.outputs.benchmark-presets != ''
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down Expand Up @@ -837,7 +837,7 @@ jobs:

cross_compile_and_test:
needs: [setup, build_all]
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'cross_compile_and_test')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down Expand Up @@ -926,7 +926,7 @@ jobs:
# can't access matrix variable to skip certain matrix jobs for presubmit).
build_and_test_android:
needs: [setup, build_all]
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'build_and_test_android')
uses: ./.github/workflows/build_and_test_android.yml
with:
runner-group: ${{ needs.setup.outputs.runner-group }}
Expand All @@ -939,7 +939,7 @@ jobs:

test_benchmark_suites:
needs: [setup, build_all, build_e2e_test_artifacts]
if: fromJson(needs.setup.outputs.should-run)
if: contains(fromJson(needs.setup.outputs.enabled-jobs), 'test_benchmark_suites')
runs-on:
- self-hosted # must come first
- runner-group=${{ needs.setup.outputs.runner-group }}
Expand Down
13 changes: 8 additions & 5 deletions .github/workflows/setup.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ name: Setup
on:
workflow_call:
outputs:
should-run:
enabled-jobs:
description: |
Whether CI should run.
value: ${{ jobs.setup.outputs.should-run }}
Which jobs should run.
value: ${{ jobs.setup.outputs.enabled-jobs }}
is-pr:
description: |
Whether the workflow has been triggered by a pull request.
Expand Down Expand Up @@ -55,7 +55,7 @@ jobs:
# parent will be the tip of main.
BASE_REF: HEAD^
outputs:
should-run: ${{ steps.configure.outputs.should-run }}
enabled-jobs: ${{ steps.configure.outputs.enabled-jobs }}
is-pr: ${{ steps.configure.outputs.is-pr }}
runner-env: ${{ steps.configure.outputs.runner-env }}
runner-group: ${{ steps.configure.outputs.runner-group }}
Expand Down Expand Up @@ -111,9 +111,12 @@ jobs:
./build_tools/github_actions/configure_ci.py
- name: "Show benchmark presets"
- name: "Show enabled options"
env:
BENCHMARK_PRESETS: ${{ steps.configure.outputs.benchmark-presets }}
ENABLED_JOBS: ${{ join(fromJson(steps.configure.outputs.enabled-jobs)) }}
run: |
echo ":green_circle: Enabled jobs: \`${ENABLED_JOBS}\`" \
>> "${GITHUB_STEP_SUMMARY}"
echo ":stopwatch: Enabled benchmarks: \`${BENCHMARK_PRESETS}\`" \
>> "${GITHUB_STEP_SUMMARY}"
10 changes: 4 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,11 @@ later culprit-finding. It is assumed that the PR author will merge their change
unless they ask someone else to merge it for them (e.g. because they don't have
write access).

## Peculiarities
## Details

Our documentation on
[repository management](https://github.com/openxla/iree/blob/main/docs/developers/developing_iree/repository_management.md)
has more information on some of the oddities in our repository setup and
workflows. For the most part, these should be transparent to normal developer
workflows.
For further details, see our
[detailed contributing guide](/docs/developers/contributing.md), which is
separate to avoid notifying everyone every time it changes.

## Community Guidelines

Expand Down
Loading

0 comments on commit 09f54d8

Please sign in to comment.