Pipelines: pass relative artifact paths to child jobs (#24085)

Passing absolute paths from pipeline generate job to downstream rebuild jobs
causes problems when the CI_PROJECT_DIR is not the same for the generate and
rebuild jobs.  This has happened, for example, when gitlab checks out the
project into a runner-specific directory and different runners are chosen
for the generate and rebuild jobs.

lib/spack/spack/ci.py

@@ -642,6 +642,25 @@ def generate_gitlab_ci_yaml(env, print_summary, output_file,
     local_mirror_dir = os.path.join(pipeline_artifacts_dir, 'mirror')
     user_artifacts_dir = os.path.join(pipeline_artifacts_dir, 'user_data')
 
+    # We communicate relative paths to the downstream jobs to avoid issues in
+    # situations where the CI_PROJECT_DIR varies between the pipeline
+    # generation job and the rebuild jobs.  This can happen when gitlab
+    # checks out the project into a runner-specific directory, for example,
+    # and different runners are picked for generate and rebuild jobs.
+    ci_project_dir = os.environ.get('CI_PROJECT_DIR')
+    rel_artifacts_root = os.path.relpath(
+        pipeline_artifacts_dir, ci_project_dir)
+    rel_concrete_env_dir = os.path.relpath(
+        concrete_env_dir, ci_project_dir)
+    rel_job_log_dir = os.path.relpath(
+        job_log_dir, ci_project_dir)
+    rel_job_repro_dir = os.path.relpath(
+        job_repro_dir, ci_project_dir)
+    rel_local_mirror_dir = os.path.relpath(
+        local_mirror_dir, ci_project_dir)
+    rel_user_artifacts_dir = os.path.relpath(
+        user_artifacts_dir, ci_project_dir)
+
     # Speed up staging by first fetching binary indices from all mirrors
     # (including the per-PR mirror we may have just added above).
     bindist.binary_index.update()
@@ -884,9 +903,9 @@ def generate_gitlab_ci_yaml(env, print_summary, output_file,
                 variables.update(job_vars)
 
                 artifact_paths = [
-                    job_log_dir,
-                    job_repro_dir,
-                    user_artifacts_dir
+                    rel_job_log_dir,
+                    rel_job_repro_dir,
+                    rel_user_artifacts_dir
                 ]
 
                 if enable_artifacts_buildcache:
@@ -1042,14 +1061,14 @@ def generate_gitlab_ci_yaml(env, print_summary, output_file,
                 version_to_clone = spack_version
 
         output_object['variables'] = {
-            'SPACK_ARTIFACTS_ROOT': pipeline_artifacts_dir,
-            'SPACK_CONCRETE_ENV_DIR': concrete_env_dir,
+            'SPACK_ARTIFACTS_ROOT': rel_artifacts_root,
+            'SPACK_CONCRETE_ENV_DIR': rel_concrete_env_dir,
             'SPACK_VERSION': spack_version,
             'SPACK_CHECKOUT_VERSION': version_to_clone,
             'SPACK_REMOTE_MIRROR_URL': remote_mirror_url,
-            'SPACK_JOB_LOG_DIR': job_log_dir,
-            'SPACK_JOB_REPRO_DIR': job_repro_dir,
-            'SPACK_LOCAL_MIRROR_DIR': local_mirror_dir,
+            'SPACK_JOB_LOG_DIR': rel_job_log_dir,
+            'SPACK_JOB_REPRO_DIR': rel_job_repro_dir,
+            'SPACK_LOCAL_MIRROR_DIR': rel_local_mirror_dir,
             'SPACK_IS_PR_PIPELINE': str(is_pr_pipeline)
         }
 
@@ -1535,7 +1554,6 @@ def reproduce_ci_job(url, work_dir):
         tty.debug('  {0}'.format(yaml_file))
 
     pipeline_yaml = None
-    pipeline_variables = None
 
     # Try to find the dynamically generated pipeline yaml file in the
     # reproducer.  If the user did not put it in the artifacts root,
@@ -1546,7 +1564,6 @@ def reproduce_ci_job(url, work_dir):
             yaml_obj = syaml.load(y_fd)
             if 'variables' in yaml_obj and 'stages' in yaml_obj:
                 pipeline_yaml = yaml_obj
-                pipeline_variables = pipeline_yaml['variables']
 
     if pipeline_yaml:
         tty.debug('\n{0} is likely your pipeline file'.format(yf))
@@ -1603,9 +1620,8 @@ def reproduce_ci_job(url, work_dir):
         # more faithful reproducer if everything appears to run in the same
         # absolute path used during the CI build.
         mount_as_dir = '/work'
-        if pipeline_variables:
-            artifacts_root = pipeline_variables['SPACK_ARTIFACTS_ROOT']
-            mount_as_dir = os.path.dirname(artifacts_root)
+        if repro_details:
+            mount_as_dir = repro_details['ci_project_dir']
             mounted_repro_dir = os.path.join(mount_as_dir, rel_repro_dir)
 
         # We will also try to clone spack from your local checkout and

lib/spack/spack/cmd/ci.py

@@ -201,6 +201,15 @@ def ci_rebuild(args):
     pr_mirror_url = get_env_var('SPACK_PR_MIRROR_URL')
     remote_mirror_url = get_env_var('SPACK_REMOTE_MIRROR_URL')
 
+    # Construct absolute paths relative to current $CI_PROJECT_DIR
+    ci_project_dir = get_env_var('CI_PROJECT_DIR')
+    pipeline_artifacts_dir = os.path.join(
+        ci_project_dir, pipeline_artifacts_dir)
+    job_log_dir = os.path.join(ci_project_dir, job_log_dir)
+    repro_dir = os.path.join(ci_project_dir, repro_dir)
+    local_mirror_dir = os.path.join(ci_project_dir, local_mirror_dir)
+    concrete_env_dir = os.path.join(ci_project_dir, concrete_env_dir)
+
     # Debug print some of the key environment variables we should have received
     tty.debug('pipeline_artifacts_dir = {0}'.format(pipeline_artifacts_dir))
     tty.debug('root_spec = {0}'.format(root_spec))
@@ -342,7 +351,8 @@ def ci_rebuild(args):
     repro_details = {
         'job_name': ci_job_name,
         'job_spec_yaml': job_spec_yaml_file,
-        'root_spec_yaml': 'root.yaml'
+        'root_spec_yaml': 'root.yaml',
+        'ci_project_dir': ci_project_dir
     }
     with open(repro_file, 'w') as fd:
         fd.write(json.dumps(repro_details))

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

@@ -41,6 +41,17 @@
 pytestmark = pytest.mark.maybeslow
 
 
+@pytest.fixture()
+def project_dir_env():
+    def _set_project_dir(path):
+        os.environ['CI_PROJECT_DIR'] = path
+
+    yield _set_project_dir
+
+    if 'CI_PROJECT_DIR' in os.environ:
+        os.environ.pop('CI_PROJECT_DIR')
+
+
 @pytest.fixture()
 def env_deactivate():
     yield
@@ -112,9 +123,10 @@ def test_specs_staging(config):
 
 
 def test_ci_generate_with_env(tmpdir, mutable_mock_env_path, env_deactivate,
-                              install_mockery, mock_packages):
+                              install_mockery, mock_packages, project_dir_env):
     """Make sure we can get a .gitlab-ci.yml from an environment file
        which has the gitlab-ci, cdash, and mirrors sections."""
+    project_dir_env(tmpdir.strpath)
     mirror_url = 'https://my.fake.mirror'
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
@@ -183,6 +195,11 @@ def test_ci_generate_with_env(tmpdir, mutable_mock_env_path, env_deactivate,
                 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')
+
 
 def _validate_needs_graph(yaml_contents, needs_graph, artifacts):
     for job_name, job_def in yaml_contents.items():
@@ -202,9 +219,10 @@ def _validate_needs_graph(yaml_contents, needs_graph, artifacts):
 
 def test_ci_generate_bootstrap_gcc(tmpdir, mutable_mock_env_path,
                                    env_deactivate, install_mockery,
-                                   mock_packages):
+                                   mock_packages, project_dir_env):
     """Test that we can bootstrap a compiler and use it as the
     compiler for a spec in the environment"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -265,9 +283,11 @@ def test_ci_generate_bootstrap_artifacts_buildcache(tmpdir,
                                                     mutable_mock_env_path,
                                                     env_deactivate,
                                                     install_mockery,
-                                                    mock_packages):
+                                                    mock_packages,
+                                                    project_dir_env):
     """Test that we can bootstrap a compiler when artifacts buildcache
     is turned on"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -329,8 +349,9 @@ def test_ci_generate_bootstrap_artifacts_buildcache(tmpdir,
 
 def test_ci_generate_with_env_missing_section(tmpdir, mutable_mock_env_path,
                                               env_deactivate, install_mockery,
-                                              mock_packages):
+                                              mock_packages, project_dir_env):
     """Make sure we get a reasonable message if we omit gitlab-ci section"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -353,8 +374,9 @@ def test_ci_generate_with_env_missing_section(tmpdir, mutable_mock_env_path,
 
 def test_ci_generate_with_cdash_token(tmpdir, mutable_mock_env_path,
                                       env_deactivate, install_mockery,
-                                      mock_packages):
+                                      mock_packages, project_dir_env):
     """Make sure we it doesn't break if we configure cdash"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -407,8 +429,10 @@ def test_ci_generate_with_cdash_token(tmpdir, mutable_mock_env_path,
 
 def test_ci_generate_with_custom_scripts(tmpdir, mutable_mock_env_path,
                                          env_deactivate, install_mockery,
-                                         mock_packages, monkeypatch):
+                                         mock_packages, monkeypatch,
+                                         project_dir_env):
     """Test use of user-provided scripts"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -496,8 +520,9 @@ def test_ci_generate_with_custom_scripts(tmpdir, mutable_mock_env_path,
 
 def test_ci_generate_pkg_with_deps(tmpdir, mutable_mock_env_path,
                                    env_deactivate, install_mockery,
-                                   mock_packages):
+                                   mock_packages, project_dir_env):
     """Test pipeline generation for a package w/ dependencies"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -549,10 +574,12 @@ def test_ci_generate_pkg_with_deps(tmpdir, mutable_mock_env_path,
 
 def test_ci_generate_for_pr_pipeline(tmpdir, mutable_mock_env_path,
                                      env_deactivate, install_mockery,
-                                     mock_packages, monkeypatch):
+                                     mock_packages, monkeypatch,
+                                     project_dir_env):
     """Test that PR pipelines do not include a final stage job for
     rebuilding the mirror index, even if that job is specifically
     configured"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -609,8 +636,10 @@ def test_ci_generate_for_pr_pipeline(tmpdir, mutable_mock_env_path,
 
 def test_ci_generate_with_external_pkg(tmpdir, mutable_mock_env_path,
                                        env_deactivate, install_mockery,
-                                       mock_packages, monkeypatch):
+                                       mock_packages, monkeypatch,
+                                       project_dir_env):
     """Make sure we do not generate jobs for external pkgs"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -649,7 +678,8 @@ def test_ci_generate_with_external_pkg(tmpdir, mutable_mock_env_path,
 
 def test_ci_rebuild(tmpdir, mutable_mock_env_path, env_deactivate,
                     install_mockery, mock_packages, monkeypatch,
-                    mock_gnupghome, mock_fetch):
+                    mock_gnupghome, mock_fetch, project_dir_env):
+    project_dir_env(tmpdir.strpath)
     working_dir = tmpdir.join('working_dir')
 
     log_dir = os.path.join(working_dir.strpath, 'logs')
@@ -790,7 +820,8 @@ def mystrip(s):
 
 def test_ci_nothing_to_rebuild(tmpdir, mutable_mock_env_path, env_deactivate,
                                install_mockery, mock_packages, monkeypatch,
-                               mock_fetch):
+                               mock_fetch, project_dir_env):
+    project_dir_env(tmpdir.strpath)
     working_dir = tmpdir.join('working_dir')
 
     mirror_dir = working_dir.join('mirror')
@@ -864,7 +895,8 @@ def fake_dl_method(spec, dest, require_cdashid, m_url=None):
 @pytest.mark.disable_clean_stage_check
 def test_push_mirror_contents(tmpdir, mutable_mock_env_path, env_deactivate,
                               install_mockery, mock_packages, mock_fetch,
-                              mock_stage, mock_gnupghome):
+                              mock_stage, mock_gnupghome, project_dir_env):
+    project_dir_env(tmpdir.strpath)
     working_dir = tmpdir.join('working_dir')
 
     mirror_dir = working_dir.join('mirror')
@@ -1033,11 +1065,13 @@ def faked(env, spec_yaml=None, packages=None, add_spec=True,
 
 def test_ci_generate_override_runner_attrs(tmpdir, mutable_mock_env_path,
                                            env_deactivate, install_mockery,
-                                           mock_packages, monkeypatch):
+                                           mock_packages, monkeypatch,
+                                           project_dir_env):
     """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"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -1174,8 +1208,10 @@ def test_ci_generate_override_runner_attrs(tmpdir, mutable_mock_env_path,
 
 def test_ci_generate_with_workarounds(tmpdir, mutable_mock_env_path,
                                       env_deactivate, install_mockery,
-                                      mock_packages, monkeypatch):
+                                      mock_packages, monkeypatch,
+                                      project_dir_env):
     """Make sure the post-processing cli workarounds do what they should"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -1274,13 +1310,14 @@ def test_ci_rebuild_index(tmpdir, mutable_mock_env_path, env_deactivate,
 def test_ci_generate_bootstrap_prune_dag(
         install_mockery_mutable_config, mock_packages, mock_fetch,
         mock_archive, mutable_config, monkeypatch, tmpdir,
-        mutable_mock_env_path, env_deactivate):
+        mutable_mock_env_path, env_deactivate, project_dir_env):
     """Test compiler bootstrapping with DAG pruning.  Specifically, make
        sure that if we detect the bootstrapped compiler needs to be rebuilt,
        we ensure the spec we want to build with that compiler is scheduled
        for rebuild as well."""
 
     # Create a temp mirror directory for buildcache usage
+    project_dir_env(tmpdir.strpath)
     mirror_dir = tmpdir.join('mirror_dir')
     mirror_url = 'file://{0}'.format(mirror_dir.strpath)
 
@@ -1404,8 +1441,9 @@ def fake_get_mirrors_for_spec(spec=None, full_hash_match=False,
 
 def test_ci_subcommands_without_mirror(tmpdir, mutable_mock_env_path,
                                        env_deactivate, mock_packages,
-                                       install_mockery):
+                                       install_mockery, project_dir_env):
     """Make sure we catch if there is not a mirror and report an error"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -1481,8 +1519,10 @@ def test_ensure_only_one_temporary_storage():
 
 def test_ci_generate_temp_storage_url(tmpdir, mutable_mock_env_path,
                                       env_deactivate, install_mockery,
-                                      mock_packages, monkeypatch):
+                                      mock_packages, monkeypatch,
+                                      project_dir_env):
     """Verify correct behavior when using temporary-storage-url-prefix"""
+    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -1533,8 +1573,10 @@ def test_ci_generate_temp_storage_url(tmpdir, mutable_mock_env_path,
 
 def test_ci_generate_read_broken_specs_url(tmpdir, mutable_mock_env_path,
                                            env_deactivate, install_mockery,
-                                           mock_packages, monkeypatch):
+                                           mock_packages, monkeypatch,
+                                           project_dir_env):
     """Verify that `broken-specs-url` works as intended"""
+    project_dir_env(tmpdir.strpath)
     spec_a = Spec('a')
     spec_a.concretize()
     a_full_hash = spec_a.full_hash()
@@ -1585,7 +1627,8 @@ def test_ci_generate_read_broken_specs_url(tmpdir, mutable_mock_env_path,
 
 def test_ci_reproduce(tmpdir, mutable_mock_env_path, env_deactivate,
                       install_mockery, mock_packages, monkeypatch,
-                      last_two_git_commits):
+                      last_two_git_commits, project_dir_env):
+    project_dir_env(tmpdir.strpath)
     working_dir = tmpdir.join('repro_dir')
     image_name = 'org/image:tag'
 
@@ -1657,7 +1700,8 @@ def test_ci_reproduce(tmpdir, mutable_mock_env_path, env_deactivate,
             repro_details = {
                 'job_name': job_name,
                 'job_spec_yaml': 'archivefiles.yaml',
-                'root_spec_yaml': 'root.yaml'
+                'root_spec_yaml': 'root.yaml',
+                'ci_project_dir': working_dir.strpath
             }
             with open(repro_file, 'w') as fd:
                 fd.write(json.dumps(repro_details))