Support full combinatorics in `gitlab-ci:mappings` by blue42u · Pull Request #32290 · spack/spack

blue42u · 2022-08-20T14:43:48Z

Currently spack ci generate chooses the first matching entry in gitlab-ci:mappings to fill attributes for a generated build-job, requiring that the entire configuration matrix is listed out explicitly. This unfortunately causes significant problems in environments with large configuration spaces, for example the environment in #31598 (spack.yaml) supports 5 operating systems, 3 architectures and 130 packages with explicit size requirements, resulting in 1300 lines (100KB) of configuration YAML.

This patch changes the logic in ci.py so that all matching entries are applied in order to the final build-job, allowing a few entries to cover an entire matrix of configurations. The environments in share/spack/gitlab/cloud_pipelines/stacks have been updated, their entries are now sorted from most general to most specific (i.e. exactly reversed). For example, these 6 entries are enough to cover all 9 possible configurations of operating system and architecture:

spack:
  gitlab-ci:
    tags: [small]
    mappings:
    - match: 'os=ubuntu18.04'
      runner-attributes:
        image: ghcr.io/spack/e4s-ubuntu-18.04
        tags: []
    - match: 'os=ubuntu20.04'
      runner-attributes:
        image: ghcr.io/spack/e4s-ubuntu-20.04
        tags: []
    - match: 'os=amzn2'
      runner-attributes:
        image: ghcr.io/spack/e4s-amazonlinux-2
        tags: []

    - match: 'target=x86_64'
      runner-attributes:
        tags: [x86_64]
    - match: 'target=aarch64'
      runner-attributes:
        tags: [aarch64]
    - match: 'target=ppc64le'
      runner-attributes:
        tags: [ppc64le]

A new remove-attributes field is added to allow a later specific match entry to override attributes that merge instead of simply override, it has similar syntax to runner-attributes but only supports tags and "acts in the reverse" when merging. For example, the following match entry will replace swap size-tags (small to large) for llvm build-jobs:

    - match: llvm
      remove-attributes:
        tags: [small]
      runner-attributes:
        tags: [large]

Documentation not yet included.

blue42u · 2022-08-20T14:47:05Z

@scottwittenburg @becker33
Does this seem like a reasonable change to the gitlab-ci semantics? The combinatorics needed for the configuration space we want to target is really becoming a problem for us.

becker33 · 2022-08-26T16:10:03Z

@blue42u I don't think I understand the goals this change accomplishes. The mapping configuration is frustratingly complicated, but it doesn't appear to be reduced in any meaningful way by this change. Aggregated across all the environment files you modified, the configuration actually grows by 6 lines, and I'm failing to see any real change in complexity either.

blue42u · 2022-08-26T16:26:14Z

@becker33 The environments currently in share/spack/gitlab/cloud_pipelines only support a single OS/arch triple at a time. They don't use combinatorics and thus don't reap any benefits from this patch.

The benefits are clear in the example outlined in the OP, the environment in question (now incorporated in #32327) is 1400 non-comment lines long, the gitlab-ci:mappings table itself is 100KB of dense, generated and unreadable YAML. Adding this patch will reduce that to around 300 very readable lines (much of which is copied verbatim from the E4S stack).

scottwittenburg

To borrow Greg's expression, I don't think it will be any less frustratingly complicated to apply runner attributes after this, but I do like the ability to cover more configuration space with fewer lines of yaml. I remember when @bollig asked for something like this, I'd be curious to hear what he thinks of this change.

I find remove-attributes to be a little unsatisfying, but I understand why it's necessary. And overall, I think this is a nice improvement. I'm interested to see how it works out. Thanks @blue42u!

bollig · 2022-09-01T17:24:24Z

Love this. This will reduce the number of separate pipeline configs and ensure each triple has consistent packages.

blue42u · 2022-09-02T22:17:32Z

Rebased, and simplified logic in _remove_attributes. No other changes.

blue42u · 2022-09-07T17:33:06Z

I'm not sure why the codecov/patch check is failing, clicking through "details" gives it a 91.67% diff coverage.

scottwittenburg

This looks good to me, thanks @blue42u 👍

scheibelp · 2022-10-12T16:31:38Z

@spackbot run pipeline

spackbot-app · 2022-10-12T16:31:43Z

I've started that pipeline for you!

blue42u · 2022-10-13T04:56:14Z

@scheibelp This is the updated version based on our discussion at the meeting today. I added a new gitlab-ci:match_behavior key that determines whether only the first match is applied (first, default) or whether all matches are merged in order (merge). I've also reverted the changes to the stacks, the only change now is setting match_behavior: first in each for clarity.

lib/spack/spack/schema/gitlab_ci.py

scottwittenburg · 2022-10-13T22:25:35Z

Is there a way to avoid duplicating the default value in the code? I tried using gitlab_ci["match_behavior"] but got KeyErrors when the entry wasn't given in the YAML

I feel like I was confused by this initially too. IIRC the default in the schema has nothing to do with populating live yaml documents, but is more just a reminder to developers who read the schema. So it serves more of a documentation purpose than anything else. Hopefully @scheibelp can straighten me out if I'm wrong here though.

scheibelp · 2022-10-13T23:59:56Z

Is there a way to avoid duplicating the default value in the code? I tried using gitlab_ci["match_behavior"] but got KeyErrors when the entry wasn't given in the YAML

I feel like I was confused by this initially too. IIRC the default in the schema has nothing to do with populating live yaml documents, but is more just a reminder to developers who read the schema. So it serves more of a documentation purpose than anything else. Hopefully @scheibelp can straighten me out if I'm wrong here though.

I made this request expecting that the default in the schema would in fact determine what is read when nothing is set, but I confirm this is not what actually happens (so e..g you cannot remove gitlab_ci.get("match_behavior", "first")). In any case I think it's worth keeping the default setting in the schema.

@spackbot

* py-execnet: 1.9.0 (spack#33282) * py-execnet: 1.9.0 * bounds * add version 1.12.3 of parallel-netcdf (spack#33286) * Add checksum for py-psutil 5.9.2 (spack#33139) * gitlab ci: Print better information about broken specs (spack#33124) When a pipeline generation job is automatically failed because it generated jobs for specs known to be broken on develop, print better information about the broken specs that were encountered. Include at a minimum the hash and the url of the job whose failure caused it to be put on the broken specs list in the first place. * meson: remove slash in path (spack#33292) * Add checksum for py-gitpython 3.1.27 (spack#33285) * Add checksum for py-gitpython 3.1.27 * Update package.py * Update var/spack/repos/builtin/packages/py-gitpython/package.py Co-authored-by: Adam J. Stewart <[email protected]> Co-authored-by: Adam J. Stewart <[email protected]> * UPC++/GASNet-EX 2022.9.0 update (spack#33277) * gasnet: Add new release hash * upcxx: Add new release hash * gasnet: misc updates * upcxx: misc updates * Fix [email protected] sha (spack#33307) * [email protected] onwards: set prefix properly (spack#33257) * hip-set-prefix-rocm5.2.0-onwards * Update var/spack/repos/builtin/packages/hip/package.py Update description Co-authored-by: Satish Balay <[email protected]> * petsc,py-petsc4py,slepc,py-slepc4py: add version 3.18.0 (spack#32938) * petsc,py-petsc4py,slepc,py-slepc4py: add version 3.18.0 * workaround for dealii build failure [with petsc version check] * pism: add compatibility fix to for [email protected] * add in hipsolver dependency * py-libensemble: updating package for v0.9.3 (spack#33298) * commit updating py-libensemble package for 0.9.3 * removed commented-out lines * Add checksum for py-oauthlib 3.2.1 (spack#33201) * Add checksum for py-oauthlib 3.2.1 * Update package.py * [@spackbot] updating style on behalf of iarspider * Update package.py * Update package.py Co-authored-by: iarspider <[email protected]> * ninja: New version 1.11.1 (spack#33215) * seacas: update to latest release (spack#33330) Add checksum for latest tag/release * gptl: new version 8.1.1; use the correct mpi fortran compiler (spack#33235) * gptl: new version 8.1.1; use the correct `mpifc` * add `F90` and `$F77` * glib: add 2.74.0 and 2.72.4 (spack#33332) * depfile: update docs (spack#33279) * rocksdb: add 7.7.3 (spack#33341) * Add checksum for py-seaborn 0.12.0 (spack#33145) * Add checksum for py-seaborn 0.12.0 * Update package.py * Update package.py * [@spackbot] updating style on behalf of iarspider * Update package.py Co-authored-by: iarspider <[email protected]> * CI: allow multiple matches to combine tags (spack#32290) Currently "spack ci generate" chooses the first matching entry in gitlab-ci:mappings to fill attributes for a generated build-job, requiring that the entire configuration matrix is listed out explicitly. This unfortunately causes significant problems in environments with large configuration spaces, for example the environment in spack#31598 (spack.yaml) supports 5 operating systems, 3 architectures and 130 packages with explicit size requirements, resulting in 1300 lines of configuration YAML. This patch adds a configuraiton option to the gitlab-ci schema called "match_behavior"; when it is set to "merge", all matching entries are applied in order to the final build-job, allowing a few entries to cover an entire matrix of configurations. The default for "match_behavior" is "first", which behaves as before this commit (only the runner attributes of the first match are used). In addition, match entries may now include a "remove-attributes" configuration, which allows matches to remove tags that have been aggregated by prior matches. This only makes sense to use with "match_behavior:merge". You can combine "runner-attributes" with "remove-attributes" to effectively override prior tags. * meson: update OneAPI compiler support patch (spack#33293) * py-tensorflow: fix zlib (spack#33349) * py-tensorflow: fix zlib * [@spackbot] updating style on behalf of haampie Co-authored-by: haampie <[email protected]> * py-meson-python: add new versions (spack#33294) * tasmanian: disable openmp by default (spack#33345) * octopus: upgrade to 12.1 (spack#33343) * py-sphinx: add v5.3 and v5.2 (spack#33356) * py-setuptools: add v65.5.0 (spack#33353) * libblastrampoline: Add versions 5.1.1, 5.2.0 (spack#33352) * nextflow: add v20.10.0 (spack#33354) * sdl2: add v2.0.22 and v2.24.1 (spack#33351) * mariadb-c-client: add 3.3.2, 3.2.7, 3.1.18, 3.0.10 (spack#33335) * py-tensorflow-hub: zlib, again. (spack#33359) * Add checksum for py-sniffio 1.3.0 (spack#32975) * py-numpy: add v1.23.4 (spack#33260) * py-jupyterlab-pygments: install from wheel to avoid cyclic dependency (spack#33278) * py-jupyterlab-pygments: avoid cyclic dependency * Fix style * Update package.py * Update package.py * [@spackbot] updating style on behalf of iarspider * Update package.py * Flake-8 * fix Co-authored-by: iarspider <[email protected]> * installer.py: show timers for binary install (spack#33305) Print a message of the form ``` Fetch mm:ss. Build: mm:ss. Total: mm:ss ``` when installing from buildcache. Previously this only happened for source builds. * Add checksum for py-astroid 2.12.7, py-astroid 2.12.10, py-setuptools 62.6.0, py-wrapt 1.14.1, py-pylint 2.15.0 (spack#32976) * Add checksum for py-astroid 2.12.7, py-setuptools 62.6.0 * Also add checksum for py-wrapt * Update package.py * Update package.py (spack#57) * Update package.py * Update package.py * Update package.py * gitlab ci: Do not force protected build jobs to run on aws runners (spack#33314) * installer.py: fix/test get_deptypes (spack#33363) Fixing an oversight in spack#32537 `get_deptypes` should depend on new `package/dependencies_cache_only` props. * Add checksum for py-grpcio-tools 1.48.1 (spack#33358) * Add checksum for py-prompt-toolkit 3.0.31 (spack#33362) Co-authored-by: Harmen Stoppels <[email protected]> Co-authored-by: Jim Edwards <[email protected]> Co-authored-by: iarspider <[email protected]> Co-authored-by: Scott Wittenburg <[email protected]> Co-authored-by: Adam J. Stewart <[email protected]> Co-authored-by: Dan Bonachea <[email protected]> Co-authored-by: Auriane R <[email protected]> Co-authored-by: eugeneswalker <[email protected]> Co-authored-by: Satish Balay <[email protected]> Co-authored-by: John-Luke Navarro <[email protected]> Co-authored-by: iarspider <[email protected]> Co-authored-by: Erik Schnetter <[email protected]> Co-authored-by: Greg Sjaardema <[email protected]> Co-authored-by: WuK <[email protected]> Co-authored-by: Michael Kuhn <[email protected]> Co-authored-by: Jonathon Anderson <[email protected]> Co-authored-by: haampie <[email protected]> Co-authored-by: Miroslav Stoyanov <[email protected]> Co-authored-by: Hans Fangohr <[email protected]> Co-authored-by: Mosè Giordano <[email protected]> Co-authored-by: Diego Alvarez <[email protected]> Co-authored-by: Wouter Deconinck <[email protected]>

spackbot-app bot added commands core PR affects Spack core functionality gitlab Issues related to gitlab integration tests General test capability(ies) labels Aug 20, 2022

blue42u force-pushed the pr-ci-multi-match branch from 44fed80 to e5f1452 Compare August 20, 2022 14:49

alalazo requested a review from scottwittenburg August 22, 2022 13:49

tldahlgren mentioned this pull request Aug 22, 2022

spack ci: add support for running stand-alone tests #27877

Merged

9 tasks

blue42u force-pushed the pr-ci-multi-match branch from e5f1452 to 833a2f7 Compare August 23, 2022 12:16

blue42u mentioned this pull request Aug 23, 2022

[WIP] Add an HPCToolkit stack #32327

Closed

3 tasks

scottwittenburg previously approved these changes Aug 31, 2022

View reviewed changes

blue42u dismissed scottwittenburg’s stale review via e44bf99 September 2, 2022 21:57

blue42u force-pushed the pr-ci-multi-match branch from 833a2f7 to e44bf99 Compare September 2, 2022 21:57

blue42u force-pushed the pr-ci-multi-match branch 2 times, most recently from 04ec226 to a472d7c Compare September 3, 2022 01:53

blue42u requested a review from scottwittenburg September 6, 2022 15:32

blue42u force-pushed the pr-ci-multi-match branch 2 times, most recently from 0b47baa to 7a6d460 Compare September 7, 2022 00:53

blue42u force-pushed the pr-ci-multi-match branch from 7a6d460 to 0b09911 Compare September 7, 2022 18:38

scottwittenburg previously approved these changes Sep 8, 2022

View reviewed changes

blue42u dismissed scottwittenburg’s stale review via b00a4cc September 9, 2022 21:58

blue42u force-pushed the pr-ci-multi-match branch 2 times, most recently from b00a4cc to 212d894 Compare September 13, 2022 17:49

tldahlgren assigned scottwittenburg Sep 13, 2022

blue42u force-pushed the pr-ci-multi-match branch from 212d894 to dc3aaad Compare September 14, 2022 14:00

blue42u force-pushed the pr-ci-multi-match branch 5 times, most recently from 5c60f38 to e71cf3b Compare October 4, 2022 14:41

blue42u force-pushed the pr-ci-multi-match branch from e71cf3b to c6e2f70 Compare October 10, 2022 17:40

scheibelp self-assigned this Oct 12, 2022

blue42u force-pushed the pr-ci-multi-match branch 2 times, most recently from 2544bbc to 52a3d0f Compare October 13, 2022 04:15

scheibelp reviewed Oct 13, 2022

View reviewed changes

lib/spack/spack/schema/gitlab_ci.py Outdated Show resolved Hide resolved

Support full combinatorics in gitlab-ci:mappings

0595dc3

blue42u force-pushed the pr-ci-multi-match branch from 52a3d0f to 0595dc3 Compare October 13, 2022 20:37

blue42u requested a review from scheibelp October 13, 2022 22:14

scheibelp approved these changes Oct 14, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

scheibelp enabled auto-merge (squash) October 14, 2022 00:23

This comment was marked as outdated.

Sign in to view

scheibelp added 2 commits October 14, 2022 11:18

Merge branch 'develop' into pr-ci-multi-match

ef5c25d

Merge branch 'develop' into pr-ci-multi-match

d309c84

scheibelp merged commit 10491e9 into spack:develop Oct 15, 2022

blue42u deleted the pr-ci-multi-match branch November 17, 2022 06:15

scottwittenburg mentioned this pull request Dec 9, 2022

[WIP] ML CI stack: matrix of targets #34353

Closed

Conversation

blue42u commented Aug 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blue42u commented Aug 20, 2022

Uh oh!

becker33 commented Aug 26, 2022

Uh oh!

blue42u commented Aug 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scottwittenburg left a comment

Choose a reason for hiding this comment

Uh oh!

bollig commented Sep 1, 2022

Uh oh!

blue42u commented Sep 2, 2022

Uh oh!

blue42u commented Sep 7, 2022

Uh oh!

scottwittenburg left a comment

Choose a reason for hiding this comment

Uh oh!

scheibelp commented Oct 12, 2022

Uh oh!

spackbot-app bot commented Oct 12, 2022

Uh oh!

blue42u commented Oct 13, 2022

Uh oh!

Uh oh!

scottwittenburg commented Oct 13, 2022

Uh oh!

scheibelp commented Oct 13, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

blue42u commented Aug 20, 2022 •

edited

Loading

blue42u commented Aug 26, 2022 •

edited

Loading