Allow overridable global runner attributes

3 files changed, 205 insertions, 11 deletions

diff --git a/lib/spack/spack/ci.py b/lib/spack/spack/ci.py
index bf2b18ebc8..e0d1c3ee2d 100644
--- a/lib/spack/spack/ci.py
+++ b/lib/spack/spack/ci.py
@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import base64
+import copy
 import datetime
 import json
 import os
@@ -424,12 +425,53 @@ def spec_matches(spec, match_string):
     return spec.satisfies(match_string)
 
 
-def find_matching_config(spec, ci_mappings):
+def copy_attributes(attrs_list, src_dict, dest_dict):
+    for runner_attr in attrs_list:
+        if runner_attr in src_dict:
+            if runner_attr in dest_dict and runner_attr == 'tags':
+                # For 'tags', we combine the lists of tags, while
+                # avoiding duplicates
+                for tag in src_dict[runner_attr]:
+                    if tag not in dest_dict[runner_attr]:
+                        dest_dict[runner_attr].append(tag)
+            elif runner_attr in dest_dict and runner_attr == 'variables':
+                # For 'variables', we merge the dictionaries.  Any conflicts
+                # (i.e. 'runner-attributes' has same variable key as the
+                # higher level) we resolve by keeping the more specific
+                # 'runner-attributes' version.
+                for src_key, src_val in src_dict[runner_attr].items():
+                    dest_dict[runner_attr][src_key] = copy.deepcopy(
+                        src_dict[runner_attr][src_key])
+            else:
+                dest_dict[runner_attr] = copy.deepcopy(src_dict[runner_attr])
+
+
+def find_matching_config(spec, gitlab_ci):
+    runner_attributes = {}
+    overridable_attrs = [
+        'image',
+        'tags',
+        'variables',
+        'before_script',
+        'script',
+        'after_script',
+    ]
+
+    copy_attributes(overridable_attrs, gitlab_ci, runner_attributes)
+
+    ci_mappings = gitlab_ci['mappings']
     for ci_mapping in ci_mappings:
         for match_string in ci_mapping['match']:
             if spec_matches(spec, match_string):
-                return ci_mapping['runner-attributes']
-    return None
+                if 'runner-attributes' in ci_mapping:
+                    copy_attributes(overridable_attrs,
+                                    ci_mapping['runner-attributes'],
+                                    runner_attributes)
+                return runner_attributes
+    else:
+        return None
+
+    return runner_attributes
 
 
 def pkg_name_from_spec_label(spec_label):
@@ -464,7 +506,6 @@ def generate_gitlab_ci_yaml(env, print_summary, output_file,
         tty.die('Environment yaml does not have "gitlab-ci" section')
 
     gitlab_ci = yaml_root['gitlab-ci']
-    ci_mappings = gitlab_ci['mappings']
 
     final_job_config = None
     if 'final-stage-rebuild-index' in gitlab_ci:
@@ -566,7 +607,7 @@ def generate_gitlab_ci_yaml(env, print_summary, output_file,
                 release_spec = root_spec[pkg_name]
 
                 runner_attribs = find_matching_config(
-                    release_spec, ci_mappings)
+                    release_spec, gitlab_ci)
 
                 if not runner_attribs:
                     tty.warn('No match found for {0}, skipping it'.format(
diff --git a/lib/spack/spack/schema/gitlab_ci.py b/lib/spack/spack/schema/gitlab_ci.py
index 122715b6db..79ffee2074 100644
--- a/lib/spack/spack/schema/gitlab_ci.py
+++ b/lib/spack/spack/schema/gitlab_ci.py
@@ -63,7 +63,7 @@ properties = {
                 'items': {
                     'type': 'object',
                     'additionalProperties': False,
-                    'required': ['match', 'runner-attributes'],
+                    'required': ['match'],
                     'properties': {
                         'match': {
                             'type': 'array',
@@ -79,12 +79,10 @@ properties = {
                                 'image': image_schema,
                                 'tags': {
                                     'type': 'array',
-                                    'default': [],
                                     'items': {'type': 'string'}
                                 },
                                 'variables': {
                                     'type': 'object',
-                                    'default': {},
                                     'patternProperties': {
                                         r'[\w\d\-_\.]+': {
                                             'type': 'string',
@@ -93,17 +91,14 @@ properties = {
                                 },
                                 'before_script': {
                                     'type': 'array',
-                                    'default': [],
                                     'items': {'type': 'string'}
                                 },
                                 'script': {
                                     'type': 'array',
-                                    'default': [],
                                     'items': {'type': 'string'}
                                 },
                                 'after_script': {
                                     'type': 'array',
-                                    'default': [],
                                     'items': {'type': 'string'}
                                 },
                             },
@@ -111,6 +106,31 @@ properties = {
                     },
                 },
             },
+            'image': image_schema,
+            'tags': {
+                'type': 'array',
+                'items': {'type': 'string'}
+            },
+            'variables': {
+                'type': 'object',
+                'patternProperties': {
+                    r'[\w\d\-_\.]+': {
+                        'type': 'string',
+                    },
+                },
+            },
+            'before_script': {
+                'type': 'array',
+                'items': {'type': 'string'}
+            },
+            'script': {
+                'type': 'array',
+                'items': {'type': 'string'}
+            },
+            'after_script': {
+                'type': 'array',
+                'items': {'type': 'string'}
+            },
             'enable-artifacts-buildcache': {
                 'type': 'boolean',
                 'default': False,
diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py
index fc9d0fd8b4..641f25b3e9 100644
--- a/lib/spack/spack/test/cmd/ci.py
+++ b/lib/spack/spack/test/cmd/ci.py
@@ -778,3 +778,136 @@ spack:
             dl_dir_list = os.listdir(dl_dir.strpath)
 
             assert(len(dl_dir_list) == 3)
+
+
+def test_ci_generate_override_runner_attrs(tmpdir, mutable_mock_env_path,
+                                           env_deactivate, install_mockery,
+                                           mock_packages):
+    """Test that we get the behavior we want with respect to the provision
+       of runner attributes like tags, variables, and scripts, both when we
+       inherit them from the top level, as well as when we override one or
+       more at the runner level"""
+    filename = str(tmpdir.join('spack.yaml'))
+    with open(filename, 'w') as f:
+        f.write("""\
+spack:
+  specs:
+    - flatten-deps
+    - a
+  mirrors:
+    some-mirror: https://my.fake.mirror
+  gitlab-ci:
+    tags:
+      - toplevel
+    variables:
+      ONE: toplevelvarone
+      TWO: toplevelvartwo
+    before_script:
+      - pre step one
+      - pre step two
+    script:
+      - main step
+    after_script:
+      - post step one
+    mappings:
+      - match:
+          - flatten-deps
+        runner-attributes:
+          tags:
+            - specific-one
+          variables:
+            THREE: specificvarthree
+      - match:
+          - dependency-install
+      - match:
+          - a
+        runner-attributes:
+          tags:
+            - specific-a
+            - toplevel
+          variables:
+            ONE: specificvarone
+            TWO: specificvartwo
+          before_script:
+            - custom pre step one
+          script:
+            - custom main step
+          after_script:
+            - custom post step one
+    final-stage-rebuild-index:
+      image: donotcare
+      tags: [donotcare]
+""")
+
+    with tmpdir.as_cwd():
+        env_cmd('create', 'test', './spack.yaml')
+        outputfile = str(tmpdir.join('.gitlab-ci.yml'))
+
+        with ev.read('test'):
+            ci_cmd('generate', '--output-file', outputfile)
+
+        with open(outputfile) as f:
+            contents = f.read()
+            print('generated contents: ')
+            print(contents)
+            yaml_contents = syaml.load(contents)
+
+            for ci_key in yaml_contents.keys():
+                if '(specs) b' in ci_key:
+                    print('Should not have staged "b" w/out a match')
+                    assert(False)
+                if '(specs) a' in ci_key:
+                    # Make sure a's attributes override variables, and all the
+                    # scripts.  Also, make sure the 'toplevel' tag doesn't
+                    # appear twice, but that a's specific extra tag does appear
+                    the_elt = yaml_contents[ci_key]
+                    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('specific-a' in the_elt['tags'])
+                    assert('toplevel' 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)
+                    assert(the_elt['script'][0] == 'custom main step')
+                    assert(len(the_elt['after_script']) == 1)
+                    assert(the_elt['after_script'][0] ==
+                           'custom post step one')
+                if '(specs) dependency-install' in ci_key:
+                    # Since the dependency-install match omits any
+                    # runner-attributes, make sure it inherited all the
+                    # top-level attributes.
+                    the_elt = yaml_contents[ci_key]
+                    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['before_script']) == 2)
+                    assert(the_elt['before_script'][0] == 'pre step one')
+                    assert(the_elt['before_script'][1] == 'pre step two')
+                    assert(len(the_elt['script']) == 1)
+                    assert(the_elt['script'][0] == 'main step')
+                    assert(len(the_elt['after_script']) == 1)
+                    assert(the_elt['after_script'][0] == 'post step one')
+                if '(specs) flatten-deps' in ci_key:
+                    # The flatten-deps match specifies that we keep the two
+                    # top level variables, but add a third specifc one.  It
+                    # also adds a custom tag which should be combined with
+                    # the top-level tag.
+                    the_elt = yaml_contents[ci_key]
+                    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('specific-one' in the_elt['tags'])
+                    assert('toplevel' 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')
+                    assert(len(the_elt['script']) == 1)
+                    assert(the_elt['script'][0] == 'main step')
+                    assert(len(the_elt['after_script']) == 1)
+                    assert(the_elt['after_script'][0] == 'post step one')