Github action for jaeger packages and docker image #2740

Ashmita152 · 2021-01-26T14:55:48Z

Signed-off-by: Ashmita Bohara [email protected]

Which problem is this PR solving?

Adds gitbub action to build docker images and packages
Part of Migrate from Travis to GitHub actions #2645

Short description of the changes

Release part is still pending. Looking at it, I think we need to use https://github.com/actions/upload-release-asset action but not quite sure how to make it work without changing our current release process.

codecov · 2021-01-26T15:08:05Z

Codecov Report

Merging #2740 (b80fa91) into master (2ff0a3d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2740   +/-   ##
=======================================
  Coverage   95.83%   95.83%           
=======================================
  Files         217      217           
  Lines        9656     9656           
=======================================
  Hits         9254     9254           
  Misses        331      331           
  Partials       71       71

Impacted Files	Coverage Δ
cmd/collector/app/server/zipkin.go	`73.07% <0.00%> (-3.85%)`	⬇️
plugin/storage/integration/integration.go	`77.90% <0.00%> (+0.55%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ff0a3d...76cf30f. Read the comment docs.

yurishkuro

Release part is still pending. Looking at it, I think we need to use https://github.com/actions/upload-release-asset action but not quite sure how to make it work without changing our current release process.

well, (a) the current release process is non-functional since Travis refuses to run out jobs, and (b) do you have reasons to suspect that scripts/travis/package-deploy.sh would not work?

.github/workflows/ci-packages-build.yml

scripts/travis/upload-all-docker-images.sh

.github/workflows/ci-packages-build.yml

- Build tar file for jaeger components Signed-off-by: Ashmita Bohara <[email protected]>

Ashmita152 · 2021-01-27T03:42:12Z

do you have reasons to suspect that scripts/travis/package-deploy.sh would not work?

Sorry my bad in not explaining my point properly. This PR doesn't add changes equivalent to https://github.com/jaegertracing/jaeger/blob/master/.travis.yml#L93-L104 part of the travis configuration. For that we need to use https://github.com/actions/upload-release-asset But I am confused about how to make it work equivalently because of this tags: true travis configuration (https://github.com/jaegertracing/jaeger/blob/master/.travis.yml#L102)

yurishkuro · 2021-01-27T03:54:06Z

But I am confused about how to make it work equivalently because of this tags: true travis configuration

One option would be to make the whole workflow contingent on the presence of a tag, but then we'd miss any compile errors due to arch differences. So the other alternative is to do what the upload to docker script is doing, i.e. check the tag explicitly and skip the upload of bundles if no tag. Then the workflow will always run.

Ashmita152 · 2021-01-27T05:34:36Z

Do you recommend having a new github action like ci-release.yml which will run only when the tag is pushed ?

on:
  push:
    tags: [ '*' ]

Ashmita152 · 2021-01-27T05:37:08Z

Also looking at the example of README of https://github.com/actions/upload-release-asset it seems like it depends on another action https://github.com/actions/create-release (this part we do manually as part of our current release process)

I was thinking maybe we can probably set draft: true in create-release action, thus changing minimal in our current new version release process.

yurishkuro · 2021-01-27T05:49:10Z

We don't want to create a release from actions, because out workflow is (1) create release manually, (2) use CI to upload artifacts into it.

It doesn't look like upload-release-asset depends on create-release, but it does need upload_url for the release (don't know why GH makes it so hard). We could use this one instead https://github.com/bruceadams/get-release, although I would've preferred an official one. There's an open issue actions/upload-release-asset#34

yurishkuro · 2021-01-27T05:50:21Z

looks like this could be used ${{ github.event.release.upload_url }}, assuming that the workflow runs at the appropriate time

Ashmita152 · 2021-01-27T05:52:34Z

Sure Yuri, Thank you for the feedback. I will try out as you suggested in my forked repo.

Signed-off-by: Ashmita Bohara <[email protected]>

Ashmita152 · 2021-01-30T17:46:34Z

I tried updating the PR as per your comments.

Since the GitHub Action https://github.com/actions/upload-release-asset doesn't support uploading multiple files in one task by using regex, I had to use https://github.com/marketplace/actions/upload-files-to-a-github-release action instead.

I tried this PR in my jaeger's fork by creating a release manually (https://github.com/Ashmita152/jaeger/releases/tag/v1.25.0). It triggered the ci-release GitHub Action whose run you can find here: https://github.com/Ashmita152/jaeger/runs/1798106742?check_suite_focus=true

.github/workflows/ci-docker-build.yml

.github/workflows/ci-build-binaries.yml

.github/workflows/ci-release.yml

Signed-off-by: Ashmita Bohara <[email protected]>

Ashmita152 · 2021-01-30T18:01:26Z

Thank you for the feedbacks Yuri. Those were nice catches. I have updated the PR removing those.

Signed-off-by: Ashmita Bohara <[email protected]>

Ashmita152 · 2021-01-30T18:32:55Z

I have created a new release in my forked repo: https://github.com/Ashmita152/jaeger/releases/tag/v1.35.0 and CI for the release job is: https://github.com/Ashmita152/jaeger/runs/1798332218?check_suite_focus=true

This time the BRANCH is populating properly:

we are on branch=v1.35.0

yurishkuro · 2021-01-30T20:11:21Z

.github/workflows/ci-build-binaries.yml

+
+    - name: Fetch git tags
+      run: |
+        git fetch --prune --unshallow --tags


why do we need this step when building binaries?

also, when we do need tags, can we achieve the same effect with this setting?

# Number of commits to fetch. 0 indicates all history for all branches and tags. # Default: 1 fetch-depth: ''

I think if we want to pull tags as part of checkout action, only option from my understanding is to set fetch-depth: 0 which will also fetch all the commit history of all branches which will increase build time. I'm unable to find an option for just pulling the tags without commit history.

maybe it's worth creating a PR for the official action to add a new parameter like tags:

.github/workflows/ci-release.yml

Signed-off-by: Ashmita Bohara <[email protected]>

yurishkuro

Thanks!

yurishkuro · 2021-02-01T05:06:36Z

@Ashmita152 seeing some odd fatal: logs in the build-binaries:

https://github.com/jaegertracing/jaeger/runs/1803971666?check_suite_focus=true#step:6:9

GOOS=linux GOARCH=amd64 make build-platform-binaries
make[1]: Entering directory '/home/runner/work/jaeger/jaeger'
fatal: No names found, cannot describe anything.

Ashmita152 · 2021-02-01T19:14:00Z

Hi @yurishkuro

build-binaries step is taking ~ 30 minutes right now since it is building for all platforms. Do you recommend dividing it into separate jobs per platform ?

yurishkuro · 2021-02-01T19:15:23Z

yeah, i think it would be neat if we could build them in parallel

Ashmita152 requested a review from a team as a code owner January 26, 2021 14:55

Ashmita152 requested a review from joe-elliott January 26, 2021 14:55

mergify bot assigned jpkrohling Jan 26, 2021

mergify bot requested a review from jpkrohling January 26, 2021 14:56

yurishkuro reviewed Jan 26, 2021

View reviewed changes

yurishkuro mentioned this pull request Jan 26, 2021

Migrate from Travis to GitHub actions #2645

Closed

16 tasks

- Build docker images for jaeger components

55117ab

- Build tar file for jaeger components Signed-off-by: Ashmita Bohara <[email protected]>

Ashmita152 added 2 commits January 31, 2021 00:26

Add action for release

372410f

Signed-off-by: Ashmita Bohara <[email protected]>

Set DEPLOY as true for package binary script

29dbdec

Signed-off-by: Ashmita Bohara <[email protected]>

yurishkuro reviewed Jan 30, 2021

View reviewed changes

.github/workflows/ci-docker-build.yml Outdated Show resolved Hide resolved

.github/workflows/ci-build-binaries.yml Outdated Show resolved Hide resolved

.github/workflows/ci-release.yml Outdated Show resolved Hide resolved

.github/workflows/ci-release.yml Outdated Show resolved Hide resolved

Ashmita152 added 2 commits January 31, 2021 01:52

Fix BRANCH env variable for release event

47dc688

Signed-off-by: Ashmita Bohara <[email protected]>

Feedbacks

7dfdf28

Signed-off-by: Ashmita Bohara <[email protected]>

Fix BRANCH for release

61c932d

Signed-off-by: Ashmita Bohara <[email protected]>

yurishkuro reviewed Jan 30, 2021

View reviewed changes

.github/workflows/ci-release.yml Show resolved Hide resolved

yurishkuro reviewed Jan 30, 2021

View reviewed changes

.github/workflows/ci-release.yml Show resolved Hide resolved

yurishkuro reviewed Jan 30, 2021

View reviewed changes

.github/workflows/ci-release.yml Show resolved Hide resolved

Feedbacks

76cf30f

Signed-off-by: Ashmita Bohara <[email protected]>

yurishkuro approved these changes Jan 31, 2021

View reviewed changes

yurishkuro merged commit 92f670b into jaegertracing:master Jan 31, 2021

Ashmita152 mentioned this pull request Feb 1, 2021

Fix errors in build-binaries task #2754

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Github action for jaeger packages and docker image #2740

Github action for jaeger packages and docker image #2740

Ashmita152 commented Jan 26, 2021 •

edited

Loading

codecov bot commented Jan 26, 2021 •

edited

Loading

yurishkuro left a comment

Ashmita152 commented Jan 27, 2021 •

edited

Loading

yurishkuro commented Jan 27, 2021

Ashmita152 commented Jan 27, 2021

Ashmita152 commented Jan 27, 2021

yurishkuro commented Jan 27, 2021

yurishkuro commented Jan 27, 2021

Ashmita152 commented Jan 27, 2021

Ashmita152 commented Jan 30, 2021

Ashmita152 commented Jan 30, 2021

Ashmita152 commented Jan 30, 2021

yurishkuro Jan 30, 2021

yurishkuro Jan 30, 2021

Ashmita152 Jan 31, 2021

yurishkuro Jan 31, 2021

yurishkuro left a comment

yurishkuro commented Feb 1, 2021

Ashmita152 commented Feb 1, 2021

yurishkuro commented Feb 1, 2021

Github action for jaeger packages and docker image #2740

Github action for jaeger packages and docker image #2740

Conversation

Ashmita152 commented Jan 26, 2021 • edited Loading

Which problem is this PR solving?

Short description of the changes

codecov bot commented Jan 26, 2021 • edited Loading

Codecov Report

yurishkuro left a comment

Choose a reason for hiding this comment

Ashmita152 commented Jan 27, 2021 • edited Loading

yurishkuro commented Jan 27, 2021

Ashmita152 commented Jan 27, 2021

Ashmita152 commented Jan 27, 2021

yurishkuro commented Jan 27, 2021

yurishkuro commented Jan 27, 2021

Ashmita152 commented Jan 27, 2021

Ashmita152 commented Jan 30, 2021

Ashmita152 commented Jan 30, 2021

Ashmita152 commented Jan 30, 2021

yurishkuro Jan 30, 2021

Choose a reason for hiding this comment

yurishkuro Jan 30, 2021

Choose a reason for hiding this comment

Ashmita152 Jan 31, 2021

Choose a reason for hiding this comment

yurishkuro Jan 31, 2021

Choose a reason for hiding this comment

yurishkuro left a comment

Choose a reason for hiding this comment

yurishkuro commented Feb 1, 2021

Ashmita152 commented Feb 1, 2021

yurishkuro commented Feb 1, 2021

Ashmita152 commented Jan 26, 2021 •

edited

Loading

codecov bot commented Jan 26, 2021 •

edited

Loading

Ashmita152 commented Jan 27, 2021 •

edited

Loading