Skip to content

[ci] Fixing dep rpm packages for forked upload - #5066

Merged
geomin12 merged 11 commits into
ROCm:mainfrom
geomin12:users/geomin12/fix-rpm-deb
May 15, 2026
Merged

geomin12 merged 11 commits into
ROCm:mainfrom
geomin12:users/geomin12/fix-rpm-deb

Conversation

@geomin12

@geomin12 geomin12 commented May 5, 2026

Copy link
Copy Markdown
Contributor

Noticied that RPM and DEB packages fail to upload: https://github.com/ROCm/TheRock/actions/runs/25237692770/job/74244690857?pr=4998 due to the build image not correctly configured (or even used!).

This change uses a proper docker image as well as proper credentials for forked repos

@ScottTodd ScottTodd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. That would do it... Thanks.

Please wait for CI to complete on this. I have some doubts about the "Install Python requirements" and "Install System requirements" steps working in the container 😅

@ScottTodd

Copy link
Copy Markdown
Member

Oops, CI didn't run on this. Need to edit

# GitHub workflow file patterns that are considered CI-related.
# Changes to workflow files matching these patterns will trigger CI runs,
# as they may affect the CI pipeline itself.
_GITHUB_WORKFLOWS_CI_PATTERNS = [
"setup.yml",
"ci*.yml",
"multi_arch*.yml",
"build*artifact*.yml",
"build*ci.yml",
"build*python_packages.yml",
"test*artifacts.yml",
"test_rocm_wheels.yml",
"test_sanity_check.yml",
"test_component.yml",
]
too

(or you can trigger the workflow directly to skip past the rest of the CI jobs)

outputs:
package_install_url: ${{ steps.upload-packages.outputs.package_repository_url }}
container:
image: ghcr.io/rocm/therock_build_manylinux_x86_64@sha256:702a5133851e6d1daf1207d2c9fbb01c2667914a5b6dc5a01faeb3ce66ea6421

@nunnikri nunnikri May 6, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current job needs to be run on Ubuntu. Ubuntu2404/Ubuntu2204

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Then this job might need to set the AWS_SHARED_CREDENTIALS_FILE env var per https://github.com/ROCm/TheRock/blob/main/docs/development/s3_buckets.md#authentication ... though I would expect the credential file on the runners to already be used by default without that 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah it isn't as it's only used to docker volume mounts, i'll fix

"test_rocm_wheels.yml",
"test_sanity_check.yml",
"test_component.yml",
"build_native*packages.yml",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, CI didn't run on this.

Thanks for the fix here. I also just sent #5089 for a general solution

@geomin12
geomin12 marked this pull request as draft May 6, 2026 23:57
@nunnikri
nunnikri requested review from ammallya and raramakr May 7, 2026 17:09
FIM43-Redeye added a commit to FIM43-Redeye/TheRock that referenced this pull request May 13, 2026
Adds `&& outputs.X != ''` to install tests, wheel tests, and PyTorch
builds in ci_linux.yml, ci_windows.yml, multi_arch_ci_linux.yml, and
multi_arch_ci_windows.yml so these jobs cleanly skip when the upstream
upload produces no URL, rather than running and failing during the
install step.

Defensive on the consumer side only; the upstream auth gap that
produces empty URLs on fork PRs is being addressed separately in ROCm#5066.
geomin12 and others added 3 commits May 14, 2026 10:41
Mount /runner/config from the dind sidecar into the job container
at /home/awsconfig/ so that AWS credentials are accessible for
forked repos where OIDC auth is not available.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add ubuntu:24.04 container with volume mount for AWS credentials
- Update AWS credentials path to /home/awsconfig/credentials.ini
- Use apt-get instead of sudo apt (running inside container)
- Remove debug credentials step

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
github.workspace resolves to host path (/home/runner/_work/...) but
inside the container the workspace is mounted at /__w/. Using relative
paths ensures both build and upload steps use consistent paths.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@geomin12
geomin12 marked this pull request as ready for review May 15, 2026 15:33
@geomin12
geomin12 requested review from ScottTodd and nunnikri May 15, 2026 15:33
@geomin12

geomin12 commented May 15, 2026 •

Copy link
Copy Markdown
Contributor Author

it works yay! sorry it took a while - will fix build pytorch wheels next after this PR

Comment on lines +56 to +62
container:
image: ubuntu:24.04
options: -v /runner/config:/home/awsconfig/
outputs:
package_repository_url: ${{ steps.upload-packages.outputs.package_repository_url }}
env:
AWS_SHARED_CREDENTIALS_FILE: /home/awsconfig/credentials.ini

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to run in a container and inject this credentials file if the credentials are already on the host?

I don't mind running these in an explicit container (looks like it just takes ~2 seconds to init: https://github.com/ROCm/TheRock/actions/runs/25897755289/job/76127352092?pr=5066)... I'm just curious why this is needed at all, as we might miss it for other workflows in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah so i thought it mounted to file path normally, but it is actually not accessible with regular directories and must be mounted via a -v option. i didn't want to adjust the config in the runner setup and we should run in containers anyway!

@geomin12
geomin12 merged commit fc4e6be into ROCm:main May 15, 2026
111 of 116 checks passed
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage May 15, 2026
@geomin12
geomin12 deleted the users/geomin12/fix-rpm-deb branch May 15, 2026 17:25
marbre added a commit that referenced this pull request May 18, 2026
This reverts commit fc4e6be. Starting
from that commit, the `Configure AWS Credentials` steps in the native
Linux packages build jobs (deb and rpm) fail with

```
Error: Could not assume role with OIDC: Not authorized to perform sts:AssumeRoleWithWebIdentit
```

due to assuming a non-existing role `*:role/therock-` (missing release
type):
https://github.com/ROCm/TheRock/actions/runs/25931650380/job/76257228730
@geomin12
geomin12 restored the users/geomin12/fix-rpm-deb branch May 18, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants