seleniumhq-community / docker-seleniarm

Multi-Arch (arm64/armhf/amd64) Docker images for the Selenium Grid Server
https://hub.docker.com/u/seleniarm
Other
248 stars 24 forks source link

Seleniarm add chromium arm64 circle #58

Open diemol opened 10 months ago

diemol commented 10 months ago

Thanks for contributing to the Docker-Selenium project! A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository. Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

Types of changes

Checklist

diemol commented 10 months ago

c.c. @fhoeben

fhoeben commented 10 months ago

I see that forgot to set the correct namespace when I cherry picked the addition of the video workflow, I've created https://github.com/seleniumhq-community/docker-seleniarm/pull/59

fhoeben commented 10 months ago

@diemol In your merge you forgot to remove NodeFirefox/Dockerfile.multi-arch that file is no longer used (and causes issue on vulnerability scan)

diemol commented 10 months ago

@jamesmortensen do you know why the push fails now?

fhoeben commented 10 months ago

@jamesmortensen do you know why the push fails now?

Maybe the user does not have sufficient privileges to create new repositories in seleniarm?

For each image I also create a target architecture specific repository. I push the target's images there before combining them into a cross platform manifest in the normal repository. This to prevent a lot of extra tags in the normal repository, there you only want to see the combined manifest. But to create the manifest the images must first be pushed to the target registry server, so I opted to push to separate repositories. But (especially on first push) the user doing that must have rights to create the repo and later to push to it. I don't know, but maybe that is the issue?

Anyway the error occurs on the initial push of the generated image so the push is going to (for instance) seleniarm/base_arm64 which I suppose was not done before.

jamesmortensen commented 10 months ago

Is there a way to do this without creating the extra repositories? (I do keep the permissions of DOCKER_USER set to minimal to avoid any mistakes when pushing. The extra manifests may be why I avoided the manifest route in the past because it clutters up the namespace with non-multi-arch images.)

I changed to DOCKER_TEST_USERNAME and DOCKER_TEST_PASSWORD for now, which publishes to the "jamesmortensen1" namespace. If the push I just did works, you'll be able to try out the images and see if they work. (The tests may pass, but with such extensive changes to the build process, versioning, OS, etc, we'll want to make sure that everything works before publishing live.)

fhoeben commented 10 months ago

@jamesmortensen

I started with just using other/extra tags in the main repositories, but that was really too much and too cluttering. I like this approach in that each repository remains fairly clean, but indeed we get many more repositories.

In another project I use buildx explicitly (with Bake, which I think I prefer over Make), that offers an option to push its output to a repository directly but only based on the sha256 (so no tag is created). These shas are then captured and used to create the manifests. It keeps the repositories nice and clean, but does add complexity to the build process. (I fear the images we load into docker from cache will not be available in the build-engine used by buildx so we have to find a way around that. I found all that just getting too complex and error-prone.

Maybe we can have a separate namespace, or just a single repository which we use as a tmp/cache? Would that be a better approach?

jamesmortensen commented 10 months ago

@diemol @fhoeben The multi-arch images deployed from the "test-ubuntu" branch. You can find them here and try them out.

https://app.circleci.com/pipelines/github/seleniumhq-community/docker-seleniarm/441/workflows/4e5e18bb-8342-46aa-b2e1-ebb649ef5ac3

jamesmortensen commented 10 months ago

@fhoeben the single cache repository sounds like a good idea. You can use "jamesmortensen1" namespace (DOCKER_TEST_USERNAME and DOCKER_TEST_PASSWORD as the temporary cache. Secrets are already setup for it.

Also, we should test the images to make sure they work, now that they're in Docker. I can look into that as well. I use https://github.com/jamesmortensen/debug-tools-for-docker-selenium to help run some basic manual tests on the images.

fhoeben commented 10 months ago

@jamesmortensen I don't believe the secrets are accessible to me yet. At least when I created a PR it didn't seem to get the docker username. Or maybe I should not have done a PR, but ...?