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 support of arm64 for image karatelabs/karate-chrome #2422

Open
jandry opened this issue Oct 23, 2023 · 28 comments
Open

Add support of arm64 for image karatelabs/karate-chrome #2422

jandry opened this issue Oct 23, 2023 · 28 comments

Comments

@jandry
Copy link
Contributor

jandry commented Oct 23, 2023

Hi,
As a growing part of developer are using M1 or M2 and that you now support more officially karatelabs/karate-chrome, it could be great to support multi os on the image adding arm64

Regards

@ptrthomas
Copy link
Member

@jandry do consider contributing code or influencing someone who can

@jandry
Copy link
Contributor Author

jandry commented Oct 26, 2023

Not sure at all that solution is working in pipeline but here is a trial PR
#2424

@ptrthomas
Copy link
Member

@jandry thanks ! I have a concern with chromium - we have teams that have been using chrome stable for years, and we like the fact that you can test on a "real browser" and not worry about the risk of differences between chrome and chromium

I am ok to ship a second docker image based on chromium if that helps, you can see how we do the release build here - after build-docker.sh runs: https://github.com/karatelabs/karate/blob/develop/.github/workflows/maven-release.yml

@jandry
Copy link
Contributor Author

jandry commented Oct 27, 2023

Make sens. I will try to do that next xeek

jandry pushed a commit to jandry/karate that referenced this issue Nov 1, 2023
jandry pushed a commit to jandry/karate that referenced this issue Nov 1, 2023
jandry pushed a commit to jandry/karate that referenced this issue Nov 1, 2023
@jandry
Copy link
Contributor Author

jandry commented Nov 1, 2023

@ptrthomas I pushed an update that should build both images

I also switch default image pushed to local docker for tests depending platform for local testing. Default still the chrome one except on mac darwin

How can I check if push is working ? That the only part where I'm not sure : right tag, reuse of cache and login working

jandry pushed a commit to jandry/karate that referenced this issue Nov 1, 2023
@ptrthomas
Copy link
Member

@jandry you mean pushing to docker hub ? it is done as a github-action I can take care of that. we'll probably release 1.5.0.RC2 in a weeks time, so I will watch how it goes and tweak then

@ptrthomas
Copy link
Member

I believe there are ways to run github actions locally if you search, but I have never tried @jandry

thanks a lot for contributing this :)

@ptrthomas ptrthomas added this to the 1.5.0 milestone Nov 1, 2023
@jandry
Copy link
Contributor Author

jandry commented Nov 1, 2023

Thanks a lot for this super Open Source test framework

Yes I wanted to test in the git action to push is working fine. I'm working with gitlab CI usually so I'm not sure how is the isolation between actions is working. Because the "build docker" action is creating a cache in target/docker that is reused in the push action to retrieve already created layers.
Do you know if the target folder is kept between actions ? (It's not the case in gitlabci). If not it will try to rebuild and will failed because of empty target folder.

@ptrthomas
Copy link
Member

@jandry I really don't know, but will watch out for this. there is a concept of "matrix build" and I think one option is to fork the build into 2 parallel jobs

@jandry
Copy link
Contributor Author

jandry commented Nov 2, 2023

From what I read seems ok because you use steps in the same job (called build) so it still on same runner with same workspace
https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idsteps

@daviddavidgit
Copy link
Contributor

So what is the status of this issue?

@ptrthomas
Copy link
Member

@daviddavidgit there is an open PR: #2424 - do contribute if you can

@daviddavidgit
Copy link
Contributor

As far as I unsterstand it correctly, the code from @jandry must be tested on github actions. @ptrthomas would it not be the easiest way if you test this PR, since you have permission to do that?

@jandry
Copy link
Contributor Author

jandry commented Jun 6, 2024

Yes , I can't do better 😅

@ptrthomas
Copy link
Member

@jandry okay, apologies - I have a lot of stuff on my plate. may I request that you tweak the PR so that there are zero changes to karate-chrome and we have a separate github action flow which does only one thing which is release karate-chromium. and to start with it is ok if all this is mac only ? I can then run only that github action and see how it goes, does that make sense ?

I also wonder if in the meantime if chrome for arm is available, I haven't checked

@daviddavidgit
Copy link
Contributor

Unfortunately there is still no linux/arm version of chrome.

@jandry
Copy link
Contributor Author

jandry commented Jun 6, 2024

@ptrthomas yes it's a what you initially asked for. So it's what I did:

This line build the chrome one only for linux/amd64 as actually
docker buildx build --push --platform linux/amd64 --cache-from=type=local,src=./target/docker -t karate-chrome -t karatelabs/karate-chrome:latest -t karatelabs/karate-chrome:${{ github.event.inputs.version }} karate-docker/karate-chrome
so same as docker build -t karate-chrome karate-docker/karate-chrome but using buildx instead

This line build the new chromium one on both platform
docker buildx build --push --platform linux/amd64,linux/arm64 --cache-from=type=local,src=./target/docker -t karate-chromium -t karatelabs/karate-chromium:latest -t karatelabs/karate-chromium:${{ github.event.inputs.version }} karate-docker/karate-chromium

I didn't isolate in two different jobs for optimization purpose. Some elements of first build are reuse is second build thanks to the cache --cache-from=type=local,src=./target/docker

@ptrthomas
Copy link
Member

@jandry okay, agreed with the approach. but trying to make sure - see screenshot below - this should be reverted right ?

image

jandry pushed a commit to jandry/karate that referenced this issue Jun 6, 2024
@jandry
Copy link
Contributor Author

jandry commented Jun 6, 2024

Yes you are right
A forgotten to revertit . I did this for local testing purpose on my mac
I pushed the revert

ptrthomas added a commit that referenced this issue Jun 6, 2024
#2422 Add support of arm64 for image karatelabs/karate-chrome
@ptrthomas
Copy link
Member

@jandry well, the attempt to run the docker push failed and I have no idea why. the login step (and credentials) were not changed at all, and they seem ok https://github.com/karatelabs/karate/actions/runs/9401006648/job/25891832379

image

@jandry
Copy link
Contributor Author

jandry commented Jun 7, 2024

That's what I couldn't test locally. locally a default name was working. Maybe because of I didn't put the "karatelabs/" before all image names

I can't push to my pull request anymore as you merged it

I think we should adjust like follow to also push the version number for the first image tag and latest for second tag

docker buildx build --platform linux/amd64 --cache-from=type=local,src=./target/docker --cache-to=type=local,dest=./target/docker -t karatelabs/karate-chrome:${{ github.event.inputs.version }} -t karatelabs/karate-chrome:latest karate-docker/karate-chrome
# build karate-chromium docker image that includes karate fatjar + maven jars for convenience
# Both platform
docker buildx build --platform linux/amd64,linux/arm64 --cache-from=type=local,src=./target/docker --cache-to=type=local,dest=./target/docker -t karatelabs/karate-chromium:${{ github.event.inputs.version }} -t karatelabs/karate-chromium:latest karate-docker/karate-chromium

@ptrthomas
Copy link
Member

@jandry please just open a new PR, because I don't know buildx and where to use the --push flag or if it is needed. I think you should be easily able to experiment with your personal docker account if needed

@jandry jandry mentioned this issue Jun 8, 2024
5 tasks
@jandry
Copy link
Contributor Author

jandry commented Jun 8, 2024

@ptrthomas done here #2571

@ptrthomas
Copy link
Member

@jandry sorry that's the wrong file. the change needs to be made in https://github.com/karatelabs/karate/blob/6f162bbb1a20e955a4b8b1ba82bb53196bdb833b/.github/workflows/maven-release.yml

I would have done it but I don't know the syntax as I said before. anyway I merged it and ran the CI job without realizing that nothing had changed

@daviddavidgit
Copy link
Contributor

daviddavidgit commented Jun 13, 2024

@ptrthomas I made the changes in the requested file, see #2573.
Is it possible that you merge the PR and run the github action?

@ptrthomas
Copy link
Member

@ptrthomas
Copy link
Member

update: have reverted to old state, since we don't have a solution yet: 351c4d2

@ptrthomas ptrthomas removed this from the 1.5.0 milestone Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants