-
Notifications
You must be signed in to change notification settings - Fork 33
Enable ppc64le build for test-tools #231
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: kabhiibm <kumar.abhishek2@ibm.com>
f75d44c to
13e6756
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enables support for the ppc64le architecture in KEDA's test-tools image to expand platform compatibility for development and testing.
- Adds
ppc64learchitecture handling to the Dockerfile for Go, kubectl, and protobuf installations - Updates the Makefile to include
ppc64lein the supported build platforms - Implements architecture-specific installation logic to handle different binary naming conventions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tools/Dockerfile | Adds ppc64le architecture support with case statements for Go, kubectl, GitHub CLI, and protobuf installations |
| Makefile | Includes linux/ppc64le in the BUILD_PLATFORMS variable for multi-platform builds |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| "amd64") PROTOC_ARCH="x86_64" ;; \ | ||
| "arm64") PROTOC_ARCH="aarch_64" ;; \ | ||
| "s390x") PROTOC_ARCH="s390_64" ;; \ | ||
| "ppc64el" | "ppc64le") PROTOC_ARCH="ppcle_64" ;; \ |
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The protobuf architecture name 'ppcle_64' appears to be incorrect. Based on the protobuf releases, the correct architecture name should be 'ppc_64' for ppc64le.
| "ppc64el" | "ppc64le") PROTOC_ARCH="ppcle_64" ;; \ | |
| "ppc64el" | "ppc64le") PROTOC_ARCH="ppc_64" ;; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please double check this, it could be a halucination
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely a hallucination if we check https://github.com/protocolbuffers/protobuf/releases/
| curl -LO "https://dl.k8s.io/$(curl -L -s https://dl.k8s.io/release/stable.txt)/bin/linux/$(dpkg --print-architecture)/kubectl.sha256" && \ | ||
| RUN ARCH=$(dpkg --print-architecture) && \ | ||
| case "$ARCH" in \ | ||
| "ppc64le" | "ppc64el") \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we have have both *le and *el?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debian uses ppc64el for their dpkg arch whereas it looks like everyone else uses ppc64le. I wouldn't think we'd need the or though unless this is future-proofing in the event we ever converted this to build in a non-debian image?
|
@kabhiibm you still around for this one? I ran across it (hilariously) working with IBM testing this downstream on |
|
@jkyros ppcle64 ? ppc64le is common ppc64el is the alternative. |
@jkyros Since the image uses
Here is the arch command I have checked on our |
|
@kabhiibm Right, right, we're all in agreement I think. I was just sloppy when I typed Ubuntu is a debian derivative so my response to Zbynek was more or less to be taken as "It's weird, but debian just calls it something different than what everyone else does in their URLs"
But to Zbynek's question, I would expect |
|
@jkyros .. I also haven't came across any situation where I had included We can remove the |
|
Hey @kabhiibm can you update test-tools/.github/workflows/pr.yml Line 47 in 78ffe90
I'd recommend leaving the current approach you have for selecting/comparing arch. Or just adding a comment to avoid future regressions. |
This PR introduces
ppc64lesupport for thetest-toolsimage, a crucial step toward building and runningKEDAon theppc64learchitecture.Key Changes:
tools/Dockerfileto supportppc64lebuilds.Makefileto enableppc64learchitecture compatibility.With these enhancements, the
test-toolsimage can be successfully built and utilized for KEDA development onppc64le.