From b503af023f413792ebdd176308fe1fe3546fd3df Mon Sep 17 00:00:00 2001 From: Lalit Sudarshan Date: Tue, 11 Aug 2026 11:20:58 -0400 Subject: [PATCH 1/5] feat(packaging): publish amdrocm-repo via workflow outputs Derives the publish destination from WorkflowOutputRoot and writes through StorageBackend rather than taking a bucket and prefix and driving boto3 directly. The new output type sits beside the content repository rather than inside it: one package is built per OS profile and several share a filename, so indexing them together would collide. Also adds the URL helpers the workflow invokes and an install-harness mode that configures the repository by installing the built package. --- .../_therock_utils/workflow_outputs.py | 29 ++ .../packaging/linux/get_url_repo_params.py | 143 +++++- .../native_linux_package_install_test.py | 323 +++++++++++- .../packaging/linux/publish_repo_package.py | 127 +++++ .../linux/tests/get_url_repo_params_test.py | 163 ++++++ .../native_linux_package_install_ut_test.py | 478 ++++++++++++++++++ .../tests/publish_repo_package_test.py | 280 ++++++++++ build_tools/tests/workflow_outputs_test.py | 68 +++ docs/development/workflow_outputs.md | 5 + 9 files changed, 1598 insertions(+), 18 deletions(-) create mode 100644 build_tools/packaging/linux/publish_repo_package.py create mode 100644 build_tools/tests/publish_repo_package_test.py diff --git a/build_tools/_therock_utils/workflow_outputs.py b/build_tools/_therock_utils/workflow_outputs.py index d1d312d3aa0..d2e71a13169 100644 --- a/build_tools/_therock_utils/workflow_outputs.py +++ b/build_tools/_therock_utils/workflow_outputs.py @@ -253,6 +253,35 @@ def native_linux_packages_log_index(self, pkg_type: str) -> StorageLocation: self.bucket, f"{self.prefix}/logs/packaging/{pkg_type}/index.html" ) + def native_linux_repo_package( + self, pkg_type: str, os_profile: str + ) -> StorageLocation: + """Location for the ``amdrocm-repo`` bootstrap package. + + Returns ``StorageLocation`` at + ``{run_id}-linux/packages/{pkg_type}/repo/{os_profile}/amdrocm-repo.{pkg_type}`` + (e.g. ``12345678901-linux/packages/deb/repo/ubuntu2404/amdrocm-repo.deb``). + + This is a standalone file fetched by URL to configure the repository, + so it sits *beside* the repository index rather than inside it: it is + never reached by ``dists/`` (deb) or ``x86_64/repodata`` (rpm) and so + is not part of any package index. Keeping it out matters because one + ``amdrocm-repo`` is built per OS profile and several share a filename, + which would collide if they were indexed together. + + The object name is fixed so the download URL is stable regardless of + the built package's versioned filename. + + Args: + pkg_type: Package type ('deb' or 'rpm'). + os_profile: Target distro profile (e.g. 'ubuntu2404', 'rhel10'). + """ + return StorageLocation( + self.bucket, + f"{self.prefix}/packages/{pkg_type}/repo/{os_profile}" + f"/amdrocm-repo.{pkg_type}", + ) + # -- Python packages -------------------------------------------------------- def python_packages(self, artifact_group: str = "") -> StorageLocation: diff --git a/build_tools/packaging/linux/get_url_repo_params.py b/build_tools/packaging/linux/get_url_repo_params.py index 0ee5e5e3b71..c0577b9f02e 100644 --- a/build_tools/packaging/linux/get_url_repo_params.py +++ b/build_tools/packaging/linux/get_url_repo_params.py @@ -24,7 +24,8 @@ regardless of release_type (if omitted, install runs without GPG verification). Subcommands: get-base-url, get-gpg-url, get-repo-sub-folder, get-repo-url, extract-gfx-arch, -get-container-image. Run with -h for examples. +get-container-image, get-public-repo-base-url, get-nightly-sub-folder, +get-container-image-map. Run with -h for examples. Maintenance (when packaging or install URLs change): @@ -36,9 +37,11 @@ """ import argparse +import json import os import re import sys +from datetime import date, datetime, timezone from pathlib import Path from urllib.parse import urlparse @@ -479,6 +482,95 @@ def cmd_repo_url(args: argparse.Namespace) -> int: return 0 +# --- public repository base URL --- + +# Public CDN base for each release line: scheme + host + the packages-multi-arch +# segment. The layout *beneath* the base is not uniform and is appended by the +# caller -- release serves per-distro only (its deb/ and rpm/x86_64/ return 403), +# nightly serves flat under a dated sub-folder only (its per-distro paths return +# 403), and prerelease serves both. The one path that is uniform across all three +# is gpg/rocm.gpg at the base. +# Lines without a public repository (e.g. ci, dev) map to an empty string, which +# callers treat as "no amdrocm-repo package for this line". +_PUBLIC_REPO_BASE_URLS = { + "prerelease": "https://rocm.prereleases.amd.com/packages-multi-arch", + "release": "https://repo.amd.com/rocm/packages-multi-arch", + "nightly": "https://rocm.nightlies.amd.com/packages-multi-arch", +} + + +def get_public_repo_base_url(release_type: str) -> str: + """Return the public CDN base URL for a release line (empty if none).""" + return _PUBLIC_REPO_BASE_URLS.get(release_type.strip().lower(), "") + + +def cmd_public_repo_base_url(args: argparse.Namespace) -> int: + gha_set_output({"repo_base_url": get_public_repo_base_url(args.release_type)}) + return 0 + + +# --- nightly sub-folder --- + + +# A workflow run id is always numeric. It is interpolated into a value written +# to $GITHUB_OUTPUT, and gha_set_output writes multi-line values with a heredoc +# whose delimiter it does not verify, so an unconstrained value could terminate +# the heredoc early and inject further step outputs. \Z (not $) so a trailing +# newline is not accepted. +_RUN_ID_RE = re.compile(r"^[0-9]+\Z") + + +def nightly_sub_folder(run_id: str, *, today: date | None = None) -> str: + """Return the dated nightly sub-folder ``YYYYMMDD-``. + + ``today`` is injectable for deterministic tests; it defaults to the current + UTC date. + + Raises: + ValueError: If ``run_id`` is not a run of digits. + """ + if not _RUN_ID_RE.match(run_id): + raise ValueError(f"run_id must be numeric, got {run_id!r}") + day = today or datetime.now(timezone.utc).date() + return f"{day:%Y%m%d}-{run_id}" + + +def cmd_nightly_sub_folder(args: argparse.Namespace) -> int: + # Only the nightly line publishes into a dated sub-folder; every other line + # publishes at the repository root, so the value is empty there. + if args.release_type and args.release_type.strip().lower() != "nightly": + gha_set_output({"repo_sub_folder": ""}) + return 0 + try: + sub_folder = nightly_sub_folder(args.run_id) + except ValueError as e: + print(f"Error: {e}", file=sys.stderr) + return 1 + gha_set_output({"repo_sub_folder": sub_folder}) + return 0 + + +# --- container image map --- + + +def get_container_image_map(os_profiles: list[str]) -> dict[str, str]: + """Map each OS profile to its container image (for a build matrix).""" + return {profile: get_container_image(profile) for profile in os_profiles} + + +def cmd_container_image_map(args: argparse.Namespace) -> int: + try: + profiles = json.loads(args.os_profiles) + except json.JSONDecodeError as e: + print(f"Error: --os-profiles is not valid JSON: {e}", file=sys.stderr) + return 1 + if not isinstance(profiles, list) or not all(isinstance(p, str) for p in profiles): + print("Error: --os-profiles must be a JSON array of strings", file=sys.stderr) + return 1 + gha_set_output({"images": json.dumps(get_container_image_map(profiles))}) + return 0 + + # --- extract-gfx-arch --- @@ -755,6 +847,55 @@ def main(argv: list[str] | None = None) -> int: ) p_img.set_defaults(func=cmd_container_image) + # get-public-repo-base-url: public CDN base for a release line + p_pub = subparsers.add_parser( + "get-public-repo-base-url", + help="Print repo_base_url= for a release line (empty for lines without a public repo, e.g. ci/dev).", + ) + p_pub.add_argument( + "--release-type", + type=str, + required=True, + help="Release line (e.g. prerelease, release, nightly)", + ) + p_pub.set_defaults(func=cmd_public_repo_base_url) + + # get-nightly-sub-folder: dated sub-folder for the nightly line + p_sub = subparsers.add_parser( + "get-nightly-sub-folder", + help="Print repo_sub_folder=YYYYMMDD- for the nightly line.", + ) + p_sub.add_argument( + "--run-id", + type=str, + required=True, + help="Workflow run id to embed in the dated sub-folder", + ) + p_sub.add_argument( + "--release-type", + type=str, + default=None, + help=( + "If set, print an empty repo_sub_folder for any line other than " + "'nightly'. If omitted, always print the dated sub-folder." + ), + ) + p_sub.set_defaults(func=cmd_nightly_sub_folder) + + # get-container-image-map: os_profile -> image map for a build matrix + p_map = subparsers.add_parser( + "get-container-image-map", + help="Print images= mapping each OS profile (JSON array input) to its container image.", + ) + p_map.add_argument( + "--os-profiles", + type=str, + required=True, + metavar="JSON", + help='JSON array of OS profiles (e.g. ["rhel8","rhel10","sles16"])', + ) + p_map.set_defaults(func=cmd_container_image_map) + args = parser.parse_args(argv) return args.func(args) diff --git a/build_tools/packaging/linux/native_linux_package_install_test.py b/build_tools/packaging/linux/native_linux_package_install_test.py index 2c2be944309..51901cbd447 100644 --- a/build_tools/packaging/linux/native_linux_package_install_test.py +++ b/build_tools/packaging/linux/native_linux_package_install_test.py @@ -137,6 +137,7 @@ import stat import subprocess import sys +import time import traceback from argparse import ArgumentParser, Namespace from pathlib import Path, PurePosixPath @@ -166,7 +167,9 @@ def _env(key: str, default: str) -> str: # documented amdgpu package manager steps set the signing key up there # ("wget .../rocm.gpg.key | gpg --dearmor | sudo tee /etc/apt/keyrings/rocm.gpg") # and point signed-by= at it. This harness writes its keyring the same way, so -# sharing the path would overwrite the key a host is already using. +# sharing the path would overwrite the key a host is already using. Distinct +# from the amdrocm-repo package's own keyring (AMDROCM_DEB_KEYRING), which is +# package-owned and lives in a different directory. APT_KEYRING_FILE = _env("ROCM_APT_KEYRING_FILE", f"/etc/apt/keyrings/{REPO_NAME}.gpg") ZYPP_REPOS_DIR = _env("ROCM_ZYPP_REPOS_DIR", "/etc/zypp/repos.d") YUM_REPOS_DIR = _env("ROCM_YUM_REPOS_DIR", "/etc/yum.repos.d") @@ -180,6 +183,21 @@ def _env(key: str, default: str) -> str: # Relative path from install prefix to rdhc binary (script); overridable via ROCM_RDHC_REL_PATH RDHC_REL_PATH = _env("ROCM_RDHC_REL_PATH", "libexec/rocm-core/rdhc.py") +# amdrocm-repo package: the on-disk paths it installs (the package's public +# contract). Used by the install-via-package mode to confirm the repo config +# landed. These are independent of REPO_NAME (which names the manually-written +# repo file used by the default setup path). +AMDROCM_REPO_ID = "amdrocm" +AMDROCM_DEB_SOURCES = "/etc/apt/sources.list.d/amdrocm.sources" +# Named amdrocm.gpg, not rocm.gpg, so it cannot collide with the keyring the +# amdgpu driver setup from repo.radeon.com installs. Must match +# build_repo_package.DEB_KEYRING_PATH. +AMDROCM_DEB_KEYRING = "/usr/share/keyrings/amdrocm.gpg" +AMDROCM_RPM_REPO_BASENAME = "amdrocm.repo" +# Named -amdrocm, not -rocm, for the same collision reason as the deb +# keyring. Must match build_repo_package.RPM_GPG_KEY_PATH. +AMDROCM_RPM_KEY = "/etc/pki/rpm-gpg/RPM-GPG-KEY-amdrocm" + # Pytest/CI only: becomes ``--rocm-version``. ENV_NATIVE_LINUX_INSTALL_ROCM_VERSION = "NATIVE_LINUX_INSTALL_ROCM_VERSION" @@ -190,6 +208,12 @@ def _env(key: str, default: str) -> str: ZYPP_CLEAN_TIMEOUT_SEC = 60 ZYPP_REFRESH_TIMEOUT_SEC = 120 DNF_CLEAN_TIMEOUT_SEC = 60 +DNF_MAKECACHE_TIMEOUT_SEC = 120 +REPO_PACKAGE_INSTALL_TIMEOUT_SEC = 120 +# Retry the post-install metadata refresh so a transient repo/CDN blip does not +# fail the (network-dependent) config-only gate. +REPO_REFRESH_ATTEMPTS = 3 +REPO_REFRESH_BACKOFF_SEC = 3 INSTALL_TIMEOUT_SEC = 1800 # 30 minutes ROCMINFO_TIMEOUT_SEC = 30 # rdhc.py ``--all`` runs the full ROCm deployment health check suite; 30s was too @@ -372,11 +396,14 @@ def __init__( rocm_version: str | None = None, gpg_key_url: str | None = None, build_variant: str = "", + repo_package_dir: str | None = None, + repo_config_only: bool = False, ): """Initialize the native Linux package install test runner. Args: - repo_url: Full repository URL (constructed in YAML) + repo_url: Full repository URL (constructed in YAML). May be empty/None when + repo_package_dir is set (the amdrocm-repo package supplies the repo URL). os_profile: OS profile (e.g., ubuntu2404, rhel8, debian12, sles15, sles16, almalinux9, centos7, azl3) release_type: Type of release ('nightly' or 'prerelease') install_prefix: Installation prefix (default: /opt/rocm/core) @@ -393,11 +420,20 @@ def __init__( inserted before the version suffix in package names so that variant packages are tested (e.g. ``amdrocm-asan7.15-gfx942`` instead of ``amdrocm7.15-gfx942``). + repo_package_dir: Directory holding the built ``amdrocm-repo`` package. When + set, the repository is configured by installing that package instead of + writing repo files directly. + repo_config_only: With ``repo_package_dir``, install the package and verify + the repo config, then stop (do not install ROCm). """ self.os_profile = os_profile.lower() self.package_type = self._derive_package_type(os_profile) - self.repo_url = repo_url.rstrip("/") + # repo_url may be empty/None in install-via-package mode (the package + # supplies it); tolerate both without raising. + self.repo_url = (repo_url or "").rstrip("/") self.release_type = release_type.lower() + self.repo_package_dir = repo_package_dir + self.repo_config_only = repo_config_only self.install_prefix = install_prefix self.gfx_arch_list = normalize_target_list( gfx_arch, lowercase=True, dedupe=True @@ -880,6 +916,184 @@ def install_rpm_packages(self) -> bool: print(f"\n[FAIL] Error during installation: {e}") return False + def _find_repo_package(self) -> Path | None: + """Locate the built amdrocm-repo package in repo_package_dir. + + Globs in Python (no shell) and requires exactly one match for this + package type. Returns the resolved path, or None on error. + """ + directory = Path(self.repo_package_dir) + if not directory.is_dir(): + print( + f"[FAIL] --repo-package-dir is not a directory: {self.repo_package_dir}" + ) + return None + pattern = ( + "amdrocm-repo*.deb" if self.package_type == "deb" else "amdrocm-repo*.rpm" + ) + matches = sorted(directory.glob(pattern)) + if not matches: + print(f"[FAIL] No {pattern} found in {self.repo_package_dir}") + return None + if len(matches) > 1: + print( + f"[FAIL] Multiple {pattern} found in {self.repo_package_dir}: {matches}" + ) + return None + return matches[0].resolve() + + def install_repo_package(self) -> bool: + """Install the amdrocm-repo package, then refresh the repo it configures. + + The package itself is unsigned, so the local install skips package + signature checks; the repository the package configures keeps its own + gpgcheck setting. The metadata refresh is scoped to the amdrocm repo. + + Returns: + True if the package installed and the repo metadata refreshed. + """ + print("\n" + "=" * 80) + print("CONFIGURING REPOSITORY VIA amdrocm-repo PACKAGE") + print("=" * 80) + + pkg = self._find_repo_package() + if pkg is None: + return False + print(f"\nRepo package: {pkg}") + + if self.package_type == "deb": + install_cmd = ["sudo", "apt", "install", "-y", str(pkg)] + elif self._is_sles(): + install_cmd = [ + "zypper", + "--non-interactive", + "--no-gpg-checks", + "install", + "-y", + str(pkg), + ] + else: + install_cmd = ["dnf", "install", "-y", "--nogpgcheck", str(pkg)] + + print(f"\nInstalling repo package: {' '.join(install_cmd)}") + try: + rc = _run_streaming(install_cmd, REPO_PACKAGE_INSTALL_TIMEOUT_SEC) + except subprocess.TimeoutExpired: + print("[FAIL] Installing amdrocm-repo package timed out") + return False + except OSError as e: + print(f"[FAIL] Error installing amdrocm-repo package: {e}") + return False + if rc != 0: + print(f"[FAIL] Failed to install amdrocm-repo package (exit code: {rc})") + return False + print("[PASS] amdrocm-repo package installed") + + # Refresh metadata for the repo the package just configured. The dnf and + # zypper refreshes are scoped to the amdrocm repo; apt has no clean + # per-source refresh, so the deb path runs a repo-wide `apt update` + # (a failing unrelated base-image source would also fail this step). + if self.package_type == "deb": + refresh_cmd = ["sudo", "apt", "update"] + refresh_timeout = APT_UPDATE_TIMEOUT_SEC + elif self._is_sles(): + refresh_cmd = [ + "zypper", + "--non-interactive", + "--gpg-auto-import-keys", + "refresh", + AMDROCM_REPO_ID, + ] + refresh_timeout = ZYPP_REFRESH_TIMEOUT_SEC + else: + refresh_cmd = [ + "dnf", + "makecache", + "--disablerepo=*", + f"--enablerepo={AMDROCM_REPO_ID}", + ] + refresh_timeout = DNF_MAKECACHE_TIMEOUT_SEC + + print(f"\nRefreshing repository metadata: {' '.join(refresh_cmd)}") + for attempt in range(1, REPO_REFRESH_ATTEMPTS + 1): + try: + rc = _run_streaming(refresh_cmd, refresh_timeout) + except subprocess.TimeoutExpired: + rc = None + print("[WARN] Repository metadata refresh timed out") + except OSError as e: + rc = None + print(f"[WARN] Error refreshing repository metadata: {e}") + if rc == 0: + print("[PASS] Repository metadata refreshed") + return True + if attempt < REPO_REFRESH_ATTEMPTS: + time.sleep(REPO_REFRESH_BACKOFF_SEC * attempt) + print( + f"Retrying repository metadata refresh " + f"({attempt + 1}/{REPO_REFRESH_ATTEMPTS})..." + ) + print("[FAIL] Failed to refresh repository metadata") + return False + + def assert_repo_configured(self) -> bool: + """Verify the amdrocm-repo package dropped the expected repo config. + + Checks the repo file exists (and looks like a ROCm repo) and, for signed + release lines, that the signing key was installed too. + + Returns: + True if the on-disk repo configuration is present and well-formed. + """ + print("\n" + "=" * 80) + print("VERIFYING REPOSITORY CONFIGURATION") + print("=" * 80) + + signed = self.release_type in ("prerelease", "release") + ok = True + + if self.package_type == "deb": + repo_file = Path(AMDROCM_DEB_SOURCES) + key_file = Path(AMDROCM_DEB_KEYRING) + # deb822 marker (independent of the repo host, which need not contain + # "rocm" for a mirror). + marker = "URIs:" + else: + repo_dir = ZYPP_REPOS_DIR if self._is_sles() else YUM_REPOS_DIR + repo_file = Path(repo_dir) / AMDROCM_RPM_REPO_BASENAME + key_file = Path(AMDROCM_RPM_KEY) + marker = "baseurl=" + + if repo_file.is_file(): + content = repo_file.read_text(encoding="utf-8", errors="replace") + print(f"[PASS] Repo file present: {repo_file}") + print(content) + # Match the marker at the start of a line so a file that only + # mentions it in a comment is not accepted as configured. + if not any(line.startswith(marker) for line in content.splitlines()): + print( + f"[FAIL] Repo file missing expected {marker!r} entry: {repo_file}" + ) + ok = False + else: + print(f"[FAIL] Repo file not found: {repo_file}") + ok = False + + if signed: + if key_file.is_file(): + print(f"[PASS] Signing key present: {key_file}") + else: + print(f"[FAIL] Signing key not found (signed line): {key_file}") + ok = False + else: + print("[INFO] Unsigned line: no signing key expected") + + if ok: + print("\n[PASS] Repository configuration verified") + else: + print("\n[FAIL] Repository configuration verification failed") + return ok + def run_repo_setup_and_install(self) -> bool: """Step 1: Repo setup and install. Run for both sanity (basic) and full test. @@ -911,9 +1125,23 @@ def run_repo_setup_and_install(self) -> bool: print( "GPU Architecture(s): (none — generic packages amdrocm, amdrocm-core-sdk)" ) - print(f"Repository URL: {self.repo_url}") + print(f"Repository URL: {self.repo_url or '(from amdrocm-repo package)'}") print(f"Packages (in order): {self.package_names}") + # Install-via-package mode: the amdrocm-repo package configures the repo. + if self.repo_package_dir: + if not self.install_repo_package(): + return False + if not self.assert_repo_configured(): + return False + if self.repo_config_only: + print("\n[PASS] Repo configured via amdrocm-repo package (config-only)") + return True + if self.package_type == "deb": + return self.install_deb_packages() + return self.install_rpm_packages() + + # Default: configure the repo directly, then install. if self.package_type == "deb": if not self.setup_deb_repository(): return False @@ -936,6 +1164,12 @@ def run_basic_verification(self) -> bool: print("STEP 2: BASIC INSTALL VERIFICATION") print("=" * 80) + if not self.install_prefix: + # Reachable when a non-config-only run omits --install-prefix; report + # the missing option instead of failing inside Path(). + print("\n[FAIL] --install-prefix is required to verify an installation") + return False + install_path = Path(self.install_prefix) if not install_path.exists(): print(f"\n[FAIL] Installation directory not found: {self.install_prefix}") @@ -1612,6 +1846,20 @@ def _build_argument_parser(*, exit_on_error: bool = True) -> ArgumentParser: metavar="DIR", help="Directory containing .deb or .rpm files. Required when --test-type is 'simulate'.", ) + parser.add_argument( + "--repo-package-dir", + type=str, + metavar="DIR", + help="Directory containing the built amdrocm-repo .deb/.rpm. When set, the repository is " + "configured by installing that package (instead of writing repo files); --repo-url and " + "--gpg-key-url become optional. Not valid with --test-type simulate.", + ) + parser.add_argument( + "--repo-config-only", + action="store_true", + help="With --repo-package-dir: install the amdrocm-repo package, verify the repo config, " + "and stop (do not install ROCm). --install-prefix is not required.", + ) parser.add_argument( "--pkg-type", type=str, @@ -1623,6 +1871,8 @@ def _build_argument_parser(*, exit_on_error: bool = True) -> ArgumentParser: def _validate_cli_args(parser: ArgumentParser, args: Namespace) -> None: if args.test_type == "simulate": + if args.repo_package_dir: + parser.error("--repo-package-dir is not valid with --test-type 'simulate'") if not args.packages_dir: parser.error("--packages-dir is required when --test-type is 'simulate'") if not args.pkg_type and not args.os_profile: @@ -1635,13 +1885,17 @@ def _validate_cli_args(parser: ArgumentParser, args: Namespace) -> None: except ValueError as e: parser.error(str(e)) return + if args.repo_config_only and not args.repo_package_dir: + parser.error("--repo-config-only requires --repo-package-dir") if not args.os_profile: parser.error( "--os-profile is required when --test-type is 'install', 'sanity', or 'full'" ) - if not args.repo_url: + # In install-via-package mode the amdrocm-repo package supplies the repo URL. + if not args.repo_url and not args.repo_package_dir: parser.error( - "--repo-url is required when --test-type is 'install', 'sanity', or 'full'" + "--repo-url is required when --test-type is 'install', 'sanity', or 'full' " + "(or provide --repo-package-dir to configure the repo via the amdrocm-repo package)" ) if args.rocm_version: try: @@ -1706,7 +1960,10 @@ def run_tests(args: Namespace) -> int: print(f"OS Profile: {args.os_profile}") print(f"Package Type (derived): {derived_package_type}") print(f"Release Type: {args.release_type}") - print(f"Repository URL: {args.repo_url}") + if args.repo_package_dir: + print(f"Repo package dir: {args.repo_package_dir}") + print(f"Repo config only: {args.repo_config_only}") + print(f"Repository URL: {args.repo_url or '(from amdrocm-repo package)'}") # Preview package-name rules before NativeLinuxPackageInstallTest is constructed. _norm = normalize_target_list(args.gfx_arch, lowercase=True, dedupe=True) if _norm: @@ -1749,13 +2006,15 @@ def run_tests(args: Namespace) -> int: test_runner = NativeLinuxPackageInstallTest( os_profile=args.os_profile, - repo_url=args.repo_url, + repo_url=args.repo_url or "", release_type=args.release_type, install_prefix=args.install_prefix, gfx_arch=args.gfx_arch, rocm_version=args.rocm_version, gpg_key_url=args.gpg_key_url, build_variant=args.build_variant, + repo_package_dir=args.repo_package_dir, + repo_config_only=args.repo_config_only, ) print("\n" + "=" * 80) @@ -1770,6 +2029,12 @@ def run_tests(args: Namespace) -> int: if not test_runner.run_repo_setup_and_install(): print("\n[FAIL] Step 1 (repo setup and install) failed.") return 1 + if args.repo_config_only: + print("\n" + "=" * 80) + print("[PASS] INSTALLATION TEST PASSED") + print("(repo-config-only: amdrocm-repo package configured the repo)") + print("=" * 80 + "\n") + return 0 if args.test_type == "install": print("\n" + "=" * 80) print("[PASS] INSTALLATION TEST PASSED") @@ -1800,7 +2065,10 @@ def run_tests(args: Namespace) -> int: def _argv_from_ci_env() -> list[str] | None: """Build CLI argv from workflow/container env (see ``test_native_linux_packages_install.yml``). - Required for sanity/full: OS_PROFILE, REPO_URL, RELEASE_TYPE, INSTALL_PREFIX. + Required for sanity/full: OS_PROFILE, RELEASE_TYPE, plus REPO_URL and + INSTALL_PREFIX — except in package mode (REPO_PACKAGE_DIR set), where the + repo is configured by the installed package so REPO_URL is not needed, and + when REPO_CONFIG_ONLY is set INSTALL_PREFIX is not needed either. Optional: GFX_ARCH, GPG_KEY_URL; ``NATIVE_LINUX_INSTALL_ROCM_VERSION`` maps to ``--rocm-version`` only when versioned package names are needed (omit for unversioned installs). """ @@ -1835,8 +2103,22 @@ def _argv_from_ci_env() -> list[str] | None: gfx_arch = gfx_raw.replace(";", " ").split() if gfx_raw else [] release_type = (os.environ.get("RELEASE_TYPE") or "").strip() install_prefix = (os.environ.get("INSTALL_PREFIX") or "").strip() + repo_package_dir = (os.environ.get("REPO_PACKAGE_DIR") or "").strip() + repo_config_only = (os.environ.get("REPO_CONFIG_ONLY") or "").strip().lower() in ( + "1", + "true", + "yes", + "on", + ) - if not (os_profile and repo_url and release_type and install_prefix): + # In install-via-package mode the amdrocm-repo package supplies the repo URL; + # in config-only mode there is no install/verify, so INSTALL_PREFIX is not + # needed either. + if not (os_profile and release_type): + return None + if not repo_url and not repo_package_dir: + return None + if not install_prefix and not repo_config_only: return None argv = [ @@ -1844,13 +2126,17 @@ def _argv_from_ci_env() -> list[str] | None: test_type, "--os-profile", os_profile, - "--repo-url", - repo_url, "--release-type", release_type, - "--install-prefix", - install_prefix, ] + if repo_url: + argv.extend(["--repo-url", repo_url]) + if install_prefix: + argv.extend(["--install-prefix", install_prefix]) + if repo_package_dir: + argv.extend(["--repo-package-dir", repo_package_dir]) + if repo_config_only: + argv.append("--repo-config-only") if gfx_arch: argv.extend(["--gfx-arch", *gfx_arch]) rocm_version = (os.environ.get(ENV_NATIVE_LINUX_INSTALL_ROCM_VERSION) or "").strip() @@ -1874,11 +2160,14 @@ def test_native_linux_package_install() -> None: if os.environ.get("GITHUB_ACTIONS") == "true": pytest.fail( "Missing required environment variables for native install test " - "(expected OS_PROFILE, REPO_URL, RELEASE_TYPE, INSTALL_PREFIX; " - "optional GFX_ARCH, NATIVE_LINUX_INSTALL_ROCM_VERSION; or for simulate: PACKAGES_DIR)." + "(expected OS_PROFILE, RELEASE_TYPE, plus REPO_URL and INSTALL_PREFIX " + "unless REPO_PACKAGE_DIR is set, and INSTALL_PREFIX is not needed with " + "REPO_CONFIG_ONLY; optional GFX_ARCH, NATIVE_LINUX_INSTALL_ROCM_VERSION; " + "or for simulate: PACKAGES_DIR)." ) pytest.skip( - "Set workflow env vars (OS_PROFILE, REPO_URL, RELEASE_TYPE, INSTALL_PREFIX); " + "Set workflow env vars (OS_PROFILE, RELEASE_TYPE, plus REPO_URL/INSTALL_PREFIX " + "unless REPO_PACKAGE_DIR/REPO_CONFIG_ONLY is set); " "optional GFX_ARCH and NATIVE_LINUX_INSTALL_ROCM_VERSION." ) diff --git a/build_tools/packaging/linux/publish_repo_package.py b/build_tools/packaging/linux/publish_repo_package.py new file mode 100644 index 00000000000..ed00193345d --- /dev/null +++ b/build_tools/packaging/linux/publish_repo_package.py @@ -0,0 +1,127 @@ +#!/usr/bin/env python3 +# Copyright Advanced Micro Devices, Inc. +# SPDX-License-Identifier: MIT + +"""Publish a built amdrocm-repo package as a standalone per-distro file. + +The package is uploaded next to the native content packages, as a +``repo//`` sibling of the per-run ``packages//`` prefix, +rather than into the repository index. A client can then fetch it directly by +URL to configure the repository. The file becomes publicly reachable once the +release promotion step copies the per-run tree to the public CDN. + +The object name is fixed (``amdrocm-repo.``) so the download URL is stable +regardless of the built package's versioned filename. + +The destination is resolved through ``WorkflowOutputRoot``, the single source of +truth for CI path layout, so the bucket and per-run prefix come from the +workflow context rather than being passed in. That needs the CI environment: +``GITHUB_REPOSITORY``, ``RELEASE_TYPE``, and the event payload used for fork +detection. There is no artifacts bucket for the ``release`` line, so that +release type raises; the publishing job is expected to be gated off it. + +Usage: + python build_tools/packaging/linux/publish_repo_package.py \ + --file repo-package-out/amdrocm-repo-7.14.0-1.el10.noarch.rpm \ + --run-id 12345678901 \ + --os-profile rhel10 \ + --pkg-type rpm +""" + +import argparse +import re +import sys +from pathlib import Path + +_THIS_DIR = Path(__file__).resolve().parent +_BUILD_TOOLS_DIR = _THIS_DIR.parent.parent + +if str(_THIS_DIR) not in sys.path: + sys.path.insert(0, str(_THIS_DIR)) +if str(_BUILD_TOOLS_DIR) not in sys.path: + sys.path.insert(0, str(_BUILD_TOOLS_DIR)) + +from _therock_utils.storage_backend import create_storage_backend +from _therock_utils.workflow_outputs import WorkflowOutputRoot + +# An os-profile becomes a path segment in the object key, so restrict it to a +# safe set of characters (no slashes, whitespace, or control characters) and +# require a leading alphanumeric. The leading character matters: without it, +# "." and ".." satisfy the character class, and ".." resolves one directory +# above the intended location when the key is joined onto a local staging +# directory. \Z (not $) so a trailing newline is not accepted. +_OS_PROFILE_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*\Z") + +# A workflow run id is always numeric, and it becomes the leading path segment +# of the object key via WorkflowOutputRoot.prefix. Unconstrained, a value such +# as "../.." escapes the destination once the key is joined onto a local +# staging directory. \Z (not $) so a trailing newline is not accepted. +_RUN_ID_RE = re.compile(r"^[0-9]+\Z") + + +def parse_args(argv=None) -> argparse.Namespace: + p = argparse.ArgumentParser( + description="Publish an amdrocm-repo package to the per-run outputs.", + ) + p.add_argument("--file", required=True, type=Path, help="Built package to upload") + p.add_argument( + "--run-id", + required=True, + help="GitHub Actions workflow run ID, which selects the per-run prefix", + ) + p.add_argument( + "--os-profile", required=True, help="Target distro profile (path segment)" + ) + p.add_argument( + "--pkg-type", + required=True, + choices=["deb", "rpm"], + help="Package type, used as the published file extension", + ) + p.add_argument( + "--output-dir", + default=None, + type=Path, + help="Stage into this local directory instead of uploading to S3", + ) + p.add_argument( + "--dry-run", + action="store_true", + help="Log the destination without writing anything", + ) + args = p.parse_args(argv) + if not _RUN_ID_RE.match(args.run_id): + p.error(f"--run-id must be numeric: {args.run_id!r}") + if not _OS_PROFILE_RE.match(args.os_profile): + p.error(f"--os-profile has an unexpected format: {args.os_profile!r}") + if not args.file.is_file(): + p.error(f"--file is not a file: {args.file}") + return args + + +def publish(args: argparse.Namespace) -> str: + """Upload the file to its per-run location. Returns the object key.""" + root = WorkflowOutputRoot.from_workflow_run(run_id=args.run_id, platform="linux") + dest = root.native_linux_repo_package(args.pkg_type, args.os_profile) + backend = create_storage_backend(staging_dir=args.output_dir, dry_run=args.dry_run) + # Name the destination the run actually writes to. --output-dir stages to a + # local tree, so reporting an s3:// URI there would describe an upload that + # did not happen. The publishing job runs under continue-on-error, which + # makes this log the only signal that it did anything. + target = ( + dest.local_path(args.output_dir) if args.output_dir is not None else dest.s3_uri + ) + action = "[DRY RUN] Would publish" if args.dry_run else "Uploading" + print(f"{action} {args.file} -> {target}") + backend.upload_file(args.file, dest) + if not args.dry_run: + print(f"Published: {target}") + return dest.relative_path + + +def main(argv=None) -> None: + publish(parse_args(argv)) + + +if __name__ == "__main__": + main() diff --git a/build_tools/packaging/linux/tests/get_url_repo_params_test.py b/build_tools/packaging/linux/tests/get_url_repo_params_test.py index da2e8dd8404..7ab516812e2 100644 --- a/build_tools/packaging/linux/tests/get_url_repo_params_test.py +++ b/build_tools/packaging/linux/tests/get_url_repo_params_test.py @@ -35,10 +35,12 @@ python3.12 build_tools/packaging/linux/tests/get_url_repo_params_test.py -v """ +import json import os import sys import tempfile import unittest +from datetime import date from pathlib import Path from unittest.mock import patch @@ -420,5 +422,166 @@ def test_get_container_image_cli(self): ) +class GetPublicRepoBaseUrlTest(unittest.TestCase): + """Tests for get_public_repo_base_url() and its subcommand.""" + + def test_prerelease(self): + self.assertEqual( + get_url_repo_params.get_public_repo_base_url("prerelease"), + "https://rocm.prereleases.amd.com/packages-multi-arch", + ) + + def test_release(self): + self.assertEqual( + get_url_repo_params.get_public_repo_base_url("release"), + "https://repo.amd.com/rocm/packages-multi-arch", + ) + + def test_nightly(self): + self.assertEqual( + get_url_repo_params.get_public_repo_base_url("nightly"), + "https://rocm.nightlies.amd.com/packages-multi-arch", + ) + + def test_ci_and_dev_are_empty(self): + self.assertEqual(get_url_repo_params.get_public_repo_base_url("ci"), "") + self.assertEqual(get_url_repo_params.get_public_repo_base_url("dev"), "") + + def test_unknown_line_is_empty(self): + self.assertEqual(get_url_repo_params.get_public_repo_base_url("bogus"), "") + + def test_case_insensitive(self): + self.assertEqual( + get_url_repo_params.get_public_repo_base_url("PreRelease"), + "https://rocm.prereleases.amd.com/packages-multi-arch", + ) + + def test_subcommand_emits_url(self): + code, output = _run_main_with_output( + ["get-public-repo-base-url", "--release-type", "prerelease"] + ) + self.assertEqual(code, 0) + self.assertIn( + "repo_base_url=https://rocm.prereleases.amd.com/packages-multi-arch", + output, + ) + + def test_subcommand_emits_empty_for_ci(self): + code, output = _run_main_with_output( + ["get-public-repo-base-url", "--release-type", "ci"] + ) + self.assertEqual(code, 0) + self.assertIn("repo_base_url=", output) + self.assertNotIn("packages-multi-arch", output) + + +class NightlySubFolderTest(unittest.TestCase): + """Tests for nightly_sub_folder() and its subcommand.""" + + def test_formats_with_injected_date(self): + self.assertEqual( + get_url_repo_params.nightly_sub_folder("12345", today=date(2026, 7, 16)), + "20260716-12345", + ) + + def test_zero_padded_month_and_day(self): + self.assertEqual( + get_url_repo_params.nightly_sub_folder("7", today=date(2026, 1, 3)), + "20260103-7", + ) + + def test_rejects_non_numeric_run_id(self): + # The value is written to $GITHUB_OUTPUT, and gha_set_output uses a + # heredoc whose delimiter it does not verify, so an embedded newline + # could terminate it early and inject further step outputs. + for bad in ["1\nEOF_mag1c\ninjected=yes", "a/b", "12345\n", "", "abc"]: + with self.subTest(run_id=bad): + with self.assertRaises(ValueError): + get_url_repo_params.nightly_sub_folder(bad) + + def test_subcommand_writes_exactly_one_output_for_hostile_input(self): + code, output = _run_main_with_output( + [ + "get-nightly-sub-folder", + "--release-type", + "nightly", + "--run-id", + "1\nEOF_mag1c\ninjected=yes", + ] + ) + self.assertNotEqual(code, 0) + self.assertNotIn("injected=yes", output) + + def test_subcommand_emits_sub_folder(self): + code, output = _run_main_with_output( + ["get-nightly-sub-folder", "--run-id", "12345"] + ) + self.assertEqual(code, 0) + # The date is the current UTC date; assert the structure, not the value. + self.assertRegex(output, r"repo_sub_folder=\d{8}-12345") + + def test_subcommand_emits_sub_folder_for_nightly(self): + code, output = _run_main_with_output( + ["get-nightly-sub-folder", "--release-type", "nightly", "--run-id", "12345"] + ) + self.assertEqual(code, 0) + self.assertRegex(output, r"repo_sub_folder=\d{8}-12345") + + def test_subcommand_emits_empty_for_other_lines(self): + # Only the nightly line publishes into a dated sub-folder; the other + # lines publish at the repository root. + for release_type in ("release", "prerelease", "ci"): + with self.subTest(release_type=release_type): + code, output = _run_main_with_output( + [ + "get-nightly-sub-folder", + "--release-type", + release_type, + "--run-id", + "12345", + ] + ) + self.assertEqual(code, 0) + self.assertIn("repo_sub_folder=", output) + self.assertNotRegex(output, r"repo_sub_folder=\S") + + +class GetContainerImageMapTest(unittest.TestCase): + """Tests for get_container_image_map() and its subcommand.""" + + def test_maps_rpm_profiles(self): + self.assertEqual( + get_url_repo_params.get_container_image_map(["rhel8", "sles16"]), + { + "rhel8": "registry.access.redhat.com/ubi8/ubi:8.10", + "sles16": "registry.suse.com/bci/bci-base:16.0", + }, + ) + + def test_subcommand_emits_json_map(self): + code, output = _run_main_with_output( + ["get-container-image-map", "--os-profiles", '["ubuntu2404"]'] + ) + self.assertEqual(code, 0) + # Recover the JSON value written after ``images=``. + line = next(l for l in output.splitlines() if l.startswith("images=")) + images = json.loads(line[len("images=") :]) + self.assertEqual( + images, {"ubuntu2404": "ghcr.io/rocm/no_rocm_image_ubuntu24_04:latest"} + ) + + def test_subcommand_rejects_invalid_json(self): + code, _ = _run_main_with_output( + ["get-container-image-map", "--os-profiles", "not-json"] + ) + self.assertEqual(code, 1) + + def test_subcommand_rejects_non_list_json(self): + code, _ = _run_main_with_output( + ["get-container-image-map", "--os-profiles", '{"a": 1}'] + ) + self.assertEqual(code, 1) + + if __name__ == "__main__": unittest.main() diff --git a/build_tools/packaging/linux/tests/native_linux_package_install_ut_test.py b/build_tools/packaging/linux/tests/native_linux_package_install_ut_test.py index c2392f95304..3acc93e6b8e 100644 --- a/build_tools/packaging/linux/tests/native_linux_package_install_ut_test.py +++ b/build_tools/packaging/linux/tests/native_linux_package_install_ut_test.py @@ -215,6 +215,14 @@ def test_gpg_import_uses_no_shell(self): self.assertTrue(call.args, f"expected a positional argv, got {call}") self.assertIsInstance(call.args[0], list) + def test_package_keyring_is_separate_from_the_harness_keyring(self): + # The amdrocm-repo package ships its own keyring in a different + # directory; the two must not be conflated. + self.assertEqual( + self.mod.AMDROCM_DEB_KEYRING, "/usr/share/keyrings/amdrocm.gpg" + ) + self.assertNotEqual(self.mod.AMDROCM_DEB_KEYRING, self.mod.APT_KEYRING_FILE) + class NormalizeTestTypeTest(unittest.TestCase): """Tests for _normalize_test_type().""" @@ -993,6 +1001,8 @@ def _base_args(self, test_type: str): pkg_type=None, rocm_version=None, build_variant="", + repo_package_dir=None, + repo_config_only=False, ) @patch.object( @@ -2429,5 +2439,473 @@ def test_real_fixed_rpath_is_reported_but_not_fatal(self): self.assertIn("/opt/rocm/lib", output) +@contextlib.contextmanager +def _clear_ci_env(): + """Remove CI env vars that _argv_from_ci_env reads so tests start clean.""" + keys = [ + "TEST_TYPE", + "OS_PROFILE", + "REPO_URL", + "GFX_ARCH", + "RELEASE_TYPE", + "INSTALL_PREFIX", + "REPO_PACKAGE_DIR", + "REPO_CONFIG_ONLY", + "GPG_KEY_URL", + "PACKAGES_DIR", + "SIMULATE_PKG_TYPE", + native_linux_package_install_test.ENV_NATIVE_LINUX_INSTALL_ROCM_VERSION, + ] + saved = {k: os.environ.pop(k, None) for k in keys} + try: + yield + finally: + for k, v in saved.items(): + if v is not None: + os.environ[k] = v + + +class RepoPackageModeCliTest(unittest.TestCase): + """CLI/constructor behavior for the install-via-package mode.""" + + def _parse(self, argv): + return native_linux_package_install_test.parse_cli_arguments( + argv, raise_instead_of_exit=True + ) + + def test_repo_package_dir_makes_repo_url_optional(self): + args = self._parse( + [ + "--os-profile", + "ubuntu2404", + "--release-type", + "prerelease", + "--repo-package-dir", + "/pkgs", + "--install-prefix", + "/opt/rocm/core", + ] + ) + self.assertEqual(args.repo_package_dir, "/pkgs") + self.assertFalse(args.repo_config_only) + + def test_config_only_drops_repo_url_and_install_prefix(self): + args = self._parse( + [ + "--os-profile", + "ubuntu2404", + "--release-type", + "prerelease", + "--repo-package-dir", + "/pkgs", + "--repo-config-only", + ] + ) + self.assertTrue(args.repo_config_only) + self.assertIsNone(args.repo_url) + self.assertIsNone(args.install_prefix) + + def test_config_only_requires_repo_package_dir(self): + with self.assertRaises(ValueError): + self._parse( + [ + "--os-profile", + "ubuntu2404", + "--release-type", + "prerelease", + "--repo-config-only", + ] + ) + + def test_repo_url_still_required_without_repo_package_dir(self): + with self.assertRaises(ValueError): + self._parse(["--os-profile", "ubuntu2404", "--release-type", "prerelease"]) + + def test_repo_package_dir_rejected_with_simulate(self): + with self.assertRaises(ValueError): + self._parse( + [ + "--test-type", + "simulate", + "--packages-dir", + "/p", + "--os-profile", + "ubuntu2404", + "--repo-package-dir", + "/pkgs", + ] + ) + + def test_constructor_tolerates_none_repo_url(self): + runner = native_linux_package_install_test.NativeLinuxPackageInstallTest( + os_profile="rhel10", + repo_url=None, + release_type="nightly", + repo_package_dir="/pkgs", + repo_config_only=True, + ) + self.assertEqual(runner.repo_url, "") + self.assertEqual(runner.repo_package_dir, "/pkgs") + self.assertTrue(runner.repo_config_only) + + +class InstallRepoPackageTest(unittest.TestCase): + """Tests for install_repo_package() command construction (mocked subprocess).""" + + def _runner(self, os_profile, pkg_dir): + return native_linux_package_install_test.NativeLinuxPackageInstallTest( + os_profile=os_profile, + repo_url="", + release_type="prerelease", + repo_package_dir=str(pkg_dir), + ) + + @patch("native_linux_package_install_test._run_streaming") + def test_deb_installs_then_apt_update(self, mock_streaming): + mock_streaming.return_value = 0 + with tempfile.TemporaryDirectory() as d: + pkg = Path(d) / "amdrocm-repo_7.14.0_all.deb" + pkg.write_text("") + runner = self._runner("ubuntu2404", d) + with _suppress_script_output(): + self.assertTrue(runner.install_repo_package()) + install_cmd, refresh_cmd = (c.args[0] for c in mock_streaming.call_args_list) + self.assertEqual( + install_cmd, ["sudo", "apt", "install", "-y", str(pkg.resolve())] + ) + self.assertEqual(refresh_cmd, ["sudo", "apt", "update"]) + + @patch("native_linux_package_install_test._run_streaming") + def test_dnf_installs_nogpgcheck_then_scoped_makecache(self, mock_streaming): + mock_streaming.return_value = 0 + with tempfile.TemporaryDirectory() as d: + pkg = Path(d) / "amdrocm-repo-7.14.0-1.el10.noarch.rpm" + pkg.write_text("") + runner = self._runner("rhel10", d) + with _suppress_script_output(): + self.assertTrue(runner.install_repo_package()) + install_cmd, refresh_cmd = (c.args[0] for c in mock_streaming.call_args_list) + self.assertEqual( + install_cmd, ["dnf", "install", "-y", "--nogpgcheck", str(pkg.resolve())] + ) + self.assertEqual( + refresh_cmd, + ["dnf", "makecache", "--disablerepo=*", "--enablerepo=amdrocm"], + ) + + @patch("native_linux_package_install_test._run_streaming") + def test_sles_installs_no_gpg_checks_then_refresh(self, mock_streaming): + mock_streaming.return_value = 0 + with tempfile.TemporaryDirectory() as d: + pkg = Path(d) / "amdrocm-repo-7.14.0-1.noarch.rpm" + pkg.write_text("") + runner = self._runner("sles16", d) + with _suppress_script_output(): + self.assertTrue(runner.install_repo_package()) + install_cmd, refresh_cmd = (c.args[0] for c in mock_streaming.call_args_list) + self.assertEqual( + install_cmd, + [ + "zypper", + "--non-interactive", + "--no-gpg-checks", + "install", + "-y", + str(pkg.resolve()), + ], + ) + self.assertEqual( + refresh_cmd, + [ + "zypper", + "--non-interactive", + "--gpg-auto-import-keys", + "refresh", + "amdrocm", + ], + ) + + @patch("native_linux_package_install_test._run_streaming") + def test_missing_package_fails_without_running(self, mock_streaming): + with tempfile.TemporaryDirectory() as d: + runner = self._runner("ubuntu2404", d) # empty dir + with _suppress_script_output(): + self.assertFalse(runner.install_repo_package()) + mock_streaming.assert_not_called() + + @patch("native_linux_package_install_test._run_streaming") + def test_multiple_packages_fails_without_running(self, mock_streaming): + with tempfile.TemporaryDirectory() as d: + (Path(d) / "amdrocm-repo_1_all.deb").write_text("") + (Path(d) / "amdrocm-repo_2_all.deb").write_text("") + runner = self._runner("ubuntu2404", d) + with _suppress_script_output(): + self.assertFalse(runner.install_repo_package()) + mock_streaming.assert_not_called() + + @patch("native_linux_package_install_test._run_streaming") + def test_install_failure_returns_false_without_refresh(self, mock_streaming): + mock_streaming.return_value = 1 # install fails + with tempfile.TemporaryDirectory() as d: + (Path(d) / "amdrocm-repo_1_all.deb").write_text("") + runner = self._runner("ubuntu2404", d) + with _suppress_script_output(): + self.assertFalse(runner.install_repo_package()) + self.assertEqual(mock_streaming.call_count, 1) # refresh not attempted + + @patch("native_linux_package_install_test.time.sleep") + @patch("native_linux_package_install_test._run_streaming") + def test_refresh_retries_then_succeeds(self, mock_streaming, mock_sleep): + # install ok, refresh fails twice, then succeeds. The refresh talks to a + # live CDN, so a transient blip must not fail the run. + mock_streaming.side_effect = [0, 1, 1, 0] + with tempfile.TemporaryDirectory() as d: + (Path(d) / "amdrocm-repo_1_all.deb").write_text("") + runner = self._runner("ubuntu2404", d) + with _suppress_script_output(): + self.assertTrue(runner.install_repo_package()) + self.assertEqual(mock_streaming.call_count, 4) # 1 install + 3 refreshes + # Backoff grows with the attempt number. + self.assertEqual([c.args[0] for c in mock_sleep.call_args_list], [3, 6]) + + @patch("native_linux_package_install_test.time.sleep") + @patch("native_linux_package_install_test._run_streaming") + def test_refresh_gives_up_after_the_attempt_limit(self, mock_streaming, mock_sleep): + mock_streaming.side_effect = [0, 1, 1, 1] + with tempfile.TemporaryDirectory() as d: + (Path(d) / "amdrocm-repo_1_all.deb").write_text("") + runner = self._runner("ubuntu2404", d) + with _suppress_script_output(): + self.assertFalse(runner.install_repo_package()) + self.assertEqual(mock_streaming.call_count, 4) + # No sleep after the final attempt. + self.assertEqual(mock_sleep.call_count, 2) + + @patch("native_linux_package_install_test.time.sleep") + @patch("native_linux_package_install_test._run_streaming") + def test_refresh_retries_on_timeout(self, mock_streaming, mock_sleep): + # A timeout is retried like a non-zero exit, not propagated. + mock_streaming.side_effect = [ + 0, + subprocess.TimeoutExpired(cmd="apt update", timeout=1), + 0, + ] + with tempfile.TemporaryDirectory() as d: + (Path(d) / "amdrocm-repo_1_all.deb").write_text("") + runner = self._runner("ubuntu2404", d) + with _suppress_script_output(): + self.assertTrue(runner.install_repo_package()) + self.assertEqual(mock_streaming.call_count, 3) + + +class AssertRepoConfiguredTest(unittest.TestCase): + """Tests for assert_repo_configured() (on-disk contract check).""" + + def _runner(self, os_profile, release_type, pkg_dir="/pkgs"): + return native_linux_package_install_test.NativeLinuxPackageInstallTest( + os_profile=os_profile, + repo_url="", + release_type=release_type, + repo_package_dir=pkg_dir, + ) + + def test_deb_signed_requires_sources_and_keyring(self): + with tempfile.TemporaryDirectory() as d: + sources = Path(d) / "amdrocm.sources" + sources.write_text( + "Types: deb\n" + "URIs: https://rocm.prereleases.amd.com/packages-multi-arch/ubuntu2404/\n" + "Signed-By: /usr/share/keyrings/amdrocm.gpg\n" + ) + keyring = Path(d) / "amdrocm.gpg" + keyring.write_text("key") + runner = self._runner("ubuntu2404", "prerelease") + with patch.multiple( + native_linux_package_install_test, + AMDROCM_DEB_SOURCES=str(sources), + AMDROCM_DEB_KEYRING=str(keyring), + ): + with _suppress_script_output(): + self.assertTrue(runner.assert_repo_configured()) + keyring.unlink() # signed line but keyring missing -> fail + with _suppress_script_output(): + self.assertFalse(runner.assert_repo_configured()) + + def test_deb_missing_sources_fails(self): + runner = self._runner("ubuntu2404", "prerelease") + with patch.multiple( + native_linux_package_install_test, + AMDROCM_DEB_SOURCES="/nonexistent/amdrocm.sources", + AMDROCM_DEB_KEYRING="/nonexistent/amdrocm.gpg", + ): + with _suppress_script_output(): + self.assertFalse(runner.assert_repo_configured()) + + def test_deb_nightly_needs_no_keyring(self): + with tempfile.TemporaryDirectory() as d: + sources = Path(d) / "amdrocm.sources" + sources.write_text("URIs: https://x/rocm/deb/\nTrusted: yes\n") + runner = self._runner("ubuntu2404", "nightly") + with patch.multiple( + native_linux_package_install_test, + AMDROCM_DEB_SOURCES=str(sources), + AMDROCM_DEB_KEYRING="/nonexistent/amdrocm.gpg", + ): + with _suppress_script_output(): + self.assertTrue(runner.assert_repo_configured()) + + def test_structurally_invalid_repo_file_fails(self): + # A repo file missing the expected deb822 marker (URIs:) is rejected, + # independent of whether the host string contains "rocm". + with tempfile.TemporaryDirectory() as d: + sources = Path(d) / "amdrocm.sources" + sources.write_text("not a valid deb822 sources file\n") + runner = self._runner("ubuntu2404", "nightly") + with patch.multiple( + native_linux_package_install_test, + AMDROCM_DEB_SOURCES=str(sources), + AMDROCM_DEB_KEYRING="/nonexistent/amdrocm.gpg", + ): + with _suppress_script_output(): + self.assertFalse(runner.assert_repo_configured()) + + def test_mirror_host_without_rocm_still_passes(self): + # deb822 with a rocm-less mirror host is valid: the marker check must + # not reject it just because the URL lacks "rocm". + with tempfile.TemporaryDirectory() as d: + sources = Path(d) / "amdrocm.sources" + sources.write_text( + "Types: deb\n" + "URIs: https://mirror.example.com/packages-multi-arch/deb/20260716-1/\n" + "Suites: stable\n" + "Trusted: yes\n" + ) + runner = self._runner("ubuntu2404", "nightly") + with patch.multiple( + native_linux_package_install_test, + AMDROCM_DEB_SOURCES=str(sources), + AMDROCM_DEB_KEYRING="/nonexistent/amdrocm.gpg", + ): + with _suppress_script_output(): + self.assertTrue(runner.assert_repo_configured()) + + def test_rpm_signed_checks_yum_dir_and_key(self): + with tempfile.TemporaryDirectory() as d: + repo = Path(d) / "amdrocm.repo" + repo.write_text( + "[amdrocm]\nbaseurl=https://x/packages-multi-arch/rpm/x86_64/\n" + ) + key = Path(d) / "RPM-GPG-KEY-amdrocm" + key.write_text("key") + runner = self._runner("rhel10", "prerelease") + with patch.multiple( + native_linux_package_install_test, + YUM_REPOS_DIR=d, + AMDROCM_RPM_KEY=str(key), + ): + with _suppress_script_output(): + self.assertTrue(runner.assert_repo_configured()) + + def test_rpm_sles_uses_zypp_dir(self): + with tempfile.TemporaryDirectory() as d: + repo = Path(d) / "amdrocm.repo" + repo.write_text("[amdrocm]\nbaseurl=https://x/rocm/rpm/x86_64/\n") + key = Path(d) / "RPM-GPG-KEY-amdrocm" + key.write_text("key") + runner = self._runner("sles16", "prerelease") + with patch.multiple( + native_linux_package_install_test, + ZYPP_REPOS_DIR=d, + AMDROCM_RPM_KEY=str(key), + ): + with _suppress_script_output(): + self.assertTrue(runner.assert_repo_configured()) + + +class RunRepoSetupDispatchTest(unittest.TestCase): + """run_repo_setup_and_install dispatch: package mode vs config-only vs default.""" + + def _runner(self, **kw): + base = dict(os_profile="ubuntu2404", repo_url="", release_type="prerelease") + base.update(kw) + return native_linux_package_install_test.NativeLinuxPackageInstallTest(**base) + + def test_config_only_stops_before_rocm_install(self): + runner = self._runner(repo_package_dir="/pkgs", repo_config_only=True) + runner.install_repo_package = MagicMock(return_value=True) + runner.assert_repo_configured = MagicMock(return_value=True) + runner.install_deb_packages = MagicMock(return_value=True) + with _suppress_script_output(): + self.assertTrue(runner.run_repo_setup_and_install()) + runner.install_repo_package.assert_called_once() + runner.assert_repo_configured.assert_called_once() + runner.install_deb_packages.assert_not_called() + + def test_package_mode_installs_rocm_when_not_config_only(self): + runner = self._runner(repo_package_dir="/pkgs", repo_config_only=False) + runner.install_repo_package = MagicMock(return_value=True) + runner.assert_repo_configured = MagicMock(return_value=True) + runner.install_deb_packages = MagicMock(return_value=True) + with _suppress_script_output(): + self.assertTrue(runner.run_repo_setup_and_install()) + runner.install_deb_packages.assert_called_once() + + def test_failed_assert_short_circuits(self): + runner = self._runner(repo_package_dir="/pkgs", repo_config_only=False) + runner.install_repo_package = MagicMock(return_value=True) + runner.assert_repo_configured = MagicMock(return_value=False) + runner.install_deb_packages = MagicMock(return_value=True) + with _suppress_script_output(): + self.assertFalse(runner.run_repo_setup_and_install()) + runner.install_deb_packages.assert_not_called() + + def test_default_path_uses_manual_setup(self): + runner = self._runner() # no repo_package_dir + runner.install_repo_package = MagicMock(return_value=True) + runner.setup_deb_repository = MagicMock(return_value=True) + runner.install_deb_packages = MagicMock(return_value=True) + with _suppress_script_output(): + self.assertTrue(runner.run_repo_setup_and_install()) + runner.setup_deb_repository.assert_called_once() + runner.install_repo_package.assert_not_called() + + +class ArgvFromCiEnvRepoPackageTest(unittest.TestCase): + """_argv_from_ci_env: install-via-package env mapping and relaxed requirements.""" + + def test_config_only_lane_needs_no_repo_url_or_install_prefix(self): + env = { + "TEST_TYPE": "sanity", + "OS_PROFILE": "ubuntu2404", + "RELEASE_TYPE": "prerelease", + "REPO_PACKAGE_DIR": "/pkgs", + "REPO_CONFIG_ONLY": "true", + } + with _clear_ci_env(), patch.dict(os.environ, env, clear=False): + argv = native_linux_package_install_test._argv_from_ci_env() + self.assertIsNotNone(argv) + self.assertIn("--repo-package-dir", argv) + self.assertEqual(argv[argv.index("--repo-package-dir") + 1], "/pkgs") + self.assertIn("--repo-config-only", argv) + self.assertNotIn("--repo-url", argv) + self.assertNotIn("--install-prefix", argv) + + def test_package_mode_with_install_prefix_no_repo_url(self): + env = { + "TEST_TYPE": "sanity", + "OS_PROFILE": "rhel10", + "RELEASE_TYPE": "prerelease", + "REPO_PACKAGE_DIR": "/pkgs", + "INSTALL_PREFIX": "/opt/rocm/core", + } + with _clear_ci_env(), patch.dict(os.environ, env, clear=False): + argv = native_linux_package_install_test._argv_from_ci_env() + self.assertIsNotNone(argv) + self.assertIn("--repo-package-dir", argv) + self.assertIn("--install-prefix", argv) + self.assertNotIn("--repo-url", argv) + self.assertNotIn("--repo-config-only", argv) + + if __name__ == "__main__": unittest.main() diff --git a/build_tools/tests/publish_repo_package_test.py b/build_tools/tests/publish_repo_package_test.py new file mode 100644 index 00000000000..ac8d8239a63 --- /dev/null +++ b/build_tools/tests/publish_repo_package_test.py @@ -0,0 +1,280 @@ +# Copyright Advanced Micro Devices, Inc. +# SPDX-License-Identifier: MIT + +"""Unit tests for publish_repo_package.py. + +publish_repo_package.py lives in build_tools/packaging/linux/, which is not on +the path that conftest.py sets up (it only adds build_tools/). Add it +explicitly. + +These tests use a real ``LocalStorageBackend`` via ``--output-dir`` rather than +mocking an S3 client, so the assertions are about bytes on disk at a real path. +Only bucket resolution is stubbed, since that reads the CI environment. +""" + +import sys +from pathlib import Path +from unittest import mock + +import pytest + +_LINUX_DIR = Path(__file__).resolve().parents[1] / "packaging" / "linux" +sys.path.insert(0, str(_LINUX_DIR)) + +import publish_repo_package as prp # noqa: E402 + +RUN_ID = "12345" +BUCKET = "therock-prerelease-artifacts" + +# The key the destination method is expected to produce, spelled out here so a +# change to the published layout has to be made deliberately in two places. +EXPECTED_RPM_KEY = "12345-linux/packages/rpm/repo/rhel10/amdrocm-repo.rpm" + + +def _argv(tmp_path, **overrides): + pkg = tmp_path / "amdrocm-repo-7.14.0-1.el10.noarch.rpm" + if not pkg.exists(): + pkg.write_bytes(b"PKG-CONTENTS") + argv = [ + "--file", + str(overrides.get("file", pkg)), + "--run-id", + overrides.get("run_id", RUN_ID), + "--os-profile", + overrides.get("os_profile", "rhel10"), + "--pkg-type", + overrides.get("pkg_type", "rpm"), + ] + if "output_dir" in overrides: + argv += ["--output-dir", str(overrides["output_dir"])] + if overrides.get("dry_run"): + argv.append("--dry-run") + return argv + + +def _stub_bucket(monkeypatch, external_repo=""): + """Stub bucket resolution, which otherwise reads the CI environment.""" + monkeypatch.setattr( + "_therock_utils.workflow_outputs._retrieve_bucket_info", + lambda **kwargs: (external_repo, BUCKET), + ) + + +# --- CLI validation ----------------------------------------------------------- + + +def test_parse_args_ok(tmp_path): + args = prp.parse_args(_argv(tmp_path)) + assert args.run_id == RUN_ID + assert args.pkg_type == "rpm" + assert args.output_dir is None + assert args.dry_run is False + + +def test_parse_args_rejects_missing_file(tmp_path): + with pytest.raises(SystemExit): + prp.parse_args(_argv(tmp_path, file=str(tmp_path / "nope.rpm"))) + + +def test_parse_args_rejects_bad_pkg_type(tmp_path): + with pytest.raises(SystemExit): + prp.parse_args(_argv(tmp_path, pkg_type="tar")) + + +@pytest.mark.parametrize( + "bad", + [ + "rhel10/../x", + "a/b", + "rhel 10", + "rhel10\n", + "", + # Dot-only and leading-dot forms pass a bare [A-Za-z0-9._-]+ character + # class. ".." in particular escapes the intended directory once the key + # is joined onto a local staging tree. + ".", + "..", + "...", + ".hidden", + ], +) +def test_parse_args_rejects_bad_os_profile(tmp_path, bad): + # os_profile becomes an object-key path segment; reject slashes, whitespace + # and anything that is not anchored on an alphanumeric. + with pytest.raises(SystemExit): + prp.parse_args(_argv(tmp_path, os_profile=bad)) + + +@pytest.mark.parametrize("good", ["ubuntu2404", "rhel8", "rhel10", "sles16"]) +def test_parse_args_accepts_real_os_profiles(tmp_path, good): + # Guards the tightened pattern against over-rejection. + assert prp.parse_args(_argv(tmp_path, os_profile=good)).os_profile == good + + +@pytest.mark.parametrize( + "bad", + [ + # run_id is the leading path segment of the object key, so "../.." + # escapes the destination once the key is joined onto a staging dir. + "../../../../etc", + "a/b", + "12345\n", + "12 345", + "", + "abc", + ], +) +def test_parse_args_rejects_non_numeric_run_id(tmp_path, bad): + with pytest.raises(SystemExit): + prp.parse_args(_argv(tmp_path, run_id=bad)) + + +def test_parse_args_accepts_a_numeric_run_id(tmp_path): + assert prp.parse_args(_argv(tmp_path, run_id="12345678901")).run_id == "12345678901" + + +# --- publish via a real local backend ----------------------------------------- + + +def test_publish_writes_file_at_expected_key(tmp_path, monkeypatch): + _stub_bucket(monkeypatch) + staging = tmp_path / "staging" + args = prp.parse_args(_argv(tmp_path, output_dir=staging)) + + key = prp.publish(args) + + assert key == EXPECTED_RPM_KEY + # Build the expected path from the key string, not by joining Path + # segments, so a separator bug on Windows is visible rather than masked. + written = staging / EXPECTED_RPM_KEY + assert written.is_file() + assert written.read_bytes() == args.file.read_bytes() + + +@pytest.mark.parametrize( + "pkg_type,os_profile", + [("deb", "ubuntu2404"), ("rpm", "rhel8"), ("rpm", "rhel10"), ("rpm", "sles16")], +) +def test_publish_key_per_profile(tmp_path, monkeypatch, pkg_type, os_profile): + _stub_bucket(monkeypatch) + staging = tmp_path / "staging" + args = prp.parse_args( + _argv(tmp_path, output_dir=staging, pkg_type=pkg_type, os_profile=os_profile) + ) + + key = prp.publish(args) + + expected = ( + f"{RUN_ID}-linux/packages/{pkg_type}/repo/{os_profile}" + f"/amdrocm-repo.{pkg_type}" + ) + assert key == expected + assert (staging / expected).is_file() + + +def test_publish_is_outside_the_repository_index(tmp_path, monkeypatch): + # The bootstrap file is fetched by URL and must not be swept into the + # content index, or the identically-named per-profile rpms would collide. + _stub_bucket(monkeypatch) + staging = tmp_path / "staging" + args = prp.parse_args(_argv(tmp_path, output_dir=staging)) + + key = prp.publish(args) + + assert "/repo/" in key + assert "/x86_64/" not in key + assert "/dists/" not in key + + +def test_publish_carries_external_repo_prefix(tmp_path, monkeypatch): + # external_repo is non-empty only for 'ci' from a fork or a non-TheRock + # repository; the key must carry it so forks do not write to the shared + # per-run prefix. + _stub_bucket(monkeypatch, external_repo="Fork-TheRock/") + staging = tmp_path / "staging" + args = prp.parse_args(_argv(tmp_path, output_dir=staging)) + + key = prp.publish(args) + + assert key == f"Fork-TheRock/{EXPECTED_RPM_KEY}" + assert (staging / key).is_file() + + +def test_publish_dry_run_writes_nothing(tmp_path, monkeypatch): + _stub_bucket(monkeypatch) + staging = tmp_path / "staging" + args = prp.parse_args(_argv(tmp_path, output_dir=staging, dry_run=True)) + + key = prp.publish(args) + + assert key == EXPECTED_RPM_KEY + assert not (staging / EXPECTED_RPM_KEY).exists() + + +def test_publish_dry_run_does_not_claim_to_have_published( + tmp_path, monkeypatch, capsys +): + # The publishing job runs under continue-on-error, so its log is the only + # signal. A dry run that prints "Published:" reports an upload that never + # happened. + _stub_bucket(monkeypatch) + args = prp.parse_args( + _argv(tmp_path, output_dir=tmp_path / "staging", dry_run=True) + ) + + prp.publish(args) + + out = capsys.readouterr().out + assert "Published:" not in out + assert "DRY RUN" in out + + +def test_publish_reports_the_local_path_when_staging(tmp_path, monkeypatch, capsys): + # With --output-dir nothing reaches S3, so an s3:// URI in the log would + # describe an upload that did not occur. + _stub_bucket(monkeypatch) + staging = tmp_path / "staging" + args = prp.parse_args(_argv(tmp_path, output_dir=staging)) + + prp.publish(args) + + out = capsys.readouterr().out + assert "s3://" not in out + assert str(staging / EXPECTED_RPM_KEY) in out + + +def test_publish_reports_the_s3_uri_when_uploading(tmp_path, monkeypatch, capsys): + _stub_bucket(monkeypatch) + args = prp.parse_args(_argv(tmp_path, dry_run=True)) + + with mock.patch("publish_repo_package.create_storage_backend"): + prp.publish(args) + + assert f"s3://{BUCKET}/{EXPECTED_RPM_KEY}" in capsys.readouterr().out + + +def test_publish_without_output_dir_selects_s3_backend(tmp_path, monkeypatch): + # Omitting --output-dir means a real S3 upload. Pin that, so a test that + # forgets the flag fails loudly here rather than reaching the network. + _stub_bucket(monkeypatch) + args = prp.parse_args(_argv(tmp_path, dry_run=True)) + assert args.output_dir is None + + with mock.patch("publish_repo_package.create_storage_backend") as factory: + prp.publish(args) + + factory.assert_called_once() + assert factory.call_args.kwargs["staging_dir"] is None + assert factory.call_args.kwargs["dry_run"] is True + + +def test_publish_rejects_the_release_line(tmp_path, monkeypatch): + # There is no artifacts bucket for the release line: every + # therock-release-* bucket has iam_role=None because release upload is + # external. The publishing job is gated off it; this is the backstop. + monkeypatch.setenv("RELEASE_TYPE", "release") + monkeypatch.setenv("GITHUB_REPOSITORY", "ROCm/TheRock") + args = prp.parse_args(_argv(tmp_path, output_dir=tmp_path / "staging")) + + with pytest.raises(ValueError, match="release_type='release' is invalid"): + prp.publish(args) diff --git a/build_tools/tests/workflow_outputs_test.py b/build_tools/tests/workflow_outputs_test.py index 9a5bc26d845..989a63b6769 100644 --- a/build_tools/tests/workflow_outputs_test.py +++ b/build_tools/tests/workflow_outputs_test.py @@ -164,6 +164,56 @@ def test_manifest(self): "99999-linux/manifests/gfx94X-dcgpu/therock_manifest.json", ) + # -- Native packages -- + + def test_native_linux_packages(self): + loc = self.root.native_linux_packages("deb") + self._assert_relative_path(loc, "99999-linux/packages/deb") + + def test_native_linux_repo_package(self): + loc = self.root.native_linux_repo_package("deb", "ubuntu2404") + self._assert_relative_path( + loc, + "99999-linux/packages/deb/repo/ubuntu2404/amdrocm-repo.deb", + ) + + def test_native_linux_repo_package_all_profiles(self): + cases = [ + ("deb", "ubuntu2404"), + ("rpm", "rhel8"), + ("rpm", "rhel10"), + ("rpm", "sles16"), + ] + for pkg_type, os_profile in cases: + with self.subTest(pkg_type=pkg_type, os_profile=os_profile): + loc = self.root.native_linux_repo_package(pkg_type, os_profile) + self._assert_relative_path( + loc, + f"99999-linux/packages/{pkg_type}/repo/{os_profile}" + f"/amdrocm-repo.{pkg_type}", + ) + + def test_repo_package_sits_outside_the_package_index(self): + # The bootstrap file is fetched by URL, so it must not land inside the + # repository index: deb indexes dists/ + pool/, rpm indexes x86_64/. + # One amdrocm-repo is built per OS profile and several share a + # filename, so indexing them together would collide. + for pkg_type, index_dirs in [ + ("deb", ("dists", "pool")), + ("rpm", ("x86_64",)), + ]: + with self.subTest(pkg_type=pkg_type): + packages = self.root.native_linux_packages(pkg_type) + repo_pkg = self.root.native_linux_repo_package(pkg_type, "ubuntu2404") + # Shares the per-run package prefix ... + self.assertTrue( + repo_pkg.relative_path.startswith(packages.relative_path + "/") + ) + # ... but never sits under an index directory. + tail = repo_pkg.relative_path[len(packages.relative_path) + 1 :] + for index_dir in index_dirs: + self.assertFalse(tail.startswith(index_dir + "/")) + # -- Python packages -- def test_python_packages(self): @@ -222,6 +272,24 @@ def test_log_dir_with_external_repo(self): "Fork-TheRock/12345-linux/logs/gfx94X-dcgpu", ) + def test_native_linux_repo_package_with_external_repo(self): + # external_repo is only ever non-empty for the 'ci' release type from a + # fork or a non-ROCm/TheRock repository, which is the only case that + # resolves to therock-ci-artifacts-external. The signed lines + # (prerelease, release) and nightly resolve to their own buckets, so + # their external_repo is "" and the key below has no leading segment. + root = WorkflowOutputRoot( + bucket="therock-ci-artifacts-external", + external_repo="Fork-TheRock/", + run_id="12345", + platform="linux", + ) + loc = root.native_linux_repo_package("rpm", "rhel10") + self.assertEqual( + loc.relative_path, + "Fork-TheRock/12345-linux/packages/rpm/repo/rhel10/amdrocm-repo.rpm", + ) + # --------------------------------------------------------------------------- # WorkflowOutputRoot — end-to-end (s3_uri, https_url, local_path via StorageLocation) diff --git a/docs/development/workflow_outputs.md b/docs/development/workflow_outputs.md index 94f5815000a..eb1074b1eef 100644 --- a/docs/development/workflow_outputs.md +++ b/docs/development/workflow_outputs.md @@ -127,6 +127,8 @@ families in parallel, producing identically-named log files (e.g., main/binary-amd64/ Packages Packages.gz + repo/{os_profile}/ + amdrocm-repo.deb (bootstrap package, not indexed) packages/rpm/ (DNF/Zypper repository) x86_64/ @@ -135,6 +137,8 @@ families in parallel, producing identically-named log files (e.g., repomd.xml primary.xml.gz ... + repo/{os_profile}/ + amdrocm-repo.rpm (bootstrap package, not indexed) python/ *.whl (generic wheels, e.g., rocm_sdk_core) @@ -299,6 +303,7 @@ To add a new output type: | ------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------- | | [`post_build_upload.py`](/build_tools/github_actions/post_build_upload.py) | `WorkflowOutputRoot` + `StorageBackend` for artifacts, logs, manifests | | [`upload_package_repo.py`](/build_tools/packaging/linux/upload_package_repo.py) | `WorkflowOutputRoot.native_linux_packages()` for deb/rpm package repositories | +| [`publish_repo_package.py`](/build_tools/packaging/linux/publish_repo_package.py) | `WorkflowOutputRoot.native_linux_repo_package()` for the amdrocm-repo package | | [`post_stage_upload.py`](/build_tools/github_actions/post_stage_upload.py) | `WorkflowOutputRoot` + `StorageBackend` for multi-arch stage logs | | [`upload_tarballs.py`](/build_tools/github_actions/upload_tarballs.py) | `WorkflowOutputRoot` + `StorageBackend` for tarballs | | [`upload_python_packages.py`](/build_tools/github_actions/upload_python_packages.py) | `WorkflowOutputRoot` + `StorageBackend` for Python wheels and index | From 58a3688f283ab546e6c03db2978e0e69ab026bd7 Mon Sep 17 00:00:00 2001 From: Lalit Sudarshan Date: Wed, 12 Aug 2026 16:04:22 -0400 Subject: [PATCH 2/5] test(packaging): add test coverage for amdrocm-repo promotion to release buckets --- .../publish_rocm_to_release_buckets_test.py | 105 +++++++++++++++++- build_tools/tests/storage_backend_test.py | 43 +++++++ 2 files changed, 147 insertions(+), 1 deletion(-) diff --git a/build_tools/github_actions/tests/publish_rocm_to_release_buckets_test.py b/build_tools/github_actions/tests/publish_rocm_to_release_buckets_test.py index 30532967c6e..0fdd9ec21cf 100644 --- a/build_tools/github_actions/tests/publish_rocm_to_release_buckets_test.py +++ b/build_tools/github_actions/tests/publish_rocm_to_release_buckets_test.py @@ -1,16 +1,25 @@ #!/usr/bin/env python """Unit tests for publish_rocm_to_release_buckets.py.""" +import datetime import os +import shutil import sys +import tempfile import unittest from pathlib import Path from unittest import mock sys.path.insert(0, os.fspath(Path(__file__).parent.parent.parent)) -from github_actions.publish_rocm_to_release_buckets import main +from github_actions.publish_rocm_to_release_buckets import ( + main, + publish_native_linux_packages, +) +from _therock_utils.s3_buckets import get_release_bucket_config +from _therock_utils.storage_backend import LocalStorageBackend from _therock_utils.storage_location import StorageLocation +from _therock_utils.workflow_outputs import WorkflowOutputRoot class TestPublishRocmToReleaseBuckets(unittest.TestCase): @@ -375,5 +384,99 @@ def test_asan_native_packages_use_separate_path(self, mock_copy): ) +class TestRepoPackagePromotion(unittest.TestCase): + """The amdrocm-repo bootstrap package is carried by the existing copy. + + ``native_linux_repo_package()`` puts the bootstrap package under + ``packages/{pkg_type}/repo/{os_profile}/``, which is inside the prefix + ``publish_native_linux_packages`` already copies. Nothing declares that + relationship, so it holds only as long as the copy stays recursive and the + two prefixes stay nested. + + These tests drive a real ``LocalStorageBackend`` and assert files on disk, + unlike the rest of this module, which mocks ``copy_directory`` and checks + the locations passed to it. Mocking the copy cannot show whether the + bootstrap package is actually transported. + """ + + RUN_ID = "12345" + PROFILES = ("ubuntu2404", "rhel10") + + def _seed(self, root, backend, staging): + """Write a repository tree plus one bootstrap package per profile.""" + for pkg_type in ("deb", "rpm"): + packages = root.native_linux_packages(pkg_type) + # Layout mirrors upload_package_repo.py: APT uses pool/ + dists/, + # rpm puts packages under x86_64/ with repodata/ alongside. + contents = ( + ["pool/main/r/rocm/rocm_7.14.0_amd64.deb", "dists/stable/Release"] + if pkg_type == "deb" + else ["x86_64/rocm-7.14.0.x86_64.rpm", "x86_64/repodata/repomd.xml"] + ) + for rel in contents: + self._write( + StorageLocation(packages.bucket, f"{packages.relative_path}/{rel}"), + staging, + ) + for profile in self.PROFILES: + self._write(root.native_linux_repo_package(pkg_type, profile), staging) + + @staticmethod + def _write(location, staging): + path = location.local_path(staging) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(b"CONTENTS") + + def _promote(self, release_type): + """Run the real promotion into a temp tree; return the staging dir.""" + staging = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, staging, ignore_errors=True) + root = WorkflowOutputRoot( + get_release_bucket_config(release_type, "packages").name, + "", + self.RUN_ID, + "linux", + ) + backend = LocalStorageBackend(staging_dir=staging) + self._seed(root, backend, staging) + publish_native_linux_packages(root, release_type, backend) + return staging + + def test_prerelease_carries_every_profile_package(self): + staging = self._promote("prerelease") + + for pkg_type in ("deb", "rpm"): + for profile in self.PROFILES: + promoted = ( + staging + / f"v4/packages/{pkg_type}/repo/{profile}/amdrocm-repo.{pkg_type}" + ) + self.assertTrue(promoted.is_file(), f"missing {promoted}") + + def test_prerelease_carries_the_repository_alongside(self): + # A test that only checked the bootstrap package would still pass if the + # copy had silently stopped carrying the repository itself. + staging = self._promote("prerelease") + + for rel in ( + "v4/packages/deb/pool/main/r/rocm/rocm_7.14.0_amd64.deb", + "v4/packages/deb/dists/stable/Release", + "v4/packages/rpm/x86_64/rocm-7.14.0.x86_64.rpm", + "v4/packages/rpm/x86_64/repodata/repomd.xml", + ): + self.assertTrue((staging / rel).is_file(), f"missing {rel}") + + def test_dated_lines_carry_the_package_too(self): + # dev and nightly promote under {date}-{run_id} rather than a fixed + # prefix, so the destination shape differs and is worth covering. The + # date is computed the same way the module computes it rather than + # hardcoded. + staging = self._promote("dev") + + dated = f"{datetime.date.today().strftime('%Y%m%d')}-{self.RUN_ID}" + promoted = staging / f"v4/deb/{dated}/repo/ubuntu2404/amdrocm-repo.deb" + self.assertTrue(promoted.is_file(), f"missing {promoted}") + + if __name__ == "__main__": unittest.main() diff --git a/build_tools/tests/storage_backend_test.py b/build_tools/tests/storage_backend_test.py index 0e7a60c2a6e..0890abd0edb 100644 --- a/build_tools/tests/storage_backend_test.py +++ b/build_tools/tests/storage_backend_test.py @@ -846,6 +846,49 @@ def test_multiple_pages(self): self.assertEqual(len(files), 2) + def test_lists_keys_nested_below_the_prefix(self): + """Keys below the prefix survive the include filtering. + + Callers rely on listing being recursive. publish_native_linux_packages + copies a whole package repository with one copy_directory call, and the + amdrocm-repo bootstrap package sits two levels down under + repo//. + + This covers the half of that guarantee that lives in this method, which + is that a nested key is returned rather than skipped. The other half is + that the request itself is recursive, and the paginator is mocked here + so this test cannot see it. test_paginate_passes_no_delimiter covers it. + """ + pages = [ + { + "Contents": [ + {"Key": "run-1/packages/deb/pool/main/r/rocm/rocm.deb"}, + {"Key": "run-1/packages/deb/repo/ubuntu2404/amdrocm-repo.deb"}, + ] + } + ] + backend = self._make_backend_with_pages(pages) + loc = StorageLocation("bucket", "run-1/packages/deb") + + files = backend.list_files(loc) + + self.assertEqual( + [f.relative_path for f in files], + [ + "run-1/packages/deb/pool/main/r/rocm/rocm.deb", + "run-1/packages/deb/repo/ubuntu2404/amdrocm-repo.deb", + ], + ) + + def test_paginate_passes_no_delimiter(self): + """A Delimiter would turn the recursive listing above into one level.""" + backend = self._make_backend_with_pages([{}]) + mock_paginator = backend._s3_client.get_paginator.return_value + + backend.list_files(StorageLocation("bucket", "run-1/packages/deb")) + + self.assertNotIn("Delimiter", mock_paginator.paginate.call_args.kwargs) + def test_paginate_uses_trailing_slash(self): """Prefix should end with / to list directory contents.""" backend = self._make_backend_with_pages([{}]) From 2d60e4eb53342ba0d4db90118e54150ce05db1b9 Mon Sep 17 00:00:00 2001 From: Lalit Sudarshan Date: Thu, 13 Aug 2026 10:49:43 -0400 Subject: [PATCH 3/5] docs(packaging): correct why per-profile repo packages collide --- build_tools/_therock_utils/workflow_outputs.py | 6 ++++-- build_tools/tests/publish_repo_package_test.py | 3 ++- build_tools/tests/workflow_outputs_test.py | 5 +++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/build_tools/_therock_utils/workflow_outputs.py b/build_tools/_therock_utils/workflow_outputs.py index d2e71a13169..73f40fcaf7b 100644 --- a/build_tools/_therock_utils/workflow_outputs.py +++ b/build_tools/_therock_utils/workflow_outputs.py @@ -266,8 +266,10 @@ def native_linux_repo_package( so it sits *beside* the repository index rather than inside it: it is never reached by ``dists/`` (deb) or ``x86_64/repodata`` (rpm) and so is not part of any package index. Keeping it out matters because one - ``amdrocm-repo`` is built per OS profile and several share a filename, - which would collide if they were indexed together. + ``amdrocm-repo`` is built per OS profile and they all carry the same + package name, which would collide if they were indexed together. The + built filenames differ, since the dist tag expands per profile, so the + collision is on the package name rather than on the file. The object name is fixed so the download URL is stable regardless of the built package's versioned filename. diff --git a/build_tools/tests/publish_repo_package_test.py b/build_tools/tests/publish_repo_package_test.py index ac8d8239a63..4a2b978f168 100644 --- a/build_tools/tests/publish_repo_package_test.py +++ b/build_tools/tests/publish_repo_package_test.py @@ -174,7 +174,8 @@ def test_publish_key_per_profile(tmp_path, monkeypatch, pkg_type, os_profile): def test_publish_is_outside_the_repository_index(tmp_path, monkeypatch): # The bootstrap file is fetched by URL and must not be swept into the - # content index, or the identically-named per-profile rpms would collide. + # content index, or the per-profile rpms would collide on the package name + # they share. _stub_bucket(monkeypatch) staging = tmp_path / "staging" args = prp.parse_args(_argv(tmp_path, output_dir=staging)) diff --git a/build_tools/tests/workflow_outputs_test.py b/build_tools/tests/workflow_outputs_test.py index 989a63b6769..77c65b4879f 100644 --- a/build_tools/tests/workflow_outputs_test.py +++ b/build_tools/tests/workflow_outputs_test.py @@ -196,8 +196,9 @@ def test_native_linux_repo_package_all_profiles(self): def test_repo_package_sits_outside_the_package_index(self): # The bootstrap file is fetched by URL, so it must not land inside the # repository index: deb indexes dists/ + pool/, rpm indexes x86_64/. - # One amdrocm-repo is built per OS profile and several share a - # filename, so indexing them together would collide. + # One amdrocm-repo is built per OS profile and they all carry the same + # package name, so indexing them together would collide. The filenames + # differ, since the dist tag expands per profile. for pkg_type, index_dirs in [ ("deb", ("dists", "pool")), ("rpm", ("x86_64",)), From e112b770a49a89157809da592e241c6f903addbb Mon Sep 17 00:00:00 2001 From: Lalit Sudarshan Date: Fri, 28 Aug 2026 12:27:49 -0400 Subject: [PATCH 4/5] feat(packaging): resolve stream parameters from the build line The workflow needs three values to build amdrocm-repo: which stream a build line configures, that stream's packages base, and its signing key URL. Add a single table carrying all three, replacing the base-URL-only map that pointed at the retired packages-multi-arch hosts. The mapping is deliberately not s3_buckets.get_release_stream(). That answers which product bucket a build publishes into, and its answers do not transfer here: it maps prerelease to rc, which serves no content on any distro, and it raises on release, because stable is promoted manually and has no artifacts bucket -- yet stable is the stream users install from. release_type stays the workflow input because it also selects bucket credentials; the stream is derived from it. The key URL is emitted whole rather than as a root for the builder to append to, so where the key lives stays a fact recorded here. Also fix three promotion tests that pinned v4/packages/ as the destination. That prefix moved to v5/rocm/core/packages/, which turned them red without the code under test changing. They now discover the destination from what the promotion actually wrote and assert only the tail, since what they exist to prove is that the copy is recursive enough to carry repo// -- not where the release lands. --- .../publish_rocm_to_release_buckets_test.py | 60 +++++++++--- .../packaging/linux/get_url_repo_params.py | 95 +++++++++++++++---- .../linux/tests/get_url_repo_params_test.py | 62 ++++++++---- 3 files changed, 169 insertions(+), 48 deletions(-) diff --git a/build_tools/github_actions/tests/publish_rocm_to_release_buckets_test.py b/build_tools/github_actions/tests/publish_rocm_to_release_buckets_test.py index 0fdd9ec21cf..d970cb287c2 100644 --- a/build_tools/github_actions/tests/publish_rocm_to_release_buckets_test.py +++ b/build_tools/github_actions/tests/publish_rocm_to_release_buckets_test.py @@ -442,40 +442,70 @@ def _promote(self, release_type): publish_native_linux_packages(root, release_type, backend) return staging + def _promoted_suffixes(self, release_type): + """Paths written outside the source prefix, relative to the destination. + + The destination prefix is a literal inside the module and has already + moved once (v4/packages -> v5/rocm/core/packages), which silently turned + these tests red without anything here changing. What they exist to prove + is that the copy is recursive enough to carry repo// -- not + where the release lands -- so the prefix is discovered rather than + pinned, and only the tail is asserted. + """ + staging = self._promote(release_type) + source = f"{self.RUN_ID}-linux/" + written = { + p.relative_to(staging).as_posix() + for p in staging.rglob("*") + if p.is_file() and not p.relative_to(staging).as_posix().startswith(source) + } + self.assertTrue(written, "promotion wrote nothing outside the source prefix") + return written + + @staticmethod + def _has_suffix(written, suffix): + return any(p.endswith(suffix) for p in written) + def test_prerelease_carries_every_profile_package(self): - staging = self._promote("prerelease") + written = self._promoted_suffixes("prerelease") for pkg_type in ("deb", "rpm"): for profile in self.PROFILES: - promoted = ( - staging - / f"v4/packages/{pkg_type}/repo/{profile}/amdrocm-repo.{pkg_type}" + suffix = f"/{pkg_type}/repo/{profile}/amdrocm-repo.{pkg_type}" + self.assertTrue( + self._has_suffix(written, suffix), + f"nothing promoted ending in {suffix}; got {sorted(written)}", ) - self.assertTrue(promoted.is_file(), f"missing {promoted}") def test_prerelease_carries_the_repository_alongside(self): # A test that only checked the bootstrap package would still pass if the # copy had silently stopped carrying the repository itself. - staging = self._promote("prerelease") + written = self._promoted_suffixes("prerelease") - for rel in ( - "v4/packages/deb/pool/main/r/rocm/rocm_7.14.0_amd64.deb", - "v4/packages/deb/dists/stable/Release", - "v4/packages/rpm/x86_64/rocm-7.14.0.x86_64.rpm", - "v4/packages/rpm/x86_64/repodata/repomd.xml", + for suffix in ( + "/deb/pool/main/r/rocm/rocm_7.14.0_amd64.deb", + "/deb/dists/stable/Release", + "/rpm/x86_64/rocm-7.14.0.x86_64.rpm", + "/rpm/x86_64/repodata/repomd.xml", ): - self.assertTrue((staging / rel).is_file(), f"missing {rel}") + self.assertTrue( + self._has_suffix(written, suffix), + f"nothing promoted ending in {suffix}; got {sorted(written)}", + ) def test_dated_lines_carry_the_package_too(self): # dev and nightly promote under {date}-{run_id} rather than a fixed # prefix, so the destination shape differs and is worth covering. The # date is computed the same way the module computes it rather than # hardcoded. - staging = self._promote("dev") + written = self._promoted_suffixes("dev") dated = f"{datetime.date.today().strftime('%Y%m%d')}-{self.RUN_ID}" - promoted = staging / f"v4/deb/{dated}/repo/ubuntu2404/amdrocm-repo.deb" - self.assertTrue(promoted.is_file(), f"missing {promoted}") + suffix = f"/deb/{dated}/repo/ubuntu2404/amdrocm-repo.deb" + self.assertTrue( + self._has_suffix(written, suffix), + f"nothing promoted ending in {suffix}; got {sorted(written)}", + ) if __name__ == "__main__": diff --git a/build_tools/packaging/linux/get_url_repo_params.py b/build_tools/packaging/linux/get_url_repo_params.py index c0577b9f02e..4d061119bd7 100644 --- a/build_tools/packaging/linux/get_url_repo_params.py +++ b/build_tools/packaging/linux/get_url_repo_params.py @@ -482,30 +482,91 @@ def cmd_repo_url(args: argparse.Namespace) -> int: return 0 -# --- public repository base URL --- - -# Public CDN base for each release line: scheme + host + the packages-multi-arch -# segment. The layout *beneath* the base is not uniform and is appended by the -# caller -- release serves per-distro only (its deb/ and rpm/x86_64/ return 403), -# nightly serves flat under a dated sub-folder only (its per-distro paths return -# 403), and prerelease serves both. The one path that is uniform across all three -# is gpg/rocm.gpg at the base. -# Lines without a public repository (e.g. ci, dev) map to an empty string, which -# callers treat as "no amdrocm-repo package for this line". -_PUBLIC_REPO_BASE_URLS = { - "prerelease": "https://rocm.prereleases.amd.com/packages-multi-arch", - "release": "https://repo.amd.com/rocm/packages-multi-arch", - "nightly": "https://rocm.nightlies.amd.com/packages-multi-arch", +# --- public repository stream (RFC0012) --- + +# Which repo.amd.com stream a build line's amdrocm-repo package configures. +# +# This is deliberately NOT s3_buckets.get_release_stream(). That function answers +# a different question -- which product bucket a build publishes into -- and its +# answers do not transfer: +# +# * it maps prerelease -> rc, and rc.repo.amd.com currently serves nothing on +# any distro, so a package pointing there could not refresh; +# * it raises on "release", because stable is promoted manually and has no +# artifacts bucket, yet stable is the stream users actually install from. +# +# So the mapping is kept separate and explicit. A line absent here yields an +# empty base URL, which callers already treat as "no amdrocm-repo package for +# this line" -- that is how prerelease is skipped until rc has content. +# +# The key is NOT under the packages base: packages live at +# /core/packages// and the key at /gpg/packages.gpg, so the +# two are carried separately rather than one being derived from the other. The +# key URL is emitted whole so that where the key lives stays a fact about the +# repository, recorded here, rather than a path assembled inside the builder. +# +# ``signed`` records whether the stream publishes a key at all. It is not an +# independent choice: the streams serving a flat per-distro tree publish +# InRelease/Release.gpg and repomd.xml.asc, and the ones serving per-build trees +# publish none of them. nightly's gpg/packages.gpg is a 404. +_STREAM_ROOTS = { + "release": { + "stream": "stable", + "root": "https://stable.repo.amd.com/rocm", + "signed": True, + }, + "nightly": { + "stream": "nightly", + "root": "https://nightly.repo.amd.com/rocm", + "signed": False, + }, } +def get_public_repo_stream(release_type: str) -> str: + """Return the stream a build line's package configures (empty if none). + + Note that the build line and the stream are not the same vocabulary and do + not always share a name: the ``release`` line configures the ``stable`` + stream. Passed to the builder as ``--stream``. + """ + entry = _STREAM_ROOTS.get(release_type.strip().lower()) + return entry["stream"] if entry else "" + + +def get_public_repo_stream_root(release_type: str) -> str: + """Return the stream root for a build line (empty if it has no stream).""" + entry = _STREAM_ROOTS.get(release_type.strip().lower()) + return entry["root"] if entry else "" + + def get_public_repo_base_url(release_type: str) -> str: - """Return the public CDN base URL for a release line (empty if none).""" - return _PUBLIC_REPO_BASE_URLS.get(release_type.strip().lower(), "") + """Return the packages base URL for a release line (empty if none).""" + root = get_public_repo_stream_root(release_type) + return f"{root}/core/packages" if root else "" + + +def get_public_repo_gpg_key_url(release_type: str) -> str: + """Return the full signing-key URL for a release line (empty if none). + + Passed to the builder as ``--gpg-key-url`` and fetched verbatim. Only a + signed stream publishes a key, so an unsigned one yields an empty value here + even though it has a base URL. + """ + entry = _STREAM_ROOTS.get(release_type.strip().lower()) + if not entry or not entry["signed"]: + return "" + return f"{entry['root']}/gpg/packages.gpg" def cmd_public_repo_base_url(args: argparse.Namespace) -> int: - gha_set_output({"repo_base_url": get_public_repo_base_url(args.release_type)}) + gha_set_output( + { + "stream": get_public_repo_stream(args.release_type), + "repo_base_url": get_public_repo_base_url(args.release_type), + "gpg_key_url": get_public_repo_gpg_key_url(args.release_type), + } + ) return 0 diff --git a/build_tools/packaging/linux/tests/get_url_repo_params_test.py b/build_tools/packaging/linux/tests/get_url_repo_params_test.py index 7ab516812e2..942f5febfb5 100644 --- a/build_tools/packaging/linux/tests/get_url_repo_params_test.py +++ b/build_tools/packaging/linux/tests/get_url_repo_params_test.py @@ -423,26 +423,27 @@ def test_get_container_image_cli(self): class GetPublicRepoBaseUrlTest(unittest.TestCase): - """Tests for get_public_repo_base_url() and its subcommand.""" + """Tests for the RFC0012 stream mapping and its subcommand.""" - def test_prerelease(self): - self.assertEqual( - get_url_repo_params.get_public_repo_base_url("prerelease"), - "https://rocm.prereleases.amd.com/packages-multi-arch", - ) - - def test_release(self): + def test_release_maps_to_the_stable_stream(self): self.assertEqual( get_url_repo_params.get_public_repo_base_url("release"), - "https://repo.amd.com/rocm/packages-multi-arch", + "https://stable.repo.amd.com/rocm/core/packages", ) - def test_nightly(self): + def test_nightly_maps_to_the_nightly_stream(self): self.assertEqual( get_url_repo_params.get_public_repo_base_url("nightly"), - "https://rocm.nightlies.amd.com/packages-multi-arch", + "https://nightly.repo.amd.com/rocm/core/packages", ) + def test_prerelease_has_no_stream_yet(self): + # The documented mapping is prerelease -> rc, but rc.repo.amd.com serves + # nothing on any distro, so a package pointing there could not refresh. + # Empty means "no amdrocm-repo package for this line", which is how the + # workflow skips it. Restoring it is one entry plus a test. + self.assertEqual(get_url_repo_params.get_public_repo_base_url("prerelease"), "") + def test_ci_and_dev_are_empty(self): self.assertEqual(get_url_repo_params.get_public_repo_base_url("ci"), "") self.assertEqual(get_url_repo_params.get_public_repo_base_url("dev"), "") @@ -452,19 +453,48 @@ def test_unknown_line_is_empty(self): def test_case_insensitive(self): self.assertEqual( - get_url_repo_params.get_public_repo_base_url("PreRelease"), - "https://rocm.prereleases.amd.com/packages-multi-arch", + get_url_repo_params.get_public_repo_base_url("Release"), + "https://stable.repo.amd.com/rocm/core/packages", + ) + + def test_key_url_is_outside_the_packages_base(self): + # packages live at /core/packages/, the key at /gpg/, so + # neither URL can be reached from the other by appending or trimming one + # segment. The builder takes the key URL whole for that reason. + base = get_url_repo_params.get_public_repo_base_url("release") + key = get_url_repo_params.get_public_repo_gpg_key_url("release") + self.assertEqual(key, "https://stable.repo.amd.com/rocm/gpg/packages.gpg") + self.assertFalse(key.startswith(base)) + self.assertFalse(base.startswith(key)) + + def test_unsigned_stream_has_no_key_url(self): + # nightly serves no InRelease, no Release.gpg and no repomd.xml.asc, and + # its gpg/packages.gpg is a 404, so it must not advertise a key. + self.assertNotEqual(get_url_repo_params.get_public_repo_base_url("nightly"), "") + self.assertEqual(get_url_repo_params.get_public_repo_gpg_key_url("nightly"), "") + + def test_release_line_maps_to_a_differently_named_stream(self): + # The vocabularies differ: the "release" build line configures the + # "stable" stream. Pin it so the two are not conflated. + self.assertEqual( + get_url_repo_params.get_public_repo_stream("release"), "stable" + ) + self.assertEqual( + get_url_repo_params.get_public_repo_stream("nightly"), "nightly" ) + self.assertEqual(get_url_repo_params.get_public_repo_stream("prerelease"), "") - def test_subcommand_emits_url(self): + def test_subcommand_emits_url_and_key(self): code, output = _run_main_with_output( - ["get-public-repo-base-url", "--release-type", "prerelease"] + ["get-public-repo-base-url", "--release-type", "release"] ) self.assertEqual(code, 0) self.assertIn( - "repo_base_url=https://rocm.prereleases.amd.com/packages-multi-arch", + "repo_base_url=https://stable.repo.amd.com/rocm/core/packages", output, ) + self.assertIn("gpg_key_url=https://stable.repo.amd.com/rocm", output) + self.assertIn("stream=stable", output) def test_subcommand_emits_empty_for_ci(self): code, output = _run_main_with_output( From c5b094c3fb33919005dc63c41251e615417344f4 Mon Sep 17 00:00:00 2001 From: Lalit Sudarshan Date: Mon, 28 Sep 2026 18:11:55 -0400 Subject: [PATCH 5/5] fix(packaging): align the repo package harness with per-stream naming The install harness looked for a repo called "amdrocm", but the package registers amdrocm-, so rpm refreshes and deb checks failed on every stream. It now reads the repo id, file names and key paths from build_repo_package instead of hardcoding them. Also: - map the prerelease line to the rc stream - replace the three repo-param subcommands with one get-repo-params - add jinja2 to the harness test requirements - tidy comments and docstrings --- .../_therock_utils/workflow_outputs.py | 16 +- .../packaging/linux/get_url_repo_params.py | 205 +++++---- .../native_linux_package_install_test.py | 138 ++++-- .../packaging/linux/publish_repo_package.py | 38 +- .../linux/tests/get_url_repo_params_test.py | 319 ++++++++----- .../native_linux_package_install_ut_test.py | 418 +++++++++++++----- .../packaging/linux/tests/requirements.txt | 2 + .../tests/publish_repo_package_test.py | 7 +- build_tools/tests/workflow_outputs_test.py | 16 +- 9 files changed, 765 insertions(+), 394 deletions(-) diff --git a/build_tools/_therock_utils/workflow_outputs.py b/build_tools/_therock_utils/workflow_outputs.py index 73f40fcaf7b..b8996ee8717 100644 --- a/build_tools/_therock_utils/workflow_outputs.py +++ b/build_tools/_therock_utils/workflow_outputs.py @@ -258,21 +258,13 @@ def native_linux_repo_package( ) -> StorageLocation: """Location for the ``amdrocm-repo`` bootstrap package. - Returns ``StorageLocation`` at ``{run_id}-linux/packages/{pkg_type}/repo/{os_profile}/amdrocm-repo.{pkg_type}`` (e.g. ``12345678901-linux/packages/deb/repo/ubuntu2404/amdrocm-repo.deb``). - This is a standalone file fetched by URL to configure the repository, - so it sits *beside* the repository index rather than inside it: it is - never reached by ``dists/`` (deb) or ``x86_64/repodata`` (rpm) and so - is not part of any package index. Keeping it out matters because one - ``amdrocm-repo`` is built per OS profile and they all carry the same - package name, which would collide if they were indexed together. The - built filenames differ, since the dist tag expands per profile, so the - collision is on the package name rather than on the file. - - The object name is fixed so the download URL is stable regardless of - the built package's versioned filename. + The package is fetched by URL, so it sits beside the repository index + rather than in it: every profile's package has the same package name, so + indexing them together would collide. The object name is fixed so the + download URL does not change with the package version. Args: pkg_type: Package type ('deb' or 'rpm'). diff --git a/build_tools/packaging/linux/get_url_repo_params.py b/build_tools/packaging/linux/get_url_repo_params.py index 4d061119bd7..95ef123c695 100644 --- a/build_tools/packaging/linux/get_url_repo_params.py +++ b/build_tools/packaging/linux/get_url_repo_params.py @@ -24,8 +24,7 @@ regardless of release_type (if omitted, install runs without GPG verification). Subcommands: get-base-url, get-gpg-url, get-repo-sub-folder, get-repo-url, extract-gfx-arch, -get-container-image, get-public-repo-base-url, get-nightly-sub-folder, -get-container-image-map. Run with -h for examples. +get-container-image, get-repo-params. Run with -h for examples. Maintenance (when packaging or install URLs change): @@ -486,49 +485,42 @@ def cmd_repo_url(args: argparse.Namespace) -> int: # Which repo.amd.com stream a build line's amdrocm-repo package configures. # -# This is deliberately NOT s3_buckets.get_release_stream(). That function answers -# a different question -- which product bucket a build publishes into -- and its -# answers do not transfer: +# Not s3_buckets.get_release_stream(), which picks the bucket a build publishes +# into and raises on "release", the line that configures stable. A line absent +# here has no public stream, and callers skip the package for it. # -# * it maps prerelease -> rc, and rc.repo.amd.com currently serves nothing on -# any distro, so a package pointing there could not refresh; -# * it raises on "release", because stable is promoted manually and has no -# artifacts bucket, yet stable is the stream users actually install from. -# -# So the mapping is kept separate and explicit. A line absent here yields an -# empty base URL, which callers already treat as "no amdrocm-repo package for -# this line" -- that is how prerelease is skipped until rc has content. -# -# The key is NOT under the packages base: packages live at -# /core/packages// and the key at /gpg/packages.gpg, so the -# two are carried separately rather than one being derived from the other. The -# key URL is emitted whole so that where the key lives stays a fact about the -# repository, recorded here, rather than a path assembled inside the builder. -# -# ``signed`` records whether the stream publishes a key at all. It is not an -# independent choice: the streams serving a flat per-distro tree publish -# InRelease/Release.gpg and repomd.xml.asc, and the ones serving per-build trees -# publish none of them. nightly's gpg/packages.gpg is a 404. +# Only the stream name and where it is served are recorded here; its shape, +# signing and repo id belong to build_repo_package. The key is at +# /gpg/packages.gpg, outside the packages base, so its URL is built here +# and passed to the builder whole. _STREAM_ROOTS = { "release": { "stream": "stable", "root": "https://stable.repo.amd.com/rocm", - "signed": True, + }, + "prerelease": { + "stream": "rc", + "root": "https://rc.repo.amd.com/rocm", }, "nightly": { "stream": "nightly", "root": "https://nightly.repo.amd.com/rocm", - "signed": False, }, } +def get_public_repo_release_types() -> tuple[str, ...]: + """Return the build lines that configure a public stream, in table order.""" + return tuple(_STREAM_ROOTS) + + def get_public_repo_stream(release_type: str) -> str: """Return the stream a build line's package configures (empty if none). Note that the build line and the stream are not the same vocabulary and do not always share a name: the ``release`` line configures the ``stable`` - stream. Passed to the builder as ``--stream``. + stream and ``prerelease`` configures ``rc``. Passed to the builder as + ``--stream``. """ entry = _STREAM_ROOTS.get(release_type.strip().lower()) return entry["stream"] if entry else "" @@ -546,28 +538,18 @@ def get_public_repo_base_url(release_type: str) -> str: return f"{root}/core/packages" if root else "" -def get_public_repo_gpg_key_url(release_type: str) -> str: +def get_public_repo_gpg_key_url(release_type: str, *, signed: bool) -> str: """Return the full signing-key URL for a release line (empty if none). Passed to the builder as ``--gpg-key-url`` and fetched verbatim. Only a - signed stream publishes a key, so an unsigned one yields an empty value here - even though it has a base URL. + signed stream publishes a key, so an unsigned one yields an empty value + even though it has a base URL. ``signed`` comes from + ``build_repo_package.is_signed(stream)``, which owns that rule. """ - entry = _STREAM_ROOTS.get(release_type.strip().lower()) - if not entry or not entry["signed"]: + root = get_public_repo_stream_root(release_type) + if not root or not signed: return "" - return f"{entry['root']}/gpg/packages.gpg" - - -def cmd_public_repo_base_url(args: argparse.Namespace) -> int: - gha_set_output( - { - "stream": get_public_repo_stream(args.release_type), - "repo_base_url": get_public_repo_base_url(args.release_type), - "gpg_key_url": get_public_repo_gpg_key_url(args.release_type), - } - ) - return 0 + return f"{root}/gpg/packages.gpg" # --- nightly sub-folder --- @@ -596,21 +578,6 @@ def nightly_sub_folder(run_id: str, *, today: date | None = None) -> str: return f"{day:%Y%m%d}-{run_id}" -def cmd_nightly_sub_folder(args: argparse.Namespace) -> int: - # Only the nightly line publishes into a dated sub-folder; every other line - # publishes at the repository root, so the value is empty there. - if args.release_type and args.release_type.strip().lower() != "nightly": - gha_set_output({"repo_sub_folder": ""}) - return 0 - try: - sub_folder = nightly_sub_folder(args.run_id) - except ValueError as e: - print(f"Error: {e}", file=sys.stderr) - return 1 - gha_set_output({"repo_sub_folder": sub_folder}) - return 0 - - # --- container image map --- @@ -619,16 +586,60 @@ def get_container_image_map(os_profiles: list[str]) -> dict[str, str]: return {profile: get_container_image(profile) for profile in os_profiles} -def cmd_container_image_map(args: argparse.Namespace) -> int: - try: - profiles = json.loads(args.os_profiles) - except json.JSONDecodeError as e: - print(f"Error: --os-profiles is not valid JSON: {e}", file=sys.stderr) - return 1 - if not isinstance(profiles, list) or not all(isinstance(p, str) for p in profiles): - print("Error: --os-profiles must be a JSON array of strings", file=sys.stderr) - return 1 - gha_set_output({"images": json.dumps(get_container_image_map(profiles))}) +# --- amdrocm-repo parameters --- + +# These mirror build_repo_package's private _FLAT / _BUILD_ID. They are compared +# against its public stream_shape(), and a test asserts the two agree. +_SHAPE_FLAT = "flat" +_SHAPE_BUILD_ID = "build_id" + + +def cmd_repo_params(args: argparse.Namespace) -> int: + """Emit every parameter the amdrocm-repo build matrix needs, in one output. + + Outputs: ``os_profiles`` and ``images`` (JSON, the matrix), then the stream + the build line configures and what follows from it -- ``stream``, + ``repo_base_url``, ``gpg_key_url``, ``repo_sub_folder``. A line with no + public stream gets all four empty, which callers read as "no package for + this line"; the matrix is still emitted. + """ + # Deferred: build_repo_package imports jinja2 at module level, and this + # script's other subcommands run in a workflow job that installs no Python + # packages. Only this subcommand needs the builder. + from build_repo_package import is_signed, list_profiles, stream_shape + + profiles = list_profiles(args.pkg_type) + outputs = { + "os_profiles": json.dumps(profiles), + "images": json.dumps(get_container_image_map(profiles)), + "stream": "", + "repo_base_url": "", + "gpg_key_url": "", + "repo_sub_folder": "", + } + stream = get_public_repo_stream(args.release_type) + if stream: + shape = stream_shape(stream) + if shape not in (_SHAPE_FLAT, _SHAPE_BUILD_ID): + print( + f"Error: stream {stream!r} has unknown shape {shape!r}", file=sys.stderr + ) + return 1 + outputs["stream"] = stream + outputs["repo_base_url"] = get_public_repo_base_url(args.release_type) + outputs["gpg_key_url"] = get_public_repo_gpg_key_url( + args.release_type, signed=is_signed(stream) + ) + # A build_id stream serves each build from its own dated folder; a flat + # one serves one tree at the root, and the builder rejects a sub-folder + # there. The run id is only validated where it is used. + if shape == _SHAPE_BUILD_ID: + try: + outputs["repo_sub_folder"] = nightly_sub_folder(args.run_id) + except ValueError as e: + print(f"Error: {e}", file=sys.stderr) + return 1 + gha_set_output(outputs) return 0 @@ -908,54 +919,38 @@ def main(argv: list[str] | None = None) -> int: ) p_img.set_defaults(func=cmd_container_image) - # get-public-repo-base-url: public CDN base for a release line - p_pub = subparsers.add_parser( - "get-public-repo-base-url", - help="Print repo_base_url= for a release line (empty for lines without a public repo, e.g. ci/dev).", + # get-repo-params: every amdrocm-repo build parameter in one output + p_rp = subparsers.add_parser( + "get-repo-params", + help=( + "Print os_profiles=, images=, stream=, repo_base_url=, gpg_key_url= and " + "repo_sub_folder= for an amdrocm-repo build. The last four are empty for " + "a line with no public repository (e.g. ci/dev)." + ), + ) + p_rp.add_argument( + "--pkg-type", + type=str, + required=True, + choices=["deb", "rpm"], + help="Package type whose OS profiles form the build matrix", ) - p_pub.add_argument( + p_rp.add_argument( "--release-type", type=str, required=True, help="Release line (e.g. prerelease, release, nightly)", ) - p_pub.set_defaults(func=cmd_public_repo_base_url) - - # get-nightly-sub-folder: dated sub-folder for the nightly line - p_sub = subparsers.add_parser( - "get-nightly-sub-folder", - help="Print repo_sub_folder=YYYYMMDD- for the nightly line.", - ) - p_sub.add_argument( + p_rp.add_argument( "--run-id", type=str, required=True, - help="Workflow run id to embed in the dated sub-folder", - ) - p_sub.add_argument( - "--release-type", - type=str, - default=None, help=( - "If set, print an empty repo_sub_folder for any line other than " - "'nightly'. If omitted, always print the dated sub-folder." + "Workflow run id, embedded in the dated build folder of a per-build " + "stream. Ignored (and not validated) for other streams." ), ) - p_sub.set_defaults(func=cmd_nightly_sub_folder) - - # get-container-image-map: os_profile -> image map for a build matrix - p_map = subparsers.add_parser( - "get-container-image-map", - help="Print images= mapping each OS profile (JSON array input) to its container image.", - ) - p_map.add_argument( - "--os-profiles", - type=str, - required=True, - metavar="JSON", - help='JSON array of OS profiles (e.g. ["rhel8","rhel10","sles16"])', - ) - p_map.set_defaults(func=cmd_container_image_map) + p_rp.set_defaults(func=cmd_repo_params) args = parser.parse_args(argv) return args.func(args) diff --git a/build_tools/packaging/linux/native_linux_package_install_test.py b/build_tools/packaging/linux/native_linux_package_install_test.py index b2c901cc6a7..32b7f83fa3d 100644 --- a/build_tools/packaging/linux/native_linux_package_install_test.py +++ b/build_tools/packaging/linux/native_linux_package_install_test.py @@ -141,6 +141,7 @@ import traceback from argparse import ArgumentParser, Namespace from pathlib import Path, PurePosixPath +from typing import NamedTuple _SCRIPT_DIR = Path(__file__).resolve().parent if str(_SCRIPT_DIR) not in sys.path: @@ -177,28 +178,33 @@ def _env(key: str, default: str) -> str: # ("wget .../rocm.gpg.key | gpg --dearmor | sudo tee /etc/apt/keyrings/rocm.gpg") # and point signed-by= at it. This harness writes its keyring the same way, so # sharing the path would overwrite the key a host is already using. Distinct -# from the amdrocm-repo package's own keyring (AMDROCM_DEB_KEYRING), which is -# package-owned and lives in a different directory. +# from the amdrocm-repo package's own keyring (build_repo_package.DEB_KEYRING_PATH), +# which is package-owned and lives in a different directory. APT_KEYRING_FILE = _env("ROCM_APT_KEYRING_FILE", f"/etc/apt/keyrings/{REPO_NAME}.gpg") ZYPP_REPOS_DIR = _env("ROCM_ZYPP_REPOS_DIR", "/etc/zypp/repos.d") YUM_REPOS_DIR = _env("ROCM_YUM_REPOS_DIR", "/etc/yum.repos.d") # Relative path from install prefix to rdhc binary (script); overridable via ROCM_RDHC_REL_PATH RDHC_REL_PATH = _env("ROCM_RDHC_REL_PATH", "libexec/rocm-core/rdhc.py") -# amdrocm-repo package: the on-disk paths it installs (the package's public -# contract). Used by the install-via-package mode to confirm the repo config -# landed. These are independent of REPO_NAME (which names the manually-written -# repo file used by the default setup path). -AMDROCM_REPO_ID = "amdrocm" -AMDROCM_DEB_SOURCES = "/etc/apt/sources.list.d/amdrocm.sources" -# Named amdrocm.gpg, not rocm.gpg, so it cannot collide with the keyring the -# amdgpu driver setup from repo.radeon.com installs. Must match -# build_repo_package.DEB_KEYRING_PATH. -AMDROCM_DEB_KEYRING = "/usr/share/keyrings/amdrocm.gpg" -AMDROCM_RPM_REPO_BASENAME = "amdrocm.repo" -# Named -amdrocm, not -rocm, for the same collision reason as the deb -# keyring. Must match build_repo_package.RPM_GPG_KEY_PATH. -AMDROCM_RPM_KEY = "/etc/pki/rpm-gpg/RPM-GPG-KEY-amdrocm" +# Where the amdrocm-repo package's deb repo file lands (rpm: ZYPP_REPOS_DIR / +# YUM_REPOS_DIR above). File names, key paths and the repo id come from +# build_repo_package; see RepoPackageContract. +AMDROCM_DEB_SOURCES_DIR = "/etc/apt/sources.list.d" + + +class RepoPackageContract(NamedTuple): + """What an installed amdrocm-repo package leaves on disk for one stream. + + Paths are POSIX strings: they name locations on the target system, not on + the host running the harness. + """ + + stream: str + repo_id: str + repo_file: str + key_file: str + signed: bool + # Timeouts (seconds) and verification threshold GPG_MKDIR_TIMEOUT_SEC = 10 @@ -210,7 +216,7 @@ def _env(key: str, default: str) -> str: DNF_MAKECACHE_TIMEOUT_SEC = 120 REPO_PACKAGE_INSTALL_TIMEOUT_SEC = 120 # Retry the post-install metadata refresh so a transient repo/CDN blip does not -# fail the (network-dependent) config-only gate. +# fail the (network-dependent) config-only run. REPO_REFRESH_ATTEMPTS = 3 REPO_REFRESH_BACKOFF_SEC = 3 INSTALL_TIMEOUT_SEC = 1800 # 30 minutes @@ -318,6 +324,47 @@ def _is_sles(self) -> bool: """Return True when the OS profile is SLES (delegates to shared helper).""" return is_sles(self.os_profile) + def _repo_package_contract(self) -> RepoPackageContract: + """Return the repo file, key and repo id the amdrocm-repo package installs. + + Read from build_repo_package, which renders the package, so the harness + looks for the names the package actually ships. The release line maps to + a stream through get_url_repo_params, the same mapping the build uses. + + Raises: + ValueError: If the release line has no public stream. + """ + # Deferred: only the package mode needs build_repo_package (and the + # jinja2 it imports), so the harness's other modes do not depend on it. + from build_repo_package import ( + DEB_KEYRING_PATH, + RPM_GPG_KEY_PATH, + is_signed, + repo_id, + ) + from get_url_repo_params import get_public_repo_stream + + stream = get_public_repo_stream(self.release_type) + if not stream: + raise ValueError( + f"release type {self.release_type!r} has no public amdrocm-repo stream" + ) + rid = repo_id(stream) + if self.package_type == "deb": + repo_file = PurePosixPath(AMDROCM_DEB_SOURCES_DIR) / f"{rid}.sources" + key_file = DEB_KEYRING_PATH + else: + repo_dir = ZYPP_REPOS_DIR if self._is_sles() else YUM_REPOS_DIR + repo_file = PurePosixPath(repo_dir) / f"{rid}.repo" + key_file = RPM_GPG_KEY_PATH + return RepoPackageContract( + stream=stream, + repo_id=rid, + repo_file=str(repo_file), + key_file=key_file, + signed=is_signed(stream), + ) + def __init__( self, repo_url: str, @@ -860,10 +907,16 @@ def install_repo_package(self) -> bool: print("CONFIGURING REPOSITORY VIA amdrocm-repo PACKAGE") print("=" * 80) + try: + contract = self._repo_package_contract() + except ValueError as e: + print(f"[FAIL] {e}") + return False pkg = self._find_repo_package() if pkg is None: return False print(f"\nRepo package: {pkg}") + print(f"Stream: {contract.stream} (repo id {contract.repo_id})") if self.package_type == "deb": install_cmd = ["sudo", "apt", "install", "-y", str(pkg)] @@ -893,10 +946,9 @@ def install_repo_package(self) -> bool: return False print("[PASS] amdrocm-repo package installed") - # Refresh metadata for the repo the package just configured. The dnf and - # zypper refreshes are scoped to the amdrocm repo; apt has no clean - # per-source refresh, so the deb path runs a repo-wide `apt update` - # (a failing unrelated base-image source would also fail this step). + # Refresh metadata for the repo the package just configured. dnf and + # zypper are scoped to it; apt update is repo-wide, so a failing + # unrelated source in the base image also fails this step. if self.package_type == "deb": refresh_cmd = ["sudo", "apt", "update"] refresh_timeout = APT_UPDATE_TIMEOUT_SEC @@ -906,7 +958,7 @@ def install_repo_package(self) -> bool: "--non-interactive", "--gpg-auto-import-keys", "refresh", - AMDROCM_REPO_ID, + contract.repo_id, ] refresh_timeout = ZYPP_REFRESH_TIMEOUT_SEC else: @@ -914,7 +966,7 @@ def install_repo_package(self) -> bool: "dnf", "makecache", "--disablerepo=*", - f"--enablerepo={AMDROCM_REPO_ID}", + f"--enablerepo={contract.repo_id}", ] refresh_timeout = DNF_MAKECACHE_TIMEOUT_SEC @@ -943,8 +995,9 @@ def install_repo_package(self) -> bool: def assert_repo_configured(self) -> bool: """Verify the amdrocm-repo package dropped the expected repo config. - Checks the repo file exists (and looks like a ROCm repo) and, for signed - release lines, that the signing key was installed too. + Checks the repo file exists and has a ``URIs:``/``baseurl=`` line and, + for signed streams, that the signing key was installed too. Where to + look comes from _repo_package_contract(). Returns: True if the on-disk repo configuration is present and well-formed. @@ -953,19 +1006,21 @@ def assert_repo_configured(self) -> bool: print("VERIFYING REPOSITORY CONFIGURATION") print("=" * 80) - signed = self.release_type in ("prerelease", "release") + try: + contract = self._repo_package_contract() + except ValueError as e: + print(f"[FAIL] {e}") + return False + signed = contract.signed + repo_file = Path(contract.repo_file) + key_file = Path(contract.key_file) ok = True if self.package_type == "deb": - repo_file = Path(AMDROCM_DEB_SOURCES) - key_file = Path(AMDROCM_DEB_KEYRING) # deb822 marker (independent of the repo host, which need not contain # "rocm" for a mirror). marker = "URIs:" else: - repo_dir = ZYPP_REPOS_DIR if self._is_sles() else YUM_REPOS_DIR - repo_file = Path(repo_dir) / AMDROCM_RPM_REPO_BASENAME - key_file = Path(AMDROCM_RPM_KEY) marker = "baseurl=" if repo_file.is_file(): @@ -987,10 +1042,10 @@ def assert_repo_configured(self) -> bool: if key_file.is_file(): print(f"[PASS] Signing key present: {key_file}") else: - print(f"[FAIL] Signing key not found (signed line): {key_file}") + print(f"[FAIL] Signing key not found (signed stream): {key_file}") ok = False else: - print("[INFO] Unsigned line: no signing key expected") + print("[INFO] Unsigned stream: no signing key expected") if ok: print("\n[PASS] Repository configuration verified") @@ -1757,7 +1812,8 @@ def _build_argument_parser(*, exit_on_error: bool = True) -> ArgumentParser: metavar="DIR", help="Directory containing the built amdrocm-repo .deb/.rpm. When set, the repository is " "configured by installing that package (instead of writing repo files); --repo-url and " - "--gpg-key-url become optional. Not valid with --test-type simulate.", + "--gpg-key-url become optional. Requires a --release-type with a public stream. " + "Not valid with --test-type simulate.", ) parser.add_argument( "--repo-config-only", @@ -1792,6 +1848,20 @@ def _validate_cli_args(parser: ArgumentParser, args: Namespace) -> None: return if args.repo_config_only and not args.repo_package_dir: parser.error("--repo-config-only requires --repo-package-dir") + if args.repo_package_dir: + # Deferred for the same reason as in _repo_package_contract: only the + # package mode needs the stream mapping. + from get_url_repo_params import ( + get_public_repo_release_types, + get_public_repo_stream, + ) + + if not get_public_repo_stream(args.release_type or ""): + parser.error( + "--repo-package-dir requires --release-type naming a line with a " + "public amdrocm-repo stream: " + + ", ".join(get_public_repo_release_types()) + ) if not args.os_profile: parser.error( "--os-profile is required when --test-type is 'install', 'sanity', or 'full'" diff --git a/build_tools/packaging/linux/publish_repo_package.py b/build_tools/packaging/linux/publish_repo_package.py index ed00193345d..cd92bca14b4 100644 --- a/build_tools/packaging/linux/publish_repo_package.py +++ b/build_tools/packaging/linux/publish_repo_package.py @@ -2,27 +2,20 @@ # Copyright Advanced Micro Devices, Inc. # SPDX-License-Identifier: MIT -"""Publish a built amdrocm-repo package as a standalone per-distro file. +r"""Publish a built amdrocm-repo package to the per-run outputs. -The package is uploaded next to the native content packages, as a -``repo//`` sibling of the per-run ``packages//`` prefix, -rather than into the repository index. A client can then fetch it directly by -URL to configure the repository. The file becomes publicly reachable once the -release promotion step copies the per-run tree to the public CDN. +The destination is ``WorkflowOutputRoot.native_linux_repo_package()``, beside +the package index (see that method for the layout). Release promotion +(``publish_rocm_to_release_buckets.py``) copies it with the rest of the per-run +``packages//`` tree. -The object name is fixed (``amdrocm-repo.``) so the download URL is stable -regardless of the built package's versioned filename. - -The destination is resolved through ``WorkflowOutputRoot``, the single source of -truth for CI path layout, so the bucket and per-run prefix come from the -workflow context rather than being passed in. That needs the CI environment: -``GITHUB_REPOSITORY``, ``RELEASE_TYPE``, and the event payload used for fork -detection. There is no artifacts bucket for the ``release`` line, so that -release type raises; the publishing job is expected to be gated off it. +The bucket and prefix come from the CI environment: ``GITHUB_REPOSITORY``, +``RELEASE_TYPE`` and the event payload used for fork detection. The ``release`` +line has no artifacts bucket and raises; the publishing job does not run for it. Usage: python build_tools/packaging/linux/publish_repo_package.py \ - --file repo-package-out/amdrocm-repo-7.14.0-1.el10.noarch.rpm \ + --file repo-package-out/amdrocm-repo-10.0.0-1.stable.el10.noarch.rpm \ --run-id 12345678901 \ --os-profile rhel10 \ --pkg-type rpm @@ -44,11 +37,8 @@ from _therock_utils.storage_backend import create_storage_backend from _therock_utils.workflow_outputs import WorkflowOutputRoot -# An os-profile becomes a path segment in the object key, so restrict it to a -# safe set of characters (no slashes, whitespace, or control characters) and -# require a leading alphanumeric. The leading character matters: without it, -# "." and ".." satisfy the character class, and ".." resolves one directory -# above the intended location when the key is joined onto a local staging +# An os-profile becomes a path segment of the object key. The leading +# alphanumeric rejects "." and "..", which would escape a local staging # directory. \Z (not $) so a trailing newline is not accepted. _OS_PROFILE_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*\Z") @@ -104,10 +94,8 @@ def publish(args: argparse.Namespace) -> str: root = WorkflowOutputRoot.from_workflow_run(run_id=args.run_id, platform="linux") dest = root.native_linux_repo_package(args.pkg_type, args.os_profile) backend = create_storage_backend(staging_dir=args.output_dir, dry_run=args.dry_run) - # Name the destination the run actually writes to. --output-dir stages to a - # local tree, so reporting an s3:// URI there would describe an upload that - # did not happen. The publishing job runs under continue-on-error, which - # makes this log the only signal that it did anything. + # Log where the file actually goes: with --output-dir that is a local path, + # and an s3:// URI would describe an upload that did not happen. target = ( dest.local_path(args.output_dir) if args.output_dir is not None else dest.s3_uri ) diff --git a/build_tools/packaging/linux/tests/get_url_repo_params_test.py b/build_tools/packaging/linux/tests/get_url_repo_params_test.py index 942f5febfb5..75be71d41e3 100644 --- a/build_tools/packaging/linux/tests/get_url_repo_params_test.py +++ b/build_tools/packaging/linux/tests/get_url_repo_params_test.py @@ -25,7 +25,9 @@ - Python 3.10 or newer - Run from TheROCK repository root - - Stdlib only; ``$GITHUB_OUTPUT`` is mocked to a temp file + - Stdlib only, except the ``get-repo-params`` and stream-consistency tests, + which import ``build_repo_package`` (jinja2, from requirements-test.txt); + ``$GITHUB_OUTPUT`` is mocked to a temp file Run:: @@ -37,6 +39,7 @@ import json import os +import subprocess import sys import tempfile import unittest @@ -52,6 +55,7 @@ if _path_str not in sys.path: sys.path.insert(0, _path_str) +import build_repo_package # noqa: E402 import get_url_repo_params # noqa: E402 _EXAMPLE = get_url_repo_params.EXAMPLE_CDN_BASE @@ -423,7 +427,7 @@ def test_get_container_image_cli(self): class GetPublicRepoBaseUrlTest(unittest.TestCase): - """Tests for the RFC0012 stream mapping and its subcommand.""" + """Tests for the RFC0012 stream mapping.""" def test_release_maps_to_the_stable_stream(self): self.assertEqual( @@ -431,19 +435,18 @@ def test_release_maps_to_the_stable_stream(self): "https://stable.repo.amd.com/rocm/core/packages", ) + def test_prerelease_maps_to_the_rc_stream(self): + self.assertEqual( + get_url_repo_params.get_public_repo_base_url("prerelease"), + "https://rc.repo.amd.com/rocm/core/packages", + ) + def test_nightly_maps_to_the_nightly_stream(self): self.assertEqual( get_url_repo_params.get_public_repo_base_url("nightly"), "https://nightly.repo.amd.com/rocm/core/packages", ) - def test_prerelease_has_no_stream_yet(self): - # The documented mapping is prerelease -> rc, but rc.repo.amd.com serves - # nothing on any distro, so a package pointing there could not refresh. - # Empty means "no amdrocm-repo package for this line", which is how the - # workflow skips it. Restoring it is one entry plus a test. - self.assertEqual(get_url_repo_params.get_public_repo_base_url("prerelease"), "") - def test_ci_and_dev_are_empty(self): self.assertEqual(get_url_repo_params.get_public_repo_base_url("ci"), "") self.assertEqual(get_url_repo_params.get_public_repo_base_url("dev"), "") @@ -461,52 +464,91 @@ def test_key_url_is_outside_the_packages_base(self): # packages live at /core/packages/, the key at /gpg/, so # neither URL can be reached from the other by appending or trimming one # segment. The builder takes the key URL whole for that reason. - base = get_url_repo_params.get_public_repo_base_url("release") - key = get_url_repo_params.get_public_repo_gpg_key_url("release") - self.assertEqual(key, "https://stable.repo.amd.com/rocm/gpg/packages.gpg") - self.assertFalse(key.startswith(base)) - self.assertFalse(base.startswith(key)) + for release_type, key_url in ( + ("release", "https://stable.repo.amd.com/rocm/gpg/packages.gpg"), + ("prerelease", "https://rc.repo.amd.com/rocm/gpg/packages.gpg"), + ): + with self.subTest(release_type=release_type): + base = get_url_repo_params.get_public_repo_base_url(release_type) + key = get_url_repo_params.get_public_repo_gpg_key_url( + release_type, signed=True + ) + self.assertEqual(key, key_url) + self.assertFalse(key.startswith(base)) + self.assertFalse(base.startswith(key)) def test_unsigned_stream_has_no_key_url(self): # nightly serves no InRelease, no Release.gpg and no repomd.xml.asc, and # its gpg/packages.gpg is a 404, so it must not advertise a key. self.assertNotEqual(get_url_repo_params.get_public_repo_base_url("nightly"), "") - self.assertEqual(get_url_repo_params.get_public_repo_gpg_key_url("nightly"), "") + self.assertEqual( + get_url_repo_params.get_public_repo_gpg_key_url("nightly", signed=False), + "", + ) + + def test_line_without_a_stream_has_no_key_url_even_if_signed(self): + self.assertEqual( + get_url_repo_params.get_public_repo_gpg_key_url("ci", signed=True), "" + ) - def test_release_line_maps_to_a_differently_named_stream(self): - # The vocabularies differ: the "release" build line configures the - # "stable" stream. Pin it so the two are not conflated. + def test_build_lines_map_to_differently_named_streams(self): + # The vocabularies differ: "release" configures "stable" and + # "prerelease" configures "rc". Pin them so the two are not conflated. self.assertEqual( get_url_repo_params.get_public_repo_stream("release"), "stable" ) + self.assertEqual(get_url_repo_params.get_public_repo_stream("prerelease"), "rc") self.assertEqual( get_url_repo_params.get_public_repo_stream("nightly"), "nightly" ) - self.assertEqual(get_url_repo_params.get_public_repo_stream("prerelease"), "") + self.assertEqual(get_url_repo_params.get_public_repo_stream("ci"), "") - def test_subcommand_emits_url_and_key(self): - code, output = _run_main_with_output( - ["get-public-repo-base-url", "--release-type", "release"] - ) - self.assertEqual(code, 0) - self.assertIn( - "repo_base_url=https://stable.repo.amd.com/rocm/core/packages", - output, + def test_release_types_in_table_order(self): + self.assertEqual( + get_url_repo_params.get_public_repo_release_types(), + ("release", "prerelease", "nightly"), ) - self.assertIn("gpg_key_url=https://stable.repo.amd.com/rocm", output) - self.assertIn("stream=stable", output) - def test_subcommand_emits_empty_for_ci(self): - code, output = _run_main_with_output( - ["get-public-repo-base-url", "--release-type", "ci"] + +class StreamRootsConsistencyTest(unittest.TestCase): + """The line -> stream table agrees with the builder's streams. + + The table records only the line -> stream name and where the stream is + served; everything else about a stream belongs to build_repo_package. These + tests keep the two sides from drifting when a stream is added to one only. + """ + + def test_every_stream_in_the_table_is_one_the_builder_knows(self): + for release_type in get_url_repo_params.get_public_repo_release_types(): + with self.subTest(release_type=release_type): + self.assertIn( + get_url_repo_params.get_public_repo_stream(release_type), + build_repo_package.STREAMS, + ) + + def test_every_builder_stream_is_reachable_from_exactly_one_line(self): + streams = [ + get_url_repo_params.get_public_repo_stream(release_type) + for release_type in get_url_repo_params.get_public_repo_release_types() + ] + for stream in build_repo_package.STREAMS: + with self.subTest(stream=stream): + self.assertEqual(streams.count(stream), 1) + + def test_shape_constants_match_the_builder(self): + shapes = set(build_repo_package.STREAM_SHAPES.values()) + self.assertEqual( + shapes, + {get_url_repo_params._SHAPE_FLAT, get_url_repo_params._SHAPE_BUILD_ID}, + ) + self.assertEqual( + build_repo_package.stream_shape("nightly"), + get_url_repo_params._SHAPE_BUILD_ID, ) - self.assertEqual(code, 0) - self.assertIn("repo_base_url=", output) - self.assertNotIn("packages-multi-arch", output) class NightlySubFolderTest(unittest.TestCase): - """Tests for nightly_sub_folder() and its subcommand.""" + """Tests for nightly_sub_folder().""" def test_formats_with_injected_date(self): self.assertEqual( @@ -529,55 +571,9 @@ def test_rejects_non_numeric_run_id(self): with self.assertRaises(ValueError): get_url_repo_params.nightly_sub_folder(bad) - def test_subcommand_writes_exactly_one_output_for_hostile_input(self): - code, output = _run_main_with_output( - [ - "get-nightly-sub-folder", - "--release-type", - "nightly", - "--run-id", - "1\nEOF_mag1c\ninjected=yes", - ] - ) - self.assertNotEqual(code, 0) - self.assertNotIn("injected=yes", output) - - def test_subcommand_emits_sub_folder(self): - code, output = _run_main_with_output( - ["get-nightly-sub-folder", "--run-id", "12345"] - ) - self.assertEqual(code, 0) - # The date is the current UTC date; assert the structure, not the value. - self.assertRegex(output, r"repo_sub_folder=\d{8}-12345") - - def test_subcommand_emits_sub_folder_for_nightly(self): - code, output = _run_main_with_output( - ["get-nightly-sub-folder", "--release-type", "nightly", "--run-id", "12345"] - ) - self.assertEqual(code, 0) - self.assertRegex(output, r"repo_sub_folder=\d{8}-12345") - - def test_subcommand_emits_empty_for_other_lines(self): - # Only the nightly line publishes into a dated sub-folder; the other - # lines publish at the repository root. - for release_type in ("release", "prerelease", "ci"): - with self.subTest(release_type=release_type): - code, output = _run_main_with_output( - [ - "get-nightly-sub-folder", - "--release-type", - release_type, - "--run-id", - "12345", - ] - ) - self.assertEqual(code, 0) - self.assertIn("repo_sub_folder=", output) - self.assertNotRegex(output, r"repo_sub_folder=\S") - class GetContainerImageMapTest(unittest.TestCase): - """Tests for get_container_image_map() and its subcommand.""" + """Tests for get_container_image_map().""" def test_maps_rpm_profiles(self): self.assertEqual( @@ -588,29 +584,154 @@ def test_maps_rpm_profiles(self): }, ) - def test_subcommand_emits_json_map(self): - code, output = _run_main_with_output( - ["get-container-image-map", "--os-profiles", '["ubuntu2404"]'] + +def _outputs(text: str) -> dict[str, str]: + """Parse single-line ``key=value`` GITHUB_OUTPUT entries into a dict.""" + return dict(line.split("=", 1) for line in text.splitlines() if "=" in line) + + +_REPO_PARAMS_KEYS = { + "os_profiles", + "images", + "stream", + "repo_base_url", + "gpg_key_url", + "repo_sub_folder", +} + + +class GetRepoParamsTest(unittest.TestCase): + """Tests for the get-repo-params subcommand.""" + + def _run(self, pkg_type, release_type, run_id="12345"): + return _run_main_with_output( + [ + "get-repo-params", + "--pkg-type", + pkg_type, + "--release-type", + release_type, + "--run-id", + run_id, + ] ) + + def test_nightly_is_unsigned_with_a_dated_build_folder(self): + code, output = self._run("rpm", "nightly") self.assertEqual(code, 0) - # Recover the JSON value written after ``images=``. - line = next(l for l in output.splitlines() if l.startswith("images=")) - images = json.loads(line[len("images=") :]) + out = _outputs(output) + self.assertEqual(set(out), _REPO_PARAMS_KEYS) + self.assertEqual(json.loads(out["os_profiles"]), ["rhel8", "rhel10", "sles16"]) self.assertEqual( - images, {"ubuntu2404": "ghcr.io/rocm/no_rocm_image_ubuntu24_04:latest"} + json.loads(out["images"]), + get_url_repo_params.get_container_image_map(["rhel8", "rhel10", "sles16"]), ) - - def test_subcommand_rejects_invalid_json(self): - code, _ = _run_main_with_output( - ["get-container-image-map", "--os-profiles", "not-json"] + self.assertEqual(out["stream"], "nightly") + self.assertEqual( + out["repo_base_url"], "https://nightly.repo.amd.com/rocm/core/packages" ) - self.assertEqual(code, 1) + self.assertEqual(out["gpg_key_url"], "") + # The date is the current UTC date; assert the structure, not the value. + self.assertRegex(out["repo_sub_folder"], r"^\d{8}-12345$") + + def test_flat_streams_are_signed_with_no_build_folder(self): + cases = { + "release": ("stable", "https://stable.repo.amd.com/rocm"), + "prerelease": ("rc", "https://rc.repo.amd.com/rocm"), + } + for release_type, (stream, root) in cases.items(): + with self.subTest(release_type=release_type): + code, output = self._run("deb", release_type) + self.assertEqual(code, 0) + out = _outputs(output) + self.assertEqual(set(out), _REPO_PARAMS_KEYS) + self.assertEqual(json.loads(out["os_profiles"]), ["ubuntu2404"]) + self.assertEqual(out["stream"], stream) + self.assertEqual(out["repo_base_url"], f"{root}/core/packages") + self.assertEqual(out["gpg_key_url"], f"{root}/gpg/packages.gpg") + self.assertEqual(out["repo_sub_folder"], "") + + def test_line_without_a_stream_still_emits_the_matrix(self): + code, output = self._run("deb", "ci") + self.assertEqual(code, 0) + out = _outputs(output) + self.assertEqual(set(out), _REPO_PARAMS_KEYS) + self.assertEqual(json.loads(out["os_profiles"]), ["ubuntu2404"]) + for key in ("stream", "repo_base_url", "gpg_key_url", "repo_sub_folder"): + self.assertEqual(out[key], "", key) + + def test_hostile_run_id_on_a_build_id_stream_writes_nothing(self): + code, output = self._run("deb", "nightly", run_id="1\nEOF_mag1c\ninjected=yes") + self.assertNotEqual(code, 0) + self.assertNotIn("injected=yes", output) + self.assertEqual(output, "") + + def test_run_id_is_not_validated_on_a_flat_stream(self): + # A flat stream has no build folder, so the run id is never used there. + code, output = self._run("deb", "release", run_id="not-a-number") + self.assertEqual(code, 0) + self.assertEqual(_outputs(output)["repo_sub_folder"], "") + + def test_outputs_follow_the_stream_shape_not_its_name(self): + # Register a build_id stream that is not called "nightly". Its outputs + # must come from its shape: a dated folder and no key. + with patch.dict(build_repo_package.STREAM_SHAPES, {"weekly": "build_id"}): + with patch.dict( + get_url_repo_params._STREAM_ROOTS, + {"dev": {"stream": "weekly", "root": "https://weekly.example/rocm"}}, + ): + code, output = self._run("deb", "dev") + self.assertEqual(code, 0) + out = _outputs(output) + self.assertEqual(out["stream"], "weekly") + self.assertRegex(out["repo_sub_folder"], r"^\d{8}-12345$") + self.assertEqual(out["gpg_key_url"], "") + + def test_rejects_an_unknown_package_type(self): + with self.assertRaises(SystemExit): + with patch("sys.stderr"): + self._run("apk", "nightly") + + +# Run in a fresh interpreter: in the test session build_repo_package is already +# in sys.modules (this file imports it), so an in-process check would pass even +# if get_url_repo_params imported it at load time. +_NO_JINJA2_PROBE = """ +import os, sys, tempfile +sys.modules["jinja2"] = None # any "import jinja2" now raises ImportError +sys.path.insert(0, sys.argv[1]) +import get_url_repo_params as g +assert "build_repo_package" not in sys.modules, "loaded build_repo_package" +with tempfile.TemporaryDirectory() as d: + os.environ["GITHUB_OUTPUT"] = os.path.join(d, "github_output") + assert g.main(["extract-gfx-arch", "--artifact-group", "gfx94X-dcgpu"]) == 0 + assert g.main(["get-container-image", "--os-profile", "ubuntu2404"]) == 0 + try: + g.main(["get-repo-params", "--pkg-type", "deb", "--release-type", "nightly", + "--run-id", "1"]) + except ImportError: + print("ok") +""" + + +class WithoutJinja2Test(unittest.TestCase): + """The script's other subcommands run without jinja2. + + test_native_linux_packages_install.yml runs extract-gfx-arch and + get-container-image in a job that installs no Python packages. Only + get-repo-params needs + build_repo_package, and it must fail when called, not when imported. + """ - def test_subcommand_rejects_non_list_json(self): - code, _ = _run_main_with_output( - ["get-container-image-map", "--os-profiles", '{"a": 1}'] + def test_other_subcommands_run_and_get_repo_params_fails_at_call_time(self): + result = subprocess.run( + [sys.executable, "-c", _NO_JINJA2_PROBE, str(_LINUX_DIR)], + capture_output=True, + text=True, + timeout=120, ) - self.assertEqual(code, 1) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip().splitlines()[-1], "ok") if __name__ == "__main__": diff --git a/build_tools/packaging/linux/tests/native_linux_package_install_ut_test.py b/build_tools/packaging/linux/tests/native_linux_package_install_ut_test.py index 1a11314a440..c881004f2d7 100644 --- a/build_tools/packaging/linux/tests/native_linux_package_install_ut_test.py +++ b/build_tools/packaging/linux/tests/native_linux_package_install_ut_test.py @@ -15,7 +15,7 @@ import sys import tempfile import unittest -from pathlib import Path +from pathlib import Path, PurePosixPath from unittest.mock import patch, MagicMock # Load the module: look in same dir as this file, then parent (covers linux/ or linux/tests/ layout). @@ -46,6 +46,12 @@ sys.modules["native_linux_package_install_test"] = native_linux_package_install_test _spec.loader.exec_module(native_linux_package_install_test) +# The harness's package mode reads its contract from these two modules; the +# tests compare against them directly. Loading the harness put its directory on +# sys.path. +import build_repo_package # noqa: E402 +import get_url_repo_params # noqa: E402 + def _noop_print(*args, **kwargs): """No-op replacement for print to suppress script output during tests.""" @@ -214,10 +220,10 @@ def test_gpg_import_uses_no_shell(self): def test_package_keyring_is_separate_from_the_harness_keyring(self): # The amdrocm-repo package ships its own keyring in a different # directory; the two must not be conflated. - self.assertEqual( - self.mod.AMDROCM_DEB_KEYRING, "/usr/share/keyrings/amdrocm.gpg" - ) - self.assertNotEqual(self.mod.AMDROCM_DEB_KEYRING, self.mod.APT_KEYRING_FILE) + package_keyring = PurePosixPath(build_repo_package.DEB_KEYRING_PATH) + harness_keyring = PurePosixPath(self.mod.APT_KEYRING_FILE) + self.assertEqual(str(package_keyring), "/usr/share/keyrings/amdrocm.gpg") + self.assertNotEqual(package_keyring.parent, harness_keyring.parent) class NormalizeTestTypeTest(unittest.TestCase): @@ -2544,6 +2550,36 @@ def test_repo_package_dir_rejected_with_simulate(self): ] ) + def test_repo_package_dir_rejects_a_line_without_a_stream(self): + # ci has no public repository, so there is no package whose config the + # harness could check. Fail at the CLI rather than mid-install. + with self.assertRaisesRegex(ValueError, "release, prerelease, nightly"): + self._parse( + [ + "--os-profile", + "rhel10", + "--release-type", + "ci", + "--repo-package-dir", + "/pkgs", + "--repo-config-only", + ] + ) + + def test_repo_package_dir_rejects_a_missing_release_type(self): + # --release-type has no default; without this check the None would reach + # the constructor's .lower() as an AttributeError. + with self.assertRaisesRegex(ValueError, "requires --release-type"): + self._parse( + [ + "--os-profile", + "rhel10", + "--repo-package-dir", + "/pkgs", + "--repo-config-only", + ] + ) + def test_constructor_tolerates_none_repo_url(self): runner = native_linux_package_install_test.NativeLinuxPackageInstallTest( os_profile="rhel10", @@ -2557,22 +2593,154 @@ def test_constructor_tolerates_none_repo_url(self): self.assertTrue(runner.repo_config_only) +def _package_runner(os_profile, release_type, pkg_dir="/pkgs"): + return native_linux_package_install_test.NativeLinuxPackageInstallTest( + os_profile=os_profile, + repo_url="", + release_type=release_type, + repo_package_dir=str(pkg_dir), + ) + + +class RepoPackageContractTest(unittest.TestCase): + """_repo_package_contract(): the names the harness looks for, per line. + + Literal values on purpose. These are the paths a user's system ends up with, + so a change to any of them should have to change this test too. + """ + + def _contract(self, os_profile, release_type): + return _package_runner(os_profile, release_type)._repo_package_contract() + + def test_deb_per_line(self): + cases = { + "release": ("stable", "amdrocm-stable", True), + "prerelease": ("rc", "amdrocm-stablerc", True), + "nightly": ("nightly", "amdrocm-nightly", False), + } + for release_type, (stream, repo_id, signed) in cases.items(): + with self.subTest(release_type=release_type): + c = self._contract("ubuntu2404", release_type) + self.assertEqual(c.stream, stream) + self.assertEqual(c.repo_id, repo_id) + self.assertEqual( + c.repo_file, f"/etc/apt/sources.list.d/{repo_id}.sources" + ) + self.assertEqual(c.key_file, "/usr/share/keyrings/amdrocm.gpg") + self.assertIs(c.signed, signed) + + def test_rpm_per_distro_directory(self): + cases = { + "rhel8": "/etc/yum.repos.d/amdrocm-stablerc.repo", + "rhel10": "/etc/yum.repos.d/amdrocm-stablerc.repo", + "sles16": "/etc/zypp/repos.d/amdrocm-stablerc.repo", + } + for os_profile, repo_file in cases.items(): + with self.subTest(os_profile=os_profile): + c = self._contract(os_profile, "prerelease") + self.assertEqual(c.repo_file, repo_file) + self.assertEqual(c.key_file, "/etc/pki/rpm-gpg/RPM-GPG-KEY-amdrocm") + self.assertIs(c.signed, True) + + def test_paths_are_posix_on_every_host(self): + # These name paths on the target system. Built with Path, they would + # render with backslashes on a Windows host. + c = self._contract("rhel10", "nightly") + self.assertNotIn("\\", c.repo_file) + self.assertEqual(str(PurePosixPath(c.repo_file)), c.repo_file) + + def test_line_without_a_stream_raises(self): + with self.assertRaisesRegex(ValueError, "no public amdrocm-repo stream"): + self._contract("rhel10", "ci") + + +def _installed_paths(pkg_type, context): + """Paths the rendered package installs, read from its own manifest. + + deb: debian/install lines are " /". rpm: the spec's %files + section, minus directives. + """ + env = build_repo_package.get_jinja_env() + if pkg_type == "deb": + text = env.get_template("template/repo/deb/install.j2").render(context) + paths = set() + for line in text.splitlines(): + if line.strip(): + src, dest = line.split() + paths.add(str(PurePosixPath(dest) / PurePosixPath(src).name)) + return paths + text = env.get_template("template/repo/rpm/amdrocm-repo.spec.j2").render(context) + files = text.split("\n%files", 1)[1].split("\n%changelog", 1)[0] + paths = set() + for line in files.splitlines()[1:]: + line = line.strip() + if line.startswith("%config"): + line = line.split(None, 1)[1] + if line and not line.startswith("%"): + paths.add(line) + return paths + + +class RepoPackageContractMatchesBuilderTest(unittest.TestCase): + """The harness checks exactly the files the package installs. + + Renders the package's manifest with the builder's own templates for every + profile and every stream a line maps to, so neither side is written into + the test. + """ + + def test_every_profile_and_stream(self): + for release_type in get_url_repo_params.get_public_repo_release_types(): + stream = get_url_repo_params.get_public_repo_stream(release_type) + signed = build_repo_package.is_signed(stream) + build_id = build_repo_package.stream_shape(stream) == "build_id" + for os_profile, profile in build_repo_package.OS_PROFILES.items(): + with self.subTest(release_type=release_type, os_profile=os_profile): + argv = [ + "--os-profile", + os_profile, + "--stream", + stream, + "--repo-base-url", + "https://example.com/rocm/core/packages", + "--rocm-version", + "10.0.0", + "--dest-dir", + "/tmp/out", + ] + if signed: + argv += [ + "--gpg-key-url", + "https://example.com/rocm/gpg/packages.gpg", + ] + if build_id: + argv += ["--repo-sub-folder", "20260101-1"] + context = build_repo_package.build_context( + build_repo_package.parse_args(argv), profile + ) + installed = _installed_paths(profile["pkg_type"], context) + + c = _package_runner( + os_profile, release_type + )._repo_package_contract() + self.assertIn(c.repo_file, installed) + if signed: + self.assertIn(c.key_file, installed) + else: + self.assertNotIn(c.key_file, installed) + + class InstallRepoPackageTest(unittest.TestCase): """Tests for install_repo_package() command construction (mocked subprocess).""" - def _runner(self, os_profile, pkg_dir): - return native_linux_package_install_test.NativeLinuxPackageInstallTest( - os_profile=os_profile, - repo_url="", - release_type="prerelease", - repo_package_dir=str(pkg_dir), - ) + def _runner(self, os_profile, pkg_dir, release_type="prerelease"): + return _package_runner(os_profile, release_type, pkg_dir) @patch("native_linux_package_install_test._run_streaming") def test_deb_installs_then_apt_update(self, mock_streaming): mock_streaming.return_value = 0 with tempfile.TemporaryDirectory() as d: - pkg = Path(d) / "amdrocm-repo_7.14.0_all.deb" + pkg = Path(d) / "amdrocm-repo_10.0.0~pre-1~ubuntu2404_all.deb" pkg.write_text("") runner = self._runner("ubuntu2404", d) with _suppress_script_output(): @@ -2584,30 +2752,36 @@ def test_deb_installs_then_apt_update(self, mock_streaming): self.assertEqual(refresh_cmd, ["sudo", "apt", "update"]) @patch("native_linux_package_install_test._run_streaming") - def test_dnf_installs_nogpgcheck_then_scoped_makecache(self, mock_streaming): + def test_dnf_installs_nogpgcheck_then_refreshes_the_stream_repo( + self, mock_streaming + ): mock_streaming.return_value = 0 with tempfile.TemporaryDirectory() as d: - pkg = Path(d) / "amdrocm-repo-7.14.0-1.el10.noarch.rpm" + pkg = Path(d) / "amdrocm-repo-20260716-12345.1.nightly.el10.noarch.rpm" pkg.write_text("") - runner = self._runner("rhel10", d) + runner = self._runner("rhel10", d, release_type="nightly") with _suppress_script_output(): self.assertTrue(runner.install_repo_package()) install_cmd, refresh_cmd = (c.args[0] for c in mock_streaming.call_args_list) self.assertEqual( install_cmd, ["dnf", "install", "-y", "--nogpgcheck", str(pkg.resolve())] ) + # The repo id the package registers is [amdrocm-nightly]; a bare + # "amdrocm" makes dnf fail with "Unknown repo". self.assertEqual( refresh_cmd, - ["dnf", "makecache", "--disablerepo=*", "--enablerepo=amdrocm"], + ["dnf", "makecache", "--disablerepo=*", "--enablerepo=amdrocm-nightly"], ) @patch("native_linux_package_install_test._run_streaming") - def test_sles_installs_no_gpg_checks_then_refresh(self, mock_streaming): + def test_sles_installs_no_gpg_checks_then_refreshes_the_stream_repo( + self, mock_streaming + ): mock_streaming.return_value = 0 with tempfile.TemporaryDirectory() as d: - pkg = Path(d) / "amdrocm-repo-7.14.0-1.noarch.rpm" + pkg = Path(d) / "amdrocm-repo-10.0.0-1.stable.sles16.noarch.rpm" pkg.write_text("") - runner = self._runner("sles16", d) + runner = self._runner("sles16", d, release_type="release") with _suppress_script_output(): self.assertTrue(runner.install_repo_package()) install_cmd, refresh_cmd = (c.args[0] for c in mock_streaming.call_args_list) @@ -2629,10 +2803,19 @@ def test_sles_installs_no_gpg_checks_then_refresh(self, mock_streaming): "--non-interactive", "--gpg-auto-import-keys", "refresh", - "amdrocm", + "amdrocm-stable", ], ) + @patch("native_linux_package_install_test._run_streaming") + def test_line_without_a_stream_fails_without_running(self, mock_streaming): + with tempfile.TemporaryDirectory() as d: + (Path(d) / "amdrocm-repo_1_all.deb").write_text("") + runner = self._runner("ubuntu2404", d, release_type="ci") + with _suppress_script_output(): + self.assertFalse(runner.install_repo_package()) + mock_streaming.assert_not_called() + @patch("native_linux_package_install_test._run_streaming") def test_missing_package_fails_without_running(self, mock_streaming): with tempfile.TemporaryDirectory() as d: @@ -2707,127 +2890,152 @@ def test_refresh_retries_on_timeout(self, mock_streaming, mock_sleep): class AssertRepoConfiguredTest(unittest.TestCase): - """Tests for assert_repo_configured() (on-disk contract check).""" + """Tests for assert_repo_configured()'s checks on the files it finds. - def _runner(self, os_profile, release_type, pkg_dir="/pkgs"): - return native_linux_package_install_test.NativeLinuxPackageInstallTest( - os_profile=os_profile, - repo_url="", - release_type=release_type, - repo_package_dir=pkg_dir, - ) + Where it looks is not tested here: each test points the contract at temp + files. The location is covered by RepoPackageContractTest (literal paths) + and RepoPackageContractMatchesBuilderTest (the package's own manifest). + """ + + def _check(self, os_profile, release_type, repo_file, key_file): + runner = _package_runner(os_profile, release_type) + real = runner._repo_package_contract() + contract = real._replace(repo_file=str(repo_file), key_file=str(key_file)) + with patch.object(runner, "_repo_package_contract", return_value=contract): + with _suppress_script_output(): + return runner.assert_repo_configured() def test_deb_signed_requires_sources_and_keyring(self): with tempfile.TemporaryDirectory() as d: - sources = Path(d) / "amdrocm.sources" + sources = Path(d) / "amdrocm-stablerc.sources" sources.write_text( "Types: deb\n" - "URIs: https://rocm.prereleases.amd.com/packages-multi-arch/ubuntu2404/\n" + "URIs: https://rc.repo.amd.com/rocm/core/packages/ubuntu2404/\n" "Signed-By: /usr/share/keyrings/amdrocm.gpg\n" ) keyring = Path(d) / "amdrocm.gpg" keyring.write_text("key") - runner = self._runner("ubuntu2404", "prerelease") - with patch.multiple( - native_linux_package_install_test, - AMDROCM_DEB_SOURCES=str(sources), - AMDROCM_DEB_KEYRING=str(keyring), - ): - with _suppress_script_output(): - self.assertTrue(runner.assert_repo_configured()) - keyring.unlink() # signed line but keyring missing -> fail - with _suppress_script_output(): - self.assertFalse(runner.assert_repo_configured()) + self.assertTrue(self._check("ubuntu2404", "prerelease", sources, keyring)) + keyring.unlink() # signed stream but keyring missing -> fail + self.assertFalse(self._check("ubuntu2404", "prerelease", sources, keyring)) def test_deb_missing_sources_fails(self): - runner = self._runner("ubuntu2404", "prerelease") - with patch.multiple( - native_linux_package_install_test, - AMDROCM_DEB_SOURCES="/nonexistent/amdrocm.sources", - AMDROCM_DEB_KEYRING="/nonexistent/amdrocm.gpg", - ): - with _suppress_script_output(): - self.assertFalse(runner.assert_repo_configured()) + self.assertFalse( + self._check( + "ubuntu2404", + "prerelease", + "/nonexistent/amdrocm-stablerc.sources", + "/nonexistent/amdrocm.gpg", + ) + ) def test_deb_nightly_needs_no_keyring(self): with tempfile.TemporaryDirectory() as d: - sources = Path(d) / "amdrocm.sources" + sources = Path(d) / "amdrocm-nightly.sources" sources.write_text("URIs: https://x/rocm/deb/\nTrusted: yes\n") - runner = self._runner("ubuntu2404", "nightly") - with patch.multiple( - native_linux_package_install_test, - AMDROCM_DEB_SOURCES=str(sources), - AMDROCM_DEB_KEYRING="/nonexistent/amdrocm.gpg", - ): - with _suppress_script_output(): - self.assertTrue(runner.assert_repo_configured()) + self.assertTrue( + self._check( + "ubuntu2404", "nightly", sources, "/nonexistent/amdrocm.gpg" + ) + ) def test_structurally_invalid_repo_file_fails(self): # A repo file missing the expected deb822 marker (URIs:) is rejected, # independent of whether the host string contains "rocm". with tempfile.TemporaryDirectory() as d: - sources = Path(d) / "amdrocm.sources" + sources = Path(d) / "amdrocm-nightly.sources" sources.write_text("not a valid deb822 sources file\n") - runner = self._runner("ubuntu2404", "nightly") - with patch.multiple( - native_linux_package_install_test, - AMDROCM_DEB_SOURCES=str(sources), - AMDROCM_DEB_KEYRING="/nonexistent/amdrocm.gpg", - ): - with _suppress_script_output(): - self.assertFalse(runner.assert_repo_configured()) + self.assertFalse( + self._check( + "ubuntu2404", "nightly", sources, "/nonexistent/amdrocm.gpg" + ) + ) + + def test_marker_only_in_a_comment_fails(self): + with tempfile.TemporaryDirectory() as d: + sources = Path(d) / "amdrocm-nightly.sources" + sources.write_text("# URIs: https://x/\nTypes: deb\n") + self.assertFalse( + self._check( + "ubuntu2404", "nightly", sources, "/nonexistent/amdrocm.gpg" + ) + ) def test_mirror_host_without_rocm_still_passes(self): # deb822 with a rocm-less mirror host is valid: the marker check must # not reject it just because the URL lacks "rocm". with tempfile.TemporaryDirectory() as d: - sources = Path(d) / "amdrocm.sources" + sources = Path(d) / "amdrocm-nightly.sources" sources.write_text( "Types: deb\n" - "URIs: https://mirror.example.com/packages-multi-arch/deb/20260716-1/\n" + "URIs: https://mirror.example.com/rocm/core/packages/ubuntu2404/20260716-1/\n" "Suites: stable\n" "Trusted: yes\n" ) - runner = self._runner("ubuntu2404", "nightly") - with patch.multiple( - native_linux_package_install_test, - AMDROCM_DEB_SOURCES=str(sources), - AMDROCM_DEB_KEYRING="/nonexistent/amdrocm.gpg", - ): - with _suppress_script_output(): - self.assertTrue(runner.assert_repo_configured()) - - def test_rpm_signed_checks_yum_dir_and_key(self): + self.assertTrue( + self._check( + "ubuntu2404", "nightly", sources, "/nonexistent/amdrocm.gpg" + ) + ) + + def test_rpm_signed_requires_repo_file_and_key(self): with tempfile.TemporaryDirectory() as d: - repo = Path(d) / "amdrocm.repo" + repo = Path(d) / "amdrocm-stable.repo" repo.write_text( - "[amdrocm]\nbaseurl=https://x/packages-multi-arch/rpm/x86_64/\n" + "[amdrocm-stable]\n" + "baseurl=https://stable.repo.amd.com/rocm/core/packages/rhel10/x86_64/\n" ) key = Path(d) / "RPM-GPG-KEY-amdrocm" key.write_text("key") - runner = self._runner("rhel10", "prerelease") - with patch.multiple( - native_linux_package_install_test, - YUM_REPOS_DIR=d, - AMDROCM_RPM_KEY=str(key), - ): - with _suppress_script_output(): - self.assertTrue(runner.assert_repo_configured()) - - def test_rpm_sles_uses_zypp_dir(self): - with tempfile.TemporaryDirectory() as d: - repo = Path(d) / "amdrocm.repo" - repo.write_text("[amdrocm]\nbaseurl=https://x/rocm/rpm/x86_64/\n") - key = Path(d) / "RPM-GPG-KEY-amdrocm" - key.write_text("key") - runner = self._runner("sles16", "prerelease") - with patch.multiple( - native_linux_package_install_test, - ZYPP_REPOS_DIR=d, - AMDROCM_RPM_KEY=str(key), - ): - with _suppress_script_output(): - self.assertTrue(runner.assert_repo_configured()) + self.assertTrue(self._check("rhel10", "release", repo, key)) + key.unlink() + self.assertFalse(self._check("rhel10", "release", repo, key)) + + def test_line_without_a_stream_fails(self): + runner = _package_runner("rhel10", "ci") + with _suppress_script_output(): + self.assertFalse(runner.assert_repo_configured()) + + +# Run in a fresh interpreter: in the test session build_repo_package is already +# in sys.modules (this file and build_repo_package_test.py import it), so an +# in-process check would pass even if the harness imported it at load time. +_NO_JINJA2_PROBE = """ +import importlib.util, sys +sys.modules["jinja2"] = None # any "import jinja2" now raises ImportError +linux_dir = sys.argv[1] +sys.path.insert(0, linux_dir) +spec = importlib.util.spec_from_file_location( + "harness", linux_dir + "/native_linux_package_install_test.py" +) +harness = importlib.util.module_from_spec(spec) +spec.loader.exec_module(harness) +harness.parse_cli_arguments( + ["--test-type", "simulate", "--packages-dir", "/p", "--pkg-type", "deb"], + raise_instead_of_exit=True, +) +assert "build_repo_package" not in sys.modules, "loaded build_repo_package" +print("ok") +""" + + +class HarnessWithoutJinja2Test(unittest.TestCase): + """The harness loads and runs its non-package modes without jinja2. + + Only the package mode needs build_repo_package (and the jinja2 it + imports), so only it may import it; the other modes keep working in an + environment that has neither. + """ + + def test_loads_and_parses_without_jinja2(self): + result = subprocess.run( + [sys.executable, "-c", _NO_JINJA2_PROBE, str(_module_path.parent)], + capture_output=True, + text=True, + timeout=120, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), "ok") class RunRepoSetupDispatchTest(unittest.TestCase): diff --git a/build_tools/packaging/linux/tests/requirements.txt b/build_tools/packaging/linux/tests/requirements.txt index 35198dd067f..c11b5e79ab1 100644 --- a/build_tools/packaging/linux/tests/requirements.txt +++ b/build_tools/packaging/linux/tests/requirements.txt @@ -3,3 +3,5 @@ pytest>=9.0.3 requests prettytable PyYAML +# The amdrocm-repo package mode reads its contract from build_repo_package. +jinja2>=3.0.0 diff --git a/build_tools/tests/publish_repo_package_test.py b/build_tools/tests/publish_repo_package_test.py index 4a2b978f168..816d3b50caf 100644 --- a/build_tools/tests/publish_repo_package_test.py +++ b/build_tools/tests/publish_repo_package_test.py @@ -215,9 +215,7 @@ def test_publish_dry_run_writes_nothing(tmp_path, monkeypatch): def test_publish_dry_run_does_not_claim_to_have_published( tmp_path, monkeypatch, capsys ): - # The publishing job runs under continue-on-error, so its log is the only - # signal. A dry run that prints "Published:" reports an upload that never - # happened. + # A dry run that prints "Published:" reports an upload that never happened. _stub_bucket(monkeypatch) args = prp.parse_args( _argv(tmp_path, output_dir=tmp_path / "staging", dry_run=True) @@ -272,7 +270,8 @@ def test_publish_without_output_dir_selects_s3_backend(tmp_path, monkeypatch): def test_publish_rejects_the_release_line(tmp_path, monkeypatch): # There is no artifacts bucket for the release line: every # therock-release-* bucket has iam_role=None because release upload is - # external. The publishing job is gated off it; this is the backstop. + # external. The publishing job does not run for that line; this is the + # backstop. monkeypatch.setenv("RELEASE_TYPE", "release") monkeypatch.setenv("GITHUB_REPOSITORY", "ROCm/TheRock") args = prp.parse_args(_argv(tmp_path, output_dir=tmp_path / "staging")) diff --git a/build_tools/tests/workflow_outputs_test.py b/build_tools/tests/workflow_outputs_test.py index 77c65b4879f..f648c3efead 100644 --- a/build_tools/tests/workflow_outputs_test.py +++ b/build_tools/tests/workflow_outputs_test.py @@ -194,11 +194,8 @@ def test_native_linux_repo_package_all_profiles(self): ) def test_repo_package_sits_outside_the_package_index(self): - # The bootstrap file is fetched by URL, so it must not land inside the - # repository index: deb indexes dists/ + pool/, rpm indexes x86_64/. - # One amdrocm-repo is built per OS profile and they all carry the same - # package name, so indexing them together would collide. The filenames - # differ, since the dist tag expands per profile. + # deb indexes dists/ + pool/ and rpm indexes x86_64/. Every profile's + # package has the same package name, so none may land in an index. for pkg_type, index_dirs in [ ("deb", ("dists", "pool")), ("rpm", ("x86_64",)), @@ -274,11 +271,10 @@ def test_log_dir_with_external_repo(self): ) def test_native_linux_repo_package_with_external_repo(self): - # external_repo is only ever non-empty for the 'ci' release type from a - # fork or a non-ROCm/TheRock repository, which is the only case that - # resolves to therock-ci-artifacts-external. The signed lines - # (prerelease, release) and nightly resolve to their own buckets, so - # their external_repo is "" and the key below has no leading segment. + # external_repo is non-empty only for 'ci' from a fork or a repository + # other than ROCm/TheRock (therock-ci-artifacts-external). prerelease and + # nightly have their own buckets with an empty external_repo; release + # has no artifacts bucket at all. root = WorkflowOutputRoot( bucket="therock-ci-artifacts-external", external_repo="Fork-TheRock/",