Add arm64 support for binaries and docker images #2176

MrXinWang · 2020-04-10T03:57:29Z

Which problem is this PR solving?

Platform support for arm64.

Resolves #1656

Short description of the changes

This commit adds building support for binaries and a Dockerfile for all-in-one container image.

Signed-off-by: Henry Wang Henry.Wang@arm.com

codecov · 2020-04-10T06:31:04Z

Codecov Report

Merging #2176 into master will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2176      +/-   ##
==========================================
+ Coverage   96.11%   96.15%   +0.03%     
==========================================
  Files         219      219              
  Lines       10658    10658              
==========================================
+ Hits        10244    10248       +4     
+ Misses        356      353       -3     
+ Partials       58       57       -1

Impacted Files	Coverage Δ
...lugin/sampling/strategystore/adaptive/processor.go	`100.00% <0.00%> (+0.79%)`	⬆️
cmd/query/app/static_handler.go	`85.83% <0.00%> (+1.66%)`	⬆️

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 06c8f7c...90b5c3f. Read the comment docs.

cmd/all-in-one/Dockerfile.aarch64

scripts/travis/build-all-in-one-image.sh

MrXinWang · 2020-04-17T09:16:01Z

Resolved.
Hi @yurishkuro sorry to bother but I think the CI machine currently has come issues with the GPG key?
As it gives me output in https://travis-ci.org/github/jaegertracing/jaeger/jobs/676105369

gpg: key 86E50310: public key "Yarn Packaging <yarn@dan.cx>" imported
gpg: Total number processed: 1
gpg:               imported: 1  (RSA: 1)
gpg: Signature made Mon 09 Mar 2020 03:52:13 PM UTC using RSA key ID 69475BAA
gpg: BAD signature from "Yarn Packaging <yarn@dan.cx>"
> GPG signature for this Yarn release is invalid! This is BAD and may mean the release has been tampered with. It is strongly recommended that you report this to the Yarn developers.

MrXinWang · 2020-05-21T09:57:37Z

Hi @yurishkuro, this PR has been a while after my "workaround" of this binary naming issue. Anything new about the "renaming the binaries consistently" or we leave it as it is? Could you please review again? Thanks!

yurishkuro · 2020-05-22T03:14:01Z

I put it on the agenda for the next project meeting, but let me ping ppl directly to see if we can resolve faster: @pavolloffay @objectiser

objectiser · 2020-05-22T13:17:52Z

@yurishkuro I'm fine with the rename, to avoid duplication.

yurishkuro · 2020-05-22T18:36:47Z

@MrXinWang do you want to (a) rebase and (b) change Makefile to have a single name format for every binary?

MrXinWang · 2020-05-25T02:49:27Z

Hi @yurishkuro.

All done :)) Please review again, thanks!

scripts/travis/build-all-in-one-image.sh

scripts/travis/package-deploy.sh

cmd/agent/Dockerfile

yurishkuro

Could you explain a bit how you tested this? Unfortunately, not all deployment steps are running on PR CI.

MrXinWang · 2020-05-27T01:47:43Z

Hi @yurishkuro ! I tested on both my x86_64 and arm64 machine with following methods:

Build all binaries and docker images by:

git clone https://github.com/jaegertracing/jaeger.git && cd jaeger
git submodule update --init --recursive
make install-tools
make docker

Use file command to check if the binary has the right architecture. Use docker run command to test if all docker images can be successfully started.

yurishkuro · 2020-05-27T03:29:57Z

Could you also try these steps?

./scripts/travis/build-docker-images.sh
make build-all-platforms && ./scripts/travis/package-deploy.sh

Also, please add a change to the CHANGELOG describing the breaking changes to binary names in the tarball distributions.

We probably also need to change some docs, e.g.

https://www.jaegertracing.io/download/ only links to amd64 tarballs, we may want to mention that there are other arch's available
https://www.jaegertracing.io/docs/1.18/windows/ shows examples with concrete binary names, which may not be correct anymore.

MrXinWang · 2020-05-27T04:22:27Z

@yurishkuro Tested these two commands, both worked well.

I have also added the related change to CHANGELOG.md

yurishkuro · 2020-05-27T04:24:22Z

minor suggestion: in the future, try to avoid force-pushing a single commit, it makes it impossible to see incremental changes, e.g. in the last one the whole Makefile is shown as changed again. We squash commits before merge anyway.

Makefile

MrXinWang · 2020-05-27T05:10:14Z

minor suggestion: in the future, try to avoid force-pushing a single commit

Thanks for the suggestion, yes that makes sense. I did some rebase work and to make the commit tree clean (the CI magically failed if I commit in 2 commits, however if I squashed them it passes...), I used squash and force push again....sorry about that...I will try to avoid this from now on.

yurishkuro

LGTM

@pavolloffay could you take another look in case I missed something?

pavolloffay · 2020-05-28T15:09:55Z

CHANGELOG.md

@@ -7,6 +7,7 @@ Changes by Version
 ### Backend Changes

 #### Breaking Changes
+* Add arm64 support for binaries and docker images. Rename the released binary name format to <Name>-<OS>-<ARCH>, tarball name format to jaeger-<Version>-<OS>-<ARCH> ([#2176](https://github.com/jaegertracing/jaeger/pull/2176), [@MrXinWang](https://github.com/MrXinWang))


The released binary names or tarball names were not changed. Or am I missing something?

The build names via make build- is an internal detail, hence we don't have to list this as a breaking change.

Add arm64 support for binaries and make architecture configurable in docker images.

The tarball names have not changed, but binary names inside them have changed to include arch.

The binary names seem the same as before:

Packaging into jaeger-1.18.0-linux-amd64.tar.gz: jaeger-1.18.0-linux-amd64/ jaeger-1.18.0-linux-amd64/jaeger-agent jaeger-1.18.0-linux-amd64/jaeger-all-in-one jaeger-1.18.0-linux-amd64/example-hotrod jaeger-1.18.0-linux-amd64/jaeger-query jaeger-1.18.0-linux-amd64/jaeger-ingester jaeger-1.18.0-linux-amd64/jaeger-collector Packaging into jaeger-1.18.0-darwin-amd64.tar.gz: jaeger-1.18.0-darwin-amd64/ jaeger-1.18.0-darwin-amd64/jaeger-agent jaeger-1.18.0-darwin-amd64/jaeger-all-in-one jaeger-1.18.0-darwin-amd64/example-hotrod jaeger-1.18.0-darwin-amd64/jaeger-query jaeger-1.18.0-darwin-amd64/jaeger-ingester jaeger-1.18.0-darwin-amd64/jaeger-collector Packaging into jaeger-1.18.0-windows-amd64.tar.gz: jaeger-1.18.0-windows-amd64/ jaeger-1.18.0-windows-amd64/jaeger-agent.exe jaeger-1.18.0-windows-amd64/jaeger-ingester.exe jaeger-1.18.0-windows-amd64/jaeger-all-in-one.exe jaeger-1.18.0-windows-amd64/jaeger-query.exe jaeger-1.18.0-windows-amd64/example-hotrod.exe jaeger-1.18.0-windows-amd64/jaeger-collector.exe

Hi @pavolloffay ! I think now the x86_64 binaries are added an amd64 after linux, before they are just -linux. Arm64 and s390x ones are the same. This can be derived from the Makefile changes, for example: here.

maybe we strip all suffixes before creating tarballs?

@yurishkuro I checked the scripts/travis/package-deploy.sh and it turns out that @pavolloffay is right, there is a staging step where we rename suffixes of all binaries (the make build outputs are indeed internal details)...So I think I can remove the breaking change in CHANGELOG.md and remove that part to minor changes.

Makefile

pavolloffay · 2020-05-28T15:12:39Z

@MrXinWang just a couple of nits. Overall it looks good.

This commit adds building support for binaries and container images. This commit also changes binary name format in Makefile to make sure every binary has a single name format: <BinaryName>-<OS>-<ARCH> Signed-off-by: Henry Wang <Henry.Wang@arm.com>

Signed-off-by: Henry Wang <Henry.Wang@arm.com>

MrXinWang · 2020-05-29T01:39:20Z

Thanks @pavolloffay. Rebased and added 90b5c3f to fix your request. Please take a look.

pavolloffay · 2020-05-29T09:25:34Z

thanks @MrXinWang

yonidavidson · 2021-05-23T07:15:32Z

Hi, can anyone direct me to the Arm based Agent Docker image? we would like to run on AWS Graviton.

MrXinWang requested a review from a team as a code owner April 10, 2020 03:57

MrXinWang mentioned this pull request Apr 10, 2020

Jaeger agent arm support #1656

Closed

yurishkuro reviewed Apr 10, 2020

View reviewed changes

cmd/all-in-one/Dockerfile.aarch64 Outdated Show resolved Hide resolved

yurishkuro reviewed Apr 10, 2020

View reviewed changes

scripts/travis/build-all-in-one-image.sh Outdated Show resolved Hide resolved

MrXinWang changed the title ~~Add arm64 support for binaries and allinone image~~ Add arm64 support for binaries and docker images Apr 17, 2020

MrXinWang requested a review from yurishkuro April 17, 2020 08:46

yurishkuro reviewed May 25, 2020

View reviewed changes

scripts/travis/build-all-in-one-image.sh Outdated Show resolved Hide resolved

yurishkuro reviewed May 25, 2020

View reviewed changes

scripts/travis/package-deploy.sh Outdated Show resolved Hide resolved

yurishkuro reviewed May 26, 2020

View reviewed changes

scripts/travis/package-deploy.sh Outdated Show resolved Hide resolved

yurishkuro reviewed May 26, 2020

View reviewed changes

cmd/agent/Dockerfile Outdated Show resolved Hide resolved

yurishkuro approved these changes May 27, 2020

View reviewed changes

yurishkuro reviewed May 27, 2020

View reviewed changes

Makefile Show resolved Hide resolved

yurishkuro reviewed May 27, 2020

View reviewed changes

Makefile Show resolved Hide resolved

yurishkuro approved these changes May 27, 2020

View reviewed changes

pavolloffay reviewed May 28, 2020

View reviewed changes

MrXinWang added 2 commits May 29, 2020 09:29

Add default GOOS in Makefile and move breaking change of CHANGELOG.md

90b5c3f

Signed-off-by: Henry Wang <Henry.Wang@arm.com>

pavolloffay approved these changes May 29, 2020

View reviewed changes

pavolloffay merged commit 2fdd1cd into jaegertracing:master May 29, 2020

jpkrohling mentioned this pull request Jun 15, 2020

arm64 support jaegertracing/jaeger-operator#1086

Closed

jpkrohling added this to the Release 1.23.0 milestone Jun 4, 2021

GaruGaru mentioned this pull request Jul 4, 2021

Docker images for linux/arm64 #3124

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add arm64 support for binaries and docker images #2176

Add arm64 support for binaries and docker images #2176

MrXinWang commented Apr 10, 2020 •

edited by pavolloffay

Loading

codecov bot commented Apr 10, 2020 •

edited

Loading

MrXinWang commented Apr 17, 2020 •

edited

Loading

MrXinWang commented May 21, 2020 •

edited

Loading

yurishkuro commented May 22, 2020

objectiser commented May 22, 2020

yurishkuro commented May 22, 2020

MrXinWang commented May 25, 2020

yurishkuro left a comment

MrXinWang commented May 27, 2020

yurishkuro commented May 27, 2020

MrXinWang commented May 27, 2020

yurishkuro commented May 27, 2020

MrXinWang commented May 27, 2020 •

edited

Loading

yurishkuro left a comment

pavolloffay May 28, 2020

pavolloffay May 28, 2020

yurishkuro May 28, 2020

pavolloffay May 28, 2020

MrXinWang May 29, 2020

yurishkuro May 29, 2020

MrXinWang May 29, 2020 •

edited

Loading

pavolloffay commented May 28, 2020

MrXinWang commented May 29, 2020

pavolloffay commented May 29, 2020

yonidavidson commented May 23, 2021

Add arm64 support for binaries and docker images #2176

Add arm64 support for binaries and docker images #2176

Conversation

MrXinWang commented Apr 10, 2020 • edited by pavolloffay Loading

Which problem is this PR solving?

Short description of the changes

codecov bot commented Apr 10, 2020 • edited Loading

Codecov Report

MrXinWang commented Apr 17, 2020 • edited Loading

MrXinWang commented May 21, 2020 • edited Loading

yurishkuro commented May 22, 2020

objectiser commented May 22, 2020

yurishkuro commented May 22, 2020

MrXinWang commented May 25, 2020

yurishkuro left a comment

Choose a reason for hiding this comment

MrXinWang commented May 27, 2020

yurishkuro commented May 27, 2020

MrXinWang commented May 27, 2020

yurishkuro commented May 27, 2020

MrXinWang commented May 27, 2020 • edited Loading

yurishkuro left a comment

Choose a reason for hiding this comment

pavolloffay May 28, 2020

Choose a reason for hiding this comment

pavolloffay May 28, 2020

Choose a reason for hiding this comment

yurishkuro May 28, 2020

Choose a reason for hiding this comment

pavolloffay May 28, 2020

Choose a reason for hiding this comment

MrXinWang May 29, 2020

Choose a reason for hiding this comment

yurishkuro May 29, 2020

Choose a reason for hiding this comment

MrXinWang May 29, 2020 • edited Loading

Choose a reason for hiding this comment

pavolloffay commented May 28, 2020

MrXinWang commented May 29, 2020

pavolloffay commented May 29, 2020

yonidavidson commented May 23, 2021

MrXinWang commented Apr 10, 2020 •

edited by pavolloffay

Loading

codecov bot commented Apr 10, 2020 •

edited

Loading

MrXinWang commented Apr 17, 2020 •

edited

Loading

MrXinWang commented May 21, 2020 •

edited

Loading

MrXinWang commented May 27, 2020 •

edited

Loading

MrXinWang May 29, 2020 •

edited

Loading