konflux: build ramalama images for s390x and ppc64le #1842
Conversation
Reviewer's GuideThis PR extends multi-arch support by including s390x and ppc64le in CI pipelines, introduces a --verify flag to control snapshot verification for cross-endian pulls, updates tests to handle endianness and skip unsupported architectures, and adjusts build scripts and dependencies for the new platforms. Sequence diagram for ramalama pull with --verify flagsequenceDiagram
actor User
participant CLI
participant ModelStore
User->>CLI: run "ramalama pull MODEL [--verify]"
CLI->>ModelStore: new_snapshot(tag, hash, files, verify=args.verify)
alt verify is True
ModelStore->>ModelStore: verify_snapshot(model_tag)
ModelStore-->>CLI: success or error
else verify is False
ModelStore-->>CLI: skip verification
end
CLI-->>User: result (success or error)
Class diagram for updated model_store.Store and CLI argument handlingclassDiagram
class Store {
+verify_snapshot(model_tag: str)
+new_snapshot(model_tag: str, snapshot_hash: str, snapshot_files: list[SnapshotFile], verify: bool = True)
}
class CLI {
+pull_parser(subparsers)
+--verify: bool
}
Store <.. CLI : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @mikebonnet, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request expands the build capabilities for various ramalama container images by introducing support for additional CPU architectures. Specifically, it enables the building and publishing of these images for ppc64le and s390x platforms, alongside the existing amd64 and arm64 architectures. This enhancement improves the reach and compatibility of the ramalama images across diverse hardware environments.
Highlights
- Expanded Architecture Support: The build configurations for multiple ramalama images have been updated to include ppc64le and s390x architectures.
- Tekton Pipeline Updates: Both pull request and push event Tekton pipeline definitions (*-pull-request.yaml and *-push.yaml) were modified to specify the new build platforms.
- Integration Pipeline Defaults: The bats-integration.yaml pipeline now defaults to testing on ppc64le and s390x in addition to amd64 and arm64.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request adds ppc64le and s390x as build platforms to numerous Tekton pipeline configurations, enabling multi-architecture image builds. While the changes are consistent across files, the widespread duplication of the build-platforms block across more than 20 files introduces a significant maintainability challenge. Future changes to the platform list will require updating all these files. I recommend exploring methods to centralize this configuration, such as using YAML anchors or a configuration management tool like Kustomize, to improve maintainability. Additionally, while enabling builds for new architectures, it's crucial to ensure all software dependencies within the container images support these platforms. I've added specific comments regarding potential issues with ollama and vLLM dependencies which appear to lack support for ppc64le and s390x.
mikebonnet
force-pushed
the
konflux-s390x-ppc64le
branch
18 times, most recently
from
48ef6f5 to
92e7631
Compare
mikebonnet
force-pushed
the
konflux-s390x-ppc64le
branch
9 times, most recently
from
16f65dc to
3a029aa
Compare
mikebonnet
force-pushed
the
konflux-s390x-ppc64le
branch
7 times, most recently
from
cd79e40 to
bca8a0b
Compare
mikebonnet
requested review from
rhatdan,
bmahabirbu,
maxamillion,
swarajpande5,
jhjaggars,
cgruver and
engelmi
as code owners
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider consolidating the repeated
build-platformslists in your Tekton pipeline YAMLs (e.g. via a shared template or common include) to reduce duplication and potential drift. - The repeated use of
getattr(args, 'verify', True)across different pull implementations could be centralized in a single helper or extension to avoid boilerplate and make flag handling more consistent. - Since
test_modelalters test behavior on big-endian architectures, it would be helpful to document its logic in your test helpers so future readers understand why model selection differs by arch.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider consolidating the repeated `build-platforms` lists in your Tekton pipeline YAMLs (e.g. via a shared template or common include) to reduce duplication and potential drift.
- The repeated use of `getattr(args, 'verify', True)` across different pull implementations could be centralized in a single helper or extension to avoid boilerplate and make flag handling more consistent.
- Since `test_model` alters test behavior on big-endian architectures, it would be helpful to document its logic in your test helpers so future readers understand why model selection differs by arch.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@rhatdan Ready for review |
ramalama/cli.py
Outdated
| ) | ||
| parser.add_argument( | ||
| "--verify", | ||
| default=True, |
There was a problem hiding this comment.
Should we add a flag to ramalama.conf to allow users to always pull without verifying?
There was a problem hiding this comment.
Added a verify option to ramalama.conf in the most recent revision.
|
LGTM with one question. |
|
Generally I think ppc64le does not need the OpenBLAS installation because they have already implemented their own custom tinyBLAS SGEMM kernel via Llamafile. But I don't have a ppc64le machine to verify so let's keep it this way until someone can check on it :) |
mikebonnet
force-pushed
the
konflux-s390x-ppc64le
branch
from
bca8a0b to
08c61a2
Compare
|
LGTM |
|
/retest ramalama-on-pull-request |
Allows "make unit-tests-in-container" to succeed. Signed-off-by: Mike Bonnet <[email protected]>
Publish ramalama images for additional platforms. Signed-off-by: Mike Bonnet <[email protected]>
GGML_VXE defaults to ON, so passing the parameter is not required. Signed-off-by: Mike Bonnet <[email protected]>
This allows models of different endianness to be pulled and inspected. Handle endianness mismatches more gracefully in the cli. Signed-off-by: Mike Bonnet <[email protected]>
Signed-off-by: Mike Bonnet <[email protected]>
mikebonnet
force-pushed
the
konflux-s390x-ppc64le
branch
from
08c61a2 to
cb67ddc
Compare
Publish ramalama images for additional platforms.
Summary by Sourcery
Extend support for big-endian architectures (s390x and ppc64le) across builds, CI pipelines, tests, and runtime; introduce a --verify flag for optional model verification after pull; and update build scripts, dependency versions, and documentation accordingly
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests: