Skip to content

Remove the need for go-bindata#10177

Merged
aanm merged 2 commits intomasterfrom
pr/tklauser/remove-go-bindata
Feb 19, 2020
Merged

Remove the need for go-bindata#10177
aanm merged 2 commits intomasterfrom
pr/tklauser/remove-go-bindata

Conversation

@tklauser
Copy link
Copy Markdown
Member

@tklauser tklauser commented Feb 13, 2020 •
edited by tgraf
Loading

Use of go-bindata dates back from times when people ran Cilium as static
binary. This has become uncommon and users either use the container
image or a package manager which will both ship /var/lib/cilium directly
so there is no need to unpack any assets via the binary.

For people still wanting to use Cilium as a static binary, e.g. for
local development provide the install-bpf Makefile target to install
the BPF assets into /var/lib/cilium.

This saves ~380 kB in the resulting cilium-agent binary:

  == daemon/cilium-agent ==
  bss                                7752192     7752160         -32
  data                                894041      651280     -242761
  dec                               64545230    64166071     -379159
  hex                                3d8e1ce     3d318b7      -5c917
  text                              55898997    55762631     -136366

Updates #10056
Fixes #10075

go-bindata is no longer used to install BPF assets.

This change is Reviewable

@tklauser tklauser added wip dont-merge/preview-only Only for preview or testing, don't merge it. release-note/misc This PR makes changes that have no direct user impact. labels Feb 13, 2020
@tklauser tklauser requested a review from eloycoto as a code owner February 13, 2020 10:17
@tklauser tklauser requested review from a team February 13, 2020 10:17
@tklauser tklauser requested review from a team as code owners February 13, 2020 10:17
@coveralls
Copy link
Copy Markdown

coveralls commented Feb 13, 2020 •
edited
Loading

Coverage Status

Coverage increased (+1.0%) to 45.543% when pulling dfcff21b84e2127af444638aec431a3720f2a839 on pr/tklauser/remove-go-bindata into 8fc4bde on master.

@tklauser tklauser force-pushed the pr/tklauser/remove-go-bindata branch from 1af404d to c604dbe Compare February 13, 2020 11:01
@tklauser
Copy link
Copy Markdown
Member Author

tklauser commented Feb 13, 2020

test-me-please

tgraf
tgraf suggested changes Feb 13, 2020
View reviewed changes
Comment thread daemon/daemon_main.go Outdated
@tklauser tklauser force-pushed the pr/tklauser/remove-go-bindata branch from c604dbe to 922355e Compare February 13, 2020 12:02
@tklauser
Copy link
Copy Markdown
Member Author

tklauser commented Feb 13, 2020 •
edited
Loading

test-me-please

@tklauser
Copy link
Copy Markdown
Member Author

tklauser commented Feb 13, 2020

test-docs-please

@tklauser
Copy link
Copy Markdown
Member Author

tklauser commented Feb 13, 2020

test-me-please

@joestringer joestringer added area/daemon Impacts operation of the Cilium daemon. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. labels Feb 13, 2020
@tklauser tklauser removed dont-merge/preview-only Only for preview or testing, don't merge it. wip labels Feb 14, 2020
@tklauser
Copy link
Copy Markdown
Member Author

tklauser commented Feb 14, 2020

test-me-please

@brb
Copy link
Copy Markdown
Member

brb commented Feb 14, 2020

CI failed due to well known flakes: K8sServicesTest_Checks_ClusterIP_Connectivity_IPv6_Connectivity_Checks_service_on_same_node and K8sPolicyTest_Basic_Test_Validate_to-entities_policies_Validate_toEntities_All. Re-running.

@brb
Copy link
Copy Markdown
Member

brb commented Feb 14, 2020

test-me-please

brb
brb reviewed Feb 14, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@brb brb left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, a productivity booster!

Did you check what error will returned to a user when running cilium-agent as a standalone binary and w/o the bpf files installed?

Also, we should probably add a GSG for running cilium as standalone (not in this PR).

@tklauser
Copy link
Copy Markdown
Member Author

tklauser commented Feb 14, 2020

Did you check what error will returned to a user when running cilium-agent as a standalone binary and w/o the bpf files installed?

level=fatal msg="BPF Verifier: NOT OK. Unable to run checker for bpf_features" error="fork/exec /var/lib/cilium/bpf/run_probes.sh: no such file or directory" subsys=linux-datapath

Do you think we should add a more specific error message, e.g. instructing the user to run make install or use a package/container image?

Also, we should probably add a GSG for running cilium as standalone (not in this PR).

What do you mean by standalone? Just running on a host without k8s/docker/etc.?

@brb
Copy link
Copy Markdown
Member

brb commented Feb 14, 2020

@tklauser

Do you think we should add a more specific error message, e.g. instructing the user to run make install or use a package/container image?

Yep, something along the lines: [..] Please run "make install-bpf".

What do you mean by standalone? Just running on a host without k8s/docker/etc.?

Running cilium-agent directly on a host, i.e. not in a constainer (e.g. ./daemon/cilium-agent ...). It doesn't matter whether k8s is provisioned or not.

@tklauser tklauser force-pushed the pr/tklauser/remove-go-bindata branch from 922355e to 3cfeed5 Compare February 14, 2020 15:49
@tklauser
Copy link
Copy Markdown
Member Author

tklauser commented Feb 14, 2020 •
edited
Loading

@brb The following message is now logged if the BPF template directory does not exist:

level=fatal msg="BPF template directory: NOT OK. Please run 'make install-bpf'" error="stat /var/lib/cilium/bpf: no such file or directory" subsys=linux-datapath

@tklauser
Copy link
Copy Markdown
Member Author

tklauser commented Feb 14, 2020

test-me-please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brb brb brb approved these changes

@aanm aanm aanm approved these changes

@eloycoto eloycoto Awaiting requested review from eloycoto

+1 more reviewer

@tgraf tgraf tgraf approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/daemon Impacts operation of the Cilium daemon. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact.

Projects

None yet

Milestone

1.8

Development

Successfully merging this pull request may close these issues.

Remove go-bindata

7 participants

@tklauser @coveralls @brb @tgraf @aanm @joestringer @vadorovsky