Address review comments

Signed-off-by: Evan Lezar <elezar@nvidia.com>
2025-06-26 18:18:24 +00:00 · 2021-11-26 15:15:46 +01:00 · 2021-11-26 15:15:46 +01:00 · fd5a1a72f0
commit fd5a1a72f0
parent 738d28dac5
5 changed files with 64 additions and 62 deletions
--- a/build/container/Dockerfile.centos
+++ b/build/container/Dockerfile.centos
@ -46,17 +46,18 @@ ENV NVIDIA_DISABLE_REQUIRE="true"
 ENV NVIDIA_VISIBLE_DEVICES=all
 ENV NVIDIA_DRIVER_CAPABILITIES=utility

-ARG ARTIFACTS_DIR
-COPY ${ARTIFACTS_DIR}/ /artifacts/packages/
+ARG ARTIFACTS_ROOT
+ARG PACKAGE_DIST
+COPY ${ARTIFACTS_ROOT}/${PACKAGE_DIST} /artifacts/packages/${PACKAGE_DIST}

 WORKDIR /artifacts/packages

 ARG PACKAGE_VERSION
 ARG PACKAGE_ARCH
 RUN yum localinstall -y \
-    ${PACKAGE_ARCH}/libnvidia-container1-${PACKAGE_VERSION}*.rpm \
-    ${PACKAGE_ARCH}/libnvidia-container-tools-${PACKAGE_VERSION}*.rpm \
-    ${PACKAGE_ARCH}/nvidia-container-toolkit-${PACKAGE_VERSION}*.rpm
+    ${PACKAGE_DIST}/${PACKAGE_ARCH}/libnvidia-container1-${PACKAGE_VERSION}*.rpm \
+    ${PACKAGE_DIST}/${PACKAGE_ARCH}/libnvidia-container-tools-${PACKAGE_VERSION}*.rpm \
+    ${PACKAGE_DIST}/${PACKAGE_ARCH}/nvidia-container-toolkit-${PACKAGE_VERSION}*.rpm

 WORKDIR /work

--- a/build/container/Dockerfile.packaging
+++ b/build/container/Dockerfile.packaging
@ -21,8 +21,8 @@ FROM nvidia/cuda:${CUDA_VERSION}-base-${BASE_DIST}

 ENV NVIDIA_CONTAINER_TOOLKIT_VERSION="${VERSION}"

-ARG ARTIFACTS_DIR
-COPY ${ARTIFACTS_DIR}/ /artifacts/packages/
+ARG ARTIFACTS_ROOT
+COPY ${ARTIFACTS_ROOT} /artifacts/packages/

 WORKDIR /artifacts/packages

--- a/build/container/Dockerfile.ubuntu
+++ b/build/container/Dockerfile.ubuntu
@ -52,17 +52,18 @@ ENV NVIDIA_DISABLE_REQUIRE="true"
 ENV NVIDIA_VISIBLE_DEVICES=all
 ENV NVIDIA_DRIVER_CAPABILITIES=utility

-ARG ARTIFACTS_DIR
-COPY ${ARTIFACTS_DIR}/ /artifacts/packages/
+ARG ARTIFACTS_ROOT
+ARG PACKAGE_DIST
+COPY ${ARTIFACTS_ROOT}/${PACKAGE_DIST} /artifacts/packages/${PACKAGE_DIST}

 WORKDIR /artifacts/packages

 ARG PACKAGE_VERSION
 ARG PACKAGE_ARCH
 RUN dpkg -i \
-    ${PACKAGE_ARCH}/libnvidia-container1_${PACKAGE_VERSION}*.deb \
-    ${PACKAGE_ARCH}/libnvidia-container-tools_${PACKAGE_VERSION}*.deb \
-    ${PACKAGE_ARCH}/nvidia-container-toolkit_${PACKAGE_VERSION}*.deb
+    ${PACKAGE_DIST}/${PACKAGE_ARCH}/libnvidia-container1_${PACKAGE_VERSION}*.deb \
+    ${PACKAGE_DIST}/${PACKAGE_ARCH}/libnvidia-container-tools_${PACKAGE_VERSION}*.deb \
+    ${PACKAGE_DIST}/${PACKAGE_ARCH}/nvidia-container-toolkit_${PACKAGE_VERSION}*.deb

 WORKDIR /work

--- a/build/container/Makefile
+++ b/build/container/Makefile
@ -31,11 +31,13 @@ IMAGE = $(IMAGE_NAME):$(IMAGE_TAG)

 ##### Public rules #####
 DEFAULT_PUSH_TARGET := ubuntu18.04
-TARGETS := ubuntu20.04 ubuntu18.04 ubi8 centos7 centos8 packaging
+TARGETS := ubuntu20.04 ubuntu18.04 ubi8 centos7 centos8

-BUILD_TARGETS := $(patsubst %, build-%, $(TARGETS))
-PUSH_TARGETS := $(patsubst %, push-%, $(TARGETS))
-TEST_TARGETS := $(patsubst %, test-%, $(filter-out packaging,$(TARGETS)))
+META_TARGETS := packaging
+
+BUILD_TARGETS := $(patsubst %,build-%,$(TARGETS) $(META_TARGETS))
+PUSH_TARGETS := $(patsubst %,push-%,$(TARGETS) $(META_TARGETS))
+TEST_TARGETS := $(patsubst %,test-%, $(TARGETS))

 .PHONY: $(TARGETS) $(PUSH_TARGETS) $(BUILD_TARGETS) $(TEST_TARGETS)

@ -57,15 +59,19 @@ push-short:
 build-%: DIST = $(*)
 build-%: DOCKERFILE = $(CURDIR)/build/container/Dockerfile.$(DOCKERFILE_SUFFIX)

+ARTIFACTS_ROOT ?= $(shell realpath --relative-to=$(CURDIR) $(DIST_DIR))
+
 # Use a generic build target to build the relevant images
-$(BUILD_TARGETS): build-%: $(ARTIFACTS_DIR)
+$(BUILD_TARGETS): build-%: $(ARTIFACTS_ROOT)
 	DOCKER_BUILDKIT=1 \
-		$(DOCKER) build --platform=linux/amd64 --pull \
+		$(DOCKER) build --pull \
+		--platform=linux/amd64 \
 		--tag $(IMAGE) \
-		--build-arg ARTIFACTS_DIR="$(ARTIFACTS_DIR)" \
+		--build-arg ARTIFACTS_ROOT="$(ARTIFACTS_ROOT)" \
 		--build-arg BASE_DIST="$(BASE_DIST)" \
 		--build-arg CUDA_VERSION="$(CUDA_VERSION)" \
 		--build-arg GOLANG_VERSION="$(GOLANG_VERSION)" \
+		--build-arg PACKAGE_DIST="$(PACKAGE_DIST)" \
 		--build-arg PACKAGE_VERSION="$(PACKAGE_VERSION)" \
 		--build-arg PACKAGE_ARCH="$(PACKAGE_ARCH)" \
 		--build-arg VERSION="$(VERSION)" \
@ -73,34 +79,30 @@ $(BUILD_TARGETS): build-%: $(ARTIFACTS_DIR)
 		$(CURDIR)


-ARTIFACTS_ROOT ?= $(shell realpath --relative-to=$(CURDIR) $(DIST_DIR))
-ARTIFACTS_DIR = $(ARTIFACTS_ROOT)/$(*)
-
-PACKAGE_ARCH = amd64
-
+build-ubuntu%: BASE_DIST = $(*)
 build-ubuntu%: DOCKERFILE_SUFFIX := ubuntu
+build-ubuntu%: PACKAGE_ARCH := amd64
+build-ubuntu%: PACKAGE_DIST = $(BASE_DIST)
 build-ubuntu%: PACKAGE_VERSION := $(LIB_VERSION)$(if $(LIB_TAG),~$(LIB_TAG))

-build-ubuntu18.04: BASE_DIST := ubuntu18.04
-build-ubuntu20.04: BASE_DIST := ubuntu20.04
-
-build-ubi8: DOCKERFILE_SUFFIX := centos
 # TODO: Update this to use the centos8 packages
-build-ubi8: ARTIFACTS_DIR = $(ARTIFACTS_ROOT)/centos7
-build-ubi8: PACKAGE_VERSION := $(LIB_VERSION)-$(if $(LIB_TAG),0.1.$(LIB_TAG),1)
 build-ubi8: BASE_DIST := ubi8
+build-ubi8: DOCKERFILE_SUFFIX := centos
 build-ubi8: PACKAGE_ARCH := x86_64
+build-ubi8: PACKAGE_DIST = centos7
+build-ubi8: PACKAGE_VERSION := $(LIB_VERSION)-$(if $(LIB_TAG),0.1.$(LIB_TAG),1)

+build-centos%: BASE_DIST = $(*)
 build-centos%: DOCKERFILE_SUFFIX := centos
-build-centos%: PACKAGE_VERSION := $(LIB_VERSION)-$(if $(LIB_TAG),0.1.$(LIB_TAG),1)
 build-centos%: PACKAGE_ARCH := x86_64
-
-build-centos7: BASE_DIST := centos7
-build-centos8: BASE_DIST := centos8
+build-centos%: PACKAGE_DIST = $(BASE_DIST)
+build-centos%: PACKAGE_VERSION := $(LIB_VERSION)-$(if $(LIB_TAG),0.1.$(LIB_TAG),1)

 build-packaging: BASE_DIST := ubuntu20.04
 build-packaging: DOCKERFILE_SUFFIX := packaging
-build-packaging: ARTIFACTS_DIR = $(ARTIFACTS_ROOT)
+build-packaging: PACKAGE_ARCH := amd64
+build-packaging: PACKAGE_DIST = all
+build-packaging: PACKAGE_VERSION := $(LIB_VERSION)$(if $(LIB_TAG),-$(LIB_TAG))

 # Test targets
 test-%: DIST = $(*)
--- a/scripts/pull-packages.sh
+++ b/scripts/pull-packages.sh
@ -16,7 +16,7 @@

 function assert_usage() {
    echo "Incorrect arguments: $*"
-    echo "$(basename ${BASH_SOURCE[0]}) ROOT PACKAGE_NAME BUILD_VERSION"
+    echo "$(basename ${BASH_SOURCE[0]}) IMAGE DIST_DIR"
    exit 1
 }

@ -25,36 +25,34 @@ set -e -x
 SCRIPTS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )"/../scripts && pwd )"
 PROJECT_ROOT="$( cd ${SCRIPTS_DIR}/.. && pwd )"

-if [[ $# -ne 0 ]]; then
+if [[ $# -ne 2 ]]; then
    assert_usage $*
 fi

-function pull_package_files() {
-    local image=$1
-    local dist_dir=$2
-    echo "Copying package files from ${image} to ${dist_dir}"
-    mkdir -p ${dist_dir}
-    docker run \
-        --rm \
-        --entrypoint="bash" \
-        -v $(pwd):$(pwd) \
-        -w $(pwd) \
-        -u $(id -u):$(id -g) \
-            ${image} \
-            -c "cp -R /artifacts/packages/* ${dist_dir}"
-}
-
-if [[ -z ${VERSION} ]]; then
-eval $(${SCRIPTS_DIR}/get-component-versions.sh)
-VERSION=${NVIDIA_CONTAINER_TOOLKIT_VERSION}${NVIDIA_CONTAINER_TOOLKIT_TAG:+-${NVIDIA_CONTAINER_TOOLKIT_TAG}}
-fi
+IMAGE=$1
+DIST_DIR=$2

 if [[ -z ${IMAGE} ]]; then
-: ${REGISTRY:=""}
-: ${IMAGE_NAME:="nvidia/container-toolkit"}
-image_tag=${VERSION}-packaging
-IMAGE=${REGISTRY:+${REGISTRY}/}${IMAGE_NAME}:${image_tag}
+    echo "ERROR: IMAGE must be non-empty"
+    exit 1
 fi
-: ${DIST_DIR:="dist-test"}

-pull_package_files ${IMAGE} ${DIST_DIR}
+if [[ -z ${DIST_DIR} ]]; then
+    echo "ERROR: DIST_DIR must be non-empty"
+    exit 1
+fi
+
+if [[ -e ${DIST_DIR} ]]; then
+    echo "ERROR: The specified DIST_DIR ${DIST_DIR} exists."
+    exit 1
+fi
+
+echo "Copying package files from ${IMAGE} to ${DIST_DIR}"
+mkdir -p ${DIST_DIR}
+docker run --rm \
+    -v $(pwd):$(pwd) \
+    -w $(pwd) \
+    -u $(id -u):$(id -g) \
+    --entrypoint="bash" \
+        ${IMAGE} \
+        -c "cp -R /artifacts/packages/* ${DIST_DIR}"