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

Create and publish linux/amd64 and linux/arm64 builds on release #504

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

mmoscher
Copy link

@mmoscher mmoscher commented Oct 12, 2022

Change log description

This PR creates mutliarch builds of pravega/zookeeper-operator and pravega/zookeeper using docker buildx when a new GitHub release is crafted.

However... the available publish job in the ci.yaml GHA workflow was broken. For example, the job does no code-checkout at all and thus was uncapable of running (finding) the Makefile. Uhrgs.

Question: Is the provided GHA worklfow and your push Makefile-target used at all? IMHO not :/ :(
Because: a) the "ALTREPO" used in the Makefile for pushing image (make push) is pointing to another docker-hub repo where another user is pushing docker-images. For example, in the pravega docker-hub repos a user called pravegaci is pushing images, whereas in the emccorp repos the user is called ecsjenkins. Since the Makefile has only one docker login call, pushing with different users in a CI environment is impossible.
b) Additionally, the emccorp/zookeeper-operator:latest tag was created 3 years ago whereas the latest tag is ~5 months old. An automated push on release to the altrepo would have created the tag "latest" as well.

All-in-all: it seems there is another CI or manual build-process in place, for what reason I'm uncertain if this PR will help at all :(

@anishakj pretty please clarify how you are publishing your docker-images. Thanks :)

Purpose of the change

What the code does

  • fixed GHA CI workflow to be runnable at all ..
  • extends GHA CI workflow to use docker buildx
  • adds build-and-push-multiarch-image and build-and-push-multiarch-zk-image targets in makefile and calls these targets in the GHA CI workflow when a release was created
  • extends zookeeper-operator Dockerfile to be capable of multiarch builds, i.e. using TARGETOS and TARGETARCH
  • leverages docker hub's capability of easily hosting mutliarch images

How to verify it

image

@mmoscher mmoscher marked this pull request as ready for review October 12, 2022 21:19
@codecov
Copy link

codecov bot commented Oct 12, 2022

Codecov Report

Base: 85.05% // Head: 85.05% // No change to project coverage 👍

Coverage data is based on head (814ff04) compared to base (ae23010).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #504   +/-   ##
=======================================
  Coverage   85.05%   85.05%           
=======================================
  Files          12       12           
  Lines        1606     1606           
=======================================
  Hits         1366     1366           
  Misses        155      155           
  Partials       85       85           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

nishant-yt and others added 15 commits October 12, 2022 23:27
…he teardown script (pravega#483)

Signed-off-by: Nishant Gupta <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
* upgrading zookeeper to 3.7.1

Signed-off-by: Amit-Singh40 <[email protected]>

* updating zookeeper version in build.gradle.kts and readme file

Signed-off-by: Amit-Singh40 <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
Signed-off-by: Sava Lifanov <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
Signed-off-by: Marco Moscher <[email protected]>
@mmoscher
Copy link
Author

@anishakj seem's that the commit singoffs are broken.
image
IMHO this is wrong.

If this PR is helpful at all, I would create a new one with a single commit introducing these changes.

@derekm derekm requested a review from anishakj October 12, 2022 21:37
@anishakj
Copy link
Contributor

@mmoscher Have you verified the binary created for arm64

@mmoscher
Copy link
Author

@anishakj nope not yet, since the build can only be done in the emulated docker platform, thus in the container itself.
Will check if the container can be validated. Any hints? How to you validate your builds? Thanks in advance!

@janhoy
Copy link

janhoy commented Nov 23, 2022

Status of this?

@mmoscher
Copy link
Author

@janhoy sorry to say, but nothing new yet! Feel free to move on and submit PR's to my fork, if you like.
Maybe I will manage to free some time next week.

@@ -137,6 +137,28 @@ build-zk-image:
docker build --build-arg VERSION=$(VERSION) --build-arg DOCKER_REGISTRY=$(DOCKER_REGISTRY) --build-arg GIT_SHA=$(GIT_SHA) -t $(APP_REPO):$(VERSION) ./docker
docker tag $(APP_REPO):$(VERSION) $(APP_REPO):latest

build-and-push-multiarch-image:
Copy link
Contributor

Choose a reason for hiding this comment

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

If the push target is obsolete now, should we just remove it?

@@ -163,7 +185,7 @@ run-local:
go run ./main.go

login:
@docker login -u "$(DOCKER_USER)" -p "$(DOCKER_PASS)"
echo "$(DOCKER_PASS)" | docker login -u "$(DOCKER_USER)" --password-stdin
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you change the login target command-line to match the test-login command-line, but then you don't use it above in the GitHub Actions workflow during the publish. Does make login not work? Do we need to bind the secrets as environment variables before calling make login?

Here is an example of binding secrets to environment variables: https://github.com/pravega/pravega/blob/master/.github/workflows/gradle.yml#L152

@anishakj
Copy link
Contributor

@mmoscher Have you verified the binary created for arm64

@mmoscher Could you please address the comments provided by @derekm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Could you release a version that supports aarch64 Support ARM64 Any plan to support aarch64?
8 participants