-
Notifications
You must be signed in to change notification settings - Fork 854
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
Multi-arch/arm64 support #672
Conversation
@Blefish Thanks for this contribution! I need to take a close look at this, but I do not have the time now due to private circumstances. It might take a month or so before I get back on this. |
@Bert-R any update on this! |
@Blefish @Bert-R By the way when i teried kafdrop with image - ghcr.io/blefish/kafdrop:latest ERROR LOGS -
PLATEFORM INFO -
DEPLOYMENT CONFIG -
i am not sure if its code error or image error |
Yup I noticed that too, at random times. Seemed to happen more often when using SSL security protocol with kafka. It looked to me like some sort of kafka communications error at first glance. I am not sure, but I think it happened to me as well on the official release, but I need to confirm this. |
@Blefish when i remove the env variable SERVER_PORT it works fine on default port. your arm image too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for this contribution and sorry for the delay.
I added one question. Furthermore, I noticed a merge conflict on pom.xml
- | ||
name: Build with Maven | ||
run: mvn -B clean integration-test package assembly:single docker:build | ||
run: mvn -B clean integration-test package assembly:single docker:build docker:push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pushes snapshot builds to Docker Hub. Why is that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi. I think that in current master, it should also push snapshots and release builds are additionally tagged with :latest.
I think the reason why this is included now within the same build command is due to buildx - it builds the container in an embedded build environment (to emulate arm64) and since it's also wrapped inside docker-maven-plugin, there is no good way to get the built image back to running environment. So buildx is asked to directly push the image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. It also makes it easier for people that want to test with a snapshot build.
@davideicardi Do you agree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it looks good to me. Do you think that we need to change something for release build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think further changes are needed. The lines below take care of the release process and they still do what happened before: apply a tag of the release and add the latest
tag.
I'm going to merge this PR.
* push during build stage as a workaround for buildx * re-tag the built snapshot if it is a release
Rebased on latest master |
@Blefish Thanks again for this contribution! |
@Bert-R i just upgraded to obsidiandynamics/kafdrop:4.0.3-SNAPSHOT (arm image). it worked without any issue in first Go.
However i felt something weird in logging (formatting) maybe i am wrong- i'll keep it running. will report here if got any issues. |
@maipal-c If you are referring to the first weird characters, I suspect it is just the Kafdrop logo not rendered correctly, probably due to the font/spacing. |
@davideicardi yea it is as issue of lens k8s IDE. Kafdrop is all good. i am attaching the log file |
This PR consists of several small changes:
Old library is deprecated and does not have buildx support
Needed for buildx built images
Needed for multi-arch Docker images
I did a brief test on my personal repo by changing GHA code a bit to publish to personal registry and to see if :latest tagging still works as expected. Also ran this on my x86_64 laptop and additionally a t4g.small (aarch64) VM.
Code for the test: https://github.com/Blefish/kafdrop/tree/multi-arch-test
And the test images: https://github.com/Blefish/kafdrop/pkgs/container/kafdrop
Closes #627