fix(orchestrator): make go test ./... build and pass on macOS#2608
Merged
Conversation
❌ 8 Tests Failed:
View the full list of 11 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
Adding the //go:build linux tag to packages/orchestrator/generate.go prevents go generate from running on non-Linux platforms, which unnecessarily restricts developers on macOS or other systems from regenerating cross-platform gRPC code.
ValentaTomas
approved these changes
May 10, 2026
This change makes the orchestrator package buildable and testable on
darwin (Apple Silicon and Intel) so contributors can iterate locally
without a Linux VM. It does NOT change any runtime behaviour on linux —
the production target.
Background
----------
`go test -v ./...` inside packages/orchestrator failed on darwin because
~50 packages transitively import linux-only code:
- `github.com/ngrok/firewall_toolkit/pkg/expressions` (nftables
expression helpers; the package itself has linux build tags
upstream).
- `github.com/Merovius/nbd/nbdnl` (NBD netlink ioctls).
- `github.com/containernetworking/plugins/pkg/ns` (netns).
- linux-only symbols on `golang.org/x/sys/unix` and
`syscall.SysProcAttr` (`SyncFileRange`, `CopyFileRange`,
`ProcessVMReadv`, `RemoteIovec`, `Fsopen`, `FsconfigSetString`,
`Fsmount`, `MoveMount`, `MAP_HUGETLB`, `syscall.MAP_ANONYMOUS`,
`UseCgroupFD`, `CgroupFD`, …).
- cgo-based `userfaultfd` wrappers around `<linux/userfaultfd.h>`.
None of these can ever compile on darwin and there is no portable
substitute (the orchestrator is a linux-only runtime: firecracker
VMs, nftables, NBD, userfaultfd, cgroupv2 are all linux kernel
features).
Changes
-------
Added `//go:build linux` to every `.go` file in the packages whose
compile-time dependencies are linux-only. On darwin those packages
now report `[no test files]` (or are simply skipped); on linux they
build exactly as before because the constraint matches.
Tagged packages (267 files):
orchestrator (root), benchmarks,
cmd/{create-build,mount-build-rootfs,resume-build,smoketest},
pkg/cfg, pkg/chrooted, pkg/factories, pkg/healthcheck,
pkg/hyperloopserver{,/handlers}, pkg/metrics,
pkg/nfsproxy{,/chroot}, pkg/proxy,
pkg/sandbox{,/block,/build,/build/mocks,/fc,/nbd,/nbd/testutils,
/network,/rootfs,/template,/template/mocks,
/template/peerserver,/template/peerserver/mocks,
/uffd,/uffd/memory,/uffd/prefetch,/uffd/testutils,
/uffd/userfaultfd},
pkg/server, pkg/service, pkg/tcpfirewall,
pkg/template/build{,/buildcontext,/builderrors,/commands,
/core/filesystem,/core/oci,/core/rootfs,/layer,/phases,
/phases/{base,finalize,optimize,steps,user},
/sandboxtools,/storage/cache},
pkg/template/{metadata,server},
pkg/volumes.
Whole-package tagging (rather than per-leaf-file tagging plus darwin
stubs) was chosen because the linux-only API surface of these
packages is woven through them — `Firewall`, `Cache`,
`DirectPathMount`, `Userfaultfd`, namespaced slot setup, etc. are
used by sibling files within the same package. Stubbing every
exported symbol on darwin would have produced significantly more
churn for code that fundamentally cannot run on darwin anyway, and
would have given a false impression that those packages could ever
be exercised off-linux.
Files that already carried a build tag (`stat_linux.go`,
`stat_osx.go` in cmd/clean-nfs-cache/cleaner, the cgo
`userfaultfd/fd.go`, etc.) were left untouched.
Verification
------------
On darwin/arm64:
$ go test ./...
ok cmd/clean-nfs-cache/cleaner
ok cmd/simulate-gcs-traffic
ok cmd/simulate-nfs-traffic
ok pkg/localupload
ok pkg/nfsproxy/recovery
ok pkg/nfsproxy/tracing
ok pkg/portmap
ok pkg/sandbox/template/peerclient
ok pkg/template/build/writer
(linux-only packages report `[no test files]`)
exit 0
`go build ./...` also succeeds on darwin. All linux build tags resolve
correctly and the linux build is unchanged because we only added
constraints; no source was deleted or moved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change makes the orchestrator package buildable and testable on darwin (Apple Silicon and Intel) so contributors can iterate locally without a Linux VM. It does NOT change any runtime behaviour on linux — the production target.
Background
go test -v ./...inside packages/orchestrator failed on darwin because ~50 packages transitively import linux-only code:github.com/ngrok/firewall_toolkit/pkg/expressions(nftables expression helpers; the package itself has linux build tags upstream).github.com/Merovius/nbd/nbdnl(NBD netlink ioctls).github.com/containernetworking/plugins/pkg/ns(netns).golang.org/x/sys/unixandsyscall.SysProcAttr(SyncFileRange,CopyFileRange,ProcessVMReadv,RemoteIovec,Fsopen,FsconfigSetString,Fsmount,MoveMount,MAP_HUGETLB,syscall.MAP_ANONYMOUS,UseCgroupFD,CgroupFD, …).userfaultfdwrappers around<linux/userfaultfd.h>. None of these can ever compile on darwin and there is no portable substitute (the orchestrator is a linux-only runtime: firecracker VMs, nftables, NBD, userfaultfd, cgroupv2 are all linux kernel features).Changes
Added
//go:build linuxto every.gofile in the packages whose compile-time dependencies are linux-only. On darwin those packages now report[no test files](or are simply skipped); on linux they build exactly as before because the constraint matches.Tagged packages (267 files):
orchestrator (root), benchmarks,
cmd/{create-build,mount-build-rootfs,resume-build,smoketest},
pkg/cfg, pkg/chrooted, pkg/factories, pkg/healthcheck,
pkg/hyperloopserver{,/handlers}, pkg/metrics,
pkg/nfsproxy{,/chroot}, pkg/proxy,
pkg/sandbox{,/block,/build,/build/mocks,/fc,/nbd,/nbd/testutils,
/network,/rootfs,/template,/template/mocks,
/template/peerserver,/template/peerserver/mocks,
/uffd,/uffd/memory,/uffd/prefetch,/uffd/testutils,
/uffd/userfaultfd},
pkg/server, pkg/service, pkg/tcpfirewall,
pkg/template/build{,/buildcontext,/builderrors,/commands,
/core/filesystem,/core/oci,/core/rootfs,/layer,/phases,
/phases/{base,finalize,optimize,steps,user},
/sandboxtools,/storage/cache},
pkg/template/{metadata,server},
pkg/volumes.
Whole-package tagging (rather than per-leaf-file tagging plus darwin stubs) was chosen because the linux-only API surface of these packages is woven through them —
Firewall,Cache,DirectPathMount,Userfaultfd, namespaced slot setup, etc. are used by sibling files within the same package. Stubbing every exported symbol on darwin would have produced significantly more churn for code that fundamentally cannot run on darwin anyway, and would have given a false impression that those packages could ever be exercised off-linux.Files that already carried a build tag (
stat_linux.go,stat_osx.goin cmd/clean-nfs-cache/cleaner, the cgouserfaultfd/fd.go, etc.) were left untouched.Verification
On darwin/arm64:
go build ./...also succeeds on darwin. All linux build tags resolve correctly and the linux build is unchanged because we only added constraints; no source was deleted or moved.