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

Add s390x support on multiarch docker images #2948

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
bae83d3
Add s390x support on multiarch docker images
kun-lu20 Mar 19, 2021
4c6cabb
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 Apr 21, 2021
c0de24a
Update tag computation and Dockerfile
kun-lu20 Apr 23, 2021
d973031
Optimize the code changes
kun-lu20 Apr 26, 2021
5273aa3
Optimize the code changes - update
kun-lu20 Apr 28, 2021
ef2e8a2
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 Apr 28, 2021
edb391d
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 May 3, 2021
7974892
Optimize the code changes - update
kun-lu20 May 4, 2021
a0362fc
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 May 5, 2021
2613b6c
Optimize the code changes - update
kun-lu20 May 10, 2021
119d8e8
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 May 11, 2021
321907c
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 May 14, 2021
170b373
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 May 28, 2021
23ede72
upgrade the base image version
kun-lu20 May 28, 2021
54ba010
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 Jun 11, 2021
027376a
Create a common shared script for docker images
kun-lu20 Jun 11, 2021
5a6b268
Refactor update
kun-lu20 Jun 14, 2021
2960e88
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 Jun 14, 2021
ba3e9bc
Remove cross-script dependencies of debugimg name
kun-lu20 Jun 15, 2021
2acd49f
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 Jun 17, 2021
4e4fcbb
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 Jun 23, 2021
878e983
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 Jun 24, 2021
18aad23
Fix the indentation issue
kun-lu20 Jun 24, 2021
56f3e4f
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 Jun 25, 2021
c492340
Add comment in debug Dockerfile
kun-lu20 Jun 25, 2021
a62c82f
Merge branch 'master' into enablement_docker_images_on_s390x
kun-lu20 Jun 30, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion .github/workflows/ci-all-in-one-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@ on:
pull_request:
branches: [ master ]

jobs:
jobs:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove whitespace

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace removed.

all-in-one:
runs-on: ubuntu-latest
services:
registry:
image: registry:2
ports:
- 5000:5000
jpkrohling marked this conversation as resolved.
Show resolved Hide resolved
steps:
- uses: actions/checkout@v2
with:
Expand All @@ -31,6 +36,12 @@ jobs:

- name: Install tools
run: make install-ci

- uses: docker/setup-qemu-action@v1

- uses: docker/setup-buildx-action@v1
with:
driver-opts: network=host

- name: Build, test, and publish all-in-one image
run: bash scripts/build-all-in-one-image.sh
Expand Down
18 changes: 14 additions & 4 deletions .github/workflows/ci-docker-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ on:
jobs:
docker-images:
runs-on: ubuntu-latest

services:
registry:
image: registry:2
ports:
- 5000:5000

steps:
- uses: actions/checkout@v2
with:
Expand All @@ -32,11 +39,14 @@ jobs:
- name: Install tools
run: make install-ci

- name: Build docker images
run: make docker
jpkrohling marked this conversation as resolved.
Show resolved Hide resolved
- uses: docker/setup-qemu-action@v1

- name: Upload docker images
run: bash scripts/upload-all-docker-images.sh
- uses: docker/setup-buildx-action@v1
with:
driver-opts: network=host

- name: Build and upload all docker images
run: bash scripts/build-upload-docker-images.sh
env:
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
QUAY_TOKEN: ${{ secrets.QUAY_TOKEN }}
19 changes: 13 additions & 6 deletions .github/workflows/ci-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ on:
jobs:
publish-release:
runs-on: ubuntu-latest
services:
registry:
image: registry:2
ports:
- 5000:5000

steps:
- uses: actions/checkout@v2
with:
Expand Down Expand Up @@ -49,13 +55,14 @@ jobs:
repo_token: ${{ secrets.GITHUB_TOKEN }}
if: steps.package-binaries.outcome == 'success'

- name: Build docker images
id: build-images
run: make docker
- uses: docker/setup-qemu-action@v1

- name: Upload docker images
run: bash scripts/upload-all-docker-images.sh
if: steps.build-images.outcome == 'success'
- uses: docker/setup-buildx-action@v1
with:
driver-opts: network=host

- name: Build and upload all docker images
run: bash scripts/build-upload-docker-images.sh
env:
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
QUAY_TOKEN: ${{ secrets.QUAY_TOKEN }}
Expand Down
86 changes: 83 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,17 @@ MOCKERY=mockery

.DEFAULT_GOAL := test-and-lint

BASE_IMAGE_MULTIARCH := localhost:5000/baseimg:$(VERSION)-$(shell echo $(ROOT_IMAGE) | tr : -)
PLATFORMS=linux/amd64,linux/s390x
INGESTER_TAG=$(subst JAGERCOMP,ingester,$(IMAGE_TAGS))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to do this for each jaeger component, isn't it the same value after all ? Also where is JAGERCOMP variable populated ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we need to, since each jaeger component has its unique name, hence the unique tag value.

JAGERCOMP is not a variable but a const string. We use it in the tag computation procedure in scripts/build-upload-docker-images.sh to get the value of variable IMAGE_TAGS. Then in Makefile L77-L84, we replace it with the name of each jaeger component in variable IMAGE_TAGS to get each actual tag value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a fan of these cross-dependences. If the string that contains JAGERCOMP is defined in the script, why can't the script resolve it as needed, without pushing the substitution into Makefile?

Also, Jaeger is spelled with 'e'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. I've corrected the spelling and moved the related code to the script.

AGENT_TAG=$(subst JAGERCOMP,agent,$(IMAGE_TAGS))
COLLECTOR_TAG=$(subst JAGERCOMP,collector,$(IMAGE_TAGS))
QUERY_TAG=$(subst JAGERCOMP,query,$(IMAGE_TAGS))
ESINDEX_TAG=$(subst JAGERCOMP,es-index-cleaner,$(IMAGE_TAGS))
ESROLLOVER_TAG=$(subst JAGERCOMP,es-rollover,$(IMAGE_TAGS))
TRACEGEN_TAG=$(subst JAGERCOMP,tracegen,$(IMAGE_TAGS))
ANONYMIZER_TAG=$(subst JAGERCOMP,anonymizer,$(IMAGE_TAGS))

.PHONY: test-and-lint
test-and-lint: test fmt lint

Expand Down Expand Up @@ -298,7 +309,8 @@ build-platform-binaries: build-agent \
build-all-in-one \
build-examples \
build-tracegen \
build-anonymizer
build-anonymizer \
build-esmapping-generator

.PHONY: build-all-platforms
build-all-platforms: build-binaries-linux build-binaries-windows build-binaries-darwin build-binaries-s390x build-binaries-arm64 build-binaries-ppc64le
Expand All @@ -309,9 +321,10 @@ docker-images-cassandra:
@echo "Finished building jaeger-cassandra-schema =============="

.PHONY: docker-images-elastic
docker-images-elastic: build-esmapping-generator-linux
docker-images-elastic:
GOOS=linux GOARCH=$(GOARCH) $(MAKE) build-esmapping-generator
docker build -t $(DOCKER_NAMESPACE)/jaeger-es-index-cleaner:${DOCKER_TAG} plugin/storage/es
docker build -t $(DOCKER_NAMESPACE)/jaeger-es-rollover:${DOCKER_TAG} plugin/storage/es -f plugin/storage/es/Dockerfile.rollover
docker build -t $(DOCKER_NAMESPACE)/jaeger-es-rollover:${DOCKER_TAG} plugin/storage/es -f plugin/storage/es/Dockerfile.rollover --build-arg TARGETARCH=$(GOARCH)
@echo "Finished building jaeger-es-indices-clean =============="

docker-images-jaeger-backend: TARGET = release
Expand Down Expand Up @@ -348,6 +361,73 @@ docker-images-only: docker-images-cassandra \
docker-images-tracegen \
docker-images-anonymizer

.PHONY: multiarch-docker
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we just move all this code into the script? Why do they need to be in the Makefile? I would restrict Makefile to building the binaries only, and then have all docker packaging etc. be done in the script.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we could. I've moved all code related to building and uploading multi-arch docker images from Makefile to the scripts.

multiarch-docker: build-binaries-linux \
build-binaries-s390x \
multiarch-docker-images-only

.PHONY: multiarch-docker-images-only
multiarch-docker-images-only: docker-images-jaeger-backend-multiarch \
docker-images-elastic-multiarch \
docker-images-tracegen-multiarch \
docker-images-anonymizer-multiarch

.PHONY: create-baseimage-multiarch
create-baseimage-multiarch:
docker buildx build -t $(BASE_IMAGE_MULTIARCH) --push \
--build-arg root_image=$(ROOT_IMAGE) \
--build-arg cert_image=$(CERT_IMAGE) \
--platform=$(PLATFORMS) \
docker/base
echo "Finished building multiarch base image =============="

.PHONY: docker-images-jaeger-backend-multiarch
docker-images-jaeger-backend-multiarch: create-baseimage-multiarch
for component in agent collector query ingester ; do \
docker buildx build --output "$(PUSHTAG)" \
--progress=plain --target release \
--build-arg base_image=$(BASE_IMAGE_MULTIARCH) \
--build-arg debug_image=$(GOLANG_IMAGE) \
--platform=$(PLATFORMS) \
--file cmd/$$component/Dockerfile \
$(subst JAGERCOMP,$$component,$(IMAGE_TAGS)) \
cmd/$$component; \
echo "Finished building $$component ==============" ; \
done

.PHONY: docker-images-elastic-multiarch
docker-images-elastic-multiarch:
docker buildx build --output "$(PUSHTAG)" \
--progress=plain \
--platform=$(PLATFORMS) \
${ESINDEX_TAG} \
plugin/storage/es
docker buildx build --output "$(PUSHTAG)" \
--progress=plain \
--platform=$(PLATFORMS) \
--file plugin/storage/es/Dockerfile.rollover \
${ESROLLOVER_TAG} \
plugin/storage/es
@echo "Finished building multiarch jaeger-es-indices-clean =============="

.PHONY: docker-images-tracegen-multiarch
docker-images-tracegen-multiarch:
docker buildx build --output "$(PUSHTAG)" \
--progress=plain \
--platform=$(PLATFORMS) \
${TRACEGEN_TAG} \
cmd/tracegen/
@echo "Finished building multiarch jaeger-tracegen =============="

.PHONY: docker-images-anonymizer-multiarch
docker-images-anonymizer-multiarch:
docker buildx build --output "$(PUSHTAG)" \
--progress=plain \
--platform=$(PLATFORMS) \
$(ANONYMIZER_TAG) \
cmd/anonymizer/
@echo "Finished building multiarch jaeger-anonymizer =============="

.PHONY: docker-push
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should delete this target, it's not used from GH Actions, and it probably incorrect given these changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've deleted this target.

docker-push:
@while [ -z "$$CONFIRM" ]; do \
Expand Down
2 changes: 1 addition & 1 deletion cmd/agent/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ ARG base_image
ARG debug_image

FROM $base_image AS release
ARG TARGETARCH=amd64
ARG TARGETARCH
ARG USER_UID=10001
COPY agent-linux-$TARGETARCH /go/bin/agent-linux
EXPOSE 5775/udp 6831/udp 6832/udp 5778/tcp
Expand Down
4 changes: 3 additions & 1 deletion cmd/all-in-one/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ ARG base_image
ARG debug_image

FROM $base_image AS release
ARG TARGETARCH=amd64
ARG TARGETARCH

# Agent zipkin.thrift compact
EXPOSE 5775/udp
Expand All @@ -28,6 +28,8 @@ EXPOSE 16686
COPY all-in-one-linux-$TARGETARCH /go/bin/all-in-one-linux
COPY sampling_strategies.json /etc/jaeger/

RUN chmod +x /go/bin/all-in-one-linux
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we adding this chmod here. Isn't the binary already executable ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right, the binary is already executable. I've removed this line from Dockerfile.


VOLUME ["/tmp"]
ENTRYPOINT ["/go/bin/all-in-one-linux"]
CMD ["--sampling.strategies-file=/etc/jaeger/sampling_strategies.json"]
Expand Down
2 changes: 1 addition & 1 deletion cmd/anonymizer/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
FROM scratch
ARG TARGETARCH=amd64
ARG TARGETARCH

COPY anonymizer-linux-$TARGETARCH /go/bin/anonymizer-linux
ENTRYPOINT ["/go/bin/anonymizer-linux"]
2 changes: 1 addition & 1 deletion cmd/collector/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ ARG base_image
ARG debug_image

FROM $base_image AS release
ARG TARGETARCH=amd64
ARG TARGETARCH
COPY collector-linux-$TARGETARCH /go/bin/collector-linux
EXPOSE 14250/tcp
ENTRYPOINT ["/go/bin/collector-linux"]
Expand Down
2 changes: 1 addition & 1 deletion cmd/ingester/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ ARG base_image
ARG debug_image

FROM $base_image AS release
ARG TARGETARCH=amd64
ARG TARGETARCH
COPY ingester-linux-$TARGETARCH /go/bin/ingester-linux
EXPOSE 14270/tcp 14271/tcp
ENTRYPOINT ["/go/bin/ingester-linux"]
Expand Down
2 changes: 1 addition & 1 deletion cmd/query/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ ARG base_image
ARG debug_image

FROM $base_image AS release
ARG TARGETARCH=amd64
ARG TARGETARCH
COPY query-linux-$TARGETARCH /go/bin/query-linux
EXPOSE 16686/tcp
ENTRYPOINT ["/go/bin/query-linux"]
Expand Down
2 changes: 1 addition & 1 deletion cmd/tracegen/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
FROM scratch
ARG TARGETARCH=amd64
ARG TARGETARCH

COPY tracegen-linux-$TARGETARCH /go/bin/tracegen-linux
ENTRYPOINT ["/go/bin/tracegen-linux"]
5 changes: 3 additions & 2 deletions plugin/storage/es/Dockerfile.rollover
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
FROM python:3-alpine3.11
ARG TARGETARCH

# Temporary fix for https://github.com/jaegertracing/jaeger/issues/1494
RUN pip install urllib3==1.24.3

RUN pip install elasticsearch elasticsearch-curator
COPY ./mappings/* /mappings/
COPY esRollover.py /es-rollover/
COPY esmapping-generator /usr/bin/
COPY esmapping-generator-linux-$TARGETARCH /usr/bin/esmapping-generator

ENTRYPOINT ["python3", "/es-rollover/esRollover.py"]
ENTRYPOINT ["python3", "/es-rollover/esRollover.py"]
Loading