-
Notifications
You must be signed in to change notification settings - Fork 256
CI: Update macos workflow to latest macos #5011
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
base: main
Are you sure you want to change the base?
Conversation
Looks like macos-13 is now depricated so replace those with macos-15 and macos-26.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughGitHub Actions workflow files are updated to use newer macOS runner versions. The macOS-13 and macOS-14 runners are replaced with macOS-15-intel, macOS-15, and macOS-26 in the CI/CD matrix configurations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@praveenkumar: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| - macOS-13 | ||
| - macOS-14 | ||
| - macOS-15-intel | ||
| - macOS-15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is preexisting, but macos-15 is a more common spelling for runner names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.github.com/en/actions/reference/runners/github-hosted-runners#standard-github-hosted-runners-for-public-repositories shows macos-15 by default is arm and macos-15-intel is the intel one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I suggest is s/macOS/macos
| uses: actions/upload-artifact@v5 | ||
| with: | ||
| name: macOS Installer (${{ matrix.os }}) | ||
| path: "./out/macos-universal/crc-macos-installer.pkg" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a universal installer, why do we need to run it both on intel and apple silicon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to make sure it works on intel and silicon both ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Building/uploading the installer on both platforms is not really useful imo, as long as we can build it on apple silicon, we’re good.
But after that there are also a few steps which install the .pkg and run crc setup which I hadn’t noticed.
Imo the installer build and the installer tests on Apple Silicon/Intel should be separate, we build one installer on Apple Silicon, and we test it on the 2 platforms, it’s closer to how we are going to release it.
And I’m not sure running crc setup is great, as it’s going to download the bundle, which can take time.
| - macOS-13 | ||
| - macOS-14 | ||
| - macOS-15 | ||
| - macOS-26 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same suggestion as in 3391f98#r2526670082
| uses: actions/upload-artifact@v5 | ||
| with: | ||
| name: macOS Installer (${{ matrix.os }}) | ||
| path: "./out/macos-universal/crc-macos-installer.pkg" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Building/uploading the installer on both platforms is not really useful imo, as long as we can build it on apple silicon, we’re good.
But after that there are also a few steps which install the .pkg and run crc setup which I hadn’t noticed.
Imo the installer build and the installer tests on Apple Silicon/Intel should be separate, we build one installer on Apple Silicon, and we test it on the 2 platforms, it’s closer to how we are going to release it.
And I’m not sure running crc setup is great, as it’s going to download the bundle, which can take time.
Looks like macos-13 is now depricated so replace those with macos-15 and macos-26.
Summary by CodeRabbit
Release Notes