Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Update self-hosted arm runner to ubuntu-24.04-arm #268

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

singh1203
Copy link
Contributor

Fixes: #267

What changes are included in this PR?

Replaced the running platform from self-hosted arm runner to ubuntu-24.04-arm

Are these changes tested?

Not locally, but can be tested by triggering the runner

Are there any user-facing changes?

No

@singh1203 singh1203 changed the title (GH:apache#267) (ci)(chore) Update to ubuntu-24.04-arm CI: Update self-hosted arm runner to ubuntu-24.04-arm Feb 5, 2025
Copy link
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
I would remove the if check for the GitHub repository owner and run on arm as we do for amd64. I mean removing:

          if [ "$GITHUB_REPOSITORY_OWNER" = "apache" ]; then
            echo "," >> "$GITHUB_OUTPUT"
            cat <<JSON >> "$GITHUB_OUTPUT"

and reorganize the code to a single JSON write.

Copy link
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

LGTM

@kou
Copy link
Member

kou commented Feb 5, 2025

Could you remove the docker-targets job? Could you define matrix in the docker job statically? We don't need to define matrix dynamically now.

@raulcd
Copy link
Member

raulcd commented Feb 5, 2025

If you want an example from what @kou suggests you can see this diff on the main apache/arrow repo where the docker-targets was removed for a matrix:
https://github.com/apache/arrow/pull/45308/files

@singh1203
Copy link
Contributor Author

Sure @kou. I'd love to help here, and Thank you @raulcd for pointing me in right direction

@singh1203
Copy link
Contributor Author

singh1203 commented Feb 5, 2025

Could you remove the docker-targets job? Could you define matrix in the docker job statically? We don't need to define matrix dynamically now.

Done. Let's trigger the runner for testing.

.github/workflows/test.yml Outdated Show resolved Hide resolved
@kou kou changed the title CI: Update self-hosted arm runner to ubuntu-24.04-arm chore: Update self-hosted arm runner to ubuntu-24.04-arm Feb 6, 2025
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

Thanks!

@kou kou merged commit 4317e63 into apache:main Feb 6, 2025
23 checks passed
@singh1203 singh1203 deleted the arm-CI branch February 7, 2025 05:56
@zeroshade
Copy link
Member

@singh1203 @kou @raulcd It looks like the ARM jobs are still failing unable to find a runner :( any ideas?

@kou
Copy link
Member

kou commented Feb 10, 2025

Which jobs did you see?
#264 ?
#266 ?

They are opened before this PR is merged. I'll rebase them.

@singh1203
Copy link
Contributor Author

Which jobs did you see? #264 ? #266 ?

They are opened before this PR is merged. I'll rebase them.

After rebase, #266 is good to go, but in #264 couple of runners are failing.
@zeroshade I guess you are referring to this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use ubuntu-24.04-arm instead of self-hosted arm runner in CI
4 participants