Push multi-arch images

[ddelange]

Motivation

Create multi-platform images for modelmesh-runtime-adapter images.

Modifications

• build for platforms linux/amd64, linux/arm64 (linux/ppc64le, linux/s390x are not supported by tensorflow)
• utilize Docker build cache for pip and dnf installs as well as go builds
• build on pull_request as well as on push to catch build breaks during PR review;
  only push images to DockerHub on PR merge (push to main)
• upgrade runtime image base from ubi-minimal:8.4 to 8.7
• use ubi8/go-toolset:1.17 for developer image instead of ubi8/ubi-minimal:8.4
• update deprecated checkout action
• fix infinite docker inside docker bug when running make run fmt inside the developer container

Related PRs

• modelmesh (Push multi-arch images  modelmesh#80)
• modelmesh-serving (Push multi-arch images modelmesh-serving#314)
• rest-proxy (Push multi-arch images rest-proxy#24)
• modelmesh-minio-examples (Enable multi-platform build modelmesh-minio-examples#3)
• fix: build all binaries for the target OS/ARCH #49

Resolves kserve/modelmesh-serving#162

[ckadner]

To avoid some of the arch related errors while installing libraries, we should probably use ubi8/go-toolset as base image instead of ubi8/minimal. Should also help to avoid complexities of building or even just installing go inside the Dockerfile

Alternatively, we could add another variable / docker build ARG for the etcd installation inside the Docker image

https://docs.docker.com/build/building/multi-platform/#building-multi-platform-images

... A list of build arguments like BUILDPLATFORM and TARGETPLATFORM is available automatically inside your Dockerfile and can be leveraged by the processes running as part of your build.

https://blog.thesparktree.com/docker-multi-arch-github-actions#architecture-specific-dockerfile-instructions

TARGETARCH - architecture component of TARGETPLATFORM

https://github.com/kserve/modelmesh-runtime-adapter/blob/main/Dockerfile#L44

... https://golang.org/dl/go${GOLANG_VERSION}.linux-amd64.tar.gz should be
... https://golang.org/dl/go${GOLANG_VERSION}.linux-${TARGETARCH}.tar.gz

Archive files listed on the corresponding download page:
https://go.dev/dl/

• go1.17.13.linux-amd64.tar.gz
• go1.17.13.linux-arm64.tar.gz
• go1.17.13.linux-armv6l.tar.gz
• go1.17.13.linux-ppc64le.tar.gz
• go1.17.13.linux-s390x.tar.gz

Default in the Dockerfile should probably be:

ARG TARGETARCH=amd64

But we might have to disable s390x for the missing gRPC/ProtoC libraries/executable

[ckadner]

@tjohnson31415 -- thanks for the pointer to not override the TARGETOS and TARGETARCH ARGs.

[tjohnson31415]

I confirmed that I could build images with binaries for two different CPU archs using the approach coded up here.

Cool Stuff!

[tjohnson31415]

I think $(MAKECMDGOALS) will cover most of these; it injects the set of targets passed into make on the cmd-line. The only explicit .PHONY you need then are ones that are dependent targets (so build.develop should be in here).
Or its fine to make this list explicit with all targets but then don't need $(MAKECMDGOALS) in here too.

[ckadner]

Thanks. I learned something and reinstated the original line :-)

[tjohnson31415]

Though the original line also has a problem I think. $(MAKECMDGOALS) is the set of goals specified on the cmd-line, but more targets will run if any of those goals have dependencies. So .PHONY: all $(MAKECMDGOALS) is only sufficient if no targets in the Makefile have dependencies. If there is a dependency, it will not be .PHONY unless explicitly listed. In this case, build.develop should still be in there.

So adding all the targets to .PHONY isn't a bad idea IMO, but there's no reason to add them all and also have $(MAKECMDGOALS).

Makefile hackery 😅

[ckadner]

Since this is tricky to understand -- and apparently was not understood by all developers who made updates to the Makefile before ;-) -- I suggest we go with explicitly listing of all goals.
However, in the current form, this still leaves the door wide open for future developers who add new goals (or remove unused ones, or rename existing ones) to overlook/forget to update the .PHONY pseudo target at the end of the Makefile. So I would prefer to “tag” each individual goal like so:

.PHONY: fmt
fmt:
	./scripts/fmt.sh

.PHONY: test
test:
	go test -coverprofile cover.out `go list ./...`

This way, the .PHONY "tag" would get deleted when the corresponding target gets deleted (or renamed along with renaming a target) and when new targets are added, very likely an existing target would be used as a “template” and the .PHONY “tag” would get copy-pasted or added for sheer “consistency” by the uninitiated

[ddelange]

Makefiles are fun 😅 I came across a help command that regexes double hashtag above the command, and forms a helptext https://github.com/ddelange/mapply/blob/0.1.21/Makefile

[ckadner]

@ddelange -- sorry I completely missed your comment. I have done a similar thing in the past as well here using grep and awk

Do you want to open a PR for this, in this and the other 4 ModelMesh repos?

[ckadner]

Thanks @tjohnson31415 -- I updated the PR with the straight-forward changes you suggested and resolved the corresponding conversations.

I left the others as unresolved for you to respond :-)
https://github.com/kserve/modelmesh-runtime-adapter/pull/38/files#conversations-menu

[tjohnson31415]

So I've been experimenting with this a bit. Does this match your understanding?

• the $HOME for root in go-toolset is not /root/ so we need to set the PIP_CACHE_DIR env var to configure pip's cache directory
• ARG interpolation doesn't work in the --mount, so the value is copied instead of using target=$PIP_CACHE_DIR

If that's the case, I think it is slightly cleaner to set the PIP_CACHE_DIR env var in the RUN command since the ARG is not really configurable.

Though I also found moby/buildkit#815 and it sounds like using ARGs in --mount can now work, but requires a newer Dockerfile syntax # syntax=docker/dockerfile:1.3.0.

[ddelange]

Hi 👋

Unless the host system (github actions) would build python wheels from source, there is no speedup from mounting pip's cache dir. We generally recommend to set PIP_NO_CACHE_DIR and rely on docker layer caching by running pip install before copying over source code. That way, you reuse the registry's layer as long as your requirements.txt doesn't change. Here's a live multi-arch example that combines a compiled C binary (libpostal) with python builds that use it.

[ckadner]

Good comments/questions @tjohnson31415 @ddelange !

Unless the host system (github actions) would build python wheels from source, there is no speedup from mounting pip's cache dir.

The cache mount is useful for subsequent builds, multi-stage and multi-platform builds. I have seen it being used and speedup the GitHub action builds quite a bit, with significant time savings for installing grpc and tensorflow

the $HOME for root in go-toolset is not /root/ so we need to set the PIP_CACHE_DIR env var to configure pip's cache directory

Yes, otherwise we see this error:

#13 12.99 The directory '/opt/app-root/src/.cache/pip/http' or its parent directory is not owned by the current user and the cache has been disabled. Please check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag.
#13 13.00 The directory '/opt/app-root/src/.cache/pip' or its parent directory is not owned by the current user and caching wheels has been disabled. check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag.

However, for pip, the inline --cache-dir CLI flag did not set the cache correctly for both HTTP download and the Wheel cache directories. So I use the ARG PIP_CACHE_DIR=/root/.cache/pip which works for both.

ARG interpolation doesn't work in the --mount, so the value is copied instead of using target=$PIP_CACHE_DIR

Yes. I have not tried the new syntax # syntax=docker/dockerfile:1.3.0 though. That's a good find.

---

References:

• https://www.docker.com/blog/faster-multi-platform-builds-dockerfile-cross-compilation-guide/
• https://github.com/moby/buildkit/blob/master/frontend/dockerfile/docs/reference.md#run---mounttypecache
• https://docs.docker.com/build/cache/
• mount=type=cache more in-depth explanation? moby/buildkit#1673
• Allow controlling cache mounts storage location moby/buildkit#1512

[tjohnson31415]

LGTM

[kserve-oss-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ddelange, tjohnson31415

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 [tjohnson31415]

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

[ddelange]

awesome work @ckadner!!