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

fix: Remove go run from all go generate commands #1050

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

timraymond
Copy link
Member

Including go run in the //go:generate pragmas is never really how go generate was intended to be used. While it's a seemingly-useful hack to pin versions of tools, it breaks cross-compilation.

Cross-compilation breaks because if the requested GOARCH differs from the one go generate is being run on, go run receives the same GOARCH and builds the tool for that platform instead. The tool will never exec correctly because of the platform mismatch.

This removes go run from all //go:generate pragmas and adds the appropriate installation of these tools to the Dockerfile. Developers are expected to have these tools installed in the same way in their development environment (in the same way as go, clang, etc.).

Fixes: #1046

Including `go run` in the //go:generate pragmas is never really how `go
generate` was intended to be used. While it's a seemingly-useful hack to
pin versions of tools, it breaks cross-compilation.

Cross-compilation breaks because if the requested GOARCH differs from
the one `go generate` is being run on, `go run` receives the same GOARCH
and builds the tool for that platform instead. The tool will never exec
correctly because of the platform mismatch.

This removes `go run` from all `//go:generate` pragmas and adds the
appropriate installation of these tools to the Dockerfile. Developers
are expected to have these tools installed in the same way in their
development environment (in the same way as go, clang, etc.).
@timraymond
Copy link
Member Author

@SRodi please give this branch a try and see if it resolves the issue

@SRodi
Copy link
Member

SRodi commented Nov 21, 2024

@SRodi please give this branch a try and see if it resolves the issue

Hi @timraymond, I checkout out this branch and tried the generate:

❯ GOARCH=arm64 go generate ./pkg/plugin/...;
pkg/plugin/api/types.go:13: running "mockgen": exec: "mockgen": executable file not found in $PATH
pkg/plugin/common/common_linux.go:20: running "mockgen": exec: "mockgen": executable file not found in $PATH
Compiled /home/srodi/src/retina/pkg/plugin/conntrack/conntrack_bpfel_arm64.o
Stripped /home/srodi/src/retina/pkg/plugin/conntrack/conntrack_bpfel_arm64.o
Wrote /home/srodi/src/retina/pkg/plugin/conntrack/conntrack_bpfel_arm64.go
/home/srodi/src/retina/pkg/plugin/dropreason/_cprog/drop_reason.c:205:13: warning: unused function 'get_packet_from_sock' [-Wunused-function]
static void get_packet_from_sock(struct packet *p, struct sock *sk)
            ^
1 warning generated.
Compiled /home/srodi/src/retina/pkg/plugin/dropreason/kprobe_bpfel_arm64.o
Stripped /home/srodi/src/retina/pkg/plugin/dropreason/kprobe_bpfel_arm64.o
Wrote /home/srodi/src/retina/pkg/plugin/dropreason/kprobe_bpfel_arm64.go
pkg/plugin/dropreason/types_linux.go:61: running "mockgen": exec: "mockgen": executable file not found in $PATH
Compiled /home/srodi/src/retina/pkg/plugin/filter/filter_bpfel_arm64.o
Stripped /home/srodi/src/retina/pkg/plugin/filter/filter_bpfel_arm64.o
Wrote /home/srodi/src/retina/pkg/plugin/filter/filter_bpfel_arm64.go
pkg/plugin/filter/types.go:12: running "mockgen": exec: "mockgen": executable file not found in $PATH
pkg/plugin/infiniband/types_linux.go:17: running "mockgen": exec: "mockgen": executable file not found in $PATH
pkg/plugin/linuxutil/types_linux.go:16: running "mockgen": exec: "mockgen": executable file not found in $PATH
Compiled /home/srodi/src/retina/pkg/plugin/packetforward/packetforward_bpfel_arm64.o
Stripped /home/srodi/src/retina/pkg/plugin/packetforward/packetforward_bpfel_arm64.o
Wrote /home/srodi/src/retina/pkg/plugin/packetforward/packetforward_bpfel_arm64.go
pkg/plugin/packetforward/types_linux.go:31: running "mockgen": exec: "mockgen": executable file not found in $PATH
Compiled /home/srodi/src/retina/pkg/plugin/packetparser/packetparser_bpfel_arm64.o
Stripped /home/srodi/src/retina/pkg/plugin/packetparser/packetparser_bpfel_arm64.o
Wrote /home/srodi/src/retina/pkg/plugin/packetparser/packetparser_bpfel_arm64.go
pkg/plugin/packetparser/types_linux.go:83: running "mockgen": exec: "mockgen": executable file not found in $PATH
make: *** [Makefile:148: generate-bpf-go] Error 1

then I run the following as mockgen tool was not installed on my system

❯ go install github.com/golang/mock/mockgen@latest
go: downloading github.com/golang/mock v1.6.0
go: downloading golang.org/x/mod v0.4.2
go: downloading golang.org/x/tools v0.1.1
go: downloading golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1
go: downloading golang.org/x/sys v0.0.0-20210510120138-977fb7262007

ensure deps are recorded in go.mod and go.sum

go mod tidy

re-run generate:

❯ GOARCH=arm64 go generate ./pkg/plugin/...;
prog.go:12:2: no required module provides package github.com/golang/mock/mockgen/model: go.mod file not found in current directory or any parent directory; see 'go help modules'
prog.go:14:2: no required module provides package github.com/microsoft/retina/pkg/plugin/api: go.mod file not found in current directory or any parent directory; see 'go help modules'
2024/11/21 13:39:49 Loading input failed: exit status 1
pkg/plugin/api/types.go:13: running "mockgen": exit status 1
prog.go:12:2: no required module provides package github.com/golang/mock/mockgen/model: go.mod file not found in current directory or any parent directory; see 'go help modules'
prog.go:14:2: no required module provides package github.com/microsoft/retina/pkg/plugin/common: go.mod file not found in current directory or any parent directory; see 'go help modules'
2024/11/21 13:39:51 Loading input failed: exit status 1
pkg/plugin/common/common_linux.go:20: running "mockgen": exit status 1
Compiled /home/srodi/src/retina/pkg/plugin/conntrack/conntrack_bpfel_arm64.o
Stripped /home/srodi/src/retina/pkg/plugin/conntrack/conntrack_bpfel_arm64.o
Wrote /home/srodi/src/retina/pkg/plugin/conntrack/conntrack_bpfel_arm64.go
/home/srodi/src/retina/pkg/plugin/dropreason/_cprog/drop_reason.c:205:13: warning: unused function 'get_packet_from_sock' [-Wunused-function]
static void get_packet_from_sock(struct packet *p, struct sock *sk)
            ^
1 warning generated.
Compiled /home/srodi/src/retina/pkg/plugin/dropreason/kprobe_bpfel_arm64.o
Stripped /home/srodi/src/retina/pkg/plugin/dropreason/kprobe_bpfel_arm64.o
Wrote /home/srodi/src/retina/pkg/plugin/dropreason/kprobe_bpfel_arm64.go
Compiled /home/srodi/src/retina/pkg/plugin/filter/filter_bpfel_arm64.o
Stripped /home/srodi/src/retina/pkg/plugin/filter/filter_bpfel_arm64.o
Wrote /home/srodi/src/retina/pkg/plugin/filter/filter_bpfel_arm64.go
Compiled /home/srodi/src/retina/pkg/plugin/packetforward/packetforward_bpfel_arm64.o
Stripped /home/srodi/src/retina/pkg/plugin/packetforward/packetforward_bpfel_arm64.o
Wrote /home/srodi/src/retina/pkg/plugin/packetforward/packetforward_bpfel_arm64.go
prog.go:12:2: no required module provides package github.com/golang/mock/mockgen/model: go.mod file not found in current directory or any parent directory; see 'go help modules'
prog.go:14:2: no required module provides package github.com/microsoft/retina/pkg/plugin/packetforward: go.mod file not found in current directory or any parent directory; see 'go help modules'
2024/11/21 13:39:56 Loading input failed: exit status 1
pkg/plugin/packetforward/types_linux.go:31: running "mockgen": exit status 1
Compiled /home/srodi/src/retina/pkg/plugin/packetparser/packetparser_bpfel_arm64.o
Stripped /home/srodi/src/retina/pkg/plugin/packetparser/packetparser_bpfel_arm64.o
Wrote /home/srodi/src/retina/pkg/plugin/packetparser/packetparser_bpfel_arm64.go

I think go.mod needs to be updated to include github.com/golang/mock

@timraymond
Copy link
Member Author

@SRodi There's something very broken with the reflection-based implementation that mockgen uses when you don't provide a -source. Adding those deps probably won't work either because it can't even find a go.mod to confirm whether or not those dependencies are added (also, github.com/microsoft/retina/pkg/plugin/common should be there implicitly). I'm recommending everyone run go generate with -skip="mockgen" in the interim as a workaround (since it's only needed for tests).

@SRodi
Copy link
Member

SRodi commented Nov 22, 2024

@timraymond the --skip="mockgen" is still not working for me

❯ GOARCH=arm64 go generate --skip="mockgen" ./pkg/plugin/...;
fork/exec /tmp/go-build2449268275/b001/exe/bpf2go: exec format error
pkg/plugin/conntrack/conntrack_linux.go:24: running "go": exit status 1
fork/exec /tmp/go-build2049352829/b001/exe/bpf2go: exec format error
pkg/plugin/dropreason/dropreason_linux.go:36: running "go": exit status 1
fork/exec /tmp/go-build3797868824/b001/exe/bpf2go: exec format error
pkg/plugin/filter/filter_map_linux.go:23: running "go": exit status 1
fork/exec /tmp/go-build1096452528/b001/exe/bpf2go: exec format error
pkg/plugin/packetforward/packetforward_linux.go:31: running "go": exit status 1
fork/exec /tmp/go-build3845592266/b001/exe/bpf2go: exec format error
pkg/plugin/packetparser/packetparser_linux.go:52: running "go": exit status 1

For contributors to eBPF programs, the make generate-bpf-go generates eBPF wrappers for plugins for all architectures. This make target uses go generate. How do you suggest to fix this problem so that we can generate both amd64 and arm64 wrappers with a single make target?

@timraymond
Copy link
Member Author

@SRodi I think we probably need to run all generate steps in a container to rule dependency versions and platforms with an iron fist. This way, make generate-bpf-go spins a container with exactly the right environment and developers' source trees are mutated by the volume mount.

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.

GOARCH=arm64 go generate is failing on x86_64
2 participants