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 s390x support #198

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

vishnubijukumaribm
Copy link

This PR introduces s390x support for the test-tools image, a crucial step toward building and running KEDA on the s390x architecture.

Key Changes:

  1. Updated tools/Dockerfile to support s390x builds.
  2. Modified the Makefile to enable s390x-architecture compatibility.
  3. Also Upgraded Golang to 1.22.12-alpine3.21 in the k6-runner/Dockerfile.

With these enhancements, the test-tools image can now be successfully built and utilized for KEDA development on s390x.

@vishnubijukumaribm
Copy link
Author

@JorTurFer could you please review and approve the changes for this PR. We are adding support for s390x architecture.

@JorTurFer
Copy link
Member

Hello
Could you give us more context about why this image is needed? I assume that it's to support s390x arch but that support is something that we can't provide as we don't have any way to cover it with at least smoke tests. Is this requirement to support officially s390x by KEDA or it's something that you will build by yourself?
We try to ensure the quality of all the projects that we offer and we can't support an architecture that we can't tests

@vishnubijukumaribm
Copy link
Author

This is to support officially s390x by KEDA.

@zroubalik
Copy link
Member

Hi, as mentioned by @JorTurFer in order to officially support another architecture, we would need to run the testsuite on this arch - we don't have servers with s390x. So this is not something we can do.

@@ -31,6 +31,8 @@ jobs:
name: arm64
- runner: ubuntu-latest
name: amd64
- runner: ubuntu-latest
Copy link
Member

Choose a reason for hiding this comment

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

this would need to be s390x runner

Copy link
Member

Choose a reason for hiding this comment

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

And this can be covered using docker buildx and emulating the arch with qemu, but we need s390x machines to run KEDA's smoke suite, and that's not something that we can emulate

Copy link

Choose a reason for hiding this comment

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

@JorTurFer this is the concern we can address by providing s390x machine in IBM Cloud. We have done the similar approach for testing buildpacks project and I have implemented github actions for the same. You can refer buildpacks lifecycle job here: https://github.com/buildpacks/lifecycle/actions/workflows/test-s390x.yml. We can follow the similar approach or other approach feasible here from KEDA community perspective.

From support and fixing on s390x perspective, our team will be working with KEDA community with fixes on s390x whenever necessary. We can discuss further if there are questions and concerns.

Copy link
Member

Choose a reason for hiding this comment

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

That is great!

@dilipgb could you please open an issue on https://github.com/kedacore/keda first to discuss this problem in general?

We need certain assurances around maintaince and access to the test machines, before we can proceed.

Copy link

Choose a reason for hiding this comment

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

I had this topic created in discussion long back and I didn't get any response. kedacore/keda#5584. @zroubalik you suggest to open an issue instead of a topic here? Please let me know I will raise issue accordingly.

Copy link
Member

Choose a reason for hiding this comment

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

I created an issue from the discussion. Let's continue there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants