Skip to content
Snippets Groups Projects

3-level CI build system for LALSuite

Merged Karl Wette requested to merge (removed):ci-3-level into master

Description

This MR implements a 3-level CI build system for LALSuite, following #559 (closed).

  1. Push events trigger a minimal CI pipeline which runs some basic build checks. The idea is that this pipeline will likely be run many times as a feature branch is developed, so it should minimize run time and resources. The follows CI jobs are run:
    1. A basic package-level build which build LALSuite libraries in sequence from the tarballs. This replaces the RPM/Debian/Conda package builds for push events only, since at this stage we only care whether LALSuite builds, not whether the packaging is working.
    2. Top-level build
    3. Documentation build
    4. Lint checks
  2. Merge requests trigger the full CI pipeline, which previously ran only as part of the nightly build. The idea is to avoid the perennially tedious situation where changes are merged into LALSuite, after the normal build passes, only for the nightly build to then fail some time later, which then requires people to go back and fix the broken build and/or revert changes (see e.g. !1997 (merged), !1999 (closed) for 2 recent examples) which wastes valuable person-power. Running the full CI pipeline at merge request time should prevent broken changes being pushed in the first place. (Note the the basic package-level build from the push CI pipeline is not build for MRs, as it's essentially redundant with the RPM/Debian/Conda package builds.)
  3. The scheduled nightly build now only runs deployment tasks, although some of those (e.g. updating Docker images) do require some build jobs to be re-run. Generally the nightly build should not run jobs that were not run during merge commits, to avoid the situation described above from occurring. Currently the only jobs in this category are the Koji build jobs, which should be fine as by this point the normal RPM builds will have passed.

(Perhaps in future, we may want to add a ~weekly pipeline which runs the full CI pipeline to test for upstream breakages, for now though I don't think that is necessary.)

If changes are being made that will (or are suspected will) affect the platform/compiler/packaging jobs, once can use special commit messages to includes those jobs in the push CI pipeline; for example, add [ci rpm] to a commit message to run the RPM packaging jobs as part of a normal push. Note that these messages are opposite from the previous [skip ...] messages which excluded jobs; these were billed as most useful for skipping the CI for e.g. documentation changes, which are relatively infrequent, and you then had to add a dozen [skip ...] messages to skip the entire CI build. I think it's more useful to have codes which include extra jobs into the minimal push pipeline which you might want to test before submitting an MR.

The new CI pipeline setup is documented in CONTRIBUTING.md.

I've also tried to minimize the number of jobs with retry: 1, to minimize cases where jobs are being rerun due to build failures. At the moment retry: 1 is only added to

  • jobs which AFAICT require external repositories (e.g. yum, apt-get) which might be down, i.e. the RPM/Debian/Conda, upgrade, platform, coverage tests;
  • MacOSX jobs, where from memory the CI runners can be a little more flaky than the Linux ones.

I've also used interruptible to denote which jobs can be cancelled when a new pipeline starts, which should also save some cycles. I've currently marked all jobs interruptible apart from MacOSX jobs (due to issues with permission errors; !1743 (merged)) and deploy jobs (which only run nightly anyway).

In order to get the full CI build to pass I've allowed the gcc:12 and upgrade:debian:bullseye to fail; once this is merged it should be more straightforward to get those fixed, as the MR CI pipeline won't pass unless they are fixed.

API Changes and Justification

Backwards Compatible Changes

  • This change does not modify any class/function/struct/type definitions in a public C header file or any Python class/function definitions
  • This change adds new classes/functions/structs/types to a public C header file or Python module

Backwards Incompatible Changes

  • This change modifies an existing class/function/struct/type definition in a public C header file or Python module
  • This change removes an existing class/function/struct/type from a public C header file or Python module

Review Status

Closes #559 (closed)

cc @adam-mercer @duncanmmacleod

Edited by Karl Wette

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • requested review from @adam-mercer and @duncanmmacleod

  • mentioned in issue #559 (closed)

  • Karl Wette added 5 commits

    added 5 commits

    • 06f023f2 - check_library_dependencies: check dependencies between CI :pkg build jobs
    • ac27a4f9 - CI: minor style/comment cleanups, brief documentation
    • c121787a - CI: trigger a (full) merge pipeline with "[ci full]" in the commit message
    • 86b3b3ab - CI: do not retry jobs by default
    • 8b3302bc - CONTRIBUTING.md: update description of CI pipeline

    Compare with previous version

  • Karl Wette added 2 commits

    added 2 commits

    • 79cf47fc - CI: allow jobs to be interrupted when a newer pipeline starts
    • c94662a8 - CONTRIBUTING.md: update description of CI pipeline

    Compare with previous version

  • Karl Wette changed the description

    changed the description

  • A deleted user added apinone docs infra_gitlab labels
    • Resolved by David Keitel

      How exactly does the push/MR pipeline separation work in practice? Is a "MR pipeline" only run when opening the MR and manually, or on every commit to the branch after a MR has been created? From these docs I think it's the latter, right?

      If so, then in order to actually make any runner load savings from this, developers will also need to be educated to change from the "make MR as early as possible" workflow that gitlab seems to encourage.

      On the other hand waveforms, where branches tend to be very long, and they already tend to do MRs only late in the game (usually after review is done), would rather have to bring their MR creation forward, as they're likely to run into platform-dependent issues and hence will need the full pipelines.

      Those are not objections; just pointing out this isn't purely a housekeeping change but will impact development workflows.

  • Karl Wette added 1 commit

    added 1 commit

    • 980e9d80 - CI: coverage job needs to be run nightly for pages job

    Compare with previous version

  • Karl Wette added 31 commits

    added 31 commits

    • 980e9d80...10cb6fce - 6 commits from branch lscsoft:master
    • 4562ee33 - CONTRIBUTING.md: minor style/wording cleanups
    • e02c0097 - CI: merge .nightly and .nightly-default builds into .ci-nightly-deploy
    • 7c8d0b2a - CI: rename .push rule to .ci-push-build
    • a3105e89 - CI: add .ci-merge-build rule for merge events
    • 74174258 - CI: run RPM/Debian/Conda jobs only on merges, and nightly deploy (for docker)
    • 82177000 - CI: run wheel jobs on merges, LALSuite tags, and nightly for deployment
    • 70a3f8fa - CI: run integration tests on merges, except also run 'toplevel' on pushes
    • cc5df312 - CI: run platform tests on merge events
    • e2b4bb7d - CI: build docker images nightly
    • 19b0cc9e - CI: run coverage on merge requests
    • a67998fb - CI: run lint checks on push and merge, except lint:depends and lint:combine-reports (merge only)
    • 8aa03d97 - CI: run compiler checks on merges
    • 218155e9 - CI: run docs on pushes, merges, and nightly for deployment
    • 71254d36 - CI: rename .tag to .ci-tag-build, .lalsuite-tag to .ci-lalsuite-tag-build
    • f7915bef - CI: remove unused rules
    • 9f998cd9 - CI: change "[skip ...]" rules to "[ci ...]" rules which include jobs
    • f7e5fd7d - CI: run merge request build on manual web trigger
    • b44ae8b4 - CI: add basic package-level build to run on pushes
    • 5ea05c00 - check_library_dependencies: check dependencies between CI :pkg build jobs
    • dc635edf - CI: minor style/comment cleanups, brief documentation
    • 101d25c7 - CI: trigger a (full) merge pipeline with "[ci full]" in the commit message
    • 1f928a42 - CI: do not retry jobs by default
    • 30e9a0bb - CI: allow jobs to be interrupted when a newer pipeline starts
    • 1e8c67ea - CI: coverage job needs to be run nightly for pages job
    • a14ae507 - CONTRIBUTING.md: update description of CI pipeline

    Compare with previous version

  • mentioned in issue #612 (closed)

  • Karl Wette added 34 commits

    added 34 commits

    • a14ae507...9db8298f - 7 commits from branch lscsoft:master
    • e956ef6c - CONTRIBUTING.md: minor style/wording cleanups
    • 63564abf - CI: merge .nightly and .nightly-default builds into .ci-nightly-deploy
    • 888b1b5d - CI: rename .push rule to .ci-push-build
    • ca52d710 - CI: add .ci-merge-build rule for merge events
    • deaedc11 - CI: run RPM/Debian/Conda jobs only on merges, and nightly deploy (for docker)
    • 19c92f35 - CI: run wheel jobs on merges, LALSuite tags, and nightly for deployment
    • 27aef6bf - CI: run integration tests on merges, except also run 'toplevel' on pushes
    • 21265372 - CI: run platform tests on merge events
    • f1701a20 - CI: build docker images nightly
    • c30ae9eb - CI: run coverage on merge requests
    • d341e663 - CI: run lint checks on push and merge, except lint:depends and lint:combine-reports (merge only)
    • fd5d2ab1 - CI: run compiler checks on merges
    • 4446824b - CI: run docs on pushes, merges, and nightly for deployment
    • 0de45aaf - CI: rename .tag to .ci-tag-build, .lalsuite-tag to .ci-lalsuite-tag-build
    • 1edac231 - CI: remove unused rules
    • c85420b2 - CI: change "[skip ...]" rules to "[ci ...]" rules which include jobs
    • a89e3ee0 - CI: run merge request build on manual web trigger
    • 407aabf9 - CI: add basic package-level build to run on pushes
    • e4d0c64a - check_library_dependencies: check dependencies between CI :pkg build jobs
    • c5c01523 - CI: minor style/comment cleanups, brief documentation
    • 2a0a7261 - CI: trigger a (full) merge pipeline with "[ci full]" in the commit message
    • 67a8ae0c - CI: do not retry jobs by default
    • 2184a28a - CI: allow jobs to be interrupted when a newer pipeline starts
    • 0385e8b3 - CI: coverage job needs to be run nightly for pages job
    • 1d0f015f - CI: pages should extend .deploy
    • 60241604 - CI: rename deploy:wheel to wheel:pypi
    • f48a6bb8 - CONTRIBUTING.md: update description of CI pipeline

    Compare with previous version

  • Karl Wette added 1 commit

    added 1 commit

    • b0a8bc9f - CONTRIBUTING.md: update description of CI pipeline

    Compare with previous version

  • Karl Wette added 1 commit

    added 1 commit

    • 777aa835 - CONTRIBUTING.md: update description of CI pipeline

    Compare with previous version

  • Karl Wette added 2 commits

    added 2 commits

    • b9e8ee72 - CI: add rule "[ci macos]" to run all MacOS jobs
    • fe9b0926 - CONTRIBUTING.md: update description of CI pipeline

    Compare with previous version

  • David Keitel resolved all threads

    resolved all threads

  • So I think this is good to go now? Looking through the changes it looks good. @duncanmmacleod and thoughts about this?

  • Karl Wette added 44 commits

    added 44 commits

    • fe9b0926...7b175414 - 15 commits from branch lscsoft:master
    • 474c15ff - CONTRIBUTING.md: minor style/wording cleanups
    • d27b8f66 - CI: merge .nightly and .nightly-default builds into .ci-nightly-deploy
    • af7d2f1b - CI: rename .push rule to .ci-push-build
    • a6e4c6f8 - CI: add .ci-merge-build rule for merge events
    • d3ce7ec6 - CI: run RPM/Debian/Conda jobs only on merges, and nightly deploy (for docker)
    • d394e440 - CI: run wheel jobs on merges, LALSuite tags, and nightly for deployment
    • ce05e2de - CI: run integration tests on merges, except also run 'toplevel' on pushes
    • 3edb2825 - CI: run platform tests on merge events
    • 8e4b8f56 - CI: build docker images nightly
    • 14283614 - CI: run coverage on merge requests
    • e255311a - CI: run lint checks on push and merge, except lint:depends and lint:combine-reports (merge only)
    • edda20c0 - CI: run compiler checks on merges
    • e3205bdd - CI: run docs on pushes, merges, and nightly for deployment
    • 5dea23e1 - CI: rename .tag to .ci-tag-build, .lalsuite-tag to .ci-lalsuite-tag-build
    • 7af24579 - CI: remove unused rules
    • e8ddb327 - CI: change "[skip ...]" rules to "[ci ...]" rules which include jobs
    • f70caad2 - CI: run merge request build on manual web trigger
    • b7805497 - CI: add basic package-level build to run on pushes
    • 9345a87f - check_library_dependencies: check dependencies between CI :pkg build jobs
    • ce0f7df1 - CI: minor style/comment cleanups, brief documentation
    • 9d10884f - CI: trigger a (full) merge pipeline with "[ci full]" in the commit message
    • 359d548a - CI: do not retry jobs by default
    • 3d7a85fe - CI: allow jobs to be interrupted when a newer pipeline starts
    • 113c9c66 - CI: coverage job needs to be run nightly for pages job
    • f1a7cfbf - CI: pages should extend .deploy
    • be622925 - CI: rename deploy:wheel to wheel:pypi
    • 2342adba - CI: add rule "[ci macos]" to run all MacOS jobs
    • c6a61c72 - CI: allow branch names to trigger CI jobs
    • d9bdbd04 - CONTRIBUTING.md: update description of CI pipeline

    Compare with previous version

  • Author Maintainer

    I've added one more feature: trigger extra CI jobs based on branch name as well as commit message text. For example, if you had a branch which refactors the Debian packages, you probably wouldn't want to add [ci debian] in every commit message to test the changes. Instead, you can name the branch to include the text -ci-debian (or _ci_debian) and it will always trigger the Debian jobs. (The MR pipeline will still be the same.)

    Otherwise this is ready to go IMHO.

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading