-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
feat: build the Docker Driver for arm64 #9247
base: main
Are you sure you want to change the base?
Conversation
20e05b7
to
d9cac9a
Compare
@tucksaun could you resolve the conflicts? |
.drone/drone.yml
Outdated
@@ -1500,6 +1500,7 @@ name: docker-driver | |||
steps: | |||
- commands: | |||
- make docker-driver-push | |||
- make docker-driver-push PLUGIN_ARCH=-arm64 |
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.
You have to change the jsonnet file uinstead. I'll generate the Drone YML then.
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.
done
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.
@jeschkies a friendly ping to let you know this PR has been rebased but requires your intervention for the .drone/drone.yml
generation 🙂
d9cac9a
to
4d7a281
Compare
1898680
to
a1b1ecc
Compare
@jeschkies conflicts resolved |
Is there any issue preventing the advancement of this pull request? |
there were new conflicts I just resolved. excluding those, I believe someone (@jeschkies?) has to regenerate the |
5a31800
to
f7c1227
Compare
@tucksaun I'm sorry. I've switched teams and have missed GitHub notifications.
I was about to mentioned the multi arch image. How is the arm image tagged differently? |
IIRC docker plugins does not support multi arch images (at least it did not
went I worked on this)
…On Thu 22 Aug 2024 at 21:21, Karsten Jeschkies ***@***.***> wrote:
@tucksaun <https://github.com/tucksaun> I'm sorry. I've switched teams
and have missed GitHub notifications.
I would have loved to have a unified x64 and arm64 build but apparently
Docker drivers does not support multi arch images.
I was about to mentioned the multi arch image. How is the arm image tagged
differently?
—
Reply to this email directly, view it on GitHub
<#9247 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGUNZUMY3BU6T4BP5DTQ3DZSZB4LAVCNFSM6AAAAABLF7PFYSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBVGU3DENRZGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi, I think your PR would be very helpful is it a lot of work to solve conflicts to this PR can be accepted? |
f7c1227
to
288105a
Compare
@francescor not much for the Dockerfile so this is done. however this is a bit different for the pipeline because Drone has been recently removed (see #14273) and it seems like the docker plugin is neither build not released anymore (I can't find any reference to it anyway). @jeschkies, this means I'm not able to update the pipeline for testing or releasing anymore 🤷 |
I see this one https://hub.docker.com/r/miacis/loki-docker-driver is related to this context? |
I have no idea |
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.
We'd still have to pushblish the new image
I'm fine with landing this but as I understand we would not be able to publish the ARM Docker image as it would override |
Hi, we really badly need the ARM images, it will allow us to move all our setup into ARM with considerable savings :) |
They should be built with different tags. The ARM version uses an explicit tag so this should not impact x64 users |
AFAICT the full releasing of the plugin needs to be restore |
I didn't build it for a while (and bad timing as I'm away for conferences) but I can try to have a look in the coming days |
288105a
to
dec89fc
Compare
@francescor I just pushed the plugin at please note I didn't have the opportunity to test it so this comes with no warranty 😁 |
lovely, thank you!
yes, of course :) |
forgive me, @tucksaun how do I pull your image?
|
`docker plugin install tucksaun/loki-docker-driver:main-arm64 --alias loki
--grant-all-permissions` should make it work
…On Fri 11 Oct 2024 at 11:25, Fra R ***@***.***> wrote:
@francescor <https://github.com/francescor> I just pushed the plugin at
tucksaun/loki-docker-driver:main-arm64
forgive me, @tucksaun <https://github.com/tucksaun> how do I pull your
image?
docker pull ghcr.io/tucksaun/loki-docker-driver:main-arm64
—
Reply to this email directly, view it on GitHub
<#9247 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGUNZUNBF6RVH4RY5KQPLDZ26KR7AVCNFSM6AAAAABLF7PFYSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBXGAYDONRXGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
thanks @tucksaun I can install it
but then apparently the "old" plugin keeps showing up :(
and indeed, I still see that x86_64 emulator:
here the inspect (which correctly shows your plugin)
|
The host is an AWS
with installed If I remove it with |
@francescor I managed to reproduce the issue, I will try to have a look today or tomorrow |
**What this PR does / why we need it**: Add ARM64 build and release of the Docker driver in Drone pipeline **Which issue(s) this PR fixes**: Fixes grafana#5682 **Special notes for your reviewer**: I would have loved to have a unified x64 and arm64 build but apparently Docker drivers does not support multi arch images. So instead I went with tweaking the build steps to allow cross building the image for ARM64 and added the instructions to do so in `drone.yml`. **Checklist** - [x] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [x] Documentation added - [ ] Tests updated - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/upgrading/_index.md`
dec89fc
to
9d2a580
Compare
@francescor should be fixed |
Super! I confirm your loki image does not need thank you so much @tucksaun I'll provide more feeds in case I hit issues, but I can see logs in our grafana, so it should be OK Now we can definitively replace swarm node with ARM instances, with great savings! Thanks! |
What this PR does / why we need it:
Add ARM64 build and release of the Docker driver in Drone pipeline
Which issue(s) this PR fixes:
Fixes #5682
Special notes for your reviewer:
I would have loved to have a unified x64 and arm64 build but apparently Docker drivers does not support multi arch images. So instead I went with tweaking the build steps to allow cross building the image for ARM64 and added the instructions to do so in
drone.yml
.It seems there's a drift in the images published on Docker Hub versus the one documented for use. I updated it here but this might be wrong.
Checklist
CONTRIBUTING.md
guide (required)CHANGELOG.md
updateddocs/sources/upgrading/_index.md