From 10491e98a876df986ce9ee78902c440fe4a8337d Mon Sep 17 00:00:00 2001 From: Jonathon Anderson <17242663+blue42u@users.noreply.github.com> Date: Sat, 15 Oct 2022 12:29:53 -0500 Subject: 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. --- lib/spack/spack/ci.py | 24 ++++++++++++++++++------ lib/spack/spack/schema/gitlab_ci.py | 11 +++++++++++ lib/spack/spack/test/cmd/ci.py | 37 ++++++++++++++++++++++++++++++------- 3 files changed, 59 insertions(+), 13 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/ci.py b/lib/spack/spack/ci.py index 33724e7ac8..c54596fea6 100644 --- 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): diff --git a/lib/spack/spack/schema/gitlab_ci.py b/lib/spack/spack/schema/gitlab_ci.py index d9da5c6ce7..7c0604aaee 100644 --- a/lib/spack/spack/schema/gitlab_ci.py +++ b/lib/spack/spack/schema/gitlab_ci.py @@ -52,6 +52,15 @@ runner_selector_schema = { "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 @@ core_shared_properties = union_dicts( ], }, }, + "match_behavior": {"type": "string", "enum": ["first", "merge"], "default": "first"}, "mappings": { "type": "array", "items": { @@ -93,6 +103,7 @@ core_shared_properties = union_dicts( "type": "string", }, }, + "remove-attributes": remove_attributes_schema, "runner-attributes": runner_selector_schema, }, }, diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py index 37189af73e..dff8484199 100644 --- a/lib/spack/spack/test/cmd/ci.py +++ b/lib/spack/spack/test/cmd/ci.py @@ -1358,8 +1358,15 @@ def test_push_mirror_contents_exceptions(monkeypatch, capsys): 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 @@ spack: gitlab-ci: tags: - toplevel + - toplevel2 variables: ONE: toplevelvarone TWO: toplevelvartwo @@ -1388,6 +1396,7 @@ spack: - main step after_script: - post step one + match_behavior: {0} mappings: - match: - flatten-deps @@ -1400,10 +1409,12 @@ spack: - dependency-install - match: - a + remove-attributes: + tags: + - toplevel2 runner-attributes: tags: - specific-a - - toplevel variables: ONE: specificvarone TWO: specificvartwo @@ -1413,10 +1424,17 @@ spack: - 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 @@ spack: 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 @@ spack: 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 @@ spack: 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" -- cgit v1.2.3-70-g09d2