Publish binaries to GitHub Releases #766

grounded042 · 2018-04-04T17:32:28Z

This is for #765. It will build executables for windows and in the travis job it will publish the windows and linux binaries with the GitHub release when the build is tagged. Out of the box the standalone binary and the query binary won't work as they need the UI files location specified.

Also, before this is merged someone with access to the repo will have to add a secure key to the travis deploy so the files can be sent along to GitHub.

Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

coveralls · 2018-04-04T17:49:07Z

Coverage remained the same at 100.0% when pulling 4488a72 on grounded042:binary_distro into 59cc364 on jaegertracing:master.

yurishkuro · 2018-04-04T17:56:35Z

scripts/travis/build-windows.sh

@@ -0,0 +1,5 @@
+#!/bin/bash


is this script necessary? I.e. why not just call make build-windows from .travis?

Yup, I can move it to just running make build-windows from .travis.yml. I was following convention and created a script file. I'll change that up.

Let's be nice to the next person that will touch this feature and keep it consistent with the other parts of the build :)

I'm confused by the two comments - should I use make build-windows or call the script?

I would say that the script is desirable, for consistency's sake. But @yurishkuro has the final word on this.

Personally I prefer minimalist approach, of the script doesn't add anything then why have it?

But since it's here already, it's fine to leave it.

yurishkuro · 2018-04-04T17:57:55Z

Makefile

 .PHONY: build-agent-linux
 build-agent-linux:
 	CGO_ENABLED=0 GOOS=linux installsuffix=cgo go build -o ./cmd/agent/agent-linux $(BUILD_INFO) ./cmd/agent/main.go

+.PHONY: build-agent-windows
+build-agent-windows:
+	CGO_ENABLED=0 GOOS=windows installsuffix=cgo go build -o ./cmd/agent/agent-windows.exe $(BUILD_INFO) ./cmd/agent/main.go


Any reason not to call this agent.exe? Is there any other platform that uses .exe extension?

Just following convention since the agent for linux is called agent-linux. I'll change it up to be agent.exe.

@yurishkuro what if we named the different binaries for windows along the lines of jaeger.<product>.exe and dropped the -windows on all of them?

my preference would be to have identical names but group them into dirs/archives by target, e.g.

jaeger-1.4.0-darwin-amd64.tar.gz - jaeger-agent - jaeger-collector - etc jaeger-1.4.0-linux-amd64.tar.gz - jaeger-agent - jaeger-collector - etc jaeger-1.4.0-windows-amd64.tar.gz - jaeger-agent.exe - jaeger-collector.exe - etc

I like it. I'll work on getting that setup @yurishkuro.

yurishkuro · 2018-04-04T18:00:51Z

.travis.yml


 after_success:
  - if [ "$COVERAGE" == true ]; then travis_retry goveralls -coverprofile=cover.out -service=travis-ci || true ; else echo 'skipping coverage'; fi

 after_failure:
  - if [ "$CROSSDOCK" == true ]; then make crossdock-logs ; else echo 'skipping crossdock'; fi
+
+deploy:


I am not familiar with travis deploy workflow - where do these files end up? And do we need a different naming conventions? E.g. https://prometheus.io/download/:

prometheus-2.2.1.darwin-amd64.tar.gz prometheus-2.2.1.linux-amd64.tar.gz prometheus-2.2.1.windows-amd64.tar.gz

Also, since these are archive files, do we want to bundle all binaries into one archive?

also, we should change these commands to only build and deploy these binaries on landing the PR rather than every commit (unless this already does this)

Those files will end up as individual assets over at the release message (e.g. https://github.com/jaegertracing/jaeger/releases/tag/v1.3.0). That's why we need someone to supply a token at YOUR_API_KEY_ENCRYPTED - it allows pushing the files back to GitHub.

Because we have on.tags: true Travis will only push files back to a release message when the commit has a tag. We could also add branch: master to further pin down when it deploys.

We could also add branch: master to further pin down when it deploys.

I don't think that's what we want:

GitHub Releases uses git tags. If the build commit does not have any tags, one will be created in the form of untagged-*, where * is a random hex string.

@jpkrohling I think that's only if you have on.branch: true. I was thinking that if you had multiple on options it would only deploy if all conditions were met. I'll look into it.

I verified from the Travis docs:

When all conditions specified in the on: section are met, your build will deploy.

So if we have on.tags: true and on.branch: master it will only deploy when it's a commit on master that has a tag already.

yurishkuro · 2018-04-04T18:02:58Z

Out of the box the standalone binary and the query binary won't work as they need the UI files location specified.

fwiw we should fix it using binary assets

Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

grounded042 · 2018-04-05T17:34:22Z

I pushed up some new commits:

Added a script to package files into archives with a version number like @yurishkuro asked for above.
Added a darwin build option for travis
Added a linux build option for travis - this was needed because we don't build all the linux binaries all at once and would make archiving everything together a little complicated

Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

yurishkuro · 2018-04-10T21:53:56Z

.travis.yml

@@ -54,9 +63,34 @@ script:
  - if [ "$DOCKER" == true ]; then bash ./scripts/travis/build-docker-images.sh ; else echo 'skipping docker images'; fi
  - if [ "$ES_INTEGRATION_TEST" == true ]; then bash ./scripts/travis/es-integration-test.sh ; else echo 'skipping elastic search integration test'; fi
  - if [ "$HOTROD" == true ]; then bash ./scripts/travis/hotrod-integration-test.sh ; else echo 'skipping hotrod example'; fi
+  - if [ "$LINUX" == true ]; then make build-linux ; else echo 'skipping linux'; fi
+  - if [ "$WINDOWS" == true ]; then make build-windows ; else echo 'skipping windows'; fi
+  - if [ "$DARWIN" == true ]; then make build-darwin ; else echo 'skipping darwin'; fi


overall this looks good, but this 3 additions concern me as they are going to significantly increase Travis build times. I'd rather have a single DEPLOY=true step that would build files for all targets, which would be a lot faster since we don't have to wait for Travis VM/container to initialize.

Also, there's now a huge amount of duplication in the make file, I think it could be simplified a lot through variables, similar to the deploy.sh file, e.g.

linux-binaries: make build-agent build-collector ... GOOS=darwin build-agent: CGO_ENABLED=0 GOOS=$(GOOS) ...

in fact, DEPLOY=true doesn't even need to be a new matrix entry, we can co-locate it with the DOCKER=true entry (since it requires linux binaries anyway).

Thanks for the input @yurishkuro! I'll have time closer to the middle of this week to make these changes.

also, correct the travis deploy to deploy the tar.gz files Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

grounded042 · 2018-04-20T17:54:14Z

@yurishkuro I've made the changes we talked about above - decreased the duplication a good bit. I left build-all-in-one-linux in the makefile since it is used for building the all in one image and I didn't want to leak setting the GOOS var outside of make.

Is there any other changes needed on this? I think I covered all the feedback. The one thing that will need to be done before the merge is setting the deploy key for travis to push the artifacts to GitHub - this will need to be done by someone who has access to publish release artifacts.

yurishkuro

lgtm - I'll merge so that I can tweak the API key, etc.

grounded042 requested review from black-adder, pavolloffay, vprithvi and yurishkuro as code owners April 4, 2018 17:32

add make and travis options for building for windows

bc13eff

Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

yurishkuro reviewed Apr 4, 2018

View reviewed changes

grounded042 added 3 commits April 5, 2018 11:29

add darwin build

0120ea5

Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

use make for windows in the travis setup since darwin uses make as well

896fa37

Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

add script to package files for deployment

d98cde9

Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

grounded042 and others added 4 commits April 5, 2018 13:35

add linux make target

19ca6e6

Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

missing 'then'

222e388

Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

cleanup white space and wrong data

527a042

Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

Merge branch 'master' into binary_distro

cf9f34d

ghost assigned yurishkuro Apr 10, 2018

ghost added the review label Apr 10, 2018

yurishkuro reviewed Apr 10, 2018

View reviewed changes

yurishkuro and others added 2 commits April 12, 2018 14:59

Merge branch 'master' into binary_distro

0810615

decrease duplication

4488a72

also, correct the travis deploy to deploy the tar.gz files Signed-off-by: Jon Carl <jon.carl.42@gmail.com>

yurishkuro approved these changes Apr 21, 2018

View reviewed changes

yurishkuro merged commit 222db21 into jaegertracing:master Apr 21, 2018

ghost removed the review label Apr 21, 2018

yurishkuro changed the title ~~add make and travis options for building for windows~~ Publish binaries to GitHub Releases Apr 27, 2018

yurishkuro mentioned this pull request Apr 27, 2018

Add pluggable shared libraries releases? jaegertracing/jaeger-client-cpp#81

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Publish binaries to GitHub Releases #766

Publish binaries to GitHub Releases #766

grounded042 commented Apr 4, 2018

coveralls commented Apr 4, 2018 •

edited

Loading

yurishkuro Apr 4, 2018

grounded042 Apr 4, 2018 •

edited

Loading

jpkrohling Apr 5, 2018

grounded042 Apr 5, 2018

jpkrohling Apr 5, 2018

yurishkuro Apr 5, 2018

yurishkuro Apr 4, 2018

grounded042 Apr 4, 2018

grounded042 Apr 4, 2018

yurishkuro Apr 4, 2018 •

edited

Loading

grounded042 Apr 5, 2018

yurishkuro Apr 4, 2018

black-adder Apr 4, 2018

grounded042 Apr 4, 2018

jpkrohling Apr 5, 2018

grounded042 Apr 5, 2018

grounded042 Apr 5, 2018 •

edited

Loading

yurishkuro commented Apr 4, 2018

grounded042 commented Apr 5, 2018

yurishkuro Apr 10, 2018

yurishkuro Apr 10, 2018

grounded042 Apr 16, 2018

grounded042 commented Apr 20, 2018

yurishkuro left a comment

Publish binaries to GitHub Releases #766

Publish binaries to GitHub Releases #766

Conversation

grounded042 commented Apr 4, 2018

coveralls commented Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

grounded042 Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yurishkuro Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grounded042 Apr 5, 2018 • edited Loading

Choose a reason for hiding this comment

yurishkuro commented Apr 4, 2018

grounded042 commented Apr 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grounded042 commented Apr 20, 2018

yurishkuro left a comment

Choose a reason for hiding this comment

coveralls commented Apr 4, 2018 •

edited

Loading

grounded042 Apr 4, 2018 •

edited

Loading

yurishkuro Apr 4, 2018 •

edited

Loading

grounded042 Apr 5, 2018 •

edited

Loading