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

bug: building with go get results in invalid binary #169

Closed
dalehamel opened this issue Aug 9, 2021 · 4 comments · Fixed by #171
Closed

bug: building with go get results in invalid binary #169

dalehamel opened this issue Aug 9, 2021 · 4 comments · Fixed by #171

Comments

@dalehamel
Copy link
Member

Building as per our docs https://github.com/iovisor/kubectl-trace#source when building with go get, the resulting binary behaves bizarrely. This is mostly due to the lack of LDFLAGS being specified, and any variable which requires git to not be populated.

The outcome of this is that the images will have no tag, so will fail to run, and the version subcommand will be empty.

To fix this, we should ensure that these variables have reasonable defaults for when they are NOT specified, for instance, we can default the version to master, and the image tag to latest, as well as ensure we are pushing the :latest tag on pushes to master.

@dalehamel
Copy link
Member Author

@leodido @fntlnz FYI

@leodido
Copy link
Member

leodido commented Aug 9, 2021

As discussed internally, I confirm those findings.

Also, I agree on the solution.

@dalehamel
Copy link
Member Author

It looks like we SHOULD be already pushing :latest, but aren't:

https://github.com/iovisor/kubectl-trace/blob/master/Makefile#L117-L121

https://github.com/iovisor/kubectl-trace/blob/master/build/scripts/ci-release-image.sh#L28-L30

I think this is because github changed how github.ref works on github actions IIRC, so it may be that it never finds master. I think I ran into this somewhere else as well. Should be an easy fix.

@dalehamel
Copy link
Member Author

https://github.com/iovisor/kubectl-trace/blob/master/Makefile#L36 we'll need to do a pass over all these variables and ensure their defaults are sane

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 a pull request may close this issue.

2 participants