-
Notifications
You must be signed in to change notification settings - Fork 20
Add s390x architecture support and configure test builds for Quay.io #63
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: dev
Are you sure you want to change the base?
Conversation
Signed-off-by: Kevin <[email protected]>
…-job pods communication secure
…ion to upstream notebooks
README.md update to include Release section Release includes following: -warning about not keeping metadata.yaml up to date. -ODH release tracker information
WalkthroughThe GitHub workflow configurations were updated to add support for the linux/s390x architecture. This includes building and publishing multi-architecture Docker images and enabling QEMU emulation for s390x in the relevant workflows. No changes were made to exported or public code entities. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant QEMU
participant Docker Buildx
Developer->>GitHub Actions: Push workflow changes
GitHub Actions->>QEMU: Setup emulation for amd64, arm64, ppc64le, s390x
GitHub Actions->>Docker Buildx: Build images for amd64, arm64, ppc64le, s390x
Docker Buildx-->>GitHub Actions: Multi-arch images built
GitHub Actions->>Registry: Publish multi-arch images
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
5e22e15
to
137f524
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/odh-build-and-publish-operator-image.yaml (1)
64-64
: Remove trailing whitespaceYAMLlint flagged trailing spaces on this blank line. Removing them will resolve formatting errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/odh-build-and-publish-operator-image.yaml
(2 hunks).github/workflows/publish-conformance-images.yaml
(1 hunks).github/workflows/publish-core-images.yaml
(2 hunks).github/workflows/template-publish-image/action.yaml
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/odh-build-and-publish-operator-image.yaml
[error] 64-64: trailing spaces
(trailing-spaces)
🔇 Additional comments (7)
.github/workflows/template-publish-image/action.yaml (1)
33-33
: Add s390x to QEMU emulation platformsIncluding
s390x
here will enable QEMU to emulate the new architecture for cross-platform builds. This aligns with the other workflows that now targetlinux/s390x
..github/workflows/publish-conformance-images.yaml (1)
13-13
: Include s390x in the conformance image platforms matrixAdding
linux/s390x
here ensures conformance test images are built for the new architecture, matching the operator and core image workflows..github/workflows/odh-build-and-publish-operator-image.yaml (2)
64-75
: Add s390x operator binary build stepThe new cross-compilation step installs
gcc-s390x-linux-gnu
andlibc6-dev-s390x-cross
, then builds the operator binary forGOARCH=s390x
. This mirrors the existingarm64
flow and correctly extends support.
95-95
: Include s390x in multi-architecture image buildUpdating
platforms: linux/amd64,linux/arm64,linux/s390x
ensures the operator container image is built for the new architecture as well..github/workflows/publish-core-images.yaml (3)
27-27
: Add s390x to training-operator core image platformsIncluding
linux/s390x
ensures the core image supports the new architecture.
31-31
: Enable s390x for training-operator-v2 core imageAdding
linux/s390x
keeps v2 in sync with primary operator support.
43-43
: Include s390x for kubectl-delivery core imageAligns
kubectl-delivery
component with the newly added architecture support.
137f524
to
7a3569f
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/odh-build-and-publish-operator-image.yaml (1)
64-64
: Remove trailing whitespace.
The blank line at line 64 contains trailing spaces, which causes a YAML lint error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/odh-build-and-publish-operator-image.yaml
(2 hunks).github/workflows/publish-conformance-images.yaml
(1 hunks).github/workflows/publish-core-images.yaml
(2 hunks).github/workflows/template-publish-image/action.yaml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/publish-conformance-images.yaml
- .github/workflows/template-publish-image/action.yaml
- .github/workflows/publish-core-images.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/odh-build-and-publish-operator-image.yaml
[error] 64-64: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/odh-build-and-publish-operator-image.yaml (2)
65-75
: Verify QEMU emulation for s390x builds.
This workflow cross-compiles the operator binary for linux/s390x but doesn’t register QEMU/binfmt support for multi-arch image creation. Please confirm that the runner environment can emulate s390x binaries or add an explicit QEMU setup step (e.g.docker/setup-qemu-action@v3
) before the container build.
95-95
: Add s390x to target platforms.
Includinglinux/s390x
in theplatforms
list correctly extends the multi-arch image build.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/odh-build-and-publish-operator-image.yaml (1)
64-64
: Remove trailing whitespace
The blank line at 64 contains trailing spaces and triggers YAMLlint. Please remove the extra spaces to keep the file clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/odh-build-and-publish-operator-image.yaml
(2 hunks).github/workflows/publish-conformance-images.yaml
(1 hunks).github/workflows/publish-core-images.yaml
(2 hunks).github/workflows/template-publish-image/action.yaml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/publish-conformance-images.yaml
- .github/workflows/template-publish-image/action.yaml
- .github/workflows/publish-core-images.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/odh-build-and-publish-operator-image.yaml
[error] 64-64: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/odh-build-and-publish-operator-image.yaml (2)
65-75
: Add s390x operator binary build step
This step mirrors the existing amd64/arm64 builds, installs the correct s390x cross-compiler toolchain, and builds the binary withGOARCH=s390x
. Ensure thatbuild/images/training-operator/Dockerfile.multiarch
is updated accordingly to copy themanager-s390x
binary.
95-95
: Include linux/s390x in multi-architecture platforms
Addinglinux/s390x
alongsidelinux/amd64,linux/arm64
enables multi-arch image builds for the s390x architecture.
Signed-off-by: Nishan Acharya <[email protected]>
7a3569f
to
d07134e
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/odh-build-and-publish-operator-image.yaml (1)
64-64
: Remove trailing whitespace.
A blank line (line 64) contains trailing spaces that may trigger lint errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/odh-build-and-publish-operator-image.yaml
(2 hunks).github/workflows/publish-conformance-images.yaml
(1 hunks).github/workflows/template-publish-image/action.yaml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/template-publish-image/action.yaml
- .github/workflows/publish-conformance-images.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/odh-build-and-publish-operator-image.yaml
[error] 64-64: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/odh-build-and-publish-operator-image.yaml (2)
65-75
: Approve s390x cross-compilation step.
The new step installs thes390x-linux-gnu-gcc
cross-compiler andlibc6-dev-s390x-cross
packages, setsGOARCH=s390x
, and builds the binary correctly.
95-95
: Approve multi-arch platform inclusion.
Addinglinux/s390x
to theplatforms
list ensures the operator image is built for s390x alongside amd64 and arm64.
@terrytangyuan could you please help to have a look onto the PR and approve the same! |
b6939ba
to
2178ac6
Compare
Add s390x Architecture Support
Changes
Added s390x binary build configuration in ODH workflow:
Updated platform support in workflows:
Testing
c.c: @modassarrana89
Summary by CodeRabbit
New Features
Chores