S390x support

[modassarrana89]

Multi-architecture support for model-registry

Description

Updated Dockerfile Updated Makefile to use buildx for multi-arch image Updated install_protoc scripts for multi-architecture

[modassarrana89]

@tarilabs I have updated all the changes as you have requested in old PR ( https://github.com/kubeflow/model-registry/pull/62 )

[modassarrana89]

This is the updated PR with all the comments suggested by @tarilabs in old PR https://github.com/kubeflow/model-registry/pull/62

[modassarrana89]

Thank you @modassarrana89 for a refreshed PR including most of the previous comments!

I've dded commit 8bcd792 as I believe necessary for the scripts/install_protoc.sh script to correctly resolve the zip to download when used in a ARM based linux container.

The problem I see in the way this PR currently stands, is that the model-registry is not actually cross-compiled:

In other words: it looks to me model registry get compiled for the platform the build is currently on (in my screenshot, arm), and then the binary copied to linux/arm64,linux/amd64,linux/s390x,linux/ppc64le, which is unlikely to work.

Further, it looks to me cross-compilation would require something ~like this in the dockerfile:

ARG TARGETOS TARGETARCH
RUN CGO_ENABLED=1 GOOS=$TARGETOS GOARCH=$TARGETARCH make clean model-registry

but currently would fail with:

go: cannot install cross-compiled binaries when GOBIN is set

which is used as part of the Makefile.

I believe solutions could be along the lines of:

• rework the build to avoid gobin IFF all the remainder buildx problems are solved after accounting for the above
• use buildx for emulation rather than cross-compilation
• other solution?

I believe a proper reproducible cross-platform build is needed, advise if you believe I might have missed on anything and how planning to solve for the mentioned issues.

Let me quickly test , once & get back on this

[modassarrana89]

Thank you @modassarrana89 for a refreshed PR including most of the previous comments! I've dded commit 8bcd792 as I believe necessary for the scripts/install_protoc.sh script to correctly resolve the zip to download when used in a ARM based linux container. The problem I see in the way this PR currently stands, is that the model-registry is not actually cross-compiled: In other words: it looks to me model registry get compiled for the platform the build is currently on (in my screenshot, arm), and then the binary copied to linux/arm64,linux/amd64,linux/s390x,linux/ppc64le, which is unlikely to work. Further, it looks to me cross-compilation would require something ~like this in the dockerfile:

ARG TARGETOS TARGETARCH
RUN CGO_ENABLED=1 GOOS=$TARGETOS GOARCH=$TARGETARCH make clean model-registry

but currently would fail with:

go: cannot install cross-compiled binaries when GOBIN is set

which is used as part of the Makefile. I believe solutions could be along the lines of:

• rework the build to avoid gobin IFF all the remainder buildx problems are solved after accounting for the above
• use buildx for emulation rather than cross-compilation
• other solution?

I believe a proper reproducible cross-platform build is needed, advise if you believe I might have missed on anything and how planning to solve for the mentioned issues.

Let me quickly test , once & get back on this

Below Github action works fine for building & pushing multi - arch image

- name: Build and Push Image
        uses: docker/build-push-action@v3
        with:
          tags: ${{ env.IMAGE_NAME }}:${{ env.TAG }}
          platforms: linux/amd64,linux/s390x,linux/ppc64le
          push: true

If ok , we can update the github workflow for build and push to use this action & then it will automatically solves our problem.

[tarilabs]

Not seeing / nor understood how the new commit would solve for:

go: cannot install cross-compiled binaries when GOBIN is set

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[modassarrana89]

@tarilabs Below Github action should works fine for building & pushing multi - arch image. Need to verify once . We can use this inplace of the current scripts for multi-arch image build procedure . If you guys are. fine with Github workflow change , I can quickly verify this approach & let you know the update.

- name: Build and Push Image
   uses: docker/build-push-action@v3
   with:
          tags: ${{ env.IMAGE_NAME }}:${{ env.TAG }}
          platforms: linux/amd64,linux/s390x,linux/ppc64le
          push: true

[tarilabs]

If you guys are. fine with Github workflow change , I can quickly verify this approach & let you know the update

We are looking into removing the GOBIN and local build toolchain, so to support more general multi-arch support, per KF 1.10 roadmap: https://github.com/kubeflow/model-registry/issues/175#:~:text=Multi%2Darch%20images%20%2D%20for%20the%20Go%20REST%20server%20specifically

We can keep this PR open, to revise this as we will rampup on those items.

[modassarrana89]

If you guys are. fine with Github workflow change , I can quickly verify this approach & let you know the update

We are looking into removing the GOBIN and local build toolchain, so to support more general multi-arch support, per KF 1.10 roadmap: #175

We can keep this PR open, to revise this as we will rampup on those items.

Ok Great. But may i know whats the deadline would be

[modassarrana89]

@tarilabs it would be great if you can share some update on GOBIN removal and local build toolchain.

[tarilabs]

@tarilabs it would be great if you can share some update on GOBIN removal and local build toolchain.

Sure! We found in general ~~gobin usage~~ and cgo usage giving us some troubles while attempting to instruct for cross-compilation.

There is also another observation that generated source code are re-generated in the docker build phase itself, which is less ideal especially considering we have them already under VCS here in this git repo, in order to point to the actual sources for the bin/image we ship.

We have some contributors actively working on this front for which we can expect some further update, per PRs already iterated on, or being currently worked on right now.

edit: with more recent changes, we just discovered gobin looks currently not an impediment to crosscompilation, so we're now looking to keep it (hence not drop its current usage). More updates to follow.

[modassarrana89]

source code are re-generated in the docker build phase itself

we have discussed previously that cross compilation is creating problem with current GOBIN setup. If GOBIN is not an issue , are you saying that cross-compilation will work

[tarilabs]

we have discussed previously that cross compilation is creating problem with current GOBIN setup. If GOBIN is not an issue , are you saying that cross-compilation will work

as mentioned in https://github.com/kubeflow/model-registry/pull/66#issuecomment-2317177110, gobin looks currently not an impediment to crosscompilation, so we're now looking to keep (gobin usage)

more updates to follow

[google-oss-prow[bot]]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment