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.

Sign up for GitHub

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

Jump to bottom

add support for ARM64 in build-tools Dockerfile #3094

Merged
merged 1 commit into from
Jun 4, 2022
Merged

add support for ARM64 in build-tools Dockerfile #3094

merged 1 commit into from
Jun 4, 2022

Conversation

xoanmm
Copy link
Contributor

@xoanmm xoanmm commented May 31, 2022 •
edited
Loading

Signed-off-by: Xoán Mallón xoanmallon.moure@docplanner.com

Initial implementation of changes to add support for ARM64 in build-tools Dockerfile

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • Tests have been added
  • A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Changelog has been updated and is aligned with our changelog requirements

Fixes #

Relates to #3092

@xoanmm xoanmm requested a review from a team as a code owner May 31, 2022 20:37
@xoanmm
Copy link
Contributor Author

xoanmm commented May 31, 2022 •
edited
Loading

The main problem of this implementation is that fossa-cli does not support ARM in Linux for the moment, they have an open issue related with that topic. It's strictly necessary use this tool in the tooling image?

@xoanmm xoanmm force-pushed the fix/update-build-tools-dockerfile-arm64 branch from 3a74fa1 to 0f83d69 Compare May 31, 2022 21:15
@xoanmm xoanmm changed the title fix: initial version for support ARM in build-tools Dockerfile add support for ARM64 in build-tools Dockerfile May 31, 2022
@JorTurFer
Copy link
Member

I'm not totally sure about if fossa should be mandatory or not... Maybe we can just install fossa in the context of the fossa CI, at least till they support arm64 (if they do it)
@kedacore/keda-maintainers ?

JorTurFer
JorTurFer reviewed Jun 1, 2022
View reviewed changes
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general. Do you have plans to update the already existing CI to use the new build-tools? Maybe in another PR after this to ensure it works?

@JorTurFer JorTurFer requested a review from zroubalik June 1, 2022 17:35
@xoanmm xoanmm force-pushed the fix/update-build-tools-dockerfile-arm64 branch from 55bf1ce to 3f079e3 Compare June 1, 2022 21:21
@xoanmm
Copy link
Contributor Author

xoanmm commented Jun 3, 2022

Looks good in general. Do you have plans to update the already existing CI to use the new build-tools? Maybe in another PR after this to ensure it works?

Yes, once is published the new image with multiarch available I would like to change the github actions workflow for validation to use a matrix for check in both architectures. WDYT?

@xoanmm xoanmm force-pushed the fix/update-build-tools-dockerfile-arm64 branch 3 times, most recently from 4f29a1e to 63665bc Compare June 4, 2022 16:09
@xoanmm
Copy link
Contributor Author

xoanmm commented Jun 4, 2022

Due problems to install fossa-cli in build-tools image with ARM, it was moved the installation to the github action workflows defined in .github/workflows/fossa.yml, also it was deleted the container section because it was failing with the container section defined with build-tools image like in this execution. Without the container works fine like in this execution WDYT?

JorTurFer
JorTurFer reviewed Jun 4, 2022
View reviewed changes
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!
Only small nits inline and please, sign the commits because right now there are some commits unsigned.

Thanks for the improvement to ARM support

.github/workflows/main-build.yml Outdated Show resolved Hide resolved
.github/workflows/fossa.yml Outdated Show resolved Hide resolved
.github/workflows/fossa.yml Outdated Show resolved Hide resolved
@xoanmm xoanmm force-pushed the fix/update-build-tools-dockerfile-arm64 branch 2 times, most recently from 6f06f60 to b9b2734 Compare June 4, 2022 20:26
@dependabot @xoanmm
chore(deps): bump docker/setup-buildx-action from 1 to 2 (kedacore#3101)
8a18a27 
Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 1 to 2.
- [Release notes](https://github.com/docker/setup-buildx-action/releases)
- [Commits](docker/setup-buildx-action@v1...v2)

---
updated-dependencies:
- dependency-name: docker/setup-buildx-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@xoanmm xoanmm force-pushed the fix/update-build-tools-dockerfile-arm64 branch from b9b2734 to 8a18a27 Compare June 4, 2022 20:27
@JorTurFer JorTurFer merged commit c00294c into kedacore:main Jun 4, 2022
@JorTurFer
Copy link
Member

Thanks a lot for the ARM support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JorTurFer JorTurFer JorTurFer approved these changes

@zroubalik zroubalik Awaiting requested review from zroubalik

Assignees
No one assigned
Labels
skip-e2e
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xoanmm @JorTurFer