Skip to content
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.

Sign up for GitHub

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

Push multi-arch images #38

Merged
merged 33 commits into from
May 5, 2023
Merged

Push multi-arch images #38

Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
3e15253
Push multi-arch images
ddelange Jan 16, 2023
4ec696f
Add --target runtime
ddelange Jan 16, 2023
2cdff97
Build not push on PR
ddelange Jan 16, 2023
77915da
Merge branch 'main' into patch-1
ckadner Mar 30, 2023
d682165
Update Dockerfile to use platform args
ckadner Mar 31, 2023
ad0b945
prettier
ckadner Mar 31, 2023
d7d2609
tensorflow not available on s390x
ckadner Mar 31, 2023
a592fc7
build platform linux/amd64
ckadner Mar 31, 2023
711af70
build platform linux/arm64
ckadner Mar 31, 2023
c4e8e4e
build platform linux/ppc64le
ckadner Mar 31, 2023
a2c7c2f
build platform linux/s390x
ckadner Mar 31, 2023
a0707f9
Cross-compile build stage
ckadner Apr 4, 2023
bb7a32e
Upgrade UBI to v8.7; exclude s390x
ckadner Apr 4, 2023
97f9ded
tensorflow not available on ppc64le
ckadner Apr 4, 2023
5ff67a8
Build dev image for BUILDPLATFORM only
ckadner Apr 4, 2023
e26aae4
Install grpcio using wheel before tensorflow
ckadner Apr 5, 2023
44dd08d
cache microdnf install packages
ckadner Apr 5, 2023
c2ee7d4
Don't set "default" for ARG TARGETOS and ARG TARGETARCH
ckadner Apr 12, 2023
f671a1f
Build developer image for all platforms
ckadner Apr 12, 2023
2d269a7
Use buildkit for unit-test
ckadner Apr 12, 2023
899841f
lint
ckadner Apr 12, 2023
5d6dc87
pre-build dev image for build platform only
ckadner Apr 12, 2023
e42267f
Build dev image for build platform only
ckadner Apr 12, 2023
c163d06
Address pip cache disabled warning
ckadner Apr 12, 2023
34aed5c
Test image layer caching
ckadner Apr 12, 2023
035c4d6
Use PIP_CACHE_DIR
ckadner Apr 12, 2023
66a483f
Use PIP_CACHE_DIR to install pre-commit
ckadner Apr 12, 2023
0026153
Address review comments
ckadner Apr 27, 2023
007b94d
Merge branch 'main' into patch-1
ckadner Apr 28, 2023
77fc304
Missing newline at end of file .dockerignore
ckadner Apr 28, 2023
6d150ad
Use ENV instead of ARG for non-build-args
ckadner May 4, 2023
78e7ee4
Mark .PHONY make targets separately
ckadner May 4, 2023
c03aa77
Harmonize GitHub workflow files
ckadner May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 58 additions & 24 deletions .github/workflows/build-and-push.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,74 @@ name: Build and Push

on:
push:
branches: [main]
tags:
- v*
branches: [main, "release-[0-9].[0-9]+"]
paths-ignore: ["**.md"]
tags: ["v*"]
pull_request:
branches: [main, "release-[0-9].[0-9]+"]
paths-ignore: ["**.md"]

env:
IMAGE_NAME: "kserve/modelmesh-runtime-adapter"

jobs:
test:
tjohnson31415 marked this conversation as resolved.
Show resolved Hide resolved
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Build develop image
run: make build.develop
- name: Run lint
run: ./scripts/develop.sh make fmt
- name: Run unit test
run: ./scripts/develop.sh make test
build:
needs: test
runs-on: ubuntu-latest
env:
IMAGE_NAME: kserve/modelmesh-runtime-adapter
CI: true
steps:
- uses: actions/checkout@v2
- name: Build runtime image
run: make build
- name: Log in to Docker Hub
run: docker login -u ${{ secrets.DOCKER_USER }} -p ${{ secrets.DOCKER_ACCESS_TOKEN }}
- name: Push to Docker Hub
- name: Checkout
uses: actions/checkout@v3

- name: Setup QEMU
uses: docker/setup-qemu-action@v2

- name: Setup Docker Buildx
uses: docker/setup-buildx-action@v2

- name: Login to DockerHub
if: github.event_name == 'push'
uses: docker/login-action@v2
with:
username: ${{ secrets.DOCKER_USER }}
password: ${{ secrets.DOCKER_ACCESS_TOKEN }}

- name: Export docker build args
run: |
# see: scripts/build_docker.sh

# Strip git ref prefix from version
VERSION=$(echo "${{ github.ref }}" | sed -e 's,.*/\(.*\),\1,')
echo $VERSION

# Generate PR tag from github.ref == "refs/pull/123/merge"
[ "$VERSION" == "merge" ] && VERSION=$(echo "${{ github.ref }}" | sed -e 's,refs/pull/\(.*\)/merge,pr-\1,')

# Use Docker `latest` tag convention
[ "$VERSION" == "main" ] && VERSION=latest

docker tag ${{ env.IMAGE_NAME }}:latest ${{ env.IMAGE_NAME }}:$VERSION
docker push ${{ env.IMAGE_NAME }}:$VERSION
git_commit_sha="$(git rev-parse HEAD)"
DOCKER_TAG="$(git rev-parse --abbrev-ref HEAD)-$(date -u +"%Y%m%dT%H%M%S%Z")"

echo "IMAGE_TAG=$VERSION" >> $GITHUB_ENV
echo "COMMIT_SHA=$git_commit_sha" >> $GITHUB_ENV
echo "IMAGE_VERSION=$DOCKER_TAG" >> $GITHUB_ENV

# print env vars for debugging
cat "$GITHUB_ENV"

- name: Build and push runtime image
uses: docker/build-push-action@v4
with:
# tensorflow not supported on PowerPC (ppc64le) or System Z (s390x)
# https://pypi.org/project/tensorflow/2.12.0/#files
# https://github.com/tensorflow/tensorflow/issues/46181
platforms: linux/amd64,linux/arm64
context: .
target: runtime
push: ${{ github.event_name != 'pull_request' }}
tags: ${{ env.IMAGE_NAME }}:${{ env.IMAGE_TAG }}
build-args: |
IMAGE_VERSION=${{ env.IMAGE_VERSION }}
COMMIT_SHA=${{ env.COMMIT_SHA }}
cache-from: type=gha
cache-to: type=gha,mode=max
14 changes: 11 additions & 3 deletions .github/workflows/unit-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,22 @@ name: Unit Test

on:
pull_request:
branches: [main]
branches:
- "main"
- "release-*"

jobs:
test:
runs-on: ubuntu-latest
env:
CI: true
DOCKER_BUILDKIT: 1
steps:
- uses: actions/checkout@v2
- name: Build develop image
- name: Checkout
uses: actions/checkout@v3
- name: Setup Docker Buildx
uses: docker/setup-buildx-action@v2
- name: Build dev image
run: make build.develop
- name: Run lint
run: ./scripts/develop.sh make fmt
Expand Down
169 changes: 111 additions & 58 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,105 +13,148 @@
# limitations under the License.

###############################################################################
# Stage 1: Create the develop, test, and build environment
# Stage 1: Create the developer image for the BUILDPLATFORM only
###############################################################################
FROM registry.access.redhat.com/ubi8/ubi-minimal:8.4 AS develop
ARG GOLANG_VERSION=1.17
FROM --platform=$BUILDPLATFORM registry.access.redhat.com/ubi8/go-toolset:$GOLANG_VERSION AS develop

ARG GOLANG_VERSION=1.17.13
ARG PROTOC_VERSION=21.5

USER root

# Install build and dev tools
RUN microdnf install \
gcc \
gcc-c++ \
make \
vim \
findutils \
diffutils \
git \
wget \
tar \
unzip \
python3 \
nodejs && \
pip3 install pre-commit

# Install go
ENV PATH /usr/local/go/bin:$PATH
RUN set -eux; \
wget -qO go.tgz "https://golang.org/dl/go${GOLANG_VERSION}.linux-amd64.tar.gz"; \
sha256sum *go.tgz; \
tar -C /usr/local -xzf go.tgz; \
go version
RUN --mount=type=cache,target=/root/.cache/dnf:rw \
dnf install -y --nodocs --setopt=cachedir=/root/.cache/dnf \
python3 \
python3-pip \
nodejs \
&& true

# Install pre-commit
ARG PIP_CACHE_DIR=/root/.cache/pip
RUN --mount=type=cache,target=$PIP_CACHE_DIR \
pip3 install pre-commit && \
pip3 list
ckadner marked this conversation as resolved.
Show resolved Hide resolved

# When using the BuildKit backend, Docker predefines a set of ARG variables with
# information on the platform of the node performing the build (build platform)
# These arguments are defined in the global scope but are not automatically available
# inside build stages. We need to expose the BUILDOS and BUILDARCH inside the build
# stage and redefine it without a value
# https://docs.docker.com/engine/reference/builder/#automatic-platform-args-in-the-global-scope
ARG BUILDOS
ARG BUILDARCH

# Install protoc
ckadner marked this conversation as resolved.
Show resolved Hide resolved
# The protoc download files use a different variation of architecture identifiers
# from the Docker BUILDARCH forms amd64, arm64, ppc64le, s390x
# protoc-22.2-linux-aarch_64.zip <- arm64
# protoc-22.2-linux-ppcle_64.zip <- ppc64le
# protoc-22.2-linux-s390_64.zip <- s390x
# protoc-22.2-linux-x86_64.zip <- amd64
# so we need to map the arch identifiers before downloading the protoc.zip using
# shell parameter expansion: with the first character of a parameter being an
# exclamation point (!) it introduces a level of indirection where the value
# of the parameter is used as the name of another variable and the value of that
# other variable is the result of the expansion, e.g. the echo statement in the
# following three lines of shell script print "x86_64"
# BUILDARCH=amd64
# amd64=x86_64
# echo ${!BUILDARCH}
RUN set -eux; \
wget -qO protoc.zip "https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/protoc-${PROTOC_VERSION}-linux-x86_64.zip"; \
sha256sum protoc.zip; \
unzip protoc.zip -x readme.txt -d /usr/local; \
protoc --version
amd64=x86_64; \
arm64=aarch_64; \
ppc64le=ppcle_64; \
s390x=s390_64; \
wget -qO protoc.zip "https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/protoc-${PROTOC_VERSION}-${BUILDOS}-${!BUILDARCH}.zip" \
&& sha256sum protoc.zip \
&& unzip protoc.zip -x readme.txt -d /usr/local \
&& protoc --version \
&& true

WORKDIR /opt/app

COPY go.mod go.sum ./

# Install go protoc plugins
ENV PATH /root/go/bin:$PATH
tjohnson31415 marked this conversation as resolved.
Show resolved Hide resolved
RUN go get google.golang.org/protobuf/cmd/protoc-gen-go \
google.golang.org/grpc/cmd/protoc-gen-go-grpc

WORKDIR /opt/app

# Download and initialize the pre-commit environments before copying the source so they will be cached
COPY .pre-commit-config.yaml ./
RUN git init && \
pre-commit install-hooks && \
rm -rf .git

# Download dependiencies before copying the source so they will be cached
COPY go.mod go.sum ./
# Download dependencies before copying the source so they will be cached
RUN go mod download

# the ubi/go-toolset image doesn't define ENTRYPOINT or CMD, but we need it to run 'make develop'
CMD /bin/bash


###############################################################################
# Stage 2: Run the build
# Stage 2: Run the go build with BUILDPLATFORM's native go compiler
###############################################################################
FROM develop AS build
FROM --platform=$BUILDPLATFORM develop AS build

LABEL image="build"

# Copy the source
COPY . ./

# Build the binaries
RUN go build -o puller model-serving-puller/main.go
RUN go build -o triton-adapter model-mesh-triton-adapter/main.go
RUN go build -o mlserver-adapter model-mesh-mlserver-adapter/main.go
RUN go build -o ovms-adapter model-mesh-ovms-adapter/main.go
RUN go build -o torchserve-adapter model-mesh-torchserve-adapter/main.go
# https://docs.docker.com/engine/reference/builder/#automatic-platform-args-in-the-global-scope
# don't provide "default" values (e.g. 'ARG TARGETARCH=amd64') for non-buildx environments,
# see https://github.com/docker/buildx/issues/510
ARG TARGETOS
ARG TARGETARCH

ARG GOOS=${TARGETOS:-linux}
ARG GOARCH=${TARGETARCH:-amd64}

# Build the binaries using native go compiler from BUILDPLATFORM but compiled output for TARGETPLATFORM
# https://www.docker.com/blog/faster-multi-platform-builds-dockerfile-cross-compilation-guide/
RUN --mount=type=cache,target=/root/.cache/go-build \
--mount=type=cache,target=/go/pkg \
go build -o puller model-serving-puller/main.go && \
go build -o puller model-serving-puller/main.go && \
ckadner marked this conversation as resolved.
Show resolved Hide resolved
go build -o triton-adapter model-mesh-triton-adapter/main.go && \
go build -o mlserver-adapter model-mesh-mlserver-adapter/main.go && \
go build -o ovms-adapter model-mesh-ovms-adapter/main.go && \
go build -o torchserve-adapter model-mesh-torchserve-adapter/main.go


###############################################################################
# Stage 3: Copy build assets to create the smallest final runtime image
###############################################################################
FROM registry.access.redhat.com/ubi8/ubi-minimal:8.4 as runtime
FROM registry.access.redhat.com/ubi8/ubi-minimal:8.7 as runtime
tjohnson31415 marked this conversation as resolved.
Show resolved Hide resolved

ARG IMAGE_VERSION
ARG COMMIT_SHA
ARG USER=2000

LABEL name="model-serving-runtime-adapter" \
version="${IMAGE_VERSION}" \
release="${COMMIT_SHA}" \
summary="Sidecar container which runs in the Model-Mesh Serving model server pods" \
description="Container which runs in each model serving pod and act as an intermediary between model-mesh and third-party model-server containers"

USER root

# install python to convert keras to tf
RUN microdnf install \
gcc \
gcc-c++ \
python38 && \
ln -sf /usr/bin/python3 /usr/bin/python && \
ln -sf /usr/bin/pip3 /usr/bin/pip && \
pip install tensorflow
# NOTE: tensorflow not supported on PowerPC (ppc64le) or System Z (s390x) https://github.com/tensorflow/tensorflow/issues/46181
RUN --mount=type=cache,target=/root/.cache/microdnf:rw \
microdnf install --setopt=cachedir=/root/.cache/microdnf \
gcc \
gcc-c++ \
python38-devel \
python38 \
&& ln -sf /usr/bin/python3 /usr/bin/python \
&& ln -sf /usr/bin/pip3 /usr/bin/pip \
&& true

# need to upgrade pip and install wheel before installing grpcio, before installing tensorflow on aarch64
# use caching to speed up multi-platform builds
ARG PIP_CACHE_DIR=/root/.cache/pip
RUN --mount=type=cache,target=$PIP_CACHE_DIR \
pip install --upgrade pip && \
pip install wheel && \
pip install grpcio && \
pip install tensorflow && \
pip list && \
pip cache info
ckadner marked this conversation as resolved.
Show resolved Hide resolved

USER ${USER}

Expand All @@ -123,6 +166,16 @@ COPY --from=build /opt/app/model-mesh-triton-adapter/scripts/tf_pb.py /opt/scrip
COPY --from=build /opt/app/ovms-adapter /opt/app/
COPY --from=build /opt/app/torchserve-adapter /opt/app/

# wait to create commit-specific LABEL until end of the build to not unnecessarily
# invalidate the cached image layers
ARG IMAGE_VERSION
ARG COMMIT_SHA

LABEL name="model-serving-runtime-adapter" \
version="${IMAGE_VERSION}" \
release="${COMMIT_SHA}" \
summary="Sidecar container which runs in the ModelMesh Serving model server pods" \
description="Container which runs in each model serving pod acting as an intermediary between ModelMesh and third-party model-server containers"

# Don't define an entrypoint. This is a multi-purpose image so the user should specify which binary they want to run (e.g. /opt/app/puller or /opt/app/triton-adapter)
# ENTRYPOINT ["/opt/app/puller"]
12 changes: 5 additions & 7 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# collect args from `make run` so that they don't run twice
ifeq (run,$(firstword $(MAKECMDGOALS)))
RUN_ARGS := $(wordlist 2,$(words $(MAKECMDGOALS)),$(MAKECMDGOALS))
ifneq ("$(wildcard /.dockerenv)","")
$(error Inside docker container, run 'make $(RUN_ARGS)')
endif
endif

all: build
Expand All @@ -27,12 +31,6 @@ build.develop:
develop: build.develop
./scripts/develop.sh

docs:
./scripts/docs.sh

docs.dev:
./scripts/docs.sh --dev

run: build.develop
./scripts/develop.sh make $(RUN_ARGS)

Expand All @@ -49,4 +47,4 @@ proto.compile:
$(eval $(RUN_ARGS):;@:)

# Remove $(MAKECMDGOALS) if you don't intend make to just be a taskrunner
.PHONY: all $(MAKECMDGOALS)
.PHONY: all build build.develop develop run test fmt proto.compile $(MAKECMDGOALS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?