-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Remove dependency from cilium-cli onto pkg/datapath
and github.com/cilium/ebpf
#38364
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
It's unused since commit cc676c7 ("bpf: replace BPF_MAP_UPDATE_ELEM syscall with ebpf-go"). Signed-off-by: Tobias Klauser <tobias@cilium.io>
Otherwise we get build errors on darwin when updating cilium/ebpf beyond v0.17.1. Signed-off-by: Tobias Klauser <tobias@cilium.io>
This reverts commit 03a2d94. Reason for revert: the dependency on pkg/datapath and thus github.com/cilium/ebpf on non-Linux platform was removed by successive commits. Signed-off-by: Tobias Klauser <tobias@cilium.io>
/test |
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.
Thanks for addressing it!
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.
@tklauser Good work!
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.
Awesome, thanks for figuring this out! I understand you used godepgraph
in the end, is that right? Might be worth recording that in the PR description in case this comes up again.
Good call. Added the |
Stub out
metrics.enableIfIndexMetrics
on all non-Linux platforms to cut the dependency onpkg/datapath
and transitively ongithub.com/cilium/ebpf
. Without this change the build fails e.g. forGOOS=darwin
in the case of cilium-cli when updating cilium/ebpf beyond v0.17.1.With the above fixed, revert the cilium/ebpf downgrade to v0.17.3, see #38334.
Also remove the unused
Errno2Outcome
helper function.Fixes #37598
The dependency graph was generated using the following command:
The resulting image is attached.