Skip to content
This repository has been archived by the owner on Jun 22, 2024. It is now read-only.

Attempt to build and test on arm64 and amd64 using upstream trunk with chromium #57

Conversation

fhoeben
Copy link

@fhoeben fhoeben commented Dec 17, 2023

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

luisfcorreia and others added 30 commits September 26, 2023 11:18
* Update Dockerfile

road to 4.13.0

* Update bootstrap.sh

Road to 4.13.0

[deploy]
Bumps [nick-invision/retry](https://github.com/nick-invision/retry) from 2.8.3 to 2.9.0.
- [Release notes](https://github.com/nick-invision/retry/releases)
- [Changelog](https://github.com/nick-fields/retry/blob/master/.releaserc.js)
- [Commits](nick-fields/retry@943e742...1467290)

---
updated-dependencies:
- dependency-name: nick-invision/retry
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* fix: update base image to use focal  20230624

* fix: update base image to use focal 20230801

---------

Co-authored-by: Diego Molina <[email protected]>
Update ingress.yaml

Co-authored-by: Diego Molina <[email protected]>
…leniumHQ#1973)

fix issue SeleniumHQ#1887: affinity rules

add affinity section to seleniumGrid.podTemplate

Co-authored-by: Diego Molina <[email protected]>
Bumps [ad-m/github-push-action](https://github.com/ad-m/github-push-action) from 0.6.0 to 0.8.0.
- [Release notes](https://github.com/ad-m/github-push-action/releases)
- [Commits](ad-m/github-push-action@40bf560...d91a481)

---
updated-dependencies:
- dependency-name: ad-m/github-push-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4.7.0 to 4.7.1.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@v4.7.0...v4.7.1)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Update Dockerfile

updating ubuntu version

[deploy]
Improve chart templates in section videoRecorder

Signed-off-by: Viet Nguyen Duc <[email protected]>
* Update Dockerfile

* Update bootstrap.sh

[deploy]
…er nodes (SeleniumHQ#1989)

* feat(autoscaling): Unified parameters to set scaled options for browser nodes

Signed-off-by: Viet Nguyen Duc <[email protected]>

* Update value to get from template .fullname

Signed-off-by: Viet Nguyen Duc <[email protected]>

---------

Signed-off-by: Viet Nguyen Duc <[email protected]>
…HQ#1994)

* Add chart parameter ingress.paths to configure custom paths

Signed-off-by: Viet Nguyen Duc <[email protected]>

* SeleniumHQ#1810 Update dafault value for ingress.hostname

Signed-off-by: Viet Nguyen Duc <[email protected]>

* Update chart README for config parameters basicAuth.

Signed-off-by: Viet Nguyen Duc <[email protected]>

---------

Signed-off-by: Viet Nguyen Duc <[email protected]>
Bumps [helm/chart-releaser-action](https://github.com/helm/chart-releaser-action) from 1.5.0 to 1.6.0.
- [Release notes](https://github.com/helm/chart-releaser-action/releases)
- [Commits](helm/chart-releaser-action@v1.5.0...v1.6.0)

---
updated-dependencies:
- dependency-name: helm/chart-releaser-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@fhoeben fhoeben force-pushed the add_chromium_arm64_circle branch from cf77754 to b57242d Compare December 17, 2023 22:05
@jamesmortensen
Copy link

Hi @fhoeben I attempted to do a test deploy from your branch, but I forgot that secrets in CircleCI are not available in forks. You can ignore my "deploy trigger" commits, or just roll them back. They shouldn't have any impact on anything you're doing.

I'm going to try to pull your changes directly into a test branch in docker-seleniarm so we can try a test deploy.

@fhoeben
Copy link
Author

fhoeben commented Dec 18, 2023

@jamesmortensen please note that I did not look at the deploy setups in the circle ci files yet. They will probably be broken as they refer to 'multi-arch targets' not present in the Makefile.

I think we should be able to just take the images created in the build job, combine them into a manifest files and push those, without the need to build anything or so in the final jobs. But I have to brush up on my docker CLI, but I see that it is now possible to build manifest locally. So we should be able to setup the job to everything but push the images and manifest removing the need to have access to any secrets to check the process.

@fhoeben fhoeben force-pushed the add_chromium_arm64_circle branch 5 times, most recently from 393ba89 to cfed167 Compare December 18, 2023 09:21
@jamesmortensen
Copy link

jamesmortensen commented Dec 18, 2023 via email

@fhoeben
Copy link
Author

fhoeben commented Dec 18, 2023

Indeed I based my branch on the seleniumHQ repository as I wanted to build images with as minimal changes to their codebase to keep tracking them easy.
I also got a comment from them they hope to incorporate arm builds into the main project in the near future, so I wanted to be as close to that as possible.

I did see the armhf support, but I didn't spend time on that (yet), so for now in this branch it is only arm64 and amd64.
I also didn't keep the Makefile changes from your branch, there was quite a lot and so far I needed almost nothing of it to get the images (that was what I was referring to when I said the make targets are not available in my branch)

I was wrong about being able to generate manifests locally, that really does require images to be pushed to a registry beforehand. (As I saw in the build of my last commit)

I could push the images I build locally to docker hub, but that would only get us arm64 images. What I did instead is have the build mark the generated image tarballs as artifacts. So you can obtain the images by downloading the two tarballs and importing their images (that part of my last commit does work, although it gets them from cache instead of the artifacts):

I think I can update my branch so that a copy of yours (with access to the secrets) would be able to generate the manifest and push everything, so you could try it out. Looks like the config should then refer to $DOCKER_USERNAME and $DOCKER_PASSWORD, correct? Or are there more secrets needed?

@jamesmortensen
Copy link

jamesmortensen commented Dec 18, 2023 via email

@fhoeben fhoeben force-pushed the add_chromium_arm64_circle branch from cfed167 to a816909 Compare December 18, 2023 12:27
@fhoeben
Copy link
Author

fhoeben commented Dec 18, 2023

So I believe you can validate the images now based on the artifacts I mentioned in my previous comments (you could push based on those yourself).

The tests pass (with a minor change I added, to check there are any downloads before inspecting the name of the first download) so I hope/assume they work.

The changes to make the actual arm image build work on ubuntu (only arm64) are in https://github.com/fhoeben/docker-seleniarm/tree/add_chromium_arm64, this is again based on SeleniumHQ trunk, with my addition to add chromium.

Then the branch this PR is based on adds the Circle CI config.

I just squash and force pushed some changes to hopefully make it a bit clearer

@fhoeben fhoeben force-pushed the add_chromium_arm64_circle branch from e4de8b2 to 0773a10 Compare December 18, 2023 20:33
@fhoeben
Copy link
Author

fhoeben commented Dec 18, 2023

@jamesmortensen I just setup my own CircleCI account and managed to build and push the images/manifests based on this branch (with a changed namespace)

CircleCI

Images:

  • fhoeben/standalone-firefox:add_chromium_arm64_fh-today
  • fhoeben/node-chromium:add_chromium_arm64_fh-today
  • etc..

@diemol diemol changed the base branch from trunk to add_chromium_arm64_circle December 19, 2023 13:02
@diemol
Copy link
Member

diemol commented Dec 19, 2023

I am merging this into https://github.com/seleniumhq-community/docker-seleniarm/tree/add_chromium_arm64_circle, so we can run CI without issues. If we need more changes, we can do more PRs towards that branch.

@diemol
Copy link
Member

diemol commented Dec 19, 2023

I see the build and test passes, but we need the secrets for the last part.

@diemol diemol marked this pull request as ready for review December 19, 2023 13:31
@diemol diemol merged commit 65c4b0b into seleniumhq-community:seleniarm_add_chromium_arm64_circle Dec 19, 2023
2 of 5 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.