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

Jump to bottom

Fix make targets which generate mig-parted packages #41

Merged
merged 2 commits into from
Apr 18, 2024
Merged

Fix make targets which generate mig-parted packages #41

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e88018f
Strip 'v' from version string when generating mig-parted packages
cdesiniotis Feb 29, 2024
20fd208
Remove ubuntu18.04 make targets
cdesiniotis Feb 29, 2024
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
25 changes: 7 additions & 18 deletions deployments/systemd/packages/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,20 @@ REGISTRY ?= nvcr.io/nvidia
IMAGE := $(REGISTRY)/$(NAME)
endif

# strip 'v' from version string
PACKAGE_VERSION := $(VERSION:v%=%)
Copy link
Member

Choose a reason for hiding this comment

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

Out of scope for this PR: in the libnvidia-container and nvidia-container-toolkit packages, I started generating the changelog automatically as part of the build to streamline things. Not sure if that's something we want to try here.


##### Public rules #####

VALID_TARGETS = tarball ubuntu18.04 ubuntu20.04 ubi8
VALID_TARGETS = tarball ubuntu20.04 ubi8

all: $(VALID_TARGETS)
.PHONY: all $(VALID_TARGETS)

tarball:
$(DOCKER) build --pull \
--build-arg PACKAGE_NAME=$(NAME) \
--build-arg PACKAGE_VERSION=$(VERSION) \
--build-arg PACKAGE_VERSION=$(PACKAGE_VERSION) \
--build-arg PACKAGE_REVISION=$(REVISION) \
--build-arg GOLANG_VERSION=$(GOLANG_VERSION) \
--tag $(IMAGE):$(vVERSION)-tarball \
Expand All @@ -45,24 +48,10 @@ tarball:
-v $$(pwd)/dist/$(@):/dist \
$(IMAGE):$(vVERSION)-$(@)
Copy link
Contributor

Choose a reason for hiding this comment

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

We seem to pre-pend the v to the VERSION var here. Does this not conflict with the following?

PACKAGE_VERSION := $(VERSION:v%=%)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vVERSION is a separate variable defined in the top-level versions.mk file and is used when we explicitly want a version string with the leading v. In this case, we are using it when constructing the image tag.


ubuntu18.04:
Copy link
Member

Choose a reason for hiding this comment

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

One concern I have here is that this will cause the packages we build to be incompatible with older debian-based systems. Note that these are not deployment images, but the images we use to build the binaries and package them.

This should be aligned with libnvidia-container or the nvidia-container-toolkit.

Copy link
Member

Choose a reason for hiding this comment

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

I have created #42 to do this. Note that we now have deb and rpm targets in this Makefile for clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elezar thanks for working on the follow-up. Are there any pending items for this PR?

Copy link
Member

Choose a reason for hiding this comment

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

I think my concerns are addressed by #42.

$(DOCKER) build --pull \
--build-arg PACKAGE_NAME=$(NAME) \
--build-arg PACKAGE_VERSION=$(VERSION) \
--build-arg PACKAGE_REVISION=$(REVISION) \
--build-arg GOLANG_VERSION=$(GOLANG_VERSION) \
--build-arg BASE_IMAGE=nvcr.io/nvidia/cuda:${CUDA_VERSION}-base-ubuntu18.04 \
--tag $(IMAGE):$(vVERSION)-ubuntu18.04 \
--file Dockerfile.ubuntu \
$(BUILD_DIR)
$(DOCKER) run \
-v $$(pwd)/dist/$(@):/dist \
$(IMAGE):$(vVERSION)-$(@)

ubuntu20.04:
$(DOCKER) build --pull \
--build-arg PACKAGE_NAME=$(NAME) \
--build-arg PACKAGE_VERSION=$(VERSION) \
--build-arg PACKAGE_VERSION=$(PACKAGE_VERSION) \
--build-arg PACKAGE_REVISION=$(REVISION) \
--build-arg GOLANG_VERSION=$(GOLANG_VERSION) \
--build-arg BASE_IMAGE=nvcr.io/nvidia/cuda:${CUDA_VERSION}-base-ubuntu20.04 \
Expand All @@ -76,7 +65,7 @@ ubuntu20.04:
ubi8:
$(DOCKER) build --pull \
--build-arg PACKAGE_NAME=$(NAME) \
--build-arg PACKAGE_VERSION=$(VERSION) \
--build-arg PACKAGE_VERSION=$(PACKAGE_VERSION) \
--build-arg PACKAGE_REVISION=$(REVISION) \
--build-arg GOLANG_VERSION=$(GOLANG_VERSION) \
--build-arg BASE_IMAGE=nvcr.io/nvidia/cuda:${CUDA_VERSION}-base-ubi8 \
Expand Down
Loading