CI: Add an environment option for build cache index update behavior (spack#51817)

* CI: Add an environment option for build cache index update behavior

Signed-off-by: Ryan Krattiger <ryan.krattiger@kitware.com>

* Remove unused default cleanup-job

The default cleanup job was using gitlab specific variables and
variables that were unexpanded by gitlab CI. Drop this as it is related
to a long since removed legacy feature "artfacts-buildcache" which has
been deprecated and removed for a few years.

Signed-off-by: Ryan Krattiger <ryan.krattiger@kitware.com>

* Add mirror metadata formatting. Display view in pruning

Views are optional metadata. Provide a way to conditionally format
mirror metadata to print view information if it exists.

Signed-off-by: Ryan Krattiger <ryan.krattiger@kitware.com>

* Add docs on new environment variable

Signed-off-by: Ryan Krattiger <ryan.krattiger@kitware.com>

---------

Signed-off-by: Ryan Krattiger <ryan.krattiger@kitware.com>

Author: kwryankrattiger

lib/spack/docs/pipelines.rst

@@ -704,3 +704,15 @@ Optional.
 Only needed if you want ``spack ci rebuild`` to trust the key you store in this variable, in which case, it will subsequently be used to sign and verify binary packages (when installing or creating build caches).
 You could also have already trusted a key Spack knows about, or if no key is present anywhere, Spack will install specs using ``--no-check-signature`` and create build caches using ``-u`` (for unsigned binaries).
 
+``SPACK_CI_BUILDCACHE_VIEW``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Optional.
+Only needed when using a ``buildcache-destination`` mirror that points at a build cache view.
+This option affects the behavior the ``reindex`` job (:ref:`rebuild_index`) can have the values ``force`` or ``append`` which mirror behavior described by ref:`cmd-spack-buildcache-update-view`.
+The default option is ``append`` because that is what is used by the Spack build farm.
+
+.. warning::
+
+   Using the ``append`` option with build cache index views is a non-atomic operation.
+   It is up to the CI maintainer to ensure that concurrent writes to the build cache are handled appropriately.

lib/spack/spack/ci/__init__.py

@@ -255,7 +255,7 @@ def rebuild_filter(s: spack.spec.Spec) -> RebuildDecision:
         if not spec_locations:
             return RebuildDecision(True, "not found anywhere")
 
-        urls = ",".join(f"{loc.url}@v{loc.version}" for loc in spec_locations)
+        urls = ",".join(f"{loc:_url@v_version? (view: _view)}" for loc in spec_locations)
         message = f"up-to-date [{urls}]"
         return RebuildDecision(False, message)
 

lib/spack/spack/ci/common.py

@@ -570,36 +570,52 @@ def init_pipeline_jobs(self, pipeline: PipelineDag):
 
     # Generate IR from the configs
     def generate_ir(self):
-        """Generate the IR from the Spack CI configurations."""
+        """Generate the IR from the Spack CI configurations.
+
+        Generate makes use of special strings that need to be expanded by python format.
+
+            env_dir: The concrete environment directory used in downstream jobs
+        """
 
         jobs = self.ir["jobs"]
 
         # Implicit job defaults
         defaults = [
             {
                 "build-job": {
-                    "script": [
-                        "cd {env_dir}",
-                        "spack env activate --without-view .",
-                        "spack ci rebuild",
-                    ]
+                    "script": ["spack env activate --without-view {env_dir}", "spack ci rebuild"]
                 }
             },
             {"noop-job": {"script": ['echo "All specs already up to date, nothing to rebuild."']}},
         ]
 
+        pipeline_mirrors = spack.mirrors.mirror.MirrorCollection(binary=True)
+        buildcache_destination = pipeline_mirrors["buildcache-destination"]
+        update_index_extra_args = []
+        if buildcache_destination.push_view:
+            update_index_extra_args.extend(["--name", buildcache_destination.push_view])
+            option = os.environ.get("SPACK_CI_BUILDCACHE_VIEW", "append")
+            if option == "append":
+                # Running this in CI relies on a guarentee from the calling context that there is
+                # only a single writter or the build cache view doesn't require a complete view
+                # after each append.
+                tty.warn("Using --append to update buildcache-destination mirror index view")
+                update_index_extra_args.extend(["-y", "--append"])
+            elif option == "force":
+                update_index_extra_args.append("--force")
+            else:
+                raise SpackCIError(f"Unrecognized value: SPACK_CI_BUILDCACHE_VIEW={option}")
+
         # Job overrides
         overrides = [
             # Reindex script
             {
                 "reindex-job": {
-                    "script:": ["spack buildcache update-index --keys {index_target_mirror}"]
-                }
-            },
-            # Cleanup script
-            {
-                "cleanup-job": {
-                    "script:": ["spack -d mirror destroy {mirror_prefix}/$CI_PIPELINE_ID"]
+                    "script:": [
+                        "spack env activate --without-view {env_dir}",
+                        "spack buildcache update-index --keys "
+                        + f"{' '.join(update_index_extra_args)} buildcache-destination",
+                    ]
                 }
             },
             # Add signing job tags

lib/spack/spack/ci/gitlab.py

@@ -378,21 +378,35 @@ def main_script_replacements(cmd):
             output_object["sign-pkgs"] = signing_job
 
         if options.rebuild_index:
+            # Create a dummy job that runs as the stage before reindex.
+            # This job will be used to ensure reindex doesn't run until
+            # the other build jobs complete.
+            stage_names.append("stage-wait")
+            wait_job = spack_ci_ir["jobs"]["noop"]["attributes"]
+            wait_job["stage"] = "stage-wait"
+            wait_job["retry"] = 0
+            wait_job["when"] = "always"
+            wait_job["script"] = ["echo 'Open the pod bay doors HAL'"]
+            wait_job["dependencies"] = []
+
+            output_object["wait-for-build-jobs"] = wait_job
+
             # Add a final job to regenerate the index
             stage_names.append("stage-rebuild-index")
             final_job = spack_ci_ir["jobs"]["reindex"]["attributes"]
 
             final_job["stage"] = "stage-rebuild-index"
-            target_mirror = options.buildcache_destination.push_url
-            final_job["script"] = unpack_script(
-                final_job["script"],
-                op=lambda cmd: cmd.replace("{index_target_mirror}", target_mirror),
-            )
+            final_job["script"] = unpack_script(final_job["script"], op=main_script_replacements)
 
             final_job["when"] = "always"
             final_job["retry"] = service_job_retries
             final_job["interruptible"] = True
-            final_job["dependencies"] = []
+            # update-index needs to download generate artifacts
+            # it also needs to wait until all of the other stages complete.
+            final_job["needs"] = [
+                {"job": generate_job_name, "pipeline": f"{generate_pipeline_id}"},
+                "wait-for-build-jobs",
+            ]
 
             output_object["rebuild-index"] = final_job
 

lib/spack/spack/test/binary_distribution.py

@@ -1449,6 +1449,54 @@ def test_mirror_metadata():
         spack.binary_distribution.MirrorMetadata.from_string("https://dummy.io/__v3@@4")
 
 
+def mirror_metadata_check_format(data, fmt, result):
+    assert fmt.format(data) == result.format(data)
+
+
+def test_mirror_metadata_format():
+    mirror_metadata = spack.binary_distribution.MirrorMetadata("https://dummy.io/__v3", 3)
+
+    # Check pass-through formatting
+    mirror_metadata_check_format(mirror_metadata, "{0:_url}", "{0.url}")
+    mirror_metadata_check_format(mirror_metadata, "{0:_version}", "{0.version}")
+    mirror_metadata_check_format(mirror_metadata, "{0:_view}", "{0.view}")
+
+    # Empty view
+    mirror_metadata_check_format(mirror_metadata, "{0:?_view}", "")
+    mirror_metadata_check_format(
+        mirror_metadata, "{0:_url?^_view^_version?^_version}", "{0.url}^{0.version}"
+    )
+    mirror_metadata_check_format(
+        mirror_metadata,
+        "{0:_url?^_view^_version?^_version?^_view^_version?^_url}",
+        "{0.url}^{0.version}^{0.url}",
+    )
+
+
+def test_mirror_metadata_format_with_view():
+    mirror_metadata = spack.binary_distribution.MirrorMetadata(
+        "https://dummy.io/__v3__@aview", 3, "aview"
+    )
+
+    # Check pass-through formatting
+    mirror_metadata_check_format(mirror_metadata, "{0:_url}", "{0.url}")
+    mirror_metadata_check_format(mirror_metadata, "{0:_version}", "{0.version}")
+    mirror_metadata_check_format(mirror_metadata, "{0:_view}", "{0.view}")
+
+    # View exists
+    mirror_metadata_check_format(mirror_metadata, "{0:?_view}", "{0.view}")
+    mirror_metadata_check_format(
+        mirror_metadata,
+        "{0:_url?^_view^_version?^_version}",
+        "{0.url}^{0.view}^{0.version}^{0.version}",
+    )
+    mirror_metadata_check_format(
+        mirror_metadata,
+        "{0:_url?^_view^_version?^_version?^_view^_version?^_url}",
+        "{0.url}^{0.view}^{0.version}^{0.version}^{0.view}^{0.version}^{0.url}",
+    )
+
+
 def test_mirror_metadata_with_view():
     mirror_metadata = spack.binary_distribution.MirrorMetadata(
         "https://dummy.io/__v3__@aview", 3, "aview"

lib/spack/spack/test/cmd/ci.py

@@ -193,14 +193,15 @@ def test_ci_generate_with_env(ci_generate_test, tmp_path: pathlib.Path, mock_bin
     assert yaml_contents["workflow"]["rules"] == [{"when": "always"}]
 
     assert "stages" in yaml_contents
-    assert len(yaml_contents["stages"]) == 6
+    assert len(yaml_contents["stages"]) == 7
     assert yaml_contents["stages"][0] == "stage-0"
-    assert yaml_contents["stages"][5] == "stage-rebuild-index"
+    assert yaml_contents["stages"][5] == "stage-wait"
+    assert yaml_contents["stages"][6] == "stage-rebuild-index"
 
     assert "rebuild-index" in yaml_contents
     rebuild_job = yaml_contents["rebuild-index"]
     assert (
-        rebuild_job["script"][0] == f"spack buildcache update-index --keys {mirror_url.as_uri()}"
+        rebuild_job["script"][1] == "spack buildcache update-index --keys  buildcache-destination"
     )
     assert rebuild_job["custom_attribute"] == "custom!"
 
@@ -332,12 +333,11 @@ def test_ci_generate_with_custom_settings(
             "git checkout ${SPACK_REF}",
             "popd",
         ]
-        assert ci_obj["script"][1].startswith("cd ")
-        ci_obj["script"][1] = "cd ENV"
+        assert ci_obj["script"][1].startswith("spack env activate --without-view ")
+        ci_obj["script"][1] = "spack env activate --without-view ENV"
         assert ci_obj["script"] == [
             "spack -d ci rebuild",
-            "cd ENV",
-            "spack env activate --without-view .",
+            "spack env activate --without-view ENV",
             "spack ci rebuild",
         ]
         assert ci_obj["after_script"] == ["rm -rf /some/path/spack"]
@@ -1674,7 +1674,8 @@ def test_ci_generate_mirror_config(
     with open(tmp_path / ".gitlab-ci.yml", encoding="utf-8") as f:
         pipeline_doc = syaml.load(f)
         assert fst not in pipeline_doc["rebuild-index"]["script"][0]
-        assert snd in pipeline_doc["rebuild-index"]["script"][0]
+        assert "env activate" in pipeline_doc["rebuild-index"]["script"][0]
+        assert "buildcache-destination" in pipeline_doc["rebuild-index"]["script"][1]
 
 
 def dynamic_mapping_setup(tmp_path: pathlib.Path):
@@ -1855,11 +1856,13 @@ def test_ci_generate_copy_only(
     # Make sure there are only two jobs and two stages
     stages = pipeline_doc["stages"]
     copy_stage = "copy"
+    wait_stage = "stage-wait"
     rebuild_index_stage = "stage-rebuild-index"
 
-    assert len(stages) == 2
+    assert len(stages) == 3
     assert stages[0] == copy_stage
-    assert stages[1] == rebuild_index_stage
+    assert stages[1] == wait_stage
+    assert stages[2] == rebuild_index_stage
 
     rebuild_index_job = pipeline_doc["rebuild-index"]
     assert rebuild_index_job["stage"] == rebuild_index_stage

lib/spack/spack/url_buildcache.py

@@ -1365,10 +1365,7 @@ def __init__(self, url: str, version: int, view: Optional[str] = None):
         self.view = view
 
     def __str__(self):
-        s = f"{self.url}__v{self.version}"
-        if self.view:
-            s += f"__{self.view}"
-        return s
+        return f"{self:_url__v_version?___view}"
 
     def __eq__(self, other):
         if not isinstance(other, MirrorMetadata):
@@ -1378,6 +1375,37 @@ def __eq__(self, other):
     def __hash__(self):
         return hash((self.url, self.version, self.view))
 
+    def __format__(self, format_spec):
+        """Format the mirror metadata
+
+        Format Spec:
+            _url:     metadata.url
+            _version: metadata.version
+            _view:    metadata.view
+            ?:        delimiter to wrap conditional printing based on optional view
+
+        Example
+
+            f"{meta_data:_url?^_view?@v_version}"
+
+            Expansion without a view:
+                https://my-mirror.com/prefix@v3
+
+            Expansion with a view:
+                https://my-mirror.com/prefix^my-view@v3
+        """
+        if not format_spec:
+            format_spec = "_url@v3?-_view"
+            return
+        out = format_spec.replace("_url", self.url)
+        out = out.replace("_version", str(self.version))
+        out = out.replace("_view", str(self.view))
+        parts = out.split("?")
+        if self.view:
+            return "".join(parts)
+        else:
+            return "".join(parts[0::2])
+
     @classmethod
     def from_string(cls, s: str):
         m = re.match(r"^(.*)__v([0-9]+)(?:__(.*))?$", s)