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

Makefiles: Disable CGO globally #10724

Merged
merged 1 commit into from Apr 2, 2020

Conversation

joestringer
Copy link
Member

@joestringer joestringer commented Mar 26, 2020

CGo is not necessary for any Cilium binaries (only proxylib), and with
the new docs builder image it's causing build issues like the following
on master for some developers:

  GEN   Documentation/cmdref
Error relocating /src/cilium/cilium: __vfprintf_chk: symbol not found
Error relocating /src/cilium/cilium: __fprintf_chk: symbol not found

Fix this by just disabling it for all targets that use $GO.

@joestringer joestringer added pending-review priority/high This is considered vital to an upcoming release. release-note/misc This PR makes changes that have no direct user impact. labels Mar 26, 2020
@joestringer joestringer requested a review from a team as a code owner March 26, 2020 18:44
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Mar 26, 2020
@joestringer
Copy link
Member Author

test-me-please

@joestringer
Copy link
Member Author

I guess we use it for proxylib, will need to fix that up..

@joestringer
Copy link
Member Author

joestringer commented Mar 26, 2020

test-me-please

Edit: Kafka flakes; fix has been merged: #10721

@christarazi
Copy link
Member

test-me-please

Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

I think you'll need to drop CGO_ENABLED=0 from the go vet commands for proxylib. See the Travis CI failure.

@joestringer
Copy link
Member Author

test-me-please

@joestringer
Copy link
Member Author

Oh, originally I thought it was go vet but it looks like it's actually the go tests in the proxylib directory.

@joestringer
Copy link
Member Author

test-me-please

@coveralls
Copy link

coveralls commented Mar 27, 2020

Coverage Status

Coverage remained the same at 45.517% when pulling 5a264bc on joestringer:submit/fix-broken-master into 26dec4c on cilium:master.

@joestringer joestringer added wip and removed pending-review priority/high This is considered vital to an upcoming release. labels Mar 27, 2020
@joestringer
Copy link
Member Author

Plenty of scary-looking failures in CI, will need to investigate further:
https://jenkins.cilium.io/job/Cilium-PR-Ginkgo-Tests-Validated/18287/

CGo is not necessary for any Cilium binaries, and with the new docs
builder image it's causing build issues like the following:

      GEN   Documentation/cmdref
    Error relocating /src/cilium/cilium: __vfprintf_chk: symbol not found
    Error relocating /src/cilium/cilium: __fprintf_chk: symbol not found

Fix this by just disabling it for all targets that use $GO.

Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer
Copy link
Member Author

joestringer commented Apr 1, 2020

test-me-please

EDIT: Only one of the kernel builds failed, due to flake #10821 which looks like infrastructure issue.

@joestringer joestringer added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed wip labels Apr 2, 2020
@aanm aanm merged commit 7626be1 into cilium:master Apr 2, 2020
1.8.0 automation moved this from In progress to Merged Apr 2, 2020
@joestringer joestringer deleted the submit/fix-broken-master branch May 22, 2020 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

6 participants