Ci backwards compat (#36045)

* CI: Fixup docs for bootstrap.

* CI: Add compatibility shim

* Add an update method for CI

Update requires manually renaming section to `ci`. After
this patch, updating and using the deprecated `gitlab-ci` section
should be possible.

* Fix typos in generate warnings

* Fixup CI schema validation

* Add unit tests for legacy CI

* Add deprecated CI stack for continuous testing

* Allow updating gitlab-ci section directly with env update

* Make warning give good advice for updating gitlab-ci

* Fix typo in CI name

* Remove white space

* Remove unneeded component of deprected-ci

5 files changed, 266 insertions, 25 deletions

diff --git a/lib/spack/docs/pipelines.rst b/lib/spack/docs/pipelines.rst
index 5d06accc4a..d594879aab 100644
--- a/lib/spack/docs/pipelines.rst
+++ b/lib/spack/docs/pipelines.rst
@@ -632,18 +632,18 @@ Here's an example of what bootstrapping some compilers might look like:
        exclude:
          - '%gcc@7.3.0 os=centos7'
          - '%gcc@5.5.0 os=ubuntu18.04'
-     gitlab-ci:
+     ci:
        bootstrap:
          - name: compiler-pkgs
            compiler-agnostic: true
-       mappings:
-         # mappings similar to the example higher up in this description
+       pipeline-gen:
+         # similar to the example higher up in this description
          ...
 
 The example above adds a list to the ``definitions`` called ``compiler-pkgs``
 (you can add any number of these), which lists compiler packages that should
 be staged ahead of the full matrix of release specs (in this example, only
-readline).  Then within the ``gitlab-ci`` section, note the addition of a
+readline).  Then within the ``ci`` section, note the addition of a
 ``bootstrap`` section, which can contain a list of items, each referring to
 a list in the ``definitions`` section.  These items can either
 be a dictionary or a string.  If you supply a dictionary, it must have a name
@@ -709,7 +709,7 @@ be reported.
 
 Take a look at the
 `schema <https://github.com/spack/spack/blob/develop/lib/spack/spack/schema/ci.py>`_
-for the gitlab-ci section of the spack environment file, to see precisely what
+for the ci section of the spack environment file, to see precisely what
 syntax is allowed there.
 
 .. _reserved_tags:
diff --git a/lib/spack/spack/ci.py b/lib/spack/spack/ci.py
index c8aa29d0bd..f7f13cb64e 100644
--- a/lib/spack/spack/ci.py
+++ b/lib/spack/spack/ci.py
@@ -756,10 +756,20 @@ def generate_gitlab_ci_yaml(
     ci_config = cfg.get("ci")
 
     if not ci_config:
-        tty.die('Environment yaml does not have "ci" section')
+        tty.warn("Environment does not have `ci` a configuration")
+        gitlabci_config = yaml_root.get("gitlab-ci")
+        if not gitlabci_config:
+            tty.die("Environment yaml does not have `gitlab-ci` config section. Cannot recover.")
+
+        tty.warn(
+            "The `gitlab-ci` configuration is deprecated in favor of `ci`.\n",
+            "To update run \n\t$ spack env update /path/to/ci/spack.yaml",
+        )
+        translate_deprecated_config(gitlabci_config)
+        ci_config = gitlabci_config
 
     # Default target is gitlab...and only target is gitlab
-    if "target" in ci_config and ci_config["target"] != "gitlab":
+    if not ci_config.get("target", "gitlab") == "gitlab":
         tty.die('Spack CI module only generates target "gitlab"')
 
     cdash_config = cfg.get("cdash")
@@ -938,6 +948,10 @@ def generate_gitlab_ci_yaml(
         env_includes.extend(include_scopes)
         env_yaml_root["spack"]["include"] = env_includes
 
+        if "gitlab-ci" in env_yaml_root["spack"] and "ci" not in env_yaml_root["spack"]:
+            env_yaml_root["spack"]["ci"] = env_yaml_root["spack"].pop("gitlab-ci")
+            translate_deprecated_config(env_yaml_root["spack"]["ci"])
+
         with open(os.path.join(concrete_env_dir, "spack.yaml"), "w") as fd:
             fd.write(syaml.dump_config(env_yaml_root, default_flow_style=False))
 
@@ -2474,3 +2488,66 @@ class CDashHandler(object):
         )
         reporter = CDash(configuration=configuration)
         reporter.test_skipped_report(directory_name, spec, reason)
+
+
+def translate_deprecated_config(config):
+    # Remove all deprecated keys from config
+    mappings = config.pop("mappings", [])
+    match_behavior = config.pop("match_behavior", "first")
+
+    build_job = {}
+    if "image" in config:
+        build_job["image"] = config.pop("image")
+    if "tags" in config:
+        build_job["tags"] = config.pop("tags")
+    if "variables" in config:
+        build_job["variables"] = config.pop("variables")
+    if "before_script" in config:
+        build_job["before_script"] = config.pop("before_script")
+    if "script" in config:
+        build_job["script"] = config.pop("script")
+    if "after_script" in config:
+        build_job["after_script"] = config.pop("after_script")
+
+    signing_job = None
+    if "signing-job-attributes" in config:
+        signing_job = {"signing-job": config.pop("signing-job-attributes")}
+
+    service_job_attributes = None
+    if "service-job-attributes" in config:
+        service_job_attributes = config.pop("service-job-attributes")
+
+    # If this config already has pipeline-gen do not more
+    if "pipeline-gen" in config:
+        return True if mappings or build_job or signing_job or service_job_attributes else False
+
+    config["target"] = "gitlab"
+
+    config["pipeline-gen"] = []
+    pipeline_gen = config["pipeline-gen"]
+
+    # Build Job
+    submapping = []
+    for section in mappings:
+        submapping_section = {"match": section["match"]}
+        if "runner-attributes" in section:
+            submapping_section["build-job"] = section["runner-attributes"]
+        if "remove-attributes" in section:
+            submapping_section["build-job-remove"] = section["remove-attributes"]
+        submapping.append(submapping_section)
+    pipeline_gen.append({"submapping": submapping, "match_behavior": match_behavior})
+
+    if build_job:
+        pipeline_gen.append({"build-job": build_job})
+
+    # Signing Job
+    if signing_job:
+        pipeline_gen.append(signing_job)
+
+    # Service Jobs
+    if service_job_attributes:
+        pipeline_gen.append({"reindex-job": service_job_attributes})
+        pipeline_gen.append({"noop-job": service_job_attributes})
+        pipeline_gen.append({"cleanup-job": service_job_attributes})
+
+    return True
diff --git a/lib/spack/spack/schema/ci.py b/lib/spack/spack/schema/ci.py
index 3fcb7fc164..59d1ac615c 100644
--- a/lib/spack/spack/schema/ci.py
+++ b/lib/spack/spack/schema/ci.py
@@ -11,6 +11,8 @@
 
 from llnl.util.lang import union_dicts
 
+import spack.schema.gitlab_ci
+
 # Schema for script fields
 # List of lists and/or strings
 # This is similar to what is allowed in
@@ -20,24 +22,27 @@ script_schema = {
     "items": {"anyOf": [{"type": "string"}, {"type": "array", "items": {"type": "string"}}]},
 }
 
+# Schema for CI image
+image_schema = {
+    "oneOf": [
+        {"type": "string"},
+        {
+            "type": "object",
+            "properties": {
+                "name": {"type": "string"},
+                "entrypoint": {"type": "array", "items": {"type": "string"}},
+            },
+        },
+    ]
+}
+
 # Additional attributes are allow
 # and will be forwarded directly to the
 # CI target YAML for each job.
 attributes_schema = {
     "type": "object",
     "properties": {
-        "image": {
-            "oneOf": [
-                {"type": "string"},
-                {
-                    "type": "object",
-                    "properties": {
-                        "name": {"type": "string"},
-                        "entrypoint": {"type": "array", "items": {"type": "string"}},
-                    },
-                },
-            ]
-        },
+        "image": image_schema,
         "tags": {"type": "array", "items": {"type": "string"}},
         "variables": {
             "type": "object",
@@ -169,7 +174,15 @@ ci_properties = {
 }
 
 #: Properties for inclusion in other schemas
-properties = {"ci": ci_properties}
+properties = {
+    "ci": {
+        "oneOf": [
+            ci_properties,
+            # Allow legacy format under `ci` for `config update ci`
+            spack.schema.gitlab_ci.gitlab_ci_properties,
+        ]
+    }
+}
 
 #: Full schema with metadata
 schema = {
@@ -179,3 +192,21 @@ schema = {
     "additionalProperties": False,
     "properties": properties,
 }
+
+
+def update(data):
+    import llnl.util.tty as tty
+
+    import spack.ci
+    import spack.environment as ev
+
+    # Warn if deprecated section is still in the environment
+    ci_env = ev.active_environment()
+    if ci_env:
+        env_config = ev.config_dict(ci_env.yaml)
+        if "gitlab-ci" in env_config:
+            tty.die("Error: `gitlab-ci` section detected with `ci`, these are not compatible")
+
+    # Detect if the ci section is using the new pipeline-gen
+    # If it is, assume it has already been converted
+    return spack.ci.translate_deprecated_config(data)
diff --git a/lib/spack/spack/schema/env.py b/lib/spack/spack/schema/env.py
index 88c1ef1c85..93f0f9129f 100644
--- a/lib/spack/spack/schema/env.py
+++ b/lib/spack/spack/schema/env.py
@@ -10,6 +10,7 @@
 """
 from llnl.util.lang import union_dicts
 
+import spack.schema.gitlab_ci  # DEPRECATED
 import spack.schema.merged
 import spack.schema.packages
 import spack.schema.projections
@@ -52,6 +53,8 @@ schema = {
             "default": {},
             "additionalProperties": False,
             "properties": union_dicts(
+                # Include deprecated "gitlab-ci" section
+                spack.schema.gitlab_ci.properties,
                 # merged configuration scope schemas
                 spack.schema.merged.properties,
                 # extra environment schema properties
@@ -130,6 +133,15 @@ def update(data):
     Returns:
         True if data was changed, False otherwise
     """
+
+    import spack.ci
+
+    if "gitlab-ci" in data:
+        data["ci"] = data.pop("gitlab-ci")
+
+    if "ci" in data:
+        return spack.ci.translate_deprecated_config(data["ci"])
+
     # There are not currently any deprecated attributes in this section
     # that have not been removed
     return False
diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py
index c2c14410ca..44f195bc3b 100644
--- a/lib/spack/spack/test/cmd/ci.py
+++ b/lib/spack/spack/test/cmd/ci.py
@@ -33,6 +33,7 @@ from spack.schema.database_index import schema as db_idx_schema
 from spack.spec import CompilerSpec, Spec
 from spack.util.pattern import Bunch
 
+config_cmd = spack.main.SpackCommand("config")
 ci_cmd = spack.main.SpackCommand("ci")
 env_cmd = spack.main.SpackCommand("env")
 mirror_cmd = spack.main.SpackCommand("mirror")
@@ -412,7 +413,7 @@ spack:
 """
         )
 
-    expect_out = 'Error: Environment yaml does not have "ci" section'
+    expect_out = "Environment does not have `ci` a configuration"
 
     with tmpdir.as_cwd():
         env_cmd("create", "test", "./spack.yaml")
@@ -1842,12 +1843,11 @@ def test_ci_generate_prune_env_vars(
 spack:
   specs:
     - libelf
-  ci:
-    pipeline-gen:
-    - submapping:
+  gitlab-ci:
+    mappings:
       - match:
           - arch=test-debian6-core2
-        build-job:
+        runner-attributes:
           tags:
             - donotcare
           image: donotcare
@@ -2290,3 +2290,124 @@ def test_cmd_first_line():
     )
 
     assert spack.cmd.first_line(doc) == first
+
+
+legacy_spack_yaml_contents = """
+spack:
+  definitions:
+    - bootstrap:
+      - cmake@3.4.3
+    - old-gcc-pkgs:
+      - archive-files
+      - callpath
+      # specify ^openblas-with-lapack to ensure that builtin.mock repo flake8
+      # package (which can also provide lapack) is not chosen, as it violates
+      # a package-level check which requires exactly one fetch strategy (this
+      # is apparently not an issue for other tests that use it).
+      - hypre@0.2.15 ^openblas-with-lapack
+  specs:
+    - matrix:
+      - [$old-gcc-pkgs]
+  mirrors:
+    test-mirror: file:///some/fake/mirror
+  {0}:
+    bootstrap:
+      - name: bootstrap
+        compiler-agnostic: true
+    match_behavior: first
+    mappings:
+      - match:
+          - arch=test-debian6-core2
+        runner-attributes:
+          tags:
+            - donotcare
+          image: donotcare
+      - match:
+          - arch=test-debian6-m1
+        runner-attributes:
+          tags:
+            - donotcare
+          image: donotcare
+    service-job-attributes:
+      image: donotcare
+      tags: [donotcare]
+  cdash:
+    build-group: Not important
+    url: https://my.fake.cdash
+    project: Not used
+    site: Nothing
+"""
+
+
+@pytest.mark.regression("36409")
+def test_gitlab_ci_deprecated(
+    tmpdir,
+    mutable_mock_env_path,
+    install_mockery,
+    mock_packages,
+    monkeypatch,
+    ci_base_environment,
+    mock_binary_index,
+):
+    mirror_url = "file:///some/fake/mirror"
+    filename = str(tmpdir.join("spack.yaml"))
+    with open(filename, "w") as f:
+        f.write(legacy_spack_yaml_contents.format("gitlab-ci"))
+
+    with tmpdir.as_cwd():
+        env_cmd("create", "test", "./spack.yaml")
+        outputfile = "generated-pipeline.yaml"
+
+        with ev.read("test"):
+            ci_cmd("generate", "--output-file", outputfile)
+
+        with open(outputfile) as f:
+            contents = f.read()
+            yaml_contents = syaml.load(contents)
+
+            found_spec = False
+            for ci_key in yaml_contents.keys():
+                if "(bootstrap)" in ci_key:
+                    found_spec = True
+                    assert "cmake" in ci_key
+            assert found_spec
+            assert "stages" in yaml_contents
+            assert len(yaml_contents["stages"]) == 6
+            assert yaml_contents["stages"][0] == "stage-0"
+            assert yaml_contents["stages"][5] == "stage-rebuild-index"
+
+            assert "rebuild-index" in yaml_contents
+            rebuild_job = yaml_contents["rebuild-index"]
+            expected = "spack buildcache update-index --keys --mirror-url {0}".format(mirror_url)
+            assert rebuild_job["script"][0] == expected
+
+            assert "variables" in yaml_contents
+            assert "SPACK_ARTIFACTS_ROOT" in yaml_contents["variables"]
+            artifacts_root = yaml_contents["variables"]["SPACK_ARTIFACTS_ROOT"]
+            assert artifacts_root == "jobs_scratch_dir"
+
+
+@pytest.mark.regression("36045")
+def test_gitlab_ci_update(
+    tmpdir,
+    mutable_mock_env_path,
+    install_mockery,
+    mock_packages,
+    monkeypatch,
+    ci_base_environment,
+    mock_binary_index,
+):
+    filename = str(tmpdir.join("spack.yaml"))
+    with open(filename, "w") as f:
+        f.write(legacy_spack_yaml_contents.format("ci"))
+
+    with tmpdir.as_cwd():
+        env_cmd("update", "-y", ".")
+
+        with open("spack.yaml") as f:
+            contents = f.read()
+            yaml_contents = syaml.load(contents)
+
+            ci_root = yaml_contents["spack"]["ci"]
+
+            assert "pipeline-gen" in ci_root