-
-
Notifications
You must be signed in to change notification settings - Fork 566
ci(docker): Add tests for macOS #636
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
Conversation
You may understand this better than me (and appreciate it's a work in progress so not trying to review it). The macos-latest github runner is an amd host, and the only arm host is macos-14 (beta) The ubuntu-latest (for private repo) is running in Azure according to this spec, and indicates it's an Intel(amd) processor. I think there's limited ability to run an arm host as a github runner. In terms of outcomes, (despite the comments on the other PR I've made about seeing differences between aarch64/arm64) isn't the principle test just for a build with buildx arg --platform linux/arm64 as well as the current linux/amd64. Effectively the whole workflow in parallel rather than just the multi-arch build step? |
arm64 exist for macOS 13. GH just not updated their docs And matrix is a parallelism by itself, we build multiplatform image just to test that this will work, during the release Check run workflows for the last commit If there were no possibility to run arm64 runners, jobs would not start at all |
I will take a look at the container structure GHA next week. if it can be quickly fixed, I will use my fork (as it was some time ago), and merge this PR with all 4 checks. If not - well, 2 arches for Ubuntu are still better than just 1 Ubuntu amd64 build. Then we will be able to test your PR right away in GHA, just in case. |
So the matrix defined still only uses runs-on {{ matrix.os }} so how does the runs-on command have any concept of the architecture? I've never seen anything in github documentation to say they provide arm64 hosts (other than the recent addition of the Mac M1 processor). I can't see anywhere in the workflow that utilises the architecture specified. |
This comment was marked as resolved.
This comment was marked as resolved.
📝 WalkthroughSummary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe build workflow configuration was updated to add MacOS amd64 and arm64 runners to the build matrix. A MacOS-specific step was introduced to set up Docker using a dedicated action before the Docker Buildx setup, conditional on Docker-related files changing. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant MacOS Runner
participant Setup Docker Action
participant Docker Buildx
GitHub Actions->>MacOS Runner: Start job with matrix.os-type=darwin
MacOS Runner->>Setup Docker Action: Run douglascamata/[email protected]
Setup Docker Action->>MacOS Runner: Docker environment ready
MacOS Runner->>Docker Buildx: Proceed with Docker Buildx setup and build steps
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
for more information, see https://pre-commit.ci
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.
for more information, see https://pre-commit.ci
This works for mac x64. |
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.
so, as Mac docker is still a linux VM which is not described anyhow in docker inspect metadata => I assume it have no difference with standard linux build, is it make any sense to test Mac-specific docker builds at all?
docker inspect
of built image on Mac x64:
For context - all that started as issue from #636. Linux arm tests whete added long time ago.
Mac x64 build works in this PR, Mac arm - still not, and according to build action used here for mac - it near impossible to make it run natively
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.
I assume it have no difference with standard linux build, is it make any sense to test Mac-specific docker builds at all?
Might there be a difference around tool paths, libs, whatever the tests rely upon? Like those common diffs in POSIX (macOS) vs GNU (Linux)?
If not, then, given the info you provided, I agree that we can take macOS build as equal to Linux build.
Maybe call out for someone with macOS to validate tests provide the same outcome in Docker and on native Mac hardware? 🤔
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.
Different tools inside linux VMs? Could be, but then it depends what users use - Docker Desktop, Rancher etc. and specific it version.
I'll close it for now, till we will face a mac-specific issues that are not detected by current tests
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.
It's probably that I didn't get the basic point of the message: did you mean that os: macos-13 os-type: darwin
in reality is a linux ubuntu-based container? What make it macOS build then? Or did you mean that underlying host is Linux anyways and hence built artifacts (like pre-compiled binaries) should be the same anyways (kinda use the same set of instructions)? 🤔 I'm definitely of context of this PR and thus most probably am mixing up things 🤷🏻
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.
os: macos-13 os-type: darwin
is Mac, but colima spin-ups Linux VM to run docker
Colima means Containers on Lima. Since Lima is aka Linux Machines. By transitivity, Colima can also mean Containers on Linux Machines.
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.
So your concern is underlying host system and not container internals, right?
run: | | ||
docker context ls | ||
# Colima usually creates the socket at ~/.colima/default/docker.sock | ||
DOCKER_SOCKET_PATH=$(docker context ls | awk '{print $4}' | grep colima) |
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.
- Using
aws
ascut
is an anti-pattern - But if you change it to something like
... | awk -F$'\t' '$4 ~ /colima/ {print $3; exit}'
, the it makes sense (theexit
is to produce only one string if there are more matches). Otherwise it should be something like... | cut -f 4 | grep colima
.
Put an
x
into the box if that apply:Description of your changes
How can we test changes