Build binaries for Linux on IBM Z / s390x architecture

[prankkelkar]

Signed-off-by: Prasanna Kelkar [email protected]

Which problem is this PR solving?

• PR not intended to resolve specific issue but As Travis CI officially supports s390x builds, adding support for same.

Short description of the changes

• Added s390x specific changes to skip usage of RACE flag as it is not supported on s390x architecture.
• Added type casting in plugin/storage/badger/stats_linux.go file to resolve mismatched types int64 and uint32 issue on s390x platform.

[jpkrohling]

Please, see #1973. There are two main problems:

1. the extra time required to perform a build. Instead of running the whole build in a new arch, only the fast unit-tests should be executed
2. what happens if there's a new problem in the future? Are you able to commit to support this when it needs maintenance? The current maintainers do not have the necessary bandwidth to support arches that aren't used by ourselves...

[codecov]

Codecov Report

Merging #1982 into master will increase coverage by 0.38%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1982      +/-   ##
==========================================
+ Coverage   96.99%   97.38%   +0.38%     
==========================================
  Files         203      206       +3     
  Lines       10064    10136      +72     
==========================================
+ Hits         9762     9871     +109     
+ Misses        264      222      -42     
- Partials       38       43       +5

Impacted Files	Coverage Δ
plugin/storage/badger/stats_linux.go	100% <100%> (ø)	⬆️
pkg/queue/bounded_queue.go	94.8% <0%> (-5.2%)	⬇️
cmd/query/app/static_handler.go	86.84% <0%> (-1.76%)	⬇️
plugin/storage/cassandra/spanstore/reader.go	100% <0%> (ø)	⬆️
cmd/collector/app/builder/span_handler_builder.go	100% <0%> (ø)	⬆️
...in/sampling/strategystore/static/strategy_store.go	100% <0%> (ø)	⬆️
cmd/agent/app/agent.go	100% <0%> (ø)	⬆️
cmd/builder/builder_options.go
cmd/agent/app/httpserver/server.go
cmd/agent/app/httpserver/srv.go	100% <0%> (ø)
... and 6 more

---

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 46d7f17...ac1d0ae. Read the comment docs.

[prankkelkar]

Please, see #1973. There are two main problems:

1. the extra time required to perform a build. Instead of running the whole build in a new arch, only the fast unit-tests should be executed
2. what happens if there's a new problem in the future? Are you able to commit to support this when it needs maintenance? The current maintainers do not have the necessary bandwidth to support arches that aren't used by ourselves...

1. I have modified the changes as per you instructions to run only unit tests for s390x architecture.
2. We already build and maintain jaeger on s390x. Our build instructions are published here

[yurishkuro]

We're currently not running any tests on Windows or MacOS platforms, we only build the binaries for them. Could we do the same for this arch? This would have minimal impact on the CI time.

[prankkelkar]

Yes. It will be great if we can have binaries released for s390x architecture which could be downloaded directly. I believe changes in Makefile to add target for s390x in build-all-platforms should be enough. Please let me know if any more changes are needed.

We're currently not running any tests on Windows or MacOS platforms, we only build the binaries for them. Could we do the same for this arch? This would have minimal impact on the CI time.

[prankkelkar]

@yurishkuro Could you please guide me with the changes needed to release binaries for s390x ?

[yurishkuro]

Follow the path of make build-all-platforms in the Makefile.

[prankkelkar]

@yurishkuro While i was adding linux s390x support i came across the names of the binaries used. for eg. collector/collector-$(GOOS)
In the case of s390x as the os name is same i.e Linux, two binaries will be created with the same name as per current code. To resolve this issue I was planning to add $(GOARCH) only to s390x binary name to differentiate it from amd64 linux.
Is there any other naming conventions we have to follow?

[yurishkuro]

Could we manage that at the directory name level, and keep the file names as is?

We may also consider not using OS in the binary name, only in the directory.

cc @jpkrohling @pavolloffay @objectiser

[jpkrohling]

Minikube adds both to the binaries:

minikube-linux-amd64
minikube-windows-amd64.exe
minikube-darwin-amd64

In our case, it would probably be too noisy, so, the idea of grouping them by OS makes sense.

[prankkelkar]

I have currently added following code to handle this scenario

.PHONY: build-binaries-s390x
build-binaries-s390x:
	GOOS=linux GOARCH=s390x $(MAKE) build-platform-binaries

.PHONY: build-agent
build-agent:
ifeq ($(GOARCH), s390x)
	CGO_ENABLED=0 installsuffix=cgo go build -o ./cmd/agent/agent-$(GOOS)-$(GOARCH) $(BUILD_INFO) ./cmd/agent/main.go
else
	CGO_ENABLED=0 installsuffix=cgo go build -o ./cmd/agent/agent-$(GOOS) $(BUILD_INFO) ./cmd/agent/main.go
endif

This will only change the name of the binary of s390x and rest will remain as it is.
I have also tweaked ./scripts/travis/package-deploy.sh to handle this change. Should i go ahead to update PR with this approach?

[yurishkuro]

sgtm

[prankkelkar]

@yurishkuro I have updated the PR to cross compile and release the binaries. Please review the code and let me know if anything needs to be changed.

[yurishkuro]

Looks reasonable, thanks.