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

update base image to the ubi9 variant #937

Merged
merged 1 commit into from
Aug 26, 2024
Merged

update base image to the ubi9 variant #937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
dist: [ubi8]
dist: [ubi9]
steps:
- uses: actions/checkout@v4
name: Check out code
Expand Down Expand Up @@ -168,7 +168,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
dist: [ubi8]
dist: [ubi9]
steps:
- uses: actions/checkout@v4
name: Check out code
Expand Down Expand Up @@ -237,9 +237,9 @@ jobs:
LOWERCASE_REPO_OWNER=$(echo "${GITHUB_REPOSITORY_OWNER}" | awk '{print tolower($0)}')
echo "LOWERCASE_REPO_OWNER=$(echo "${GITHUB_REPOSITORY_OWNER}" | awk '{print tolower($0)}')" >> $GITHUB_ENV

echo "OPERATOR_VERSION=${COMMIT_SHORT_SHA}-ubi8" >> $GITHUB_ENV
echo "OPERATOR_VERSION=${COMMIT_SHORT_SHA}-ubi9" >> $GITHUB_ENV
Copy link
Member

Choose a reason for hiding this comment

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

Note: Since we only provide one flavour of image we could just drop the -ubi9 suffix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I am assuming there is a rationale behind the ubi8 suffix? If it's safe enough to drop it, I am all for the change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cdesiniotis @shivamerla thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, safe to drop ubi suffix as we don't reference those tags in the Helm charts or OLM.

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 Looks like I'll have to make changes in multiple places to drop the DIST suffix. I'd like to make that change in a follow-up PR.

I'd like to limit the changeset to ubi9 upgrade in this PR

echo "OPERATOR_IMAGE=ghcr.io/${LOWERCASE_REPO_OWNER}/gpu-operator" >> $GITHUB_ENV
echo "VALIDATOR_VERSION=${COMMIT_SHORT_SHA}-ubi8" >> $GITHUB_ENV
echo "VALIDATOR_VERSION=${COMMIT_SHORT_SHA}-ubi9" >> $GITHUB_ENV
echo "VALIDATOR_IMAGE=ghcr.io/${LOWERCASE_REPO_OWNER}/gpu-operator/gpu-operator-validator" >> $GITHUB_ENV

echo "instance_hostname=ubuntu@${{ steps.get_public_dns_name.outputs.result }}" >> $GITHUB_ENV
Expand Down Expand Up @@ -289,9 +289,9 @@ jobs:
LOWERCASE_REPO_OWNER=$(echo "${GITHUB_REPOSITORY_OWNER}" | awk '{print tolower($0)}')
echo "LOWERCASE_REPO_OWNER=$(echo "${GITHUB_REPOSITORY_OWNER}" | awk '{print tolower($0)}')" >> $GITHUB_ENV

echo "OPERATOR_VERSION=${COMMIT_SHORT_SHA}-ubi8" >> $GITHUB_ENV
echo "OPERATOR_VERSION=${COMMIT_SHORT_SHA}-ubi9" >> $GITHUB_ENV
echo "OPERATOR_IMAGE=ghcr.io/${LOWERCASE_REPO_OWNER}/gpu-operator" >> $GITHUB_ENV
echo "VALIDATOR_VERSION=${COMMIT_SHORT_SHA}-ubi8" >> $GITHUB_ENV
echo "VALIDATOR_VERSION=${COMMIT_SHORT_SHA}-ubi9" >> $GITHUB_ENV
echo "VALIDATOR_IMAGE=ghcr.io/${LOWERCASE_REPO_OWNER}/gpu-operator/gpu-operator-validator" >> $GITHUB_ENV

echo "instance_hostname=ubuntu@${{ steps.get_public_dns_name.outputs.result }}" >> $GITHUB_ENV
Expand Down
6 changes: 3 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ include $(CURDIR)/versions.mk

MODULE := github.com/NVIDIA/gpu-operator
BUILDER_IMAGE ?= golang:$(GOLANG_VERSION)
DIST ?= ubi8
DIST ?= ubi9

ifeq ($(IMAGE_NAME),)
REGISTRY ?= nvcr.io/nvidia/cloud-native
Expand Down Expand Up @@ -252,8 +252,8 @@ coverage: unit-test
go tool cover -func=$(COVERAGE_FILE).no-mocks

##### Public rules #####
DISTRIBUTIONS := ubi8
DEFAULT_PUSH_TARGET := ubi8
DISTRIBUTIONS := ubi9
DEFAULT_PUSH_TARGET := ubi9

PUSH_TARGETS := $(patsubst %,push-%, $(DISTRIBUTIONS))
BUILD_TARGETS := $(patsubst %,build-%, $(DISTRIBUTIONS))
Expand Down
10 changes: 5 additions & 5 deletions docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@

ARG GOLANG_VERSION=x.x.x

FROM nvcr.io/nvidia/cuda:12.6.0-base-ubi8 as builder
FROM nvcr.io/nvidia/cuda:12.6.0-base-ubi9 as builder

RUN yum install -y wget make git gcc
RUN dnf install -y wget make git gcc

ARG GOLANG_VERSION=0.0.0
RUN set -eux; \
Expand All @@ -25,7 +25,7 @@ RUN set -eux; \
case "${arch##*-}" in \
x86_64 | amd64) ARCH='amd64' ;; \
ppc64el | ppc64le) ARCH='ppc64le' ;; \
aarch64) ARCH='arm64' ;; \
aarch64 | arm64) ARCH='arm64' ;; \
cdesiniotis marked this conversation as resolved.
Show resolved Hide resolved
*) echo "unsupported architecture" ; exit 1 ;; \
esac; \
wget -nv -O - https://storage.googleapis.com/golang/go${GOLANG_VERSION}.linux-${ARCH}.tar.gz \
Expand Down Expand Up @@ -55,7 +55,7 @@ ARG VERSION="unknown"
ARG GIT_COMMIT="unknown"
RUN make gpu-operator

FROM nvcr.io/nvidia/cuda:12.6.0-base-ubi8
FROM nvcr.io/nvidia/cuda:12.6.0-base-ubi9

# Remove CUDA libs(compat etc) in favor of libs installed by the NVIDIA driver
RUN dnf remove -y cuda-*
Expand Down Expand Up @@ -97,7 +97,7 @@ COPY deployments/gpu-operator/charts/node-feature-discovery/crds/nfd-api-crds.ya
# Install / upgrade packages here that are required to resolve CVEs
ARG CVE_UPDATES
Copy link
Member

Choose a reason for hiding this comment

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

In general, I would recommend removing this block entirely. We have largely stopped using this envvar across our projects -- especially since we've streamlined our waiver process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, agreed . I think this block was added as a stopgap to quickly fix CVEs at the time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can make this change in a follow up PR

RUN if [ -n "${CVE_UPDATES}" ]; then \
yum update -y ${CVE_UPDATES} && \
dnf update -y ${CVE_UPDATES} && \
cdesiniotis marked this conversation as resolved.
Show resolved Hide resolved
rm -rf /var/cache/yum/*; \
fi

Expand Down
10 changes: 5 additions & 5 deletions validator/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
ARG CUDA_SAMPLE_IMAGE=undefined
ARG GOLANG_VERSION=x.x.x

FROM nvcr.io/nvidia/cuda:12.6.0-base-ubi8 as build
FROM nvcr.io/nvidia/cuda:12.6.0-base-ubi9 as build

RUN yum install -y wget make git gcc
RUN dnf install -y wget make git gcc

ARG GOLANG_VERSION=0.0.0
RUN set -eux; \
Expand All @@ -26,7 +26,7 @@ RUN set -eux; \
case "${arch##*-}" in \
x86_64 | amd64) ARCH='amd64' ;; \
ppc64el | ppc64le) ARCH='ppc64le' ;; \
aarch64) ARCH='arm64' ;; \
aarch64 | arm64) ARCH='arm64' ;; \
*) echo "unsupported architecture" ; exit 1 ;; \
esac; \
wget -nv -O - https://storage.googleapis.com/golang/go${GOLANG_VERSION}.linux-${ARCH}.tar.gz \
Expand All @@ -51,7 +51,7 @@ FROM ${CUDA_SAMPLE_IMAGE} AS sample-builder
RUN mkdir /artifacts
RUN cp /cuda-samples/vectorAdd /artifacts/vectorAdd

FROM nvcr.io/nvidia/cuda:12.6.0-base-ubi8
FROM nvcr.io/nvidia/cuda:12.6.0-base-ubi9

# Remove CUDA libs(compat etc) in favor of libs installed by the NVIDIA driver
RUN dnf remove -y cuda-*
Expand Down Expand Up @@ -87,7 +87,7 @@ LABEL vsc-ref=${GIT_COMMIT}
# Install / upgrade packages here that are required to resolve CVEs
ARG CVE_UPDATES
Copy link
Member

Choose a reason for hiding this comment

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

Same here on removing the block.

RUN if [ -n "${CVE_UPDATES}" ]; then \
yum update -y ${CVE_UPDATES} && \
dnf update -y ${CVE_UPDATES} && \
rm -rf /var/cache/yum/*; \
fi

Expand Down
6 changes: 3 additions & 3 deletions validator/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ OUT_IMAGE_TAG = $(OUT_IMAGE_VERSION)-$(DIST)
OUT_IMAGE = $(OUT_IMAGE_NAME):$(OUT_IMAGE_TAG)

##### Public rules #####
DISTRIBUTIONS := ubi8 ubuntu20.04
DEFAULT_PUSH_TARGET := ubi8
DISTRIBUTIONS := ubi9
DEFAULT_PUSH_TARGET := ubi9

PUSH_TARGETS := $(patsubst %,push-%, $(DISTRIBUTIONS))
BUILD_TARGETS := $(patsubst %,build-%, $(DISTRIBUTIONS))
Expand Down Expand Up @@ -87,7 +87,7 @@ $(BUILD_TARGETS): build-%:
--build-arg VERSION="$(VERSION)" \
--build-arg GIT_COMMIT="$(GIT_COMMIT)" \
--build-arg BUILDER_IMAGE="$(BUILDER_IMAGE)" \
--build-arg CUDA_SAMPLE_IMAGE=nvcr.io/nvidia/k8s/cuda-sample:vectoradd-cuda$(CUDA_SAMPLES_VERSION)-$(*) \
tariq1890 marked this conversation as resolved.
Show resolved Hide resolved
--build-arg CUDA_SAMPLE_IMAGE=nvcr.io/nvidia/k8s/cuda-sample:vectoradd-cuda$(CUDA_SAMPLES_VERSION)-ubi8 \
--build-arg GOLANG_VERSION="$(GOLANG_VERSION)" \
--build-arg CVE_UPDATES="$(CVE_UPDATES)" \
--file $(DOCKERFILE) $(BUILD_DIR)
Expand Down