-
Notifications
You must be signed in to change notification settings - Fork 349
Switch to distroless base image #1417
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
||
# Remove CUDA libs(compat etc) in favor of libs installed by the NVIDIA driver | ||
RUN dnf remove -y cuda-* | ||
USER 0:0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does validator need to run as root?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm it doesn't appear I have anything written in my notes concerning this (I made these changes some time ago). My best guess is that this was needed for the driver-validation
container where we create the /dev/char
symlinks. But it is definitely worth validating and removing this if not required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, once you find out why it's needed and we are certain that there is no way around this, please update the PR description with the details
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we run chroot
to check for host driver installation, which needs to be run with root user.
Signed-off-by: Christopher Desiniotis <[email protected]>
Signed-off-by: Christopher Desiniotis <[email protected]>
c6f7ff4
to
046ee45
Compare
@@ -55,10 +55,15 @@ ARG VERSION="unknown" | |||
ARG GIT_COMMIT="unknown" | |||
RUN make gpu-operator | |||
|
|||
FROM nvcr.io/nvidia/cuda:12.8.1-base-ubi9 | |||
# Download must-gather dependency: `kubectl` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can avoid packaging kubectl
binary too. This often have CVEs. For openshift, we can publish a minimal image into RH registry like these. Which is used only during log collection using oc adm must-gather --image <debug-image>
. For K8s users, they will use the script directly from the GH repo anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently still need it for the upgrade-crd
and cleanup-crd
helm hooks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, those can be replaced with the crd-apply
tool that the NO is using.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure, Shiva. We do have that plan. So, It will be done in a follow-up PR.
No description provided.