CI: allow multiple matches to combine tags (#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 #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.
2022-10-15 12:29:53 -05:00 · 2022-10-15 12:29:53 -05:00 · 10491e98a8
commit 10491e98a8
parent 898c0b45fb
20 changed files with 76 additions and 13 deletions
--- a/lib/spack/spack/ci.py
+++ b/lib/spack/spack/ci.py
@ -397,6 +397,14 @@ def _spec_matches(spec, match_string):
    return spec.satisfies(match_string)


+def _remove_attributes(src_dict, dest_dict):
+    if "tags" in src_dict and "tags" in dest_dict:
+        # For 'tags', we remove any tags that are listed for removal
+        for tag in src_dict["tags"]:
+            while tag in dest_dict["tags"]:
+                dest_dict["tags"].remove(tag)
+
+
 def _copy_attributes(attrs_list, src_dict, dest_dict):
    for runner_attr in attrs_list:
        if runner_attr in src_dict:
@ -430,19 +438,23 @@ def _find_matching_config(spec, gitlab_ci):

    _copy_attributes(overridable_attrs, gitlab_ci, runner_attributes)

-    ci_mappings = gitlab_ci["mappings"]
-    for ci_mapping in ci_mappings:
+    matched = False
+    only_first = gitlab_ci.get("match_behavior", "first") == "first"
+    for ci_mapping in gitlab_ci["mappings"]:
        for match_string in ci_mapping["match"]:
            if _spec_matches(spec, match_string):
+                matched = True
+                if "remove-attributes" in ci_mapping:
+                    _remove_attributes(ci_mapping["remove-attributes"], runner_attributes)
                if "runner-attributes" in ci_mapping:
                    _copy_attributes(
                        overridable_attrs, ci_mapping["runner-attributes"], runner_attributes
                    )
-                return runner_attributes
-    else:
-        return None
+                break
+        if matched and only_first:
+            break

-    return runner_attributes
+    return runner_attributes if matched else None


 def _pkg_name_from_spec_label(spec_label):
--- a/lib/spack/spack/schema/gitlab_ci.py
+++ b/lib/spack/spack/schema/gitlab_ci.py
@ -52,6 +52,15 @@
    "properties": runner_attributes_schema_items,
 }

+remove_attributes_schema = {
+    "type": "object",
+    "additionalProperties": False,
+    "required": ["tags"],
+    "properties": {
+        "tags": {"type": "array", "items": {"type": "string"}},
+    },
+}
+

 core_shared_properties = union_dicts(
    runner_attributes_schema_items,
@ -80,6 +89,7 @@
                ],
            },
        },
+        "match_behavior": {"type": "string", "enum": ["first", "merge"], "default": "first"},
        "mappings": {
            "type": "array",
            "items": {
@ -93,6 +103,7 @@
                            "type": "string",
                        },
                    },
+                    "remove-attributes": remove_attributes_schema,
                    "runner-attributes": runner_selector_schema,
                },
            },
--- a/lib/spack/spack/test/cmd/ci.py
+++ b/lib/spack/spack/test/cmd/ci.py
@ -1358,8 +1358,15 @@ def failing_access(*args, **kwargs):
    assert expect_msg in std_out


+@pytest.mark.parametrize("match_behavior", ["first", "merge"])
 def test_ci_generate_override_runner_attrs(
-    tmpdir, mutable_mock_env_path, install_mockery, mock_packages, monkeypatch, ci_base_environment
+    tmpdir,
+    mutable_mock_env_path,
+    install_mockery,
+    mock_packages,
+    monkeypatch,
+    ci_base_environment,
+    match_behavior,
 ):
    """Test that we get the behavior we want with respect to the provision
    of runner attributes like tags, variables, and scripts, both when we
@ -1378,6 +1385,7 @@ def test_ci_generate_override_runner_attrs(
  gitlab-ci:
    tags:
      - toplevel
+      - toplevel2
    variables:
      ONE: toplevelvarone
      TWO: toplevelvartwo
@ -1388,6 +1396,7 @@ def test_ci_generate_override_runner_attrs(
      - main step
    after_script:
      - post step one
+    match_behavior: {0}
    mappings:
      - match:
          - flatten-deps
@ -1400,10 +1409,12 @@ def test_ci_generate_override_runner_attrs(
          - dependency-install
      - match:
          - a
+        remove-attributes:
+          tags:
+            - toplevel2
        runner-attributes:
          tags:
            - specific-a
-            - toplevel
          variables:
            ONE: specificvarone
            TWO: specificvartwo
@ -1413,10 +1424,17 @@ def test_ci_generate_override_runner_attrs(
            - custom main step
          after_script:
            - custom post step one
+      - match:
+          - a
+        runner-attributes:
+          tags:
+            - specific-a-2
    service-job-attributes:
      image: donotcare
      tags: [donotcare]
-"""
+""".format(
+                match_behavior
+            )
        )

    with tmpdir.as_cwd():
@ -1449,9 +1467,12 @@ def test_ci_generate_override_runner_attrs(
                    assert the_elt["variables"]["ONE"] == "specificvarone"
                    assert the_elt["variables"]["TWO"] == "specificvartwo"
                    assert "THREE" not in the_elt["variables"]
-                    assert len(the_elt["tags"]) == 2
+                    assert len(the_elt["tags"]) == (2 if match_behavior == "first" else 3)
                    assert "specific-a" in the_elt["tags"]
+                    if match_behavior == "merge":
+                        assert "specific-a-2" in the_elt["tags"]
                    assert "toplevel" in the_elt["tags"]
+                    assert "toplevel2" not in the_elt["tags"]
                    assert len(the_elt["before_script"]) == 1
                    assert the_elt["before_script"][0] == "custom pre step one"
                    assert len(the_elt["script"]) == 1
@ -1466,8 +1487,9 @@ def test_ci_generate_override_runner_attrs(
                    assert the_elt["variables"]["ONE"] == "toplevelvarone"
                    assert the_elt["variables"]["TWO"] == "toplevelvartwo"
                    assert "THREE" not in the_elt["variables"]
-                    assert len(the_elt["tags"]) == 1
-                    assert the_elt["tags"][0] == "toplevel"
+                    assert len(the_elt["tags"]) == 2
+                    assert "toplevel" in the_elt["tags"]
+                    assert "toplevel2" in the_elt["tags"]
                    assert len(the_elt["before_script"]) == 2
                    assert the_elt["before_script"][0] == "pre step one"
                    assert the_elt["before_script"][1] == "pre step two"
@ -1484,9 +1506,10 @@ def test_ci_generate_override_runner_attrs(
                    assert the_elt["variables"]["ONE"] == "toplevelvarone"
                    assert the_elt["variables"]["TWO"] == "toplevelvartwo"
                    assert the_elt["variables"]["THREE"] == "specificvarthree"
-                    assert len(the_elt["tags"]) == 2
+                    assert len(the_elt["tags"]) == 3
                    assert "specific-one" in the_elt["tags"]
                    assert "toplevel" in the_elt["tags"]
+                    assert "toplevel2" in the_elt["tags"]
                    assert len(the_elt["before_script"]) == 2
                    assert the_elt["before_script"][0] == "pre step one"
                    assert the_elt["before_script"][1] == "pre step two"
--- a/share/spack/gitlab/cloud_pipelines/stacks/aws-ahug-aarch64/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/aws-ahug-aarch64/spack.yaml
@ -252,6 +252,7 @@ spack:
      - spack -d ci rebuild > >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_out.txt) 2> >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_err.txt >&2)

    image: { "name": "ghcr.io/spack/e4s-amazonlinux-2:v2022-03-21", "entrypoint": [""] }
+    match_behavior: first
    mappings:
      - match:
        - llvm
--- a/share/spack/gitlab/cloud_pipelines/stacks/aws-ahug/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/aws-ahug/spack.yaml
@ -253,6 +253,7 @@ spack:
      - spack -d ci rebuild > >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_out.txt) 2> >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_err.txt >&2)

    image: { "name": "ghcr.io/spack/e4s-amazonlinux-2:v2022-03-21", "entrypoint": [""] }
+    match_behavior: first
    mappings:
      - match:
        - llvm
--- a/share/spack/gitlab/cloud_pipelines/stacks/aws-isc-aarch64/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/aws-isc-aarch64/spack.yaml
@ -159,6 +159,7 @@ spack:
      - spack -d ci rebuild > >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_out.txt) 2> >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_err.txt >&2)

    image: { "name": "ghcr.io/spack/e4s-amazonlinux-2:v2022-03-21", "entrypoint": [""] }
+    match_behavior: first
    mappings:
      - match:
        - llvm
--- a/share/spack/gitlab/cloud_pipelines/stacks/aws-isc/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/aws-isc/spack.yaml
@ -171,6 +171,7 @@ spack:
      - spack -d ci rebuild > >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_out.txt) 2> >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_err.txt >&2)

    image: { "name": "ghcr.io/spack/e4s-amazonlinux-2:v2022-03-21", "entrypoint": [""] }
+    match_behavior: first
    mappings:
      - match:
        - llvm
--- a/share/spack/gitlab/cloud_pipelines/stacks/build_systems/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/build_systems/spack.yaml
@ -45,6 +45,7 @@ spack:
      name: "ghcr.io/spack/e4s-ubuntu-18.04:v2021-10-18"
      entrypoint: [ "" ]

+    match_behavior: first
    mappings:
      - match:
          - cmake
--- a/share/spack/gitlab/cloud_pipelines/stacks/data-vis-sdk/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/data-vis-sdk/spack.yaml
@ -55,6 +55,7 @@ spack:
      - if [[ -r /mnt/key/intermediate_ci_signing_key.gpg ]]; then spack gpg trust /mnt/key/intermediate_ci_signing_key.gpg; fi
      - if [[ -r /mnt/key/spack_public_key.gpg ]]; then spack gpg trust /mnt/key/spack_public_key.gpg; fi
      - spack -d ci rebuild
+    match_behavior: first
    mappings:
      - match:
          - llvm
--- a/share/spack/gitlab/cloud_pipelines/stacks/e4s-mac/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/e4s-mac/spack.yaml
@ -51,6 +51,7 @@ spack:
      - mkdir -p ${SPACK_ARTIFACTS_ROOT}/user_data
      - spack -d ci rebuild > >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_out.txt) 2> >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_err.txt >&2)

+    match_behavior: first
    mappings:
      - match: ['os=monterey']
        runner-attributes:
--- a/share/spack/gitlab/cloud_pipelines/stacks/e4s-on-power/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/e4s-on-power/spack.yaml
@ -221,6 +221,7 @@ spack:
      - spack config add "config:install_tree:projections:${SPACK_JOB_SPEC_PKG_NAME}:'morepadding/{architecture}/{compiler.name}-{compiler.version}/{name}-{version}-{hash}'"
      - spack -d ci rebuild

+    match_behavior: first
    mappings:
      - match:
        - cuda
--- a/share/spack/gitlab/cloud_pipelines/stacks/e4s-oneapi/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/e4s-oneapi/spack.yaml
@ -282,6 +282,7 @@ spack:

    image: ecpe4s/ubuntu20.04-runner-x86_64-oneapi:2022-07-01
    
+    match_behavior: first
    mappings:
      - match:
          - hipblas
--- a/share/spack/gitlab/cloud_pipelines/stacks/e4s/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/e4s/spack.yaml
@ -261,6 +261,7 @@ spack:
    broken-tests-packages:
      - gptune

+    match_behavior: first
    mappings:
      - match:
          - hipblas
--- a/share/spack/gitlab/cloud_pipelines/stacks/ml-cpu/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/ml-cpu/spack.yaml
@ -97,6 +97,7 @@ spack:
      - if [[ -r /mnt/key/spack_public_key.gpg ]]; then spack gpg trust /mnt/key/spack_public_key.gpg; fi
      - spack -d ci rebuild > >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_out.txt) 2> >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_err.txt >&2)

+    match_behavior: first
    mappings:
      - match:
          - llvm
--- a/share/spack/gitlab/cloud_pipelines/stacks/ml-cuda/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/ml-cuda/spack.yaml
@ -100,6 +100,7 @@ spack:
      - if [[ -r /mnt/key/spack_public_key.gpg ]]; then spack gpg trust /mnt/key/spack_public_key.gpg; fi
      - spack -d ci rebuild > >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_out.txt) 2> >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_err.txt >&2)

+    match_behavior: first
    mappings:
      - match:
          - llvm
--- a/share/spack/gitlab/cloud_pipelines/stacks/ml-rocm/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/ml-rocm/spack.yaml
@ -103,6 +103,7 @@ spack:
      - if [[ -r /mnt/key/spack_public_key.gpg ]]; then spack gpg trust /mnt/key/spack_public_key.gpg; fi
      - spack -d ci rebuild > >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_out.txt) 2> >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_err.txt >&2)

+    match_behavior: first
    mappings:
      - match:
          - llvm
--- a/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws-aarch64/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws-aarch64/spack.yaml
@ -67,6 +67,7 @@ spack:
      - spack -d ci rebuild > >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_out.txt) 2> >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_err.txt >&2)

    image: { "name": "ghcr.io/spack/e4s-amazonlinux-2:v2022-03-21", "entrypoint": [""] }
+    match_behavior: first
    mappings:
      - match:
        - llvm
--- a/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/radiuss-aws/spack.yaml
@ -72,6 +72,7 @@ spack:
      - spack -d ci rebuild > >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_out.txt) 2> >(tee ${SPACK_ARTIFACTS_ROOT}/user_data/pipeline_err.txt >&2)

    image: { "name": "ghcr.io/spack/e4s-amazonlinux-2:v2022-03-21", "entrypoint": [""] }
+    match_behavior: first
    mappings:
      - match:
        - llvm
--- a/share/spack/gitlab/cloud_pipelines/stacks/radiuss/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/radiuss/spack.yaml
@ -72,6 +72,7 @@ spack:
      - if [[ -r /mnt/key/intermediate_ci_signing_key.gpg ]]; then spack gpg trust /mnt/key/intermediate_ci_signing_key.gpg; fi
      - if [[ -r /mnt/key/spack_public_key.gpg ]]; then spack gpg trust /mnt/key/spack_public_key.gpg; fi
      - spack -d ci rebuild
+    match_behavior: first
    mappings:
      - match:
          - lbann
--- a/share/spack/gitlab/cloud_pipelines/stacks/tutorial/spack.yaml
+++ b/share/spack/gitlab/cloud_pipelines/stacks/tutorial/spack.yaml
@ -74,6 +74,7 @@ spack:
      - spack -d ci rebuild

    image: { "name": "ghcr.io/spack/tutorial-ubuntu-18.04:v2021-11-02", "entrypoint": [""] }
+    match_behavior: first
    mappings:
      - match:
          - cmake