Skip to content

Commit

Permalink
chore: Reuse existing developer image (#66)
Browse files Browse the repository at this point in the history
#### Motivation

Each time running lint using the developer image, i.e. `make run fmt`,
the `build.develop` Make target is executed, calling
`scripts/build_docker.sh` which (re)builds the developer image each
time, which despite using cached image layers takes extra time and
clutters the terminal with Docker build output that is not relevant with
respect to the actual make goal having been invoked.

```
[modelmesh-runtime-adapter] (main=)$ make run fmt

Makefile:74: warning: overriding commands for target `fmt'
Makefile:60: warning: ignoring old commands for target `fmt'
./scripts/build_docker.sh --target develop
[+] Building 1.1s (15/15) FINISHED                                                                                                                            docker:colima
 => [internal] load build definition from Dockerfile                                                                                                                   0.0s
 => => transferring dockerfile: 7.60kB                                                                                                                                 0.0s
 => [internal] load .dockerignore                                                                                                                                      0.0s
 => => transferring context: 93B                                                                                                                                       0.0s
 => [internal] load metadata for registry.access.redhat.com/ubi8/go-toolset:1.17                                                                                       1.0s
 => [develop  1/10] FROM registry.access.redhat.com/ubi8/go-toolset:1.17@sha256:db868166dd2ea38bdb8507e0cb1c9a295622fb6d8bdd70718af1405679ca0a19                       0.0s
 => [internal] load build context                                                                                                                                      0.0s
 => => transferring context: 98B                                                                                                                                       0.0s
 => CACHED [develop  2/10] RUN --mount=type=cache,target=/root/.cache/dnf:rw     dnf install --setopt=cachedir=/root/.cache/dnf -y --nodocs         python3         p  0.0s
 => CACHED [develop  3/10] RUN --mount=type=cache,target=/root/.cache/pip     pip3 install pre-commit                                                                  0.0s
 => CACHED [develop  4/10] RUN set -eux;     amd64=x86_64;     arm64=aarch_64;     ppc64le=ppcle_64;     s390x=s390_64;     wget -qO protoc.zip "https://github.com/p  0.0s
 => CACHED [develop  5/10] WORKDIR /opt/app                                                                                                                            0.0s
 => CACHED [develop  6/10] COPY go.mod go.sum ./                                                                                                                       0.0s
 => CACHED [develop  7/10] RUN true     && go get google.golang.org/grpc/cmd/protoc-gen-go-grpc     && go install google.golang.org/protobuf/cmd/protoc-gen-go         0.0s
 => CACHED [develop  8/10] COPY .pre-commit-config.yaml ./                                                                                                             0.0s
 => CACHED [develop  9/10] RUN git init &&     pre-commit install-hooks &&     rm -rf .git                                                                             0.0s
 => CACHED [develop 10/10] RUN go mod download                                                                                                                         0.0s
 => exporting to image                                                                                                                                                 0.0s
 => => exporting layers                                                                                                                                                0.0s
 => => writing image sha256:feb1d14c1a753a7e4494b9b53f8132fec927c39e565eacede9ea2a460a78ebec                                                                           0.0s
 => => naming to docker.io/kserve/modelmesh-runtime-adapter-develop:reuse_existing_dev_image-20231027T220733PDT                                                        0.0s
 => => naming to docker.io/kserve/modelmesh-runtime-adapter-develop:latest                                                                                             0.0s
./scripts/develop.sh make fmt
./scripts/fmt.sh
golangci-lint............................................................Passed
prettier.................................................................Passed
```

#### Modifications

- Add flag `--use-existing` to `scripts/build_docker.sh` to skip the
Docker build it the image exists
- Add internal Make target `use.develop` which call
`scripts/build_docker.sh` with the new flag `--use-existing`
- Add env vars `DOCKER_USER` and `IMAGE_TAG` to `scripts/develop.sh` to
be consistent with `scripts/build_docker.sh`

#### Result

```
[modelmesh-runtime-adapter_ckadner] (reuse_existing_dev_image=)$ make run fmt

Makefile:74: warning: overriding commands for target `fmt'
Makefile:60: warning: ignoring old commands for target `fmt'
./scripts/build_docker.sh --target develop --use-existing
./scripts/develop.sh make fmt
./scripts/fmt.sh
golangci-lint............................................................Passed
prettier.................................................................Passed
```

Signed-off-by: Christian Kadner <[email protected]>
Signed-off-by: Rafael Vasquez <[email protected]>
Co-authored-by: Rafael Vasquez <[email protected]>
  • Loading branch information
ckadner and rafvasq authored Oct 30, 2023
1 parent 2eeb488 commit fa0c724
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 7 deletions.
9 changes: 7 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,19 @@ build:
build.develop:
./scripts/build_docker.sh --target develop

.PHONY: use.develop
## Check if developer image exists, build it if it doesn't
use.develop:
./scripts/build_docker.sh --target develop --use-existing

.PHONY: develop
## Run interactive shell inside developer container
develop: build.develop
develop: use.develop
./scripts/develop.sh

.PHONY: run
## Run make target inside developer container (e.g. `make run fmt`)
run: build.develop
run: use.develop
./scripts/develop.sh make $(RUN_ARGS)

.PHONY: test
Expand Down
16 changes: 14 additions & 2 deletions scripts/build_docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# distributed under the License is distributed on an "AS IS" BASIS,
# 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.#
# limitations under the License.

USAGE="$(
cat <<EOF
Expand Down Expand Up @@ -40,6 +40,10 @@ while (("$#")); do
-h | --help)
usage
;;
--use-existing)
use_existing=true
shift 1
;;
-t | --target)
if [ -n "$2" ] && [ "${2:0:1}" != "-" ]; then
DOCKER_TARGET=$2
Expand Down Expand Up @@ -67,14 +71,22 @@ done

DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"

cd "$DIR/.." ||
cd "$DIR/.."

IMAGE_SUFFIX=""

if [ "${DOCKER_TARGET}" != "runtime" ]; then
IMAGE_SUFFIX="-${DOCKER_TARGET}"
fi

if [[ $use_existing == "true" ]]; then
DOCKER_IMAGE="${DOCKER_USER}/modelmesh-runtime-adapter${IMAGE_SUFFIX}:${IMAGE_TAG}"
if docker image inspect "${DOCKER_IMAGE}" >/dev/null 2>&1; then
echo "Using existing image: ${DOCKER_IMAGE}"
exit 0
fi
fi

declare -a docker_args=(
--target "${DOCKER_TARGET}"
-t "${DOCKER_USER}/modelmesh-runtime-adapter${IMAGE_SUFFIX}:${DOCKER_TAG}"
Expand Down
10 changes: 7 additions & 3 deletions scripts/develop.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# distributed under the License is distributed on an "AS IS" BASIS,
# 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.#
# limitations under the License.

USAGE="$(
cat <<EOF
Expand All @@ -22,11 +22,15 @@ usage: $0 [optional command]
[-h | --help] Display this help
EOF
)"

usage() {
echo "$USAGE" >&2
exit 1
}

DOCKER_USER=${DOCKER_USER:-"kserve"}
IMAGE_TAG=${IMAGE_TAG:-"latest"}

# PARAMS=""

# while (("$#")); do
Expand All @@ -50,7 +54,7 @@ usage() {
# eval set -- "$PARAMS"

DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
cd "${DIR}/.." ||
cd "${DIR}/.."

# Make sure .bash_history exists and is a file
touch .bash_history
Expand All @@ -73,4 +77,4 @@ fi
# Run the develop container with local source mounted in
docker run --rm \
"${docker_run_args[@]}" \
kserve/modelmesh-runtime-adapter-develop:latest "$@"
"${DOCKER_USER}/modelmesh-runtime-adapter-develop:${IMAGE_TAG}" "$@"

0 comments on commit fa0c724

Please sign in to comment.