Fix library driver library resolution in Alpine containers #1530
Conversation
Pull Request Test Coverage Report for Build 20072385429Details
💛 - Coveralls |
Signed-off-by: Evan Lezar <[email protected]>
Signed-off-by: Evan Lezar <[email protected]>
elezar
force-pushed
the
fix-alpine-support
branch
from
0b9f8cc to
a1f918d
Compare
|
/cherry-pick release-1.18 |
rahulait
left a comment
rahulait
left a comment
There was a problem hiding this comment.
LGTM. I'll let folks who have more context to this repo approve the PR.
cdesiniotis
left a comment
cdesiniotis
left a comment
There was a problem hiding this comment.
Thanks @elezar. I have one minor suggestion about logging. I am approving this PR assuming that comment gets addressed. My other two comments are questions that should not be viewed as blockers.
|
|
||
| // Also output the folders to the alpine .path file as required. | ||
| if err := createAlpinePathFileIfRequired(append(filteredDirectories, systemSearchPaths...)...); err != nil { | ||
| return err |
There was a problem hiding this comment.
Let's add a more descriptive error message here.
| It("nvidia-smi should succeed when using the nvidia-container-runtime-hook", Label("legacy"), func(ctx context.Context) { | ||
| output, _, err := runner.Run(`docker run --rm --runtime=nvidia -e NVIDIA_VISIBLE_DEVICES=all -e NVIDIA_DRIVER_CAPABILITIES=all \ | ||
| alpinecompat nvidia-smi -L`) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(output).To(Equal(hostOutput)) | ||
| }) |
There was a problem hiding this comment.
Question -- was this test case passing before the relevant changes in this PR? If so, what is libnvidia-container doing to handle containers that use musl instead of glibc?
| return outputListToFile(pathFile, dirs...) | ||
| } | ||
|
|
||
| func isAlpine() bool { |
There was a problem hiding this comment.
Potentially out-of-scope -- is there an easy way of detecting if musl is the standard C library used in a distro? If yes, would we be open to expanding this change to be distro-agnostic? Meaning we would create the /etc/ld-musl-$ARCH.path file for any container that uses musl?
A quick LLM query suggested the below:
ldd --version 2>&1 | grep -q musl && echo "Using musl" || echo "Using glibc"
4 participants
Fixes #1526