fix: fix Makefile and Dockerfile for idempotent images and to support multi-arch builds Fixes RHOAIENG-12078

[dhirajsb]

Description

Cleanup Makefile and Dockerfile to make them reproducable
Add multi-arch platform builds

How Has This Been Tested?

CI will verify Makefile and Dockerfile changes
Dockerfile changes can be tested locally by updating model-registry.yaml

Merge criteria:

• All the commits have been signed-off (To pass the DCO check)

• The commits have meaningful messages; the author will squash them after approval or in case of manual merges will ask to merge with squash.
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has manually tested the changes and verified that the changes work.
• Code changes follow the kubeflow contribution guidelines.

If you have UI changes

• The developer has added tests or explained why testing cannot be added.
• Included any necessary screenshots or gifs if it was a UI change.
• Verify that UI/UX changes conform the UX guidelines for Kubeflow.

[dhirajsb]

I'll continue on cleaning it up for multi-arch build support tomorrow.

There is something interesting happening though. Try editing model-registry.yaml to add a new endpoint (duplicate the first path with a different path and operationId) and see the behavior.

[dhirajsb]

The type_asserts.go file is doing what @isinyaaa mentioned. Let's see if we can sort the output in some way to make it idempotent.

[dhirajsb]

I can use regular expressions for extracting and sorting the functions to fix this. I'll add that tomorrow.

[dhirajsb]

Made gen_type_assert.sh idempotent. I'll add docker env variables for multi-platform builds and a makefile target next.

[dhirajsb]

@tarilabs @isinyaaa you can try out multi-platform builds using a command similar to:

IMG_ORG=dhirajsb make image/buildx

This command built and pushed images to my dockerhub repo https://hub.docker.com/r/dhirajsb/model-registry/tags

Give it a try and let me know what happens on other platforms.

[dhirajsb]

As an experiment, I built the project in different ways and on my admittedly beefy laptop I saw the following build times:

Buildx with emulation, no BUILDPLATFORM:
[+] Building 2184.9s (94/94) FINISHED                                    docker-container:model-registry-builder
Buildx with cross compilation with non-optimized Makefile build:
[+] Building 593.3s (82/82) FINISHED                                     docker-container:model-registry-builder
Buildx with cross compilation and optimized Makefile:
[+] Building 590.7s (86/86) FINISHED                                     docker-container:model-registry-builder

[tarilabs]

Thank you very much @dhirajsb , output as requested:

[+] Building 314.4s (86/86) FINISHED                                                                                                                                                                                                                                                                                              docker-container:model-registry-builder

Appreciate ordering in the type asserts file generation too!

We will need a way to test these, possibly by leveraging the e2e as mentioned before wiring it as the shipped images, but this is great progress imho and many thanks again!
🚀

/lgtm

[dhirajsb]

@tarilabs is this supposed to auto-merge after your lgtm?

[isinyaaa]

I haven't been able to run make image/buildx using podman desktop on mac. Made sure to update it and I'm still getting:

$ make image/buildx
docker buildx create --use --name model-registry-builder
Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
Error: unknown flag: --use
See 'podman buildx --help'
make: *** [image/buildx] Error 125

when I list the commands, however, there's not even a create cmd. @tarilabs can you confirm your podman and podman-desktop version? Mine are v1.12.0 for desktop and 5.2.2 for podman.

[dhirajsb]

@isinyaaa podman doesn't support the same options for buildx. I'm testing a change locally to fix that.

[dhirajsb]

I added podman support for using buildx. But unfortunately there is no command parity. And, the podman buildx build also seems to run in a very un-optimized fashion.

@isinyaaa test it and let us know.

[isinyaaa]

I was able to test with docker and podman on mac using:

make image/buildx

and

DOCKER=podman make image/buildx

respectively. Can report it worked fine, although I've lost track of the running times because I had to reboot.
Can try again if you need that info.

[isinyaaa]

can't we make a separate rule for pushing? we're not only building for that purpose AFAIK, but also to be able to have those images locally.

[dhirajsb]

Docker doesn't support that, it doesn't keep manifests locally and have to be pushed to a registry.

[isinyaaa]

IIUC it does cache results, but they're not exported to docker by default.
To do that, you can use --load on the build cmd.
I'm not sure if you can push it subsequently at no cost though, or if you have to build again.

[dhirajsb]

There are open issues with --load and it didn't work for me. Multi-platform builds are useful for publishing to remote repos, not as much for local use.
E.g. we'll do it in CI on PR merge and not for pulls.

[isinyaaa]

strangely, I'm also not able to build again:

$ make image/buildx
# docker uses builder containers
docker buildx create --use --name model-registry-builder
ERROR: existing instance for "model-registry-builder" but no append mode, specify the node name to make changes for existing instances
make: *** [image/buildx] Error 1

[dhirajsb]

The rebuild error sounds like the docker buildx rm... command didn't run the last time. You can modify it to avoid this error by adding the append flag as well. Feel free to do so if you want to try it.

[isinyaaa]

LGTM!

[tarilabs]

• fix: fix Makefile and Dockerfile for idempotent images and to support multi-arch builds Fixes RHOAIENG-12078 #322 (review)
• fix: fix Makefile and Dockerfile for idempotent images and to support multi-arch builds Fixes RHOAIENG-12078 #322 (review)

/lgtm
/approve

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: isinyaaa, tarilabs

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [tarilabs]

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