Repository navigation
feat(packaging): add amdrocm-repo package for ROCm repository setup - #6901
lsudarsh-amd wants to merge 2 commits into
Conversation
Adds a configuration package that points apt, dnf, or zypper to a public ROCm repository, so ROCm can be installed and updated with the native package manager instead of a manual multi-step repository setup. The package is generated per OS profile and release line. Repository URLs are derived per line, because the prerelease and release repositories are published per distro while the nightly repository is published per package type under a dated sub-folder. The package version carries the release line, so packages built from different lines are never confusable. For signed lines the signing key is fetched at build time, verified against a pinned fingerprint, and embedded. It is never stored in the source tree. This targets the repository layout currently served. That layout is expected to change, so URL derivation is isolated in one function to keep the migration small.
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
therock-pr-bot
Bot
added
the
Not ready to Review
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
| @@ -0,0 +1,3 @@ | |||
| #!/usr/bin/make -f | |||
| %: | |||
| dh $@ | |||
There was a problem hiding this comment.
DEB rules — missing --root-owner-group
.sources and rocm.gpg files may be owned by the build user instead of root
The new template/repo/deb/rules.j2 is:
#!/usr/bin/make -f
%:
dh $@The existing template/debian_rules.j2 used by all other ROCm packages has:
override_dh_builddeb:
dh_builddeb -- -Zxz --root-owner-groupThere was a problem hiding this comment.
Good catch, thanks for pointing this out. I'll add the permissions setup.
| mkdir -p %{buildroot}/etc/pki/rpm-gpg | ||
| install -m 644 %{_sourcedir}/RPM-GPG-KEY-rocm %{buildroot}{{ rpm_gpg_key_path }} | ||
| {% endif %} | ||
|
|
There was a problem hiding this comment.
RPM spec — missing %defattr(-, root, root, -)
.repo and GPG key files may be non-root owned if rpmbuild
runs as non-root**
The existing rpm_specfile.j2 has %defattr(-, root, root, -) in %files
to ensure all packaged files are owned root:root. The new repo spec omits it.
There was a problem hiding this comment.
Good catch, thanks for pointing this out. I'll add the permissions setup.
|
Its a huge change. it took me couple of hours to get a fair understanding on the PR details. Thought will put the details i found important here with the help of AI so that it can help other reviewers. What is the amdrocm-repo package?A lightweight "bootstrap" package — installs no ROCm software itself, only One package is built per OS profile ( Files inside the packageDEB package (
|
| Installed path | Description |
|---|---|
/etc/apt/sources.list.d/amdrocm.sources |
Repository definition (deb822 format) |
/usr/share/keyrings/rocm.gpg |
AMD signing key — binary (dearmored), signed lines only |
RPM package (.rpm)
| Installed path | Description |
|---|---|
/etc/yum.repos.d/amdrocm.repo |
Repository definition (INI format) |
/etc/pki/rpm-gpg/RPM-GPG-KEY-rocm |
AMD signing key — armored ASCII, signed lines only |
Package content by build type
DEB — amdrocm.sources content
Types: deb
URIs: <baseurl>
Suites: stable
Components: main
Architectures: amd64
Signed-By: /usr/share/keyrings/rocm.gpg ← prerelease / release
# OR
Trusted: yes ← nightly (unsigned)
RPM — amdrocm.repo content
[amdrocm]
name=AMD ROCm
baseurl=<baseurl>
enabled=1
gpgcheck=1 ← prerelease / release
gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-rocm
# OR
gpgcheck=0 ← nightly (unsigned)Repository URL (<baseurl>) per build type
| Release type | DEB baseurl | RPM baseurl |
|---|---|---|
| ci | https://rocm.prereleases.amd.com/packages-multi-arch/ubuntu2404/ |
https://rocm.prereleases.amd.com/packages-multi-arch/rhel10/x86_64/ |
| prerelease | same as ci | same as ci |
| nightly | https://rocm.nightlies.amd.com/packages-multi-arch/deb/YYYYMMDD-<id>/ |
https://rocm.nightlies.amd.com/packages-multi-arch/rpm/YYYYMMDD-<id>/x86_64/ |
| release | https://repo.amd.com/rocm/packages-multi-arch/ubuntu2404/ |
https://repo.amd.com/rocm/packages-multi-arch/rhel10/x86_64/ |
Signing key embedded?
| Release type | Signed? | Key embedded? |
|---|---|---|
| ci | ✅ (prerelease key) | ✅ |
| prerelease | ✅ | ✅ |
| nightly | ❌ | ❌ |
| release | ✅ | ✅ |
Note on DEB format: The
.sourcesextension (deb822 multi-line format)
is the modern APT format introduced in Debian 9 / Ubuntu 20.04, preferred
over the old single-line.listformat. Both are equivalent —aptreads
both the same way. Deb822 is cleaner for multi-value fields and makes
Signed-By:explicit per-repository.
Key logic details
GPG key download logic
The signing key is fetched at build time from <repo_base_url>/gpg/rocm.gpg
and never stored in the source tree.
Steps:
- Enforce HTTPS — rejects any non-HTTPS URL before fetching
- Fetch with retries — up to 3 attempts with exponential backoff; transient
network errors retried, non-200/non-retryable HTTP errors fail immediately - Re-check HTTPS after redirects — prevents a HTTPS→HTTP silent downgrade
- Size bound — reads at most 1 MiB; rejects oversized responses
- Fingerprint verification — checks the fetched key against pinned
fingerprintD0F004A0025A1145C7807FCD0701EAC4D5E02107; any mismatch fails
the build, ensuring only the expected AMD ROCm key is embedded - DEB only — dearmors the key (
gpg --dearmor) to binary before embedding
at/usr/share/keyrings/rocm.gpg(DEB requires binary keyrings for
Signed-By:) - RPM — embeds armored key as-is at
/etc/pki/rpm-gpg/RPM-GPG-KEY-rocm
--gpg-key-file allows supplying the key from disk for offline/test builds,
skipping the fetch and fingerprint check.
4-step workflow in multi_arch_build_native_linux_packages.yml
Step 1 — setup_repo_params (always runs)
What: Derives the build matrix and parameters for the repo package:
- OS profiles for this package type (e.g.
["ubuntu2404", "rhel10", "sles16"]) - Container image per profile (per-distro build environment)
- Public repo base URL for this release line (empty for ci/dev — they have no
public repo) - Nightly sub-folder (
YYYYMMDD-<run_id>) — only for nightly, where the dated
sub-folder is baked into the repository URL
Why: Centralises derivation so all downstream jobs get consistent values
without each re-deriving independently.
Step 2 — test_repo_package (CI/PR builds only, release_type == 'ci')
What: Builds against the live prerelease repository (regardless of the
triggering CI context), installs the package in a per-distro container, and
verifies it configures the repository correctly. Stops before actually
installing ROCm — config-only validation.
Why: PR builds need a real repository to test against. Using the prerelease
repo exercises the same URL shape end users get. --verify-repo-url is not
passed here — the prerelease repo is assumed reachable and URL reachability is
not the concern in CI (it's tested in stage_repo_package).
Step 3 — stage_repo_package (prerelease / nightly / release)
What: Builds the amdrocm-repo package for each OS profile with real
release-line parameters, including:
--verify-repo-url— fails the build if the configured repository is not
reachable (a no-op for nightly since its dated sub-folder doesn't exist yet)- GPG key fetched, fingerprint-verified, embedded
- Built
.deb/.rpmuploaded as a GitHub Actions artifact
Why continue-on-error: true: A staging failure (e.g. the repository URL
is temporarily unreachable) must never block the release of the content
packages. The soft-fail is surfaced as a workflow warning, not a hard error.
Step 4 — publish_repo_package (prerelease / nightly only — NOT release)
What: Downloads the artifact from Step 3 and uploads it to S3:
s3://<bucket>/<run_id>-linux/packages/<pkg_type>/repo/<os_profile>/amdrocm-repo.<ext>
The fixed filename amdrocm-repo.<ext> means the download URL is stable
across versions. Calls s3.head_object after upload to confirm the file
landed.
Why not for release: The release artifacts bucket grants no write role to
CI. The release package is published manually by the ROCm release process
instead.
Why continue-on-error: true: A publish failure must not block the
content package release. Surfaced as a workflow warning.
S3 location and how it is determined
Bucket
Resolved by write_artifacts_bucket_info.py → get_artifacts_bucket_config_for_workflow_run():
| Release type | Bucket |
|---|---|
| ci | therock-ci-artifacts |
| dev | therock-dev-artifacts |
| nightly | therock-nightly-artifacts |
| prerelease | therock-prerelease-artifacts |
| release | therock-release-artifacts (no write role — manual only) |
Prefix and full key
<ARTIFACT_RUN_ID>-linux/packages/<pkg_type>/repo/<os_profile>/amdrocm-repo.<ext>
Example for prerelease DEB ubuntu2404, run 29284182052:
s3://therock-prerelease-artifacts/29284182052-linux/packages/deb/repo/ubuntu2404/amdrocm-repo.deb
The prefix <run_id>-linux/packages/<pkg_type> is constructed in the
workflow YAML and passed as --prefix to publish_repo_package.py. The
script appends /repo/<os_profile>/amdrocm-repo.<ext> to form the final key.
Difference vs upload_package_repo.py (content packages)
Content packages (upload_package_repo.py) |
Repo package (publish_repo_package.py) |
|
|---|---|---|
| Bucket | Derived internally via WorkflowOutputRoot |
Passed externally via --bucket |
| Prefix | Derived internally via WorkflowOutputRoot.native_linux_packages() |
Constructed in workflow YAML, passed via --prefix |
| What is uploaded | Full repo tree (dists/, x86_64/repodata/, all .deb/.rpm) |
Single file amdrocm-repo.<ext> |
| Post-upload check | None | s3.head_object confirms upload |
| Metadata regeneration | Yes — merges Packages.gz / repomd.xml |
No — single file, no index |
| Deduplication | Yes — skips packages already in S3 | No — always overwrites (fixed filename) |
Both land in the same <run_id>-linux/packages/<pkg_type>/ prefix tree under
the same artifacts bucket. The repo package sits as a sibling at
repo/<os_profile>/ alongside the content repo (dists/ for DEB,
x86_64/ for RPM). The same promotion step that copies content packages to
the public CDN picks up the repo package as well.
ScottTodd
left a comment
ScottTodd
left a comment
There was a problem hiding this comment.
Yeah there's a lot going on here. Please file a github issue for this feature request and get alignment on the goals and implementation approach before we proceed any further with reviewing the code.
| @@ -1 +1 @@ | |||
| repo | |||
| /repo | |||
There was a problem hiding this comment.
Rather than make edit, we can just remove this .gitignore file now. We haven't used the repo tool since #128
There was a problem hiding this comment.
Sounds good. I've removed this file, should be reflected in the follow up PR.
There was a problem hiding this comment.
Its a huge change. it took me couple of hours to get a fair understanding on the PR details.
+1, please follow https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md#using-github-issues-for-feature-development and file a tracking issue announcing intent to work on complex changes first. Then, this could be split into at least two PRs:
- The code to build the new package
- The workflow changes that use that code (this could also be flag guarded)
There was a problem hiding this comment.
Understood. I've opened a new github issue to track the repo package work here: #6986. This is meant to track the initial phase of work to integrate the main framework for the repo package and service the current repo layout. I'll create a separate github issue to track the work around updating the content of the repo package for the new repo layout.
That's a fair point, this is a big change, and it should be divided up into smaller changesets. I'm working on splitting this up into a few PRs:
- The package builder, templates, unit tests, and documentation.
- Publishing support: a WorkflowOutputRoot method for the new output location, the publish script, and the install test harness changes.
- The workflow changes that build and publish the package.
- Adding build_tools/packaging/linux/tests/ to the pytest collection paths so the unit tests in that directory actually run. This will also contribute to Organize project unit tests so all are run continuously #6927. Currently tracked by PR fix(ci): run get_url_repo_params tests and correct stale assertions #7004.
I'll close this PR and use the others to track this work. Thanks for the initial review and feedback!
| # Build the amdrocm-repo package and verify it configures the repository, for | ||
| # each OS profile of this package type. This runs on pull-request builds and is | ||
| # a required check. It builds against the live prerelease repository, which is | ||
| # published in the same per-distro layout the release line uses, so this | ||
| # exercises the shape end users get. The nightly and release lines are covered | ||
| # by stage_repo_package, which verifies their repository URL at build time. | ||
| test_repo_package: | ||
| name: Test amdrocm-repo config (${{ matrix.os_profile }}) | ||
| needs: [setup_repo_params] | ||
| if: inputs.release_type == 'ci' |
There was a problem hiding this comment.
We're not going to make test_repo_package a required check on CI. In TheRock currently only pre-commit and unit-tests are required, and we may make ci_summary required in the future:
TheRock/.github/workflows/multi_arch_ci.yml
Lines 166 to 173 in 6a7a3ed
Required checks must be run on every pull request, and we shouldn't require that documentation changes, changes that only affect Windows, changes that only affect PyTorch Linux builds, etc. run this job.
Tests for native linux packages should go in one of these places:
- In or near
Simulated install Testunder the existingbuild_native_packagesjob just above in this file - https://github.com/ROCm/TheRock/blob/main/.github/workflows/test_native_linux_packages_install.yml
- https://github.com/ROCm/TheRock/blob/main/.github/workflows/unit_tests.yml via tests like https://github.com/ROCm/TheRock/blob/main/build_tools/packaging/linux/tests/native_linux_package_install_ut_test.py (not currently running? see Organize project unit tests so all are run continuously #6927)
- https://github.com/ROCm/TheRock/blob/main/.github/workflows/test_artifacts_structure.yml via tests like https://github.com/ROCm/TheRock/blob/main/tests/test_artifact_structure.py
| - name: Resolve prerelease repository base URL | ||
| id: prerelease | ||
| run: | | ||
| $PYTHON_CMD build_tools/packaging/linux/get_url_repo_params.py \ | ||
| get-public-repo-base-url --release-type prerelease | ||
|
|
||
| - name: Build and validate amdrocm-repo (config-only) | ||
| run: | | ||
| # Build against the live prerelease repository, install the package, and | ||
| # verify it configures the repository (config-only stops before ROCm). | ||
| dest="$(mktemp -d)" | ||
| $PYTHON_CMD build_tools/packaging/linux/build_repo_package.py \ | ||
| --os-profile "${{ matrix.os_profile }}" \ | ||
| --release-type prerelease \ | ||
| --repo-base-url "${{ steps.prerelease.outputs.repo_base_url }}" \ | ||
| --rocm-version "${{ inputs.rocm_version }}" \ | ||
| --dest-dir "$dest" | ||
| $PYTHON_CMD build_tools/packaging/linux/native_linux_package_install_test.py \ | ||
| --repo-package-dir "$dest" \ | ||
| --repo-config-only \ | ||
| --os-profile "${{ matrix.os_profile }}" \ | ||
| --release-type prerelease |
There was a problem hiding this comment.
Don't test in prod. CI should not be touching --release-type prerelease, even just to read.
There was a problem hiding this comment.
Will do, I'll make the adjustments.
| def publish(args: argparse.Namespace) -> str: | ||
| """Upload the file and confirm it landed. Returns the object key.""" | ||
| # Deferred import: boto3 is only needed for the actual upload, so key | ||
| # derivation stays importable without it. | ||
| import boto3 |
There was a problem hiding this comment.
Using an inline import to avoid a helper function not needing an import is not worth it here, since the whole purpose of the script is to publish. Follow the style guide and put imports at the top: https://github.com/ROCm/TheRock/blob/main/docs/development/style_guides/python_style_guide.md#import-organization
There was a problem hiding this comment.
I'll move this to the top.
| key = object_key(args.prefix, args.os_profile, args.pkg_type) | ||
| s3 = boto3.client("s3", endpoint_url=args.endpoint_url) | ||
| print(f"Uploading {args.file} -> s3://{args.bucket}/{key}") | ||
| s3.upload_file(str(args.file), args.bucket, key) |
There was a problem hiding this comment.
@ScottTodd how the s3 bucket determined and how the files are published is different here. I think its better to use the same WorkflowOutputRoot fns for deermining s3 bucket and prefix, same way other workflows uses. Would like to have your expert opinion here
Yes. Follow https://github.com/ROCm/TheRock/blob/main/docs/development/s3_buckets.md and https://github.com/ROCm/TheRock/blob/main/docs/development/workflow_outputs.md . Code like this should not be using boto3 or computing keys directly, it should use those classes (which allow for uploading to local directories for testing, forces users to follow the conventions, etc.).
See also the yml code on this PR that should be trimmed:
--bucket "${{ steps.bucket-info.outputs.bucket }}" \
--prefix "${ARTIFACT_RUN_ID}-linux/packages/${{ inputs.native_package_type }}" \There was a problem hiding this comment.
Sure, I'll update this to use the same methodology as the other workflows.
|
Following the review here, this work is split into four smaller pull requests, tracked by #6986:
The review comments here are addressed across those four, and each describes what changed. This branch is not being force-pushed, so the diff and the discussion stay readable for reference. Thank you @ScottTodd and @nunnikri for the initial review. The PR split and the CI rework came directly out of it. |
3 participants
Adds a configuration package that points apt, dnf, or zypper to a public ROCm repository, so ROCm can be installed and updated with the native package manager instead of a manual multi-step repository setup.
The package is generated per OS profile and release line. Repository URLs are derived per line, because the prerelease and release repositories are published per distro while the nightly repository is published per package type under a dated sub-folder. The package version carries the release line, so packages built from different lines are never confusable.
For signed lines the signing key is fetched at build time, verified against a pinned fingerprint, and embedded. It is never stored in the source tree.
This targets the repository layout currently served. That layout is expected to change, so URL derivation is isolated in one function to keep the migration small.
Motivation
Installing ROCm from a public repository means adding the repository definition
by hand, fetching and installing the signing key, then refreshing the package
manager. The sequence is easy to get wrong and awkward to script.
amdrocm-reporeplaces those steps with a single package install. It isgenerated per OS profile and release line, and the existing native-packaging
workflow builds, validates, stages, and publishes it.
Technical Details
build_repo_package.pyrenders the package from the Jinja templates undertemplate/repo/. Repository URLs are derived per release line, because thelines do not share a layout: prerelease and release are published per distro,
nightly per package type under a dated sub-folder. The package version carries
the release line so that two lines never produce an identically named package.
Without it, installing one line over another reports success and leaves the
previous repository configured.
For signed lines the signing key is fetched over https at build time,
size-bounded, and verified against a pinned fingerprint before it is embedded.
It is never stored in the source tree. Nightly is unsigned, so that package
ships no key and disables signature checking.
--verify-repo-urlfails the build unless the configured repository serves anindex, catching a package that installs cleanly but cannot refresh. It is a
no-op for nightly, whose sub-folder is published by the same run.
publish_repo_package.pyuploads the package as a standalone per-distro filebeside the content packages, outside the repository index.
setup_repo_build_deps.shinstalls the packaging tools per OS profile.docs/packaging/rocm_repo_setup.mddocuments setup for end users.The workflow gains four additive jobs: resolve parameters, validate the
configuration, stage, and publish. No existing job or input changed.
Incidentally,
.pre-commit-config.yamlexcludestemplate/repo/deb/rules.j2from
forbid-tabs, since a debianrulesfile is a Makefile and needs hardtabs.
build_tools/.gitignoreanchorsrepoto/reposo it stops matchingtemplate/repo/.requirements-test.txtgainsjinja2, which the new testsimport.
Test Plan
Unit tests cover URL derivation, signing-key handling, input validation,
template rendering, and the S3 key layout.
A new required CI check builds the package for every OS profile and verifies it
configures the repository against the live prerelease repository. It needs no
GPU and does not install ROCm.
Containerized runs then repeat that against every release line, going as far as
resolving the ROCm dependency tree and checking signatures.
Test Result
Unit tests: 291 passed. Ten failures elsewhere are pre-existing on
main,confirmed by running the same suites from a clean
origin/maintree andcomparing failure sets.
Signing is verified rather than assumed.
apt-get updatevalidatesInReleasethrough
Signed-By, andrpm --checksigreportssignatures OKon the threerpm profiles. For nightly the opposite is asserted, with no key shipped and
verification disabled.
Negative paths behave correctly. An unreachable repository fails the build with
no package produced, a malformed
--rocm-versionis rejected, and a missinggpgnames the tool rather than raising a traceback. Installing release overprerelease at the same
--rocm-versionupgrades the package and switches theconfigured repository. Publishing was verified against a local S3-compatible
endpoint using environment credentials only, matching the OIDC path.
Submission Checklist
Closes #6986.