Support full combinatorics in gitlab-ci:mappings

[blue42u]

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]

@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]

@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]

@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]

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

[blue42u]

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

[blue42u]

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]

@spackbot run pipeline

[spackbot-app]

I've started that pipeline for you!

[blue42u]

@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.

[scottwittenburg]

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]

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.